feat(webapp): enforce watch plan limits - #4556
Conversation
Refuse a watch whose window exceeds the plan's agentWatchMaxHours, or that would push the org past its agentWatchers count, with a new watch_limit_reached result carrying an upgrade hint. Plan limits are a floor below the existing code ceilings (min(plan, WATCH_MAX_HOURS=24) and the per-chat cap of 3, which still apply independently). Fails open: an absent limit resolves to unlimited, so self-hosted is unaffected and the upgrade nudge is gated on billing presence. TRI-12863
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
…63' into feat/agent-watch-limits-tri-12863
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return { | ||
| ok: false, | ||
| code: "watch_limit_reached", | ||
| error: hint("That watch window is longer than your plan allows."), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟡 Dashboard watch card refusals from plan limits return a server error instead of a normal rejection
The new plan-limit refusal is returned with a code the dashboard's watch-submit endpoint doesn't recognise (code: "watch_limit_reached" at apps/webapp/app/services/dashboardAgentWatches.server.ts:345), so a perfectly ordinary "your plan doesn't allow this" answer is sent back as an internal server error.
Impact: Users hitting their plan's watch limit from the dashboard card trigger 500 responses, which pollute error monitoring and can be treated as outages rather than expected rejections.
Status mapping in the dashboard route lacks the new refusal code
createDashboardAgentWatch now returns watch_limit_reached for both the window floor (apps/webapp/app/services/dashboardAgentWatches.server.ts:342-348) and the watcher-count floor (apps/webapp/app/services/dashboardAgentWatches.server.ts:367-374). This code propagates through submitDashboardAgentWatch (SubmitWatchErrorCode = CreateWatchErrorCode | "request_conflict").
The MCP route was updated (apps/webapp/app/routes/api.v1.dashboard-agent.watches.ts:113-118 maps it to 409), but the dashboard resource route's ladder at apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts:542-551 only lists limit_reached, duplicate, request_conflict, invalid_target, chat_not_found, not_configured, and otherwise falls through to 500. A plan-limit refusal therefore returns HTTP 500 with the upgrade message in the body.
Prompt for agents
The new refusal code `watch_limit_reached` produced by createDashboardAgentWatch is not handled by the dashboard watch-submit route's HTTP status ladder in apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts (around lines 542-551), so plan-limit refusals fall through to 500. The MCP route api.v1.dashboard-agent.watches.ts was updated to map it to 409. Update the dashboard route's mapping to treat watch_limit_reached the same way (409), keeping the two routes consistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Plan floors sit below the code ceilings (min(plan, ceiling)). Fails open: an absent | ||
| // limit resolves to unlimited, so neither floor bites on self-hosted. | ||
| const planLimits = await resolveLimits(environment.organizationId); | ||
| if (spec.maxHours > effectiveWatchMaxHours(planLimits.maxHours)) { | ||
| return { | ||
| ok: false, | ||
| code: "watch_limit_reached", | ||
| error: hint("That watch window is longer than your plan allows."), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔍 Window floor also refuses one-shots, unlike the watcher-count floor
The window check runs before the immediate check, while the watcher-count check deliberately runs after it (apps/webapp/app/services/dashboardAgentWatches.server.ts:364-374) so that a one-shot consumes no slot. Consequence: a request whose condition is already satisfied — which would create no row at all — is still refused purely because the requested window exceeds the plan's agentWatchMaxHours. If the intent is "plan limits constrain what is actually persisted", the window check should arguably also sit after the immediate check, or the card should clamp the window instead of refusing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Filled by cloud billing (TRI-12863 P0). Absent until then, and always on self-hosted, so | ||
| // the fallback applies and the plan floor is off. | ||
| const WATCH_MAX_HOURS_LIMIT_KEY = "agentWatchMaxHours" as keyof Limits; | ||
| const WATCH_COUNT_LIMIT_KEY = "agentWatchers" as keyof Limits; |
There was a problem hiding this comment.
🔍 Limit keys are cast to keyof Limits before they exist in the platform type
agentWatchMaxHours and agentWatchers are asserted into keyof Limits. Until the cloud billing side ships those keys, getLimit will never find them and both resolve to the unlimited sentinel — the documented fail-open behaviour. Worth tracking that the string names here match exactly what billing emits; a typo would silently keep limits disabled forever with no signal, since the fallback path is indistinguishable from "not configured".
Was this helpful? React with 👍 or 👎 to provide feedback.
| async function readLimit(organizationId: string, key: keyof Limits): Promise<number> { | ||
| const cached = await getCachedLimit(organizationId, key, UNLIMITED_WATCH_LIMIT); | ||
| // A cache error leaves `val` empty; fall open to unlimited. | ||
| return cached.val ?? UNLIMITED_WATCH_LIMIT; | ||
| } |
There was a problem hiding this comment.
🔍 A plan limit configured as 0 is treated as unlimited
readLimit delegates to getCachedLimit -> getLimit, which does if (!result) return fallback (apps/webapp/app/services/platform.v3.server.ts:426). A plan that legitimately sets agentWatchers: 0 or agentWatchMaxHours: 0 is therefore indistinguishable from an absent limit and falls open to the unlimited sentinel. Worth knowing before the cloud side (TRI-12863 P0) starts publishing these keys — a "no watches" tier cannot be expressed with 0.
Was this helpful? React with 👍 or 👎 to provide feedback.
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
| export async function countActiveWatchesForOrg( | ||
| db: DashboardAgentDb, | ||
| params: { organizationId: string } | ||
| ): Promise<number> { | ||
| const rows = await db | ||
| .select({ count: sql<number>`count(*)::int` }) | ||
| .from(watches) | ||
| .where(and(eq(watches.status, "active"), eq(watches.organizationId, params.organizationId))); | ||
| return rows[0]?.count ?? 0; | ||
| } |
There was a problem hiding this comment.
🔍 Expired-but-unswept watches still consume plan slots
countActiveWatchesForOrg counts every row with status = 'active', including watches whose expiresAt has already passed but whose tick/sweep hasn't yet flipped them to expired. On an org at its plan count, this can transiently refuse a legitimate new watch until the sweep runs. Adding expiresAt > now() to the predicate (or relying on the sweep cadence being short) would avoid the surprise; worth confirming the sweep interval relative to the smallest plan count.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Past the cap the card will never render; force it terminal without the render | ||
| // path so it leaves the queue instead of looping forever. | ||
| if (attempts !== null && attempts >= MAX_SWEEP_ATTEMPTS) { | ||
| try { | ||
| await forceAbandon({ id: investigation.id, note: UNSETTLED_INVESTIGATION_NOTE }); | ||
| result.abandoned++; | ||
| logger.warn( | ||
| "Dashboard agent investigation sweep: abandoned a card past the attempt cap", | ||
| { | ||
| investigationId: investigation.id, | ||
| chatId: investigation.chatId, | ||
| attempts, | ||
| } | ||
| ); | ||
| continue; | ||
| } catch (abandonError) { | ||
| logger.error("Dashboard agent investigation sweep: failed to abandon a poison card", { | ||
| investigationId: investigation.id, | ||
| chatId: investigation.chatId, | ||
| error: abandonError, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 Attempt cap can force-abandon a renderable card after transient settle failures
The cap treats 5 failed settles as proof the card "will never render", but settleInvestigationAndCloseCard can also throw for transient reasons (e.g. a conflict inside appendChatMessageOnceByChatId, a connection reset while the attempt-recording write still succeeds). The maintenance job runs every 5 minutes with maxAttempts: 1 (apps/webapp/app/v3/commonWorker.server.ts:165-172), so ~25 minutes of intermittent-but-not-total failure is enough to force-settle a perfectly renderable row without its closing card — which is exactly the permanent spinner settleInvestigationAndCloseCard exists to prevent. Note a full DB outage is safe here (the attempt write fails too, leaving attempts null). Consider only counting attempts for the non-renderable error, or resetting sweep_attempts when the error differs.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const activeCount = await countActiveWatches(environment.organizationId); | ||
| if (activeCount >= planLimits.watchers) { | ||
| return { | ||
| ok: false, | ||
| code: "watch_limit_reached", | ||
| error: hint("You've reached the number of active watches your plan allows."), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔍 Refusals are recorded in the submission ledger, so a retry after upgrading replays the refusal
A watch_limit_reached refusal from the card submit path goes through refuse(...), which writes the refusal into the submission ledger; a subsequent retry with the same clientRequestId replays the recorded refusal verbatim instead of re-evaluating (apps/webapp/app/services/dashboardAgentWatches.server.ts:764-778). A user who upgrades their plan and retries the same card must start a fresh submission (new request id) to succeed. This matches the existing ledger semantics for other refusals, but plan limits are the first refusal class that a user can clear themselves, so the UX is worth checking on the card side.
Was this helpful? React with 👍 or 👎 to provide feedback.
| --- | ||
| area: webapp | ||
| type: fix | ||
| --- | ||
|
|
||
| A single stuck assistant investigation can no longer hold up others from being tidied away. |
There was a problem hiding this comment.
🟡 Pull request bundles two unrelated changes
The change set combines the watch plan-limit feature with an unrelated fix to how stuck assistant investigations are swept (.server-changes/dashboard-agent-investigation-sweep-backoff.md), which the repository's contribution rules forbid.
Impact: Reviewers and release notes mix two independent behaviours, and either half cannot be reverted on its own.
Rule reference
CONTRIBUTING.md states: "We only accept PRs that address a single issue. Please do not submit PRs containing multiple unrelated fixes or features. If you have multiple contributions, open a separate PR for each one." This PR contains the watch plan-limit feature (apps/webapp/app/services/dashboardAgentWatchLimits.server.ts, dashboardAgentWatches.server.ts) and the investigation sweep backoff (apps/webapp/app/services/dashboardAgentInvestigationSweep.server.ts, new sweep_attempts schema/migration).
Was this helpful? React with 👍 or 👎 to provide feedback.
| async function readLimit(organizationId: string, key: keyof Limits): Promise<number> { | ||
| const cached = await getCachedLimit(organizationId, key, UNLIMITED_WATCH_LIMIT); | ||
| // A cache error leaves `val` empty; fall open to unlimited. | ||
| return cached.val ?? UNLIMITED_WATCH_LIMIT; | ||
| } | ||
|
|
||
| /** | ||
| * The org's plan floors for watches. Fails open: an absent limit (self-hosted, or before the | ||
| * cloud side ships) resolves to the unlimited sentinel, so neither floor bites. | ||
| */ | ||
| export async function resolveWatchPlanLimits(organizationId: string): Promise<WatchPlanLimits> { | ||
| const [maxHours, watchers] = await Promise.all([ | ||
| readLimit(organizationId, WATCH_MAX_HOURS_LIMIT_KEY), | ||
| readLimit(organizationId, WATCH_COUNT_LIMIT_KEY), | ||
| ]); | ||
| return { maxHours, watchers }; | ||
| } |
There was a problem hiding this comment.
🔍 Plan-limit resolution is not wrapped in a fail-open try/catch, unlike the message quota
resolveWatchPlanLimits relies on getCachedLimit never throwing to fail open. The analogous message-quota reader wraps the whole resolution in a try/catch and returns undefined (no cap) on any throw (apps/webapp/app/services/dashboardAgentQuota.server.ts:55-70). If the platform cache layer ever throws (e.g. a Redis error surfacing out of platformCache.limits.swr), watch creation will reject with a 500 rather than falling open to unlimited, which contradicts the "fails open" contract documented on this function.
Was this helpful? React with 👍 or 👎 to provide feedback.
The card-submit route's status ladder didn't handle watch_limit_reached, so a plan-limit refusal fell through to HTTP 500. Match the MCP route and return 409.
drizzle-kit generated them unformatted, failing the oxfmt --check code-quality gate.
1 participant
What & why. Watches now honour a plan's watch limits. A watch whose window exceeds the plan's
agentWatchMaxHours, or that would push the org past itsagentWatcherscount, is refused with a newwatch_limit_reachedresult and an upgrade hint (a chat line on the card, HTTP 409 on the API).Key decisions.
min(plan, WATCH_MAX_HOURS=24)for the window, and the per-chat cap of 3 still applies independently. Plans only tighten, never loosen.isBillingConfigured().TRI-12863