From 1e7e6ded35165fa553b63849041795788324a0cd Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Sat, 11 Jul 2026 17:41:03 -0400 Subject: [PATCH] refactor(config): rename parse-config to lint and reject unknown keys Swap the parse-config command to cascade lint (no external consumers to protect) and make top-level manifest parsing strict: an unknown or misspelled config key is now a hard error carrying a did-you-mean suggestion, rather than being silently dropped by the lenient decoder. Strictness mirrors the proven per-component catch-all one level up: an inline Extra map on TrunkConfig captures unmodeled keys, which a validation walk rejects against the reflected known-field set with a Levenshtein-nearest suggestion. Legitimately open maps stay open. The four generated lanes (validate_check, merge_queue, pr_preview, hotfix check-job), the two e2e scenarios, cascade's own build-cli smoke test, the byte-identical golden baseline, the schema description copies, and the CLI and manifest docs all move to lint. lint keeps the JSON validity surface the lanes gate on behind --json. Stale project: fixtures and the typed-remarshal note are updated for the new strictness. Verification: go build/test/-race and golangci-lint clean on the root and e2e modules; new regression tests fail before and pass after. Signed-off-by: Joshua Temple --- .github/workflows/build-cli.yaml | 2 +- CONTRIBUTING.md | 2 +- cmd/cascade/main_test.go | 14 +-- docs/public/manifest.schema.json | 2 +- docs/src/content/docs/guides/companions.md | 2 +- .../content/docs/internals/coverage-matrix.md | 2 +- docs/src/content/docs/reference/cli.md | 26 ++-- docs/src/content/docs/reference/manifest.md | 14 +-- docs/src/content/docs/reference/versioning.md | 2 +- e2e/scenarios/14-validate-check.yaml | 2 +- e2e/scenarios/15-merge-queue.yaml | 4 +- e2e/scenarios/58-lint-strict-rejection.yaml | 73 +++++++++++ internal/config/command.go | 71 +++++++---- internal/config/parse.go | 4 + internal/config/schema_v1_e2e_test.go | 2 +- internal/config/schema_version_e2e_test.go | 2 +- internal/config/strict.go | 119 ++++++++++++++++++ internal/config/strict_test.go | 87 +++++++++++++ internal/config/types.go | 9 +- internal/generate/command_test.go | 1 - internal/generate/hotfix.go | 4 +- internal/generate/hotfix_test.go | 4 +- internal/generate/merge_queue.go | 10 +- internal/generate/merge_queue_test.go | 2 +- internal/generate/pr_preview.go | 4 +- internal/generate/pr_preview_test.go | 5 +- ...hub__workflows__cascade-hotfix.yaml.golden | 2 +- internal/generate/validate_check.go | 11 +- internal/generate/validate_check_test.go | 8 +- internal/orchestrate/nochange_skip_test.go | 1 - internal/orchestrate/orchestrator_test.go | 1 - .../orchestrate/typed_remarshal_drop_test.go | 23 ++-- internal/schema/command.go | 2 +- internal/schema/manifest.schema.json | 2 +- schema/manifest.schema.json | 2 +- 35 files changed, 417 insertions(+), 104 deletions(-) create mode 100644 e2e/scenarios/58-lint-strict-rejection.yaml create mode 100644 internal/config/strict.go create mode 100644 internal/config/strict_test.go diff --git a/.github/workflows/build-cli.yaml b/.github/workflows/build-cli.yaml index 6cbcf7b1..fdec54b8 100644 --- a/.github/workflows/build-cli.yaml +++ b/.github/workflows/build-cli.yaml @@ -38,7 +38,7 @@ jobs: run: | ./cascade version ./cascade --help - ./cascade parse-config --help + ./cascade lint --help ./cascade detect-changes --help ./cascade generate-changelog --help ./cascade generate-workflow --help diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bb923fc..3234436b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,7 +53,7 @@ cascade holds to a few conventions in its own codebase and in the workflows it g - **Additive manifest changes**: new fields are always optional with sensible defaults, so existing manifest files keep working across minor version bumps. - **Path fields reach every path sink**: a manifest field that widens which files a component reacts to must thread through all three places a path is consumed, or it is a silent bug. The emitted `on: push` paths filter fires the workflow, per-callback change detection decides which builds and deploys run, and the version commit range decides the bump. A field that reaches only some of these triggers a run that then no-ops, or bumps a version whose builds skip as unchanged. When you add such a field, add a test that asserts the shared path reaches each sink. -- **A breaking generator or validation change moves with the fleet, in the same change**: a change that makes a previously valid manifest invalid, such as rejecting a field `parse-config` used to accept, is breaking even when it ships as a `fix:`. The fleet repin re-stamps every example repository onto the release-candidate binary and regenerates its workflows before any suite runs, so a manifest still carrying the now-rejected shape fails that repin, not the intended test. Before landing a validation change that can reject something that used to pass, scan the fleet example repos for that shape and migrate any that use it in the same pull request, alongside the doc's migration note. This generalizes the existing rule that a fleet suite and the eligibility logic it exercises are one coupled unit (see [Making a change](#making-a-change)); it applies to validation, not only to eligibility. +- **A breaking generator or validation change moves with the fleet, in the same change**: a change that makes a previously valid manifest invalid, such as rejecting a field `lint` used to accept, is breaking even when it ships as a `fix:`. The fleet repin re-stamps every example repository onto the release-candidate binary and regenerates its workflows before any suite runs, so a manifest still carrying the now-rejected shape fails that repin, not the intended test. Before landing a validation change that can reject something that used to pass, scan the fleet example repos for that shape and migrate any that use it in the same pull request, alongside the doc's migration note. This generalizes the existing rule that a fleet suite and the eligibility logic it exercises are one coupled unit (see [Making a change](#making-a-change)); it applies to validation, not only to eligibility. - **Every generated workflow kind carries executing coverage**: each workflow the generator emits (`orchestrate`, `promote`, `external-update`, and the `cascade-` lanes) is mapped to the e2e scenarios and fleet lanes that run it in `internal/coverage/registry.yaml`. The coverage gate derives the emitted kinds straight from the generator source and fails when an emitted kind has no registry entry, so a new generated workflow cannot ship without a scenario or lane that exercises it. When you add a generated workflow kind, add its entry pointing at the scenario or fleet lane that runs it; a referenced scenario or lane that does not exist also fails the gate. - **Assert a runtime outcome, never the script that produces it**: an `e2e/` or fleet-suite assertion for a load-bearing behavior must compare against a runtime artifact that differs when the behavior regresses: a state leaf (`state.` sha/version/ref), a job conclusion (`success`/`skipped`/`failure`), a preflight output, a tag, a release, a branch, a pull request, or a line the running job actually logged (`expect_log`). A passing assertion must be reachable only by the behavior working at run time. Never assert a behavior by grepping emitted script source for a marker that also appears in that source: a `workflow_files.contains`/`not_contains` check over a generated `.yaml` proves a string was rendered, not that the logic ran, and stays green when the runtime behavior is deleted because the marker text is still literally present in the file. As a cautionary example, the state-write retry loop was once "covered" by grepping the emitted `orchestrate.yaml` for `cascade-state-write: exhausted attempts=10`. That branch never runs on the happy path, yet the string is always present in the script, so the check was unconditionally green and a regression that replaced the whole loop with a single `git push` would have shipped green; the fix asserts the marker the running job emits (`cascade-state-write: ok attempt=1`) and proves it red-able by breaking the emission. Restrict `workflow_files` checks to behaviors whose entire effect is the generated shape (a `concurrency:` block, a `timeout-minutes:` value, a real-GitHub-only step act cannot execute) and label those scenarios generation-only in the header. When a behavior is genuinely un-runnable in act, its executing proof lives on the fleet, and a generation-only e2e cell is a ceiling that must be labeled as such, never credited as runtime coverage. The bar: a generator or behavior change adds or updates an assertion a reviewer can turn red by reverting the behavior alone, leaving the emitted string in place. - **Callback isolation**: generated workflows call your workflows via `workflow_call`, and cascade never reaches into your callback logic. diff --git a/cmd/cascade/main_test.go b/cmd/cascade/main_test.go index 3b790d72..7cbe9c55 100644 --- a/cmd/cascade/main_test.go +++ b/cmd/cascade/main_test.go @@ -78,7 +78,7 @@ func TestHelpCommand(t *testing.T) { } } -func TestParseConfigCommand(t *testing.T) { +func TestLintCommand(t *testing.T) { // Create a temporary config file tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "trunk-config.yaml") @@ -104,9 +104,9 @@ func TestParseConfigCommand(t *testing.T) { t.Fatalf("Failed to write config file: %v", err) } - stdout, stderr, err := runCLI("parse-config", "--config", configPath) + stdout, stderr, err := runCLI("lint", "--json", "--config", configPath) if err != nil { - t.Fatalf("parse-config command failed: %v\nstderr: %s", err, stderr) + t.Fatalf("lint --json command failed: %v\nstderr: %s", err, stderr) } // Verify JSON output @@ -135,7 +135,7 @@ func TestParseConfigCommand(t *testing.T) { } } -func TestParseConfigCommand_InvalidConfig(t *testing.T) { +func TestLintCommand_InvalidConfig(t *testing.T) { // Create an invalid config file tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "invalid-config.yaml") @@ -149,7 +149,7 @@ builds: [] t.Fatalf("Failed to write config file: %v", err) } - stdout, _, err := runCLI("parse-config", "--config", configPath) + stdout, _, err := runCLI("lint", "--json", "--config", configPath) // The command should succeed but report validation errors if err != nil { t.Logf("Command returned error (expected for invalid config): %v", err) @@ -170,8 +170,8 @@ builds: [] } } -func TestParseConfigCommand_FileNotFound(t *testing.T) { - stdout, _, err := runCLI("parse-config", "--config", "/nonexistent/path/config.yaml") +func TestLintCommand_FileNotFound(t *testing.T) { + stdout, _, err := runCLI("lint", "--json", "--config", "/nonexistent/path/config.yaml") // CLI returns JSON with valid=false for file errors if err != nil { diff --git a/docs/public/manifest.schema.json b/docs/public/manifest.schema.json index 33702d4b..dfbe32e2 100644 --- a/docs/public/manifest.schema.json +++ b/docs/public/manifest.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://stablekernel.github.io/cascade/manifest.schema.json", "title": "cascade manifest", - "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade parse-config remains the authority for semantic and cross-field rules.", + "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade lint remains the authority for semantic and cross-field rules.", "type": "object", "additionalProperties": false, "properties": { diff --git a/docs/src/content/docs/guides/companions.md b/docs/src/content/docs/guides/companions.md index 044e2cc1..02698797 100644 --- a/docs/src/content/docs/guides/companions.md +++ b/docs/src/content/docs/guides/companions.md @@ -49,7 +49,7 @@ Enable it when more than one person edits the manifest, or when you want the man ## Merge queue: validate the merge-group candidate -When the repository uses GitHub's merge queue, the merge-queue companion adds a `merge_group`-triggered lane that validates the queued candidate: it runs `cascade parse-config` and a dry-run `cascade orchestrate setup` against the merge-group commit before it is allowed to land. +When the repository uses GitHub's merge queue, the merge-queue companion adds a `merge_group`-triggered lane that validates the queued candidate: it runs `cascade lint` and a dry-run `cascade orchestrate setup` against the merge-group commit before it is allowed to land. ```yaml ci: diff --git a/docs/src/content/docs/internals/coverage-matrix.md b/docs/src/content/docs/internals/coverage-matrix.md index 9f782b23..2be82c4e 100644 --- a/docs/src/content/docs/internals/coverage-matrix.md +++ b/docs/src/content/docs/internals/coverage-matrix.md @@ -165,7 +165,7 @@ not a coverage gap. | `cascade init` and scaffold | unit plus CLI | `internal/initcmd`, `internal/scaffold` | The scaffold self-checks through the real generator before any file is written | | `cascade verify` (drift) | unit plus every harness scenario | `internal/verify` | Committed workflows match manifest-regenerated bytes; drift exits non-zero | | `cascade plan` (diff preview) | unit plus CLI | `33-plan-diff`, `internal/plan` | The per-file unified diff is produced without writing files | -| `parse-config`, `schema`, `next-version`, `detect-changes`, `generate-changelog` | unit | per-package tests | Pure logic: parsing, version calculation, change detection, changelog assembly | +| `lint`, `schema`, `next-version`, `detect-changes`, `generate-changelog` | unit | per-package tests | Pure logic: parsing, version calculation, change detection, changelog assembly | | `cascade status` and `status consistency` | unit plus harness | `27-verify-orphan`, `internal/status` | State is reported and orphan env branches are flagged, and deleted on the remote with `--fix` | | `branch-protection` and `environments` emit | unit plus CLI | `internal/branchprotection`, `internal/environments` | The JSON body and env config are emitted for the operator (applying them is GitHub-side) | diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 0f19bc79..a4786bf2 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -33,7 +33,7 @@ These flags are available on every command. Most commands (`generate-workflow`, `verify`, `plan`, `graph`, `status`, and the promotion lifecycle commands) auto-detect the manifest at `.github/manifest.yaml` when -`--config` is not given. Two commands are the exception: `parse-config` and +`--config` is not given. Two commands are the exception: `lint` and `detect-changes` default `--config` to the literal path `cicd-config.yaml` and do NOT auto-detect. Pass `--config` explicitly to point either one at a different file. @@ -42,7 +42,7 @@ auto-detect. Pass `--config` explicitly to point either one at a different file. Commands are grouped by how often you reach for them: - **Everyday**: `version`, `init`, `generate-workflow`, `verify`, `plan`, `status`, `graph` -- **Preview and inspect**: `simulate`, `parse-config`, `detect-changes` +- **Preview and inspect**: `simulate`, `lint`, `detect-changes` - **Promotion lifecycle**: `orchestrate`, `promote`, `hotfix`, `rollback` - **Releases and versioning**: `next-version`, `generate-changelog`, `manage-release` - **Multi-repo**: `external` @@ -414,14 +414,20 @@ Subcommand-specific flags: `promote` takes `--mode` (`default` or `cascade`) and `--target`; `rollback` takes `--env` (required), `--to`, and `--deployable`; `hotfix` takes `--env` (required), `--fix`, and `--merge-sha`; `release` takes only the shared flags. -### parse-config +### lint -Parse and validate the manifest and print it as JSON. This command defaults `--config` to -the literal `cicd-config.yaml` path and does NOT auto-detect `.github/manifest.yaml`; pass +Parse a manifest, validate it, and report any errors or warnings. Unknown or misspelled +keys are rejected with a "did you mean" suggestion, so a typo or a field pasted at the wrong +level is a hard error rather than a silently ignored line. By default the report is +human-readable and the command exits non-zero when the manifest is invalid; pass `--json` to +emit the parsed config and validation result (`valid`/`errors`/`warnings`) as JSON, the +surface the generated validation lanes consume. This command defaults `--config` to the +literal `cicd-config.yaml` path and does NOT auto-detect `.github/manifest.yaml`; pass `--config` to point it at another file. ```bash -cascade parse-config --config .github/manifest.yaml +cascade lint --config .github/manifest.yaml +cascade lint --json --config .github/manifest.yaml ``` #### Flags @@ -429,12 +435,12 @@ cascade parse-config --config .github/manifest.yaml | Flag | Type | Default | Description | |------|------|---------|-------------| | `--config`, `-c` | string | `cicd-config.yaml` | Path to the manifest file (no auto-detection) | -| `--output`, `-o` | string | `json` | Output format (`json`) | +| `--json` | bool | `false` | Emit the parsed config and validation result as JSON | ### detect-changes Determine which builds and deploys are triggered by file changes between two commits. Like -`parse-config`, this command defaults `--config` to the literal `cicd-config.yaml` path and +`lint`, this command defaults `--config` to the literal `cicd-config.yaml` path and does NOT auto-detect `.github/manifest.yaml`. ```bash @@ -1032,7 +1038,7 @@ cascade schema --output manifest.schema.json | `--output`, `-o` | string | stdout | Write the schema to a file instead of stdout | The same schema is published at -`https://stablekernel.github.io/cascade/manifest.schema.json`. `parse-config` remains the +`https://stablekernel.github.io/cascade/manifest.schema.json`. `lint` remains the authority for semantic and cross-field rules; the schema covers structure, types, enums, and hover docs. @@ -1325,7 +1331,7 @@ Many commands accept `--json` (or `--gha-output` inside Actions) for machine-rea Raise log verbosity with the global `--trace` flag: ```bash -cascade --trace parse-config --config .github/manifest.yaml +cascade --trace lint --config .github/manifest.yaml ``` Trace logs include: diff --git a/docs/src/content/docs/reference/manifest.md b/docs/src/content/docs/reference/manifest.md index 4e299b7c..2a6aa1e1 100644 --- a/docs/src/content/docs/reference/manifest.md +++ b/docs/src/content/docs/reference/manifest.md @@ -48,7 +48,7 @@ Every field below lives under `ci.config` unless stated otherwise. The `ci.state ### Editor support -cascade ships a hand-authored JSON Schema. Registering it gives autocomplete, type checking, enum hints, and hover docs while you author the manifest. The schema covers structure, types, and enums; `cascade parse-config` remains the authority for semantic and cross-field rules. +cascade ships a hand-authored JSON Schema. Registering it gives autocomplete, type checking, enum hints, and hover docs while you author the manifest. The schema covers structure, types, and enums; `cascade lint` remains the authority for semantic and cross-field rules. The schema is published at: @@ -139,7 +139,7 @@ ci: **Relationship to `tag_prefix`.** `tag_prefix` still sets the prefix on its own when `tag_grammar` is absent. When both `tag_prefix` and `tag_grammar.prefix` are set, -`tag_grammar.prefix` wins, and `cascade parse-config` emits a non-fatal warning naming both +`tag_grammar.prefix` wins, and `cascade lint` emits a non-fatal warning naming both keys so the redundancy is visible. Resolution is well defined either way; the warning is advisory only. @@ -677,7 +677,7 @@ ci: | `workflow_run` | emitted | Wires the `workflow_run` trigger. | | `merge_group` | rejected | Not allowed. `extra_triggers` attaches to the side-effecting orchestrate workflow, which cuts release tags, publishes releases, and runs deploys while writing state, so a speculative merge-queue build could publish a real release from a candidate commit. cascade rejects `extra_triggers.merge_group` at validate. To gate pull requests inside a merge queue, set [`merge_queue.enabled`](#merge_queue), which emits a read-only validation lane. | -Migrating from a manifest that set `extra_triggers.merge_group`: remove that entry and set `merge_queue.enabled: true` instead. The read-only merge-queue lane runs `cascade parse-config` and a dry-run `cascade orchestrate setup` against the queued candidate without cutting tags, publishing releases, or writing state. +Migrating from a manifest that set `extra_triggers.merge_group`: remove that entry and set `merge_queue.enabled: true` instead. The read-only merge-queue lane runs `cascade lint` and a dry-run `cascade orchestrate setup` against the queued candidate without cutting tags, publishing releases, or writing state. ### rollback @@ -752,7 +752,7 @@ Every Deployments API step carries an `if: ${{ github.server_url == 'https://git | Field | Status | Type | Default | Description | |-------|--------|------|---------|-------------| -| `enabled` | emitted | bool | false | Emit `.github/workflows/cascade-validate.yaml`, a `pull_request` check that runs `cascade parse-config` and fails on an invalid manifest. | +| `enabled` | emitted | bool | false | Emit `.github/workflows/cascade-validate.yaml`, a `pull_request` check that runs `cascade lint` and fails on an invalid manifest. | The check validates cascade's own configuration only, requests `contents: read` alone, and has no dry-run or comment side effects. @@ -760,7 +760,7 @@ The check validates cascade's own configuration only, requests `contents: read` | Field | Status | Type | Default | Description | |-------|--------|------|---------|-------------| -| `enabled` | emitted | bool | false | Emit `.github/workflows/cascade-merge-queue.yaml`, a `merge_group`-triggered lane that runs `cascade parse-config` and a dry-run `cascade orchestrate setup` against the merge-group candidate. | +| `enabled` | emitted | bool | false | Emit `.github/workflows/cascade-merge-queue.yaml`, a `merge_group`-triggered lane that runs `cascade lint` and a dry-run `cascade orchestrate setup` against the merge-group candidate. | The lane is read-only, which is exactly what a merge queue needs: it validates the queued candidate without cutting tags, publishing releases, or writing state. This is the supported way to participate in a merge queue. Attaching the raw `merge_group` event to the side-effecting orchestrate workflow through `extra_triggers.merge_group` is rejected at validate, because a speculative merge-queue build could otherwise publish a real release from a candidate commit. @@ -870,7 +870,7 @@ just each component's `path`, byte-identical to before the fields existed. ### Validation rules -`cascade parse-config` rejects a `components:` block that breaks isolation: +`cascade lint` rejects a `components:` block that breaks isolation: - Each component must set `path` (relative, no `..`) and `tag_prefix`. - Component names must be identifier-safe. @@ -1000,7 +1000,7 @@ The implicit `release` slot tracks the most recently published (non-draft) GitHu ## Validation rules -`cascade parse-config` enforces the semantic rules the schema alone cannot: +`cascade lint` enforces the semantic rules the schema alone cannot: - `schema_version` should be `1`. Omitting it emits a warning. - Environment, build, and deploy names must be identifier-safe (letters, digits, underscores). The generator-owned names `environment` and `dry_run` are reserved and cannot be used as `dispatch_inputs`. diff --git a/docs/src/content/docs/reference/versioning.md b/docs/src/content/docs/reference/versioning.md index 3a31c6fc..ab209689 100644 --- a/docs/src/content/docs/reference/versioning.md +++ b/docs/src/content/docs/reference/versioning.md @@ -39,7 +39,7 @@ On load, the CLI applies these rules: | above `CurrentSchemaVersion` | Rejected. The manifest needs a newer CLI; upgrade the `cli_version` pin. | | negative | Rejected as invalid. | -A rejected manifest is fatal: the CLI reports the error and produces no workflows. A warning is non-fatal and surfaces on stderr and in the `warnings` field of `parse-config` JSON output. +A rejected manifest is fatal: the CLI reports the error and produces no workflows. A warning is non-fatal and surfaces on stderr and in the `warnings` field of `lint --json` output. ### Schema-version to CLI-version matrix diff --git a/e2e/scenarios/14-validate-check.yaml b/e2e/scenarios/14-validate-check.yaml index 76de3d62..aed57937 100644 --- a/e2e/scenarios/14-validate-check.yaml +++ b/e2e/scenarios/14-validate-check.yaml @@ -35,5 +35,5 @@ steps: - "paths:" - ".github/manifest.yaml" - "validate-manifest:" - - "cascade parse-config" + - "cascade lint --json" - "token: ${{ github.token }}" diff --git a/e2e/scenarios/15-merge-queue.yaml b/e2e/scenarios/15-merge-queue.yaml index 7079f5af..48443a52 100644 --- a/e2e/scenarios/15-merge-queue.yaml +++ b/e2e/scenarios/15-merge-queue.yaml @@ -8,7 +8,7 @@ description: | Generator-output verification plus a runtime proof of the gate. The positive step runs the lane on merge_group against a valid manifest and - asserts the merge-queue-validate job succeeds (parse-config .valid, then a + asserts the merge-queue-validate job succeeds (lint .valid, then a dry-run orchestrate setup). The negative step overwrites the manifest with a breaking one and asserts the same lane reds, proving a bad config cannot merge. @@ -40,7 +40,7 @@ steps: - "merge_group" - "contents: read" - "merge-queue-validate:" - - "cascade parse-config" + - "cascade lint --json" - "cascade --dry-run orchestrate setup" - "token: ${{ github.token }}" diff --git a/e2e/scenarios/58-lint-strict-rejection.yaml b/e2e/scenarios/58-lint-strict-rejection.yaml new file mode 100644 index 00000000..3c644650 --- /dev/null +++ b/e2e/scenarios/58-lint-strict-rejection.yaml @@ -0,0 +1,73 @@ +name: "Lint Strict Rejection" +description: | + Verifies that the renamed cascade lint command rejects an unknown or + misspelled top-level manifest key at runtime, and that the merge_queue lane + (which runs cascade lint --json as its validity gate) reds when a candidate + carries such a key. + + The positive step runs the merge_group lane against a valid manifest and + asserts the merge-queue-validate job succeeds. The negative step overwrites + the manifest with one carrying a misspelled top-level key (enviroments) and + asserts the same lane reds, proving strict unknown-key rejection blocks a + merge that a lenient parse would have silently accepted. + +config: + trunk_branch: main + environments: [dev] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: [] + merge_queue: + enabled: true + +steps: + - name: "Initial commit; assert the merge-queue lane runs cascade lint --json" + action: commit + commit: + message: "feat: add app" + files: + src/app.go: | + package main + func main() {} + expect: + workflow_files: + - path: ".github/workflows/cascade-merge-queue.yaml" + contains: + - "merge-queue-validate:" + - "cascade lint --json" + + - name: "POSITIVE: valid candidate passes the merge_group lane" + action: run_workflow + run_workflow: + workflow_path: ".github/workflows/cascade-merge-queue.yaml" + event: merge_group + expect: + jobs: + merge-queue-validate: success + + - name: "Overwrite the manifest with a misspelled top-level key" + action: commit + commit: + message: "test: stage a manifest with an unknown top-level key" + files: + .github/manifest.yaml: | + ci: + config: + trunk_branch: main + environments: [dev] + enviroments: [staging] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + merge_queue: + enabled: true + + - name: "NEGATIVE: unknown-key candidate is blocked by the merge_group lane" + action: run_workflow + expect_failure: true + run_workflow: + workflow_path: ".github/workflows/cascade-merge-queue.yaml" + event: merge_group diff --git a/internal/config/command.go b/internal/config/command.go index 57abf9a4..158fc121 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -10,55 +10,76 @@ import ( "github.com/stablekernel/cascade/internal/log" ) -// NewCommand creates the parse-config command +// NewCommand creates the lint command, which parses and validates a manifest. +// By default it prints a human-readable report and exits non-zero when the +// manifest is invalid. With --json it emits the machine-readable ParseResult +// (the valid/errors/warnings surface the generated workflow lanes gate on). func NewCommand() *cobra.Command { var configPath string - var outputFormat string + var jsonOutput bool cmd := &cobra.Command{ - Use: "parse-config", - Short: "Parse and validate cicd-config.yaml", - Long: `Parse a cicd-config.yaml file and output its contents as JSON. -Validates the configuration and reports any errors.`, + Use: "lint", + Short: "Lint and validate a cascade manifest", + Long: `Parse a cascade manifest, validate it, and report any errors or warnings. +Unknown or misspelled keys are rejected with a "did you mean" suggestion. + +By default the report is human-readable and the command exits non-zero when the +manifest is invalid. Use --json to emit the parsed config and validation result +as JSON (valid/errors/warnings), which the generated validation lanes consume.`, + SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return runParseConfig(configPath, outputFormat) + return runLint(configPath, jsonOutput) }, } - cmd.Flags().StringVarP(&configPath, "config", "c", "cicd-config.yaml", "Path to cicd-config.yaml") - cmd.Flags().StringVarP(&outputFormat, "output", "o", "json", "Output format (json)") + cmd.Flags().StringVarP(&configPath, "config", "c", "cicd-config.yaml", "Path to the manifest file") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit the parsed config and validation result as JSON") return cmd } -func runParseConfig(configPath, outputFormat string) error { +func runLint(configPath string, jsonOutput bool) error { cfg, err := Parse(configPath) if err != nil { - // Output error as JSON for workflow consumption - result := ParseResult{ - Valid: false, - Errors: []string{err.Error()}, + if jsonOutput { + // Preserve the machine surface the lanes gate on: a parse failure is + // reported as {valid:false, errors:[...]} with a zero exit so the + // lane's jq gate, not the exit code, decides pass/fail. + return outputJSON(ParseResult{Valid: false, Errors: []string{err.Error()}}) } - return outputJSON(result) + return err } errors := Validate(cfg) warnings, _ := cfg.ValidateSchemaVersion() warnings = append(warnings, cfg.TagGrammarWarnings()...) + + if jsonOutput { + for _, w := range warnings { + log.Warn("%s", w) + } + return outputJSON(ParseResult{ + Config: *cfg, + BuildNames: GetBuildNames(cfg), + DeployNames: GetDeployNames(cfg), + Valid: len(errors) == 0, + Errors: errors, + Warnings: warnings, + }) + } + for _, w := range warnings { log.Warn("%s", w) } - - result := ParseResult{ - Config: *cfg, - BuildNames: GetBuildNames(cfg), - DeployNames: GetDeployNames(cfg), - Valid: len(errors) == 0, - Errors: errors, - Warnings: warnings, + for _, e := range errors { + log.Error("%s", e) } - - return outputJSON(result) + if len(errors) > 0 { + return fmt.Errorf("manifest is invalid: %d error(s)", len(errors)) + } + log.Info("manifest is valid") + return nil } func outputJSON(v interface{}) error { diff --git a/internal/config/parse.go b/internal/config/parse.go index 96102e2a..37b9da4f 100644 --- a/internal/config/parse.go +++ b/internal/config/parse.go @@ -323,6 +323,10 @@ func Validate(cfg *TrunkConfig) []string { errors = append(errors, validateSecrets("validate", v.Secrets)...) } + // Unknown/misspelled top-level keys are hard errors (front-1 strictness), + // mirroring the per-component unknown-field rejection one level up. + errors = append(errors, validateUnknownTopLevel(cfg)...) + // Config-level structural validation for v1 reserved fields. errors = append(errors, validateConfigLevel(cfg)...) errors = append(errors, validateComponents(cfg)...) diff --git a/internal/config/schema_v1_e2e_test.go b/internal/config/schema_v1_e2e_test.go index d531b4d8..57e57f57 100644 --- a/internal/config/schema_v1_e2e_test.go +++ b/internal/config/schema_v1_e2e_test.go @@ -7,7 +7,7 @@ import ( ) // fullSurfaceManifest exercises the full v1 schema surface (every reserved-shape -// field) through the on-disk load path the parse-config command uses, asserting +// field) through the on-disk load path the lint command uses, asserting // it parses cleanly and validates with no errors. const fullSurfaceManifest = `ci: config: diff --git a/internal/config/schema_version_e2e_test.go b/internal/config/schema_version_e2e_test.go index 8401030f..eb25b8e1 100644 --- a/internal/config/schema_version_e2e_test.go +++ b/internal/config/schema_version_e2e_test.go @@ -10,7 +10,7 @@ import ( // TestSchemaVersionE2E exercises the full on-disk manifest load path // (file -> ParseWithKey -> Validate + ValidateSchemaVersion) the way the -// parse-config command does, covering the schema-version compatibility gate: +// lint command does, covering the schema-version compatibility gate: // // - a valid schema_version parses and validates cleanly // - an incompatible (too-new) schema_version is rejected, blocking generation diff --git a/internal/config/strict.go b/internal/config/strict.go new file mode 100644 index 00000000..30a3d740 --- /dev/null +++ b/internal/config/strict.go @@ -0,0 +1,119 @@ +package config + +import ( + "fmt" + "reflect" + "sort" + "strings" +) + +// knownTrunkFields returns the set of modeled top-level config keys, derived +// from the yaml struct tags on TrunkConfig. Reflecting the tags keeps the +// known-field set in lockstep with the struct, so a new field never has to be +// hand-registered in a second place. The inline catch-all (Extra) and any +// yaml:"-" field are excluded. +func knownTrunkFields() []string { + t := reflect.TypeOf(TrunkConfig{}) + fields := make([]string, 0, t.NumField()) + for i := 0; i < t.NumField(); i++ { + tag := t.Field(i).Tag.Get("yaml") + if tag == "" || tag == "-" { + continue + } + name := strings.Split(tag, ",")[0] + if name == "" { + // yaml:",inline" and similar carry no field name. + continue + } + fields = append(fields, name) + } + sort.Strings(fields) + return fields +} + +// validateUnknownTopLevel rejects any top-level config key that is not a modeled +// TrunkConfig field. Unknown keys are captured by the inline Extra map; each one +// becomes a hard error with a "did you mean X?" suggestion when a close modeled +// field exists. This mirrors the per-component strictness in validateComponents +// one level up, closing the previously-lenient top level. +func validateUnknownTopLevel(cfg *TrunkConfig) []string { + if len(cfg.Extra) == 0 { + return nil + } + known := knownTrunkFields() + var errs []string + for _, key := range sortedKeys(toAnyKeyed(cfg.Extra)) { + if suggestion := suggestField(key, known); suggestion != "" { + errs = append(errs, fmt.Sprintf("config has unknown field %q; did you mean %q?", key, suggestion)) + } else { + errs = append(errs, fmt.Sprintf("config has unknown field %q", key)) + } + } + return errs +} + +// suggestField returns the modeled field closest to key by Levenshtein distance, +// or "" when nothing is close enough to be a helpful suggestion. The threshold +// scales with the key length so short keys need a near-exact match while longer +// keys tolerate a typo or two, avoiding misleading suggestions for a genuinely +// out-of-place field. +func suggestField(key string, known []string) string { + threshold := len(key) / 3 + if threshold < 2 { + threshold = 2 + } + best := "" + bestDist := threshold + 1 + for _, candidate := range known { + d := levenshtein(key, candidate) + if d < bestDist { + bestDist = d + best = candidate + } + } + if bestDist > threshold { + return "" + } + return best +} + +// levenshtein computes the edit distance between two strings using the standard +// two-row dynamic-programming table. +func levenshtein(a, b string) int { + ra, rb := []rune(a), []rune(b) + la, lb := len(ra), len(rb) + if la == 0 { + return lb + } + if lb == 0 { + return la + } + prev := make([]int, lb+1) + curr := make([]int, lb+1) + for j := 0; j <= lb; j++ { + prev[j] = j + } + for i := 1; i <= la; i++ { + curr[0] = i + for j := 1; j <= lb; j++ { + cost := 1 + if ra[i-1] == rb[j-1] { + cost = 0 + } + curr[j] = min3(curr[j-1]+1, prev[j]+1, prev[j-1]+cost) + } + prev, curr = curr, prev + } + return prev[lb] +} + +func min3(a, b, c int) int { + m := a + if b < m { + m = b + } + if c < m { + m = c + } + return m +} diff --git a/internal/config/strict_test.go b/internal/config/strict_test.go new file mode 100644 index 00000000..26172bcc --- /dev/null +++ b/internal/config/strict_test.go @@ -0,0 +1,87 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func writeStrictManifest(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "manifest.yaml") + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatalf("write manifest: %v", err) + } + return p +} + +func errsContain(errs []string, sub string) bool { + for _, e := range errs { + if strings.Contains(e, sub) { + return true + } + } + return false +} + +// TestValidate_RejectsMisspelledTopLevelKey verifies front-1 strictness: an +// unknown/misspelled top-level config key is a hard error carrying a +// did-you-mean suggestion, rather than being silently dropped by the lenient +// decoder (the pre-strictness behavior). +func TestValidate_RejectsMisspelledTopLevelKey(t *testing.T) { + const body = `ci: + config: + trunk_branch: main + enviroments: [dev, prod] +` + cfg, err := Parse(writeStrictManifest(t, body)) + if err != nil { + t.Fatalf("Parse() error: %v", err) + } + errs := Validate(cfg) + if !errsContain(errs, "enviroments") { + t.Fatalf("expected an error naming the unknown key 'enviroments', got: %v", errs) + } + if !errsContain(errs, "did you mean") || !errsContain(errs, "environments") { + t.Fatalf("expected a 'did you mean environments?' suggestion, got: %v", errs) + } +} + +// TestValidate_RejectsComponentScopedFieldAtTopLevel verifies a field that is +// only meaningful under components. (here: path) is rejected when pasted +// at the top level, instead of being silently ignored. +func TestValidate_RejectsComponentScopedFieldAtTopLevel(t *testing.T) { + const body = `ci: + config: + trunk_branch: main + environments: [dev, prod] + path: services/api +` + cfg, err := Parse(writeStrictManifest(t, body)) + if err != nil { + t.Fatalf("Parse() error: %v", err) + } + errs := Validate(cfg) + if !errsContain(errs, "path") || !errsContain(errs, "unknown") { + t.Fatalf("expected an unknown-field error naming 'path', got: %v", errs) + } +} + +// TestValidate_AcceptsKnownTopLevelKeys guards against false positives: a +// manifest using only modeled top-level fields must stay valid under strictness. +func TestValidate_AcceptsKnownTopLevelKeys(t *testing.T) { + const body = `ci: + config: + trunk_branch: main + environments: [dev, prod] + tag_prefix: v +` + cfg, err := Parse(writeStrictManifest(t, body)) + if err != nil { + t.Fatalf("Parse() error: %v", err) + } + if errs := Validate(cfg); len(errs) != 0 { + t.Fatalf("Validate() rejected a manifest of known fields: %v", errs) + } +} diff --git a/internal/config/types.go b/internal/config/types.go index 1a74a148..787eea05 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -197,6 +197,13 @@ type TrunkConfig struct { // every component's extra_paths, and applies only when a components block is // present. Empty by default, so a manifest without it is unchanged. SharedPaths []string `yaml:"shared_paths,omitempty" json:"shared_paths,omitempty"` + + // Extra captures any top-level config key that is not a modeled TrunkConfig + // field, mirroring the per-component catch-all on ComponentConfig. A populated + // Extra map is rejected by validateUnknownTopLevel with a did-you-mean + // suggestion, so a misspelled or misplaced key is a hard error instead of + // being silently dropped by the lenient yaml decoder. It is never serialized. + Extra map[string]any `yaml:",inline" json:"-"` } // ConcurrencyConfig overrides the default concurrency: block emitted on the @@ -850,7 +857,7 @@ func (c *TrunkConfig) GetAllReleaseArtifacts() []struct { return artifacts } -// ParseResult is the output of parse-config command +// ParseResult is the JSON output of the lint command (lint --json). type ParseResult struct { Config TrunkConfig `json:"config"` BuildNames []string `json:"build_names"` diff --git a/internal/generate/command_test.go b/internal/generate/command_test.go index a295ac25..ffdd32b9 100644 --- a/internal/generate/command_test.go +++ b/internal/generate/command_test.go @@ -67,7 +67,6 @@ func defaultOpts(configPath, outputPath string) generateOptions { // validManifestContent returns a valid manifest with ci: key at top level const validManifestContent = `ci: config: - project: test-project trunk_branch: main environments: [dev] builds: diff --git a/internal/generate/hotfix.go b/internal/generate/hotfix.go index 04367a15..ab481100 100644 --- a/internal/generate/hotfix.go +++ b/internal/generate/hotfix.go @@ -591,7 +591,7 @@ func (g *HotfixGenerator) writeApplyJob(sb *strings.Builder) { sb.WriteString(" done\n") } -// writeCheckJob emits the parse-config validity gate that runs while a hotfix PR +// writeCheckJob emits the lint validity gate that runs while a hotfix PR // against an env/* branch is open or closing. Full required-status-check // designation against the open PR is the operator's branch-protection // responsibility, surfaced by the apply job's protection-warning step. @@ -610,7 +610,7 @@ func (g *HotfixGenerator) writeCheckJob(sb *strings.Builder) { sb.WriteString(" - name: Validate manifest\n") sb.WriteString(" run: |\n") fmt.Fprintf(sb, " MANIFEST_FILE=\"%s\"\n", g.getManifestFilePath()) - sb.WriteString(" RESULT=$(cascade parse-config --config \"$MANIFEST_FILE\")\n") + sb.WriteString(" RESULT=$(cascade lint --json --config \"$MANIFEST_FILE\")\n") sb.WriteString(" echo \"$RESULT\"\n") sb.WriteString(" VALID=$(echo \"$RESULT\" | jq -r '.valid // false')\n") sb.WriteString(" if [[ \"$VALID\" != \"true\" ]]; then\n") diff --git a/internal/generate/hotfix_test.go b/internal/generate/hotfix_test.go index 77daf33a..9a42b45e 100644 --- a/internal/generate/hotfix_test.go +++ b/internal/generate/hotfix_test.go @@ -263,8 +263,8 @@ func TestHotfixGenerator_Jobs(t *testing.T) { assert.Contains(t, content, "cascade hotfix plan") assert.Contains(t, content, "cascade hotfix finalize") - // check job runs the parse-config validity gate. - assert.Contains(t, content, "cascade parse-config") + // check job runs the lint validity gate. + assert.Contains(t, content, "cascade lint --json") } func TestHotfixGenerator_ConflictPath(t *testing.T) { diff --git a/internal/generate/merge_queue.go b/internal/generate/merge_queue.go index dd2fd81f..be7bcf4f 100644 --- a/internal/generate/merge_queue.go +++ b/internal/generate/merge_queue.go @@ -11,7 +11,7 @@ import ( // MergeQueueGenerator emits the opt-in merge-queue validation lane. When // config.merge_queue.enabled is set, cascade generates a merge_group-triggered // workflow that validates the prospective trunk commit with cascade's own -// logic: it runs `cascade parse-config` as a validity gate and a dry-run +// logic: it runs `cascade lint` as a validity gate and a dry-run // `cascade orchestrate setup` to preview the build/deploy decisions against the // merge-group candidate ref. The lane is read-only (no state writes, no // releases, no deploys) and reports a status the merge queue can require. @@ -80,7 +80,7 @@ func (g *MergeQueueGenerator) writeHeader(sb *strings.Builder) { sb.WriteString("#\n") sb.WriteString("# Merge-queue validation lane (opt-in via merge_queue.enabled).\n") sb.WriteString("# Runs on merge_group against the would-be-trunk commit and validates it\n") - sb.WriteString("# with cascade's own logic: a parse-config validity gate plus a dry-run\n") + sb.WriteString("# with cascade's own logic: a lint validity gate plus a dry-run\n") sb.WriteString("# orchestrate setup that previews the build/deploy decisions. The lane is\n") sb.WriteString("# read-only (no state writes, releases, or deploys) and reports a status\n") sb.WriteString("# the merge queue can require.\n") @@ -120,12 +120,12 @@ func (g *MergeQueueGenerator) writeJob(sb *strings.Builder) { // gh release download against the public stablekernel/cascade repository. sb.WriteString(" token: ${{ github.token }}\n") - // Validity gate: parse-config reports validity in its JSON output rather - // than via exit code, so gate on the parsed result. + // Validity gate: lint --json reports validity in its JSON output, so gate + // on the parsed result. sb.WriteString(" - name: Validate Manifest\n") sb.WriteString(" run: |\n") fmt.Fprintf(sb, " MANIFEST_FILE=\"%s\"\n", g.getManifestFilePath()) - sb.WriteString(" RESULT=$(cascade parse-config --config \"$MANIFEST_FILE\")\n") + sb.WriteString(" RESULT=$(cascade lint --json --config \"$MANIFEST_FILE\")\n") sb.WriteString(" echo \"$RESULT\"\n") sb.WriteString(" VALID=$(echo \"$RESULT\" | jq -r '.valid // false')\n") sb.WriteString(" if [[ \"$VALID\" != \"true\" ]]; then\n") diff --git a/internal/generate/merge_queue_test.go b/internal/generate/merge_queue_test.go index 7f5369e8..20367ae9 100644 --- a/internal/generate/merge_queue_test.go +++ b/internal/generate/merge_queue_test.go @@ -58,7 +58,7 @@ func TestMergeQueueGenerator_Steps(t *testing.T) { // Validate gate plus the dry-run orchestrate preview against the candidate. assert.Contains(t, content, "uses: actions/checkout@") assert.Contains(t, content, "uses: stablekernel/cascade/.github/actions/setup-cli@") - assert.Contains(t, content, "cascade parse-config --config") + assert.Contains(t, content, "cascade lint --json --config") assert.Contains(t, content, "cascade --dry-run orchestrate setup") // Validity gated on the parsed result. diff --git a/internal/generate/pr_preview.go b/internal/generate/pr_preview.go index bd3f813d..9fc9038b 100644 --- a/internal/generate/pr_preview.go +++ b/internal/generate/pr_preview.go @@ -127,12 +127,12 @@ func (g *PRPreviewGenerator) writeJob(sb *strings.Builder) { } } -// writeValidateStep runs cascade parse-config as a required check. A malformed +// writeValidateStep runs cascade lint as a required check. A malformed // manifest exits non-zero here and fails the PR before merge. func (g *PRPreviewGenerator) writeValidateStep(sb *strings.Builder) { sb.WriteString(" - name: Validate manifest\n") sb.WriteString(" run: |\n") - fmt.Fprintf(sb, " cascade parse-config --config %s\n", g.config.GetManifestFile()) + fmt.Fprintf(sb, " cascade lint --config %s\n", g.config.GetManifestFile()) sb.WriteString("\n") } diff --git a/internal/generate/pr_preview_test.go b/internal/generate/pr_preview_test.go index 5dd34385..415087f2 100644 --- a/internal/generate/pr_preview_test.go +++ b/internal/generate/pr_preview_test.go @@ -121,7 +121,7 @@ func TestPRPreviewGenerator_Enabled(t *testing.T) { assert.NotContains(t, content, "pull-requests: write") // The three read-only steps. - assert.Contains(t, content, "cascade parse-config") + assert.Contains(t, content, "cascade lint --config") assert.Contains(t, content, "cascade detect-changes") assert.Contains(t, content, "cascade --dry-run orchestrate setup") @@ -231,7 +231,6 @@ func TestPRPreviewGenerator_ValidYAML(t *testing.T) { func TestRunGenerateWorkflow_PRPreview(t *testing.T) { const previewManifest = `ci: config: - project: test-project trunk_branch: main environments: [dev] pr_preview: @@ -267,7 +266,7 @@ func TestRunGenerateWorkflow_PRPreview(t *testing.T) { content := string(raw) assert.Contains(t, content, "name: Cascade PR Preview") assert.Contains(t, content, "on:\n pull_request:") - assert.Contains(t, content, "cascade parse-config") + assert.Contains(t, content, "cascade lint --config") assert.Contains(t, content, "pull-requests: write") assert.Contains(t, content, "actions/github-script") } diff --git a/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden b/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden index 4b80dbf3..10ec78a3 100644 --- a/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden +++ b/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden @@ -260,7 +260,7 @@ jobs: - name: Validate manifest run: | MANIFEST_FILE=".github/manifest.yaml" - RESULT=$(cascade parse-config --config "$MANIFEST_FILE") + RESULT=$(cascade lint --json --config "$MANIFEST_FILE") echo "$RESULT" VALID=$(echo "$RESULT" | jq -r '.valid // false') if [[ "$VALID" != "true" ]]; then diff --git a/internal/generate/validate_check.go b/internal/generate/validate_check.go index 18330063..3fa37b00 100644 --- a/internal/generate/validate_check.go +++ b/internal/generate/validate_check.go @@ -10,7 +10,7 @@ import ( // ValidateCheckGenerator emits the opt-in manifest-validation PR check. When // config.validate_check.enabled is set, cascade generates a lightweight -// pull_request workflow that runs `cascade parse-config` against the manifest +// pull_request workflow that runs `cascade lint` against the manifest // and fails when the manifest is invalid, so a malformed configuration cannot // merge to trunk. The check validates cascade's own configuration only: it does // not run the consumer's build/test CI, requests contents: read alone, and has @@ -73,7 +73,7 @@ func (g *ValidateCheckGenerator) writeHeader(sb *strings.Builder) { fmt.Fprintf(sb, "# Regenerate with: cascade generate-workflow --config %s\n", g.getManifestFilePath()) sb.WriteString("#\n") sb.WriteString("# Manifest-validation PR check (opt-in via validate_check.enabled).\n") - sb.WriteString("# Runs `cascade parse-config` against the manifest on pull_request and\n") + sb.WriteString("# Runs `cascade lint` against the manifest on pull_request and\n") sb.WriteString("# fails when the configuration is invalid, so a malformed manifest cannot\n") sb.WriteString("# merge. Validates cascade's own configuration only; it does not run the\n") sb.WriteString("# repository's build or test suites.\n") @@ -120,10 +120,9 @@ func (g *ValidateCheckGenerator) writeJob(sb *strings.Builder) { sb.WriteString(" echo \"::error::$MANIFEST_FILE not found\"\n") sb.WriteString(" exit 1\n") sb.WriteString(" fi\n") - sb.WriteString(" # parse-config reports validity in its JSON output (valid: false on\n") - sb.WriteString(" # parse or schema errors) rather than via exit code, so gate on the\n") - sb.WriteString(" # parsed result.\n") - sb.WriteString(" RESULT=$(cascade parse-config --config \"$MANIFEST_FILE\")\n") + sb.WriteString(" # lint --json reports validity in its JSON output (valid: false on\n") + sb.WriteString(" # parse or schema errors), so gate on the parsed result.\n") + sb.WriteString(" RESULT=$(cascade lint --json --config \"$MANIFEST_FILE\")\n") sb.WriteString(" echo \"$RESULT\"\n") sb.WriteString(" VALID=$(echo \"$RESULT\" | jq -r '.valid // false')\n") sb.WriteString(" if [[ \"$VALID\" != \"true\" ]]; then\n") diff --git a/internal/generate/validate_check_test.go b/internal/generate/validate_check_test.go index d78663dc..17349364 100644 --- a/internal/generate/validate_check_test.go +++ b/internal/generate/validate_check_test.go @@ -55,12 +55,12 @@ func TestValidateCheckGenerator_Steps(t *testing.T) { content, err := gen.Generate() require.NoError(t, err) - // Checkout + setup-cli + the parse-config validity gate. + // Checkout + setup-cli + the lint validity gate. assert.Contains(t, content, "uses: actions/checkout@") assert.Contains(t, content, "uses: stablekernel/cascade/.github/actions/setup-cli@") - assert.Contains(t, content, "cascade parse-config --config") + assert.Contains(t, content, "cascade lint --json --config") - // parse-config reports validity in JSON (valid: false), not via exit code, + // lint --json reports validity in JSON (valid: false), not via exit code, // so the workflow must gate on the parsed result. assert.Contains(t, content, "jq -r '.valid // false'") assert.Contains(t, content, "exit 1") @@ -143,7 +143,7 @@ func TestValidateCheckGeneratorE2E(t *testing.T) { content, err := gen.Generate() require.NoError(t, err) assert.Contains(t, content, "on:\n pull_request:") - assert.Contains(t, content, "cascade parse-config") + assert.Contains(t, content, "cascade lint --json") // Omitting the block is non-breaking: the generator reports disabled. bare := `ci: diff --git a/internal/orchestrate/nochange_skip_test.go b/internal/orchestrate/nochange_skip_test.go index 01589d22..be45ef8b 100644 --- a/internal/orchestrate/nochange_skip_test.go +++ b/internal/orchestrate/nochange_skip_test.go @@ -76,7 +76,6 @@ func writeManifest(t *testing.T, repoDir, stateSHA string) string { manifestPath := filepath.Join(repoDir, ".github", "manifest.yaml") manifest := `ci: config: - project: nochange-test trunk_branch: main environments: [] builds: diff --git a/internal/orchestrate/orchestrator_test.go b/internal/orchestrate/orchestrator_test.go index f07a3053..07083f5a 100644 --- a/internal/orchestrate/orchestrator_test.go +++ b/internal/orchestrate/orchestrator_test.go @@ -253,7 +253,6 @@ func TestNewOrchestrator(t *testing.T) { // Write a manifest with ci: key at top level config := `ci: config: - project: test-project trunk_branch: main environments: - dev diff --git a/internal/orchestrate/typed_remarshal_drop_test.go b/internal/orchestrate/typed_remarshal_drop_test.go index 889fc04b..5013dbe6 100644 --- a/internal/orchestrate/typed_remarshal_drop_test.go +++ b/internal/orchestrate/typed_remarshal_drop_test.go @@ -27,14 +27,18 @@ const remarshalManifest = `ci: version: v0.1.0-rc.0 ` -// TestTypedRemarshal_UnmodeledConfig_IsDropped documents the lossy state-write -// mechanism behind the SHA-pin drift (#372/#390). The original finalize write -// re-serialized the whole manifest through the typed CICDFile struct. That -// round-trip omits any key the running binary does not model and discards the -// top-level manifest-key wrapper, so a pin_mode:sha repo loses its pin on the -// next state commit and reads as permanent drift. This test pins the failing -// behavior so the contrast with the WriteManifestState fix is explicit. -func TestTypedRemarshal_UnmodeledConfig_IsDropped(t *testing.T) { +// TestTypedRemarshal_IsLossy documents the lossy state-write mechanism behind +// the SHA-pin drift (#372/#390). The original finalize write re-serialized the +// whole manifest through the typed CICDFile struct. That round-trip discards the +// top-level manifest-key wrapper (and reflows comments and formatting), so a +// state commit written this way reshapes the file. This test pins that lossiness +// so the contrast with the WriteManifestState fix is explicit. +// +// Note: the inline top-level catch-all (TrunkConfig.Extra) now carries unmodeled +// config keys through the typed round-trip, so future_field survives here where +// it once dropped. The durable lossiness the fix guards against is the wrapper +// drop, which persists. +func TestTypedRemarshal_IsLossy(t *testing.T) { file, err := config.ParseManifestBytes([]byte(remarshalManifest), "ci") if err != nil { t.Fatalf("ParseManifestBytes: %v", err) @@ -46,9 +50,6 @@ func TestTypedRemarshal_UnmodeledConfig_IsDropped(t *testing.T) { } got := string(out) - if strings.Contains(got, "future_field") { - t.Errorf("typed remarshal was expected to drop the unmodeled future_field, but it survived:\n%s", got) - } if strings.Contains(got, "ci:") { t.Errorf("typed remarshal was expected to drop the 'ci:' wrapper, but it survived:\n%s", got) } diff --git a/internal/schema/command.go b/internal/schema/command.go index fa825cb7..0091a87f 100644 --- a/internal/schema/command.go +++ b/internal/schema/command.go @@ -20,7 +20,7 @@ func NewCommand() *cobra.Command { The schema describes the structure, types, and allowed values of a manifest file. Point your editor at it for autocomplete, type checking, and hover docs -while authoring .github/manifest.yaml. cascade parse-config remains the +while authoring .github/manifest.yaml. cascade lint remains the authority for semantic and cross-field rules. Examples: diff --git a/internal/schema/manifest.schema.json b/internal/schema/manifest.schema.json index 33702d4b..dfbe32e2 100644 --- a/internal/schema/manifest.schema.json +++ b/internal/schema/manifest.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://stablekernel.github.io/cascade/manifest.schema.json", "title": "cascade manifest", - "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade parse-config remains the authority for semantic and cross-field rules.", + "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade lint remains the authority for semantic and cross-field rules.", "type": "object", "additionalProperties": false, "properties": { diff --git a/schema/manifest.schema.json b/schema/manifest.schema.json index 33702d4b..dfbe32e2 100644 --- a/schema/manifest.schema.json +++ b/schema/manifest.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://stablekernel.github.io/cascade/manifest.schema.json", "title": "cascade manifest", - "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade parse-config remains the authority for semantic and cross-field rules.", + "description": "Structure, types, and enums for a cascade manifest file. The manifest is nested under a top key (default \"ci\", configurable via config.manifest_key) and carries the authoring surface (config), plus cascade-managed promotion state (state, latest_release). This schema powers editor autocomplete and hover docs; cascade lint remains the authority for semantic and cross-field rules.", "type": "object", "additionalProperties": false, "properties": {