fix(#60): surface db:push failures instead of swallowing the exit code

packages/core/src/loop/boringstack/gate-stages.ts

@@ -6,7 +6,7 @@ import { composeGate, differentialStage } from "../../gate/gate-runner";
 import type { IValidateResult, IErrorItem } from "../../validate";
 import type { IProvider } from "../../inference";
 import type { IFeature } from "../greenfield/greenfield.types";
-import type { Exec } from "./exec";
+import type { Exec, IExecResult } from "./exec";
 import { runBoringstackGate } from "./gate";
 import { extractFailures, ESLINT_PROGRAM_UNPARSABLE } from "./extract-failures";
 import { verifyFeatureReachable } from "./reachability";
@@ -407,6 +407,58 @@ async function enrichUnparsable(
     `it (there may be more than one broken file; every one must parse before the cascade clears).`;
 }
 
+/** `db:push` applies the Drizzle schema to Postgres every cycle (what a dev gets on save).
+ *  Its exit code used to be DISCARDED, so a failed push — a broken schema, or the DB not
+ *  reachable — was swallowed and the gate then ran against a stale/out-of-sync DB, surfacing
+ *  confusing downstream errors instead of the real cause. Turn the failure into a gate error
+ *  carrying the raw output + a decision rule.
+ *
+ *  We do NOT auto-classify schema-vs-infra from the text. Repeated review proved that
+ *  unwinnable: Postgres echoes ARBITRARY user identifiers AND unquoted DATA in error text
+ *  (`column "epipe"`, `DETAIL: Key (name)=(socket hang up) …`, `Failing row contains (1, EPIPE)`),
+ *  so any substring token — even quote-blanked or word-bounded — can match a genuine schema/data
+ *  failure and mis-steer the model to "bring the stack up / do NOT edit the schema" (the
+ *  stuck-inducing direction). Instead we hand the model the raw error and let IT decide, and set
+ *  no `file` (guessing app.schema.ts would be wrong for a real infra failure). A clean infra
+ *  HARD-abort belongs to the settleGate seam — #47. */
+function dbPushError(result: IExecResult): IErrorItem {
+  const raw = `${result.stdout}\n${result.stderr}`.trim();
+  const excerpt =
+    raw.length <= 3_000
+      ? raw
+      : `${raw.slice(0, 1_800)}\n… output truncated …\n${raw.slice(-1_000)}`;
+
+  // The key drives stuck-detection (and differential suppression). It MUST reflect the specific
+  // failure, not a constant: a constant key makes a DIFFERENT failure look like "no progress"
+  // (and could suppress a genuinely new one). Hash the (stdout, stderr) pair VERBATIM — no
+  // lowercasing/whitespace-collapsing/digit-masking/trimming (none is semantics-preserving for
+  // Postgres output; `value1` vs `value2` are distinct), and NO cwd stripping (that would collapse
+  // `value=<cwd>` and `value=` to the same key). `JSON.stringify([stdout, stderr])` is an
+  // UNFORGEABLE serialization: it escapes every byte (incl. NUL) and encodes the array boundary,
+  // so a byte can't slide between the two streams to forge a collision (`["a","b"]` !==
+  // `["a","b"]`) — unlike any single-delimiter join, since the streams can contain the
+  // delimiter too. A db:push FAILURE is deterministic, so this is a stable key across cycles for
+  // the same error while ANY real difference changes it.
+  const fingerprint = Bun.hash(
+    JSON.stringify([result.stdout, result.stderr])
+  ).toString(36);
+
+  const guidance =
+    "Read the error above and decide: a column/table/type/constraint/data problem means your " +
+    "SCHEMA is wrong — fix `apps/api/src/clients/postgres/schema/app.schema.ts` (the DB is now " +
+    "out of sync, so the rest of the gate is unreliable until this passes). A connection / auth / " +
+    "TLS / `database (or role) does not exist` / `too many clients` problem is INFRASTRUCTURE, " +
+    "not a code fix — bring the BoringStack stack up (dev.sh up) and check the DB environment; do " +
+    "NOT edit the schema for it.";
+
+  return {
+    key: `db-push:${fingerprint}`,
+    rule: "db-push",
+    phase: 1,
+    message: `bun run db:push failed (exit ${String(result.code)}):\n${excerpt}\n\n${guidance}`,
+  };
+}
+
 export function boringstackCommandStage(
   cwd: string,
   exec: Exec,
@@ -415,10 +467,22 @@ export function boringstackCommandStage(
   return {
     async run(): Promise<IValidateResult> {
       await autofixApps(cwd, exec);
-      await exec(["bun", "run", "db:push", "--", "--force"], {
+
+      // db:push syncs the Drizzle schema to Postgres. Its exit code is NOT discarded: a failed
+      // push means the DB is out of sync, so running the gate against it would produce confusing
+      // downstream errors instead of the real cause. Surface it and stop here.
+      const push = await exec(["bun", "run", "db:push", "--", "--force"], {
         cwd: join(cwd, "apps/api"),
       });
 
+      if (push.code !== 0) {
+        return {
+          passed: false,
+          errors: [dbPushError(push)],
+          output: `${push.stdout}\n${push.stderr}`,
+        };
+      }
+
       const result = await runBoringstackGate(cwd, exec);
 
       if (result.passed) {

packages/core/tests/boringstack-gate-stages.test.ts

@@ -25,9 +25,46 @@ const feature: IFeature = {
   attempts: 0,
 };
 
+// The command stage runs setup steps (eslint-cache clear, lint:fix, format, db:push) BEFORE
+// the gate. Those succeed; only the GATE returns the test's code/stdout. Modelling that here
+// keeps a RED-gate test (code 1) exercising the gate path, not the new db:push short-circuit.
 const execWith =
   (code: number, stdout: string): Exec =>
-  async () => ({ code, stdout, stderr: "" });
+  async (argv) => {
+    const cmd = argv.join(" ");
+
+    if (/lint:fix|(?:^|\s)format(?:\s|$)|db:push|eslintcache/u.test(cmd)) {
+      return { code: 0, stdout: "", stderr: "" };
+    }
+
+    return { code, stdout, stderr: "" };
+  };
+
+// An exec whose db:push FAILS with the given output (everything else succeeds / gate is green),
+// to drive the command stage's db:push-failure path. Records every command it's asked to run
+// (into `calls`, when provided) so a test can PROVE the gate was never invoked afterward.
+const execDbPushFails =
+  (pushOutput: string, calls: string[] = []): Exec =>
+  async (argv) => {
+    const cmd = argv.join(" ");
+
+    calls.push(cmd);
+
+    if (cmd.includes("db:push")) {
+      return { code: 1, stdout: pushOutput, stderr: "" };
+    }
+
+    return { code: 0, stdout: "all good", stderr: "" };
+  };
+
+// Every real `bun run db:push` failure begins with this drizzle-kit preamble (it mentions
+// `drizzle.config.ts` and "database"). The db:push tests prepend it so they run against
+// production-shaped output rather than bare one-line errors.
+const DRIZZLE_PREAMBLE =
+  "No config path provided, using default 'drizzle.config.ts'\n" +
+  "Reading config file '/app/apps/api/drizzle.config.ts'\n" +
+  "Using 'pg' driver for database querying\n" +
+  "[i] pulling schema from database...\n";
 
 describe("boringstackCommandStage", () => {
   test("green gate → passed, no errors", async () => {
@@ -399,6 +436,182 @@ UNFAMILIAR TOOL FAILURE: the useful downstream detail`;
     expect(error?.message).toContain("useful downstream detail");
     expect(error?.message).not.toContain("healthy API output");
   });
+
+  test("a failed db:push is SURFACED (not swallowed) and the gate is NOT run against a stale DB", async () => {
+    // #60: db:push's exit code used to be discarded, so a schema-sync failure was silent and
+    // the gate ran against an out-of-sync DB. A non-zero push now short-circuits to a gate
+    // failure carrying the push output, WITHOUT running the gate.
+    const push =
+      'Error: column "amount" cannot be cast automatically to type integer';
+    const calls: string[] = [];
+    const stage = boringstackCommandStage(
+      "/tmp/clone",
+      execDbPushFails(push, calls)
+    );
+    const r = await stage.run("/tmp/clone");
+
+    expect(r.passed).toBe(false);
+    expect(r.errors).toHaveLength(1);
+
+    const err = r.errors[0];
+
+    expect(err?.rule).toBe("db-push");
+    // The key derives from the failure fingerprint, never a constant — so a DIFFERENT error isn't
+    // mistaken for "no progress" by stuck detection.
+    expect(err?.key.startsWith("db-push:")).toBe(true);
+    expect(err?.message).toContain("db:push failed");
+    // The raw push output is preserved so the real cause is visible.
+    expect(err?.message).toContain("cannot be cast automatically");
+
+    // PROVE the gate was never invoked after the push failed. The command stage runs its setup
+    // steps (autofix, then db:push) and, on a push failure, returns immediately — so db:push
+    // must be the LAST command executed. Asserting "nothing ran after db:push" is robust to the
+    // gate's exact argv shape (a brittle `bash -lc` match would silently pass if that changed).
+    expect(calls.some((c) => c.includes("db:push"))).toBe(true);
+    expect(calls.at(-1)).toContain("db:push");
+  });
+
+  test("does NOT auto-classify schema vs infra — surfaces the raw error + a decision rule, no guessed file", async () => {
+    // Repeated review proved schema-vs-infra CANNOT be reliably inferred from the text: Postgres
+    // echoes arbitrary identifiers AND unquoted data in error detail — `column "epipe"`, a
+    // `DETAIL: Key (name)=(socket hang up) …`, `Failing row contains (1, EPIPE)` — so any token
+    // matcher mis-steers real schema/data failures toward "infra / do NOT edit the schema" (the
+    // stuck-inducing direction). So the error instead carries the raw output + a decision rule
+    // naming BOTH levers and lets the model choose, and sets NO file (guessing app.schema.ts would
+    // be wrong for a genuine infra failure).
+    const outputs = [
+      DRIZZLE_PREAMBLE + 'error: column "amount" does not exist', // schema-shaped
+      "Error: connect ECONNREFUSED 127.0.0.1:5432", // infra-shaped
+      // Adversarial: a data value that any substring classifier would have flipped to "infra".
+      DRIZZLE_PREAMBLE +
+        "error: duplicate key value violates unique constraint\n" +
+        "DETAIL:  Key (name)=(socket hang up) already exists.",
+    ];
+
+    for (const out of outputs) {
+      const r = await boringstackCommandStage(
+        "/tmp/clone",
+        execDbPushFails(out)
+      ).run("/tmp/clone");
+      const err = r.errors[0];
+
+      expect(err?.key.startsWith("db-push:")).toBe(true);
+      // No guessed file — classification is the model's call, from the raw error.
+      expect(err?.file).toBeUndefined();
+      // The decision rule names BOTH levers (the schema file AND the infra action) every time.
+      expect(err?.message).toContain("app.schema.ts");
+      expect(err?.message).toContain("dev.sh up");
+    }
+  });
+
+  test("the key can't be forged by sliding a byte between stdout and stderr (unambiguous serialization)", async () => {
+    // Different (stdout, stderr) pairs that would serialize identically under a single-delimiter
+    // join must still key differently — otherwise a genuinely different failure reads as "no
+    // progress". JSON.stringify([stdout, stderr]) is unforgeable because it escapes EVERY byte.
+    const NUL = String.fromCharCode(0);
+
+    const keyFor = async (stdout: string, stderr: string): Promise<string> => {
+      const exec: Exec = async (argv) =>
+        argv.join(" ").includes("db:push")
+          ? { code: 1, stdout, stderr }
+          : { code: 0, stdout: "all good", stderr: "" };
+      const r = await boringstackCommandStage("/tmp/clone", exec).run(
+        "/tmp/clone"
+      );
+
+      return r.errors[0]?.key ?? "";
+    };
+
+    // A '\n' join collides on these; a NUL join collides on the NUL-carrying pair below.
+    expect(await keyFor("a\n", "b")).not.toBe(await keyFor("a", "\nb"));
+    // The streams THEMSELVES contain NUL: `a\0`+`b` and `a`+`\0b` both become `a\0\0b` under a
+    // NUL join. They must still key differently.
+    expect(await keyFor(`a${NUL}`, "b")).not.toBe(await keyFor("a", `${NUL}b`));
+  });
+
+  test("the surfaced output is raw (cwd not stripped) and failures differing only by a cwd path stay DISTINCT", async () => {
+    // Regression: stripping every `cwd` occurrence collapsed `value=<cwd>` and `value=` to one key
+    // and mangled the displayed error. The raw output must be preserved and both key distinctly.
+    const cwd = "/tmp/clone";
+
+    const keyAndMsg = async (
+      out: string
+    ): Promise<{ key: string; message: string }> => {
+      const r = await boringstackCommandStage(cwd, execDbPushFails(out)).run(
+        cwd
+      );
+
+      return {
+        key: r.errors[0]?.key ?? "",
+        message: r.errors[0]?.message ?? "",
+      };
+    };
+
+    const withPath = await keyAndMsg(`error: bad default value=${cwd}/x`);
+    const withoutPath = await keyAndMsg("error: bad default value=");
+
+    // The cwd is preserved in the surfaced message (raw, not stripped)…
+    expect(withPath.message).toContain(cwd);
+    // …and the two outputs, which differ ONLY by the cwd occurrence, key differently.
+    expect(withPath.key).not.toBe(withoutPath.key);
+  });
+
+  test("distinct db:push failures get DISTINCT keys even behind a shared preamble; the same failure is stable", async () => {
+    // Stuck detection keys on the error. Real drizzle-kit output opens with a long shared
+    // preamble and the distinguishing error is in the TAIL — so a prefix-based key would
+    // collapse different failures to "no progress". The key must hash the WHOLE output.
+    const preamble =
+      "No config path provided, using default 'drizzle.config.ts'\n" +
+      "Reading config file '/app/apps/api/drizzle.config.ts'\n" +
+      "Using 'pg' driver for database querying\n" +
+      "[i] pulling schema from database...\n" +
+      "Applying migration to Postgres at postgres://localhost:5432/app ...\n";
+
+    const run = async (out: string): Promise<string> => {
+      const r = await boringstackCommandStage(
+        "/tmp/clone",
+        execDbPushFails(out)
+      ).run("/tmp/clone");
+
+      return r.errors[0]?.key ?? "";
+    };
+
+    // Two DIFFERENT schema errors sharing the (well over 120-char) preamble.
+    const errA =
+      preamble + 'PostgreSQL error: column "amount" cannot be cast to integer';
+    const errB =
+      preamble + 'PostgreSQL error: relation "invoice" already exists';
+
+    const keyA1 = await run(errA);
+    const keyA2 = await run(errA);
+    const keyB = await run(errB);
+
+    expect(preamble.length).toBeGreaterThan(120); // the exact collision the head-slice caused
+    expect(keyA1).toBe(keyA2); // same failure → stable key
+    expect(keyA1).not.toBe(keyB); // different failure (only the tail differs) → different key
+
+    // Failures differing ONLY in a digit are genuinely distinct (identifier suffix, type size)
+    // and must NOT collapse — masking digits would make them share a key and hide progress.
+    const keyD1 = await run(preamble + 'error: column "value1" does not exist');
+    const keyD2 = await run(preamble + 'error: column "value2" does not exist');
+    const keyV20 = await run(preamble + "error: type varchar(20) too small");
+    const keyV30 = await run(preamble + "error: type varchar(30) too small");
+
+    expect(keyD1).not.toBe(keyD2);
+    expect(keyV20).not.toBe(keyV30);
+
+    // Postgres quoted identifiers are CASE-SENSITIVE: "value" and "Value" are different
+    // columns, so their failures must key differently (a lowercasing normalization collapsed
+    // them). The raw-bytes hash preserves case.
+    const keyLower = await run(
+      preamble + 'error: column "value" does not exist'
+    );
+    const keyUpper = await run(
+      preamble + 'error: column "Value" does not exist'
+    );
+
+    expect(keyLower).not.toBe(keyUpper);
+  });
 });
 
 describe("signatureToError", () => {