feat(featureFlags): employee-gated session feature flag overrides via ?ff= - #15033
Merged
Conversation
Adds an opt-in registry of feature flags that Comfy employees can override per browser tab with ?ff=name or ?ff=name:value, resolved ahead of the dev localStorage override, remote config and the server flag. Overrides are captured into sessionStorage so they survive reloads and in-app navigation but die with the tab, and are only applied for a signed-in user with a verified @comfy.org address on the cloud distribution.
…emoised Persist the query string the overrides were captured from, so re-reading the same URL neither re-warns nor rewrites storage. This drops the module-level memo, which per docs/guidance/vitest.md forced the test file to re-import behind vi.resetModules() for every case.
🎭 Playwright: ✅ 1802 passed, 0 failed · 2 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 8.69 MB gzip 🔴 +487 BDetailsSummary
Category Glance App Entry Points — 3.67 kB (baseline 3.67 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.36 MB (baseline 1.36 MB) • 🔴 +1 BGraph editor runtime, canvas, workflow orchestration
Status: 2 added / 2 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 15 added / 15 removed / 2 unchanged Panels & Settings — 565 kB (baseline 565 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 16 unchanged User & Accounts — 27 kB (baseline 27 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 4 unchanged Editors & Dialogs — 125 kB (baseline 125 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 8 added / 8 removed UI Components — 67.1 kB (baseline 67.1 kB) • ⚪ 0 BReusable component library chunks
Status: 13 added / 13 removed / 1 unchanged Data & Services — 3.5 MB (baseline 3.5 MB) • 🔴 +687 BStores, services, APIs, and repositories
Status: 15 added / 15 removed / 2 unchanged Utilities & Hooks — 550 kB (baseline 550 kB) • 🔴 +268 BHelpers, composables, and utility bundles
Status: 18 added / 18 removed / 20 unchanged Vendor & Third-Party — 16.3 MB (baseline 16.3 MB) • 🔴 +103 BExternal libraries and shared vendor chunks
Status: 2 added / 2 removed / 15 unchanged Other — 14 MB (baseline 14 MB) • ⚪ 0 BBundles that do not match a named category
Status: 101 added / 101 removed / 185 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-11T07:50:20.130Z",
"gitSha": "4d0fa0ea10908f9bd8e02cac4239c02482f26bc9",
"branch": "glary/feature-flag-url-overrides",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2023.021,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.339999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 563.4920000000001,
"heapDeltaBytes": 5844024,
"heapUsedBytes": 70195664,
"domNodes": 18,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 16.302,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2021.6159999999945,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.875,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 507.324,
"heapDeltaBytes": -17585048,
"heapUsedBytes": 46576036,
"domNodes": -279,
"jsHeapTotalBytes": 23044096,
"scriptDurationMs": 16.802999999999997,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1872.5749999999834,
"styleRecalcs": 75,
"styleRecalcDurationMs": 36.738,
"layouts": 12,
"layoutDurationMs": 3.366,
"taskDurationMs": 880.494,
"heapDeltaBytes": -12018908,
"heapUsedBytes": 52311960,
"domNodes": -281,
"jsHeapTotalBytes": 23306240,
"scriptDurationMs": 121.806,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000007,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1801.3349999999946,
"styleRecalcs": 74,
"styleRecalcDurationMs": 35.145999999999994,
"layouts": 12,
"layoutDurationMs": 3.0789999999999997,
"taskDurationMs": 828.1070000000001,
"heapDeltaBytes": 9672,
"heapUsedBytes": 64239416,
"domNodes": 55,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 119.18999999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1721.2319999999863,
"styleRecalcs": 32,
"styleRecalcDurationMs": 15.527,
"layouts": 6,
"layoutDurationMs": 0.526,
"taskDurationMs": 384.11,
"heapDeltaBytes": 8561100,
"heapUsedBytes": 72918744,
"domNodes": 79,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 18.749999999999996,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1722.2589999998945,
"styleRecalcs": 33,
"styleRecalcDurationMs": 15.598,
"layouts": 6,
"layoutDurationMs": 0.528,
"taskDurationMs": 372.0640000000001,
"heapDeltaBytes": 8580700,
"heapUsedBytes": 72861860,
"domNodes": 79,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 17.810000000000002,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 562.9659999999603,
"styleRecalcs": 11,
"styleRecalcDurationMs": 7.173000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 364.37,
"heapDeltaBytes": -11168604,
"heapUsedBytes": 53159940,
"domNodes": 18,
"jsHeapTotalBytes": 25952256,
"scriptDurationMs": 58.97599999999999,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "dom-widget-clipping",
"durationMs": 578.1279999999924,
"styleRecalcs": 11,
"styleRecalcDurationMs": 6.8290000000000015,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 374.992,
"heapDeltaBytes": -11235016,
"heapUsedBytes": 53132960,
"domNodes": 18,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 58.57300000000001,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-idle",
"durationMs": 2070.9620000000086,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.906999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 681.924,
"heapDeltaBytes": 7484212,
"heapUsedBytes": 69155900,
"domNodes": -280,
"jsHeapTotalBytes": 2461696,
"scriptDurationMs": 107.898,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2033.5540000000947,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.769999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 667.4,
"heapDeltaBytes": 7421948,
"heapUsedBytes": 67149928,
"domNodes": -284,
"jsHeapTotalBytes": 3248128,
"scriptDurationMs": 103.52399999999999,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2142.774999999972,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.389000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1248.7240000000002,
"heapDeltaBytes": 4206492,
"heapUsedBytes": 65336624,
"domNodes": -284,
"jsHeapTotalBytes": 4489216,
"scriptDurationMs": 423.44,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2187.718000000018,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.354000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1245.793,
"heapDeltaBytes": 4433888,
"heapUsedBytes": 65015428,
"domNodes": -287,
"jsHeapTotalBytes": 3440640,
"scriptDurationMs": 421.80400000000003,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3117.15700000002,
"styleRecalcs": 66,
"styleRecalcDurationMs": 14.297,
"layouts": 60,
"layoutDurationMs": 6.967999999999999,
"taskDurationMs": 1298.365,
"heapDeltaBytes": 21111168,
"heapUsedBytes": 83096188,
"domNodes": 14,
"jsHeapTotalBytes": 7864320,
"scriptDurationMs": 509.9180000000001,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3196.8219999999974,
"styleRecalcs": 65,
"styleRecalcDurationMs": 13.517000000000001,
"layouts": 60,
"layoutDurationMs": 7.058,
"taskDurationMs": 1482.106,
"heapDeltaBytes": -2218252,
"heapUsedBytes": 60066176,
"domNodes": -291,
"jsHeapTotalBytes": 6393856,
"scriptDurationMs": 522.8670000000001,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2022.736000000009,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.821999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 670.2239999999999,
"heapDeltaBytes": 6430772,
"heapUsedBytes": 66840620,
"domNodes": -284,
"jsHeapTotalBytes": 4296704,
"scriptDurationMs": 106.88699999999999,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2021.8730000000278,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.353999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 684.8510000000001,
"heapDeltaBytes": 7259664,
"heapUsedBytes": 67824264,
"domNodes": -283,
"jsHeapTotalBytes": 3510272,
"scriptDurationMs": 107.33500000000001,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 590.7960000000116,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.361000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 405.424,
"heapDeltaBytes": -10458980,
"heapUsedBytes": 53822192,
"domNodes": 20,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 122.696,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 558.9820000000145,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.516,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 387.63599999999997,
"heapDeltaBytes": -10734584,
"heapUsedBytes": 53906864,
"domNodes": 20,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 114.89000000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 1995.9519999999884,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.120999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 495.646,
"heapDeltaBytes": -17655832,
"heapUsedBytes": 46740532,
"domNodes": -279,
"jsHeapTotalBytes": 24092672,
"scriptDurationMs": 13.847,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 2039.480000000026,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.263,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 500.25,
"heapDeltaBytes": -18680200,
"heapUsedBytes": 45780720,
"domNodes": 2,
"jsHeapTotalBytes": 23044096,
"scriptDurationMs": 13.603,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1743.710999999962,
"styleRecalcs": 78,
"styleRecalcDurationMs": 33.893,
"layouts": 16,
"layoutDurationMs": 3.713,
"taskDurationMs": 798.852,
"heapDeltaBytes": -16429436,
"heapUsedBytes": 48015184,
"domNodes": -280,
"jsHeapTotalBytes": 23568384,
"scriptDurationMs": 88.602,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1737.7970000000005,
"styleRecalcs": 77,
"styleRecalcDurationMs": 33.841,
"layouts": 16,
"layoutDurationMs": 4.128000000000001,
"taskDurationMs": 793.654,
"heapDeltaBytes": -17716376,
"heapUsedBytes": 46759116,
"domNodes": -280,
"jsHeapTotalBytes": 24616960,
"scriptDurationMs": 90.48200000000001,
"eventListeners": -153,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1400.480000000016,
"styleRecalcs": 17,
"styleRecalcDurationMs": 29.557,
"layouts": 13,
"layoutDurationMs": 12.133000000000001,
"taskDurationMs": 935.3970000000003,
"heapDeltaBytes": 31032228,
"heapUsedBytes": 97823704,
"domNodes": 13673,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 34.086000000000006,
"eventListeners": 2375,
"totalBlockingTimeMs": 137,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8264.202000000012,
"styleRecalcs": 250,
"styleRecalcDurationMs": 35.028000000000006,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4509.715,
"heapDeltaBytes": 13017676,
"heapUsedBytes": 72645076,
"domNodes": -281,
"jsHeapTotalBytes": 6324224,
"scriptDurationMs": 1440.601,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8304.745000000024,
"styleRecalcs": 250,
"styleRecalcDurationMs": 35.189,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4412.447999999999,
"heapDeltaBytes": 11717948,
"heapUsedBytes": 71177712,
"domNodes": -281,
"jsHeapTotalBytes": 6324224,
"scriptDurationMs": 1365.839,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.669999999999952,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 16920.018000000025,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16862.836,
"heapDeltaBytes": -59029736,
"heapUsedBytes": 167207104,
"domNodes": -8312,
"jsHeapTotalBytes": -7282688,
"scriptDurationMs": 540.2510000000001,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-idle",
"durationMs": 16842.01799999994,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16799.467999999997,
"heapDeltaBytes": -38292676,
"heapUsedBytes": 166100604,
"domNodes": -8312,
"jsHeapTotalBytes": -9379840,
"scriptDurationMs": 563.3240000000001,
"eventListeners": -16387,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20550.42199999997,
"styleRecalcs": 144,
"styleRecalcDurationMs": 16.158999999999978,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20497.696,
"heapDeltaBytes": -29666180,
"heapUsedBytes": 176986992,
"domNodes": -8312,
"jsHeapTotalBytes": -10498048,
"scriptDurationMs": 902.3129999999999,
"eventListeners": -16381,
"totalBlockingTimeMs": 148,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 20867.22599999996,
"styleRecalcs": 145,
"styleRecalcDurationMs": 17.828999999999983,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20824.563000000002,
"heapDeltaBytes": -39149820,
"heapUsedBytes": 166940180,
"domNodes": -8312,
"jsHeapTotalBytes": -8855552,
"scriptDurationMs": 928.6589999999999,
"eventListeners": -16385,
"totalBlockingTimeMs": 217,
"frameDurationMs": 18.333333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 452.0919999999933,
"styleRecalcs": 15,
"styleRecalcDurationMs": 21.897999999999996,
"layouts": 3,
"layoutDurationMs": 1.182,
"taskDurationMs": 111.947,
"heapDeltaBytes": 5060508,
"heapUsedBytes": 68561088,
"domNodes": 128,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 8.924999999999999,
"eventListeners": 99,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 465.6150000000707,
"styleRecalcs": 16,
"styleRecalcDurationMs": 20.354999999999997,
"layouts": 3,
"layoutDurationMs": 1.2109999999999999,
"taskDurationMs": 112.403,
"heapDeltaBytes": 5165792,
"heapUsedBytes": 68381368,
"domNodes": 143,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 8.456999999999999,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
}
]
} |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds cloud-only, session-scoped feature flag overrides. Query parameters are parsed and persisted in ChangesSession feature flag overrides
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant useFeatureFlags
participant api
participant sessionFeatureFlagOverride
participant sessionStorage
participant getCurrentUserIdentity
Browser->>sessionFeatureFlagOverride: provide ff query parameters
sessionFeatureFlagOverride->>sessionStorage: persist session overrides
useFeatureFlags->>sessionFeatureFlagOverride: request flag override
sessionFeatureFlagOverride->>getCurrentUserIdentity: resolve authentication
getCurrentUserIdentity-->>sessionFeatureFlagOverride: return verified employee state
sessionFeatureFlagOverride-->>useFeatureFlags: return typed override or undefined
useFeatureFlags->>api: request server feature fallback
api-->>useFeatureFlags: return resolved feature value
useFeatureFlags-->>Browser: resolve feature flag value
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 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 |
huang47
added
the
preview-cpu
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/composables/useFeatureFlags.test.ts`:
- Around line 533-541: Remove the test case around useFeatureFlags that mocks
getSessionOverride to return true for ServerFeatureFlag.BILLING_CONTROL_ENABLED
while isCloud is false, since that scenario is unreachable and the flag is not
registered for overrides. Do not retain the expectation unless you instead
register an intended cloud-only auth-gated override and set isCloud to true.
In `@src/utils/sessionFeatureFlagOverride.test.ts`:
- Around line 16-18: Introduce or reuse a project-owned authentication adapter
that exposes only the user fields required by isComfyEmployee(), update the
production code to call that adapter instead of VueFire directly, and change the
test mock in sessionFeatureFlagOverride.test.ts to mock the adapter rather than
vuefire’s useCurrentUser.
In `@src/utils/sessionFeatureFlagOverride.ts`:
- Around line 180-183: Remove rawValue from the warning emitted by the session
feature-flag override validation in
src/utils/sessionFeatureFlagOverride.ts:180-183, leaving only the flag name and
expected type. Update the related test in
src/utils/sessionFeatureFlagOverride.test.ts:121-129 to assert the redacted
warning and verify that the submitted invalid value is not logged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 731317a6-9035-4972-84fa-bbe8c5a3764a
📒 Files selected for processing (4)
src/composables/useFeatureFlags.test.tssrc/composables/useFeatureFlags.tssrc/utils/sessionFeatureFlagOverride.test.tssrc/utils/sessionFeatureFlagOverride.ts
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #15033 +/- ##
==========================================
+ Coverage 80.79% 80.84% +0.04%
==========================================
Files 1872 1873 +1
Lines 121619 121892 +273
Branches 37546 37105 -441
==========================================
+ Hits 98268 98539 +271
+ Misses 22812 22804 -8
- Partials 539 549 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Drops the opt-in registry: any flag can now be overridden from `?ff=`, and `api.getServerFeature` consults the session override too so flags that never went through `resolveFlag` (max_upload_size, node_replacements) are covered. Value typing moves from the registry to JSON parsing, matching the `ff:` localStorage override, so `:false` is boolean and `:12` a number while anything JSON rejects stays a string. The verified @comfy.org gate is now the only boundary, so the rejected-value diagnostic no longer echoes user input. Adds a project-owned currentUserIdentity adapter so tests mock our own seam rather than vuefire, and fixes the auth-gated precedence test to assert a reachable scenario.
|
Heads up for reviewers: the PR description above is now partly stale. 1. No whitelist. 2. Coverage extended to 3. Typing moved from the registry to JSON parsing, matching the sibling
The security posture changed and deserves a look: the registry used to be a second boundary, so the verified Verified in a real browser on the cloud build — including the two cases that only exist because of this change ( Gates: Screenshots |
Contributor
There was a problem hiding this comment.
This feels overly refactored
There was a problem hiding this comment.
Agreed — deleted in 3a26b58. The whole file, the src/platform/auth/identity/ directory it lived in, and the exported UserIdentity type are gone.
You put your finger on something I should have caught myself: that module existed only to move a test mock boundary off vuefire (a CodeRabbit nitpick citing "Don't Mock What You Don't Own"). I added production structure to satisfy a test preference, which is backwards — and it wasn't free: a new directory and an exported type for one 6-line caller, plus a fresh object allocated on every flag read just to narrow a type.
The employee check is now a local function in sessionFeatureFlagOverride.ts:
function isComfyEmployee(): boolean {
try {
const user = useCurrentUser().value
if (!user?.emailVerified) return false
return user.email?.toLowerCase().endsWith(EMPLOYEE_EMAIL_DOMAIN) ?? false
} catch {
return false
}
}The test mocks vuefire again, and I'm fine with that trade: useCurrentUser() returning a ref is a trivial, stable contract, and a human "this is over-abstracted" beats a bot's mocking preference.
One thing I deliberately kept, now folded into that function's docstring — it must not read authStore. authStore imports useFeatureFlags, so going through it puts the entire app module graph behind every feature-flag read. That's not theoretical: it broke useTurnstile.test.ts and nodeReplacementStore.test.ts with api.addEventListener is not a function because ComfyApp got constructed as an import side effect. Both still pass, along with the rest.
Net: −25 lines, one fewer directory, same behaviour. Re-verified in the browser after the change (employee gate, lookalike domain, unverified address, number/string/boolean parsing) plus typecheck, lint and knip clean.
huang47
marked this pull request as ready for review
August 11, 2026 06:53
dosubot
Bot
added
the
size:L
…er module The currentUserIdentity adapter existed only to move a test mock boundary off vuefire, and cost a new directory, an exported type and a per-read object copy for one 6-line caller. Reading VueFire directly in isComfyEmployee keeps the one constraint that matters -- never import authStore -- with less machinery.
Contributor
Author
|
@Glary-Bot Use characterization tests to make sure we have enough coverage before making changes and split the changes into granular, atomic commits. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/utils/sessionFeatureFlagOverride.ts`:
- Line 1: Update getSessionOverride to avoid calling useCurrentUser directly
from synchronous utility and store paths; instead accept a resolved identity or
delegate identity lookup through a project-owned adapter initialized within a
valid injectable Vue context. Preserve valid employee overrides when
api.getServerFeature() is called without component setup, and add coverage for
that invocation path.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9ed87834-7a60-4449-91c3-546809116e55
📒 Files selected for processing (2)
src/utils/sessionFeatureFlagOverride.test.tssrc/utils/sessionFeatureFlagOverride.ts
…de layer Pins the behaviour getServerFeature had with no override in front of it: verbatim server values, falsy values that must not fall back to the default, dot-notation paths, and default handling. Verified to pass against main's unmodified api.ts, so it characterises existing behaviour rather than the change on this branch.
serverSupportsFeature does not delegate to getServerFeature; it repeats the override lookup itself, so it honoured the ff: localStorage override but skipped the session override. Flags read through it -- such as supports_progress_text_metadata in the websocket parser -- were silently un-overridable. Adds coverage for reaching api.getServerFeature from plain module scope with no active Vue instance, which is how stores and utilities call it.
|
@huang47 both done — Characterization testsAdded to To prove they characterize existing behaviour rather than my change, I swapped in main's unmodified Identical both sides, so the pre-existing resolution behaviour is provably unchanged. The falsy-value cases were the ones worth pinning — inserting a They paid for themselves immediately. Writing the companion coverage for the serverSupportsFeature(featureName: string): boolean {
const override = getDevOverride<boolean>(featureName) // ← honoured
if (override !== undefined) return override // session override: skipped
return get(this.serverFeatureFlags.value, featureName) === true
}Flags read through it were silently un-overridable, including Atomic commitsThe two new commits are split by concern — Your criticism lands on I've held off on one deliberate ground: it needs a force-push, and there are four live review threads on this PR (yours, christian's, two of CodeRabbit's) anchored to the current commits. The final tree wouldn't change, so line-anchored comments should survive, but commit-anchored context would shift mid-conversation. Say go and I'll do it — I didn't want to reshuffle history underneath an active review unasked. Current state: 6 commits, 15,859 unit tests passing, |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/scripts/api.featureFlags.test.ts`:
- Around line 437-439: Update the suite’s afterEach cleanup to clear
sessionStorage and restore the URL in addition to clearing localStorage,
ensuring session overrides and URL state cannot leak between tests. Preserve the
existing localStorage reset behavior.
In `@src/scripts/api.sessionOverride.test.ts`:
- Around line 14-23: Remove the vuefire mock around the module-scope tests and
let the real VueFire behavior exercise its default-app fallback. Keep API
precedence coverage focused on the project-owned session-override boundary, and
move identity-resolution coverage to the project-owned upstream adapter rather
than mocking the external dependency.
- Around line 78-83: Add a test alongside the existing “reports the override
through serverSupportsFeature too” case that sets
serverFeatureFlags.value.some_flag to true, applies the query override
“some_flag:false” via window.history.replaceState, and asserts
api.serverSupportsFeature('some_flag') returns false, verifying explicit false
precedence.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b1f1128-4b8b-4aec-95ae-f5e97c83572e
📒 Files selected for processing (3)
src/scripts/api.featureFlags.test.tssrc/scripts/api.sessionOverride.test.tssrc/scripts/api.ts
… override The characterization suite cleared only localStorage, so a stray ?ff= URL or sessionStorage entry could change its results now that getServerFeature reads both. Resets all three around every case. Adds the missing falsy case: an explicit ?ff=flag:false must turn a server value of true off through both serverSupportsFeature and getServerFeature, so a future truthiness check cannot pass unnoticed. Also corrects the suite docstring, which claimed to verify VueFire's default-app fallback -- the vuefire stub bypasses it, and that guarantee is verified against a real Firebase app in the browser instead.
christian-byrne
approved these changes
Aug 11, 2026
Contributor
There was a problem hiding this comment.
Ideally we could centralize with other query state readers (so it's not hidden away and unexpected) and have this be an extension + use the onAuthUserResolved hook (for dependency direction and disableability) -- but I think it's the best shape if this needs to load before extensions.
christian-byrne
approved these changes
Aug 11, 2026
| } | ||
|
|
||
| function readStoredState(): StoredState { | ||
| let raw: string | null |
Contributor
There was a problem hiding this comment.
Nit: I believe we have some shared helpers for storage I/O
There was a problem hiding this comment.
I went looking, and I don't think either candidate fits — but the reasons are specific rather than "didn't feel like it", so tell me if you disagree and I'll change it.
src/platform/workflow/persistence/base/storageIO.ts is the closest thing to a shared storage I/O module, but it's workflow-draft-specific and would actively break this. Every export is typed to DraftIndexV2 / DraftPayloadV2 / ActivePathPointer, keys come from StorageKeys.draftIndex(workspaceId), and — the real problem — it carries module-level write gating:
let workflowWritesBlocked = false
function writeStorage(storage, key, value) {
if (!storageAvailable || workflowWritesBlocked) return // silent no-op
...
}prepareWorkflowWorkspaceTransition() flips that flag, so switching workspace would silently stop persisting feature-flag overrides. Borrowing it would couple flag overrides to workflow-persistence global state for no gain.
VueUse useStorage is the genuine shared helper and is used elsewhere in the repo (versionCompatibilityStore, useConflictAcknowledgment). Two things rule it out here:
- It's eager — calling it creates a watcher and a
storagelistener at call time. This module is imported bysrc/scripts/api.ts, so that side effect would land in nearly every module graph and every test file. Keeping this module inert on import is the constraint that made the earlierauthStoreversion unworkable. - It caches into a ref, so it wouldn't observe an external
sessionStorage.clear()— which is exactly how the app-level clear and the tests reset state. Cached-ref semantics would drift from actual storage.
On the duplication itself: you're right that preservedQueryManager.ts hand-rolls the same try/catch + JSON + sessionStorage shape, so this is the second occurrence. I'd rather not extract a shared helper for two call sites — that's the same instinct that produced the currentUserIdentity adapter you (correctly) told me to delete. Happy to do it as a follow-up if you want it as a real utility with its own tests, rather than something I invent mid-PR.
|
@christian-byrne on the review summary — thanks for the approval. I checked your load-order premise rather than just agreeing with it, and it holds:
So an extension registering on There's a second reason the hook wouldn't be sufficient on its own: On centralizing with the other query-state readers — Happy to file both as follow-up issues if you want them tracked rather than left in a PR thread. |
Contributor
Author
@Glary-Bot file both as follow-up issues in linear |
huang47
enabled auto-merge
August 11, 2026 13:46
Contributor
Author
|
@coderabbitai approve |
Contributor
✅ Action performedComments resolved and changes approved. |
|
@huang47 both filed in Linear, on Frontend Engineering, low priority,
Both reference this PR and FE-1471 as origin, and carry the concrete findings from the review rather than just the headline — the load-order evidence for FE-1551, and for FE-1552 the two candidate helpers that were considered and why each was rejected, including that I deliberately did not attach them to this PR as GitHub-linked issues. Linear's PR attachments drive close-on-merge automation, and these are follow-ups this PR doesn't resolve — linking them would auto-close both when #15033 merges. They reference the PR by URL in the body instead. Neither is a duplicate; I searched first. Worth noting FE-1108 is the same shape of ticket (extract-a-shared-helper follow-up from a review thread), so I matched its conventions. Also left unassigned and without a project — no existing FE project looked like a clean fit for frontend tech debt, and I'd rather they sit in Triage for you to route than be mis-filed. |
christian-byrne
pushed a commit
to balpreetgrowthnatives/ComfyUI_frontend
that referenced
this pull request
Aug 16, 2026
…veFlag (Comfy-Org#15125) ## Summary `nodeLibraryEssentialsEnabled` hand-rolled its own resolution and read remote config *before* the override layers, so `?ff=node_library_essentials_enabled:false` and the `ff:` localStorage dev override were both silently ignored — this getter now goes through the shared `resolveFlag` helper like every other remote-config-backed flag. ## Changes - **What**: `nodeLibraryEssentialsEnabled` now calls `resolveFlag(...)`, so precedence is session override → dev override → remote config → server value, matching every other flag in `useFeatureFlags`. The nightly/dev default-on moves *below* the override lookup: `isNightly || import.meta.env.DEV` becomes `resolveFlag`'s `defaultValue` instead of an unconditional early `return true`. - **Behavior**: on a stable/production build nothing changes (`isNightly || DEV` is `false` there, which is the default the old code already passed). Two deltas, both intentional: (1) an employee `?ff=` link or an `ff:` dev override now actually changes the Essentials tab, and (2) on nightly and local dev builds an explicitly-served value (`/features` remote config or the server feature flag) now wins over the built-in on — previously nothing could turn the Essentials tab off locally, which is half of the reported bug. - **Tests**: one case in the existing `session override precedence` block asserting a `false` session override beats a `true` remote-config value, plus a `nodeLibraryEssentialsEnabled` block covering the nightly axis — nightly + nothing served → on, nightly + remote-config `false` → off, nightly + served server `false` → off. All three `false` cases are red on `main` (it returns `true`) and green with the fix. ## Review Focus - Chesterton's fence on the early return: `git log -L` shows the getter arrived in Comfy-Org#9067, whose description states the intent as *"Defaults to `true` in dev/nightly builds, `false` in production. Overridable via remote config or server feature flags"* — i.e. dev/nightly was always meant as a **default**, and the early return only reads as a hard override because the `?ff=` session layer (Comfy-Org#15033) landed ~6 months later and this getter never opted in. Making it `resolveFlag`'s `defaultValue` is the shape that commit describes. - The one visible change beyond override precedence is nightly/dev now honoring an explicitly-served `false` (item 2 above). Flagging it explicitly rather than burying it: it is the intended semantics, but it is a behavior change for anyone on nightly whose `/features` endpoint serves this flag. The cursor-review panel raised this; it is now pinned by the `nodeLibraryEssentialsEnabled` test block. - `linearToggleEnabled` has the same `if (isNightly) return true` short-circuit above its `resolveFlag` call and is un-overridable on nightly for the same reason. Deliberately left alone here to keep this diff to one getter; worth a separate change. ## Test Plan - `pnpm exec vitest run src/composables/useFeatureFlags.test.ts src/components/sidebar/tabs/NodeLibrarySidebarTabV2.test.ts src/components/searchbox/v2/NodeSearchContent.test.ts` — 79 passed (the two component specs are the flag's consumers). - `pnpm typecheck` — clean. `oxfmt --check` and `eslint` on both changed files — clean.
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
3 participants
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
Lets a verified Comfy employee override any server feature flag for their own cloud browser tab with
?ff=nameor?ff=name:value. This supports testing branches and bugs against real cloud data without changing remote configuration.Changes
src/utils/sessionFeatureFlagOverride.ts.useFeatureFlagsand directapi.getServerFeature/serverSupportsFeaturereads.?ff=parameters and JSON value semantics:?ff=feature→true?ff=feature:false→false?ff=max_upload_size:209715200→ number?ff=signup_turnstile:enforce→ string?ff=→ clear all overrides for the tabsessionStorage: they survive reload/navigation and disappear when the tab closes.@comfy.orgusers in the cloud distribution. The identity gate is re-evaluated on every read so overrides activate after auth resolves and stop applying after sign-out.Review notes
window.location.searchbecause feature flags are consumed before Vue Router and extension setup are available.useCurrentUser()is used instead ofauthStoreto avoid theauthStore → useFeatureFlagsimport cycle and app-wide module side effects.Verification
Current GitHub checks pass, including unit, E2E, Storybook, lint/format, website build/E2E, binary size, telemetry, OSS assets, and cloud frontend build dispatch.
Manually verified in cloud and localhost distributions for employee gating, lookalike and unverified addresses, precedence, typed values, reload/navigation persistence, per-tab isolation, clearing, and non-cloud inactivity.
Follow-ups