diff --git a/.gitignore b/.gitignore index 4c841288..4eb8e772 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,6 @@ packages/core/scripts/gen-plan.ts # harness-generated acceptance specs + playwright artifacts (never commit) apps/*/e2e/_acceptance/ test-results/ + +# local build/panel logs (never commit) +/scratchpad-*.log* diff --git a/packages/core/scripts/headless-build.ts b/packages/core/scripts/headless-build.ts index 30818f3e..10b3a2a8 100644 --- a/packages/core/scripts/headless-build.ts +++ b/packages/core/scripts/headless-build.ts @@ -83,6 +83,29 @@ export function resolveWorkspaceDir(dir: string): string { } } +/** + * The autonomous builder defaults expert rescue ON so a stalled feature is + * handed to the configured `capabilities.expert` model instead of parking. + * Explicit env wins: a caller that already set TSFORGE_EXPERT_RESCUE (including + * "0" to opt out) keeps their value; only an unset flag defaults to "1". + */ +export function resolveExpertRescueFlag(current: string | undefined): string { + return current ?? "1"; +} + +/** + * Apply the expert-rescue default onto an environment object (the wiring around + * {@link resolveExpertRescueFlag}). Mutates `env.TSFORGE_EXPERT_RESCUE` in place so + * the autonomous builder defaults it on while an explicit value is preserved. + */ +export function applyExpertRescueDefault( + env: Record +): void { + env.TSFORGE_EXPERT_RESCUE = resolveExpertRescueFlag( + env.TSFORGE_EXPERT_RESCUE + ); +} + /** * Parse headless-build command-line arguments into typed fields. * Flags (--log-file, --plan) and positionals (prompt, dir) can appear in any order. @@ -352,6 +375,11 @@ async function main(): Promise { process.env.VALKEY_HOST ??= "localhost"; process.env.VALKEY_PORT ??= String(valkeyPort); + // This is a real autonomous builder: enable expert rescue by default so a + // stalled feature is handed to the configured `capabilities.expert` model + // instead of parking. Explicit env wins (set TSFORGE_EXPERT_RESCUE=0 to opt out). + applyExpertRescueDefault(process.env); + process.stdout.write( `isolated ports → postgres ${String(pgPort)} · api ${String(apiPort)} · valkey ${String(valkeyPort)}\n` ); diff --git a/packages/core/src/loop/boringstack/acceptance/e2e-generator.ts b/packages/core/src/loop/boringstack/acceptance/e2e-generator.ts index d10edfdc..3e81c752 100644 --- a/packages/core/src/loop/boringstack/acceptance/e2e-generator.ts +++ b/packages/core/src/loop/boringstack/acceptance/e2e-generator.ts @@ -227,6 +227,121 @@ function generateParentSeedingCode( return codeBlocks.join("\n"); } +/** + * Render a field value to a code literal string. + * For numeric types (exact match), renders as a bare number or falls back to string for NaN. + * For boolean types (exact match), renders as a bare boolean. + * For everything else, renders as a JSON string. + * Complexity: ≤ 20 (normalized type match with fallback). + */ +function renderFieldValue(field: { type: string; valid: string }): string { + const normalized = field.type.toLowerCase().trim(); + + // Numeric types: exact match against known numeric type names + const numericTypes = new Set([ + "number", + "integer", + "int", + "float", + "double", + "decimal", + "numeric", + ]); + + if (numericTypes.has(normalized)) { + const num = Number(field.valid); + + // Guard NaN → fall back to JSON string + if (Number.isNaN(num)) { + return JSON.stringify(field.valid); + } + + return String(num); + } + + // Boolean types: exact match against known boolean type names + const booleanTypes = new Set(["boolean", "bool"]); + + if (booleanTypes.has(normalized)) { + return field.valid.trim().toLowerCase() === "true" ? "true" : "false"; + } + + // Everything else: JSON string + return JSON.stringify(field.valid); +} + +/** + * Render an invalid override value (for negative tests) to a code literal string. + * Type-aware rendering: tests the constraint with bare types (not strings). + * Rules: + * - empty string "" → stays as "" (tests required-empty constraint) + * - numeric field + token round-trips exactly (`String(Number(value)) === value`) → + * bare number (tests numeric constraint); NON-canonical tokens ("01", "1e5", "0x10", + * " 5 ", huge→Infinity) → JSON string (Number() would mutate them / bare "01" is an + * octal SyntaxError in strict mode) + * - boolean field + value is EXACTLY "true"/"false" → bare boolean (no trim/case-fold) + * - boolean field + other token (incl. " false ", "FALSE") → JSON string (type-rejection) + * - everything else → JSON string (default fallback) + * Complexity: ≤ 20 (three type checks with normalized matching). + */ +function renderInvalidOverride(field: { type: string }, value: string): string { + // Empty string: stay as empty string literal to test required-empty constraint + if (value === "") { + return '""'; + } + + const normalized = field.type.toLowerCase().trim(); + + // Numeric types: bare number for finite values (tests numeric constraint) + const numericTypes = new Set([ + "number", + "integer", + "int", + "float", + "double", + "decimal", + "numeric", + ]); + + if (numericTypes.has(normalized)) { + // Emit a bare number ONLY when the token ROUND-TRIPS exactly through Number: + // `String(Number(value)) === value`. This is the true canonical-literal test — + // it accepts "-1"/"1.5" (constraint probes) but rejects tokens Number() would + // MUTATE or that are invalid JS: "01"/"00" (would be an octal SyntaxError in the + // strict-mode spec, and Number("01")→1), "1e5"→100000, "0x10"→16, " 5 "→5, and + // huge digit strings → Infinity. Those fall through to a raw string, which + // exercises type-rejection instead of silently testing a different value. + const asNumber = Number(value); + + if (Number.isFinite(asNumber) && String(asNumber) === value) { + return value; + } + + return JSON.stringify(value); + } + + // Boolean types: bare boolean ONLY for the exact tokens "true"/"false". Any other + // token — including " false " or "FALSE" — is sent as a raw string so we test the + // EXACT value the plan specified (type/value rejection), never a folded valid bool. + const booleanTypes = new Set(["boolean", "bool"]); + + if (booleanTypes.has(normalized)) { + if (value === "true") { + return "true"; + } + + if (value === "false") { + return "false"; + } + + // Non-boolean token (e.g., "notabool") for a boolean field: JSON string to test type-rejection + return JSON.stringify(value); + } + + // Everything else: JSON string + return JSON.stringify(value); +} + /** * Generate field fill steps for form inputs. * FIX D: Type-aware field fills (select/checkbox/date/number) instead of blanket .fill(). @@ -348,61 +463,88 @@ function generateRowCellAssertions( /** * Generate negative test blocks. - * Safely interpolates field names, invalid values, and entity name using JSON.stringify. + * B2: Ensures test title and assertions use JSON.stringify for injection-safe escaping. + * B3: Overrides the invalid field using renderInvalidOverride (type-aware), except + * required-empty "" stays as empty string to test missing-required constraint. * * Negatives work by: - * 1. Record initial row count before creating the form - * 2. Open create form and emit parentSeedingCode (so FK variables are declared) - * 3. Fill all fields validly - * 4. Override ONLY the target field with invalid value - * 5. Submit and reload - * 6. Assert row count did NOT increase — the invalid record was rejected and not persisted + * 1. Authenticate and seed parent entities via API + * 2. Build a payload with all required non-FK fields (valid values) + FK fields (seeded ids) + * 3. Override the target field with the invalid value (type-rendered via renderInvalidOverride) + * 4. POST directly to /api/v1/ and assert a 400 or 422 response (validation error codes) * - * This approach is robust: it doesn't depend on visible error elements and reliably - * distinguishes API rejection from form validation failures. + * This deterministic API-level check proves validation is enforced without depending on + * browser form interaction or visible error elements. Only accepting 400/422 prevents + * false-positive passes from 401/403/404/409 auth/routing/conflict errors. */ function generateNegativeBlocks( entity: IEntityAcceptance, - ids: ReturnType, - fieldFillSteps: string, parentSeedingCode: string ): string { return entity.negatives .map((neg) => { - const fieldTestId = ids.field(neg.field); + // B2: Test title uses JSON.stringify to escape backticks/interpolation in neg.value const testTitle = `negative: ${entity.id} rejects ${neg.field}=${neg.value}`; - return ` test(${JSON.stringify(testTitle)}, async ({ page, authedPage }) => { - await authedPage.dashboard.goto(); - await navigateTo${entity.id}(page); + // Build valid field assignments (required non-FK fields) + const validFieldAssignments = entity.fields + .filter((f) => !f.optional) + .filter((f) => !entity.parents.some((p) => p.fkField === f.name)) + .map((f) => ` ${f.name}: ${renderFieldValue(f)}`); - // Record initial row count before attempting to create an invalid record - const rowsBefore = await page.getByTestId("${ids.row}").count(); + // Build FK field assignments + const fkFieldAssignments = entity.parents.map( + (p) => ` ${p.fkField}: ${p.key}Id` + ); - // Open create form - await page.getByTestId("${ids.create}").click(); - await page.getByTestId("${ids.form}").waitFor({ state: "visible", timeout: 5000 }); + // Combine all assignments, filtering out empties + const allAssignments = [ + ...validFieldAssignments, + ...fkFieldAssignments, + ].filter((s) => s.length > 0); + const payloadFields = allAssignments.join(",\n"); + + // B3: Render the override value type-aware via renderInvalidOverride to test the constraint + // with bare types (not strings). This ensures invalid numeric values like "-1" render as + // bare -1 (testing the constraint), and invalid booleans like "notabool" render as the + // string (testing type-rejection). + const fieldDef = entity.fields.find((f) => f.name === neg.field); + const overrideValue = renderInvalidOverride( + fieldDef ?? { type: "string" }, + neg.value + ); - // Emit parent seeding code so FK variables (e.g., companyId) are declared -${parentSeedingCode} + // B2: Build error message safely without backtick interpolation of plan data + const errorMsg = `expected ${entity.key} to reject ${neg.field} with a validation error (400/422)`; - // Fill all fields with valid values -${fieldFillSteps} + return ` test(${JSON.stringify(testTitle)}, async ({ page, authedPage }) => { + await authedPage.dashboard.goto(); - // Override the target field with the invalid value (clear first, then fill) - await page.getByTestId("${fieldTestId}").clear(); - await page.getByTestId("${fieldTestId}").fill(${JSON.stringify(neg.value)}); +${parentSeedingCode} - // Submit with invalid input - await page.getByTestId("${ids.submit}").click(); + const apiBase = process.env.VITE_API_BASE || "http://localhost:7331"; + const cookieHeader = await page + .context() + .cookies() + .then((cs) => cs.map((c) => \`\${c.name}=\${c.value}\`).join("; ")); - // Reload to ensure the invalid record would persist if accepted by the backend - await page.reload(); - await page.waitForURL(/\\/${entity.key}/); + // A payload that is valid EXCEPT for the field under test (overridden with the invalid value). + const payload: Record = { +${payloadFields} + }; + payload[${JSON.stringify(neg.field)}] = ${overrideValue}; - // Assert: the row count did NOT increase — the invalid record was rejected - const rowsAfter = await page.getByTestId("${ids.row}").count(); - await expect(rowsAfter).toBe(rowsBefore, { timeout: 5000 }); + const res = await page.request.post(\`\${apiBase}/api/v1/${entity.key}\`, { + headers: { "Content-Type": "application/json", Cookie: cookieHeader }, + data: payload, + }); + + // Invalid input MUST be rejected with a validation error (400 or 422), not any 4xx. + // 401/403/404/409 are auth/routing/conflict errors that do not prove field validation. + expect( + [400, 422].includes(res.status()), + ${JSON.stringify(errorMsg)} + \`, got \${res.status()}\` + ).toBe(true); }); `; }) @@ -442,12 +584,7 @@ export function generateEntitySpec( entity.id, spec ); - const negativeBlocks = generateNegativeBlocks( - entity, - ids, - fieldFillSteps, - parentSeedingCode - ); + const negativeBlocks = generateNegativeBlocks(entity, parentSeedingCode); // FIX E/FIX 3: Type-aware unique identity value // Find the first field with a string/text type (NOT email by type or name) for the unique marker diff --git a/packages/core/src/loop/boringstack/acceptance/e2e-runner.ts b/packages/core/src/loop/boringstack/acceptance/e2e-runner.ts index 17d6a6be..09803d97 100644 --- a/packages/core/src/loop/boringstack/acceptance/e2e-runner.ts +++ b/packages/core/src/loop/boringstack/acceptance/e2e-runner.ts @@ -144,44 +144,6 @@ function isInfraError(text: string): boolean { return infraIndicators.some((indicator) => text.includes(indicator)); } -/** - * Extract error messages from a parsed Playwright JSON report. - * Collects ONLY top-level report.errors[].message (not per-test errors). - * Per-test assertion errors must NOT drive suite-wide infra classification. - * Returns empty string if unparseable or no errors found. - */ -function extractReportErrorText(rawStdout: string): string { - if (rawStdout.trim().length === 0) { - return ""; - } - - let parsed: unknown; - - try { - parsed = JSON.parse(rawStdout); - } catch { - return ""; - } - - if (!isPlaywrightReport(parsed)) { - return ""; - } - - const errors: string[] = []; - - // Collect top-level report errors ONLY - // Global setup/launch failures land here; per-test assertion errors do not. - if (Array.isArray(parsed.errors)) { - for (const err of parsed.errors) { - if ("message" in err && typeof err.message === "string") { - errors.push(err.message); - } - } - } - - return errors.join("\n"); -} - /** * Extract the error message from a test result, if present. */ @@ -247,38 +209,39 @@ function walkSuites( } /** - * Parse Playwright JSON reporter output into IAcceptanceResult[]. - * Returns null if the output is not valid JSON (infra error case). + * Parse Playwright JSON output and extract acceptance results. + * Returns parsed report (or null if invalid JSON) and results array (may be empty). */ -function parsePlaywrightJSON( +function parsePlaywrightResults( jsonOut: string, entity: IEntityAcceptance -): IAcceptanceResult[] | null { +): { parsed: IPlaywrightReportType | null; results: IAcceptanceResult[] } { + const results: IAcceptanceResult[] = []; + if (jsonOut.trim().length === 0) { - return null; + return { parsed: null, results }; } - let parsed: unknown; + let parsedUnknown: unknown; try { - parsed = JSON.parse(jsonOut); + parsedUnknown = JSON.parse(jsonOut); } catch { - return null; + return { parsed: null, results }; } - if (!isPlaywrightReport(parsed)) { - return null; + if (!isPlaywrightReport(parsedUnknown)) { + return { parsed: null, results }; } - const results: IAcceptanceResult[] = []; - - if (parsed.suites !== undefined) { - for (const suite of parsed.suites) { + // Extract per-test results from the parsed report + if (parsedUnknown.suites !== undefined) { + for (const suite of parsedUnknown.suites) { walkSuites(suite, entity, results); } } - return results; + return { parsed: parsedUnknown, results }; } /** @@ -311,19 +274,30 @@ function cleanupFile(filePath: string): void { /** * Classify a nonzero exit: determine if it's infrastructure (should retry) or real failure. * Examines both stderr and parsed JSON report errors (if present). - * Whenever parseResult is not null, preserves it in the outcome and derives meaningful detail - * from the first failing result (the actual assertion text), with fallback to stderr. + * Preserves parseResult when the JSON was successfully parsed (parsedReport !== null), + * even if parseResult is empty (no matching tests). Distinguishes between: + * - parsedReport !== null: JSON parsed successfully (real test failure or no matches) + * - parsedReport === null: JSON unparseable (possibly infra issue) * Returns: { outcome, shouldRetry } where outcome is undefined for retry, or final result for done. */ function classifyNonzeroExit( result: { code: number; stdout: string; stderr: string }, - parseResult: IAcceptanceResult[] | null + parsedReport: IPlaywrightReportType | null, + parseResult: IAcceptanceResult[] ): { outcome?: IAcceptanceOutcome; shouldRetry: boolean; } { // Combine stderr with extracted report errors to form the complete error text - const reportErrorText = extractReportErrorText(result.stdout); + // Extract top-level report errors from the already-parsed report (no re-parse) + const reportErrorText = + parsedReport !== null && Array.isArray(parsedReport.errors) + ? parsedReport.errors + .map((err) => (typeof err.message === "string" ? err.message : "")) + .filter((msg) => msg.length > 0) + .join("\n") + : ""; + const combinedErrorText = result.stderr + (reportErrorText !== "" ? "\n" + reportErrorText : ""); @@ -336,9 +310,10 @@ function classifyNonzeroExit( } // No infra pattern detected — treat as real failure - // ALWAYS preserve parseResult when it's not null (never drop parsed results) - if (parseResult !== null) { - // Derive meaningful detail: use the first failing result's detail, or fall back to stderr + // Preserve parseResult whenever the JSON parsed successfully (parsedReport !== null) + // This includes both successful parses with matches AND successful parses with no matches + if (parsedReport !== null) { + // JSON parsed successfully: derive meaningful detail from results or stderr let detail = result.stderr; // Find the first failed result to extract its assertion message @@ -361,7 +336,7 @@ function classifyNonzeroExit( }; } - // Truly unparseable JSON + unknown error text stays a REAL failure (not infra) + // JSON unparseable + unknown error text — stay a REAL failure (not infra) return { outcome: { ok: false, @@ -377,27 +352,43 @@ function classifyNonzeroExit( /** * Process a single exec result for entity acceptance testing. - * Returns: { outcome, shouldRetry } where outcome is the final result or undefined to continue/retry. + * Parses Playwright JSON and extracts results. + * Returns: { outcome, shouldRetry, parseResult, parsed } where outcome is the final result or undefined to continue/retry. + * parsed is non-null only when JSON was successfully parsed (even if no specs matched). + * This distinction (null vs []) lets the runner preserve stale results from an earlier + * successful parse when a later attempt has unparseable JSON. */ -function processExecResult( +export function processExecResult( result: { code: number; stdout: string; stderr: string }, entity: IEntityAcceptance, requiredSteps: AcceptStep[] ): { outcome?: IAcceptanceOutcome; shouldRetry: boolean; + parseResult: IAcceptanceResult[]; + parsed: IPlaywrightReportType | null; } { - const parseResult = parsePlaywrightJSON(result.stdout, entity); + // Parse JSON and extract acceptance results + const { parsed: parsedReport, results: parseResult } = parsePlaywrightResults( + result.stdout, + entity + ); if (result.code === 0) { return { - outcome: summarize(parseResult ?? [], requiredSteps), + outcome: summarize(parseResult, requiredSteps), shouldRetry: false, + parseResult, + parsed: parsedReport, }; } // Nonzero exit code: classify as infra or real failure - return classifyNonzeroExit(result, parseResult); + return { + ...classifyNonzeroExit(result, parsedReport, parseResult), + parseResult, + parsed: parsedReport, + }; } /** @@ -411,10 +402,23 @@ function processChainResults( ): IAcceptanceOutcome | undefined { // Honor exit code: nonzero means failure if (result.code !== 0) { - // Check if this is an infrastructure error; always pass allResults so results are preserved + // Parse to get the report object for error extraction + // Use first entity as representative; we only need the parsed report structure + const firstEntity = spec.entities[0]; + + let parsedReport: IPlaywrightReportType | null = null; + + if (firstEntity) { + const { parsed } = parsePlaywrightResults(result.stdout, firstEntity); + + parsedReport = parsed; + } + + // Check if this is an infrastructure error; pass allResults for preservation const { outcome, shouldRetry } = classifyNonzeroExit( result, - allResults.length > 0 ? allResults : null + parsedReport, + allResults ); if (shouldRetry) { @@ -527,20 +531,18 @@ export function makeBoringstackAcceptanceRunner(exec: Exec): IAcceptanceRunner { } ); - const { outcome, shouldRetry } = processExecResult( - result, - entity, - requiredSteps - ); + const { outcome, shouldRetry, parseResult, parsed } = + processExecResult(result, entity, requiredSteps); if (outcome !== undefined) { return outcome; } - // Preserve parsed results even if classified as infra (for diagnostics) - const parseResult = parsePlaywrightJSON(result.stdout, entity); - - if (parseResult !== null) { + // Preserve parsed results even if classified as infra (for diagnostics). + // Distinguish unparseable (parsed === null) from valid-empty (parsed !== null, parseResult === []): + // - Unparseable (null): keep the previous lastResults from an earlier successful parse + // - Valid-empty (parsed !== null): overwrite lastResults with the latest valid parse (even if empty) + if (parsed !== null) { lastResults = parseResult; } @@ -614,9 +616,12 @@ export function makeBoringstackAcceptanceRunner(exec: Exec): IAcceptanceRunner { const allResults: IAcceptanceResult[] = []; for (const entity of spec.entities) { - const parseResult = parsePlaywrightJSON(result.stdout, entity); + const { results: parseResult } = parsePlaywrightResults( + result.stdout, + entity + ); - if (parseResult !== null) { + if (parseResult.length > 0) { allResults.push(...parseResult); } } diff --git a/packages/core/src/loop/boringstack/acceptance/testid-contract.ts b/packages/core/src/loop/boringstack/acceptance/testid-contract.ts index 5e003ed3..9ea96911 100644 --- a/packages/core/src/loop/boringstack/acceptance/testid-contract.ts +++ b/packages/core/src/loop/boringstack/acceptance/testid-contract.ts @@ -96,6 +96,7 @@ ${ - In the table row: \`{record.${entity.shows[0] ?? entity.fields[0]?.name ?? "name"}}\` **Where to add them:** +- **Navigation (IMPORTANT — outside the feature directory):** add \`data-testid="${ids.nav}"\` to the "${entity.id}" link in the SHARED sidebar (\`apps/ui/src/components/core/AppSidebar/\`), NOT in the feature folder. A feature that isn't linked in the sidebar is unreachable and will fail end-to-end acceptance — every feature MUST be added to the sidebar navigation. - List page component: add \`data-testid="${ids.list}"\` to the list/table container, \`data-testid="${ids.empty}"\` to the empty state - Create button: add \`data-testid="${ids.create}"\` to the button that opens the form - Form component: add \`data-testid="${ids.form}"\` to the \`
\`, \`data-testid="${ids.submit}"\` to the submit button, and \`data-testid="${ids.field("...")}\` to each input @@ -133,7 +134,13 @@ export function checkTestIds( continue; } - if (!source.includes(`data-testid="${id}"`)) { + // Match either quote style: the generated JSX is single-quoted + // (`data-testid='x'`) after prettier/eslint, while hand-written examples + // often use double quotes. Missing EITHER form means the hook is absent. + const hasDouble = source.includes(`data-testid="${id}"`); + const hasSingle = source.includes(`data-testid='${id}'`); + + if (!hasDouble && !hasSingle) { errors.push(id); } } diff --git a/packages/core/src/loop/boringstack/build.ts b/packages/core/src/loop/boringstack/build.ts index c7a4d538..154a2126 100644 --- a/packages/core/src/loop/boringstack/build.ts +++ b/packages/core/src/loop/boringstack/build.ts @@ -103,6 +103,21 @@ export const APP_SCHEMA_FILE = * instructed (refinePrompt) to only ADD its feature's keys, never touch others'. */ export const LOCALE_GLOB = "apps/ui/src/lib/i18n/locales/**"; +/** The shared app sidebar navigation component. Features are unreachable (fail browser + * acceptance tests) if the model doesn't register a NavLink for the feature in the + * sidebar. ADD-ONLY shared file: the worker adds only its own NavLink, accepted like + * APP_SCHEMA_FILE/LOCALE_GLOB (filesystem scope grants write, differential gate + judge + * catch any regression to another feature's nav). It's instructed (refinePrompt) to ADD + * ONLY its feature's link, never modify another feature's entry or remove entries. */ +export const APP_SIDEBAR_FILE = + "apps/ui/src/components/core/AppSidebar/AppSidebar.tsx"; + +/** The shared router configuration file. A feature without a route entry is unreachable + * (knip flags the page as unused). ADD-ONLY shared file: the worker adds only its own + * route, same trade-off as APP_SIDEBAR_FILE. It's instructed (refinePrompt) to ADD ONLY + * its feature's route entry, never modify another feature's route. */ +export const APP_ROUTES_FILE = "apps/ui/src/app/router/routes.tsx"; + export function scopeFor(name: string): string[] { const camel = toCamelCase(name); @@ -116,6 +131,11 @@ export function scopeFor(name: string): string[] { // Same story for i18n: any UI string is a locale key, and the keys live in // shared locale files — the model must be able to add the keys it references. LOCALE_GLOB, + // The sidebar and router are shared UI files. A feature is unreachable (fails + // browser acceptance) unless the model adds a NavLink to the sidebar and a route + // to the router. Add-only: the model may ADD its feature's entry, never modify others'. + APP_SIDEBAR_FILE, + APP_ROUTES_FILE, ]; } diff --git a/packages/core/src/loop/boringstack/refine-prompt.ts b/packages/core/src/loop/boringstack/refine-prompt.ts index c0910206..f5a1df2c 100644 --- a/packages/core/src/loop/boringstack/refine-prompt.ts +++ b/packages/core/src/loop/boringstack/refine-prompt.ts @@ -178,6 +178,27 @@ Go through \`tests/helpers/db\` for db/schema — never import \`drizzle-orm\` o --- +## BoringStack UI component conventions — write it RIGHT the first time + +The UI eslint (react-component-architecture) is strict; these rules cause the most churn. Copy the scaffold's gate-green example \`apps/ui/src/features/accounts/components/JoinRequestsPage/JoinRequestsPage.tsx\` — a list with per-row actions, exactly the shape your edit/delete rows need. + +**No arrow functions or .bind in JSX props (\`jsx-no-bind\` — the #1 churn source).** \`onClick={() => onEdit(row.id)}\` is REJECTED. For per-row actions add a curried helper in \`${camel}.utils.ts\`: +\`\`\`ts +export const makeIdHandler = + (fn: (id: string) => void) => (id: string) => () => fn(id); +\`\`\` +then in the component body \`const editHandler = makeIdHandler(onEdit);\` (and \`deleteHandler\`), and in JSX \`onClick={editHandler(row.id)}\`. The prop is a call expression, not a literal arrow, so the rule passes. This curried-helper pattern is the scaffold's own gate-green convention (see JoinRequestsPage example). For a fully stable handler, a per-row child component with a useCallback is an alternative, but the call-expression helper is the established pattern here. For a zero-arg handler pass a named function reference (\`onClick={handleSubmit}\`), never an inline arrow. + +**Extract computed lists.** Build the rows as a const in the body (\`const renderRows = items.map((row) => (…));\`) or a hook — never inline a complex \`.map()\` with logic in the returned JSX ("Extract this computation into a hook"). + +**One concern per file.** The \`.tsx\` is presentational only. Data-fetching hooks live in the generated PascalCase \`${feature.id}.queries.ts\` (list/fetch queries) and \`${feature.id}.mutations.ts\` (create/update/delete mutations) — FILL THESE, do NOT create parallel \`${camel}.hooks.ts\` files. Component view-state hooks go in a co-located \`.hooks.ts\` next to the component. Pure helpers in \`${camel}.utils.ts\` / \`.utils.ts\`. Types in \`${camel}.types.ts\`. NEVER put JSX in a \`.ts\` file (JSX only compiles in \`.tsx\`; a \`.ts\` with \`\` throws "Parsing error: '>' expected"). Never mix a hook + component + type in one module. + +**Every visible string via \`t("features.${camel}.")\`** — no hardcoded JSX text (button labels like "Edit"/"Delete" included). + +**Make the feature reachable.** Add a sidebar link to \`apps/ui/src/components/core/AppSidebar/AppSidebar.tsx\` (add your feature's entry to the \`APP_SIDEBAR_NAV_ITEMS\` array, following the existing format) and register its route in \`apps/ui/src/app/router/routes.tsx\` (add a route pointing at your feature's page component). Both files are in your editable scope — ADD ONLY your feature's entries, never modify another feature's link/route (same rule as the shared schema + locale files). A feature missing from the sidebar or router is unreachable and fails browser acceptance. + +--- + ## Domain-Fill Instructions - **Persist for real**: every domain field must be a real column on the \`${camel}\` table (see Persistence above) that the service inserts/selects/updates via Drizzle. NEVER hold a field only in memory to satisfy a test — that stores nothing and is a failed implementation. @@ -192,10 +213,12 @@ Go through \`tests/helpers/db\` for db/schema — never import \`drizzle-orm\` o ## Freeze -⚠️ **FREEZE**: Only the files above for the **${feature.id}** resource are editable — that includes adding your entity's columns to the \`${camel}\` table in the app schema. All other files are locked. Do not modify: +⚠️ **FREEZE**: Only the files above for the **${feature.id}** resource are editable. The shared app schema, locale files, AppSidebar.tsx, and routes.tsx are the explicit ADD-ONLY exceptions: the model may edit them only to add entries for this feature, never to modify other features' entries or remove them. All other files are locked. Do not modify: - Any table OTHER than \`${camel}\` in the app schema; no migrations +- Any locale namespace OTHER than \`features.${camel}\` in the locale files +- Any sidebar link OTHER than your feature's link; do not modify other features' links +- Any route OTHER than your feature's route; do not modify other features' routes - Root configuration files -- Routes wiring (already done for this resource) - Other resources' files If you need to make a change elsewhere, the build has already locked it. Rebase this feature once it passes the gate. diff --git a/packages/core/tests/boringstack-build.test.ts b/packages/core/tests/boringstack-build.test.ts index 970fb5ab..d842b231 100644 --- a/packages/core/tests/boringstack-build.test.ts +++ b/packages/core/tests/boringstack-build.test.ts @@ -356,6 +356,19 @@ describe("scopeFor", () => { expect(scope).toContain(APP_SCHEMA_FILE); expect(scope).toContain(LOCALE_GLOB); }); + + test("includes shared sidebar and router files (add-only, not edit existing entries)", () => { + const scope = scopeFor("Company"); + + // The sidebar and router are shared UI files. A feature is unreachable (fails + // browser acceptance tests) if the model doesn't register a NavLink for the feature + // in the sidebar and a route entry in the router. Add-only: the model may ADD its + // feature's entry, never modify another feature's entry or remove entries. + expect(scope).toContain( + "apps/ui/src/components/core/AppSidebar/AppSidebar.tsx" + ); + expect(scope).toContain("apps/ui/src/app/router/routes.tsx"); + }); }); describe("rescueFileFor", () => { diff --git a/packages/core/tests/boringstack-e2e-generator.test.ts b/packages/core/tests/boringstack-e2e-generator.test.ts index d6c451cf..7a3e67d5 100644 --- a/packages/core/tests/boringstack-e2e-generator.test.ts +++ b/packages/core/tests/boringstack-e2e-generator.test.ts @@ -142,6 +142,28 @@ describe("E2E spec generator", () => { expect(spec).toContain("negative: Company rejects website=not-a-url"); }); + test("negative tests use API-level POST to assert 4xx rejection (deterministic, no browser race)", () => { + const spec = generateEntitySpec(company); + + // Should POST directly to /api/v1/company (API-level check, deterministic) + expect(spec).toContain("page.request.post"); + expect(spec).toContain("/api/v1/company"); + expect(spec).toContain("Content-Type"); + expect(spec).toContain("application/json"); + expect(spec).toContain("Cookie"); + // Should assert validation error codes (400 or 422), not any 4xx (fail-open prevention) + expect(spec).toContain("[400, 422].includes("); + // Should NOT contain the fail-open toBeGreaterThanOrEqual(400) pattern + expect(spec).not.toContain("toBeGreaterThanOrEqual(400)"); + expect(spec).not.toContain("toBeLessThan(500)"); + // Should build payload with valid fields + overridden invalid field + expect(spec).toContain("Record"); + expect(spec).toContain("payload["); + // Old browser-based approach is gone + expect(spec).not.toContain(".waitForResponse"); + expect(spec).not.toContain("const createdOk = await successfulCreate"); + }); + test("generateEntitySpec includes empty state check", () => { const spec = generateEntitySpec(company); @@ -419,6 +441,39 @@ describe("E2E spec generator - Relationships", () => { expect(spec).toContain("Company-for-Contact"); }); + test("negative tests for child entity include parent seeding in API payload", () => { + const contact: IEntityAcceptance = { + id: "Contact", + key: "contact", + nav: "Contacts", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "John Doe", + invalid: [], + }, + ], + shows: ["name"], + screens: ["list", "form"], + parents: [{ entity: "Company", key: "company", fkField: "companyId" }], + negatives: [{ field: "name", value: "", why: "name is required" }], + acceptanceCheck: "create a contact", + }; + + const spec = generateEntitySpec(contact); + + // Negative test should seed parent first (for FK reference in payload) + expect(spec).toContain("Seed a parent Company record"); + // Then POST to API with payload including seeded companyId + expect(spec).toContain("page.request.post"); + expect(spec).toContain("/api/v1/contact"); + expect(spec).toContain("companyId: companyId"); + // Negative test title format preserved + expect(spec).toContain("negative: Contact rejects name="); + }); + test("generateEntitySpec with parent relationship selects parent in form", () => { const contact: IEntityAcceptance = { id: "Contact", @@ -1458,3 +1513,674 @@ describe("FIX F: relationship linkage asserted after reload", () => { expect(parentCellAssertionIndex).toBeGreaterThan(reloadedRowIndex); }); }); + +describe("FIX 1, 2, 3: negative test hardening (400/422 only, type-correct payloads, empty payload)", () => { + test("FIX 1: negative tests assert 400/422 validation error codes only", () => { + const spec = generateEntitySpec(company); + + // FIX 1: Should contain the strict [400, 422].includes check + expect(spec).toContain("[400, 422].includes("); + // Should NOT contain the fail-open toBeGreaterThanOrEqual(400) pattern + expect(spec).not.toContain("toBeGreaterThanOrEqual(400)"); + expect(spec).not.toContain("toBeLessThan(500)"); + }); + + test("FIX 2: numeric field renders as bare number in negative payload, not JSON string", () => { + const entityWithNumericField: IEntityAcceptance = { + id: "Product", + key: "product", + nav: "Products", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Widget", + invalid: [], + }, + { + name: "quantity", + type: "number", + optional: false, + valid: "42", + invalid: [], + }, + ], + shows: ["name", "quantity"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "name", value: "", why: "name is required" }], + acceptanceCheck: "create a product", + }; + + const spec = generateEntitySpec(entityWithNumericField); + + // FIX 2: The valid quantity field should render as a bare number literal (42) + // NOT as a JSON string ("42") + expect(spec).toContain("quantity: 42"); + // Verify the quoted form is NOT present (that would be the fail-open bug) + expect(spec).not.toContain('quantity: "42"'); + }); + + test("FIX 3: entity with only optional fields generates empty payload {}, not { , }", () => { + const entityOptionalOnly: IEntityAcceptance = { + id: "Config", + key: "config", + nav: "Configs", + fields: [ + { + name: "setting1", + type: "string", + optional: true, + valid: "value1", + invalid: [], + }, + { + name: "setting2", + type: "string", + optional: true, + valid: "value2", + invalid: [], + }, + ], + shows: ["setting1"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "setting1", value: "bad", why: "invalid" }], + acceptanceCheck: "create a config", + }; + + const spec = generateEntitySpec(entityOptionalOnly); + + // FIX 3: Should contain valid empty payload syntax: {}, not { , } + expect(spec).toContain("const payload: Record = {"); + expect(spec).not.toContain("{ , }"); + // Verify the payload is compilable (no syntax error) + expect(spec).toContain("payload["); + }); + + test("B1: boolean field renders as bare true/false (no quotes) in valid payload", () => { + const entityWithBoolean: IEntityAcceptance = { + id: "Feature", + key: "feature", + nav: "Features", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Feature1", + invalid: [], + }, + { + name: "enabled", + type: "boolean", + optional: false, + valid: "true", + invalid: [], + }, + ], + shows: ["name", "enabled"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "name", value: "", why: "name is required" }], + acceptanceCheck: "create a feature", + }; + + const spec = generateEntitySpec(entityWithBoolean); + + // B1: Boolean field should render as bare true (not "true") + expect(spec).toContain("enabled: true"); + // Verify the quoted form is NOT present + expect(spec).not.toContain('enabled: "true"'); + }); + + test("B1: string field companion stays JSON-quoted next to numeric field", () => { + const entityWithStringAndNumeric: IEntityAcceptance = { + id: "Item", + key: "item", + nav: "Items", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Item1", + invalid: [], + }, + { + name: "price", + type: "number", + optional: false, + valid: "99.99", + invalid: [], + }, + ], + shows: ["name", "price"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "price", value: "-1", why: "price must be positive" }, + ], + acceptanceCheck: "create an item", + }; + + const spec = generateEntitySpec(entityWithStringAndNumeric); + + // B1: String field name should stay JSON-quoted + expect(spec).toContain('name: "Item1"'); + // Numeric field should be bare number + expect(spec).toContain("price: 99.99"); + }); + + test("B1: substring-trap type (appointment/interval) treated as string, not numeric", () => { + const entityWithTrapType: IEntityAcceptance = { + id: "Meeting", + key: "meeting", + nav: "Meetings", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Meeting1", + invalid: [], + }, + { + name: "appointment", + type: "appointment", + optional: false, + valid: "2024-01-01T10:00:00Z", + invalid: [], + }, + ], + shows: ["name", "appointment"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "appointment", value: "invalid", why: "invalid date" }, + ], + acceptanceCheck: "create a meeting", + }; + + const spec = generateEntitySpec(entityWithTrapType); + + // B1: "appointment" type should be treated as string (contains "int" but is NOT numeric) + // So it should be JSON-quoted + expect(spec).toContain('appointment: "2024-01-01T10:00:00Z"'); + }); + + test("B2: injection escaping in negative test title and assertion", () => { + const entityWithDangerousData: IEntityAcceptance = { + id: "Config", + key: "config", + nav: "Configs", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Config1", + invalid: [], + }, + { + name: "value", + type: "string", + optional: false, + valid: "safe", + invalid: [], + }, + ], + shows: ["name", "value"], + screens: ["list", "form"], + parents: [], + negatives: [ + { + field: "value", + value: "dangerous`${code}injection", + why: "test injection", + }, + ], + acceptanceCheck: "create a config", + }; + + const spec = generateEntitySpec(entityWithDangerousData); + + // B2: The dangerous backtick and ${} should appear ESCAPED in the assertion message + // NOT as a raw unescaped sequence that could break the spec + const testTitleMatch = + /test\("negative:[^"]*dangerous.*injection"[^)]*\)/.exec(spec); + + expect(testTitleMatch).toBeTruthy(); + // The raw dangerous sequence should NOT appear unescaped in the assertion + // (it should be escaped as part of JSON.stringify) + expect(spec).not.toContain("${code}injection`"); + }); + + test("B3: canonical numeric invalid renders as a bare number (tests the constraint)", () => { + const entityWithNumericNegative: IEntityAcceptance = { + id: "Product", + key: "product", + nav: "Products", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Product1", + invalid: [], + }, + { + name: "stock", + type: "integer", + optional: false, + valid: "10", + invalid: [], + }, + ], + shows: ["name", "stock"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "stock", value: "-1", why: "stock must be non-negative" }, + ], + acceptanceCheck: "create a product", + }; + + const spec = generateEntitySpec(entityWithNumericNegative); + + // B3: Invalid override for numeric field renders as bare number (not string) + // to exercise the numeric range/constraint, not type-rejection. + // "-1" is finite, so it renders as bare -1, testing the constraint. + expect(spec).toContain('payload["stock"] = -1'); + }); + + test("B3: NON-canonical numeric invalid stays a raw string (not mutated by Number)", () => { + const entity: IEntityAcceptance = { + id: "Product", + key: "product", + nav: "Products", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "P1", + invalid: [], + }, + { + name: "stock", + type: "integer", + optional: false, + valid: "10", + invalid: [], + }, + ], + shows: ["name", "stock"], + screens: ["list", "form"], + parents: [], + // "01" does NOT round-trip (Number("01")→1, String(1)!=="01") and a bare `01` + // would be an OCTAL SyntaxError in the strict-mode spec — so it must stay a raw + // string, testing type-rejection rather than corrupting to a valid `1`. + negatives: [ + { + field: "stock", + value: "01", + why: "leading-zero token is not a valid integer input", + }, + ], + acceptanceCheck: "create a product", + }; + + const spec = generateEntitySpec(entity); + + expect(spec).toContain('payload["stock"] = "01"'); + // Must NOT emit a bare octal literal (SyntaxError) or a mutated value. + expect(spec).not.toContain('payload["stock"] = 01'); + expect(spec).not.toContain('payload["stock"] = 1;'); + }); + + test("B3: type-render invalid boolean token as string (testing type-rejection)", () => { + const entityWithBooleanNegative: IEntityAcceptance = { + id: "Feature", + key: "feature", + nav: "Features", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Feature1", + invalid: [], + }, + { + name: "active", + type: "bool", + optional: false, + valid: "true", + invalid: [], + }, + ], + shows: ["name", "active"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "active", value: "notabool", why: "invalid boolean" }, + ], + acceptanceCheck: "create a feature", + }; + + const spec = generateEntitySpec(entityWithBooleanNegative); + + // B3: Invalid boolean token (not "true"/"false") renders as JSON string + // to exercise type-rejection. "notabool" is not a valid boolean, so it + // renders as the string "notabool" to test the type constraint. + expect(spec).toContain('payload["active"] = "notabool"'); + }); + + test("B3: type-render valid boolean value as bare boolean", () => { + const entityWithValidBooleanNegative: IEntityAcceptance = { + id: "Feature", + key: "feature", + nav: "Features", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Feature1", + invalid: [], + }, + { + name: "active", + type: "bool", + optional: false, + valid: "true", + invalid: [], + }, + ], + shows: ["name", "active"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "active", value: "false", why: "active must be true" }, + ], + acceptanceCheck: "create a feature", + }; + + const spec = generateEntitySpec(entityWithValidBooleanNegative); + + // B3: Valid boolean value "false" renders as bare false + // to exercise the boolean value constraint (not type-rejection). + expect(spec).toContain('payload["active"] = false'); + }); + + test("B3: required-empty negative keeps empty string as empty string", () => { + const entityWithRequiredEmpty: IEntityAcceptance = { + id: "Article", + key: "article", + nav: "Articles", + fields: [ + { + name: "title", + type: "string", + optional: false, + valid: "Article1", + invalid: [], + }, + ], + shows: ["title"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "title", value: "", why: "title is required" }], + acceptanceCheck: "create an article", + }; + + const spec = generateEntitySpec(entityWithRequiredEmpty); + + // B3: Required-empty ("") should stay as empty string literal "" + expect(spec).toContain('payload["title"] = ""'); + }); + + test("B3: NaN in numeric field falls back to JSON string", () => { + const entityWithNaNValue: IEntityAcceptance = { + id: "Measurement", + key: "measurement", + nav: "Measurements", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Measurement1", + invalid: [], + }, + { + name: "value", + type: "float", + optional: false, + valid: "notanumber", + invalid: [], + }, + ], + shows: ["name", "value"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "value", value: "invalid", why: "invalid number" }], + acceptanceCheck: "create a measurement", + }; + + const spec = generateEntitySpec(entityWithNaNValue); + + // B1: When valid field is not a number (NaN), fall back to JSON string + // "notanumber" becomes "notanumber" (quoted string) + expect(spec).toContain('value: "notanumber"'); + }); +}); + +describe("PART C: negative check hardening tests", () => { + test("C1: boolean field renders as bare true/false (no quotes) in valid payload", () => { + const entityWithBoolean: IEntityAcceptance = { + id: "Feature", + key: "feature", + nav: "Features", + fields: [ + { + name: "enabled", + type: "boolean", + optional: false, + valid: "true", + invalid: [], + }, + { + name: "name", + type: "string", + optional: false, + valid: "Feature1", + invalid: [], + }, + ], + shows: ["name"], + screens: ["list", "form"], + parents: [], + negatives: [{ field: "name", value: "", why: "name is required" }], + acceptanceCheck: "create a feature", + }; + + const spec = generateEntitySpec(entityWithBoolean); + + // Boolean field should render as bare true (no quotes) + expect(spec).toContain("enabled: true"); + // Verify the quoted form is NOT present + expect(spec).not.toContain('enabled: "true"'); + }); + + test("C2: string companion field stays quoted when paired with numeric", () => { + const entityWithNumericAndString: IEntityAcceptance = { + id: "Product", + key: "product", + nav: "Products", + fields: [ + { + name: "name", + type: "string", + optional: false, + valid: "Widget", + invalid: [], + }, + { + name: "price", + type: "number", + optional: false, + valid: "99.99", + invalid: [], + }, + ], + shows: ["name", "price"], + screens: ["list", "form"], + parents: [], + negatives: [ + { field: "price", value: "-1", why: "price cannot be negative" }, + ], + acceptanceCheck: "create a product", + }; + + const spec = generateEntitySpec(entityWithNumericAndString); + + // Numeric field should render as bare number + expect(spec).toContain("price: 99.99"); + expect(spec).not.toContain('price: "99.99"'); + // String field should stay quoted + expect(spec).toContain('name: "Widget"'); + }); + + test("C3: substring-trap type (appointment/interval/constraint) treated as string, not number", () => { + const entityWithSubstringTrap: IEntityAcceptance = { + id: "Meeting", + key: "meeting", + nav: "Meetings", + fields: [ + { + name: "appointmentTime", + type: "appointment", + optional: false, + valid: "2024-01-15T10:00:00Z", + invalid: [], + }, + { + name: "duration", + type: "interval", + optional: false, + valid: "PT1H", + invalid: [], + }, + ], + shows: ["appointmentTime"], + screens: ["list", "form"], + parents: [], + negatives: [ + { + field: "appointmentTime", + value: "invalid-time", + why: "invalid appointment", + }, + ], + acceptanceCheck: "create a meeting", + }; + + const spec = generateEntitySpec(entityWithSubstringTrap); + + // Both substring-trap types should be quoted (treated as strings, not numbers) + expect(spec).toContain('appointmentTime: "2024-01-15T10:00:00Z"'); + expect(spec).toContain('duration: "PT1H"'); + // Verify they are NOT rendered as bare numbers + expect(spec).not.toContain("appointmentTime: 0"); + expect(spec).not.toContain("duration: 0"); + }); + + test("C4: injection escaping - backticks and ${ in neg.value are JSON.stringify-escaped", () => { + const entityWithInjection: IEntityAcceptance = { + id: "Template", + key: "template", + nav: "Templates", + fields: [ + { + name: "content", + type: "string", + optional: false, + valid: "Hello", + invalid: [], + }, + ], + shows: ["content"], + screens: ["list", "form"], + parents: [], + negatives: [ + { + field: "content", + value: "`inject` and ${evil}", + why: "template injection", + }, + ], + acceptanceCheck: "create a template", + }; + + const spec = generateEntitySpec(entityWithInjection); + + // The dangerous sequence should NOT appear unescaped in the spec + expect(spec).not.toContain("payload[content] = `inject` and ${evil}"); + // Instead, it should be JSON.stringify-escaped + expect(spec).toContain('payload["content"]'); + // The value should be properly escaped as a JSON string + expect(spec).toContain('"`inject` and ${evil}"'); + }); + + test("C5: negative assertion uses [400, 422].includes() exactly (not fail-open pattern)", () => { + const spec = generateEntitySpec(company); + + // Must use strict [400, 422].includes() check + expect(spec).toContain("[400, 422].includes(res.status())"); + // Should NOT use fail-open patterns + expect(spec).not.toContain("toBeGreaterThanOrEqual(400)"); + expect(spec).not.toContain("toBeLessThan(500)"); + expect(spec).not.toContain(">= 400"); + expect(spec).not.toContain("< 500"); + }); + + test("C6: required-empty negative keeps empty string literal, not coerced to 0 or null", () => { + const entityWithRequired: IEntityAcceptance = { + id: "User", + key: "user", + nav: "Users", + fields: [ + { + name: "email", + type: "email", + optional: false, + valid: "user@example.com", + invalid: [], + }, + ], + shows: ["email"], + screens: ["list", "form"], + parents: [], + negatives: [ + { + field: "email", + value: "", + why: "email is required", + }, + ], + acceptanceCheck: "create a user", + }; + + const spec = generateEntitySpec(entityWithRequired); + + // The override for required-empty should be the empty string literal "" + expect(spec).toContain('payload["email"] = ""'); + // Should NOT be coerced to 0, null, or false + expect(spec).not.toContain('payload["email"] = 0'); + expect(spec).not.toContain('payload["email"] = null'); + expect(spec).not.toContain('payload["email"] = false'); + }); +}); diff --git a/packages/core/tests/boringstack-e2e-runner.test.ts b/packages/core/tests/boringstack-e2e-runner.test.ts index fb30aee3..0a80a734 100644 --- a/packages/core/tests/boringstack-e2e-runner.test.ts +++ b/packages/core/tests/boringstack-e2e-runner.test.ts @@ -4,7 +4,10 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import type { Exec } from "../src/loop/boringstack/exec"; -import { makeBoringstackAcceptanceRunner } from "../src/loop/boringstack/acceptance/e2e-runner"; +import { + makeBoringstackAcceptanceRunner, + processExecResult, +} from "../src/loop/boringstack/acceptance/e2e-runner"; import { chainCreateTitle } from "../src/loop/boringstack/acceptance/e2e-generator"; import type { IEntityAcceptance, @@ -62,6 +65,72 @@ const testEntity: IEntityAcceptance = { acceptanceCheck: "create a company", }; +const CREATE_TITLE = "create Company: form fill, submit, row appears"; + +function parseableReport(ok: boolean): string { + return JSON.stringify({ + suites: [ + { + title: "e2e/company.spec.ts", + suites: [], + specs: [ + { + title: CREATE_TITLE, + ok, + tests: [ + { + results: [ + { + status: ok ? "passed" : "failed", + ...(ok ? {} : { error: { message: "boom" } }), + }, + ], + }, + ], + }, + ], + }, + ], + stats: { expected: 1, unexpected: ok ? 0 : 1, flaky: 0, skipped: 0 }, + errors: [], + }); +} + +test("processExecResult threads the parsed results back (exit 0, parseable)", () => { + const res = processExecResult( + { code: 0, stdout: parseableReport(true), stderr: "" }, + testEntity, + ["create"] + ); + + // The parse happens once here and is returned so run() need not re-parse. + expect(res.parseResult.length).toBeGreaterThan(0); + expect(res.parseResult.some((r) => r.step === "create" && r.ok)).toBe(true); + expect(res.outcome).toBeDefined(); +}); + +test("processExecResult threads parsed results even on a nonzero exit (diagnostics)", () => { + const res = processExecResult( + { code: 1, stdout: parseableReport(false), stderr: "" }, + testEntity, + ["create"] + ); + + // Real failure: results are preserved (not discarded) for steer/diagnostics. + expect(res.parseResult.length).toBeGreaterThan(0); + expect(res.parseResult.some((r) => r.step === "create")).toBe(true); +}); + +test("processExecResult returns empty parseResult when stdout is unparseable", () => { + const res = processExecResult( + { code: 1, stdout: "", stderr: "some error" }, + testEntity, + ["create"] + ); + + expect(res.parseResult.length).toBe(0); +}); + test("runner: fake Exec returning nested Playwright JSON report parses correctly", async () => { const report = { suites: [ @@ -947,3 +1016,131 @@ test("FIX A: parseStep recognizes all chain-create titles", async () => { expect(outcome.results.length).toBe(3); expect(outcome.results.every((r) => r.step === "create")).toBe(true); }); + +test("FIX 1b: runner preserves earlier parsed result when later attempt is unparseable", async () => { + let attemptCount = 0; + + const fakeExec: Exec = async () => { + attemptCount++; + + if (attemptCount === 1) { + // First attempt: parseable JSON with results (infra error in stderr) + return { + code: 1, + stdout: JSON.stringify({ + suites: [ + { + title: "e2e/company.spec.ts", + specs: [ + { + title: "create Company: form fill, submit, row appears", + ok: false, + tests: [ + { + results: [ + { + status: "failed", + error: { message: "Row did not appear" }, + }, + ], + }, + ], + }, + ], + }, + ], + stats: { expected: 1, unexpected: 1, flaky: 0, skipped: 0 }, + errors: [], + }), + stderr: "ECONNREFUSED: connection timeout", + }; + } + + if (attemptCount === 2) { + // Second attempt: still infra error, unparseable (empty stdout) + return { + code: 1, + stdout: "", + stderr: "ECONNREFUSED: infrastructure failure", + }; + } + + // Third attempt: still unparseable + return { + code: 1, + stdout: "", + stderr: "ECONNREFUSED: final infra failure", + }; + }; + + const runner = makeBoringstackAcceptanceRunner(fakeExec); + const ctx = createTestCtx(); + + const outcome = await runner.run(testEntity, ctx); + + // Must be classified as infra error after 3 attempts + expect(outcome.infraError).toBeDefined(); + // CRITICAL: results from the first (parseable) attempt must be preserved + expect(outcome.results.length).toBeGreaterThan(0); + expect(outcome.results[0]?.step).toBe("create"); + expect(outcome.results[0]?.detail).toBe("Row did not appear"); +}); + +test("FIX 1b: runner overwrites earlier result when later attempt is valid-empty []", async () => { + let attemptCount = 0; + + const fakeExec: Exec = async () => { + attemptCount++; + + if (attemptCount === 1) { + // First attempt: parseable JSON with one result + infra error in stderr + return { + code: 1, + stdout: JSON.stringify({ + suites: [ + { + title: "e2e/company.spec.ts", + specs: [ + { + title: "create Company: form fill, submit, row appears", + ok: true, + tests: [{ results: [{ status: "passed" }] }], + }, + ], + }, + ], + stats: { expected: 1, unexpected: 0, flaky: 0, skipped: 0 }, + errors: [], + }), + stderr: "ECONNREFUSED: connection timeout", + }; + } + + // Second attempt: parseable JSON but no matching test specs (empty results, still infra error) + return { + code: 1, + stdout: JSON.stringify({ + suites: [ + { + title: "e2e/company.spec.ts", + specs: [], + }, + ], + stats: { expected: 0, unexpected: 0, flaky: 0, skipped: 0 }, + errors: [], + }), + stderr: "ECONNREFUSED: connection still failing", + }; + }; + + const runner = makeBoringstackAcceptanceRunner(fakeExec); + const ctx = createTestCtx(); + + const outcome = await runner.run(testEntity, ctx); + + // Must be classified as infra error + expect(outcome.infraError).toBeDefined(); + // CRITICAL: results must be the LATEST valid-empty [] parse (empty array from second attempt) + // NOT the earlier result with "create" from first attempt + expect(outcome.results.length).toBe(0); +}); diff --git a/packages/core/tests/boringstack-refine-prompt.test.ts b/packages/core/tests/boringstack-refine-prompt.test.ts index 8efa9534..f3dc6ed7 100644 --- a/packages/core/tests/boringstack-refine-prompt.test.ts +++ b/packages/core/tests/boringstack-refine-prompt.test.ts @@ -430,4 +430,51 @@ describe("refinePrompt", () => { expect(prompt).toContain("Do NOT run the gate through the shell"); expect(prompt).not.toContain("Do NOT run the gate yourself"); }); + + it("front-loads BoringStack UI component conventions (makeIdHandler list-row idiom, module split, api-client typed unwrap)", () => { + const feature: IFeature = { + id: "Invoice", + desc: "Customer billing record", + passes: false, + attempts: 0, + }; + + const prompt = refinePrompt(feature); + + // Must teach the UI conventions section. + expect(prompt).toContain("BoringStack UI component conventions"); + // Must teach the #1 jsx-no-bind churn source. + expect(prompt).toContain("jsx-no-bind"); + // Must teach the makeIdHandler curried list-row idiom to avoid arrow functions in JSX. + expect(prompt).toContain("makeIdHandler"); + // Must forbid JSX in .ts files (JSX only compiles in .tsx). + expect(prompt).toContain("JSX only compiles in `.tsx`"); + expect(prompt).toContain("Parsing error: '>' expected"); + // Must teach module separation (component in .tsx, hooks in .hooks.ts, helpers in .utils.ts). + expect(prompt).toContain(".hooks.ts"); + expect(prompt).toContain(".utils.ts"); + expect(prompt).toContain(".types.ts"); + // (api-client data-fetching idiom is taught by the existing data-fetching + // section, not the UI-component conventions — no duplicate/contradictory copy here.) + }); + + it("teaches the reachability contract: register nav + route, ADD-ONLY", () => { + const feature: IFeature = { + id: "Company", + desc: "A business the sales team works with", + passes: false, + attempts: 0, + }; + + const prompt = refinePrompt(feature); + + // Must instruct wiring the feature into BOTH the sidebar and the router so it is reachable. + expect(prompt).toContain("AppSidebar"); + expect(prompt).toContain("routes.tsx"); + // The sidebar registration is via the scaffold's nav-items array (which renders + // the nav- testid the acceptance gate navigates by). + expect(prompt).toContain("APP_SIDEBAR_NAV_ITEMS"); + // Must carry the add-only boundary for those shared files. + expect(prompt).toContain("ADD ONLY"); + }); }); diff --git a/packages/core/tests/boringstack-testid-contract.test.ts b/packages/core/tests/boringstack-testid-contract.test.ts index 869559ac..603c2493 100644 --- a/packages/core/tests/boringstack-testid-contract.test.ts +++ b/packages/core/tests/boringstack-testid-contract.test.ts @@ -104,6 +104,16 @@ describe("buildTestIdGuide", () => { expect(guide).toContain("Where to add them"); }); + test("directs the nav testid to the shared sidebar (reachability)", () => { + const guide = buildTestIdGuide(contact); + const ids = testIdsFor(contact.key); + + // The nav hook lives OUTSIDE the feature dir; the guide must tell the + // model to wire it into the sidebar, or features are built unreachable. + expect(guide).toContain(ids.nav); + expect(guide).toContain("AppSidebar"); + }); + test("guide includes all entity fields as required inputs", () => { const guide = buildTestIdGuide(contact); @@ -155,6 +165,32 @@ describe("checkTestIds", () => { expect(result).toEqual([]); }); + test("accepts SINGLE-quoted testids (prettier/eslint emit data-testid='x')", () => { + // Regression: the generated JSX is single-quoted, but the gate previously + // matched only double quotes and reported every present testid as missing, + // parking every feature. A single-quoted source with ALL testids must pass. + const required = requiredTestIds(contact); + const source = required.map((id) => `data-testid='${id}'`).join(" "); + + const result = checkTestIds(new Map([["test.tsx", source]]), contact); + + expect(result).toEqual([]); + }); + + test("still detects a genuinely missing testid in single-quoted source", () => { + const ids = testIdsFor(contact.key); + const required = requiredTestIds(contact); + // single-quoted source missing only the "name" field testid + const source = required + .filter((id) => id !== ids.field("name")) + .map((id) => `data-testid='${id}'`) + .join(" "); + + const result = checkTestIds(new Map([["test.tsx", source]]), contact); + + expect(result).toContain(ids.field("name")); + }); + test("does NOT require nav testid in feature dir (nav lives in shared sidebar)", () => { const ids = testIdsFor(contact.key); const required = requiredTestIds(contact); diff --git a/packages/core/tests/headless-build-args.test.ts b/packages/core/tests/headless-build-args.test.ts index 46b92bdf..2b8737e1 100644 --- a/packages/core/tests/headless-build-args.test.ts +++ b/packages/core/tests/headless-build-args.test.ts @@ -5,8 +5,40 @@ import { join } from "node:path"; import { parseHeadlessArgs, resolveWorkspaceDir, + resolveExpertRescueFlag, + applyExpertRescueDefault, } from "../scripts/headless-build"; +describe("resolveExpertRescueFlag", () => { + test("defaults to '1' (rescue on) when the flag is unset", () => { + expect(resolveExpertRescueFlag(undefined)).toBe("1"); + }); + + test("keeps an explicit '0' (opt-out wins)", () => { + expect(resolveExpertRescueFlag("0")).toBe("0"); + }); + + test("keeps an explicit '1'", () => { + expect(resolveExpertRescueFlag("1")).toBe("1"); + }); +}); + +describe("applyExpertRescueDefault (the env wiring)", () => { + test("sets the flag to '1' on an env that has none", () => { + const env: { TSFORGE_EXPERT_RESCUE?: string } = {}; + + applyExpertRescueDefault(env); + expect(env.TSFORGE_EXPERT_RESCUE).toBe("1"); + }); + + test("preserves an explicit opt-out ('0')", () => { + const env = { TSFORGE_EXPERT_RESCUE: "0" }; + + applyExpertRescueDefault(env); + expect(env.TSFORGE_EXPERT_RESCUE).toBe("0"); + }); +}); + describe("parseHeadlessArgs", () => { test("parses prompt and dir from positional arguments", () => { const result = parseHeadlessArgs(["build x", "/clone"]);