# Close the fields Task 17 definition-version contract gap

## Objective

Starting from clean code commit `789e049d`, finish only the remaining `@platform-modules/fields` Task 17 gap: `FieldDefinitionVersion.definition` must be the complete, immutable, strictly parsed `FieldGroup` contract in `docs/specs/2026-08-08-fields-complete-design.md`, while remaining the single contract Task 7 will persist. Preserve the already-green genuine D1/embedded-real-Postgres conformance fixtures and the completed public-barrel work. End with both Task 17 acceptance commands, content/fields typecheck and build, an independent `gpt-5.6-sol/low` review with no P1/P2, a clean tree, and a commit.

## Sources of truth and constraints

Read in this order before editing:

1. `docs/specs/2026-08-08-fields-complete-design.md`, especially §2 (field definitions), §4 (recursive bounds), §5 (conditional rules), §6 (location rules), §7 (the final `FieldGroup` and immutable `FieldDefinitionVersion`), §15 (registered/custom type forward compatibility), §17 (trust-boundary requirements), §18 (acceptance), and §19/Task 7 migration compatibility.
2. `docs/plans/2026-08-08-mod-cms-content-model-completion.md`, Task 17 and Task 7.
3. The linked program specs `docs/specs/2026-08-08-content-types-complete-design.md` and `docs/specs/2026-08-08-mod-cms-content-model-completion-design.md` for shared transaction/package boundaries; do not broaden this repair into content or host implementation.
4. `docs/standards/coding-standard.md` §2 and §4 and `docs/standards/authz.md` for seam/error/trust-boundary review.

Hard scope:

- Expected implementation/test files are `packages/fields/src/schema.ts`, `packages/fields/src/definition-version.test.ts`, and, only where required to expose or pin the corrected contract, `packages/fields/src/index.ts` and `packages/fields/src/index.test.ts`. Touch `packages/fields/src/store.ts` or `packages/fields/src/conformance.test.ts` only if the corrected type requires a narrow compatibility adjustment; do not rewrite their completed behavior.
- Do not add Task 7 tables/migrations or Task 8 value engines/registries. Make this contract reusable by those tasks rather than adding a temporary weak alias.
- Do not duplicate, replace, skip, or mock the genuine Miniflare D1 and embedded real-Postgres conformance work already present.
- Never modify `apps/mod-cms`, `../mod-cms`, distro work, Posts-list work, or unrelated files.
- For every Node or pnpm command, prefix exactly `PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH`.
- Treat command exit status as authoritative and resolve warnings/type failures as well as test failures.

## Baseline and RED proof

1. Confirm `git status --short --branch`, `git rev-parse HEAD`, and `git log -1 --oneline`; implementation code must begin at `789e049d` with no pre-existing code changes. The planner-created `specs/838b4573_fields-definition-version.md` is an expected process artifact, not implementation drift.
2. Re-read current `schema.ts`, `store.ts`, `index.ts`, `definition-version.test.ts`, `index.test.ts`, and `conformance.test.ts`. Record that the current snapshot parser/type still uses the legacy group (`label`, one `{entityType, subType}` location, optional numeric `position`, 11 flat field families) rather than the final group (`title`, nested location-rule groups, required `active`, complete group settings, and complete discriminated recursive fields).
3. First add a focused failing test/type fixture that uses the final public shape through the package root or schema public entry point. It should fail against `789e049d` because the valid final `FieldGroup` cannot typecheck/parse and the legacy shape is still accepted. Run only that test to preserve a clear RED signal.

## Implementation

### 1. Freeze the final public definition shape in `schema.ts`

