feat: support optional flag rename governance to #85349380
This commit is contained in:
1 parent
38e387bcd6
commit
bfd836064d
6 files changed
+241
-60
No files matched your search
@@ -1,6 +1,6 @@
|
||||
# CLI flag 兼容迁移治理
|
||||
|
||||
本文定义一种受控迁移:保留旧 flag 的可执行兼容性,但把它从 Help 与 Agent Schema 中隐藏,并将新的规范 flag 提升为必填。它只解决这一种精确变更,不是通用 breaking-change 豁免。
|
||||
本文定义一种受控迁移:保留旧 flag 的可执行兼容性,但把它从 Help 与 Agent Schema 中隐藏,并将新的规范 flag 设为唯一可见入口。迁移必须保持原 flag 的 requiredness:optional 只能迁到 optional,required 只能迁到 required。它只解决这一种精确变更,不是通用 breaking-change 豁免。
|
||||
|
||||
同名 flag 的精确类型迁移属于另一类评审机制,只能进入
|
||||
`internal/interfacesnapshot/reviewed.go` 与 legacy smoke helper 的镜像表;flag rename
|
||||
@@ -50,7 +50,7 @@ scripts/policy/interface-migrations/approved-flag-migrations-v1.json
|
||||
| 阶段 | PR 可以做什么 | 必须满足的快照状态 |
|
||||
|---|---|---|
|
||||
| 1. 治理审批 | 新增 `state: pending` 的精确记录;不得在同一个 PR 修改产品 surface | candidate 和 merge-base 都与记录中的 `before` 完全一致;该记录不改变 stable 的判断 |
|
||||
| 2. 产品迁移 | merge-base 已拥有 `pending` 后,按记录一次性切到精确 `after`,并把记录改为 `state: consumed` | legacy 仍存在但由 visible 变 hidden,且声明 `alias_of`;canonical 达到记录的必填状态 |
|
||||
| 2. 产品迁移 | merge-base 已拥有 `pending` 后,按记录一次性切到精确 `after`,并把记录改为 `state: consumed` | legacy 仍存在但由 visible 变 hidden,且声明 `alias_of`;canonical 的 requiredness 与 legacy 迁移前完全一致 |
|
||||
| 3. 保留回执 | 产品 PR 合入后,如果 stable 仍是 `before`,继续保留 `consumed` | merge-base 或 stable 仍有任一份尚未达到 `after` |
|
||||
| 4. 单独清理 | 当 merge-base 和 stable 都已经是 `after`,在后续 PR 删除该记录 | 两份参考快照均精确匹配 `after`;继续保留过期回执会被门禁拒绝 |
|
||||
|
||||
@@ -122,7 +122,7 @@ scripts/policy/interface-migrations/approved-flag-migrations-v1.json
|
||||
一条 base-owned、状态正确且前后快照精确匹配的记录,只会从普通兼容报告中移除以下两类预期 finding:
|
||||
|
||||
1. legacy flag 的 `flag_became_hidden`(visible → hidden);
|
||||
2. canonical flag 的 `required_flag_added`(新增时即必填)或 `flag_became_required`(已有 flag 从可选变必填)。
|
||||
2. required legacy 被新增的 required canonical 替代时产生的 `required_flag_added`。已有 canonical 的 requiredness 不允许借 rename 改变。
|
||||
|
||||
以下变化仍按普通兼容规则阻塞,不能被迁移记录掩盖:
|
||||
|
||||
@@ -145,7 +145,7 @@ legacy 名改为 canonical 名。Schema adapter 只接受已经由三方 Interfa
|
||||
`required` / `cli_required` 或重写 constraint;
|
||||
- rename 前后的 `type`、`property`、`interface_type`、default、format、enum 与
|
||||
`required_when` 必须完全一致;
|
||||
- `required` / `cli_required` 只能保持不变或按审批从 `false` 提升为 `true`,禁止降低;
|
||||
- `required` / `cli_required` 必须在 rename 前后完全一致,升高或降低都失败;
|
||||
- constraint 只允许在同一 tool 内按已枚举的 legacy → canonical map 做 member 替换、
|
||||
排序与去重;group kind、非迁移 member 或 group 增删仍然阻塞;
|
||||
- 多个 legacy 指向同一 canonical 时,所有历史 parameter signature 必须一致,否则
|
||||
|
||||
@@ -324,11 +324,20 @@ func (m FlagMigration) validate() error {
|
||||
if m.Canonical.After.Hidden {
|
||||
return fmt.Errorf("canonical flag must remain visible")
|
||||
}
|
||||
if !m.Canonical.After.Required {
|
||||
return fmt.Errorf("canonical flag must be required after migration")
|
||||
// Requiredness belongs to the one logical parameter. The hidden legacy
|
||||
// spelling must not remain independently required, while the canonical
|
||||
// spelling inherits the exact before-state contract. If the canonical flag
|
||||
// already existed, a rename receipt cannot authorize changing it either.
|
||||
if m.Legacy.After.Required {
|
||||
return fmt.Errorf("legacy compatibility alias must not remain independently required after migration")
|
||||
}
|
||||
if m.Canonical.Before.Present && m.Canonical.Before.Required {
|
||||
return fmt.Errorf("canonical flag must be absent or optional before migration")
|
||||
if m.Legacy.Before.Required != m.Canonical.After.Required {
|
||||
return fmt.Errorf(
|
||||
"flag requiredness must be preserved from legacy before to canonical after",
|
||||
)
|
||||
}
|
||||
if m.Canonical.Before.Present && m.Canonical.Before.Required != m.Canonical.After.Required {
|
||||
return fmt.Errorf("canonical flag requiredness must remain unchanged when already present")
|
||||
}
|
||||
if m.Legacy.After.AliasOf != m.Canonical.Name {
|
||||
return fmt.Errorf(
|
||||
@@ -681,12 +690,6 @@ func flagMigrationAuthorizesChange(
|
||||
change.Kind == "required_flag_added" {
|
||||
return true
|
||||
}
|
||||
if migration.Canonical.Before.Present &&
|
||||
!migration.Canonical.Before.Required &&
|
||||
migration.Canonical.After.Required &&
|
||||
change.Kind == "flag_became_required" {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -188,7 +188,7 @@ func TestCrossPlatformCoverageFlagMigrationManifestParserEdges(t *testing.T) {
|
||||
|
||||
func TestCrossPlatformCoverageFlagMigrationManifestRejectsEveryContractDrift(t *testing.T) {
|
||||
optionalCanonical := func() FlagMigrationManifest {
|
||||
manifest := coverageManifest(FlagMigrationPending)
|
||||
manifest := coverageOptionalManifest(FlagMigrationPending)
|
||||
manifest.Migrations[0].Canonical.Before = FlagMigrationState{
|
||||
Present: true,
|
||||
Type: "string",
|
||||
@@ -269,6 +269,36 @@ func TestCrossPlatformCoverageFlagMigrationManifestRejectsEveryContractDrift(t *
|
||||
mutate: func(m *FlagMigrationManifest) { m.Migrations[0].Canonical.After = FlagMigrationState{} },
|
||||
wantErr: "canonical flag must be present after migration",
|
||||
},
|
||||
{
|
||||
name: "required legacy cannot become optional canonical",
|
||||
make: func() FlagMigrationManifest { return coverageManifest(FlagMigrationPending) },
|
||||
mutate: func(m *FlagMigrationManifest) { m.Migrations[0].Canonical.After.Required = false },
|
||||
wantErr: "requiredness must be preserved from legacy before to canonical after",
|
||||
},
|
||||
{
|
||||
name: "optional legacy cannot become required canonical",
|
||||
make: func() FlagMigrationManifest { return coverageOptionalManifest(FlagMigrationPending) },
|
||||
mutate: func(m *FlagMigrationManifest) { m.Migrations[0].Canonical.After.Required = true },
|
||||
wantErr: "requiredness must be preserved from legacy before to canonical after",
|
||||
},
|
||||
{
|
||||
name: "existing canonical cannot change requiredness",
|
||||
make: func() FlagMigrationManifest { return coverageManifest(FlagMigrationPending) },
|
||||
mutate: func(manifest *FlagMigrationManifest) {
|
||||
manifest.Migrations[0].Canonical.Before = FlagMigrationState{
|
||||
Present: true,
|
||||
Type: "string",
|
||||
Scope: "local",
|
||||
}
|
||||
},
|
||||
wantErr: "canonical flag requiredness must remain unchanged when already present",
|
||||
},
|
||||
{
|
||||
name: "hidden legacy alias is not independently required",
|
||||
make: func() FlagMigrationManifest { return coverageManifest(FlagMigrationPending) },
|
||||
mutate: func(m *FlagMigrationManifest) { m.Migrations[0].Legacy.After.Required = true },
|
||||
wantErr: "legacy compatibility alias must not remain independently required",
|
||||
},
|
||||
{
|
||||
name: "legacy after declares alias target",
|
||||
make: func() FlagMigrationManifest { return coverageManifest(FlagMigrationPending) },
|
||||
@@ -711,8 +741,32 @@ func TestCrossPlatformCoverageCompareAllWithFlagMigrationsInheritedAndOptionalCa
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("existing optional canonical becomes required", func(t *testing.T) {
|
||||
pending := coverageManifest(FlagMigrationPending)
|
||||
t.Run("optional canonical is introduced without becoming required", func(t *testing.T) {
|
||||
pending := coverageOptionalManifest(FlagMigrationPending)
|
||||
consumed := coverageOptionalManifest(FlagMigrationConsumed)
|
||||
before := coverageMigrationSnapshot(pending.Migrations[0], false, false)
|
||||
after := coverageMigrationSnapshot(pending.Migrations[0], true, false)
|
||||
|
||||
ordinary := Compare(after, before, "merge-base")
|
||||
if !hasFlagChange(ordinary.Blocking, "flag_became_hidden", pending.Migrations[0].Command, pending.Migrations[0].Legacy.Name) {
|
||||
t.Fatalf("fixture did not create flag_became_hidden: %#v", ordinary.Blocking)
|
||||
}
|
||||
if hasFlagChange(ordinary.Blocking, "required_flag_added", pending.Migrations[0].Command, pending.Migrations[0].Canonical.Name) {
|
||||
t.Fatalf("optional canonical was treated as required: %#v", ordinary.Blocking)
|
||||
}
|
||||
report, err := CompareAllWithFlagMigrations(
|
||||
after,
|
||||
map[string]Snapshot{"merge-base": before, "stable": before},
|
||||
pending,
|
||||
consumed,
|
||||
)
|
||||
if err != nil || !report.Compatible {
|
||||
t.Fatalf("optional rename = (%#v, %v), want compatible", report, err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("existing optional canonical remains optional", func(t *testing.T) {
|
||||
pending := coverageOptionalManifest(FlagMigrationPending)
|
||||
pending.Migrations[0].Canonical.Before = FlagMigrationState{
|
||||
Present: true,
|
||||
Type: "string",
|
||||
@@ -725,8 +779,8 @@ func TestCrossPlatformCoverageCompareAllWithFlagMigrationsInheritedAndOptionalCa
|
||||
after := coverageMigrationSnapshot(pending.Migrations[0], true, false)
|
||||
|
||||
ordinary := Compare(after, before, "merge-base")
|
||||
if !hasFlagChange(ordinary.Blocking, "flag_became_required", pending.Migrations[0].Command, pending.Migrations[0].Canonical.Name) {
|
||||
t.Fatalf("fixture did not create flag_became_required: %#v", ordinary.Blocking)
|
||||
if hasFlagChange(ordinary.Blocking, "flag_became_required", pending.Migrations[0].Command, pending.Migrations[0].Canonical.Name) {
|
||||
t.Fatalf("fixture changed canonical requiredness: %#v", ordinary.Blocking)
|
||||
}
|
||||
report, err := CompareAllWithFlagMigrations(
|
||||
after,
|
||||
@@ -735,7 +789,54 @@ func TestCrossPlatformCoverageCompareAllWithFlagMigrationsInheritedAndOptionalCa
|
||||
consumed,
|
||||
)
|
||||
if err != nil || !report.Compatible {
|
||||
t.Fatalf("optional-to-required migration = (%#v, %v), want compatible", report, err)
|
||||
t.Fatalf("existing optional canonical migration = (%#v, %v), want compatible", report, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestCrossPlatformCoverageOptionalFlagMigrationLifecycleRemainsHostile(t *testing.T) {
|
||||
pending := coverageOptionalManifest(FlagMigrationPending)
|
||||
consumed := coverageOptionalManifest(FlagMigrationConsumed)
|
||||
empty := coverageEmptyManifest()
|
||||
migration := pending.Migrations[0]
|
||||
before := coverageMigrationSnapshot(migration, false, false)
|
||||
after := coverageMigrationSnapshot(migration, true, false)
|
||||
|
||||
t.Run("candidate cannot self authorize", func(t *testing.T) {
|
||||
_, err := CompareAllWithFlagMigrations(
|
||||
after,
|
||||
map[string]Snapshot{"merge-base": before, "stable": before},
|
||||
empty,
|
||||
pending,
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), "cannot authorize its own interface change") {
|
||||
t.Fatalf("candidate self-authorization error = %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("partial application remains rejected", func(t *testing.T) {
|
||||
partial := coverageMigrationSnapshot(migration, false, false)
|
||||
partial.Commands[len(partial.Commands)-1].LocalFlags[0].Hidden = true
|
||||
_, err := CompareAllWithFlagMigrations(
|
||||
partial,
|
||||
map[string]Snapshot{"merge-base": before, "stable": before},
|
||||
pending,
|
||||
consumed,
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), "partially applied flag migration") {
|
||||
t.Fatalf("partial optional migration error = %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("consumed receipt remains stale after every reference converges", func(t *testing.T) {
|
||||
_, err := CompareAllWithFlagMigrations(
|
||||
after,
|
||||
map[string]Snapshot{"merge-base": after, "stable": after},
|
||||
consumed,
|
||||
consumed,
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), "stale after all references reached the after state") {
|
||||
t.Fatalf("stale optional migration error = %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -886,6 +987,13 @@ func coverageManifest(state string) FlagMigrationManifest {
|
||||
}
|
||||
}
|
||||
|
||||
func coverageOptionalManifest(state string) FlagMigrationManifest {
|
||||
manifest := coverageManifest(state)
|
||||
manifest.Migrations[0].Legacy.Before.Required = false
|
||||
manifest.Migrations[0].Canonical.After.Required = false
|
||||
return manifest
|
||||
}
|
||||
|
||||
func coverageEmptyManifest() FlagMigrationManifest {
|
||||
return FlagMigrationManifest{Version: FlagMigrationManifestVersion, Migrations: []FlagMigration{}}
|
||||
}
|
||||
|
||||
@@ -37,6 +37,21 @@ const validFlagMigrationManifestJSON = `{
|
||||
]
|
||||
}`
|
||||
|
||||
func optionalFlagMigrationManifestJSON() string {
|
||||
manifest := strings.Replace(
|
||||
validFlagMigrationManifestJSON,
|
||||
`"before": {"present": true, "type": "string", "required": true, "scope": "local"}`,
|
||||
`"before": {"present": true, "type": "string", "scope": "local"}`,
|
||||
1,
|
||||
)
|
||||
return strings.Replace(
|
||||
manifest,
|
||||
`"after": {"present": true, "type": "string", "required": true, "scope": "local"}`,
|
||||
`"after": {"present": true, "type": "string", "scope": "local"}`,
|
||||
1,
|
||||
)
|
||||
}
|
||||
|
||||
func TestCrossPlatformCoverageReadFlagMigrationManifestRejectsUnknownFields(t *testing.T) {
|
||||
_, err := ReadFlagMigrationManifest(strings.NewReader(`{
|
||||
"version": 1,
|
||||
@@ -110,6 +125,9 @@ func TestCrossPlatformCoverageReadFlagMigrationManifestValidatesExactEntries(t *
|
||||
if _, err := ReadFlagMigrationManifest(strings.NewReader(validFlagMigrationManifestJSON)); err != nil {
|
||||
t.Fatalf("ReadFlagMigrationManifest(valid) error = %v", err)
|
||||
}
|
||||
if _, err := ReadFlagMigrationManifest(strings.NewReader(optionalFlagMigrationManifestJSON())); err != nil {
|
||||
t.Fatalf("ReadFlagMigrationManifest(optional rename) error = %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -162,24 +180,34 @@ func TestCrossPlatformCoverageReadFlagMigrationManifestValidatesExactEntries(t *
|
||||
wantErr: "canonical flag must remain visible",
|
||||
},
|
||||
{
|
||||
name: "canonical remains optional",
|
||||
name: "required legacy becomes optional canonical",
|
||||
input: strings.Replace(
|
||||
validFlagMigrationManifestJSON,
|
||||
`"after": {"present": true, "type": "string", "required": true, "scope": "local"}`,
|
||||
`"after": {"present": true, "type": "string", "scope": "local"}`,
|
||||
1,
|
||||
),
|
||||
wantErr: "canonical flag must be required after migration",
|
||||
wantErr: "requiredness must be preserved from legacy before to canonical after",
|
||||
},
|
||||
{
|
||||
name: "canonical was already required",
|
||||
name: "optional legacy becomes required canonical",
|
||||
input: strings.Replace(
|
||||
optionalFlagMigrationManifestJSON(),
|
||||
`"after": {"present": true, "type": "string", "scope": "local"}`,
|
||||
`"after": {"present": true, "type": "string", "required": true, "scope": "local"}`,
|
||||
1,
|
||||
),
|
||||
wantErr: "requiredness must be preserved from legacy before to canonical after",
|
||||
},
|
||||
{
|
||||
name: "existing canonical changes requiredness",
|
||||
input: strings.Replace(
|
||||
validFlagMigrationManifestJSON,
|
||||
`"before": {"present": false}`,
|
||||
`"before": {"present": true, "type": "string", "required": true, "scope": "local"}`,
|
||||
`"before": {"present": true, "type": "string", "scope": "local"}`,
|
||||
1,
|
||||
),
|
||||
wantErr: "canonical flag must be absent or optional before migration",
|
||||
wantErr: "canonical flag requiredness must remain unchanged when already present",
|
||||
},
|
||||
}
|
||||
|
||||
@@ -215,9 +243,10 @@ func TestCrossPlatformCoverageFlagMigrationManifestRejectsDuplicateAndInexactCon
|
||||
canonicalDrift := manifest
|
||||
canonicalDrift.Migrations = append([]FlagMigration(nil), manifest.Migrations...)
|
||||
canonicalDrift.Migrations[0].Canonical.Before = FlagMigrationState{
|
||||
Present: true,
|
||||
Type: "string",
|
||||
Scope: "local",
|
||||
Present: true,
|
||||
Type: "string",
|
||||
Required: true,
|
||||
Scope: "local",
|
||||
}
|
||||
canonicalDrift.Migrations[0].Canonical.After.Type = "stringSlice"
|
||||
if err := canonicalDrift.Validate(); err == nil || !strings.Contains(err.Error(), "canonical flag type") {
|
||||
|
||||
@@ -1313,6 +1313,8 @@ func validateRenamedSchemaParameter(
|
||||
oldParameter parameterSchema,
|
||||
newParameter parameterSchema,
|
||||
) error {
|
||||
// The migration authorizes only the CLI spelling change. Requiredness is
|
||||
// part of the parameter contract in both projections and must remain exact.
|
||||
if oldParameter.Type != newParameter.Type ||
|
||||
oldParameter.Property != newParameter.Property ||
|
||||
oldParameter.InterfaceType != newParameter.InterfaceType ||
|
||||
@@ -1328,17 +1330,17 @@ func validateRenamedSchemaParameter(
|
||||
migration.Canonical.Name,
|
||||
)
|
||||
}
|
||||
if oldParameter.Required && !newParameter.Required {
|
||||
if oldParameter.Required != newParameter.Required {
|
||||
return fmt.Errorf(
|
||||
"approved flag migration %q Schema parameter %q -> %q became optional",
|
||||
"approved flag migration %q Schema parameter %q -> %q changed requiredness",
|
||||
migration.Command,
|
||||
migration.Legacy.Name,
|
||||
migration.Canonical.Name,
|
||||
)
|
||||
}
|
||||
if oldParameter.CLIRequired && !newParameter.CLIRequired {
|
||||
if oldParameter.CLIRequired != newParameter.CLIRequired {
|
||||
return fmt.Errorf(
|
||||
"approved flag migration %q Schema parameter %q -> %q stopped being cli_required",
|
||||
"approved flag migration %q Schema parameter %q -> %q changed cli_required",
|
||||
migration.Command,
|
||||
migration.Legacy.Name,
|
||||
migration.Canonical.Name,
|
||||
|
||||
@@ -1218,8 +1218,11 @@ func TestCrossPlatformCoverageSchemaFlagMigrationNormalizesExactRename(t *testin
|
||||
t.Fatalf("normalized baseline retained legacy parameter %q", legacy)
|
||||
}
|
||||
}
|
||||
if canonical := tool.Parameters["conversation-id"]; !canonical.Required || !canonical.CLIRequired {
|
||||
t.Fatalf("canonical required transition was not normalized: %#v", canonical)
|
||||
if canonical := tool.Parameters["conversation-id"]; canonical.Required || canonical.CLIRequired {
|
||||
t.Fatalf("optional canonical rename changed requiredness: %#v", canonical)
|
||||
}
|
||||
if canonical := tool.Parameters["message-id"]; !canonical.Required || !canonical.CLIRequired {
|
||||
t.Fatalf("required canonical rename changed requiredness: %#v", canonical)
|
||||
}
|
||||
if tool.Constraints != current.Products["chat"].Tools["chat.edit_message"].Constraints {
|
||||
t.Fatalf("constraints were not normalized: %s", tool.Constraints)
|
||||
@@ -1269,6 +1272,39 @@ func TestCrossPlatformCoverageSchemaFlagMigrationRejectsSemanticDrift(t *testing
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
want string
|
||||
mutate func(*parameterSchema)
|
||||
}{
|
||||
{name: "optional required promotion", want: "changed requiredness", mutate: func(parameter *parameterSchema) {
|
||||
parameter.Required = true
|
||||
}},
|
||||
{name: "optional cli_required promotion", want: "changed cli_required", mutate: func(parameter *parameterSchema) {
|
||||
parameter.CLIRequired = true
|
||||
}},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
current := schemaFlagMigrationContract(true)
|
||||
product := current.Products["chat"]
|
||||
tool := product.Tools["chat.edit_message"]
|
||||
canonical := tool.Parameters["conversation-id"]
|
||||
test.mutate(&canonical)
|
||||
tool.Parameters["conversation-id"] = canonical
|
||||
product.Tools["chat.edit_message"] = tool
|
||||
current.Products["chat"] = product
|
||||
|
||||
_, err := normalizeSchemaFlagMigrations(
|
||||
schemaFlagMigrationContract(false),
|
||||
current,
|
||||
schemaFlagMigrationAuthorizations(),
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), test.want) {
|
||||
t.Fatalf("normalizeSchemaFlagMigrations() error = %v, want %q", err, test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCrossPlatformCoverageSchemaFlagMigrationAdapterBranches(t *testing.T) {
|
||||
@@ -1321,9 +1357,10 @@ func TestCrossPlatformCoverageSchemaFlagMigrationAdapterBranches(t *testing.T) {
|
||||
t.Fatal("missing candidate product was normalized away")
|
||||
}
|
||||
|
||||
canonicalOnly := schemaFlagMigrationAuthorizations()[0]
|
||||
canonicalOnly.Legacy.Name = "legacy-not-published-in-schema"
|
||||
driftedCanonical := cloneContract(current)
|
||||
canonicalOnlyMigration := schemaFlagMigrationAuthorizations()[0]
|
||||
canonicalOnlyMigration.Legacy.Name = "legacy-not-published-in-schema"
|
||||
canonicalOnlyBaseline := schemaFlagMigrationContract(true)
|
||||
driftedCanonical := cloneContract(canonicalOnlyBaseline)
|
||||
product = driftedCanonical.Products["chat"]
|
||||
tool = product.Tools["chat.edit_message"]
|
||||
canonical := tool.Parameters["conversation-id"]
|
||||
@@ -1331,7 +1368,7 @@ func TestCrossPlatformCoverageSchemaFlagMigrationAdapterBranches(t *testing.T) {
|
||||
tool.Parameters["conversation-id"] = canonical
|
||||
product.Tools["chat.edit_message"] = tool
|
||||
driftedCanonical.Products["chat"] = product
|
||||
normalized, err = normalizeSchemaFlagMigrations(baseline, driftedCanonical, []interfacesnapshot.FlagMigration{canonicalOnly})
|
||||
normalized, err = normalizeSchemaFlagMigrations(canonicalOnlyBaseline, driftedCanonical, []interfacesnapshot.FlagMigration{canonicalOnlyMigration})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -1339,24 +1376,24 @@ func TestCrossPlatformCoverageSchemaFlagMigrationAdapterBranches(t *testing.T) {
|
||||
t.Fatalf("canonical-only Schema drift was hidden: %s", failures)
|
||||
}
|
||||
|
||||
canonicalOptional := schemaFlagMigrationContract(true)
|
||||
product = canonicalOptional.Products["chat"]
|
||||
promotedCanonical := cloneContract(canonicalOnlyBaseline)
|
||||
product = promotedCanonical.Products["chat"]
|
||||
tool = product.Tools["chat.edit_message"]
|
||||
canonical = tool.Parameters["conversation-id"]
|
||||
canonical.Required = false
|
||||
canonical.CLIRequired = false
|
||||
canonical.Required = true
|
||||
canonical.CLIRequired = true
|
||||
tool.Parameters["conversation-id"] = canonical
|
||||
product.Tools["chat.edit_message"] = tool
|
||||
canonicalOptional.Products["chat"] = product
|
||||
promotedCanonical.Products["chat"] = product
|
||||
normalized, err = normalizeSchemaFlagMigrations(
|
||||
canonicalOptional,
|
||||
schemaFlagMigrationContract(true),
|
||||
[]interfacesnapshot.FlagMigration{canonicalOnly},
|
||||
canonicalOnlyBaseline,
|
||||
promotedCanonical,
|
||||
[]interfacesnapshot.FlagMigration{canonicalOnlyMigration},
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if failures := strings.Join(checkCompatibility(normalized, schemaFlagMigrationContract(true)), "\n"); !strings.Contains(failures, "newly required") || !strings.Contains(failures, "newly cli_required") {
|
||||
if failures := strings.Join(checkCompatibility(normalized, promotedCanonical), "\n"); !strings.Contains(failures, "newly required") || !strings.Contains(failures, "newly cli_required") {
|
||||
t.Fatalf("canonical-only required promotion was hidden: %s", failures)
|
||||
}
|
||||
|
||||
@@ -1392,12 +1429,19 @@ func TestCrossPlatformCoverageSchemaFlagMigrationAdapterBranches(t *testing.T) {
|
||||
}
|
||||
|
||||
old := parameterSchema{Required: true, CLIRequired: true}
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], old, parameterSchema{CLIRequired: true}); err == nil || !strings.Contains(err.Error(), "became optional") {
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], old, parameterSchema{CLIRequired: true}); err == nil || !strings.Contains(err.Error(), "changed requiredness") {
|
||||
t.Fatalf("direct required decline error = %v", err)
|
||||
}
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], old, parameterSchema{Required: true}); err == nil || !strings.Contains(err.Error(), "stopped being cli_required") {
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], old, parameterSchema{Required: true}); err == nil || !strings.Contains(err.Error(), "changed cli_required") {
|
||||
t.Fatalf("direct cli_required decline error = %v", err)
|
||||
}
|
||||
optional := parameterSchema{}
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], optional, parameterSchema{Required: true}); err == nil || !strings.Contains(err.Error(), "changed requiredness") {
|
||||
t.Fatalf("direct required promotion error = %v", err)
|
||||
}
|
||||
if err := validateRenamedSchemaParameter(schemaFlagMigrationAuthorizations()[0], optional, parameterSchema{CLIRequired: true}); err == nil || !strings.Contains(err.Error(), "changed cli_required") {
|
||||
t.Fatalf("direct cli_required promotion error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCrossPlatformCoverageSchemaFlagMigrationRejectsPartialAndUnrelatedChanges(t *testing.T) {
|
||||
@@ -1471,9 +1515,8 @@ func TestCrossPlatformCoverageSchemaFlagMigrationRejectsPartialAndUnrelatedChang
|
||||
t.Fatalf("constraint rewrite without Schema parameter evidence was hidden: %s", failures)
|
||||
}
|
||||
|
||||
// A baseline that already contains only the canonical parameter may receive
|
||||
// a required promotion, but that is not evidence that a stray legacy name in
|
||||
// constraints belongs to the migration.
|
||||
// A baseline that already contains only the canonical parameter is not
|
||||
// evidence that a stray legacy name in constraints belongs to the migration.
|
||||
canonicalOnly := schemaFlagMigrationContract(true)
|
||||
product = canonicalOnly.Products["chat"]
|
||||
tool = product.Tools["chat.edit_message"]
|
||||
@@ -1749,13 +1792,10 @@ func schemaFlagMigrationContract(after bool) schemaContract {
|
||||
InterfaceType: "string",
|
||||
}
|
||||
parameters := map[string]parameterSchema{
|
||||
"conversation-id": conversation,
|
||||
"unrelated": {Type: `"string"`, Property: "unrelated"},
|
||||
"unrelated": {Type: `"string"`, Property: "unrelated"},
|
||||
}
|
||||
constraints := `{"require_one_of":[["conversation-id","group","id"]]}`
|
||||
constraints := `{"require_one_of":[["group","id"]]}`
|
||||
if after {
|
||||
conversation.Required = true
|
||||
conversation.CLIRequired = true
|
||||
parameters["conversation-id"] = conversation
|
||||
parameters["message-id"] = legacyMessage
|
||||
constraints = `{"require_one_of":[["conversation-id"]]}`
|
||||
@@ -1788,7 +1828,6 @@ func schemaFlagMigrationAuthorizations() []interfacesnapshot.FlagMigration {
|
||||
Scope: "local",
|
||||
}
|
||||
conversationAfter := conversationBefore
|
||||
conversationAfter.Required = true
|
||||
messageBefore := interfacesnapshot.FlagMigrationState{
|
||||
Present: true,
|
||||
Type: "string",
|
||||
@@ -1808,7 +1847,7 @@ func schemaFlagMigrationAuthorizations() []interfacesnapshot.FlagMigration {
|
||||
},
|
||||
Canonical: interfacesnapshot.FlagMigrationSide{
|
||||
Name: "conversation-id",
|
||||
Before: conversationBefore,
|
||||
Before: interfacesnapshot.FlagMigrationState{},
|
||||
After: conversationAfter,
|
||||
},
|
||||
},
|
||||
@@ -1823,7 +1862,7 @@ func schemaFlagMigrationAuthorizations() []interfacesnapshot.FlagMigration {
|
||||
},
|
||||
Canonical: interfacesnapshot.FlagMigrationSide{
|
||||
Name: "conversation-id",
|
||||
Before: conversationBefore,
|
||||
Before: interfacesnapshot.FlagMigrationState{},
|
||||
After: conversationAfter,
|
||||
},
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user