fix(flags): validate persisted route-gate values - #3351
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9246b97441
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
7920993 to
e880496
Compare
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
e880496 to
48e325a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48e325a4e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7668a30398
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
8161b04 to
ca7ade3
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ca7ade3 to
a0cbf1c
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
a0cbf1c to
ee8fc83
Compare
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ee8fc83 to
ebe289e
Compare
|
@codex review Please review exact current head ebe289e against base a62d1ec after #3310 merged and this PR was rebased. Focused feature-flag proof is 64/64 across the three suites and Vue typecheck passes. Qwen's two storage-guard notes were source-validated as non-blocking; report only merge-blocking correctness or security defects. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Chris0Jeky
left a comment
There was a problem hiding this comment.
Coordinator verification: real route-gate bug, correctly fixed. On main, restore() spreads unvalidated localStorage JSON into flags (featureFlagStore.ts line 34): a tampered devTools true-string is truthy and passes the router guard at router/index.ts line 453, and false-strings stick flags on. The PR normalizes to declared booleans only. Persist/restore failure paths are also real (quota throws, blocked-storage getItem throws outside try on main) and the fix handles both with surfaced error state plus the hasUnsavedChanges guard so stale storage cannot clobber an unsaved update. Prior Codex P1/P2 threads were maintainer-verified fixed and the final state matches. Tests discriminate against defaults on every type-confusion case. Full CI green. Recommend merge once mergeable resolves.
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh continuation review of all four changed files at actual head ebe289e0b89f99bb60cc5f3e965ef89a08c54f37 (the body still named faac2a9): known-key boolean normalization excludes unexpected payload shapes; failed writes retain session state across route-triggered restores; persistenceError exposes unsaved state in Profile Settings and clears on successful persistence/restore. Reviewed the final recovered-load-error commit and its focused regression. All three prior review threads are resolved, and their corrections are present in this head. No blocking defect found in this bounded local-preference slice.
Freshly inspected exact-head required workflow 35676336663: all 18 jobs succeeded, including both frontend lint/typecheck/build/coverage jobs, both backend and API platforms, E2E, migrations, containers and security. Self-Test 35676336098 and Extended 35676336594 also succeeded. The current GitHub merge ref 5d15cbb5dac3ce56e554fb27a6633a64b73e1a7c binds this head to current main f180c225168e8d4ca417cd5a379363c55d4a4c37 without conflict. The CI run qualifies the exact source head; it is not a claim that this newly generated merge ref was separately executed.
No new code or tests were authored in this review, and no local Vue execution is claimed. Ready for merge under the user's explicit maintenance request; feature flags remain presentation preferences, not server authorization.
Closes #3350.
Final qualification and merge
Merged at
edd07b7a514fe98b5e3764e3b3b7838153a8be62from exact source headebe289e0b89f99bb60cc5f3e965ef89a08c54f37.Required CI 35676336663 completed successfully across all 18 jobs, including both frontend lint/typecheck/build/coverage jobs, both backend and API platforms, E2E, migrations, containers and security. Self-Test 35676336098 and Extended 35676336594 succeeded. All three review threads are resolved; final continuation review 5297205871 inspected the complete four-file diff and the final recovered-load-error correction.
Before merge, GitHub's synthetic merge
5d15cbb5dac3ce56e554fb27a6633a64b73e1a7cbound this head to mainf180c225168e8d4ca417cd5a379363c55d4a4c37without conflict. The cited CI qualifies the source head, not a separate execution of that newly generated merge ref. No local Vue execution is claimed by the continuation review.Summary
Hardens the local feature-flag persistence boundary used during app startup, router gating and Profile Settings:
The storage key, defaults, valid payload behavior and existing store calls remain compatible. Feature flags remain local presentation/developer preferences, not a server authorization boundary.
Historical test-first evidence
Test-only head
a22c2acaa96a0db4736af0a56646687f15897307ran canonical Node 24 qualification. Ubuntu lint, typecheck, production build and PWA validation passed before JUnit recorded 7,158 tests, exactly six intended shape/storage-boundary failures and zero errors. Initial correction9246b9744174b76b0b0418b1929d443b54bd154bthen passed its full matrix.Codex review identified failed writes being overwritten by a subsequent restore. Test-only
b49fccaf90e17d8a11ae8280a09f04088a1f0890added separate unsaved setFlag/resetAll regressions. A supplemental actual-module runner reproduced the stale overwrite before the correction and preserved the in-memory choice afterward. Headfaac2a9was an intermediate checkpoint, not the final qualified head.Later review corrections added visible persistence failure feedback and recovery clearing, with the dedicated store and view regressions present in final head
ebe289e0.No backend, DTO, schema, migration, dependency, workflow, control-plane or human-action change.