feat: typed prisma.compute.ts compute config#84
Open
AmanVarshney01 wants to merge 17 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a complete compute-config system ( ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
|
Adds a typed, committed deploy configuration for Prisma Compute and makes the whole app command surface config-aware. Config: - prisma.compute.ts (.mts/.js/.mjs/.cjs) with defineComputeConfig from the new @prisma/cli/config export; single `app` or multi-target `apps` with name, root, framework, entry, httpPort, env, and build per app - compile-time safety (literal unions, app/apps exclusivity, typo suggestions) plus runtime validation reporting every issue at once - loaded via jiti with an alias so configs resolve @prisma/cli/config even when the package is not installed locally Resolution: - upward discovery from cwd to the source root (.git/workspace markers); nearest config wins and discovery never escapes the repository - the config directory is the project directory: local pin, state cache, and the --db schema scan anchor there; config-relative paths (root, env.file) resolve from the config file - target inference: commands run inside a target's root select it - app selection precedence: --app > PRISMA_APP_ID > config target > remembered selection > inference Command surface: - app deploy/build/run and every management command (show, open, logs, list-deploys, promote, rollback, remove, domain *) accept an [app] target argument - bare app deploy with a multi-app config deploys every target in declaration order, fail-fast with already-live/not-attempted reporting; per-app flags are rejected in deploy-all - run/build share deploy's framework detection (one detection, three commands), driven by a framework capability registry that replaces scattered framework literals Build settings: - the config build block owns build settings; otherwise they are inferred with sources shown; prisma.app.json is no longer read or written - leftover matching files warn, customized files fail with BUILD_SETTINGS_MIGRATION_REQUIRED including the exact build block - workspace builds: package-manager detection walks up to the workspace root, ancestor node_modules/.bin dirs join the build PATH, and Next.js standalone staging flattens bun's isolated store alongside pnpm's Database (--db flag flow, no config surface in this PR): - provision-only: deploy creates the branch database and wires DATABASE_URL/DIRECT_URL but never runs schema, migration, or generate commands; it suggests the detected command and a one-time connection URL via database connection create instead Packaging: - exports map with ./config (+ d.ts), jiti dependency, publish-prep now carries exports into the published manifest Spec updated across command-spec, resource-model, output- and error-conventions. 462 tests.
c12 was evaluated for config loading but the compute config loads via jiti directly; remove the dead dependency. Add a test workflow running the CLI suite and build on ubuntu and windows so Windows stays verified in CI.
AmanVarshney01
force-pushed
the
feat/prisma-compute-config
branch
from
9458ee3 to
6383c70
Compare
The skills CLI is contributor-machine DX and its '*' matching is broken on Windows, which failed pnpm install before any tests ran. Guard the prepare script behind a CI check.
- load shebang-bearing scripts via runtime file-URL imports (the transform pipeline broke on Windows; also removes two implicit-any suppressions) - normalize readlink output before asserting symlink targets - replace a posix-only /usr/bin/true bin symlink with a portable no-op build script - expect native path separators in project show display assertions
Out-of-root shebang scripts fail vitest's Windows transform regardless of import style, so the publish-prep and resolve-cli-version suites now exercise the scripts as child processes — which also matches how CI invokes them. prepare-cli-publish gains --source-dir so tests can drive it fully from the CLI. Home-shortened display paths now render posix style uniformly instead of Windows' mixed '~/code\apple'.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/tests/app-branch-database.test.ts (1)
772-781: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAssert the new provision-only
--dbcontract instead of stubbing schema setup.The PR changes
--dbto provision the database and wire env vars only, but these tests still mockrunBranchDatabaseSchemaSetup()to succeed. That means the suite would also pass if deploy regressed and started running Prisma schema setup again. Replace these stubs with a spy/assertion that schema setup is not invoked so the new behavior is actually protected.Also applies to: 895-904, 1013-1023, 1288-1297
🤖 Prompt for 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. In `@packages/cli/tests/app-branch-database.test.ts` around lines 772 - 781, Tests are still stubbing runBranchDatabaseSchemaSetup to succeed, hiding regressions; update the tests in packages/cli/tests/app-branch-database.test.ts to stop mocking its behavior and instead spy/assert that runBranchDatabaseSchemaSetup (from ../src/lib/app/branch-database) is NOT invoked when the CLI is run with the new provision-only --db contract. Concretely, remove the vi.doMock that mockResolvedValue for runBranchDatabaseSchemaSetup (or change it to re-export the real module), create a spy/mock reference to runBranchDatabaseSchemaSetup and add assertions like expect(runBranchDatabaseSchemaSetup).not.toHaveBeenCalled() after exercising the CLI, and apply the same replacement to the other test blocks flagged in the comment (the two other occurrences in this file).packages/cli/src/lib/app/production-deploy-gate.ts (1)
135-187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the resolved app target in production rerun guidance.
These helpers now always suggest bare
prisma-cli app deploy ...commands. In the new compute-config flow, if the blocked deploy was for a specific target, dropping that target can turn the retry into deploy-all or a target-required failure. Please thread the resolved target/original command into these errors so everynextSteps,nextActions, andhumanLinesexample replays the same app selection.🤖 Prompt for 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. In `@packages/cli/src/lib/app/production-deploy-gate.ts` around lines 135 - 187, The helper errors drop the resolved app target from their guidance; update productionDeployRequiresFlagError and productionDeployConfirmationRequiredError to accept a resolved target/original command (e.g., add a parameter like resolvedTarget or originalCommand) and interpolate that into all suggested invocations and examples: nextSteps, each nextActions.command, nextActions.commands arrays, and humanLines so the shown commands replay the exact app selection (e.g., "prisma-cli app deploy <resolvedTarget> --prod" and "prisma-cli app deploy <resolvedTarget> --prod --yes"); ensure string formatting handles when the target is empty (fall back to the current bare command) and update both function signatures and all message fields to use the parameter.packages/cli/src/lib/app/preview-build.ts (1)
241-245:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe Next.js full-tree fallback still drops hoisted workspace runtime dependencies.
When
output: "standalone"is absent, this path stages the artifact by copying onlyappPath. In npm/yarn/pnpm workspaces,next startcan still resolve runtime packages from ancestornode_modules, so the build can succeed and the staged artifact still fail at boot because those hoisted dependencies were never packaged. The standalone path was updated to be workspace-aware; this fallback needs the same source-root/ancestor dependency treatment.Also applies to: 294-303
🤖 Prompt for 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. In `@packages/cli/src/lib/app/preview-build.ts` around lines 241 - 245, The fallback that calls stageNextjsFullTreeFallbackArtifact currently copies only appPath and therefore omits hoisted workspace runtime dependencies; update the fallback to use the same workspace-aware ancestor dependency staging as the standalone code path (reuse the helper used by the standalone flow — the workspace-aware staging/collector function used by the standalone staging logic — so ancestor node_modules/runtime packages are included), and apply this change both where stageNextjsFullTreeFallbackArtifact is invoked (the fallback at directoryExists(...) false) and the other fallback site referenced around the 294-303 region; modify stageNextjsFullTreeFallbackArtifact (or wrap it) to detect workspace root/ancestor node_modules and copy hoisted runtime deps into the artifact rather than only copying appPath.
🤖 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 `@docs/product/command-spec.md`:
- Line 807: The canonical example in the compute-config docs includes a
`database` block that contradicts the stated schema (compute config does not
declare databases in beta) and will lead to invalid
configs/COMPUTE_CONFIG_INVALID; update the docs by removing the `database` block
from the canonical compute config example (or else change the earlier sentence
to state that a `database` field is allowed), and ensure the text references the
current beta behavior (databases remain controlled via the `--db`/`--no-db`
flags) so the `compute config` example, the note about databases, and the
example's `database` block are consistent.
- Around line 798-799: Multiple docs define conflicting contracts for
compute-config target resolution, local state anchoring, and error codes; pick a
single canonical model and update the other files to match it: choose the
authoritative behavior for project root detection (that `.prisma/local.json` and
`.prisma/cli/state.json` live in the discovered config directory and `--db` only
scans there for prompts), the config export shape (that configs may
default-export defineComputeConfig({ ... }) from `@prisma/cli/config` and that
defineComputeConfig is an identity helper so plain object exports work), and the
stable error code conventions; then edit docs/product/command-spec.md,
docs/product/resource-model.md, and docs/product/error-conventions.md so they
state the exact same rules and examples for compute-config target resolution,
local state anchoring, and error code naming/values.
In `@packages/cli/src/controllers/app.ts`:
- Around line 543-548: The code currently calls parseEnvInputs once for the
entire merged.envInputs using projectDir when merged.envInputsFromConfig is
true, causing CLI --env paths to resolve incorrectly; instead, split
merged.envInputs into config-sourced and cli-sourced groups, call
parseEnvInputs(configGroup, base=projectDir) and parseEnvInputs(cliGroup,
base=context.runtime.cwd), then merge the two parsed results and pass that
merged result into toOptionalEnvVars; update the logic around
merged.envInputsFromConfig, parseEnvInputs, toOptionalEnvVars, and ensure
projectDir and context.runtime.cwd are used as the respective bases.
- Around line 3041-3046: The code currently sets configAppName to
compute.target?.name ?? compute.target?.key, which incorrectly uses the config
identifier (compute.target.key) as a deployed app name; change this so
configAppName is only seeded when the config explicitly sets a name
(compute.target?.name) or when you derive the app name using the same inference
logic deploy uses from the target root (do not fall back to
compute.target?.key). Locate the callsite that uses resolveComputeTargetOrThrow
and the returned configAppName variable and replace the fallback to
compute.target?.key with either undefined or the result of reusing deploy’s
app-name inference against compute.target.root (the same inference function used
by deploy), ensuring only explicit target.name or the inferred name are used.
In `@packages/cli/src/lib/app/branch-database-deploy.ts`:
- Line 103: The call to inspectBranchDatabaseSignal currently uses
options.projectDir which is the compute-config dir and causes scanning of
sibling apps; instead thread the resolved app root (the target app root resolved
earlier in the flow) into inspectBranchDatabaseSignal so it scopes detection to
the selected app, leaving options.projectDir unchanged for local pin/state
anchoring; update the call site where
inspectBranchDatabaseSignal(options.projectDir, context.runtime.signal) is
invoked to pass the resolved target root variable (e.g., targetRoot or appRoot)
while keeping context.runtime.signal the same.
In `@packages/cli/src/lib/app/compute-config.ts`:
- Around line 468-477: The validator currently treats empty-string env values as
invalid by checking `typeof varValue !== "string" || varValue.length === 0`
inside the loop over `Object.entries(value.vars)` (in the compute config
validation block handling `value.vars`), which contradicts the public
`ComputeEnvConfig.vars: Record<string,string>` contract; change the check to
only verify that `varValue` is a string (remove the `length === 0` rejection) so
values like `""` are accepted and still pushed into `envInputs` as `VAR=`; keep
the same error message path using `label` and `varName` when the type is not a
string.
In `@packages/cli/src/lib/project/setup.ts`:
- Around line 56-68: The success result currently uses formatSetupDirectory(...)
which collapses the path to a basename and thus reports the wrong directory when
the pin was written outside context.runtime.cwd; update the return value in the
async Result generator to use the actual bound directory used for the pin (the
directory parameter passed into the function, or otherwise compute the correct
relative path from context.runtime.cwd to that directory) instead of the
collapsed formatSetupDirectory output, and ensure the same fix is applied to the
other return site noted (the block around the second return using
formatSetupDirectory). Reference: the directory parameter,
writeLocalResolutionPin, ensureLocalResolutionPinGitignore, and
formatSetupDirectory.
In `@packages/cli/src/shell/diagnostics-output.ts`:
- Around line 39-43: The code currently derives home from env.HOME; replace this
with a shared helper (e.g., getUserHome) that checks env.HOME, then
env.USERPROFILE, then combines env.HOMEDRIVE+env.HOMEPATH if present, and
returns a path.resolve(...) or null; export it from a common util module and
import it into packages/cli/src/shell/diagnostics-output.ts and
packages/cli/src/presenters/project.ts and use getUserHome() instead of the
env.HOME lookup so the existing logic that compares resolved to home (and builds
the ~/ relative path) works correctly on Windows.
In `@packages/cli/tests/app-controller.test.ts`:
- Around line 152-218: Add a failing "deploy-all" test: in the same test suite
mock deployApp to reject for the first target (e.g., when options.appName ===
"api") and resolve for others, call runAppDeploy(context, undefined, {
projectRef: "proj_123" }) and assert that deployApp was only called for "api"
(not "web") and that the controller stopped on the first error (result indicates
failure or the promise rejects); use the existing
requireComputeAuth/createPreviewAppProvider mocks and the same temp cwd setup so
the new test mirrors the successful case but validates fail-fast behavior for
runAppDeploy and deployApp.
- Around line 1463-1469: The test currently only asserts the command fragment in
the migration guidance; update the assertion in the runAppDeploy test to verify
the entire migrated build block is present in the `fix` text (e.g., include both
`command: "bun run custom-build"` and `outputDirectory: "dist"` and the
surrounding `build:` block), so that the `BUILD_SETTINGS_MIGRATION_REQUIRED`
error surfaces the full migration snippet; locate the expectation around
runAppDeploy in this test and replace the partial expect.stringContaining with
an assertion that checks the full migrated build block (or multiple
expect.stringContaining checks for `build:`, the command, and `outputDirectory`)
for complete coverage.
In `@packages/cli/tests/helpers/deploy-result.ts`:
- Around line 6-12: The helper asSingleDeployResult is too generic (T extends {
result: unknown }) and will misfire for other commands like runAppListDeploys
that also have a top-level result.deployments; update it to accept a
deploy-specific success shape instead (e.g., define a DeployCommandSuccess
interface/type with the exact result shape returned by the deploy command and
use T extends DeployCommandSuccess) or create a specialized helper (e.g.,
asSingleDeployResultForDeployCommand or assertSingleDeployResult) that narrows
to that deploy-result type; update the function signature and type assertions to
use that deploy-specific type (referencing asSingleDeployResult) so the
discriminator only applies to deploy-vs-deploy-all results.
---
Outside diff comments:
In `@packages/cli/src/lib/app/preview-build.ts`:
- Around line 241-245: The fallback that calls
stageNextjsFullTreeFallbackArtifact currently copies only appPath and therefore
omits hoisted workspace runtime dependencies; update the fallback to use the
same workspace-aware ancestor dependency staging as the standalone code path
(reuse the helper used by the standalone flow — the workspace-aware
staging/collector function used by the standalone staging logic — so ancestor
node_modules/runtime packages are included), and apply this change both where
stageNextjsFullTreeFallbackArtifact is invoked (the fallback at
directoryExists(...) false) and the other fallback site referenced around the
294-303 region; modify stageNextjsFullTreeFallbackArtifact (or wrap it) to
detect workspace root/ancestor node_modules and copy hoisted runtime deps into
the artifact rather than only copying appPath.
In `@packages/cli/src/lib/app/production-deploy-gate.ts`:
- Around line 135-187: The helper errors drop the resolved app target from their
guidance; update productionDeployRequiresFlagError and
productionDeployConfirmationRequiredError to accept a resolved target/original
command (e.g., add a parameter like resolvedTarget or originalCommand) and
interpolate that into all suggested invocations and examples: nextSteps, each
nextActions.command, nextActions.commands arrays, and humanLines so the shown
commands replay the exact app selection (e.g., "prisma-cli app deploy
<resolvedTarget> --prod" and "prisma-cli app deploy <resolvedTarget> --prod
--yes"); ensure string formatting handles when the target is empty (fall back to
the current bare command) and update both function signatures and all message
fields to use the parameter.
In `@packages/cli/tests/app-branch-database.test.ts`:
- Around line 772-781: Tests are still stubbing runBranchDatabaseSchemaSetup to
succeed, hiding regressions; update the tests in
packages/cli/tests/app-branch-database.test.ts to stop mocking its behavior and
instead spy/assert that runBranchDatabaseSchemaSetup (from
../src/lib/app/branch-database) is NOT invoked when the CLI is run with the new
provision-only --db contract. Concretely, remove the vi.doMock that
mockResolvedValue for runBranchDatabaseSchemaSetup (or change it to re-export
the real module), create a spy/mock reference to runBranchDatabaseSchemaSetup
and add assertions like
expect(runBranchDatabaseSchemaSetup).not.toHaveBeenCalled() after exercising the
CLI, and apply the same replacement to the other test blocks flagged in the
comment (the two other occurrences in this file).
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ff946fd2-7cd6-4732-bc6e-afb254968a7f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
.github/workflows/test.ymldocs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mddocs/product/resource-model.mdpackage.jsonpackages/cli/package.jsonpackages/cli/src/commands/app/index.tspackages/cli/src/config.tspackages/cli/src/controllers/app.tspackages/cli/src/lib/app/branch-database-deploy.tspackages/cli/src/lib/app/branch-database.tspackages/cli/src/lib/app/compute-config-discovery.tspackages/cli/src/lib/app/compute-config.tspackages/cli/src/lib/app/frameworks.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/src/lib/app/preview-build.tspackages/cli/src/lib/app/production-deploy-gate.tspackages/cli/src/lib/diagnostics.tspackages/cli/src/lib/fs/source-root.tspackages/cli/src/lib/project/resolution.tspackages/cli/src/lib/project/setup.tspackages/cli/src/presenters/app.tspackages/cli/src/presenters/project.tspackages/cli/src/shell/diagnostics-output.tspackages/cli/src/shell/runtime.tspackages/cli/src/types/app.tspackages/cli/tests/app-branch-database.test.tspackages/cli/tests/app-build.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/app-env-vars.test.tspackages/cli/tests/app-local-dev.test.tspackages/cli/tests/app-presenter.test.tspackages/cli/tests/compute-config.test.tspackages/cli/tests/helpers.tspackages/cli/tests/helpers/deploy-result.tspackages/cli/tests/production-deploy-gate.test.tspackages/cli/tests/publish-prep.test.tspackages/cli/tests/resolve-cli-version.test.tspackages/cli/tsdown.config.tsscripts/prepare-cli-publish.mjsscripts/prepare-skills.mjs
- drop the stale database block from the spec's canonical config example - add COMPUTE_CONFIG_TARGET_REQUIRED/UNKNOWN to error-conventions and the config-directory anchoring rule to resource-model - extract a shared shortenHomePath helper with Windows home fallbacks (USERPROFILE, HOMEDRIVE+HOMEPATH) for display paths - report the real bound directory when project binding targets an ancestor compute-config directory instead of a misleading basename - document the non-empty contract on ComputeEnvConfig.vars - lock in deploy-all fail-fast with a failing-target test, assert the full migrated build block, and type the deploy-result test helper
Member
Author
|
Triaged all 11 CodeRabbit findings — 7 addressed in 17bf777, 3 declined as incorrect, 1 partially: Fixed:
Declined:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/product/resource-model.md`:
- Line 40: Update the sentence to explicitly state that the pin file
`.prisma/local.json` is colocated in the same directory as a discovered
`prisma.compute.ts` (i.e., physically moved there) and that the "local CLI state
cache" is anchored to that config directory (clarify it includes the pin plus
CLI runtime artifacts such as build staging and local node_modules cache where
applicable), describe the fallback behavior when no `prisma.compute.ts` is found
(fall back to project root), and add a parenthetical noting that discovery is
bounded to the nearest `prisma.compute.ts` up to repository root markers to
clarify the search scope.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b8e4d200-a070-4e24-96ee-0d0b7c0da36f
📒 Files selected for processing (10)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/resource-model.mdpackages/cli/src/config.tspackages/cli/src/lib/fs/home-path.tspackages/cli/src/lib/project/setup.tspackages/cli/src/presenters/project.tspackages/cli/src/shell/diagnostics-output.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/helpers/deploy-result.ts
💤 Files with no reviewable changes (1)
- docs/product/command-spec.md
Registry entries make the CLI's deploy detection and config contract cover the same frameworks as the git-push path. Their build strategies own their builds, so config build blocks are rejected for them (BUILD_SETTINGS_UNSUPPORTED) and inferred settings describe exactly what the strategy runs.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/product/error-conventions.md (1)
173-230:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop the retired
APP_CONFIG_INVALIDcode.
COMPUTE_CONFIG_INVALIDnow covers compute-config load/validation failures, but the MVP list still advertisesAPP_CONFIG_INVALIDas a stable code. That leaves the canonical taxonomy inconsistent with the implementation and suggests a code path that no longer exists.🤖 Prompt for 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. In `@docs/product/error-conventions.md` around lines 173 - 230, The docs still list the retired error code APP_CONFIG_INVALID; remove that token from the enumerated list and any explanatory text and replace references with the active COMPUTE_CONFIG_INVALID where appropriate (see the symbols APP_CONFIG_INVALID and COMPUTE_CONFIG_INVALID in the diff), and ensure the "Recommended meanings" section documents COMPUTE_CONFIG_INVALID as the canonical code for compute-config load/validation failures; also scan the file for any other mentions of APP_CONFIG_INVALID and delete or rewrite them to reference COMPUTE_CONFIG_INVALID so the taxonomy matches the implementation.docs/product/command-spec.md (1)
108-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict auto-creation to
app deploy.This fallback chain still includes “create the inferred app,” but the next paragraph applies it to
show,open,logs,list-deploys,promote,rollback,remove, anddomain. That would let read-only commands mutate remote state when no app exists.🤖 Prompt for 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. In `@docs/product/command-spec.md` around lines 108 - 126, The docs currently allow automatic app creation ("create the inferred app") for all commands; restrict auto-creation so only the `app deploy` command performs step 7 (automatic creation of the inferred app in the resolved branch). Update the fallback order in the command spec: keep steps 1–6, 8–9 for management/read-only commands (`show`, `open`, `logs`, `list-deploys`, `promote`, `rollback`, `remove`, `domain`) but remove step 7 for those commands, and explicitly state that only `app deploy` will perform creation when no existing app matches; ensure the paragraph describing management commands reflects this change and clarifies that non-deploy commands will not mutate remote state.
🤖 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/controllers/app.ts`:
- Around line 167-173: The current gate skips config-backed build settings when
merged.buildType is "auto", causing committed build blocks to be ignored; update
the logic in controllers/app.ts so you resolve the detected framework/strategy
before deciding to skip config-backed settings (or, after auto-detection,
re-evaluate buildType and call assertConfigBackedBuildSettings() and
resolveConfiguredPreviewBuildSettings() if the detected strategy is
config-backed). Concretely, ensure compute.framework/strategy detection runs
prior to the if (compute.target?.build && buildType !== "auto") check (or add a
post-detection check that reassigns buildType and invokes
assertConfigBackedBuildSettings and resolveConfiguredPreviewBuildSettings for
compute.target.build when the detected strategy is config-backed) so committed
build blocks trigger BUILD_SETTINGS_UNSUPPORTED or are applied via
resolveConfiguredPreviewBuildSettings as appropriate.
---
Outside diff comments:
In `@docs/product/command-spec.md`:
- Around line 108-126: The docs currently allow automatic app creation ("create
the inferred app") for all commands; restrict auto-creation so only the `app
deploy` command performs step 7 (automatic creation of the inferred app in the
resolved branch). Update the fallback order in the command spec: keep steps 1–6,
8–9 for management/read-only commands (`show`, `open`, `logs`, `list-deploys`,
`promote`, `rollback`, `remove`, `domain`) but remove step 7 for those commands,
and explicitly state that only `app deploy` will perform creation when no
existing app matches; ensure the paragraph describing management commands
reflects this change and clarifies that non-deploy commands will not mutate
remote state.
In `@docs/product/error-conventions.md`:
- Around line 173-230: The docs still list the retired error code
APP_CONFIG_INVALID; remove that token from the enumerated list and any
explanatory text and replace references with the active COMPUTE_CONFIG_INVALID
where appropriate (see the symbols APP_CONFIG_INVALID and COMPUTE_CONFIG_INVALID
in the diff), and ensure the "Recommended meanings" section documents
COMPUTE_CONFIG_INVALID as the canonical code for compute-config load/validation
failures; also scan the file for any other mentions of APP_CONFIG_INVALID and
delete or rewrite them to reference COMPUTE_CONFIG_INVALID so the taxonomy
matches the implementation.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 39374dc9-8169-4d68-9f73-c99fc944a144
📒 Files selected for processing (9)
docs/product/command-spec.mddocs/product/error-conventions.mdpackages/cli/src/config.tspackages/cli/src/controllers/app.tspackages/cli/src/lib/app/compute-config.tspackages/cli/src/lib/app/frameworks.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/tests/app-build.test.tspackages/cli/tests/compute-config.test.ts
…nfig The contract (types, framework registry, validation, discovery, jiti loading) now lives in @prisma/compute-sdk@0.23.0. The CLI deletes its copies (config.ts, compute-config-discovery.ts, frameworks.ts, fs/source-root.ts) and keeps only CLI concerns: flag/config precedence merging and CliError presentation. The @prisma/cli/config export is removed rather than re-exported — configs import the helper from @prisma/compute-sdk/config, which the loader resolves without a local install. jiti moves to the SDK.
…m review app build with a config build block but no declared framework now detects the framework the way deploy does instead of silently ignoring the block (FRAMEWORK_NOT_DETECTED when nothing is detectable). Docs: retire APP_CONFIG_INVALID in favor of the COMPUTE_CONFIG_*/BUILD_SETTINGS_* codes and scope auto-creation (resolution step 7) to app deploy only.
…e-config Conflicts resolved to this branch's semantics (main's side predates the compute-config feature); the new Biome formatting and lint rules were then applied across the branch.
selectPrismaNextConfig gains mode-specific overloads so supported and unsupported selections carry their actual target unions, and the deploy-specific asSingleDeployResult helper is no longer misapplied to show/domain/show-deploy/rollback results in tests.
readLocalGitBranch only looked at cwd/.git, so a deploy run from inside a monorepo package found no repository and silently fell back to the main branch — deploying feature-branch code to main. It now walks up like git does, with the nearest repository as a hard boundary (detached HEAD stays null rather than escaping to an outer repo). Also fixes the command spec to say the local pin anchors at the config directory, matching resource-model.md and the implementation.
The Read more line pointed at docs/product/command-spec.md — a file in this repository, meaningless to anyone running the published CLI. Removed until a public docs URL exists; the project/app/deployment header stays.
After a deploy-all, bare app logs follows the remembered selection (the last target deployed), so suggesting it under every app pointed at the wrong logs for all but one. Each block now suggests prisma-cli app logs <target>.
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.
What
Adds
prisma.compute.ts— a typed, committed deploy configuration for Prisma Compute — and makes the entireappcommand surface config-aware. One config declares the deployable system (apps, roots, frameworks, ports, env, build settings); the CLI derives everything else, with flags as one-shot overrides and detection filling whatever is omitted.Design rules
.git/workspace markers); never escapes the repository; location-only discovery (no config evaluation) at CLI bootstrap..prisma/local.jsonpin, the state cache, and the--dbschema scan anchor there; config-relative paths resolve from the config file so a committed config means the same thing from any cwd.--app>PRISMA_APP_ID> config target (positional[app]or cwd-inferred, deepest root wins) > remembered selection > inference.--envflags replace config env entirely.app deployagainst a multi-app config deploys the system (declaration order, fail-fast with already-live/not-attempted reporting, per-app flags rejected,--create-projectbinds once).@prisma/compute-sdk/config— adding a framework is one registry entry plus its strategy. Covers nextjs, nuxt, astro, hono, tanstack-start, bun (matching the git-push path). For nuxt/astro the strategy owns the build, so a configbuildblock is rejected (BUILD_SETTINGS_UNSUPPORTED) instead of silently ignored; with a build block and no declared framework,app buildresolves the framework like deploy does before applying it.Also in this PR
prisma.app.jsonremoved: never read or written. Leftovers that match inference warn for deletion; customized ones fail withBUILD_SETTINGS_MIGRATION_REQUIREDshowing the exactbuildblock — builds never change silently.node_modules/.bindirs join the build PATH; Next.js standalone staging flattens bun's isolated store alongside pnpm's.--dbis provision-only: creates the branch database and wiresDATABASE_URL/DIRECT_URL; never runsdb push/migrate deploy/generate. Suggests the detected schema command plusdatabase connection create <id>(env values are write-only on the platform by design).resolvePrismaInvocation, pinnednpx prisma@<x>fallback): the rebase keeps feat(app): degrade gracefully when standalone output or the prisma CLI is missing #82's Next.js full-tree fallback work intact and removes only the schema-command execution it improved, since that execution path no longer exists.@prisma/compute-sdk/config(extracted in prisma/project-compute#86, released as 0.23.0) — single source of truth shared with the build-runner. The CLI keeps only its concerns: flag/config precedence merging and CliError presentation. There is no@prisma/cli/configexport; configs import the helper from@prisma/compute-sdk/config, which the loader resolves without a local install. Publish-prep now carriesexportsinto the published manifest (was silently dropping it).test.ymlworkflow running the CLI suite and build on ubuntu and windows; new-code Windows audit done (platform-aware separators/delimiters throughout).c12dependency.prisma→prisma-cli).New error codes
COMPUTE_CONFIG_INVALID,COMPUTE_CONFIG_TARGET_REQUIRED,COMPUTE_CONFIG_TARGET_UNKNOWN,BUILD_SETTINGS_MIGRATION_REQUIRED,BUILD_SETTINGS_UNSUPPORTED. Retired:APP_CONFIG_INVALID.Verification
bunx create-better-t-stackscaffold (bun workspaces + catalogs + workspace imports) deployed with a 20-line config — which surfaced and fixed the bun-store staging bug.Out of scope / follow-ups
databaseas config surface (designed, deliberately deferred; spec carries a reserved-growth-path note).compute-branching.mdstages sketch (branch mapping in config) needs a deliberate reconciliation.JSON output shapes changed in beta: deploy-all aggregate result,
deploySettings.confignow{ path: string|null, status: "config"|"inferred" },branchDatabase.schemaremoved.