From bfd836064ded9b3e394de640f4a1c95fc0378b7d Mon Sep 17 00:00:00 2001 From: wxianfeng Date: Tue, 11 Aug 2026 22:01:22 +0800 Subject: [PATCH] feat: support optional flag rename governance to #85349380 --- docs/cli-interface-flag-migrations.md | 8 +- internal/interfacesnapshot/migrations.go | 23 ++-- .../migrations_coverage_test.go | 120 +++++++++++++++++- internal/interfacesnapshot/migrations_test.go | 45 +++++-- scripts/policy/schema-compat/main.go | 10 +- scripts/policy/schema-compat/main_test.go | 95 ++++++++++---- 6 files changed, 241 insertions(+), 60 deletions(-) diff --git a/docs/cli-interface-flag-migrations.md b/docs/cli-interface-flag-migrations.md index ada97b55..37bc3d8e 100644 --- a/docs/cli-interface-flag-migrations.md +++ b/docs/cli-interface-flag-migrations.md @@ -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 必须一致，否则 diff --git a/internal/interfacesnapshot/migrations.go b/internal/interfacesnapshot/migrations.go index 3d806c39..7bd04bcd 100644 --- a/internal/interfacesnapshot/migrations.go +++ b/internal/interfacesnapshot/migrations.go @@ -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 } diff --git a/internal/interfacesnapshot/migrations_coverage_test.go b/internal/interfacesnapshot/migrations_coverage_test.go index 5e4731f1..bdee575f 100644 --- a/internal/interfacesnapshot/migrations_coverage_test.go +++ b/internal/interfacesnapshot/migrations_coverage_test.go @@ -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{}} } diff --git a/internal/interfacesnapshot/migrations_test.go b/internal/interfacesnapshot/migrations_test.go index 01fc03a9..a7996dba 100644 --- a/internal/interfacesnapshot/migrations_test.go +++ b/internal/interfacesnapshot/migrations_test.go @@ -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") { diff --git a/scripts/policy/schema-compat/main.go b/scripts/policy/schema-compat/main.go index 1b70e747..1f55eae4 100644 --- a/scripts/policy/schema-compat/main.go +++ b/scripts/policy/schema-compat/main.go @@ -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, diff --git a/scripts/policy/schema-compat/main_test.go b/scripts/policy/schema-compat/main_test.go index 121f9299..f33f09c5 100644 --- a/scripts/policy/schema-compat/main_test.go +++ b/scripts/policy/schema-compat/main_test.go @@ -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, }, },