Repository navigation
fix(cli): scaffold drift gate — templates must compile - #13
Conversation
- t comes from @ultimat3/schema, not action/query/jobs/entity - tag comes from @ultimat3/cache, not @ultimat3/policy - entity templates use the real column builders (uuid/text/money/ timestamp/invariant) and $row/$name/$assert, not an imagined t.* chain - app.config.ts: defineConfig with a real AppConfigInput body, exported as a named `config` — the repo forbids default exports - packages/mcp: defineAppMcp, exported as `mcp`; its test asserts the AppMcp shape it actually returns - generated package.json gains @ultimat3/cache + @ultimat3/schema Co-Authored-By: Claude <noreply@anthropic.com>
- add `scaffold-typecheck.ts`: writes `x new` + every `x g` generator to a
sandbox and compiles it with the real tsc against the real workspace packages
- `contract · generated code compiles` fails on any diagnostic outside
KNOWN_GAPS, and on any pinned gap that stops reproducing
- fix the 51 diagnostics it found: action imports resolve one directory up,
repo speaks `db()` + `sql` instead of a builder that never existed, service
input derives from the row, Solid 2's `For` yields an accessor, the app's MCP
projects from the registry, `health` says `allow('public')` out loud
- generated `packages/db` wraps `@ultimat3/db` — drops the drizzle client that
duplicated it, and the drizzle dependency with it
- generated apps carry `types/scss.d.ts`; table names are snake_case plurals
Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdded a real TypeScript compiler harness for generated scaffolds. Updated generated templates for current schema, policy, database, MCP, route, job, and testing APIs. ChangesScaffold Template Alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI scaffold harness
participant Sandbox as Temporary sandbox
participant Workspace as Workspace package sources
participant TSC as TypeScript compiler
CLI->>Sandbox: Write generated fixture files
CLI->>Workspace: Link workspace dependencies and path mappings
CLI->>TSC: Run tsc --noEmit
TSC-->>CLI: Return compiler diagnostics
CLI-->>CLI: Report unexpected diagnostics and stale known gaps
``
</details>
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>
### ❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
| :----------------: | :--------- | :----------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
<details>
<summary>✅ Passed checks (4 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :--------------------------------------------------------------------------------------------------------------- |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the CLI scaffold compilation gate and the template fixes that are the main changes. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches 💡 1</summary>
<!-- finishing_touch_suggestion:docstrings -->
<details>
<summary>📝 Generate docstrings 💡</summary>
- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch
</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Commit unit tests in branch `fix/scaffold-template-symbols`
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/scaffold-typecheck.ts`:
- Around line 168-174: The typecheckScaffold fixture-writing loop must prevent
GeneratedFile.path from escaping the temporary sandbox. Resolve each file path
against dir, reject absolute or traversal-resolved paths outside dir before
Bun.write, and throw the package’s UltimateError subclass using a stable X_*
code owned by src/errors.ts, preserving the cause and an executable fix:
message.
- Around line 100-125: Update KNOWN_GAPS, matches, unexpectedIn, and staleGapsIn
so each expected diagnostic occurrence is represented separately and matched
against its sandbox-relative file and occurrence, with each actual match
consumed only once; report surplus matching diagnostics as unexpected and mark
only unfulfilled entries stale. Add coverage for two identical matching
diagnostics to ensure an unpinned extra occurrence fails.
- Around line 67-84: Update parseDiagnostics to split output using a CRLF-aware
delimiter (/
?\n/) so AT_FILE matches file diagnostics from Windows-style
output; add a corresponding CRLF test case in the scaffold typecheck tests
covering the preserved file diagnostic and KNOWN_GAPS evaluation.
- Around line 5-7: Document the rationale for the node: imports adjacent to the
imports in scaffold-typecheck.ts: explain that node:fs, node:os, and node:path
are required for temporary-directory creation, cleanup, symlinking, and path
resolution. Keep the existing imports unchanged.
In `@packages/cli/src/templates/entity.ts`:
- Around line 88-108: The generated entity fixture in
packages/cli/src/templates/entity.ts:88-108 uses bigint minor units while the
guideline-facing Money type uses number; update the row fixture and
negative-price assertion to use numeric 1000 and -1, unless the entity column
builder is intentionally bigint. In
packages/cli/src/templates/scaffold-repo.ts:222-225, keep the numeric minor
values and ensure they match the type declared by the money() column builder.
- Around line 46-78: Rename the second parameter of repoSource from snake to
table, then update every interpolation that uses ${snake} in the SQL statements
and dbDrift call to use ${table}; preserve the existing generated behavior.
In `@packages/cli/src/templates/query.ts`:
- Around line 21-30: The generated query and action templates use hyphenated
plural table names instead of the snake_case identifiers created by entityFiles.
In packages/cli/src/templates/query.ts lines 21-30, derive the snake_case table
name from feature.pluralKebab and pass and use it consistently in
querySource/from; in packages/cli/src/templates/action.ts lines 61-66, apply the
same conversion for mutatorSource/tx.table.
In `@packages/cli/src/templates/route.ts`:
- Around line 17-19: Derive JS_BUDGET from the existing BUDGET definition
instead of maintaining a separate hardcoded map. Update the JS_BUDGET
declaration near REVALIDATE to project each surface’s JavaScript budget from
BUDGET, preserving the assertions that reference JS_BUDGET and the route
emission using BUDGET.
In `@packages/cli/src/templates/scaffold-app.ts`:
- Around line 101-102: Update the scaffold templates and x g policy generation
to create and use one canonical PermissionRegistry via definePermissions(),
replacing inline dashboard:read, admin:read, and ${feature}:read/write literals.
Ensure generated apps/admin consumes the shared permissions declaration from
packages/admin/src/permissions.ts, and reference that registry when assigning
the policy permission.
In `@packages/cli/src/templates/scaffold-repo.ts`:
- Around line 366-373: Update the generated test in mcpTest so the optional
tool.description is validated without directly accessing .length when it is
absent. Preserve the requirement that every projected tool has a non-empty
description, but make missing descriptions produce a failed expectation rather
than a TypeError.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc372327-a133-4f90-bd4d-67a06decb75e
📒 Files selected for processing (12)
packages/cli/src/scaffold-typecheck.contract.test.tspackages/cli/src/scaffold-typecheck.test.tspackages/cli/src/scaffold-typecheck.tspackages/cli/src/templates/action.tspackages/cli/src/templates/entity.tspackages/cli/src/templates/job.tspackages/cli/src/templates/policy.tspackages/cli/src/templates/query.tspackages/cli/src/templates/resource.tspackages/cli/src/templates/route.tspackages/cli/src/templates/scaffold-app.tspackages/cli/src/templates/scaffold-repo.ts
- naming: NameSet owns `snake` + `table`; query/action/mutator now emit the entity's snake_case table instead of the kebab plural (`x g resource user-profile` generated `"user-profiles"` against a `user_profiles` table) - route: derive the test's js budget from BUDGET instead of a second map - entity: rename repoSource's `snake` param to `table`; note why money minor units are bigint on the row - scaffold-repo: assert `tool.description` itself, not its length - scaffold-typecheck: document why node:fs/os/path are unavoidable; split diagnostics on /\r?\n/ so CRLF output still matches a pin; pin each known gap to an exact file + message and spend it once, so a surplus occurrence fails the gate; reject a generated path that resolves outside the sandbox with the new X_SCAFFOLD_PATH_ESCAPE Co-Authored-By: Claude <noreply@anthropic.com>
x newscaffolded 79 files that could not compile: templates are template-literal strings, so no compiler ever read them, andcmd-generate.test.tsonly asserted they parse.packages/cli/src/scaffold-typecheck.tswritesx new --exampleplus everyx ggenerator to a sandbox and runs the realtscagainst the real workspace packages.contract · generated code compilesfails on any diagnostic outsideKNOWN_GAPS— and on any pinned gap that stops reproducing, so a pin cannot outlive its bug. Lights upx verify'scontractstep, which had nothing to run before.3b2fe45):tfrom@ultimat3/schema,tagfrom@ultimat3/cache,defineApp→defineConfig,defineTools→defineAppMcp, entity template rewritten against the real column builders.60d6f2c): action imports resolve one directory up (actions/is a subdirectory);repo.tsspeaksdb()+sqlinstead of a query builder@ultimat3/dbnever exported;Create<F>Inputderives from the row; Solid 2'sForyields an accessor;defineAppMcpprojects from the registry viainclude: 'exposed'rather than re-listing;healthdeclaresallow('public'), sincecan()takes aresource:verb. Generatedpackages/dbnow wraps@ultimat3/db— the drizzle client duplicated it, and thedrizzle-ormdependency goes with it. Generated apps carrytypes/scss.d.ts; table names are snake_case plurals.One pinned gap remains:
InvariantColumnsis an index-signature type, soc.titleisColumnExpr | undefinedundernoUncheckedIndexedAccess. Every hand-written entity inexamples/dummyreproduces it identically — the fix is a typed column proxy in@ultimat3/entity, not a different template.bun run verify15/15 · 1168 tests, 0 failures.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit