feat: web dashboard pages — views, components, tests, and review fixes#353
Closed
Aureliolo wants to merge 10 commits into
Closed
feat: web dashboard pages — views, components, tests, and review fixes#353Aureliolo wants to merge 10 commits into
Aureliolo wants to merge 10 commits into
Conversation
Cherry-picked from feat/web-dashboard-pages onto current main. Adds 11 page views (Dashboard, TaskBoard, ApprovalQueue, AgentProfiles, AgentDetail, BudgetPanel, MessageFeed, OrgChart, Settings, MeetingLogs, ArtifactBrowser) and 25 feature components. Removes PlaceholderHome. Fixes companyStore.error references to use configError/departmentsError.
…aceholders - Add canWrite RBAC guard to ApprovalActions (security: prevent read-only users from invoking approve/reject mutations) - Replace console.error in DashboardPage with user-facing toast warning - Use DEFAULT_PAGE_SIZE constant in TaskListView instead of hardcoded 50 - Fix AgentDetailPage retry to call fetchAgentData() instead of full reload - Add aria-describedby on form inputs in LoginPage, SetupPage, SettingsPage - Add GitHub issue links to MeetingLogs (#264) and ArtifactBrowser (#233) placeholder pages - Remove stale "PR 2" comment from NAV_ITEMS constant
…ponents 30 new test files covering all page views (11) and feature components (19): - Agent, Approval, Budget, Dashboard, Message, OrgChart, Task components - All page-level views including placeholder pages - 387 total tests (up from 175), all passing with lint + type-check clean
…ling, immutability Pre-reviewed by 6 agents, 40 findings addressed: - Type safety: replace string props with union types (StatusBadge, OrgNode, ApprovalActions, KanbanBoard, TaskFilters, TaskDetailPanel, TaskCreateDialog) - Derive theme types from api/types.ts instead of duplicating - Accessibility: add for/id label pairs, aria-required, RouterLink for nav - Error handling: try/catch WebSocket setup, sanitizeForLog, try/finally - Immutability: fix parameter mutation in formatUptime, spread in tasksByStatus - Security: WS message size check, route param validation, auth error on invalid expiresIn - RBAC: implement mustChangePassword guard, read-only created_by from auth - Rename total_cost → total_cost_usd for consistency - Polish: extract nested ternary to computed (OrgNode), move imports to top - Docs: update CLAUDE.md package structure, operations.md status note - Tests: add LoginPage, SetupPage, TaskCreateDialog, TaskDetailPanel tests; fix existing test assertions for RouterLink and type changes
…docs - Fix O(n²) tasksByStatus computed property (use push instead of spread) - Sanitize WebSocket error logs to prevent JWT leakage (all page views) - Wrap unhandled Promise.all in try/catch across 6 page views - Fix ErrorBoundary retry to preserve active task filters - Remove unused getErrorMessage import from TaskBoardPage - Add accessible label/id to reject-reason textarea (ApprovalActions) - Fix TaskFilters emit type to Partial<TaskFilterType> - Increase WS_MAX_MESSAGE_SIZE from 4KB to 128KB - Add tests: useLoginLockout, useAuth, sanitizeForLog, mustChangePassword guard - Update operations.md to reflect placeholder status of Meeting Logs, Artifact Browser, and Settings capabilities - Fix Docker web build EACCES — chown /app to build user before npm ci Reviewed by: Copilot, Greptile, Gemini, code-reviewer, pr-test-analyzer, silent-failure-hunter, docs-consistency, issue-resolution-verifier
- Fix SpendingSummary 24h window bug (slice(-24) dropped newest buckets) - Add immediate:true to TaskDetailPanel watch (init edit fields on mount) - Add accessible label for cancel-reason textarea (TaskDetailPanel) - Add accessible label for reject-reason textarea (ApprovalActions) - Fix MessageFeedPage retry to preserve active channel filter - Fix SystemStatus to show "Unknown" instead of "Down" when health null - Add aria-hidden to decorative MetricCard icon - Add ARIA role/aria-pressed to TaskBoardPage view toggle buttons - Sanitize WS error logs in TaskBoardPage (prevent JWT leak) Reviewed by: CodeRabbit (26 comments), Greptile (2 comments)
…es, DAST perms - auth.ts: validate expiresIn for NaN/non-finite values - AgentDetailPage: watch route param to refetch on navigation - TaskDetailPanel: add accessible labels for edit-mode form controls - SpendingChart: use UTC date functions for consistent timezone grouping - MessageList: compute near-bottom state before DOM update for auto-scroll - ApprovalQueuePage: add loading state to approve/reject, log catch blocks, add aria-label to status filter dropdown - ApprovalActions: accept loading prop, disable buttons during action - TaskCreateDialog: add aria-required to required form fields - DashboardPage.test: fix mock to match HealthStatus type shape - RecentApprovals.test: remove unused pushMock variable - dast.yml: add issues:write permission for ZAP scan results
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive web dashboard with multiple page views and feature components, backed by thorough test coverage and incorporating several rounds of review fixes. The changes enhance the user interface, improve security and accessibility, and ensure robust error handling and type safety. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Greptile SummaryThis PR adds 10 dashboard page views, 20+ feature components, and 452 tests across 55 files, completing the web frontend for the SynthOrg dashboard. It also incorporates four rounds of review fixes covering security (JWT sanitization in logs), accessibility, performance, and error handling. The implementation is well-structured and consistent across most pages, but one critical logic bug and two style issues were found. Key findings:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Router
participant Page as Page (e.g. DashboardPage)
participant WsStore as WebSocketStore
participant Backend as Backend WS
User->>Router: navigate to /
Router->>Page: mount DashboardPage
Page->>WsStore: connect(token) [only if !connected]
WsStore->>Backend: open WebSocket
Backend-->>WsStore: connection established
Page->>WsStore: subscribe(['tasks','budget','approvals'])
Page->>WsStore: onChannelEvent('tasks', handler)
Page->>WsStore: onChannelEvent('budget', handler)
Page->>WsStore: onChannelEvent('approvals', handler)
Note over User,Backend: User navigates away then back
User->>Router: navigate to /tasks → then back to /
Router->>Page: mount DashboardPage (WS already connected)
Page->>WsStore: if (!connected) → FALSE, skip all subscribe/onChannelEvent
Note over Page,WsStore: ⚠️ Bug: no subscriptions registered, live updates broken
Backend-->>WsStore: push tasks/budget/approvals events
WsStore->>Page: no handlers registered → events dropped
Prompt To Fix All With AIThis is a comment left during a code review.
Path: web/src/views/DashboardPage.vue
Line: 36-42
Comment:
**Real-time updates silently disabled when WS is already connected**
`wsStore.subscribe` and all `wsStore.onChannelEvent` calls are nested inside the `if (authStore.token && !wsStore.connected)` guard. If the user navigates to the Dashboard after visiting any other page (Tasks, Budget, Approvals, etc.) the WebSocket is already connected, the condition is `false`, and **none** of the three channel subscriptions or event handlers are ever registered. The Dashboard will display stale data and receive no live updates for the rest of the session.
Every other page in this PR (e.g. `ApprovalQueuePage`, `TaskBoardPage`, `BudgetPanelPage`) correctly keeps only the `wsStore.connect` call inside the guard and places `subscribe`/`onChannelEvent` unconditionally after it:
```suggestion
try {
if (authStore.token && !wsStore.connected) {
wsStore.connect(authStore.token)
}
wsStore.subscribe(['tasks', 'budget', 'approvals'])
wsStore.onChannelEvent('tasks', taskStore.handleWsEvent)
wsStore.onChannelEvent('budget', budgetStore.handleWsEvent)
wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent)
} catch (err) {
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: web/src/views/SettingsPage.vue
Line: 36-43
Comment:
**`retryFetch` missing `catch` — unhandled promise rejection on store errors**
`retryFetch` uses `try/finally` but has no `catch`. If either `companyStore.fetchConfig()` or `providerStore.fetchProviders()` throws (e.g. a network error that bypasses the store's internal error state), the rejection propagates as an unhandled promise rejection — no error is logged, and the `ErrorBoundary` only shows an error if the store sets its own error state.
Every other `retryFetch` in this PR (`BudgetPanelPage`, `OrgChartPage`) includes a `catch` block. For consistency with the CLAUDE.md "never silently swallow" rule:
```suggestion
async function retryFetch() {
loading.value = true
try {
await Promise.all([companyStore.fetchConfig(), providerStore.fetchProviders()])
} catch (err) {
console.error('Settings data fetch failed:', sanitizeForLog(err))
} finally {
loading.value = false
}
}
```
Note: this also requires adding `import { sanitizeForLog } from '@/utils/logging'` to the imports at the top of the file.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: web/src/components/tasks/KanbanColumn.vue
Line: 17-23
Comment:
**Reliance on undocumented `_underlying_vm_` private property**
`event.item._underlying_vm_` is an internal Vue instance property that `vue-draggable-plus` does not officially expose. The comment already acknowledges this risk. If the library bumps its Vue integration (which has happened in the `vue-draggable-plus` changelog), this property can disappear or be renamed without a semver break, causing `task` to be `undefined` on every drag — silently breaking the entire kanban board without a runtime error.
A more resilient alternative is to read the task data from the model array using the drag event's index, which is part of the public `SortableEvent` API:
```typescript
function handleAdd(event: { newIndex?: number; item: HTMLElement }) {
// newIndex is the position of the dropped item in this column's task list
if (event.newIndex !== undefined) {
const task = props.tasks[event.newIndex]
if (task) emit('task-added', task)
}
}
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 5b7f7ec |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the main Web UI dashboard experience (views + feature components) and wires it into the router/stores, with broad Vitest coverage and several review-driven hardening changes (accessibility, logging sanitization, perf tweaks).
Changes:
- Added multiple dashboard page views (Dashboard, Tasks, Messages, Approvals, Agents, Org Chart, Budget, Settings) plus placeholder pages.
- Introduced new feature components for tasks/approvals/budget/dashboard/org-chart/messages, and updated shared UI utilities (theme/status badges/formatting/constants).
- Expanded/updated tests across views, components, composables, router guards, and utilities.
Reviewed changes
Copilot reviewed 92 out of 93 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/views/TaskBoardPage.vue | New tasks board/list page with WS subscription, filtering, detail/create dialogs |
| web/src/views/SetupPage.vue | Adds aria-describedby wiring for setup form errors |
| web/src/views/SettingsPage.vue | New settings page (company/providers + password change form) |
| web/src/views/PlaceholderHome.vue | Removes placeholder home view |
| web/src/views/OrgChartPage.vue | New org chart view using VueFlow + stores |
| web/src/views/MessageFeedPage.vue | New message feed view with channel selector and WS updates |
| web/src/views/MeetingLogsPage.vue | New placeholder meeting logs page linking to GitHub issue |
| web/src/views/LoginPage.vue | Adds aria-describedby wiring for login form errors |
| web/src/views/DashboardPage.vue | New dashboard overview page with widgets + WS hookup |
| web/src/views/BudgetPanelPage.vue | New budget monitoring page with charts/tables + WS updates |
| web/src/views/ArtifactBrowserPage.vue | New placeholder artifact browser page linking to GitHub issue |
| web/src/views/ApprovalQueuePage.vue | New approval queue page with filtering + detail sidebar/actions |
| web/src/views/AgentProfilesPage.vue | New agents grid page with WS updates |
| web/src/views/AgentDetailPage.vue | New agent detail page with param validation + refetch-on-route-change |
| web/src/utils/format.ts | Refactors uptime formatting to avoid mutating parameters |
| web/src/utils/constants.ts | Increases WS max message size; trims NAV_ITEMS comment |
| web/src/styles/theme.ts | Aligns Status/Priority/Risk types to API types and expands status color mapping |
| web/src/stores/websocket.ts | Adds max message size guard and improves error logging on send failures |
| web/src/stores/tasks.ts | Types tasksByStatus by TaskStatus and uses push-based grouping |
| web/src/stores/auth.ts | Validates expiresIn with Number.isFinite and throws on invalid session duration |
| web/src/router/index.ts | Registers all new page routes (lazy-loaded) and replaces placeholder home |
| web/src/router/guards.ts | Enforces must-change-password redirect to settings |
| web/src/composables/usePolling.ts | Sanitizes logged polling errors |
| web/src/components/tasks/TaskListView.vue | New table-based tasks list component |
| web/src/components/tasks/TaskFilters.vue | New filters component for status/assignee with reset |
| web/src/components/tasks/TaskDetailPanel.vue | New task detail sidebar with edit/transition/cancel actions |
| web/src/components/tasks/TaskCreateDialog.vue | New create-task dialog with validation and form reset-on-open |
| web/src/components/tasks/TaskCard.vue | New accessible clickable task card |
| web/src/components/tasks/KanbanColumn.vue | New draggable kanban column emitting task-added events |
| web/src/components/tasks/KanbanBoard.vue | New kanban board rendering columns in status order |
| web/src/components/org-chart/OrgNode.vue | New org chart node renderer with styling by node type |
| web/src/components/messages/MessageList.vue | New message list with auto-scroll-to-bottom behavior |
| web/src/components/messages/MessageItem.vue | New message item renderer |
| web/src/components/messages/ChannelSelector.vue | New channel dropdown selector |
| web/src/components/dashboard/SystemStatus.vue | New system status widget (health + WS state + uptime/version) |
| web/src/components/dashboard/SpendingSummary.vue | New spending widget with recent hourly chart option |
| web/src/components/dashboard/RecentApprovals.vue | New “recent approvals” widget with link to approvals |
| web/src/components/dashboard/MetricCard.vue | New metric card component |
| web/src/components/dashboard/ActiveTasksSummary.vue | New “active tasks” widget with link to tasks |
| web/src/components/common/StatusBadge.vue | Tightens value typing to API-derived status/priority/risk unions |
| web/src/components/budget/SpendingChart.vue | New daily spending bar chart component (UTC grouping) |
| web/src/components/budget/BudgetConfigDisplay.vue | New budget config summary display |
| web/src/components/budget/AgentSpendingTable.vue | New per-agent cost aggregation table |
| web/src/components/approvals/ApprovalDetail.vue | New approval detail display component |
| web/src/components/approvals/ApprovalCard.vue | New accessible approval card component |
| web/src/components/approvals/ApprovalActions.vue | New approve/reject UI with confirm + loading/disable states |
| web/src/components/agents/AgentMetrics.vue | New agent metrics/details component |
| web/src/components/agents/AgentCard.vue | New accessible agent card component |
| web/src/tests/views/TaskBoardPage.test.ts | Adds TaskBoardPage unit tests |
| web/src/tests/views/SetupPage.test.ts | Adds SetupPage unit tests |
| web/src/tests/views/SettingsPage.test.ts | Adds SettingsPage unit tests |
| web/src/tests/views/OrgChartPage.test.ts | Adds OrgChartPage unit tests |
| web/src/tests/views/MessageFeedPage.test.ts | Adds MessageFeedPage unit tests |
| web/src/tests/views/MeetingLogsPage.test.ts | Adds MeetingLogsPage unit tests |
| web/src/tests/views/LoginPage.test.ts | Adds LoginPage unit tests |
| web/src/tests/views/DashboardPage.test.ts | Adds DashboardPage unit tests |
| web/src/tests/views/BudgetPanelPage.test.ts | Adds BudgetPanelPage unit tests |
| web/src/tests/views/ArtifactBrowserPage.test.ts | Adds ArtifactBrowserPage unit tests |
| web/src/tests/views/ApprovalQueuePage.test.ts | Adds ApprovalQueuePage unit tests |
| web/src/tests/views/AgentProfilesPage.test.ts | Adds AgentProfilesPage unit tests |
| web/src/tests/views/AgentDetailPage.test.ts | Adds AgentDetailPage unit tests |
| web/src/tests/utils/sanitizeForLog.test.ts | Adds sanitizeForLog tests |
| web/src/tests/router/guards.test.ts | Adds must-change-password redirect tests |
| web/src/tests/composables/usePolling.test.ts | Updates polling error logging expectation to sanitized string |
| web/src/tests/composables/useLoginLockout.test.ts | Adds login lockout composable tests |
| web/src/tests/composables/useAuth.test.ts | Adds useAuth composable tests |
| web/src/tests/components/TaskListView.test.ts | Adds TaskListView tests |
| web/src/tests/components/TaskFilters.test.ts | Adds TaskFilters tests |
| web/src/tests/components/TaskCreateDialog.test.ts | Adds TaskCreateDialog tests |
| web/src/tests/components/TaskCard.test.ts | Adds TaskCard tests |
| web/src/tests/components/SystemStatus.test.ts | Adds SystemStatus tests |
| web/src/tests/components/StatusBadge.test.ts | Adjusts StatusBadge unknown-value test for new typing |
| web/src/tests/components/SpendingSummary.test.ts | Adds SpendingSummary tests |
| web/src/tests/components/SpendingChart.test.ts | Adds SpendingChart tests |
| web/src/tests/components/RecentApprovals.test.ts | Adds RecentApprovals tests |
| web/src/tests/components/OrgNode.test.ts | Adds OrgNode tests |
| web/src/tests/components/MetricCard.test.ts | Adds MetricCard tests |
| web/src/tests/components/MessageList.test.ts | Adds MessageList tests |
| web/src/tests/components/MessageItem.test.ts | Adds MessageItem tests |
| web/src/tests/components/ChannelSelector.test.ts | Adds ChannelSelector tests |
| web/src/tests/components/BudgetConfigDisplay.test.ts | Adds BudgetConfigDisplay tests |
| web/src/tests/components/ApprovalDetail.test.ts | Adds ApprovalDetail tests |
| web/src/tests/components/ApprovalCard.test.ts | Adds ApprovalCard tests |
| web/src/tests/components/ApprovalActions.test.ts | Adds ApprovalActions tests |
| web/src/tests/components/AgentSpendingTable.test.ts | Adds AgentSpendingTable tests |
| web/src/tests/components/AgentMetrics.test.ts | Adds AgentMetrics tests |
| web/src/tests/components/AgentCard.test.ts | Adds AgentCard tests |
| web/src/tests/components/ActiveTasksSummary.test.ts | Adds ActiveTasksSummary tests |
| web/package-lock.json | Updates lockfile (adds vite ^6 entry) |
| docs/design/operations.md | Updates Web UI feature status section text |
| docker/web/Dockerfile | Ensures /app exists and is owned by non-root build user |
| CLAUDE.md | Updates repo layout documentation for new web views/components/composables |
Files not reviewed (1)
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+77
to
+81
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('tasks', taskStore.handleWsEvent) | ||
| wsStore.offChannelEvent('budget', budgetStore.handleWsEvent) | ||
| wsStore.offChannelEvent('approvals', approvalStore.handleWsEvent) | ||
| }) |
Comment on lines
+69
to
+72
| if (typeof event.data === 'string' && event.data.length > WS_MAX_MESSAGE_SIZE) { | ||
| console.error('WebSocket message exceeds max size, discarding') | ||
| return | ||
| } |
Comment on lines
+58
to
+60
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('tasks', taskStore.handleWsEvent) | ||
| }) |
Comment on lines
+37
to
+39
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('messages', messageStore.handleWsEvent) | ||
| }) |
Comment on lines
+37
to
+39
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('budget', budgetStore.handleWsEvent) | ||
| }) |
Comment on lines
+55
to
+57
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('approvals', approvalStore.handleWsEvent) | ||
| }) |
Comment on lines
+38
to
+40
| onUnmounted(() => { | ||
| wsStore.offChannelEvent('agents', agentStore.handleWsEvent) | ||
| }) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This is a massive and impressive pull request that adds a significant amount of functionality to the web dashboard. The new pages, components, and tests are well-structured and demonstrate a high level of quality. I'm particularly impressed with the robust error handling, security considerations (like sanitizing logs and validating inputs), and the thoughtful UX improvements (like auto-scrolling and Promise.allSettled for dashboard loading). The code is clean, well-typed, and follows modern Vue 3 practices. I only found one medium-severity maintainability concern regarding the use of an internal library property in the Kanban board implementation. Overall, this is an excellent contribution.
Comment on lines
+17
to
+22
| function handleAdd(event: { item: HTMLElement & { _underlying_vm_?: Task } }) { | ||
| const task = event.item?._underlying_vm_ | ||
| if (task) { | ||
| emit('task-added', task) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The handleAdd function relies on event.item._underlying_vm_ to get the task data. This accesses an internal, undocumented property of the vue-draggable-plus library, which makes the implementation fragile and prone to breaking on library updates. While this pattern might be common in examples, relying on private APIs is a maintainability risk.
I'd suggest investigating if vue-draggable-plus or the underlying SortableJS provide a more stable, public API to retrieve the data of the moved item from the event object. For example, some libraries might pass the data item in a custom event detail or allow retrieval via event.oldIndex from the source list. If a public API is not available, this is an acceptable risk, but it would be good to add a comment in the code acknowledging this dependency on an internal implementation detail.
Owner
Author
|
@coderabbitai review |
…hardening - Add wsStore.unsubscribe() on unmount for all 6 page views (subscription leak) - Fix SpendingSummary UTC date methods to match SpendingChart - Log errors in TaskBoardPage filter catch blocks (no silent swallows) - Pass ?tab=user query param on mustChangePassword redirect - Read tab query param in SettingsPage to open correct tab - Sanitize socket.onerror — stop logging raw event that exposes JWT - Add fragile-API comment on _underlying_vm_ in KanbanColumn - Add issues:write permission to DAST workflow (ZAP 403 fix) - Add PR trigger to Docker workflow for build validation on branches - Gate Docker push/sign/login behind event_name != pull_request - Ensure non-root user in all backend Dockerfile stages
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a full-feature frontend: 13+ new Vue pages, many new components (agents, approvals, budget, dashboard, messages, org-chart, tasks), ~40+ unit tests, router and store updates, WebSocket integration and message-size guard, non-root Docker builds, CI workflow permission/gating tweaks, logging sanitization, and several utility/type adjustments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant DashboardPage as Dashboard Page
participant WebSocket as WebSocket
participant API as REST API
participant Stores as Pinia Stores
participant Components as UI Components
User->>Browser: Navigate to dashboard
Browser->>DashboardPage: mount
par Fetch initial data
DashboardPage->>API: GET /health
DashboardPage->>API: GET /analytics
DashboardPage->>API: GET /tasks?limit=5
DashboardPage->>API: GET /budgets/config
DashboardPage->>API: GET /budgets/records
DashboardPage->>API: GET /approvals?limit=5
and Connect WS
DashboardPage->>WebSocket: connect(token)
WebSocket-->>DashboardPage: connected
DashboardPage->>WebSocket: subscribe channels (health, analytics, tasks, budget, approvals)
end
API-->>Stores: populate stores
WebSocket-->>Stores: channel events
Stores-->>Components: reactive data
Components-->>Browser: render widgets
Browser-->>User: show dashboard
sequenceDiagram
actor User
participant TaskBoard as Task Board Page
participant KanbanBoard as Kanban Board
participant KanbanColumn as Kanban Column
participant DragDropLib as Drag-Drop Lib
participant API as Task API
participant Stores as Task Store
User->>TaskBoard: open Kanban view
TaskBoard->>Stores: fetch tasks
API-->>Stores: return tasks
Stores->>TaskBoard: tasksByStatus
TaskBoard->>KanbanBoard: pass tasksByStatus
KanbanBoard->>KanbanColumn: render columns
KanbanColumn->>DragDropLib: bind v-model
User->>KanbanColumn: drag task
DragDropLib->>KanbanColumn: add event
KanbanColumn->>TaskBoard: emit task-moved (task, targetStatus)
TaskBoard->>Stores: updateTask (status)
Stores->>API: PATCH /tasks/{id}
API-->>Stores: updated task
Stores-->>KanbanBoard: updated tasksByStatus
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #353 +/- ##
=======================================
Coverage 93.90% 93.90%
=======================================
Files 447 447
Lines 20803 20803
Branches 2010 2010
=======================================
Hits 19535 19535
Misses 981 981
Partials 287 287 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…lint - Tighten mustChangePassword guard to check tab=user query param - Add try/catch + error toast to all task mutation handlers - Log errors in BudgetPanel and OrgChart retryFetch catch blocks - Remove unused WS channel subscriptions from Dashboard - Fix sandbox Dockerfile: non-root user in all stages, hadolint DL3008 ignore
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/views/SetupPage.vue (1)
68-96:⚠️ Potential issue | 🟡 MinorDon't attach password-only errors to the username field.
errorcan hold"Passwords do not match"or the minimum-length message, so Line 68 makes the username input announce an unrelated description. Either keep the alert form-level only, or bindaria-describedbyper field so only the affected password controls referencesetup-error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/views/SetupPage.vue` around lines 68 - 96, Remove the username input's aria-describedby binding and instead attach the alert only to the password controls: create a computed (e.g. passwordError) that returns the current error only when it relates to passwords (e.g. contains "Password" / "match" or the MIN_PASSWORD_LENGTH message), then change the two InputText elements with id="password" and id="confirm" to use :aria-describedby="passwordError ? 'setup-error' : undefined" and remove the same binding from the username InputText; keep the alert div as-is (id="setup-error") so only password-related errors are announced for those fields.web/src/stores/websocket.ts (1)
74-78:⚠️ Potential issue | 🟠 MajorParse-error logging path still bypasses
sanitizeForLog.Line 77 logs
parseErrdirectly, which is inconsistent with the sanitized error logging strategy used elsewhere in this store.Suggested fix
try { data = JSON.parse(event.data) } catch (parseErr) { - console.error('Failed to parse WebSocket message:', parseErr) + console.error('Failed to parse WebSocket message:', sanitizeForLog(parseErr)) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/websocket.ts` around lines 74 - 78, The parse-error catch in the WebSocket message handler logs parseErr directly; change that log to pass the error through the existing sanitizeForLog helper (e.g., use sanitizeForLog(parseErr)) so the message follows the same sanitized logging strategy used elsewhere in this store (refer to sanitizeForLog and the parseErr variable in the try/catch inside the onmessage handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/components/ActiveTasksSummary.test.ts`:
- Around line 8-11: The test claims to verify navigation but only checks the
href; update the test so clicking the mocked RouterLink asserts navigation:
modify the vi.mock('vue-router') RouterLink to invoke pushMock on click (e.g.,
template or click handler that calls pushMock(to)), then in the test find the
link, call link.trigger('click') and replace the href-only assertion with
expect(pushMock).toHaveBeenCalledWith('/tasks'); reference pushMock, RouterLink,
and the existing link.attributes('href') check to locate and change the
assertions.
In `@web/src/__tests__/components/AgentCard.test.ts`:
- Around line 14-29: The test mock's personality object currently sets
collaboration to an invalid value using a type assertion ("collaboration:
'cooperative' as never"); replace that with a valid CollaborationPreference
literal ('independent', 'pair', or 'team')—e.g., change the collaboration
property in the personality object within AgentCard.test.ts to 'team' to satisfy
TypeScript and remove the unsafe as never cast.
In `@web/src/__tests__/components/AgentMetrics.test.ts`:
- Line 18: The test fixture in AgentMetrics.test.ts is suppressing a type error
by using the cast "collaboration: 'cooperative' as never"; remove the "as never"
cast and replace the invalid string with a valid CollaborationPreference value
(for example set collaboration: 'team') so the fixture conforms to the
CollaborationPreference type and surfaces real type issues during compilation.
In `@web/src/__tests__/components/MessageList.test.ts`:
- Around line 67-72: The test "renders empty state when no messages" only checks
MessageItemStub count; update it to also assert that the MessageList.vue
empty-state marker or copy is rendered when mountList([]) is used — locate the
empty-state element (e.g. by the component's empty-state class or aria role or
the specific no-messages copy) using wrapper.find(...) or wrapper.text() and add
expectations that the element exists and contains the expected text. Keep the
test name and use the existing mountList and MessageItemStub references so the
test fails if the empty-state UI is removed or altered.
In `@web/src/__tests__/components/MetricCard.test.ts`:
- Around line 21-27: The test 'renders icon' for MetricCard currently only
checks that an <i> element exists; update it to assert the icon class is applied
by checking the found element's class list includes the expected 'pi pi-users'
(use the same props passed into mount for clarity). Locate the test in
MetricCard.test.ts (the it('renders icon') block), get the icon via
wrapper.find('i') and replace or extend the expect to verify icon.classes() or
icon.attributes('class') contains 'pi' and 'pi-users' so the prop-to-rendering
mapping is validated.
In `@web/src/__tests__/components/OrgNode.test.ts`:
- Around line 95-103: The test "does not show department type badge for
non-department nodes" currently only asserts 'Department' is absent, which
misses a regression where a 'Team' badge could appear; update the test
(OrgNode.test.ts) to assert that no type-badge span is rendered for
non-department nodes by checking that there are zero spans matching the
component's type-badge output (e.g., no span whose text equals the formatted
label for the node type or no element with the type-badge class) when mounting
OrgNode with props { data: { label: 'Backend Team', type: 'team' } } so the test
fails if any badge for non-department types is rendered.
In `@web/src/__tests__/composables/useLoginLockout.test.ts`:
- Around line 37-43: The test name is misleading: it says "network/5xx" but only
triggers generic network errors; rename the test case in the file's spec for
useLoginLockout (the it(...) block that calls recordFailure(new Error('Network
error')) and asserts locked.value) to "does not lock on network errors" so the
description matches the exercised scenario and test output is precise.
- Around line 53-60: The test hardcodes the expected number of failures (5)
instead of using the production threshold; update the test to import or read the
same LOGIN_MAX_ATTEMPTS constant used by the composable and use it to drive the
loop and assertions (e.g., call useLoginLockout(), loop LOGIN_MAX_ATTEMPTS - 1
times asserting no lock, then call recordFailure once more and assert the "Too
many failed attempts" message and locked.value === true); apply the same change
to the other affected assertions around lines 92-93 so tests always reflect the
composable's actual threshold.
- Around line 13-14: The current isAxiosError mock in useLoginLockout.test.ts is
too permissive (it returns true for any object with an isAxiosError key, even
false); update the mock so it only returns true when the property is strictly
true — e.g. change the predicate in the isAxiosError implementation to check
that err is an object/non-null and that (err as any).isAxiosError === true (or
typeof (err as any).isAxiosError === 'boolean' && (err as any).isAxiosError ===
true) so credential-error classification tests don't get false positives.
In `@web/src/__tests__/views/AgentProfilesPage.test.ts`:
- Around line 90-94: The test "shows loading skeleton when store is loading and
no agents" is non-diagnostic because expect(wrapper.text()).toBeTruthy() can
pass for unrelated text; replace it with an assertion that specifically checks
for the loading UI. Mount AgentProfilesPage with the store state set to loading:
true and agents: [] (or mock fetchAgents to leave loading true), then assert the
loading skeleton element exists (e.g. wrapper.find('.loading-skeleton').exists()
or wrapper.findComponent(LoadingSkeleton).exists()) and that agent list elements
are absent; use flushPromises or nextTick if you need to wait for reactivity.
In `@web/src/__tests__/views/LoginPage.test.ts`:
- Around line 59-63: The test currently mocks the entire 'vue' module to replace
onUnmounted which is brittle; instead refactor useLoginLockout to accept an
injectable cleanup hook or timer service (e.g., a cleanupCallback or
timerFactory parameter) and update LoginPage.test.ts to provide a test double
for that injected dependency rather than mocking vue; locate the useLoginLockout
function and its consumers (e.g., LoginPage component) and add an optional
parameter or composable option to supply the cleanup/timer, then in tests inject
a stubbed cleanup function or fake timer service instead of mocking onUnmounted.
In `@web/src/components/approvals/ApprovalDetail.vue`:
- Around line 39-42: The template calls formatDate(approval.expires_at) but
approval.expires_at can be null; update the ApprovalDetail.vue rendering to
guard or handle nulls like the decided_at field does—e.g., check
approval.expires_at before calling formatDate (or ensure formatDate itself
accepts null and returns an empty/placeholder string); modify the code around
formatDate and approval.expires_at to conditionally render a placeholder when
null so no runtime error occurs.
In `@web/src/components/budget/SpendingChart.vue`:
- Around line 22-29: The tooltip formatter currently types params as Array<{
name: string; value: number }>, which can be a single object or an array
depending on ECharts usage; update the formatter signature on the tooltip object
(tooltip.formatter) to use the official ECharts types (e.g.,
TooltipComponentFormatter or TooltipFormatterParams) or a union like
Record<string, any> | Array<Record<string, any>> and normalize inside the
function (coerce to an array, check length, then access params[0].name and
params[0].value safely) while keeping trigger: 'axis' handling intact.
In `@web/src/components/dashboard/RecentApprovals.vue`:
- Around line 20-21: The empty-state copy in RecentApprovals.vue is misleading
for non-pending items; update the template string in the div with
v-if="approvals.length === 0" (inside the RecentApprovals component) to a
status-neutral message such as "No recent approvals" so the displayed text
aligns with the prop contract that the list may include approved or rejected
items.
In `@web/src/components/dashboard/SpendingSummary.vue`:
- Line 42: The y-axis formatter (formatter: (v: number) => `$${v.toFixed(2)}`)
and the header display (totalCost.toFixed(4)) use different decimal precision;
pick a consistent precision and update both places accordingly—either change the
formatter to use toFixed(4) or change totalCost.toFixed(4) to toFixed(2); locate
and update the formatter function and the totalCost usage in SpendingSummary.vue
to the chosen toFixed(n) so the header and chart axis match.
In `@web/src/components/messages/MessageList.vue`:
- Around line 27-34: The message list container in MessageList.vue (the div with
ref="listRef" that renders MessageItem for each msg from messages) is missing
ARIA attributes for screen readers; update that div to include role="log" and
aria-live="polite" so assistive technologies announce new messages appropriately
while preserving the existing ref and v-for usage.
In `@web/src/components/tasks/KanbanColumn.vue`:
- Around line 17-24: The handleAdd function currently reads the dragged Task
from the undocumented event.item._underlying_vm_; replace this with the
documented API by reading the dragged item from event.data instead. Update the
function signature/type to expect event.data (e.g., event: { data?: Task }) and
emit('task-added', event.data) only when event.data is present, ensuring you
remove any reliance on _underlying_vm_ in handleAdd.
In `@web/src/components/tasks/TaskCreateDialog.vue`:
- Around line 113-145: The Dropdown components (task-type, task-priority,
task-assignee, task-complexity) are using the id prop which doesn't associate
labels with the underlying input in PrimeVue v3; change those Dropdown props
from id="..." to inputId="..." so their <label for="..."> elements correctly
reference the inner input. Do not change the InputText (task-project) id prop
unless it is a composite InputNumber — only replace id with inputId on the
Dropdown components (symbols: Dropdown with v-model="type", v-model="priority",
v-model="assignedTo", v-model="complexity").
- Line 31: The budgetLimit ref can be null from PrimeVue InputNumber; before
building/emitting the payload (where budgetLimit is read and sent, e.g. in the
submit/emit handler and any places around the component methods that reference
budgetLimit) add defensive validation that coerces null to a numeric value (for
example treat null as 0 or omit the field as your API requires) and ensure the
emitted object uses a Number: const safeBudget = budgetLimit.value ?? 0; use
Number(safeBudget) (or delete payload.budget_limit if you prefer to omit) so
budget_limit is never null when calling the API or emitting events. Ensure the
same check is applied wherever budgetLimit is accessed (the handlers referenced
around lines 74-90 and line 149) to prevent sending budget_limit: null.
In `@web/src/components/tasks/TaskDetailPanel.vue`:
- Around line 61-69: saveEdit currently emits raw inputs allowing
whitespace-only titles/descriptions to be saved; update saveEdit to trim
editTitle.value and editDescription.value, validate that at least one of the
trimmed values (or whichever fields are required by your existing predicate) is
non-empty before calling emit('save', ...), and return early without closing
editing.value if validation fails; also mirror this same trimmed non-empty
predicate on the Save button logic referenced around lines 118-119 so the button
is disabled for whitespace-only inputs. Use the symbols saveEdit, editTitle,
editDescription, editPriority, editing, and the existing emit('save', ...) call
to locate where to add the trim/validation and where to update the Save button
predicate.
- Around line 41-54: The reset logic currently only watches props.task, so if
the panel is closed and reopened with the same task object the draft state
(editing, showCancel, editTitle, editDescription, editPriority, cancelReason)
persists; update the watcher to also react to the panel open/visible prop (e.g.,
watch [() => props.task, () => props.isOpen] or add a separate watch on
props.isOpen/visible) and run the same reset block when the panel becomes open
(isOpen/visible === true) so editing, showCancel and draft fields are cleared on
reopen even if props.task is unchanged.
In `@web/src/components/tasks/TaskListView.vue`:
- Around line 22-33: The DataTable is used in server-side pagination mode
(providing tasks, total and emitting page) but missing the lazy prop, causing
client-side pagination of the tasks array; update the DataTable component usage
to include the lazy prop so PrimeVue treats pagination as lazy/server-side
(i.e., add lazy to the <DataTable ...> attributes alongside :value="tasks",
:total-records="total", :rows="DEFAULT_PAGE_SIZE" and the existing `@page` and
`@row-click` handlers).
In `@web/src/stores/tasks.ts`:
- Around line 23-31: The computed tasksByStatus reducer currently creates
grouped with a plain object literal which allows prototype-key injection via
dynamic keys (task.status); change grouped creation in the tasksByStatus
computed to use a null-prototype object (e.g. via Object.create(null)) so
dynamic keys cannot affect Object.prototype, keep the rest of the loop intact
and ensure the grouped variable's TypeScript type still matches
Partial<Record<TaskStatus, Task[]>> (use an appropriate cast if needed) when
returning grouped.
In `@web/src/stores/websocket.ts`:
- Around line 68-72: socket.onmessage currently checks only event.data.length
(UTF-16 code units) and ignores binary types; change the guard to measure byte
length across types: for strings use TextEncoder().encode(event.data).length,
for ArrayBuffer/SharedArrayBuffer/TypedArray use .byteLength, and for Blob use
its .size (await if needed when not immediately available). Use
WS_MAX_MESSAGE_SIZE as the byte-limit constant and bail out/log when the
computed byte length exceeds it; keep the check inside the socket.onmessage
handler and ensure you handle all event.data variants.
In `@web/src/views/ApprovalQueuePage.vue`:
- Around line 67-79: The handlers handleApprove and handleReject currently only
handle resolved results and do not catch promise rejections from
approvalStore.approve / approvalStore.reject, so rejections bubble up and no
toast is shown; add a catch block around the await call (or wrap the try with a
try/catch/finally) to catch errors, call toast.add with an error severity
(include approvalStore.error or the caught error message), ensure
actionLoading.value is set false in finally, and keep the existing
success/failed-result logic (update selected.value on success) so both rejected
and resolved-but-unsuccessful paths show an error toast and do not leak
exceptions.
- Around line 29-30: The status filter can be set to null by the Dropdown's
show-clear and is currently forwarded raw, causing ?status=null in API calls;
update the filter normalization in ApprovalQueuePage.vue (functions
filterByStatus and the other similar handlers around the 97-103 and 110-119
blocks) to convert null to undefined or omit the status key before calling
fetchApprovals() — e.g., derive a normalizedStatus from statusFilter.value
(treat null as undefined) and pass either { status: normalizedStatus } only when
defined or build the params object without the status property when
normalizedStatus is undefined so the API doesn't receive "null".
In `@web/src/views/ArtifactBrowserPage.vue`:
- Around line 8-25: The ArtifactBrowserPage.vue currently renders only a
placeholder EmptyState (using AppShell, PageHeader, EmptyState) and does not
implement the /artifacts route functionality; replace the placeholder with a
real artifact browsing UI by creating or using an ArtifactList component and
implementing data fetching in the ArtifactBrowserPage setup() (or mounted) to
call the backend artifacts API, handle loading/error states, paginate/search as
needed, and render items with links to artifact detail pages; ensure the
PageHeader remains, remove the Coming Soon block, and wire the route so
/artifacts displays the fetched list and proper error messages (reference
ArtifactBrowserPage.vue, EmptyState, PageHeader, and a new ArtifactList
component or fetchArtifacts method).
In `@web/src/views/DashboardPage.vue`:
- Around line 38-41: The 'system' channel is subscribed but no handler updates
health/value so SystemStatus becomes stale; either remove 'system' from
wsStore.subscribe or add a channel handler for 'system' that updates the
reactive health state (the same state set in getHealth()) so subsequent WS
events refresh SystemStatus—locate wsStore.subscribe and add
wsStore.onChannelEvent('system', ...) to call the existing health updater (or
update health.value directly) or simply drop 'system' from the subscribe list.
- Around line 35-42: DashboardPage.vue calls wsStore.connect(authStore.token)
without the same !wsStore.connected guard used elsewhere; update the logic to
check wsStore.connected before calling wsStore.connect so connection attempts
are consistent and idempotent with other views. Specifically, wrap the existing
connect/subscribe/onChannelEvent calls (wsStore.connect, wsStore.subscribe,
wsStore.onChannelEvent('tasks' -> taskStore.handleWsEvent, 'budget' ->
budgetStore.handleWsEvent, 'approvals' -> approvalStore.handleWsEvent)) in a
conditional that only invokes wsStore.connect(authStore.token) when
!wsStore.connected, preserving the subsequent subscription and event handler
registration behavior.
In `@web/src/views/OrgChartPage.vue`:
- Around line 41-55: The current ID generation (deptId, teamId) concatenates raw
names with '-' causing collisions (e.g., dept="ops-platform", team="api" vs
dept="ops", team="platform-api"); create and use a helper like
generateSafeId(prefix, ...parts) that joins parts using a delimiter-safe
encoding (e.g., encodeURIComponent or base64) or a separator that is escaped,
replace all usages that build IDs (deptId, teamId and the edge id/target/source
construction referenced around the edge generation functions) to call this
helper, and update any code that decodes IDs (currently slicing strings) to
parse by splitting on the known prefix or by decoding only the last segment
(e.g., decode the last id segment) instead of naive string slicing so VueFlow
IDs remain unique and stable.
In `@web/src/views/SettingsPage.vue`:
- Around line 29-35: The current code initializes activeTab once from
route.query.tab (using VALID_TABS and tabParam) and uses the deprecated
TabView/TabPanel; migrate to PrimeVue v4's Tabs by replacing TabView/TabPanel
with the Tabs component bound to a string-backed reactive ref (activeTab) and
wire two-way sync with the router: keep VALID_TABS for validation, ensure
activeTab is a ref validated against VALID_TABS, add a watch on route.query.tab
to update activeTab when the URL changes, and add a watch on activeTab to
push/replace the updated tab value into the route query (so both URL->state and
state->URL stay in sync); update any template bindings from the old value prop
to the new v-model/value binding expected by Tabs and remove usages tied to the
deprecated TabView API.
In `@web/src/views/TaskBoardPage.vue`:
- Around line 48-52: The retry path only re-fetches tasks but not agents,
leaving agentStore.error unresolved; update the retry logic in TaskBoardPage.vue
(the retry handler near the initial Promise.all block and the other retry at
line ~172) to call and await both taskStore.fetchTasks({ limit: 200 }) and
agentStore.fetchAgents() together (e.g., use Promise.all([...]) or await both
calls) so the retry rehydrates both dependencies (refer to fetchTasks and
fetchAgents in the retry handlers).
- Around line 68-109: The handlers handleTransition, handleSave, handleCancel,
and handleCreate currently only check falsy returns and will leave rejected
promises unhandled; wrap the await calls to taskStore.transitionTask,
taskStore.updateTask, taskStore.cancelTask, and taskStore.createTask in
try/catch (or use await ... .catch) so rejected promises are caught, call
toast.add with an error severity in the catch block using taskStore.error or the
caught error.message as the summary, and preserve the existing success branches
(e.g., setting selectedTask.value or createVisible.value) inside the try when
result is truthy.
- Around line 145-159: Replace the two custom toggle buttons with PrimeVue's
SelectButton: import and register SelectButton (e.g., import SelectButton from
'primevue/selectbutton' and add it to components), create an options array like
[{ label: 'Board', value: 'kanban' }, { label: 'List', value: 'list' }], and
bind the component to the existing viewMode via v-model (e.g.,
v-model="viewMode" :options="viewOptions"); remove the manual :class,
:aria-pressed and `@click` handlers on the old <button>s so the SelectButton
manages state, styling and accessibility for the viewMode toggle (references:
viewMode, viewOptions, SelectButton, TaskBoardPage.vue).
---
Outside diff comments:
In `@web/src/stores/websocket.ts`:
- Around line 74-78: The parse-error catch in the WebSocket message handler logs
parseErr directly; change that log to pass the error through the existing
sanitizeForLog helper (e.g., use sanitizeForLog(parseErr)) so the message
follows the same sanitized logging strategy used elsewhere in this store (refer
to sanitizeForLog and the parseErr variable in the try/catch inside the
onmessage handler).
In `@web/src/views/SetupPage.vue`:
- Around line 68-96: Remove the username input's aria-describedby binding and
instead attach the alert only to the password controls: create a computed (e.g.
passwordError) that returns the current error only when it relates to passwords
(e.g. contains "Password" / "match" or the MIN_PASSWORD_LENGTH message), then
change the two InputText elements with id="password" and id="confirm" to use
:aria-describedby="passwordError ? 'setup-error' : undefined" and remove the
same binding from the username InputText; keep the alert div as-is
(id="setup-error") so only password-related errors are announced for those
fields.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b042ded-e5ac-40ae-9bc3-d0f619ae3b92
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (95)
.github/workflows/dast.yml.github/workflows/docker.ymlCLAUDE.mddocker/backend/Dockerfiledocker/web/Dockerfiledocs/design/operations.mdweb/src/__tests__/components/ActiveTasksSummary.test.tsweb/src/__tests__/components/AgentCard.test.tsweb/src/__tests__/components/AgentMetrics.test.tsweb/src/__tests__/components/AgentSpendingTable.test.tsweb/src/__tests__/components/ApprovalActions.test.tsweb/src/__tests__/components/ApprovalCard.test.tsweb/src/__tests__/components/ApprovalDetail.test.tsweb/src/__tests__/components/BudgetConfigDisplay.test.tsweb/src/__tests__/components/ChannelSelector.test.tsweb/src/__tests__/components/MessageItem.test.tsweb/src/__tests__/components/MessageList.test.tsweb/src/__tests__/components/MetricCard.test.tsweb/src/__tests__/components/OrgNode.test.tsweb/src/__tests__/components/RecentApprovals.test.tsweb/src/__tests__/components/SpendingChart.test.tsweb/src/__tests__/components/SpendingSummary.test.tsweb/src/__tests__/components/StatusBadge.test.tsweb/src/__tests__/components/SystemStatus.test.tsweb/src/__tests__/components/TaskCard.test.tsweb/src/__tests__/components/TaskCreateDialog.test.tsweb/src/__tests__/components/TaskDetailPanel.test.tsweb/src/__tests__/components/TaskFilters.test.tsweb/src/__tests__/components/TaskListView.test.tsweb/src/__tests__/composables/useAuth.test.tsweb/src/__tests__/composables/useLoginLockout.test.tsweb/src/__tests__/composables/usePolling.test.tsweb/src/__tests__/router/guards.test.tsweb/src/__tests__/utils/sanitizeForLog.test.tsweb/src/__tests__/views/AgentDetailPage.test.tsweb/src/__tests__/views/AgentProfilesPage.test.tsweb/src/__tests__/views/ApprovalQueuePage.test.tsweb/src/__tests__/views/ArtifactBrowserPage.test.tsweb/src/__tests__/views/BudgetPanelPage.test.tsweb/src/__tests__/views/DashboardPage.test.tsweb/src/__tests__/views/LoginPage.test.tsweb/src/__tests__/views/MeetingLogsPage.test.tsweb/src/__tests__/views/MessageFeedPage.test.tsweb/src/__tests__/views/OrgChartPage.test.tsweb/src/__tests__/views/SettingsPage.test.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/__tests__/views/TaskBoardPage.test.tsweb/src/components/agents/AgentCard.vueweb/src/components/agents/AgentMetrics.vueweb/src/components/approvals/ApprovalActions.vueweb/src/components/approvals/ApprovalCard.vueweb/src/components/approvals/ApprovalDetail.vueweb/src/components/budget/AgentSpendingTable.vueweb/src/components/budget/BudgetConfigDisplay.vueweb/src/components/budget/SpendingChart.vueweb/src/components/common/StatusBadge.vueweb/src/components/dashboard/ActiveTasksSummary.vueweb/src/components/dashboard/MetricCard.vueweb/src/components/dashboard/RecentApprovals.vueweb/src/components/dashboard/SpendingSummary.vueweb/src/components/dashboard/SystemStatus.vueweb/src/components/messages/ChannelSelector.vueweb/src/components/messages/MessageItem.vueweb/src/components/messages/MessageList.vueweb/src/components/org-chart/OrgNode.vueweb/src/components/tasks/KanbanBoard.vueweb/src/components/tasks/KanbanColumn.vueweb/src/components/tasks/TaskCard.vueweb/src/components/tasks/TaskCreateDialog.vueweb/src/components/tasks/TaskDetailPanel.vueweb/src/components/tasks/TaskFilters.vueweb/src/components/tasks/TaskListView.vueweb/src/composables/usePolling.tsweb/src/router/guards.tsweb/src/router/index.tsweb/src/stores/auth.tsweb/src/stores/tasks.tsweb/src/stores/websocket.tsweb/src/styles/theme.tsweb/src/utils/constants.tsweb/src/utils/format.tsweb/src/views/AgentDetailPage.vueweb/src/views/AgentProfilesPage.vueweb/src/views/ApprovalQueuePage.vueweb/src/views/ArtifactBrowserPage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/DashboardPage.vueweb/src/views/LoginPage.vueweb/src/views/MeetingLogsPage.vueweb/src/views/MessageFeedPage.vueweb/src/views/OrgChartPage.vueweb/src/views/PlaceholderHome.vueweb/src/views/SettingsPage.vueweb/src/views/SetupPage.vueweb/src/views/TaskBoardPage.vue
💤 Files with no reviewable changes (1)
- web/src/views/PlaceholderHome.vue
Comment on lines
+8
to
+11
| vi.mock('vue-router', () => ({ | ||
| useRouter: () => ({ push: pushMock }), | ||
| RouterLink: { props: ['to'], template: '<a :href="to"><slot /></a>' }, | ||
| })) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ActiveTasksSummary.test.ts" -type fRepository: Aureliolo/synthorg
Length of output: 119
🏁 Script executed:
cat -n ./web/src/__tests__/components/ActiveTasksSummary.test.tsRepository: Aureliolo/synthorg
Length of output: 4332
Test name doesn't match implementation: assertion only checks href attribute, not navigation.
The test "navigates to /tasks when "View all" is clicked" clears pushMock at line 91 but never triggers a click or verifies the mock was called. The current test only asserts link.attributes('href').toBe('/tasks'), which doesn't exercise Vue Router. Either rename the test to reflect that it only validates the href value, or implement click-triggered navigation in the RouterLink mock and update the assertion to expect(pushMock).toHaveBeenCalledWith('/tasks').
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/components/ActiveTasksSummary.test.ts` around lines 8 - 11,
The test claims to verify navigation but only checks the href; update the test
so clicking the mocked RouterLink asserts navigation: modify the
vi.mock('vue-router') RouterLink to invoke pushMock on click (e.g., template or
click handler that calls pushMock(to)), then in the test find the link, call
link.trigger('click') and replace the href-only assertion with
expect(pushMock).toHaveBeenCalledWith('/tasks'); reference pushMock, RouterLink,
and the existing link.attributes('href') check to locate and change the
assertions.
Comment on lines
+67
to
+72
| it('renders empty state when no messages', () => { | ||
| const wrapper = mountList([]) | ||
|
|
||
| const items = wrapper.findAllComponents(MessageItemStub) | ||
| expect(items).toHaveLength(0) | ||
| }) |
Contributor
There was a problem hiding this comment.
The empty-state test doesn't verify the empty state.
This only proves that no MessageItem stubs rendered. If MessageList.vue regresses to rendering nothing at all, the test still passes. Please assert on the empty-state marker/copy the component is supposed to show when messages is empty.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/components/MessageList.test.ts` around lines 67 - 72, The
test "renders empty state when no messages" only checks MessageItemStub count;
update it to also assert that the MessageList.vue empty-state marker or copy is
rendered when mountList([]) is used — locate the empty-state element (e.g. by
the component's empty-state class or aria role or the specific no-messages copy)
using wrapper.find(...) or wrapper.text() and add expectations that the element
exists and contains the expected text. Keep the test name and use the existing
mountList and MessageItemStub references so the test fails if the empty-state UI
is removed or altered.
Comment on lines
+21
to
+27
| it('renders icon', () => { | ||
| const wrapper = mount(MetricCard, { | ||
| props: { title: 'Agents', value: '5', icon: 'pi pi-users' }, | ||
| }) | ||
| const icon = wrapper.find('i') | ||
| expect(icon.exists()).toBe(true) | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider asserting the icon class for stronger verification.
The test verifies the icon element exists but doesn't confirm it has the expected classes. This could pass even if the icon prop isn't correctly applied.
♻️ Proposed improvement
it('renders icon', () => {
const wrapper = mount(MetricCard, {
props: { title: 'Agents', value: '5', icon: 'pi pi-users' },
})
const icon = wrapper.find('i')
expect(icon.exists()).toBe(true)
+ expect(icon.classes()).toContain('pi-users')
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('renders icon', () => { | |
| const wrapper = mount(MetricCard, { | |
| props: { title: 'Agents', value: '5', icon: 'pi pi-users' }, | |
| }) | |
| const icon = wrapper.find('i') | |
| expect(icon.exists()).toBe(true) | |
| }) | |
| it('renders icon', () => { | |
| const wrapper = mount(MetricCard, { | |
| props: { title: 'Agents', value: '5', icon: 'pi pi-users' }, | |
| }) | |
| const icon = wrapper.find('i') | |
| expect(icon.exists()).toBe(true) | |
| expect(icon.classes()).toContain('pi-users') | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/components/MetricCard.test.ts` around lines 21 - 27, The
test 'renders icon' for MetricCard currently only checks that an <i> element
exists; update it to assert the icon class is applied by checking the found
element's class list includes the expected 'pi pi-users' (use the same props
passed into mount for clarity). Locate the test in MetricCard.test.ts (the
it('renders icon') block), get the icon via wrapper.find('i') and replace or
extend the expect to verify icon.classes() or icon.attributes('class') contains
'pi' and 'pi-users' so the prop-to-rendering mapping is validated.
Comment on lines
+41
to
+55
| const deptId = `dept-${dept.name}` | ||
| result.push({ | ||
| id: deptId, | ||
| position: { x: 0, y }, | ||
| data: { label: formatLabel(dept.name), type: 'department' }, | ||
| type: 'orgNode', | ||
| }) | ||
| y += 120 | ||
|
|
||
| for (const team of dept.teams) { | ||
| const teamId = `team-${dept.name}-${team.name}` | ||
| result.push({ | ||
| id: teamId, | ||
| position: { x: 50, y }, | ||
| data: { label: team.name, type: 'team' }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "OrgChartPage.vue" | head -5Repository: Aureliolo/synthorg
Length of output: 94
🏁 Script executed:
cat -n ./web/src/views/OrgChartPage.vueRepository: Aureliolo/synthorg
Length of output: 5125
Make the flow IDs collision-safe.
These IDs are built by joining raw names with - delimiters. Inputs like dept="ops-platform", team="api" and dept="ops", team="platform-api" both produce team-ops-platform-api, breaking VueFlow's uniqueness requirement, edge targeting, and routing.
💡 Safe pattern
+function flowId(...parts: string[]) {
+ return parts.map(encodeURIComponent).join('::')
+}
+
const nodes = computed<Node[]>(() => {
const result: Node[] = []
let y = 0
const agentIndex = new Map(agentStore.agents.map((a) => [a.name, a]))
for (const dept of companyStore.departments) {
- const deptId = `dept-${dept.name}`
+ const deptId = flowId('dept', dept.name)
result.push({
id: deptId,
position: { x: 0, y },
data: { label: formatLabel(dept.name), type: 'department' },
type: 'orgNode',
})
@@
- const teamId = `team-${dept.name}-${team.name}`
+ const teamId = flowId('team', dept.name, team.name)
@@
- id: `agent-${memberName}`,
+ id: flowId('agent', memberName),Apply the same helper to all ID generation (edges at lines 90, 97) and decode from the last id segment if needed instead of string slicing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/OrgChartPage.vue` around lines 41 - 55, The current ID
generation (deptId, teamId) concatenates raw names with '-' causing collisions
(e.g., dept="ops-platform", team="api" vs dept="ops", team="platform-api");
create and use a helper like generateSafeId(prefix, ...parts) that joins parts
using a delimiter-safe encoding (e.g., encodeURIComponent or base64) or a
separator that is escaped, replace all usages that build IDs (deptId, teamId and
the edge id/target/source construction referenced around the edge generation
functions) to call this helper, and update any code that decodes IDs (currently
slicing strings) to parse by splitting on the known prefix or by decoding only
the last segment (e.g., decode the last id segment) instead of naive string
slicing so VueFlow IDs remain unique and stable.
Comment on lines
+29
to
+35
| const VALID_TABS = ['company', 'providers', 'user'] as const | ||
| const tabParam = String(route.query.tab ?? 'company') | ||
| const activeTab = ref(VALID_TABS.includes(tabParam as typeof VALID_TABS[number]) ? tabParam : 'company') | ||
|
|
||
| const providerEntries = computed(() => | ||
| Object.entries(providerStore.providers).map(([name, config]) => ({ name, config })), | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check PrimeVue version and current tab API usage in this file.
jq -r '.dependencies.primevue // .devDependencies.primevue // empty' web/package.json
rg -n --type=vue "TabView|TabPanel|activeIndex|:value=" web/src/views/SettingsPage.vueRepository: Aureliolo/synthorg
Length of output: 109
🌐 Web query:
For the PrimeVue version declared in web/package.json, what are the supported props for TabView and TabPanel? Specifically: is string-based valuesupported on TabView/TabPanel, or shouldactiveIndex be used?
💡 Result:
PrimeVue TabView / TabPanel (as implemented in PrimeVue v3.x)
From the PrimeVue type definitions:
TabView supported props include (not exhaustive of pass-through related types):
activeIndex?: numberlazy?: booleanscrollable?: booleantabindex?: numberselectOnFocus?: booleanpreviousButtonProps?,nextButtonProps?,prevIcon?,nextIcon?,pt?,ptOptions?,unstyled?
There is novalueprop onTabView. [1]
TabPanel supported props include:
header?: stringheaderStyle?,headerClass?,headerProps?,headerActionProps?contentStyle?,contentClass?,contentProps?disabled?: booleanpt?
There is novalueprop onTabPanel. [2]
Conclusion: string-based value is not supported on TabView/TabPanel; you should use activeIndex (typically via v-model:activeIndex). [1][2]
PrimeVue v4+ note
PrimeVue v4 deprecates TabView in favor of the newer Tabs components. The Tabs/Tab/TabPanel setup supports value (often used as a string) for selection. [3][4]
So:
- If your
web/package.jsonhas PrimeVue v3.x → useactiveIndexonTabView. - If you want string-based
valueselection → migrate to PrimeVue Tabs (v4+). [3][4]
Sources:
[1] PrimeVue TabView.d.ts (v3.41.1) on unpkg.
[2] PrimeVue TabPanel.d.ts (v3.30.1) on unpkg.
[3] PrimeVue v4 migration guide (TabView → Tabs).
[4] PrimeVue Tabs documentation showing value usage on Tab/TabPanel.
🏁 Script executed:
#!/bin/bash
# Check actual component usage in the file (without --type=vue)
rg -n "TabView|Tabs|TabPanel|activeIndex" web/src/views/SettingsPage.vueRepository: Aureliolo/synthorg
Length of output: 432
🏁 Script executed:
#!/bin/bash
# Also check the specific lines mentioned (29-35 and 81-120)
echo "=== Lines 29-35 ==="
sed -n '29,35p' web/src/views/SettingsPage.vue
echo ""
echo "=== Lines 81-120 ==="
sed -n '81,120p' web/src/views/SettingsPage.vueRepository: Aureliolo/synthorg
Length of output: 2153
TabView is deprecated in PrimeVue v4; migrate to Tabs component with reactive query sync.
The code uses deprecated TabView with value binding, which is unsupported in both PrimeVue v3 and v4. Additionally, activeTab is initialized once from route.query.tab (line 31), so route changes won't sync.
For PrimeVue v4, replace TabView/TabPanel with the new Tabs component, which supports string-based value and is reactive:
🔧 Suggested fix
+import Tabs from 'primevue/tabs'
+import TabList from 'primevue/tablist'
+import TabPanels from 'primevue/tabpanels'
+import Tab from 'primevue/tab'
+import TabPanel from 'primevue/tabpanel'
-import TabView from 'primevue/tabview'
-import TabPanel from 'primevue/tabpanel' const VALID_TABS = ['company', 'providers', 'user'] as const
-const tabParam = String(route.query.tab ?? 'company')
-const activeTab = ref(VALID_TABS.includes(tabParam as typeof VALID_TABS[number]) ? tabParam : 'company')
+type SettingsTab = (typeof VALID_TABS)[number]
+const activeTab = computed<SettingsTab>(() => {
+ const q = String(route.query.tab ?? 'company')
+ return VALID_TABS.includes(q as SettingsTab) ? (q as SettingsTab) : 'company'
+})- <TabView v-else :value="activeTab">
+ <Tabs v-else :value="activeTab">
+ <TabList>
- <TabPanel header="Company" value="company">
+ <Tab value="company">Company</Tab>
+ <Tab value="providers">Providers</Tab>
+ <Tab value="user">User</Tab>
+ </TabList>
+ <TabPanels>
+ <TabPanel>
<!-- Company content -->
</TabPanel>
- <TabPanel header="Providers" value="providers">
+ <TabPanel>
<!-- Providers content -->
</TabPanel>
- <TabPanel header="User" value="user">
+ <TabPanel>
<!-- User content -->
</TabPanel>
+ </TabPanels>
- </TabView>
+ </Tabs>Also applies to: 81–120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/SettingsPage.vue` around lines 29 - 35, The current code
initializes activeTab once from route.query.tab (using VALID_TABS and tabParam)
and uses the deprecated TabView/TabPanel; migrate to PrimeVue v4's Tabs by
replacing TabView/TabPanel with the Tabs component bound to a string-backed
reactive ref (activeTab) and wire two-way sync with the router: keep VALID_TABS
for validation, ensure activeTab is a ref validated against VALID_TABS, add a
watch on route.query.tab to update activeTab when the URL changes, and add a
watch on activeTab to push/replace the updated tab value into the route query
(so both URL->state and state->URL stay in sync); update any template bindings
from the old value prop to the new v-model/value binding expected by Tabs and
remove usages tied to the deprecated TabView API.
Comment on lines
+48
to
+52
| try { | ||
| await Promise.all([ | ||
| taskStore.fetchTasks({ limit: 200 }), | ||
| agentStore.fetchAgents(), | ||
| ]) |
Contributor
There was a problem hiding this comment.
Retry needs to refetch agents too.
Initial load depends on both fetchTasks() and fetchAgents(), and the error boundary also surfaces agentStore.error. The current retry only repairs half of that dependency chain.
💡 Minimal fix
+async function retryFetch() {
+ await Promise.all([
+ taskStore.fetchTasks(filters.value),
+ agentStore.fetchAgents(),
+ ])
+}- <ErrorBoundary :error="taskStore.error ?? agentStore.error" `@retry`="() => taskStore.fetchTasks(filters)">
+ <ErrorBoundary :error="taskStore.error ?? agentStore.error" `@retry`="retryFetch">Also applies to: 172-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/TaskBoardPage.vue` around lines 48 - 52, The retry path only
re-fetches tasks but not agents, leaving agentStore.error unresolved; update the
retry logic in TaskBoardPage.vue (the retry handler near the initial Promise.all
block and the other retry at line ~172) to call and await both
taskStore.fetchTasks({ limit: 200 }) and agentStore.fetchAgents() together
(e.g., use Promise.all([...]) or await both calls) so the retry rehydrates both
dependencies (refer to fetchTasks and fetchAgents in the retry handlers).
Comment on lines
+145
to
+159
| <div class="flex rounded-lg border border-slate-700" role="group" aria-label="View mode"> | ||
| <button | ||
| :class="['px-3 py-1.5 text-xs', viewMode === 'kanban' ? 'bg-brand-600 text-white' : 'text-slate-400']" | ||
| :aria-pressed="viewMode === 'kanban'" | ||
| @click="viewMode = 'kanban'" | ||
| > | ||
| Board | ||
| </button> | ||
| <button | ||
| :class="['px-3 py-1.5 text-xs', viewMode === 'list' ? 'bg-brand-600 text-white' : 'text-slate-400']" | ||
| :aria-pressed="viewMode === 'list'" | ||
| @click="viewMode = 'list'" | ||
| > | ||
| List | ||
| </button> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if PrimeVue is in project dependencies
cat package.json web/package.json 2>/dev/null | grep -A 20 '"dependencies"'Repository: Aureliolo/synthorg
Length of output: 592
🏁 Script executed:
# Check the actual TaskBoardPage.vue file to understand context
wc -l web/src/views/TaskBoardPage.vueRepository: Aureliolo/synthorg
Length of output: 97
🏁 Script executed:
# Search for PrimeVue component usage patterns in the codebase
rg "SelectButton|primevue" web/src --type vue -B 2 -A 2Repository: Aureliolo/synthorg
Length of output: 89
🏁 Script executed:
# Read TaskBoardPage.vue to understand the full context
cat -n web/src/views/TaskBoardPage.vueRepository: Aureliolo/synthorg
Length of output: 8369
🏁 Script executed:
# Search for SelectButton and PrimeVue usage in vue files
rg "SelectButton|from.*primevue" web/src -A 2 -B 2Repository: Aureliolo/synthorg
Length of output: 10361
Replace the custom buttons with PrimeVue SelectButton component.
This custom button implementation violates the coding guideline "Use PrimeVue components for the Vue 3 dashboard UI". PrimeVue's SelectButton component provides repo-standard theming, accessibility behavior, and state management without the custom markup. Since the file already imports PrimeVue's Button component, using SelectButton for the view mode toggle aligns with the established patterns in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/TaskBoardPage.vue` around lines 145 - 159, Replace the two
custom toggle buttons with PrimeVue's SelectButton: import and register
SelectButton (e.g., import SelectButton from 'primevue/selectbutton' and add it
to components), create an options array like [{ label: 'Board', value: 'kanban'
}, { label: 'List', value: 'list' }], and bind the component to the existing
viewMode via v-model (e.g., v-model="viewMode" :options="viewOptions"); remove
the manual :class, :aria-pressed and `@click` handlers on the old <button>s so the
SelectButton manages state, styling and accessibility for the viewMode toggle
(references: viewMode, viewOptions, SelectButton, TaskBoardPage.vue).
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the initial full Web UI dashboard (Vue 3 + PrimeVue + Tailwind) with major page views, feature components, routing/guards, and supporting infra (WebSocket hardening, Docker/CI updates), plus broad unit test coverage.
Changes:
- Added multiple new dashboard pages (Dashboard, Tasks, Messages, Approvals, Agents, Org Chart, Budget, Settings) and placeholder pages (Meeting Logs, Artifacts), and wired them into the router.
- Introduced many new feature components (tasks kanban/list/detail/create, messaging feed, approvals queue/actions, org chart nodes, budget charts/tables, dashboard widgets) and updated shared styling/types.
- Expanded/updated unit tests across views/components/stores/composables/router, and adjusted Docker/CI workflows for builds and security tooling.
Reviewed changes
Copilot reviewed 96 out of 97 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/views/SetupPage.vue | Adds aria-describedby wiring for form errors. |
| web/src/views/SettingsPage.vue | New settings page (company/providers overview + password change). |
| web/src/views/PlaceholderHome.vue | Removes placeholder home view. |
| web/src/views/OrgChartPage.vue | New org chart page using VueFlow + store fetch. |
| web/src/views/MessageFeedPage.vue | New real-time message feed page with WS integration. |
| web/src/views/MeetingLogsPage.vue | Placeholder “coming soon” meeting logs view. |
| web/src/views/LoginPage.vue | Adds aria-describedby wiring for form errors. |
| web/src/views/DashboardPage.vue | New dashboard overview page + WS subscriptions + health check. |
| web/src/views/BudgetPanelPage.vue | New budget page + WS + charts/tables. |
| web/src/views/ArtifactBrowserPage.vue | Placeholder “coming soon” artifacts view. |
| web/src/views/ApprovalQueuePage.vue | New approvals queue page with detail sidebar + actions. |
| web/src/views/AgentProfilesPage.vue | New agents listing page + WS. |
| web/src/views/AgentDetailPage.vue | New agent detail page w/ route param validation + refetch. |
| web/src/utils/format.ts | Hardens uptime formatting against non-finite/negative values. |
| web/src/utils/constants.ts | Increases WS max message size; updates nav items comment. |
| web/src/styles/theme.ts | Aligns status/priority/risk types with API types; expands status colors. |
| web/src/stores/websocket.ts | Adds max message size discard + sanitizes/expands WS error logging. |
| web/src/stores/tasks.ts | Improves typing/perf of tasksByStatus grouping. |
| web/src/stores/auth.ts | Hardens setToken validation by throwing on invalid expiry. |
| web/src/router/index.ts | Registers all new page routes; uses lazy-loaded views. |
| web/src/router/guards.ts | Enforces must-change-password redirect to Settings user tab. |
| web/src/composables/usePolling.ts | Sanitizes polling error logs. |
| web/src/components/tasks/TaskListView.vue | New task list table view. |
| web/src/components/tasks/TaskFilters.vue | New task filter controls. |
| web/src/components/tasks/TaskDetailPanel.vue | New task detail sidebar with edit/transition/cancel actions. |
| web/src/components/tasks/TaskCreateDialog.vue | New task creation dialog. |
| web/src/components/tasks/TaskCard.vue | New kanban task card. |
| web/src/components/tasks/KanbanColumn.vue | New draggable kanban column. |
| web/src/components/tasks/KanbanBoard.vue | New kanban board composed of columns. |
| web/src/components/org-chart/OrgNode.vue | New org chart node renderer. |
| web/src/components/messages/MessageList.vue | New scrollable message list with near-bottom autoscroll. |
| web/src/components/messages/MessageItem.vue | New message item renderer. |
| web/src/components/messages/ChannelSelector.vue | New channel dropdown selector. |
| web/src/components/dashboard/SystemStatus.vue | New system status widget. |
| web/src/components/dashboard/SpendingSummary.vue | New spending widget with hourly aggregation chart. |
| web/src/components/dashboard/RecentApprovals.vue | New recent approvals widget. |
| web/src/components/dashboard/MetricCard.vue | New metric card widget. |
| web/src/components/dashboard/ActiveTasksSummary.vue | New active tasks widget. |
| web/src/components/common/StatusBadge.vue | Tightens badge value types; refines color selection logic. |
| web/src/components/budget/SpendingChart.vue | New daily spending chart. |
| web/src/components/budget/BudgetConfigDisplay.vue | New budget config summary display. |
| web/src/components/budget/AgentSpendingTable.vue | New per-agent spending aggregation table. |
| web/src/components/approvals/ApprovalDetail.vue | New approval detail display. |
| web/src/components/approvals/ApprovalCard.vue | New approval card. |
| web/src/components/approvals/ApprovalActions.vue | New approval approve/reject actions with confirm + UX states. |
| web/src/components/agents/AgentMetrics.vue | New agent metrics/details panel. |
| web/src/components/agents/AgentCard.vue | New agent summary card. |
| web/src/tests/views/TaskBoardPage.test.ts | Adds TaskBoard page tests. |
| web/src/tests/views/SetupPage.test.ts | Adds Setup page tests. |
| web/src/tests/views/SettingsPage.test.ts | Adds Settings page tests. |
| web/src/tests/views/OrgChartPage.test.ts | Adds OrgChart page tests. |
| web/src/tests/views/MessageFeedPage.test.ts | Adds MessageFeed page tests. |
| web/src/tests/views/MeetingLogsPage.test.ts | Adds MeetingLogs placeholder page tests. |
| web/src/tests/views/LoginPage.test.ts | Adds Login page tests. |
| web/src/tests/views/DashboardPage.test.ts | Adds Dashboard page tests. |
| web/src/tests/views/BudgetPanelPage.test.ts | Adds Budget page tests. |
| web/src/tests/views/ArtifactBrowserPage.test.ts | Adds ArtifactBrowser placeholder page tests. |
| web/src/tests/views/ApprovalQueuePage.test.ts | Adds ApprovalQueue page tests. |
| web/src/tests/views/AgentProfilesPage.test.ts | Adds AgentProfiles page tests. |
| web/src/tests/views/AgentDetailPage.test.ts | Adds AgentDetail page tests. |
| web/src/tests/utils/sanitizeForLog.test.ts | Adds sanitizeForLog tests. |
| web/src/tests/router/guards.test.ts | Adds must-change-password redirect tests. |
| web/src/tests/composables/usePolling.test.ts | Updates polling error expectation for sanitized log output. |
| web/src/tests/composables/useLoginLockout.test.ts | Adds login lockout composable tests. |
| web/src/tests/composables/useAuth.test.ts | Adds useAuth composable tests. |
| web/src/tests/components/TaskListView.test.ts | Adds TaskListView component tests. |
| web/src/tests/components/TaskFilters.test.ts | Adds TaskFilters component tests. |
| web/src/tests/components/TaskCreateDialog.test.ts | Adds TaskCreateDialog component tests. |
| web/src/tests/components/TaskCard.test.ts | Adds TaskCard component tests. |
| web/src/tests/components/SystemStatus.test.ts | Adds SystemStatus component tests. |
| web/src/tests/components/StatusBadge.test.ts | Updates StatusBadge fallback typing test. |
| web/src/tests/components/SpendingSummary.test.ts | Adds SpendingSummary component tests. |
| web/src/tests/components/SpendingChart.test.ts | Adds SpendingChart component tests. |
| web/src/tests/components/RecentApprovals.test.ts | Adds RecentApprovals component tests. |
| web/src/tests/components/OrgNode.test.ts | Adds OrgNode component tests. |
| web/src/tests/components/MetricCard.test.ts | Adds MetricCard component tests. |
| web/src/tests/components/MessageList.test.ts | Adds MessageList component tests. |
| web/src/tests/components/MessageItem.test.ts | Adds MessageItem component tests. |
| web/src/tests/components/ChannelSelector.test.ts | Adds ChannelSelector component tests. |
| web/src/tests/components/BudgetConfigDisplay.test.ts | Adds BudgetConfigDisplay component tests. |
| web/src/tests/components/ApprovalDetail.test.ts | Adds ApprovalDetail component tests. |
| web/src/tests/components/ApprovalCard.test.ts | Adds ApprovalCard component tests. |
| web/src/tests/components/ApprovalActions.test.ts | Adds ApprovalActions component tests. |
| web/src/tests/components/AgentSpendingTable.test.ts | Adds AgentSpendingTable component tests. |
| web/src/tests/components/AgentMetrics.test.ts | Adds AgentMetrics component tests. |
| web/src/tests/components/AgentCard.test.ts | Adds AgentCard component tests. |
| web/src/tests/components/ActiveTasksSummary.test.ts | Adds ActiveTasksSummary component tests. |
| web/package-lock.json | Updates lockfile (incl. Vite v6). |
| docs/design/operations.md | Updates ops doc to reflect current Web UI status/scope. |
| docker/web/Dockerfile | Ensures non-root build user can write /app. |
| docker/sandbox/Dockerfile | Ensures node-base stage ends non-root; adds packaging comments. |
| docker/backend/Dockerfile | Runs uv sync as non-root build user; drops root in setup stage. |
| CLAUDE.md | Updates repo structure documentation for new web folders/views. |
| .github/workflows/docker.yml | Runs docker build/scans on PRs; skips push/signing on PRs. |
| .github/workflows/dast.yml | Grants issues: write permission for DAST workflow. |
Files not reviewed (1)
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+32
| const VALID_TABS = ['company', 'providers', 'user'] as const | ||
| const tabParam = String(route.query.tab ?? 'company') | ||
| const activeTab = ref(VALID_TABS.includes(tabParam as typeof VALID_TABS[number]) ? tabParam : 'company') | ||
|
|
| <TabView v-else :value="activeTab"> | ||
| <!-- Company Config --> | ||
| <TabPanel header="Company" value="company"> | ||
| <div v-if="companyStore.config" class="space-y-4"> |
There was a problem hiding this comment.
TabView tabs are unresponsive to user clicks
<TabView :value="activeTab"> uses a one-way binding. In PrimeVue's controlled mode, activeTab must be kept in sync via v-model or @update:value; without a handler the emitted update:value event is discarded and activeTab never changes. After the initial render, clicking the "Providers" or "User" tab headers does nothing — the user is permanently stuck on the first tab shown.
This is especially impactful for the mustChangePassword redirect, which lands on ?tab=user. After the password is changed and the user later navigates back to /settings directly (defaulting to 'company'), they still cannot switch to the User tab without refreshing.
Suggested change
| <div v-if="companyStore.config" class="space-y-4"> | |
| <TabView v-else v-model:value="activeTab"> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/views/SettingsPage.vue
Line: 84
Comment:
**TabView tabs are unresponsive to user clicks**
`<TabView :value="activeTab">` uses a one-way binding. In PrimeVue's controlled mode, `activeTab` must be kept in sync via `v-model` or `@update:value`; without a handler the emitted `update:value` event is discarded and `activeTab` never changes. After the initial render, clicking the "Providers" or "User" tab headers does nothing — the user is permanently stuck on the first tab shown.
This is especially impactful for the `mustChangePassword` redirect, which lands on `?tab=user`. After the password is changed and the user later navigates back to `/settings` directly (defaulting to `'company'`), they still cannot switch to the User tab without refreshing.
```suggestion
<TabView v-else v-model:value="activeTab">
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+42
to
+51
| watch( | ||
| () => messageStore.activeChannel, | ||
| async (channel) => { | ||
| try { | ||
| await messageStore.fetchMessages(channel ?? undefined) | ||
| } catch { | ||
| // Store handles errors internally | ||
| } | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Silent catch inconsistent with PR's no-silent-swallow policy
The watch callback catches errors from fetchMessages but discards them silently. The comment "Store handles errors internally" is an assumption — if the store throws (e.g. a network error that bypasses the store's internal guard), the rejection is dropped with no log entry and no user feedback.
The onMounted handler in this same file correctly logs every caught error. This watch callback should follow the same pattern for consistency and to satisfy the CLAUDE.md rule "handle explicitly, never silently swallow".
Suggested change
| watch( | |
| () => messageStore.activeChannel, | |
| async (channel) => { | |
| try { | |
| await messageStore.fetchMessages(channel ?? undefined) | |
| } catch { | |
| // Store handles errors internally | |
| } | |
| }, | |
| ) | |
| watch( | |
| () => messageStore.activeChannel, | |
| async (channel) => { | |
| try { | |
| await messageStore.fetchMessages(channel ?? undefined) | |
| } catch (err) { | |
| console.error('Channel fetch failed:', sanitizeForLog(err)) | |
| } | |
| }, | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/views/MessageFeedPage.vue
Line: 42-51
Comment:
**Silent catch inconsistent with PR's no-silent-swallow policy**
The `watch` callback catches errors from `fetchMessages` but discards them silently. The comment "Store handles errors internally" is an assumption — if the store throws (e.g. a network error that bypasses the store's internal guard), the rejection is dropped with no log entry and no user feedback.
The `onMounted` handler in this same file correctly logs every caught error. This watch callback should follow the same pattern for consistency and to satisfy the CLAUDE.md rule "handle explicitly, never silently swallow".
```suggestion
watch(
() => messageStore.activeChannel,
async (channel) => {
try {
await messageStore.fetchMessages(channel ?? undefined)
} catch (err) {
console.error('Channel fetch failed:', sanitizeForLog(err))
}
},
)
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+62
to
+77
| const agent = agentIndex.get(memberName) | ||
| result.push({ | ||
| id: `agent-${memberName}`, | ||
| position: { x: 100 + i * 200, y }, | ||
| data: { | ||
| label: memberName, | ||
| type: 'agent', | ||
| status: agent?.status, | ||
| role: agent?.role, | ||
| level: agent?.level, | ||
| }, | ||
| type: 'orgNode', | ||
| }) | ||
| } | ||
| y += 120 | ||
| } |
There was a problem hiding this comment.
Duplicate node IDs when an agent belongs to multiple teams
Node IDs are generated as agent-${memberName}. If the same agent name appears in more than one team (e.g. a board_member who is also in an engineering team), nodes will contain duplicate IDs. VueFlow silently deduplicates or skips duplicate-keyed nodes, so the second team's rendering of that agent is dropped and its outgoing edge (teamId → agent-${member}) points to a node that VueFlow considers part of the first team — producing a broken or misleading chart.
Consider making agent IDs unique per occurrence:
id: `agent-${dept.name}-${team.name}-${memberName}`,You'd also need to update the edges and the onNodeClick handler (strip the dept/team prefix to recover the actual agent name):
function onNodeClick(event: { node: Node }) {
if (event.node.id.startsWith('agent-')) {
// id format: agent-<dept>-<team>-<name>
const name = event.node.data.label as string
router.push(`/agents/${encodeURIComponent(name)}`)
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/views/OrgChartPage.vue
Line: 62-77
Comment:
**Duplicate node IDs when an agent belongs to multiple teams**
Node IDs are generated as `agent-${memberName}`. If the same agent name appears in more than one team (e.g. a `board_member` who is also in an engineering team), `nodes` will contain duplicate IDs. VueFlow silently deduplicates or skips duplicate-keyed nodes, so the second team's rendering of that agent is dropped and its outgoing edge (`teamId → agent-${member}`) points to a node that VueFlow considers part of the first team — producing a broken or misleading chart.
Consider making agent IDs unique per occurrence:
```typescript
id: `agent-${dept.name}-${team.name}-${memberName}`,
```
You'd also need to update the edges and the `onNodeClick` handler (strip the `dept`/`team` prefix to recover the actual agent name):
```typescript
function onNodeClick(event: { node: Node }) {
if (event.node.id.startsWith('agent-')) {
// id format: agent-<dept>-<team>-<name>
const name = event.node.data.label as string
router.push(`/agents/${encodeURIComponent(name)}`)
}
}
```
How can I resolve this? If you propose a fix, please make it concise.…anitization
- Fix invalid CollaborationPreference type in AgentCard/AgentMetrics tests
- Fix misleading empty-state copy in RecentApprovals ("pending" → "recent")
- Add role="log" and aria-live="polite" to MessageList for screen readers
- Change Dropdown id → input-id for proper label association (TaskCreate, TaskDetail)
- Sanitize WebSocket JSON parse errors with sanitizeForLog
- Add catch blocks to ApprovalQueuePage approve/reject handlers
- Add !wsStore.connected guard to DashboardPage WebSocket connect
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/sandbox/Dockerfile (1)
16-21:⚠️ Potential issue | 🟠 MajorSandbox user cannot write to
/workspace.The
/workspacedirectory is created as root with default 755 permissions, but thesandboxuser (uid 10001) cannot write to it—only read and execute. This breaks typical sandbox workflows like cloning repos or writing temporary files.🔧 Proposed fix
RUN mkdir -p /workspace \ - && useradd --uid 10001 --no-create-home --shell /usr/sbin/nologin sandbox + && useradd --uid 10001 --no-create-home --shell /usr/sbin/nologin sandbox \ + && chown sandbox:sandbox /workspace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/sandbox/Dockerfile` around lines 16 - 21, The /workspace directory is created as root with default permissions so the newly created sandbox user (created via the useradd call and switched to with USER sandbox) cannot write to it; update the Dockerfile RUN that creates /workspace (the line containing "mkdir -p /workspace && useradd --uid 10001 ... sandbox") to change ownership or permissions so sandbox can write (e.g., chown the directory to UID 10001 or use chmod to make it writable) before switching to USER sandbox, ensuring subsequent WORKDIR /workspace and USER sandbox operations run as a writable directory.
♻️ Duplicate comments (3)
web/src/views/OrgChartPage.vue (1)
41-53:⚠️ Potential issue | 🟠 MajorMake VueFlow IDs collision-safe and decode-safe.
Line 42/52/64/87/89/98 build IDs by joining raw names with
-, which can collide and break VueFlow uniqueness. Line 111 also depends on fragile prefix stripping. Use a delimiter-safe encoded ID helper for all node/edge IDs and decode only the final segment for routing.Suggested fix
+function flowId(prefix: string, ...parts: string[]) { + return [prefix, ...parts.map((p) => encodeURIComponent(p))].join('::') +} + +function flowIdLastPart(id: string) { + const parts = id.split('::') + return decodeURIComponent(parts[parts.length - 1] ?? '') +} + const nodes = computed<Node[]>(() => { const result: Node[] = [] let y = 0 @@ for (const dept of companyStore.departments) { - const deptId = `dept-${dept.name}` + const deptId = flowId('dept', dept.name) @@ for (const team of dept.teams) { - const teamId = `team-${dept.name}-${team.name}` + const teamId = flowId('team', dept.name, team.name) @@ result.push({ - id: `agent-${memberName}`, + id: flowId('agent', memberName), @@ const edges = computed<Edge[]>(() => { const result: Edge[] = [] @@ for (const dept of companyStore.departments) { - const deptId = `dept-${dept.name}` + const deptId = flowId('dept', dept.name) for (const team of dept.teams) { - const teamId = `team-${dept.name}-${team.name}` + const teamId = flowId('team', dept.name, team.name) result.push({ - id: `${deptId}-${teamId}`, + id: flowId('edge', deptId, teamId), source: deptId, target: teamId, animated: true, }) for (const member of team.members) { result.push({ - id: `${teamId}-agent-${member}`, + id: flowId('edge', teamId, flowId('agent', member)), source: teamId, - target: `agent-${member}`, + target: flowId('agent', member), }) } } } @@ function onNodeClick(event: { node: Node }) { - if (event.node.id.startsWith('agent-')) { - const name = event.node.id.replace('agent-', '') + if (event.node.id.startsWith('agent::')) { + const name = flowIdLastPart(event.node.id) router.push(`/agents/${encodeURIComponent(name)}`) } }Also applies to: 64-65, 87-101, 109-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/views/OrgChartPage.vue` around lines 41 - 53, IDs built by concatenating raw names (deptId, teamId, etc.) can collide and break VueFlow; create a small helper pair (e.g., encodeNodeId(value: string) and decodeNodeId(encoded: string)) that safely encodes each name segment (URI-encode or base64) and joins segments with a fixed delimiter, then replace all inline id constructions (deptId, teamId, any edge id creation, and the node creation blocks using type 'orgNode') to use encodeNodeId for every segment; for lookups/routing (the code that strips prefixes around line ~111) decode only the final segment with decodeNodeId to get the original name. Ensure every place that currently builds or parses IDs uses these helpers so ID generation/parsing is collision- and decode-safe.web/src/views/TaskBoardPage.vue (2)
165-180: 🛠️ Refactor suggestion | 🟠 MajorUse PrimeVue
SelectButtonfor the view-mode toggle.This bespoke toggle is still bypassing the repo-standard component stack in a file that already uses PrimeVue. Swapping it to
SelectButtonwill align the control with the dashboard's theming and accessibility baseline.🎛️ Minimal refactor
+import SelectButton from 'primevue/selectbutton'+const viewOptions = [ + { label: 'Board', value: 'kanban' }, + { label: 'List', value: 'list' }, +] as const- <div class="flex rounded-lg border border-slate-700" role="group" aria-label="View mode"> - <button - :class="['px-3 py-1.5 text-xs', viewMode === 'kanban' ? 'bg-brand-600 text-white' : 'text-slate-400']" - :aria-pressed="viewMode === 'kanban'" - `@click`="viewMode = 'kanban'" - > - Board - </button> - <button - :class="['px-3 py-1.5 text-xs', viewMode === 'list' ? 'bg-brand-600 text-white' : 'text-slate-400']" - :aria-pressed="viewMode === 'list'" - `@click`="viewMode = 'list'" - > - List - </button> - </div> + <SelectButton + v-model="viewMode" + :options="viewOptions" + optionLabel="label" + optionValue="value" + size="small" + aria-label="View mode" + />As per coding guidelines, "Use PrimeVue components for the Vue 3 dashboard UI".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/views/TaskBoardPage.vue` around lines 165 - 180, Replace the custom two-button toggle with PrimeVue's SelectButton: import SelectButton from 'primevue/selectbutton' and register it in the component (or add to the components object / setup return), create an options array like [{ label: 'Board', value: 'kanban' }, { label: 'List', value: 'list' }], and replace the <div> containing the two <button> elements by a <SelectButton> bound to v-model="viewMode" (or :modelValue and `@update`:modelValue in composition API) with :options="viewOptions" and any needed class/aria attributes to match existing styling and accessibility; ensure references to viewMode remain unchanged elsewhere (e.g., any computed or watchers).
48-55:⚠️ Potential issue | 🟠 MajorRetry still can't recover the full page state.
Mount depends on
fetchTasks({ limit: 200 })andfetchAgents(), but the later reload paths only rerun task fetches. If agents fail once,agentStore.errorstays wedged behind the error boundary, and the task query no longer matches the initial dataset after the first interaction.🔁 Minimal fix
+const TASK_PAGE_SIZE = 200 + +async function fetchTasksWithFilters(nextFilters: TaskFilterType = filters.value) { + await taskStore.fetchTasks({ ...nextFilters, limit: TASK_PAGE_SIZE }) +} + +async function retryFetch() { + await Promise.all([ + fetchTasksWithFilters(), + agentStore.fetchAgents(), + ]) +} + onMounted(async () => { try { if (authStore.token && !wsStore.connected) { wsStore.connect(authStore.token) } wsStore.subscribe(['tasks']) wsStore.onChannelEvent('tasks', taskStore.handleWsEvent) } catch (err) { console.error('WebSocket setup failed:', sanitizeForLog(err)) } try { - await Promise.all([ - taskStore.fetchTasks({ limit: 200 }), - agentStore.fetchAgents(), - ]) + await retryFetch() } catch (err) { console.error('Initial data fetch failed:', sanitizeForLog(err)) } }) async function handleFilterUpdate(newFilters: TaskFilterType) { filters.value = { ...filters.value, ...newFilters } try { - await taskStore.fetchTasks(filters.value) + await fetchTasksWithFilters(filters.value) } catch (err) { console.error('Filter fetch failed:', sanitizeForLog(err)) } } async function handleFilterReset() { filters.value = {} try { - await taskStore.fetchTasks({}) + await fetchTasksWithFilters({}) } catch (err) { console.error('Filter reset fetch failed:', sanitizeForLog(err)) } }- <ErrorBoundary :error="taskStore.error ?? agentStore.error" `@retry`="() => taskStore.fetchTasks(filters)"> + <ErrorBoundary :error="taskStore.error ?? agentStore.error" `@retry`="retryFetch">Also applies to: 135-149, 192-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/views/TaskBoardPage.vue` around lines 48 - 55, The mount logic calls both taskStore.fetchTasks and agentStore.fetchAgents but your retry/reload flows only re-run task fetches, leaving agentStore.error stuck and causing state mismatch; update the retry/reload handlers (the code paths that call taskStore.fetchTasks on user interaction) to also call agentStore.fetchAgents (and/or wrap both calls in the same Promise.all used at mount), and ensure agentStore.error is cleared/reset before or after a successful agentStore.fetchAgents so the error boundary can recover; reference the existing functions fetchTasks, fetchAgents, taskStore.fetchTasks, agentStore.fetchAgents and agentStore.error when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/views/BudgetPanelPage.vue`:
- Around line 22-25: The subscribe call is being queued even when authentication
is missing, so ensure subscriptions are only added after auth is present: wrap
wsStore.subscribe(['budget']) inside the same conditional that checks
authStore.token and wsStore.connected (the block that calls
wsStore.connect(authStore.token)), or guard the subscribe call with
authStore.token (e.g., only call wsStore.subscribe when authStore.token is
truthy); reference wsStore.connect(), wsStore.subscribe(), authStore.token and
wsStore.connected when making the change so the subscription is enqueued only
after authentication/connection is attempted.
In `@web/src/views/DashboardPage.vue`:
- Around line 35-41: Duplicate websocket channel and handler wiring in
DashboardPage.vue (wsStore.subscribe and wsStore.onChannelEvent calls) can drift
between mount/unmount — extract a shared constant or map (e.g., CHANNELS =
['tasks','budget','approvals'] and HANDLERS = { tasks: taskStore.handleWsEvent,
budget: budgetStore.handleWsEvent, approvals: approvalStore.handleWsEvent }) and
use that single source when calling wsStore.subscribe(CHANNELS) and iterating
HANDLERS to call wsStore.onChannelEvent(channel, handler) both on mount and on
cleanup; update the code that currently calls wsStore.connect(authStore.token),
wsStore.subscribe, wsStore.onChannelEvent and the corresponding unmount logic to
reference these shared CHANNELS/HANDLERS to eliminate duplication.
---
Outside diff comments:
In `@docker/sandbox/Dockerfile`:
- Around line 16-21: The /workspace directory is created as root with default
permissions so the newly created sandbox user (created via the useradd call and
switched to with USER sandbox) cannot write to it; update the Dockerfile RUN
that creates /workspace (the line containing "mkdir -p /workspace && useradd
--uid 10001 ... sandbox") to change ownership or permissions so sandbox can
write (e.g., chown the directory to UID 10001 or use chmod to make it writable)
before switching to USER sandbox, ensuring subsequent WORKDIR /workspace and
USER sandbox operations run as a writable directory.
---
Duplicate comments:
In `@web/src/views/OrgChartPage.vue`:
- Around line 41-53: IDs built by concatenating raw names (deptId, teamId, etc.)
can collide and break VueFlow; create a small helper pair (e.g.,
encodeNodeId(value: string) and decodeNodeId(encoded: string)) that safely
encodes each name segment (URI-encode or base64) and joins segments with a fixed
delimiter, then replace all inline id constructions (deptId, teamId, any edge id
creation, and the node creation blocks using type 'orgNode') to use encodeNodeId
for every segment; for lookups/routing (the code that strips prefixes around
line ~111) decode only the final segment with decodeNodeId to get the original
name. Ensure every place that currently builds or parses IDs uses these helpers
so ID generation/parsing is collision- and decode-safe.
In `@web/src/views/TaskBoardPage.vue`:
- Around line 165-180: Replace the custom two-button toggle with PrimeVue's
SelectButton: import SelectButton from 'primevue/selectbutton' and register it
in the component (or add to the components object / setup return), create an
options array like [{ label: 'Board', value: 'kanban' }, { label: 'List', value:
'list' }], and replace the <div> containing the two <button> elements by a
<SelectButton> bound to v-model="viewMode" (or :modelValue and
`@update`:modelValue in composition API) with :options="viewOptions" and any
needed class/aria attributes to match existing styling and accessibility; ensure
references to viewMode remain unchanged elsewhere (e.g., any computed or
watchers).
- Around line 48-55: The mount logic calls both taskStore.fetchTasks and
agentStore.fetchAgents but your retry/reload flows only re-run task fetches,
leaving agentStore.error stuck and causing state mismatch; update the
retry/reload handlers (the code paths that call taskStore.fetchTasks on user
interaction) to also call agentStore.fetchAgents (and/or wrap both calls in the
same Promise.all used at mount), and ensure agentStore.error is cleared/reset
before or after a successful agentStore.fetchAgents so the error boundary can
recover; reference the existing functions fetchTasks, fetchAgents,
taskStore.fetchTasks, agentStore.fetchAgents and agentStore.error when making
this change.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b71db20-aff8-40ce-84eb-df7339f22472
📒 Files selected for processing (7)
docker/sandbox/Dockerfileweb/src/__tests__/router/guards.test.tsweb/src/router/guards.tsweb/src/views/BudgetPanelPage.vueweb/src/views/DashboardPage.vueweb/src/views/OrgChartPage.vueweb/src/views/TaskBoardPage.vue
📜 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). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (6)
!(docs/design/operations.md|.claude/**|**/node_modules/**)
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like
example-provider,example-large-001,test-provider,test-small-001, or size aliases
Files:
web/src/router/guards.tsweb/src/views/TaskBoardPage.vuedocker/sandbox/Dockerfileweb/src/views/OrgChartPage.vueweb/src/views/BudgetPanelPage.vueweb/src/__tests__/router/guards.test.tsweb/src/views/DashboardPage.vue
web/**/*.{js,ts,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint for JavaScript/TypeScript linting in the web dashboard via
npm --prefix web run lint
Files:
web/src/router/guards.tsweb/src/views/TaskBoardPage.vueweb/src/views/OrgChartPage.vueweb/src/views/BudgetPanelPage.vueweb/src/__tests__/router/guards.test.tsweb/src/views/DashboardPage.vue
web/**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Pinia for state management in the web dashboard
Files:
web/src/router/guards.tsweb/src/__tests__/router/guards.test.ts
web/**/*.{js,ts,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Dashboard (npm) audit runs per-PR via
dashboard-auditjob checking critical and high vulnerabilities
Files:
web/src/router/guards.tsweb/src/__tests__/router/guards.test.ts
web/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.vue: Usevue-tsctype checking for Vue components vianpm --prefix web run type-check
Runnpm --prefix web run buildfor production builds of the web dashboard
Use PrimeVue components for the Vue 3 dashboard UI
Use Tailwind CSS for styling the Vue 3 dashboard
Files:
web/src/views/TaskBoardPage.vueweb/src/views/OrgChartPage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/DashboardPage.vue
web/**/__tests__/**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit testing the web dashboard via
npm --prefix web run test
Files:
web/src/__tests__/router/guards.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.022Z
Learning: Applies to web/**/*.{js,ts,json} : Dashboard (npm) audit runs per-PR via `dashboard-audit` job checking critical and high vulnerabilities
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to web/src/components/**/*.vue : Organize web dashboard components by feature in `web/src/components/`
Applied to files:
web/src/views/TaskBoardPage.vueweb/src/views/OrgChartPage.vueweb/src/views/BudgetPanelPage.vueweb/src/views/DashboardPage.vue
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to web/**/*.vue : Use Tailwind CSS for styling the Vue 3 dashboard
Applied to files:
web/src/views/TaskBoardPage.vueweb/src/views/DashboardPage.vue
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to web/**/*.vue : Use PrimeVue components for the Vue 3 dashboard UI
Applied to files:
web/src/views/TaskBoardPage.vueweb/src/views/DashboardPage.vue
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to docker/Dockerfile*backend* : Run Docker backend container as non-root user (UID 65532) with CIS hardening
Applied to files:
docker/sandbox/Dockerfile
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to docker/Dockerfile*backend* : Configure Docker backend builds with 3-stage strategy (builder → setup → distroless runtime) using Chainguard Python base image
Applied to files:
docker/sandbox/Dockerfile
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to docker/Dockerfile*web* : Configure Docker web builds with nginxinc/nginx-unprivileged base image for Vue 3 SPA with API/WebSocket proxy to backend
Applied to files:
docker/sandbox/Dockerfile
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to docker/Dockerfile* : Lint Dockerfiles with hadolint via pre-commit hook and CI job
Applied to files:
docker/sandbox/Dockerfile
📚 Learning: 2026-03-13T17:15:09.021Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.021Z
Learning: Applies to web/src/stores/**/*.{js,ts} : Organize Pinia stores in `web/src/stores/` for features like auth, agents, tasks, budget, messages, approvals, websocket, analytics, company, and providers
Applied to files:
web/src/views/BudgetPanelPage.vue
📚 Learning: 2026-03-13T17:15:09.022Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T17:15:09.022Z
Learning: Applies to web/**/*.{js,ts,json} : Dashboard (npm) audit runs per-PR via `dashboard-audit` job checking critical and high vulnerabilities
Applied to files:
web/src/views/DashboardPage.vue
🧬 Code graph analysis (1)
web/src/__tests__/router/guards.test.ts (2)
web/src/stores/auth.ts (1)
useAuthStore(8-173)web/src/router/guards.ts (1)
authGuard(12-42)
🪛 GitHub Check: Dashboard Lint
web/src/views/DashboardPage.vue
[warning] 62-62:
Generic Object Injection Sink
🔇 Additional comments (11)
web/src/router/guards.ts (1)
35-39: Must-change-password guard logic looks correct.This correctly enforces password change by normalizing navigation to
settings?tab=userand avoids re-redirect when already on the intended target.web/src/__tests__/router/guards.test.ts (3)
98-110: Good coverage for forced redirect when password change is required.This test validates the primary enforcement path and expected redirect target.
112-124: Good normalization test for/settingswithouttab=user.This correctly confirms the guard rewrites the destination to the required user tab.
126-138: Good allow-path test forsettings?tab=user.This protects against accidental redirect loops by verifying
next()is called without arguments.docker/sandbox/Dockerfile (2)
2-3: Clarification:USER nodeis for scanner compliance, not runtime security.The
USERdirective in thenode-basestage doesn't enhance the final image's security since this stage is only used as aCOPY --fromsource—the USER setting isn't propagated. However, this satisfies security scanners that flag root-running stages, and it's good hygiene for potential future modifications.
11-14: LGTM on hadolint DL3008 ignore with documented rationale.Suppressing the version-pinning warning is acceptable here since the git version is intentionally tied to base image updates via Dependabot. The rationale comment makes this clear for future maintainers.
web/src/views/OrgChartPage.vue (1)
25-31: Good hardening on error logging path.Using
sanitizeForLog(err)in Line 29 is the right safeguard to avoid sensitive leakage in client logs.web/src/views/DashboardPage.vue (2)
43-45: Good defensive logging for WebSocket setup failures.Using
sanitizeForLog(err)at Line 44 is the right safeguard against sensitive-token leakage in error paths.
49-71: Nice partial-failure handling for first paint.
Promise.allSettled+ targeted toast notification gives resilient loading behavior without hard-failing the whole dashboard.web/src/views/TaskBoardPage.vue (1)
68-128: Nice hardening of the async mutation flows.These handlers now catch rejected promises, sanitize the logged payload, and still surface a toast to the user instead of leaking unhandled async errors through the UI.
web/src/views/BudgetPanelPage.vue (1)
21-34: Nice separation between real-time setup and snapshot loading.Keeping the WebSocket setup and initial REST fetch in separate
tryblocks means a live-update hiccup will not block the first budget render, and both failure paths stay sanitized.
Comment on lines
+22
to
+25
| if (authStore.token && !wsStore.connected) { | ||
| wsStore.connect(authStore.token) | ||
| } | ||
| wsStore.subscribe(['budget']) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
WS_FILE="$(fd '^websocket\.(ts|js)$' web/src/stores | head -n1)"
if [ -z "${WS_FILE:-}" ]; then
echo "Could not find websocket store under web/src/stores"
exit 1
fi
echo "== websocket store: $WS_FILE =="
rg -n -C4 '\b(connect|subscribe|unsubscribe|onChannelEvent|offChannelEvent)\s*\(' "$WS_FILE"
echo
echo "== BudgetPanelPage call site =="
sed -n '20,30p' web/src/views/BudgetPanelPage.vueRepository: Aureliolo/synthorg
Length of output: 2958
subscribe() is safe before connection, but should only be called after authentication is confirmed.
The WebSocket store queues subscriptions in activeSubscriptions, re-subscribing all tracked channels when the socket opens. However, line 25 calls subscribe(['budget']) unconditionally while connect() on line 23 only runs if authStore.token exists. If the token is absent, the socket never connects and the queued subscription is never sent. Move the subscribe() call inside the if (authStore.token && !wsStore.connected) block, or conditionally call it only when authStore.token is present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/BudgetPanelPage.vue` around lines 22 - 25, The subscribe call
is being queued even when authentication is missing, so ensure subscriptions are
only added after auth is present: wrap wsStore.subscribe(['budget']) inside the
same conditional that checks authStore.token and wsStore.connected (the block
that calls wsStore.connect(authStore.token)), or guard the subscribe call with
authStore.token (e.g., only call wsStore.subscribe when authStore.token is
truthy); reference wsStore.connect(), wsStore.subscribe(), authStore.token and
wsStore.connected when making the change so the subscription is enqueued only
after authentication/connection is attempted.
Comment on lines
+35
to
+41
| try { | ||
| if (authStore.token) { | ||
| wsStore.connect(authStore.token) | ||
| wsStore.subscribe(['tasks', 'budget', 'approvals']) | ||
| wsStore.onChannelEvent('tasks', taskStore.handleWsEvent) | ||
| wsStore.onChannelEvent('budget', budgetStore.handleWsEvent) | ||
| wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prevent mount/unmount drift by centralizing WS channels and handlers.
Lines 38-41 and Lines 78-81 duplicate channel + handler wiring. A shared constant/map reduces mismatch risk during future edits.
♻️ Proposed refactor
const wsStore = useWebSocketStore()
const authStore = useAuthStore()
const toast = useToast()
const health = ref<HealthStatus | null>(null)
const loading = ref(true)
+const DASHBOARD_CHANNELS = ['tasks', 'budget', 'approvals'] as const
onMounted(async () => {
// Connect WebSocket (non-fatal if it fails)
try {
if (authStore.token) {
wsStore.connect(authStore.token)
- wsStore.subscribe(['tasks', 'budget', 'approvals'])
+ wsStore.subscribe([...DASHBOARD_CHANNELS])
wsStore.onChannelEvent('tasks', taskStore.handleWsEvent)
wsStore.onChannelEvent('budget', budgetStore.handleWsEvent)
wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent)
}
} catch (err) {
console.error('WebSocket setup failed:', sanitizeForLog(err))
}
@@
onUnmounted(() => {
- wsStore.unsubscribe(['tasks', 'budget', 'approvals'])
+ wsStore.unsubscribe([...DASHBOARD_CHANNELS])
wsStore.offChannelEvent('tasks', taskStore.handleWsEvent)
wsStore.offChannelEvent('budget', budgetStore.handleWsEvent)
wsStore.offChannelEvent('approvals', approvalStore.handleWsEvent)
})Also applies to: 77-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/views/DashboardPage.vue` around lines 35 - 41, Duplicate websocket
channel and handler wiring in DashboardPage.vue (wsStore.subscribe and
wsStore.onChannelEvent calls) can drift between mount/unmount — extract a shared
constant or map (e.g., CHANNELS = ['tasks','budget','approvals'] and HANDLERS =
{ tasks: taskStore.handleWsEvent, budget: budgetStore.handleWsEvent, approvals:
approvalStore.handleWsEvent }) and use that single source when calling
wsStore.subscribe(CHANNELS) and iterating HANDLERS to call
wsStore.onChannelEvent(channel, handler) both on mount and on cleanup; update
the code that currently calls wsStore.connect(authStore.token),
wsStore.subscribe, wsStore.onChannelEvent and the corresponding unmount logic to
reference these shared CHANNELS/HANDLERS to eliminate duplication.
Comment on lines
+36
to
+42
| if (authStore.token && !wsStore.connected) { | ||
| wsStore.connect(authStore.token) | ||
| wsStore.subscribe(['tasks', 'budget', 'approvals']) | ||
| wsStore.onChannelEvent('tasks', taskStore.handleWsEvent) | ||
| wsStore.onChannelEvent('budget', budgetStore.handleWsEvent) | ||
| wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent) | ||
| } |
There was a problem hiding this comment.
Real-time updates silently disabled when WS is already connected
wsStore.subscribe and all wsStore.onChannelEvent calls are nested inside the if (authStore.token && !wsStore.connected) guard. If the user navigates to the Dashboard after visiting any other page (Tasks, Budget, Approvals, etc.) the WebSocket is already connected, the condition is false, and none of the three channel subscriptions or event handlers are ever registered. The Dashboard will display stale data and receive no live updates for the rest of the session.
Every other page in this PR (e.g. ApprovalQueuePage, TaskBoardPage, BudgetPanelPage) correctly keeps only the wsStore.connect call inside the guard and places subscribe/onChannelEvent unconditionally after it:
Suggested change
| if (authStore.token && !wsStore.connected) { | |
| wsStore.connect(authStore.token) | |
| wsStore.subscribe(['tasks', 'budget', 'approvals']) | |
| wsStore.onChannelEvent('tasks', taskStore.handleWsEvent) | |
| wsStore.onChannelEvent('budget', budgetStore.handleWsEvent) | |
| wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent) | |
| } | |
| try { | |
| if (authStore.token && !wsStore.connected) { | |
| wsStore.connect(authStore.token) | |
| } | |
| wsStore.subscribe(['tasks', 'budget', 'approvals']) | |
| wsStore.onChannelEvent('tasks', taskStore.handleWsEvent) | |
| wsStore.onChannelEvent('budget', budgetStore.handleWsEvent) | |
| wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent) | |
| } catch (err) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/views/DashboardPage.vue
Line: 36-42
Comment:
**Real-time updates silently disabled when WS is already connected**
`wsStore.subscribe` and all `wsStore.onChannelEvent` calls are nested inside the `if (authStore.token && !wsStore.connected)` guard. If the user navigates to the Dashboard after visiting any other page (Tasks, Budget, Approvals, etc.) the WebSocket is already connected, the condition is `false`, and **none** of the three channel subscriptions or event handlers are ever registered. The Dashboard will display stale data and receive no live updates for the rest of the session.
Every other page in this PR (e.g. `ApprovalQueuePage`, `TaskBoardPage`, `BudgetPanelPage`) correctly keeps only the `wsStore.connect` call inside the guard and places `subscribe`/`onChannelEvent` unconditionally after it:
```suggestion
try {
if (authStore.token && !wsStore.connected) {
wsStore.connect(authStore.token)
}
wsStore.subscribe(['tasks', 'budget', 'approvals'])
wsStore.onChannelEvent('tasks', taskStore.handleWsEvent)
wsStore.onChannelEvent('budget', budgetStore.handleWsEvent)
wsStore.onChannelEvent('approvals', approvalStore.handleWsEvent)
} catch (err) {
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+36
to
+43
|
|
||
| const currentPassword = ref('') | ||
| const newPassword = ref('') | ||
| const confirmPassword = ref('') | ||
| const pwdError = ref<string | null>(null) | ||
|
|
||
| async function retryFetch() { | ||
| loading.value = true |
There was a problem hiding this comment.
retryFetch missing catch — unhandled promise rejection on store errors
retryFetch uses try/finally but has no catch. If either companyStore.fetchConfig() or providerStore.fetchProviders() throws (e.g. a network error that bypasses the store's internal error state), the rejection propagates as an unhandled promise rejection — no error is logged, and the ErrorBoundary only shows an error if the store sets its own error state.
Every other retryFetch in this PR (BudgetPanelPage, OrgChartPage) includes a catch block. For consistency with the CLAUDE.md "never silently swallow" rule:
Suggested change
| const currentPassword = ref('') | |
| const newPassword = ref('') | |
| const confirmPassword = ref('') | |
| const pwdError = ref<string | null>(null) | |
| async function retryFetch() { | |
| loading.value = true | |
| async function retryFetch() { | |
| loading.value = true | |
| try { | |
| await Promise.all([companyStore.fetchConfig(), providerStore.fetchProviders()]) | |
| } catch (err) { | |
| console.error('Settings data fetch failed:', sanitizeForLog(err)) | |
| } finally { | |
| loading.value = false | |
| } | |
| } |
Note: this also requires adding import { sanitizeForLog } from '@/utils/logging' to the imports at the top of the file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/views/SettingsPage.vue
Line: 36-43
Comment:
**`retryFetch` missing `catch` — unhandled promise rejection on store errors**
`retryFetch` uses `try/finally` but has no `catch`. If either `companyStore.fetchConfig()` or `providerStore.fetchProviders()` throws (e.g. a network error that bypasses the store's internal error state), the rejection propagates as an unhandled promise rejection — no error is logged, and the `ErrorBoundary` only shows an error if the store sets its own error state.
Every other `retryFetch` in this PR (`BudgetPanelPage`, `OrgChartPage`) includes a `catch` block. For consistency with the CLAUDE.md "never silently swallow" rule:
```suggestion
async function retryFetch() {
loading.value = true
try {
await Promise.all([companyStore.fetchConfig(), providerStore.fetchProviders()])
} catch (err) {
console.error('Settings data fetch failed:', sanitizeForLog(err))
} finally {
loading.value = false
}
}
```
Note: this also requires adding `import { sanitizeForLog } from '@/utils/logging'` to the imports at the top of the file.
How can I resolve this? If you propose a fix, please make it concise.
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.
Summary
Bug Fixes
onUnmountedcleanup across Dashboard, Tasks, Messages, Approvals, Budget, and Agents pages/settingswithout the required?tab=userquery param, bypassing the password change enforcement — tightened guard to require both route name AND query param match!wsStore.connectedguard allowed redundant WebSocket connections on the dashboardhandleApprove/handleRejecthadtry/finallybut nocatch— errors propagated silently without user feedbackgetMonth/getDate/getHours) causing inconsistent chart grouping across timezones — fixed to use UTC methodsAgentCard/AgentMetricstests used'cooperative' as neverforCollaborationPreferencefield — corrected to valid union member'team'idinstead ofinputIdon PrimeVue Dropdowns — labels weren't associated with the focusable input elementSecurity Fixes
onerrorhandler was logging the full error event (which contains the connection URL with JWT token as query param) — replaced with generic error messageSyntaxError(which includes message content) — wrapped withsanitizeForLogto prevent sensitive data leakage in logssanitizeForLogto all catch blocks across views and stores to prevent leaking internal state or user data in console outputnoderespectively)Accessibility Improvements
aria-requiredon required fieldsrole="log"andaria-live="polite"for screen reader announcements on new messages?tab=validated against known tab values to prevent invalid tab statePerformance
tasksByStatusgrouping via push instead of spreadCloses #233
Test plan
npm run lint— 0 errors (8 pre-existing warnings)npm run type-check— passesnpm run test— 453 tests pass across 55 filescanWritecomposable