Repository navigation
Limit free Cloud VM creates with Stack credits - #3437
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds plan-aware VM create-credit configuration and resolution (new free-plan env vars, README updates), implements plan+provider item-id/cost resolution in the billing gateway, threads billing-team selection through request handling and workflows, introduces workflow error-normalization and billing-failure recording, and adds tests for reservation, refund, auth/team selection, and workflow recovery. ChangesPlan-Aware Create-Credit Flow & Billing-Team Integration
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant BillingGateway
participant StackAuth
participant DB
Client->>API: POST /api/vm (payload, optional billingTeamId / X-Cmux-Team-Id)
API->>API: resolveVmEntitlements(requestedBillingTeamId, requireTeam)
API->>BillingGateway: reserveCreate(planId, provider, billingTeamId, idempotencyKey)
BillingGateway->>BillingGateway: resolve itemId via plan/env keys
BillingGateway->>BillingGateway: resolve amount via plan+provider/env keys
BillingGateway->>StackAuth: getItem(itemId)
StackAuth-->>BillingGateway: item
BillingGateway->>StackAuth: tryDecreaseQuantity(itemId, amount)
alt success
StackAuth-->>BillingGateway: success
BillingGateway->>DB: insert reservation (idempotent)
BillingGateway-->>API: reservation token/details
API-->>Client: 200 Created (providerVmId or reservation)
else insufficient quantity
StackAuth-->>BillingGateway: failure
BillingGateway->>DB: mark create failed (billing_reserve_failed)
BillingGateway-->>API: VmCreateCreditsInsufficientError (itemId, amount)
API-->>Client: 402/4xx error
end
Note over BillingGateway,StackAuth: refund -> getItem(itemId) then increaseQuantity(itemId, amount)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR changes free-plan Cloud VM creates to unconditionally consume a Stack Auth
Confidence Score: 3/5Not safe to merge as-is — free-plan creates will silently fail on deployments without Stack Auth, with no documented opt-out. One P1 finding: the unconditional free-plan Stack Auth requirement removes the previous no-op path with no env-var escape hatch, making this a breaking change for self-hosted setups. Two P2 findings do not lower the score further but indicate additional robustness gaps. web/services/vms/billingGateway.ts — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[reserveCreate called] --> B{createCreditItemId}
B -->|plan-specific env var set| C[use plan-specific item ID]
B -->|global CMUX_VM_CREATE_CREDIT_ITEM_ID set| D[use global item ID]
B -->|planId == 'free'| E[use DEFAULT_FREE_CREATE_CREDIT_ITEM_ID]
B -->|paid plan, nothing configured| F[return null → kind: none]
C --> G{isStackConfigured?}
D --> G
E --> G
G -->|no| H[throw VmBillingError]
G -->|yes| I[getItem from Stack Auth]
I --> J[tryDecreaseQuantity]
J -->|success| K[return stack_item reservation]
J -->|fails| L[throw VmCreateCreditsInsufficientError]
F --> M[return none reservation]
Reviews (1): Last reviewed commit: "Limit free Cloud VM creates with Stack c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| return normalizedPlanId(planId) === "free" ? DEFAULT_FREE_CREATE_CREDIT_ITEM_ID : null; |
There was a problem hiding this comment.
createCreditItemId unconditionally returns DEFAULT_FREE_CREATE_CREDIT_ITEM_ID for any free-plan request when neither a plan-specific nor global item ID env var is set. This means isStackConfigured() is always reached for free plans, and if Stack Auth isn't configured, the gateway throws — wrapping the error as VmBillingError and blocking the create entirely.
Previously, omitting CMUX_VM_CREATE_CREDIT_ITEM_ID produced a no-op gateway that allowed creates without billing. Any self-hosted deployment that has free-plan users but no Stack Auth will silently break after this change with no documented migration path or env-var opt-out.
Setting CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID= (empty) doesn't help — the code falls through to the hardcoded default anyway. Consider returning null when Stack Auth is not configured, or document and enforce a clear way to opt out (e.g., a sentinel value like CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=none).
| function normalizedPlanId(planId: string): string { | ||
| const normalized = planId.trim().toLowerCase(); | ||
| return normalized || "free"; | ||
| } |
There was a problem hiding this comment.
billingPlanId silently treated as "free"
normalizedPlanId("") returns "free", so an accidental empty string for billingPlanId is silently promoted to the free plan and starts consuming free-plan Stack Auth create credits. An explicit guard or a distinct error would make the misuse visible rather than silently routing the request through the free-plan billing path.
| function planEnvKey(planId: string): string { | ||
| return normalizedPlanId(planId).replace(/[^a-zA-Z0-9]/g, "_").toUpperCase(); | ||
| } |
There was a problem hiding this comment.
toLowerCase
normalizedPlanId lowercases the plan ID before planEnvKey calls replace(/[^a-zA-Z0-9]/g, "_"). After toLowerCase(), no uppercase letters can be present, so A-Z in the character class is never matched. The regex still works correctly, but replacing it with /[^a-z0-9]/g would make the intent clearer.
| function planEnvKey(planId: string): string { | |
| return normalizedPlanId(planId).replace(/[^a-zA-Z0-9]/g, "_").toUpperCase(); | |
| } | |
| function planEnvKey(planId: string): string { | |
| return normalizedPlanId(planId).replace(/[^a-z0-9]/g, "_").toUpperCase(); | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/tests/vm-billing-gateway.test.ts (1)
53-63: ⚡ Quick winAdd a free-plan + Stack-misconfigured regression test.
Current tests verify paid plans can return
{ kind: "none" }without Stack, but don’t lock the opposite rule for free plans. Adding that case will guard this PR’s primary behavior.🧪 Suggested test addition
+ test("fails free-plan reservations when Stack Auth is not configured", async () => { + stackConfigured = false; + const gateway = makeStackVmBillingGateway({}); + + const error = await Effect.runPromise( + gateway.reserveCreate(createInput()).pipe(Effect.flip), + ); + + expect(error).toMatchObject({ operation: "reserveCreate" }); + expect(getItem).not.toHaveBeenCalled(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/tests/vm-billing-gateway.test.ts` around lines 53 - 63, Add a new test that asserts free plans require create credits when the Stack is misconfigured: copy the pattern in the existing test "does not require create credits for paid plans by default" but set billingPlanId to "free" (use makeStackVmBillingGateway, stackConfigured = false, and createInput({ billingPlanId: "free" })), call gateway.reserveCreate and assert the reservation is not { kind: "none" } (e.g., expect a credit-required response) and that getItem was called; this mirrors the existing paid-plan test but flips the plan to "free" to guard the regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/services/vms/README.md`:
- Line 220: Update the README sentence that currently states the gateway
reserves "one Stack Auth create credit" to reflect that the gateway reserves a
configurable amount (default 1); specifically mention the configuration keys
like CMUX_VM_PLAN_*_CREATE_CREDIT_COST (and global defaults) and keep the
team-scoped item name cmux-vm-create-credit in the description so it reads:
reserves the configured create-credit amount (default 1) and refunds that amount
on provisioning failure.
---
Nitpick comments:
In `@web/tests/vm-billing-gateway.test.ts`:
- Around line 53-63: Add a new test that asserts free plans require create
credits when the Stack is misconfigured: copy the pattern in the existing test
"does not require create credits for paid plans by default" but set
billingPlanId to "free" (use makeStackVmBillingGateway, stackConfigured = false,
and createInput({ billingPlanId: "free" })), call gateway.reserveCreate and
assert the reservation is not { kind: "none" } (e.g., expect a credit-required
response) and that getItem was called; this mirrors the existing paid-plan test
but flips the plan to "free" to guard the regression.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e7c7961-a9e5-4725-81ad-8a71171b0ecf
📒 Files selected for processing (4)
web/.env.exampleweb/services/vms/README.mdweb/services/vms/billingGateway.tsweb/tests/vm-billing-gateway.test.ts
| ## Usage, limits, and pricing | ||
|
|
||
| The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items when `CMUX_VM_CREATE_CREDIT_ITEM_ID` is configured. The create workflow inserts the idempotent VM row first, reserves one Stack Auth create credit only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists. | ||
| The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items. The free plan uses the team-scoped item `cmux-vm-create-credit` by default. Configure the Stack Auth free product as team-owned, include-by-default, and grant 20 of that item with no repeat and no expiry. The create workflow inserts the idempotent VM row first, reserves one Stack Auth create credit only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists. |
There was a problem hiding this comment.
Document configurable create-credit amount instead of fixed “one credit”.
Line 220 says reservation is always one credit, but the gateway can reserve a configured amount (CMUX_VM_PLAN_*_CREATE_CREDIT_COST* / global defaults). Please word this as “configured amount (default 1)” to avoid pricing/runbook drift.
✏️ Suggested doc patch
-The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items. The free plan uses the team-scoped item `cmux-vm-create-credit` by default. Configure the Stack Auth free product as team-owned, include-by-default, and grant 20 of that item with no repeat and no expiry. The create workflow inserts the idempotent VM row first, reserves one Stack Auth create credit only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists.
+The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items. The free plan uses the team-scoped item `cmux-vm-create-credit` by default. Configure the Stack Auth free product as team-owned, include-by-default, and grant 20 of that item with no repeat and no expiry. The create workflow inserts the idempotent VM row first, reserves the configured Stack Auth create-credit amount (default `1`) only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items. The free plan uses the team-scoped item `cmux-vm-create-credit` by default. Configure the Stack Auth free product as team-owned, include-by-default, and grant 20 of that item with no repeat and no expiry. The create workflow inserts the idempotent VM row first, reserves one Stack Auth create credit only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists. | |
| The usage ledger is in Postgres. VM create pricing gates use Stack Auth payment items. The free plan uses the team-scoped item `cmux-vm-create-credit` by default. Configure the Stack Auth free product as team-owned, include-by-default, and grant 20 of that item with no repeat and no expiry. The create workflow inserts the idempotent VM row first, reserves the configured Stack Auth create-credit amount (default `1`) only for a newly inserted row, calls the provider, and refunds the credit if provisioning fails before a usable VM exists. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/services/vms/README.md` at line 220, Update the README sentence that
currently states the gateway reserves "one Stack Auth create credit" to reflect
that the gateway reserves a configurable amount (default 1); specifically
mention the configuration keys like CMUX_VM_PLAN_*_CREATE_CREDIT_COST (and
global defaults) and keep the team-scoped item name cmux-vm-create-credit in the
description so it reads: reserves the configured create-credit amount (default
1) and refunds that amount on provisioning failure.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 282b7eaa06
ℹ️ 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".
| const global = env.CMUX_VM_CREATE_CREDIT_ITEM_ID?.trim(); | ||
| if (global) return global; | ||
|
|
||
| return normalizedPlanId(planId) === "free" ? DEFAULT_FREE_CREATE_CREDIT_ITEM_ID : null; |
There was a problem hiding this comment.
This new fallback forces every free create through reserveCreate, but createVm does not mark the inserted VM row as failed when reserveCreate returns VmCreateCreditsInsufficientError (the failure happens before the provider/error-handling branches in web/services/vms/workflows.ts). Because beginCreate inserts status provisioning and active-limit checks count provisioning rows (web/services/vms/repository.ts), the first out-of-credits attempt leaves a hidden row that permanently consumes the free plan’s single active slot, so subsequent creates return vm_active_limit_exceeded even after credits are replenished.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/.env.example`:
- Around line 51-55: Update the env example guidance and ordering: clarify that
CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID is the disable switch (set it to empty
to disable) and not CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST, move the
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST line before
CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID to satisfy dotenv-linter ordering, and
adjust the surrounding comment text to reflect these facts; reference the
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST and CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID
symbols when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c08f97c-280f-4287-8196-825564bf1b59
📒 Files selected for processing (4)
web/.env.exampleweb/services/vms/README.mdweb/services/vms/billingGateway.tsweb/tests/vm-billing-gateway.test.ts
✅ Files skipped from review due to trivial changes (2)
- web/services/vms/README.md
- web/services/vms/billingGateway.ts
| # Stack Auth create-credit gating. The free plan consumes one team-scoped Stack Auth | ||
| # item credit per successful create. Configure the Stack Auth free/include-by-default | ||
| # product to grant 20 of this item. Set this to "none" to disable create credits. | ||
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=cmux-vm-create-credit | ||
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST= |
There was a problem hiding this comment.
Fix the disable guidance and env key order here.
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST cannot be set to "none"; only the item-id env vars act as the disable switch. Also, dotenv-linter wants the cost key before the item-id key, so this block will currently fail lint and mislead anyone copying the example.
♻️ Suggested correction
# Stack Auth create-credit gating. The free plan consumes one team-scoped Stack Auth
# item credit per successful create. Configure the Stack Auth free/include-by-default
-# product to grant 20 of this item. Set this to "none" to disable create credits.
-CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=cmux-vm-create-credit
+# product to grant 20 of this item. Set CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID to
+# "none" to disable create credits.
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST=
+CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=cmux-vm-create-credit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Stack Auth create-credit gating. The free plan consumes one team-scoped Stack Auth | |
| # item credit per successful create. Configure the Stack Auth free/include-by-default | |
| # product to grant 20 of this item. Set this to "none" to disable create credits. | |
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=cmux-vm-create-credit | |
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST= | |
| # Stack Auth create-credit gating. The free plan consumes one team-scoped Stack Auth | |
| # item credit per successful create. Configure the Stack Auth free/include-by-default | |
| # product to grant 20 of this item. Set CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID to | |
| # "none" to disable create credits. | |
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST= | |
| CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=cmux-vm-create-credit |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 55-55: [UnorderedKey] The CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST key should go before the CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/.env.example` around lines 51 - 55, Update the env example guidance and
ordering: clarify that CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID is the disable
switch (set it to empty to disable) and not
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST, move the
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST line before
CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID to satisfy dotenv-linter ordering, and
adjust the surrounding comment text to reflect these facts; reference the
CMUX_VM_PLAN_FREE_CREATE_CREDIT_COST and CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID
symbols when making the change.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/services/vms/errors.ts (1)
101-112: 💤 Low valueConsider deriving
vmWorkflowErrorTagsfrom the error classes to avoid duplication.The tag strings are manually duplicated from the class definitions. If a new error class is added or a tag renamed, this set could fall out of sync with the
VmWorkflowErrorunion type.A safer approach would be to derive the set from the actual classes:
const vmWorkflowErrorTags = new Set([ VmDatabaseError._tag, VmProviderOperationError._tag, // ... ] as const);However,
Data.TaggedErrorclasses may not expose a static_tagproperty in Effect 3.x. If that's the case, this manual approach is acceptable but requires diligence when adding new error types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/services/vms/errors.ts` around lines 101 - 112, The vmWorkflowErrorTags set is manually duplicating tag strings that are defined by the Vm* error classes (e.g., VmDatabaseError, VmProviderOperationError) and can drift from the VmWorkflowError union; modify vmWorkflowErrorTags to derive its entries from the error classes themselves (use each class's static tag property such as VmDatabaseError._tag, VmProviderOperationError._tag, etc.) so new/renamed errors stay in sync with VmWorkflowError, and if Effect 3.x's Data.TaggedError does not expose a static _tag, export or add a stable static tag property on each Vm* error class and reference those properties from vmWorkflowErrorTags instead of hard‑coding strings.web/services/vms/workflows.ts (1)
108-134: 💤 Low valueBest-effort side effects could leave inconsistent state, but trade-off is reasonable.
The
Effect.allrunsmarkCreateFailedandrecordUsageEventindependently (not transactionally, per the repository implementation). If one succeeds and the other fails, partial state is possible:
- VM marked
failedwithout avm.create.billing_failedevent, or vice versa.The
catchAll(() => Effect.void)swallows these failures to ensure the primary billing error propagates. This is a reasonable trade-off since:
- The billing failure is the critical signal to the caller.
- The side effects are for observability and recovery guidance.
- Transactional coupling would complicate the error handling flow.
Consider logging when these side effects fail (before swallowing) for operational visibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/services/vms/workflows.ts` around lines 108 - 134, The best-effort side-effect block using Effect.tapError -> Effect.all calls repo.markCreateFailed and repo.recordUsageEvent and then swallows any errors with Effect.catchAll(() => Effect.void); add logging of any failures from that Effect.all before they are discarded so operational teams see when the observability side-effects fail. Concretely, wrap or append an Effect.tapError (or Effect.tapBoth) after the Effect.all to call a logger (e.g., processLogger.error or an available logger) with context including create.vm.id, input.userId, and the error, then continue to Effect.catchAll(() => Effect.void) so the primary billing error still propagates; reference the existing symbols Effect.tapError, Effect.all, Effect.catchAll, repo.markCreateFailed, and repo.recordUsageEvent when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/services/vms/errors.ts`:
- Around line 128-133: The function effectFiberFailureCause currently looks up a
symbol by its description string which is fragile; update it to use Effect's
stable public API by importing Runtime from "effect" and checking for
Runtime.FiberFailureCauseId on the err object (instead of searching
descriptions) and, if present, return (err as Record<symbol,
unknown>)[Runtime.FiberFailureCauseId], otherwise return null; modify the import
and the body of effectFiberFailureCause to use Runtime.FiberFailureCauseId (and
optionally Runtime.FiberFailureId where relevant) to access the cause.
---
Nitpick comments:
In `@web/services/vms/errors.ts`:
- Around line 101-112: The vmWorkflowErrorTags set is manually duplicating tag
strings that are defined by the Vm* error classes (e.g., VmDatabaseError,
VmProviderOperationError) and can drift from the VmWorkflowError union; modify
vmWorkflowErrorTags to derive its entries from the error classes themselves (use
each class's static tag property such as VmDatabaseError._tag,
VmProviderOperationError._tag, etc.) so new/renamed errors stay in sync with
VmWorkflowError, and if Effect 3.x's Data.TaggedError does not expose a static
_tag, export or add a stable static tag property on each Vm* error class and
reference those properties from vmWorkflowErrorTags instead of hard‑coding
strings.
In `@web/services/vms/workflows.ts`:
- Around line 108-134: The best-effort side-effect block using Effect.tapError
-> Effect.all calls repo.markCreateFailed and repo.recordUsageEvent and then
swallows any errors with Effect.catchAll(() => Effect.void); add logging of any
failures from that Effect.all before they are discarded so operational teams see
when the observability side-effects fail. Concretely, wrap or append an
Effect.tapError (or Effect.tapBoth) after the Effect.all to call a logger (e.g.,
processLogger.error or an available logger) with context including create.vm.id,
input.userId, and the error, then continue to Effect.catchAll(() => Effect.void)
so the primary billing error still propagates; reference the existing symbols
Effect.tapError, Effect.all, Effect.catchAll, repo.markCreateFailed, and
repo.recordUsageEvent when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79f7df8e-a8b9-4d19-a7cc-4887e1144797
📒 Files selected for processing (3)
web/services/vms/errors.tsweb/services/vms/workflows.tsweb/tests/vm-workflows.test.ts
| function effectFiberFailureCause(err: object): unknown { | ||
| const symbol = Object.getOwnPropertySymbols(err).find((candidate) => | ||
| candidate.description === "effect/Runtime/FiberFailure/Cause" | ||
| ); | ||
| return symbol ? (err as Record<symbol, unknown>)[symbol] : null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Effect-TS FiberFailure Cause symbol description stable API
💡 Result:
In Effect (Effect-TS), the FiberFailure “cause” you can inspect is represented by the runtime’s Cause value. The stable public API pieces you can rely on are: 1) FiberFailure has a symbol/id that is stable for identification - FiberFailureId: Symbol.for("effect/Runtime/FiberFailure") 2) FiberFailure contains the underlying Cause - FiberFailure is defined as an Error with: - a [FiberFailureId] symbol field - a [FiberFailureCauseId] field containing a Cause 3) FiberFailureCauseId is a (unique) symbol for the Cause payload - FiberFailureCauseId is exported as a unique symbol. 4) The Cause type has stable tags for interruption and for failures - The Cause union includes: Empty, Fail, Die, Interrupt, Sequential, Parallel - Interrupt contains fiberId and represents fiber interruption. Practical implication for your query (“Effect-TS FiberFailure cause symbol description stable API”): - Use Runtime.FiberFailureId to detect FiberFailure in an unknown error. - Use Runtime.FiberFailureCauseId (or the Cause value inside the FiberFailure) to retrieve the full Cause tree, which you can then pattern-match using Cause.isFailType / isDie / isInterruptType / etc. The symbol-based identification is the stable mechanism exposed by the library. If you meant a human-readable “description” for FiberFailureCauseId itself: the public docs/code treat it as an identifier symbol (not a message string). The “description” is effectively the embedded Cause value (with tags like Interrupt) rather than any string attached to FiberFailureCauseId.
Citations:
- 1: https://effect-ts.github.io/effect/effect/Runtime.ts.html
- 2: https://github.com/Effect-TS/effect/blob/82996bce/packages/effect/src/Runtime.ts
- 3: https://effect.website/docs/data-types/cause/
🏁 Script executed:
# Find package.json and check Effect version
find . -name "package.json" -type f | head -5 | xargs grep -l "effect" 2>/dev/null | head -1 | xargs cat | grep -A 2 -B 2 '"effect"'Repository: manaflow-ai/cmux
Length of output: 192
🏁 Script executed:
# Examine the imports and context in the errors.ts file
head -50 web/services/vms/errors.tsRepository: manaflow-ai/cmux
Length of output: 1613
🏁 Script executed:
# View the context around the effectFiberFailureCause function (lines 120-140)
sed -n '120,140p' web/services/vms/errors.tsRepository: manaflow-ai/cmux
Length of output: 961
🏁 Script executed:
# Check all imports in the file to see if Runtime is already imported
grep -n "^import\|^export" web/services/vms/errors.ts | head -20Repository: manaflow-ai/cmux
Length of output: 1684
🏁 Script executed:
# Search for usage of effectFiberFailureCause in the codebase
rg "effectFiberFailureCause" --type tsRepository: manaflow-ai/cmux
Length of output: 222
🏁 Script executed:
# Check the Effect library structure to verify Runtime exports
npm list effect 2>/dev/null | grep effect || echo "Checking package.json directly"; grep '"effect"' package.jsonRepository: manaflow-ai/cmux
Length of output: 91
🏁 Script executed:
# Create a small test to verify Runtime symbols are available and correct
cat > /tmp/test_effect.js << 'EOF'
// Check if we can access Runtime from effect
try {
const Runtime = require('effect/Runtime');
console.log("Runtime module exists");
console.log("FiberFailureId:", typeof Runtime.FiberFailureId, Runtime.FiberFailureId?.description);
console.log("FiberFailureCauseId:", typeof Runtime.FiberFailureCauseId, Runtime.FiberFailureCauseId?.description);
} catch (e) {
console.log("Error accessing Runtime:", e.message);
// Try alternative path
try {
const EffectModule = require('effect');
console.log("Effect module keys:", Object.keys(EffectModule).filter(k => k.includes('Runtime') || k.includes('FiberFailure')).slice(0, 10));
} catch (e2) {
console.log("Error with effect module:", e2.message);
}
}
EOF
node /tmp/test_effect.js 2>&1 || echo "Node test failed, checking docs instead"Repository: manaflow-ai/cmux
Length of output: 249
🏁 Script executed:
# Search the repository for any existing use of Runtime from effect
rg "from.*effect.*Runtime|import.*Runtime" --type tsRepository: manaflow-ai/cmux
Length of output: 187
Replace symbol description lookup with Effect's stable public API.
The function searches for a symbol by its description string "effect/Runtime/FiberFailure/Cause", which is an internal implementation detail. Effect exports stable public symbols for this: Runtime.FiberFailureId (to identify FiberFailure) and Runtime.FiberFailureCauseId (to access the Cause). Replace the current implementation with the public API to eliminate version fragility:
import { Runtime } from "effect";
function effectFiberFailureCause(err: object): unknown {
return Runtime.FiberFailureCauseId in err ? (err as Record<symbol, unknown>)[Runtime.FiberFailureCauseId] : null;
}This uses the officially supported stable symbols rather than relying on internal symbol descriptions that can change across Effect versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/services/vms/errors.ts` around lines 128 - 133, The function
effectFiberFailureCause currently looks up a symbol by its description string
which is fragile; update it to use Effect's stable public API by importing
Runtime from "effect" and checking for Runtime.FiberFailureCauseId on the err
object (instead of searching descriptions) and, if present, return (err as
Record<symbol, unknown>)[Runtime.FiberFailureCauseId], otherwise return null;
modify the import and the body of effectFiberFailureCause to use
Runtime.FiberFailureCauseId (and optionally Runtime.FiberFailureId where
relevant) to access the cause.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/services/vms/README.md (1)
226-228:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDoc still describes a fixed one-credit reservation.
The implementation now resolves a per-plan/global create-credit amount, so this runbook is stale. Please describe the workflow as reserving and refunding the configured amount (default
1) instead of always “one” credit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/services/vms/README.md` around lines 226 - 228, Update the README text to stop saying a fixed “one” credit is reserved and instead state that the system reserves and refunds the configured create-credit amount (default 1) for the plan/global setting; reference the relevant env vars CMUX_VM_CREATE_CREDIT_ITEM_ID and per-plan CMUX_VM_PLAN_<PLAN>_CREATE_CREDIT_ITEM_ID, note that the create workflow inserts an idempotent VM row (billing_team_id, billing_plan_id), reserves the configured amount only for newly inserted rows, calls the provider, and refunds that configured amount if provisioning fails before a usable VM exists.
🧹 Nitpick comments (1)
web/app/api/vm/route.ts (1)
44-64: ⚡ Quick winPull billing-team resolution out of the route handlers.
GET and POST now each do request extraction, entitlement resolution, typed-error mapping, and span setup inline. That duplicates business logic in the App Router boundary and makes the two paths easy to drift. A shared helper/Effect for “resolve requested VM billing context” would keep these handlers parse-only and leave them with one boundary call plus HTTP mapping.
As per coding guidelines,
web/app/api/**/*.{ts,tsx}: Keep Next route handlers thin: parse the request, run one Effect program at the boundary, map typed errors to HTTP responses, and treat unexpected defects separately.Also applies to: 186-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/vm/route.ts` around lines 44 - 64, Extract the inline billing-team resolution logic into a shared helper (e.g., resolveRequestedVmBillingContext) that takes the Request, user and span and returns a typed result containing billingTeamId, billingCustomerType, planId or throws the existing typed errors; move the code that calls requestedVmTeamIdFromRequest, resolveVmEntitlements, sets billingTeamId and calls setSpanAttributes into that helper, keep the route handlers (GET/POST) to only parse the request and call this single helper, and map typed errors using isVmBillingTeamResolutionError to JSON responses while letting unexpected errors bubble as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/services/vms/entitlements.ts`:
- Around line 63-85: The team-billing paths currently fall back to the caller's
userBillingPlanId (see the requestedTeamId branch returning billingPlanId:
team.billingPlanId ?? user.userBillingPlanId and the user.billingCustomerType
=== "team" branch), which must be removed; change those returns to use only the
team's billingPlanId and, if missing, the system default/free plan (do not
reference user.userBillingPlanId). Also update authedUserFromStackUser() so its
precomputed billingPlanId does not apply the user-level fallback when the
context is team-billed—ensure team contexts read only team.billingPlanId (or
default) and only non-team contexts may use user.userBillingPlanId.
---
Duplicate comments:
In `@web/services/vms/README.md`:
- Around line 226-228: Update the README text to stop saying a fixed “one”
credit is reserved and instead state that the system reserves and refunds the
configured create-credit amount (default 1) for the plan/global setting;
reference the relevant env vars CMUX_VM_CREATE_CREDIT_ITEM_ID and per-plan
CMUX_VM_PLAN_<PLAN>_CREATE_CREDIT_ITEM_ID, note that the create workflow inserts
an idempotent VM row (billing_team_id, billing_plan_id), reserves the configured
amount only for newly inserted rows, calls the provider, and refunds that
configured amount if provisioning fails before a usable VM exists.
---
Nitpick comments:
In `@web/app/api/vm/route.ts`:
- Around line 44-64: Extract the inline billing-team resolution logic into a
shared helper (e.g., resolveRequestedVmBillingContext) that takes the Request,
user and span and returns a typed result containing billingTeamId,
billingCustomerType, planId or throws the existing typed errors; move the code
that calls requestedVmTeamIdFromRequest, resolveVmEntitlements, sets
billingTeamId and calls setSpanAttributes into that helper, keep the route
handlers (GET/POST) to only parse the request and call this single helper, and
map typed errors using isVmBillingTeamResolutionError to JSON responses while
letting unexpected errors bubble as before.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2997008-73aa-46cd-a2d5-7aa5caefa3c9
📒 Files selected for processing (9)
Sources/Cloud/VMClient.swiftweb/app/api/vm/route.tsweb/services/vms/README.mdweb/services/vms/auth.tsweb/services/vms/entitlements.tsweb/services/vms/repository.tsweb/services/vms/routeHelpers.tsweb/services/vms/workflows.tsweb/tests/vm-route-auth.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/services/vms/workflows.ts
| const requestedTeamId = normalizedOptionalString(options.requestedBillingTeamId); | ||
| if (requestedTeamId) { | ||
| const team = user.teams.find((candidate) => candidate.id === requestedTeamId); | ||
| if (!team) { | ||
| throw new VmBillingTeamResolutionError({ | ||
| code: "vm_billing_team_not_found", | ||
| status: 403, | ||
| message: "The requested billing team is not available for this Stack Auth user.", | ||
| }); | ||
| } | ||
| return { | ||
| billingCustomerType: "team", | ||
| billingTeamId: team.id, | ||
| billingPlanId: team.billingPlanId ?? user.userBillingPlanId, | ||
| }; | ||
| } | ||
|
|
||
| if (user.billingCustomerType === "team") { | ||
| return { | ||
| billingCustomerType: "team", | ||
| billingTeamId: user.billingTeamId, | ||
| billingPlanId: user.billingPlanId ?? user.userBillingPlanId, | ||
| }; |
There was a problem hiding this comment.
Don’t inherit a team plan from userBillingPlanId.
For team-billed requests, the new team.billingPlanId ?? user.userBillingPlanId fallback can grant a team the caller’s personal paid plan whenever the team metadata is missing or stale. That changes maxActiveVms and can bypass the free-plan create-credit gate for the wrong billing team. Team paths should use team metadata only here and fall back to the default/free plan if the team has no plan marker.
Suggested direction
if (requestedTeamId) {
const team = user.teams.find((candidate) => candidate.id === requestedTeamId);
if (!team) {
throw new VmBillingTeamResolutionError({
code: "vm_billing_team_not_found",
status: 403,
message: "The requested billing team is not available for this Stack Auth user.",
});
}
return {
billingCustomerType: "team",
billingTeamId: team.id,
- billingPlanId: team.billingPlanId ?? user.userBillingPlanId,
+ billingPlanId: team.billingPlanId,
};
}
if (user.billingCustomerType === "team") {
return {
billingCustomerType: "team",
billingTeamId: user.billingTeamId,
- billingPlanId: user.billingPlanId ?? user.userBillingPlanId,
+ billingPlanId: user.billingPlanId,
};
}Also align authedUserFromStackUser() so its precomputed billingPlanId does not apply the same user-level fallback for team contexts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const requestedTeamId = normalizedOptionalString(options.requestedBillingTeamId); | |
| if (requestedTeamId) { | |
| const team = user.teams.find((candidate) => candidate.id === requestedTeamId); | |
| if (!team) { | |
| throw new VmBillingTeamResolutionError({ | |
| code: "vm_billing_team_not_found", | |
| status: 403, | |
| message: "The requested billing team is not available for this Stack Auth user.", | |
| }); | |
| } | |
| return { | |
| billingCustomerType: "team", | |
| billingTeamId: team.id, | |
| billingPlanId: team.billingPlanId ?? user.userBillingPlanId, | |
| }; | |
| } | |
| if (user.billingCustomerType === "team") { | |
| return { | |
| billingCustomerType: "team", | |
| billingTeamId: user.billingTeamId, | |
| billingPlanId: user.billingPlanId ?? user.userBillingPlanId, | |
| }; | |
| const requestedTeamId = normalizedOptionalString(options.requestedBillingTeamId); | |
| if (requestedTeamId) { | |
| const team = user.teams.find((candidate) => candidate.id === requestedTeamId); | |
| if (!team) { | |
| throw new VmBillingTeamResolutionError({ | |
| code: "vm_billing_team_not_found", | |
| status: 403, | |
| message: "The requested billing team is not available for this Stack Auth user.", | |
| }); | |
| } | |
| return { | |
| billingCustomerType: "team", | |
| billingTeamId: team.id, | |
| billingPlanId: team.billingPlanId, | |
| }; | |
| } | |
| if (user.billingCustomerType === "team") { | |
| return { | |
| billingCustomerType: "team", | |
| billingTeamId: user.billingTeamId, | |
| billingPlanId: user.billingPlanId, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/services/vms/entitlements.ts` around lines 63 - 85, The team-billing
paths currently fall back to the caller's userBillingPlanId (see the
requestedTeamId branch returning billingPlanId: team.billingPlanId ??
user.userBillingPlanId and the user.billingCustomerType === "team" branch),
which must be removed; change those returns to use only the team's billingPlanId
and, if missing, the system default/free plan (do not reference
user.userBillingPlanId). Also update authedUserFromStackUser() so its
precomputed billingPlanId does not apply the user-level fallback when the
context is team-billed—ensure team contexts read only team.billingPlanId (or
default) and only non-team contexts may use user.userBillingPlanId.
This branch was successfully deployed
1 participant
Summary
cmux-vm-create-credititem by defaultTests
bun tsc --noEmitbun testbun run db:checkgit diff --checkSummary by cubic
Limits free Cloud VM creates by spending Stack Auth team credits on the free plan and adds explicit team billing selection and filtering to avoid ambiguous creates.
New Features
cmux-vm-create-creditper create by default. SupportsCMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_IDandCMUX_VM_PLAN_FREE_CREATE_CREDIT_COST[_<PROVIDER>], plus globalCMUX_VM_CREATE_CREDIT_ITEM_IDandCMUX_VM_CREATE_CREDIT_COST[_<PROVIDER>]. Set any item id tonone/disabled/off/falseto disable. On reserve errors, mark the VM failed withbilling_reserve_failedand emitvm.create.billing_failed. Error unwrapping is clearer.X-Cmux-Team-IdorbillingTeamId/teamIdin the request and filters list by team. Validates membership; returnsvm_billing_team_requiredorvm_billing_team_not_foundwhen needed. The Swift client now sendsX-Cmux-Team-Id.Migration
cmux-vm-create-credit. Enable personal team creation on sign-up so single-team users can create without an explicit team.CMUX_VM_PLAN_FREE_CREATE_CREDIT_ITEM_ID=none. Paid plans only use credits whenCMUX_VM_PLAN_<PLAN>_CREATE_CREDIT_ITEM_IDorCMUX_VM_CREATE_CREDIT_ITEM_IDis set.Written for commit c04e92f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Chores
Bug Fixes
Tests