feat(schema): dependentRequired parser + runtime enforcement (#193)#197
Merged
feat(schema): dependentRequired parser + runtime enforcement (#193)#197
Conversation
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>
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>
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.
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]
```
Implementation
Test plan
Out of scope (follow-ups)