Skip to content

Commit fb03e9b

Browse files
Dumbrisclaude
Dumbris
and
claude
committed
fix(quarantine): review round 9 — new allOf branches are checked for required names (spec 120)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent df93544 commit fb03e9b

2 files changed

Lines changed: 227 additions & 7 deletions

File tree

‎internal/runtime/testdata/contract_delta_cases.json‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3882,6 +3882,173 @@
38823882
"additionalProperties": true
38833883
},
38843884
"signals": []
3885+
},
3886+
{
3887+
"name": "allOf introduced that requires an existing optional parameter (review r9)",
3888+
"before": {
3889+
"type": "object",
3890+
"properties": {
3891+
"to": {
3892+
"type": "string"
3893+
},
3894+
"subject": {
3895+
"type": "string"
3896+
},
3897+
"body": {
3898+
"type": "string"
3899+
}
3900+
},
3901+
"required": [
3902+
"to",
3903+
"body"
3904+
]
3905+
},
3906+
"after": {
3907+
"type": "object",
3908+
"properties": {
3909+
"to": {
3910+
"type": "string"
3911+
},
3912+
"subject": {
3913+
"type": "string"
3914+
},
3915+
"body": {
3916+
"type": "string"
3917+
}
3918+
},
3919+
"required": [
3920+
"to",
3921+
"body"
3922+
],
3923+
"allOf": [
3924+
{
3925+
"required": [
3926+
"subject"
3927+
]
3928+
}
3929+
]
3930+
},
3931+
"signals": [
3932+
"GATE-SCHEMA-SHAPE:new-required:allOf/subject"
3933+
]
3934+
},
3935+
{
3936+
"name": "allOf gains a branch that requires a parameter (review r9)",
3937+
"before": {
3938+
"type": "object",
3939+
"properties": {
3940+
"to": {
3941+
"type": "string"
3942+
},
3943+
"subject": {
3944+
"type": "string"
3945+
},
3946+
"body": {
3947+
"type": "string"
3948+
}
3949+
},
3950+
"required": [
3951+
"to",
3952+
"body"
3953+
],
3954+
"allOf": [
3955+
{
3956+
"maxProperties": 10
3957+
}
3958+
]
3959+
},
3960+
"after": {
3961+
"type": "object",
3962+
"properties": {
3963+
"to": {
3964+
"type": "string"
3965+
},
3966+
"subject": {
3967+
"type": "string"
3968+
},
3969+
"body": {
3970+
"type": "string"
3971+
}
3972+
},
3973+
"required": [
3974+
"to",
3975+
"body"
3976+
],
3977+
"allOf": [
3978+
{
3979+
"maxProperties": 10
3980+
},
3981+
{
3982+
"required": [
3983+
"subject"
3984+
]
3985+
}
3986+
]
3987+
},
3988+
"signals": [
3989+
"GATE-SCHEMA-SHAPE:new-required:allOf/1/subject"
3990+
]
3991+
},
3992+
{
3993+
"name": "allOf gains a branch requiring a nested field",
3994+
"before": {
3995+
"type": "object",
3996+
"properties": {
3997+
"to": {
3998+
"type": "string"
3999+
},
4000+
"subject": {
4001+
"type": "string"
4002+
},
4003+
"body": {
4004+
"type": "string"
4005+
}
4006+
},
4007+
"required": [
4008+
"to",
4009+
"body"
4010+
],
4011+
"allOf": [
4012+
{
4013+
"maxProperties": 10
4014+
}
4015+
]
4016+
},
4017+
"after": {
4018+
"type": "object",
4019+
"properties": {
4020+
"to": {
4021+
"type": "string"
4022+
},
4023+
"subject": {
4024+
"type": "string"
4025+
},
4026+
"body": {
4027+
"type": "string"
4028+
}
4029+
},
4030+
"required": [
4031+
"to",
4032+
"body"
4033+
],
4034+
"allOf": [
4035+
{
4036+
"maxProperties": 10
4037+
},
4038+
{
4039+
"properties": {
4040+
"options": {
4041+
"required": [
4042+
"forward_to"
4043+
]
4044+
}
4045+
}
4046+
}
4047+
]
4048+
},
4049+
"signals": [
4050+
"GATE-SCHEMA-SHAPE:new-required:allOf/1/forward_to"
4051+
]
38854052
}
38864053
]
38874054
}

