> For the complete documentation index, see [llms.txt](https://faction-os.gitbook.io/faction-os-docs/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://faction-os.gitbook.io/faction-os-docs/.spec_system/archive/sessions/phase19-session15-metrics-and-notifications/code-review.md). # Code Review and Repair Report **Session ID**: `phase19-session15-metrics-and-notifications` **Package**: cross-cutting **Reviewed**: 2026-06-28 **Scope**: All uncommitted changes in the working tree **Result**: RESOLVED ## Review Surface **Files reviewed** (all uncommitted changes before this report was created): * `.spec_system/state.json` - tracked-modified * `.spec_system/specs/phase19-session15-metrics-and-notifications/implementation-notes.md` - untracked * `.spec_system/specs/phase19-session15-metrics-and-notifications/security-compliance.md` - untracked * `.spec_system/specs/phase19-session15-metrics-and-notifications/spec.md` - untracked * `.spec_system/specs/phase19-session15-metrics-and-notifications/tasks.md` - untracked * `apps/server/README_server.md` - tracked-modified * `apps/server/src/lib/claudeSessionImport.ts` - tracked-modified * `apps/server/src/lib/codexSessionImport.ts` - tracked-modified * `apps/server/src/lib/commandCenterValidation.ts` - tracked-modified * `apps/server/src/lib/localSessionImport.ts` - tracked-modified * `apps/server/src/lib/missionArtifacts.ts` - tracked-modified * `apps/server/src/lib/notificationRouting.ts` - untracked * `apps/server/src/lib/startupSessionImport.ts` - tracked-modified * `apps/server/src/managers/orchestrationCommandCenter.ts` - tracked-modified * `apps/server/src/managers/usageMetricsManager.ts` - untracked * `apps/server/src/routes/commandCenter.ts` - tracked-modified * `apps/server/src/routes/commandCenterNotifications.ts` - untracked * `apps/server/src/server.ts` - tracked-modified * `apps/server/tests/claudeSessionImport.test.ts` - tracked-modified * `apps/server/tests/codexSessionImport.test.ts` - tracked-modified * `apps/server/tests/commandCenterManager.test.ts` - tracked-modified * `apps/server/tests/commandCenterNotifications.test.ts` - untracked * `apps/server/tests/commandCenterRoutes.test.ts` - tracked-modified * `apps/server/tests/notificationRouting.test.ts` - untracked * `apps/server/tests/usageMetricsManager.test.ts` - untracked * `apps/web/README_web.md` - tracked-modified * `apps/web/src/components/SettingsDrawer.tsx` - tracked-modified * `apps/web/src/components/orchestration/AttentionWorkbench.tsx` - tracked-modified * `apps/web/src/components/orchestration/CommandCenterPanes.tsx` - tracked-modified * `apps/web/src/components/orchestration/MetricsAndNotificationsPanel.tsx` - untracked * `apps/web/src/components/orchestration/OrchestrationHeader.tsx` - tracked-modified * `apps/web/src/components/orchestration/OrchestrationShell.tsx` - tracked-modified * `apps/web/src/components/orchestration/OrchestrationTabs.tsx` - tracked-modified * `apps/web/src/lib/commandCenterMetrics.ts` - untracked * `apps/web/src/lib/commandCenterNotifications.ts` - untracked * `apps/web/src/lib/commandCenterUi.ts` - tracked-modified * `apps/web/src/lib/hostedOperations.ts` - tracked-modified * `apps/web/src/lib/notifications.ts` - tracked-modified * `apps/web/src/lib/orchestrationApi.ts` - tracked-modified * `apps/web/src/lib/useNotifications.ts` - tracked-modified * `apps/web/src/store/useGameStore.ts` - tracked-modified * `apps/web/src/store/useWsClient.ts` - tracked-modified * `apps/web/tests/MetricsAndNotificationsPanel.test.tsx` - untracked * `apps/web/tests/OrchestrationPanel.test.tsx` - tracked-modified * `apps/web/tests/SettingsHostedAnalytics.test.tsx` - tracked-modified * `apps/web/tests/commandCenterMetrics.test.ts` - untracked * `apps/web/tests/commandCenterNotifications.test.ts` - untracked * `apps/web/tests/commandCenterStore.test.ts` - tracked-modified * `apps/web/tests/hostedOperations.test.ts` - tracked-modified * `apps/web/tests/notifications.test.ts` - tracked-modified * `apps/web/tests/orchestrationApi.test.ts` - tracked-modified * `apps/web/tests/useNotifications.test.tsx` - tracked-modified * `docs/api/README_api.md` - tracked-modified * `packages/protocol/README_protocol.md` - tracked-modified * `packages/protocol/src/orchestrationCommandCenter.ts` - tracked-modified * `packages/protocol/tests/events.test.ts` - tracked-modified * `packages/protocol/tests/orchestrationCommandCenter.test.ts` - tracked-modified **Inventory commands**: `git status --short`, `git diff HEAD`, `git diff --cached`, `git ls-files --others --exclude-standard` ## Findings by Severity ### Critical * No findings. ### High * No findings. ### Medium * `apps/server/src/lib/notificationRouting.ts` - `validateWebPushSubscription` accepted finite but out-of-range `expirationTime` values and then called `new Date(expirationTime).toISOString()`, which could throw `RangeError` and turn a malformed subscribe request into an uncontrolled server failure. Fix: added non-negative Date-range bounds before formatting and added helper tests for out-of-range and negative timestamps. Status: FIXED. ### Low * `apps/server/src/routes/commandCenterNotifications.ts` - subscribe and unsubscribe routes saved Web Push readiness with generic subscription states such as `ready` or `consent_required` instead of the action-specific `subscribed` or `unsubscribed` state. API callers could infer the top-level status, but WebSocket-only clients and readiness cards could remain semantically stale. Fix: overrode the Web Push readiness subscription status on subscribe, unsubscribe, and stored-subscription reads; extended route tests for response and WebSocket frames. Status: FIXED. ## Assumptions and Deliberate Non-Fixes * Existing non-ASCII characters in `apps/web/src/lib/useNotifications.ts` and `apps/web/tests/useNotifications.test.tsx` predate this session. The added-line non-ASCII scan is clean, and untracked session files are ASCII-only. I left the pre-existing comment arrows and achievement glyph unchanged to avoid unrelated churn outside the changed hunks. * The web `Subscribe` action remains an unavailable guardrail by design. The session explicitly preserves inactive hosted Web Push delivery by default, and the implemented API client plus server route can validate safe subscription summaries without the UI claiming active delivery. ## Behavior Changes * Invalid Web Push `expirationTime` values now return a bounded validation error instead of risking an uncontrolled server exception. * Web Push readiness records now surface `subscriptionStatus: "subscribed"` after subscribe and `subscriptionStatus: "unsubscribed"` after unsubscribe while keeping `pushDeliveryActive: false`. ## Verification * Tests: `npm test -- notificationRouting commandCenterNotifications` - PASS - 3 files passed, 9 tests passed. * Tests: `npm test` - PASS - 259 files passed, 1 skipped; 2945 tests passed, 1 skipped. * Type checker: `npm --workspace packages/protocol run typecheck` - PASS. * Type checker: `npm --workspace apps/server run typecheck` - PASS. * Type checker: `npm --workspace apps/web run typecheck` - PASS. * Formatter: `npm run format:check` - PASS - Biome checked 747 files with no fixes needed. * Linter: `npm run lint` - PASS - Biome checked 749 files with no fixes needed. * Whitespace: `git diff --check` - PASS. * ASCII: added-line non-ASCII scan over `git diff --unified=0 HEAD` - PASS. * LF: changed and untracked CRLF scan - PASS. * Untracked ASCII: untracked-file non-ASCII scan - PASS. * Final diff re-read: no remaining session-introduced issues found. ## Summary 1. Reviewed 57 implementation, documentation, test, and spec files before creating this report; this report is the additional creview artifact. 2. Findings: 0 critical, 0 high, 1 medium, 1 low; all resolved with focused tests. 3. Deliberate non-fixes are limited to pre-existing non-ASCII content outside added lines and the intentional inactive Web Push delivery posture. 4. Verification passed across focused tests, full Vitest, protocol/server/web typechecks, Biome format/lint, whitespace, ASCII, and LF checks. --- # Agent Instructions This documentation is published with GitBook. GitBook is the documentation platform designed so that both humans and AI agents can read, navigate, and reason over technical content effectively. Learn more at gitbook.com. ## Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter, and the optional `goal` query parameter: ``` GET https://faction-os.gitbook.io/faction-os-docs/.spec_system/archive/sessions/phase19-session15-metrics-and-notifications/code-review.md?ask=&goal= ``` `ask` is the immediate question: it should be specific, self-contained, and written in natural language. `goal` is optional and describes the broader end goal you are ultimately trying to accomplish on behalf of the user. GitBook uses it to tailor the answer towards what is most useful for that goal. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.