From d41ea586bf9514009e95137bd6befa8d8021c8fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=89=E6=BE=9C?= Date: Mon, 13 Jul 2026 14:01:26 +0800 Subject: [PATCH] fix(schema): enforce catalog and interface completeness --- internal/cli/runtime_schema.go | 1 + internal/cli/schema_agent_metadata/index.json | 2 +- internal/cli/schema_agent_metadata_audit.json | 2 +- internal/cli/schema_agent_metadata_test.go | 4 +- internal/cli/schema_catalog.json | 5 +- internal/cli/schema_completeness.go | 71 ++++++++++++++++--- internal/cli/schema_completeness_test.go | 70 ++++++++++++++++++ internal/cli/schema_mcp_metadata.json | 1 + internal/cli/schema_parameter_bindings.json | 2 +- internal/generator/cmd_schema_catalog/main.go | 3 + scripts/policy/check-schema-catalog.sh | 44 ++++++++++-- skills/mono/schema-hints/README.md | 10 +++ .../mono/schema-hints/selection-review.json | 6 +- 13 files changed, 199 insertions(+), 22 deletions(-) diff --git a/internal/cli/runtime_schema.go b/internal/cli/runtime_schema.go index daebd698..2fc245da 100644 --- a/internal/cli/runtime_schema.go +++ b/internal/cli/runtime_schema.go @@ -76,6 +76,7 @@ type embeddedMCPMetadata struct { } type embeddedMCPMetadataCoverage struct { + SurfaceScope string `json:"surface_scope,omitempty"` SourceServices int `json:"source_services,omitempty"` SnapshotServices int `json:"snapshot_services,omitempty"` MissingServices []string `json:"missing_services,omitempty"` diff --git a/internal/cli/schema_agent_metadata/index.json b/internal/cli/schema_agent_metadata/index.json index 628d07b3..273e746e 100644 --- a/internal/cli/schema_agent_metadata/index.json +++ b/internal/cli/schema_agent_metadata/index.json @@ -1,6 +1,6 @@ { "version": 1, - "source_hash": "sha256:b9fd6c4de190c413312561049ed2bbb80b272aad127ba2c0a5082364761012ee", + "source_hash": "sha256:b0788dd79d817258e2fd5869e4b32eff5ab81b6d20f34e7eb131446670402003", "surface_hash": "sha256:75c63f35d9b15dfb4df09136bfd250906751cfeb7f14bce08cf092097859baad", "coverage": { "surface_products": 20, diff --git a/internal/cli/schema_agent_metadata_audit.json b/internal/cli/schema_agent_metadata_audit.json index b0977eca..fe0094d8 100644 --- a/internal/cli/schema_agent_metadata_audit.json +++ b/internal/cli/schema_agent_metadata_audit.json @@ -1,6 +1,6 @@ { "version": 1, - "source_hash": "sha256:b9fd6c4de190c413312561049ed2bbb80b272aad127ba2c0a5082364761012ee", + "source_hash": "sha256:b0788dd79d817258e2fd5869e4b32eff5ab81b6d20f34e7eb131446670402003", "surface_hash": "sha256:75c63f35d9b15dfb4df09136bfd250906751cfeb7f14bce08cf092097859baad", "source_files": 101, "hint_files": 13, diff --git a/internal/cli/schema_agent_metadata_test.go b/internal/cli/schema_agent_metadata_test.go index a63991f6..67e1abe0 100644 --- a/internal/cli/schema_agent_metadata_test.go +++ b/internal/cli/schema_agent_metadata_test.go @@ -140,6 +140,7 @@ func TestRuntimeSchemaReportsEmbeddedInterfaceMetadata(t *testing.T) { SourceRevision: "revision-test", SourceHash: "sha256:interface-test", Coverage: embeddedMCPMetadataCoverage{ + SurfaceScope: "source_revision", SourceTools: 10, SurfaceTools: 2, MatchedTools: 1, @@ -159,7 +160,8 @@ func TestRuntimeSchemaReportsEmbeddedInterfaceMetadata(t *testing.T) { if summary["source"] != "cli-registry" || summary["source_hash"] != "sha256:interface-test" || summary["tool_count"] != 1 { t.Fatalf("interface metadata summary = %#v", summary) } - if summary["source_revision"] != "revision-test" || summary["coverage"] == nil { + coverage, _ := summary["coverage"].(embeddedMCPMetadataCoverage) + if summary["source_revision"] != "revision-test" || coverage.SurfaceScope != "source_revision" { t.Fatalf("interface metadata provenance = %#v", summary) } diff --git a/internal/cli/schema_catalog.json b/internal/cli/schema_catalog.json index 19eb8fb2..a72fd525 100644 --- a/internal/cli/schema_catalog.json +++ b/internal/cli/schema_catalog.json @@ -1,12 +1,12 @@ { "version": 1, - "source_hash": "sha256:3e93a6610ff04533eff6758bdc71c758ed052f873a8206ccfaf35912706d581f", + "source_hash": "sha256:a2ad77f7d8c0816a0e553a5ee5306c6ca0f2139edb83207c35eefecd2411f72f", "surface_hash": "sha256:75c63f35d9b15dfb4df09136bfd250906751cfeb7f14bce08cf092097859baad", "catalog": { "agent_metadata": { "products_with_metadata": 20, "source": "embedded-skill-metadata", - "source_hash": "sha256:b9fd6c4de190c413312561049ed2bbb80b272aad127ba2c0a5082364761012ee", + "source_hash": "sha256:b0788dd79d817258e2fd5869e4b32eff5ab81b6d20f34e7eb131446670402003", "surface_hash": "sha256:75c63f35d9b15dfb4df09136bfd250906751cfeb7f14bce08cf092097859baad", "surface_products": 20, "surface_tools": 538, @@ -26,6 +26,7 @@ "snapshot_services": 53, "source_services": 54, "source_tools": 969, + "surface_scope": "source_revision", "surface_tools": 504, "unmatched_tools": 85 }, diff --git a/internal/cli/schema_completeness.go b/internal/cli/schema_completeness.go index 1c935ffe..72399645 100644 --- a/internal/cli/schema_completeness.go +++ b/internal/cli/schema_completeness.go @@ -37,7 +37,8 @@ type runtimeSchemaExclusionGroup struct { var embeddedRuntimeSchemaExclusionsJSON []byte // RuntimeSchemaCompletenessReport compares the public executable Cobra leaves -// with the commands represented by runtime Schema annotations. +// with a reviewed Schema command set, such as runtime annotations or the final +// generated Catalog. type RuntimeSchemaCompletenessReport struct { Covered []string Excluded []string @@ -100,10 +101,70 @@ func ValidateEmbeddedRuntimeSchemaCompleteness(root *cobra.Command) error { return nil } +// ValidateSchemaCatalogDeliveryCompleteness enforces that every public Cobra +// leaf is queryable from the newly built Catalog snapshot (or has an exact, +// reviewed exclusion). It deliberately validates the in-memory build result, +// never the previously embedded Catalog, so stale generated data cannot prove +// its own completeness. +func ValidateSchemaCatalogDeliveryCompleteness(root *cobra.Command, snapshot SchemaCatalogSnapshot) error { + if _, err := ApplyEmbeddedManualSchemaHints(root); err != nil { + return err + } + exclusions, err := EmbeddedRuntimeSchemaExclusions() + if err != nil { + return err + } + report := SchemaCatalogDeliveryCompleteness(root, snapshot, exclusions) + if len(report.Missing) > 0 { + return fmt.Errorf("public Cobra leaves missing from final Schema Catalog or reviewed exclusions: %s", strings.Join(report.Missing, ", ")) + } + if len(report.InvalidExclusions) > 0 { + return fmt.Errorf("invalid runtime schema exclusions: %s", strings.Join(report.InvalidExclusions, ", ")) + } + if len(report.StaleExclusions) > 0 { + return fmt.Errorf("stale runtime schema exclusions: %s", strings.Join(report.StaleExclusions, ", ")) + } + return nil +} + // RuntimeSchemaCompleteness scans the real command tree in the reverse // direction: every public executable leaf must either carry a Schema identity // or have a reviewed exclusion with a non-empty reason. func RuntimeSchemaCompleteness(root *cobra.Command, exclusions []RuntimeSchemaExclusion) RuntimeSchemaCompletenessReport { + coveredPaths := map[string]bool{} + for _, entry := range collectRuntimeSchemaEntries(root) { + addSchemaCoveredPath(coveredPaths, entry.CLIPath) + addSchemaCoveredPath(coveredPaths, entry.PrimaryCLIPath) + for _, alias := range entry.Aliases { + addSchemaCoveredPath(coveredPaths, alias) + } + } + return runtimeSchemaCompletenessAgainstPaths(root, exclusions, coveredPaths) +} + +// SchemaCatalogDeliveryCompleteness scans the real command tree against paths +// present in the final generated snapshot. A runtime annotation alone is not +// sufficient: the command must survive the reviewed surface filter and be +// addressable through its primary CLI path or an alias. +func SchemaCatalogDeliveryCompleteness(root *cobra.Command, snapshot SchemaCatalogSnapshot, exclusions []RuntimeSchemaExclusion) RuntimeSchemaCompletenessReport { + coveredPaths := map[string]bool{} + for _, detail := range snapshot.Tools { + addSchemaCoveredPath(coveredPaths, schemaString(detail["cli_path"])) + addSchemaCoveredPath(coveredPaths, schemaString(detail["primary_cli_path"])) + for _, alias := range schemaStringSlice(detail["aliases"]) { + addSchemaCoveredPath(coveredPaths, alias) + } + } + return runtimeSchemaCompletenessAgainstPaths(root, exclusions, coveredPaths) +} + +func addSchemaCoveredPath(coveredPaths map[string]bool, rawPath string) { + if path := normalizeSchemaCLIPath(rawPath); path != "" { + coveredPaths[path] = true + } +} + +func runtimeSchemaCompletenessAgainstPaths(root *cobra.Command, exclusions []RuntimeSchemaExclusion, coveredPaths map[string]bool) RuntimeSchemaCompletenessReport { report := RuntimeSchemaCompletenessReport{} exclusionByPath := make(map[string]RuntimeSchemaExclusion, len(exclusions)) for _, exclusion := range exclusions { @@ -118,14 +179,6 @@ func RuntimeSchemaCompleteness(root *cobra.Command, exclusions []RuntimeSchemaEx seenPublic := map[string]bool{} usedExclusions := map[string]bool{} - coveredPaths := map[string]bool{} - for _, entry := range collectRuntimeSchemaEntries(root) { - coveredPaths[normalizeSchemaCLIPath(entry.CLIPath)] = true - coveredPaths[normalizeSchemaCLIPath(entry.PrimaryCLIPath)] = true - for _, alias := range entry.Aliases { - coveredPaths[normalizeSchemaCLIPath(alias)] = true - } - } walkPublicRunnableLeaves(root, func(leaf *cobra.Command) { path := normalizeSchemaCLIPath(strings.Join(commandPathParts(leaf), " ")) if path == "" { diff --git a/internal/cli/schema_completeness_test.go b/internal/cli/schema_completeness_test.go index 249487f0..b62afd7e 100644 --- a/internal/cli/schema_completeness_test.go +++ b/internal/cli/schema_completeness_test.go @@ -57,3 +57,73 @@ func TestRuntimeSchemaCompletenessRejectsInvalidAndStaleExclusions(t *testing.T) t.Fatalf("missing = %v", report.Missing) } } + +func TestSchemaCatalogDeliveryCompletenessRejectsAnnotatedLeafOutsideSurface(t *testing.T) { + root := &cobra.Command{Use: "dws"} + product := &cobra.Command{Use: "sample"} + covered := &cobra.Command{Use: "covered", Run: func(*cobra.Command, []string) {}} + omitted := &cobra.Command{Use: "omitted", Run: func(*cobra.Command, []string) {}} + AttachRuntimeSchema(covered, "sample", "covered", "test") + AttachRuntimeSchema(omitted, "sample", "omitted", "test") + product.AddCommand(covered, omitted) + root.AddCommand(product) + + annotationReport := RuntimeSchemaCompleteness(root, nil) + if !reflect.DeepEqual(annotationReport.Covered, []string{"sample covered", "sample omitted"}) || len(annotationReport.Missing) != 0 { + t.Fatalf("annotation report = %#v", annotationReport) + } + + snapshot, err := BuildSchemaCatalogSnapshot(root, SchemaCatalogBuildOptions{ + AllowedCanonicalPaths: map[string]bool{"sample.covered": true}, + }) + if err != nil { + t.Fatalf("BuildSchemaCatalogSnapshot: %v", err) + } + report := SchemaCatalogDeliveryCompleteness(root, snapshot, nil) + if !reflect.DeepEqual(report.Covered, []string{"sample covered"}) { + t.Fatalf("covered = %v", report.Covered) + } + if !reflect.DeepEqual(report.Missing, []string{"sample omitted"}) { + t.Fatalf("missing = %v", report.Missing) + } +} + +func TestSchemaCatalogDeliveryCompletenessAcceptsExactReviewedExclusion(t *testing.T) { + root := &cobra.Command{Use: "dws"} + product := &cobra.Command{Use: "sample"} + covered := &cobra.Command{Use: "covered", Run: func(*cobra.Command, []string) {}} + excluded := &cobra.Command{Use: "excluded", Run: func(*cobra.Command, []string) {}} + product.AddCommand(covered, excluded) + root.AddCommand(product) + + snapshot := SchemaCatalogSnapshot{Tools: map[string]map[string]any{ + "sample.covered": {"primary_cli_path": "sample covered"}, + }} + report := SchemaCatalogDeliveryCompleteness(root, snapshot, []RuntimeSchemaExclusion{{ + CLIPath: "sample excluded", Reason: "reviewed test exclusion", Reviewed: true, + }}) + if !reflect.DeepEqual(report.Covered, []string{"sample covered"}) || + !reflect.DeepEqual(report.Excluded, []string{"sample excluded"}) || + len(report.Missing) != 0 || len(report.StaleExclusions) != 0 { + t.Fatalf("delivery report = %#v", report) + } +} + +func TestSchemaCatalogDeliveryCompletenessAcceptsDeliveredAlias(t *testing.T) { + root := &cobra.Command{Use: "dws"} + product := &cobra.Command{Use: "sample"} + legacy := &cobra.Command{Use: "legacy", Run: func(*cobra.Command, []string) {}} + product.AddCommand(legacy) + root.AddCommand(product) + + snapshot := SchemaCatalogSnapshot{Tools: map[string]map[string]any{ + "sample.current": { + "primary_cli_path": "sample current", + "aliases": []string{"sample legacy"}, + }, + }} + report := SchemaCatalogDeliveryCompleteness(root, snapshot, nil) + if !reflect.DeepEqual(report.Covered, []string{"sample legacy"}) || len(report.Missing) != 0 { + t.Fatalf("delivery report = %#v", report) + } +} diff --git a/internal/cli/schema_mcp_metadata.json b/internal/cli/schema_mcp_metadata.json index 6659fd52..a11eea94 100644 --- a/internal/cli/schema_mcp_metadata.json +++ b/internal/cli/schema_mcp_metadata.json @@ -4,6 +4,7 @@ "source_revision": "4574f7022c32cf4c033e9b7b4156e2fec815fed8", "source_hash": "sha256:17251f74a4f76142457cc0e87251ecb86c1e3bda0cdf2edc02f351589716ecbc", "coverage": { + "surface_scope": "source_revision", "source_services": 54, "snapshot_services": 53, "missing_services": [ diff --git a/internal/cli/schema_parameter_bindings.json b/internal/cli/schema_parameter_bindings.json index ccca6155..5590472a 100644 --- a/internal/cli/schema_parameter_bindings.json +++ b/internal/cli/schema_parameter_bindings.json @@ -1,6 +1,6 @@ { "version": 2, - "source_catalog_hash": "sha256:3e93a6610ff04533eff6758bdc71c758ed052f873a8206ccfaf35912706d581f", + "source_catalog_hash": "sha256:a2ad77f7d8c0816a0e553a5ee5306c6ca0f2139edb83207c35eefecd2411f72f", "historical_binding_count": 311, "migrations": { "contact.get_dept_info_by_dept_id --id": "contact.get_dept_info_by_dept_id --dept", diff --git a/internal/generator/cmd_schema_catalog/main.go b/internal/generator/cmd_schema_catalog/main.go index 3c057420..83cb594d 100644 --- a/internal/generator/cmd_schema_catalog/main.go +++ b/internal/generator/cmd_schema_catalog/main.go @@ -67,6 +67,9 @@ func main() { if err != nil { fail(err) } + if err := cli.ValidateSchemaCatalogDeliveryCompleteness(root, snapshot); err != nil { + fail(fmt.Errorf("validate final Catalog delivery completeness: %w", err)) + } encoded, err := json.MarshalIndent(snapshot, "", " ") if err != nil { fail(fmt.Errorf("encode catalog: %w", err)) diff --git a/scripts/policy/check-schema-catalog.sh b/scripts/policy/check-schema-catalog.sh index 9011c819..28bd1218 100755 --- a/scripts/policy/check-schema-catalog.sh +++ b/scripts/policy/check-schema-catalog.sh @@ -18,7 +18,7 @@ fi surface_count="$(wc -l <"$tmp/surface" | tr -d ' ')" catalog_count="$(jq -r '.tools | length' internal/cli/schema_catalog.json)" catalog_product_count="$(jq -r '.catalog.count' internal/cli/schema_catalog.json)" -interface_surface_count="$(jq -r '.coverage.surface_tools' internal/cli/schema_mcp_metadata.json)" +mcp_snapshot_surface_count="$(jq -r '.coverage.surface_tools' internal/cli/schema_mcp_metadata.json)" agent_surface_count="$(jq -r '.coverage.surface_tools' internal/cli/schema_agent_metadata/index.json)" agent_product_count="$(jq -r '.coverage.products_with_metadata' internal/cli/schema_agent_metadata/index.json)" agent_selection_count="$(jq -r '[.coverage.tools_with_use_when, .coverage.tools_with_avoid_when, .coverage.tools_with_examples, .coverage.tools_with_interface_mode] | min' internal/cli/schema_agent_metadata/index.json)" @@ -26,9 +26,43 @@ if [ "$catalog_count" != "$surface_count" ] || [ "$agent_surface_count" != "$surface_count" ] || [ "$agent_product_count" != "$catalog_product_count" ] || [ "$agent_selection_count" != "$surface_count" ]; then - printf 'schema counts disagree: surface=%s catalog=%s products=%s interface=%s agent=%s/%s\n' \ + printf 'current schema counts disagree: surface=%s catalog=%s products=%s agent=%s/%s\n' \ "$surface_count" "$catalog_count" "$catalog_product_count" \ - "$interface_surface_count" "$agent_product_count" "$agent_surface_count" >&2 + "$agent_product_count" "$agent_surface_count" >&2 + exit 1 +fi + +if ! jq -e ' + .version == 1 and + ((.source_revision // "") | length) > 0 and + .coverage.surface_scope == "source_revision" and + .coverage.source_services == (.coverage.snapshot_services + (.coverage.missing_services | length)) and + .coverage.surface_tools == (.coverage.matched_tools + .coverage.unmatched_tools) and + .coverage.source_tools >= .coverage.surface_tools and + .coverage.matched_tools == (.tools | length) and + .coverage.aliased_tools <= .coverage.matched_tools +' internal/cli/schema_mcp_metadata.json >/dev/null; then + printf 'MCP source-revision snapshot coverage is inconsistent: snapshot_surface=%s\n' \ + "$mcp_snapshot_surface_count" >&2 + exit 1 +fi + +if ! jq -e ' + .coverage.source_tools == (.tools | length) and + .coverage.eligible_tools == (.tools | length) and + .coverage.matched_tools == (.tools | length) and + .coverage.unmatched_tools == 0 +' skills/mono/schema-hints/selection-review.json >/dev/null; then + printf '%s\n' 'reviewed Agent selection coverage is stale' >&2 + exit 1 +fi + +if ! jq -e --arg surface_count "$surface_count" ' + .coverage.source_tools == ($surface_count | tonumber) and + .coverage.matched_tools == (.tools | length) and + all(.tools[]; ((.interface_mode // "") | length) > 0) +' skills/mono/schema-hints/runtime-surface-completeness.json >/dev/null; then + printf '%s\n' 'runtime-surface interface disposition coverage is stale' >&2 exit 1 fi @@ -61,6 +95,8 @@ if ! jq -e --arg surface_count "$surface_count" ' (if .interface_mode == "mcp" then .interface_ref != null and .availability == "available" else true end) and (if .interface_mode == "composite" or .interface_mode == "local" then .interface_ref == null and .availability == "available" and ((.interface_reason // "") | length) > 0 else true end) and (if .interface_mode == "unavailable" then .interface_ref == null and .availability == "unavailable" and ((.interface_reason // "") | length) > 0 else true end) and + ((((.agent_source_refs // []) | index("skills/mono/schema-hints/selection-review.json")) != null) or + (((.agent_source_refs // []) | index("skills/mono/schema-hints/runtime-surface-completeness.json")) != null)) and (if .effect == "destructive" then .risk == "high" and .confirmation == "user_required" else true end) and (if .risk == "high" then .confirmation == "user_required" else true end) ) @@ -162,7 +198,7 @@ if rg -n '\.ListTools\(' internal/app internal/cli --glob '*.go'; then fi go test ./internal/cli \ - -run '^(TestEmbeddedSchemaCatalog.*|TestSchemaUsesEmbeddedCatalogWithoutRuntimeLoad|TestWalkLeafCommandsTraversesAnnotatedHiddenSubtree)$' \ + -run '^(TestEmbeddedSchemaCatalog.*|TestSchemaCatalogDeliveryCompleteness.*|TestSchemaUsesEmbeddedCatalogWithoutRuntimeLoad|TestWalkLeafCommandsTraversesAnnotatedHiddenSubtree)$' \ -count=1 go test ./internal/app \ -run '^(TestEmbeddedSchemaContractMapsToExecutableTree|TestRuntimeSchemaCompletenessCoversPublicCommandTree|TestRegisterPluginHTTPServerDoesNotProbeEndpoint|TestRegisterStdioServerFromManifestDoesNotStartProcess)$' \ diff --git a/skills/mono/schema-hints/README.md b/skills/mono/schema-hints/README.md index b7abd59b..547eedb4 100644 --- a/skills/mono/schema-hints/README.md +++ b/skills/mono/schema-hints/README.md @@ -52,6 +52,16 @@ The missing `notify` MCP service is separately dispositioned in `internal/cli/schema_mcp_service_review.json`; it is outside the public command surface and must not trigger runtime discovery. +`internal/cli/schema_mcp_metadata.json.coverage.surface_tools` describes only +the immutable MCP import at its declared `source_revision`; it is not the +current CLI/Catalog tool count. Its `coverage.surface_scope` must remain +`source_revision`, and policy verifies the snapshot's internal matched and +unmatched arithmetic. Current Catalog interface coverage is instead proved for +every generated tool: each tool must have one valid `interface_mode` / +availability disposition and retain provenance to `selection-review.json` or +`runtime-surface-completeness.json`. This makes newly added CLI tools explicit +without rewriting historical MCP evidence. + Interface metadata may enrich type and description, but MCP `required` never promotes an optional Cobra flag. CLI required/one-of/conditional rules must be represented by current Cobra markers, typed runtime constraints, or reviewed parameter hints. ```json diff --git a/skills/mono/schema-hints/selection-review.json b/skills/mono/schema-hints/selection-review.json index 81786ee9..7be71e84 100644 --- a/skills/mono/schema-hints/selection-review.json +++ b/skills/mono/schema-hints/selection-review.json @@ -7,9 +7,9 @@ "channel": "open-source" }, "coverage": { - "source_tools": 504, - "eligible_tools": 504, - "matched_tools": 504, + "source_tools": 505, + "eligible_tools": 505, + "matched_tools": 505, "unmatched_tools": 0 }, "tools": {