fix(billing): route subscription/sign-in/credit preconditions to modal, out of error panel (FE-878)#12785
Merged
dante01yoon merged 4 commits intoJun 19, 2026
Conversation
…l, out of error panel (FE-878)
dosubot
Bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughThis PR introduces a centralized account precondition handling system that classifies runtime errors (sign-in, subscription, credits) and routes them to dedicated modals. It replaces inline error-matching logic with a structured classification model and integrates precondition detection throughout the execution flow and application-level error handlers. The paywall error is now subscription-ally handled! 🎭 ChangesAccount Precondition Error System
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 1680 passed, 0 failed · 1 flaky📊 Browser Reports
|
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12785 +/- ##
===========================================
- Coverage 75.50% 62.14% -13.37%
===========================================
Files 1568 1458 -110
Lines 96293 75108 -21185
Branches 27647 21167 -6480
===========================================
- Hits 72710 46675 -26035
- Misses 22808 28089 +5281
+ Partials 775 344 -431
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1154 files with indirect coverage changes 🚀 New features to boost your workflow:
|
2 tasks
…(FE-878)
The free-tier queue paywall arrives on the POST /prompt path as { type: PAYMENT_REQUIRED, message: "Subscription required to queue workflows" }, which the execution_error-only routing did not cover, so it still surfaced as a raw error in the panel. Teach the runtime matcher this message and short-circuit the queuePrompt catch to open the precondition modal, keeping it out of the error panel and error count. Add an e2e regression test for the queue paywall plus a control for ordinary queue errors.
Fixes #12840
![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: 1
🧹 Nitpick comments (1)
browser_tests/tests/subscriptionPaywallError.spec.ts (1)
46-49: ⚡ Quick winAssert the paywall modal is shown, not only that the overlay is hidden.
This assertion is currently negative-only. If both overlay and precondition modal fail to appear, the test still passes. Add a positive assertion for the subscription dialog to lock the routing contract.
🤖 Prompt for 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. In `@browser_tests/tests/subscriptionPaywallError.spec.ts` around lines 46 - 49, The current test assertion in the subscriptionPaywallError.spec.ts file only verifies that the error overlay is hidden (a negative assertion), which means the test passes even if the subscription paywall modal fails to appear. Add a positive assertion that explicitly checks the subscription dialog or paywall modal is visible/shown (using toBeVisible() or similar) alongside the existing toBeHidden() assertion for the error overlay to ensure both conditions are properly tested and lock the routing contract.
🤖 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/app.ts`:
- Around line 1675-1685: The precondition routing logic in the code checks if
error.response.error is an object to determine preconditionResponseError, but
this field can also be a string value. Modify the condition that sets
preconditionResponseError to handle both cases: when error.response.error is an
object (extract type and message properties as currently done) and when it is a
string (treat the string itself as the exception message or type appropriately).
This ensures that both object-shaped and string-shaped prompt execution errors
are properly routed through the resolveAccountPrecondition function instead of
falling through to generic error handling.
---
Nitpick comments:
In `@browser_tests/tests/subscriptionPaywallError.spec.ts`:
- Around line 46-49: The current test assertion in the
subscriptionPaywallError.spec.ts file only verifies that the error overlay is
hidden (a negative assertion), which means the test passes even if the
subscription paywall modal fails to appear. Add a positive assertion that
explicitly checks the subscription dialog or paywall modal is visible/shown
(using toBeVisible() or similar) alongside the existing toBeHidden() assertion
for the error overlay to ensure both conditions are properly tested and lock the
routing contract.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: d32d8418-dc53-4a8d-9a38-67a4999fd954
📒 Files selected for processing (4)
browser_tests/tests/subscriptionPaywallError.spec.tssrc/platform/errorCatalog/accountPreconditionRouting.test.tssrc/platform/errorCatalog/runtimeErrorMatcher.tssrc/scripts/app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/platform/errorCatalog/accountPreconditionRouting.test.ts
…r-message-feels-choppyabrupt
dante01yoon
added
needs-backport
AustinMroz
previously approved these changes
Jun 16, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/errorCatalog/accountPreconditionRouting.test.ts (1)
17-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an overlap-case test so precedence doesn’t play hide-and-seek.
The suite validates individual classifications, but it does not assert the
documented sign-in > subscription > credits precedence for payloads that could
match multiple precondition patterns. That’s a small gap with a big safety
rhyme: one extra test now avoids future “why did this route?” time.Proposed test addition
describe('resolveAccountPrecondition', () => { + it('prefers sign-in when sign-in and credits patterns both appear', () => { + expect( + resolveAccountPrecondition({ + exceptionType: 'RuntimeError', + exceptionMessage: + 'Unauthorized: Please login first to use this node. Payment Required: Please add credits to your account to use this node.' + }) + ).toBe('sign_in') + }) + it('classifies a sign-in error', () => {As per coding guidelines, "Write tests for all changes, especially bug fixes to catch future regressions."
🤖 Prompt for 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. In `@src/platform/errorCatalog/accountPreconditionRouting.test.ts` around lines 17 - 84, Add a new test case in the describe block for resolveAccountPrecondition that validates the documented sign-in > subscription > credits precedence by creating a payload that matches multiple precondition patterns and asserting that the highest-precedence classification is returned. For example, craft an exception that contains keywords matching both sign-in and subscription patterns, and verify the function returns the sign-in classification rather than subscription, ensuring the precedence rules are correctly enforced in the resolveAccountPrecondition implementation.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/platform/errorCatalog/accountPreconditionRouting.test.ts`:
- Around line 17-84: Add a new test case in the describe block for
resolveAccountPrecondition that validates the documented sign-in > subscription
> credits precedence by creating a payload that matches multiple precondition
patterns and asserting that the highest-precedence classification is returned.
For example, craft an exception that contains keywords matching both sign-in and
subscription patterns, and verify the function returns the sign-in
classification rather than subscription, ensuring the precedence rules are
correctly enforced in the resolveAccountPrecondition implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9d5f53b-8d01-4960-aa57-872bc6c99b48
📒 Files selected for processing (3)
src/platform/errorCatalog/accountPreconditionRouting.test.tssrc/platform/errorCatalog/accountPreconditionRouting.tssrc/platform/errorCatalog/runtimeErrorMatcher.ts
mathtone
pushed a commit
to sketchyplans/ComfyUI-frontend-fork1
that referenced
this pull request
Jun 17, 2026
…(FE-978) (Comfy-Org#12786) ## Summary Cancelled / inactive team plans keep members but lock runs; the run button and the subscription-required dialog are now role-aware — owners are routed to the pricing/subscribe flow, members (who cannot subscribe) see "contact your workspace owner to resubscribe". ## Changes - **What**: `SubscribeToRun.vue` becomes a role-aware locked run button (owner → "Subscribe to Run"; member → neutral locked "Run" + contact-owner tooltip; both open the subscription dialog). `SubscriptionRequiredDialogContentWorkspace.vue` branches on role (member → read-only contact-owner panel, no pricing/subscribe affordance; owner → existing pricing/preview; member view suppressed for `out_of_credits` so the active-but-low-credits path is unchanged). `subscription.inactive.*` i18n keys. - **Breaking**: none. ## Review Focus - Role source = `useWorkspaceUI().permissions.value.canManageSubscription` (owner / personal = true, member = false) — the same accessor `SubscriptionPanelContentWorkspace.vue` uses. - **No BE work**: the run-gate already exists server-side (`InactiveSubscriptionError`; `is_active` checked before funds). The lock is gated on `is_active`, the same field the orchestrator uses, so FE/BE stay consistent; leftover-credits-while-inactive remains blocked by design. - Complements Comfy-Org#12785 (FE-878 precondition→modal routing); disjoint file sets. Design: DES-197, Figma 3253-18670 / 3253-18671 / 3246-13962. - Tests: `SubscribeToRun` (4) / `CloudRunButtonWrapper` (3) / `SubscriptionRequiredDialogContentWorkspace` role cases — member sees contact-owner (no subscribe), owner sees pricing, run locks on `!is_active` and unlocks when active (22 total); full `test:unit` green. Fixes FE-978
dante01yoon
added a commit
that referenced
this pull request
Jun 18, 2026
AustinMroz
approved these changes
Jun 18, 2026
dante01yoon
deleted the
jaewon/fe-878-bug-subscription-required-error-message-feels-choppyabrupt
branch
|
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
Account preconditions (sign-in / subscription / credits) on running a workflow now open their modal directly and stay out of the error panel + error count — previously
subscription_requiredfell through to a red "1 ERROR — Subscription required to queue workflows" banner. This covers both paths: theexecution_errorwebsocket event and thePOST /prompt402 queue paywall ({ type: "PAYMENT_REQUIRED", message: "Subscription required to queue workflows" }), which is the exact payload reported in #12840.Changes
execution_erroris classified by a pureaccountPreconditionRoutingresolver (precedence sign-in > subscription > credits) and routed to the existing modal viauseAccountPreconditionDialog;executionStorereturns early for preconditions so they never populatelastExecutionError/lastPromptError/lastNodeErrors→ fully excluded from the panel andtotalErrorCount. Runtime credit error at a node → credits modal (out of panel; can name the node).queuePromptcatch resolves the same precondition from thePOST /prompt402 response and opens the modal, short-circuiting beforelastPromptError, so the queue paywall stays out of the panel too. The runtime matcher learns the"Subscription required to queue workflows"message.Before / After
Free-tier queue paywall (
POST /prompt→ 402) on a cloud build:Before — raw
Subscription required to queue workflowssurfaced in the error panel (no actionable upgrade):After — clean subscription modal opens; nothing in the error panel or error count:
Review Focus
accountPreconditionRouting/useAccountPreconditionDialog/executionStore— each precondition routes to its modal and is excluded from the panel/count; precedence resolves on co-occurrence. Plus Playwrightbrowser_tests/tests/subscriptionPaywallError.spec.ts— the queue paywall (402) stays out of the error panel, with a control asserting ordinary queue errors still surface. typecheck / oxlint / eslint / stylelint / oxfmt / knip clean.Fixes FE-878
Fixes #12840