feat(web): Security Company OS foundation - #93
Merged
Conversation
Capture inspection findings, API gaps, security controls, and delivery order. Update PRODUCT/DESIGN and add ADR 0007 for the OS shell decision.
Typed client, status vocabulary, session/me, command summary, missions, and audit list APIs. Fixture adapters for teams and capability packs.
Replace narrow ops nav with CompanyOsShell. Wire Command, Operations, Missions, Teams, Agents path, Capabilities, Governance Inbox, Audit, and Integrations to real APIs where available.
📝 WalkthroughWalkthroughThe PR establishes a Security Company OS web foundation with a new shell, shared status and data contracts, authorized API/domain layers, React Query access, governance workflows, and operational pages for command, missions, audit, operations, integrations, capabilities, and teams. ChangesSecurity Company OS foundation
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (8)
apps/web/types/status.test.ts (1)
8-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all mapper aliases and fallback behavior.
The suite omits branches for
configured,waiting,suspended,cancelled/canceled,expired, approval-waiting states,archived, null/undefined/empty inputs, and unknown values. Add table-driven cases so changes to this foundational normalization layer cannot silently regress governance or operational rendering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/types/status.test.ts` around lines 8 - 29, Expand the “status vocabulary mappers” tests with table-driven cases covering every alias and fallback branch in toHealthState, toApprovalState, and toOperationalState, including configured, waiting, suspended, cancelled/canceled, expired, approval-waiting states, archived, null, undefined, empty inputs, and unknown values. Assert each input maps to its expected normalized output while preserving the existing cases.apps/web/features/audit/audit-view.tsx (1)
16-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDebounce audit search inputs.
filtersrecomputes on every keystroke inq/action/targetType, and each change produces a newqueryKeyforuseAuditEvents, firing a network request per character typed.♻️ Debounce the search input
- const filters = useMemo( - () => ({ - q: q || undefined, - action: action || undefined, - targetType: targetType || undefined, - limit: "50", - }), - [q, action, targetType], - ); + const [debouncedQ] = useDebouncedValue(q, 300); + const filters = useMemo( + () => ({ + q: debouncedQ || undefined, + action: action || undefined, + targetType: targetType || undefined, + limit: "50", + }), + [debouncedQ, action, targetType], + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/features/audit/audit-view.tsx` around lines 16 - 32, Debounce the audit filter values used by the filters useMemo and useAuditEvents so typing in q, action, or targetType does not trigger a request for every keystroke. Preserve the existing undefined handling and limit value, while ensuring the query key updates only after the debounce interval.apps/web/components/os/os-command-palette.tsx (1)
85-142: 🎯 Functional Correctness | 🔵 TrivialCommand palette input isn't wired as an ARIA combobox.
The input drives a
role="listbox"list via arrow keys, but lacksrole="combobox",aria-expanded,aria-controls, andaria-activedescendantpointing at the highlighted<li id=...>. Screen readers won't announce which option is currently selected as the user navigates with arrows.♻️ Proposed fix (sketch)
<input id="os-command-input" ref={inputRef} + role="combobox" + aria-expanded={open} + aria-controls="os-command-listbox" + aria-activedescendant={filtered[selectedIndex] ? `os-command-option-${selectedIndex}` : undefined} value={query}- <ul className="max-h-80 overflow-y-auto p-1" role="listbox"> + <ul id="os-command-listbox" className="max-h-80 overflow-y-auto p-1" role="listbox"> {filtered.map((command, index) => { const Icon = command.icon; return ( - <li key={command.href} role="option" aria-selected={index === selectedIndex}> + <li + key={command.href} + id={`os-command-option-${index}`} + role="option" + aria-selected={index === selectedIndex} + >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/components/os/os-command-palette.tsx` around lines 85 - 142, Update the command palette input and option markup in the input keydown/listbox flow: give the input combobox semantics with role="combobox", aria-expanded, aria-controls referencing the listbox, and aria-activedescendant referencing the highlighted option. Add stable IDs to the rendered option elements and ensure the active descendant is unset when no option is selected, while preserving the existing keyboard selection behavior.apps/web/features/approvals/governance-inbox.test.ts (1)
1-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSource-substring assertions won't catch real behavior regressions.
These tests only grep the source file for literal strings; they don't render
GovernanceInboxor simulate selecting/approving/rejecting. A regression like reason/confirmation state leaking across approval selections would still pass this suite. Consider React Testing Library tests that actually exercise selection, validation, and decision submission for this governance-critical component.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/features/approvals/governance-inbox.test.ts` around lines 1 - 21, The tests in “Governance inbox” only inspect source text and do not verify runtime behavior. Replace the substring assertions with React Testing Library tests that render GovernanceInbox, simulate selecting approvals and approve/reject actions, validate required reasons and high-impact confirmation, and confirm decision.mutateAsync submission; include a selection-switch case to ensure reason and confirmation state do not leak between approvals.apps/web/app/page.tsx (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
ControlPlaneDashboard.
ControlPlaneDashboardis only referenced in its own file and is no longer wired into any route, so this view and its embeddedOpsShellusage are dead code. Remove it, or rewire it into a live route.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/page.tsx` around lines 1 - 5, Remove the unused ControlPlaneDashboard component and its embedded OpsShell usage, since HomePage currently renders CommandView and no live route references the dashboard. Alternatively, wire ControlPlaneDashboard into an active route if it is still required, ensuring no dead component remains.apps/web/lib/api/client.test.ts (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
apiRequest/buildUrl, not justApiClientError.The critical paths — 401 redirect, non-ok status mapping, missing
dataenvelope, and relative vs. absolute URL building — are untested. Given the SSR concern raised onclient.ts, a test exercisingapiGet/apiPostwith a mockedfetchwould catch regressions here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/api/client.test.ts` around lines 1 - 19, Expand the tests in the ApiClientError suite to cover apiRequest and buildUrl through apiGet/apiPost with a mocked fetch. Verify 401 responses trigger the redirect behavior, other non-ok responses map to ApiClientError, responses missing the data envelope are handled correctly, and relative and absolute URLs are built as expected, including SSR-safe behavior.apps/web/lib/mission-web-domain.ts (1)
44-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDuplicate limit-parsing and unfriendly error on invalid
limit.The same
z.coerce.number().int().min(1).max(100).parse(...)appears in bothlistWebMissionsandlistWebMissionRuns. Since.parse()throwsZodError, an invalid?limit=value bubbles up throughproblemResponse's genericErrorbranch, returning the raw Zod issues string as the 400detailinstead of a clean message.Extract a small helper and use
.safeParse()to throw a friendlyApiProblem:♻️ Suggested helper
+function parseLimit(limitRaw?: string | null): number { + const result = z.coerce.number().int().min(1).max(100).safeParse(limitRaw ?? 50); + if (!result.success) { + throw new ApiProblem(400, "Invalid limit", "limit must be an integer between 1 and 100."); + } + return result.data; +}Also applies to: 79-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/mission-web-domain.ts` around lines 44 - 57, Extract the shared limit validation used by listWebMissions and listWebMissionRuns into a small helper, replacing duplicated z.coerce.number().int().min(1).max(100).parse logic. Have the helper use safeParse and throw an ApiProblem with a clean 400 message when the value is invalid, while preserving the default limit of 50 and valid range of 1–100.apps/web/lib/audit-domain.test.ts (1)
4-18: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftReplace source-text assertions with executable domain tests. These checks pass when matching text exists in unreachable or incorrectly wired code; they do not prove capability denial, organisation-scoped query execution, or redaction behavior.
apps/web/lib/audit-domain.test.ts#L4-L18: invokelistAuditEventswith mocked database/auth dependencies; verify denial prevents querying, predicates include the subject organisation, and returned metadata is redacted.apps/web/lib/mission-web-domain.test.ts#L4-L18: invoke mission functions with mocked dependencies; verify denial prevents querying and each query is scoped to the subject organisation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/audit-domain.test.ts` around lines 4 - 18, Replace the source-text assertions in apps/web/lib/audit-domain.test.ts:4-18 with executable tests invoking listAuditEvents using mocked auth and database dependencies; verify capability denial prevents database access, queries include the subject organisation predicate, and returned metadata is redacted. Replace the source-text assertions in apps/web/lib/mission-web-domain.test.ts:4-18 with executable tests invoking the mission functions using mocked dependencies; verify denied access prevents querying and every query is scoped to the subject organisation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/os/company-os-shell.tsx`:
- Around line 337-397: Extract the duplicated sign-out logic into a shared
handleSignOut handler in the component, and use it for both desktop and mobile
sign-out buttons. Have the handler navigate to /login on successful
authClient.signOut(), and catch rejected sign-out promises to provide
user-visible error feedback without leaving the failure unhandled.
In `@apps/web/components/os/metric-tile.tsx`:
- Around line 38-42: Update the linked metric tile styling in the className
construction so metric.href retains a visible focus indicator for keyboard
users; replace the focus-visible:outline-none behavior with the project’s
established focus ring or equivalent visible focus style, while preserving the
existing hover styling and non-linked tile appearance.
In `@apps/web/features/approvals/governance-inbox.tsx`:
- Around line 36-39: Reset the per-approval decision state whenever selection
changes through setSelectedId in the governance inbox: clear reason, uncheck
confirmHighImpact, and clear message when switching approvals. Preserve
append-only messages, timelines, evidence metadata, and audit events; only reset
the transient decision fields associated with the selected approval.
- Around line 22-29: Move high-impact confirmation enforcement from the
browser’s riskSeverity flow into the server-side approval decision
domain/service used by POST /api/v1/approvals/:id/decisions. Extend request
validation and decision creation to require and persist auditable
confirmHighImpact evidence for high-impact approvals, while preserving normal
approval behavior for lower-risk decisions and rejecting direct API requests
that omit confirmation.
In `@apps/web/features/audit/audit-view.tsx`:
- Around line 19-20: Reset both selected and expandedId whenever the audit
filters change or audit.data refreshes, including clearing filters, so the
detail aside cannot retain records from a stale result set. Update the relevant
filter-change and data-refresh logic in the audit view while preserving normal
selection behavior for the current results.
In `@apps/web/features/integrations/integrations-view.tsx`:
- Around line 161-178: Update the error-state condition in the integrations view
to render ErrorState when either controlPlane or connectors reports an error,
while preserving the existing retry behavior for both sources. Ensure the empty
state is not shown when either fetch has failed, including cases where the other
source returns no cards.
- Around line 138-143: Validate the unknown values in controlPlane.data and
connectors.data with the existing Zod schemas before passing them to
controlPlaneCards or connectorCards; only use successfully parsed values,
otherwise fall back to empty arrays. Remove the direct ControlPlaneSlice and
ConnectorRow[] casts in the cards construction and ensure malformed nested
responses cannot reach either card-builder.
In `@apps/web/features/operations/operations-view.tsx`:
- Around line 192-197: Interactive table rows cannot be selected via keyboard.
In apps/web/features/operations/operations-view.tsx:192-197,
apps/web/features/audit/audit-view.tsx:126-130, and
apps/web/features/capabilities/capabilities-view.tsx:39-43, update each row’s
existing onClick selection behavior by adding tabIndex={0}, role="button", and
an onKeyDown handler that triggers the same selection for Enter and Space.
- Around line 77-79: Update the approvalState mapping in the operations view to
use the task’s linked approval decision/status from the domain models, such as
schema.approvals, rather than deriving it from approvalRequired. Preserve the
toApprovalState conversion while ensuring pending, approved, and rejected
decisions are reflected in the work queue.
In `@apps/web/lib/api/client.ts`:
- Around line 22-36: The buildUrl function discards the computed origin for
relative paths, preventing server-side fetch from resolving the URL. Update its
non-browser return path to preserve the absolute base URL when running
server-side, while retaining browser-relative URLs if required by the existing
API behavior; ensure server-side session-cookie handling can receive a
resolvable URL.
In `@apps/web/lib/api/fixtures/capabilities.ts`:
- Around line 10-92: Keep browser fixtures fully synthetic by replacing
project-specific agent names, connector/integration identifiers, and skill paths
in the capability fixture entries with neutral synthetic values; update
apps/web/lib/api/fixtures/capabilities.ts lines 10-92 accordingly. Apply the
same replacement to project-specific agent and integration references in
apps/web/lib/api/fixtures/teams.ts lines 9-86, preserving the fixture structure
and relationships while using synthetic names and paths.
In `@apps/web/lib/command-summary-domain.ts`:
- Around line 62-81: Update the pending approvals flow around pendingApprovals
so the “Pending approvals” metric and pendingApprovalCount use an uncapped,
organisation-scoped count query rather than the limit(25) result. Keep the
existing bounded query for displaying approval records, and ensure the count
still filters to pending status and only runs when canApprove is true.
- Around line 209-224: Update the run query in the command-summary flow to
select the latest run for each agent before applying any overall row limit,
using a per-agent rank or lateral-query approach. Ensure lastByAgent receives
each agent’s actual most recent run so status, attention items, and failed-run
metrics remain accurate; do not retain the current global limit-before-grouping
behavior.
- Around line 510-513: Update the summary partial calculation near overallHealth
and pendingApprovalCount to include !canReadTasks and to mark the result partial
when neither agent-read access nor admin access is available. Preserve the
existing canAdmin, canApprove, and canReadWorkflows checks.
In `@apps/web/lib/session-domain.ts`:
- Around line 19-43: Extract the duplicated actor lookup from getSessionContext
into a shared helper such as findHumanActorByEmail, preserving the
identityReference and human actorType filters and selected fields. Update both
getSessionContext and apiSubject in api-context.ts to call this helper, keeping
their existing not-found and authorization behavior unchanged.
In `@apps/web/types/os.ts`:
- Around line 171-197: Update the exported CapabilityPack and TeamSummary types
to expose a single consistent source: DataSource provenance field, removing or
renaming CapabilityPack’s existing source: string provider-metadata field as
needed to avoid ambiguity. Preserve all other fields and ensure fixture-backed
models use the source contract expected by the UI, including source: "fixture"
for fixture adapters.
- Around line 114-136: Update MissionSummary.status and MissionRunSummary.status
to use the shared OperationalState type, or an explicit mission-state union
mapped at the boundary, instead of plain string values. Keep the remaining
fields and existing status semantics unchanged while ensuring consumers cannot
receive arbitrary untyped backend statuses.
In `@apps/web/types/status.ts`:
- Around line 64-95: Update toApprovalState and toOperationalState so
unrecognized non-empty status strings return an explicit unknown state instead
of not-required or queued, while preserving existing null/empty defaults. Add
unknown to the canonical ApprovalState and OperationalState vocabularies and
corresponding badge configuration, or enforce rejection at the API boundary if
that is the established contract.
In `@docs/architecture/0007-security-company-os-ui.md`:
- Around line 3-5: Update the ADR status date in the Status section to a date no
later than July 29, 2026, using the actual acceptance date; alternatively,
change the status to pending if acceptance has not occurred.
---
Nitpick comments:
In `@apps/web/app/page.tsx`:
- Around line 1-5: Remove the unused ControlPlaneDashboard component and its
embedded OpsShell usage, since HomePage currently renders CommandView and no
live route references the dashboard. Alternatively, wire ControlPlaneDashboard
into an active route if it is still required, ensuring no dead component
remains.
In `@apps/web/components/os/os-command-palette.tsx`:
- Around line 85-142: Update the command palette input and option markup in the
input keydown/listbox flow: give the input combobox semantics with
role="combobox", aria-expanded, aria-controls referencing the listbox, and
aria-activedescendant referencing the highlighted option. Add stable IDs to the
rendered option elements and ensure the active descendant is unset when no
option is selected, while preserving the existing keyboard selection behavior.
In `@apps/web/features/approvals/governance-inbox.test.ts`:
- Around line 1-21: The tests in “Governance inbox” only inspect source text and
do not verify runtime behavior. Replace the substring assertions with React
Testing Library tests that render GovernanceInbox, simulate selecting approvals
and approve/reject actions, validate required reasons and high-impact
confirmation, and confirm decision.mutateAsync submission; include a
selection-switch case to ensure reason and confirmation state do not leak
between approvals.
In `@apps/web/features/audit/audit-view.tsx`:
- Around line 16-32: Debounce the audit filter values used by the filters
useMemo and useAuditEvents so typing in q, action, or targetType does not
trigger a request for every keystroke. Preserve the existing undefined handling
and limit value, while ensuring the query key updates only after the debounce
interval.
In `@apps/web/lib/api/client.test.ts`:
- Around line 1-19: Expand the tests in the ApiClientError suite to cover
apiRequest and buildUrl through apiGet/apiPost with a mocked fetch. Verify 401
responses trigger the redirect behavior, other non-ok responses map to
ApiClientError, responses missing the data envelope are handled correctly, and
relative and absolute URLs are built as expected, including SSR-safe behavior.
In `@apps/web/lib/audit-domain.test.ts`:
- Around line 4-18: Replace the source-text assertions in
apps/web/lib/audit-domain.test.ts:4-18 with executable tests invoking
listAuditEvents using mocked auth and database dependencies; verify capability
denial prevents database access, queries include the subject organisation
predicate, and returned metadata is redacted. Replace the source-text assertions
in apps/web/lib/mission-web-domain.test.ts:4-18 with executable tests invoking
the mission functions using mocked dependencies; verify denied access prevents
querying and every query is scoped to the subject organisation.
In `@apps/web/lib/mission-web-domain.ts`:
- Around line 44-57: Extract the shared limit validation used by listWebMissions
and listWebMissionRuns into a small helper, replacing duplicated
z.coerce.number().int().min(1).max(100).parse logic. Have the helper use
safeParse and throw an ApiProblem with a clean 400 message when the value is
invalid, while preserving the default limit of 50 and valid range of 1–100.
In `@apps/web/types/status.test.ts`:
- Around line 8-29: Expand the “status vocabulary mappers” tests with
table-driven cases covering every alias and fallback branch in toHealthState,
toApprovalState, and toOperationalState, including configured, waiting,
suspended, cancelled/canceled, expired, approval-waiting states, archived, null,
undefined, empty inputs, and unknown values. Assert each input maps to its
expected normalized output while preserving the existing cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a5022c5-49de-4066-bee2-93a041b87078
📒 Files selected for processing (57)
DESIGN.mdPRODUCT.mdapps/web/app/api/v1/audit/events/route.tsapps/web/app/api/v1/command/summary/route.tsapps/web/app/api/v1/missions/[id]/route.tsapps/web/app/api/v1/missions/[id]/runs/route.tsapps/web/app/api/v1/missions/route.tsapps/web/app/api/v1/session/me/route.tsapps/web/app/approvals/page.tsxapps/web/app/audit/page.tsxapps/web/app/capabilities/page.tsxapps/web/app/integrations/page.tsxapps/web/app/layout.tsxapps/web/app/missions/[id]/page.tsxapps/web/app/missions/page.tsxapps/web/app/operations/page.tsxapps/web/app/page.tsxapps/web/app/teams/page.tsxapps/web/components/branding.test.tsapps/web/components/control-plane-dashboard.tsxapps/web/components/ops-shell.tsxapps/web/components/os/company-os-shell.test.tsapps/web/components/os/company-os-shell.tsxapps/web/components/os/empty-state.tsxapps/web/components/os/error-state.tsxapps/web/components/os/metric-tile.tsxapps/web/components/os/os-command-palette.tsxapps/web/components/os/skeleton.tsxapps/web/components/status/status-badges.tsxapps/web/features/approvals/governance-inbox.test.tsapps/web/features/approvals/governance-inbox.tsxapps/web/features/audit/audit-view.tsxapps/web/features/capabilities/capabilities-view.tsxapps/web/features/command/command-view.tsxapps/web/features/integrations/integrations-view.tsxapps/web/features/missions/mission-detail-view.tsxapps/web/features/missions/missions-view.tsxapps/web/features/operations/operations-view.tsxapps/web/features/teams/teams-view.tsxapps/web/lib/api/client.test.tsapps/web/lib/api/client.tsapps/web/lib/api/fixtures/capabilities.tsapps/web/lib/api/fixtures/fixtures.test.tsapps/web/lib/api/fixtures/teams.tsapps/web/lib/audit-domain.test.tsapps/web/lib/audit-domain.tsapps/web/lib/command-summary-domain.tsapps/web/lib/mission-web-domain.test.tsapps/web/lib/mission-web-domain.tsapps/web/lib/queries/hooks.tsapps/web/lib/queries/keys.tsapps/web/lib/session-domain.tsapps/web/types/os.tsapps/web/types/status.test.tsapps/web/types/status.tsdocs/architecture/0007-security-company-os-ui.mddocs/plans/security-company-os-foundation.md
Comment on lines
+337
to
+397
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon" | ||
| aria-label="Toggle theme" | ||
| onClick={toggleTheme} | ||
| > | ||
| {theme === "light" ? ( | ||
| <Moon className="size-4" /> | ||
| ) : ( | ||
| <Sun className="size-4" /> | ||
| )} | ||
| </Button> | ||
|
|
||
| <div className="hidden items-center gap-2 border-l border-border pl-2 tablet:flex"> | ||
| <div className="text-right"> | ||
| <p className="max-w-[10rem] truncate text-xs font-medium"> | ||
| {actor?.displayName ?? "Operator"} | ||
| </p> | ||
| <p className="max-w-[10rem] truncate text-xs text-muted-foreground"> | ||
| {actor?.email ?? "—"} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon" | ||
| aria-label="Sign out" | ||
| onClick={() => { | ||
| void authClient.signOut().then(() => router.push("/login")); | ||
| }} | ||
| > | ||
| <LogOut className="size-4" /> | ||
| </Button> | ||
| </div> | ||
|
|
||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon" | ||
| className="tablet:hidden" | ||
| aria-label="Sign out" | ||
| onClick={() => { | ||
| void authClient.signOut().then(() => router.push("/login")); | ||
| }} | ||
| > | ||
| <LogOut className="size-4" /> | ||
| </Button> | ||
|
|
||
| {mobileOpen ? ( | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon" | ||
| className="desktop:hidden" | ||
| aria-label="Close navigation" | ||
| onClick={() => setMobileOpen(false)} | ||
| > | ||
| <X className="size-4" /> | ||
| </Button> | ||
| ) : null} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sign-out lacks error handling and is duplicated.
authClient.signOut().then(() => router.push("/login")) has no .catch() in either the desktop (L365-367) or mobile (L379-381) button. If signOut() rejects (network error), the promise is silently swallowed, the user isn't navigated away, sees no feedback, and may believe they've signed out on a shared device while the session persists.
🔒 Proposed fix: extract a single handler with error handling
+ function handleSignOut() {
+ void authClient
+ .signOut()
+ .catch((error) => {
+ console.error("Sign-out failed", error);
+ })
+ .finally(() => {
+ router.push("/login");
+ });
+ }
+
return (Then replace both onClick={() => { void authClient.signOut().then(() => router.push("/login")); }} occurrences with onClick={handleSignOut}.
📝 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
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| aria-label="Toggle theme" | |
| onClick={toggleTheme} | |
| > | |
| {theme === "light" ? ( | |
| <Moon className="size-4" /> | |
| ) : ( | |
| <Sun className="size-4" /> | |
| )} | |
| </Button> | |
| <div className="hidden items-center gap-2 border-l border-border pl-2 tablet:flex"> | |
| <div className="text-right"> | |
| <p className="max-w-[10rem] truncate text-xs font-medium"> | |
| {actor?.displayName ?? "Operator"} | |
| </p> | |
| <p className="max-w-[10rem] truncate text-xs text-muted-foreground"> | |
| {actor?.email ?? "—"} | |
| </p> | |
| </div> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| aria-label="Sign out" | |
| onClick={() => { | |
| void authClient.signOut().then(() => router.push("/login")); | |
| }} | |
| > | |
| <LogOut className="size-4" /> | |
| </Button> | |
| </div> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| className="tablet:hidden" | |
| aria-label="Sign out" | |
| onClick={() => { | |
| void authClient.signOut().then(() => router.push("/login")); | |
| }} | |
| > | |
| <LogOut className="size-4" /> | |
| </Button> | |
| {mobileOpen ? ( | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| className="desktop:hidden" | |
| aria-label="Close navigation" | |
| onClick={() => setMobileOpen(false)} | |
| > | |
| <X className="size-4" /> | |
| </Button> | |
| ) : null} | |
| function handleSignOut() { | |
| void authClient | |
| .signOut() | |
| .catch((error) => { | |
| console.error("Sign-out failed", error); | |
| }) | |
| .finally(() => { | |
| router.push("/login"); | |
| }); | |
| } | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| aria-label="Toggle theme" | |
| onClick={toggleTheme} | |
| > | |
| {theme === "light" ? ( | |
| <Moon className="size-4" /> | |
| ) : ( | |
| <Sun className="size-4" /> | |
| )} | |
| </Button> | |
| <div className="hidden items-center gap-2 border-l border-border pl-2 tablet:flex"> | |
| <div className="text-right"> | |
| <p className="max-w-[10rem] truncate text-xs font-medium"> | |
| {actor?.displayName ?? "Operator"} | |
| </p> | |
| <p className="max-w-[10rem] truncate text-xs text-muted-foreground"> | |
| {actor?.email ?? "—"} | |
| </p> | |
| </div> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| aria-label="Sign out" | |
| onClick={handleSignOut} | |
| > | |
| <LogOut className="size-4" /> | |
| </Button> | |
| </div> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| className="tablet:hidden" | |
| aria-label="Sign out" | |
| onClick={handleSignOut} | |
| > | |
| <LogOut className="size-4" /> | |
| </Button> | |
| {mobileOpen ? ( | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="icon" | |
| className="desktop:hidden" | |
| aria-label="Close navigation" | |
| onClick={() => setMobileOpen(false)} | |
| > | |
| <X className="size-4" /> | |
| </Button> | |
| ) : null} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/os/company-os-shell.tsx` around lines 337 - 397, Extract
the duplicated sign-out logic into a shared handleSignOut handler in the
component, and use it for both desktop and mobile sign-out buttons. Have the
handler navigate to /login on successful authClient.signOut(), and catch
rejected sign-out promises to provide user-visible error feedback without
leaving the failure unhandled.
Comment on lines
+38
to
+42
| const className = cn( | ||
| "block rounded-md border bg-card p-3 transition-colors", | ||
| tone, | ||
| metric.href && "hover:bg-muted/40 focus-visible:outline-none", | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing focus indicator on linked metric tiles.
focus-visible:outline-none is applied whenever metric.href is set, but no replacement focus style (ring/box-shadow) is added, so keyboard users tabbing to a metric link get no visible focus indicator.
♿ Proposed fix: restore a visible focus ring
const className = cn(
"block rounded-md border bg-card p-3 transition-colors",
tone,
- metric.href && "hover:bg-muted/40 focus-visible:outline-none",
+ metric.href &&
+ "hover:bg-muted/40 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
);📝 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
| const className = cn( | |
| "block rounded-md border bg-card p-3 transition-colors", | |
| tone, | |
| metric.href && "hover:bg-muted/40 focus-visible:outline-none", | |
| ); | |
| const className = cn( | |
| "block rounded-md border bg-card p-3 transition-colors", | |
| tone, | |
| metric.href && | |
| "hover:bg-muted/40 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/os/metric-tile.tsx` around lines 38 - 42, Update the
linked metric tile styling in the className construction so metric.href retains
a visible focus indicator for keyboard users; replace the
focus-visible:outline-none behavior with the project’s established focus ring or
equivalent visible focus style, while preserving the existing hover styling and
non-linked tile appearance.
Comment on lines
+22
to
+29
| function riskSeverity(summary: string): "medium" | "high" | "critical" { | ||
| const lower = summary.toLowerCase(); | ||
| if (lower.includes("isolate") || lower.includes("disable") || lower.includes("delete")) | ||
| return "critical"; | ||
| if (lower.includes("modify") || lower.includes("publish") || lower.includes("enrich")) | ||
| return "high"; | ||
| return "medium"; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'confirmHighImpact|highImpact|high[-_ ]?impact' apps/web/lib apps/web/app/api -iRepository: jusso-dev/Muster
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate governance inbox =="
fd -a 'governance-inbox\.tsx$' . | sed 's#^\./##'
echo
echo "== file outline =="
ast-grep outline apps/web/features/approvals/governance-inbox.tsx --view expanded || true
echo
echo "== relevant governance inbox lines =="
cat -n apps/web/features/approvals/governance-inbox.tsx | sed -n '1,120p'
echo
echo "== all repo occurrences of decision payload and approval mutations =="
rg -n "decision\.mutateAsync|status.*reason|riskSummary|riskSeverity|decision\.mutate|mutateAsync\\(\\{ id, status|confirmHighImpact|highImpact|high[-_ ]?impact" apps/web -i || true
echo
echo "== approval route files =="
fd -a '.*' apps/web/app/api | rg 'approval|approvals' | sed 's#^\./##' | head -100Repository: jusso-dev/Muster
Length of output: 9920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== API route decision =="
cat -n apps/web/app/api/v1/approvals/[id]/decisions/route.ts | sed -n '1,220p'
echo
echo "== API route list =="
for f in apps/web/app/api/v1/approvals/route.ts apps/web/app/api/v1/approvals/\[id\]/route.ts; do
echo "--- $f"
cat -n "$f" | sed -n '1,220p'
done
echo
echo "== approval hooks =="
cat -n apps/web/lib/queries/hooks.ts | sed -n '1,120p'
echo
echo "== app config hooks and request callers =="
rg -n "fetch\\(|fetchClient|/api/v1/approvals|/approvals|decision|approve|reject" apps/web/lib apps/web/components apps/web/features apps/web/app -i || trueRepository: jusso-dev/Muster
Length of output: 1699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate decision implementation =="
rg -n "class ApprovalDomainService|decide\\(|decide\\(" apps/web/lib -i
echo
echo "== approval domain service =="
fd -a 'integration-action-domain\.ts$' . | sed 's#^\./##'
echo
echo "== decision implementation excerpt =="
file="$(fd 'integration-action-domain\.ts$' . | head -1)"
wc -l "$file"
rg -n "decide\\(|approval|decision|riskSummary|highImpact|confirm|approved|rejected" "$file" -C 8
echo
echo "== domain file excerpt around decisions =="
python3 - <<'PY'
from pathlib import Path
p = next(Path('.').rglob('integration-action_DOMAIN.ts')) if False else next(Path('.').rglob('integration-action-domain.ts'))
text = p.read_text()
checks = ["class ApprovalDomainService", "async decide", "approve", "reject"]
for check in checks:
i = text.find(check)
print(f"\n--- first occurrence: {check!r} at {i} ---")
if i >= 0:
start = max(0, text.rfind("\nexport", 0, i))
end = text.find("\nexport", i + 1)
print(text[start:end if end >= 0 else i + 6000])
PYRepository: jusso-dev/Muster
Length of output: 34876
Enforce high-impact confirmation server-side, not just in the browser.
The client checkbox gates only client-side decision.mutateAsync({ id, status, reason }); it never sends confirmHighImpact, and POST /api/v1/approvals/:id/decisions only validates { status, reason } before appending the decision. Move high-impact approval requirements into the domain/service decision flow so direct API requests cannot bypass the confirmation and create an auditable approval without server-side evidence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/features/approvals/governance-inbox.tsx` around lines 22 - 29, Move
high-impact confirmation enforcement from the browser’s riskSeverity flow into
the server-side approval decision domain/service used by POST
/api/v1/approvals/:id/decisions. Extend request validation and decision creation
to require and persist auditable confirmHighImpact evidence for high-impact
approvals, while preserving normal approval behavior for lower-risk decisions
and rejecting direct API requests that omit confirmation.
Source: Coding guidelines
Comment on lines
+36
to
+39
| const [selectedId, setSelectedId] = useState<string | null>(focusId); | ||
| const [reason, setReason] = useState(""); | ||
| const [confirmHighImpact, setConfirmHighImpact] = useState(false); | ||
| const [message, setMessage] = useState(""); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reason/confirmation state leaks across approval selections.
Selecting a different approval via setSelectedId(row.id) (Line 138) doesn't reset reason, confirmHighImpact, or message. A reason typed for approval A, or a checked "high-impact reviewed" box for approval A, silently carries over to approval B if the user switches selection before submitting — defeating the per-action confirmation this workflow is meant to enforce and risking an incorrect reason being written to the audit trail.
🔒 Proposed fix: reset decision state on selection change
<button
type="button"
- onClick={() => setSelectedId(row.id)}
+ onClick={() => {
+ setSelectedId(row.id);
+ setReason("");
+ setConfirmHighImpact(false);
+ setMessage("");
+ }}
className={`w-full px-3 py-3 text-left hover:bg-muted/50 ${As per coding guidelines, "Preserve append-only messages, timelines, evidence metadata, and audit events."
Also applies to: 134-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/features/approvals/governance-inbox.tsx` around lines 36 - 39, Reset
the per-approval decision state whenever selection changes through setSelectedId
in the governance inbox: clear reason, uncheck confirmHighImpact, and clear
message when switching approvals. Preserve append-only messages, timelines,
evidence metadata, and audit events; only reset the transient decision fields
associated with the selected approval.
Source: Coding guidelines
Comment on lines
+19
to
+20
| const [expandedId, setExpandedId] = useState<string | null>(null); | ||
| const [selected, setSelected] = useState<AuditEventSummary | null>(null); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset selected/expandedId when filters change or data refreshes.
selected and expandedId are never cleared on filter changes (including "Clear filters") or on audit.data refresh, so the detail aside can show an event from a previous, now-stale, result set once the new page of records loads.
🐛 Reset stale selection on filter change
+ useEffect(() => {
+ setSelected(null);
+ setExpandedId(null);
+ }, [filters]);Also applies to: 87-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/features/audit/audit-view.tsx` around lines 19 - 20, Reset both
selected and expandedId whenever the audit filters change or audit.data
refreshes, including clearing filters, so the detail aside cannot retain records
from a stale result set. Update the relevant filter-change and data-refresh
logic in the audit view while preserving normal selection behavior for the
current results.
Comment on lines
+19
to
+43
| const db = database(); | ||
| const [actor] = await db | ||
| .select({ | ||
| id: schema.actors.id, | ||
| displayName: schema.actors.displayName, | ||
| organisationId: schema.actors.organisationId, | ||
| actorType: schema.actors.actorType, | ||
| capabilityAssignments: schema.actors.capabilityAssignments, | ||
| identityReference: schema.actors.identityReference, | ||
| }) | ||
| .from(schema.actors) | ||
| .where( | ||
| and( | ||
| eq(schema.actors.identityReference, session.user.email), | ||
| eq(schema.actors.actorType, "human"), | ||
| ), | ||
| ) | ||
| .limit(1); | ||
|
|
||
| if (!actor) | ||
| throw new ApiProblem( | ||
| 403, | ||
| "Forbidden", | ||
| "No organisation actor is linked to this account.", | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Actor-resolution logic duplicated with api-context.ts's apiSubject().
This block (session lookup → identityReference/actorType="human" filter) is nearly identical to apiSubject() in apps/web/lib/api-context.ts (see context snippet). Two independent implementations of security-critical actor resolution risk silent divergence (e.g. one adds an org-status check or capability filter tweak the other misses).
Consider extracting a shared helper (e.g. findHumanActorByEmail(email)) used by both apiSubject() and getSessionContext().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/session-domain.ts` around lines 19 - 43, Extract the duplicated
actor lookup from getSessionContext into a shared helper such as
findHumanActorByEmail, preserving the identityReference and human actorType
filters and selected fields. Update both getSessionContext and apiSubject in
api-context.ts to call this helper, keeping their existing not-found and
authorization behavior unchanged.
Comment on lines
+114
to
+136
| export type MissionSummary = { | ||
| id: string; | ||
| name: string; | ||
| description: string; | ||
| status: string; | ||
| capabilityEnvelope: string[]; | ||
| scheduleHint: string | null; | ||
| hermesProfile: string | null; | ||
| killSwitch: boolean; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| }; | ||
|
|
||
| export type MissionRunSummary = { | ||
| id: string; | ||
| missionId: string; | ||
| status: string; | ||
| idempotencyKey: string; | ||
| hermesProfile: string | null; | ||
| error: string | null; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Type mission states against the shared status contract.
MissionSummary.status and MissionRunSummary.status are plain string values (Lines 118 and 130), so arbitrary backend statuses can bypass the canonical vocabulary and reach consumers unnormalized. Use OperationalState, or define an explicit mission-state union and map it at the boundary instead of exposing an untyped string.
Suggested contract change
export type MissionSummary = {
id: string;
name: string;
description: string;
- status: string;
+ status: OperationalState;
...
};
export type MissionRunSummary = {
id: string;
missionId: string;
- status: string;
+ status: OperationalState;
...
};📝 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
| export type MissionSummary = { | |
| id: string; | |
| name: string; | |
| description: string; | |
| status: string; | |
| capabilityEnvelope: string[]; | |
| scheduleHint: string | null; | |
| hermesProfile: string | null; | |
| killSwitch: boolean; | |
| createdAt: string; | |
| updatedAt: string; | |
| }; | |
| export type MissionRunSummary = { | |
| id: string; | |
| missionId: string; | |
| status: string; | |
| idempotencyKey: string; | |
| hermesProfile: string | null; | |
| error: string | null; | |
| createdAt: string; | |
| updatedAt: string; | |
| }; | |
| export type MissionSummary = { | |
| id: string; | |
| name: string; | |
| description: string; | |
| status: OperationalState; | |
| capabilityEnvelope: string[]; | |
| scheduleHint: string | null; | |
| hermesProfile: string | null; | |
| killSwitch: boolean; | |
| createdAt: string; | |
| updatedAt: string; | |
| }; | |
| export type MissionRunSummary = { | |
| id: string; | |
| missionId: string; | |
| status: OperationalState; | |
| idempotencyKey: string; | |
| hermesProfile: string | null; | |
| error: string | null; | |
| createdAt: string; | |
| updatedAt: string; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/types/os.ts` around lines 114 - 136, Update MissionSummary.status
and MissionRunSummary.status to use the shared OperationalState type, or an
explicit mission-state union mapped at the boundary, instead of plain string
values. Keep the remaining fields and existing status semantics unchanged while
ensuring consumers cannot receive arbitrary untyped backend statuses.
Comment on lines
+171
to
+197
| export type CapabilityPack = { | ||
| id: string; | ||
| name: string; | ||
| description: string; | ||
| version: string; | ||
| source: string; | ||
| category: string; | ||
| installed: boolean; | ||
| enabled: boolean; | ||
| validationStatus: "valid" | "invalid" | "unknown"; | ||
| requiredConnectors: string[]; | ||
| allowedAgentRoles: string[]; | ||
| approvalRequired: boolean; | ||
| dataClassification: string; | ||
| origin: DataSource; | ||
| }; | ||
|
|
||
| export type TeamSummary = { | ||
| id: string; | ||
| name: string; | ||
| purpose: string; | ||
| memberCount: number; | ||
| agentCount: number; | ||
| activeMissions: number; | ||
| workload: number; | ||
| origin: DataSource; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Standardize fixture provenance in the exported types.
CapabilityPack exposes both source: string and origin: DataSource, while TeamSummary uses origin; this does not enforce the documented source: "fixture" | "api" contract and makes provenance easy to confuse with provider metadata. Rename the provider field if necessary, but expose one consistent source: DataSource field on fixture-backed models.
Suggested contract change
export type CapabilityPack = {
...
- source: string;
+ provider: string;
...
- origin: DataSource;
+ source: DataSource;
};
export type TeamSummary = {
...
- origin: DataSource;
+ source: DataSource;
};As per coding guidelines, fixture adapters must be labelled source: fixture in UI and types.
📝 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
| export type CapabilityPack = { | |
| id: string; | |
| name: string; | |
| description: string; | |
| version: string; | |
| source: string; | |
| category: string; | |
| installed: boolean; | |
| enabled: boolean; | |
| validationStatus: "valid" | "invalid" | "unknown"; | |
| requiredConnectors: string[]; | |
| allowedAgentRoles: string[]; | |
| approvalRequired: boolean; | |
| dataClassification: string; | |
| origin: DataSource; | |
| }; | |
| export type TeamSummary = { | |
| id: string; | |
| name: string; | |
| purpose: string; | |
| memberCount: number; | |
| agentCount: number; | |
| activeMissions: number; | |
| workload: number; | |
| origin: DataSource; | |
| }; | |
| export type CapabilityPack = { | |
| id: string; | |
| name: string; | |
| description: string; | |
| version: string; | |
| source: string; | |
| category: string; | |
| installed: boolean; | |
| enabled: boolean; | |
| validationStatus: "valid" | "invalid" | "unknown"; | |
| requiredConnectors: string[]; | |
| allowedAgentRoles: string[]; | |
| approvalRequired: boolean; | |
| dataClassification: string; | |
| origin: DataSource; | |
| }; | |
| export type TeamSummary = { | |
| id: string; | |
| name: string; | |
| purpose: string; | |
| memberCount: number; | |
| agentCount: number; | |
| activeMissions: number; | |
| workload: number; | |
| origin: DataSource; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/types/os.ts` around lines 171 - 197, Update the exported
CapabilityPack and TeamSummary types to expose a single consistent source:
DataSource provenance field, removing or renaming CapabilityPack’s existing
source: string provider-metadata field as needed to avoid ambiguity. Preserve
all other fields and ensure fixture-backed models use the source contract
expected by the UI, including source: "fixture" for fixture adapters.
Source: Coding guidelines
Comment on lines
+64
to
+95
| /** Map approval row status into ApprovalState. */ | ||
| export function toApprovalState(value: string | null | undefined): ApprovalState { | ||
| if (!value) return "not-required"; | ||
| const v = value.toLowerCase(); | ||
| if (v === "pending") return "pending"; | ||
| if (v === "approved") return "approved"; | ||
| if (v === "rejected") return "rejected"; | ||
| if (v === "expired") return "expired"; | ||
| if (v === "cancelled" || v === "canceled") return "cancelled"; | ||
| return "not-required"; | ||
| } | ||
|
|
||
| /** Map task / run status into OperationalState. */ | ||
| export function toOperationalState( | ||
| value: string | null | undefined, | ||
| ): OperationalState { | ||
| if (!value) return "queued"; | ||
| const v = value.toLowerCase(); | ||
| if (v === "backlog" || v === "todo" || v === "open" || v === "queued") | ||
| return "queued"; | ||
| if (v === "in_progress" || v === "running" || v === "active") return "running"; | ||
| if (v === "waiting" || v === "awaiting_approval" || v === "blocked_on_approval") | ||
| return "waiting"; | ||
| if (v === "blocked") return "blocked"; | ||
| if (v === "review" || v === "in_review") return "review"; | ||
| if (v === "done" || v === "completed" || v === "closed" || v === "resolved") | ||
| return "completed"; | ||
| if (v === "failed" || v === "error") return "failed"; | ||
| if (v === "cancelled" || v === "canceled" || v === "archived") | ||
| return "cancelled"; | ||
| return "queued"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not collapse unknown states into actionable states.
toApprovalState maps any unrecognized non-empty value to not-required, which can hide an approval requirement. toOperationalState maps unrecognized values to queued, falsely presenting invalid or new backend states as queued work. Preserve an explicit unknown state in the canonical vocabulary and badge configuration, or reject these values at the API boundary; keep null/empty handling separate from unknown strings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/types/status.ts` around lines 64 - 95, Update toApprovalState and
toOperationalState so unrecognized non-empty status strings return an explicit
unknown state instead of not-required or queued, while preserving existing
null/empty defaults. Add unknown to the canonical ApprovalState and
OperationalState vocabularies and corresponding badge configuration, or enforce
rejection at the API boundary if that is the established contract.
Comment on lines
+3
to
+5
| ## Status | ||
|
|
||
| Accepted (2026-07-30) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a non-future acceptance date.
This ADR is marked Accepted (2026-07-30), but the current date is July 29, 2026. Use the actual acceptance date or keep the ADR pending until it is accepted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/0007-security-company-os-ui.md` around lines 3 - 5, Update
the ADR status date in the Status section to a date no later than July 29, 2026,
using the actual acceptance date; alternatively, change the status to pending if
acceptance has not occurred.
This was referenced Jul 29, 2026
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
1 participant
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
First foundation pass transforming Muster web into a Security Company OS shell while preserving PostgreSQL authority, org scoping, approvals, audit, and ADR 0006 (chat stays in Slack).
CompanyOsShellwith Command, Operations, Missions, Teams, Agents, Capabilities, Approvals, Audit, Integrations, Settingssession/me,command/summary,missions,audit/eventsAPIs added
GET /api/v1/session/meGET /api/v1/command/summaryGET /api/v1/missions(+ detail/runs)GET /api/v1/audit/eventsMigrations
None.
Test plan
apps/webunit tests (87 pass)apps/webtypecheckScreenshots
Pending live capture after deploy (dark OS shell across primary nav).
Security
ApprovalDomainServiceSummary by CodeRabbit