diff --git a/packages/core/scripts/headless-build.ts b/packages/core/scripts/headless-build.ts index 10b3a2a8..51d0d010 100644 --- a/packages/core/scripts/headless-build.ts +++ b/packages/core/scripts/headless-build.ts @@ -262,12 +262,33 @@ async function driveBuild( // flags (drive-to-green, guidance, pull_conventions, the WS-G `check` tool) and is // unit-tested to advertise `check`, so this wiring can't silently regress. The i18n // editGuard is stateful (one per build) so it's supplied here, not in the flags. + // A/B lever (near-green rotation): DeepSeek's thinking mode wants temp ~1.0 to avoid + // CoT collapse → repetitive loops; the build otherwise falls through to 0.2. Set + // TSFORGE_BUILD_TEMPERATURE to run the treatment arm. Unset/invalid → session default. + const rawBuildTemp = process.env.TSFORGE_BUILD_TEMPERATURE; + const parsedBuildTemp = + rawBuildTemp === undefined || rawBuildTemp === "" + ? Number.NaN + : Number(rawBuildTemp); + const buildTemperature = Number.isFinite(parsedBuildTemp) + ? parsedBuildTemp + : undefined; + + if (buildTemperature !== undefined) { + report({ + kind: "tool", + task: "boringstack", + message: `⚙ build temperature override: ${String(buildTemperature)} (TSFORGE_BUILD_TEMPERATURE)`, + }); + } + const host = await createBoringstackHostSession({ provider, cwd: dir, contextWindow, maxTurns: LOOP_LIMITS.webMaxTurns, report, + temperature: buildTemperature, // BoringStack overlays (composed, see makeBoringstackBuildGuard): (1) veto // deletion of a feature translation key the model authored earlier this build; // (2) veto creating a same-basename .test.ts + .test.tsx twin that orphans the diff --git a/packages/core/src/loop/acceptance/acceptance-spec.ts b/packages/core/src/loop/acceptance/acceptance-spec.ts index 7768cc2f..70b19c5e 100644 --- a/packages/core/src/loop/acceptance/acceptance-spec.ts +++ b/packages/core/src/loop/acceptance/acceptance-spec.ts @@ -120,17 +120,34 @@ function negativesFor( return out; } +/** The implicit auth owner every boringstack entity gets via automatic `userId` scoping. A + * "belongs to a User" relationship documents OWNERSHIP — it is NOT a user-selectable parent FK, + * so it must never become a parent `{companies.map(…)}\` → "Extract this computation into a hook or helper function". Compute EVERY list/derived value as a \`const\` in the component body, then reference the BARE identifier in JSX: \`\`\`tsx const renderRows = items.map((row) => (…)); diff --git a/packages/core/src/loop/near-green-checkpoint.ts b/packages/core/src/loop/near-green-checkpoint.ts index 15658543..87e7b849 100644 --- a/packages/core/src/loop/near-green-checkpoint.ts +++ b/packages/core/src/loop/near-green-checkpoint.ts @@ -72,6 +72,15 @@ const COMPLETION_CLASS_RULES: ReadonlySet = new Set([ "reachability", "i18n-locale-keys-used", "logic-files-require-test-sibling", + // A near-green state whose sole remaining error is a MISSING required test-id or an UNWIRED + // feature is the same hollow trap: the only non-destructive fix is to ADD the create/edit/ + // delete UI (form + buttons + testids), which transiently spikes the count. Count-only WS-B + // reverted exactly this and parked the model at 1 (live: valbuild23 stuck on testid-presence + // → sprayed adding the UI → rolled back ×3 → parked). 4-model panel confirmed this is THE + // trap, not the taste rules. These IDs are emitted by gate-stages.ts (the e2e testid contract + // + the feature-wiring check). + "testid-presence", + "feature-wiring", ]); /** Whether a gate error clears only by adding code (see COMPLETION_CLASS_RULES). Matches the diff --git a/packages/core/src/loop/turn.ts b/packages/core/src/loop/turn.ts index 26482484..fbab8b83 100644 --- a/packages/core/src/loop/turn.ts +++ b/packages/core/src/loop/turn.ts @@ -2269,7 +2269,8 @@ export async function captureNearGreenCheckpoint( export async function rollbackNearGreen( ctx: ILoopCtx, state: ILoopState, - sprayCount: number + sprayCount: number, + sprayErrors: readonly IErrorItem[] = [] ): Promise { const cp = state.nearGreenCheckpoint; @@ -2348,13 +2349,28 @@ export async function rollbackNearGreen( .map((e) => ` - ${e.message}`) .join("\n"); + // Show the model the NEW errors its reverted change INTRODUCED (the spray delta), not just the + // errors it was already at — otherwise it's a blind random walk: it can't learn which pattern to + // avoid because the evidence was just reverted off disk. Keyed diff against the checkpoint so + // only genuinely-new errors show. (4-model panel: hiding the spray was a core reason it circled.) + const cpKeys = new Set(cp.errors.map((e) => e.key)); + const introduced = sprayErrors.filter((e) => !cpKeys.has(e.key)); + const introducedNote = + introduced.length > 0 + ? `\n\nThose reverted edits INTRODUCED these new error(s) — do NOT re-introduce them:\n${introduced + .slice(0, 20) + .map((e) => ` - ${e.message}`) + .join("\n")}` + : ""; + ctx.messages.push({ role: "user", content: `Your last change made things WORSE — the gate went from ${String(cp.errorCount)} ` + `to ${String(sprayCount)} error(s) — so I reverted those edits back to your best ` + `state (${String(cp.errorCount)} error(s) left). Do NOT rewrite files or start over. ` + - `Make a SMALL, targeted fix for ONLY these remaining errors, one at a time:\n${errorList}`, + `Make a SMALL, targeted fix for ONLY these remaining errors, one at a time:\n${errorList}` + + introducedNote, }); } @@ -2365,7 +2381,8 @@ export async function rollbackNearGreen( async function nearGreenRollbackStep( ctx: ILoopCtx, state: ILoopState, - curr: number + curr: number, + gateErrors: readonly IErrorItem[] ): Promise { if (!flags.nearGreenCheckpoint()) { return false; @@ -2400,7 +2417,7 @@ async function nearGreenRollbackStep( state.nearGreenRollbacks ?? 0 ) ) { - await rollbackNearGreen(ctx, state, curr); + await rollbackNearGreen(ctx, state, curr, gateErrors); return true; } @@ -2671,7 +2688,7 @@ export async function settleGate( // WS-B: revert a count spray to the best on-disk near-green state (returning BEFORE // checkStuck — a revert must not advance the ladder). - if (await nearGreenRollbackStep(ctx, state, curr)) { + if (await nearGreenRollbackStep(ctx, state, curr, gateErrors)) { return null; } diff --git a/packages/core/tests/acceptance-spec.test.ts b/packages/core/tests/acceptance-spec.test.ts index 91505f6a..2bc09751 100644 --- a/packages/core/tests/acceptance-spec.test.ts +++ b/packages/core/tests/acceptance-spec.test.ts @@ -94,6 +94,63 @@ test("planToAcceptanceSpec: relationships parse to parent + fkField", () => { ]); }); +test("planToAcceptanceSpec: 'belongs to a User' (implicit auth owner) yields NO parent — not a phantom `aId`/`userId` select/seed", () => { + const spec = planToAcceptanceSpec({ + product: "p", + slices: [ + { + entity: { + id: "Bookmark", + desc: "d", + fields: [{ name: "title", type: "string" }], + relationships: ["belongs to a User"], + rules: [], + }, + ui: { screens: ["list"], action: "a", shows: [], nav: "Bookmarks" }, + verification: { + mustRemainTrue: [], + mustNotHappen: ["x"], + acceptanceCheck: "x", + }, + }, + ], + }); + + // The article "a" is NOT captured as a phantom parent, and the auth owner "User" is excluded — + // so no parent select/seed (which parked valbuild23 with `POST /api/v1/a` → 404). + expect(spec.entities[0]?.parents).toEqual([]); + expect(spec.entities[0]?.fields.some((f) => f.name.endsWith("Id"))).toBe( + false + ); +}); + +test("planToAcceptanceSpec: 'belongs to a Company' (leading article) parses to Company, not the article", () => { + const spec = planToAcceptanceSpec({ + product: "p", + slices: [ + { + entity: { + id: "Deal", + desc: "d", + fields: [{ name: "name", type: "string" }], + relationships: ["belongs to a Company"], + rules: [], + }, + ui: { screens: ["list"], action: "a", shows: [], nav: "Deals" }, + verification: { + mustRemainTrue: [], + mustNotHappen: ["x"], + acceptanceCheck: "x", + }, + }, + ], + }); + + expect(spec.entities[0]?.parents).toEqual([ + { entity: "Company", key: "company", fkField: "companyId" }, + ]); +}); + test("planToAcceptanceSpec: negatives derive missing-required + bad-email", () => { const spec = planToAcceptanceSpec(plan); const company = spec.entities[0]; diff --git a/packages/core/tests/acceptance-steer.test.ts b/packages/core/tests/acceptance-steer.test.ts index 50bc1f29..21eb6ac8 100644 --- a/packages/core/tests/acceptance-steer.test.ts +++ b/packages/core/tests/acceptance-steer.test.ts @@ -245,3 +245,57 @@ test("acceptanceSteer: CREATE step message mentions form submission", () => { expect(steer).toContain("Issue"); expect(steer).toContain("create"); }); + +test("acceptanceSteer: form-didn't-close failure steers at closing the form on success, NOT at opening/persistence", () => { + const entity = makeEntity("Bookmark"); + + // Real Playwright signature when the create form submits but never hides: the + // failure is classified "create" (the hidden-assert lives in the create test), + // but the true fix is closing the form on success — not opening it or fixing persistence. + const outcome: IAcceptanceOutcome = { + ok: false, + results: [ + { entity: "Bookmark", step: "nav", ok: true, detail: "" }, + { entity: "Bookmark", step: "list", ok: true, detail: "" }, + { + entity: "Bookmark", + step: "create", + ok: false, + detail: + 'TimeoutError: locator.waitFor: Timeout 10000ms exceeded.\nCall log:\n - waiting for getByTestId(\'bookmark-form\') to be hidden\n 24 × locator resolved to visible
', + }, + ], + }; + + const steer = acceptanceSteer(entity, outcome); + + expect(steer).toContain("Bookmark"); + expect(steer).toContain("did not close"); + expect(steer).toContain("onSuccess"); + // Must NOT emit the misleading generic create-step message that tells the model the + // form failed to OPEN — that opposite steer is the whole bug being fixed here. + expect(steer).not.toContain("was not visible"); +}); + +test("acceptanceSteer: a genuine form-didn't-OPEN create failure still uses the generic create message", () => { + const entity = makeEntity("Bookmark"); + + // No "to be hidden"/"resolved to visible" signature → NOT a form-close failure; + // the detail-aware branch must fall through to the generic create-step message. + const outcome: IAcceptanceOutcome = { + ok: false, + results: [ + { + entity: "Bookmark", + step: "create", + ok: false, + detail: "getByTestId('bookmark-create') not found", + }, + ], + }; + + const steer = acceptanceSteer(entity, outcome); + + expect(steer).toContain("was not visible"); + expect(steer).not.toContain("did not close"); +}); diff --git a/packages/core/tests/boringstack-build.test.ts b/packages/core/tests/boringstack-build.test.ts index 1095174f..f57930d1 100644 --- a/packages/core/tests/boringstack-build.test.ts +++ b/packages/core/tests/boringstack-build.test.ts @@ -247,7 +247,7 @@ describe("boringstackDeps.implement", () => { expect(seen.join("\n")).not.toContain("separate slices"); }); - test("syncs DB after generation but before sending to the model", async () => { + test("delegates the DB sync to generate — no redundant, result-ignoring harness-level db:push", async () => { const host = createHost(); const execCalls: { argv: string[]; cwd: string }[] = []; @@ -257,25 +257,33 @@ describe("boringstackDeps.implement", () => { return { code: 0, stdout: "", stderr: "" }; }; + let generateCalled = false; + const deps = boringstackDeps({ host, cwd: "/repo", exec, evaluator: createEvaluator(), - generate: async () => undefined, + generate: async () => { + generateCalled = true; + }, generateUi: async () => undefined, }); await deps.implement(feature("Invoice"), state()); - const forCmd = (cmd: string): string[] => - execCalls - .filter((c) => c.argv.join(" ") === cmd) - .map((c) => c.cwd) - .sort(); + // The DB sync is `generate`'s responsibility: the real generateResource runs the + // headless-safe dbPushForce (recover-or-throw) — proven in boringstack-generate + + // db-push tests. The harness delegates to it… + expect(generateCalled).toBe(true); + // …and issues NO db:push of its own. A second harness-level push would be + // redundant, and (since its result was ignored) could re-introduce the + // swallowed-failure false-green the db:push fix removes. + const harnessPushes = execCalls.filter( + (c) => c.argv.join(" ") === "bun run db:push -- --force" + ); - // db:push is called to sync the STUB schema before the model gets the prompt - expect(forCmd("bun run db:push -- --force")).toContain("/repo/apps/api"); + expect(harnessPushes).toHaveLength(0); }); test("a revisit tells the model which escalation approaches were already exhausted", async () => { diff --git a/packages/core/tests/boringstack-gate-stages.test.ts b/packages/core/tests/boringstack-gate-stages.test.ts index f39f3e99..fc39edff 100644 --- a/packages/core/tests/boringstack-gate-stages.test.ts +++ b/packages/core/tests/boringstack-gate-stages.test.ts @@ -25,15 +25,20 @@ 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. +// The command stage runs setup steps (eslint-cache clear, lint:fix, format, db:push, and the +// db-push recovery's `bun -e` drop) BEFORE the gate. Those succeed independently; 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 db:push short-circuit. const execWith = (code: number, stdout: string): Exec => async (argv) => { const cmd = argv.join(" "); + const isDrop = argv[1] === "-e"; - if (/lint:fix|(?:^|\s)format(?:\s|$)|db:push|eslintcache/u.test(cmd)) { + if ( + isDrop || + /lint:fix|(?:^|\s)format(?:\s|$)|db:push|eslintcache/u.test(cmd) + ) { return { code: 0, stdout: "", stderr: "" }; } @@ -78,6 +83,30 @@ describe("boringstackCommandStage", () => { expect(r.errors).toEqual([]); }); + test("db:push that never migrates (rename crash survives recovery) → stage FAILS, never false-greens", async () => { + // db:push always emits the headless rename crash (exits 0 but never migrated); + // the gate command itself would be "green". The stage must short-circuit on the + // failed migration and NOT report the green gate. + const crash = + "Error: Interactive prompts require a TTY terminal\n at promptColumnsConflicts"; + + const exec: Exec = async (argv) => { + if (argv[1] === "run" && argv[2] === "db:push") { + return { code: 0, stdout: "", stderr: crash }; + } + + return { code: 0, stdout: "all good", stderr: "" }; + }; + + const stage = boringstackCommandStage("/tmp/clone", exec, [], "bookmark"); + const r = await stage.run("/tmp/clone"); + + expect(r.passed).toBe(false); + // Surfaced via #60's db:push error reporter (fingerprinted key, schema-vs-infra guidance). + expect(r.errors[0]?.rule).toBe("db-push"); + expect(r.errors[0]?.message).toContain("column"); + }); + test("red gate → each failure signature becomes an IErrorItem (key = signature)", async () => { const out = "1:1 error Unexpected no-console\nerror TS2322: bad"; const stage = boringstackCommandStage("/tmp/clone", execWith(1, out)); diff --git a/packages/core/tests/boringstack-generate.test.ts b/packages/core/tests/boringstack-generate.test.ts index 9d1e8be3..9261377e 100644 --- a/packages/core/tests/boringstack-generate.test.ts +++ b/packages/core/tests/boringstack-generate.test.ts @@ -96,6 +96,33 @@ describe("generateResource", () => { await rm(tmpDir, { recursive: true, force: true }); } }); + + test("throws when db:push cannot migrate (headless rename crash) — never proceeds on a stale DB", async () => { + // Everything succeeds EXCEPT db:push, which emits drizzle-kit's interactive rename + // crash on every attempt (exits 0 — the swallow). dbPushForce cannot recover, so it + // must surface a non-zero result and generateResource must THROW (not proceed to + // generate:api against a DB that never got the new columns → runtime 500s). + const tmpDir = await createTestEnv(); + + try { + const crash = + "Error: Interactive prompts require a TTY terminal\n at promptColumnsConflicts"; + + const exec = async (argv: readonly string[]) => { + if (argv[1] === "run" && argv[2] === "db:push") { + return { code: 0, stdout: "", stderr: crash }; + } + + return { code: 0, stdout: "", stderr: "" }; + }; + + await expect(generateResource(tmpDir, "Invoice", exec)).rejects.toThrow( + /Interactive prompts require a TTY/ + ); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } + }); }); describe("generateResource idempotency (retry-safe)", () => { diff --git a/packages/core/tests/boringstack-refine-prompt.test.ts b/packages/core/tests/boringstack-refine-prompt.test.ts index a722bb59..952983a2 100644 --- a/packages/core/tests/boringstack-refine-prompt.test.ts +++ b/packages/core/tests/boringstack-refine-prompt.test.ts @@ -738,8 +738,12 @@ describe("refinePrompt", () => { // Teaches the void-operator wrap that makes the handler return void. expect(prompt).toContain("void handleSubmit(onSubmit)(event)"); // The submit handler uses fire-and-forget `mutate` (errors → onError), NOT `mutateAsync` - // which would reject and make the `void` wrapper discard a rejected promise. - expect(prompt).toContain("createMutation.mutate(input)"); + // which would reject and make the `void` wrapper discard a rejected promise — AND it + // closes the form on success via a per-call onSuccess (the deterministic acceptance wall + // both A/B control builds parked on: form submits but never hides). + expect(prompt).toContain( + "createMutation.mutate(input, { onSuccess: closeForm })" + ); expect(prompt).toContain("unhandled rejection"); // Must steer AWAY from copying LoginPage verbatim (it uses mutateAsync+try/catch to // navigate on the result) so the model doesn't paste mutateAsync into the void wrapper. @@ -757,10 +761,20 @@ describe("refinePrompt", () => { // that caused build53's rename churn. expect(prompt).toContain("CompanyCreateInput"); expect(prompt).not.toContain("ICompanyCreateInput"); - // The useCallback depends on the STABLE `mutate`, not the unstable mutation result object, - // so the handler stays referentially stable (avoids TanStack's no-unstable-deps). - expect(prompt).toContain("[createMutation.mutate]"); + // The useCallback depends on the STABLE `mutate` (+ the stable `closeForm`), not the + // unstable mutation result object, so the handler stays referentially stable + // (avoids TanStack's no-unstable-deps). + expect(prompt).toContain("[createMutation.mutate, closeForm]"); expect(prompt).toContain("no-unstable-deps"); + // Close-on-success is taught as a REQUIRED behaviour, not optional polish — this is the + // fix for the form-doesn't-hide acceptance wall. Lock the idiom's key pieces. + expect(prompt).toContain("Close the create/edit form on SUCCESS"); + expect(prompt).toContain("waits for the form to DISAPPEAR"); + expect(prompt).toContain("closeForm"); + expect(prompt).toContain("openCreate"); + expect(prompt).toContain("view.showForm"); + // Must NOT teach optimistic pre-resolve closing (would hide the form on a failed create). + expect(prompt).toContain("close it ONLY in `onSuccess`"); }); it("warns that every NEW component needs the FULL sibling set → don't extract sub-components", () => { diff --git a/packages/core/tests/db-push.test.ts b/packages/core/tests/db-push.test.ts new file mode 100644 index 00000000..27a38f8c --- /dev/null +++ b/packages/core/tests/db-push.test.ts @@ -0,0 +1,138 @@ +import { test, expect } from "bun:test"; +import { dbPushForce } from "../src/loop/boringstack/db-push"; +import type { Exec, IExecResult } from "../src/loop/boringstack/exec"; + +const ok: IExecResult = { code: 0, stdout: "Changes applied", stderr: "" }; + +/** The exact drizzle-kit failure a name-less plan triggers headlessly. NOTE the + * `code: 0` — `bun run db:push` exits ZERO on this crash (async rejection is + * swallowed), so the signature must be detected from the OUTPUT, not the exit code. + * This is the regression the fix exists for. */ +const RENAME_PROMPT_FAIL: IExecResult = { + code: 0, + stdout: "[✓] Pulling schema from database...", + stderr: + "Error: Interactive prompts require a TTY terminal (process.stdin.isTTY " + + "is false).\n at promptColumnsConflicts\n at columnsResolver", +}; + +/** Record every argv the exec is asked to run, and return scripted results. */ +function recordingExec(results: IExecResult[]): { + exec: Exec; + calls: string[][]; +} { + const calls: string[][] = []; + let i = 0; + + const exec: Exec = (argv) => { + calls.push([...argv]); + const r = results[i] ?? ok; + + i += 1; + + return Promise.resolve(r); + }; + + return { exec, calls }; +} + +test("dbPushForce: push succeeds → single push, no drop", async () => { + const { exec, calls } = recordingExec([ok]); + + const res = await dbPushForce("/api", exec, "bookmark"); + + expect(res.code).toBe(0); + expect(calls).toHaveLength(1); + expect(calls[0]).toEqual(["bun", "run", "db:push", "--", "--force"]); +}); + +test("dbPushForce: rename-prompt failure → drops the entity table, then retries a clean push", async () => { + const { exec, calls } = recordingExec([RENAME_PROMPT_FAIL, ok, ok]); + + const res = await dbPushForce("/api", exec, "bookmark"); + + expect(res.code).toBe(0); + expect(calls).toHaveLength(3); + // 1: initial push (fails), 2: drop via bun -e, 3: retry push (clean create) + expect(calls[0]).toEqual(["bun", "run", "db:push", "--", "--force"]); + expect(calls[1]?.[0]).toBe("bun"); + expect(calls[1]?.[1]).toBe("-e"); + expect(calls[1]?.[2] ?? "").toContain( + 'DROP TABLE IF EXISTS "app"."bookmark" CASCADE' + ); + expect(calls[2]).toEqual(["bun", "run", "db:push", "--", "--force"]); +}); + +test("dbPushForce: failure that is NOT the rename prompt → NOT masked (no drop, no retry)", async () => { + // A genuinely broken schema (the model's own compile error) must surface, not be + // hidden by a table drop + retry. + const brokenSchema: IExecResult = { + code: 1, + stdout: "", + stderr: 'error: relation "app.foo" type "jsonb" is not defined', + }; + const { exec, calls } = recordingExec([brokenSchema]); + + const res = await dbPushForce("/api", exec, "bookmark"); + + expect(res.code).toBe(1); + expect(calls).toHaveLength(1); +}); + +test("dbPushForce: rename-prompt crash but NO entityTable → cannot recover, surfaces as non-zero (never false-green)", async () => { + const { exec, calls } = recordingExec([RENAME_PROMPT_FAIL]); + + const res = await dbPushForce("/api", exec); + + // The crash exits 0, but a non-recoverable swallowed crash MUST be normalized to a + // non-zero code so the caller/gate can never read it as success. + expect(res.code).not.toBe(0); + expect(res.stderr).toContain("Interactive prompts require a TTY"); + expect(calls).toHaveLength(1); +}); + +test("dbPushForce: an unsafe (non-identifier) entityTable is never interpolated into SQL, and surfaces as non-zero", async () => { + const { exec, calls } = recordingExec([RENAME_PROMPT_FAIL]); + + const res = await dbPushForce("/api", exec, "bookmark; DROP TABLE users;--"); + + // Rejected by the identifier guard → no drop, no retry — but still surfaced. + expect(res.code).not.toBe(0); + expect(res.stderr).toContain("Interactive prompts require a TTY"); + expect(calls).toHaveLength(1); +}); + +test("dbPushForce: retry STILL crashes (e.g. drop was a no-op) → surfaces as non-zero, never false-green", async () => { + // First push crashes → drop → retry ALSO crashes (DATABASE_URL unset so the drop + // no-op'd). The persistent swallowed crash must be normalized to non-zero. + const { exec, calls } = recordingExec([ + RENAME_PROMPT_FAIL, + ok, // the drop exec + RENAME_PROMPT_FAIL, // retry still crashes + ]); + + const res = await dbPushForce("/api", exec, "bookmark"); + + expect(res.code).not.toBe(0); + expect(res.stderr).toContain("Interactive prompts require a TTY"); + expect(calls).toHaveLength(3); +}); + +test("dbPushForce: a throwing drop exec does not reject — the retry is the source of truth", async () => { + let call = 0; + + const exec: Exec = (argv) => { + call += 1; + + if (argv[1] === "-e") { + return Promise.reject(new Error("drop connection refused")); + } + + // push #1 crashes, retry (#2 push) succeeds cleanly + return Promise.resolve(call === 1 ? RENAME_PROMPT_FAIL : ok); + }; + + const res = await dbPushForce("/api", exec, "bookmark"); + + expect(res.code).toBe(0); +}); diff --git a/packages/core/tests/near-green-checkpoint-loop.test.ts b/packages/core/tests/near-green-checkpoint-loop.test.ts index 169dd4a6..b8bf9103 100644 --- a/packages/core/tests/near-green-checkpoint-loop.test.ts +++ b/packages/core/tests/near-green-checkpoint-loop.test.ts @@ -634,6 +634,145 @@ test("rollbackNearGreen resets the convergence guards + tombstones, without touc } }); +test("rollbackNearGreen SHOWS the model the new errors its reverted change introduced (spray delta), not just the ones it was already at", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); + + try { + await Bun.write(join(dir, "feature.ts"), "export const GOOD = 1;\n"); + const ctx: ILoopCtx = { + task: { + id: "t", + intent: "test", + accept: "", + files: ["**/*"], + context: [], + }, + cwd: dir, + tsService: null, + report: () => undefined, + messages: [], + tool: { touched: new Set(["a.ts"]) }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => ({ + passed: false, + errors: [], + output: "", + }), + }, + }, + }; + const checkpoint = await captureNearGreenCheckpoint(ctx, 1, [ + { key: "cp1", message: "the one remaining testid error" }, + ]); + const state: ILoopState = { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: 5, + noNewLow: 0, + redGates: 0, + errorAge: new Map(), + lastGateCount: 5, + edits: 0, + regressions: 1, + ttsrInterrupts: 0, + steerLevel: 0, + conventionsEnabled: false, + plateauBest: 1, + nearGreenCheckpoint: checkpoint, + nearGreenRollbacks: 0, + }; + + // The spray still carries the original cp error PLUS two it just introduced. + await rollbackNearGreen(ctx, state, 5, [ + { key: "cp1", message: "the one remaining testid error" }, + { + key: "n1", + message: "JSX props should not use arrow functions", + rule: "react/jsx-no-bind", + }, + { + key: "n2", + message: "unsafe argument of type any", + rule: "no-unsafe-argument", + }, + ]); + + const msg = ctx.messages.at(-1)?.content ?? ""; + + // The remaining error is still shown… + expect(msg).toContain("the one remaining testid error"); + // …AND the model is told exactly what it broke (the delta vs the checkpoint), framed as introduced. + expect(msg).toContain("INTRODUCED"); + expect(msg).toContain("JSX props should not use arrow functions"); + expect(msg).toContain("unsafe argument of type any"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("rollbackNearGreen with NO spray errors passed omits the 'introduced' section (backward-compatible)", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); + + try { + await Bun.write(join(dir, "feature.ts"), "export const GOOD = 1;\n"); + const ctx: ILoopCtx = { + task: { + id: "t", + intent: "test", + accept: "", + files: ["**/*"], + context: [], + }, + cwd: dir, + tsService: null, + report: () => undefined, + messages: [], + tool: { touched: new Set() }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => ({ + passed: false, + errors: [], + output: "", + }), + }, + }, + }; + const checkpoint = await captureNearGreenCheckpoint(ctx, 1, [ + { key: "cp1", message: "the one remaining error" }, + ]); + const state: ILoopState = { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: 5, + noNewLow: 0, + redGates: 0, + errorAge: new Map(), + lastGateCount: 5, + edits: 0, + regressions: 1, + ttsrInterrupts: 0, + steerLevel: 0, + conventionsEnabled: false, + plateauBest: 1, + nearGreenCheckpoint: checkpoint, + nearGreenRollbacks: 0, + }; + + await rollbackNearGreen(ctx, state, 5); + + const msg = ctx.messages.at(-1)?.content ?? ""; + + expect(msg).toContain("the one remaining error"); + expect(msg).not.toContain("INTRODUCED"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + test("rollbackNearGreen reverts OUT-OF-SCOPE package.json + binary lockfile (ROLLBACK_EXTRA_FILES)", async () => { const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); diff --git a/packages/core/tests/near-green-checkpoint.test.ts b/packages/core/tests/near-green-checkpoint.test.ts index 0cf802b6..db45072f 100644 --- a/packages/core/tests/near-green-checkpoint.test.ts +++ b/packages/core/tests/near-green-checkpoint.test.ts @@ -27,6 +27,11 @@ test("isCompletionClass: reliable add-code rules (reachability/i18n-locale-keys- // #61/#65: a missing colocated test clears ONLY by ADDING the test file — reliably add-only, so // WS-B must not revert the logic modules the model just added (build34/36 ground on this). expect(isCompletionClass(err("logic-files-require-test-sibling"))).toBe(true); + // #61/panel: a missing required test-id or an unwired feature clears ONLY by ADDING the create/ + // edit/delete UI (a hollow list-only page) — reliably add-only, so WS-B must not revert the UI + // the model just added (valbuild23 parked on testid-presence exactly this way). + expect(isCompletionClass(err("testid-presence"))).toBe(true); + expect(isCompletionClass(err("feature-wiring"))).toBe(true); // `judge` is EXCLUDED — it can reject defects in existing code, not only hollowness, so it // isn't a reliable add-only signal (would falsely disable WS-B on a fixable judge rejection). expect(isCompletionClass(err("judge"))).toBe(false);