Kill E2E_REQUIRED; infer DB required-ness from CI

.github/workflows/ci.yml

@@ -75,8 +75,6 @@ jobs:
       - run: bun run check:react-ui-pin
       - run: bun run check:tool-package-pins
       - run: bun run check:tool-package-freshness
-        env:
-          CHECK_BASE_REF: ${{ github.event.pull_request.base.sha }}
 
   e2e:
     runs-on: ubuntu-latest
@@ -117,8 +115,7 @@ jobs:
       # CI stays green. Fail loudly instead: the env file must exist
       # with a usable DATABASE_URL and SESSION_SECRET, Postgres must be
       # reachable, and git (the workflow-asset publication path) must be
-      # present. E2E_REQUIRED=1 below turns any remaining skip into a
-      # hard failure.
+      # present. CI=true turns any remaining skip into a hard failure.
       - name: Assert the e2e test env is wired
         run: |
           test -f .env
@@ -130,7 +127,6 @@ jobs:
       - name: Run the walking skeleton
         run: bun run test:e2e
         env:
-          E2E_REQUIRED: "1"
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench
 
   isolation:
@@ -175,7 +171,6 @@ jobs:
       - name: Run the two-org isolation suite
         run: bun test test/isolation
         env:
-          E2E_REQUIRED: "1"
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench
 
   db-suites:
@@ -230,7 +225,7 @@ jobs:
 
       # apps/hub's own DB-backed suites (e.g. the sign-up rate-limit
       # proof) never run under the plain `checks` job, which has no
-      # Postgres. E2E_REQUIRED=1 turns a would-be skip here into a hard
+      # Postgres. CI=true turns a would-be skip here into a hard
       # failure, so a misconfigured invocation can't pass vacuously.
       - name: Run the hub's database-backed suites
         run: |
@@ -239,21 +234,19 @@ jobs:
           set +a
           bun test apps/hub/test
         env:
-          E2E_REQUIRED: "1"
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench
 
       # Every DB-gated suite under packages/* and apps/hub/src gates on
       # DATABASE_URL and describe.skip's without one, so build-test (no
       # Postgres) never runs them. Find them by the same signal they
       # gate on rather than hardcoding a file list, so a newly added
-      # suite gets picked up automatically. E2E_REQUIRED=1 turns a
-      # would-be skip into a hard failure.
+      # suite gets picked up automatically. CI=true turns a would-be
+      # skip into a hard failure.
       - name: Run the database-backed package suites
         run: |
           set -a
           . ./.env
           set +a
           bun --env-file="${GITHUB_WORKSPACE}/.env" test $(grep -rl DATABASE_URL --include='*.test.ts' packages apps | grep -v apps/hub/test)
         env:
-          E2E_REQUIRED: "1"
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench

AGENTS.md

@@ -57,6 +57,31 @@ behind human approval.
   logical change per commit; commit messages are written for a public
   audience.
 - Worktrees live in `.worktrees/<branch>`; branch = `cl-<issue#>-<slug>`.
+- Remaining test/CI flags (what breaks if unset):
+  - `DATABASE_URL` — DB-gated suites skip locally with a banner naming
+    `docker compose -f docker-compose.test.yml up -d`. On CI jobs that
+    provision Postgres (`e2e`, `isolation`, `db-suites`), `CI=true`
+    turns a missing URL into a hard failure via `dbGate`. The unit
+    jobs (`build-test`, `structural`, `lint`, `typecheck`) never
+    provision Postgres and still skip.
+  - `HUB_DATA_DIR` — hub boot fails. This is required runtime config,
+    not a test flag.
+  - `CI` — set automatically by GitHub Actions (`true`). Not a caller
+    flag; `dbGate` infers required-ness from it.
+  - `ISOLATION_DATABASE_URL` — optional override for the isolation
+    suite; unset falls through to `DATABASE_URL`.
+  - `E2E_PROVIDER` / `E2E_PROVIDER_API_KEY` / `OLLAMA_BASE_URL` — live
+    inference e2e (walkthrough, greeting-delivery, play, proof
+    scripts). Unset stays on the noop/stub path; the suite still runs.
+  - `EVAL_PROVIDER` / `EVAL_PROVIDER_API_KEY` — live evals. Unset is
+    plumbing-only (`bun run eval` still runs against the stub).
+  - `WORKBENCH_CHECK_CONCURRENCY` — override `scripts/run-all.ts`
+    parallelism. Unset uses a core-count heuristic (all cores on
+    GitHub Actions, cores-2 locally).
+  - `E2E_LOG_DIR` — optional directory for hub/sidecar e2e logs.
+    Unset does not capture them.
+  - `CHROME_PATH` — browser walkthrough looks for Chrome here first.
+    Unset falls through to platform defaults and fails if none exist.
 - Tests are meaningful red/green tests only — no coverage theater. Unit
   tests for pure modules sit next to the source they cover
   (`src/**/*.test.ts`); multi-module / DOM / composition suites stay under
