From 900750cc4359fd0d2e1bba920dd61c40770cb20a Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Tue, 15 Sep 2026 10:34:40 +0200 Subject: [PATCH] fix(agent): make the tooling's own tests hold in a product checkout Three tooling tests assumed the template's shape and failed in the first product built on it: - the generator tests created a Projects resource, which the product already owned; they now pick the first free name from a candidate list, and the record-scope mutant takes the table name instead of assuming projects. - the generator itself broke on a product with a few subjects: Prettier formats a wrapped union with leading pipes, and splicing X | after the equals sign produced X | | Y. The union is rebuilt from its members. - the pre-push regression asserted the exact compose profile list but pinned only two of the five optional overlays; a product with Mailpit and BullMQ enabled in compose/.env added their profiles. All five are pinned off in that step. --- scripts/ci/pre-push-regression.sh | 4 ++ tools/agent-evals/mutants.ts | 10 ++-- tools/agent/generate/account-resource.ts | 48 +++++++++++++++-- tools/agent/generate/fixture.ts | 26 ++++++++++ tools/agent/generate/generate.test.ts | 66 +++++++++++++++++++++--- tools/agent/hardening.test.ts | 14 +++-- 6 files changed, 150 insertions(+), 18 deletions(-) diff --git a/scripts/ci/pre-push-regression.sh b/scripts/ci/pre-push-regression.sh index 0e9c0c47..3c5a5361 100755 --- a/scripts/ci/pre-push-regression.sh +++ b/scripts/ci/pre-push-regression.sh @@ -86,7 +86,11 @@ echo 'PASS local startup forwards selected host ports' # project flags. Stub docker captures arguments without contacting a daemon. cp "$ROOT/infra/compose/compose/dev.sh" "$FIXTURE/infra/compose/compose/dev.sh" printf 'POSTGRES_HOST_PORT=5432\nVALKEY_HOST_PORT=6379\n' > "$FIXTURE/ports.env" +# Every optional overlay is pinned off: a product checkout may enable Mailpit, +# BullMQ or WUD in its compose .env, and the assertion below is the exact +# profile list. ENV_FILE="$FIXTURE/ports.env" STACK=dev WITH_OBSERVABILITY=0 WITH_GLITCHTIP=0 \ + WITH_MAILPIT=0 WITH_BULLMQ=0 WITH_WUD=0 \ POSTGRES_HOST_PORT=55432 VALKEY_HOST_PORT=56379 \ /bin/bash "$FIXTURE/infra/compose/compose/dev.sh" config assert_contains "$FIXTURE/docker" '--profile dev config|55432|56379' diff --git a/tools/agent-evals/mutants.ts b/tools/agent-evals/mutants.ts index 5ec152af..83d38c5f 100644 --- a/tools/agent-evals/mutants.ts +++ b/tools/agent-evals/mutants.ts @@ -1,11 +1,15 @@ /** Deliberate edits to the trusted reference only; candidates need not share its source shape. */ export function withoutRecordAccountPredicate( source: string, - index: 0 | 1 + index: 0 | 1, + table = "projects" ): string { const matches = [ ...source.matchAll( - /and\(\s*eq\(projects.accountId, accountId\),\s*eq\(projects.id, id\),?\s*\)/g + new RegExp( + `and\\(\\s*eq\\(${table}\\.accountId, accountId\\),\\s*eq\\(${table}\\.id, id\\),?\\s*\\)`, + "g" + ) ), ]; @@ -21,7 +25,7 @@ export function withoutRecordAccountPredicate( return ( source.slice(0, match.index) + - "eq(projects.id, id)" + + `eq(${table}.id, id)` + source.slice(match.index + match[0].length) ); } diff --git a/tools/agent/generate/account-resource.ts b/tools/agent/generate/account-resource.ts index b5119788..ce9f922a 100644 --- a/tools/agent/generate/account-resource.ts +++ b/tools/agent/generate/account-resource.ts @@ -95,11 +95,7 @@ export function planAccountResource( return replaceOnce(source, anchors[0], `"${singular}", "all"] as const`); }); patch("apps/api/src/lib/acl/acl.types.ts", (source) => - replaceOnce( - source, - "export type SubjectInstance =", - `export interface I${singular}Subject extends ForcedSubject<"${singular}"> { readonly accountId: string; }\n\nexport type SubjectInstance = I${singular}Subject |` - ) + addSubjectInstance(source, singular) ); patch("apps/api/src/lib/acl/ability.ts", (source) => replaceOnce( @@ -220,3 +216,45 @@ export function planAccountResource( return changes; } + +const SUBJECT_UNION = /export type SubjectInstance =([\s\S]*?);/; + +/** + * Prettier switches the union to leading pipes once it wraps, so splicing + * `X |` after the equals sign breaks a product that already has a few + * subjects. Rebuild the declaration from its members instead; the formatter + * settles the final layout. + */ +export function addSubjectInstance(source: string, singular: string): string { + const match = SUBJECT_UNION.exec(source); + + if ( + match === null || + source.slice(match.index + 1).includes("export type SubjectInstance =") + ) { + throw new Error("Expected one patch anchor: export type SubjectInstance"); + } + + const subject = `I${singular}Subject`; + const members = (match[1] ?? "") + .split("|") + .map((member) => member.trim()) + .filter((member) => member !== ""); + + if (members.includes(subject)) { + throw new Error(`Subject already declared: ${subject}`); + } + + const declaration = [ + `export interface ${subject} extends ForcedSubject<"${singular}"> { readonly accountId: string; }`, + "", + "export type SubjectInstance =", + ...[subject, ...members].map((member) => ` | ${member}`), + ].join("\n"); + + return ( + source.slice(0, match.index) + + `${declaration};` + + source.slice(match.index + match[0].length) + ); +} diff --git a/tools/agent/generate/fixture.ts b/tools/agent/generate/fixture.ts index 229fe9ba..52216277 100644 --- a/tools/agent/generate/fixture.ts +++ b/tools/agent/generate/fixture.ts @@ -56,3 +56,29 @@ export function copyFixture(root: string, destination: string): void { ); } } + +const FIXTURE_RESOURCE_CANDIDATES = [ + "Projects", + "Widgets", + "Ledgers", + "Beacons", + "Quotas", + "Tickets", +] as const; + +/** + * Resource names the generator can create in this checkout. A product built + * on the template may already own `projects` or `widgets`; the tooling tests + * must exercise the generator without colliding with what the product ships. + */ +export function fixtureResourceNames(root: string, count: number): string[] { + const free = FIXTURE_RESOURCE_CANDIDATES.filter( + (name) => !existsSync(join(root, "apps/api/src/api", name.toLowerCase())) + ); + + if (free.length < count) { + throw new Error("Not enough free fixture resource names in this checkout"); + } + + return free.slice(0, count); +} diff --git a/tools/agent/generate/generate.test.ts b/tools/agent/generate/generate.test.ts index 27e75a27..ed280895 100644 --- a/tools/agent/generate/generate.test.ts +++ b/tools/agent/generate/generate.test.ts @@ -11,13 +11,21 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { planAccountResource } from "./account-resource"; -import { copyFixture } from "./fixture"; +import { addSubjectInstance, planAccountResource } from "./account-resource"; +import { copyFixture, fixtureResourceNames } from "./fixture"; import { formatEdits } from "./format"; import { apply, replaceOnce } from "./patch"; const root = join(import.meta.dir, "../../.."); +const requireName = (name: string | undefined): string => { + if (name === undefined) { + throw new Error("Fixture resource name is absent"); + } + + return name; +}; + test("all patch preconditions are checked before any write", () => { const dir = mkdtempSync(join(tmpdir(), "bs-patch-")); @@ -85,13 +93,18 @@ test("account generation works in a stripped and already extended checkout", asy writeFileSync(join(dir, dependency), original + "\n"); expect(readFileSync(join(root, dependency), "utf8")).toBe(original); + const [primary, secondary] = fixtureResourceNames(dir, 2); + const primaryName = requireName(primary); + const secondaryName = requireName(secondary); const first = await formatEdits( dir, - planAccountResource(dir, "Projects", "team-read-admin-write") + planAccountResource(dir, primaryName, "team-read-admin-write") ); apply(dir, first, true); - expect(existsSync(join(dir, "apps/api/src/api/projects"))).toBe(false); + expect( + existsSync(join(dir, "apps/api/src/api", primaryName.toLowerCase())) + ).toBe(false); apply(dir, first); const before = readFileSync( join(dir, "apps/api/src/config/routes/routes.ts"), @@ -99,7 +112,7 @@ test("account generation works in a stripped and already extended checkout", asy ); expect(() => - planAccountResource(dir, "Projects", "team-read-admin-write") + planAccountResource(dir, primaryName, "team-read-admin-write") ).toThrow(); expect( readFileSync(join(dir, "apps/api/src/config/routes/routes.ts"), "utf8") @@ -108,12 +121,12 @@ test("account generation works in a stripped and already extended checkout", asy dir, await formatEdits( dir, - planAccountResource(dir, "Widgets", "team-read-admin-write") + planAccountResource(dir, secondaryName, "team-read-admin-write") ) ); expect( readFileSync(join(dir, "apps/api/src/lib/acl/acl.constants.ts"), "utf8") - ).toContain('"Widget"'); + ).toContain(`"${secondaryName.slice(0, -1)}"`); expect( readFileSync(join(dir, "apps/api/src/api/widgets/index.ts"), "utf8") ).toContain('export { widgetsService } from "./widgets.service"'); @@ -125,3 +138,42 @@ test("account generation works in a stripped and already extended checkout", asy rmSync(dir, { recursive: true, force: true }); } }, 120000); + +test("a new subject joins the union in either Prettier layout", () => { + const trailing = `export type SubjectInstance = + ITeamMemberSubject | ISiteSubject | IAccountSubject; + +export type AppSubject = Subject | SubjectInstance;`; + const leading = `export type SubjectInstance = + | IWidgetSubject + | IProjectSubject + | ITeamMemberSubject;`; + const membersOf = (source: string): string[] => + (/export type SubjectInstance =([\s\S]*?);/.exec(source)?.[1] ?? "") + .split("|") + .map((member) => member.trim()) + .filter((member) => member !== ""); + + for (const source of [trailing, leading]) { + const result = addSubjectInstance(source, "Ledger"); + const members = membersOf(result); + + expect(result).toContain( + 'export interface ILedgerSubject extends ForcedSubject<"Ledger">' + ); + expect(members).toEqual(["ILedgerSubject", ...membersOf(source)]); + expect(result).not.toContain("| |"); + // Everything after the union declaration is untouched. + expect(result.endsWith(source.slice(source.indexOf(";") + 1))).toBe(true); + } + + expect(() => addSubjectInstance(leading, "Widget")).toThrow( + "already declared" + ); + expect(() => addSubjectInstance("export type Other = A;", "Ledger")).toThrow( + "anchor" + ); + expect(() => + addSubjectInstance(`${trailing}\n${trailing}`, "Ledger") + ).toThrow("anchor"); +}); diff --git a/tools/agent/hardening.test.ts b/tools/agent/hardening.test.ts index 5f05f3a4..ac9d4e5f 100644 --- a/tools/agent/hardening.test.ts +++ b/tools/agent/hardening.test.ts @@ -11,6 +11,7 @@ import { import { tmpdir } from "node:os"; import { join } from "node:path"; import { withoutRecordAccountPredicate } from "../agent-evals/mutants"; +import { fixtureResourceNames } from "./generate/fixture"; import { identifyCheckout } from "./checkout"; import { planAccountResource } from "./generate/account-resource"; import { formatEdits } from "./generate/format"; @@ -162,19 +163,26 @@ test("UI wrapper distinguishes coverage and warning failures from process errors }); test("record-scope mutants hit both reference predicates and refuse drift", async () => { + const name = requireValue( + fixtureResourceNames(root, 1)[0], + "Fixture resource name is absent" + ); + const table = name.toLowerCase(); const edits = await formatEdits( root, - planAccountResource(root, "Projects", "team-read-admin-write") + planAccountResource(root, name, "team-read-admin-write") ); const service = requireValue( edits.find((plannedEdit) => - plannedEdit.path.endsWith("projects.service.ts") + plannedEdit.path.endsWith(`${table}.service.ts`) ), "Required fixture edit is absent" ).after; for (const index of [0, 1] as const) { - expect(withoutRecordAccountPredicate(service, index)).not.toBe(service); + expect(withoutRecordAccountPredicate(service, index, table)).not.toBe( + service + ); } expect(() =>