-
Notifications
You must be signed in to change notification settings - Fork 2
Config files layer: discovered up to the repo root, merged per key #233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
wmadden-electric
wants to merge
20
commits into
main
Choose a base branch
from
config-rulings
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
20ee1e4
Record the config-file resolution slice: decided spec, dispatch plan,…
wmadden-electric 1664501
Remove the deprecated defineConfig alias before launch
wmadden-electric 8011098
Engine 0.3.0: this slice changes the engine's public surface
wmadden-electric 8e7964f
loadConfig resolves a chain of config files instead of one
wmadden-electric 32a9dab
Review rework for the chain loader
wmadden-electric f801b50
Sections merge per key over the config chain, with provenance
wmadden-electric 350d1c5
Harden section resolution: guarded reads, strict path lookup, frozen …
wmadden-electric 5ba8a31
Skills reads resolve through the engine's chain resolver
wmadden-electric 1331eaa
Out-of-handler skills reads go through the Runtime loader seam
wmadden-electric c99418a
init below an ancestor config scaffolds only, and the e2e rerun worka…
wmadden-electric f381e5e
init reads the config chain from the context, and machine output says…
wmadden-electric 721231c
Docs and records follow the config-chain truth, plus three review nits
wmadden-electric 3eb998a
Config resolution review fixes: realpath parent links, user-code merg…
wmadden-electric cb83ae5
Ledger: two config-chain review findings whose fixes live outside thi…
wmadden-electric 056d38f
Config section validators receive the resolved value's provenance
wmadden-electric 1167cd3
Ledger: the family path fix shipped in tandem, not deferred
wmadden-electric 3153d08
Address review: --config reaches the post-login tip; docs match --con…
wmadden-electric 85a6e4e
Merge main: the chain engine release becomes 0.4.0
wmadden-electric d081406
Guard the prototype lookup in the conformance provenance builder
wmadden-electric d0aca8a
Register the three parent-directive diagnostics in the error reference
wmadden-electric File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
46 changes: 46 additions & 0 deletions
46
.drive/projects/prisma-cli-v8/plans/config-file-resolution.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Config-file resolution — dispatch plan | ||
|
|
||
| Slice contract: `specs/config-file-resolution.md`. One PR into `main`. Sequential dispatches; each hands the next a state where `pnpm typecheck` and `pnpm exec turbo run test --concurrency=1` are green (the sequential run — the parallel `pnpm test` has a known engine-dist race). Engine-surface hazard applies to every dispatch: verify `@prisma/cli-engine`'s committed version is still unpublished before merging the PR; if a train shipped it, bump the engine version first. | ||
|
|
||
| ## D1 — The loader discovers a chain | ||
|
|
||
| **Outcome:** `loadConfig` resolves an ordered chain of config files instead of one: anchor directory (cwd, or the `--config` file's directory) upward to the first `.git` directory; `parent: false` ends collection, `parent: "path"` names the next link explicitly (cycle-checked, may cross the boundary), no `.git` above means anchor-only. `parent` joins the reserved top-level keys (loader strips it like the marker; `reservedConfigSectionName` covers it; construction-time rejection includes it). Every file on the chain is evaluated with the existing marker/version/unreadable classification, each failure naming its file. `LoadedConfig` becomes a chain shape — per-file `{path, sections}` in nearest-first order plus file-level diagnostics — and every existing consumer (`needs.ts`, hosts, engine tests, the skills reader temporarily via a nearest-file adapter) compiles and passes against it with single-file behavior unchanged: one file in cwd behaves exactly as today. | ||
|
|
||
| **Builds on:** clean main. **Hands to D2:** the chain type, discovery green under new engine tests (boundary stop, `parent` forms, cycle guard, anchored fixtures per the spec's test-anchoring requirement), all suites green. | ||
|
|
||
| **Focus:** the loader doc comment says "cwd only, no walking up" — it and the `EVALUATE_ONE_FILE_ONLY` rationale need rewriting to the new truth. Symlink-resolve the anchor. Windows realpath on every chain comparison. The unknown-key check in `needs.ts` iterates the chain per file from this dispatch on. | ||
|
|
||
| ## D2 — Sections merge per key with provenance | ||
|
|
||
| **Outcome:** `ConfigSection` gains optional `merge(parent, child)`; the engine default merges per key at the section's top level and replaces below. `checkConfiguration` folds each needed section over the chain nearest-first, validates the merged view (validators unchanged), and hands `ctx.config` the resolved value. Every resolved value carries provenance; post-merge validation diagnostics name the contributing file; relative-path resolution against the declaring file is provided as an engine helper the provenance makes possible (sections opt in by resolving paths through it). Merging never mutates the frozen exports. | ||
|
|
||
| **Builds on:** D1's chain. **Hands to D3:** engine tests covering shadowing, fall-through (nested file lacking a section the root has), partial merge (root `skills.check` + package `skills.agents`), provenance in error copy, and the ORM absence-error firing only on a chain with no `orm` section anywhere; all suites green. | ||
|
|
||
| **Focus:** `merge` must be optional and type-backward-compatible — the shipped orm/composer dists implement `ConfigSection` against the current engine and must keep working with the default. `__proto__`/`fromEntries` discipline extends to the merged object. | ||
|
|
||
| ## D3 — One resolver in the product | ||
|
|
||
| **Outcome:** `readProjectSkillsConfig` and the out-of-handler reads (skills staleness notice, post-login tip) resolve through the engine's chain resolver; the hand-rolled `existsSync` + direct `loadConfig` path and D1's temporary adapter are deleted. From any subdirectory, the staleness notice and the skills commands agree on the governing config. The skills section keeps its null-collapsing contract for out-of-handler callers. | ||
|
|
||
| **Builds on:** D2's resolver. **Hands to D4:** exactly one resolution code path, skills unit tests green from nested-directory fixtures. | ||
|
|
||
| **Focus:** the "one stat before paying for transpile" property `readProjectSkillsConfig` had should survive — chain discovery is stat-only until a file exists; keep the no-config fast path. | ||
|
|
||
| ## D4 — Subdirectory init scaffolds only | ||
|
|
||
| **Outcome:** `prisma init` run in a directory whose discovered chain contains an ancestor config skips the skills sync, the `postinstall` script, and the `prisma` devDependency by default, reporting each as skipped-with-reason; explicit flags still opt in; root init (no ancestor config) is byte-for-byte unchanged. Unit tests cover both shapes; the init e2e gains the subdirectory case. If D1's chain work removed the cause of the e2e rerun workaround (`e2e/init.e2e.ts:189-200`), the workaround comes out; otherwise its comment is updated to name what still forces it. | ||
|
|
||
| **Builds on:** D3 (init detects the ancestor through the same resolver as everything else). **Hands to D5:** init behavior finished, suites and init e2e green. | ||
|
|
||
| ## D5 — Docs, records, full verification | ||
|
|
||
| **Outcome:** user-facing docs describe discovery, merging, `parent`, and the two-config layout (the config documentation surface plus `docs/product/*` touchpoints that mention config today); the ledger closes the stale pathe entry (the loader realpath fix shipped in engine 0.2.2) and records this slice's rulings; the spec's status line gains the landed date. Full verification per AGENTS.md, including conformance (`pnpm exec turbo run conformance --filter @prisma/cli --force`) and the sequential test run; the engine-version-unpublished check from the plan header re-verified at PR-open. | ||
|
|
||
| **Builds on:** D4. **Hands to:** slice-DoD; PR-open. | ||
|
|
||
| ## Hazard inventory (2026-08-25) | ||
|
|
||
| - Engine surface changes ride the unpublished engine version or force the three-repo re-peer chain — check at start AND at merge; the version can publish out from under a long-running slice. | ||
| - This repository contains fixture `prisma.config.ts` files and will gain more; every loader/resolver test must pin its chain (temp dirs outside the repo, or explicit `parent: false` fixtures) or a real ancestor config leaks in — the exact failure the prior round's review caught. | ||
| - `tests/e2e-coverage.test.ts` parses `src/cli.ts` as text; D4 does not touch the mount table, but any drive-by edit there must keep the `mountedCommands` literal's shape. | ||
| - The parallel `pnpm test` race (engine dist rebuild vs cli tests) predates this slice; verify with the sequential run and do not chase it here. |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.