Skip to content

Commit 307fc40

Browse files
Dumbrisclaude
Dumbris
and
claude
committed
fix(quarantine): review round 6 — combinators compare retained branches, unconstrained means no validation keyword (spec 120)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 594f0eb commit 307fc40

2 files changed

Lines changed: 230 additions & 15 deletions

File tree

‎internal/runtime/testdata/contract_delta_cases.json‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3043,6 +3043,201 @@
30433043
"signals": [
30443044
"GATE-SCHEMA-SHAPE:additional-properties:/"
30453045
]
3046+
},
3047+
{
3048+
"name": "allOf grows while branch 0 changes its default (review r6)",
3049+
"before": {
3050+
"type": "object",
3051+
"properties": {
3052+
"to": {
3053+
"allOf": [
3054+
{
3055+
"type": "string",
3056+
"default": "safe"
3057+
}
3058+
]
3059+
},
3060+
"subject": {
3061+
"type": "string"
3062+
},
3063+
"body": {
3064+
"type": "string"
3065+
}
3066+
},
3067+
"required": [
3068+
"to",
3069+
"body"
3070+
]
3071+
},
3072+
"after": {
3073+
"type": "object",
3074+
"properties": {
3075+
"to": {
3076+
"allOf": [
3077+
{
3078+
"type": "string",
3079+
"default": "attacker"
3080+
},
3081+
{}
3082+
]
3083+
},
3084+
"subject": {
3085+
"type": "string"
3086+
},
3087+
"body": {
3088+
"type": "string"
3089+
}
3090+
},
3091+
"required": [
3092+
"to",
3093+
"body"
3094+
]
3095+
},
3096+
"signals": [
3097+
"GATE-SCHEMA-SHAPE:default:to/allOf/0"
3098+
]
3099+
},
3100+
{
3101+
"name": "anyOf shrinks while branch 0 changes type",
3102+
"before": {
3103+
"type": "object",
3104+
"properties": {
3105+
"to": {
3106+
"anyOf": [
3107+
{
3108+
"type": "string"
3109+
},
3110+
{
3111+
"type": "object"
3112+
}
3113+
]
3114+
},
3115+
"subject": {
3116+
"type": "string"
3117+
},
3118+
"body": {
3119+
"type": "string"
3120+
}
3121+
},
3122+
"required": [
3123+
"to",
3124+
"body"
3125+
]
3126+
},
3127+
"after": {
3128+
"type": "object",
3129+
"properties": {
3130+
"to": {
3131+
"anyOf": [
3132+
{
3133+
"type": "integer"
3134+
}
3135+
]
3136+
},
3137+
"subject": {
3138+
"type": "string"
3139+
},
3140+
"body": {
3141+
"type": "string"
3142+
}
3143+
},
3144+
"required": [
3145+
"to",
3146+
"body"
3147+
]
3148+
},
3149+
"signals": [
3150+
"GATE-SCHEMA-SHAPE:type:to/anyOf/0"
3151+
]
3152+
},
3153+
{
3154+
"name": "additionalProperties annotation-only schema replaced by true (review r6)",
3155+
"before": {
3156+
"type": "object",
3157+
"properties": {
3158+
"to": {
3159+
"type": "string"
3160+
},
3161+
"subject": {
3162+
"type": "string"
3163+
},
3164+
"body": {
3165+
"type": "string"
3166+
}
3167+
},
3168+
"required": [
3169+
"to",
3170+
"body"
3171+
],
3172+
"additionalProperties": {
3173+
"x-note": "annotation"
3174+
}
3175+
},
3176+
"after": {
3177+
"type": "object",
3178+
"properties": {
3179+
"to": {
3180+
"type": "string"
3181+
},
3182+
"subject": {
3183+
"type": "string"
3184+
},
3185+
"body": {
3186+
"type": "string"
3187+
}
3188+
},
3189+
"required": [
3190+
"to",
3191+
"body"
3192+
],
3193+
"additionalProperties": true
3194+
},
3195+
"signals": []
3196+
},
3197+
{
3198+
"name": "additionalProperties {type: string} removed stays held",
3199+
"before": {
3200+
"type": "object",
3201+
"properties": {
3202+
"to": {
3203+
"type": "string"
3204+
},
3205+
"subject": {
3206+
"type": "string"
3207+
},
3208+
"body": {
3209+
"type": "string"
3210+
}
3211+
},
3212+
"required": [
3213+
"to",
3214+
"body"
3215+
],
3216+
"additionalProperties": {
3217+
"type": "string"
3218+
}
3219+
},
3220+
"after": {
3221+
"type": "object",
3222+
"properties": {
3223+
"to": {
3224+
"type": "string"
3225+
},
3226+
"subject": {
3227+
"type": "string"
3228+
},
3229+
"body": {
3230+
"type": "string"
3231+
}
3232+
},
3233+
"required": [
3234+
"to",
3235+
"body"
3236+
]
3237+
},
3238+
"signals": [
3239+
"GATE-SCHEMA-SHAPE:additional-properties:/"
3240+
]
30463241
}
30473242
]
30483243
}

