fix: correct out-of-date atmos-manifest workflow schema (#2708)#2710
Conversation
) The published atmos-manifest JSON Schema modeled only 21 of ~96 authored workflow_step fields and 3 of 10 workflow_manifest fields under additionalProperties:false, so editors using the published $schema flagged valid 1.222.0 workflows (e.g. dependencies.tools, interactive choose/input steps). - Add all missing workflow_step (75) and workflow_manifest (7) fields with descriptions. - Set workflow_step additionalProperties:true (lenient, polymorphic step union) and make level a plain string. - Add shared workflow_viewport/workflow_show/workflow_container definitions. - Add a reflection ratchet test to prevent future drift, plus regression tests for the issue's snippets and every step-type family.
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
|
Intended semver label: |
📝 WalkthroughWalkthroughUpdates the atmos-manifest JSON schema to add missing workflow_manifest and workflow_step properties (dependencies, env, container, output, viewport, show, options, default, etc.), relaxes workflow_step additionalProperties, and adds Go tests enforcing schema coverage and validation regressions. ChangesWorkflow schema catch-up
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (2)
1885-2481: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStep-field expansion aligns well with
WorkflowStep; a few enumerable fields could useenum.The new
workflow_stepproperties (options,default,placeholder,source,viewport,backgroundoneOf string/boolean,foroneOf string/array, etc.) match the Go struct shapes shown inpkg/schema/workflow.go, andadditionalProperties: truecorrectly matches the PR's intent to stop rejecting unmodeled/future keys. A couple of fields document an exact value set in their description but aren't constrained withenum, unlike sibling definitions elsewhere in this same file (backend_type,output.mode,fail.mode):♻️ Example: tighten `method` and `print`
"method": { "type": "string", - "description": "HTTP method for the http step type: GET (default), POST, PUT, PATCH, DELETE, HEAD, or OPTIONS." + "description": "HTTP method for the http step type: GET (default), POST, PUT, PATCH, DELETE, HEAD, or OPTIONS.", + "enum": ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"] }, ... "print": { "type": "string", - "description": "Print policy for the say step type: fallback (default), always, or never." + "description": "Print policy for the say step type: fallback (default), always, or never.", + "enum": ["fallback", "always", "never"] },Since
additionalProperties: truealready prevents rejection of unknown keys, addingenumto already-typed fields only tightens value validation — it won't reintroduce the false-positive class this PR fixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json` around lines 1885 - 2481, The new workflow_step schema in the `workflow_step` definition should be tightened by adding enum constraints to fields whose descriptions already specify a fixed set of values, such as `method`, `print`, and any similar scalar fields in this block. Update the corresponding properties in `workflow_step` to use `enum` while keeping `additionalProperties: true` unchanged so unknown keys remain accepted, and verify the constrained values stay aligned with the behavior modeled by `WorkflowStep` and the other schema definitions like `output.mode` and `fail.mode`.
1736-1884: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNew shared definitions look correct; consider
enumfor small fixed-vocabulary fields.
workflow_viewport,workflow_show, andworkflow_containercorrectly mirror the tri-state/boolean and container-config shapes described. One improvement opportunity:workflow_container.pulldocuments an exact value set ("always, missing, or never") but is typed as a bare string — anenumhere would catch typos without reintroducing the false-positive problem this PR is fixing (enums only constrain the value of an already-declared key, they don't reject unknown keys).♻️ Example: tighten `pull`
"pull": { "type": "string", - "description": "Image pull policy: always, missing, or never." + "description": "Image pull policy: always, missing, or never.", + "enum": ["always", "missing", "never"] },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json` around lines 1736 - 1884, The shared schema definitions for workflow settings look good, but the workflow_container.pull field is still too loose because it is a plain string even though only a small fixed set of values is valid. Update workflow_container in the manifest schema to use an enum for pull, keeping the existing oneOf/include/object structure intact, and leave the other fields like workflow_viewport, workflow_show, and the container object unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json`:
- Around line 1885-2481: The new workflow_step schema in the `workflow_step`
definition should be tightened by adding enum constraints to fields whose
descriptions already specify a fixed set of values, such as `method`, `print`,
and any similar scalar fields in this block. Update the corresponding properties
in `workflow_step` to use `enum` while keeping `additionalProperties: true`
unchanged so unknown keys remain accepted, and verify the constrained values
stay aligned with the behavior modeled by `WorkflowStep` and the other schema
definitions like `output.mode` and `fail.mode`.
- Around line 1736-1884: The shared schema definitions for workflow settings
look good, but the workflow_container.pull field is still too loose because it
is a plain string even though only a small fixed set of values is valid. Update
workflow_container in the manifest schema to use an enum for pull, keeping the
existing oneOf/include/object structure intact, and leave the other fields like
workflow_viewport, workflow_show, and the container object unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 848b861a-a283-4473-ada7-d8966dc9d1c4
📒 Files selected for processing (3)
pkg/datafetcher/schema_workflow_coverage_test.gopkg/datafetcher/schema_workflow_validation_test.gowebsite/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.223.0-rc.7. |
2 participants
what
atmos-manifestJSON Schema (served atatmos.toolsand mirrored to SchemaStore) back in sync with the workflow features shipped in1.222.0, so editors using# yaml-language-server: $schema=…stop drawing false-positive errors on valid workflows.workflow_manifest(the object under each named workflow): add the 7 missing workflow-definition fields —dependencies,working_directory,env,container,output,viewport,show(reusing the existingdependenciesdefinition via$ref).workflow_step: add the 75 missing step fields with descriptions, spanning every step-type family (options/default/interactive/tty/script, plushttp,cast,container,style,log,say,env,exit,emulator,junit,require,workdir,table, control/parallel). SetadditionalProperties: trueand makelevela plain string (see why).workflow_viewport,workflow_show,workflow_container(the latter also acceptsfalseto run on the host).schema_workflow_coverage_test.go) that fails the build if aWorkflowStep/WorkflowDefinitionfield is ever authored without a matching schema property — the same "you can't forget the schema" pattern already used for top-level/component sections.schema_workflow_validation_test.go) reproducing the issue's exact snippets, covering one field from every step-type family, all workflow-level fields, and confirming typed fields (e.g.options,code) are still validated.why
dependencies,options,default), but the schema had drifted far more broadly: it modeled only 21 of ~96 authored step fields and 3 of 10 workflow-definition fields, both withadditionalProperties: false. Any workflow using a newer field produced misleading "Property X is not allowed" red squiggles on config that Atmos parses and runs correctly.level:values outside a log-level set. Workflow steps are a lenient, polymorphic union of ~30 step types and Atmos ignores unknown keys at runtime, soadditionalProperties: true(plus fully enumerated known fields for autocomplete + type checks) matches real behavior and eliminates the whole false-positive class — present and future — mirroring howproviders/templatesare modeled.levelis a plain string field, so it is no longer constrained by an enum.atmos validate stacksis unchanged.references
examples/quick-start-advanced/stacks/workflows/{backend,validation}.yaml(and the mirrored doc) usecommand: true— an unquoted-YAML-boolean footgun where a string command belongs; the schema correctly flags it.commands:) and profile files (auth:) — the issue's "ideally" ask; purely additive (no false-positives today) and a larger separate effort.Summary by CodeRabbit
Bug Fixes
Tests