fix(core): close isCompatible() correctness holes for cross-app duck typing#80
Merged
Conversation
…typing Require field.required === true on the candidate instead of mere presence (a candidate declaring a required field optional no longer passes), add a text/string read-compatibility table so the spec's own motivating example works, and recurse into object properties and array items so nested required-field mismatches are caught. Scope isCompatible() explicitly as read-compatibility in both the code and the spec, matching #54. Refs #54
…table The spec previously duplicated the runtime implementation in a code block, which is exactly what let it drift from the real function (the bug this PR fixes). Describe the rule in prose, present the kind compatibility relation as a table, and point to the source/tests as the single authoritative reference instead.
…emas A candidate schema can originate from another app's Type definition — the untrusted side of duck-typed consumption — so deeply nested array/object fields could recurse unboundedly and overflow the call stack. Cap recursion at 32 levels (matching MAX_VALIDATION_DEPTH in validate.ts) and fail closed: past the limit, treat as incompatible rather than risk a crash.
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.
Summary
Fixes #54.
isCompatible()had two correctness holes and a spec example that contradicted the implementation, which matters because #47 promotes it to the official cross-app duck-typing mechanism.required. A candidate with an optionaltitlepassed a required-titlecheck, so a consumer trustingisCompatible()could readundefinedfrom a field it was told was guaranteed. Fixed by requiringfield.required === trueon the candidate.textvsstring: these are distinctScalarFieldKinds compared with strict equality, but the spec's own motivating example ({ text: string }) doesn't work under the shipped predicate since long-form text is typically declaredkind: 'text'. Added aREAD_COMPATIBLEtable makingstring⇄textmutually acceptable (identical value sets) while every other kind pair stays exact — notablydateis not compatible withstring, sincedatecarries a parse/validity guarantee a plain string doesn't.object.propertiesorarray.itemscould silently drop a required sub-field. Now recurses using the same (strengthened) predicate.Also scopes
isCompatible()explicitly as read-compatibility (licenses consuming records, not writing them) in both the code doc comment anddocs/spec.md§Type compatibility, whose inlined copy of the function is updated to match.Test plan
text/stringaccepted both directions;date/stringstill rejected both directions; nested object required-subfield mismatch; nested array item-kind mismatch (both a true mismatch and a text/string-equivalent pair); the spec's own example as a fixture.pnpm --filter @haverstack/core test— all 256 tests passpnpm --filter @haverstack/core typecheckandlint— cleangit stashthat the pre-existingwire-typestypecheck failure is unrelated to this change (missing built@haverstack/coreoutput, not a type error)Generated by Claude Code