ci(observability-map): widen the obsmap filter to what the suite reads

The filter watched apps/webapp/app/routes only, which was narrower than the
suite's actual coupling. webappSymbols.test.ts walks all of apps/webapp/app and
fails when a guard, sensitive or audit symbol stops resolving, so renaming e.g.
requireUserId in app/services/session.server.ts matched the webapp filter and
nothing else: no job ran this suite and the break landed on main, or on the next
unrelated internal-packages PR. integration.test.ts also asserts on the text of
observability-map.yml, which no filter watched at all, so editing the report
workflow alone ran nothing.

Derived the coupling set from the code rather than from the comment. Outside its
own directory the suite reads apps/webapp/app (whole tree for symbols, the route
subtree for the scan), packages/plugins/src, internal-packages/rbac/src, and
four workflow files. packages/plugins/src and internal-packages/rbac/src stay
out: internal already matches packages/** and internal-packages/**, and
unit-tests-internal.yml runs the same suite, so listing them here would run it
twice. A new test pins that reasoning.

Cost, over the last 400 commits on main: 31% touch routes, 52% touch
apps/webapp/app, so the job fires on roughly half of PRs instead of roughly a
third. It is the cheap one, a single 4x runner with no containers and no
database.

Reported by Devin on #4455.

Author: 1stvamp

.github/workflows/pr_checks.yml

@@ -82,20 +82,33 @@ jobs:
               - 'pnpm-lock.yaml'
               - 'pnpm-workspace.yaml'
               - 'turbo.json'
-            # Routes only, and that is the whole reason this filter exists: one test in
-            # @internal/observability-map scans the live route tree, and without this a webapp-only
-            # PR that renames a route merges green and the break lands on main, or on the next
-            # unrelated internal-packages PR.
+            # The whole webapp app tree, not just its routes, and that is the whole reason this
+            # filter exists. Two tests in @internal/observability-map read it: integration.test.ts
+            # scans the live route tree, and webappSymbols.test.ts walks all of apps/webapp/app and
+            # fails when a guard, sensitive or audit symbol stops resolving. Routes-only was this
+            # filter's own bug: renaming e.g. requireUserId in app/services/session.server.ts
+            # matched `webapp` and nothing else, so no job ran the suite and the break landed on
+            # main, or on the next unrelated internal-packages PR.
             #
-            # The package's own paths are deliberately NOT here. `internal` above already matches
-            # `internal-packages/**`, and `unit-tests-internal.yml` runs `turbo run test --filter
-            # "@internal/*"`, which picks up @internal/observability-map and runs the same vitest
-            # suite including that route-tree test. Listing the package here as well ran the suite
-            # twice on every PR touching it, which was this filter's own doing.
+            # The cost of the wider set, measured over the last 400 commits on main: 31% touch
+            # routes, 52% touch apps/webapp/app, so the job goes from firing on roughly a third of
+            # PRs to roughly a half. It is the cheap one -- a single 4x runner, no containers, no
+            # database, no prisma generate -- which is what makes that affordable.
+            #
+            # observability-map.yml is here because integration.test.ts asserts on its text and no
+            # other filter watches it, so editing the report workflow alone ran nothing at all.
+            #
+            # Deliberately NOT here: this package's own paths, and packages/plugins/src and
+            # internal-packages/rbac/src, the other two trees webappSymbols.test.ts reads.
+            # `internal` above already matches `internal-packages/**` and `packages/**`, and
+            # `unit-tests-internal.yml` runs `turbo run test --filter "@internal/*"`, which picks up
+            # @internal/observability-map and runs the same vitest suite. Listing them here as well
+            # ran the suite twice on every PR touching them, which was this filter's own doing.
             obsmap:
-              - 'apps/webapp/app/routes/**'
+              - 'apps/webapp/app/**'
               - '.github/workflows/pr_checks.yml'
               - '.github/workflows/unit-tests-observability-map.yml'
+              - '.github/workflows/observability-map.yml'
               - 'package.json'
               - 'pnpm-lock.yaml'
               - 'pnpm-workspace.yaml'

.github/workflows/unit-tests-observability-map.yml

@@ -36,7 +36,8 @@ jobs:
       - name: 📥 Download deps
         run: pnpm install --frozen-lockfile
 
-      # One test in this suite scans apps/webapp/app/routes, which is why the filter that gates
-      # this workflow watches those paths as well as the package's own.
+      # This suite reads apps/webapp/app (the route tree for the scan, the whole app tree for the
+      # symbol check) and the report workflow's text, which is why the filter that gates this
+      # workflow watches all of those and not only the routes folder.
       - name: 🧪 Run tests
         run: pnpm --filter @internal/observability-map run test

internal-packages/observability-map/src/integration.test.ts

@@ -14,21 +14,29 @@ import { buildReport } from "./score.js";
 import { SCORED_CHECK_IDS } from "./checks/index.js";
 
 /**
- * The one deliberate coupling to `apps/webapp/app/routes` in the suite. Everything else, including
- * the CLI tests, runs against a fixture tree of this package's own making.
+ * This file's deliberate coupling to `apps/webapp/app/routes`. It is not the suite's only one, and
+ * saying it was is what let the paths filter be written for this file alone:
+ * `webappSymbols.test.ts` walks all of `apps/webapp/app`, `packages/plugins/src` and
+ * `internal-packages/rbac/src`, and `mutationCorpus.test.ts` scans the route tree behind an env
+ * gate. Everything else, including the CLI tests, runs against a fixture tree of this package's own
+ * making.
  *
  * The coupling is acceptable because nothing here names a route or a count: the scan must not
  * crash, the entry point count must sit inside a wide band, and parse failures must be zero. Those
  * survive routes being added, renamed and deleted, and they are the only things a fixture tree
  * cannot tell us, since a fixture only contains shapes somebody thought to write down.
  *
  * What runs this for a webapp pull request is `.github/workflows/unit-tests-observability-map.yml`,
- * called from `pr_checks.yml` behind an `obsmap` paths filter covering `apps/webapp/app/routes/**`,
- * and listed in the `all-checks` aggregate so it actually gates. A pull request touching this
- * PACKAGE reaches the same test by the other road: `internal` already matches
- * `internal-packages/**`, and `unit-tests-internal.yml` runs `turbo run test --filter "@internal/*"`
- * over this package too. So both directions are gated, and neither is gated twice; the `obsmap`
- * filter used to name the package as well, which ran this suite twice on every PR touching it.
+ * called from `pr_checks.yml` behind an `obsmap` paths filter covering the whole of
+ * `apps/webapp/app` plus the report workflow, and listed in the `all-checks` aggregate so it
+ * actually gates. The filter is wider than this file's own coupling because the suite's is:
+ * `webappSymbols.test.ts` walks all of `apps/webapp/app`, and the describes below read
+ * `observability-map.yml`. A pull request touching this PACKAGE, or `packages/plugins/src` or
+ * `internal-packages/rbac/src`, reaches the same test by the other road: `internal` already matches
+ * `internal-packages/**` and `packages/**`, and `unit-tests-internal.yml` runs `turbo run test
+ * --filter "@internal/*"` over this package too. So every direction is gated and none is gated
+ * twice; the `obsmap` filter used to name the package as well, which ran this suite twice on every
+ * PR touching it.
  *
  * Two shapes were tried and rejected on the way here. Widening `pr_checks.yml`'s `internal` filter
  * to the route paths ran all eighteen internal packages, twelve shards with postgres, clickhouse,
@@ -179,9 +187,30 @@ describe("the package's tests are wired into the gate", () => {
     expect(read(REUSABLE)).toContain("workflow_call");
   });
 
-  it("watches the live route tree, which is the only thing the internal filter misses", () => {
+  // Round 5. The filter watched `apps/webapp/app/routes/**` while the suite reads more than that,
+  // so a rename outside the routes folder matched only `webapp`, ran no job that runs this suite,
+  // and broke the build for whoever pushed next. Asserted as the whole set the suite reads and no
+  // other filter covers, rather than as the one path that prompted the filter, because the routes
+  // entry looked complete right up until it wasn't.
+  it("watches every webapp path the internal filter misses, not just the routes folder", () => {
     const filter = read(PR_CHECKS).split("            obsmap:")[1]!.split("            cli:")[0]!;
-    expect(filter).toContain("'apps/webapp/app/routes/**'");
+    // webappSymbols.test.ts walks all of apps/webapp/app, not just routes.
+    expect(filter).toContain("'apps/webapp/app/**'");
+    // The report workflow, whose text the two describes above assert on. No other filter names it.
+    expect(filter).toContain("'.github/workflows/observability-map.yml'");
+  });
+
+  // The other two trees webappSymbols.test.ts reads. They belong to `internal`, not here, and this
+  // pins the reason so the obvious-looking addition has to argue with a test first.
+  it("leaves the two non-webapp roots it reads to the internal filter", () => {
+    const text = read(PR_CHECKS);
+    const obsmap = text.split("            obsmap:")[1]!.split("            cli:")[0]!;
+    expect(obsmap).not.toContain("packages/plugins");
+    expect(obsmap).not.toContain("internal-packages/rbac");
+
+    const internal = text.split("            internal:")[1]!.split("            # ")[0]!;
+    expect(internal).toContain("'packages/**'");
+    expect(internal).toContain("'internal-packages/**'");
   });
 
   // Round E item 6. `internal` matches `internal-packages/**` and `unit-tests-internal.yml` runs