[Schema Consistency] Schema Consistency Check – 2026-03-27 #23209
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Schema Consistency Checker. A newer discussion is available at Discussion #23314. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
RuntimesConfigstruct fields againstruntimesConfigToMap()output, then compared all schema top-level property names againstFrontmatterConfigJSON struct tags using shell diffCritical Issues
HIGH:
runtimesConfigToMap()Silently Drops 5 of 11 Runtimespkg/workflow/frontmatter_types.go:677–763—runtimesConfigToMap()only serializes 6 runtimes butRuntimesConfigdefines 11.Runtimes in struct but NOT emitted:
dotnet(line 31),elixir(line 32),haskell(line 33),java(line 34),ruby(line 35)All five are valid runtimes confirmed in:
main_workflow_schema.json(patternProperties, examples at lines 3834–3845 and 3956+)runtime_definitions.go(lines 55, 65, 91, 101, 137)RuntimesConfigstruct (frontmatter_types.go:31–35)The bug manifests when
RuntimesTypedis populated (i.e.ParseFrontmatterConfig()succeeds):ToMap()at line 561 callsruntimesConfigToMap(fc.RuntimesTyped)which discards those 5 runtimes. IfRuntimesTypedis nil the rawRuntimesmap is used instead, which preserves all keys correctly.countRuntimes()(lines 467–491) has the same gap — it counts only node, python, go, uv, bun, deno (6 of 11).Impact: Any code that calls
FrontmatterConfig.ToMap()on a workflow with dotnet/elixir/haskell/java/ruby runtime overrides will silently lose those overrides. The main compiler path (runtime_detection.go:29) usesworkflowData.Runtimes(rawmap[string]any) directly, so compiled workflows are unaffected today — but this is a silent correctness trap for any new code that depends onToMap().MEDIUM:
runtimesConfigToMap()Also Dropsaction-repoandaction-versionfor All RuntimesRuntimeConfigstruct definesActionRepoandActionVersion(frontmatter_types.go:17–18) andparseRuntimesConfig()correctly extracts them (lines 305–306). ButruntimesConfigToMap()only emitsversionandif—action-repoandaction-versionare never written back.These fields are schema-documented and fully functional at compile time (runtime_overrides.go:40–42 reads them from the raw map). The
frontmatter.mddocs show an example (lines 152–221) withaction-repoandaction-version. So the only gap is the round-trip throughruntimesConfigToMap().No test in
frontmatter_types_test.gocoversaction-repo/action-versionround-trip — the existingTestRuntimesConfigToMapWithIfConditiononly checksversionandif.Confirmed Persisting Issues
CONFIRMED:
status-commentSchema Description Still Stale (Strategy-30, -35)main_workflow_schema.jsonstill says:But
compiler_safe_outputs.go:192–195still auto-enablesStatusComment = trueforslash_command/label_commandtriggers when not explicitly set.Two pending changesets will change this behavior:
minor-decouple-status-comment.md(major breaking change): removes auto-enable entirelyminor-enable-reaction-status-comment-by-default.md(minor): re-enables auto-default for both triggersUntil those are released the schema description is wrong. After release the schema description will need to be updated again to reflect the new default-on behavior. The description should be updated alongside whichever changeset ships last.
MEDIUM:
FrontmatterConfig.ToMap()Omits Five Schema FieldsThe following fields are defined in the
FrontmatterConfigstruct and in the schema but are never emitted byToMap()(frontmatter_types.go:519–675):Private(*bool)RateLimit(*RateLimitConfig)Observability(*ObservabilityConfig)InlinedImports(bool)Resources([]string)Current impact is low:
FrontmatterConfig.ToMap()is not called in the main compiler path —WorkflowDatauses rawmap[string]anyfields populated directly. However this is a correctness bug that would silently drop real user configuration for any code that depends onToMap().Additionally,
includeis aFrontmatterConfigfield (line 189) and emitted byToMap()(line 662) but is absent from the schema — either it is a legacy/internal field or it needs a schema entry. Similarly,version(line 146) is in the struct but not in the schema.Recommendations
Fix
runtimesConfigToMap()immediately — add serialization blocks for dotnet, elixir, haskell, java, ruby (mirroring the existing node/python/go/uv/bun/deno pattern), and addaction-repo/action-versionemission to all runtime blocks. Add a round-trip test.Fix
countRuntimes()in parallel — add the 5 missing runtime checks (impact: log messages undercount runtimes).Update
status-commentschema description when theminor-decouple-status-comment+minor-enable-reaction-status-comment-by-defaultchangesets are released. Suggested description: "Whether to post status comments (started/completed) on the triggering item. Defaults to true for slash_command and label_command triggers. Set to false to disable."Add ToMap() completeness test — a table-driven test that round-trips all
FrontmatterConfigfields throughParseFrontmatterConfig→ToMap()and asserts no fields are silently dropped.Clarify
includeandversionin schema — add schema entries if they are valid frontmatter fields, or addadditionalProperties: falseenforcement with a comment explaining why they're struct-only.Schema Analysis Method
Strategy Performance
Next Steps
runtimesConfigToMap()— add dotnet/elixir/haskell/java/ruby + action-repo/action-versioncountRuntimes()— add the 5 missing runtime countersruntimesConfigToMapcovering all 11 runtimes and all RuntimeConfig fieldsToMap()completeness regression test forFrontmatterConfigstatus-commentschema description when decouple changeset shipsincludeandversiontop-level fields — add or intentionally excludeReferences:
Beta Was this translation helpful? Give feedback.
All reactions