‎internal/runtime/tool_contract_delta.go‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,11 @@ func walkCombinator(path string, b, a map[string]any, keyword string, growWidens
571571
case !bHas && aHas:
572572
if growWidens {
573573
*out = append(*out, sig)
574-
} else if carriesSteering(aList) {
574+
} else {
575575
// A new allOf narrows, but a default/const/enum in it steers
576-
// (review r8 P1).
577-
*out = append(*out, shape("new-property", childPath(path, keyword)))
576+
// (review r8 P1) and a required in it makes a parameter
577+
// mandatory (review r9 P1).
578+
newBranchSignals(childPath(path, keyword), aList, out)
578579
}
579580
default:
580581
// Branches that survive unchanged are matched by VALUE first (a
@@ -587,18 +588,70 @@ func walkCombinator(path string, b, a map[string]any, keyword string, growWidens
587588
walkSchemaNode(childPath(path, keyword+"/"+fmt.Sprint(aRest[i].index)), bRest[i].node, aRest[i].node, out)
588589
}
589590
// Live branches with no counterpart: a new allOf branch narrows, but
590-
// one that carries a default/const/enum steers (review r8 P1).
591+
// one that carries a default/const/enum steers (review r8 P1) and one
592+
// that carries a required makes a parameter mandatory (review r9 P1).
591593
for i := len(bRest); i < len(aRest); i++ {
592-
if carriesSteering(aRest[i].node) {
593-
*out = append(*out, shape("new-property", childPath(path, keyword+"/"+fmt.Sprint(aRest[i].index))))
594-
}
594+
newBranchSignals(childPath(path, keyword+"/"+fmt.Sprint(aRest[i].index)), aRest[i].node, out)
595595
}
596596
if len(aList) != len(bList) && (len(aList) > len(bList)) == growWidens {
597597
*out = append(*out, sig)
598598
}
599599
}
600600
}
601601

602+
// newBranchSignals reports what a schema (or schema list) that has no
603+
// approved counterpart would change about what the model sends: a
604+
// default/const/enum anywhere (new-property) and every required name
605+
// anywhere (new-required).
606+
func newBranchSignals(path string, node any, out *[]Signal) {
607+
if carriesSteering(node) {
608+
*out = append(*out, shape("new-property", path))
609+
}
610+
for _, name := range requiredAnywhere(node) {
611+
*out = append(*out, shape("new-required", childPath(path, name)))
612+
}
613+
}
614+
615+
// requiredAnywhere lists, sorted and deduplicated, every name a schema
616+
// subtree requires (schema-aware walk, like carriesSteering).
617+
func requiredAnywhere(node any) []string {
618+
seen := map[string]bool{}
619+
var walk func(any)
620+
walk = func(n any) {
621+
if list, isList := n.([]any); isList {
622+
for _, item := range list {
623+
walk(item)
624+
}
625+
return
626+
}
627+
schema, ok := asObject(n)
628+
if !ok {
629+
return
630+
}
631+
for _, name := range sortedStrings(schema["required"]) {
632+
seen[name] = true
633+
}
634+
for _, k := range schemaKeys(schema) {
635+
switch {
636+
case nameMapKeywords[k]:
637+
names, _ := asObject(schema[k])
638+
for _, name := range schemaKeys(names) {
639+
walk(names[name])
640+
}
641+
case subschemaKeywords[k]:
642+
walk(schema[k])
643+
}
644+
}
645+
}
646+
walk(node)
647+
out := make([]string, 0, len(seen))
648+
for name := range seen {
649+
out = append(out, name)
650+
}
651+
sort.Strings(out)
652+
return out
653+
}
654+
602655
type indexedBranch struct {
603656
index int
604657
node any

0 commit comments

Comments
 (0)