diff --git a/docs/architecture/adrs/0005-config-sections-declare-their-shape.md b/docs/architecture/adrs/0005-config-sections-declare-their-shape.md index e7895288..55f5ccb3 100644 --- a/docs/architecture/adrs/0005-config-sections-declare-their-shape.md +++ b/docs/architecture/adrs/0005-config-sections-declare-their-shape.md @@ -52,7 +52,7 @@ Several designs were tried before this one, and each put the knowledge in the wr - `defineConfigSection({ name, schema })` derives the section's validator. The engine runs it on the merged section value with its provenance, after discovery and merging, so defaults declared in the schema apply once to the merged value and never let one file's default shadow another file's authored value. A relative `path` default is declared as a thunk, `["path", "=", () => "./migrations"]`, which arktype evaluates and morphs when the default is applied, so it resolves against the nearest file like an authored value; a relative literal default would be stored unresolved, and is refused when the schema is defined. - Each arktype error becomes a `CLI.CONFIG_FIELD_INVALID` diagnostic carrying `meta.section`, `meta.field`, and `where.path`, the file that declared the field's top-level key, so a chain of files still tells the user which one to fix. - The validated value of a plain-object section carries `baseDir`, the directory of the nearest file declaring the section, for commands that need the project's location rather than one of its files. The key is reserved: a config file that writes it is refused. -- Resolving a path or applying a default transforms the value, and arktype clones what it is given before it transforms it, so a config file's own objects are never written to. Its built-in clone rebuilds every object it reaches, which would hand a command a lookalike of the codec table or the contract serializer the file built. The engine supplies a clone that rebuilds plain objects and arrays and nothing else, so a class instance, a `Map` or a function reaches the command exactly as the file constructed it. +- A value the config file constructs, such as a descriptor, a client or any class instance, is declared with `reference(schema)`. Validation checks it against `schema`, and the command receives the file's own object. Resolving a path or applying a default transforms the section, and arktype clones what it is given before it transforms it, so a config file's own objects are never written to. The engine supplies that clone: it copies every value except the declared references. Nothing is kept by guessing from a value's type; the schema says which values are references. A reference cannot contain a `path` or a default, because resolving one would write into the file's own object, and `reference` refuses such a schema when it is defined. - An absent section is validated as an empty object: a schema whose fields are all optional accepts it, and a required field is reported by name. - `defineConfigSection({ name, validate })` remains for a section a schema cannot express; such a validator resolves its own path fields through `resolveSectionPath`. diff --git a/packages/cli-conformance/src/checks/tarball.ts b/packages/cli-conformance/src/checks/tarball.ts index 8ce15c41..177313a2 100644 --- a/packages/cli-conformance/src/checks/tarball.ts +++ b/packages/cli-conformance/src/checks/tarball.ts @@ -78,6 +78,8 @@ export interface PinException extends Suppression { readonly familyPackage: string; readonly familyPin: string; readonly shellPin: string; + /** Limits the exception to one channel, such as a family's dev build that only the dev channel installs. */ + readonly channel?: PublishChannel; } export interface TarballInput { @@ -187,7 +189,7 @@ export async function checkTarball( // biome-ignore lint/performance/noAwaitInLoops: sandboxes install one at a time so a failure names its package and concurrent npm installs cannot confound each other findings.push(...(await sandboxFindings(input, name, entry, packed, io))); } - return applyExceptions(findings, input.exceptions); + return applyExceptions(findings, input.exceptions, input.channel); } /** @@ -498,16 +500,22 @@ async function installedPinFindings( /** * A mismatch covered by a recorded exception is suppressed but still * printed. Every pin finding in a run whose observed shell and family - * pins match the exception's triple is covered; anything else fails. + * pins match the exception's triple is covered; anything else fails. An + * exception that names a channel covers only runs for that channel. */ function applyExceptions( findings: readonly Finding[], exceptions: readonly PinException[], + channel: PublishChannel, ): readonly Finding[] { - if (exceptions.length === 0) return findings; + const applicable = exceptions.filter( + (exception) => + exception.channel === undefined || exception.channel === channel, + ); + if (applicable.length === 0) return findings; return findings.map((entry) => { if (entry.kind !== "engine-pin-mismatch") return entry; - const covering = exceptions.find( + const covering = applicable.find( (exception) => entry.summary.includes(exception.familyPin) && entry.summary.includes(exception.shellPin) && diff --git a/packages/cli-conformance/tests/tarball.test.ts b/packages/cli-conformance/tests/tarball.test.ts index 6f1b41d4..85bef867 100644 --- a/packages/cli-conformance/tests/tarball.test.ts +++ b/packages/cli-conformance/tests/tarball.test.ts @@ -145,6 +145,34 @@ describe("checkTarball", () => { expect(pins.every((f) => f.suppressedBy !== undefined)).toBe(true); }); + test("an exception limited to the dev channel covers a dev publish and not a release", async () => { + const devOnly = { + familyPackage: "@prisma/composer", + familyPin: "0.0.9", + shellPin: "8.0.0-rc.1", + reason: "the dev channel installs the family's dev build", + removeWhen: "the family releases against the shell's engine", + channel: "dev" as const, + }; + const pinsOf = ( + findings: readonly { kind: string; suppressedBy?: unknown }[], + ) => findings.filter((f) => f.kind === "engine-pin-mismatch"); + + const dev = await checkTarball( + input({ channel: "dev", exceptions: [devOnly] }), + fakeIo(), + ); + const release = await checkTarball( + input({ channel: "release", exceptions: [devOnly] }), + fakeIo(), + ); + + expect(pinsOf(dev).every((f) => f.suppressedBy !== undefined)).toBe(true); + expect(pinsOf(release).some((f) => f.suppressedBy === undefined)).toBe( + true, + ); + }); + test("the exception does not cover the same family arriving at a third version", async () => { const findings = await checkTarball( input({ diff --git a/packages/cli-engine/AGENTS.md b/packages/cli-engine/AGENTS.md index b9bd8147..8a5442b8 100644 --- a/packages/cli-engine/AGENTS.md +++ b/packages/cli-engine/AGENTS.md @@ -8,13 +8,13 @@ Config section schemas (`configSchema`, the `path` keyword, `validateSectionWith Before you write code that walks a schema, copies values around validation, or repairs arktype's output afterwards, read the arktype docs at https://arktype.io/docs, in particular Configuration, Morphs and Scopes. arktype has documented options for most problems that look like they need bespoke code. Never read arktype's compiled node tree (`schema.internal`, `.structure`, `.branches`, `.in`): it is not a public API. -This engine once shipped a copy-and-restore walk over that node tree, about 200 lines, to stop arktype rebuilding the objects a config file built. arktype's documented `clone` option replaced it with about twenty. +This engine once shipped a copy-and-restore walk over that node tree, about 200 lines, to stop arktype rebuilding the objects a config file built. arktype's documented `clone` option, with references declared in the schema, replaced it. ### What a transformation does to its input - A morph is any transformation: `.pipe()`, `=>`, or a default value. When at least one morph applies anywhere in a value, arktype clones the whole input first and writes each result into the clone. Without morphs, validation returns the input itself. - The default clone keeps prototypes but rebuilds every plain object and class instance. Identity is lost, private `#fields` are lost, and `===` checks against shared objects fail. Functions, `Map` and `Set` are kept as they are. -- The clone is the `clone` config option. `configScope` sets it to `copyPlainParts`, which copies only plain objects and arrays, so everything else a config file constructed reaches the command unchanged. Keep it: a family's config objects must not be rebuilt. +- The clone is the `clone` config option. `configScope` sets it to `copyExceptReferences`, which copies every value except those the schema declared with `reference(schema)`. A section schema must declare as a reference every value the config file constructs (descriptors, clients, class instances); anything else is copied. `reference` records each value it validates in the current validation's set of references, and arktype runs those checks before it clones. - `clone: false` writes into the caller's input and throws on frozen input. `structuredClone` throws on functions and drops prototypes. Neither is a substitute. ### Other behaviour the config schemas rely on @@ -23,3 +23,4 @@ This engine once shipped a copy-and-restore walk over that node tree, about 200 - In `.pipe((value, ctx) => ...)`, `ctx.path` is the key path of the value within the section. The `path` keyword uses its first key to find the config file that declared the value. - A path given to `ctx.error` or `ctx.reject` inside a `.narrow()` is taken from the root, not from the narrowed value. Prepend `ctx.path` to report under the narrowed value, or better, declare the fields so arktype reports each one itself. - Undeclared keys are kept by default, and missing keys are reported in alphabetical order. +- A reference cannot contain a `path` or a default: `reference` compares `schema.in.expression` with `schema.expression`, which differ when the schema transforms, and throws when the schema is defined. diff --git a/packages/cli-engine/src/config-schema.ts b/packages/cli-engine/src/config-schema.ts index f1f79d06..40cb7db0 100644 --- a/packages/cli-engine/src/config-schema.ts +++ b/packages/cli-engine/src/config-schema.ts @@ -5,12 +5,17 @@ import type { SectionValidation } from "./config-section"; import type { Diagnostic } from "./protocol"; /** - * The provenance of the section being validated, published for the - * duration of one synchronous schema run so the `path` keyword can - * resolve each value against the file that declared its top-level key. + * The section being validated, published for the duration of one + * synchronous schema run: its provenance, so the `path` keyword can resolve + * each value against the file that declared its top-level key, and the + * values the schema declared references, so the clone keeps them. */ let current: - | { readonly name: string; readonly provenance: SectionProvenance } + | { + readonly name: string; + readonly provenance: SectionProvenance; + readonly references: WeakSet; + } | undefined; /** The file that declared the top-level key a value sits under, else the nearest file. */ @@ -60,7 +65,11 @@ const configScope = scope( }, { clone: (original: original): original => - copyPlainParts(original, new Map()) as original, + copyExceptReferences( + original, + current?.references, + new Map(), + ) as original, }, ); @@ -92,8 +101,9 @@ export type ConfigSchema = Type; /** * The validated value a schema produces: its output type plus `baseDir`, * the directory of the nearest file declaring the section, which the engine - * adds to a plain-object value. `baseDir` is reserved: a schema may not - * declare it and a config file may not write it. + * adds to a plain-object value that is not a {@link reference}. `baseDir` + * is reserved: a schema may not declare it and a config file may not write + * it. */ export type ConfigSchemaValue = S["infer"] & { readonly baseDir?: string; @@ -109,20 +119,18 @@ function isPlainObject(value: unknown): value is Record { /** * Before arktype applies a morph it clones the value, so resolving a path or - * applying a default never writes into what the caller passed in. Its own - * clone rebuilds every object it reaches. A config file's objects cannot - * survive that: a codec table, a contract serializer, anything whose - * behaviour lives in the instance rather than in its keys comes back as a - * lookalike that no longer works. - * - * So the scope above clones through arktype's `clone` option instead, and - * rebuilds only the plain objects and arrays a schema can write into. - * Everything else a config file constructed reaches the command as the file - * built it. `seen` carries the copies made so far, so a value that refers - * back to itself is copied once rather than followed forever. + * applying a default never writes into what the caller passed in. The scope + * above supplies the clone through arktype's `clone` option: it copies every + * value except the ones the schema declared with {@link reference}, which + * reach the command as the config file built them. `seen` carries the copies + * made so far, so a value that refers back to itself is copied once. */ -function copyPlainParts(value: unknown, seen: Map): unknown { - if (!Array.isArray(value) && !isPlainObject(value)) { +function copyExceptReferences( + value: unknown, + references: WeakSet | undefined, + seen: Map, +): unknown { + if (typeof value !== "object" || value === null || references?.has(value)) { return value; } const copied = seen.get(value); @@ -133,19 +141,55 @@ function copyPlainParts(value: unknown, seen: Map): unknown { const elements: unknown[] = []; seen.set(value, elements); for (const element of value) { - elements.push(copyPlainParts(element, seen)); + elements.push(copyExceptReferences(element, references, seen)); } return elements; } - const entries: Record = {}; - seen.set(value, entries); + const copy: Record = Object.create( + Object.getPrototypeOf(value), + ); + seen.set(value, copy); for (const key of Reflect.ownKeys(value)) { - entries[key] = copyPlainParts( + copy[key] = copyExceptReferences( (value as Record)[key], + references, seen, ); } - return entries; + return copy; +} + +/** + * Declares a value the config file constructs, such as a descriptor, a + * client or any class instance: validation checks it against `schema`, and + * the command receives the file's own object, never a copy. Every value not + * declared this way is copied before arktype writes into the section. + * + * ```ts + * const toySchema = configSchema({ + * target: reference(configSchema({ kind: "'target'", id: "string" })), + * "out?": "path", + * }); + * ``` + * + * `schema` cannot contain a `path` or a default: resolving one would write + * into the config file's own object, so such a schema is refused here. + */ +export function reference(schema: S): S { + if (schema.in.expression !== schema.expression) { + throw new Error( + `@prisma/cli-engine: a reference cannot contain a path or a default, because resolving it would write into the config file's own object: ${schema.expression}`, + ); + } + return schema.narrow((value) => { + if ( + (typeof value === "object" && value !== null) || + typeof value === "function" + ) { + current?.references.add(value); + } + return true; + }) as S; } function fieldDiagnostic( @@ -178,7 +222,9 @@ function fieldDiagnostic( * section is validated as an empty object, so a schema whose fields are * all optional accepts it and a required field is reported by name. A * plain-object value comes back frozen and carrying `baseDir`, the - * directory of the nearest file declaring the section. Never throws for + * directory of the nearest file declaring the section, unless the whole + * section is a reference, which comes back as the file's own object. Never + * throws for * any input: arktype reports problems as errors, and a `path` value is * only ever resolved here. */ @@ -195,7 +241,8 @@ export function validateSectionWithSchema( }; } const previous = current; - current = { name, provenance }; + const references = new WeakSet(); + current = { name, provenance, references }; try { const out: unknown = schema(raw === undefined ? {} : raw); if (out instanceof type.errors) { @@ -208,7 +255,7 @@ export function validateSectionWithSchema( } const nearest = provenance.files[0]; const value = - isPlainObject(out) && nearest !== undefined + isPlainObject(out) && nearest !== undefined && !references.has(out) ? Object.freeze({ ...out, baseDir: dirname(nearest) }) : out; return { ok: true, value: value as ConfigSchemaValue, diagnostics: [] }; diff --git a/packages/cli-engine/src/exports/index.ts b/packages/cli-engine/src/exports/index.ts index 8ff861da..c983feb5 100644 --- a/packages/cli-engine/src/exports/index.ts +++ b/packages/cli-engine/src/exports/index.ts @@ -60,6 +60,7 @@ export { type ConfigSchema, type ConfigSchemaValue, configSchema, + reference, validateSectionWithSchema, } from "../config-schema"; export { diff --git a/packages/cli-engine/tests/config-schema.test.ts b/packages/cli-engine/tests/config-schema.test.ts index 3a13fd90..d1d6775b 100644 --- a/packages/cli-engine/tests/config-schema.test.ts +++ b/packages/cli-engine/tests/config-schema.test.ts @@ -11,6 +11,7 @@ import { defineCommand, defineConfigSection, loadConfig, + reference, type SectionProvenance, validateSectionWithSchema, } from "@prisma/cli-engine"; @@ -168,17 +169,17 @@ describe("validateSectionWithSchema", () => { expect(dir).toBe("/app"); }); - test("what a config file constructed reaches the command working, frozen section or not", () => { + test("a value declared a reference is the config file's own object, frozen section or not", () => { class Serializer { deserialize(json: unknown): unknown { return json; } } - const checkedOnly = configSchema("object").narrow(() => true); + const constructed = reference(configSchema("object")); const schema = configSchema({ - target: checkedOnly, - "contract?": { source: checkedOnly, "output?": "path" }, - "extensions?": [checkedOnly, "[]"], + target: constructed, + "contract?": { source: constructed, "output?": "path" }, + "extensions?": [constructed, "[]"], migrations: [ { dir: ["path", "=", () => "./migrations"] }, "=", @@ -194,12 +195,10 @@ describe("validateSectionWithSchema", () => { return this.kind; }, }); + const source = Object.freeze({ load }); const raw = Object.freeze({ target, - contract: Object.freeze({ - source: Object.freeze({ load }), - output: "./out.json", - }), + contract: Object.freeze({ source, output: "./out.json" }), extensions: Object.freeze([target]), }); @@ -212,10 +211,10 @@ describe("validateSectionWithSchema", () => { extensions: (typeof target)[]; migrations: { dir: string }; }; - expect(value.target.serializer).toBe(serializer); + expect(value.target).toBe(target); expect(value.target.create()).toBe("target"); - expect(value.contract.source.load).toBe(load); - expect(value.extensions[0].serializer).toBe(serializer); + expect(value.contract.source).toBe(source); + expect(value.extensions[0]).toBe(target); expect(value.contract.output).toBe("/app/out.json"); expect(value.migrations.dir).toBe("/app/migrations"); }); @@ -256,10 +255,9 @@ describe("validateSectionWithSchema", () => { expect(value.out).toBe("/app/o"); }); - test("a checked-only value survives a section whose root has a narrow and defaults", () => { - const checkedOnly = configSchema("object").narrow(() => true); + test("a reference survives a section whose root has a narrow and defaults", () => { const schema = configSchema({ - family: checkedOnly, + family: reference(configSchema("object")), migrations: [ { dir: ["path", "=", () => "./migrations"] }, "=", @@ -272,10 +270,7 @@ describe("validateSectionWithSchema", () => { const result = validateSectionWithSchema("toy", schema, { family }, single); if (!result.ok) throw new Error(JSON.stringify(result.diagnostics)); - expect((result.value as { family: typeof family }).family).toEqual(family); - expect((result.value as { family: typeof family }).family.create).toBe( - create, - ); + expect((result.value as { family: typeof family }).family).toBe(family); expect( (result.value as { migrations: { dir: string } }).migrations.dir, ).toBe("/app/migrations"); @@ -305,13 +300,12 @@ describe("validateSectionWithSchema", () => { expect(value.out).toBe("/app/o"); }); - test("a union resolves paths in the branch the value matches, keeping the rest", () => { - const checkedOnly = configSchema("object").narrow(() => true); + test("a union resolves paths in the branch the value matches, and keeps its references", () => { const schema = configSchema({ either: [ { kind: "'a'", "dir?": "path" }, "|", - { kind: "'b'", inner: checkedOnly }, + { kind: "'b'", inner: reference(configSchema("object")) }, ], }); const inner = { keep: () => 1 }; @@ -337,11 +331,11 @@ describe("validateSectionWithSchema", () => { ).toBe(inner); }); - test("a tuple resolves paths by position, prefix and postfix alike", () => { - const checkedOnly = configSchema("object").narrow(() => true); + test("a tuple resolves paths and keeps references by position, prefix and postfix alike", () => { + const when = reference(configSchema("Date")); const schema = configSchema({ - pair: ["path", checkedOnly], - tail: ["path", "...", "object[]", "path"], + pair: ["path", when], + tail: ["path", "...", [when, "[]"], "path"], }); const second = new Date(1); const middle = new Date(2); @@ -469,13 +463,43 @@ describe("validateSectionWithSchema", () => { expect("c" in deeper).toBe(false); }); - test("a value that is not a plain object keeps its identity and data", () => { - const schema = configSchema({ when: "Date" }); - const when = new Date(0); + test("a value not declared a reference is copied, even one the config file constructed", () => { + class Box { + constructor(readonly n: number) {} + } + const schema = configSchema({ box: "object", "out?": "path" }); + const box = new Box(1); - const result = validateSectionWithSchema("toy", schema, { when }, single); + const result = validateSectionWithSchema( + "toy", + schema, + { box, out: "./o" }, + single, + ); - expect(result.ok && result.value.when).toBe(when); + if (!result.ok) throw new Error(JSON.stringify(result.diagnostics)); + const value = result.value as { box: Box; out: string }; + expect(value.box).not.toBe(box); + expect(value.box).toBeInstanceOf(Box); + expect(value.box.n).toBe(1); + }); + + test("a section whose whole value is a reference is the file's own object", () => { + const schema = reference(configSchema("object")); + const section = { client: { connect: () => 1 } }; + + const result = validateSectionWithSchema("toy", schema, section, single); + + expect(result.ok && result.value).toBe(section); + }); + + test("a reference cannot contain a path or a default", () => { + expect(() => reference(configSchema({ dir: "path" }))).toThrow( + "a reference cannot contain a path or a default", + ); + expect(() => reference(configSchema({ n: "number = 1" }))).toThrow( + "a reference cannot contain a path or a default", + ); }); test("a validation started by a morph inside another does not lose the outer context", () => { diff --git a/packages/cli-engine/tests/engine.test.ts b/packages/cli-engine/tests/engine.test.ts index 82f1b01f..79ea2429 100644 --- a/packages/cli-engine/tests/engine.test.ts +++ b/packages/cli-engine/tests/engine.test.ts @@ -43,6 +43,7 @@ describe("main export", () => { "positional", "readActiveAccessToken", "realpathOr", + "reference", "resolveSectionOverChain", "resolveSectionPath", "telemetryCommandGroup", diff --git a/packages/cli/scripts/conformance.ts b/packages/cli/scripts/conformance.ts index 133cfb41..112bfa13 100644 --- a/packages/cli/scripts/conformance.ts +++ b/packages/cli/scripts/conformance.ts @@ -125,6 +125,16 @@ async function tarball(): Promise { removeWhen: "composer-cli releases peering 0.6.1 and the follow-up bump PR pins that release", }, + { + familyPackage: "@prisma/composer-cli", + familyPin: "0.5.0", + shellPin: "0.6.1", + reason: + "the dev channel installs composer-cli's dev build, which peers engine 0.5.0; engine 0.6.1 must publish before composer-cli can peer it", + removeWhen: + "composer-cli releases peering 0.6.1 and the follow-up bump PR pins that release", + channel: "dev", + }, { familyPackage: "@prisma/orm-toolchain", familyPin: "0.4.0",