- Replace the legacy model dependency in `FieldDefinitionVersion.definition` with one canonical Task 17 snapshot contract structurally matching the fields design spec:
  - `FieldGroup`: `key`, `title`, `fields`, nested OR-of-AND `location`, optional `order`, `position`, `style`, `labelPlacement`, `instructionPlacement`, `hideOnScreen`, required `active`, and optional `description`.
  - The complete built-in discriminated field-definition surface from §2, including base metadata, settings, wrappers, presentation, defaults where permitted, and conditional rules.
  - Recursive `group`, `repeater`, and `flexible` definitions/layouts and clone references, preserving the exact public names and enum/value shapes in the spec.
  - Location paths/rules and conditional paths/rules as data-only contracts. Do not accept executable callbacks, `RegExp` instances, expressions, or provider code.
- Keep one authoritative definition type for `FieldDefinitionVersion`, `NormalizedFieldsRegistryRecord`, and future Task 7 immutable-version persistence. Do not leave `definition` as `FieldStorageValue`, the legacy model group, a permissive record, or a second migration-only alias.
- Keep the existing legacy pre-migration store APIs operational without broad refactoring. If a name collision with the old package model exists, use an explicit legacy internal alias and expose the final snapshot type unambiguously at the definition-version seam; do not silently widen the final type with legacy optional properties.
- Preserve forward compatibility with Task 8 custom descriptors by keeping per-field parsing dispatch separable from recursive group traversal. In this Task 17 implementation, accept every specified built-in family and fail closed on unregistered/unknown families; do not invent a module-global registry or a weaker `unknown` settings escape hatch. Task 8 must be able to add explicit immutable-registry dispatch without changing `FieldDefinitionVersion` or Task 7’s row shape.

### 2. Replace the legacy parser with a strict bounded final-contract parser

- Parse from `unknown` and return a newly normalized, deeply frozen snapshot. Never return or retain caller-owned arrays/objects; nested settings, choices, paths, conditional/location groups, layouts, child fields, wrappers/data maps, and metadata must all be copied and frozen.
- Enforce exact keys at every object level and the exact required/optional keys for each discriminant. Reject the old `{label, location:{entityType...}}` group, missing `active`, unknown group/base/settings/rule keys, unknown field types, malformed enum values, and wrong primitive/container kinds.
- Validate stable normalized keys, duplicate sibling field keys, duplicate flexible-layout keys, and recursive/path identities needed for an unambiguous future Task 7 snapshot. Validate applicable numeric/integer bounds and min/max relationships, finite numbers, bounded arrays/maps/strings, allowed return/projection enums, default-value exclusions for password and presentation-only fields, and safe-pattern data/flags without executing imported patterns.
- Parse `LocationRuleGroups` as an outer OR array of inner AND arrays with the §6 operator/value matrix. Parse `ConditionalRuleGroups` with operator-specific required values, finite numeric comparisons, bounded paths, quantifiers, and bounded safe-pattern objects.
- Retain hard defaults from the spec/current seam: maximum recursive definition depth 12, maximum 1,000 fields/nodes, bounded repeated collections/strings, and a bounded total payload budget. Bounds must cover the entire nested graph, not only top-level fields.
- Replace the current `JSON.stringify`-first size check: it can execute getters/`toJSON`, detects accessors too late, and measures UTF-16 length rather than a safe traversal budget. Inspect own property descriptors before reading values, require plain data objects/arrays, reject accessors, cycles, symbol/forbidden prototype-pollution keys, and count bytes/nodes/depth during a bounded traversal before allocation-heavy mapping. Use web-standard primitives only; no new `node:` runtime imports.
- Continue validating the version envelope (`groupKey`, positive safe `revision`, SHA-256-shaped canonical hash, valid copied date/ISO timestamp, known origin, and definition key matching `groupKey`) and preserve the structural `FieldDefinitionVersionError` guard. Do not expand this final gap into definition-version persistence or Task 7 migration behavior.

### 3. Keep the public barrel complete

