feat(webapp): dashboard agent — Watch - #4525
Conversation
🦋 Changeset detectedLatest commit: e954d82 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:
WalkthroughAdded watch creation and configuration for runs, queues, errors, and health reports. Added scheduled checks, lifecycle handling, wake notifications, automatic investigations, unread tracking, and cross-browser activity polling. Added email, Slack, and webhook alert delivery with subscription management. Added dashboard-agent tools, APIs, persistence, worker tasks, scenario tooling, documentation, and extensive unit and integration coverage. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@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 424 measured of 440 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. |
712396b to
e110e90
Compare
bd4d4a0 to
887f5b6
Compare
c0f0058 to
e7432a8
Compare
887f5b6 to
17a0f07
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…r_view The flag was computed inside `ViewBlocks`, which sees the blocks of a single `render_view` part. A turn that renders the investigation card in one call and the actions block in another gave each call its own answer, and the duplicate Watch button came back. It is now computed where every part of the message is in scope; a card can still add its own offer, never drop the turn's.
The poll deduped fresh wakes against a 50-id localStorage set alone, so inside the feed's 15-minute window a second browser — or one whose site data was cleared — toasted wakes the user had already read. The payload's own `unread` flag now gates the toast as well. A wake landing in an open chat stays unread until that chat's next read, so it still toasts.
`SimpleTooltip` drops its trigger from the tab order unless `tabbable` is set, and the chip's label tooltip is the only place its status, cadence and expiry are written. The cancel tooltip beside it already passed the prop.
The seed script refused a non-local Redis or ClickHouse host outright, but sent the target's API key to whatever `APP_ORIGIN` named. All three now go through one guard, which also fixes it for a bracketed IPv6 host — `URL.hostname` hands back `[::1]`, which the old set membership never matched.
…t read The column was added nullable and every reader treats NULL as unread, so the first load after rollout reported every pre-existing chat unread.
… ClickHouse The chat route now pulls in the watch services, and through them a ClickHouse client built at import time from an unset env var.
The file the review comment named was missed: it opened a pool per case and never closed one. Adds the sibling files' afterEach and their 30s case timeout.
…that isn't there The queue's live row read collapsed every non-ok response into "no live row", so a 401, a 429 or a 5xx reached the model as exists:false — the queue does not exist. Only a 404 is evidence of absence now; anything else reports exists:"unknown" with the status, and the prompt says unknown is never missing.
The wake poll's callback closed over `open` from the render that started it, so once the panel opened the subtraction never applied. The panel's open state now goes through a ref, like the visible chat already does, rather than adding `open` to the effect's deps and restarting the poll on every open and close.
…watch A watch can expire or be cancelled with nothing written back into the transcript, so asking the summariser for "any watch that is running" preserved an old confirmation as current state.
The guard compared a slice against its own start, so it could not fail. Move the decision into takeNavigateIntent and drive it across two commits instead.
The unmount case repeated its neighbour and no query string can reach unmountTeardown. Guard the pathname tracking that does decide it.
…token pk_ is browser-shipped and environment-bound. Nothing routes it to the query API today, so the cap costs no caller anything and the helper stops promising the wrong thing.
The seeder and its `scenarios:watch` script were only ever a way to reproduce a watch condition by hand. The guidebook now states the conditions themselves, so there is nothing left for the kit to be the answer to.
…oduce it The guidebook was a walkthrough of the scenario kit: a command, some clicks, and the sentence that came back. It is now a reference of conditions, derived from the checks rather than from the old prose, so a reader can predict an outcome without running anything. Corrects, among others: the queue Investigate rule (a zero or unset concurrency limit is never saturation), the run-panel Investigate rule (it needs an error block as well as a failed status), the claim that a backed-up queue never offers an investigate chip (the page registry offers both), and the claim that every wake is an LLM call (only attention outcomes and consented investigations are).
…lable checks - 0003 catches the last_read_at backfill up on databases where 0002 already ran. - The per-watch check endpoint records a look, not a check, when it read nothing. - A suggested prompt is consumed once it is sent, not when it is clicked. - Closing the panel settles the launcher dot instead of waiting for the poll. - Say why the oldest-age reader's 50-key page cannot under-report. - Suppress error-classification on the check route, with the reason on the record.
…een, and let an admin preview's watches alert The launcher's work count is now taken with the chat the panel has on screen left out — in the panel's own list and in the poll's server-side count alike — instead of subtracting one afterwards, which under-counted whenever that chat held nothing unseen and left a stale count on the closing edge. The alert gate stopped deciding the admin preview differently from the agent's own gate: `canAccessDashboardAgent` reads `admin` off the user row when the caller has no session, so a watch an admin could create can still alert. A `run_start` watch now offers the same Customize variants as the rest of its family.
…it up navigateIntentApplies documents startedPath: null as 'this tab never saw the turn start', but the path was inferred from the turn going in flight - and a resumed turn goes in flight too, so the tab stamped wherever it happened to be and the rule always matched. Stamp the path where a turn is actually started instead, which leaves it null for a turn this tab only resumed.
004f088 to
e954d82
Compare
| 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.
🔍 Adopting a reserved watch that has already resolved confirms it as still watching
In the converge path, a reserved watch row found under the submission's watchId is adopted whenever it is not cancelled (dashboardAgentWatches.server.ts:842-855). If the first attempt created the watch and it then fired or expired before the retry ran, settleCreated still writes the watching confirmation block — "Watching X until … It reports once, then stops." — for a watch that has already reported.
Narrow (needs a crash between create and outcome-record, plus a fast-resolving condition), and the wake itself would land separately, but the confirmation would read as stale. Checking reserved.status === "active" before adopting, or picking the confirmation wording from the row's status, would close it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // A React handler, not a global hotkey, so Esc stays scoped to the panel. | ||
| onKeyDown={(event) => { | ||
| if (event.key !== "Escape" || event.defaultPrevented) return; | ||
| if ( | ||
| !escapeClosesPanel({ | ||
| key: event.key, | ||
| defaultPrevented: event.defaultPrevented, | ||
| targetInsidePanel: panelRef.current?.contains(event.target as Node) ?? false, | ||
| }) | ||
| ) | ||
| return; | ||
| event.preventDefault(); | ||
| onClose(); | ||
| }} |
There was a problem hiding this comment.
🔍 Escape scoping change alters behaviour for anything portalled inside the panel's React tree
escapeClosesPanel now requires the DOM target to be a descendant of the panel (DashboardAgentPanel.tsx:538-545). This fixes the delete-dialog case, but it is a behavioural change for every current and future portalled layer rendered from inside the panel: tooltips, popovers, selects, and the composer's own overlays. Escape inside any of them no longer closes the panel, even when the layer does not itself handle Escape.
The watch chips' SimpleTooltip (WatchChips.tsx:85-108) and the report footer's popovers are the concrete cases in this diff. If any of those does not call preventDefault and does not close on Escape itself, Escape becomes a no-op there — previously it closed the panel. Worth a manual pass with the panel open.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const [breakdown, oldestQueuedAt] = await Promise.all([ | ||
| engine | ||
| .concurrencyKeyBreakdown(environment, queueName, { limit: OLDEST_AGE_CK_LIMIT }) | ||
| .catch(() => null), | ||
| engine.oldestMessageInQueue(environment, queueName).catch(() => null), | ||
| ]); | ||
|
|
||
| // A partial read would under-report the wait and silently miss the SLA, so either read | ||
| // failing makes the whole reading unavailable rather than a healthy zero. | ||
| if (breakdown === null || oldestQueuedAt === null) return null; | ||
|
|
||
| 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.
🔍 Queue oldest-age reader conflates "empty queue" with "read failed" if the engine returns null
readWatchQueueOldestAge treats oldestQueuedAt === null as an unreadable source and returns null, which the check turns into unavailable (dashboardAgentWatchChecks.server.ts:150-169). The .catch(() => null) makes null the failure sentinel.
That only holds if engine.oldestMessageInQueue never returns null for a legitimately empty queue — the downstream code seems to expect undefined there (typeof oldestQueuedAt === "number" is the only positive branch). If the engine returns null for an empty queue, every queue_oldest_age watch on an idle queue reports unavailable forever instead of pending, and the watch expires with unverified_at_expiry rather than "stayed under the limit". Worth confirming the engine's contract, and using a distinct sentinel (e.g. a wrapped result) rather than null for the failure.
Was this helpful? React with 👍 or 👎 to provide feedback.
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.