[DO NOT MERGE] feat(events): Convert saved event automation triggers in the heartbeat

packages/docs/src/content/docs/concepts/automations.md

@@ -61,7 +61,7 @@ whenever a pull request opens in getsentry/junior, review it
 
 Each plugin decides which events can run this way, and each plugin page lists them. Some events need a filter. For example, GitHub issue, pull request, and release events need a `repo` filter, and Linear issue events need a `teamKey` or `issueIdentifier` filter. Junior names the missing filter when it cannot save the automation. Junior saves filter values in the case that event data uses, so `getsentry/Junior` matches `getsentry/junior`. One event runs the automation once, even when the plugin publishes the event for more than one resource, such as each project of a metric alert. Slack channel-message automations always need their own channel.
 
-When you name one exact resource, Junior saves it as the filters that select the same events. For example, one pull request becomes `repo` and `pullRequest` filters. An automation keeps its exact resource when filters would miss events or match more events, and its details say why. Junior does not save a trigger whose filter can never match its resource. When you update a trigger, `match` changes only the filters that it names: a value sets a filter, `null` removes it, and other filters keep their saved values.
+When you name one exact resource, Junior saves it as the filters that select the same events. For example, one pull request becomes `repo` and `pullRequest` filters. Junior changes older automations the same way, and each change saves a new version that names Junior as the editor. An automation keeps its exact resource when filters would miss events or match more events, and its details say why. Junior does not save a trigger whose filter can never match its resource. When you update a trigger, `match` changes only the filters that it names: a value sets a filter, `null` removes it, and other filters keep their saved values.
 
 A required filter limits how much an automation covers and how noisy it is. It is not access control. Junior does not check that the person who creates the automation can see the repository, project, or team in the filter. An automation with no exact resource posts what it receives to its destination channel. For example, a broad Sentry trigger or GitHub deployment trigger can post private project or repository names in that channel. Create broad automations only in channels where everyone can see those resources.
 

packages/junior-dashboard/src/client/http.ts

@@ -6,18 +6,25 @@ export class DashboardApiError extends Error {
   readonly status: number;
   readonly fields?: Record<string, string[]>;
   readonly code?: string;
+  /** Who made the newer change on a conflict, when the API knows. */
+  readonly changedBy?: string;
   readonly apiError?: string;
 
   constructor(
     path: string,
     status: number,
     apiError?: string,
-    detail?: { fields?: Record<string, string[]>; code?: string },
+    detail?: {
+      fields?: Record<string, string[]>;
+      code?: string;
+      changedBy?: string;
+    },
   ) {
     super(`${path} returned ${status}`);
     this.status = status;
     this.fields = detail?.fields;
     this.code = detail?.code;
+    this.changedBy = detail?.changedBy;
     if (apiError?.trim()) this.apiError = apiError.trim();
   }
 }
