-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(webapp): enforce watch plan limits #4556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/agent-message-quota-tri-12863
Are you sure you want to change the base?
Changes from all commits
f9d674e
00933ab
063e41e
21f5461
2dd410d
7d2efc9
bce03cb
2db7c39
3fbc04a
743644b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| area: webapp | ||
| type: feature | ||
| --- | ||
|
|
||
| Watches now respect your plan's limits: free plans can run a limited number of watches at once and for a shorter window, with a prompt to upgrade for more. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| area: webapp | ||
| type: fix | ||
| --- | ||
|
|
||
| A single stuck assistant investigation can no longer hold up others from being tidied away. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,11 @@ | |
|
|
||
| import { | ||
| listStaleOpenInvestigations, | ||
| recordInvestigationSweepAttempt, | ||
| settleInvestigationAndCloseCard, | ||
| settleInvestigationAsInconclusive, | ||
| type Investigation, | ||
| type SettledInvestigation, | ||
| type SettledInvestigationCard, | ||
| } from "@internal/dashboard-agent-db"; | ||
| import { UNSETTLED_INVESTIGATION_NOTE } from "@internal/dashboard-agent-contracts"; | ||
|
|
@@ -22,6 +25,13 @@ export const INVESTIGATION_STALE_MS = 30 * 60 * 1000; | |
| /** Per-run cap. Oldest first, so the rest land next run. */ | ||
| const SWEEP_BATCH_LIMIT = 100; | ||
|
|
||
| /** | ||
| * After this many failed settle attempts a row is force-abandoned: settled `inconclusive` | ||
| * WITHOUT the closing card, so a card that never renders leaves the queue instead of | ||
| * looping forever. The rare stuck spinner is the price of not starving every other row. | ||
| */ | ||
| export const MAX_SWEEP_ATTEMPTS = 5; | ||
|
|
||
| export type InvestigationSweepResult = { | ||
| /** Stale `in_progress` rows seen. */ | ||
| stale: number; | ||
|
|
@@ -30,6 +40,8 @@ export type InvestigationSweepResult = { | |
| closed: number; | ||
| /** A turn (or another sweep) settled it first. */ | ||
| alreadySettled: number; | ||
| /** Rows past the attempt cap, force-settled without a card so they leave the queue. */ | ||
| abandoned: number; | ||
| failed: number; | ||
| }; | ||
|
|
||
|
|
@@ -46,6 +58,10 @@ export type InvestigationSweepDeps = { | |
| chatId: string; | ||
| note: string; | ||
| }) => Promise<SettledInvestigationCard | null>; | ||
| /** Record a failed settle out-of-band; returns the new attempt count, or null if gone. */ | ||
| recordAttempt?: (params: { id: string }) => Promise<number | null>; | ||
| /** Force a poison row terminal without the failing render path. */ | ||
| forceAbandon?: (params: { id: string; note: string }) => Promise<SettledInvestigation | null>; | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -61,12 +77,17 @@ export async function sweepDashboardAgentInvestigations( | |
| deps.listStale ?? ((params) => listStaleOpenInvestigations(dashboardAgentDb, params)); | ||
| const settleAndClose = | ||
| deps.settleAndClose ?? ((params) => settleInvestigationAndCloseCard(dashboardAgentDb, params)); | ||
| const recordAttempt = | ||
| deps.recordAttempt ?? ((params) => recordInvestigationSweepAttempt(dashboardAgentDb, params)); | ||
| const forceAbandon = | ||
| deps.forceAbandon ?? ((params) => settleInvestigationAsInconclusive(dashboardAgentDb, params)); | ||
|
|
||
| const result: InvestigationSweepResult = { | ||
| stale: 0, | ||
| settled: 0, | ||
| closed: 0, | ||
| alreadySettled: 0, | ||
| abandoned: 0, | ||
| failed: 0, | ||
| }; | ||
|
|
||
|
|
@@ -93,10 +114,49 @@ export async function sweepDashboardAgentInvestigations( | |
| result.settled++; | ||
| if (outcome.closed) result.closed++; | ||
| } catch (error) { | ||
| // The settle rolled back, so the row is still `in_progress`. Record the attempt in | ||
| // its own write — this rotates the row to the back of the sweep order (see | ||
| // `listStaleOpenInvestigations`) so it can't pin the head and starve newer rows. | ||
| let attempts: number | null = null; | ||
| try { | ||
| attempts = await recordAttempt({ id: investigation.id }); | ||
| } catch (recordError) { | ||
| logger.error("Dashboard agent investigation sweep: failed to record a sweep attempt", { | ||
| investigationId: investigation.id, | ||
| chatId: investigation.chatId, | ||
| error: recordError, | ||
| }); | ||
| } | ||
|
|
||
| // 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, | ||
| }); | ||
| } | ||
| } | ||
|
Comment on lines
+131
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 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 Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| result.failed++; | ||
| logger.error("Dashboard agent investigation sweep: failed to settle an investigation", { | ||
| investigationId: investigation.id, | ||
| chatId: investigation.chatId, | ||
| attempts, | ||
| error, | ||
| }); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import type { Limits } from "@trigger.dev/platform"; | ||
| import { WATCH_MAX_HOURS } from "@internal/dashboard-agent-contracts"; | ||
| import { getCachedLimit, isBillingConfigured } from "./platform.v3.server"; | ||
|
|
||
| // The unlimited sentinel, matching the message quota (TRI-12863 P1). Never Infinity: it | ||
| // serializes to null in the limit cache. | ||
| export const UNLIMITED_WATCH_LIMIT = 100_000_000; | ||
|
|
||
| // 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; | ||
|
Comment on lines
+9
to
+12
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Limit keys are cast to
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| export type WatchPlanLimits = { | ||
| /** Longest window one watch may run for, in hours. */ | ||
| maxHours: number; | ||
| /** How many active watches the org may run at once. */ | ||
| watchers: number; | ||
| }; | ||
|
|
||
| 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; | ||
| } | ||
|
Comment on lines
+21
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 A plan limit configured as 0 is treated as unlimited
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| /** | ||
| * 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 }; | ||
| } | ||
|
Comment on lines
+21
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Plan-limit resolution is not wrapped in a fail-open try/catch, unlike the message quota
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| /** | ||
| * The window ceiling actually in force: the plan floor under the code ceiling. A plan that | ||
| * allows 100 hours still caps at {@link WATCH_MAX_HOURS}. | ||
| */ | ||
| export function effectiveWatchMaxHours(planMaxHours: number): number { | ||
| return Math.min(planMaxHours, WATCH_MAX_HOURS); | ||
| } | ||
|
|
||
| /** | ||
| * A watch-limit refusal, plus an upgrade nudge when billing is present. Self-hosted never | ||
| * hits this (fails open above), and the nudge is gated so a stray refusal stays quiet there. | ||
| */ | ||
| export function watchLimitHint(base: string, billingConfigured = isBillingConfigured()): string { | ||
| return billingConfigured ? `${base} Upgrade your plan for more.` : base; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import { | |
| cancelWatch, | ||
| chatExists, | ||
| claimWatchSubmission, | ||
| countActiveWatchesForOrg, | ||
| createChat, | ||
| createWatch, | ||
| generateWatchId, | ||
|
|
@@ -68,6 +69,12 @@ import { | |
| import { watchCreationCheckDeps } from "~/services/dashboardAgentWatchChecks.server"; | ||
| import { normalizeErrorFingerprint } from "~/services/dashboardAgentWatchErrorChecks"; | ||
| import { subscribeUserToWatchAlerts } from "~/services/dashboardAgentWatchAlerts.server"; | ||
| import { | ||
| effectiveWatchMaxHours, | ||
| resolveWatchPlanLimits, | ||
| watchLimitHint, | ||
| type WatchPlanLimits, | ||
| } from "~/services/dashboardAgentWatchLimits.server"; | ||
| import { | ||
| mintDashboardAgentWatchBatchToken, | ||
| mintDashboardAgentWatchToken, | ||
|
|
@@ -165,6 +172,7 @@ export async function authorizeWatchEnvironmentById(params: { | |
|
|
||
| export type CreateWatchErrorCode = | ||
| | "limit_reached" | ||
| | "watch_limit_reached" | ||
| | "duplicate" | ||
| | "invalid_target" | ||
| | "chat_not_found" | ||
|
|
@@ -273,6 +281,12 @@ export async function createDashboardAgentWatch(params: { | |
| scheduleTick?: typeof scheduleWatchTick; | ||
| /** Skip the real trigger-config gate when a tick scheduler is injected. */ | ||
| configured?: () => boolean; | ||
| /** Plan floors on window and count. Fails open to unlimited when absent. */ | ||
| resolveLimits?: (organizationId: string) => Promise<WatchPlanLimits>; | ||
| /** Org-wide active-watch count, for the watcher-count floor. */ | ||
| countActiveWatches?: (organizationId: string) => Promise<number>; | ||
| /** Gates the upgrade nudge, so self-hosted stays quiet. */ | ||
| billingConfigured?: () => boolean; | ||
| }; | ||
| }): Promise<CreateDashboardAgentWatchResult> { | ||
| const { environment, userId, chatId } = params; | ||
|
|
@@ -284,6 +298,11 @@ export async function createDashboardAgentWatch(params: { | |
| const buildCheckDeps = params.deps?.checkDeps ?? watchCreationCheckDeps; | ||
| const scheduleTick = params.deps?.scheduleTick ?? scheduleWatchTick; | ||
| const isDashboardAgentConfigured = params.deps?.configured ?? isDashboardAgentConfiguredDefault; | ||
| const resolveLimits = params.deps?.resolveLimits ?? resolveWatchPlanLimits; | ||
| const countActiveWatches = | ||
| params.deps?.countActiveWatches ?? | ||
| ((organizationId: string) => countActiveWatchesForOrg(dashboardAgentDb, { organizationId })); | ||
| const hint = (base: string) => watchLimitHint(base, params.deps?.billingConfigured?.()); | ||
| const checkDeps = buildCheckDeps(environment, now); | ||
|
|
||
| if (!isDashboardAgentConfigured()) { | ||
|
|
@@ -317,6 +336,17 @@ export async function createDashboardAgentWatch(params: { | |
| }); | ||
| if (!precheck.ok) return creationGuardrailError(precheck); | ||
|
|
||
| // 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."), | ||
| }; | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| } | ||
|
Comment on lines
+343
to
+348
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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 ( Status mapping in the dashboard route lacks the new refusal code
The MCP route was updated ( Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+339
to
+348
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 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 ( Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| // `since` is server-set so the model can't backdate a recurrence window. | ||
| const persistedSpec: PersistedWatchSpec = | ||
| spec.kind === "error_recurrence" ? { ...spec, since: now.toISOString() } : spec; | ||
|
|
@@ -331,6 +361,18 @@ export async function createDashboardAgentWatch(params: { | |
| return { ok: true, watching: false, identity, immediate }; | ||
| } | ||
|
|
||
| // Counted only now the immediate check didn't answer: a one-shot creates no row and so | ||
| // consumes no watcher slot. The per-chat cap of 3 still applies independently, in | ||
| // `createWatch`. | ||
| 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."), | ||
| }; | ||
| } | ||
|
Comment on lines
+367
to
+374
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Refusals are recorded in the submission ledger, so a retry after upgrading replays the refusal A Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| const expiresAt = new Date(now.getTime() + spec.maxHours * 60 * 60 * 1000); | ||
|
|
||
| const created = await createWatch(dashboardAgentDb, { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 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.mdstates: "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, newsweep_attemptsschema/migration).Was this helpful? React with 👍 or 👎 to provide feedback.