diff --git a/CHANGELOG.md b/CHANGELOG.md index 3269dd94..52b39353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,18 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic - [ ] Update the `[Unreleased]` compare link to the new tag. - [ ] Create and push a signed `vX.Y.Z` tag from `master`. +### Added + +- `doctor --json` `overrides` now reports every resolver override state + except `parent_group_open` (internal runner-to-runner plumbing, never a + user override): `failure_policy`, `install_pms`, `output_grouping` + (`group_output`/`github_group_parallel`/`parallel_grouped`), + `prefer_sources`, `script_policy`, and `task_source_pins`. Previously + only `pm`/`pm_by_ecosystem`/`runner`/`prefer_runners`/`fallback`/ + `on_mismatch`/`explain`/`no_warnings`/`quiet` were surfaced, so `-k`/`-K`, + `[tasks].prefer`, `[tasks.overrides]`, `[install]`, and `[github]`/ + `[parallel]` config could be set without `doctor` ever showing it. + ### Changed - **Breaking:** `doctor --json` and `why --json` now always emit the @@ -44,12 +56,28 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic suffix (`doctor.v3.schema.json` → `doctor.schema.json`, etc.); the 10 superseded schema/example files are deleted. +- `doctor --json` `overrides.fallback`, `on_mismatch`, `pm`, + `pm_by_ecosystem`, `runner`, and `prefer_runners` are now closed enums in + `doctor.schema.json` (with the accepted values documented per variant), + not generic strings — editors and validators can now catch a typo'd + override value against the committed schema instead of silently + accepting anything. `pm_by_ecosystem`'s keys are constrained the same + way: the schema now lists the seven ecosystem names explicitly instead + of allowing any string key, and its values are plain (non-nullable) + package-manager labels — the report never emits a `null` there. + `failure_policy`, `script_policy`, and `install_pms` (new fields, see + Added above) get the same closed-enum treatment from the start. + ### Fixed - `runner schema --all` no longer surfaces a raw Rust panic if the init-template generator ever drifts from `RunnerConfig` in a released binary (the drift-guard test should already catch this before merge); it now reports a clean CLI error instead. +- `doctor --json` `overrides.quiet` is now listed as required in + `doctor.schema.json`, like every other boolean override — it was kept + optional for compatibility with the pre-collapse `doctor` v3 schema, + which this same release already removed. ## [0.18.1] - 2026-07-04 diff --git a/schemas/doctor.example.json b/schemas/doctor.example.json index ad388d3d..e75ab02f 100644 --- a/schemas/doctor.example.json +++ b/schemas/doctor.example.json @@ -25,9 +25,9 @@ "name": "runner", "version": "0.18.1", "schema_versions": { - "doctor": 3, - "list": 2, - "why": 3 + "doctor": 1, + "list": 1, + "why": 1 } }, "project": { @@ -38,14 +38,24 @@ }, "overrides": { "explain": false, + "failure_policy": "fail-fast", "fallback": "probe", + "install_pms": [], "no_warnings": false, "on_mismatch": "warn", + "output_grouping": { + "github_group_parallel": true, + "group_output": true, + "parallel_grouped": false + }, "pm": null, "pm_by_ecosystem": {}, "prefer_runners": [], + "prefer_sources": [], "quiet": false, - "runner": null + "runner": null, + "script_policy": "default", + "task_source_pins": {} }, "ecosystems": [ { diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 14b77653..f532f33c 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -275,6 +275,46 @@ }, "additionalProperties": false }, + "FailurePolicy": { + "description": "Failure policy for a chain. `FailFast` is the default and matches\n`make -j` semantics in parallel mode (let running siblings finish,\ndon't start new ones).", + "oneOf": [ + { + "description": "Stop the chain on the first failing task. In parallel mode,\nalready-running siblings complete naturally.", + "type": "string", + "const": "fail-fast" + }, + { + "description": "Run every task to completion regardless of failures. Final exit\ncode reflects the first failure.", + "type": "string", + "const": "keep-going" + }, + { + "description": "Parallel only: SIGKILL siblings on first failure (`std::process::Child::kill`).\nSequential callers accept this silently (no-op). Catch-able SIGTERM\nsemantics would need a libc/nix dep — deferred to a follow-up.", + "type": "string", + "const": "kill-on-fail" + } + ] + }, + "FallbackPolicy": { + "description": "What to do when no signal in steps 2–6 matches.\n\nSet via `--fallback` / `RUNNER_FALLBACK` / `[resolution].fallback`.", + "oneOf": [ + { + "description": "Walk `$PATH` in canonical order and pick the first installed PM.\nErrors if nothing matches.", + "type": "string", + "const": "probe" + }, + { + "description": "Legacy: silently default to `npm` so dispatch is attempted even\nwhen nothing is detected. Useful for backwards compatibility.", + "type": "string", + "const": "npm" + }, + { + "description": "Refuse to proceed when no signal matches; error out with a list of\nsources that were checked.", + "type": "string", + "const": "error" + } + ] + }, "Invocation": { "description": "How this report came to be: the exact process invocation.", "type": "object", @@ -300,65 +340,237 @@ }, "additionalProperties": false }, + "MismatchPolicy": { + "description": "How to react when manifest declaration (step 5) and lockfile (step 6)\ndisagree about which package manager the project uses.\n\nSet via `--on-mismatch` / `RUNNER_ON_MISMATCH` /\n`[resolution].on_mismatch`. Independent from\n`devEngines.packageManager` `onFail` — that policy governs whether\nthe *declared* PM can actually run; this one governs whether the\nresolver tolerates the declaration disagreeing with the install\nstate at all.", + "oneOf": [ + { + "description": "Emit a `package.json` warning, prefer the declaration (Corepack\nsemantics — the lockfile is most likely stale).", + "type": "string", + "const": "warn" + }, + { + "description": "Stay silent; prefer the declaration.", + "type": "string", + "const": "ignore" + }, + { + "description": "Bail with [`super::ResolveError::MismatchPolicyError`]. Intended for\nCI guardrails where a mismatch should block the run.", + "type": "string", + "const": "error" + } + ] + }, + "OutputGrouping": { + "description": "The three grouping toggles bundled so [`Overrides`] doesn't tip\nclippy's bool-count lint; each mirrors a same-named field on\n[`ResolutionOverrides`].", + "type": "object", + "required": [ + "github_group_parallel", + "group_output", + "parallel_grouped" + ], + "properties": { + "github_group_parallel": { + "description": "Group parallel output under GitHub Actions\n(`[github].group_parallel`).", + "type": "boolean" + }, + "group_output": { + "description": "Broad GitHub Actions grouping switch (`[github].group_output`).", + "type": "boolean" + }, + "parallel_grouped": { + "description": "Group parallel output outside GitHub Actions\n(`[parallel].grouped`).", + "type": "boolean" + } + }, + "additionalProperties": false + }, "Overrides": { - "description": "Effective override stack, labels only. Provenance (cli/env/config)\nstays on the flat `list`/`info` surface.", + "description": "Effective override stack, labels only. Provenance (cli/env/config) stays on the flat `list`/`info` surface.\n\nCovers every field on [`ResolutionOverrides`] except `parent_group_open`, which is internal\nrunner-to-runner plumbing (an inherited env marker, never a user override) and has nothing\nmeaningful to report — see the drift guard test at the bottom of this file, which enforces that\na future field can't land on `ResolutionOverrides` and silently miss both this struct and its\nexclusion list.", "type": "object", "required": [ "explain", + "failure_policy", "fallback", + "install_pms", "no_warnings", "on_mismatch", + "output_grouping", "pm", "pm_by_ecosystem", "prefer_runners", - "runner" + "prefer_sources", + "quiet", + "runner", + "script_policy", + "task_source_pins" ], "properties": { "explain": { "type": "boolean" }, + "failure_policy": { + "$ref": "#/$defs/FailurePolicy" + }, "fallback": { - "type": "string" + "$ref": "#/$defs/FallbackPolicy" + }, + "install_pms": { + "type": "array", + "items": { + "$ref": "#/$defs/PackageManager" + } }, "no_warnings": { "type": "boolean" }, "on_mismatch": { - "type": "string" + "$ref": "#/$defs/MismatchPolicy" + }, + "output_grouping": { + "$ref": "#/$defs/OutputGrouping" }, "pm": { - "type": [ - "null", - "string" + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } ] }, "pm_by_ecosystem": { "type": "object", - "additionalProperties": { - "type": [ - "null", - "string" - ] - } + "properties": { + "deno": { + "$ref": "#/$defs/PackageManager" + }, + "go": { + "$ref": "#/$defs/PackageManager" + }, + "node": { + "$ref": "#/$defs/PackageManager" + }, + "php": { + "$ref": "#/$defs/PackageManager" + }, + "python": { + "$ref": "#/$defs/PackageManager" + }, + "ruby": { + "$ref": "#/$defs/PackageManager" + }, + "rust": { + "$ref": "#/$defs/PackageManager" + } + }, + "additionalProperties": false }, "prefer_runners": { "type": "array", "items": { - "type": "string" + "$ref": "#/$defs/TaskRunner" + } + }, + "prefer_sources": { + "type": "array", + "items": { + "$ref": "#/$defs/TaskSourceLabel" } }, "quiet": { "type": "boolean" }, "runner": { - "type": [ - "null", - "string" + "anyOf": [ + { + "$ref": "#/$defs/TaskRunner" + }, + { + "type": "null" + } ] + }, + "script_policy": { + "$ref": "#/$defs/ScriptPolicy" + }, + "task_source_pins": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/$defs/TaskSourceLabel" + } + } } }, "additionalProperties": false }, + "PackageManager": { + "description": "A dependency manager detected via lockfile or config presence.", + "oneOf": [ + { + "description": "npm — detected via `package-lock.json`.", + "type": "string", + "const": "npm" + }, + { + "description": "Yarn — detected via `yarn.lock`.", + "type": "string", + "const": "yarn" + }, + { + "description": "pnpm — detected via `pnpm-lock.yaml`.", + "type": "string", + "const": "pnpm" + }, + { + "description": "Bun — detected via `bun.lockb` or `bun.lock`.", + "type": "string", + "const": "bun" + }, + { + "description": "Cargo (Rust) — detected via `Cargo.toml`.", + "type": "string", + "const": "cargo" + }, + { + "description": "Deno — detected via `deno.json` / `deno.jsonc`.", + "type": "string", + "const": "deno" + }, + { + "description": "uv (Python) — detected via `uv.lock`.", + "type": "string", + "const": "uv" + }, + { + "description": "Poetry (Python) — detected via `poetry.lock` or Poetry `pyproject.toml` markers.", + "type": "string", + "const": "poetry" + }, + { + "description": "Pipenv (Python) — detected via `Pipfile` / `Pipfile.lock`.", + "type": "string", + "const": "pipenv" + }, + { + "description": "Go modules — detected via `go.mod`.", + "type": "string", + "const": "go" + }, + { + "description": "Bundler (Ruby) — detected via `Gemfile`.", + "type": "string", + "const": "bundler" + }, + { + "description": "Composer (PHP) — detected via `composer.json`.", + "type": "string", + "const": "composer" + } + ] + }, "ProjectInfo": { "description": "Project anchoring facts.", "type": "object", @@ -461,6 +673,26 @@ }, "additionalProperties": false }, + "ScriptPolicy": { + "description": "Install-time lifecycle-script execution policy for `runner install`.\n\nLifecycle/build scripts (`postinstall`, native-extension compilation,\n…) are the primary supply-chain attack surface during dependency\ninstalls. This knob lets a project deny them across the package managers\nthat expose a skip mechanism, or force them on across the managers that\ncan express it — the latter matters because several package managers\n(npm, pnpm, …) are moving to scripts-off-by-default in upcoming majors.\n\nSet via `--no-scripts` (deny) / `--scripts` (force on) on the CLI,\n`RUNNER_INSTALL_SCRIPTS` (env), or `[install].scripts` (config), highest\nfirst.", + "oneOf": [ + { + "description": "Leave each package manager at its own built-in default: npm,\nyarn-classic, pnpm (<10) and composer run dependency scripts, while\nbun, pnpm (>=10) and deno already deny them.", + "type": "string", + "const": "default" + }, + { + "description": "Skip lifecycle scripts wherever the package manager exposes a skip\nmechanism (npm/yarn/pnpm/bun `--ignore-scripts`, composer\n`--no-scripts`, yarn-berry `YARN_ENABLE_SCRIPTS=false`); deno already\ndenies by default. Managers without one (cargo, go, bundler, and the\nPython backends uv/poetry/pipenv) warn and proceed unchanged.", + "type": "string", + "const": "deny" + }, + { + "description": "Force lifecycle scripts on wherever the package manager can express it:\nnpm `--no-ignore-scripts`, yarn-berry `YARN_ENABLE_SCRIPTS=true`, deno\n`--allow-scripts` (allow all). Managers that already run scripts by\ndefault (composer, cargo, go, bundler, the Python backends, yarn-classic)\nare satisfied without a flag. bun and pnpm (>=10) deny dependency build\nscripts by default and re-enable them only through a manifest allowlist\n(`trustedDependencies` / `onlyBuiltDependencies`) that runner must not\nwrite, so they warn that force-on is not flag-expressible.", + "type": "string", + "const": "allow" + } + ] + }, "Severity": { "description": "Severity of a conflict or diagnostic. The draft's `debug`/`error`\nlevels join when something emits them.", "type": "string", @@ -516,6 +748,46 @@ }, "additionalProperties": false }, + "TaskRunner": { + "description": "A task runner detected via config file presence.", + "oneOf": [ + { + "description": "Turborepo — detected via `turbo.json` / `turbo.jsonc`.", + "type": "string", + "const": "turbo" + }, + { + "description": "Nx — detected via `nx.json`.", + "type": "string", + "const": "nx" + }, + { + "description": "GNU Make — detected via `Makefile` / `GNUmakefile` / `makefile`.", + "type": "string", + "const": "make" + }, + { + "description": "just — detected via case-insensitive `justfile` / `.justfile`.", + "type": "string", + "const": "just" + }, + { + "description": "go-task — detected via `Taskfile.yml` and variants. Serializes as\n`\"task\"` (matching [`Self::label`]) — `kebab-case` alone would\nproduce `\"go-task\"`, the accepted parse *alias*, not the canonical\nlabel.", + "type": "string", + "const": "task" + }, + { + "description": "mise — detected via `mise.toml` / `.mise.toml`.", + "type": "string", + "const": "mise" + }, + { + "description": "bacon — detected via `bacon.toml`.", + "type": "string", + "const": "bacon" + } + ] + }, "TaskSourceLabel": { "type": "string", "enum": [ diff --git a/src/chain/mod.rs b/src/chain/mod.rs index b44ad673..c567f55f 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -72,7 +72,9 @@ pub(crate) enum ChainItemKind { /// Failure policy for a chain. `FailFast` is the default and matches /// `make -j` semantics in parallel mode (let running siblings finish, /// don't start new ones). -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum FailurePolicy { /// Stop the chain on the first failing task. In parallel mode, /// already-running siblings complete naturally. diff --git a/src/cmd/schema.rs b/src/cmd/schema.rs index c098ab1c..5d9422f1 100644 --- a/src/cmd/schema.rs +++ b/src/cmd/schema.rs @@ -532,7 +532,6 @@ fn output_schema(command: &'static str) -> Result { set_object_field(&mut schema, "description", json!(description(command))); patch_schema_version_const(&mut schema); patch_source_schema(&mut schema, command); - patch_schema_compat(&mut schema, command); Ok(schema) } @@ -591,28 +590,21 @@ fn patch_source_schema(schema: &mut Value, command: &str) { patch_task_info_source(defs); patch_why_task(defs); patch_def_field(defs, "SourceEntry", "kind", "TaskSourceLabel"); + patch_overrides_source_labels(defs); } -fn patch_schema_compat(schema: &mut Value, command: &str) { - if command == "doctor" { - // The structured doctor report existed before `quiet`; keep - // additive fields optional so the committed schema still - // validates payloads emitted before that field landed. - remove_required_def_field(schema, "Overrides", "quiet"); - } -} - -fn remove_required_def_field(schema: &mut Value, def_name: &'static str, field: &'static str) { - let Some(required) = schema - .get_mut("$defs") - .and_then(Value::as_object_mut) - .and_then(|defs| defs.get_mut(def_name)) - .and_then(|definition| definition.get_mut("required")) - .and_then(Value::as_array_mut) - else { +/// `Overrides.prefer_sources`/`task_source_pins` hold the same structured +/// source labels as `SourceEntry.kind`, command-dependent like it — reuse +/// `TaskSourceLabel` instead of leaving them generic strings. (Every other +/// `Overrides` label field is backed by a real enum and gets its schema +/// constraint straight from `#[derive(schemars::JsonSchema)]` on that enum +/// — no hand-built schema needed there.) +fn patch_overrides_source_labels(defs: &mut Map) { + if !defs.contains_key("Overrides") { return; - }; - required.retain(|name| name.as_str() != Some(field)); + } + patch_def_array_items(defs, "Overrides", "prefer_sources", "TaskSourceLabel"); + patch_def_map_array_items(defs, "Overrides", "task_source_pins", "TaskSourceLabel"); } fn patch_task_info_source(defs: &mut Map) { @@ -634,21 +626,63 @@ fn patch_why_task(defs: &mut Map) { patch_def_field(defs, "WhyTask", "provider", "ProviderLabel"); } +/// Mutable handle on `$defs..properties.`, the shared +/// navigation prefix of every schema patch below. +fn field_schema_mut<'a>( + defs: &'a mut Map, + def_name: &str, + field: &str, +) -> Option<&'a mut Value> { + defs.get_mut(def_name) + .and_then(|definition| definition.get_mut("properties")) + .and_then(Value::as_object_mut) + .and_then(|properties| properties.get_mut(field)) +} + +fn def_ref(target_def: &str) -> Value { + json!({ "$ref": format!("#/$defs/{target_def}") }) +} + fn patch_def_field( defs: &mut Map, def_name: &'static str, field: &'static str, target_def: &'static str, ) { - let Some(field_schema) = defs - .get_mut(def_name) - .and_then(|definition| definition.get_mut("properties")) - .and_then(Value::as_object_mut) - .and_then(|properties| properties.get_mut(field)) - else { - return; - }; - *field_schema = json!({ "$ref": format!("#/$defs/{target_def}") }); + if let Some(field_schema) = field_schema_mut(defs, def_name, field) { + *field_schema = def_ref(target_def); + } +} + +/// Like [`patch_def_field`], but for an array-typed field — constrains its +/// `items` schema instead of the field itself. +fn patch_def_array_items( + defs: &mut Map, + def_name: &'static str, + field: &'static str, + target_def: &'static str, +) { + if let Some(items) = field_schema_mut(defs, def_name, field) + .and_then(|field_schema| field_schema.get_mut("items")) + { + *items = def_ref(target_def); + } +} + +/// Like [`patch_def_array_items`], but for a map-of-array field — constrains +/// the array items nested under `additionalProperties`. +fn patch_def_map_array_items( + defs: &mut Map, + def_name: &'static str, + field: &'static str, + target_def: &'static str, +) { + if let Some(items) = field_schema_mut(defs, def_name, field) + .and_then(|field_schema| field_schema.get_mut("additionalProperties")) + .and_then(|additional| additional.get_mut("items")) + { + *items = def_ref(target_def); + } } fn task_source_label_schema(command: &str) -> Value { @@ -716,51 +750,6 @@ fn description(command: &str) -> String { mod tests { use serde_json::Value; - use super::output_schema; - - fn overrides_def(schema: &Value) -> &Value { - schema - .get("$defs") - .and_then(Value::as_object) - .and_then(|defs| defs.get("Overrides")) - .expect("schema should define Overrides") - } - - fn quiet_is_optional(schema: &Value) -> bool { - overrides_def(schema) - .get("required") - .and_then(Value::as_array) - .is_some_and(|required| !required.iter().any(|name| name.as_str() == Some("quiet"))) - } - - fn quiet_type(schema: &Value) -> Option<&str> { - overrides_def(schema) - .get("properties") - .and_then(Value::as_object) - .and_then(|properties| properties.get("quiet")) - .and_then(|quiet| quiet.get("type")) - .and_then(Value::as_str) - } - - #[test] - fn doctor_schema_keeps_quiet_optional_for_compat() { - let schema = output_schema::>("doctor") - .expect("doctor schema should render"); - - assert!(quiet_is_optional(&schema)); - assert_eq!(quiet_type(&schema), Some("boolean")); - } - - #[test] - fn committed_doctor_schema_keeps_quiet_optional_for_compat() { - let raw = std::fs::read_to_string("schemas/doctor.schema.json") - .expect("committed doctor schema should be readable"); - let schema: Value = serde_json::from_str(&raw).expect("schema should parse as JSON"); - - assert!(quiet_is_optional(&schema)); - assert_eq!(quiet_type(&schema), Some("boolean")); - } - #[test] fn committed_doctor_example_includes_quiet_override() { let raw = std::fs::read_to_string("schemas/doctor.example.json") diff --git a/src/resolver/types.rs b/src/resolver/types.rs index 43b59f26..05947218 100644 --- a/src/resolver/types.rs +++ b/src/resolver/types.rs @@ -125,7 +125,9 @@ pub(crate) struct ResolutionOverrides { /// What to do when no signal in steps 2–6 matches. /// /// Set via `--fallback` / `RUNNER_FALLBACK` / `[resolution].fallback`. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum FallbackPolicy { /// Walk `$PATH` in canonical order and pick the first installed PM. /// Errors if nothing matches. @@ -168,7 +170,9 @@ impl FallbackPolicy { /// Set via `--no-scripts` (deny) / `--scripts` (force on) on the CLI, /// `RUNNER_INSTALL_SCRIPTS` (env), or `[install].scripts` (config), highest /// first. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum ScriptPolicy { /// Leave each package manager at its own built-in default: npm, /// yarn-classic, pnpm (<10) and composer run dependency scripts, while @@ -219,7 +223,9 @@ impl ScriptPolicy { /// the *declared* PM can actually run; this one governs whether the /// resolver tolerates the declaration disagreeing with the install /// state at all. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum MismatchPolicy { /// Emit a `package.json` warning, prefer the declaration (Corepack /// semantics — the lockfile is most likely stale). diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index 17f59132..171e6303 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -32,10 +32,15 @@ use std::path::Path; use serde::Serialize; use super::labels::structured_source_label; +use crate::chain::FailurePolicy; use crate::cmd::run::{resolve_python_pm, select_task_entry, source_depth, source_priority}; -use crate::resolver::{ResolutionOverrides, ResolutionStep, Resolver}; +use crate::resolver::{ + FallbackPolicy, MismatchPolicy, ResolutionOverrides, ResolutionStep, Resolver, ScriptPolicy, +}; use crate::tool::node::detect_pm_from_manifest; -use crate::types::{DetectionWarning, Ecosystem, PackageManager, ProjectContext, Task, TaskSource}; +use crate::types::{ + DetectionWarning, Ecosystem, PackageManager, ProjectContext, Task, TaskRunner, TaskSource, +}; /// `runner doctor --json` payload. #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] @@ -143,21 +148,49 @@ struct ProjectInfo { workspace: Option, } -/// Effective override stack, labels only. Provenance (cli/env/config) -/// stays on the flat `list`/`info` surface. +/// Effective override stack, labels only. Provenance (cli/env/config) stays on the flat `list`/`info` surface. +/// +/// Covers every field on [`ResolutionOverrides`] except `parent_group_open`, which is internal +/// runner-to-runner plumbing (an inherited env marker, never a user override) and has nothing +/// meaningful to report — see the drift guard test at the bottom of this file, which enforces that +/// a future field can't land on `ResolutionOverrides` and silently miss both this struct and its +/// exclusion list. #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[derive(Debug, Serialize)] #[cfg_attr(feature = "schema", schemars(deny_unknown_fields))] struct Overrides { explain: bool, - fallback: &'static str, + fallback: FallbackPolicy, + failure_policy: FailurePolicy, + install_pms: Vec, no_warnings: bool, + output_grouping: OutputGrouping, quiet: bool, - on_mismatch: &'static str, - pm: Option<&'static str>, - pm_by_ecosystem: BTreeMap>, - prefer_runners: Vec<&'static str>, - runner: Option<&'static str>, + on_mismatch: MismatchPolicy, + pm: Option, + pm_by_ecosystem: BTreeMap, + prefer_runners: Vec, + prefer_sources: Vec<&'static str>, + runner: Option, + script_policy: ScriptPolicy, + task_source_pins: BTreeMap>, +} + +/// The three grouping toggles bundled so [`Overrides`] doesn't tip +/// clippy's bool-count lint; each mirrors a same-named field on +/// [`ResolutionOverrides`]. +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Serialize)] +#[cfg_attr(feature = "schema", schemars(deny_unknown_fields))] +struct OutputGrouping { + /// Broad GitHub Actions grouping switch (`[github].group_output`). + group_output: bool, + /// Group parallel output under GitHub Actions + /// (`[github].group_parallel`). + github_group_parallel: bool, + /// Group parallel output outside GitHub Actions + /// (`[parallel].grouped`). + parallel_grouped: bool, } /// One detected ecosystem and the PM decision made for it. @@ -496,18 +529,44 @@ fn runner_info() -> RunnerInfo { fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { Overrides { explain: overrides.explain, - fallback: overrides.fallback.label(), + fallback: overrides.fallback, + failure_policy: overrides.failure_policy, + install_pms: overrides.install_pms.clone(), no_warnings: overrides.no_warnings, + output_grouping: OutputGrouping { + group_output: overrides.group_output, + github_group_parallel: overrides.github_group_parallel, + parallel_grouped: overrides.parallel_grouped, + }, quiet: overrides.quiet, - on_mismatch: overrides.on_mismatch.label(), - pm: overrides.pm.as_ref().map(|o| o.pm.label()), + on_mismatch: overrides.on_mismatch, + pm: overrides.pm.as_ref().map(|o| o.pm), pm_by_ecosystem: overrides .pm_by_ecosystem .iter() - .map(|(eco, o)| (eco.label().to_string(), Some(o.pm.label()))) + .map(|(&eco, o)| (eco, o.pm)) + .collect(), + prefer_runners: overrides.prefer_runners.clone(), + prefer_sources: overrides + .prefer_sources + .iter() + .map(|&source| structured_source_label(source)) + .collect(), + runner: overrides.runner.as_ref().map(|o| o.runner), + script_policy: overrides.script_policy, + task_source_pins: overrides + .task_source_overrides + .iter() + .map(|(name, sources)| { + ( + name.clone(), + sources + .iter() + .map(|&source| structured_source_label(source)) + .collect(), + ) + }) .collect(), - prefer_runners: overrides.prefer_runners.iter().map(|r| r.label()).collect(), - runner: overrides.runner.as_ref().map(|o| o.runner.label()), } } @@ -1249,4 +1308,166 @@ mod tests { let status = tool["probe"]["status"].as_str().expect("probe status"); assert!(status == "found" || status == "missing"); } + + #[test] + fn report_surfaces_previously_missing_override_fields() { + use std::collections::BTreeMap; + + use crate::chain::FailurePolicy; + use crate::resolver::ScriptPolicy; + use crate::types::PackageManager; + + let overrides = ResolutionOverrides { + failure_policy: FailurePolicy::KeepGoing, + group_output: false, + github_group_parallel: false, + parallel_grouped: true, + install_pms: vec![PackageManager::Npm, PackageManager::Pnpm], + script_policy: ScriptPolicy::Deny, + prefer_sources: vec![TaskSource::Justfile, TaskSource::CargoAliases], + task_source_overrides: BTreeMap::from([( + "build".to_string(), + vec![TaskSource::Justfile], + )]), + ..ResolutionOverrides::default() + }; + + let ctx = context(vec![]); + let report = DoctorReport::build(&ctx, &overrides, false); + let json = serde_json::to_value(&report).expect("report should serialize"); + + let ov = &json["overrides"]; + assert_eq!(ov["failure_policy"], "keep-going"); + assert_eq!(ov["output_grouping"]["group_output"], false); + assert_eq!(ov["output_grouping"]["github_group_parallel"], false); + assert_eq!(ov["output_grouping"]["parallel_grouped"], true); + assert_eq!(ov["install_pms"], serde_json::json!(["npm", "pnpm"])); + assert_eq!(ov["script_policy"], "deny"); + assert_eq!( + ov["prefer_sources"], + serde_json::json!(["just", "cargo-alias"]) + ); + assert_eq!(ov["task_source_pins"]["build"], serde_json::json!(["just"])); + } + + /// Drift guard: every field on [`ResolutionOverrides`] must appear + /// either in the reflected [`Overrides`] schema or in the exclusion + /// list below (with a reason). The macro's single field list both + /// exhaustively destructures the struct (a new field fails to compile + /// until listed) and feeds the checked names, so the list can't go + /// stale relative to the destructure. + /// + /// Two fields are reported under a different name/shape than + /// `ResolutionOverrides` uses, both to dodge clippy lints: + /// `task_source_overrides` reports as `task_source_pins` + /// (`struct_field_names` — it would otherwise end with the struct's + /// own name), and `group_output`/`github_group_parallel`/ + /// `parallel_grouped` nest under `output_grouping` + /// (`struct_excessive_bools`). `RENAMED`/the `output_grouping` unnest + /// below account for both. + #[cfg(feature = "schema")] + #[test] + fn every_resolution_overrides_field_is_reported_or_excluded() { + // Internal runner-to-runner plumbing (an inherited env marker), + // never a user override — nothing meaningful to report. + const EXCLUDED: &[&str] = &["parent_group_open"]; + // resolver field name -> name it's actually reported under. + const RENAMED: &[(&str, &str)] = &[("task_source_overrides", "task_source_pins")]; + + // One list, two jobs: exhaustively destructure ResolutionOverrides + // (a new field fails to compile until added here) and name the + // fields the assertion loop checks. + macro_rules! resolution_overrides_fields { + ($($field:ident),* $(,)?) => {{ + let ResolutionOverrides { $($field: _),* } = ResolutionOverrides::default(); + [$(stringify!($field)),*] + }}; + } + let resolution_overrides_fields = resolution_overrides_fields![ + pm, + pm_by_ecosystem, + runner, + prefer_runners, + prefer_sources, + task_source_overrides, + fallback, + on_mismatch, + no_warnings, + quiet, + explain, + failure_policy, + group_output, + github_group_parallel, + parallel_grouped, + install_pms, + script_policy, + parent_group_open, + ]; + + let schema = serde_json::to_value(schemars::schema_for!(super::Overrides)) + .expect("Overrides schema should serialize"); + let top_properties = schema["properties"] + .as_object() + .expect("Overrides schema must have properties"); + let mut reported: std::collections::BTreeSet<&str> = + top_properties.keys().map(String::as_str).collect(); + + // Unnest OutputGrouping so its 3 fields match by their + // ResolutionOverrides names instead of living behind a + // container the resolver struct doesn't have. + reported.remove("output_grouping"); + let grouping_def = top_properties["output_grouping"]["$ref"] + .as_str() + .and_then(|r| r.strip_prefix("#/$defs/")) + .expect("output_grouping field must $ref a $defs entry"); + let grouping_properties = schema["$defs"][grouping_def]["properties"] + .as_object() + .unwrap_or_else(|| panic!("{grouping_def}: expected a properties object")); + reported.extend(grouping_properties.keys().map(String::as_str)); + + for field in resolution_overrides_fields { + if EXCLUDED.contains(&field) { + assert!( + !reported.contains(field), + "{field}: excluded field must not also appear in Overrides" + ); + continue; + } + let reported_name = RENAMED + .iter() + .find_map(|&(from, to)| (from == field).then_some(to)) + .unwrap_or(field); + assert!( + reported.contains(reported_name), + "{field}: ResolutionOverrides field is neither reported by Overrides (as \ + {reported_name:?}) nor on the EXCLUDED allowlist — add it to one" + ); + } + } + + /// The closed key set depends on `Ecosystem` variants carrying no doc + /// comments (see `src/types.rs`); a `///` there silently reverts the + /// map to open `additionalProperties`. This pins the shape. + #[cfg(feature = "schema")] + #[test] + fn pm_by_ecosystem_schema_keys_stay_closed() { + let schema = serde_json::to_value(schemars::schema_for!(super::Overrides)) + .expect("Overrides schema should serialize"); + let map_schema = &schema["properties"]["pm_by_ecosystem"]; + + assert_eq!( + map_schema["additionalProperties"], + serde_json::json!(false), + "pm_by_ecosystem must reject unknown keys" + ); + let keys: Vec<&str> = map_schema["properties"] + .as_object() + .expect("pm_by_ecosystem must enumerate its keys") + .keys() + .map(String::as_str) + .collect(); + let mut expected: Vec<&str> = Ecosystem::ALL.iter().map(|eco| eco.label()).collect(); + expected.sort_unstable(); + assert_eq!(keys, expected); + } } diff --git a/src/types.rs b/src/types.rs index 258c301d..00234e8e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -8,25 +8,58 @@ use std::path::PathBuf; /// in `runner.toml` applies only when resolving for [`Ecosystem::Node`]. /// Deno is its own ecosystem even though its package manager can also /// dispatch `package.json` scripts. +/// +/// Variants use `//`, not `///`: a per-variant doc comment defeats +/// `BTreeMap`'s closed-key-set schema optimization for `Overrides.pm_by_ecosystem`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum Ecosystem { - /// Node.js (npm, yarn, pnpm, bun). + // Node.js (npm, yarn, pnpm, bun). Node, - /// Deno. + // Deno. Deno, - /// Python (uv, poetry, pipenv). + // Python (uv, poetry, pipenv). Python, - /// Rust (cargo). + // Rust (cargo). Rust, - /// Go. + // Go. Go, - /// Ruby (bundler). + // Ruby (bundler). Ruby, - /// PHP (composer). + // PHP (composer). Php, } +/// Ordered by [`Self::label`] so `BTreeMap` keys serialize +/// alphabetically, matching the `String`-keyed flat `info`/`list` surface. +impl Ord for Ecosystem { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.label().cmp(other.label()) + } +} + +impl PartialOrd for Ecosystem { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl Ecosystem { + /// Every variant, for tests that need the closed set (schema + /// assertions, serialization drift tests). + #[cfg(test)] + pub(crate) const ALL: [Self; 7] = [ + Self::Node, + Self::Deno, + Self::Python, + Self::Rust, + Self::Go, + Self::Ruby, + Self::Php, + ]; + /// Lower-case label used in human messages, JSON output, and /// override origins. Single source of truth so `doctor --json` and /// resolver warnings agree on the spelling. @@ -44,7 +77,9 @@ impl Ecosystem { } /// A dependency manager detected via lockfile or config presence. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum PackageManager { /// npm — detected via `package-lock.json`. Npm, @@ -73,7 +108,9 @@ pub(crate) enum PackageManager { } /// A task runner detected via config file presence. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize)] +#[serde(rename_all = "kebab-case")] pub(crate) enum TaskRunner { /// Turborepo — detected via `turbo.json` / `turbo.jsonc`. Turbo, @@ -83,7 +120,11 @@ pub(crate) enum TaskRunner { Make, /// just — detected via case-insensitive `justfile` / `.justfile`. Just, - /// go-task — detected via `Taskfile.yml` and variants. + /// go-task — detected via `Taskfile.yml` and variants. Serializes as + /// `"task"` (matching [`Self::label`]) — `kebab-case` alone would + /// produce `"go-task"`, the accepted parse *alias*, not the canonical + /// label. + #[serde(rename = "task")] GoTask, /// mise — detected via `mise.toml` / `.mise.toml`. Mise, @@ -956,6 +997,46 @@ mod tests { use super::version_matches; use super::{DetectionWarning, PackageManager}; + /// The serde `kebab-case` renames and the hand-written `label()` + /// methods are parallel sources of the same strings; this pins them + /// together so a new variant can't silently split the two surfaces + /// (the way `TaskRunner::GoTask` would without its explicit rename). + #[test] + fn serialized_labels_match_label_methods() { + use super::{Ecosystem, TaskRunner}; + use crate::resolver::{FallbackPolicy, MismatchPolicy, ScriptPolicy}; + + fn json_str(value: T) -> String { + serde_json::to_value(value) + .expect("enum should serialize") + .as_str() + .expect("enum should serialize as a string") + .to_string() + } + + for eco in Ecosystem::ALL { + assert_eq!(json_str(eco), eco.label()); + } + for &pm in PackageManager::all() { + assert_eq!(json_str(pm), pm.label()); + } + for &runner in TaskRunner::all() { + assert_eq!(json_str(runner), runner.label()); + } + for fallback in FallbackPolicy::ALL { + assert_eq!(json_str(fallback), fallback.label()); + } + for mismatch in MismatchPolicy::ALL { + assert_eq!(json_str(mismatch), mismatch.label()); + } + for script in ScriptPolicy::SETTABLE { + assert_eq!(Some(json_str(script).as_str()), script.label()); + } + // Default has no user-settable label; the report surface still + // needs a stable spelling. + assert_eq!(json_str(ScriptPolicy::Default), "default"); + } + #[test] fn dotted_versions_match_segment_boundaries_only() { assert!(version_matches("20.11", "20.11.0"));