diff --git a/scripts/policy/schema-compat/main.go b/scripts/policy/schema-compat/main.go index 84447246..e003eef2 100644 --- a/scripts/policy/schema-compat/main.go +++ b/scripts/policy/schema-compat/main.go @@ -730,7 +730,6 @@ func checkParameterCompatibility(toolPath, name string, oldParameter, newParamet new string }{ {name: "type", old: oldParameter.Type, new: newParameter.Type}, - {name: "property", old: oldParameter.Property, new: newParameter.Property}, {name: "default", old: oldParameter.Default, new: newParameter.Default}, {name: "interface_default", old: oldParameter.InterfaceDefault, new: newParameter.InterfaceDefault}, {name: "format", old: oldParameter.Format, new: newParameter.Format}, @@ -739,6 +738,13 @@ func checkParameterCompatibility(toolPath, name string, oldParameter, newParamet failures = append(failures, fmt.Sprintf("schema tool %q parameter %q changed %s", toolPath, name, field.name)) } } + // Property remaps are incompatible unless a reviewed correction replaces + // flag-name inference with the ParamDecl-declared MCP field. Arbitrary + // non-empty A→B remaps remain a contract break. + if oldParameter.Property != newParameter.Property && + !compatibleReviewedPropertyCorrection(toolPath, name, oldParameter.Property, newParameter.Property) { + failures = append(failures, fmt.Sprintf("schema tool %q parameter %q changed property", toolPath, name)) + } // Clearing interface_type is accepted as compatible: a deliberate, // wire-visible policy decision taken with the pinned MCP metadata // retirement. Production no longer projects MCP-sourced types unless @@ -783,6 +789,48 @@ func enumNarrowed(oldValues, newValues []string) bool { return false } +// reviewedPropertyCorrection is an exact allowlist entry for replacing a +// historical wire property (typically flag-name inference) with the MCP field +// declared on leaf ParamDecl.Property. Do not use this to bless arbitrary +// remaps: each entry must name tool path, parameter, old property, and new +// property exactly. +type reviewedPropertyCorrection struct { + toolPath string + param string + oldProperty string + newProperty string +} + +// Keep this list tiny and evidence-backed. Prefer ParamDecl declaration; only +// add an entry when schema-compat would otherwise freeze a wrong inferred +// property that agents bind against. +var reviewedPropertyCorrections = []reviewedPropertyCorrection{ + { + toolPath: "doc/doc.whiteboard_insert", + param: "parent-block", + oldProperty: "parentBlock", + newProperty: "referenceBlockId", + }, + { + toolPath: "doc/doc.whiteboard_insert", + param: "ref-block", + oldProperty: "refBlock", + newProperty: "referenceBlockId", + }, +} + +func compatibleReviewedPropertyCorrection(toolPath, param, oldProperty, newProperty string) bool { + for _, entry := range reviewedPropertyCorrections { + if entry.toolPath == toolPath && + entry.param == param && + entry.oldProperty == oldProperty && + entry.newProperty == newProperty { + return true + } + } + return false +} + func mergeContracts(historical, current schemaContract) (schemaContract, []string) { failures := checkCompatibility(historical, current) if len(failures) > 0 { diff --git a/scripts/policy/schema-compat/main_test.go b/scripts/policy/schema-compat/main_test.go index 4cdbfc18..35843d58 100644 --- a/scripts/policy/schema-compat/main_test.go +++ b/scripts/policy/schema-compat/main_test.go @@ -596,6 +596,67 @@ func TestCrossPlatformCoverageSchemaCompatMCPRetirementAndConstraintExpansion(t } } +func TestCrossPlatformCoverageSchemaCompatReviewedPropertyCorrection(t *testing.T) { + if !compatibleReviewedPropertyCorrection( + "doc/doc.whiteboard_insert", "ref-block", "refBlock", "referenceBlockId", + ) { + t.Fatal("reviewed whiteboard ref-block correction must be accepted") + } + if !compatibleReviewedPropertyCorrection( + "doc/doc.whiteboard_insert", "parent-block", "parentBlock", "referenceBlockId", + ) { + t.Fatal("reviewed whiteboard parent-block correction must be accepted") + } + if compatibleReviewedPropertyCorrection( + "doc/doc.whiteboard_insert", "ref-block", "refBlock", "otherProperty", + ) { + t.Fatal("non-reviewed new property must remain incompatible") + } + if compatibleReviewedPropertyCorrection( + "doc/doc.create", "title", "title", "subject", + ) { + t.Fatal("unlisted tool/param remap must remain incompatible") + } + + baseline := schemaContract{ + Products: map[string]productSchema{ + "doc": {Tools: map[string]toolSchema{ + "doc.whiteboard_insert": { + PrimaryCLIPath: "doc whiteboard insert", + InterfaceMode: "composite", + Availability: "available", + Effect: "write", + Risk: "medium", + Confirmation: "user_required", + Idempotency: "unknown", + Parameters: map[string]parameterSchema{ + "ref-block": {Type: "string", Property: "refBlock"}, + "parent-block": {Type: "string", Property: "parentBlock"}, + }, + }, + }}, + }, + } + corrected := cloneContract(baseline) + corrected.Products["doc"].Tools["doc.whiteboard_insert"].Parameters["ref-block"] = parameterSchema{ + Type: "string", Property: "referenceBlockId", + } + corrected.Products["doc"].Tools["doc.whiteboard_insert"].Parameters["parent-block"] = parameterSchema{ + Type: "string", Property: "referenceBlockId", + } + if failures := checkCompatibility(baseline, corrected); len(failures) != 0 { + t.Fatalf("reviewed property corrections should pass: %v", failures) + } + + broken := cloneContract(baseline) + broken.Products["doc"].Tools["doc.whiteboard_insert"].Parameters["ref-block"] = parameterSchema{ + Type: "string", Property: "subject", + } + if failures := checkCompatibility(baseline, broken); !strings.Contains(strings.Join(failures, "\n"), "changed property") { + t.Fatalf("unreviewed property remap should fail: %v", failures) + } +} + func TestCrossPlatformCoverageSchemaCompatAdditiveConstraintEvolution(t *testing.T) { oldTool := toolSchema{ Parameters: map[string]parameterSchema{