fix: enforce feature-flag gate in route guard (#524)#562
Conversation
Add `requiresFlag` meta to all feature-flagged routes and a `beforeEach` guard that redirects to /workspace/home when the flag is disabled. The guard calls `featureFlags.restore()` before checking, so hard-refresh / direct URL navigation is blocked even before App.vue mounts. Adds a `declare module 'vue-router'` RouteMeta augmentation for type safety. Affected routes: activity (4), automations/queue, review, automations/chat, ops/cli, ops/endpoints, ops/logs, settings/profile, settings/access, archive.
31 tests covering: no-flag routes (allowed), all 13 flagged routes when enabled (allowed) and disabled (redirect to home), hard-refresh localStorage restore scenario, and exhaustive route/flag mapping validation.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Self-review findingsAll gated routes covered? Yes — 13 routes across 5 feature flags ( Flag read source? Hard refresh / direct nav? The guard calls Redirect destination?
Security note: Feature flags live in localStorage and can be manually edited. This is intentional (Settings UI writes there too) and acceptable — this is a UX/discoverability gate, not a security boundary. Backend endpoints remain independently authenticated. No issues requiring follow-up fixes. |
There was a problem hiding this comment.
Code Review
This pull request implements a feature-flag navigation guard in the Vue router, restricting access to specific routes based on enabled flags. It includes updates to route metadata and a new test suite for the guard logic. Feedback suggests dynamically generating the list of valid flags in the tests to prevent the test from becoming out of sync with the feature flag definitions.
| const validFlags = [ | ||
| 'newShell', 'newAuth', 'newAccess', 'newActivity', | ||
| 'newOps', 'newAutomation', 'newArchive', | ||
| ] as const |
There was a problem hiding this comment.
This hardcoded list of validFlags can get out of sync with the FeatureFlags type if keys are added or renamed. This could lead to the test passing while coverage is incomplete.
To make this test more robust, you can generate this list dynamically from defaultFeatureFlags. This ensures that the test always checks against the current set of feature flags.
You will need to add import { defaultFeatureFlags } from '../../types/feature-flags' at the top of the file.
const validFlags = Object.keys(defaultFeatureFlags)
Second adversarial review (independent)Verdict: No correctness bugs found. One minor maintainability note. Checklist findings1. Flag defaults on first visit 2. 3. Gemini's dynamic flag list suggestion 4. Auth guard interaction / redirect loop 5. Exhaustiveness: redirect-only routes SummaryThe implementation is correct. The only actionable note from this review is the Gemini maintainability suggestion (item 3), which is worth a follow-up but not a merge blocker. No fixes applied. |
The route guard added in this PR redirects to Home when a feature flag is disabled. E2E tests navigate to gated routes (ops, activity, archive) without setting flags, causing 3 failures. Fix: inject all flags as enabled into localStorage alongside the auth token.
Summary
requiresFlagmeta field in the routerbeforeEachguard checks that meta field and redirects to/workspace/homewhen the flag is offRoot cause
Feature flags only removed sidebar links via
ShellSidebar.vue'savailableNavItemscomputed. The router itself had no guard, so typing/workspace/ops/clidirectly always loaded the surface regardless of the flag state.Affected routes gated
/workspace/activity(4 variants)newActivity/workspace/reviewnewAutomation/workspace/automations/queuenewAutomation/workspace/automations/chatnewAutomation/workspace/ops/clinewOps/workspace/ops/endpointsnewOps/workspace/ops/logsnewOps/workspace/settings/profilenewAuth/workspace/settings/accessnewAccess/workspace/archivenewArchiveFix
declare module 'vue-router' { interface RouteMeta { requiresFlag?: keyof FeatureFlags } }augmentation for type-safe metarequiresFlagto all gated route definitionsbeforeEachguard: readsto.meta.requiresFlag, callsfeatureFlags.restore()(so it works on hard refresh beforeApp.vuemounts), then redirects to/workspace/homeif the flag is disabledTest plan
npm run typecheckpasses (zero errors)npx vitest --runpasses — 31 new tests, 1107 pre-existing all green