feat(webapp): dashboard agent — Watch - #4525
Conversation
"Tell me when this run finishes", "ping me if that error comes back". A watch checks on its own cadence, reports once, and stops within 24 hours. The user confirms a pre-filled card, so nothing starts behind their back; the answer lands in the chat, and by email if they asked for it. Restores the feature this branch's base PR set aside, unchanged: the card and chips, the wake banner and toast, the unread badge, the checks for runs, queues, errors and health, the batch scheduler and its backstops, the alert channel and its email, and the agent's schedule_watch tool with the prompt that governs it.
🦋 Changeset detectedLatest commit: f5e8580 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
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:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const watchCard = watchDraft ? ( | ||
| <WatchCard | ||
| draft={watchDraft} | ||
| onChange={setWatchDraft} | ||
| onSubmit={() => void submitWatch()} | ||
| onCancel={dismissWatchCard} | ||
| pending={watchPending} | ||
| error={watchError} | ||
| /> | ||
| ) : null; |
There was a problem hiding this comment.
🟡 A half-filled watch form follows you into another chat and starts the watch there
The pending watch form is kept in the panel's state (watchDraft at apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:426) when you move to a different conversation, so confirming it afterwards attaches the watch — and every later update it sends — to the conversation you moved to instead of the one it was offered in.
Impact: The update you asked for lands in a different conversation than the one you asked in, and the launcher's unread mark points you at the wrong place.
Why the draft outlives the chat it belongs to
watchDraft is panel-level state. It is cleared on submit (setWatchDraft(null) in submitWatch), on explicit cancel (dismissWatchCard), and when the organization changes (apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:284), but neither switchChat (apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:443-448), newChat (:436-441) nor the openChatRequest effect (:290-298, which the wake toast's "Open chat" drives) resets it.
watchCard is rendered by whichever child is mounted (:562 for DashboardAgentChat, :580 for DashboardAgentDraft), so the same card reappears under the newly opened transcript. submitWatch then posts chatId from the current active?.chatId (:391), so the server creates the watch — and later the wake — under the newly opened chat.
The explicit setWatchDraft(null) on the organization switch shows the same reset is wanted on a context change; the chat switch is the case that was missed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Recorded even on the final evaluation. Guarded on `active`, so a concurrent | ||
| // fire/expire wins and this no-ops. | ||
| await recordWatchCheck(dashboardAgentDb, { | ||
| id: watchId, | ||
| lastResult: { | ||
| result: outcome.result, | ||
| facts: outcome.facts, | ||
| observed: outcome.observed, | ||
| final: body.final === true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🔍 The per-watch check endpoint records an unavailable result, which the batch path deliberately avoids
dashboardAgentWatchBatch.server.ts is explicit that an unavailable outcome must not be recorded, because "writing it would move lastCheckedAt and overwrite the facts a streak lives in" (it calls recordWatchAttempt instead). This endpoint records unconditionally.
I walked this through and it does not currently corrupt a queue_stalled streak: the per-watch tick claims the row before calling this endpoint, so claimed.lastResult still holds the pre-check facts, and runWatchLifecycle then writes { checkFailed: true, previous: claimed.lastResult }, which previousCheckFacts unwraps. The lastCheckedAt advance is also harmless on the per-watch chain, which reschedules itself rather than relying on dueness.
It does become wrong if a watch is later polled by a batch chain while this endpoint is still being hit for it, since isDue reads lastCheckedAt — an unreadable source would then defer the watch a whole cadence. Worth aligning the two paths.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const fresh = (data.wakes ?? []).filter((wake) => !toastedWakes.current.has(wake.watchId)); | ||
| for (const wake of fresh) rememberToasted(wake.watchId); | ||
|
|
There was a problem hiding this comment.
🔍 Recent wakes are toasted even when already read, on any browser without the local dedupe
The toast source is data.wakes, which is recent deliveries (the loader's 15-minute window), not unread ones — the unread flag is used only for the dot. Dedupe is toastedWakes, a localStorage set capped at 50 ids. So a user who reads a wake on machine A and then opens the dashboard on machine B (or after clearing site data) within 15 minutes gets a persistent, never-expiring toast for a wake they have already read. The unread field is already on the payload; filtering the toast list by it (or by !wake.unread as an extra dedupe) would avoid re-announcing read wakes without weakening the in-chat-toast behaviour.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const [watching, setWatching] = useState(false); | ||
| useEffect(() => { | ||
| const sync = () => { | ||
| if ( | ||
| shouldPollWakeFeed({ | ||
| serverUnreadWakes: initialUnreadWakes, | ||
| serverHasActiveWatches: hasActiveWatches, | ||
| organizationId: organization.id, | ||
| }) | ||
| ) | ||
| setWatching(true); | ||
| }; | ||
| sync(); | ||
| return subscribeWatchActivity(sync); | ||
| }, [organization.id, initialUnreadWakes, hasActiveWatches]); |
There was a problem hiding this comment.
🔍 The wake poll never stops once started, even after everything is resolved
watching is a one-way latch: sync only ever calls setWatching(true), and forgetWatchActivity (called from loadHistory when no chat has an active watch or unread wake) neither notifies subscribers nor flips the flag. The file comments state this is deliberate ("Once any says yes this tab keeps polling"), so a long-lived tab keeps making one request a minute for the rest of the session even after the last watch resolves and is read. Worth confirming that is acceptable at fleet scale, or gating the latch off when a poll returns unreadWakes === 0 and the history shows no active watch.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx (1)
141-168: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA reload requested after a write can reuse an older in-flight request.
loadHistoryreturns the in-flight promise when one exists. The callers at Line 417 (submitWatch) and Line 501 (cancelWatch) run right after a server write. If a history fetch started before that write, the caller receives the older response, and the new watch chip or the removal does not appear until the next reload.Consider queuing a follow-up request when one is already in flight.
♻️ Sketch: chain a fresh request instead of reusing the in-flight one
- const loadHistory = useCallback(async () => { - if (historyInFlight.current) return historyInFlight.current; - const request = (async () => { + const loadHistory = useCallback(async () => { + const previous = historyInFlight.current; + const request = (async () => { + // Wait out an older request, so a reload after a write never reuses its response. + if (previous) await previous; try {
🧹 Nitpick comments (22)
apps/webapp/app/v3/commonWorker.server.ts (1)
165-174: 🩺 Stability & Availability | 🔵 TrivialNote the overlap between the visibility timeout and the cron period.
visibilityTimeoutMsis 5 minutes and the cron period is also 5 minutes. If a sweep exceeds the visibility timeout, the message becomes visible again and a second run can start while the first is still working.maxAttempts: 1limits retries but does not prevent that re-delivery.The existing
dashboardAgent.maintenanceentry uses the same values, so this matches current practice. Confirm thatsweepDashboardAgentWatchesandrearmDashboardAgentWatchBatchesare safe to run concurrently, or raise the visibility timeout above the period.internal-packages/emails/src/index.tsx (1)
136-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
headlinein the subject.The subject interpolates
data.identity, which is an internal condition key such asrun_finished:run_abc123. The payload also carriesheadline, the human sentence the panel shows. Readingheadlinefirst gives a clearer subject and keeps the email consistent with the in-app wording.
headlineis optional, so keepidentityas the fallback.♻️ Proposed subject change
case "alert-dashboard-agent-watch": { return { - subject: `[${data.organization}] Watch update: ${data.identity}`, + subject: `[${data.organization}] Watch update: ${data.headline ?? data.identity}`, component: <AlertDashboardAgentWatchEmail {...data} />, }; }apps/webapp/app/services/dashboardAgentWatchRunChecks.ts (1)
19-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep the terminal-status list in sync with
~/v3/taskStatus.
FINAL_STATUSEScurrently matchesFINAL_RUN_STATUSES, but imports should derive from the canonical source when possible. If the module must keep no server-side imports, add a type-level assertion that fails when this list drifts fromFINAL_RUN_STATUSES.apps/webapp/app/services/dashboardAgentWatches.server.ts (1)
930-954: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the shared
TriggerClientifTriggerOptionsincludestrigger.
TriggerClientis exported from@trigger.dev/sdk,tasks.triggeracceptsdelay,idempotencyKey, andversion, butidempotencyKeyTTLis only accepted by batch trigger options. If the single-task schedule path needs TTL, move it to batch scheduling; otherwise, create one module-level client for eachapiOriginto avoid repeated setup on each watch tick.Source: Coding guidelines
apps/webapp/app/services/dashboardAgentWatchSweep.server.ts (1)
126-139: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueA rejected authorization promise is cached for the whole sweep.
authorizeOncePerSweepstores the pending promise before it settles. Ifauthorizerejects, for example on a transient database error, every remaining watch in that group reuses the rejected promise. All those rows are then counted as failed in one sweep run instead of being retried independently. The next sweep run recovers them, so the impact is bounded. Consider evicting the entry on rejection.♻️ Proposed eviction on rejection
const cached = seen.get(key); if (cached) return cached; - const pending = authorize(watch); + const pending = authorize(watch).catch((error) => { + seen.delete(key); + throw error; + }); seen.set(key, pending); return pending;apps/webapp/app/services/dashboardAgentWatchToken.server.ts (1)
176-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
bearerTokenonly matches the exact schemeBearer.The scheme name in an
Authorizationheader is case-insensitive. A header ofbearer <token>leaves the prefix in place, so the extracted value fails the token prefix check and the caller receives 401. Match the scheme case-insensitively and allow repeated whitespace.♻️ Proposed scheme matching
- const value = raw.replace(/^Bearer /, "").trim(); + const value = raw.replace(/^Bearer\s+/i, "").trim();apps/webapp/app/services/dashboardAgentWatchBatch.server.ts (1)
167-175: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe authorization cache key omits
environmentId.
authorizeOncekeys on user, organization, and project. The equivalent helper inapps/webapp/app/services/dashboardAgentWatchSweep.server.tsat Line 132 also includesenvironmentId. The batch is scoped to a single environment byparams, so the two keys agree today. Align the keys so a future change to the row loader cannot silently reuse another environment's authorization.♻️ Proposed key alignment
- const key = `${watch.userId}:${watch.organizationId}:${watch.projectId}`; + const key = `${watch.userId}:${watch.organizationId}:${watch.projectId}:${watch.environmentId}`;apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.fired.ts (1)
94-99: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA failing release masks the enqueue error and strands the claim.
If
releaseWatchAlertDispatchthrows, the rethrow at Line 98 is never reached and the caller sees the release error instead of the enqueue error. The claim also stays held, so no later attempt can send the alert. Swallow and log the release failure, then rethrow the original error.♻️ Proposed error handling
try { await enqueueWatchFiredAlert(watch, "fired"); } catch (error) { - await releaseWatchAlertDispatch(dashboardAgentDb, { id: watch.id, terminalStatus: "fired" }); + await releaseWatchAlertDispatch(dashboardAgentDb, { + id: watch.id, + terminalStatus: "fired", + }).catch((releaseError) => + logger.error("Dashboard agent watch alert claim couldn't be released", { + watchId, + releaseError, + }) + ); throw error; }internal-packages/dashboard-agent/src/watch-delivery.ts (1)
176-177: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDetect a lost delivery fence when
markWatchDeliveredreturns null.
markWatchDeliveredreturnsWatch | null. A null result means theclaimIdfence no longer matches, so another deliverer took the claim after the stale window elapsed. The current code ignores that result and continues tonotifyFiredandnotifyInvestigate. Downstream dedup limits the damage, but the lost fence is invisible in logs.Log the null result so a duplicate-wake incident is diagnosable.
♻️ Proposed change
- await deps.store.markWatchDelivered({ id: claimed.id, claimId }); + const marked = await deps.store.markWatchDelivered({ id: claimed.id, claimId }); + if (!marked) { + // The claim was reclaimed after the stale window, so another deliverer may also + // append. The action id dedups the wake; record the race for diagnosis. + logger.warn("dashboard-agent watch delivery lost its claim after appending", { + watchId: claimed.id, + claimId, + }); + }internal-packages/dashboard-agent/src/watch-narration.ts (1)
50-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an exhaustiveness guard to the
categoryswitch.The switch has no
default. IfWatchPresentation["category"]gains a member, this function returnsundefinedat runtime while its declared return type staysstring. An exhaustiveness check turns that into a compile error instead. The repository already usesassert-neverfor this pattern inapps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.ts.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx (1)
98-120: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider running the promoted-prompt lookup and the activity read concurrently.
This loader runs on every environment-scoped page load.
getPromotedDashboardAgentPromptandreadDashboardAgentWakeActivityare both gated onhasDashboardAgentAccessand are independent, but they are awaited one after the other. That adds two serial round trips to a hot path.Run them with
Promise.allto remove one round trip. Keep the per-read failure isolation so a store outage still lets the dashboard load.♻️ Proposed concurrent read
- const promotedDashboardAgentPrompt = hasDashboardAgentAccess - ? await getPromotedDashboardAgentPrompt({ - orgFeatureFlags: (project.organization.featureFlags as Record<string, unknown>) ?? {}, - }) - : null; - - // One narrow read per page load, so the wake signal reaches a browser that has never opened - // the panel — including one whose watch hasn't fired yet. The poll never asks for this. - let dashboardAgentActivity: DashboardAgentWakeActivity = { - unreadWakes: 0, - hasActiveWatches: false, - }; - if (hasDashboardAgentAccess) { - try { - dashboardAgentActivity = await readDashboardAgentWakeActivity(dashboardAgentDb, { - organizationId: project.organization.id, - userId: user.id, - }); - } catch (error) { - // The dashboard must load even when the agent's store doesn't answer. - logger.error("Failed to read dashboard agent wake activity", { error }); - } - } + const NO_ACTIVITY: DashboardAgentWakeActivity = { unreadWakes: 0, hasActiveWatches: false }; + + // One narrow read per page load, so the wake signal reaches a browser that has never opened + // the panel — including one whose watch hasn't fired yet. The poll never asks for this. + const [promotedDashboardAgentPrompt, dashboardAgentActivity] = hasDashboardAgentAccess + ? await Promise.all([ + getPromotedDashboardAgentPrompt({ + orgFeatureFlags: (project.organization.featureFlags as Record<string, unknown>) ?? {}, + }), + readDashboardAgentWakeActivity(dashboardAgentDb, { + organizationId: project.organization.id, + userId: user.id, + }).catch((error) => { + // The dashboard must load even when the agent's store doesn't answer. + logger.error("Failed to read dashboard agent wake activity", { error }); + return NO_ACTIVITY; + }), + ]) + : [null, NO_ACTIVITY];apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.alerts.new/route.tsx (1)
55-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the alert-type list into one constant.
The literal list is now repeated on line 57 and line 61. This change had to update both branches by hand. The next alert type carries the same risk: if only the array branch is updated, a single-checkbox submission fails validation while a multi-checkbox submission succeeds.
Declare the values once and reuse them in both branches.
♻️ Proposed single source for the alert types
+const AlertTypeEnum = z.enum([ + "TASK_RUN", + "DEPLOYMENT_FAILURE", + "DEPLOYMENT_SUCCESS", + "DASHBOARD_AGENT_WATCH", +]); + const FormSchema = z .object({ - alertTypes: z - .array( - z.enum(["TASK_RUN", "DEPLOYMENT_FAILURE", "DEPLOYMENT_SUCCESS", "DASHBOARD_AGENT_WATCH"]) - ) - .min(1) - .or( - z.enum(["TASK_RUN", "DEPLOYMENT_FAILURE", "DEPLOYMENT_SUCCESS", "DASHBOARD_AGENT_WATCH"]) - ), + alertTypes: z.array(AlertTypeEnum).min(1).or(AlertTypeEnum),apps/webapp/test/dashboardAgentWatchCardRequestId.test.ts (1)
142-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the status of the keyed submit as well.
Line 160 only checks that the response body is not
invalid_request. A 500 response with a different error code also passes. Assert the expected status to keep the control case meaningful.♻️ Proposed change
- expect(await withKey.json()).not.toMatchObject({ code: "invalid_request" }); + expect(withKey.status).not.toBe(400); + expect(await withKey.json()).not.toMatchObject({ code: "invalid_request" });internal-packages/dashboard-agent-contracts/src/blocks.test.ts (1)
255-259: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the parsed watch spec, not only the intent kind.
The strict schema could strip or default fields inside
intent.specand this assertion would still pass. Add an assertion on the parsed spec so the round-trip covers the payload.♻️ Proposed addition
const strict = viewBlockSchema.parse({ ...body, ...envelope }); expect(strict.type === "actions" && strict.actions[0].intent.kind).toBe("watch"); + expect(strict.type === "actions" && strict.actions[0].intent).toMatchObject(watchAction.intent);apps/webapp/test/dashboardAgentWatchToken.test.ts (1)
12-12: 📐 Maintainability & Code Quality | 🔵 TrivialImport
USER_ACTOR_TOKEN_PREFIXinstead of hardcoding"tr_uat_".
@trigger.dev/rbacre-exportsUSER_ACTOR_TOKEN_PREFIXfrom@trigger.dev/plugins, so use that constant instead of duplicating the prefix in the watch-token tests and keep the cross-token prefix changes in sync.[low_effort和low_reward]
apps/webapp/test/dashboardAgentWatches.test.ts (1)
1473-1481: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNarrow
activeWatchonwatching.
result.okstill includes thewatching: falsebranch, which does not providewatchIdorexpiresAt. The callers read those for check/token operations, so add thewatchingguard before returning.♻️ Proposed narrowing
async function activeWatch(seeded: Seeded) { const result = await create({ seeded }); if (!result.ok) throw new Error(`watch not created: ${result.code}`); + if (!result.watching) throw new Error("expected an active watch"); return result; }Source: Coding guidelines
internal-packages/dashboard-agent/src/watch-actions.test.ts (1)
505-523: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the missing-
userIdfallback.
narrateWatchWakehas a branch atwatch-actions.tslines 540-547 that runs whenclientData.userIdis absent. It logs an error and callspersistMessageswith the whole transcript. The comment on lines 532-535 states that a wholesale write can drop host-appended blocks, so this branch trades correctness for delivery on purpose.No test in this file exercises it. A test that sends
WAKEwithclientDatalackinguserId, then assertscalls.appendMessageis empty andcalls.persistMessageshas one entry, pins that deliberate trade-off in place.Do you want me to generate this test?
internal-packages/dashboard-agent/src/watch-actions.ts (1)
423-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe Haiku wake lane emits no telemetry and no cache metrics.
The Sonnet branch on lines 439-455 ends with
...resolved.toAISDKTelemetry(). The Haiku branch on lines 425-438 does not.conductWatchInvestigation(lines 796-818) and the agent's ownrun(lines 571-582 indashboard-agent.ts) both record telemetry and callrecordPromptCacheUsage.The comment on lines 371-375 says Haiku handles the common wake and Sonnet only the consented-investigation wake. So the lane with the most traffic is the one with no observability.
Add telemetry to the Haiku branch so wake narrations appear alongside every other model call.
♻️ Proposed change
maxOutputTokens: HAIKU_WAKE_MAX_OUTPUT_TOKENS, + ...resolved.toAISDKTelemetry(), })apps/webapp/app/components/dashboard-agent/WakeBanner.tsx (1)
113-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one semantic-icon map. Both files declare an identical
SEMANTIC_ICONrecord that mapsWatchSemanticIconto a Heroicons glyph. A new icon added to the contract must then be added in two places, and the two maps can drift.
apps/webapp/app/components/dashboard-agent/WakeBanner.tsx#L113-L119: export this map (or move it to a small shared module next toagent-badges) so it is the single definition.apps/webapp/app/components/dashboard-agent/WatchChips.tsx#L38-L44: delete the local copy and import the shared map. This file already importswakePresentationfromWakeBanner.apps/webapp/app/components/dashboard-agent/WatchCard.tsx (1)
131-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssociate the
Fieldlabel with its controls.
Fieldrenders the label as a plainspan. The pickers built fromChoiceare bare buttons. A screen reader announces "when it finishes" with no indication that it belongs to "Tell me". The numeric inputs carryaria-label, so only the picker rows are affected.Add a group role and connect it to the label.
♻️ Proposed change
function Field({ label, children }: { label: string; children: React.ReactNode }) { + const labelId = useId(); return ( <div className="flex flex-col gap-1"> - <span className="text-xxs uppercase tracking-wide text-text-faint">{label}</span> - <div className="flex flex-wrap items-center gap-1">{children}</div> + <span id={labelId} className="text-xxs uppercase tracking-wide text-text-faint"> + {label} + </span> + <div role="group" aria-labelledby={labelId} className="flex flex-wrap items-center gap-1"> + {children} + </div> </div> ); }apps/webapp/app/components/dashboard-agent/watch-card.ts (2)
118-121: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
withWindowclamps but does not snap to an offered option.The module comment at Line 5 states that "the window is always one of the offered options".
withWindowonly clamps betweenWATCH_WINDOW_HOURS_OPTIONS[0]andWATCH_MAX_HOURS. A value such as7passes through unchanged and is not an offered option. Today the card only passes values fromWATCH_WINDOW_HOURS_OPTIONS, so the mismatch is not visible.WatchCarddocuments a free-text pre-fill path at Line 145, which could pass an arbitrary number.Snap to the nearest offered option, in the same way
clampCadencedoes.♻️ Proposed change
export function withWindow(draft: WatchDraft, maxHours: number): WatchDraft { - const clamped = Math.min(Math.max(maxHours, WATCH_WINDOW_HOURS_OPTIONS[0]), WATCH_MAX_HOURS); + const clamped = Math.min(Math.max(maxHours, WATCH_WINDOW_HOURS_OPTIONS[0]!), WATCH_MAX_HOURS); + const snapped = + WATCH_WINDOW_HOURS_OPTIONS.find((option) => option >= clamped) ?? + WATCH_WINDOW_HOURS_OPTIONS[WATCH_WINDOW_HOURS_OPTIONS.length - 1]!; - return { ...draft, spec: { ...draft.spec, maxHours: clamped } as WatchSpec }; + return { ...draft, spec: { ...draft.spec, maxHours: snapped } as WatchSpec }; }
157-181: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
watchDraftErrorruns a Zod parse on every card render.
WatchCardcallswatchDraftError(draft)at Line 164 ofapps/webapp/app/components/dashboard-agent/WatchCard.tsx, directly in the render body. Each keystroke in the threshold input reparses the spec throughwatchSpecSchema.Memoize the result in the card, keyed on
draft.Based on learnings, this repository treats Zod as a boundary validation tool for API handlers and storage reads/writes, not as inline render-time validation inside React components, to avoid per-render schema-parse overhead.
♻️ Proposed change in `WatchCard.tsx`
- const localError = watchDraftError(draft); + const localError = useMemo(() => watchDraftError(draft), [draft]);Source: Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f5885caf-6411-4004-a096-446ba10f9f27
⛔ Files ignored due to path filters (2)
apps/webapp/test/__snapshots__/dashboardAgentWatchWording.test.ts.snapis excluded by!**/*.snapinternal-packages/dashboard-agent/src/__snapshots__/prompt-prefix.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (140)
.server-changes/dashboard-agent.mdapps/webapp/app/components/dashboard-agent/DashboardAgent.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentChat.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentDraft.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentHistory.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentSuggestedPrompts.tsxapps/webapp/app/components/dashboard-agent/ReportView.tsxapps/webapp/app/components/dashboard-agent/WakeBanner.tsxapps/webapp/app/components/dashboard-agent/WatchButton.tsxapps/webapp/app/components/dashboard-agent/WatchCard.tsxapps/webapp/app/components/dashboard-agent/WatchChips.tsxapps/webapp/app/components/dashboard-agent/WatchResultBlock.tsxapps/webapp/app/components/dashboard-agent/WatchWakeToast.tsxapps/webapp/app/components/dashboard-agent/chat-layout.test.tsapps/webapp/app/components/dashboard-agent/chat-layout.tsxapps/webapp/app/components/dashboard-agent/dashboardAgentLauncher.tsxapps/webapp/app/components/dashboard-agent/demo/fixtures/page-context.tsapps/webapp/app/components/dashboard-agent/list-row.tsxapps/webapp/app/components/dashboard-agent/message-quota.tsapps/webapp/app/components/dashboard-agent/pending-intents.test.tsapps/webapp/app/components/dashboard-agent/pending-intents.tsapps/webapp/app/components/dashboard-agent/report-sparkline.tsxapps/webapp/app/components/dashboard-agent/suggested-prompts/page-prompts.tsapps/webapp/app/components/dashboard-agent/suggested-prompts/prompt-chips.tsapps/webapp/app/components/dashboard-agent/suggested-prompts/resolver.test.tsapps/webapp/app/components/dashboard-agent/suggested-prompts/resolver.tsapps/webapp/app/components/dashboard-agent/suggested-prompts/signal-prompts.tsapps/webapp/app/components/dashboard-agent/tool-labels.tsapps/webapp/app/components/dashboard-agent/turn-error.test.tsapps/webapp/app/components/dashboard-agent/turn-error.tsapps/webapp/app/components/dashboard-agent/view-actions.test.tsapps/webapp/app/components/dashboard-agent/view-catalog.tsxapps/webapp/app/components/dashboard-agent/wake-banner.test.tsapps/webapp/app/components/dashboard-agent/wake-poll.test.tsapps/webapp/app/components/dashboard-agent/wake-poll.tsapps/webapp/app/components/dashboard-agent/watch-activity.test.tsapps/webapp/app/components/dashboard-agent/watch-activity.tsapps/webapp/app/components/dashboard-agent/watch-card.test.tsapps/webapp/app/components/dashboard-agent/watch-card.tsapps/webapp/app/components/dashboard-agent/watch-chips.test.tsapps/webapp/app/components/dashboard-agent/watch-chips.tsapps/webapp/app/components/dashboard-agent/watch-recommendations.tsapps/webapp/app/components/queues/queue-thresholds.tsapps/webapp/app/presenters/v3/ApiAlertChannelPresenter.server.tsapps/webapp/app/presenters/v3/dashboardAgent/block-text.tsapps/webapp/app/presenters/v3/dashboardAgent/index.tsapps/webapp/app/presenters/v3/dashboardAgent/watch-wording.tsapps/webapp/app/presenters/v3/reports/ReportPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.alerts.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.alerts/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues_.$queueParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsxapps/webapp/app/routes/api.v1.dashboard-agent.alerts.$channelId.tsapps/webapp/app/routes/api.v1.dashboard-agent.alerts.tsapps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.check.tsapps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.fired.tsapps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.investigate.tsapps/webapp/app/routes/api.v1.dashboard-agent.watches.batch-check.tsapps/webapp/app/routes/api.v1.dashboard-agent.watches.tsapps/webapp/app/routes/resources.dashboard-agent.alerts.$channelId.unsubscribe.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsxapps/webapp/app/services/dashboardAgentAlertContext.server.tsapps/webapp/app/services/dashboardAgentAlertUnsubscribeToken.server.tsapps/webapp/app/services/dashboardAgentWatchAlerts.server.tsapps/webapp/app/services/dashboardAgentWatchBatch.server.tsapps/webapp/app/services/dashboardAgentWatchCheckBase.tsapps/webapp/app/services/dashboardAgentWatchChecks.server.tsapps/webapp/app/services/dashboardAgentWatchChecks.tsapps/webapp/app/services/dashboardAgentWatchErrorChecks.tsapps/webapp/app/services/dashboardAgentWatchHealthChecks.tsapps/webapp/app/services/dashboardAgentWatchInvestigate.server.tsapps/webapp/app/services/dashboardAgentWatchQueueChecks.tsapps/webapp/app/services/dashboardAgentWatchRunChecks.tsapps/webapp/app/services/dashboardAgentWatchSweep.server.tsapps/webapp/app/services/dashboardAgentWatchToken.server.tsapps/webapp/app/services/dashboardAgentWatches.server.tsapps/webapp/app/v3/alertsWorker.server.tsapps/webapp/app/v3/commonWorker.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.tsapps/webapp/package.jsonapps/webapp/seed-watch-scenarios.mtsapps/webapp/test/dashboardAgentBodyCap.test.tsapps/webapp/test/dashboardAgentTranscriptStore.test.tsapps/webapp/test/dashboardAgentWakeActivity.test.tsapps/webapp/test/dashboardAgentWatchAlertFanout.test.tsapps/webapp/test/dashboardAgentWatchAlertOwnerScope.test.tsapps/webapp/test/dashboardAgentWatchBatchFairness.test.tsapps/webapp/test/dashboardAgentWatchBatchRecording.test.tsapps/webapp/test/dashboardAgentWatchCardAtomicity.test.tsapps/webapp/test/dashboardAgentWatchCardRequestId.test.tsapps/webapp/test/dashboardAgentWatchChecks.test.tsapps/webapp/test/dashboardAgentWatchCreationReads.test.tsapps/webapp/test/dashboardAgentWatchInvestigate.test.tsapps/webapp/test/dashboardAgentWatchSweepBoundary.test.tsapps/webapp/test/dashboardAgentWatchTenancy.test.tsapps/webapp/test/dashboardAgentWatchToken.test.tsapps/webapp/test/dashboardAgentWatchWording.test.tsapps/webapp/test/dashboardAgentWatches.test.tsapps/webapp/test/reportHealth.test.tsinternal-packages/dashboard-agent-contracts/src/blocks.test.tsinternal-packages/dashboard-agent-contracts/src/blocks.tsinternal-packages/dashboard-agent-contracts/src/contracts.test.tsinternal-packages/dashboard-agent-contracts/src/index.tsinternal-packages/dashboard-agent-contracts/src/intent.tsinternal-packages/dashboard-agent-contracts/src/watch-wording.tsinternal-packages/dashboard-agent/GUIDEBOOK.mdinternal-packages/dashboard-agent/README.mdinternal-packages/dashboard-agent/src/agent-runtime.tsinternal-packages/dashboard-agent/src/compaction.test.tsinternal-packages/dashboard-agent/src/compaction.tsinternal-packages/dashboard-agent/src/dashboard-agent.eval.tsinternal-packages/dashboard-agent/src/dashboard-agent.test.tsinternal-packages/dashboard-agent/src/dashboard-agent.tsinternal-packages/dashboard-agent/src/eval-policy.tsinternal-packages/dashboard-agent/src/index.tsinternal-packages/dashboard-agent/src/step-cache.tsinternal-packages/dashboard-agent/src/tool-alerts.tsinternal-packages/dashboard-agent/src/tool-investigations.tsinternal-packages/dashboard-agent/src/tool-schemas.tsinternal-packages/dashboard-agent/src/tools.tsinternal-packages/dashboard-agent/src/watch-actions.test.tsinternal-packages/dashboard-agent/src/watch-actions.tsinternal-packages/dashboard-agent/src/watch-batch.tsinternal-packages/dashboard-agent/src/watch-delivery.tsinternal-packages/dashboard-agent/src/watch-lifecycle.tsinternal-packages/dashboard-agent/src/watch-narration.test.tsinternal-packages/dashboard-agent/src/watch-narration.tsinternal-packages/dashboard-agent/src/watch-task-adapters.tsinternal-packages/dashboard-agent/src/watch-tick.test.tsinternal-packages/dashboard-agent/src/watch-tick.tsinternal-packages/dashboard-agent/src/watch-tools.tsinternal-packages/database/prisma/migrations/20260729120000_add_dashboard_agent_watch_alert_type/migration.sqlinternal-packages/database/prisma/schema.prismainternal-packages/emails/emails/alert-dashboard-agent-watch.tsxinternal-packages/emails/src/index.tsx
| const watchProposedRef = useRef<Set<string> | null>(null); | ||
| if (watchProposedRef.current === null) { | ||
| watchProposedRef.current = new Set(); | ||
| pendingWatchIntents(initialMessages, watchProposedRef.current); | ||
| } | ||
| useEffect(() => { | ||
| const pending = pendingWatchIntents(messages, watchProposedRef.current!); | ||
| const proposed = pending.at(-1); | ||
| if (proposed) onWatchIntent?.(proposed.spec); | ||
| }, [messages, onWatchIntent]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the schedule_watch tool registration and all pending-intent consumers.
ast-grep outline internal-packages/dashboard-agent/src/tools.ts --items all --match 'schedule_watch'
rg -n -C 6 --glob '*.ts' --glob '*.tsx' \
'tool-schedule_watch|pendingWatchIntents|schedule_watch' \
internal-packages/dashboard-agent apps/webappRepository: triggerdotdev/trigger.dev
Length of output: 33921
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pending-intents.ts =="
sed -n '1,120p' apps/webapp/app/components/dashboard-agent/pending-intents.ts
echo
echo "== relevant DashboardAgentChat section =="
sed -n '250,320p' apps/webapp/app/components/dashboard-agent/DashboardAgentChat.tsx
echo
echo "== watch tool implementation =="
sed -n '1,120p' internal-packages/dashboard-agent/src/watch-tools.ts
echo
echo "== tests for multi proposal behavior =="
sed -n '50,130p' apps/webapp/app/components/dashboard-agent/pending-intents.test.tsRepository: triggerdotdev/trigger.dev
Length of output: 7744
🌐 Web query:
Vercel AI SDK scheduled tools tool_call_state output-available multiple tool calls completion transcript
💡 Result:
In the Vercel AI SDK, tool calling involves a defined lifecycle that tracks the state of a tool call as it progresses from request to execution and completion [1][2]. When handling streaming tool calls in the UI (specifically with the AI SDK UI's useChat or related hooks), these states are often used to conditionally render components or updates [3]. The specific states frequently observed in tool_call_state contexts include [3][4]: - input-streaming: The LLM is actively generating the arguments for the tool call [3][4]. - input-available: The complete arguments for the tool call have been generated and validated, but execution may not have finished [1][3][4]. - output-available: The tool execution has completed, and the resulting output is ready for display or further processing [3][4]. - output-error: The tool execution failed, providing an error message [3][4]. For multiple tool calls, the SDK manages these via multi-step generations [2][5]. In a streaming context (e.g., using streamText), you can use callbacks like onToolExecutionStart, onToolExecutionEnd, or onStepFinish to monitor the lifecycle of these calls [1][2]. Each step in a multi-step sequence allows the model to receive the tool results and continue its generation [2][5]. To manage the completion transcript and aggregate these tool results, the SDK provides the steps property in the result object, or the onFinish callback, which contains the full history of text, tool calls, and results across all steps [2]. When building custom UIs, developers typically track these by toolCallId to map fragmented streaming chunks (input/output) to their respective UI components [3][6].
Citations:
- 1: https://ai-sdk.dev/docs/ai-sdk-core/tools-and-tool-calling
- 2: https://ai-sdk.dev/v5/docs/ai-sdk-core/tools-and-tool-calling
- 3: https://ai-play.vercel.app/docs/ai-sdk-ui/chatbot-tool-usage
- 4: https://cdn.jsdelivr.net/npm/@incremark/chat-core@1.0.2/dist/message-CAInMqAT.d.ts
- 5: http://sdk.vercel.ai/docs/foundations/tools
- 6:
ToolMessageargs field langchain-ai/deepagents#3346
🌐 Web query:
site:ai-sdk.dev AI SDK multiple tool calls single stream output-available transcript order
💡 Result:
In the Vercel AI SDK, when a model performs multiple tool calls, the order and stream handling are managed through the SDK's core architecture as follows: Parallel Tool Calls: Some models support calling multiple tools in parallel within a single generation step. When this occurs, the model generates multiple tool calls simultaneously. These calls are exposed together in the toolCalls property of the result object or as distinct tool-call parts within the stream [1][2]. Stream Output Order: The stream consists of a sequence of TextStreamPart chunks. When multiple tool calls are triggered, they are typically streamed as discrete tool-call parts. While specific implementations may vary based on the model provider, the SDK processes these parts as they are generated. For UI applications, you can consume these streams using utilities like readUIMessageStream or the useChat hook, which parse the stream and provide structured access to text, tool calls, and tool results [3][4]. Multi-Step Execution: If you require complex workflows where tools must be executed in a specific sequence (e.g., the output of one tool informs the next), you should use multi-step calls rather than parallel calls [5][6]. By enabling stopWhen (e.g., isStepCount(5)), the AI SDK allows the model to pause after generating tool calls, execute them, and automatically continue the generation with the tool results in the message history [5][7][6]. This maintains the logical order of operations across multiple request-response cycles [8][7]. Tool Definition Order: If you need to ensure a stable request shape (for instance, to optimize provider-side caching), you can use the toolOrder option in streamText. Tools listed in toolOrder are sent to the provider first, in the specified order, followed by any remaining tools [5]. This option controls the order in which tools are presented to the model, not the order in which the model chooses to call them [5].
Citations:
- 1: https://ai-sdk.dev/v7/cookbook/node/call-tools-in-parallel
- 2: https://ai-sdk.dev/cookbook/node/call-tools-in-parallel
- 3: https://ai-sdk.dev/docs/ai-sdk-ui/chatbot-tool-usage
- 4: https://ai-sdk.dev/v5/docs/ai-sdk-ui/reading-ui-message-streams
- 5: https://ai-sdk.dev/docs/ai-sdk-core/tools-and-tool-calling
- 6: https://ai-sdk.dev/resources/recipes/next/call-tools-multiple-steps
- 7: https://ai-sdk.dev/v5/docs/ai-sdk-core/tools-and-tool-calling
- 8: https://ai-sdk.dev/resources/recipes/next/stream-text-multistep
Do not discard earlier watch proposals from one transcript update.
pendingWatchIntents adds each completed tool-schedule_watch call to watchProposedRef.current, but the effect only sends pending.at(-1) when it updates. If one transcript update contains multiple completed watch tool calls, only the final one reaches onWatchIntent; queue all pending proposals, or mark each proposal seen after displaying it.
| function read(): string[] { | ||
| try { | ||
| const raw = window.localStorage.getItem(STORAGE_KEY); | ||
| return raw ? (JSON.parse(raw) as string[]) : []; | ||
| } catch { | ||
| // Storage unavailable; treated as "nothing known yet". | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| export function hasWatchActivity(organizationId: string): boolean { | ||
| if (typeof window === "undefined") return false; | ||
| return read().includes(organizationId); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard against a non-array value in storage.
read() catches parse failures, but it does not check the parsed shape. If STORAGE_KEY holds valid JSON that is not an array, read() returns that value and the includes call on line 27 throws a TypeError. The throw escapes hasWatchActivity, so the sync callback in DashboardAgent.tsx (line 153) and the subscribeWatchActivity listener both fail. The same value also breaks the spread on line 36.
Return an empty array unless the parsed value is an array.
🛡️ Proposed fix
function read(): string[] {
try {
const raw = window.localStorage.getItem(STORAGE_KEY);
- return raw ? (JSON.parse(raw) as string[]) : [];
+ if (!raw) return [];
+ const parsed = JSON.parse(raw) as unknown;
+ return Array.isArray(parsed) ? parsed.filter((id): id is string => typeof id === "string") : [];
} catch {
// Storage unavailable; treated as "nothing known yet".
return [];
}
}📝 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.
| function read(): string[] { | |
| try { | |
| const raw = window.localStorage.getItem(STORAGE_KEY); | |
| return raw ? (JSON.parse(raw) as string[]) : []; | |
| } catch { | |
| // Storage unavailable; treated as "nothing known yet". | |
| return []; | |
| } | |
| } | |
| export function hasWatchActivity(organizationId: string): boolean { | |
| if (typeof window === "undefined") return false; | |
| return read().includes(organizationId); | |
| } | |
| function read(): string[] { | |
| try { | |
| const raw = window.localStorage.getItem(STORAGE_KEY); | |
| if (!raw) return []; | |
| const parsed = JSON.parse(raw) as unknown; | |
| return Array.isArray(parsed) | |
| ? parsed.filter((id): id is string => typeof id === "string") | |
| : []; | |
| } catch { | |
| // Storage unavailable; treated as "nothing known yet". | |
| return []; | |
| } | |
| } | |
| export function hasWatchActivity(organizationId: string): boolean { | |
| if (typeof window === "undefined") return false; | |
| return read().includes(organizationId); | |
| } |
| <SimpleTooltip | ||
| side="bottom" | ||
| content={watchChipTooltip(watch)} | ||
| button={<span className="max-w-[12rem] truncate">{label}</span>} | ||
| /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make the chip tooltip reachable by keyboard.
The tooltip trigger is a plain <span>, so it is not focusable. Keyboard and screen-reader users cannot read watchChipTooltip. The cancel control below already passes tabbable. Add tabbable here as well.
♿ Proposed fix
<SimpleTooltip
+ tabbable
side="bottom"
content={watchChipTooltip(watch)}
button={<span className="max-w-[12rem] truncate">{label}</span>}
/>📝 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.
| <SimpleTooltip | |
| side="bottom" | |
| content={watchChipTooltip(watch)} | |
| button={<span className="max-w-[12rem] truncate">{label}</span>} | |
| /> | |
| <SimpleTooltip | |
| tabbable | |
| side="bottom" | |
| content={watchChipTooltip(watch)} | |
| button={<span className="max-w-[12rem] truncate">{label}</span>} | |
| /> |
| // Stable per (email, project), so asking twice re-enables one channel. | ||
| deduplicationKey: `dashboard-agent-watch:${email}`, | ||
| channel: { type: "EMAIL", email }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Build the deduplication key with the exported helper.
dashboardAgentWatchAlerts.server.ts exports watchAlertDeduplicationKey, and this file already imports from that module. Rebuilding the literal here splits the key format across two modules. If the helper changes, this route stops matching the existing channel and creates a duplicate instead of re-enabling one. The same literal is also duplicated in apps/webapp/test/dashboardAgentWatches.test.ts at line 1713.
♻️ Proposed fix
import {
canUseDashboardAgentEmailAlerts,
DASHBOARD_AGENT_WATCH_ALERT_TYPE,
+ watchAlertDeduplicationKey,
} from "~/services/dashboardAgentWatchAlerts.server"; // Stable per (email, project), so asking twice re-enables one channel.
- deduplicationKey: `dashboard-agent-watch:${email}`,
+ deduplicationKey: watchAlertDeduplicationKey(email),📝 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.
| // Stable per (email, project), so asking twice re-enables one channel. | |
| deduplicationKey: `dashboard-agent-watch:${email}`, | |
| channel: { type: "EMAIL", email }, | |
| }); | |
| // Stable per (email, project), so asking twice re-enables one channel. | |
| deduplicationKey: watchAlertDeduplicationKey(email), | |
| channel: { type: "EMAIL", email }, | |
| }); |
Source: Coding guidelines
| // Recorded even on the final evaluation. Guarded on `active`, so a concurrent | ||
| // fire/expire wins and this no-ops. | ||
| await recordWatchCheck(dashboardAgentDb, { | ||
| id: watchId, | ||
| lastResult: { | ||
| result: outcome.result, | ||
| facts: outcome.facts, | ||
| observed: outcome.observed, | ||
| final: body.final === true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This route records unavailable results; the batch path deliberately does not.
recordWatchCheck is called for every outcome here, including unavailable. The batch evaluator at apps/webapp/app/services/dashboardAgentWatchBatch.server.ts Lines 212-226 skips the write for unavailable and calls recordWatchAttempt instead, with the stated reason that writing it would move lastCheckedAt and overwrite the facts a streak lives in.
Both paths write the same lastResult column for the same rows. A queue_stalled watch that loses one read through this route loses its notDecreasingStreak facts, while the same failure through the batch path preserves them. The comment at Line 146 states the intent is to freeze a streak, but the write at Line 161 defeats it.
Apply the same split here.
🐛 Proposed fix to match the batch path
- // Recorded even on the final evaluation. Guarded on `active`, so a concurrent
- // fire/expire wins and this no-ops.
- await recordWatchCheck(dashboardAgentDb, {
- id: watchId,
- lastResult: {
- result: outcome.result,
- facts: outcome.facts,
- observed: outcome.observed,
- final: body.final === true,
- },
- });
+ // Recorded even on the final evaluation. Guarded on `active`, so a concurrent
+ // fire/expire wins and this no-ops. `unavailable` means nothing was read, so it is
+ // recorded as an attempt instead: writing it would overwrite the facts a streak lives in.
+ if (outcome.result !== "unavailable") {
+ await recordWatchCheck(dashboardAgentDb, {
+ id: watchId,
+ lastResult: {
+ result: outcome.result,
+ facts: outcome.facts,
+ observed: outcome.observed,
+ final: body.final === true,
+ },
+ });
+ } else {
+ await recordWatchAttempt(dashboardAgentDb, { id: watchId });
+ }Update the import:
-import { cancelWatch, getWatch, recordWatchCheck } from "`@internal/dashboard-agent-db`";
+import {
+ cancelWatch,
+ getWatch,
+ recordWatchAttempt,
+ recordWatchCheck,
+} from "`@internal/dashboard-agent-db`";📝 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.
| // Recorded even on the final evaluation. Guarded on `active`, so a concurrent | |
| // fire/expire wins and this no-ops. | |
| await recordWatchCheck(dashboardAgentDb, { | |
| id: watchId, | |
| lastResult: { | |
| result: outcome.result, | |
| facts: outcome.facts, | |
| observed: outcome.observed, | |
| final: body.final === true, | |
| }, | |
| }); | |
| // Recorded even on the final evaluation. Guarded on `active`, so a concurrent | |
| // fire/expire wins and this no-ops. `unavailable` means nothing was read, so it is | |
| // recorded as an attempt instead: writing it would overwrite the facts a streak lives in. | |
| if (outcome.result !== "unavailable") { | |
| await recordWatchCheck(dashboardAgentDb, { | |
| id: watchId, | |
| lastResult: { | |
| result: outcome.result, | |
| facts: outcome.facts, | |
| observed: outcome.observed, | |
| final: body.final === true, | |
| }, | |
| }); | |
| } else { | |
| await recordWatchAttempt(dashboardAgentDb, { id: watchId }); | |
| } |
| export const renderViewSchema = tool({ | ||
| description: | ||
| "Render a structured view in the dashboard panel: a stack of catalog blocks, instead of plain prose. The catalog has four blocks: `diagnosis` (the 'why did this run fail?' failure card, after gathering evidence with the read/source tools), `chart` (a line/bar chart of run_query results), `actions` (a row of 1-3 buttons offering next steps — an `ask` intent sends the labelled question as the user's next message, a `navigate` intent takes the user to a page), and `investigation` (a live card for a hypothesis-driven investigation: report the state and the tool assigns and keeps its identity, so re-rendering it updates the same card). The result carries the `investigationId` it assigned — pass that back as `investigationId` when you render the same investigation again, including on a later turn. An investigation is rendered at least TWICE: once as `in_progress` when you open it, then again with the same `investigationId` carrying the final outcome (`concluded` or `inconclusive`), as the last tool call of the turn. A card left at `in_progress` is an unfinished answer whatever your prose says: the user is left watching a spinner. Keep any accompanying message to a one-line lead-in.", | ||
| "Render a structured view in the dashboard panel: a stack of catalog blocks, instead of plain prose. The catalog has four blocks: `diagnosis` (the 'why did this run fail?' failure card, after gathering evidence with the read/source tools), `chart` (a line/bar chart of run_query results), `actions` (a row of 1-3 buttons offering next steps — a `watch` intent opens the watch configuration card pre-filled with the spec you composed, an `ask` intent sends the labelled question as the user's next message), and `investigation` (a live card for a hypothesis-driven investigation: report the state and the tool assigns and keeps its identity, so re-rendering it updates the same card). The result carries the `investigationId` it assigned — pass that back as `investigationId` when you render the same investigation again, including on a later turn. An investigation is rendered at least TWICE: once as `in_progress` when you open it, then again with the same `investigationId` carrying the final outcome (`concluded` or `inconclusive`), as the last tool call of the turn. A card left at `in_progress` is an unfinished answer whatever your prose says: the user is left watching a spinner. Keep any accompanying message to a one-line lead-in.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the stale navigate action instruction.
This description permits only watch and ask actions. The same system prompt later tells ranking-chart responses to add a navigate action. That action can make render_view reject an otherwise valid chart response.
Update the ranking-chart guidance to use only supported action intents, or restore navigate to the action contract.
| async function resolveInvestigationId(args: { | ||
| action: WatchInvestigateAction; | ||
| chatId: string; | ||
| projectRef: string; | ||
| environmentRef: string; | ||
| }): Promise<string | undefined> { | ||
| const { action, chatId, projectRef, environmentRef } = args; | ||
| if (action.investigationId) return action.investigationId; | ||
|
|
||
| const store = getStore(); | ||
| const open = await store.findOpenInvestigation({ | ||
| chatId, | ||
| createdAfter: new Date(Date.now() - CONSENTED_INVESTIGATION_LOOKBACK_MS), | ||
| }); | ||
| if (open) return open.id; | ||
|
|
||
| const seeded = await store.upsertInvestigationRevision({ | ||
| chatId, | ||
| projectRef, | ||
| environmentRef, | ||
| state: { | ||
| outcome: "in_progress", | ||
| severity: "warn", | ||
| confidence: "low", | ||
| title: `Investigating ${wakeSubject(action)}`, | ||
| headline: `The watch on ${wakeSubject(action)} resolved to something that needs attention. Looking into why.`, | ||
| hypotheses: [], | ||
| evidence: [], | ||
| startedAt: new Date().toISOString(), | ||
| }, | ||
| }); | ||
| return seeded.ok ? seeded.id : undefined; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the findOpenInvestigation signature and every filter it supports.
rg -n -C15 'findOpenInvestigation' --type=ts
# Check whether the investigation state or row can carry a watchId.
rg -n -C4 'watchId' internal-packages/dashboard-agent/src/agent-runtime.tsRepository: triggerdotdev/trigger.dev
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching watch-actions/agent-runtime:"
fd -a 'watch-actions\.ts|agent-runtime\.ts' . || true
file="$(fd 'watch-actions\.ts' . | head -n1 || true)"
if [ -n "$file" ]; then
echo "--- file: $file"
wc -l "$file"
echo "--- locate resolveInvestigationId/findOpenInvestigation/openConsentedInvestigation"
rg -n -C8 'resolveInvestigationId|findOpenInvestigation|openConsentedInvestigation|CONSENTED_INVESTIGATION_LOOKBACK_MS' "$file" || true
fi
runtime="$(fd 'agent-runtime\.ts' . | head -n1 || true)"
if [ -n "$runtime" ]; then
echo "--- runtime: $runtime"
wc -l "$runtime"
echo "--- locate findOpenInvestigation/watchId"
rg -n -C12 'findOpenInvestigation|watchId' "$runtime" || true
fi
echo "--- all findOpenInvestigation definitions/usages in ts"
rg -n -C12 'findOpenInvestigation|watchId' -g '*.ts' . || trueRepository: triggerdotdev/trigger.dev
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd '^watch-actions\.ts$' internal-packages/dashboard-agent/src -t f | head -n1)"
runtime="$(fd '^agent-runtime\.ts$' internal-packages/dashboard-agent/src -t f | head -n1)"
store="$(fd -i 'agent.*store|store.*|storage.*|database.*|runtime.*\.ts$|^database\.ts$' internal-packages/dashboard-agent/src -t f | head -n20 || true)"
echo "--- watch-actions target file"
echo "$file"
wc -l "$file"
echo "--- lines 520-610"
sed -n '520,610p' "$file" | nl -ba -v520
echo "--- lines 300-390"
sed -n '300,390p' "$file" | nl -ba -v300
echo "--- lines 830-880"
sed -n '830,880p' "$file" | nl -ba -v830
echo "--- candidate store files"
echo "$store"
echo "--- findOpenInvestigation definitions/usages in internal-packages/dashboard-agent/src"
rg -n -C12 'findOpenInvestigation|openConsentedInvestigation|upsertInvestigationRevision|CONSENTED_INVESTIGATION_LOOKBACK_MS|WatchInvestigateAction' internal-packages/dashboard-agent/src || true
echo "--- watchId in internal-packages/dashboard-agent/src"
rg -n -C3 'watchId' internal-packages/dashboard-agent/src || true
echo "--- investigation interfaces/types in internal-packages/dashboard-agent"
rg -n -C8 'Investigation|investigation|watchId' internal-packages/dashboard-agent -g '*.ts' --max-count 120 || trueRepository: triggerdotdev/trigger.dev
Length of output: 366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="internal-packages/dashboard-agent/src/watch-actions.ts"
runtime="$(fd '^agent-runtime\.ts$' internal-packages/dashboard-agent/src -t f | head -n1 || true)"
echo "--- lines 520-610"
sed -n '520,610p' "$file"
echo
echo "--- lines 300-390"
sed -n '300,390p' "$file"
echo
echo "--- lines 830-880"
sed -n '830,880p' "$file"
echo "--- findOpenInvestigation definitions/usages in internal-packages/dashboard-agent/src"
rg -n -C12 'findOpenInvestigation|openConsentedInvestigation|upsertInvestigationRevision|CONSENTED_INVESTIGATION_LOOKBACK_MS|WatchInvestigateAction' internal-packages/dashboard-agent/src || true
echo "--- watchId in internal-packages/dashboard-agent/src"
rg -n -C3 'watchId' internal-packages/dashboard-agent/src || true
echo "--- investigation interfaces/types in internal-packages/dashboard-agent"
rg -n -C8 'Investigation|investigation|watchId' internal-packages/dashboard-agent -g '*.ts' || trueRepository: triggerdotdev/trigger.dev
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("internal-packages/dashboard-agent/src/watch-actions.ts")
text = path.read_text()
patterns = {
"resolveInvestigationId": r"async function resolveInvestigationId\((.*?)\n\): Promise<.*?> \{([\s\S]*?)\n\}",
"openConsentedInvestigation": r"async function openConsentedInvestigation\((.*?)\n\): Promise<.*?> \{([\s\S]*?)\n\}",
"findOpenInvestigation_call": r"const open = await store\.findOpenInvestigation\(\{([^\}]+)\}\);",
"seed_state": r"state: \{([\s\S]*?)\n\s+\}\s*,",
}
for name, pat in patterns.items():
match = re.search(pat, text, re.DOTALL)
print(f"--- {name}: line {text[:match.start()].count(chr(10))+1 if match else 'NONE'}")
if match:
print(" ".join(match.group(0).split()))
else:
print("NO MATCH")
runtime_file = Path("internal-packages/dashboard-agent/src/agent-runtime.ts")
if runtime_file.exists():
print("--- agent-runtime watcher declarations/findOpenInvestigation")
for line in runtime_file.read_text(errors="ignore").splitlines():
if any(tok in line for tok in ["findOpenInvestigation", "watchId", "type DashboardAgentStore", "interface DashboardAgentStore"]):
print(line)
else:
print("--- agent-runtime.ts: missing")
PYRepository: triggerdotdev/trigger.dev
Length of output: 1140
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- findOpenInvestigationForChat occurrences"
rg -n -C12 'findOpenInvestigationForChat|findOpenInvestigationForChat\s*\(' -g '*.ts' . || true
echo "--- surrounding store/watch investigation contract files"
fd -t f -i '.*investigation.*|.*store.*|.*watch.*' internal-packages internal-packages/dashboard-agent apps/webapp | sed -n '1,200p'Repository: triggerdotdev/trigger.dev
Length of output: 21194
Correlate the seed with the watch before reusing the card.
findOpenInvestigationForChat filters on chatId, outcome = 'in_progress', and createdAfter only. The seed omits watchId, so a normal card created by the same user in the same chat within 30 minutes can be selected by this watch. Store the watch id on the seeded investigation state and require it in the tier-two lookup.
| if (!result.ok) { | ||
| // A chat deleted mid-investigation is a race, not a fault: nothing settled, and | ||
| // there is no transcript left to close the card in. | ||
| const message = "dashboard-agent watch investigation couldn't close its card"; | ||
| const details = { chatId, investigationId, error: result.error }; | ||
| if (result.error === "chat_missing") logger.warn(message, details); | ||
| else logger.error(message, details); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A non-ok settle reports success and the card never closes.
The doc comment on lines 605-606 states that nothing is caught and that the failure has to reach the action so the retry is a real retry. That holds for thrown errors. It does not hold here.
When settleInvestigationCard returns { ok: false }, this block logs and returns. closeCardInTranscript resolves, the finally at line 860 completes, and the action reports success. Nothing retries.
chat_missing is correctly a race and a warn is right for it. Every other result.error is logged at error level, which says the code treats it as a fault — yet it is still swallowed. The card stays in_progress and the panel shows a spinner that nothing stops, which is the exact failure the test on lines 889-922 was written to prevent.
Throw on the error cases and return only on chat_missing.
🐛 Proposed fix
if (!result.ok) {
// A chat deleted mid-investigation is a race, not a fault: nothing settled, and
// there is no transcript left to close the card in.
const message = "dashboard-agent watch investigation couldn't close its card";
const details = { chatId, investigationId, error: result.error };
- if (result.error === "chat_missing") logger.warn(message, details);
- else logger.error(message, details);
- return;
+ if (result.error === "chat_missing") {
+ logger.warn(message, details);
+ return;
+ }
+ // Anything else left the card open, so the action must fail and retry.
+ logger.error(message, details);
+ throw new Error(`${message}: ${result.error}`);
}📝 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.
| if (!result.ok) { | |
| // A chat deleted mid-investigation is a race, not a fault: nothing settled, and | |
| // there is no transcript left to close the card in. | |
| const message = "dashboard-agent watch investigation couldn't close its card"; | |
| const details = { chatId, investigationId, error: result.error }; | |
| if (result.error === "chat_missing") logger.warn(message, details); | |
| else logger.error(message, details); | |
| return; | |
| } | |
| if (!result.ok) { | |
| // A chat deleted mid-investigation is a race, not a fault: nothing settled, and | |
| // there is no transcript left to close the card in. | |
| const message = "dashboard-agent watch investigation couldn't close its card"; | |
| const details = { chatId, investigationId, error: result.error }; | |
| if (result.error === "chat_missing") { | |
| logger.warn(message, details); | |
| return; | |
| } | |
| // Anything else left the card open, so the action must fail and retry. | |
| logger.error(message, details); | |
| throw new Error(`${message}: ${result.error}`); | |
| } |
| await deps.store.recordWatchCheck({ | ||
| id: claimed.id, | ||
| lastResult: { checkFailed: true, detail: check.detail, previous: claimed.lastResult }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Stop nesting previous on every consecutive failed check.
Each unavailable check writes previous: claimed.lastResult. When the next check also fails, the new lastResult wraps the previous wrapper. Consecutive failures therefore nest without a bound, and the stored JSON grows quadratically with the number of failures. A watch runs up to 24 hours, so at a one-minute cadence the chain can reach hundreds of levels.
The blob also propagates outward. expiredFacts copies watch.lastResult into lastObservation for an unverified expiry, enqueueWatchFiredAlert copies it into the alert payload facts, and #sendWebhook serializes those facts into the webhook body.
Keep only the last real observation instead of the whole chain.
🐛 Proposed fix
+ const previous = (claimed.lastResult as Record<string, unknown> | null) ?? null;
await deps.store.recordWatchCheck({
id: claimed.id,
- lastResult: { checkFailed: true, detail: check.detail, previous: claimed.lastResult },
+ lastResult: {
+ checkFailed: true,
+ detail: check.detail,
+ // Carry the last real observation forward, never the previous failure
+ // wrapper, so repeated failures cannot nest without a bound.
+ previous: previous?.checkFailed ? previous.previous : previous,
+ },
});📝 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.
| await deps.store.recordWatchCheck({ | |
| id: claimed.id, | |
| lastResult: { checkFailed: true, detail: check.detail, previous: claimed.lastResult }, | |
| }); | |
| const previous = (claimed.lastResult as Record<string, unknown> | null) ?? null; | |
| await deps.store.recordWatchCheck({ | |
| id: claimed.id, | |
| lastResult: { | |
| checkFailed: true, | |
| detail: check.detail, | |
| // Carry the last real observation forward, never the previous failure | |
| // wrapper, so repeated failures cannot nest without a bound. | |
| previous: previous?.checkFailed ? previous.previous : previous, | |
| }, | |
| }); |
| export default function Email(props: AlertDashboardAgentWatchEmailProps) { | ||
| const { | ||
| identity, | ||
| headline, | ||
| tone, | ||
| note, | ||
| noteLine, | ||
| firedAt, | ||
| facts, | ||
| dashboardLink, | ||
| unsubscribeLink, | ||
| organization, | ||
| project, | ||
| environment, | ||
| } = { ...previewDefaults, ...props }; | ||
|
|
||
| const details = [identity, ...facts.slice(0, 3).map((fact) => `${fact.label}: ${fact.value}`)]; | ||
| const accentColor = TONE_COLOR[tone ?? "neutral"] ?? TONE_COLOR.neutral; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the previewDefaults merge. It injects preview copy into real emails.
Line 107 merges previewDefaults under props. Zod omits absent optional keys rather than setting them to undefined, so any optional field missing from a real payload takes the preview value instead.
The effects on a delivered email:
unsubscribeLinkabsent → the email renders the "Turn off these alerts" link at Lines 157-163 pointing athttps://cloud.trigger.dev/unsubscribe. The user gets a consent control that does not belong to their subscription.noteLineabsent → the email statesYou asked to be told when: tell me when the nightly invoice run finishes, which is another example's text.headlineandtoneabsent → the preview headline and the success accent are used.
The component already handles every optional field explicitly: headline ?? fallbackHeadline(identity) at Lines 115 and 138, noteLine ?? ... at Line 142, and tone ?? "neutral" at Line 110. The merge makes all of those fallbacks unreachable. Destructure props directly and keep previewDefaults for the previewer only.
🐛 Proposed fix to stop preview values reaching real emails
-export default function Email(props: AlertDashboardAgentWatchEmailProps) {
+export default function Email(props: AlertDashboardAgentWatchEmailProps) {
const {
identity,
headline,
tone,
note,
noteLine,
firedAt,
facts,
dashboardLink,
unsubscribeLink,
organization,
project,
environment,
- } = { ...previewDefaults, ...props };
+ } = props;Keep previewDefaults exported for the React Email previewer instead:
export const PreviewProps = previewDefaults;📝 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.
| export default function Email(props: AlertDashboardAgentWatchEmailProps) { | |
| const { | |
| identity, | |
| headline, | |
| tone, | |
| note, | |
| noteLine, | |
| firedAt, | |
| facts, | |
| dashboardLink, | |
| unsubscribeLink, | |
| organization, | |
| project, | |
| environment, | |
| } = { ...previewDefaults, ...props }; | |
| const details = [identity, ...facts.slice(0, 3).map((fact) => `${fact.label}: ${fact.value}`)]; | |
| const accentColor = TONE_COLOR[tone ?? "neutral"] ?? TONE_COLOR.neutral; | |
| export default function Email(props: AlertDashboardAgentWatchEmailProps) { | |
| const { | |
| identity, | |
| headline, | |
| tone, | |
| note, | |
| noteLine, | |
| firedAt, | |
| facts, | |
| dashboardLink, | |
| unsubscribeLink, | |
| organization, | |
| project, | |
| environment, | |
| } = props; | |
| const details = [identity, ...facts.slice(0, 3).map((fact) => `${fact.label}: ${fact.value}`)]; | |
| const accentColor = TONE_COLOR[tone ?? "neutral"] ?? TONE_COLOR.neutral; |
…s-watch # Conflicts: # apps/webapp/app/components/dashboard-agent/dashboardAgentLauncher.tsx
@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: |
Observability mapAs of 20/100 over 425 measured of 441 entry points (base 19, up 1) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
| /** How often a recovery watch polls, and how long it lives. Aggregate conditions floor at 5m. */ | ||
| const RECOVERY_WATCH = { checkEveryMinutes: 5, maxHours: 6 } as const; |
There was a problem hiding this comment.
🔍 Recovery watch window in the report footer disagrees with the recommendation helper
RECOVERY_WATCH here uses maxHours: 6, but healthWatchRecommendation in apps/webapp/app/components/dashboard-agent/watch-recommendations.ts:70-78 builds the same health_recovery spec with maxHours: 2 (and the same 5-minute cadence). Two entry points for the same watch therefore pre-fill different windows. Not a correctness bug — both are valid specs the user can edit — but worth deciding which is intended so the two agree.
Was this helpful? React with 👍 or 👎 to provide feedback.
| create_alert: tool({ | ||
| ...createAlertSchema, | ||
| execute: async ({ email }) => { | ||
| if (!hasAuth) return NO_AUTH; | ||
| if (!ctx.chatId) return { error: "No chat is available to create an alert from." }; | ||
| const result = await alertsRequest("POST", "/api/v1/dashboard-agent/alerts", { | ||
| chatId: ctx.chatId, | ||
| channel: "email", | ||
| ...(email ? { email } : {}), | ||
| }); | ||
| if ("error" in result) return result; | ||
| return { created: true, alert: (result.data as { alert?: unknown } | undefined)?.alert }; | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🟡 The assistant cannot tell the user which address an email alert was set up for
The confirmation of a newly created email alert is read from a field the server never sends (data.alert at internal-packages/dashboard-agent/src/tool-alerts.ts:89), so the assistant is left with nothing to name.
Impact: After setting up an email alert the assistant can only say it worked, never to which address, and cannot show the alert it just created.
Response shape mismatch between the create-alert endpoint and the tool that reads it
The route responds with a flat object: return json({ id: channel.id, type: channel.type, target: email, enabled: channel.enabled }) (apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts:185). There is no alert key anywhere in the success body.
The tool reads (result.data as { alert?: unknown } | undefined)?.alert, so it always resolves to undefined and returns { created: true, alert: undefined }.
The unit test in internal-packages/dashboard-agent/src/dashboard-agent.test.ts mocks the response as { ok: true, alert: { id: "alert_2", type: "EMAIL" } }, which is not the shape the real route produces — hence the mismatch is not caught.
Prompt for agents
The `create_alert` tool in internal-packages/dashboard-agent/src/tool-alerts.ts reads `data.alert` from the POST /api/v1/dashboard-agent/alerts response, but that route (apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts) returns a flat `{ id, type, target, enabled }` object with no `alert` key. Pick one shape and make both sides agree — either wrap the route's success body in `{ alert: {...} }`, or have the tool pass through the flat fields. Note the existing unit test mocks the wrapped shape, so it should be updated to mirror the real route's body once the contract is settled.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (res.status === 403) { | ||
| return { | ||
| error: | ||
| data?.reason === "email_alerts_not_configured" | ||
| ? "Email delivery isn't set up on this instance, so an email alert can't be created. Tell the user that, and that watch results still show in the dashboard." | ||
| : "Email alerts aren't enabled here. Tell the user that, and that watch results still show in the dashboard.", | ||
| }; | ||
| } | ||
| if (res.status === 400 && data?.code === "email_not_allowed") { |
There was a problem hiding this comment.
🟡 When email delivery is not configured, the assistant gives the wrong explanation for a refused alert
The reason an alert was refused is looked for under a name the server never uses (data?.reason at internal-packages/dashboard-agent/src/tool-alerts.ts:45), so the specific explanation is never reached.
Impact: When this installation simply has no email delivery set up, the assistant tells the user email alerts are disabled for them, which sends them looking in the wrong place.
The refusal code travels as `code`, not `reason`
The route sends json({ error: "Alerts are not available here", code: gate.reason }, { status: 403 }) (apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts:154) — the deny reason is under code.
The tool branches on data?.reason === "email_alerts_not_configured", which is always undefined, so every 403 falls through to the generic "Email alerts aren't enabled here." message, including the email_alerts_not_configured case that has its own wording written for it.
The adjacent 400 branch in the same helper correctly reads data?.code === "email_not_allowed", showing the inconsistency is unintentional. The unit test mocks { error: "denied", reason: "..." }, which does not match the real route body.
| if (res.status === 403) { | |
| return { | |
| error: | |
| data?.reason === "email_alerts_not_configured" | |
| ? "Email delivery isn't set up on this instance, so an email alert can't be created. Tell the user that, and that watch results still show in the dashboard." | |
| : "Email alerts aren't enabled here. Tell the user that, and that watch results still show in the dashboard.", | |
| }; | |
| } | |
| if (res.status === 400 && data?.code === "email_not_allowed") { | |
| // 403 is a capability refusal and `code` says which one. | |
| if (res.status === 403) { | |
| return { | |
| error: | |
| data?.code === "email_alerts_not_configured" | |
| ? "Email delivery isn't set up on this instance, so an email alert can't be created. Tell the user that, and that watch results still show in the dashboard." | |
| : "Email alerts aren't enabled here. Tell the user that, and that watch results still show in the dashboard.", | |
| }; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const messages = data.messages; | ||
| if (active?.chatId === data.chatId) { | ||
| setAppendedMessages((current) => ({ | ||
| chatId: data.chatId!, | ||
| messages, | ||
| seq: (current?.seq ?? 0) + 1, | ||
| })); | ||
| } else { | ||
| // No session: nothing is streaming and the records are the whole chat. | ||
| setActive({ chatId: data.chatId, messages, session: null }); | ||
| } |
There was a problem hiding this comment.
🟡 A watch submitted while a chat is still loading can leave the wrong conversation on screen
The conversation created by submitting a watch is installed (setActive(...) at apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:413) without invalidating any chat-open still in flight, so a slower earlier open can land afterwards and replace it.
Impact: The user submits a watch, sees its confirmation appear, and then the panel silently swaps to a different conversation.
Missing `openChatRequestSeq` bump on the watch-submit path
Every other place in the panel that replaces active first invalidates in-flight opens by bumping the guard: newChat (apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:439) and the org-change effect (:281) both do openChatRequestSeq.current += 1 before setActive. openChat and createChat each capture const seq = ++openChatRequestSeq.current and drop their result if seq !== openChatRequestSeq.current.
submitWatch calls setActive({ chatId: data.chatId, messages, session: null }) without touching the counter, so an openChat started before the submit (for example the mount-time restore of the last chat, or a toast's "Open chat") still considers itself current and overwrites the watch's chat when its fetch resolves. The reverse also holds: a chat opened while the submit POST is in flight is clobbered by the submit's setActive.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Only a real evaluation is recorded, final or not: `unavailable` means nothing was read, | ||
| // so writing it would move `lastCheckedAt` and overwrite the facts a streak lives in. | ||
| // Guarded on `active`, and never touches `tickCount`. | ||
| if (outcome.result !== "unavailable") { | ||
| await recordWatchCheck(dashboardAgentDb, { | ||
| id: watch.id, | ||
| lastResult: { | ||
| result: outcome.result, | ||
| facts: outcome.facts, | ||
| observed: outcome.observed, | ||
| final, | ||
| }, | ||
| }); | ||
| } else { | ||
| // Looked at, not checked: this rotates the watch out of its group's head without | ||
| // touching its dueness or the facts its streak lives in. | ||
| await recordWatchAttempt(dashboardAgentDb, { id: watch.id }); | ||
| } |
There was a problem hiding this comment.
🔍 The batch check records the tick on both sides
evaluateGroup in the webapp already calls recordWatchCheck for every non-unavailable verdict, and the agent-side lifecycle (runWatchLifecycle → deps.store.recordWatchCheck for a pending result) records it again from the entry the same call returned. Harmless as written — the write is idempotent and guarded on active — but it means lastResult/lastCheckedAt are written twice per pending tick, and the two sides also disagree on what an unreadable check does (recordWatchAttempt here versus the {checkFailed: true, previous} wrapper in the per-watch lane). Worth confirming the divergence is deliberate.
Was this helpful? React with 👍 or 👎 to provide feedback.
An investigation card carries its own watch button, and the prompt asked for a watch offer on top of it. The card wins — it is the one with the pre-filled spec — and the prompt now says so too.
| const service = new CreateAlertChannelService(); | ||
| const channel = await service.call(environment.project.externalRef, userId, { | ||
| name: `Watch alerts for ${email}`, | ||
| alertTypes: [DASHBOARD_AGENT_WATCH_ALERT_TYPE], | ||
| environmentTypes: [environment.type], | ||
| // Stable per (email, project), so asking twice re-enables one channel. | ||
| deduplicationKey: `dashboard-agent-watch:${email}`, | ||
| channel: { type: "EMAIL", email }, | ||
| }); |
There was a problem hiding this comment.
🟡 Subscribing to watch emails in one environment silently turns them off for another
A new watch-alert subscription reuses the same per-user channel and replaces its environment list with only the current environment (environmentTypes: [environment.type] at apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts:180), so an earlier subscription made in a different environment stops delivering.
Impact: A user who turns on watch emails in a second environment quietly stops receiving them for the first, with no warning anywhere.
The deduplication key makes the create an overwrite
Both the agent route (apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts:176-184) and subscribeUserToWatchAlerts (apps/webapp/app/services/dashboardAgentWatchAlerts.server.ts:210-217) call CreateAlertChannelService with deduplicationKey = dashboard-agent-watch:{email} and environmentTypes: [environment.type].
CreateAlertChannelService treats a matching deduplication key as an update and overwrites the row wholesale (apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts:77-88), including environmentTypes. So subscribing from production and later from staging leaves the channel with environmentTypes: ["STAGING"].
The fan-out filters on environmentTypes: { has: environment.type } (apps/webapp/app/v3/services/alerts/deliverDashboardAgentWatchAlert.server.ts:158-166), so production watch fires stop producing an email. The confirmation block still reads "You'll get an email as well as the chat" for the new subscription, and nothing tells the user the old one was dropped.
A fix would union the new environment type with the channel's existing list (read the channel by deduplication key first), or key the channel per (email, environment type).
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Converge: an attempt that died mid-create left its row under the reserved id. | ||
| const reservedWatchId = submission.watchId ?? generateWatchId(); | ||
| const reserved = await getWatch(dashboardAgentDb, { id: reservedWatchId }); | ||
| if (reserved) { | ||
| if (reserved.status === "cancelled") { | ||
| // The previous attempt created it and then took it back. The id is spent, so this | ||
| // submission can't be completed; a fresh submit gets a fresh request id. | ||
| return refuse({ | ||
| code: "internal", | ||
| error: "The watch couldn't be scheduled. Nothing is being watched.", | ||
| }); | ||
| } | ||
| // `unavailable` isn't recoverable here: it belonged to the attempt that died. | ||
| return settleCreated({ watchId: reserved.id, unavailable: false, adopted: true }); | ||
| } |
There was a problem hiding this comment.
🔍 A submit refused after a watch already exists cancels it, but the retry path can only fail
In settleCreated, if the ledger write loses the race and the winning outcome doesn't name this watch, the freshly created watch is cancelled with reason superseded and the winner is replayed (apps/webapp/app/services/dashboardAgentWatches.server.ts:808-818). Combined with the converge branch, a reserved id that ends up cancelled makes every later retry of the same clientRequestId refuse permanently (apps/webapp/app/services/dashboardAgentWatches.server.ts:834-843) — the user has to submit the card again to get a fresh request id. That is deliberate per the comments, but it means a transient race can turn a submit into an unrecoverable refusal for that card instance; worth confirming the UI re-mints clientRequestId after a refusal (it keeps the same one across retries in DashboardAgentPanel.submitWatch).
Was this helpful? React with 👍 or 👎 to provide feedback.
…s-watch # Conflicts: # apps/webapp/app/components/dashboard-agent/view-actions.test.ts # apps/webapp/app/components/dashboard-agent/view-actions.ts
| <WatchButton | ||
| spec={errorWatchRecommendation(ErrorId.toFriendlyId(errorGroup.fingerprint))} | ||
| /> |
There was a problem hiding this comment.
🟡 The same error can be watched twice, because two places name it differently
The error detail page names the watched error with the prefixed form of its id (errorWatchRecommendation(ErrorId.toFriendlyId(...)) at apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx:601) while every other place that offers the same watch uses the bare form, so the duplicate guard can't tell them apart.
Impact: A user who sets the same "tell me if this error comes back" watch from the error page and from an investigation card ends up with two watches, two chat wakes and two emails for one recurrence, and one of them shows a link that leads nowhere.
How the two identities diverge, and where the broken link comes from
The store dedupes watches on watchIdentity(spec), which for this kind is literally error_recurrence:${spec.fingerprint} (internal-packages/dashboard-agent-contracts/src/watch.ts:164-165) — it does no normalization.
Two producers disagree about what goes in spec.fingerprint:
- The error page passes
ErrorId.toFriendlyId(errorGroup.fingerprint), i.e.error_c4b4a797397a9c43. - The investigation card's "Watch for a repeat" action passes the fingerprint parsed out of a
trigger://URI (internal-packages/dashboard-agent/src/tool-investigations.ts:98-104), and those URIs are built with the prefix already stripped (internal-packages/dashboard-agent/src/tool-evidence.ts:124), i.e.c4b4a797397a9c43.
So the identities are error_recurrence:error_c4b4a797397a9c43 vs error_recurrence:c4b4a797397a9c43. precheckWatchCreation / createWatch compare on identity, so the second create is not refused as a duplicate — it takes a second of the three per-chat slots and fires its own wake and alert. Both actually fire, because the check normalizes the value before querying (normalizeErrorFingerprint → ErrorId.toId in apps/webapp/app/services/dashboardAgentWatchErrorChecks.ts:15-17), so both watches resolve on the same occurrence.
The chip label hides the divergence too: watchChipLabel runs the value through shortFingerprint, which strips error_, so both chips read identically.
Second consequence: the wake narration builds the object link straight from spec.fingerprint (internal-packages/dashboard-agent/src/watch-actions.ts:263). For an error-page-created watch that yields trigger://…/error/error_c4b4a797397a9c43, a fingerprint that resolves to nothing.
Prompt for agents
Watch specs for `error_recurrence` are created with two different spellings of the same fingerprint, and the store's dedup key is the raw string.
`watchIdentity` in internal-packages/dashboard-agent-contracts/src/watch.ts returns `error_recurrence:${spec.fingerprint}` verbatim. The error detail route passes `ErrorId.toFriendlyId(fingerprint)` (prefixed, e.g. `error_abc…`) into `errorWatchRecommendation`, while the investigation card's watch action in internal-packages/dashboard-agent/src/tool-investigations.ts passes the value parsed from a `trigger://` URI, which has already had the `error_` prefix stripped by tool-evidence.ts. The result is two distinct identities for the same error group, so the per-chat duplicate guard and the `MAX_ACTIVE_WATCHES_PER_CHAT` cap both treat them as unrelated, and the user gets two wakes and two alerts for one recurrence.
Separately, internal-packages/dashboard-agent/src/watch-actions.ts builds the wake's `trigger://…/error/{fingerprint}` link straight from `spec.fingerprint`, so a prefixed value produces an unresolvable link.
Pick one canonical form (the bare fingerprint looks right, since that is what the `trigger://` URI scheme and ClickHouse already use) and normalize at a single choke point — either in `watchIdentity`/`watchSpecSchema` in the contracts package, or in `errorWatchRecommendation` — so every producer of an `error_recurrence` spec lands on the same identity. Check the presenter helpers (`shortFingerprint`, `watchSubjectLabel`, `watchChipLabel`) still read correctly after the change, and add a test asserting the error page and the investigation card produce the same `watchIdentity`.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (resolved.resolution === "condition_met") { | ||
| // Keyed on the watch, so the wake's own notification can't double-alert it. | ||
| try { | ||
| await enqueueWatchFiredAlert(transitioned, "fired"); | ||
| } catch (error) { | ||
| logger.error("Dashboard agent watch sweep: failed to enqueue the fired alert", { | ||
| watchId: watch.id, | ||
| error, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 A watch recovered by the background safety net can email the same alert twice
The alert for a watch that the background safety net finishes is queued (enqueueWatchFiredAlert(transitioned, "fired") at apps/webapp/app/services/dashboardAgentWatchSweep.server.ts:284) without first taking the once-only marker, so the follow-up report of the same watch takes that marker and queues the alert a second time.
Impact: Users can receive the same watch alert email (and Slack message / webhook) twice.
Mechanism: the sweep bypasses `claimWatchAlertDispatch`, so the `/fired` callback still claims
The once-only guard for a watch's alert fan-out is claimWatchAlertDispatch, which sets alertDispatchKey only when it is still null (internal-packages/dashboard-agent-db/src/watch-queries.ts:952-969). The normal tick path respects it: the delivery calls notifyFired → POST /api/v1/dashboard-agent/watches/:id/fired, which claims and then enqueues (apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.fired.ts:85-99).
finalizeOverdueWatch in the sweep enqueues the fan-out directly and never claims. It then calls deliver(transitioned) (apps/webapp/app/services/dashboardAgentWatchSweep.server.ts:294), which schedules the watcher task; the watcher delivers the wake and posts /fired, whose claimWatchAlertDispatch succeeds because alertDispatchKey is still null — and enqueues v3.deliverDashboardAgentWatchAlert again.
The job id watch-alert:{watchId} only deduplicates while the job is still queued: SimpleQueue.enqueue re-adds the item under the same id once the earlier job has been processed and removed (packages/redis-worker/src/queue.ts:107-127). Since the watcher task takes seconds to start, the first fan-out (and its per-channel jobs, keyed the same way) will usually already have completed, so the second enqueue is a genuinely new delivery.
Prompt for agents
In apps/webapp/app/services/dashboardAgentWatchSweep.server.ts, finalizeOverdueWatch enqueues the fired alert directly. Every other caller of enqueueWatchFiredAlert first takes the row's once-only marker via claimWatchAlertDispatch (see apps/webapp/app/routes/api.v1.dashboard-agent.watches.$watchId.fired.ts), which is what stops a second fan-out. Because the sweep skips the claim, the /fired callback that follows the sweep's own scheduled delivery still claims successfully and enqueues a second fan-out; the redis-worker job id only deduplicates while the job is queued, so once the first fan-out has been processed the second one really delivers. Make the sweep claim the dispatch before enqueueing (and release it on failure, like the /fired route does).
Was this helpful? React with 👍 or 👎 to provide feedback.
… kind was guessed wrong The metrics route answers an unknown queue with zeroes, so asking for the wrong kind read as an idle queue. get_queue now tries the other kind before believing them.
…hips A paused queue can neither drain nor grow, so every condition it could watch for is a promise nothing keeps until someone resumes it.
The page chips come from the page registry, not the signals — so hiding the saturation signal left both the investigate and the watch chip on a paused queue.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Metrics are a window: they cannot show a pause, and they cannot show a backlog that arrived after the window. get_queue now carries the queue's live row, and the prompt leads with it.
…top reading empty as absent
Metrics authorize as a query read; paused, depth and limit live on the queue row, which is a queues read the agent's token did not carry — so a live lookup 403'd and read as a missing queue.
| ): Promise<WatchQueueOldestAge | null> { | ||
| const [breakdown, oldestQueuedAt] = await Promise.all([ | ||
| engine.concurrencyKeyBreakdown(environment, queueName, { limit: OLDEST_AGE_CK_LIMIT }), | ||
| engine.oldestMessageInQueue(environment, queueName), | ||
| ]); | ||
|
|
||
| const waitingKeys = breakdown.keys.filter((key) => key.queued > 0); | ||
| const ageMs = | ||
| waitingKeys.length > 0 | ||
| ? waitingKeys.reduce((max, key) => Math.max(max, now.getTime() - key.oldestEnqueuedAt), 0) | ||
| : typeof oldestQueuedAt === "number" | ||
| ? Math.max(0, now.getTime() - oldestQueuedAt) | ||
| : null; | ||
|
|
||
| return { ageMs, source: "live_queue", current: true, asOf: now }; | ||
| } |
There was a problem hiding this comment.
🟡 A wait-time watch on a deleted queue keeps waiting instead of reporting that the queue is gone
A wait-time watch never learns that the queue it is watching has disappeared, because the reader that feeds it always reports an answer (readWatchQueueOldestAge at apps/webapp/app/services/dashboardAgentWatchChecks.server.ts:149-164 never returns null), so the check keeps waiting until the whole window runs out.
Impact: A watch on a queue that has been removed reports "nothing happened" hours later instead of telling the user straight away that there is nothing left to watch.
The `queue_not_found` branch is unreachable with the production reader
checkQueueOldestAge (apps/webapp/app/services/dashboardAgentWatchQueueChecks.ts:265-281) resolves terminal_unsatisfied / queue_not_found only when deps.readQueueOldestAge(...) returns null, and then confirms with queueExists. But the wired implementation always returns { ageMs, source: "live_queue", current: true } — ageMs is null when nothing is waiting, which the check treats as pending. An engine failure rejects instead, which checkWatch turns into unavailable, not null.
So for a missing queue the watch stays pending for the whole window and finally resolves window_completed ("stayed under 5m"), even though the contracts and presenter explicitly support the condition_impossible headline "…queue no longer exists" for queue_oldest_age (see apps/webapp/app/components/dashboard-agent/wake-banner.test.ts:253-269). The depth-based kinds get this right because readWatchQueueDepth can return null.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ols actually use Most tools spend an environment JWT with a hardcoded scope list; the delegated token's cap only ceilings that exchange. Queues were missing from the list, so the live lookup 403'd and read as a queue that does not exist.
| {queue.paused ? null : ( | ||
| <WatchButton spec={queueWatchRecommendation(queue.name, { oldestWaitMs })} /> | ||
| )} |
There was a problem hiding this comment.
🔴 Setting up a watch from a task queue's page fails because the wrong queue name is sent
The queue's shortened display name is handed to the watch (queueWatchRecommendation(queue.name, …) at apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues_.$queueParam/route.tsx:348) instead of the queue's real internal name, so the server can't find the queue and refuses to start the watch.
Impact: On a task queue — the most common kind — clicking Watch… always answers "That target doesn't exist in this environment", so queue watches can never be created from the queue page.
Display name vs. engine name on the queue detail page
QueueRetrievePresenter's toQueueItem strips the task/ prefix (apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts:166), so queue.name is e.g. my-task. The page therefore computes const fullName = queue.type === "task" ? + "task/${queue.name}" + : queue.name (…queues_.$queueParam/route.tsx:126) and uses fullName for every engine and metrics call (engine.concurrencyKeyBreakdown, engine.oldestMessageInQueue, useQueueMetric).
The new WatchButton is the one caller that passes queue.name. That value flows into WatchSpec.queue, and creation validates the target with watchQueueExistsOnPrimary → prisma.taskQueue.findFirst({ runtimeEnvironmentId, name: queueName }) (apps/webapp/app/services/dashboardAgentWatchChecks.server.ts:69-77), where TaskQueue.name is stored with the task/ prefix. validateWatchTarget (apps/webapp/app/services/dashboardAgentWatches.server.ts:204-221) then returns invalid_target.
The same mismatch feeds the suggested-prompt text: queueAgentPageContext also reads the stripped name (apps/webapp/app/components/dashboard-agent/suggested-prompts/page-mappers.ts:218), so the "Watch the {name} queue" chip asks the model for a spec with the same unusable name.
| {queue.paused ? null : ( | |
| <WatchButton spec={queueWatchRecommendation(queue.name, { oldestWaitMs })} /> | |
| )} | |
| {queue.paused ? null : ( | |
| <WatchButton spec={queueWatchRecommendation(fullName, { oldestWaitMs })} /> | |
| )} |
Was this helpful? React with 👍 or 👎 to provide feedback.
| -- AlterEnum | ||
| ALTER TYPE "public"."ProjectAlertType" ADD VALUE IF NOT EXISTS 'DASHBOARD_AGENT_WATCH'; |
There was a problem hiding this comment.
🔍 New migration sorts before two already-merged migrations
This migration is timestamped 20260729120000, but main already contains 20260731160000_add_worker_deployment_environment_id_created_at_index and 20260806100000_add_background_worker_task_project_id_slug_created_at_index. Once merged, the new folder sorts before two migrations that have already been applied.
pnpm run db:migrate uses prisma migrate deploy, which applies pending migrations regardless of order, so deployments are fine. But prisma migrate dev (used by anyone running db:migrate:dev:create afterwards) reports the sequence as applied out of order and prompts to reset. Renaming the folder to a timestamp after the current head before merge avoids that for everyone else.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function readWatchQueueOldestAge( | ||
| environment: AuthenticatedEnvironment, | ||
| queueName: string, | ||
| now: Date = new Date() | ||
| ): Promise<WatchQueueOldestAge | null> { | ||
| const [breakdown, oldestQueuedAt] = await Promise.all([ | ||
| engine.concurrencyKeyBreakdown(environment, queueName, { limit: OLDEST_AGE_CK_LIMIT }), | ||
| engine.oldestMessageInQueue(environment, queueName), | ||
| ]); | ||
|
|
||
| const waitingKeys = breakdown.keys.filter((key) => key.queued > 0); | ||
| const ageMs = | ||
| waitingKeys.length > 0 | ||
| ? waitingKeys.reduce((max, key) => Math.max(max, now.getTime() - key.oldestEnqueuedAt), 0) | ||
| : typeof oldestQueuedAt === "number" | ||
| ? Math.max(0, now.getTime() - oldestQueuedAt) | ||
| : null; | ||
|
|
||
| return { ageMs, source: "live_queue", current: true, asOf: now }; | ||
| } |
There was a problem hiding this comment.
🔍 A missing queue can never resolve a queue_oldest_age watch as impossible
readWatchQueueOldestAge always returns a reading ({ ageMs, source: "live_queue", current: true }) and never null — an empty or non-existent queue simply yields ageMs: null, and a reader failure throws rather than returning null.
checkQueueOldestAge only reaches its queue_not_found → terminal_unsatisfied branch when the reading is null (apps/webapp/app/services/dashboardAgentWatchQueueChecks.ts:267-281), so that branch is unreachable in production wiring. A queue_oldest_age watch on a deleted queue stays pending until its window closes instead of resolving as condition_impossible, unlike the depth-based kinds, whose reader does return null. Worth aligning the reader so the two families behave the same.
Was this helpful? React with 👍 or 👎 to provide feedback.
| <WatchButton | ||
| spec={errorWatchRecommendation(ErrorId.toFriendlyId(errorGroup.fingerprint))} | ||
| /> |
There was a problem hiding this comment.
🔍 Two different fingerprint forms can produce two watches for the same error
The error page builds the spec from ErrorId.toFriendlyId(errorGroup.fingerprint), i.e. error_<hash>, while the investigation card's watch_recurrence action builds it from the parsed URI's raw fingerprint (internal-packages/dashboard-agent/src/tool-investigations.ts:91-108) and the check normalises with ErrorId.toId at evaluation time.
Dedup happens on watchIdentity(spec), which is derived from the un-normalised spec, so error_recurrence:error_abc and error_recurrence:abc are distinct identities. A user who arms a recurrence watch from the error page and again from an investigation card gets two live watches for the same error, both counting against the 3-per-chat cap and both waking the chat. Normalising the fingerprint in watchIdentity (or in the spec schema) would close this.
Was this helpful? React with 👍 or 👎 to provide feedback.
The task was one environment claim; the exchange's own auth and scope ceiling did not need rewriting. Its cap intersection is main's again, and the route keeps only the claim check and the acting client.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…anel A watch wake was the only thing that raised the dot and the highlight. An answer or a settled card that landed while the panel was closed now does the same — without a toast, which stays a wake's alone.
| useEffect(() => { | ||
| if (!active?.chatId) return; | ||
| const chatId = active.chatId; | ||
| onChatRead?.(chatId); | ||
| justRead.current.add(chatId); | ||
| setChats((previous) => | ||
| previous.map((chat) => (chat.id === chatId ? { ...chat, hasUnreadWake: false } : chat)) | ||
| ); | ||
| // Read again on the way out: a wake can land while the chat is open. | ||
| return () => { | ||
| onChatRead?.(chatId); | ||
| justRead.current.add(chatId); | ||
| }; | ||
| }, [active?.chatId, onChatRead]); |
There was a problem hiding this comment.
🟡 The "new activity" dot on the chat button disappears too early
The count of chats with unseen activity is lowered by one (setUnreadWork((count) => Math.max(0, count - 1)) at apps/webapp/app/components/dashboard-agent/DashboardAgent.tsx:232) both when a chat is opened and again when it is left, and even for chats that had nothing new, so the dot vanishes while other chats still hold updates the person has never seen.
Impact: People stop being told about chats that finished work or were woken by a watch, because the indicator has already been cleared.
Why the counter is decremented twice per visit
markChatRead is passed to the panel as onChatRead (apps/webapp/app/components/dashboard-agent/DashboardAgent.tsx:303). In apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:318-331 the effect calls onChatRead?.(chatId) in the effect body and again in its cleanup:
useEffect(() => {
if (!active?.chatId) return;
const chatId = active.chatId;
onChatRead?.(chatId); // decrement #1
...
return () => {
onChatRead?.(chatId); // decrement #2
...
};
}, [active?.chatId, onChatRead]);So one visit to a chat (open, then switch chat or close the panel, which unmounts DashboardAgentPanel) subtracts 2 from a count that should only ever go down by 1 — and only if that chat actually had unread work. Switching between two already-read chats subtracts further. loadHistory re-derives the true count (onUnreadWorkChange?.(settled.filter((chat) => chat.hasUnreadWork).length) at apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:164), but it is not called on chat open, so the drifted value can persist for the whole session.
A fix would be to make the read idempotent per chat (only decrement when the chat is currently marked unread in chats), rather than decrementing unconditionally on both edges.
Was this helpful? React with 👍 or 👎 to provide feedback.
| setChats((previous) => | ||
| previous.map((chat) => (chat.id === chatId ? { ...chat, hasUnreadWake: false } : chat)) | ||
| ); |
There was a problem hiding this comment.
🟡 A chat you just opened keeps showing as having new activity in the list
Opening a chat only clears its watch-wake marker locally ({ ...chat, hasUnreadWake: false } at apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:324) and leaves the "work you haven't seen" marker set, so the chat you are reading right now stays highlighted and pinned to the top of the list.
Impact: The chat currently on screen looks unread, and jumps above older chats, until something else refreshes the list.
Why the two markers diverge
chatIsUnread (apps/webapp/app/components/dashboard-agent/DashboardAgentHistory.tsx:55-57) is true when either hasUnreadWake or hasUnreadWork is set, and unreadFirst (apps/webapp/app/components/dashboard-agent/DashboardAgentHistory.tsx:60-62) sorts those chats to the top with an unread dot on the row.
The read-on-open effect clears only hasUnreadWake in local state, while the server-refresh mask a few lines above clears both:
read.has(chat.id) ? { ...chat, hasUnreadWake: false, hasUnreadWork: false } : chat(apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx:159-161)
openChat does not call loadHistory, so nothing re-fetches the list on open; the stale hasUnreadWork survives until the next turn settles or a watch is created.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const rendered = latestRevisionBlocks(blocks); | ||
| const watchOfferedOnCard = cardAlreadyOffersWatch(rendered); |
There was a problem hiding this comment.
🔍 The de-duplication of the "one watch button per answer" rule is scoped per view block array, not per answer
cardAlreadyOffersWatch(rendered) is evaluated inside ViewBlocks, which renders the blocks of a single render_view / data-view part. The comment on withoutWatchActions says "an answer that does both shows the same button twice", but if the model emits the investigation card and the actions block from two separate render_view calls in one turn, each ViewBlocks instance computes its own watchOfferedOnCard and the duplicate button survives. If duplicate suppression is meant to be per-turn, the flag needs to be lifted to DashboardAgentTurn (which already walks all parts of a message) rather than computed per block array.
Was this helpful? React with 👍 or 👎 to provide feedback.
…losed panel DashboardAgent declared turnStarted but nothing set it, so a question asked and then left with the panel closed raised no dot until the next reload. The panel now reports turn activity up to DashboardAgent, which latches onto the chat whose turn is running. Closing the panel reports nothing, so the latch holds; re-opening the chat with the turn over clears it.
A custom queue's name has nothing to do with any task id, but the agent searched the deployed task list for a task named after the queue, found none, and reported the queue's tasks as deleted or renamed. get_queue now returns consumerTasks for a custom queue — the deployed task slugs whose queue config names it — and the prompt says an absent task of that name is not evidence about the queue.
…iew and schedule_watch The block catalog and the watch spec spelled every variant out in full, and the serialized tool definitions are paid on every call of every chat. Two collapses: the seven simple evidence kinds become one member with a `kind` enum, and the watch kinds that take the same fields share a member each. Same payloads accepted, same payloads refused.
| // Work that finished while the chat was closed: the transcript moved on after the | ||
| // last time its owner looked. A wake is one way that happens, an answer is another. | ||
| hasUnreadWork: | ||
| chat.lastMessageAt !== null && | ||
| (chat.lastReadAt === null || chat.lastMessageAt > chat.lastReadAt), |
There was a problem hiding this comment.
🔍 Every pre-existing chat becomes "unread work" on first load after rollout
hasUnreadWork treats lastReadAt === null as unread, and the last_read_at column (drizzle migration 0002_watches_and_chat_messages.sql:91) is added nullable with no backfill. So the first time a user loads the dashboard after this ships, every chat they have ever had that carries a lastMessageAt is reported unread: countChatsWithUnreadWork lights the launcher dot, and unreadFirst reorders and highlights the whole history list.
It clears as soon as each chat is opened, and the feature is access-gated, so this may be an accepted cost — but if not, backfilling last_read_at = last_message_at (or treating chats whose lastMessageAt predates the column as read) avoids a one-off wall of false unreads.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ed error id Sibling tests were updated when the fingerprint stopped truncating; the snapshot was not.
…ing one Each test replays the migrations inside its own budget, which overruns vitest's 5s default on a loaded CI host and times out a different test each run.
| if ("error" in result) return result; | ||
| return { created: true, alert: (result.data as { alert?: unknown } | undefined)?.alert }; | ||
| }, |
There was a problem hiding this comment.
🔍 create_alert reads an alert field the route never returns
The tool returns { created: true, alert: result.data?.alert }, but apps/webapp/app/routes/api.v1.dashboard-agent.alerts.ts:186 responds with { id, type, target, enabled } — no ok and no alert wrapper. So alert is always undefined and the model is told an alert was created with no detail about where it will go.
The unit test in internal-packages/dashboard-agent/src/dashboard-agent.test.ts mocks { ok: true, alert: {...} }, which is why the mismatch isn't caught: the mock and the real route disagree. Either wrap the route's response as { ok: true, alert: {...} } or read the flat fields in the tool.
Was this helpful? React with 👍 or 👎 to provide feedback.
The queue detail page presents a task queue with its `task/` prefix stripped, but `TaskQueue.name` keeps it, so every watch the page offered on a task's own queue was refused as a missing target.
The sweep alerted without claiming, so the wake it then scheduled claimed successfully and alerted again — a recovered watch emailed twice.
The errors page cites `error_<fingerprint>` and the agent's tools cite the bare one, so the same error could carry two watches — two wakes and two emails per recurrence — and the prefixed spelling produced an unresolvable link in the wake.
…adable The reader always reported a live queue, so a queue_oldest_age watch on a deleted queue sat pending for its whole window and an engine failure read as a healthy zero wait.
…o close Only a deleted chat is swallowed now. A refused close left the card in_progress while the action reported success, so the panel span forever with nothing to retry it.
1 participant
Stacked on #4529, which is stacked on #4418. Merge those first.
Watch is the agent noticing something later: you ask it to tell you when a condition holds, and it answers when it does — or when it can't any more.
What's inside
(chatId, clientRequestId), so a retried submission replays instead of duplicating.How to review
GUIDEBOOK.md — local setup and a walkthrough of all 15 scenarios.