feat: migrate web dashboard from Radix UI to Base UI + activate CSP nonce + personality trimming WS notification#1074
Closed
feat: migrate web dashboard from Radix UI to Base UI + activate CSP nonce + personality trimming WS notification#1074
Conversation
WalkthroughThis PR replaces Radix UI and Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the web dashboard from Radix UI to Base UI and implements a comprehensive Content Security Policy (CSP) nonce infrastructure using nginx-level injection and Level 3 directive splitting. It also introduces a personality trimming notification system that sends WebSocket events from the AgentEngine to the dashboard to display live toasts. Feedback identifies critical Python syntax errors in exception handling within the backend and several instances in the frontend where Tailwind CSS v4 transition properties must be explicitly defined for translate and scale utilities to animate correctly.
| event.model_dump_json(), | ||
| channels=[CHANNEL_AGENTS], | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
In Python 3, multiple exceptions must be caught using a tuple. The current syntax except MemoryError, RecursionError: is invalid and will result in a SyntaxError. Additionally, using a comma without parentheses in an except clause is deprecated syntax from Python 2 that would attempt to alias the exception to a variable, which in this case would shadow the RecursionError class.
except (MemoryError, RecursionError):| "engine", | ||
| "personality_trimming_notify", | ||
| ) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
In Python 3, multiple exceptions must be caught using a tuple. The current syntax except MemoryError, RecursionError: is invalid and will result in a SyntaxError. This also applies to the second occurrence of this pattern in this file.
except (MemoryError, RecursionError):| return | ||
| try: | ||
| await self._personality_trim_notifier(payload) | ||
| except MemoryError, RecursionError: |
Contributor
There was a problem hiding this comment.
In Python 3, multiple exceptions must be caught using a tuple. The current syntax except MemoryError, RecursionError: is invalid and will result in a SyntaxError.
except (MemoryError, RecursionError):| 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', | ||
| 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', | ||
| 'rounded-xl border border-border-bright bg-surface p-card shadow-lg', | ||
| 'transition-[opacity,transform,scale] duration-200 ease-out', |
Contributor
There was a problem hiding this comment.
Per the updated guidelines in web/CLAUDE.md (line 119), Tailwind v4 requires explicit transition properties for translate and scale as they now compile to dedicated CSS properties rather than the transform shorthand. Since this component uses both -translate-x-1/2 and scale-95, both properties should be listed in the transition.
Suggested change
| 'transition-[opacity,transform,scale] duration-200 ease-out', | |
| 'transition-[opacity,translate,scale] duration-200 ease-out', |
web/src/components/ui/dialog.tsx
Outdated
| 'data-[state=open]:animate-in data-[state=closed]:animate-out', | ||
| 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', | ||
| 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', | ||
| 'transition-[opacity,transform,scale] duration-200 ease-out', |
Contributor
There was a problem hiding this comment.
Tailwind v4 requires explicit transition properties for translate and scale. The transform property in the transition list will not animate the translation or scaling applied via Tailwind v4 utility classes.
Suggested change
| 'transition-[opacity,transform,scale] duration-200 ease-out', | |
| 'transition-[opacity,translate,scale] duration-200 ease-out', |
| <Popover.Popup | ||
| className={cn( | ||
| 'z-50 w-80 rounded-xl border border-border-bright bg-surface p-card shadow-lg', | ||
| 'transition-[opacity,transform,scale] duration-200 ease-out', |
Contributor
There was a problem hiding this comment.
Tailwind v4 requires explicit transition properties for translate and scale. The transform property in the transition list will not animate the scaling applied via scale-95.
Suggested change
| 'transition-[opacity,transform,scale] duration-200 ease-out', | |
| 'transition-[opacity,translate,scale] duration-200 ease-out', |
|
|
||
| <Menu.Portal> | ||
| <Menu.Positioner align="end" sideOffset={4}> | ||
| <Menu.Popup className="z-50 w-36 rounded-lg border border-border bg-card py-1 shadow-[var(--so-shadow-card-hover)] transition-[opacity,transform,scale] duration-150 ease-out data-[closed]:opacity-0 data-[starting-style]:opacity-0 data-[ending-style]:opacity-0 data-[closed]:scale-95 data-[starting-style]:scale-95 data-[ending-style]:scale-95"> |
Contributor
There was a problem hiding this comment.
Tailwind v4 requires explicit transition properties for translate and scale. The transform property in the transition list will not animate the scaling applied via scale-95.
Suggested change
| <Menu.Popup className="z-50 w-36 rounded-lg border border-border bg-card py-1 shadow-[var(--so-shadow-card-hover)] transition-[opacity,transform,scale] duration-150 ease-out data-[closed]:opacity-0 data-[starting-style]:opacity-0 data-[ending-style]:opacity-0 data-[closed]:scale-95 data-[starting-style]:scale-95 data-[ending-style]:scale-95"> | |
| <Menu.Popup className="z-50 w-36 rounded-lg border border-border bg-card py-1 shadow-[var(--so-shadow-card-hover)] transition-[opacity,translate,scale] duration-150 ease-out data-[closed]:opacity-0 data-[starting-style]:opacity-0 data-[ending-style]:opacity-0 data-[closed]:scale-95 data-[starting-style]:scale-95 data-[ending-style]:scale-95"> |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. OpenSSF Scorecard
Scanned Files
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the web dashboard’s headless UI primitives from Radix UI to Base UI, enables end-to-end CSP nonce propagation for inline <style> injection, and adds a new WebSocket event + frontend toast for personality trimming so operators get real-time visibility when token budgets force trimming.
Changes:
- Replace Radix UI primitives and
cmdkwith Base UI primitives andcmdk-base, updating affected components/pages and transition data-attributes. - Activate CSP nonce flow from nginx →
index.htmlmeta →getCspNonce()→ Base UICSPProvider+ Framer MotionMotionConfig, and tighten CSP directives. - Add
personality_trimming_notifysetting +personality.trimmedWS event type; publish best-effort fromAgentEnginevia a new API notifier factory; display a global toast in the dashboard.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/vite.config.ts | Updates vendor chunk split config for Base UI + cmdk-base dependencies. |
| web/src/styles/global.css | Updates shadcn token bridge comment to reflect Base UI. |
| web/src/stores/agents.ts | Extends WS event handling to include personality.trimmed toast dispatch + improved payload validation/logging for status events. |
| web/src/pages/workflows/WorkflowTableView.tsx | Migrates workflow actions dropdown from Radix DropdownMenu to Base UI Menu. |
| web/src/pages/workflows/WorkflowCard.tsx | Migrates workflow card actions dropdown from Radix DropdownMenu to Base UI Menu. |
| web/src/pages/workflow-editor/VersionDiffViewer.tsx | Updates dialog animation comment to Base UI wording. |
| web/src/pages/tasks/TaskCreateDialog.tsx | Migrates Radix Dialog usage to Base UI Dialog primitives and Tailwind v4 transition attrs. |
| web/src/pages/providers/ProviderFormModal.tsx | Migrates Radix Dialog usage to Base UI Dialog primitives. |
| web/src/pages/providers/ModelPullDialog.tsx | Migrates Radix AlertDialog usage to Base UI AlertDialog primitives. |
| web/src/pages/OrgEditPage.tsx | Migrates Radix Tabs to Base UI Tabs and updates active-state selectors. |
| web/src/pages/org-edit/PackSelectionDialog.tsx | Migrates Radix Dialog usage to Base UI Dialog primitives. |
| web/src/pages/org-edit/DepartmentCreateDialog.tsx | Migrates Radix Dialog usage to Base UI Dialog primitives. |
| web/src/pages/org-edit/AgentCreateDialog.tsx | Migrates Radix Dialog usage to Base UI Dialog primitives. |
| web/src/lib/csp.ts | Implements nonce reader with caching + warning/error logging for missing/placeholder nonce. |
| web/src/hooks/useGlobalNotifications.ts | Adds global WS subscription hook to route events into stores + toast on WS failures. |
| web/src/hooks/useCommandPalette.ts | Exposes setOpen(value) to wire Base UI / cmdk-base onOpenChange. |
| web/src/components/ui/theme-toggle.tsx | Migrates Radix Popover to Base UI Popover primitives/positioner. |
| web/src/components/ui/slot.tsx | Adds local Slot helper using Base UI mergeProps for <Button asChild>. |
| web/src/components/ui/dialog.tsx | Migrates shared Dialog wrapper from Radix to Base UI primitives. |
| web/src/components/ui/confirm-dialog.tsx | Migrates ConfirmDialog from Radix AlertDialog to Base UI AlertDialog primitives. |
| web/src/components/ui/command-palette.tsx | Replaces cmdk + Radix FocusScope wrapper with cmdk-base dialog-hosted palette. |
| web/src/components/ui/button.tsx | Switches Slot import/usage from Radix to local Slot helper. |
| web/src/components/layout/AppLayout.tsx | Mounts useGlobalNotifications() once for app-wide WS-driven notifications. |
| web/src/App.tsx | Wraps app with Base UI CSPProvider and passes nonce to Framer Motion MotionConfig. |
| web/src/api/types.ts | Adds personality.trimmed to WS event type union + values list. |
| web/src/tests/stores/agents.test.ts | Adds tests verifying personality.trimmed triggers an info toast. |
| web/src/tests/pages/approvals/ApprovalDetailDrawer.test.tsx | Updates test comment for Base UI AlertDialog behavior. |
| web/src/tests/hooks/useGlobalNotifications.test.ts | Adds tests for global bindings, dispatch, and WS failure toasts. |
| web/src/tests/components/ui/confirm-dialog.test.tsx | Adjusts assertions for Base UI onOpenChange signature and focus-guard elements. |
| web/src/tests/App.test.tsx | Mocks useGlobalNotifications to prevent real WS connections during App tests. |
| web/security-headers.conf | Updates CSP to nonce-based style-src-elem + style-src-attr split directives. |
| web/package.json | Removes Radix/cmdk deps; adds @base-ui/react and pins cmdk-base. |
| web/nginx.conf | Adds per-request nonce mapping and enables sub_filter substitution of __CSP_NONCE__. |
| web/index.html | Enables <meta name="csp-nonce" content="__CSP_NONCE__"> for runtime nonce read. |
| web/components.json | Switches shadcn style preset from radix-nova to base-vega. |
| web/CLAUDE.md | Updates dashboard tech stack + documents Base UI conventions and adoption decisions. |
| tests/unit/settings/test_engine_settings.py | Adds setting registry test for engine.personality_trimming_notify and updates count. |
| tests/unit/engine/test_personality_trim_notifier.py | Adds unit tests for best-effort notifier behavior and cancellation propagation. |
| src/synthorg/settings/definitions/engine.py | Registers engine.personality_trimming_notify setting definition. |
| src/synthorg/observability/events/prompt.py | Adds prompt.personality.notify_failed event constant. |
| src/synthorg/engine/agent_engine.py | Adds notifier type + constructor param; emits best-effort notification on trim. |
| src/synthorg/api/ws_models.py | Adds WsEventType.PERSONALITY_TRIMMED. |
| src/synthorg/api/app.py | Adds make_personality_trim_notifier() factory bound to ChannelsPlugin. |
| docs/security.md | Rewrites CSP section to document nonce infrastructure and split directives rationale. |
| docs/guides/deployment.md | Updates documented CSP header and adds explanation of nonce + split directives. |
| docs/design/ux-research.md | Updates accessibility primitives references from Radix to Base UI. |
| docs/design/ux-guidelines.md | Updates command palette library reference to cmdk-base. |
| docs/design/page-structure.md | Updates command palette library reference to cmdk-base. |
| docs/design/engine.md | Documents the personality trimming notification behavior and wiring contract. |
| docs/design/brand-and-ux.md | Updates component inventory references from Radix/cmdk to Base UI/cmdk-base. |
| docs/architecture/tech-stack.md | Updates Web UI stack description to Base UI + CSP nonce mention. |
| CLAUDE.md | Updates top-level web dashboard dependency list from Radix/cmdk to Base UI/cmdk-base. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
24
to
38
| # Gzip: serve pre-compressed .gz files built by Vite stage; fall back to | ||
| # on-the-fly compression for anything not pre-compressed. | ||
| gzip_static on; | ||
| gzip on; | ||
| gzip_vary on; | ||
| gzip_types text/plain text/css application/json application/javascript text/xml image/svg+xml; | ||
| gzip_min_length 256; | ||
|
|
||
| # CSP nonce activation (not yet enabled): | ||
| # 1. Add: map $request_id $csp_nonce { default $request_id; } | ||
| # (or generate via set_by_lua / sub_filter from a random source) | ||
| # 2. Add: sub_filter '__CSP_NONCE__' '$csp_nonce'; | ||
| # sub_filter_once off; | ||
| # 3. Uncomment <meta name="csp-nonce"> in index.html | ||
| # 4. Replace 'unsafe-inline' with 'nonce-$csp_nonce' in security-headers.conf | ||
| # See also: web/src/lib/csp.ts (runtime reader) and App.tsx (MotionConfig) | ||
| # CSP nonce injection. sub_filter substitutes the __CSP_NONCE__ placeholder | ||
| # in index.html with the per-request nonce from the map above. Default | ||
| # sub_filter_types already includes text/html, and sub_filter is inert on | ||
| # any asset that doesn't contain the placeholder string. | ||
| sub_filter '__CSP_NONCE__' '$csp_nonce'; | ||
| sub_filter_once off; | ||
|
|
There was a problem hiding this comment.
sub_filter will not run when nginx serves the pre-compressed index.html.gz via gzip_static on (see docker/web/Dockerfile: it gzips *.html). Most browsers send Accept-Encoding: gzip, so the response will likely contain the literal __CSP_NONCE__ placeholder, causing getCspNonce() to return undefined and CSP to block Base UI / Framer Motion injected styles.
Fix: ensure index.html is always served uncompressed before filtering (e.g., set gzip_static off; in location = /index.html (and any SPA fallback path) so the nonce placeholder is substituted, then let gzip on compress the filtered response), or stop precompressing *.html in the build.
web/security-headers.conf
Outdated
Comment on lines
+7
to
+13
| # CSP Level 3 directive splitting for style sources: | ||
| # style-src-elem 'self' 'nonce-...' -- locks <style> elements to the per-request nonce | ||
| # style-src-attr 'unsafe-inline' -- required by Floating UI (used internally by Base UI) | ||
| # for positioning, because inline style attributes cannot carry CSP nonces per the CSS | ||
| # spec. Not a practical XSS vector -- style attributes cannot execute JavaScript. | ||
| # connect-src 'self' covers same-origin ws: (http) or wss: (https) matching the page scheme. Never mix schemes. | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; connect-src 'self'; img-src 'self' data:; font-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'" always; | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src-elem 'self' 'nonce-$csp_nonce'; style-src-attr 'unsafe-inline'; connect-src 'self'; img-src 'self' data:; font-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'" always; |
There was a problem hiding this comment.
The CSP header no longer includes a style-src fallback. Browsers that don’t support style-src-attr (notably Firefox/Safari, as noted in docs) will ignore style-src-attr and fall back to style-src (or default-src if absent), which will block inline style attributes used by Floating UI/Base UI and likely break popovers/menus.
Fix: add an explicit style-src fallback (e.g. style-src 'self' 'unsafe-inline') while keeping style-src-elem 'self' 'nonce-$csp_nonce' for modern browsers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1074 +/- ##
==========================================
+ Coverage 89.62% 89.63% +0.01%
==========================================
Files 744 744
Lines 43625 43672 +47
Branches 4379 4382 +3
==========================================
+ Hits 39099 39146 +47
Misses 3751 3751
Partials 775 775 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/deployment.md`:
- Around line 136-139: The CSP in the deployment guide currently relies on
style-src-elem and style-src-attr (see the documented Content-Security-Policy
header string and references to style-src-elem/style-src-attr and Floating UI)
but lacks a style-src fallback; update the guide to either add a conservative
fallback like including style-src 'unsafe-inline' (or a more restrictive
alternative you choose) to the CSP header example so older browsers that don't
support style-src-attr will still allow Floating UI inline positioning, or
explicitly state minimum supported browser versions (e.g., Firefox ≥108, Safari
≥15.4) and call out that older/unpatched browsers will break positioned
elements—make the change next to the CSP header example and the CSP Nonce
Infrastructure paragraph so readers see the fallback or browser requirements
together.
In `@src/synthorg/engine/agent_engine.py`:
- Around line 1004-1023: The code currently returns on any settings read
exception which suppresses the default-true behavior; initialize notify_enabled
= True before the try and do not return on general exceptions so the default
remains; update the exception tuple to use Python syntax except (MemoryError,
RecursionError): re-raise, and in the generic except block log
PROMPT_PERSONALITY_NOTIFY_FAILED (including
agent_id/agent_name/task_id/trim_tier) but leave notify_enabled as True so the
notifier will still run; adjust the flow around self._config_resolver.get_bool
in agent_engine.py to reflect these changes.
In `@tests/unit/engine/test_personality_trim_notifier.py`:
- Around line 205-237: The test currently calls the private method
_maybe_notify_personality_trim on AgentEngine directly; change it to exercise
the public behavior instead by invoking AgentEngine.run() (or another public
entry point) with a provider fixture and inputs that will cause a personality
trim when config_resolver is None, and assert the AsyncMock notifier was awaited
with the payload; if you intentionally want to keep testing the private branch,
add a short comment above the test mentioning the coupling to the private method
and why that's acceptable, referencing AgentEngine,
_maybe_notify_personality_trim, notifier (AsyncMock) and mock_provider_factory
so reviewers can locate the code.
In `@web/src/components/ui/slot.tsx`:
- Around line 43-48: Replace the bare console.warn in the dev-only branch with
the app logger: import createLogger from '@/lib/logger', instantiate a logger
for this component (e.g., const logger = createLogger('Slot') or similar) and
call logger.warn(...) with the same warning string inside the if
(import.meta.env.DEV) block so the message follows project logging conventions
and uses createLogger instead of console.warn.
In `@web/src/hooks/useGlobalNotifications.ts`:
- Around line 42-68: The reconnectExhaustedRef is set to true on exhaustion and
never reset, so add logic to clear it when the WS reconnects: in the same hook
file (around the useEffect that checks reconnectExhausted), add an effect that
watches the WS connection state (e.g., a connected boolean from your WebSocket
hook or a provided prop) and sets reconnectExhaustedRef.current = false when
connected becomes true; this ensures future exhaustion events
(reconnectExhausted) can re-trigger the error toast and logging (referencing
reconnectExhaustedRef and reconnectExhausted in the existing useEffect).
In `@web/src/lib/csp.ts`:
- Around line 46-50: The branch checking for value === '__CSP_NONCE__' should
not call log.error; change that log.error call to a non-error-level log (e.g.,
log.debug or remove it) so local/dev servers that intentionally ship the literal
'__CSP_NONCE__' do not generate false-positive errors; keep the current safe
behavior of returning undefined. Locate the check in web/src/lib/csp.ts (the
code that reads value and compares to '__CSP_NONCE__') and replace the log.error
invocation with a lower-severity log or no log while preserving the existing
return path.
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Around line 68-74: The AlertDialog.Close usage currently passes a static JSX
element; change its render prop to be a function that accepts the injected props
and forwards them to the Button so the dialog receives the close behavior.
Locate the AlertDialog.Close instance in ModelPullDialog and replace the static
element with a render callback that receives props (e.g., render={(props) =>
...}) and spreads those props into the Button component (Button {...props}
variant="outline" size="sm") so the Button gets the onClick/close handlers.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Line 145: The Dialog.Root is wired directly to onOpenChange which lets the
dialog close during submission; guard the onOpenChange by checking the
component's submitting/busy state (the same pattern used in
PackSelectionDialog/DepartmentCreateDialog): wrap the existing onOpenChange prop
in a handler that returns early when submitting is true (i.e., if (submitting)
return) before calling the original onOpenChange, and pass that guarded handler
to Dialog.Root instead of onOpenChange directly (reference Dialog.Root and the
local submitting state / the create/submit handler).
In `@web/src/pages/workflows/WorkflowTableView.tsx`:
- Around line 81-120: The WorkflowTableView Menu styling differs from
WorkflowCard.tsx causing inconsistency: in the Menu.Popup and Menu.Item
definitions in WorkflowTableView.tsx (Menu.Root/Menu.Popup and each Menu.Item)
replace bg-popover with bg-card, change cursor-pointer to cursor-default on
Menu.Item classes, and change highlighted class data-[highlighted]:bg-accent/10
to data-[highlighted]:bg-surface to match WorkflowCard.tsx; optionally extract
the shared class string into a constant or create a reusable WorkflowActionsMenu
component used by both files to avoid duplication.
🪄 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: 096c6930-2401-4c02-8777-d5774669b3a7
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (52)
CLAUDE.mddocs/architecture/tech-stack.mddocs/design/brand-and-ux.mddocs/design/engine.mddocs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/ux-research.mddocs/guides/deployment.mddocs/security.mdsrc/synthorg/api/app.pysrc/synthorg/api/ws_models.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/observability/events/prompt.pysrc/synthorg/settings/definitions/engine.pytests/unit/engine/test_personality_trim_notifier.pytests/unit/settings/test_engine_settings.pyweb/CLAUDE.mdweb/components.jsonweb/index.htmlweb/nginx.confweb/package.jsonweb/security-headers.confweb/src/App.tsxweb/src/__tests__/App.test.tsxweb/src/__tests__/components/ui/confirm-dialog.test.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/agents.test.tsweb/src/api/types.tsweb/src/components/layout/AppLayout.tsxweb/src/components/ui/button.tsxweb/src/components/ui/command-palette.tsxweb/src/components/ui/confirm-dialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/slot.tsxweb/src/components/ui/theme-toggle.tsxweb/src/hooks/useCommandPalette.tsweb/src/hooks/useGlobalNotifications.tsweb/src/lib/csp.tsweb/src/pages/OrgEditPage.tsxweb/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/org-edit/DepartmentCreateDialog.tsxweb/src/pages/org-edit/PackSelectionDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/providers/ProviderFormModal.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflows/WorkflowCard.tsxweb/src/pages/workflows/WorkflowTableView.tsxweb/src/stores/agents.tsweb/src/styles/global.cssweb/vite.config.ts
Comment on lines
+205
to
+237
| async def test_notifier_fires_without_config_resolver( | ||
| self, | ||
| mock_provider_factory: type[MockCompletionProvider], | ||
| ) -> None: | ||
| """When config_resolver is None, the notify setting defaults to enabled. | ||
|
|
||
| Covers the ``self._config_resolver is None`` branch in | ||
| ``_maybe_notify_personality_trim`` -- without a resolver the default | ||
| behavior is to fire the notifier (opt-out only via explicit setting). | ||
| """ | ||
| notifier = AsyncMock() | ||
| provider = mock_provider_factory([_make_completion_response()]) | ||
| engine = AgentEngine( | ||
| provider=provider, | ||
| personality_trim_notifier=notifier, | ||
| # config_resolver intentionally omitted | ||
| ) | ||
|
|
||
| payload: dict[str, object] = { | ||
| "agent_id": "agent-1", | ||
| "agent_name": "Test Agent", | ||
| "task_id": "task-1", | ||
| "before_tokens": 600, | ||
| "after_tokens": 200, | ||
| "max_tokens": 300, | ||
| "trim_tier": 2, | ||
| "budget_met": True, | ||
| } | ||
| await engine._maybe_notify_personality_trim(payload) | ||
|
|
||
| assert notifier.await_count == 1 | ||
| assert notifier.await_args is not None | ||
| assert notifier.await_args.args[0] == payload |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Direct call to private method _maybe_notify_personality_trim.
Testing the config_resolver is None branch directly on the private method couples the test to internal implementation. If this method is refactored or renamed, this test will break even if the public behavior remains correct.
Consider testing via the public engine.run() path by providing a fixture that triggers trimming without a resolver. If branch coverage is the goal, this is acceptable but worth documenting the trade-off in a comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/test_personality_trim_notifier.py` around lines 205 - 237,
The test currently calls the private method _maybe_notify_personality_trim on
AgentEngine directly; change it to exercise the public behavior instead by
invoking AgentEngine.run() (or another public entry point) with a provider
fixture and inputs that will cause a personality trim when config_resolver is
None, and assert the AsyncMock notifier was awaited with the payload; if you
intentionally want to keep testing the private branch, add a short comment above
the test mentioning the coupling to the private method and why that's
acceptable, referencing AgentEngine, _maybe_notify_personality_trim, notifier
(AsyncMock) and mock_provider_factory so reviewers can locate the code.
web/src/components/ui/slot.tsx
Outdated
Comment on lines
+43
to
+48
| if (import.meta.env.DEV) { | ||
| console.warn( | ||
| '[Slot] asChild received a non-element child. Props were not forwarded. ' + | ||
| 'Wrap the content in a single React element.', | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use createLogger instead of bare console.warn per coding guidelines.
The coding guidelines require using createLogger from @/lib/logger instead of bare console.warn in application code. While this is a dev-only warning, the guideline applies uniformly.
♻️ Proposed fix
import { Children, cloneElement, isValidElement } from 'react'
import type { HTMLAttributes, ReactNode, Ref, MutableRefObject } from 'react'
import { mergeProps } from '@base-ui/react/merge-props'
+import { createLogger } from '@/lib/logger'
+
+const log = createLogger('slot')
/**Then update the warning:
if (!isValidElement<Record<string, unknown>>(child)) {
if (import.meta.env.DEV) {
- console.warn(
- '[Slot] asChild received a non-element child. Props were not forwarded. ' +
- 'Wrap the content in a single React element.',
+ log.warn(
+ 'asChild received a non-element child. Props were not forwarded. Wrap the content in a single React element.',
)
}
return child
}As per coding guidelines: "Always use createLogger from @/lib/logger instead of bare console.warn, console.error, or console.debug in application code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/slot.tsx` around lines 43 - 48, Replace the bare
console.warn in the dev-only branch with the app logger: import createLogger
from '@/lib/logger', instantiate a logger for this component (e.g., const logger
= createLogger('Slot') or similar) and call logger.warn(...) with the same
warning string inside the if (import.meta.env.DEV) block so the message follows
project logging conventions and uses createLogger instead of console.warn.
web/src/lib/csp.ts
Outdated
Comment on lines
+46
to
+50
| } else if (value === '__CSP_NONCE__') { | ||
| // Placeholder survived: nginx sub_filter is misconfigured in production. | ||
| log.error('CSP nonce placeholder not substituted', { | ||
| impact: 'nginx sub_filter is misconfigured -- CSP will block inline styles', | ||
| }) |
There was a problem hiding this comment.
Don’t log the unsubstituted placeholder as a production error.
index.html now ships the literal __CSP_NONCE__, so running outside nginx will hit this branch on every local page load. The function already degrades safely by returning undefined; logging it as error turns the normal dev-server path into a false-positive CSP failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/lib/csp.ts` around lines 46 - 50, The branch checking for value ===
'__CSP_NONCE__' should not call log.error; change that log.error call to a
non-error-level log (e.g., log.debug or remove it) so local/dev servers that
intentionally ship the literal '__CSP_NONCE__' do not generate false-positive
errors; keep the current safe behavior of returning undefined. Locate the check
in web/src/lib/csp.ts (the code that reads value and compares to
'__CSP_NONCE__') and replace the log.error invocation with a lower-severity log
or no log while preserving the existing return path.
Comment on lines
+68
to
+74
| <AlertDialog.Close | ||
| render={ | ||
| <Button variant="outline" size="sm"> | ||
| Cancel | ||
| </Button> | ||
| } | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct usage of the render prop for AlertDialog.Close in @base-ui/react? Does it automatically inject onClick handlers?
💡 Result:
The AlertDialog.Close component in @base-ui/react supports a render prop for customization. It is re-exported from Dialog.Close, which accepts a render prop of type ReactElement | function that receives props: HTMLProps and state: DialogClose.State to render a custom element. The render prop receives standard HTML props (like onClick) that are automatically enhanced with close behavior. Using it automatically injects the onClick handler (or equivalent event handling) to close the dialog when activated, following Base UI's pattern for interactive primitives like Close buttons. Example usage from docs and patterns: <AlertDialog.Close render={(props) => <button {...props}>Custom Close} /> The props include the necessary event handlers for closing.
Citations:
- 1: https://base-ui.com/react/components/alert-dialog
- 2: https://base-ui.com/react/components/dialog
- 3: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/alert-dialog/root/AlertDialogRoot.tsx
- 4: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/alert-dialog/index.parts.ts
- 5: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/dialog/root/DialogRoot.tsx
Fix AlertDialog.Close render prop to use function callback instead of static element.
The render prop must be a function that receives and forwards the injected props to the Button:
Correct pattern
<AlertDialog.Close
render={(props) => (
<Button {...props} variant="outline" size="sm">
Cancel
</Button>
)}
/>The current implementation passes a static JSX element, which prevents the Button from receiving the automatically injected close behavior props (e.g., onClick handler). This breaks the dialog close functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/ModelPullDialog.tsx` around lines 68 - 74, The
AlertDialog.Close usage currently passes a static JSX element; change its render
prop to be a function that accepts the injected props and forwards them to the
Button so the dialog receives the close behavior. Locate the AlertDialog.Close
instance in ModelPullDialog and replace the static element with a render
callback that receives props (e.g., render={(props) => ...}) and spreads those
props into the Button component (Button {...props} variant="outline" size="sm")
so the Button gets the onClick/close handlers.
138e5ef to
ef5acd4
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
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/providers/ModelPullDialog.tsx (1)
23-35:⚠️ Potential issue | 🟠 MajorThe deferred-cancel path never closes the modal because there's no mechanism to trigger
onClose()afterpullingModelclears.When the user dismisses while
pullingModelis truthy,onOpenChange(false)fires and callshandleCancel(), which only dispatchescancelPull()without callingonClose(). SinceonOpenChange()is the only trigger for the close logic, andpullingModelclearing from the store produces no state change event that would callonOpenChange(false)again, the dialog never closes. The comment's assumption that "the nextonOpenChange(false)... bypullingModelbecoming null" occurs is incorrect—there's no mechanism wired to trigger it.A
closeAfterCancelstate latch plus auseEffectwatchingpullingModelwould properly bridge this gap: set the latch when user attempts close, then callonClose()when the effect detectspullingModelcleared and the latch is true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` around lines 23 - 35, The modal never actually closes when cancelling a running pull because handleCancel() only calls cancelPull() while pullingModel is truthy and never later calls onClose(); add a boolean latch (e.g., closeAfterCancel state) and set it to true inside handleCancel() when pullingModel exists, then add a useEffect that watches pullingModel and closeAfterCancel and calls onClose() (and clears the latch) once pullingModel becomes null; keep existing behavior for the non-pulling branch (onClose() immediately) and ensure you reference handleCancel, cancelPull, pullingModel, onClose, onOpenChange, and the new closeAfterCancel/useEffect.
♻️ Duplicate comments (1)
docs/guides/deployment.md (1)
136-139:⚠️ Potential issue | 🟡 MinorDocument minimum browser requirements for CSP Level 3 directives.
The CSP uses Level 3 directive splitting (
style-src-elem,style-src-attr), which requires Firefox ≥108 (Dec 2022) or Safari ≥15.4 (Mar 2022). Older browsers will fall back throughstyle-src→default-src 'self', blocking Floating UI's inline positioning styles. Consider adding a brief note about supported browser versions:📝 Suggested addition
The CSP uses Level 3 directive splitting: `style-src-elem` locks `<style>` elements to the per-request nonce (injected by nginx `sub_filter` substituting `$request_id` into `<meta name="csp-nonce">`), while `style-src-attr 'unsafe-inline'` covers the transient inline positioning styles set by Floating UI (used internally by Base UI). See [`docs/security.md` → CSP Nonce Infrastructure](../security.md#csp-nonce-infrastructure) for the full flow -- any reverse proxy in front of the web container must preserve `sub_filter` substitution and the matching CSP header, otherwise inline styles will be blocked. + +> **Browser requirements**: CSP Level 3 directive splitting requires Firefox ≥108, Safari ≥15.4, or Chromium ≥105. Older browsers may fail to render positioned elements (dropdowns, tooltips).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/deployment.md` around lines 136 - 139, Add a short "Browser support" note to the CSP discussion explaining that the Level 3 directives used (style-src-elem, style-src-attr) require modern browsers: Firefox >=108 and Safari >=15.4 (or newer Chromium-based browsers that support the split directives), and that older browsers will fall back to style-src/default-src and may block Floating UI inline positioning styles; reference the CSP Nonce Infrastructure paragraph and recommend ensuring clients meet these minimum versions or include a fallback guidance for proxies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/agent_engine.py`:
- Around line 1018-1026: The notifier call in run() that emits
PROMPT_PERSONALITY_NOTIFY_FAILED must be bounded so a slow/hung notifier cannot
stall execution: wrap the awaited notifier callback invocation in
asyncio.wait_for(...) with a small timeout (e.g., 0.5s), and catch TimeoutError
and general Exception to emit PROMPT_PERSONALITY_NOTIFY_FAILED and continue, but
re-raise MemoryError, RecursionError, and asyncio.CancelledError per the
publisher contract; ensure the same pattern is applied to the other notifier
call around lines 1068-1081 so both notification paths are timeout-bounded and
failures are logged/emitted without blocking run().
In `@web/CLAUDE.md`:
- Around line 114-120: Update the Select guidance to remove the recommendation
to prefer Base UI Select and clearly state that Select is intentionally not
adopted: contributors must reuse the existing SelectField (native)
implementation instead of creating or preferring Base UI Select; update any
example lines that show `import { Select }` or instructions to use Base UI
`Select` so they reference the shared `SelectField`/native approach, and add a
reminder to always reuse components from web/src/components/ui/ before creating
new primitives (also adjust the similar guidance around the other referenced
lines). Ensure references to `Select` vs `SelectField` and any import/usage
examples and the adoption table are consistent so the doc no longer contradicts
itself.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 170-189: The test currently flips mockUseWebSocket's return value
after unmount but never triggers any post-unmount event, so it doesn't verify
cleanup; update the mock to expose a controllable event emitter (e.g., have
mockUseWebSocket return an object with subscribe/unsubscribe or an emit method)
and in the test call the emitter after unmount to simulate a websocket status
change, then assert useToastStore.getState().toasts remains empty; alternatively
remove the ineffective test if you prefer not to add an emitter. Ensure changes
reference useGlobalNotifications, mockUseWebSocket, renderHook/unmount and
assert useToastStore.getState().toasts.
In `@web/src/components/ui/dialog.tsx`:
- Around line 78-82: The DialogDescription component currently applies 'text-sm
text-muted' which uses a muted surface color; change the default classname in
DialogDescription (the BaseDialog.Description usage) to use 'text-sm
text-muted-foreground' (optionally combined with any incoming className) so the
shared dialog description uses the readable muted foreground tone consistent
with ConfirmDialog and other UI components.
In `@web/src/components/ui/theme-toggle.tsx`:
- Around line 114-118: Replace the hardcoded shadow utility used in the class
list inside the ThemeToggle component with the design token shadow variable:
remove the 'shadow-lg' token and use the CSS variable-based shadow
(var(--so-shadow-card-hover)) instead (use an arbitrary value class or inline
style as your Tailwind setup requires). Keep the existing border token
'border-border-bright' and ensure the class array in theme-toggle.tsx that
currently contains 'shadow-lg' is updated to reference the tokenized shadow so
the component uses var(--so-shadow-card-hover).
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Line 2: The file imports and uses Base UI's AlertDialog which duplicates the
shared dialog surface and is semantically incorrect for a data-entry/progress
modal; replace the AlertDialog usage with the shared Dialog component from
web/src/components/ui/dialog by changing the import to Dialog and updating all
AlertDialog-related components/usages (e.g., AlertDialog, AlertDialogTrigger,
AlertDialogContent, AlertDialogHeader, AlertDialogFooter) to their Dialog
equivalents (Dialog, DialogTrigger, DialogContent, DialogHeader, DialogFooter or
the actual exported names in the shared component) and ensure props/state for
open/close and title/body/actions map to the shared Dialog API.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 97-106: The render-phase state resets using prevOpenRef,
setForm(INITIAL_FORM), setErrors({}), and setSubmitError(null) must be moved
into a useEffect that watches the open prop so we only reset when open
transitions from true to false; implement a useEffect that keeps prevOpenRef (or
computes previousOpen) and when previousOpenRef.current === true && open ===
false call setForm(INITIAL_FORM), setErrors({}), setSubmitError(null), then
update prevOpenRef.current = open inside the effect (do not call these setters
during render).
In `@web/src/stores/agents.ts`:
- Around line 274-292: The payload handling currently warns when agentName,
before, and after are null but still shows a generic toast; update the handler
to early-return when all three fields are invalid (agentName === null && before
=== null && after === null) so no toast is shown for completely malformed
payloads—locate the block that calls log.warn(...) and the subsequent
useToastStore.getState().add(...) and add a conditional return immediately after
the warning when all three are null to prevent displaying the misleading toast.
---
Outside diff comments:
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Around line 23-35: The modal never actually closes when cancelling a running
pull because handleCancel() only calls cancelPull() while pullingModel is truthy
and never later calls onClose(); add a boolean latch (e.g., closeAfterCancel
state) and set it to true inside handleCancel() when pullingModel exists, then
add a useEffect that watches pullingModel and closeAfterCancel and calls
onClose() (and clears the latch) once pullingModel becomes null; keep existing
behavior for the non-pulling branch (onClose() immediately) and ensure you
reference handleCancel, cancelPull, pullingModel, onClose, onOpenChange, and the
new closeAfterCancel/useEffect.
---
Duplicate comments:
In `@docs/guides/deployment.md`:
- Around line 136-139: Add a short "Browser support" note to the CSP discussion
explaining that the Level 3 directives used (style-src-elem, style-src-attr)
require modern browsers: Firefox >=108 and Safari >=15.4 (or newer
Chromium-based browsers that support the split directives), and that older
browsers will fall back to style-src/default-src and may block Floating UI
inline positioning styles; reference the CSP Nonce Infrastructure paragraph and
recommend ensuring clients meet these minimum versions or include a fallback
guidance for proxies.
🪄 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: d0797906-068c-4c6f-b6e0-a12c5dc36e98
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
CLAUDE.mddocs/architecture/tech-stack.mddocs/design/brand-and-ux.mddocs/design/engine.mddocs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/ux-research.mddocs/guides/deployment.mddocs/security.mdsrc/synthorg/api/app.pysrc/synthorg/api/ws_models.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/observability/events/prompt.pysrc/synthorg/settings/definitions/engine.pytests/unit/api/test_personality_trim_notifier_factory.pytests/unit/engine/test_personality_trim_notifier.pytests/unit/settings/test_engine_settings.pyweb/CLAUDE.mdweb/components.jsonweb/index.htmlweb/nginx.confweb/package.jsonweb/security-headers.confweb/src/App.tsxweb/src/__tests__/App.test.tsxweb/src/__tests__/components/ui/confirm-dialog.test.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/agents.test.tsweb/src/api/types.tsweb/src/components/layout/AppLayout.tsxweb/src/components/ui/button.tsxweb/src/components/ui/command-palette.tsxweb/src/components/ui/confirm-dialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/slot.tsxweb/src/components/ui/theme-toggle.tsxweb/src/hooks/useCommandPalette.tsweb/src/hooks/useGlobalNotifications.tsweb/src/lib/csp.tsweb/src/pages/OrgChartPage.tsxweb/src/pages/OrgEditPage.tsxweb/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/org-edit/DepartmentCreateDialog.tsxweb/src/pages/org-edit/PackSelectionDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/providers/ProviderFormModal.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflows/WorkflowCard.tsxweb/src/pages/workflows/WorkflowTableView.tsxweb/src/stores/agents.tsweb/src/styles/global.cssweb/vite.config.ts
src/synthorg/engine/agent_engine.py
Outdated
Comment on lines
+1018
to
+1026
| Emits ``PROMPT_PERSONALITY_NOTIFY_FAILED`` on two paths: (1) the setting | ||
| read raised an exception (``reason="failed to read | ||
| personality_trimming_notify setting; fail-open with default | ||
| notify_enabled=True"``), in which case the method proceeds with the | ||
| built-in default and still invokes the notifier; (2) the notifier | ||
| callback itself raised (``reason="notifier callback raised"``). In | ||
| both cases :class:`MemoryError`, :class:`RecursionError`, and | ||
| :class:`asyncio.CancelledError` propagate per the best-effort publisher | ||
| contract -- notification never silently blocks task execution. |
There was a problem hiding this comment.
Bound the notifier call so best-effort notifications cannot stall run().
This callback is awaited inline on the main execution path. A slow or hung notifier implementation now blocks prompt preparation indefinitely, even though trim notifications are documented as non-critical and failures are intentionally swallowed.
⏱️ Suggested fix
+ notifier_timeout_seconds = 2.0
try:
- await self._personality_trim_notifier(payload)
+ async with asyncio.timeout(notifier_timeout_seconds):
+ await self._personality_trim_notifier(payload)
+ except TimeoutError:
+ logger.warning(
+ PROMPT_PERSONALITY_NOTIFY_FAILED,
+ agent_id=agent_id,
+ agent_name=agent_name,
+ task_id=task_id,
+ trim_tier=trim_tier,
+ reason="notifier callback timed out",
+ timeout_seconds=notifier_timeout_seconds,
+ exc_info=True,
+ )
except MemoryError, RecursionError:
raise
except Exception:
logger.warning(
PROMPT_PERSONALITY_NOTIFY_FAILED,Also applies to: 1068-1081
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/agent_engine.py` around lines 1018 - 1026, The notifier
call in run() that emits PROMPT_PERSONALITY_NOTIFY_FAILED must be bounded so a
slow/hung notifier cannot stall execution: wrap the awaited notifier callback
invocation in asyncio.wait_for(...) with a small timeout (e.g., 0.5s), and catch
TimeoutError and general Exception to emit PROMPT_PERSONALITY_NOTIFY_FAILED and
continue, but re-raise MemoryError, RecursionError, and asyncio.CancelledError
per the publisher contract; ensure the same pattern is applied to the other
notifier call around lines 1068-1081 so both notification paths are
timeout-bounded and failures are logged/emitted without blocking run().
Comment on lines
+170
to
+189
| it('stops emitting toasts after unmount', () => { | ||
| mockUseWebSocket.mockReturnValue({ | ||
| connected: true, | ||
| reconnectExhausted: false, | ||
| setupError: null, | ||
| }) | ||
|
|
||
| const { unmount } = renderHook(() => useGlobalNotifications()) | ||
| unmount() | ||
|
|
||
| // After unmount, changing the mock's return value and not re-rendering | ||
| // should not produce any new toasts. This guards against effect re-runs | ||
| // that could occur if the hook leaked a subscription. | ||
| mockUseWebSocket.mockReturnValue({ | ||
| connected: false, | ||
| reconnectExhausted: true, | ||
| setupError: null, | ||
| }) | ||
|
|
||
| expect(useToastStore.getState().toasts).toHaveLength(0) |
There was a problem hiding this comment.
This unmount test does not exercise teardown.
After unmount(), the hook never re-runs and the mocked useWebSocket return value is never read again, so flipping the mock state cannot detect a leaked subscription. This assertion will stay green even if cleanup is broken; to verify teardown, the mock needs to expose a controllable post-unmount event path (or this test should be removed).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts` around lines 170 -
189, The test currently flips mockUseWebSocket's return value after unmount but
never triggers any post-unmount event, so it doesn't verify cleanup; update the
mock to expose a controllable event emitter (e.g., have mockUseWebSocket return
an object with subscribe/unsubscribe or an emit method) and in the test call the
emitter after unmount to simulate a websocket status change, then assert
useToastStore.getState().toasts remains empty; alternatively remove the
ineffective test if you prefer not to add an emitter. Ensure changes reference
useGlobalNotifications, mockUseWebSocket, renderHook/unmount and assert
useToastStore.getState().toasts.
| @@ -1,5 +1,5 @@ | |||
| import { useState } from 'react' | |||
| import { AlertDialog } from 'radix-ui' | |||
| import { AlertDialog } from '@base-ui/react/alert-dialog' | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and inspect the shared Dialog component
find web/src/components/ui -name "dialog*" -type fRepository: Aureliolo/synthorg
Length of output: 135
🏁 Script executed:
# Examine the shared Dialog component
cat -n web/src/components/ui/dialog.tsx 2>/dev/null | head -100Repository: Aureliolo/synthorg
Length of output: 3673
🏁 Script executed:
# Examine ModelPullDialog.tsx import and usage
cat -n web/src/pages/providers/ModelPullDialog.tsx 2>/dev/null | head -70Repository: Aureliolo/synthorg
Length of output: 3729
🏁 Script executed:
# See the full ModelPullDialog.tsx to check the cancel flow logic
cat -n web/src/pages/providers/ModelPullDialog.tsx 2>/dev/nullRepository: Aureliolo/synthorg
Length of output: 5947
🏁 Script executed:
# Check if ConfirmDialog exists and what it is
cat -n web/src/components/ui/confirm-dialog.tsx 2>/dev/null | head -50Repository: Aureliolo/synthorg
Length of output: 2136
Use the shared Dialog component here instead of raw AlertDialog.
A shared dialog wrapper already exists in web/src/components/ui/dialog.tsx; dropping to Base UI primitives duplicates this shared surface. Per coding guidelines: "ALWAYS reuse existing components from web/src/components/ui/ before creating new ones".
Base UI's AlertDialog is semantically for confirmations or alerts requiring user affirmation before proceeding. This is a data-entry + progress modal—neither a confirmation nor an alert. Use the generic shared Dialog instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/ModelPullDialog.tsx` at line 2, The file imports and
uses Base UI's AlertDialog which duplicates the shared dialog surface and is
semantically incorrect for a data-entry/progress modal; replace the AlertDialog
usage with the shared Dialog component from web/src/components/ui/dialog by
changing the import to Dialog and updating all AlertDialog-related
components/usages (e.g., AlertDialog, AlertDialogTrigger, AlertDialogContent,
AlertDialogHeader, AlertDialogFooter) to their Dialog equivalents (Dialog,
DialogTrigger, DialogContent, DialogHeader, DialogFooter or the actual exported
names in the shared component) and ensure props/state for open/close and
title/body/actions map to the shared Dialog API.
Comment on lines
+97
to
+106
| // Reset form state on close (render-phase check mirroring AgentCreateDialog / | ||
| // DepartmentCreateDialog / PackSelectionDialog so reopening does not show | ||
| // stale input from the previous session). | ||
| const prevOpenRef = useRef(open) | ||
| if (!open && prevOpenRef.current) { | ||
| setForm(INITIAL_FORM) | ||
| setErrors({}) | ||
| setSubmitError(null) | ||
| } | ||
| prevOpenRef.current = open |
There was a problem hiding this comment.
Render-phase state updates violate React rules and may cause infinite loops.
Calling setForm, setErrors, and setSubmitError during render (outside of an effect or event handler) is a React anti-pattern that can cause extra re-renders or infinite loops in concurrent rendering. The pattern works coincidentally in some cases but is not supported behavior.
Use useEffect to reset state when open transitions from true to false:
🐛 Proposed fix using useEffect
- // Reset form state on close (render-phase check mirroring AgentCreateDialog /
- // DepartmentCreateDialog / PackSelectionDialog so reopening does not show
- // stale input from the previous session).
- const prevOpenRef = useRef(open)
- if (!open && prevOpenRef.current) {
- setForm(INITIAL_FORM)
- setErrors({})
- setSubmitError(null)
- }
- prevOpenRef.current = open
+ // Reset form state when dialog closes so reopening does not show stale input.
+ const prevOpenRef = useRef(open)
+ useEffect(() => {
+ if (!open && prevOpenRef.current) {
+ setForm(INITIAL_FORM)
+ setErrors({})
+ setSubmitError(null)
+ }
+ prevOpenRef.current = open
+ }, [open])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 97 - 106, The
render-phase state resets using prevOpenRef, setForm(INITIAL_FORM),
setErrors({}), and setSubmitError(null) must be moved into a useEffect that
watches the open prop so we only reset when open transitions from true to false;
implement a useEffect that keeps prevOpenRef (or computes previousOpen) and when
previousOpenRef.current === true && open === false call setForm(INITIAL_FORM),
setErrors({}), setSubmitError(null), then update prevOpenRef.current = open
inside the effect (do not call these setters during render).
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/src/components/ui/theme-toggle.tsx (1)
188-188:⚠️ Potential issue | 🟡 MinorUse standard semantic color token
text-muted-foregroundinstead of non-standardtext-text-muted.Line 188 uses
text-text-muted(with doubletext-prefix), which is not a defined token in the design system. The correct semantic class istext-muted-foreground, used consistently across the codebase in 20+ other locations.Fix
- className="text-xs text-text-muted hover:text-foreground" + className="text-xs text-muted-foreground hover:text-foreground"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/theme-toggle.tsx` at line 188, Replace the non-standard token in the ThemeToggle component: locate the element in web/src/components/ui/theme-toggle.tsx where className currently contains "text-xs text-text-muted hover:text-foreground" (inside the ThemeToggle component) and change "text-text-muted" to the semantic token "text-muted-foreground" so the className reads "text-xs text-muted-foreground hover:text-foreground".web/src/pages/providers/ModelPullDialog.tsx (1)
117-117:⚠️ Potential issue | 🟡 MinorUse the correct semantic color token.
text-text-secondaryappears to have a doubled prefix. Per design system guidelines, semantic text color classes follow the patterntext-{token}(e.g.,text-muted,text-foreground).-<p className="text-center text-sm text-text-secondary truncate"> +<p className="text-center text-sm text-muted truncate">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` at line 117, In ModelPullDialog (the JSX paragraph rendering the description), replace the incorrect utility class "text-text-secondary" in the <p className="text-center text-sm text-text-secondary truncate"> element with the correct semantic token (e.g., "text-muted" or the project's approved semantic token such as "text-foreground"); update the className to "text-center text-sm text-muted truncate" (or substitute the approved token) so it follows the text-{token} convention.src/synthorg/engine/agent_engine.py (1)
928-956:⚠️ Potential issue | 🟠 MajorDon't let the override lookup re-enable trimming.
If Line 929 successfully reads
personality_trimming_enabled=Falseand Line 933 then fails, the sharedexceptpath leavestrimming_enabled=True. A transient failure inpersonality_max_tokens_overridecan therefore undo an explicit operator disablement of personality trimming.🛠️ Proposed fix
if self._config_resolver is not None: try: - resolved_enabled = await self._config_resolver.get_bool( + trimming_enabled = await self._config_resolver.get_bool( "engine", "personality_trimming_enabled", ) - resolved_override = await self._config_resolver.get_int( + except MemoryError, RecursionError: + raise + except Exception: + logger.warning( + EXECUTION_ENGINE_ERROR, + agent_id=agent_id, + task_id=task_id, + note="failed to read ENGINE setting, using default", + failed_keys=("personality_trimming_enabled",), + fallback_trimming_enabled=True, + exc_info=True, + ) + try: + resolved_override = await self._config_resolver.get_int( "engine", "personality_max_tokens_override", ) except MemoryError, RecursionError: raise except Exception: logger.warning( EXECUTION_ENGINE_ERROR, agent_id=agent_id, task_id=task_id, - note="failed to read ENGINE settings, using defaults", - failed_keys=( - "personality_trimming_enabled", - "personality_max_tokens_override", - ), - fallback_trimming_enabled=True, + note="failed to read ENGINE setting, using default", + failed_keys=("personality_max_tokens_override",), fallback_tokens_override=None, exc_info=True, ) else: - trimming_enabled = resolved_enabled if resolved_override > 0: tokens_override = resolved_override🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/agent_engine.py` around lines 928 - 956, The code currently reads resolved_enabled and resolved_override together so a failure on the override call can leave the default trimming_enabled=True and undo an explicit False; fix by assigning trimming_enabled immediately after a successful call to self._config_resolver.get_bool (i.e. set trimming_enabled = resolved_enabled right after resolved_enabled is returned), then call self._config_resolver.get_int in a separate try/except that only affects tokens_override (resolved_override) and never mutates trimming_enabled; ensure exceptions from get_int are caught and logged similarly but do not change trimming_enabled or re-enable trimming.web/src/components/ui/dialog.tsx (1)
5-15: 🧹 Nitpick | 🔵 TrivialDerive the root wrapper props from
BaseDialog.Rootto avoid API drift.This interface already narrows Base UI’s surface:
onOpenChangeloseseventDetails, andchildrenno longer accepts the documented render-function form. That forces any consumer that needs close reasons or trigger payloads to bypass the shared wrapper. TypingDialogPropsagainstComponentProps<typeof BaseDialog.Root>keeps the wrapper aligned with the library API. (base-ui.com)Suggested fix
-export interface DialogProps { - open: boolean - onOpenChange: (open: boolean) => void - children: React.ReactNode -} +export interface DialogProps + extends Pick<ComponentProps<typeof BaseDialog.Root>, 'open' | 'onOpenChange' | 'children'> {}import type { ComponentProps } from 'react'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/dialog.tsx` around lines 5 - 15, The DialogProps interface should be derived from React's ComponentProps for BaseDialog.Root to keep our wrapper aligned with Base UI's API: replace the explicit DialogProps declaration with a type based on ComponentProps<typeof BaseDialog.Root> (so children can be the render-function form and onOpenChange preserves eventDetails), import ComponentProps from 'react', and keep the Dialog component signature using that derived DialogProps; update references to DialogProps and ensure BaseDialog.Root usage in the Dialog({ open, onOpenChange, children }: DialogProps) function remains unchanged.
♻️ Duplicate comments (2)
web/src/pages/providers/ModelPullDialog.tsx (2)
88-94:⚠️ Potential issue | 🔴 Critical
AlertDialog.Closerender prop is still a static element—this breaks close functionality.The render prop must be a function that receives and spreads the injected props (including
onClick) onto theButton. The current static JSX element won't receive the auto-close handler.Correct pattern
<AlertDialog.Close - render={ + render={(props) => ( <Button + {...props} variant="outline" size="sm" > Cancel </Button> - } + )} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` around lines 88 - 94, AlertDialog.Close is being passed a static JSX element so it doesn't receive the injected close handler; change the render prop to a function that accepts the passed props and spreads them onto the Button (e.g., use a render function that receives props and returns <Button {...props} variant="outline" size="sm">Cancel</Button>) so the injected onClick/aria props from AlertDialog.Close are applied and the dialog actually closes.
2-2: 🛠️ Refactor suggestion | 🟠 MajorPrefer the shared
Dialogcomponent over rawAlertDialog.
AlertDialogis semantically for confirmations requiring user affirmation before dismissal. This is a data-entry + progress modal—use the generic sharedDialogfromweb/src/components/ui/dialog.tsxto avoid duplicating the shared surface and to match the component's actual behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` at line 2, The file imports and uses AlertDialog for ModelPullDialog but this modal is a data-entry/progress UI and should use the shared Dialog component instead; update the import to pull Dialog from web/src/components/ui/dialog.tsx, replace AlertDialog usages (props/components named AlertDialog, AlertDialogTrigger, AlertDialogContent, etc.) with the corresponding Dialog API used across the app (e.g., Dialog, DialogTrigger, DialogContent, DialogTitle/Description as applicable), and adjust any AlertDialog-specific props (confirm/leastDestructiveButton handlers or semantic props) to the generic Dialog equivalents in the ModelPullDialog component so the shared surface and behavior are reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/agent_engine.py`:
- Around line 166-168: The three public payload fields agent_id, agent_name, and
task_id are plain str but must use the project's NotBlankStr type; update their
annotations in agent_engine.py to use NotBlankStr (imported from core.types) for
these identifiers (allowing optional variants if needed, e.g., NotBlankStr |
None) so they follow the public contract and avoid manual whitespace
validators—search for the symbols agent_id, agent_name, and task_id and replace
their type annotations with NotBlankStr and add the core.types import if
missing.
In `@web/CLAUDE.md`:
- Around line 147-158: The recommendation text mentions "Tooltip" but the
Adoption Decisions table omits it; update the doc so they match by either adding
a new table entry marking `Tooltip` as **Adopted** (matching Dialog/Menu/etc.)
or by removing "Tooltip" from the sentence on line ~158; to fix quickly, add
`Tooltip` to the adopted row in the table alongside `Dialog`, `AlertDialog`,
`Popover`, `Tabs`, `Menu` (use the same "Adopted" decision and a short rationale
like "Used directly across primitives for accessibility") so table and
recommendation text are consistent.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 40-45: The test currently invokes the WebSocket subscription by
position (bindings[0]!.handler(...)), which is brittle; instead find the binding
whose channel === 'agents' from the bindings array and call its handler. Update
both occurrences where bindings[0]!.handler(...) is used in the test (after
retrieving const { bindings } = options ...) to resolve const agentBinding =
bindings.find(b => b.channel === 'agents') and invoke agentBinding!.handler(...)
so the test is resilient to reordering or additional subscriptions; keep using
mockUseWebSocket and the WsEvent payloads unchanged.
In `@web/src/components/ui/dialog.tsx`:
- Around line 90-102: DialogCloseButton currently renders a bespoke icon-button
using BaseDialog.Close, which duplicates styles instead of reusing the shared
Button; update DialogCloseButton to import and use the existing Button component
(from '@/components/ui/button') and render BaseDialog.Close wrapping or
rendering the Button so the Button's tokens/interaction states are reused,
forward className and aria-label from DialogCloseButtonProps, place the X icon
as the Button child, and ensure any existing cn class composition (and
focus/hover styles) are moved into Button usage so styling and behavior remain
identical.
In `@web/src/stores/agents.ts`:
- Line 164: The structured log calls pass attacker-controlled values directly
(e.g., the agent name and websocket status) to log.warn; update the calls
(notably the log.warn at the site using the variable name and the other
occurrences around the section handling websocket status) to wrap those fields
with sanitizeForLog() before embedding them in the structured object so
untrusted strings cannot bypass logger sanitization (e.g., replace direct uses
of name and status in the object arg with sanitizeForLog(name) and
sanitizeForLog(status)).
- Around line 229-233: The websocket payload is dereferenced without a
null/undefined guard: in the agent.status_changed branch (check
event.event_type), add an early guard after casting payload (the payload
variable) such as if (payload == null) { log.warn(...); return } so accessing
payload.agent_id and payload.status cannot throw; similarly add a null/undefined
guard for the personality.trimmed payload handling. Also, when logging
attacker-controlled fields like status, wrap them with sanitizeForLog(status) in
the structured log (see sanitizeForLog and VALID_RUNTIME_STATUSES) so the
warning uses sanitized status data.
---
Outside diff comments:
In `@src/synthorg/engine/agent_engine.py`:
- Around line 928-956: The code currently reads resolved_enabled and
resolved_override together so a failure on the override call can leave the
default trimming_enabled=True and undo an explicit False; fix by assigning
trimming_enabled immediately after a successful call to
self._config_resolver.get_bool (i.e. set trimming_enabled = resolved_enabled
right after resolved_enabled is returned), then call
self._config_resolver.get_int in a separate try/except that only affects
tokens_override (resolved_override) and never mutates trimming_enabled; ensure
exceptions from get_int are caught and logged similarly but do not change
trimming_enabled or re-enable trimming.
In `@web/src/components/ui/dialog.tsx`:
- Around line 5-15: The DialogProps interface should be derived from React's
ComponentProps for BaseDialog.Root to keep our wrapper aligned with Base UI's
API: replace the explicit DialogProps declaration with a type based on
ComponentProps<typeof BaseDialog.Root> (so children can be the render-function
form and onOpenChange preserves eventDetails), import ComponentProps from
'react', and keep the Dialog component signature using that derived DialogProps;
update references to DialogProps and ensure BaseDialog.Root usage in the
Dialog({ open, onOpenChange, children }: DialogProps) function remains
unchanged.
In `@web/src/components/ui/theme-toggle.tsx`:
- Line 188: Replace the non-standard token in the ThemeToggle component: locate
the element in web/src/components/ui/theme-toggle.tsx where className currently
contains "text-xs text-text-muted hover:text-foreground" (inside the ThemeToggle
component) and change "text-text-muted" to the semantic token
"text-muted-foreground" so the className reads "text-xs text-muted-foreground
hover:text-foreground".
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Line 117: In ModelPullDialog (the JSX paragraph rendering the description),
replace the incorrect utility class "text-text-secondary" in the <p
className="text-center text-sm text-text-secondary truncate"> element with the
correct semantic token (e.g., "text-muted" or the project's approved semantic
token such as "text-foreground"); update the className to "text-center text-sm
text-muted truncate" (or substitute the approved token) so it follows the
text-{token} convention.
---
Duplicate comments:
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Around line 88-94: AlertDialog.Close is being passed a static JSX element so
it doesn't receive the injected close handler; change the render prop to a
function that accepts the passed props and spreads them onto the Button (e.g.,
use a render function that receives props and returns <Button {...props}
variant="outline" size="sm">Cancel</Button>) so the injected onClick/aria props
from AlertDialog.Close are applied and the dialog actually closes.
- Line 2: The file imports and uses AlertDialog for ModelPullDialog but this
modal is a data-entry/progress UI and should use the shared Dialog component
instead; update the import to pull Dialog from web/src/components/ui/dialog.tsx,
replace AlertDialog usages (props/components named AlertDialog,
AlertDialogTrigger, AlertDialogContent, etc.) with the corresponding Dialog API
used across the app (e.g., Dialog, DialogTrigger, DialogContent,
DialogTitle/Description as applicable), and adjust any AlertDialog-specific
props (confirm/leastDestructiveButton handlers or semantic props) to the generic
Dialog equivalents in the ModelPullDialog component so the shared surface and
behavior are reused.
🪄 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: 3df9d5d3-a6ca-4f83-817c-cede2b23e9d1
📒 Files selected for processing (8)
src/synthorg/engine/agent_engine.pyweb/CLAUDE.mdweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/__tests__/stores/agents.test.tsweb/src/components/ui/dialog.tsxweb/src/components/ui/theme-toggle.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/stores/agents.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/agent_engine.py`:
- Around line 1086-1088: The 2s asyncio.timeout around await
self._personality_trim_notifier(payload) won't interrupt synchronous blocking
inside the notifier (see _on_personality_trimmed in src/synthorg/api/app.py
which calls channels_plugin.publish(...) synchronously); fix by making the
notifier non-blocking or moving the blocking publish off the event loop: either
(A) convert channels_plugin.publish and the call chain used by
_personality_trim_notifier/_on_personality_trimmed to async and await them, or
(B) wrap the blocking publish call in a background thread (e.g., use
asyncio.to_thread or loop.run_in_executor) inside _on_personality_trimmed so the
publish runs off the event loop and the timeout on _personality_trim_notifier
can cancel as expected. Ensure references to asyncio.timeout,
_personality_trim_notifier, _on_personality_trimmed, and channels_plugin.publish
are updated accordingly.
In `@web/CLAUDE.md`:
- Around line 145-150: Clarify that the adopted Base UI primitives (Dialog,
AlertDialog, Popover, Tabs, Menu) are imported directly from Base UI subpaths
rather than wrapped by shadcn/ui, and update the intro to state that CSPProvider
is also wired directly in App.tsx (alongside MotionConfig) for nonce
propagation; explicitly instruct contributors to import these primitives from
Base UI (not to generate shadcn wrappers) so it's unambiguous where to reference
Dialog, AlertDialog, Popover, Tabs, Menu, CSPProvider, and App.tsx in future
changes.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 146-176: Add a test in useGlobalNotifications.test that mirrors
the existing setupError dedupe sequence but for reconnectExhausted: use
mockUseWebSocket to simulate first a down state with reconnectExhausted: true
(and connected: false) and assert one warning toast, then simulate a healthy
reconnect (connected: true, reconnectExhausted: false) and assert no toasts,
then simulate a second down with reconnectExhausted: true and assert a new toast
appears; reference the same renderHook/rerender usage and the useToastStore, and
ensure this verifies reconnectExhaustedRef is reset by the reconnect path in
useGlobalNotifications.
In `@web/src/components/ui/dialog.tsx`:
- Around line 93-104: The shared close control uses a plain <Button> inside
BaseDialog.Close which defaults to type="submit" in forms; set the Button's HTML
type explicitly to "button" so clicking the close icon only dismisses the dialog
and doesn't trigger form submission—update the Button element used in
BaseDialog.Close (the shared close control) to include type="button".
🪄 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: 71d5148a-bc09-4886-aa1c-db0fbd37e544
📒 Files selected for processing (5)
src/synthorg/engine/agent_engine.pyweb/CLAUDE.mdweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/dialog.tsxweb/src/stores/agents.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: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.warn,console.error, orconsole.debugin application code
Use variable namelogfor logger instances (e.g.,const log = createLogger('module-name'))
Uselog.debug()for dev-only logging (stripped in production),log.warn()for warnings, andlog.error()for errors
Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs
Files:
web/src/stores/agents.tsweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/dialog.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,css}: Use Tailwind semantic color classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) instead of hardcoded hex values
Usefont-sansorfont-monofor typography (maps to Geist tokens), never setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders instead of hardcoded values
Do NOT usergba()with hardcoded values -- use design token variables
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files and catches design system violations; fix all violations before proceeding
Files:
web/src/stores/agents.tsweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/dialog.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Do NOT hardcode Framer Motion transition durations -- use presets from@/lib/motion
CSS side-effect imports in TypeScript files should use type declarations (/// <reference types="vite/client" />covers this); TypeScript 6 defaultsnoUncheckedSideEffectImportsto true
web/src/**/*.{ts,tsx}: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones
NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and@/lib/motionpresets
React dashboard: Node.js 22+, TypeScript 6.0+, dependencies inweb/package.json(React 19, react-router, shadcn/ui, Radix UI, Tailwind CSS 4, Zustand,@tanstack/react-query,@xyflow/react,@dagrejs/dagre, d3-force,@dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, Storybook 10, MSW, Vitest,@testing-library/react, fast-check, ESLint, Playwright)
Files:
web/src/stores/agents.tsweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/dialog.tsx
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/ui/**/*.{ts,tsx}: Place new shared components inweb/src/components/ui/with descriptive kebab-case filename
Export props as a TypeScript interface in new components
Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Importcnfrom@/lib/utilsfor conditional class merging in component files
Files:
web/src/components/ui/dialog.tsx
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649
Use PEP 758 except syntax: useexcept A, B:(no parentheses) -- ruff enforces this on Python 3.14
Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves
Never mix static config fields with mutable runtime fields in one model
Models: Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters (ruff)
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Validate: at system boundaries (user input, external APIs, config files)
Use ruff check and ruff format for li...
Files:
src/synthorg/engine/agent_engine.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exception:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr))
Variable name for logger: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module undersynthorg.observability.events(imported directly, never as string literals)
Structured kwargs: alwayslogger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases
Files:
src/synthorg/engine/agent_engine.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/engine/agent_engine.py
🧠 Learnings (80)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to web/src/**/*.{ts,tsx} : React dashboard: Node.js 22+, TypeScript 6.0+, dependencies in `web/package.json` (React 19, react-router, shadcn/ui, Radix UI, Tailwind CSS 4, Zustand, tanstack/react-query, xyflow/react, dagrejs/dagre, d3-force, dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, Storybook 10, MSW, Vitest, testing-library/react, fast-check, ESLint, Playwright)
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:28:28.895Z
Learning: Web dashboard: see `web/CLAUDE.md` for commands, design system, and component inventory
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to docker/Dockerfile.web : `nginxinc/nginx-unprivileged` base with non-root user (UID 101), Vue 3 SPA (PrimeVue + Tailwind CSS), SPA routing, API/WebSocket proxy to backend
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T18:57:46.655Z
Learning: Applies to web/package.json : Web dashboard Node.js 22+, TypeScript 6.0+, dependencies in `web/package.json`
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Applied to files:
web/src/stores/agents.tsweb/CLAUDE.md
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/stores/agents.tsweb/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Wrap attacker-controlled fields inside structured objects with `sanitizeForLog()` before embedding in logs
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go through `sanitizeArg`
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Always use `createLogger` from `@/lib/logger` instead of bare `console.warn`, `console.error`, or `console.debug` in application code
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Toast` / `ToastContainer` component for success/error/warning/info notifications with auto-dismiss queue (mount `ToastContainer` once in AppLayout)
Applied to files:
web/src/stores/agents.tsweb/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.{ts,tsx} : Vitest unit tests must use coverage scoped to files changed vs origin/main branch
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.test.{ts,tsx} : Web dashboard: Use React Hypothesis (fast-check) for property-based testing with fc.assert + fc.property
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `storybook/test` instead of `storybook/test` and `storybook/actions` instead of `storybook/addon-actions` (Storybook 10 import path changes)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/__tests__/**/*.test.{ts,tsx} : Use property-based testing with fast-check in React tests (`fc.assert` + `fc.property`)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-31T14:28:28.895Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:28:28.895Z
Learning: Web dashboard: see `web/CLAUDE.md` for commands, design system, and component inventory
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to web/src/**/*.{ts,tsx} : React dashboard: Node.js 22+, TypeScript 6.0+, dependencies in `web/package.json` (React 19, react-router, shadcn/ui, Radix UI, Tailwind CSS 4, Zustand, tanstack/react-query, xyflow/react, dagrejs/dagre, d3-force, dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, Storybook 10, MSW, Vitest, testing-library/react, fast-check, ESLint, Playwright)
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : When creating new shared web components, place in web/src/components/ui/ with kebab-case filename, create .stories.tsx alongside with all states (default, hover, loading, error, empty), export props as TypeScript interface, use design tokens exclusively with no hardcoded colors/fonts/spacing, and import cn from `@/lib/utils` for conditional class merging
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-01T20:43:51.878Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T20:43:51.878Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/components/ui/*.{tsx,ts} : For new shared React components: place in web/src/components/ui/ with kebab-case filename, create .stories.tsx with all states, export props as TypeScript interface, use design tokens exclusively
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `SelectField` component for labeled select dropdown with error/hint and placeholder support
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Always reuse existing components from web/src/components/ui/ before creating new ones
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Use design tokens exclusively in new components -- no hardcoded colors, fonts, or spacing
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Web dashboard shadows/borders: use token variables (var(--so-shadow-card-hover), border-border, border-bright); never hardcode shadow or border values
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use `color?` and `animated?` props for Sparkline component (inline SVG trend lines)
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to site/src/**/*.{ts,tsx,astro} : Landing page: Astro 6, astrojs/react, React 19, Tailwind CSS 4, js-yaml in `site/package.json`
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Skeleton` / `SkeletonCard` / `SkeletonMetric` / `SkeletonTable` / `SkeletonText` components for loading placeholders matching component shapes (shimmer animation, respects `prefers-reduced-motion`)
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ConfirmDialog` component for confirmation modal (Radix AlertDialog) with `default` / `destructive` variants and `loading` state
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use `@/components/ui/animated-presence` (Framer Motion AnimatePresence keyed by route) for page transitions
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `CommandPalette` component for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands via `useCommandPalette` hook
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `EmptyState` component for no-data / no-results placeholder with icon, title, description, optional action button
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `StaggerGroup` / `StaggerItem` components for card entrance stagger container with configurable delay
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `TokenUsageBar` component for segmented horizontal meter bar for token usage (multi-segment with auto-colors, `role="meter"`, animated transitions)
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ThemeToggle` component for Radix Popover with 5-axis theme controls (color, density, typography, animation, sidebar), rendered in StatusBar
Applied to files:
web/CLAUDE.mdweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `CodeMirrorEditor` component for CodeMirror 6 editor with JSON/YAML modes, design-token dark theme, line numbers, bracket matching, `readOnly` support
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions — use design tokens and `@/lib/motion` presets
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to web/src/**/*.{ts,tsx} : NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions in web code — use design tokens and `@/lib/motion` presets
Applied to files:
web/CLAUDE.md
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Drawer` component for slide-in panel (`side` prop: left or right, default right) with overlay, spring animation, focus trap, Escape-to-close, optional header (`title`), `ariaLabel` for accessible name (one of `title` or `ariaLabel` required), and `contentClassName` override
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Export props as a TypeScript interface in new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Export props as a TypeScript interface for new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT hardcode Framer Motion transition durations -- use presets from `@/lib/motion`
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Use density-aware spacing tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Use Tailwind semantic color classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`, etc.) or CSS variables (`var(--so-accent)`) instead of hardcoded hex values
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use density-aware tokens (p-card, gap-section-gap, gap-grid-gap) or standard Tailwind spacing; never hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `StatusBadge` component for agent/task/system status indicators (colored dot + optional built-in label toggle)
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `AgentCard` component for agent display (avatar, name, role, status, current task)
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Import `cn` from `@/lib/utils` for conditional class merging in component files
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) -- ruff enforces this on Python 3.14
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use `NotBlankStr` from `core.types` for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to **/*.py : Use `NotBlankStr` (from `core.types`) for all identifier/name fields -- including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants -- instead of manual whitespace validators
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to **/*.py : Use `NotBlankStr` (from `core.types`) for all identifier/name fields, including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants, instead of manual whitespace validators
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 with adopted conventions: use computed_field for derived values instead of storing + validating redundant fields; use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to {src/synthorg/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx}} : NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small`
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-04-05T07:25:14.510Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T07:25:14.510Z
Learning: Applies to src/synthorg/**/*.py : Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small` as aliases
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget package (budget/): cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError)
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/engine/agent_engine.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/engine/agent_engine.py
🔇 Additional comments (1)
web/src/stores/agents.ts (1)
231-315: Nice hardening around malformed WS events.The top-level payload guard plus sanitized warning fields makes malformed broker events fail closed instead of throwing in the store, and the
personality.trimmedpath now suppresses noise when the payload carries nothing actionable. As per coding guidelines,web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.*, and wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs.
…once + personality trimming WS notification Close #1070. - Replace radix-ui + cmdk with @base-ui/react@1.3.0 + cmdk-base@1.0.0. 14 component files migrated (button with local Slot helper; dialog, confirm-dialog, theme-toggle, command-palette wrappers; six page-level Dialog/AlertDialog files; OrgEditPage Tabs; two workflow Menu files). components.json switched from radix-nova to base-vega. 115 packages removed, 8 added. - Activate end-to-end CSP nonce pipeline. App.tsx wraps <CSPProvider nonce> + <MotionConfig nonce>. nginx generates per-request nonces from $request_id and substitutes __CSP_NONCE__ in index.html via sub_filter. security-headers.conf replaces 'style-src unsafe-inline' with CSP Level 3 split: style-src-elem 'self' 'nonce-$csp_nonce' + style-src-attr 'unsafe-inline' (for Floating UI positioning, not a practical XSS vector). - Fold in #1064: personality trimming WebSocket notification. - Add engine.personality_trimming_notify setting (BOOLEAN, default true). - Add WsEventType.PERSONALITY_TRIMMED enum member. - Add personality_trim_notifier callback param to AgentEngine, invoked best-effort from _prepare_context with the same payload as the PROMPT_PERSONALITY_TRIMMED log event. - Add frontend handler in agents store that dispatches a toast on personality.trimmed events. - New useGlobalNotifications hook mounted in AppLayout subscribes to the agents channel app-wide so the toast renders regardless of current page. - 5 new notifier tests + 4 new toast-handler tests. - Rewrite docs/security.md: delete the Accepted Risk section entirely and replace with CSP Nonce Infrastructure (current-state only). Update web/CLAUDE.md, docs/design/{brand-and-ux,ux-research}.md, docs/architecture/tech-stack.md, root CLAUDE.md to reference Base UI / cmdk-base. Add Base UI Adoption Decisions table in web/CLAUDE.md documenting keep-custom decisions for Toast (Zustand integration), Drawer (Framer Motion + motion token enforcement), Meter (ProgressGauge already has proper a11y), SelectField (native mobile picker). - Bundle size: vendor-ui 150 kB -> 230 kB (+80 kB / +28 kB gzip). Net +3% on total dist weight. The growth is Base UI's more comprehensive primitive surface (Floating UI, focus management, transition tracking); the tradeoff is first-class CSP nonce support and active upstream maintenance. - Test results: 14213 backend tests (unit + integration) pass, 2413 frontend tests pass, Storybook builds, lint + type-check clean on both sides. Zero Radix references in tracked files.
Implements all valid findings from the 10-agent pre-PR review swarm on PR #1070. Details per area: Backend / engine: - agent_engine._maybe_notify_personality_trim: switch from payload.get() to direct subscript for guaranteed keys, expand log context with agent_name + trim_tier, add reason labels to distinguish setting-read vs. callback failure log sites. - AgentEngine.__init__: add has_personality_trim_notifier to the debug log so misconfigured wiring is visible at startup. - Rename PROMPT_PERSONALITY_TRIM_NOTIFY_FAILED to PROMPT_PERSONALITY_NOTIFY_FAILED to match the dot-hierarchy naming convention used by the rest of the prompt event constants (prompt.policy.validation_failed style). - Update PersonalityTrimNotifier docstring to point at the new api.app.make_personality_trim_notifier factory. - New api.app.make_personality_trim_notifier factory: returns a ready-to-wire async callback bound to the live ChannelsPlugin. External engine runners call this factory and pass the result into AgentEngine so personality trimming events reach the dashboard end-to-end. - Tests: add test_notifier_fires_without_config_resolver (covers the config_resolver=None branch) and test_cancelled_error_propagates (asserts asyncio.CancelledError is never swallowed by the best-effort try/except). Frontend: - lib/csp.ts: rewrite the stale docstring to describe the active nonce flow; log.warn / log.error when the meta tag is missing or contains the un-substituted __CSP_NONCE__ placeholder so production CSP failures are visible instead of silent. - stores/agents.ts: log.warn on malformed personality.trimmed payloads instead of silently falling back to a generic toast; length-bound agent_name to 64 chars as defence-in-depth against oversized values; log.warn on unknown agent.status_changed status values. - hooks/useGlobalNotifications.ts: surface setupError and reconnectExhausted via warning / error toasts so a silent WebSocket death does not leave users wondering why notifications stopped. New test file covers bindings, dispatch, and error-toast paths. - components/ui/slot.tsx: compose the Slot's own ref with any child ref via a composeRefs helper so neither is silently dropped; add a dev-only console.warn when asChild receives a non-element child; drop the incorrect mergeProps<'div'> generic; rewrite the docstring to reflect current state (only Button uses this helper, all other primitives use Base UI's native render prop). - components/ui/command-palette.tsx: fix the entrance/exit animation -- in Tailwind v4, translate compiles to the dedicated CSS translate: property rather than transform: shorthand, so the transition list needs transition-[opacity,translate] instead of transition-[opacity,transform]. - components/ui/dialog.tsx: drop the no-op backdrop-filter from the backdrop's transition property list. - Design tokens: replace p-6 with p-card on the 5 migrated Dialog / AlertDialog popups (confirm-dialog, AgentCreateDialog, DepartmentCreateDialog, PackSelectionDialog, TaskCreateDialog), replace p-4 with p-card on the theme-toggle popover popup, and replace gap-4 with gap-grid-gap on the 3 grid-cols-2 blocks inside TaskCreateDialog -- matches the precedent set by ModelPullDialog and the density-aware spacing token rules. - package.json: pin cmdk-base to exact 1.0.0 (not ^1.0.0) since it is a smaller community package without the security track record of major MUI deps. Docs: - guides/deployment.md: replace the stale style-src 'unsafe-inline' CSP string with the active CSP Level 3 split directives and link to security.md, so operators configuring reverse proxies copy the correct value. - design/engine.md: document the new personality_trimming_notify setting, the personality.trimmed WebSocket event contract, the agents-channel dispatch, the dashboard toast rendering, and the wiring contract via make_personality_trim_notifier. - design/ux-guidelines.md, design/page-structure.md: update the two remaining stale cmdk references to cmdk-base. - web/CLAUDE.md: add a Base UI primitive recipe to the Creating New Components checklist (subpath imports, render prop, Portal / Backdrop / Popup / Positioner, data-attribute names for animations, Tailwind v4 translate / scale caveat). Remove historical phrasing. Verification: 14054 backend unit tests pass, 2419 frontend tests pass (+6 new), ruff + mypy + eslint + tsc all clean.
Critical / infrastructure:
- nginx: disable gzip_static on index.html so sub_filter can substitute the
CSP nonce placeholder (gzip_static serves precompressed .gz files verbatim
and bypasses response body filters). Apply the same fix to the SPA fallback
location and re-include the security-headers snippet + Cache-Control:
no-cache there so deep-link routes carry the nonce + CSP.
- CSP: add style-src 'self' 'nonce-...' 'unsafe-inline' fallback alongside
the Level 3 style-src-elem/style-src-attr split so browsers without CSP3
support do not fall through to default-src and break Floating UI.
- nginx: scope per-request CSP emission to HTML responses only (drop the
security-headers include from /assets/, add proxy_hide_header +
pass-through on /api/ and /api/v1/ws so the backend's own CSP is not
overlaid).
- nginx comment: stop overstating $request_id as 'cryptographically
random'; describe the 128-bit pseudo-random nature and threat model.
- docs: drop remaining 'not Radix's Overlay + Content' migration note from
web/CLAUDE.md; update docs/design/ux-research.md cmdk references to
cmdk-base.
Backend (engine / api / tests):
- agent_engine: rename msg= kwarg to note= in the settings-read failure
log (msg collides with structlog reserved key and could crash the
fallback warning).
- agent_engine: _maybe_notify_personality_trim is now fail-open when the
config resolver raises -- a transient settings failure must not silently
disable notifications that the operator enabled. The warning log still
fires with an explicit fail-open reason.
- Introduce PersonalityTrimPayload TypedDict; annotate the notifier alias,
factory return, and call sites so mypy catches payload key drift.
- make_personality_trim_notifier: annotate return as PersonalityTrimNotifier,
log failures with the full payload context under
PROMPT_PERSONALITY_NOTIFY_FAILED, document that channels_plugin.publish
is synchronous so the async signature does not mislead readers.
- Docstrings: clarify best-effort semantics, note MemoryError/RecursionError
re-raise and CancelledError propagation, document the dual emission paths
of PROMPT_PERSONALITY_NOTIFY_FAILED (setting read + notifier call).
- Tests: add test_personality_trim_notifier_factory (happy path, error
swallow, reuse, BaseException propagation); parametrize the notifier
matrix in test_personality_trim_notifier; add MemoryError/RecursionError
propagation cases for both the notifier and the setting-read branch;
replace brittle len() count assertion in test_engine_settings with a
set-containment check.
Frontend (Base UI migration fixes):
- All 11 dialog/popover/menu files: transition-[opacity,transform,scale] ->
transition-[opacity,translate,scale] (the 'transform' token is dead in
Tailwind v4 which compiles translate-*/scale-* to dedicated CSS props).
- OrgChartPage: useMemo the useRegisterCommands array so the effect does
not thrash command-palette subscribers on every render.
- OrgEditPage Tabs: add isTabValue type guard instead of 'as TabValue'
casts; tighten handleTabChange parameter type.
- command-palette: surface action errors via a toast + always-on log in
production (previously silent unless DEV); log localStorage read/write
errors via the project logger.
- csp.ts: downgrade the '__CSP_NONCE__ placeholder present' log to debug
in DEV so the Vite dev server does not spam false-positive errors on
every page load; keep error-level log in production.
- slot.tsx: replace bare console.warn with createLogger('slot').
- useGlobalNotifications: reset dedupe refs when the WS successfully
reconnects so flapping connections emit fresh toasts instead of silent
forever after the first failure.
- agents store: log warnings on fetchAgents/fetchAgentDetail error paths
so backend failures are correlatable from the frontend log stream.
- confirm-dialog: remove unnecessary e.preventDefault(); add log.warn on
swallowed confirm errors; expose data-variant for testable styling.
- TaskCreateDialog: add the prevOpenRef reset-on-close pattern used by the
other create dialogs; guard onOpenChange against closing during submit.
- Workflow delete dialogs: trust ConfirmDialog's own auto-close path
instead of double-flipping state.
- ModelPullDialog: document the intentional two-phase close during pull.
- WorkflowTableView menu: unify styling with WorkflowCard (bg-card,
cursor-default, data-[highlighted]:bg-surface).
- useCommandPalette: add explicit void return, readonly parameter, and
document the memoization requirement.
Design tokens:
- AppLayout PageLoadingFallback: space-y-4/gap-4 -> space-y-section-gap /
gap-grid-gap; main gutter p-6 -> p-card.
- DialogHeader: px-6 py-4 -> p-card (propagates to VersionDiffViewer).
- VersionDiffViewer summary + body: px-6 py-3/4 -> p-card.
- ProviderFormModal header + body: px-6 py-4/5 -> p-card.
- OrgEditPage tab panel gutter: pt-6 -> pt-section-gap.
- TaskCreateDialog: text-[13px]/[11px]/[10px] -> text-body-sm/compact/micro.
Frontend tests:
- useGlobalNotifications: brittle length check -> shape assertion; new
dedupe-across-rerenders, ref-reset-on-reconnect, and unmount-cleanup
tests; parametrize the setup-error / reconnect-exhausted toast cases.
- agents.test: new tests for .slice(0, 64) agent_name length bound, NaN/
Infinity token rejection, and non-string agent_name fallback.
- confirm-dialog.test: assert data-variant instead of className substring.
- App.test: drop the weak getAllByText('SynthOrg') assertion.
- ApprovalDetailDrawer.test: per-test handler factory (resetAllMocks) so
queued rejections do not leak; scope Approve/Reject confirm-button
lookups to the alertdialog via within(); simplify focus-trap test to
assert the application-level aria-modal contract instead of
re-implementing Base UI's focus-trap internals.
Coverage:
- Pre-existing ruff/mypy clean on all changed Python modules; 14229 Python
unit+integration tests pass (+16 new); 2425 web Vitest tests pass (+6
new); ESLint zero warnings; TypeScript type-check clean; Storybook builds.
…latency On CI with --detect-async-leaks + coverage enabled, the initial import of AppLayout (which transitively pulls framer-motion, cmdk-base, Base UI primitives, and every lazy page) takes 5-9s before the Suspense fallback resolves. The previous 5000ms budget was exactly at the edge and flaked intermittently as new transitively-lazy modules were added. Raise the waitFor timeout to 8000ms and the outer test timeout to 15000ms with headroom over observed CI timings. This is lazy module resolution, not a time-based race -- no deterministic mock can short-circuit actual Node import work, so a wider budget is the correct fix.
- agent_engine: bound the personality-trim notifier call with
asyncio.timeout(2.0) so a slow or hung notifier cannot stall run().
Timeout failures emit PROMPT_PERSONALITY_NOTIFY_FAILED with a distinct
'notifier callback timed out (>2s)' reason. Docstring updated to list
all three failure paths (setting read, timeout, callback raised).
- ModelPullDialog: close the modal after a cancel-during-pull by latching
closeAfterCancelRef on cancel and dispatching onClose() from a useEffect
once pullingModel clears. Uses a ref rather than state to avoid the
@eslint-react/set-state-in-effect warning while still bridging the gap
the earlier comment-only fix left open.
- web/CLAUDE.md: resolve the Select guidance contradiction -- the
'Creating New Components' section no longer tells contributors to
prefer Base UI Select. The authoritative list lives in the Adoption
Decisions table below.
- dialog.tsx: DialogDescription's shared default was 'text-muted' (muted
surface color) instead of 'text-muted-foreground' (readable muted
text). Every consumer inheriting the default now gets the same
contrast as ConfirmDialog.
- theme-toggle.tsx: replace hardcoded shadow-lg on the Popover Popup
with the tokenized shadow-[var(--so-shadow-card-hover)], matching the
rest of the dashboard.
- agents store personality.trimmed handler: suppress the toast entirely
when every payload field is missing/invalid -- a 'An agent personality
was trimmed' toast with no name or numbers provided no actionable
information. The existing warn log retains the diagnostic signal.
Corresponding test renamed from 'uses default label when agent_name is
missing' to 'suppresses the toast when every payload field is missing'.
- useGlobalNotifications unmount test: the previous version flipped the
mocked WS state after unmount but never re-rendered, so it could not
actually verify teardown. Replaced with a baseline 'unmount does not
throw and leaves the store untouched' assertion; deep cleanup coverage
belongs to useWebSocket's own test suite (which owns the subscription).
Not fixed (rejected as invalid):
- TaskCreateDialog render-phase state reset: CodeRabbit flagged the
if (!open && prevOpenRef.current) { setForm(...) } pattern as a React
anti-pattern, but setting state during render based on a prop
transition is an officially supported React idiom (see the React docs
'Storing information from previous renders'). This same pattern lives
in AgentCreateDialog, DepartmentCreateDialog, and PackSelectionDialog
with passing tests + eslint. Changing it would diverge from three
sibling dialogs for a non-issue.
- agent_engine: type PersonalityTrimPayload identifier fields (agent_id, agent_name, task_id) as NotBlankStr per CLAUDE.md identifier convention. Pydantic validators don't fire inside a TypedDict at runtime, but the alias documents intent and keeps the notifier contract consistent with the rest of the codebase. - web/CLAUDE.md: resolve the Tooltip inconsistency -- the 'Creating New Components' and 'Base UI Adoption Decisions' sections no longer mention Tooltip as preferred Base UI, since the adoption table does not list it as adopted. Contributors are directed to reach for existing primitives first and add a row to the table if a real Tooltip requirement appears. - stores/agents: null-guard WsEvent.payload before casting to Record<string, unknown>. The TypeScript 'as' cast is compile-time only -- a malformed broker sending null would throw on the next property access. The guard drops the event and logs the drop. - stores/agents: sanitize attacker-controlled values in log.warn calls -- 'agent' (URL route param) in fetchAgentDetail, 'status' in the unknown-status branch of the agent.status_changed handler, and 'event_type' in the new payload-null guard. Matches the 'sanitizeForLog attacker-controlled fields' rule in web/CLAUDE.md. - dialog.tsx: reuse the shared Button component inside DialogCloseButton via Base UI's render prop, instead of duplicating the icon-button styles. Keeps tokens, focus states, and hover semantics centralized. - dialog.tsx: DialogDescription was text-muted (already fixed in prior commit, retained here). - useGlobalNotifications.test: resolve the 'agents' binding by channel name via bindings.find(b => b.channel === 'agents') instead of by index, so adding unrelated subscriptions upstream cannot silently break the suite.
- api/app.py: the asyncio.timeout() guard on the notifier call was ineffective against a synchronous blocking publish because timeouts only cancel at await points. Wrap channels_plugin.publish() in asyncio.to_thread() so the timeout has a real cancel checkpoint and a pathological channels-plugin implementation cannot block the event loop. Docstring updated to explain the why. - dialog.tsx DialogCloseButton: add explicit type="button" to the shared close control so a dialog containing a <form> cannot accidentally submit the form when the close icon is clicked. - web/CLAUDE.md Adoption Decisions: resolve the 'via shadcn/ui' vs 'used directly, no wrapper layer' contradiction. components.json is the shadcn style setting, but the adopted primitives are imported directly from @base-ui/react/* subpaths with no wrapper layer. Contributors are told to use the direct-import path. - useGlobalNotifications.test: add a mirror of the setupError dedupe reset sequence for reconnectExhaustedRef so a flapping connection that exhausts reconnects twice emits two toasts (not one silent regression). All 5 other items CodeRabbit re-posted in this round are duplicates of findings already fixed in the prior commit (they were flagged at line:null because CodeRabbit's re-scan matched against the diff span rather than the resolved code). The agents.ts line-246 null-cast concern is resolved by the new null guard at line 232, which returns before any cast on a null payload.
824ae2d to
949f08f
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/nginx.conf (1)
117-152:⚠️ Potential issue | 🟠 MajorRe-add non-CSP security headers in the proxied
/apilocations.Both proxy locations now declare their own
add_header Content-Security-Policy, which stops nginx inheritance of/etc/nginx/security-headers.conf. Normal upstream responses still get HSTS/X-Frame-Options/etc. from the backend middleware, but nginx-generated errors (502/504/timeout) originate from the proxy itself and will now lack these headers except CSP. Re-include the security-headers snippet in both locations, or add the remaining headers explicitly alongside the CSP pass-through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/nginx.conf` around lines 117 - 152, The proxied locations location /api/v1/ws and location /api/ add only a passthrough Content-Security-Policy (add_header Content-Security-Policy $upstream_http_content_security_policy always;) which prevents nginx from inheriting /etc/nginx/security-headers.conf for nginx-generated error responses; update both location blocks (the ones with proxy_hide_header Content-Security-Policy and add_header Content-Security-Policy) to re-include the security headers snippet (e.g., include /etc/nginx/security-headers.conf;) or explicitly add the remaining security headers (HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, etc.) alongside the CSP passthrough so nginx-originated 502/504/timeout responses also carry the full set of security headers.
♻️ Duplicate comments (3)
web/src/pages/providers/ModelPullDialog.tsx (1)
88-94:⚠️ Potential issue | 🔴 Critical
AlertDialog.Closerender prop must be a function callback, not a static element.The
renderprop currently receives a static JSX element. Base UI'sAlertDialog.Close(re-exported fromDialog.Close) expects a function that receives and forwards the injected props—including theonClickhandler that closes the dialog. Without this, clicking the Cancel button will not close the dialog.🐛 Proposed fix
<AlertDialog.Close - render={ + render={(props) => ( <Button variant="outline" size="sm"> Cancel </Button> - } + )} />Wait—that still doesn't spread props. The correct fix:
<AlertDialog.Close - render={ - <Button variant="outline" size="sm"> - Cancel - </Button> - } + render={(props) => ( + <Button {...props} variant="outline" size="sm"> + Cancel + </Button> + )} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` around lines 88 - 94, AlertDialog.Close's render prop is incorrectly passed a static JSX element so the injected props (including onClick to close the dialog) are never forwarded; update the usage of AlertDialog.Close in ModelPullDialog.tsx to pass a function callback (e.g. props => <Button {...props} variant="outline" size="sm">Cancel</Button>) so the forwarded props from AlertDialog.Close are spread into the Button component, ensuring the dialog close handler and accessibility props are preserved.web/src/pages/tasks/TaskCreateDialog.tsx (1)
97-106:⚠️ Potential issue | 🔴 CriticalMove the close reset out of render.
These setters still run during render when
openflips tofalse, which violates React’s render rules and can trigger extra renders or loops in concurrent mode. Reset on thetrue -> falsetransition inside an effect instead.Suggested fix
-import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' @@ const prevOpenRef = useRef(open) - if (!open && prevOpenRef.current) { - setForm(INITIAL_FORM) - setErrors({}) - setSubmitError(null) - } - prevOpenRef.current = open + useEffect(() => { + if (!open && prevOpenRef.current) { + setForm(INITIAL_FORM) + setErrors({}) + setSubmitError(null) + } + prevOpenRef.current = open + }, [open])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 97 - 106, The reset logic currently runs during render when open transitions to false (using prevOpenRef/ref check), which violates React rules; move this into an effect: create a useEffect that watches open and when it changes from true to false (track previous open with prevOpenRef inside the effect) call setForm(INITIAL_FORM), setErrors({}) and setSubmitError(null) there, and update prevOpenRef.current = open inside the same effect so no state-setters run during render; ensure you keep the existing INITIAL_FORM and ref variable names (prevOpenRef, open, setForm, setErrors, setSubmitError).src/synthorg/engine/agent_engine.py (1)
1150-1152:⚠️ Potential issue | 🟠 Major
asyncio.timeout()only helps if the notifier yields.If
make_personality_trim_notifier()still callschannels_plugin.publish(...)synchronously, thisawaitcan block_prepare_context()well past 2 seconds because task cancellation does not preempt blocking work on the event loop. Please verify that the concrete notifier awaits a non-blocking publish path or offloads synchronous work.Run this read-only check to confirm the concrete notifier implementation:
#!/bin/bash set -euo pipefail printf '%s\n' '--- notifier factory ---' rg -n -C4 'make_personality_trim_notifier|channels_plugin\.publish|asyncio\.to_thread|run_in_executor|await .*publish' src/synthorg/api/app.py printf '\n%s\n' '--- publish signature ---' rg -n -C3 'class ChannelsPlugin|def publish\(' src/synthorgExpect the factory to either
awaitan async publish or offload sync work viaasyncio.to_thread/ an executor. A barechannels_plugin.publish(...)inside anasync defconfirms the timeout is not actually enforcing the bound.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/agent_engine.py` around lines 1150 - 1152, The asyncio.timeout call around awaiting self._personality_trim_notifier in _prepare_context won't stop synchronous blocking inside the notifier; inspect the notifier produced by make_personality_trim_notifier and confirm it either awaits an async publish (await channels_plugin.publish(...)) or offloads blocking work (use asyncio.to_thread or loop.run_in_executor) rather than calling channels_plugin.publish(...) synchronously; if it calls publish synchronously, change the factory-produced async notifier to await an async publish API or wrap the blocking publish call with asyncio.to_thread/run_in_executor so _personality_trim_notifier yields and the asyncio.timeout can actually enforce the 2s bound.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/security.md`:
- Around line 133-136: Remove or reword the claim that "X-Frame-Options: DENY"
mitigates same-page UI redress: locate the bullet referencing `position: fixed;
z-index: 99999` and `X-Frame-Options: DENY` and drop the justification that it
mitigates injected `style` attributes; instead state that `X-Frame-Options` only
prevents framing (framing attacks) and does not protect against same-page CSS
injection, and keep/clarify that the stronger protection is provided by the CSP
directive `style-src-elem 'self' 'nonce-...'`.
In `@src/synthorg/api/app.py`:
- Around line 249-255: Update the docstring around the best-effort publisher
paragraph to explicitly distinguish which failures are swallowed and which
propagate: state that ordinary publish failures (logged via
PROMPT_PERSONALITY_NOTIFY_FAILED) are caught and suppressed so they don’t
usually block task execution, but that MemoryError and RecursionError are
re-raised per the project-wide best-effort publisher pattern and
asyncio.CancelledError (a BaseException subclass) is allowed to propagate;
remove or reword the absolute phrase like “never block task execution” so it
reflects this nuance. Ensure the text references the best-effort publisher
behavior and the specific exceptions (MemoryError, RecursionError,
asyncio.CancelledError) so readers understand the exact contract.
In `@tests/unit/engine/test_personality_trim_notifier.py`:
- Around line 169-197: Add a new unit test (e.g.,
test_notifier_timeout_is_swallowed) that exercises the TimeoutError branch in
_maybe_notify_personality_trim by configuring AgentEngine with
personality_trim_notifier set to an AsyncMock that blocks on an
asyncio.Event().wait(), patching the notifier timeout to a short value and
mocking time.monotonic() and asyncio.sleep() to make timing deterministic, then
call engine.run(...) and assert the engine completes successfully
(result.is_success is True) and the notifier was awaited (notifier.await_count
== 1) while the blocking notifier does not hang the task; ensure
notify_enabled=True and reuse symbols AgentEngine, personality_trim_notifier and
AsyncMock in the test.
- Around line 73-82: The test matrix row (True, 0, True, True, 0) in
tests/unit/engine/test_personality_trim_notifier.py relies on current default
profile sizes and can fail if defaults tighten; change that row to a tuple that
guarantees no trimming regardless of defaults (e.g., replace the 0 override with
a very large budget like 100000 or another sentinel that disables trimming) so
the notifier behavior remains driven by a guaranteed non-trimming setup while
keeping the expected notifier count 0.
In `@web/package.json`:
- Line 47: The package.json dependency "cmdk-base" is pinned to "1.0.0"; update
the version string to a caret range for consistency by replacing "cmdk-base":
"1.0.0" with "cmdk-base": "^1.0.0" so it behaves like the other dependencies and
allows future patch/minor updates; ensure you run npm/yarn install afterwards to
update lockfile.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 59-91: The test currently asserts toast content but should only
verify delegation: modify the test in useGlobalNotifications.test.ts to spy/mock
useAgentsStore.getState().updateFromWsEvent and assert that
agentsBinding.handler(...) invokes updateFromWsEvent with the received WsEvent;
remove assertions that inspect useToastStore.toasts (title/variant) here and
leave toast-shape/content assertions to web/src/stores/agents.ts tests, keeping
references to mockUseWebSocket, the agents binding lookup, and
agentsBinding.handler when wiring the event through.
- Around line 93-123: Replace the hand-picked table-driven tests with
property-based tests using fast-check: write an fc.property that generates
sequences of WebSocket states (objects with connected:boolean,
reconnectExhausted:boolean, setupError:string|null) and for each generated
sequence mock mockUseWebSocket to return the sequence values in order (via
mockReturnValueOnce or a mockImplementation that shifts through the array), then
call renderHook(() => useGlobalNotifications()) after simulating the sequence
and assert invariants against useToastStore.getState().toasts — e.g., only one
toast exists for the final state with variant/title matching the final state
(and that duplicate-equivalent states do not create extra toasts); update tests
around useGlobalNotifications, mockUseWebSocket, useToastStore (and mirrored
changes in the other block noted 130-210) to use fc.assert + fc.property per the
project guideline.
In `@web/src/__tests__/stores/agents.test.ts`:
- Around line 474-600: Replace the hand-crafted personality.trimmed examples
with fast-check property tests: write fc.property cases that call
useAgentsStore.getState().updateFromWsEvent(...) and then assert the resulting
useToastStore.getState().toasts behavior via fc.assert; generate agent_name as
either string (use a bounded string generator and slice to 64 for the
capped-name assertion) or non-string values to exercise the default "An agent"
fallback, generate before_tokens/after_tokens as finite numbers (fc.float/
double filtered with Number.isFinite) and also include non-finite cases
(Infinity/NaN) to assert fallback description, and include an entirely-empty
payload case to assert no toast; keep references to updateFromWsEvent,
useAgentsStore, and useToastStore when locating the code to convert each
existing it(...) into fc.property + fc.assert tests.
In `@web/src/components/ui/confirm-dialog.tsx`:
- Around line 67-88: Prevent the dialog from being dismissed while loading by
ignoring external close attempts and hiding the close control: (1) stop
rendering AlertDialog.Close when loading so backdrop/close-button clicks can't
trigger a close; (2) wrap the onOpenChange handler used by this component so
that when it receives false (attempt to close) it no-ops if loading is true,
otherwise forwards to the original onOpenChange; reference the AlertDialog.Close
component, the onOpenChange prop/handler, the loading flag and the onConfirm
callback so callers retain dialog state during in-flight confirm attempts.
In `@web/src/hooks/useGlobalNotifications.ts`:
- Around line 43-47: The code logs the websocket setupError directly as a
structured field which bypasses sanitizer; in the useEffect handler (references:
setupError, lastSetupErrorRef.current, and log.warn) wrap setupError with
sanitizeForLog() or pass it as a separate argument to log.warn so the logger
runs its scalar sanitization (e.g., log.warn('Global notifications WebSocket
setup failed', sanitizeForLog(setupError)) or log.warn('Global notifications
WebSocket setup failed', { setupError: sanitizeForLog(setupError) })); keep the
existing behavior of updating lastSetupErrorRef.current and calling
useToastStore.getState().add unchanged.
In `@web/src/pages/org-edit/DepartmentCreateDialog.tsx`:
- Around line 86-93: The Dialog.Popup className uses a hardcoded "shadow-lg";
replace that token with the design variable by swapping "shadow-lg" for
"shadow-[var(--so-shadow-card-hover)]" inside the cn(...) call for the
Dialog.Popup component so the dialog uses the tokenized shadow like other modals
(e.g., ProviderFormModal.tsx); keep the rest of the className entries and
formatting unchanged.
In `@web/src/pages/org-edit/PackSelectionDialog.tsx`:
- Around line 141-148: The Dialog.Popup component's className currently uses the
hardcoded tailwind class "shadow-lg"; update the class list in
PackSelectionDialog's Dialog.Popup (the className prop on the Dialog.Popup
element) to remove "shadow-lg" and instead apply the design token shadow
variable (e.g., use the CSS token var(--so-shadow-card-hover) via a class or
inline style that matches the project's token usage convention), preserving the
rest of the classes and transitions; ensure you follow the same token approach
used elsewhere (like DepartmentCreateDialog.tsx) so the popup uses the tokenized
shadow rather than a hardcoded Tailwind shadow.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 297-305: The FormField wrapper in TaskCreateDialog should be
replaced with the accessible InputField and SelectField components from
web/src/components/ui/ to bind labels and errors correctly: replace each
FormField usage in TaskCreateDialog (Title, Description, Type, Priority,
Project, Created By, Assigned To, Complexity, Budget Limit) with InputField for
text/number/textarea inputs and SelectField for selects; for Description pass
multiline={true} to InputField. Ensure you pass an explicit id/htmlFor, name,
value, onChange, required, and the error message to InputField/SelectField so
they can set aria-invalid, aria-errormessage/aria-describedby and render
hint/error UI; remove the old unbound <label> FormField component or keep it
only if refactored to delegate to InputField/SelectField.
In `@web/src/pages/workflows/WorkflowCard.tsx`:
- Around line 103-107: The onConfirm handler in WorkflowCard calls
onDelete(workflow.id) but does not return its result, so ConfirmDialog cannot
observe the promise or its rejection; update the WorkflowCardProps type so
onDelete accepts and returns void | Promise<void> and change the onConfirm arrow
to return the result of onDelete(workflow.id) (so ConfirmDialog sees the async
result), referencing the onConfirm callback in WorkflowCard, the onDelete prop,
and WorkflowsPage.handleDelete which awaits the delete operation.
In `@web/src/stores/agents.ts`:
- Around line 284-287: The code currently accepts any finite number for
beforeRaw/afterRaw, which allows negatives and non-integers; update the guard so
these token counts must be non-negative integers: replace the checks for
beforeRaw/afterRaw with something like "typeof beforeRaw === 'number' &&
Number.isInteger(beforeRaw) && beforeRaw >= 0 ? beforeRaw : null" (and same for
afterRaw), using the variables beforeRaw, afterRaw and assigning to before and
after in the same location in web/src/stores/agents.ts to let malformed payloads
fall through to the existing error path.
---
Outside diff comments:
In `@web/nginx.conf`:
- Around line 117-152: The proxied locations location /api/v1/ws and location
/api/ add only a passthrough Content-Security-Policy (add_header
Content-Security-Policy $upstream_http_content_security_policy always;) which
prevents nginx from inheriting /etc/nginx/security-headers.conf for
nginx-generated error responses; update both location blocks (the ones with
proxy_hide_header Content-Security-Policy and add_header
Content-Security-Policy) to re-include the security headers snippet (e.g.,
include /etc/nginx/security-headers.conf;) or explicitly add the remaining
security headers (HSTS, X-Frame-Options, X-Content-Type-Options,
Referrer-Policy, etc.) alongside the CSP passthrough so nginx-originated
502/504/timeout responses also carry the full set of security headers.
---
Duplicate comments:
In `@src/synthorg/engine/agent_engine.py`:
- Around line 1150-1152: The asyncio.timeout call around awaiting
self._personality_trim_notifier in _prepare_context won't stop synchronous
blocking inside the notifier; inspect the notifier produced by
make_personality_trim_notifier and confirm it either awaits an async publish
(await channels_plugin.publish(...)) or offloads blocking work (use
asyncio.to_thread or loop.run_in_executor) rather than calling
channels_plugin.publish(...) synchronously; if it calls publish synchronously,
change the factory-produced async notifier to await an async publish API or wrap
the blocking publish call with asyncio.to_thread/run_in_executor so
_personality_trim_notifier yields and the asyncio.timeout can actually enforce
the 2s bound.
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Around line 88-94: AlertDialog.Close's render prop is incorrectly passed a
static JSX element so the injected props (including onClick to close the dialog)
are never forwarded; update the usage of AlertDialog.Close in
ModelPullDialog.tsx to pass a function callback (e.g. props => <Button
{...props} variant="outline" size="sm">Cancel</Button>) so the forwarded props
from AlertDialog.Close are spread into the Button component, ensuring the dialog
close handler and accessibility props are preserved.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 97-106: The reset logic currently runs during render when open
transitions to false (using prevOpenRef/ref check), which violates React rules;
move this into an effect: create a useEffect that watches open and when it
changes from true to false (track previous open with prevOpenRef inside the
effect) call setForm(INITIAL_FORM), setErrors({}) and setSubmitError(null)
there, and update prevOpenRef.current = open inside the same effect so no
state-setters run during render; ensure you keep the existing INITIAL_FORM and
ref variable names (prevOpenRef, open, setForm, setErrors, setSubmitError).
🪄 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: a47cd314-04a4-4dae-9d60-7432e2d77ddf
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
CLAUDE.mddocs/architecture/tech-stack.mddocs/design/brand-and-ux.mddocs/design/engine.mddocs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/ux-research.mddocs/guides/deployment.mddocs/security.mdsrc/synthorg/api/app.pysrc/synthorg/api/ws_models.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/observability/events/prompt.pysrc/synthorg/settings/definitions/engine.pytests/unit/api/test_personality_trim_notifier_factory.pytests/unit/engine/test_personality_trim_notifier.pytests/unit/settings/test_engine_settings.pyweb/CLAUDE.mdweb/components.jsonweb/index.htmlweb/nginx.confweb/package.jsonweb/security-headers.confweb/src/App.tsxweb/src/__tests__/App.test.tsxweb/src/__tests__/components/ui/confirm-dialog.test.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/__tests__/pages/approvals/ApprovalDetailDrawer.test.tsxweb/src/__tests__/stores/agents.test.tsweb/src/api/types.tsweb/src/components/layout/AppLayout.tsxweb/src/components/ui/button.tsxweb/src/components/ui/command-palette.tsxweb/src/components/ui/confirm-dialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/slot.tsxweb/src/components/ui/theme-toggle.tsxweb/src/hooks/useCommandPalette.tsweb/src/hooks/useGlobalNotifications.tsweb/src/lib/csp.tsweb/src/pages/OrgChartPage.tsxweb/src/pages/OrgEditPage.tsxweb/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/org-edit/DepartmentCreateDialog.tsxweb/src/pages/org-edit/PackSelectionDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/providers/ProviderFormModal.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflows/WorkflowCard.tsxweb/src/pages/workflows/WorkflowTableView.tsxweb/src/stores/agents.tsweb/src/styles/global.cssweb/vite.config.ts
Comment on lines
+169
to
+197
| async def test_notifier_failure_is_swallowed( | ||
| self, | ||
| sample_agent_with_personality: AgentIdentity, | ||
| sample_task_with_criteria: Task, | ||
| mock_provider_factory: type[MockCompletionProvider], | ||
| ) -> None: | ||
| """Exceptions raised inside the notifier never break task execution.""" | ||
| notifier = AsyncMock(side_effect=RuntimeError("pub broken")) | ||
| resolver = _make_resolver( | ||
| trimming_enabled=True, | ||
| max_tokens_override=10, | ||
| notify_enabled=True, | ||
| ) | ||
| provider = mock_provider_factory([_make_completion_response()]) | ||
| engine = AgentEngine( | ||
| provider=provider, | ||
| config_resolver=resolver, | ||
| personality_trim_notifier=notifier, | ||
| ) | ||
|
|
||
| # Should complete without raising even though notifier blows up. | ||
| result = await engine.run( | ||
| identity=sample_agent_with_personality, | ||
| task=sample_task_with_criteria, | ||
| ) | ||
|
|
||
| assert notifier.await_count == 1 | ||
| assert result.is_success is True | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a timeout-path test for the new non-blocking guarantee.
This module covers generic exceptions and propagated BaseException subclasses, but not the dedicated TimeoutError branch in _maybe_notify_personality_trim(). A slow notifier is now part of the contract, so it deserves a direct test with a shorter patched timeout and a notifier that waits on an asyncio.Event().
As per coding guidelines, "For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic; for tasks that must block indefinitely until cancelled, use asyncio.Event().wait() instead of asyncio.sleep(large_number)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/test_personality_trim_notifier.py` around lines 169 - 197,
Add a new unit test (e.g., test_notifier_timeout_is_swallowed) that exercises
the TimeoutError branch in _maybe_notify_personality_trim by configuring
AgentEngine with personality_trim_notifier set to an AsyncMock that blocks on an
asyncio.Event().wait(), patching the notifier timeout to a short value and
mocking time.monotonic() and asyncio.sleep() to make timing deterministic, then
call engine.run(...) and assert the engine completes successfully
(result.is_success is True) and the notifier was awaited (notifier.await_count
== 1) while the blocking notifier does not hang the task; ensure
notify_enabled=True and reuse symbols AgentEngine, personality_trim_notifier and
AsyncMock in the test.
| "class-variance-authority": "^0.7.1", | ||
| "clsx": "^2.1.1", | ||
| "cmdk": "^1", | ||
| "cmdk-base": "1.0.0", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if cmdk-base version 1.0.0 exists on npm and see available versions
curl -s https://registry.npmjs.org/cmdk-base | jq '.versions | keys | .[-5:]'Repository: Aureliolo/synthorg
Length of output: 119
cmdk-base is strictly pinned at version 1.0.0.
Unlike other dependencies using caret ranges (^), cmdk-base has an exact version constraint. Since 1.0.0 is the latest available version, this strict pinning is defensible for a new package replacement. However, consider using "^1.0.0" for consistency with the rest of your dependencies and to allow future patch updates automatically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/package.json` at line 47, The package.json dependency "cmdk-base" is
pinned to "1.0.0"; update the version string to a caret range for consistency by
replacing "cmdk-base": "1.0.0" with "cmdk-base": "^1.0.0" so it behaves like the
other dependencies and allows future patch/minor updates; ensure you run
npm/yarn install afterwards to update lockfile.
Comment on lines
297
to
305
| function FormField({ label, error, required, children }: { label: string; error?: string; required?: boolean; children: React.ReactNode }) { | ||
| return ( | ||
| <div> | ||
| <label className="mb-1 block text-[11px] font-semibold uppercase tracking-wider text-text-muted"> | ||
| <label className="mb-1 block text-compact font-semibold uppercase tracking-wider text-text-muted"> | ||
| {label}{required && <span className="text-danger"> *</span>} | ||
| </label> | ||
| {children} | ||
| {error && <p className="mt-0.5 text-[10px] text-danger">{error}</p>} | ||
| {error && <p className="mt-0.5 text-micro text-danger">{error}</p>} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and inspect TaskCreateDialog.tsx
fd "TaskCreateDialog.tsx" --type fRepository: Aureliolo/synthorg
Length of output: 102
🏁 Script executed:
# Read the file to see the FormField component and its usage
cat -n "web/src/pages/tasks/TaskCreateDialog.tsx" | head -350 | tail -100Repository: Aureliolo/synthorg
Length of output: 5347
🏁 Script executed:
# Check the InputField and SelectField components
fd "InputField\|SelectField" --type f --extension tsxRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# Search for InputField and SelectField definitions
rg "InputField|SelectField" --type ts --type tsxRepository: Aureliolo/synthorg
Length of output: 89
🏁 Script executed:
# Check the components/ui directory structure
ls -la "web/src/components/ui/" 2>/dev/null | head -50Repository: Aureliolo/synthorg
Length of output: 3526
🏁 Script executed:
# Look for any field-related components
fd "Field" "web/src/components/ui/" --type fRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# List all remaining files in components/ui
ls -la "web/src/components/ui/" | tail -20Repository: Aureliolo/synthorg
Length of output: 1455
🏁 Script executed:
# Check if select-field exists
fd "select" "web/src/components/ui/" -iRepository: Aureliolo/synthorg
Length of output: 147
🏁 Script executed:
# Read the InputField component
cat "web/src/components/ui/input-field.tsx"Repository: Aureliolo/synthorg
Length of output: 3034
🏁 Script executed:
cat "web/src/components/ui/select-field.tsx"Repository: Aureliolo/synthorg
Length of output: 2239
Refactor FormField to use InputField and SelectField components for proper accessibility.
The current FormField component renders an unbound <label> element, making all inputs and selects inaccessible to assistive technology. TaskCreateDialog uses this wrapper for 9 form fields (Title, Description, Type, Priority, Project, Created By, Assigned To, Complexity, Budget Limit).
Replace these with InputField (for text/number/textarea inputs) and SelectField (for selects) from web/src/components/ui/, which properly bind labels via htmlFor, set aria-invalid/aria-errormessage/aria-describedby attributes, and handle error/hint display. For the Description field, use InputField with multiline={true}.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 297 - 305, The
FormField wrapper in TaskCreateDialog should be replaced with the accessible
InputField and SelectField components from web/src/components/ui/ to bind labels
and errors correctly: replace each FormField usage in TaskCreateDialog (Title,
Description, Type, Priority, Project, Created By, Assigned To, Complexity,
Budget Limit) with InputField for text/number/textarea inputs and SelectField
for selects; for Description pass multiline={true} to InputField. Ensure you
pass an explicit id/htmlFor, name, value, onChange, required, and the error
message to InputField/SelectField so they can set aria-invalid,
aria-errormessage/aria-describedby and render hint/error UI; remove the old
unbound <label> FormField component or keep it only if refactored to delegate to
InputField/SelectField.
Valid items fixed:
- docs/security.md: correct the X-Frame-Options justification -- it
prevents framing by a foreign origin, not same-page UI redress via an
injected style attribute. Same-page redress is covered by different
mitigations.
- api/app.py: clarify the best-effort docstring to distinguish ordinary
Exception publish failures (swallowed) from MemoryError/RecursionError
(re-raised) and asyncio.CancelledError (propagates as a BaseException).
Matches the design page wording.
- agent_engine test matrix: replace the profile-default-dependent
'(True, 0, True, True, 0)' no-trim row with an explicit
'(False, 10, True, True, 0)' trimming-disabled row. Tightening the
default personality budget in future work cannot silently flip this
matrix row.
- agent_engine tests: add test_notifier_timeout_is_swallowed covering
the new 2s TimeoutError branch. Patches asyncio.timeout down to 10ms
and uses asyncio.Event().wait() for a cancellation-safe hang.
- useGlobalNotifications.test: convert the 'forwards personality.trimmed'
test to assert delegation to the agents store (via a vi.spyOn on
updateFromWsEvent) instead of the toast content. Toast shape lives in
the agents store test suite; unrelated copy changes no longer cascade
into this hook's tests.
- confirm-dialog: block Escape and backdrop dismissal while
loading=true. Without this, callers that clear state on close (e.g.
ApprovalDetailDrawer resetting its 'comment' state) drop the user's
retry context mid-operation even though the component intends to stay
open on failure.
- useGlobalNotifications.ts: wrap the setupError value in sanitizeForLog
before embedding in structured log arguments. setupError originates
from WebSocket transport errors which can surface messages derived
from untrusted response bodies.
- DepartmentCreateDialog + PackSelectionDialog: replace hardcoded
shadow-lg with the tokenized shadow-[var(--so-shadow-card-hover)],
matching the other migrated dialogs in this PR.
- TaskCreateDialog FormField: bind label-to-input using the implicit
label-association form (label element wraps the form control).
Previously the label was a sibling with no htmlFor/id pair, so
assistive tech could not associate it with any of the 9 form fields.
- WorkflowCard + WorkflowTableView: return the onDelete promise from
the ConfirmDialog's onConfirm handler, and widen the onDelete prop
type to 'void | Promise<void>' so ConfirmDialog can observe rejection
and keep the dialog open on failure. The parent WorkflowsPage passes
an async handleDelete that was previously not being awaited.
- stores/agents personality.trimmed handler: validate before_tokens /
after_tokens as non-negative integers (not just finite numbers), so
a malformed payload with negative or fractional values falls through
to the generic fallback description instead of rendering nonsense.
- nginx.conf /api and /api/v1/ws locations: re-declare the non-CSP
security headers (X-Content-Type-Options, X-Frame-Options,
Referrer-Policy, Permissions-Policy, HSTS) alongside the CSP
pass-through. Without this, nginx-generated error responses (502/504/
timeout) on proxied paths were missing the full hardening set because
defining any add_header in a location block disables server-level
inheritance.
Rejected findings (with justification):
- web/package.json cmdk-base exact pin: intentional supply-chain posture
for a single-maintainer package. Pin is deliberate, not a bug.
- fast-check refactor of the whole useGlobalNotifications / agents test
suites: adopting property-based testing for these state machines is a
larger architectural change that belongs in a follow-up PR. The
current hand-picked cases cover the documented behavioural invariants.
- ModelPullDialog AlertDialog.Close render={<Button .../>} 'static
element is wrong' finding: false positive. Base UI's render prop
accepts ReactElement | render-function; the static-element form is
the canonical pattern used in 10+ places across this PR, and all
associated tests pass.
There was a problem hiding this comment.
Actionable comments posted: 6
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/org-edit/PackSelectionDialog.tsx (1)
74-86: 🧹 Nitpick | 🔵 TrivialConsider moving render-phase state resets to
useEffectfor consistency.This file uses the same render-phase state reset pattern as
TaskCreateDialog.tsxandDepartmentCreateDialog.tsx. While it works in practice, React documentation recommends performing side effects (including state updates based on prop changes) inuseEffectrather than during render.If the pattern is refactored in
TaskCreateDialog.tsx, consider applying the same change here for consistency across all dialog components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/org-edit/PackSelectionDialog.tsx` around lines 74 - 86, In PackSelectionDialog, state resets for open/close transitions are being performed during render using prevOpenRef/open and direct calls to setLoading, setError, and setApplying; move this logic into a useEffect inside PackSelectionDialog that watches the open prop and prevOpenRef (or simply open) so state updates occur in an effect: on open setLoading(true), setError(null), setApplying(null); on close setLoading(false), setError(null), setApplying(null); and update prevOpenRef.current = open inside the effect (or derive previous-open via a ref) to match the refactor used in TaskCreateDialog.tsx and DepartmentCreateDialog.tsx.
♻️ Duplicate comments (3)
web/src/pages/tasks/TaskCreateDialog.tsx (1)
297-312: 🧹 Nitpick | 🔵 TrivialFormField uses valid implicit label association, but consider using existing components.
The implicit label association (label wrapping the control) is valid per WHATWG spec and works for accessibility. However, coding guidelines recommend reusing existing
InputFieldandSelectFieldcomponents fromweb/src/components/ui/which provide consistent styling, proper ARIA attributes (aria-invalid,aria-describedby), and error/hint handling.This is a recommended refactor for consistency, not a blocker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 297 - 312, The custom FormField component should be replaced with the project's existing form components to ensure consistent styling and ARIA handling: update places using FormField to use the InputField and SelectField components from web/src/components/ui/, passing label, required, error (or hint) props and ensuring aria-invalid and aria-describedby are set via those components; remove the FormField function or convert it into a thin wrapper that delegates to InputField/SelectField so error messages and accessibility attributes are handled by the shared components (keep using the same label text and required flag when mapping props).tests/unit/engine/test_personality_trim_notifier.py (1)
229-240:⚠️ Potential issue | 🟡 MinorAssert notifier invocation in the timeout test to avoid false positives.
This test currently passes even if notification dispatch is skipped before the callback await. Add an await-count assertion via
AsyncMockwrapping so the timeout branch is definitively exercised.✅ Proposed tightening for the timeout-path assertion
- async def slow_notifier(_payload: PersonalityTrimPayload) -> None: + async def slow_notifier(_payload: PersonalityTrimPayload) -> None: await asyncio.Event().wait() + notifier = AsyncMock(side_effect=slow_notifier) provider = mock_provider_factory([_make_completion_response()]) engine = AgentEngine( provider=provider, - personality_trim_notifier=slow_notifier, + personality_trim_notifier=notifier, ) # Must not raise -- TimeoutError is swallowed by the best-effort guard. await engine._maybe_notify_personality_trim(_make_sample_payload()) + assert notifier.await_count == 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/engine/test_personality_trim_notifier.py` around lines 229 - 240, Replace the hand-written slow_notifier with an AsyncMock that simulates a never-resolving await (e.g. slow_notifier = AsyncMock(side_effect=asyncio.Event().wait)) and pass that mock as personality_trim_notifier to AgentEngine; after awaiting engine._maybe_notify_personality_trim(_make_sample_payload()), assert the mock was awaited (e.g. assert slow_notifier.await_count == 1) so the timeout branch is actually exercised for _maybe_notify_personality_trim.web/src/__tests__/hooks/useGlobalNotifications.test.ts (1)
97-214: 🛠️ Refactor suggestion | 🟠 MajorUse fast-check for the websocket state-transition invariants.
These cases are still encoding the dedupe/reset rules with a few hand-picked traces. Rewrite this block as
fc.assert(fc.property(...))over generatedconnected/reconnectExhausted/setupErrorsequences so the state machine is exercised more exhaustively.As per coding guidelines,
web/**/*.test.{ts,tsx}: Web dashboard property-based testing with fast-check (fc.assert+fc.property).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts` around lines 97 - 214, Replace the hand-written scenario tests around useGlobalNotifications with a fast-check property test: use fc.assert(fc.property(...)) to generate sequences/arrays of websocket state objects (fields: connected, reconnectExhausted, setupError) and drive the hook by mocking mockUseWebSocket, calling renderHook(() => useGlobalNotifications()) and rerender() for each step; after replaying each generated trace assert the same invariants currently checked by the examples (toasts length and deduplication/reset behavior against useToastStore, ensuring that identical setupError values are deduped across consecutive renders but are emitted again after a successful reconnect, and that reconnectExhausted dedupe is similarly reset on recovery). Ensure the property shrinks failing traces so the CI shows minimal counterexamples and keep existing helper calls (mockUseWebSocket, renderHook, rerender, useToastStore.getState().toasts) to locate and reuse existing test plumbing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/security.md`:
- Around line 98-100: Update the security docs to avoid asserting unconditional
CSPRNG for nginx's $request_id used in web/nginx.conf (the line "map $request_id
$csp_nonce { default $request_id; }"): change the wording from
"cryptographically random" to "pseudo-random identifier" and add a short caveat
that nginx will use CSPRNG (RAND_bytes) only when compiled/linked with OpenSSL
and otherwise falls back to a seeded PRNG, so deployments without OpenSSL may
not provide full CSPRNG guarantees.
In `@web/nginx.conf`:
- Around line 74-76: The Cache-Control header in the nginx "location /assets/"
block is set with the "always" flag which causes the immutable long-lived header
to be sent on error responses; edit the location /assets/ stanza to remove the
"always" token from the add_header Cache-Control directive so the header is only
applied to successful responses (keep the X-Content-Type-Options "nosniff"
header as-is).
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 59-95: Add a global cleanup to the test suite by adding an
afterEach hook that calls vi.restoreAllMocks() so spies are always removed even
if assertions fail; in the test file containing the 'delegates
personality.trimmed events to the agents store' spec (the test that creates spy
= vi.spyOn(useAgentsStore.getState(), 'updateFromWsEvent')), add afterEach(() =>
{ vi.restoreAllMocks() }) at the suite level to ensure the spy is restored
regardless of test outcome (you can keep or remove the inline spy.mockRestore()
but the afterEach must be present).
In `@web/src/pages/org-edit/DepartmentCreateDialog.tsx`:
- Around line 99-105: In DepartmentCreateDialog replace the undocumented render
prop on Dialog.Close with children: remove the render={...} usage and instead
nest the Button (variant="ghost" size="icon" aria-label="Close") containing the
X icon as the child of Dialog.Close (i.e., <Dialog.Close> <Button> <X />
</Button> </Dialog.Close>), keeping the same Button and X components and props.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 170-174: The dialog's class list in TaskCreateDialog.tsx currently
uses the hardcoded utility shadow "shadow-lg"; update that token to the design
system variable by replacing "shadow-lg" with
"shadow-[var(--so-shadow-card-hover)]" inside the same class string (the array
containing 'rounded-xl border border-border-bright bg-surface p-card shadow-lg',
'transition-[opacity,translate,scale] duration-200 ease-out', ...), ensuring the
rest of the class array remains unchanged so this dialog follows the tokenized
shadow convention used by DepartmentCreateDialog and PackSelectionDialog.
In `@web/src/stores/agents.ts`:
- Around line 284-313: The code accepts any non-negative integers for
beforeRaw/afterRaw so an event where after > before can produce nonsensical
"trimmed: 120 → 600 tokens"; update the post-parse validation to treat
non-decreasing token counts as malformed by checking the parsed values and, if
before !== null && after !== null && after > before, log a warning and set after
(or both) to null so the code falls back to the generic message; modify the
block using isTokenCount/beforeRaw/afterRaw/agentName/displayName/description to
perform this check immediately after computing before and after and ensure the
resulting description uses the generic copy when token ordering is invalid.
---
Outside diff comments:
In `@web/src/pages/org-edit/PackSelectionDialog.tsx`:
- Around line 74-86: In PackSelectionDialog, state resets for open/close
transitions are being performed during render using prevOpenRef/open and direct
calls to setLoading, setError, and setApplying; move this logic into a useEffect
inside PackSelectionDialog that watches the open prop and prevOpenRef (or simply
open) so state updates occur in an effect: on open setLoading(true),
setError(null), setApplying(null); on close setLoading(false), setError(null),
setApplying(null); and update prevOpenRef.current = open inside the effect (or
derive previous-open via a ref) to match the refactor used in
TaskCreateDialog.tsx and DepartmentCreateDialog.tsx.
---
Duplicate comments:
In `@tests/unit/engine/test_personality_trim_notifier.py`:
- Around line 229-240: Replace the hand-written slow_notifier with an AsyncMock
that simulates a never-resolving await (e.g. slow_notifier =
AsyncMock(side_effect=asyncio.Event().wait)) and pass that mock as
personality_trim_notifier to AgentEngine; after awaiting
engine._maybe_notify_personality_trim(_make_sample_payload()), assert the mock
was awaited (e.g. assert slow_notifier.await_count == 1) so the timeout branch
is actually exercised for _maybe_notify_personality_trim.
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 97-214: Replace the hand-written scenario tests around
useGlobalNotifications with a fast-check property test: use
fc.assert(fc.property(...)) to generate sequences/arrays of websocket state
objects (fields: connected, reconnectExhausted, setupError) and drive the hook
by mocking mockUseWebSocket, calling renderHook(() => useGlobalNotifications())
and rerender() for each step; after replaying each generated trace assert the
same invariants currently checked by the examples (toasts length and
deduplication/reset behavior against useToastStore, ensuring that identical
setupError values are deduped across consecutive renders but are emitted again
after a successful reconnect, and that reconnectExhausted dedupe is similarly
reset on recovery). Ensure the property shrinks failing traces so the CI shows
minimal counterexamples and keep existing helper calls (mockUseWebSocket,
renderHook, rerender, useToastStore.getState().toasts) to locate and reuse
existing test plumbing.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 297-312: The custom FormField component should be replaced with
the project's existing form components to ensure consistent styling and ARIA
handling: update places using FormField to use the InputField and SelectField
components from web/src/components/ui/, passing label, required, error (or hint)
props and ensuring aria-invalid and aria-describedby are set via those
components; remove the FormField function or convert it into a thin wrapper that
delegates to InputField/SelectField so error messages and accessibility
attributes are handled by the shared components (keep using the same label text
and required flag when mapping props).
🪄 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: e141ee34-2db7-4783-98cb-f005a727e483
📒 Files selected for processing (13)
docs/security.mdsrc/synthorg/api/app.pytests/unit/engine/test_personality_trim_notifier.pyweb/nginx.confweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/confirm-dialog.tsxweb/src/hooks/useGlobalNotifications.tsweb/src/pages/org-edit/DepartmentCreateDialog.tsxweb/src/pages/org-edit/PackSelectionDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/pages/workflows/WorkflowCard.tsxweb/src/pages/workflows/WorkflowTableView.tsxweb/src/stores/agents.ts
Comment on lines
+99
to
+105
| <Dialog.Close | ||
| render={ | ||
| <Button variant="ghost" size="icon" aria-label="Close"> | ||
| <X className="size-4" /> | ||
| </Button> | ||
| } | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct API for Base UI Dialog.Close render prop in @base-ui/react version 1.3.0?
💡 Result:
The Dialog.Close component in @base-ui/react version 1.3.0 is a dismiss button that closes the dialog when activated. It does not have a documented "render prop" in the official API reference. Instead, it accepts children as React nodes, which can include custom render logic. Usage example from official docs: <Dialog.Popup> <Dialog.Title>Example dialog</Dialog.Title> <Dialog.Close>Close</Dialog.Close> </Dialog.Popup> Children can be any React element, function component, or render prop pattern via children function: <Dialog.Close> {({ provided }) => ( <button {...provided}>Custom Close</button> )} </Dialog.Close> However, no explicit "render" prop or specific render prop API for Dialog.Close is mentioned in the v1.3.0 TypeScript definitions or documentation. The standard way is using children for custom rendering. For full control, use onOpenChange on Dialog.Root with reason === 'closePress' to handle close logic. The official API docs do not list a specific render prop for Dialog.Close; it follows Base UI's standard compound component pattern where children receive implicit props via context if needed.
Citations:
- 1: https://base-ui.com/react/components/dialog
- 2: https://cdn.jsdelivr.net/npm/@base-ui/react@1.3.0/esm/dialog/root/DialogRoot.d.ts
- 3: https://base-ui.com/react/overview/releases/v1-3-0
Replace render prop with children for Dialog.Close (undocumented API).
The render prop is not part of the documented Base UI Dialog.Close API in @base-ui/react v1.3.0. Use children instead:
Correct pattern
<Dialog.Close>
<Button variant="ghost" size="icon" aria-label="Close">
<X className="size-4" />
</Button>
</Dialog.Close>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/org-edit/DepartmentCreateDialog.tsx` around lines 99 - 105, In
DepartmentCreateDialog replace the undocumented render prop on Dialog.Close with
children: remove the render={...} usage and instead nest the Button
(variant="ghost" size="icon" aria-label="Close") containing the X icon as the
child of Dialog.Close (i.e., <Dialog.Close> <Button> <X /> </Button>
</Dialog.Close>), keeping the same Button and X components and props.
- docs/security.md: refine the $request_id claim to match nginx's
conditional behavior. nginx uses CSPRNG (RAND_bytes) when built
against OpenSSL and falls back to a seeded PRNG otherwise. Both
provide 128-bit uniqueness, which is what the CSP nonce threat model
requires; the new wording avoids overstating CSPRNG on non-OpenSSL
builds.
- nginx.conf /assets/: remove the `always` flag from Cache-Control.
With `always`, a transient 404 (e.g. during a bad deploy) gets
pinned in the browser/CDN cache for a year with the immutable
policy, requiring manual cache clearing. Without `always`, the
header only applies to successful responses (which is what we want).
X-Content-Type-Options keeps `always` because nosniff is safe to
emit on error responses.
- useGlobalNotifications.test: add afterEach(vi.restoreAllMocks) so a
failing test that bails before its inline mockRestore() call cannot
leak a spy into subsequent tests.
- dialog.tsx, confirm-dialog.tsx, AgentCreateDialog.tsx,
ModelPullDialog.tsx, command-palette.tsx, TaskCreateDialog.tsx:
replace the remaining hardcoded `shadow-lg` on dialog/popup
surfaces with the tokenized `shadow-[var(--so-shadow-card-hover)]`,
completing the dialog-surface shadow migration across the PR.
- stores/agents personality.trimmed handler: reject contradictory
payloads where after_tokens > before_tokens. Trimming must reduce
token count, so a swapped payload falls through to the generic
fallback description instead of rendering "trimmed: 120 → 600 tokens".
Rejected (false positives):
- Dialog.Close render prop "not documented": the `render` prop IS
supported on Dialog.Close and AlertDialog.Close per the official
Base UI docs and is used in 10+ places across this PR with passing
tests. CoderRabbit's web query returned stale information.
- PackSelectionDialog render-phase state reset "move to useEffect":
the `if (!open && prevOpenRef.current) { setState(...) }` pattern is
officially supported by React (see "Storing information from
previous renders" in the React docs). The same pattern is used in
three sibling dialogs with passing tests + eslint.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/ui/command-palette.tsx (1)
111-131:⚠️ Potential issue | 🟠 MajorAsync command actions will not have errors caught or toasted.
The comment mentions "Delete agent" as a use case, which is typically an async API call. Since
cmd.action()is not awaited, if it returns a Promise that rejects:
- The rejection escapes the try-catch
- No error is logged or toasted
close()executes immediately, making the user believe the action succeededThis directly contradicts the stated intent to prevent users from believing "destructive actions completed when they did not."
🐛 Proposed fix to handle async actions
const handleSelect = useCallback( (cmd: CommandItem) => { addRecentId(cmd.id) - try { - cmd.action() - } catch (err) { + const execute = async () => { + try { + await cmd.action() + } catch (err) { + log.error('Command action failed', { commandId: cmd.id, label: cmd.label }, err) + useToastStore.getState().add({ + variant: 'error', + title: 'Command failed', + description: `"${cmd.label}" did not complete: ${getErrorMessage(err)}`, + }) + } + } + execute().catch((err) => { // Always log + toast so users see when a destructive command (e.g. // "Delete agent") fails instead of the palette closing silently as // if the action succeeded. Getting this wrong would mean users // believe destructive actions completed when they did not. log.error('Command action failed', { commandId: cmd.id, label: cmd.label }, err) - useToastStore.getState().add({ - variant: 'error', - title: 'Command failed', - description: `"${cmd.label}" did not complete: ${getErrorMessage(err)}`, - }) - } + }) close() }, [close], )Alternatively, a cleaner approach using
voidto explicitly fire-and-forget while handling errors:const handleSelect = useCallback( (cmd: CommandItem) => { addRecentId(cmd.id) - try { - cmd.action() - } catch (err) { - // Always log + toast so users see when a destructive command (e.g. - // "Delete agent") fails instead of the palette closing silently as - // if the action succeeded. Getting this wrong would mean users - // believe destructive actions completed when they did not. - log.error('Command action failed', { commandId: cmd.id, label: cmd.label }, err) - useToastStore.getState().add({ - variant: 'error', - title: 'Command failed', - description: `"${cmd.label}" did not complete: ${getErrorMessage(err)}`, - }) - } + void Promise.resolve(cmd.action()).catch((err: unknown) => { + log.error('Command action failed', { commandId: cmd.id, label: cmd.label }, err) + useToastStore.getState().add({ + variant: 'error', + title: 'Command failed', + description: `"${cmd.label}" did not complete: ${getErrorMessage(err)}`, + }) + }) close() }, [close], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/command-palette.tsx` around lines 111 - 131, The handler handleSelect currently calls cmd.action() without awaiting it, so rejections escape the try/catch and close() runs immediately; change handleSelect to be async and await the result of CommandItem.action (e.g., await Promise.resolve(cmd.action())) so both sync and async actions are handled, move close() into a finally block so the palette only closes after the action completes, and keep the existing error handling (log.error and useToastStore.add) inside the catch; also update the useCallback dependency list to include any referenced symbols used in the function (addRecentId, log, getErrorMessage, useToastStore) so the callback stays stable.
♻️ Duplicate comments (5)
web/src/__tests__/hooks/useGlobalNotifications.test.ts (2)
105-222:⚠️ Potential issue | 🟠 MajorMigrate these WebSocket state-transition tests to fast-check properties.
These scenarios are state-machine behavior but remain example/table-driven. Convert them to
fc.assert(fc.property(...))to satisfy the repository testing rule and improve transition coverage.As per coding guidelines, "web/**/*.test.{ts,tsx}: Web dashboard property-based testing with fast-check (
fc.assert+fc.property)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts` around lines 105 - 222, The tests currently use example/table-driven and sequential it blocks; convert them to fast-check property tests by replacing the it/it.each blocks for useGlobalNotifications with fc.assert(fc.property(...)) properties that generate sequences of WebSocket state transitions and assert toast outcomes; specifically, write properties that (1) generate single-state inputs to cover warning vs error toasts (use mockUseWebSocket + renderHook(() => useGlobalNotifications()) and check useToastStore.getState().toasts for expectedVariant/expectedTitle), (2) generate multiple identical re-renders to verify deduplication of setupError (targeting the dedupe logic in useGlobalNotifications and lastSetupErrorRef), and (3) generate state sequences that include recovery transitions to verify both setupError and reconnectExhausted refs reset (use rerender() between mockUseWebSocket.mockReturnValue(...) calls and assert useToastStore.getState().toasts lengths); ensure each test uses fc.assert(fc.property(...)) and shrinkable generators for wsState objects (connected, reconnectExhausted, setupError) to satisfy the repository rule.
2-2:⚠️ Potential issue | 🔴 CriticalImport
afterEachfrom Vitest.
afterEach(...)is used on Line 27 but not imported on Line 2, causing a type or runtime error.Proposed fix
-import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts` at line 2, The test file uses afterEach(...) but doesn't import it; update the import statement that currently lists beforeEach, describe, expect, it, vi to also include afterEach so the symbol is available (i.e., add afterEach to the named imports from 'vitest' in the test file where useGlobalNotifications.test.ts imports Vitest helpers).web/src/pages/providers/ModelPullDialog.tsx (2)
88-94:⚠️ Potential issue | 🟠 Major
AlertDialog.Closerender prop should use function callback.Same issue as flagged previously: the
renderprop passes a static element instead of a function that spreads the injected props onto theButton. This prevents the cancel button from receiving the close handler.Correct pattern
<AlertDialog.Close render={(props) => ( <Button {...props} variant="outline" size="sm"> Cancel </Button> )} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` around lines 88 - 94, The AlertDialog.Close usage currently passes a static element to the render prop so the injected close handler isn't wired; update the AlertDialog.Close render prop to be a function (accepting props) and spread those props onto the Button (e.g., AlertDialog.Close render={(props) => <Button {...props} .../>}) so the cancel Button receives the close handler and other injected props; modify the component around AlertDialog.Close and the Button accordingly.
2-2: 🛠️ Refactor suggestion | 🟠 MajorUse the shared
Dialogcomponent instead of rawAlertDialog.Per coding guidelines, always reuse components from
web/src/components/ui/before creating new ones. A sharedDialogwrapper exists, andAlertDialogis semantically intended for confirmations requiring user affirmation—not data-entry/progress modals like this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/ModelPullDialog.tsx` at line 2, The file imports and uses AlertDialog; replace that with the shared Dialog wrapper from web/src/components/ui by changing the import to Dialog and updating the JSX usage in the ModelPullDialog component (swap AlertDialog, AlertDialogTrigger, AlertDialogContent, etc. to the corresponding Dialog, DialogTrigger, DialogContent, DialogHeader/Title/Description or the wrapper's API) so the modal uses the shared Dialog component; ensure any props (open/onOpenChange, title/description, cancel/confirm buttons) are mapped to the Dialog API and remove AlertDialog-specific semantics since this is a progress/data-entry modal.web/src/pages/tasks/TaskCreateDialog.tsx (1)
297-310:⚠️ Potential issue | 🟠 MajorKeep the validation message out of the implicit
<label>.This wrapper renders the error
<p>inside the same<label>as the control, so screen readers fold the validation copy into the control's accessible name instead of treating it as separate error/help text. Render the error after the label/control pair and associate it witharia-describedby/aria-errormessage, or switch these fields over to the sharedInputField/SelectFieldcomponents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 297 - 310, FormField currently places the error <p> inside the same <label>, causing screen readers to include validation text in the control's accessible name; update FormField so the <label> only wraps the visible label and the form control (children) and move the error <p> to render after the label/control pair (outside the <label>), generate a stable error element id (e.g., errorId) and expose it by adding aria-describedby or aria-errormessage to the control (or change callers to use the shared InputField/SelectField which can accept the errorId/aria props) so the error text is announced as separate help/error text rather than part of the label.
🤖 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/nginx.conf`:
- Around line 147-150: Add a short explanatory comment adjacent to the
proxy_hide_header Content-Security-Policy / add_header Content-Security-Policy
$upstream_http_content_security_policy always block stating that when nginx
itself returns error pages (502/504) $upstream_http_content_security_policy will
be empty and no CSP will be emitted for those /api/* responses, and that this is
expected because API responses are JSON and other security headers (e.g.,
X-Frame-Options, Strict-Transport-Security) are still applied; duplicate the
same comment where the same proxy_hide_header + add_header pattern appears
elsewhere (the second occurrence of these directives) so future maintainers
understand the behavior.
In `@web/src/pages/org-edit/AgentCreateDialog.tsx`:
- Around line 87-89: The header close control in AgentCreateDialog remains
interactive while submission is in flight even though onOpenChange ignores
closes; find the header Dialog.Close element(s) (the same control mirrored in
the footer) and add disabled={submitting} to them so the header close is
disabled during submission to match the footer Cancel button behavior; ensure
you apply the same change to the other Dialog.Close instances referenced around
lines 103-109 so all close controls are disabled while submitting.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 156-163: The header close control (Dialog.Close) remains focusable
during submission even though onOpenChange suppresses closes; update the header
close button to be disabled when submitting (matching the footer Cancel
behavior) so it is not focusable or operable while a create request is in
flight. Locate the Dialog.Close used in the dialog header and add the same
disabled={submitting} prop (or render a non-focusable element / aria-disabled +
tabIndex={-1} when submitting) and ensure its onClick/no-op behavior is
consistent; apply the same change for the other dialog instance referenced
around the 181-187 range.
---
Outside diff comments:
In `@web/src/components/ui/command-palette.tsx`:
- Around line 111-131: The handler handleSelect currently calls cmd.action()
without awaiting it, so rejections escape the try/catch and close() runs
immediately; change handleSelect to be async and await the result of
CommandItem.action (e.g., await Promise.resolve(cmd.action())) so both sync and
async actions are handled, move close() into a finally block so the palette only
closes after the action completes, and keep the existing error handling
(log.error and useToastStore.add) inside the catch; also update the useCallback
dependency list to include any referenced symbols used in the function
(addRecentId, log, getErrorMessage, useToastStore) so the callback stays stable.
---
Duplicate comments:
In `@web/src/__tests__/hooks/useGlobalNotifications.test.ts`:
- Around line 105-222: The tests currently use example/table-driven and
sequential it blocks; convert them to fast-check property tests by replacing the
it/it.each blocks for useGlobalNotifications with fc.assert(fc.property(...))
properties that generate sequences of WebSocket state transitions and assert
toast outcomes; specifically, write properties that (1) generate single-state
inputs to cover warning vs error toasts (use mockUseWebSocket + renderHook(() =>
useGlobalNotifications()) and check useToastStore.getState().toasts for
expectedVariant/expectedTitle), (2) generate multiple identical re-renders to
verify deduplication of setupError (targeting the dedupe logic in
useGlobalNotifications and lastSetupErrorRef), and (3) generate state sequences
that include recovery transitions to verify both setupError and
reconnectExhausted refs reset (use rerender() between
mockUseWebSocket.mockReturnValue(...) calls and assert
useToastStore.getState().toasts lengths); ensure each test uses
fc.assert(fc.property(...)) and shrinkable generators for wsState objects
(connected, reconnectExhausted, setupError) to satisfy the repository rule.
- Line 2: The test file uses afterEach(...) but doesn't import it; update the
import statement that currently lists beforeEach, describe, expect, it, vi to
also include afterEach so the symbol is available (i.e., add afterEach to the
named imports from 'vitest' in the test file where
useGlobalNotifications.test.ts imports Vitest helpers).
In `@web/src/pages/providers/ModelPullDialog.tsx`:
- Around line 88-94: The AlertDialog.Close usage currently passes a static
element to the render prop so the injected close handler isn't wired; update the
AlertDialog.Close render prop to be a function (accepting props) and spread
those props onto the Button (e.g., AlertDialog.Close render={(props) => <Button
{...props} .../>}) so the cancel Button receives the close handler and other
injected props; modify the component around AlertDialog.Close and the Button
accordingly.
- Line 2: The file imports and uses AlertDialog; replace that with the shared
Dialog wrapper from web/src/components/ui by changing the import to Dialog and
updating the JSX usage in the ModelPullDialog component (swap AlertDialog,
AlertDialogTrigger, AlertDialogContent, etc. to the corresponding Dialog,
DialogTrigger, DialogContent, DialogHeader/Title/Description or the wrapper's
API) so the modal uses the shared Dialog component; ensure any props
(open/onOpenChange, title/description, cancel/confirm buttons) are mapped to the
Dialog API and remove AlertDialog-specific semantics since this is a
progress/data-entry modal.
In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 297-310: FormField currently places the error <p> inside the same
<label>, causing screen readers to include validation text in the control's
accessible name; update FormField so the <label> only wraps the visible label
and the form control (children) and move the error <p> to render after the
label/control pair (outside the <label>), generate a stable error element id
(e.g., errorId) and expose it by adding aria-describedby or aria-errormessage to
the control (or change callers to use the shared InputField/SelectField which
can accept the errorId/aria props) so the error text is announced as separate
help/error text rather than part of the label.
🪄 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: b5dcfab6-6eca-44a3-94ab-c5cc815c58cc
📒 Files selected for processing (10)
docs/security.mdweb/nginx.confweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/components/ui/command-palette.tsxweb/src/components/ui/confirm-dialog.tsxweb/src/components/ui/dialog.tsxweb/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/stores/agents.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: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.warn,console.error, orconsole.debugin application code
Use variable namelogfor logger instances (e.g.,const log = createLogger('module-name'))
Uselog.debug()for dev-only logging (stripped in production),log.warn()for warnings, andlog.error()for errors
Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs
Files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/components/ui/command-palette.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/pages/**/*.{ts,tsx}: Always reuse existing components fromweb/src/components/ui/before creating new ones
UseStatusBadgecomponent for agent/task/system status indicators (colored dot + optional built-in label toggle)
UseMetricCardcomponent for numeric KPIs with sparkline, change badge, progress bar
UseSparklinecomponent for inline SVG trend lines withcolor?andanimated?props (used inside MetricCard or standalone)
UseSectionCardcomponent for titled card wrapper with icon and action slot
UseAgentCardcomponent for agent display (avatar, name, role, status, current task)
UseDeptHealthBarcomponent for department utilization (animated fill bar +health?+agentCount)
UseProgressGaugecomponent for circular or linear gauge for budget/utilization (variant?defaults to'circular',max?defaults to 100)
UseStatPillcomponent for compact inline label + value pair
UseAvatarcomponent for circular initials avatar with optionalborderColor?prop
UseToast/ToastContainercomponent for success/error/warning/info notifications with auto-dismiss queue (mountToastContaineronce in AppLayout)
UseSkeleton/SkeletonCard/SkeletonMetric/SkeletonTable/SkeletonTextcomponents for loading placeholders matching component shapes (shimmer animation, respectsprefers-reduced-motion)
UseEmptyStatecomponent for no-data / no-results placeholder with icon, title, description, optional action button
UseErrorBoundarycomponent for React error boundary with retry --levelprop:page/section/component
UseConfirmDialogcomponent for confirmation modal (Radix AlertDialog) withdefault/destructivevariants andloadingstate
UseCommandPalettecomponent for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands viauseCommandPalettehook
UseInlineEditcomponent for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Use...
Files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,css}: Use Tailwind semantic color classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) instead of hardcoded hex values
Usefont-sansorfont-monofor typography (maps to Geist tokens), never setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders instead of hardcoded values
Do NOT usergba()with hardcoded values -- use design token variables
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files and catches design system violations; fix all violations before proceeding
Files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/components/ui/command-palette.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Do NOT hardcode Framer Motion transition durations -- use presets from@/lib/motion
CSS side-effect imports in TypeScript files should use type declarations (/// <reference types="vite/client" />covers this); TypeScript 6 defaultsnoUncheckedSideEffectImportsto trueTypeScript files: use TypeScript 6.0+, type all code, zero ESLint warnings; reuse existing components from
web/src/components/ui/before creating new ones; NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions—use design tokens and@/lib/motionpresets
Files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/components/ui/command-palette.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/ui/**/*.{ts,tsx}: Place new shared components inweb/src/components/ui/with descriptive kebab-case filename
Export props as a TypeScript interface in new components
Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Importcnfrom@/lib/utilsfor conditional class merging in component files
Files:
web/src/components/ui/command-palette.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
web/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Web dashboard property-based testing with fast-check (
fc.assert+fc.property)
Files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
🧠 Learnings (62)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T13:59:09.628Z
Learning: Applies to web/package.json : Web dashboard dependencies: Node.js 22+, TypeScript 6.0+, React 19, react-router, shadcn/ui, Radix UI, Tailwind CSS 4, Zustand, tanstack/react-query, xyflow/react, dagrejs/dagre, d3-force, dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, CodeMirror 6, Storybook 10, MSW, Vitest, testing-library/react, fast-check, ESLint, Playwright
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to docker/Dockerfile.web : `nginxinc/nginx-unprivileged` base with non-root user (UID 101), Vue 3 SPA (PrimeVue + Tailwind CSS), SPA routing, API/WebSocket proxy to backend
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ConfirmDialog` component for confirmation modal (Radix AlertDialog) with `default` / `destructive` variants and `loading` state
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `AgentCard` component for agent display (avatar, name, role, status, current task)
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/stores/agents.tsweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `InlineEdit` component for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ThemeToggle` component for Radix Popover with 5-axis theme controls (color, density, typography, animation, sidebar), rendered in StatusBar
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Drawer` component for slide-in panel (`side` prop: left or right, default right) with overlay, spring animation, focus trap, Escape-to-close, optional header (`title`), `ariaLabel` for accessible name (one of `title` or `ariaLabel` required), and `contentClassName` override
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Import `cn` from `@/lib/utils` for conditional class merging in component files
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-05T13:59:09.628Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T13:59:09.628Z
Learning: Applies to web/package.json : Web dashboard dependencies: Node.js 22+, TypeScript 6.0+, React 19, react-router, shadcn/ui, Radix UI, Tailwind CSS 4, Zustand, tanstack/react-query, xyflow/react, dagrejs/dagre, d3-force, dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, CodeMirror 6, Storybook 10, MSW, Vitest, testing-library/react, fast-check, ESLint, Playwright
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/components/ui/*.{tsx,ts} : For new shared React components: place in web/src/components/ui/ with kebab-case filename, create .stories.tsx with all states, export props as TypeScript interface, use design tokens exclusively
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
Applied to files:
web/src/pages/org-edit/AgentCreateDialog.tsxweb/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `CommandPalette` component for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands via `useCommandPalette` hook
Applied to files:
web/src/components/ui/command-palette.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.{ts,tsx} : Vitest unit tests must use coverage scoped to files changed vs origin/main branch
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Toast` / `ToastContainer` component for success/error/warning/info notifications with auto-dismiss queue (mount `ToastContainer` once in AppLayout)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.tsweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.test.{ts,tsx} : Web dashboard: Use React Hypothesis (fast-check) for property-based testing with fc.assert + fc.property
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `storybook/test` instead of `storybook/test` and `storybook/actions` instead of `storybook/addon-actions` (Storybook 10 import path changes)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.ts
📚 Learning: 2026-04-05T13:59:09.628Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-05T13:59:09.628Z
Learning: Applies to web/**/*.test.{ts,tsx} : Web dashboard property-based testing with fast-check (`fc.assert` + `fc.property`)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/__tests__/**/*.test.{ts,tsx} : Use property-based testing with fast-check in React tests (`fc.assert` + `fc.property`)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.ts
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/__tests__/hooks/useGlobalNotifications.test.tsweb/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Wrap attacker-controlled fields inside structured objects with `sanitizeForLog()` before embedding in logs
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go through `sanitizeArg`
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Always use `createLogger` from `@/lib/logger` instead of bare `console.warn`, `console.error`, or `console.debug` in application code
Applied to files:
web/src/stores/agents.ts
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to docker/Dockerfile.web : `nginxinc/nginx-unprivileged` base with non-root user (UID 101), Vue 3 SPA (PrimeVue + Tailwind CSS), SPA routing, API/WebSocket proxy to backend
Applied to files:
web/nginx.conf
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Applied to files:
web/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsx
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Alert the user and explain why if implementation deviates from the design spec; do NOT silently diverge
Applied to files:
web/src/pages/providers/ModelPullDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `InputField` component for labeled text input with error/hint display, optional multiline textarea mode
Applied to files:
web/src/pages/providers/ModelPullDialog.tsxweb/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ToggleField` component for labeled toggle switch (role="switch") with optional description text
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `TaskStatusIndicator` component for task status dot with optional label and pulse animation (accepts `TaskStatus`)
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `PriorityBadge` component for task priority colored pill badge (critical/high/medium/low)
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `SelectField` component for labeled select dropdown with error/hint and placeholder support
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `SliderField` component for labeled range slider with custom value formatter and aria-live display
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Web dashboard shadows/borders: use token variables (var(--so-shadow-card-hover), border-border, border-bright); never hardcode shadow or border values
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Use token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`) for shadows and borders instead of hardcoded values
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`) for shadows and borders. NEVER hardcode shadow or border values
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use token variables (var(--so-shadow-card-hover), border-border, border-bright) for shadows/borders; never hardcode values
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use token variables for shadows/borders (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`); no hardcoded values
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Use design tokens exclusively in new components -- no hardcoded colors, fonts, or spacing
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Do not use rgba() with hardcoded values -- use design token variables
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Do NOT use `rgba()` with hardcoded values -- use design token variables
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions — use design tokens and `@/lib/motion` presets
Applied to files:
web/src/pages/tasks/TaskCreateDialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Export props as a TypeScript interface in new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Export props as a TypeScript interface for new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT hardcode Framer Motion transition durations -- use presets from `@/lib/motion`
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Use density-aware spacing tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Use Tailwind semantic color classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`, etc.) or CSS variables (`var(--so-accent)`) instead of hardcoded hex values
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use density-aware tokens (p-card, gap-section-gap, gap-grid-gap) or standard Tailwind spacing; never hardcode pixel values for layout spacing
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-01T20:43:51.878Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T20:43:51.878Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `StatusBadge` component for agent/task/system status indicators (colored dot + optional built-in label toggle)
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Security: SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies: disabled/weighted/per-category/milestone), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume).
Applied to files:
docs/security.md
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use `color?` and `animated?` props for Sparkline component (inline SVG trend lines)
Applied to files:
web/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use `@/components/ui/animated-presence` (Framer Motion AnimatePresence keyed by route) for page transitions
Applied to files:
web/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use semantic Tailwind classes (text-foreground, bg-card, text-accent, text-success, bg-danger) or CSS variables (var(--so-*)); never hardcode hex values in .tsx/.ts files
Applied to files:
web/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-*)`) for colors. NEVER hardcode hex values or rgba() in `.tsx`/`.ts` files
Applied to files:
web/src/components/ui/confirm-dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{ts,tsx,css} : Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions in web code — use design tokens and `@/lib/motion` presets
Applied to files:
web/src/components/ui/confirm-dialog.tsx
Comment on lines
+147
to
+150
| # Pass the backend's own Content-Security-Policy verbatim instead of | ||
| # layering the dashboard's nonce-bearing CSP on top of it. | ||
| proxy_hide_header Content-Security-Policy; | ||
| add_header Content-Security-Policy $upstream_http_content_security_policy always; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Note: nginx-generated errors (502/504) will lack a CSP header on /api/*.
The proxy_hide_header + add_header $upstream_http_content_security_policy pattern correctly passes through the backend's CSP on successful proxied responses. However, when nginx itself generates an error (e.g., backend unreachable), $upstream_http_content_security_policy is empty and no CSP header is emitted.
This is acceptable because:
- API responses are JSON, not rendered as HTML.
- The non-CSP security headers (
X-Frame-Options,Strict-Transport-Security, etc.) are still present on all responses including errors.
Consider adding a brief comment noting this expected behaviour for future maintainers.
📝 Suggested clarifying comment
# Pass the backend's own Content-Security-Policy verbatim instead of
# layering the dashboard's nonce-bearing CSP on top of it.
+ # Note: on nginx-generated errors (502/504/timeout), no CSP header is
+ # emitted because there is no upstream response -- acceptable for API
+ # JSON responses which are not rendered as HTML.
proxy_hide_header Content-Security-Policy;
add_header Content-Security-Policy $upstream_http_content_security_policy always;Also applies to: 176-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/nginx.conf` around lines 147 - 150, Add a short explanatory comment
adjacent to the proxy_hide_header Content-Security-Policy / add_header
Content-Security-Policy $upstream_http_content_security_policy always block
stating that when nginx itself returns error pages (502/504)
$upstream_http_content_security_policy will be empty and no CSP will be emitted
for those /api/* responses, and that this is expected because API responses are
JSON and other security headers (e.g., X-Frame-Options,
Strict-Transport-Security) are still applied; duplicate the same comment where
the same proxy_hide_header + add_header pattern appears elsewhere (the second
occurrence of these directives) so future maintainers understand the behavior.
Comment on lines
+87
to
+89
| <Dialog.Root open={open} onOpenChange={(v: boolean) => { if (!submitting) onOpenChange(v) }}> | ||
| <Dialog.Portal> | ||
| <Dialog.Overlay className="fixed inset-0 z-50 bg-background/80 backdrop-blur-sm data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0" /> | ||
| <Dialog.Content | ||
| <Dialog.Backdrop className="fixed inset-0 z-50 bg-background/80 backdrop-blur-sm transition-opacity duration-200 ease-out data-[closed]:opacity-0 data-[starting-style]:opacity-0 data-[ending-style]:opacity-0" /> |
There was a problem hiding this comment.
Disable the header close control while submission is in flight.
onOpenChange already ignores close requests during submitting, so this button stays interactive but cannot actually dismiss the dialog. Pass disabled={submitting} here to match the footer Cancel button and avoid a dead control.
🩹 Proposed fix
<Dialog.Close
render={
- <Button variant="ghost" size="icon" aria-label="Close">
+ <Button variant="ghost" size="icon" aria-label="Close" disabled={submitting}>
<X className="size-4" />
</Button>
}
/>Also applies to: 103-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/org-edit/AgentCreateDialog.tsx` around lines 87 - 89, The
header close control in AgentCreateDialog remains interactive while submission
is in flight even though onOpenChange ignores closes; find the header
Dialog.Close element(s) (the same control mirrored in the footer) and add
disabled={submitting} to them so the header close is disabled during submission
to match the footer Cancel button behavior; ensure you apply the same change to
the other Dialog.Close instances referenced around lines 103-109 so all close
controls are disabled while submitting.
Comment on lines
+156
to
+163
| <Dialog.Root | ||
| open={open} | ||
| onOpenChange={(next: boolean) => { | ||
| // Prevent backdrop click / Escape from closing the dialog while a | ||
| // create request is in flight, matching the guard pattern used by | ||
| // the other create dialogs. | ||
| if (!submitting) onOpenChange(next) | ||
| }} |
There was a problem hiding this comment.
Disable the header close control while submission is in flight.
This dialog now suppresses close requests during submitting, but the header “X” still looks operable. Disable it like the footer Cancel button so users do not get a focusable control that does nothing.
🩹 Proposed fix
<Dialog.Close
render={
- <Button variant="ghost" size="icon" aria-label="Close">
+ <Button variant="ghost" size="icon" aria-label="Close" disabled={submitting}>
<X className="size-4" />
</Button>
}
/>Also applies to: 181-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/tasks/TaskCreateDialog.tsx` around lines 156 - 163, The header
close control (Dialog.Close) remains focusable during submission even though
onOpenChange suppresses closes; update the header close button to be disabled
when submitting (matching the footer Cancel behavior) so it is not focusable or
operable while a create request is in flight. Locate the Dialog.Close used in
the dialog header and add the same disabled={submitting} prop (or render a
non-focusable element / aria-disabled + tabIndex={-1} when submitting) and
ensure its onClick/no-op behavior is consistent; apply the same change for the
other dialog instance referenced around the 181-187 range.
This was referenced Apr 5, 2026
Closed
Closed
Aureliolo
added a commit
that referenced
this pull request
Apr 6, 2026
…nce, rebuild org chart page, and fix agent routing (#1083) Closes #1070. Subsumes #1064. Replaces the previously closed PR #1074. Same branch, clean single-narrative summary of what actually landed. ## Summary Eight bundled changes that all touch the same frontend files (plus one backend semantics fix for the analytics overview): 1. **Radix UI to Base UI migration.** `radix-ui` + `cmdk` fully removed (115 transitive packages); `@base-ui/react@1.3.0` + `cmdk-base@1.0.0` installed (8 packages). `components.json` switches from `radix-nova` to `base-vega`. 14 component files migrated: `button.tsx` (via a local `<Slot>` helper over `@base-ui/react/merge-props`), `dialog.tsx`, `confirm-dialog.tsx`, `theme-toggle.tsx`, `command-palette.tsx` (now `cmdk-base` hosted inside Base UI Dialog for focus trap + escape + portal), six page-level Dialog / AlertDialog files, `OrgEditPage.tsx` Tabs, and both Workflow Menu files. Data attributes rewritten (`data-[state=open]` to `data-[open]`, `data-[state=active]` to `data-[active]`, plus `data-[starting-style]` / `data-[ending-style]` for transition lifecycle). 2. **CSP nonce activation, end-to-end.** `nginx.conf` generates a per-request nonce from `$request_id` and substitutes it into `__CSP_NONCE__` via `sub_filter`. `index.html` exposes the nonce on `<meta name="csp-nonce">`. `web/src/lib/csp.ts` reads it (rejecting the un-substituted placeholder and logging warnings when missing/placeholder so production misconfig is visible). `App.tsx` wraps the tree in `<CSPProvider nonce>` (Base UI) + `<MotionConfig nonce>` (Framer Motion). `security-headers.conf` replaces `style-src 'self' 'unsafe-inline'` with CSP Level 3 split directives: `style-src-elem 'self' 'nonce-$csp_nonce'` (locks down `<style>` elements) + `style-src-attr 'unsafe-inline'` (required by Floating UI positioning, not a practical XSS vector: `style` attributes cannot execute JavaScript). 3. **Personality trimming WebSocket notification** (folded in from #1064): - New `engine.personality_trimming_notify` setting (BOOLEAN, default `true`). - New `WsEventType.PERSONALITY_TRIMMED` enum member. - New `personality_trim_notifier` callback parameter on `AgentEngine`, invoked best-effort from `_prepare_context` (re-raises `MemoryError` / `RecursionError` / `asyncio.CancelledError`; logs `prompt.personality.notify_failed` and swallows everything else). - New `synthorg.api.app.make_personality_trim_notifier` factory that returns a ready-to-wire async callback bound to the live `ChannelsPlugin`. External engine runners (CLI workers, K8s jobs) call this factory and pass the result into `AgentEngine`. - Frontend: `personality.trimmed` added to `WsEventType` + `WS_EVENT_TYPE_VALUES`. New `useGlobalNotifications` hook mounted in `AppLayout` subscribes to the `agents` channel globally; `agents` store dispatches an info toast with `agent_name` (length-bounded) + `before to after` tokens when the event arrives, and logs a warning on malformed payloads. 4. **Org chart page full rebuild.** - New synthetic Owner node above the root department, accepting an array-shaped payload for future multi-user ownership (#1082). Fixed `w-[240px]` / `h-[90px]` so layout centering matches rendered size. - Restructured hierarchy: Executive is the root department box with the CEO / CTO living inside it as regular agents (not standalone top-level nodes); all other departments hang below via a single cross-dept edge row. Matches real-world org charts and avoids the "edge cuts through the executive box" problem because inter-dept lines now originate from the root dept box's bottom edge. - Dagre is used for ordering only; a post-layout shift pass enforces an exact 48px gap between the owner row, the root department, and the non-root department row (the prior `minlen` approach was quantized into 50px rank jumps). - Custom `<HierarchyEdge>` SVG L-path replaces React Flow's `getSmoothStepPath`, which had a Z-shape fanout limitation that silently ignored `centerY` on cross-x edges. L-path routes cleanly with a single vertical bend. - Live particle flow: new `useLiveEdgeActivity` hook subscribes to the messages store and pulses particles along edges that carry traffic in the last 3s. Three modes via segmented control: Flow (always on), Live (traffic-driven, default), Off. SVG `animateMotion` with uniform Manhattan-distance speed so particles don't visually "slow down" on longer edges. - Department group node: dashed empty-state border with inline "+ Add agent" CTA, status dots (active / idle / error / offline) with tooltip legend, budget utilization bar, LEAD badge on department leads. Header height is computed from live toggle state so agent cards never overlap chrome. - Inline toolbar toggles replace the previous popover view menu: particle-flow segmented control + per-feature icon toggles for add-agent CTA, lead badges, budget bars, status dots, minimap. Status-dots toggle has a multi-line tooltip explaining the color legend. - Search overlay (Ctrl+F) with match dimming and keyboard nav across matches. - Collapse / expand per department with chevron. - Minimap togglable, default off, custom node colors; no labels (unreadable at minimap scale). - New persistence store `useOrgChartPrefs` (`web/src/stores/org-chart-prefs.ts`): `particleFlowMode: 'live'`, add-agent + lead badge + budget bar visible, status dots + minimap hidden. 5. **Agent routing by id, not name.** `/agents/:agentName` switched to `/agents/:agentId`. Names can contain arbitrary characters that broke backend lookup after URL encoding. `AgentDetailPage` resolves the agent from `useCompanyStore` by id (falling back to name for legacy config) and passes the resolved name to the data hook. Navigation sites updated: `AgentGridView`, `OrgChartPage`, `ProjectTeamSection`. 6. **Dynamic department filter.** Department dropdown on the agents page now reads from `useCompanyStore().config.departments` instead of the hardcoded `DEPARTMENT_NAME_VALUES` enum, so it matches the user-configured department list (wizard, packs, manual edits). Uses `display_name` when provided, falls back to `formatLabel(name)`. 7. **Coming-soon gates on missing backend CRUD.** New shared constant in `web/src/pages/org-edit/coming-soon.ts` points at issue #1081. Nine mutation paths in the org-edit dialogs / drawers surface a banner explaining the limitation instead of hitting 405 responses. 8. **Analytics "active agents" runtime semantics.** `_resolve_agent_counts` in `analytics.py` now computes "active" as agents currently executing a task (task status `IN_PROGRESS`, `assigned_to` in the employed set), not "agents with employment status ACTIVE". The previous semantics conflated HR lifecycle with runtime state and produced the surprising "4 active / 0 idle / 0 tasks" display on the overview dashboard. `_assemble_overview` now passes `all_tasks` into the helper so the computation reuses the already-fetched task list. ## Reviewer attention items 1. **Combobox criterion (issue §4a)**: the issue text says "provider model search (currently a filtered list in `ProviderFormModal`)" should use Combobox. That filtered list does not exist in the current repo: the only filter in `ProviderFormModal.tsx` is a static 3-item auth-type filter, not a typeahead. The PR documents the keep-decision in `web/CLAUDE.md` -> Base UI Adoption Decisions ("Combobox, Autocomplete: Not adopted: no current typeahead call sites in the dashboard that would benefit. Re-evaluate when filterable selects become a feature requirement."). 2. **Bundle size direction**: the issue anticipated a net reduction; the actual delta is a 3% increase (`vendor-ui` 150 kB to 230 kB, total dist 3.4M to 3.5M). The tradeoff is Base UI's more comprehensive primitive surface (Floating UI positioning, focus management, transition tracking) versus the narrow subset shadcn previously pulled from Radix. First-class CSP nonce support, broader component coverage, and active upstream maintenance justify the delta. 3. **Personality trimming end-to-end wiring**: `AgentEngine` is not constructed anywhere under `src/` today (only in tests): it is library code that external runners construct. This PR provides the `make_personality_trim_notifier` factory so external runners can wire the callback in one line, but the in-tree API layer does not itself construct engines. The frontend toast path is fully wired and will render when any engine host emits the event. Closing the in-tree wiring gap is a separate architectural decision, out of scope. 4. **`style-src-attr 'unsafe-inline'`**: retained for Floating UI's transient inline positioning styles (`style="position: fixed; top: ...; left: ..."`). `style` attributes cannot execute JavaScript and cannot carry CSP nonces per the CSS spec. This is the narrowest permissive the CSP spec allows. Documented in `docs/security.md` -> CSP Nonce Infrastructure. ## Verification All gates green: - `uv run ruff check src/ tests/` -- clean - `uv run ruff format --check src/ tests/` -- 1541 files already formatted - `uv run mypy src/ tests/` -- clean (1541 source files) - `uv run python -m pytest tests/ -n 8 -k analytics` -- 59 passed - `npm --prefix web run lint` -- zero warnings - `npm --prefix web run type-check` -- clean - `npm --prefix web run test` -- 2406 passed (203 files) Zero Radix references remain in tracked files. ## Non-goals - Migrating to a different primitive system (staying on shadcn/ui + Base UI). - Wiring the personality-trim notifier into a new in-tree engine construction site (separate scope). - Adopting Toast / Drawer / Meter / Select from Base UI (decisions documented in `web/CLAUDE.md`). - Backend CRUD endpoints for department edits (tracked in #1081) and multi-user ownership permissions (tracked in #1082). The frontend is ready for both; the owner node's array-shaped payload and the coming-soon gates are the hooks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1070. Subsumes #1064.
Summary
Three bundled changes that cut across the same files:
Radix UI → Base UI migration.
radix-ui+cmdkare fully removed (115 transitive packages);@base-ui/react@1.3.0+cmdk-base@1.0.0are installed (8 packages).components.jsonswitches fromradix-novatobase-vega. 14 component files migrated:button.tsx(via a local<Slot>helper over@base-ui/react/merge-props),dialog.tsx,confirm-dialog.tsx,theme-toggle.tsx,command-palette.tsx(nowcmdk-basehosted inside Base UI Dialog for focus trap + escape + portal), six page-level Dialog / AlertDialog files,OrgEditPage.tsxTabs, and both Workflow Menu files. Data attributes rewritten (data-[state=open]→data-[open],data-[state=active]→data-[active], plusdata-[starting-style]/data-[ending-style]for transition lifecycle).CSP nonce activation, end-to-end.
nginx.confgenerates a per-request nonce from$request_idand substitutes it into__CSP_NONCE__viasub_filter.index.htmlexposes the nonce on<meta name="csp-nonce">.web/src/lib/csp.tsreads it (rejecting the un-substituted placeholder and logging warnings when missing/placeholder so production misconfig is visible).App.tsxwraps the tree in<CSPProvider nonce>(Base UI) +<MotionConfig nonce>(Framer Motion).security-headers.confreplacesstyle-src 'self' 'unsafe-inline'with CSP Level 3 split directives:style-src-elem 'self' 'nonce-$csp_nonce'(locks down<style>elements) +style-src-attr 'unsafe-inline'(required by Floating UI positioning, not a practical XSS vector —styleattributes cannot execute JavaScript).Personality trimming WebSocket notification (folded in from feat: WebSocket notification when personality trimming activates #1064):
engine.personality_trimming_notifysetting (BOOLEAN, defaulttrue).WsEventType.PERSONALITY_TRIMMEDenum member.personality_trim_notifiercallback parameter onAgentEngine, invoked best-effort from_prepare_context(re-raisesMemoryError/RecursionError/asyncio.CancelledError; logsprompt.personality.notify_failedand swallows everything else).synthorg.api.app.make_personality_trim_notifierfactory that returns a ready-to-wire async callback bound to the liveChannelsPlugin. External engine runners (CLI workers, K8s jobs) call this factory and pass the result intoAgentEngine.personality.trimmedadded toWsEventType+WS_EVENT_TYPE_VALUES. NewuseGlobalNotificationshook mounted inAppLayoutsubscribes to theagentschannel globally;agentsstore dispatches an info toast withagent_name(length-bounded) +before → aftertokens when the event arrives, and logs a warning on malformed payloads.Pre-PR review
Ran a swarm of 10 specialist agents in parallel. Every valid finding was implemented:
csp.tsdocstring; obsolete CSP indocs/guides/deployment.mdtranslatetransition fix;.get()→ direct subscript; removed unnecessary noqa; added missing test coveragep-6→p-cardon 5 dialogs + 3gap-4→gap-grid-gap; event-name dot-hierarchy rename; expanded notifier log context; Slot composeRefs + dev warning; historical language cleanup; exact-pincmdk-base; new test file foruseGlobalNotificationsmergeProps<'div'>generic; no-opbackdrop-filterin transition;CancelledErrorpropagation testTwo commits on the branch:
feat: migrate web dashboard from Radix UI to Base UI + activate CSP nonce + personality trimming WS notification— the migration itself (46 files, +1153/−3279).refactor: address pre-PR agent findings for Radix to Base UI migration— all 28 review findings applied (24 files, +466/−64).Verification
All gates green on both commits:
uv run ruff check src/ tests/— cleanuv run mypy src/ tests/— clean (1533 source files)uv run python -m pytest tests/ -n 8 -m "unit or integration"— 14213 passed, 8 skipped (unit + integration)npm --prefix web run lint— zero warningsnpm --prefix web run type-check— cleannpm --prefix web run test— 2419 passed (+6 new: personality-trimmed toast handler, notifierconfig_resolver=Nonebranch,CancelledErrorpropagation,useGlobalNotificationsbindings / dispatch / error toasts)npm --prefix web run storybook:build— builds cleanlyZero Radix references remain in tracked files (
git grep "Radix\|@radix-ui\|radix-ui"→ empty).Bundle size delta
vendor-ui: 150 kB → 230 kB (+80 kB / +28 kB gzip)The growth is Base UI's more comprehensive primitive surface (Floating UI positioning, focus management, transition tracking) versus the subset of Radix primitives shadcn previously pulled in. The tradeoff is first-class CSP nonce support, broader component coverage, and active upstream maintenance. Trade is acceptable given the security win.
Reviewer attention items
Combobox criterion (issue §4a): the issue text says "provider model search (currently a filtered list in
ProviderFormModal)" should use Combobox. That filtered list does not exist in the current repo — the only filter inProviderFormModal.tsxis a static 3-item auth-type filter, not a typeahead. The PR documents the keep-decision inweb/CLAUDE.md→ Base UI Adoption Decisions ("Combobox, Autocomplete: Not adopted — no current typeahead call sites in the dashboard that would benefit. Re-evaluate when filterable selects become a feature requirement.").Bundle size direction: the issue anticipated a net reduction; the actual delta is a 3% increase. The tradeoff is documented above and in the commit message.
Personality trimming end-to-end wiring:
AgentEngineis not constructed anywhere undersrc/today (only in tests) — it is library code that external runners construct. This PR provides themake_personality_trim_notifierfactory so external runners can wire the callback in one line, but the in-tree API layer does not itself construct engines. The frontend toast path is fully wired and will render when any engine host emits the event. Closing the in-tree wiring gap is a separate architectural decision, out of scope.style-src-attr 'unsafe-inline': retained for Floating UI's transient inline positioning styles (style="position: fixed; top: ...; left: ...").styleattributes cannot execute JavaScript and cannot carry CSP nonces per the CSS spec. This is the narrowest permissive the CSP spec allows. Documented indocs/security.md→ CSP Nonce Infrastructure.Non-goals
web/CLAUDE.md).Pre-reviewed by 10 agents, 28 findings addressed.