fix(runner): give Tier-2 compile diagnostics a constant title and one bucket (DEV-2854)

runner/apps/authoring/src/sentry.ts

@@ -17,6 +17,7 @@ import {
 import { ApiError } from "./apiError.js";
 import { resolveReporting } from "./reportingGate.js";
 import { isEdgelessForeignSessionStart, isOfficeScannerRejection } from "./eventGate.js";
+import { tier2StderrReport } from "./tier2Report.js";
 
 const DSN = import.meta.env.VITE_SENTRY_DSN as string | undefined;
 
@@ -270,20 +271,37 @@ export function reportDemoEvent(payload: MonitorPayload, context: DemoEventConte
     return;
   }
   if (!demoRelayBudget.admit(clean.kind, message, clean.stack)) return;
+  // DEV-2854: a recognised Tier-2 compiler diagnostic collapses across TS codes into one
+  // flat, constant-titled bucket instead of the per-message fingerprint below. Never fed
+  // into `demoRelayBudget.admit` above — that stays keyed on the raw message, so 20
+  // distinct diagnostics in one bad editing session still consume 20 of
+  // `MONITOR_EVENT_CEILING` rather than collapsing and losing their `extra` after the
+  // first. See `tier2Report.ts` for why.
+  const tier2 = tier2StderrReport(clean.kind, message);
   const tags: Record<string, string> = {
     surface: DEMO_SURFACE,
     kind: clean.kind,
     tier: String(context.tier),
     framework: context.framework,
+    ...(tier2 ? tier2.tags : {}),
   };
   if (context.demoId) tags.demo_id = context.demoId;
   const captureContext = {
     tags,
-    fingerprint: [DEMO_SURFACE, clean.kind, normalizeMonitorMessage(message)],
+    fingerprint: tier2
+      ? tier2.fingerprint
+      : [DEMO_SURFACE, clean.kind, normalizeMonitorMessage(message)],
     level: (clean.kind === "error" || clean.kind === "rejection" ? "error" : "warning") as
       | "error"
       | "warning",
-    ...(clean.url ? { extra: { url: clean.url } } : {}),
+    ...(clean.url || tier2
+      ? {
+          extra: {
+            ...(clean.url ? { url: clean.url } : {}),
+            ...(tier2 ? tier2.extra : {}),
+          },
+        }
+      : {}),
   };
 
   // An exception (with the preview's own stack) for a throw; a message for the
@@ -308,7 +326,11 @@ export function reportDemoEvent(payload: MonitorPayload, context: DemoEventConte
   // MONITOR_URL_MAX and host-redacted by `sanitizeMonitorPayload`, and because it never
   // enters the fingerprint, a crafted payload posting a thousand distinct urls still
   // produces one issue, titled with whichever arrived first.
-  const display = clean.kind === "network" && clean.url ? `${message}: ${clean.url}` : message;
+  const display = tier2
+    ? tier2.display
+    : clean.kind === "network" && clean.url
+      ? `${message}: ${clean.url}`
+      : message;
   Sentry.captureMessage(display, captureContext);
 }
 

runner/apps/authoring/src/tier2Report.ts

@@ -0,0 +1,108 @@
+/**
+ * What a Tier-2 compiler-diagnostic `stderr` line becomes in Sentry (DEV-2854, Sentry
+ * DEMOS-3K/3M/4F/3H and friends).
+ *
+ * Split out of `sentry.ts` for the same reason as `tier1Report.ts` and `reportingGate.ts`:
+ * that file pulls `@sentry/react` and reads `import.meta.env`, so `node --test` cannot
+ * import it and nothing in it can be pinned by a unit test. Keep this module import-free —
+ * the grouping rule is the whole of what it decides, and `pipeline/tier2-report.test.mjs`
+ * imports it as source.
+ *
+ * DEV-2854 was filed against the wrong site and asked for a change that was already made:
+ *
+ *  - These events are `Sentry.captureMessage(...)` from `reportDemoEvent` below (reached
+ *    via `ContainerRuntime.onStderr`), tagged `kind: "stderr"`, `level: "warning"`, Sentry
+ *    `Type: default` — NOT `App.tsx`'s `Sentry.captureException(e, { tags: { context:
+ *    "tier2-runtime" } })`. That catch-all's own issues are all `TypeError: Failed to
+ *    fetch` and have never once carried a compiler diagnostic; this module has nothing to
+ *    do with it and must not be wired there.
+ *  - `normalizeMonitorMessage`'s quoted-string rule (used in the fingerprint at
+ *    `sentry.ts:263` for every other `kind`) already collapses messages within one TS
+ *    diagnostic code — DEMOS-3M holds six distinct quoted identifiers in one issue. Keying
+ *    a new fingerprint on the code would be a no-op on top of that.
+ *
+ * What is actually broken, and what this module fixes:
+ *
+ *  1. **Title flap.** A fingerprint coarser than the message, with the raw message still in
+ *     the title, means the issue title names whichever sample arrived last (the same
+ *     defect `tier1Report.ts` documents for `COMPILE_TITLE`). DEMOS-3K's title says
+ *     `',' expected` while its newest event says `')' expected`.
+ *  2. **Cross-code spread.** One bad Angular-editing session mints 20+ distinct TS codes,
+ *     each its own single-event issue, because nothing groups across codes.
+ *
+ * The fix is a flat fingerprint across every recognised diagnostic, a constant title, and
+ * the code preserved as a facet (`ts_code`) rather than folded into either. That is the
+ * **invariant to hold: constant title iff flat fingerprint** — a title that varies with the
+ * code on a fingerprint that does not would flap exactly like today's does, which is why
+ * there is no `"Tier-2 compile failed (TS1005)"` middle option here.
+ *
+ * The recogniser below is an **allowlist on purpose**: it matches a TS diagnostic code in
+ * diagnostic position and nothing else. NG codes (`NG8001`, `NG8002` — plausibly our own
+ * Angular starter's `HotTableModule` wiring), `Failure reason:`, `::…::` install-failure
+ * markers, vite/vue internal errors, and `Could not resolve` are all untouched **by
+ * construction** — they keep reporting through the unchanged bare-message path in
+ * `sentry.ts`, so nothing that should stay loud goes quiet by falling through a denylist
+ * gap. `pipeline/tier2-report.test.mjs` pins every one of those as a guard against a future
+ * rewrite that swaps this allowlist for a denylist.
+ */
+
+/** The constant title for every recognised Tier-2 compiler diagnostic, for all time. The
+ *  raw line it replaces rides in `extra.compileDiagnostic` instead, which takes no part in
+ *  grouping or titling. */
+const TIER2_COMPILE_TITLE = "Tier-2 compile failed";
+
+/** A TS diagnostic code in diagnostic position — the code immediately followed by a colon
+ *  and a space, as esbuild/tsc emit it (`TS1005: ',' expected.`). Requires a word boundary
+ *  before `TS` and the trailing `: ` so a bare `TS1005` mentioned in prose, with no code
+ *  frame around it, does not match. */
+const TS_CODE_IN_DIAGNOSTIC_POSITION = /\bTS\d{4,5}:\s/;
+
+/** Every code found in diagnostic position, for the multi-code check below. Kept as its own
+ *  literal (not derived from `TS_CODE_IN_DIAGNOSTIC_POSITION` via `.source`, which would
+ *  need its own `g` flag stitched on) but written to the identical shape by hand — the two
+ *  must agree on what counts as "a code", so change them together. */
+const TS_CODE_GLOBAL = /\bTS\d{4,5}(?=:\s)/g;
+
+export interface Tier2StderrReport {
+  fingerprint: string[];
+  tags: Record<string, string>;
+  extra: Record<string, string>;
+  display: string;
+}
+
+/**
+ * Decide how a Tier-2 `stderr` line is reported, or that it is not.
+ *
+ * `null` for anything other than `kind === "stderr"` with a recognised TS diagnostic code:
+ * the caller keeps today's per-message fingerprint and title unchanged. Not reclassified,
+ * not merged, no synthetic title — an unrecognised line needs nothing extra to keep working,
+ * since `normalizeMonitorMessage`'s quoted-string rule already groups the no-code case
+ * (DEMOS-3H's `Unexpected "}"` / `Unexpected ","`).
+ */
+export function tier2StderrReport(kind: string, message: string): Tier2StderrReport | null {
+  if (kind !== "stderr") return null;
+  if (!TS_CODE_IN_DIAGNOSTIC_POSITION.test(message)) return null;
+
+  const codes = new Set<string>();
+  for (const match of message.matchAll(TS_CODE_GLOBAL)) codes.add(match[0]);
+  // First match wins for the tag; a line naming more than one distinct code (a repeated,
+  // truncated diagnostic block) omits the tag rather than pick arbitrarily. The
+  // fingerprint below is unaffected either way — it never carries a code.
+  const singleCode = codes.size === 1 ? [...codes][0] : undefined;
+
+  return {
+    // Flat, never keyed on the code: per-code keying leaves ~20 issues per bad editing
+    // session and is unbounded in the TS vocabulary. `framework` / `tier` deliberately
+    // stay out too — they are already tags in `sentry.ts`, and the house rule there is
+    // that instrumentation facets go beside the fingerprint, never inside it.
+    fingerprint: ["demo-runtime", "stderr", "tier2-compile"],
+    tags: {
+      kind_class: "tier2-compile",
+      ...(singleCode ? { ts_code: singleCode } : {}),
+    },
+    // The raw line, verbatim and bounded/host-redacted upstream by `sanitizeMonitorPayload`
+    // — never in `display` or the fingerprint, which is what keeps the title constant.
+    extra: { compileDiagnostic: message },
+    display: TIER2_COMPILE_TITLE,
+  };
+}

runner/pipeline/tier2-report.test.mjs

@@ -0,0 +1,115 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { tier2StderrReport } from "../apps/authoring/src/tier2Report.ts";
+
+// DEV-2854 / Sentry DEMOS-3K, DEMOS-3M, DEMOS-4F, DEMOS-3H and friends. Tier-2 compiler
+// diagnostics were fingerprinted per (already-collapsed-per-code) message, so a bad
+// Angular-editing session opened one issue per TS diagnostic code and each issue's title
+// named whichever sample arrived last. These are the grouping rules that flatten the
+// bucket across codes and keep the title constant.
+
+const TS1005 = `✘ [ERROR] TS1005: ',' expected. [plugin angular-compiler]`;
+const TS1005B = `✘ [ERROR] TS1005: ')' expected. [plugin angular-compiler]`;
+const TS2304 = `✘ [ERROR] TS2304: Cannot find name '$B$4'. [plugin angular-compiler]`;
+const TS7006 = `✘ [ERROR] TS7006: Parameter 'amount' implicitly has an 'any' type. [plugin angular-compiler]`;
+const TS1109 = `✘ [ERROR] TS1109: Expression expected. [plugin angular-compiler]`;
+const NOCODE = `✘ [ERROR] Unexpected "}"`;
+const NG8001 = `✘ [ERROR] NG8001: 'hot-table' is not a known element:`;
+const NG8002 = `✘ [ERROR] NG8002: Can't bind to 'settings' since it isn't a known property of 'hot-table'.`;
+const FAILURE = `Failure reason:`;
+const RESOLVE = `✘ [ERROR] Could not resolve "./app/App"`;
+// A truncated, repeated diagnostic block — the DEMOS-52 shape. Two occurrences of the
+// *same* code, so this is not the "more than one distinct code" case, but the raw text
+// carries the code twice, which the multi-code-detection logic must not mistake for two
+// distinct codes.
+const TS2345_REPEATED =
+  `✘ [ERROR] TS2345: Argument of type 'string' is not assignable to parameter of type 'number'. […] ` +
+  `✘ [ERROR] TS2345: Argument of type 'string' is not assignable to parameter of type 'number'. […]`;
+// A line naming two genuinely distinct codes.
+const TS_MULTI_CODE = `${TS1005} ${TS2304}`;
+
+// --- Load-bearing: demonstrably false on master, true after ---------------------------
+
+test("TS1005 and TS2304 share one fingerprint (cross-code-spread defect)", () => {
+  const a = tier2StderrReport("stderr", TS1005);
+  const b = tier2StderrReport("stderr", TS2304);
+  assert.deepEqual(a.fingerprint, b.fingerprint);
+  assert.deepEqual(a.fingerprint, ["demo-runtime", "stderr", "tier2-compile"]);
+});
+
+test("the display string is the same constant across TS1005 / TS2304 / TS7006 / TS1109 (title-flap defect)", () => {
+  const displays = [TS1005, TS2304, TS7006, TS1109].map(
+    (message) => tier2StderrReport("stderr", message).display,
+  );
+  assert.deepEqual(new Set(displays), new Set(["Tier-2 compile failed"]));
+  // On master this equals the raw message, so it is not constant across samples.
+  assert.notEqual(tier2StderrReport("stderr", TS1005).display, TS1005);
+});
+
+// --- Behavioural contract ---------------------------------------------------------------
+
+test("the raw line reaches extra.compileDiagnostic verbatim, and nowhere else", () => {
+  const r = tier2StderrReport("stderr", TS1005);
+  assert.equal(r.extra.compileDiagnostic, TS1005);
+  assert.equal(r.display, "Tier-2 compile failed");
+  assert.ok(!r.display.includes(TS1005));
+  assert.ok(!r.fingerprint.join("|").includes(TS1005));
+  assert.ok(!r.fingerprint.join("|").includes("TS1005"));
+});
+
+test("ts_code is set from the diagnostic and is absent from the fingerprint", () => {
+  const a = tier2StderrReport("stderr", TS1005);
+  const b = tier2StderrReport("stderr", TS7006);
+  assert.equal(a.tags.ts_code, "TS1005");
+  assert.equal(b.tags.ts_code, "TS7006");
+  assert.ok(!a.fingerprint.includes("TS1005"));
+  assert.ok(!b.fingerprint.includes("TS7006"));
+});
+
+test("a line with more than one distinct code gets the flat fingerprint but no ts_code tag", () => {
+  const r = tier2StderrReport("stderr", TS_MULTI_CODE);
+  assert.deepEqual(r.fingerprint, ["demo-runtime", "stderr", "tier2-compile"]);
+  assert.equal(r.tags.ts_code, undefined, "first-match-wins is rejected in favour of omitting");
+});
+
+test("a repeated occurrence of the same code is still a single distinct code", () => {
+  const r = tier2StderrReport("stderr", TS2345_REPEATED);
+  assert.equal(r.tags.ts_code, "TS2345");
+});
+
+test("TS1005 and TS1005B (different messages, same code) already share a fingerprint via the existing quoted-string rule", () => {
+  // This was already true on master through normalizeMonitorMessage; asserting it here
+  // pins that the new flat fingerprint does not accidentally split same-code samples.
+  const a = tier2StderrReport("stderr", TS1005);
+  const b = tier2StderrReport("stderr", TS1005B);
+  assert.deepEqual(a.fingerprint, b.fingerprint);
+  assert.equal(a.tags.ts_code, b.tags.ts_code);
+});
+
+// --- Contract / over-widening guards ----------------------------------------------------
+// These pass either way today (the allowlist already excludes them) — they are not
+// fix-provers. Their job is to fail if this module is ever rewritten from an allowlist to
+// a denylist and one of these populations is accidentally swept in.
+
+test("guard: a line with no TS code returns null", () => {
+  assert.equal(tier2StderrReport("stderr", NOCODE), null);
+});
+
+test("guard: NG8001 / NG8002 (Angular template diagnostics, plausibly ours) return null", () => {
+  assert.equal(tier2StderrReport("stderr", NG8001), null);
+  assert.equal(tier2StderrReport("stderr", NG8002), null);
+});
+
+test("guard: 'Failure reason:' and 'Could not resolve' return null", () => {
+  assert.equal(tier2StderrReport("stderr", FAILURE), null);
+  assert.equal(tier2StderrReport("stderr", RESOLVE), null);
+});
+
+test("guard: a bare TS code not in diagnostic position does not match", () => {
+  assert.equal(tier2StderrReport("stderr", "See TS1005 in the manual for details"), null);
+});
+
+test("guard: kind !== 'stderr' returns null even with a recognised code", () => {
+  assert.equal(tier2StderrReport("console-error", TS1005), null);
+  assert.equal(tier2StderrReport("error", TS1005), null);
+});