feat(cli): wire the CLI to the framework it ships - #17
Conversation
- delete manifest-scan.ts, openapi.ts, surfaces.ts (400 LOC of duplicates) - app-load.ts loads an app into the framework's own registries; a primitive is what action()/entity()/defineRoute() registered, never a source scan - app-manifest.ts projects them with @ultimat3/manifest (retires 1455 LOC that had zero consumers); app-openapi.ts with @ultimat3/action - app-boundaries.ts delegates the three surface rules to @ultimat3/render's checkSurfaceBoundary — transitive and chain-naming, unlike the old walk — and keeps only the two layer rules no package owns - x verify gates on X_MANIFEST_DRIFT / X_MANIFEST_BREAKING / a stale openapi.json instead of the CLI's own two-shape diff - budgets.ts takes byte parsing from render; x routes prints describeRoutes() - a module that will not import is now a finding, not a silently missing primitive Co-Authored-By: Claude <noreply@anthropic.com>
- dev-runtime.ts starts the services dev-services.ts resolved: PGlite on disk, the real Postgres job queue on it, the in-process event bus and transport, a local-directory disk — each installed as the ambient accessor production installs - dev-render.ts serves every registered `route` through render's own mode function, so static/isr/ssr/stream/spa each earn their real headers and a real <head>; actions mount as POST /api/<resource>/<verb> - dev-hooks.ts wires the pipeline's `authorize` seam to the app's own Policy objects — action registry for actions, the route's declared permission for pages; deny when nothing is registered - dev-roles.ts makes --role real: web + sync + worker + scheduler start and stop; `migrate`/`replicator` are refused with X_CLI_BAD_FLAG rather than silently ignored. The role table is core's, not a CLI copy - render: head-seo.ts is the one binding of `HeadRenderers` to seo's tag vocabulary, so no caller writes a fourth private converter - http: ctx.actor is never null. `asCtx` publishes the request context as core's `Ctx`, whose `actor` is an `Actor` — an unauthenticated call now carries the anonymous actor, which is what stopped every public action from 500ing on `isAnonymous(null)` Co-Authored-By: Claude <noreply@anthropic.com>
- @ultimat3/admin gains a second door, `@ultimat3/admin/dev`: the /_x half is no longer re-exported from the package root, so mounting the panels never drags a Solid component tree into the `x dev` process - dev-dashboard.ts mounts admin's 9 panels and adds only the two no registry can answer — `services` and `boundaries` — plus the three `defaultDevSources` hooks this process owns (runSql on the live dev database, the caught outbox, emitted-vs-committed manifest). The four JSON stub endpoints the CLI used to serve are deleted - dev-runtime.ts installs a memory mail driver, so the mail panel reads a real outbox instead of throwing X_NOT_IMPLEMENTED - mcp-host.ts is the `DevCapabilities` half of @ultimat3/mcp's DevHost — db, migrations, queue depth, tests, logs, manifest, errors, verify. The descriptions come from `frameworkIntrospection`, so nothing here is a second catalog of routes, entities, actions or policies - cmd-mcp.ts drops its private 4-tool list and hand-rolled JSON-RPC for @ultimat3/mcp's 13-tool server, both transports, and structural SQL refusals; HTTP mints a per-process bearer token reported in --json - `cli -> admin` declared in scripts/lib/tiers.ts with its rationale: x dev mounts the dashboard, it never grows a second one 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: 24 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 (47)
📝 WalkthroughWalkthroughThis PR adds shared CLI app loading and manifest generation, a full ChangesCLI app inspection and generated contracts
Development runtime
MCP development host
Package boundary and rendering contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 29
🤖 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/admin/CLAUDE.md`:
- Line 5: Update the package boundary statement in CLAUDE.md to explicitly
document the CLI as the exception that imports `@ultimat3/admin/dev`, while
preserving the rule that admin views and the root barrel must not import
anything from src/dev/.
In `@packages/admin/src/dev/index.ts`:
- Around line 1-6: Compress the header comment above the dev entry point to no
more than four lines while preserving its essential explanation that
`@ultimat3/admin/dev` is a separate development-only entry point for mounting /_x
and avoiding the production component tree.
In `@packages/cli/src/app-boundaries.ts`:
- Around line 129-134: Update the X_BOUNDARY_SERVICE_TO_HTTP finding in the
app-boundary analysis to set fix to an exact runnable x ... command rather than
prose, while preserving the existing guidance in the cause and docs fields.
- Around line 121-134: Move the new CLI output strings into the flat catalog in
messages.ts and render them through msg(). In packages/cli/src/app-boundaries.ts
lines 121-134, replace the inline cause and fix templates in the boundary
findings with message keys and the required specifier/path parameters. In
packages/cli/src/cmd-routes.ts lines 62-64, replace the inline “routes” count
text with a messages.ts key using the count parameter.
- Around line 13-14: The node:* imports are required for Bun compatibility and
must remain; add why-comments at packages/cli/src/app-boundaries.ts lines 13-14,
packages/cli/src/app-load.ts line 6, and packages/cli/src/app-manifest.test.ts
lines 6-7 documenting their uses for POSIX graph paths, root-relative path
conversion, fixture path joining, and recursive fixture cleanup, respectively,
following CLAUDE.md line 87.
In `@packages/cli/src/app-load.ts`:
- Line 54: Update loadApp() to reload previously seen app paths during x dev
rescans by resetting the relevant registries and invalidating changed modules
before rebuilding the manifest; ensure changed route, action, and query exports
are reflected. Add fixture coverage for each changed data type.
In `@packages/cli/src/app-manifest.ts`:
- Around line 55-61: Update the package metadata loading flow around the
manifest identity construction to reject malformed package.json content or
missing/non-string name and version fields instead of returning app@0.0.0.
Return a finding or raise an UltimateError using a stable X_* code, and include
an actionable remediation command in the error; preserve the existing behavior
only when valid metadata is present.
- Line 5: Add a concise comment next to the node:path join import explaining
that Bun exposes the required platform-aware join API through node:path,
documenting the CLAUDE.md exception without changing the import.
In `@packages/cli/src/budgets.ts`:
- Around line 39-40: Update the budget verification flow around
byPath.get(route.url) so routes with declared JS or LCP budgets and no measured
stats produce an actionable registered finding instead of being skipped; only
skip routes without declared budgets. Add coverage for the missing-stat case and
ensure x verify checks the resulting finding.
In `@packages/cli/src/cmd-dev.ts`:
- Around line 131-138: Update the watchApp reload callback and its surrounding
route setup so appManifest rejections are caught and recorded as findings,
preserving the dev process. Also replace the boot-time buildId value passed to
appRoutes and startRoles with a getter or equivalent dynamic lookup of
state.manifest.buildId, so responses use the current build ID after reloads.
In `@packages/cli/src/cmd-manifest.ts`:
- Around line 43-61: Move the inline user-facing strings in the manifest check
flow to catalog entries in messages.ts. Add message keys for the fresh/stale
summary and stale-manifest cause, then use msg() at the corresponding summary
and cause expressions in the check handling near readAppManifest; preserve the
existing dynamic buildId interpolation and output behavior.
- Around line 66-78: The manifest command must not persist partial projections
when loader findings exist. In packages/cli/src/cmd-manifest.ts:66-78, guard
both writeAppManifest and OpenAPI output on findings being empty; otherwise
return the findings unchanged. In packages/cli/src/cmd-generate.ts:270-286, skip
writeAppManifest when findings are present, merge those findings into the
command result, and mark the command unsuccessful.
In `@packages/cli/src/cmd-mcp.test.ts`:
- Around line 97-152: Update serveHttp and the mcpCommand HTTP test lifecycle so
the server handle is returned or otherwise exposed when starting Bun.serve, then
store it during beforeAll and stop it in an afterAll hook. Ensure cleanup also
releases the lazy MCP services while preserving the existing request assertions.
- Around line 90-94: Await the promise returned by the rejects assertion in the
“an unknown transport is refused before the app is loaded” test. Update the test
so completion waits for expect(...).rejects.toThrow(...) and fails when
mcpCommand.run resolves or rejects with an unexpected error.
In `@packages/cli/src/cmd-mcp.ts`:
- Line 36: Ensure all renderer-visible facts are represented in command data: in
packages/cli/src/cmd-mcp.ts at lines 36-36 and 81-81, add the sorted
DEV_TOOL_SCOPES values as scopes to the catalog and serveHttp data objects; in
packages/cli/src/cmd-dev.ts at lines 196-212, add the manifest path as manifest
and move the inline manifest and introspect messages into messages.ts under
cli.dev.manifest and cli.dev.introspect.
- Around line 54-83: Update serveHttp to return the Bun.serve handle alongside
the existing command result, preserving the response data and announcing
behavior. Make the caller responsible for shutdown by exposing the handle needed
to stop the listener and call host.close(), and adjust the serveHttp return type
and callers accordingly so tests and embedding callers can release both
resources.
In `@packages/cli/src/cmd-verify.ts`:
- Around line 83-93: Update the verification step’s applies predicate and run
flow to support either generated file independently: apply when x.manifest.json
or openapi.json exists, generate the current manifest once, include
contractFindings only when a committed manifest is available, and run
specFindings whenever openapi.json exists. Preserve passed behavior when neither
file exists and use the existing manifest/spec path symbols.
In `@packages/cli/src/dev-dashboard.test.ts`:
- Around line 83-87: Update the panelFor helper to avoid throwing a bare Error
when no matching panel exists. Prefer replacing the throw with an assertion
failure; otherwise use the repository’s UltimateError subclass from ./errors
with a stable X_* code, cause, and executable fix instruction.
In `@packages/cli/src/dev-render.ts`:
- Around line 69-72: Update the ISR flow in packages/cli/src/dev-render.ts:69-72
and its related fixture/assertions in packages/cli/src/dev-render.test.ts:71-80
so the response cache header derives s-maxage from the declared revalidate.ttl
rather than the hardcoded 60-second value; keep SSR’s fixed s-maxage=30 policy
unchanged, and adjust the test expectations or fixture to verify the TTL-driven
header and expiry behavior.
In `@packages/cli/src/dev-roles.ts`:
- Around line 97-119: The startSync function currently derives the sync port
from options.port and constructs an unbound URL. Pass the requested port
directly to listenSyncNode so port 0 remains ephemeral, then return the
listener’s existing server.url.origin as url; keep listener.stop and node.stop
shutdown behavior unchanged.
In `@packages/cli/src/dev-runtime.ts`:
- Around line 106-137: Ensure startup failures unwind partially initialized
resources: in packages/cli/src/dev-runtime.ts lines 106-137, wrap the
post-startDb boot sequence in try/catch and, on failure, clear the database
client with setDbClient(undefined), close db, then rethrow; in
packages/cli/src/dev-roles.ts lines 121-138, record each role’s stop callback as
it starts and invoke recorded callbacks in reverse order on failure before
rethrowing, so a failed startSync also stops earlier roles.
- Around line 40-51: Update startDb to use the existing pgliteDataDir parser
from `@ultimat3/db` instead of manually removing PGLITE_SCHEME with slice. Reuse
the same parsing approach already used by mcp-host.ts, and remove the
now-unnecessary local scheme constant if it has no other uses.
In `@packages/cli/src/mcp-host.ts`:
- Around line 1-4: Split the independent concerns in mcp-host.ts into focused
modules: move database targeting symbols databaseTarget, safeLabel,
postgresBranch, and pgliteBranch to mcp-db-target.ts; CLI error symbols
CLI_FIXES, isCliCode, and explainErrorCode to mcp-errors.ts; and parseBunTest
with its helpers to mcp-test-output.ts. Keep lazyServices, capabilities, and
createDevMcpServer in mcp-host.ts, updating imports and exports so existing
behavior and tests remain unchanged.
- Around line 214-231: Update lazyServices so running() checks the closed state
before starting services; after close() has completed or begun, it must not
invoke startServices or create a new boot, while preserving the existing
started-promise behavior for calls before closure.
- Around line 255-272: Update runQuery to enforce the requested limit at the
database or streaming layer before DbClient.query materializes all rows, while
preserving the original SQL unchanged. Support EXPLAIN and SHOW commands without
wrapping statements in a derived SELECT, and retain accurate rowCount and
truncated behavior.
In `@packages/cli/tsconfig.json`:
- Around line 14-16: Add a TypeScript project reference for ../http alongside
the existing ../admin reference in packages/cli/tsconfig.json, ensuring
packages/cli/src imports the composite `@ultimat3/http` workspace project through
the configured project boundary.
In `@packages/render/src/head-seo.test.ts`:
- Around line 1-4: Add a one- to four-line module header before the imports in
the head-seo test module, stating that it tests the
`@ultimat3/seo-to-HeadRenderers` adapter and protects its expected rendering
behavior.
In `@wiki/Error-Codes.md`:
- Line 238: Update the X_MANIFEST_STALE entry in the error-code table to
document only stale openapi.json files; keep X_MANIFEST_DRIFT as the code for
stale manifest files, matching the cmd-verify.ts verification flow.
In `@wiki/Upgrading.md`:
- Around line 44-45: Update the breaking-contract row in the upgrade
documentation to identify X_MANIFEST_BREAKING as the error from verifyContract,
and change X_CONTRACT_DRIFT to describe client/server build skew. Replace the
nonexistent x contract accept command with the registered x verify --json
workflow, including a runnable remediation command or step.
🪄 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: 49783fc5-12f8-4529-80dd-6c3c74972b4d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!**/bun.lock
📒 Files selected for processing (58)
.gitignoreCLAUDE.mddocs/architecture/01-package-map.mdpackages/admin/CLAUDE.mdpackages/admin/README.mdpackages/admin/package.jsonpackages/admin/src/dev/index.tspackages/admin/src/index.tspackages/cli/CLAUDE.mdpackages/cli/README.mdpackages/cli/package.jsonpackages/cli/src/app-boundaries.test.tspackages/cli/src/app-boundaries.tspackages/cli/src/app-load.tspackages/cli/src/app-manifest.test.tspackages/cli/src/app-manifest.tspackages/cli/src/app-openapi.tspackages/cli/src/budgets.tspackages/cli/src/cmd-dev.test.tspackages/cli/src/cmd-dev.tspackages/cli/src/cmd-generate.tspackages/cli/src/cmd-manifest.tspackages/cli/src/cmd-mcp.test.tspackages/cli/src/cmd-mcp.tspackages/cli/src/cmd-routes.tspackages/cli/src/cmd-verify.tspackages/cli/src/dev-dashboard.test.tspackages/cli/src/dev-dashboard.tspackages/cli/src/dev-hooks.test.tspackages/cli/src/dev-hooks.tspackages/cli/src/dev-render.test.tspackages/cli/src/dev-render.tspackages/cli/src/dev-roles.test.tspackages/cli/src/dev-roles.tspackages/cli/src/dev-runtime.tspackages/cli/src/dev-services.tspackages/cli/src/index.tspackages/cli/src/manifest-scan.tspackages/cli/src/mcp-host.test.tspackages/cli/src/mcp-host.tspackages/cli/src/messages.tspackages/cli/src/openapi.tspackages/cli/src/surfaces.tspackages/cli/tsconfig.jsonpackages/http/CLAUDE.mdpackages/http/src/context.tspackages/http/src/pipeline.tspackages/http/src/request.tspackages/render/CLAUDE.mdpackages/render/src/head-seo.test.tspackages/render/src/head-seo.tspackages/render/src/index.tsscripts/boundaries.tsscripts/lib/tiers.tstsconfig.base.jsonwiki/CLI-Reference.mdwiki/Error-Codes.mdwiki/Upgrading.md
💤 Files with no reviewable changes (4)
- packages/cli/src/openapi.ts
- packages/cli/src/manifest-scan.ts
- packages/cli/src/dev-services.ts
- packages/cli/src/surfaces.ts
Correctness: - assertReadOnlyQuery returned the literal-stripped statement, and db.query executed it — `select 'delete from posts' as note` ran as `select as note`. Validation still reads the stripped form; the return is now verbatim. - ISR emitted a fixed s-maxage=60, ignoring the route's declared revalidate.ttl. - listenSyncNode hid its bound address; `port + 1` turned `--port 0` into a request for port 1. It now reports the socket it bound, and its shutdown hook is unregistered by stop() instead of hanging the next process-wide drain. - x dev: an unhandled rejection in the watcher took the dev server down; a failed reload is a finding now. appIdentity no longer fabricates app@0.0.0 (new X_APP_PACKAGE_INVALID) — the manifest version is the compatibility gate. - x manifest / x g no longer persist a manifest or openapi.json built from a partial load. x verify checks openapi.json even with no committed manifest. - A route declaring an unmeasured budget is X_BUDGET_UNMEASURED, not a pass. - startServices / startRoles unwind in reverse on a failed boot. - mcp host: running() after close() refused; the HTTP transport returns a stop handle, so the socket and the lazy services are releasable. Quality: - mcp-host.ts (379 LOC) split into mcp-db-target / mcp-errors / mcp-test-output. - Boundary findings carry runnable `x g ...` fixes; CLI strings moved to messages.ts; scopes and the manifest path ride in `data`, not only in `lines`. - No bare Error in tests; an unawaited `rejects` assertion could never fail. - node:* imports carry why-comments; docs and wiki error rows corrected. Co-Authored-By: Claude <noreply@anthropic.com>
The CLI shipped 12 framework packages and imported almost none of them. Three commits close that gap.
afc9ff9) — deletesmanifest-scan.ts/openapi.ts/surfaces.ts(~400 LOC of duplicates) for thin adapters over@ultimat3/manifest(previously zero consumers),@ultimat3/actionand@ultimat3/render. A primitive is whataction()/entity()/defineRoute()registered, never a source scan; load failures surface as findings instead of being.catch()-swallowed.x devboots the app (67f0ef8) — PGlite on disk, the real Postgres job queue, in-process events/transport, a local-directory disk, each installed as the ambient accessor production installs. Routes serve through render's own mode function, actions mount atPOST /api/<resource>/<verb>,--rolestarts web/sync/worker/scheduler for real. Fixes@ultimat3/httpsettingctx.actor = null, which 500'd every public action./_xand the MCP server (92b8cea) —@ultimat3/admingains a second door (@ultimat3/admin/dev) so mounting the panels never pulls a Solid component tree intox dev; the CLI's four JSON stub endpoints are replaced by admin's 9 panels plus the 2 only this process can answer.x mcp servedrops its private 4-tool list and hand-rolled JSON-RPC for@ultimat3/mcp's 13-tool dev server, both transports, structural SQL refusals, and a per-process bearer token.cli → adminis declared inscripts/lib/tiers.tswith its rationale:x devmounts the dashboard, it never grows a second one.Gate:
bun run verify15/15 green. Fullbun test1575 pass / 25 fail — the failure set is the unchanged pre-existingexamples/dummyfixture drift (page/subscribe/seed/evaluate), zero framework failures.Not in scope:
x buildstill does not runx verify's static steps first — that is the next task on this group's list.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
x devnow supports role selection, live runtime status, dashboard panels, route rendering, service monitoring, and manifest drift reporting.Bug Fixes
Documentation