@@ -29,14 +36,17 @@ async function throwDashboardApiError(
   let apiError: string | undefined;
   let fields: Record<string, string[]> | undefined;
   let code: string | undefined;
+  let changedBy: string | undefined;
   try {
     const body = (await response.json()) as {
       error?: unknown;
       code?: unknown;
       fields?: unknown;
+      changedBy?: unknown;
     };
     if (typeof body.error === "string") apiError = body.error;
     if (typeof body.code === "string") code = body.code;
+    if (typeof body.changedBy === "string") changedBy = body.changedBy;
     if (body.fields && typeof body.fields === "object") {
       fields = Object.fromEntries(
         Object.entries(body.fields).filter(
@@ -52,6 +62,7 @@ async function throwDashboardApiError(
   throw new DashboardApiError(path, response.status, apiError, {
     fields,
     code,
+    changedBy,
   });
 }
 

packages/junior-dashboard/src/client/pages/automations/AutomationEditor.tsx

@@ -114,6 +114,10 @@ export function AutomationEditor(props: {
   });
   const conflict =
     save.error instanceof DashboardApiError && save.error.code === "conflict";
+  const changedByJunior =
+    conflict &&
+    save.error instanceof DashboardApiError &&
+    save.error.changedBy === "junior";
   const change = (value: Partial<AutomationDraft>) =>
     setDraft((current) => ({ ...current, ...value }));
   const fieldError = (key: string) =>
@@ -171,18 +175,22 @@ export function AutomationEditor(props: {
           <FormNotice
             tone={conflict ? "warning" : "error"}
             title={
-              conflict
-                ? "This automation changed while you were editing."
-                : "Changes could not be saved."
+              changedByJunior
+                ? "Junior changed this trigger while you were editing."
+                : conflict
+                  ? "This automation changed while you were editing."
+                  : "Changes could not be saved."
             }
           >
             <p className="mt-1">
               Your edits are still here.{" "}
-              {conflict
-                ? "Review the latest version before saving again."
-                : save.error instanceof DashboardApiError
-                  ? save.error.apiError
-                  : "Check the fields below, or try again."}
+              {changedByJunior
+                ? "It now uses filters that select the same events. Review the latest version before saving again."
+                : conflict
+                  ? "Review the latest version before saving again."
+                  : save.error instanceof DashboardApiError
+                    ? save.error.apiError
+                    : "Check the fields below, or try again."}
             </p>
             {conflict ? (
               <Button

packages/junior/src/api/automations/routes.ts

@@ -59,7 +59,11 @@ function editErrorResponse(error: unknown): Response {
   if (error instanceof AutomationConflictError) {
     return jsonResponse(
       automationEditErrorSchema,
-      { error: error.message, code: "conflict" },
+      {
+        error: error.message,
+        code: "conflict",
+        ...(error.changedBy ? { changedBy: error.changedBy } : undefined),
+      },
       { status: 409 },
     );
   }

packages/junior/src/api/schema/automation.ts

@@ -410,6 +410,8 @@ export const automationEditErrorSchema = z
     error: z.string(),
     code: z.enum(["invalid_edit", "conflict", "not_found"]),
     fields: z.record(z.string(), z.array(z.string())).optional(),
+    /** Set on a conflict when Junior made the only newer change. */
+    changedBy: z.literal("junior").optional(),
   })
   .strict();
 

packages/junior/src/chat/agent-dispatch/heartbeat.ts

@@ -1,3 +1,6 @@
+import { getDb } from "@/chat/db";
+import { convertSavedEventAutomationTriggers } from "@/chat/event-automations/trigger-conversion";
+import { getEventCatalog } from "@/chat/events/runtime-catalog";
 import { runTimerWatchHeartbeat } from "@/chat/events/timers";
 import { getPlugins } from "@/chat/plugins/agent-hooks";
 import { logException, logInfo } from "@/chat/logging";
@@ -215,6 +218,13 @@ export async function runHeartbeat(args: {
   } catch (error) {
     logException(error, "scheduled_automations.heartbeat.failed");
   }
+  try {
+    // Saved triggers with an identifier move to filters. Slice D of #2362
+    // removes this step with the identifier column.
+    await convertSavedEventAutomationTriggers(getDb(), getEventCatalog());
+  } catch (error) {
+    logException(error, "event_automation.trigger.conversion_failed");
+  }
   try {
     await runTimerWatchHeartbeat({ nowMs: args.nowMs, queue });
   } catch (error) {

packages/junior/src/chat/automations/edit.ts

@@ -10,6 +10,7 @@ import type { AutomationEdit, AutomationUpdate } from "@/api/schema/automation";
 import { getDb } from "@/chat/db";
 import { resolveTaskOutcomes } from "@/chat/task-outcomes";
 import {
+  getEventAutomation,
   saveEventAutomation,
   setEventAutomationStatus,
   type StoredEventAutomation,
@@ -24,14 +25,19 @@ import {
   type ScheduleIntent,
 } from "@/chat/scheduled-automations/schedule-intent";
 import type { ScheduledAutomation } from "@/chat/scheduled-automations/types";
-import { automationRevision, requireAutomationRevision } from "./revision";
+import {
+  AutomationConflictError,
+  automationRevision,
+  requireAutomationRevision,
+} from "./revision";
 import {
   resolveViewerTaskCandidate,
   ViewerTaskNotFoundError,
   type TaskCandidate,
 } from "./read";
 import {
   automationDefinition,
+  listAutomationVersions,
   readAutomationVersion,
   sameDefinitionValue,
 } from "./versions";
@@ -130,6 +136,26 @@ export async function readViewerAutomationEdit(
   return editView(automation, automation.ownedByViewer);
 }
 
+/**
+ * Return whether the only change after `revision` is Junior's trigger change:
+ * the newest version is Junior's, and the event automation with the trigger
+ * of the version before it has that revision.
+ */
+async function changedOnlyByJunior(
+  id: string,
+  revision: string,
+): Promise<boolean> {
+  const db = getDb();
+  const task = await getEventAutomation(db, id);
+  const [latest, previous] = await listAutomationVersions(db, id, 2);
+  if (!task || latest?.editedBy !== "junior" || previous?.kind !== "event")
+    return false;
+  return (
+    automationRevision({ ...task, trigger: previous.definition.trigger }) ===
+    revision
+  );
+}
+
 /** Save a partial edit against the read revision, without dispatching work. */
 export async function updateViewerAutomation(
   user: User,
@@ -139,8 +165,19 @@ export async function updateViewerAutomation(
   const current = await requireEditableAutomation(user, id);
   if (input.kind !== current.kind)
     throw new AutomationEditError("The trigger type cannot change.", "kind");
-  requireAutomationRevision(current.task, input.revision);
-  return saveViewerEdit(user, current, input);
+  try {
+    requireAutomationRevision(current.task, input.revision);
+    return await saveViewerEdit(user, current, input);
+  } catch (error) {
+    if (
+      error instanceof AutomationConflictError &&
+      input.kind === "event" &&
+      (await changedOnlyByJunior(id, input.revision))
+    ) {
+      throw new AutomationConflictError("junior");
+    }
+    throw error;
+  }
 }
 
 /** `restored` carries version values that edit input cannot express, such as a cleared title. */

packages/junior/src/chat/automations/revision.ts

@@ -4,12 +4,21 @@ import { ToolInputError } from "@/chat/tools/execution/tool-input-error";
 import type { EventAutomation } from "@/chat/event-automations/types";
 import type { ScheduledAutomation } from "@/chat/scheduled-automations/types";
 
+/**
+ * A stale edit. `changedBy` is `"junior"` when the only newer change is
+ * Junior's own trigger change, so the editor can say who changed it.
+ */
 export class AutomationConflictError extends ToolInputError {
-  constructor() {
+  readonly changedBy?: "junior";
+
+  constructor(changedBy?: "junior") {
     super(
-      "This Automation changed after it was loaded. Reload it before saving.",
+      changedBy === "junior"
+        ? "Junior changed this Automation's trigger to filters that select the same events after it was loaded. Reload it before saving."
+        : "This Automation changed after it was loaded. Reload it before saving.",
     );
     this.name = "AutomationConflictError";
+    if (changedBy) this.changedBy = changedBy;
   }
 }
 

packages/junior/src/chat/automations/versions.ts

@@ -13,6 +13,11 @@ import { canonicalJson } from "./revision";
 
 type VersionedAutomation = ScheduledAutomation | EventAutomation;
 
+/** The person who saves a version, or `"junior"` when Junior saves it. */
+export type AutomationVersionEditor = NonNullable<
+  AutomationVersion["editedBy"]
+>;
+
 /** Return the versioned part of an Automation. Lifecycle and run state are excluded. */
 export function automationDefinition(task: VersionedAutomation) {
   const common = {
@@ -43,7 +48,7 @@ export async function recordAutomationVersion(
   db: JuniorDatabase,
   task: VersionedAutomation,
   current: VersionedAutomation | undefined,
-  editedBy?: EventAutomation["createdBy"],
+  editedBy?: AutomationVersionEditor,
 ): Promise<void> {
   const definition = automationDefinition(task);
   if (current && sameDefinitionValue(automationDefinition(current), definition))

packages/junior/src/chat/event-automations/README.md

@@ -68,12 +68,18 @@ After a conversion the scope lives in `match`, so replacing the whole object
 would make the trigger broader. The dashboard sends the whole trigger from
 its form, which shows every saved filter.
 
-Saved rows keep their identifiers until a later change converts them. Saved
-versions keep their old triggers, and the version schema still reads
-`identifier` and `resourceType`. The version schema also reads
-`editedBy: "junior"` for a version that Junior saves when it changes a trigger
-itself. Nothing writes it yet. The reader comes first, so a release that
-writes it can roll back to this one and keep version history readable.
+Each heartbeat checks at most 50 saved rows that have an identifier, no
+saved reason, and the same identifier in the row and the trigger. It saves
+each row with the revision that it read, under the row lock, and records a
+version with `editedBy: "junior"`. A row that changed after the read is left
+for the next heartbeat. A row that cannot convert keeps its identifier, stays
+active, and saves the reason, so later heartbeats skip it. A dashboard edit
+that read the row before the conversion fails as a conflict, and the dashboard
+says that Junior changed the trigger. The edit API checks this by giving the
+saved row the trigger of the version before Junior's and comparing its
+revision. Slice D of #2362 decides what happens to kept rows before the
+identifier column goes away. Saved versions keep their old triggers, and the
+version schema still reads `identifier` and `resourceType`.
 
 `searchEventTypes` exposes the same enabled plugin catalog used by
 watches and event automations. Create and update tools accept only

packages/junior/src/chat/event-automations/store.ts

@@ -1,13 +1,27 @@
 import { requireAutomationRevision } from "@/chat/automations/revision";
-import { recordAutomationVersion } from "@/chat/automations/versions";
+import {
+  recordAutomationVersion,
+  type AutomationVersionEditor,
+} from "@/chat/automations/versions";
 import {
   eventMatches,
   type Event,
   type EventData,
   type EventMatch,
   type User,
 } from "@sentry/junior-plugin-api";
-import { and, asc, desc, eq, inArray, isNull, ne, or, sql } from "drizzle-orm";
+import {
+  and,
+  asc,
+  desc,
+  eq,
+  inArray,
+  isNotNull,
+  isNull,
+  ne,
+  or,
+  sql,
+} from "drizzle-orm";
 import type { JuniorDatabase } from "@/db/db";
 import { juniorAutomations } from "@/db/schema/automations";
 import {
@@ -200,13 +214,14 @@ export async function createEventAutomation(
 
 /**
  * Replace an existing non-deleted event automation under a row lock.
- * Pass `editedBy` when a person changes the definition so its version names them.
+ * Pass `editedBy` when a person or Junior changes the definition so its
+ * version names them.
  */
 export async function saveEventAutomation(
   db: JuniorDatabase,
   task: EventAutomation,
   expectedRevision?: string,
-  editedBy?: EventAutomation["createdBy"],
+  editedBy?: AutomationVersionEditor,
 ): Promise<StoredEventAutomation | undefined> {
   const columns = eventAutomationColumns(task);
   return db.transaction(async (tx) => {
@@ -284,6 +299,38 @@ export async function listDeletedEventAutomationsCreatedBy(
   return rows.map(parseEventAutomationRow);
 }
 
+/**
+ * List up to `limit` live event automations whose trigger still has an
+ * identifier and no saved reason to keep it. Skip a row whose identifier
+ * column and saved trigger disagree, because ingest never runs it and a save
+ * could make it broad. Skip a row that does not parse; ingest logs it when an
+ * event reaches it.
+ */
+export async function listEventAutomationsToConvert(
+  db: JuniorDatabase,
+  limit: number,
+): Promise<StoredEventAutomation[]> {
+  const rows = await db
+    .select()
+    .from(juniorAutomations)
+    .where(
+      and(
+        activeEventAutomationWhere(),
+        isNotNull(juniorAutomations.identifier),
+        sql`${juniorAutomations.record}->'trigger'->>'identifier' = ${juniorAutomations.identifier}`,
+        sql`${juniorAutomations.record}->'trigger'->'identifierReason' is null`,
+      ),
+    )
+    .orderBy(asc(juniorAutomations.createdAtMs), asc(juniorAutomations.id))
+    .limit(limit);
+  return rows.flatMap((row) => {
+    const parsed = eventAutomationSchema.safeParse(
+      eventAutomationRowInput(row),
+    );
+    return parsed.success ? [withRowFields(row, parsed.data)] : [];
+  });
+}
+
 /**
  * Select live rows whose identifier is one of these, and rows with no
  * identifier. A row with no identifier runs for every resource of its