feat(config): introduce app config file#63
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThis PR adds prisma.app.json config handling for preview app builds: read-or-create lifecycle with validation, framework-default resolution (Next.js, TanStack Start, Bun), package-manager detection, build command execution, Next.js static config extraction, CLI output for created/used settings, wiring into preview build strategies and deploy provider, and tests plus docs updates. ChangesBuild Settings Configuration for Preview Deploys
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/product/error-conventions.md (1)
157-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
APP_CONFIG_INVALIDto the stable MVP list, not just the meanings section.Line 213 introduces a new public code, but the authoritative MVP list in Lines 161-200 still omits it. That makes the error contract internally inconsistent for agents and CI that treat the stable-set list as the source of truth.
🤖 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 157 - 213, The MVP list omits the `APP_CONFIG_INVALID` code while it appears in the meanings section; update the stable MVP codes list (the bullet list under "These codes are the minimum stable set for the MVP") to include `APP_CONFIG_INVALID` so the authoritative list matches the meanings section (add the exact token `APP_CONFIG_INVALID` to the list of codes).
🤖 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/lib/app/preview-build-settings.ts`:
- Around line 297-346: The code currently defaults an unknown package manager to
"bun", which can write a Bun-specific build command incorrectly; change
resolvePackageManager to return Promise<PackageManager | undefined> (remove the
final return "bun") so it yields undefined when no package manager is inferred
(keep using packageManagerFromPackageJson and the lockfile checks), then update
the caller that uses resolvePackageManager (the if branch that builds command
when hasBuildScript(...) is true) to check for a truthy packageManager and only
return { command: `${packageManager} run build`, source: "package.json
scripts.build" } when packageManager is present; otherwise fall back to
returning the provided fallback.command and fallback.source. Ensure function
signatures/types are updated accordingly.
- Around line 440-457: The current regex-based readStaticDistDir incorrectly
captures distDir from comments/strings/unrelated objects in raw next.config.*;
update readStaticDistDir (and the readNextConfig → resolveNextOutputRoot flow)
to parse the source AST and extract distDir only from the exported Next config
object with high confidence: use a JS/TS parser (e.g., `@babel/parser` or
ts-morph) to find the module.exports/export default object, read a direct string
Literal value for distDir (reject computed/concatenated values), validate it is
a simple relative path (no absolute, "..", or path traversal) and only then
return the normalized posix path for outputDirectory/prisma.app.json; otherwise
return undefined so mistaken/commented values cannot persist.
- Around line 103-116: The code unconditionally writes a standalone path for
Next.js (`joinPosix(outputRoot, "standalone")`) even when next.config may not
enable output:"standalone", causing stale persisted settings; update
resolvePreviewBuildSettings to detect the actual Next.js output mode (use or
extend resolveNextOutputRoot to return the resolved outputRoot plus the output
mode or call a helper that inspects next.config for
output:"standalone"|"export") and only set outputDirectory to
joinPosix(outputRoot, "standalone") and set outputDirectorySource to "Next.js
output" when the detected mode is "standalone"; for other modes (e.g., "export"
or default) either set outputDirectory to the plain outputRoot or leave it unset
and ensure resolveOrCreatePreviewBuildSettings does not persist an inferred
standalone path before the build runs (persist only when outputDirectorySource
confirms standalone or after a successful build).
In `@packages/cli/src/lib/app/preview-build.ts`:
- Around line 363-369: The NEXT_CONFIG_FILENAMES array in preview-build.ts
should not include "next.config.cjs" because Next.js doesn't support that root
filename; remove "next.config.cjs" from that constant and update the other
in-repo Next.js candidate lists to match (remove "next.config.cjs" wherever Next
config filename lists are defined), specifically in the detectNextConfig() logic
in packages/cli/src/controllers/app.ts and the filename arrays in local-dev.ts
and preview-build-settings.ts so all detections use only supported names (e.g.,
next.config.js, .mjs, .ts, .mts) and stay consistent across functions.
---
Outside diff comments:
In `@docs/product/error-conventions.md`:
- Around line 157-213: The MVP list omits the `APP_CONFIG_INVALID` code while it
appears in the meanings section; update the stable MVP codes list (the bullet
list under "These codes are the minimum stable set for the MVP") to include
`APP_CONFIG_INVALID` so the authoritative list matches the meanings section (add
the exact token `APP_CONFIG_INVALID` to the list of codes).
🪄 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: 320e6bca-e80f-4a78-91d1-b0d8d81d1849
📒 Files selected for processing (11)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mddocs/product/resource-model.mdpackages/cli/src/controllers/app.tspackages/cli/src/lib/app/bun-project.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/src/lib/app/preview-build.tspackages/cli/src/lib/app/preview-provider.tspackages/cli/tests/app-build.test.tspackages/cli/tests/app-controller.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/cli/src/lib/app/preview-build-settings.ts (1)
450-456:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe parsed Next.js
outputmode is still ignored.These lines now read
output, butresolvePreviewBuildSettings()still always persists${outputRoot}/standalonefor Next.js. For default-mode oroutput: "export"apps, that writes a staleprisma.app.json, andpackages/cli/src/lib/app/preview-build.ts:229-244later treats that persisted directory as mandatory standalone output. Only append/standalonewhen the parsed mode is actuallystandalone.🤖 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-settings.ts` around lines 450 - 456, The code reads the Next.js output mode into the local variable output (via readStaticStringProperty) but resolvePreviewBuildSettings() and the persistence logic still unconditionally append "/standalone"; update resolvePreviewBuildSettings() and any persistence in preview-build.ts to only append "/standalone" when output === "standalone" (use the parsed output variable), otherwise persist the outputRoot as-is (or append nothing) so default or "export" modes don't produce a stale standalone directory; look for symbols readStaticStringProperty, output, resolvePreviewBuildSettings, and the persistence logic in preview-build.ts and gate the "/standalone" suffix behind output === "standalone".
🤖 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/lib/app/preview-build-settings.ts`:
- Around line 297-301: The current branch returns the framework fallback when
packageManager is falsy, which silently replaces a declared package.json
scripts.build; change the logic so that if packageManager is falsy but
packageJson.scripts?.build exists you preserve and return that literal script
(set command to packageJson.scripts.build and source to something like
"package.json:scripts.build"); if neither packageManager nor
packageJson.scripts?.build exist then fail closed (throw or return an explicit
error/null) instead of returning fallback.command/fallback.source. Update the
code paths around packageManager, fallback, and the returned { command, source }
to implement this behavior.
In `@packages/cli/tests/app-build.test.ts`:
- Around line 357-379: The test currently includes Bun-specific signals (e.g.,
package.json.module and index.ts) so next.config.cjs isn’t isolated; update the
fixture so the only potential Next.js indicator is next.config.cjs by removing
Bun signals: write a minimal package.json (e.g., {} or with only benign fields,
no "module" or "type":"module"), remove or avoid creating index.ts that suggests
Bun, and keep only next.config.cjs in appPath; then call
resolvePreviewBuildStrategy({ appPath, buildType: "auto" }) and assert it
resolves to the non-Next fallback (the existing expectation of buildType: "bun")
to ensure next.config.cjs is not treated as a Next.js signal.
---
Duplicate comments:
In `@packages/cli/src/lib/app/preview-build-settings.ts`:
- Around line 450-456: The code reads the Next.js output mode into the local
variable output (via readStaticStringProperty) but resolvePreviewBuildSettings()
and the persistence logic still unconditionally append "/standalone"; update
resolvePreviewBuildSettings() and any persistence in preview-build.ts to only
append "/standalone" when output === "standalone" (use the parsed output
variable), otherwise persist the outputRoot as-is (or append nothing) so default
or "export" modes don't produce a stale standalone directory; look for symbols
readStaticStringProperty, output, resolvePreviewBuildSettings, and the
persistence logic in preview-build.ts and gate the "/standalone" suffix behind
output === "standalone".
🪄 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: 342c5440-2b83-4498-bcce-a87223e93261
📒 Files selected for processing (5)
docs/product/error-conventions.mdpackages/cli/src/controllers/app.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/src/lib/app/preview-build.tspackages/cli/tests/app-build.test.ts
💤 Files with no reviewable changes (2)
- packages/cli/src/controllers/app.ts
- packages/cli/src/lib/app/preview-build.ts
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 (1)
docs/product/command-spec.md (1)
631-633:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow the Build Command rule to match actual fallback behavior.
This wording currently reads as unconditional. Implementation uses
<package-manager> run buildonly when a package manager is inferred; otherwise it preserves the literalscripts.buildcommand. Please document that branch explicitly to avoid a spec/behavior contract mismatch.Suggested doc edit
- - `Build Command` prefers `<package-manager> run build` when `package.json` has `scripts.build` + - `Build Command` uses `<package-manager> run build` when `package.json` has `scripts.build` and a package manager can be inferred; otherwise it uses the literal `scripts.build` value🤖 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 631 - 633, Update the "Build Command" rule to explicitly describe the conditional branches: when package manager is inferred use "<package-manager> run build"; when no package manager is inferred preserve the literal "scripts.build" command from package.json; and only if there is no scripts.build at all fall back to the framework default (e.g., "next build"). Reference the existing terms "Build Command", "scripts.build", and "package-manager" so readers can locate the corresponding implementation behavior and ensure the doc matches the code path that infers a package manager vs the path that preserves the literal script.
♻️ Duplicate comments (1)
packages/cli/src/lib/app/preview-build-settings.ts (1)
117-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't persist
<distDir>/standaloneunless Next output mode is actuallystandalone.
readStaticNextConfig()already extractsoutput, but the Next.js branch always writesjoinPosix(outputRoot, "standalone"). This can persist an invalidoutputDirectoryintoprisma.app.jsonfor non-standalone builds.Suggested fix
-const outputRoot = await resolveNextOutputRoot(options.appPath, options.signal); +const nextConfig = await readNextConfig(options.appPath, options.signal); +const outputRoot = nextConfig.distDir ?? ".next"; +const isStandalone = nextConfig.output === "standalone"; return { buildCommand: buildCommand.command, buildCommandSource: buildCommand.source, - outputDirectory: joinPosix(outputRoot, "standalone"), - outputDirectorySource: outputRoot === ".next" ? "Next.js output" : "next.config distDir", + outputDirectory: isStandalone ? joinPosix(outputRoot, "standalone") : outputRoot, + outputDirectorySource: isStandalone + ? (outputRoot === ".next" ? "Next.js output" : "next.config distDir") + : (outputRoot === ".next" ? "Next.js output root" : "next.config distDir"), };In Next.js, what output directories are produced for `output: "standalone"` vs default/non-standalone builds, and should tools assume `<distDir>/standalone` only when `output` is explicitly `"standalone"`?Also applies to: 414-417, 456-463
🤖 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-settings.ts` around lines 117 - 123, The code unconditionally appends "standalone" to outputRoot; instead detect Next's output mode (the extracted `output` from readStaticNextConfig / the local variable holding Next config) and only set outputDirectory to joinPosix(outputRoot, "standalone") when output === "standalone"; otherwise set outputDirectory to outputRoot (or the appropriate distDir), and adjust outputDirectorySource accordingly. Update the same logic in the other spots noted (around the functions/returns that use resolveNextOutputRoot and outputDirectorySource) so all branches only assume a "<distDir>/standalone" path when the Next config's `output` equals "standalone".
🤖 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`:
- Line 333: The deploy result currently only writes build-settings to stderr via
maybeRenderDeployBuildSettings(buildSettingsResolution) and does not include
build-settings metadata in the structured JSON response; update the code that
assembles the deploy result JSON (the block that currently omits build-settings)
to embed build-settings metadata from buildSettingsResolution into the JSON
output returned by the deploy command so that prisma-cli app deploy --json
contains fields for created/reused status, config path, build command source,
and output directory; ensure maybeRenderDeployBuildSettings remains for human
output but also add those specific properties into the structured deploy result
object so both human and machine consumers see them.
---
Outside diff comments:
In `@docs/product/command-spec.md`:
- Around line 631-633: Update the "Build Command" rule to explicitly describe
the conditional branches: when package manager is inferred use
"<package-manager> run build"; when no package manager is inferred preserve the
literal "scripts.build" command from package.json; and only if there is no
scripts.build at all fall back to the framework default (e.g., "next build").
Reference the existing terms "Build Command", "scripts.build", and
"package-manager" so readers can locate the corresponding implementation
behavior and ensure the doc matches the code path that infers a package manager
vs the path that preserves the literal script.
---
Duplicate comments:
In `@packages/cli/src/lib/app/preview-build-settings.ts`:
- Around line 117-123: The code unconditionally appends "standalone" to
outputRoot; instead detect Next's output mode (the extracted `output` from
readStaticNextConfig / the local variable holding Next config) and only set
outputDirectory to joinPosix(outputRoot, "standalone") when output ===
"standalone"; otherwise set outputDirectory to outputRoot (or the appropriate
distDir), and adjust outputDirectorySource accordingly. Update the same logic in
the other spots noted (around the functions/returns that use
resolveNextOutputRoot and outputDirectorySource) so all branches only assume a
"<distDir>/standalone" path when the Next config's `output` equals "standalone".
🪄 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: 245279a5-c988-47a9-b05a-66a8e3795e5c
📒 Files selected for processing (9)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/output-conventions.mddocs/product/resource-model.mdpackages/cli/src/controllers/app.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/src/lib/app/preview-provider.tspackages/cli/tests/app-build.test.tspackages/cli/tests/app-controller.test.ts
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.
Summary
prisma.app.jsonas a repo-owned app build settings file for config-backed deploys.buildCommandandoutputDirectoryvalues without overwriting them..prisma/local.json.Validation
pnpm --filter @prisma/cli exec vitest run tests/app-build.test.ts tests/app-controller.test.ts tests/app-bun-compat.test.tspassed.git diff --checkpassed.preview-build.tsis 780 lines; new settings module is 488 lines.pnpm --filter @prisma/cli exec tsc --noEmitreports existing unrelated baseline failures intests/helpers.ts,tests/project-real-mode.test.ts,tests/publish-prep.test.ts, andtests/resolve-cli-version.test.ts. No touched app config/build files are in the TypeScript failure set.