@@ -109,11 +134,12 @@ scripts/checks/report-error.ts --write-baseline` after fixing (or
   package's own `package.json` version — `check:tool-package-pins` catches
   the mismatch. A tool package's `src/` changing without a version bump is
   the other half of that class, caught separately by
-  `check:tool-package-freshness`: it diffs against the base ref (CI passes
-  `CHECK_BASE_REF`; locally it falls back to the merge base with
+  `check:tool-package-freshness`: it diffs against the base ref (a
+  GitHub `pull_request` job reads `pull_request.base.sha` from
+  `GITHUB_EVENT_PATH`; locally it falls back to the merge base with
   `origin/main`) and flags any tool package whose `src/` differs from that
   base while its version stayed the same. With no base ref available
-  (no `origin/main`, no `CHECK_BASE_REF`) it no-ops, deferring to CI as the
+  (no `origin/main`, no pull-request event) it no-ops, deferring to CI as the
   authoritative run.
 - Every package needs a `LICENSE` file (canonical LGPL-2.1-or-later text,
   copy from any existing `packages/*/LICENSE`) — `check:licenses` fails

IMPLEMENTATION.md

@@ -99,9 +99,10 @@ suite needs zero real credentials: `startHub` only forwards an explicit
 env allowlist, so a real `ANTHROPIC_API_KEY` in the shell never reaches
 the spawned hub, and every inference source in a test points at the hub's
 own `noop-inference` endpoint or an unreachable placeholder host —
-enforced by `assertNeverRealProvider` in `scripts/e2e/harness.ts`. CI sets
-`E2E_REQUIRED=1` so a missing `DATABASE_URL` fails loudly there instead of
-silently skipping.
+enforced by `assertNeverRealProvider` in `scripts/e2e/harness.ts`. CI infers
+required-ness from `CI=true` so a missing `DATABASE_URL` fails loudly there
+instead of silently skipping. Start Postgres locally with
+`docker compose -f docker-compose.test.yml up -d`.
 
 ## Deployment
 

README.md

@@ -185,9 +185,10 @@ in any order:
 [Running locally](#running-locally)). Each suite reuses a sibling
 `<database>_e2e` database that it owns outright — drops and rebuilds its
 own schema on every run — so it can never touch your working database.
-Without `DATABASE_URL` the suite skips with a warning; CI sets
-`E2E_REQUIRED=1` so that skip fails loudly there instead of silently
-passing.
+Without `DATABASE_URL` the suite skips with a warning naming
+`docker compose -f docker-compose.test.yml up -d`; CI infers
+required-ness from `CI=true` so that skip fails loudly there instead of
+silently passing.
 
 **Needs zero real credentials.** The whole suite — smoke and non-smoke
 files alike — runs with no API keys, no paid credentials, and no network
@@ -212,9 +213,9 @@ DB-gated tests (migrations, Drizzle-backed stores, route integration
 tests) that skip via `describeIfDb` when `DATABASE_URL` is unset. A
 skip is never silent: `scripts/e2e/db-gate.ts`'s `dbGate` — what
 `describeIfDb` is built on — prints an unmissable summary naming every
-skipped suite at the end of the run, and honors the same
-`E2E_REQUIRED=1` convention as the e2e suite above, turning a skip into
-a hard failure. CI sets `E2E_REQUIRED=1` for exactly this reason.
+skipped suite at the end of the run, and infers required-ness from
+`CI=true` the same way the e2e suite above does, turning a skip into
+a hard failure on CI jobs that provision Postgres.
 
 To run these locally against the same Postgres CI uses:
 

apps/hub/test/rate-limit.test.ts

@@ -31,14 +31,8 @@ if (!databaseUrl) {
   // Without a database there is nothing meaningful to assert; skipping
   // loudly beats a vacuous pass. Set DATABASE_URL or
   // ISOLATION_DATABASE_URL to run this against a real Postgres. In CI,
-  // E2E_REQUIRED=1 turns that skip into a loud failure so this proof can
-  // never silently vanish from the pipeline (mirrors scripts/e2e/harness.ts).
-  if (process.env["E2E_REQUIRED"] === "1") {
-    throw new Error(
-      "E2E_REQUIRED=1 but DATABASE_URL is not set; the sign-up rate-limit " +
-        "suite would be skipped. Set DATABASE_URL to a reachable Postgres.",
-    );
-  }
+  // dbGate's CI=true convention turns that skip into a loud failure
+  // (resolveDatabaseUrl throws) so this proof can never silently vanish.
   test.skip("sign-up rate limiting (set DATABASE_URL or ISOLATION_DATABASE_URL to run)", () => {});
 } else {
   await prepareDatabase(databaseUrl);

packages/chat/test/migrations.test.ts

@@ -1,6 +1,6 @@
 // DB-gated: skipped when no DATABASE_URL is reachable (a fresh
 // checkout still runs the unit gates), and turned into a loud failure
-// by E2E_REQUIRED=1 so the suite can never silently vanish from CI —
+// by CI=true so the suite can never silently vanish from CI —
 // mirroring scripts/e2e/harness.ts's e2eDatabaseUrl/baseUrlToE2eUrl.
 // Runs against its own scratch database, never the developer's or the
 // walking-skeleton suite's, so a failure here can never corrupt either.

packages/evals/src/store/store.test.ts

@@ -1,24 +1,19 @@
 // Round-trips EvalRunResult through a real, uniquely-named scratch
 // Postgres database — never the developer's own DATABASE_URL. Skips
 // without DATABASE_URL (like every other real-Postgres suite in this
-// repo); E2E_REQUIRED=1 turns that skip into a loud CI failure.
+// repo); CI=true turns that skip into a loud CI failure.
 import { afterAll, describe, expect, test } from "bun:test";
 import postgres from "postgres";
 
 import { applyEvalsMigrations } from "./migrations.ts";
 import { createPostgresEvalRunStore } from "./pg-store.ts";
 import type { EvalRunResult } from "../types.ts";
+import { assertDatabaseConfigured } from "../../../../scripts/e2e/db-gate.ts";
 
 function scratchDatabaseUrl(): string | undefined {
   const base = process.env["DATABASE_URL"];
   if (base === undefined || base === "") {
-    if (process.env["E2E_REQUIRED"] === "1") {
-      throw new Error(
-        "E2E_REQUIRED=1 but DATABASE_URL is not set; the evals store " +
-          "suite would be skipped. Set DATABASE_URL to a reachable " +
-          "Postgres.",
-      );
-    }
+    assertDatabaseConfigured(undefined, "evals store suite");
     return undefined;
   }
   const url = new URL(base);

packages/webhook-triggers/test/migrations.test.ts

@@ -1,6 +1,6 @@
 // DB-gated: skipped when no DATABASE_URL is reachable (a fresh
 // checkout still runs the unit gates), and turned into a loud failure
-// by E2E_REQUIRED=1 so the suite can never silently vanish from CI —
+// by CI=true so the suite can never silently vanish from CI —
 // mirroring `@corbits/chat`'s own migrations test. Runs against its
 // own scratch database, never the developer's or the walking-skeleton
 // suite's, so a failure here can never corrupt either.

scripts/checks/db-gate.ts

@@ -1,7 +1,7 @@
 // check:db-gate — every DB-gated suite must skip through dbGate
 // (scripts/e2e/db-gate.ts), never a hand-rolled
 // `databaseUrl ? describe.skip : describe` ternary. A hand-rolled gate
-// skips in total silence and never honors E2E_REQUIRED=1 — exactly
+// skips in total silence and never honors CI=true — exactly
 // the gap CL-7279 closed, one file at a time, until the next merge
 // added a new hand-rolled copy the sweep never saw. This check makes
 // the invariant self-enforcing instead of a one-time sweep.
@@ -49,7 +49,7 @@ export function auditDbGate(
     report.violations.push(
       `${relPath}: hand-rolls a describeIfDb gate instead of using dbGate ` +
         `from scripts/e2e/db-gate.ts. A hand-rolled gate skips silently ` +
-        `and never honors E2E_REQUIRED=1 — replace it with ` +
+        `and never honors CI=true — replace it with ` +
         `\`const describeIfDb = dbGate(databaseUrl, import.meta.path);\`.`,
     );
   }

scripts/checks/report-error.ts

@@ -56,9 +56,10 @@
 //
 // This follows check:tool-package-freshness's own precedent for scoping a
 // new invariant to what changed rather than retroactively flagging
-// pre-existing state: CI passes CHECK_BASE_REF; locally this falls back to
-// the merge base with origin/main; with neither available the touched-file
-// half of the gate no-ops (new-vs-baseline enforcement still applies).
+// pre-existing state: on a GitHub pull_request job the base SHA comes
+// from the event payload; locally this falls back to the merge base with
+// origin/main; with neither available the touched-file half of the gate
+// no-ops (new-vs-baseline enforcement still applies).
 //
 // Regenerate the baseline after fixing (or newly opting out) entries:
 //   bun run scripts/checks/report-error.ts --write-baseline
@@ -547,7 +548,7 @@ async function main(): Promise<void> {
     ? parseBaseline(await baselineFile.text())
     : new Set<string>();
 
-  const baseRef = resolveBaseRef(root, process.env["CHECK_BASE_REF"]);
+  const baseRef = resolveBaseRef(root);
   const changedLines =
     baseRef === undefined
       ? undefined
@@ -561,7 +562,7 @@ async function main(): Promise<void> {
   });
   if (baseRef === undefined) {
     report.notes.push(
-      "no base ref (no origin/main, no CHECK_BASE_REF); skipping the " +
+      "no base ref (no origin/main, no pull_request.base.sha); skipping the " +
         "touched-line ratchet — CI supplies the base ref for the " +
         "authoritative run. New-vs-baseline enforcement still applies.",
     );

scripts/checks/test/tool-package-freshness.test.ts

@@ -1,8 +1,12 @@
 import { describe, expect, test } from "bun:test";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import path from "node:path";
 
 import {
   auditFreshness,
   packagesWithChangedSource,
+  pullRequestBaseSha,
   readToolPackageNames,
 } from "../tool-package-freshness";
 
@@ -97,3 +101,32 @@ describe("scope", () => {
     expect(names).toEqual(["github-tools", "memory-tools"]);
   });
 });
+
+describe("pullRequestBaseSha", () => {
+  test("reads pull_request.base.sha from GITHUB_EVENT_PATH", () => {
+    const dir = mkdtempSync(path.join(tmpdir(), "base-ref-"));
+    const eventPath = path.join(dir, "event.json");
+    writeFileSync(
+      eventPath,
+      JSON.stringify({ pull_request: { base: { sha: "abc123def" } } }),
+    );
+    expect(pullRequestBaseSha({ GITHUB_EVENT_PATH: eventPath })).toBe(
+      "abc123def",
+    );
+    rmSync(dir, { recursive: true, force: true });
+  });
+
+  test("returns undefined when GITHUB_EVENT_PATH is unset", () => {
+    expect(pullRequestBaseSha({})).toBeUndefined();
+  });
+
+  test("returns undefined when the event is not a pull_request", () => {
+    const dir = mkdtempSync(path.join(tmpdir(), "base-ref-"));
+    const eventPath = path.join(dir, "event.json");
+    writeFileSync(eventPath, JSON.stringify({ ref: "refs/heads/main" }));
+    expect(
+      pullRequestBaseSha({ GITHUB_EVENT_PATH: eventPath }),
+    ).toBeUndefined();
+    rmSync(dir, { recursive: true, force: true });
+  });
+});