Quality, security, and accessibility pass#2
Merged
Conversation
Multi-agent review of the codebase surfaced concrete fixes across four of the judged dimensions. Verified with typecheck, production build, 62 unit tests, and runtime probes of the security headers and validation. Security - next.config: add security headers (X-Frame-Options, nosniff, Referrer-Policy, Permissions-Policy) + a production Content-Security-Policy that allows the Google Maps hosts. - demand route: validate `attendance` before use (was an unguarded toLocaleString() outside the try -> unhandled 500 on any malformed body). - sim route: reject non-finite tickMs/toMinute (prevented setInterval(NaN)). - briefing route: normalize incidents/recentEvents arrays before use. - tasks route: bound title/detail length; gemini: guard JSON.parse. Code quality - Extract shared helpers: nextPhase (phase folding), clampPriority, worstDensityPct/insideEstimate; single-source STAFF_ROLES and SUPPLY_ITEMS (removed 4 duplicated role lists + 2 supply lists across routes). - Remove dead code (engine arrivalsLeft), name magic numbers (MIN/MAX_TICK_MS, CATCHUP_EVENT_COUNT, SIM_TICK_FAST_MS, DENSITY_OVERRIDE_DECAY_MIN, MAX_HISTORY), rename GateState.queue -> queueLength, replace useReducerConnected with useState, fix stale gemini comment, mark the reserved OpsSnapshot stream variant. - Components: dedupe congestionColor -> CONGESTION_COLOR, use DENSITY_ALERT_PCT instead of a literal 85, derive sim-speed options from constants. Accessibility (WCAG) - New accessible Modal (role=dialog, aria-modal, focus trap, Escape, focus restore) now backs all three ops overlays. - role=alert on the evacuation banner; role=status live regions on connection status; role=log on the incident queue; aria-live on early warnings; one <h1> per page; aria-labels on the copilot input and icon-only close; traffic-advisory severity no longer color-only; prefers-reduced-motion honored. React correctness - voice-radio: hold the recognizer in a ref and abort on unmount (was a leaked mic session + setState-after-unmount). - venue-map: tear down map overlays on unmount. - weather fetch effects: guard setState against unmount. Testing - +19 tests (now 62): nextPhase, clampPriority/STAFF_ROLES, and the client dashReducer + worstDensityPct/insideEstimate derivations.
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.
Multi-agent review of the codebase (AI/API layer, simulator/lib core, React components, plus dedicated accessibility and security audits) turned up concrete fixes across four of the six judged dimensions. Everything here is verified:
tsc --noEmitclean, productionnext buildsucceeds, 62 unit tests pass (up from 43), and I probed the running server to confirm the security headers and validation behave.Security
next.config.ts):X-Frame-Options: DENY,X-Content-Type-Options: nosniff,Referrer-Policy,Permissions-Policy, and a production-only CSP that whitelists the Google Maps hosts. Verified at runtime./api/demand: validateattendancebefore use — it was an unguarded.toLocaleString()outside the try block, so any malformed body returned an unhandled 500. Now a clean 400. Verified:POST {}→ 400./api/sim: reject non-finitetickMs/toMinute(previously fedsetInterval(NaN)). Verified:POST {"type":"speed"}→ 400./api/briefing: normalizeincidents/recentEventsarrays before use./api/tasks: bound title/detail length.gemini.ts: guardJSON.parse.Code quality
nextPhase,clampPriority,worstDensityPct/insideEstimate— and single-sourcedSTAFF_ROLES/SUPPLY_ITEMS, removing 4 duplicated role lists + 2 supply lists across routes.arrivalsLeft), named magic numbers, renamedGateState.queue → queueLength, replaced a hand-rolled reducer withuseState, fixed a stale comment, marked the reservedOpsSnapshotvariant.congestionColor→ sharedCONGESTION_COLOR, literal85→DENSITY_ALERT_PCT, sim-speed options derived from constants.Accessibility (WCAG 2.1)
Modal(role=dialog, aria-modal, focus trap, Escape, focus restore) backs all three ops overlays.role="alert"on the evacuation banner;role="status"live regions on connection status;role="log"on the incident queue;aria-liveon early warnings; one<h1>per page;aria-labels on the copilot input + icon-only close; traffic-advisory severity no longer color-only;prefers-reduced-motionhonored.React correctness
voice-radio: recognizer held in a ref and aborted on unmount (was a leaked mic session + setState-after-unmount).venue-map: overlays torn down on unmount. Weatherfetcheffects guarded against unmount.Testing
nextPhase,clampPriority/STAFF_ROLES, and the clientdashReducer+ derivation helpers.Untouched: Efficiency (already 100) and Problem-Statement Alignment (98).