Skip to content

feat(schema): dependentRequired parser + runtime enforcement (#193)#197

Merged
zeevdr merged 3 commits intomainfrom
dependent-required
Apr 27, 2026
Merged

feat(schema): dependentRequired parser + runtime enforcement (#193)#197
zeevdr merged 3 commits intomainfrom
dependent-required

Conversation

@zeevdr
Copy link
Copy Markdown
Member

@zeevdr zeevdr commented Apr 27, 2026

Closes #193.

Summary

Adds JSON Schema 2020-12 `dependentRequired:` keyword to decree's schema YAML format end-to-end. Forms the second of three Phase-1 subtasks for #76 (the cross-field validation umbrella) and lets schema authors express "if field A is set, field B must also be set" without reaching for CEL.

Behavior

```yaml
fields:
payments.refunds_enabled: { type: bool }
payments.refund_window: { type: duration, nullable: true }

dependentRequired:
payments.refunds_enabled: [payments.refund_window]
```

  • Triggers on non-null. A rule fires only when the trigger has a non-null value in the post-merge config. Setting trigger to null clears the requirement.
  • Lint at `ImportSchema`. Trigger and every dependent must reference a defined field; trigger may not list itself; no duplicate dependents.
  • Runtime enforcement at every config write — `SetField` / `SetFields` / `ImportConfig` / `RollbackToVersion` — inside the existing transaction, against the post-merge snapshot. Race safety relies on Postgres MVCC + the existing `UNIQUE(tenant_id, version)` constraint on `config_versions`. See follow-up issue Retry config writes on CreateConfigVersion UNIQUE conflicts #196 for the caller-side retry on UNIQUE conflicts.

Implementation

  • `proto/centralconfig/v1/types.proto`: new `DependentRequiredEntry` message + `Schema.dependent_required = 12`.
  • `db/migrations/001_initial_schema.sql`: new `dependent_required jsonb NOT NULL DEFAULT '[]'` column on `schema_versions` (squashed into 001 per alpha policy).
  • `internal/schema/dependent_required.go` (new): `validateDependentRequiredAgainstFields`, `marshalDependentRequired`, `UnmarshalDependentRequired`, `CheckDependentRequired`.
  • `internal/schema/yaml.go`: `SchemaYAML.DependentRequired` field + `validateDependentRequiredYAML` lint + proto<->YAML converters.
  • `internal/schema/service.go`: ImportSchema reads, lints, and persists the rules; GetSchema/ExportSchema round-trip them.
  • `internal/validation/factory.go`: new `GetDependentRequired` and `InvalidateRules` methods. The factory now caches both per-field validators and dependentRequired bytes per tenant.
  • `internal/config/service.go`: new `fetchDependentRequiredRules`, `enforceDependentRequiredInTx`, `mapDependentRequiredErr` — wired at all four write sites.
  • `internal/config/store_pg.go`: `RunInTx` now binds both write and read query handles to the tx so reads inside fn observe staged writes (otherwise the post-merge snapshot read for the dependentRequired check would return pre-tx state on real PG).
  • `docs/concepts/schemas-and-fields.md`: new "Cross-field dependencies" section.

Test plan

  • Lint unit tests for `validateDependentRequiredYAML` and `validateDependentRequiredAgainstFields` — empty / valid / unknown trigger / unknown dependent / self-reference / duplicate.
  • Marshal / unmarshal round-trip + edge cases (empty array, malformed JSON degrades to nil).
  • `CheckDependentRequired` runtime — trigger absent / present-with-deps-present / present-with-dep-missing / first-violation-returned.
  • proto<->YAML converters — stable order, empty-input semantics, full round-trip.
  • YAML parser path: full `unmarshalSchemaYAML` covering valid, unknown trigger, unknown dependent, self-reference.
  • SetField runtime — trigger-set-without-dependent rejected, both-present allowed, trigger-absent allowed, trigger-set-to-null allowed.
  • SetFields aggregate path — single snapshot read for one rule check across multiple writes.
  • RollbackToVersion + ImportConfig runtime — both reject when post-merge state violates rules.
  • Validator factory rules cache — synctest-driven concurrent populator test under `-race`, plus invalidation forces re-fetch.
  • `make test` passes (full suite, race detector on).
  • `make lint` passes.

Out of scope (follow-ups)

zeevdr and others added 2 commits April 27, 2026 17:25
WIP for #193 — runtime check at config writes not yet wired. Compiles.

- Adds DependentRequiredEntry proto + Schema.dependent_required field
- Adds dependent_required jsonb column to schema_versions (squashed into
  001 per alpha policy)
- YAML parser accepts top-level dependentRequired key, lints existence +
  no-self-reference at schema-validate time
- Domain SchemaVersion + storage layer plumbing through PG + memory store
- ImportSchema validates and persists rules
- New helpers in internal/schema/dependent_required.go: validate /
  marshal / UnmarshalDependentRequired / CheckDependentRequired (runtime
  check; not yet wired to config writes)

Next: wire CheckDependentRequired into SetField/SetFields/ImportConfig
INSIDE the existing transactions for race safety. Plus tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review findings on PR for #193:

- store_pg.go: bind both write and read query handles to the same
  transaction in RunInTx so reads inside the tx observe staged writes.
  The dependentRequired check needs the post-merge snapshot; reading
  from the read pool returned pre-tx state, which would have silently
  let violations through against real Postgres (unit tests passed
  because the in-memory mockStore RunInTx is a no-op pass-through).
- schema.Service: take *validation.ValidatorFactory instead of
  *validation.ValidatorCache. UpdateTenant now invalidates both the
  per-field validator cache AND the dependentRequired rules cache —
  the previous code invalidated only the former, leaving stale rules
  after a tenant schema-version change.
- Drop unused IsTypedValueNonNull helper (dead code; the runtime check
  inspects DB rows, not staged TypedValues).
- convert.go: schemaToProto now calls UnmarshalDependentRequired
  instead of inlining a duplicate JSON unmarshal.

Tests:
- New TestRollbackToVersion_DependentRequired_Rejected and
  TestImportConfig_DependentRequired_Rejected to exercise the rule
  check on the rollback and import paths (only SetField/SetFields had
  coverage previously).
- TestUpdateTenant_SchemaVersionInvalidatesCache now uses a real
  ValidatorFactory (via test-only adapter bridging schema.mockStore's
  GetSchemaVersionParams to validation.SchemaVersionKey).

Docs:
- docs/concepts/schemas-and-fields.md gains a "Cross-field
  dependencies" section documenting the dependentRequired keyword
  semantics, lint, and runtime enforcement, plus a forward link to the
  CEL design brief for cases dependentRequired cannot express.

Lint:
- Rename dependentRequiredViolation → dependentRequiredError per
  errname convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zeevdr zeevdr added this to the Schema Spec v0.1.0 milestone Apr 27, 2026
@zeevdr zeevdr added server Server changes size: M Moderate — a day or two, clear scope priority: P1 Current milestone work labels Apr 27, 2026
Generated by make generate docs after adding the proto message in the
parent commit. Required by CI's "Docs check" gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zeevdr zeevdr merged commit a8c766a into main Apr 27, 2026
16 checks passed
@zeevdr zeevdr deleted the dependent-required branch April 27, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: P1 Current milestone work server Server changes size: M Moderate — a day or two, clear scope

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add dependentRequired: to schema spec v0.1.0 (parser + runtime enforcement)

1 participant