- Export the corrected definition-version/group/field/rule/settings types needed to consume `FieldDefinitionVersion` from `packages/fields/src/index.ts` without removing any existing runtime export.
- Avoid duplicate ambiguous `FieldGroup` exports. If legacy group types must remain for existing pre-migration functions, give the internal/compatibility shape an explicit alias while ensuring `FieldDefinitionVersion['definition']` is exactly the final contract and not a union or permissive superset.
- Keep `parseFieldDefinitionVersion`, `FieldDefinitionVersionError`, and `isFieldDefinitionVersionError` publicly reachable. Do not change package exports or add a new subpath unless the existing root/schema exports cannot express the authoritative contract.

## Discriminating tests only

Extend `packages/fields/src/definition-version.test.ts` (and `index.test.ts` only for barrel visibility) rather than adding broad duplicate suites:

1. A public type/runtime fixture with final group metadata, nested location OR/AND rules, conditional logic, and representative scalar plus recursive fields (for example text + repeater/flexible child) parses successfully; the returned definition and every tested nested object/array are detached and frozen. Mutating the input after parsing cannot alter output.
2. A compile-time assertion imports the public contract and proves `FieldDefinitionVersion['definition']` accepts the final `FieldGroup`; include a targeted `@ts-expect-error` for the legacy label/single-location shape so a future widening is caught.
3. A compact table covers all final built-in field discriminants with their minimal valid settings, while representative invalid cases prove unknown family/unknown settings and operator-specific malformed rules fail through `isFieldDefinitionVersionError`.
4. Boundary cases prove acceptance at the supported depth/count/size boundary and rejection immediately over it, plus rejection of duplicate nested keys/layout keys, accessors or `toJSON`, cyclic/prototype-polluting input, non-finite numbers, contradictory bounds, and oversized nested collections. Keep fixtures generated compactly; do not duplicate adapter conformance.
5. Retain version-envelope tests (hash format, key match, origin, revision/date normalization). Update the old “valid” fixture to the authoritative final group instead of keeping legacy acceptance.

Run the focused tests after implementation:

```bash
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields exec vitest run --config vitest.config.ts src/definition-version.test.ts src/index.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields typecheck
```

## Required Task 17 acceptance

Run every command separately with the required PATH so an individual exit status is visible:

```bash
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content exec vitest run --config vitest.config.ts src/conformance.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields exec vitest run --config vitest.config.ts src/conformance.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content typecheck
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields typecheck
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content build
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields build
```

Also run `git diff --check`. Do not substitute a package-wide mock test for either named conformance command.

## Independent review and completion

1. After the diff and all gates are green, run an independent `gpt-5.6-sol/low` reviewer against the complete diff from `789e049d` and all linked sources: the fields design spec, content-types design spec, mod-cms content-model completion design, Task 17/Task 7 plan sections, coding standard, and authz standard. Explicitly ask it to report only actionable findings with P1/P2/P3 severity and to check:
   - exact final `FieldGroup`/field/rule shape rather than the legacy contract;
   - strict trust-boundary behavior, full-graph bounds, no getter/`toJSON` execution, deep immutability, and fail-closed unknown settings/types;
   - one Task 7-forward definition-version contract with no weak alias;
   - no regression/replacement of real D1/Postgres conformance, public exports, transaction identity, or unrelated package behavior;
   - scope compliance.
2. Wait for the review. Fix every P1/P2, rerun the focused tests and all six required acceptance/typecheck/build commands, then repeat independent review if any P1/P2 required code changes. Success requires a final review report with P1/P2 none; P3 may be recorded only if genuinely non-blocking and outside this fixed scope.
3. Inspect `git diff --name-only 789e049d` and reject any change under forbidden/unrelated paths. Run `git diff --check` and `git status --short`.
4. Commit the final scoped change with an imperative subject such as `Complete the fields definition version contract`. Include the planner spec artifact if it remains uncommitted so the final tree is clean; do not edit its contents during implementation. Verify `git status --porcelain` is empty and record the final commit SHA, six gate exit statuses, focused test/typecheck result, and final review P1/P2-none result in the handoff.

Task 17 is complete only when the corrected public contract is committed, the worktree is clean, every required command exits zero, and the independent final review reports no P1/P2.