‎internal/runtime/tool_contract_delta.go‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -572,12 +572,16 @@ func walkCombinator(path string, b, a map[string]any, keyword string, growWidens
572572
if growWidens {
573573
*out = append(*out, sig)
574574
}
575-
case len(aList) == len(bList):
576-
for i := range bList {
575+
default:
576+
// The retained branches are compared pairwise whatever the length
577+
// change, so a default or type moved inside branch 0 is not hidden by
578+
// an appended branch (review r6 P1); the length rule applies on top.
579+
for i := 0; i < len(bList) && i < len(aList); i++ {
577580
walkSchemaNode(childPath(path, keyword+"/"+fmt.Sprint(i)), bList[i], aList[i], out)
578581
}
579-
case (len(aList) > len(bList)) == growWidens:
580-
*out = append(*out, sig)
582+
if len(aList) != len(bList) && (len(aList) > len(bList)) == growWidens {
583+
*out = append(*out, sig)
584+
}
581585
}
582586
}
583587

@@ -633,27 +637,43 @@ func carriesSteering(node any) bool {
633637
return false
634638
}
635639

640+
// validationKeywords are the JSON Schema keywords that can reject a value.
641+
// Anything else (text, default, examples, $id, vendor x-* annotations) is
642+
// not a constraint (review r6 P2).
643+
var validationKeywords = map[string]bool{
644+
"type": true, "$ref": true, "enum": true, "const": true, "pattern": true, "format": true,
645+
"minimum": true, "maximum": true, "exclusiveMinimum": true, "exclusiveMaximum": true, "multipleOf": true,
646+
"minLength": true, "maxLength": true, "minItems": true, "maxItems": true, "minProperties": true, "maxProperties": true,
647+
"uniqueItems": true, "required": true, "properties": true, "patternProperties": true, "additionalProperties": true,
648+
"items": true, "prefixItems": true, "additionalItems": true, "contains": true, "minContains": true, "maxContains": true,
649+
"not": true, "anyOf": true, "oneOf": true, "allOf": true, "if": true, "then": true, "else": true,
650+
"dependencies": true, "dependentRequired": true, "dependentSchemas": true, "propertyNames": true,
651+
"unevaluatedProperties": true, "unevaluatedItems": true,
652+
}
653+
636654
// schemaIsUnconstrained reports whether a schema object constrains nothing
637-
// once its text keywords are removed (`{}` accepts every value, like `true`).
655+
// (`{}` accepts every value, like `true`): no validation keyword with a
656+
// non-empty value. Text, default, examples and unknown annotation keywords do
657+
// not count; an empty required list or an empty properties map does not either
658+
// (review r5).
638659
func schemaIsUnconstrained(schema map[string]any) bool {
639-
stripped, _ := asObject(stripText(schema))
640-
// default steers but validates nothing, and its removal passes (FR-004).
641-
delete(stripped, "default")
642-
// Empty collections constrain nothing either (review r5): required: [],
643-
// properties: {} and the other name maps with no entries.
644-
for k, v := range stripped {
660+
for k, v := range schema {
661+
if !validationKeywords[k] {
662+
continue
663+
}
645664
switch t := v.(type) {
646665
case []any:
647-
if len(t) == 0 && (k == "required" || k == "allOf" || k == "anyOf" || k == "oneOf" || k == "prefixItems" || k == "examples") {
648-
delete(stripped, k)
666+
if len(t) == 0 {
667+
continue
649668
}
650669
case map[string]any:
651670
if len(t) == 0 && nameMapKeywords[k] {
652-
delete(stripped, k)
671+
continue
653672
}
654673
}
674+
return false
655675
}
656-
return len(stripped) == 0
676+
return true
657677
}
658678

659679
func hasKey(m map[string]any, key string) bool {

0 commit comments

Comments
 (0)