fix(boringstack): navigable-by-construction + front-loaded UI idiom guide (kills jsx-no-bind wall) + API-negative hardening#169
Merged
Conversation
… list-row idiom, module split, api-client typed unwrap) — kills the jsx-no-bind near-green wall
…uplicate api-client bullet
The added UI-component conventions section had an api-client bullet that (a) contradicted the
existing data-fetching guide and (b) resurrected the forbidden 'if (error) throw error' idiom
(the scaffold client THROWS via middleware; the real usage is const { data } = await
apiClient.X(...); if (!data?.data) throw; return data.data). Removed the bullet (api-client is
already taught, correctly, by the data-fetching section) and its test assertions. The section
keeps its UI-component idioms: makeIdHandler (jsx-no-bind), render-list extraction, module split,
JSX-only-in-.tsx, no-hardcoded-text.
… navigable; correct UI module-split guide; fix API-negative boolean override + runner minors
FIX 1: scopeFor() now includes APP_SIDEBAR_FILE and APP_ROUTES_FILE constants,
allowing models to register feature navigation and routes. Features were unreachable
by construction (locked scope prevented sidebar/router edits); now features can be
discovered and pass browser acceptance tests.
FIX 2: refine-prompt.ts corrected:
- Module-split guidance now matches the real scaffold structure:
data-fetching hooks live in <Feature>.queries.ts / <Feature>.mutations.ts
(not ${camel}.hooks.ts)
- Added nav+route registration instruction with sidebar/router wiring requirements
- Updated Freeze section to clarify APP_SIDEBAR_FILE and APP_ROUTES_FILE are
ADD-ONLY exceptions for feature entries
FIX 3: e2e-generator.ts API-negative override corruption fixed:
- Invalid override values now sent verbatim via JSON.stringify(neg.value)
so "notabool" stays the string "notabool" (not coerced to false)
- Only VALID companion fields get type-rendering; override does not
- Test updated to expect string values for invalid overrides
FIX 4: e2e-runner.ts minors:
- extractReportErrorText double-parse removed: extract report errors
directly from already-parsed report object
- lastResults overwrite guard added: only update on non-empty parseResult
to preserve earlier diagnostics when stdout is unparseable
…verride, docs FIX 1a: Add test asserting scopeFor includes AppSidebar.tsx and routes.tsx - Verifies shared sidebar/router files are add-only in scope - Complements existing tests for schema and locale files FIX 1b: Add tests for runner lastResults preservation on retry - Test 1: later unparseable attempt preserves earlier parsed result - Test 2: later valid-empty [] replaces earlier results - Validates null-vs-empty distinction in processExecResult FIX 2: Fix runner.lastResults to distinguish unparseable (null) from valid-empty ([]) - processExecResult now returns parsed status (null or object) - Runner checks parsed !== null to overwrite lastResults (not length > 0) - Preserves results from earlier successful parse when later attempt unparseable - Overwrites with latest valid parse (including empty []) FIX 3: Make negative override type-aware with renderInvalidOverride helper - Create helper: empty "" stays "", numeric finite → bare number, bool true/false → bare bool - Non-bool token for bool field → JSON string (type-rejection test) - Fix contradictory test: update title to match "notabool" string assertion - Add test: bare "false" renders as boolean false - Update generateNegativeBlocks JSDoc to reference renderInvalidOverride FIX 4: Add rationale doc notes - build.ts: Expand APP_SIDEBAR_FILE/APP_ROUTES_FILE comments explaining add-only trade-off - refine-prompt.ts: Note makeIdHandler is scaffold's gate-green convention (JoinRequestsPage) All tests pass (363 total). No `as` casts (except `as const`), no `any`, no eslint-disable, complexity ≤ 20.
…rals; nav/add-only + non-canonical tests; fix test title
- renderInvalidOverride: emit a bare number ONLY for a canonical decimal literal
(/^-?\d+(\.\d+)?$/); non-canonical tokens ("01","0x10",whitespace,"1e5") stay
raw strings so Number() can't mutate an intentionally-invalid value into a valid one.
- tests: non-canonical numeric stays a raw string; refine-prompt reachability contract
(AppSidebar + routes.tsx + APP_SIDEBAR_NAV_ITEMS + ADD ONLY); fixed the numeric
test title to match its bare-number assertion.
… 01 SyntaxError); exact boolean match; gitignore dedup - renderInvalidOverride numeric: bare only when String(Number(value))===value (true round-trip). Rejects '01' (bare 01 = octal SyntaxError in strict mode), '1e5', '0x10', ' 5 ', huge→Infinity → all stay raw strings. Test now uses '01' (the octal case). - boolean: exact 'true'/'false' only (no trim/case-fold) so ' false '/'FALSE' stay raw strings testing the exact value. - JSDoc updated to the round-trip contract; .gitignore scratchpad patterns deduped.
agjs
added a commit
that referenced
this pull request
Jul 22, 2026
…e-quote testids, nav guide, expert rescue, negative-test race) (#168) * fix(acceptance): testid gate matches single-quoted JSX + nav-sidebar guide + expert rescue on Live CRM build (post-#167) surfaced three issues that made every slice park: - checkTestIds matched only double-quoted data-testid="...", but prettier/eslint emit single-quoted data-testid='...' → every present testid read as MISSING → every feature false-failed the testid stage. Now matches either quote. (This survived 11 panel rounds + all unit tests because the fixtures were double-quoted; added single-quote regression tests — the exact blind spot.) - buildTestIdGuide never told the model WHERE the nav testid goes; features were built but not wired into the shared AppSidebar (unreachable). Guide now directs nav-<entity> to apps/ui/src/components/core/AppSidebar/. - headless-build never enabled TSFORGE_EXPERT_RESCUE, so stalled features parked instead of being handed to the configured capabilities.expert model. Now on by default for the autonomous builder (explicit env wins). * test(headless-build): cover TSFORGE_EXPERT_RESCUE default via resolveExpertRescueFlag helper Panel BLOCK (4/4 agree): the new expert-rescue default env had no test. Extract a pure resolveExpertRescueFlag(current) helper (unset -> '1'; explicit '0'/'1' preserved) and test all three cases. * fix(acceptance): negative-test race + runner double-parse (deferred #167 minors) Folding the remaining deferred minors into this PR: - Negative tests replaced a broken one-shot `expect(rowsAfter).toBe(rowsBefore, {timeout})` (toBe ignores the timeout arg) with: waitForLoadState('networkidle') before reload (so a slow accept can't be missed) + a web-first retrying `expect(getByTestId(row)).toHaveCount(rowsBefore)`. Added a test locking it. - processExecResult now returns the parsed results so run() reuses them instead of re-parsing the same Playwright stdout on the retry path. (The remaining #167 minor — extracting the generator's identity-field detection — is deliberately deferred: the sites diverge on email handling (email-include for parent seeding vs email-exclude for identity per the round-11 fix), the functions are under the cognitive-complexity limit, and a refactor here is exactly what regressed the generator across rounds 9-11. Not worth the regression risk.) * fix(acceptance): real negative-test oracle + test the parseResult threading & env wiring Addresses the panel BLOCK on the folded-in minors: - Negative tests now prove rejection by the ABSENCE of a successful (2xx) create request (waitForResponse listener started before the submit click), replacing the racy count/networkidle approach that could false-pass on a late refetch or an already-idle SPA. Immune to pagination, shared-DB accumulation, and reload timing. - Export processExecResult + tests: parseResult is threaded back on exit-0, on a nonzero exit (diagnostics preserved), and is null when stdout is unparseable. - Extract applyExpertRescueDefault(env) so the process.env wiring itself is tested (unset -> '1', explicit '0' preserved), not just the pure helper. * refactor(acceptance): drop the negative-case step from the E2E gate The negative-case acceptance step (proving "invalid input was rejected") is hard to oracle in black-box browser E2E tests. Remove negatives cleanly from the acceptance spec layer while keeping the plan-layer mustNotHappen concept (which stays as a plan-level rule). Removes: - INegativeCase interface and negatives field from IEntityAcceptance - "negative" from AcceptStep union type - negativesFor, deriveNegatives, addConstraintNegatives, addMustNotHappenNegatives functions from acceptance-spec - generateNegativeBlocks from e2e-generator - negative case handling from e2e-runner and acceptance-steer - all negative-specific test cases and fixtures Keeps the gate's core value: nav, list, create, persist, update, delete, and relational-linkage verification. Verification still sees mustNotHappen in the plan; acceptance just doesn't consume it for negatives anymore. * feat(acceptance): reinstate the negative step as a deterministic API-level 4xx check (replaces the racy browser oracle) — POST invalid data to /api/v1/<entity>, assert 4xx; preserves validation enforcement * fix(acceptance): round-5 API-level negative hardening FIX 1: Accept only validation codes 400/422 (not any 4xx) - Changed assertion from fail-open toBeGreaterThanOrEqual(400) && toBeLessThan(500) - Now strictly checks [400, 422].includes(status) - Prevents false positives from 401/403/404/409 auth/routing/conflict errors FIX 2: Type-correct payload field values in negative tests - Extracted renderFieldValue() helper (complexity ≤ 20) - Numeric fields (type contains number/int/float/decimal) render as bare numbers (42 not "42") - Boolean fields (type contains bool) render as bare booleans (true not "true") - Other fields render as JSON strings (backwards compatible) - Prevents 4xx failures from unrelated field type mismatches FIX 3: Fix empty payload syntax error - Changed payloadFields from unconditional comma pattern to array + filter - Empty entity (only optional fields) now generates { } not { , } - Payload still applies target field override via payload[field] = value FIX 4: Remove dead parameters from generateNegativeBlocks - Removed unused _ids and _fieldFillSteps parameters - Updated call site in generateEntitySpec FIX 5: Parse Playwright JSON stdout once - Extracted parsePlaywrightOnce() that parses once and returns both report object + results - extractReportErrorText() now takes pre-parsed IPlaywrightReportType (not raw string) - classifyNonzeroExit() accepts parsed report object instead of raw stdout - Eliminates duplicate JSON.parse() calls in nonzero exit path FIX 6: Strengthen negative test suite - Assert generated spec uses [400, 422].includes check (not fail-open pattern) - Add test for numeric field renders as bare literal in payload - Add test for empty payload edge case (only optional fields) * fix(acceptance): negative check hardening r6 (parse revert, type-render, injection-safe) Part A: Revert round-5 "parse once" micro-optimization in e2e-runner.ts - Remove parsePlaywrightOnce function and "parse once" machinery - Restore classifyNonzeroExit to check parsedReport !== null (not parseResult.length > 0) This preserves the null (unparseable) vs [] (parsed, no matches) distinction - extractReportErrorText now takes stdout and parses it (harmless re-parse on nonzero path) - Keep processExecResult returning parseResult (tested and fine) - Adds parsePlaywrightResults helper for clean extraction Part B: Fix 3 negative check holes in e2e-generator.ts - B1 (renderFieldValue): Use EXACT type matching (set-based, not substring) Fix substring traps (appointment/interval/constraint treated as string not number) NaN falls back to string, not silently to "0" Numeric types: {number, integer, int, float, double, decimal, numeric} Boolean types: {boolean, bool} - B2 (injection-safe): Build error messages with JSON.stringify, not backtick interpolation Plan data (entity.key, neg.field) no longer interpolated directly into backticks Prevents backtick/interpolation injection in generated code - B3 (type-render override): Use renderFieldValue for invalid values, except "" Numeric invalid (e.g., "-1") renders as bare number, not string Boolean invalid (e.g., "notabool") renders as false, not string Required-empty "" stays as empty string literal for missing-required test Part C: Tests lock the new behavior - C1: Boolean renders as bare true/false (no quotes) - C2: String stays quoted when paired with numeric - C3: Substring-trap types (appointment, interval, constraint) are strings - C4: Injection escaping - backticks and ${ in plan data are JSON.stringify-safe - C5: Assertion uses [400, 422].includes() exactly (fail-open prevention) - C6: Required-empty negative keeps "" literal, no coercion Complexity: ≤20 (renderFieldValue uses sets, simple logic) No `as` except `as const`, no `any`, no eslint-disable * fix(boringstack): navigable-by-construction + front-loaded UI idiom guide (kills jsx-no-bind wall) + API-negative hardening (#169) * feat(boringstack): front-load UI component conventions (makeIdHandler list-row idiom, module split, api-client typed unwrap) — kills the jsx-no-bind near-green wall * fix(boringstack): correct the UI-conventions guide — drop the wrong/duplicate api-client bullet The added UI-component conventions section had an api-client bullet that (a) contradicted the existing data-fetching guide and (b) resurrected the forbidden 'if (error) throw error' idiom (the scaffold client THROWS via middleware; the real usage is const { data } = await apiClient.X(...); if (!data?.data) throw; return data.data). Removed the bullet (api-client is already taught, correctly, by the data-fetching section) and its test assertions. The section keeps its UI-component idioms: makeIdHandler (jsx-no-bind), render-list extraction, module split, JSX-only-in-.tsx, no-hardcoded-text. * fix(boringstack): put sidebar+router in feature scope so features are navigable; correct UI module-split guide; fix API-negative boolean override + runner minors FIX 1: scopeFor() now includes APP_SIDEBAR_FILE and APP_ROUTES_FILE constants, allowing models to register feature navigation and routes. Features were unreachable by construction (locked scope prevented sidebar/router edits); now features can be discovered and pass browser acceptance tests. FIX 2: refine-prompt.ts corrected: - Module-split guidance now matches the real scaffold structure: data-fetching hooks live in <Feature>.queries.ts / <Feature>.mutations.ts (not ${camel}.hooks.ts) - Added nav+route registration instruction with sidebar/router wiring requirements - Updated Freeze section to clarify APP_SIDEBAR_FILE and APP_ROUTES_FILE are ADD-ONLY exceptions for feature entries FIX 3: e2e-generator.ts API-negative override corruption fixed: - Invalid override values now sent verbatim via JSON.stringify(neg.value) so "notabool" stays the string "notabool" (not coerced to false) - Only VALID companion fields get type-rendering; override does not - Test updated to expect string values for invalid overrides FIX 4: e2e-runner.ts minors: - extractReportErrorText double-parse removed: extract report errors directly from already-parsed report object - lastResults overwrite guard added: only update on non-empty parseResult to preserve earlier diagnostics when stdout is unparseable * chore: untrack accidentally-committed build log + gitignore scratchpad-*.log* * fix(panel-r2): tests, runner null-vs-empty fix, type-aware negative override, docs FIX 1a: Add test asserting scopeFor includes AppSidebar.tsx and routes.tsx - Verifies shared sidebar/router files are add-only in scope - Complements existing tests for schema and locale files FIX 1b: Add tests for runner lastResults preservation on retry - Test 1: later unparseable attempt preserves earlier parsed result - Test 2: later valid-empty [] replaces earlier results - Validates null-vs-empty distinction in processExecResult FIX 2: Fix runner.lastResults to distinguish unparseable (null) from valid-empty ([]) - processExecResult now returns parsed status (null or object) - Runner checks parsed !== null to overwrite lastResults (not length > 0) - Preserves results from earlier successful parse when later attempt unparseable - Overwrites with latest valid parse (including empty []) FIX 3: Make negative override type-aware with renderInvalidOverride helper - Create helper: empty "" stays "", numeric finite → bare number, bool true/false → bare bool - Non-bool token for bool field → JSON string (type-rejection test) - Fix contradictory test: update title to match "notabool" string assertion - Add test: bare "false" renders as boolean false - Update generateNegativeBlocks JSDoc to reference renderInvalidOverride FIX 4: Add rationale doc notes - build.ts: Expand APP_SIDEBAR_FILE/APP_ROUTES_FILE comments explaining add-only trade-off - refine-prompt.ts: Note makeIdHandler is scaffold's gate-green convention (JoinRequestsPage) All tests pass (363 total). No `as` casts (except `as const`), no `any`, no eslint-disable, complexity ≤ 20. * fix(panel-r3): numeric negative override only bare for canonical literals; nav/add-only + non-canonical tests; fix test title - renderInvalidOverride: emit a bare number ONLY for a canonical decimal literal (/^-?\d+(\.\d+)?$/); non-canonical tokens ("01","0x10",whitespace,"1e5") stay raw strings so Number() can't mutate an intentionally-invalid value into a valid one. - tests: non-canonical numeric stays a raw string; refine-prompt reachability contract (AppSidebar + routes.tsx + APP_SIDEBAR_NAV_ITEMS + ADD ONLY); fixed the numeric test title to match its bare-number assertion. * fix(panel-r4): numeric override canonical via round-trip (fixes octal 01 SyntaxError); exact boolean match; gitignore dedup - renderInvalidOverride numeric: bare only when String(Number(value))===value (true round-trip). Rejects '01' (bare 01 = octal SyntaxError in strict mode), '1e5', '0x10', ' 5 ', huge→Infinity → all stay raw strings. Test now uses '01' (the octal case). - boolean: exact 'true'/'false' only (no trim/case-fold) so ' false '/'FALSE' stay raw strings testing the exact value. - JSDoc updated to the round-trip contract; .gitignore scratchpad patterns deduped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #168. Driven by a live CRM build that (once #168's fixes were in) parked on the near-green rotation.
Headline harness fixes (both validated live on a fresh build)
scopeFor()now includes the sharedAppSidebar.tsx+routes.tsx(add-only, exactly like the existing schema/locale shared-file exceptions), so a feature worker can register its sidebar link + route. Previously the freeze locked those files → features were unreachable by construction (the true root of build19's hollow-nav). Live: the model now edits both (10× sidebar, 2× router).refine-prompt.ts: themakeIdHandlerlist-row idiom (the scaffold's own gate-green pattern) that satisfiesjsx-no-bind— the dominant near-green churn. Live: jsx-no-bind dropped 50 → ~4. Plus render-list extraction, the real module split, no-hardcoded-text.API-negative check hardening
Deterministic API-level negative (POST invalid → assert 400/422, not any 4xx); type-aware invalid override via a round-trip canonical check (
String(Number(v))===v— '01'/'1e5'/'0x10'/huge stay raw strings, avoiding an octal SyntaxError and Number() mutation); exact boolean match; runner null-vs-empty result preservation. Tests added throughout.Known limitation (filed as follow-up): the sidebar/router add-only boundary is enforced by prompt + the differential gate, not the filesystem — the same accepted trade-off as the existing shared schema/locale scope (structural per-feature add-only enforcement is a separate, known-hard problem).
4-model panel: PASS. typecheck + lint + tests green.