Resolve the composer section's configPath against the file that declared it - #262
Draft
wmadden-electric wants to merge 1 commit into
Draft
Resolve the composer section's configPath against the file that declared it#262wmadden-electric wants to merge 1 commit into
wmadden-electric wants to merge 1 commit into
Conversation
…red it prisma.config.ts is now a chain rather than a single file: the engine discovers every file from the command's cwd up to the repo root and merges their sections per key. A `composer` section written once at the repo root therefore reaches commands run in any subdirectory — and resolving its `configPath` against the process cwd names a different file in every one of them, which is almost never the file the author meant. The section validator now resolves `configPath` itself, through the engine's `resolveSectionPath`, against the file whose section declared that key; an absolute path passes through unchanged. Everything downstream — the pipeline and the config loader — receives an absolute path and no longer resolves anything against cwd. This needs @prisma/cli-engine 0.3.0, whose `ConfigSection.validate` takes the resolved section's provenance as a second argument and whose `LoadedConfig` is the chain rather than one file. Every engine declaration moves to 0.3.0 together, as check-cli-engine-pin requires. 0.3.0 is not published yet (it comes from prisma/prisma-cli#233), so `pnpm install` cannot resolve it and pnpm-lock.yaml cannot be regenerated here. CI stays red until the engine publishes; the lockfile update belongs to the commit that follows the publish. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
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
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 changed
The
composerconfig section resolves itsconfigPathagainst the prisma.config.ts file that declared it, instead of against the directory the command happens to run in.prisma.config.ts is now a chain rather than a single file: prisma-cli discovers every config file from the command's cwd up to the repo root and merges the sections per key. A
composersection written once at the repo root therefore reaches commands run in any subdirectory — and aconfigPathresolved against the process cwd names a different file in each of them, which is almost never the file the author meant.The fix follows the pattern engine 0.3.0 defines: the section validator resolves the path-valued key and returns an absolute path, so nothing downstream ever resolves against cwd.
packages/0-framework/3-tooling/cli/src/family/section.ts—validatetakes the section's provenance as its second argument and runsconfigPaththrough the engine'sresolveSectionPath. An absolute path passes through unchanged.resolveSectionPaththrows when the key is missing from the provenance, which cannot happen for a key just read out of the section, but a section validator must never throw — so even that case becomes aCONFIG.FIELD_INVALIDdiagnostic rather than an internal error blamed on composer.src/load-config.tsandsrc/pipeline.ts—configPathis documented and treated as absolute; the resolution againstcwdis gone.0.3.0together (the internal CLI, composer-cli's peer and dev pins, the target extension, and the four examples), which is whatcheck-cli-engine-pinrequires.Tests
src/family/__tests__/engine-cli.test.tsgains the real regression test: a two-file chain where the root file is the only one declaringconfigPath, with the run's cwd two directories below it. Both wrong answers are visible — resolving against cwd, or against the nearest file on the chain, would name/repo/apps/shop/prisma-composer.config.tsinstead of/repo/prisma-composer.config.ts.src/family/__tests__/section.test.tscovers the validator directly: relative resolves against the declaring file, absolute passes through, and a provenance missing the key fails with a diagnostic instead of throwing.host-adapter.test.tsasserts the resolved absolute path arrives at the operation, end to end through the real Runtime.path.resolve(configPath)(cwd-based) fails 6 of these tests, including both engine-level ones — the tests fail when the behavior breaks.What is green, and what is not
Verified locally against a packed
@prisma/cli-engine@0.3.0tarball installed into the workspace:tsc --noEmitacross all 78 workspace typecheck tasks — clean.turbo run testacross all 66 test tasks — clean, including the local-dev integration suite.@internal/clion its own: 259 tests pass.biome checkon the touched sources — clean.pnpm check:cli-engine-pin— passes at0.3.0.CI will be red until
@prisma/cli-engine@0.3.0publishes (it comes from prisma/prisma-cli#233).pnpm installcannot resolve0.3.0from the registry, so it fails before any job runs, andpnpm-lock.yamlcannot be regenerated in this PR. The lockfile update belongs to the commit that follows the publish — the same shape as the previous engine bumps.🤖 Generated with Claude Code