Repository navigation
feat(events): Reject event filters that cannot match - #2381
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
7 screenshot changes — 7 changed · 0 added · 0 removed Review screenshots in Frameshift |
This was referenced Oct 29, 2026
sentry-junior
Bot
force-pushed
the
feat/reject-unmatchable-filters
branch
from
October 29, 2026 19:48
e5a05da to
f3a9979
Compare
dcramer
added a commit
that referenced
this pull request
Oct 29, 2026
**Not a breaking change.** Users, plugins, saved automations, and the dashboard API see the same behavior as before. This PR changes how core decides two existing rules, and it adds docs. ## What this changes Core has its own event resource types: Workspace snapshots, timers, and Slack channel messages. Two rules apply to them: - Event automations cannot use Workspace snapshot or timer events. - `watchEvents` cannot watch timers (use `watchTimer`) or Slack channel messages (use an event automation). Before, core hard-coded these rules as checks on the namespace strings `"junior"`, `"timer"`, and `"slack"`. Now each core registration declares its rules in a `limits` map, keyed by resource type (`EventResourceTypeLimits` in `chat/events/catalog.ts`): - `automation: false` for `workspace_snapshot` and `timer`. - `watch: false` for `timer` and Slack `channel`. Each one also sets `watchUnavailableMessage`, which tells the agent what to use instead. The messages are the same as before. The limits sit beside the resource types, not on them. So the resource type objects stay plain plugin-api `PluginEventType` values. ## Why it is not breaking - **Plugin API:** no change. `limits` is on a core-only registration type. `@sentry/junior-plugin-api` does not change, and plugins cannot set limits. - **Same results:** the new checks accept and reject the same inputs as the old ones. All `junior` resource types set `automation: false`, so that namespace is still left out of automations. `slack` has only `channel`, so the watch rejection covers the same inputs. - **Saved automations:** availability does not change. Every place that checks it reads the same filtered catalog. - **Dashboard API:** `/event-catalog` returns the same shape. It builds each entry from the plugin-api resource type only, so it does not need to remove core fields. A new core limit cannot break the strict response schema. - **Public exports:** none of the changed code is exported from `@sentry/junior`. ## Docs - **`deployment.*` events:** the GitHub and Vercel READMEs and the public plugin pages now say what each event means. Vercel `deployment.error` is the same as GitHub `deployment.failed`, not GitHub `deployment.error`. - **`chat/events/README.md`:** now covers `eventKey`, `occurredAtMs`, `objectType`, the reserved `junior` and `slack` namespaces, the `limits` map, and short steps to add an event type. Part of #2362, item 5. An earlier commit added regex identifier patterns for Linear and Sentry, and a later commit removes them. #2362 item 6 is replacing regex patterns with structured identifier parts. ## Merge order with #2381 #2381 deletes `pluginSupportsEvent`, and this PR still uses it. Whichever PR merges second must rebase onto the other and resolve that conflict. ## Checks - `pnpm typecheck` and `pnpm lint` pass. - The event catalog, watch, search, snapshot, Slack channel automation, heartbeat timer, Workspace tool, and automation edit tests pass. The automation edit test covers `/api/automations/event-catalog`. - One test, "delivers the current automation card and retains its reference in web history" in `event-automations.test.ts`, also fails on `main` without this change. <!-- junior-request-attribution:start --> via **David Cramer**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A43a3a6dbf6e402edc2e89348be3f2e04 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent%3A43a3a6dbf6e402edc2e89348be3f2e04) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A43a3a6dbf6e402edc2e89348be3f2e04/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: David Cramer <david@sentry.io>
dcramer
marked this pull request as ready for review
October 29, 2026 19:51
A trigger could save a filter that never matches. The filter could name a field that no selected event sets, or a field that a plugin removed. For example, a GitHub issue.closed trigger with a senderId filter saved and then never ran. Match fields now take an optional `events` list: the events that set the field. `eventTriggerProblems` in plugin-api is the one check for a trigger's events and filters. Core uses it when it saves an event automation (tools and dashboard API) or a Watch, and rejects the trigger with every problem in plain words. The dashboard editor uses the same check instead of its own copy. It hides filters that a selected event does not set, and it lists the problems of a saved trigger that no longer fits the catalog. GitHub declares the events for its filters: issue comment filters are set only on issue.comment.created, deployment filters only on deployment events, and pull request filters only on pull request events. The pull request comment webhook has no head branch. Linear, Sentry, and Vercel need no declarations. Refs #2362
Address design review feedback on the unmatchable-filter check. - Merge repository match fields with mergeEventMatchFields, which unions the events lists and rejects conflicting copies of one field. - Test GitHub filter declarations against the webhook normalizers and the lazy pull request lookups, instead of against themselves. - Report a saved trigger as unavailable when eventTriggerProblems finds a problem, and list the problems in Slack details, tool results, and the dashboard. The list SQL uses the same rule. - Move the unknown resource type check into the shared plugin-api check. - Say "filter" in agent, Slack, and dashboard text. Agent errors name the match.<key> input and the plugin display name. Refs #2362 Co-Authored-By: David Cramer <david@sentry.io> Co-Authored-By: David Cramer <david@sentry.io>
This was referenced Oct 29, 2026
sentry-junior
Bot
force-pushed
the
feat/reject-unmatchable-filters
branch
from
October 29, 2026 20:03
f3a9979 to
7d3ed7e
Compare
dcramer
approved these changes
Oct 29, 2026
dcramer
added a commit
that referenced
this pull request
Oct 29, 2026
…2385) Stacked on #2381. Item 4 of #2362: how event triggers show in the dashboard and Slack. ## What changed There is now **one shared trigger description** in `packages/junior-plugin-api/src/event-triggers.ts`: ```ts interface EventTriggerDescription { events: string[]; // "pull request opened" filters: string[]; // "authors includes alice", "base is main or release" problems: string[]; // why the saved trigger cannot work } ``` - `describeEventTrigger(resourceTypes, selection)` builds it. It takes the resource types of the source, like `eventTriggerProblems`. - `problems` are the messages of #2381's `eventTriggerProblems`. So every surface shows the same words as the save check. This includes the unknown resource type and "The plugin for this source is not enabled." - A `list: true` field reads "includes X". List values on a normal field read "is X or Y". - `eventTriggerText(resource, description)` writes one line for cards and lists: `Pull request · pull request opened · filters: isDraft is false`. - Core wraps this as `describeRegisteredEventTrigger(catalog, selection)` in `chat/events/catalog.ts`. `describeEventAutomationTrigger(trigger, catalog)` in `event-automations/tool-support.ts` applies the automation catalog (`automation: false` resource types from #2379 are left out). ## API (hard cutover) - The event automation summary now returns `trigger: EventTriggerDescription`. It no longer returns raw `events`, `match`, `triggerAvailable`, or `triggerProblems`. - **`triggerAvailable` is removed** from the summary and the edit response. A trigger is unavailable when `trigger.problems` is not empty. No consumer of the edit response used it. - `eventAutomationTriggerProblems` and `eventAutomationTriggerAvailable` are replaced by `describeEventAutomationTrigger`. - The agent tool result keeps its own `trigger.available` and `trigger.problems`. It now gets them from the same description. ## Surfaces All of these use the shared description now, and their old formatting code is removed: - **Dashboard list row**: one line from `eventTriggerText`. Unavailable comes from `trigger.problems`. - **Dashboard details and the read-only view**: event names plus a new `TriggerFilters` list ("Every filter must match:", or "No filters. Runs on every selected event."). The read-only view no longer shows a JSON dump. - **Dashboard status**: lists the saved trigger's problems with "Edit the trigger to fix it." - **Dashboard editor**: event checkboxes and list values use the shared words. Agent guidance under events is removed. The stale-trigger notice shows the saved trigger in words, not JSON. The text says "filter" everywhere. - **Dashboard version history**: each Event version shows its events and filters with `TriggerFilters`, not raw JSON. - **Slack card** (`eventAutomationToolResult`): `eventTriggerText` replaces the raw `key = "json"` strings. Status and warning come from `problems`. - **Slack details** (`automation-details.ts`): a new **Filters** field. Status and **Needs attention** come from `trigger.problems`. Identifier display is out of scope. Structured identifier parts are #2362 item 6. ## Version history Each Event version in the version list API now also has `trigger: EventTriggerDescription`. The server builds it with the enabled plugins. The saved `definition` stays raw, so "make active" still posts it back unchanged. The new `listedAutomationVersionSchema` holds one list item. The stored `automationVersionSchema` is unchanged. ## Verification Rebased on #2381 at `7d3ed7ed9` (after #2379 merged). CI runs only for PRs into `main`, so these checks ran locally. - `pnpm typecheck` and `pnpm lint` pass. - Vitest: `unit/events/*`, `integration/api/automations`, `integration/api/automation-edits`, `integration/slack/automation-details-webhook`, and the dashboard suite (320 tests) pass. - `unit/events/match.test.ts` has representative formatter cases. - `integration/api/automation-edits` checks the version list descriptions: one clean trigger and one with a problem. - `integration/event-automations.test.ts`: 14 of 15 pass. "delivers the current automation card" fails on the generated title in my sandbox. It fails the same way on the bare #2381 branch, so this change does not cause it. Its trigger-line assertion passes. - Dashboard e2e: `automations.spec.ts` and `automation-editor.spec.ts` pass (9 of 9). The unavailable-trigger scenario checks the problem text. <!-- junior-request-attribution:start --> via **David Cramer**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A05172b2fc7efb1362b221426bec7e50b --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent%3A05172b2fc7efb1362b221426bec7e50b) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A05172b2fc7efb1362b221426bec7e50b/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: David Cramer <david@sentry.io>
sentry-junior Bot
added a commit
that referenced
this pull request
Oct 29, 2026
searchEventTypes kept its own copy of the match field schema. That copy did not allow the events list that #2381 added, so the tool failed for any plugin with event-specific filter fields, such as GitHub. Use the plugin contract schema instead, so the agent sees which events each filter matches. Refs #2362 Co-Authored-By: David Cramer <david@sentry.io>
dcramer
added a commit
that referenced
this pull request
Oct 29, 2026
Refs #2362. This fixes a bug that #2381 added to `main`. ## Root cause #2381 added an optional `events` list to filter fields (`eventMatchFieldSchema` in `@sentry/junior-plugin-api`). `packages/junior/src/chat/tools/search-event-types.ts` kept its own strict copy of that schema for its output, and the copy did not get `events`. GitHub now declares `events` on fields such as `environment`, `workflowId`, and `workflowPath`. So every `searchEventTypes` call that lists GitHub fails with `ZodError: Unrecognized key: "events"`. ## Effect - #2383 fails its integration and eval checks. The integration eval "Event automation management > when asked what events are available, search without creating anything" fails with this error. The behavioral eval logs show the same error many times. - #2383 adds more filter fields with `events` (Sentry `level`, `priority`, `issueCategory`, `alertRuleId`), so it cannot pass until this fix is on `main`. ## Fix - The tool output now uses `eventMatchFieldSchema` from `@sentry/junior-plugin-api`, not a copy. A new key in the plugin contract can no longer break this tool. - The agent now also sees each filter's `events` list. The agent needs it to choose events that a filter can match, because #2381 rejects triggers that cannot match. ## Tests - The existing `searches enabled event types` test in `tests/unit/tools/events.test.ts` now gives the GitHub resource type a filter field with an `events` list, and expects that field in the result. Without the fix, the test fails with the same `ZodError` as CI. - `tests/unit/tools` and `tests/unit/events`: 109 passed. - `@sentry/junior` typecheck and `pnpm lint` passed. <!-- junior-request-attribution:start --> via **David Cramer**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A7dce642a33dbc3d879f220267a40f04a --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent%3A7dce642a33dbc3d879f220267a40f04a) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A7dce642a33dbc3d879f220267a40f04a/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: David Cramer <david@sentry.io>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 2 of #2362 ("Reject filters that cannot match"). Refs #2362. This PR does not close the issue.
What changes
Before this change, you could save a filter on a field that a selected event never sets, such as a pull request field on a
deployment.*event. The trigger then never matched, and nothing told you why.eventsthat set the field.eventTriggerProblems(packages/junior-plugin-api/src/event-triggers.ts) is the one check for a selection of resource type, events, and filters. It also reports an unknown resource type, so core and the dashboard use the same function.mergeEventMatchFieldsmerges the match fields of resource types that share a parent, such as a repository. It unions theeventslists and throws when two copies of one field have a different kind, enum, or list.watchEventsrun the check (requireValidEventTriggerinchat/events/catalog.ts) before they save. It replacespluginSupportsEventandrequireSupportedEventMatch, which this PR deletes. Agent errors name the filter and its input key, such asFilter "base" (match.base), and use the plugin display name, such as "This GitHub pull request trigger cannot be saved." Watch errors say "watch cannot be created".automationEventCatalog, so resource types withautomation: false(Workspace snapshots, timers) stay out of automations and report unavailable when saved.watchEventsstill checkseventWatchUnavailablefirst, sowatch: falseresource types (timers, Slack channels) keep theirwatchUnavailableMessagetext.authorUsernamefilter, now reports unavailable. The problems go to the dashboard (triggerProblems), the Slack details "Needs attention" field, and the event automation tool results (trigger.problems). The SQL rule for the dashboard list ("Unavailable" filter and attention count) matcheseventTriggerProblems.mergeEventMatchFields, so a shared key such assenderIdkeeps every event that sets it. A new test runs full webhook deliveries for every repository event (deployments, issues, issue and pull request comments, pull request lifecycle, reviews, review comments, and check suites) throughnormalizeGitHubEventsand the lazy lookups inwebhooks/pull-request-match-data.ts. It fails when an event sets a filter that itseventslist omits, or when a list names an event that never sets it. Linear, Sentry, Vercel, and Slack fields need no changes.Decisions
Checks
The branch is rebased on
mainafter #2379. Two conflicts, both resolved:chat/events/catalog.ts: the registration type keeps bothdisplayName(this PR) and the core-onlylimitsmap (ref(events): Declare core event limits per resource type #2379).chat/event-automations/tool-support.ts:eventAutomationTriggerAvailableuses the shared problem check, which already runs againstautomationEventCatalog.No
pluginSupportsEventorrequireSupportedEventMatchcallers remain.pnpm lint: passedjunior,junior-plugin-api,junior-github,junior-dashboard,junior-linear,junior-verceljunior-githubtests: 254 passed, with the new declaration testjuniorunit tests: 1580 passedjuniorintegration tests for automations, watches, events, annotation cards, and Slack details (10 files): 62 passed, 1 failed. The failure isevent-automations"delivers the current automation card". It fails the same way onmainin this sandbox, so this branch does not cause it.automation-editssaves a renamed GitHub filter directly. It checks that the summary reportstriggerAvailable: falsewith the problem, and that the "Unavailable" list returns the Automation.automations.spec.ts): 5 passed--
View Junior Session [Sentry]