From 984a65e0ea9dd77d794098b892490311eecdcad8 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 03:23:53 +0200 Subject: [PATCH 01/10] feat(schema): surface all ResolutionOverrides fields in doctor --json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OverridesV3 (now Overrides) only carried pm/runner/prefer_runners/ fallback/on_mismatch/explain/no_warnings/quiet — a field could land on ResolutionOverrides and CLI/env/config but never reach doctor --json, which is exactly the drift #77 flagged with quiet. Add the rest: - prefer_sources ([tasks].prefer) - task_source_pins ([tasks.overrides]; reported name avoids clippy's struct_field_names lint on task_source_overrides) - failure_policy (-k/-K / RUNNER_KEEP_GOING/RUNNER_KILL_ON_FAIL / [chain]) - output_grouping.{group_output,github_group_parallel,parallel_grouped} ([github].group_output/group_parallel, [parallel].grouped; nested to keep Overrides under clippy's struct_excessive_bools threshold) - install_pms (RUNNER_INSTALL_PMS / [install].pms) - script_policy (--no-scripts/--scripts / RUNNER_INSTALL_SCRIPTS / [install].scripts) parent_group_open is the one ResolutionOverrides field left out: internal runner-to-runner plumbing (an inherited env marker), never a user override. Added a drift guard test (every_resolution_overrides_field_is_reported_or_excluded) mirroring #77's KNOWN_SCHEMA-vs-RunnerConfig structural check: every ResolutionOverrides field must appear in Overrides or on its exclusion list, so a future field can't silently miss both again. Closes #81 --- CHANGELOG.md | 11 ++ schemas/doctor.example.json | 18 +++- schemas/doctor.schema.json | 64 +++++++++++- src/schema/doctor.rs | 197 +++++++++++++++++++++++++++++++++++- 4 files changed, 281 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54035b4d..623ea3ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,17 @@ 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: + `failure_policy`, `install_pms`, `output_grouping` + (`group_output`/`github_group_parallel`/`parallel_grouped`), + `prefer_sources`, `script_policy`, and `task_source_pins`. Previously + only `pm`/`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 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..4a07577b 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -300,32 +300,74 @@ }, "additionalProperties": false }, + "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", + "runner", + "script_policy", + "task_source_pins" ], "properties": { "explain": { "type": "boolean" }, + "failure_policy": { + "type": "string" + }, "fallback": { "type": "string" }, + "install_pms": { + "type": "array", + "items": { + "type": "string" + } + }, "no_warnings": { "type": "boolean" }, "on_mismatch": { "type": "string" }, + "output_grouping": { + "$ref": "#/$defs/OutputGrouping" + }, "pm": { "type": [ "null", @@ -347,6 +389,12 @@ "type": "string" } }, + "prefer_sources": { + "type": "array", + "items": { + "type": "string" + } + }, "quiet": { "type": "boolean" }, @@ -355,6 +403,18 @@ "null", "string" ] + }, + "script_policy": { + "type": "string" + }, + "task_source_pins": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": false diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index 340fa181..4d98ea78 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -32,9 +32,10 @@ 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::{ - FallbackPolicy, MismatchPolicy, ResolutionOverrides, ResolutionStep, Resolver, + FallbackPolicy, MismatchPolicy, ResolutionOverrides, ResolutionStep, Resolver, ScriptPolicy, }; use crate::tool::node::detect_pm_from_manifest; use crate::types::{DetectionWarning, Ecosystem, PackageManager, ProjectContext, Task, TaskSource}; @@ -145,21 +146,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, + failure_policy: &'static str, + install_pms: Vec<&'static str>, 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>, + prefer_sources: Vec<&'static str>, runner: Option<&'static str>, + script_policy: &'static str, + 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. @@ -503,7 +532,18 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { FallbackPolicy::Npm => "npm", FallbackPolicy::Error => "error", }, + failure_policy: match overrides.failure_policy { + FailurePolicy::FailFast => "fail-fast", + FailurePolicy::KeepGoing => "keep-going", + FailurePolicy::KillOnFail => "kill-on-fail", + }, + install_pms: overrides.install_pms.iter().map(|pm| pm.label()).collect(), 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: match overrides.on_mismatch { MismatchPolicy::Warn => "warn", @@ -517,7 +557,30 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { .map(|(eco, o)| (eco.label().to_string(), Some(o.pm.label()))) .collect(), prefer_runners: overrides.prefer_runners.iter().map(|r| r.label()).collect(), + prefer_sources: overrides + .prefer_sources + .iter() + .map(|&source| structured_source_label(source)) + .collect(), runner: overrides.runner.as_ref().map(|o| o.runner.label()), + script_policy: match overrides.script_policy { + ScriptPolicy::Default => "default", + ScriptPolicy::Deny => "deny", + ScriptPolicy::Allow => "allow", + }, + task_source_pins: overrides + .task_source_overrides + .iter() + .map(|(name, sources)| { + ( + name.clone(), + sources + .iter() + .map(|&source| structured_source_label(source)) + .collect(), + ) + }) + .collect(), } } @@ -1259,4 +1322,132 @@ 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). `RESOLUTION_OVERRIDES_FIELDS` is the + /// necessarily hand-maintained side — `ResolutionOverrides` has no + /// `JsonSchema` derive of its own (it's an internal resolver type, + /// not a JSON-facing one) — so it must be kept in sync with + /// `src/resolver/types.rs` by hand; this test only catches drift + /// against `Overrides`, not against the struct itself. + /// + /// 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() { + const RESOLUTION_OVERRIDES_FIELDS: &[&str] = &[ + "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", + ]; + // 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")]; + + 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" + ); + } + } } From fe6e90bef5ad6d1dd9a81207c3bc1755ff4f3cdd Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 08:54:01 +0200 Subject: [PATCH 02/10] docs(changelog): correct overrides changelog wording Note parent_group_open's exclusion explicitly, add pm_by_ecosystem to the "previously surfaced" list (it predates this PR, isn't new). Skipped the schemas/doctor.schema.json required-list finding: quiet staying out of required is deliberate (remove_required_def_field, back-compat), already guarded by two tests. --- CHANGELOG.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33025bc1..4a932425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,12 +17,13 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic ### Added -- `doctor --json` `overrides` now reports every resolver override state: - `failure_policy`, `install_pms`, `output_grouping` +- `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`/`runner`/`prefer_runners`/`fallback`/`on_mismatch`/ - `explain`/`no_warnings`/`quiet` were surfaced, so `-k`/`-K`, + 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. From 113fd1f56e533001a1296e94ac2e349d858cafef Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 09:07:28 +0200 Subject: [PATCH 03/10] fix(schema): make quiet required in Overrides, drop dead v3-compat shim remove_required_def_field/patch_schema_compat kept quiet optional for compat with doctor v3, which 2d397fe already deleted (schema-version now rejects anything but 1). No compat boundary left to preserve. Removed the shim and its two tests; quiet now required like every other Overrides field. --- schemas/doctor.schema.json | 1 + src/cmd/schema.rs | 68 -------------------------------------- 2 files changed, 1 insertion(+), 68 deletions(-) diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 4a07577b..35fce356 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -339,6 +339,7 @@ "pm_by_ecosystem", "prefer_runners", "prefer_sources", + "quiet", "runner", "script_policy", "task_source_pins" diff --git a/src/cmd/schema.rs b/src/cmd/schema.rs index c098ab1c..1e337415 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) } @@ -593,28 +592,6 @@ fn patch_source_schema(schema: &mut Value, command: &str) { patch_def_field(defs, "SourceEntry", "kind", "TaskSourceLabel"); } -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 { - return; - }; - required.retain(|name| name.as_str() != Some(field)); -} - fn patch_task_info_source(defs: &mut Map) { patch_def_field(defs, "TaskInfo", "source", "TaskSourceLabel"); } @@ -716,51 +693,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") From 259dbc758a3ff8fdfd8fadd45577e542e93cedeb Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 10:02:19 +0200 Subject: [PATCH 04/10] fix(schema): co-locate failure_policy/script_policy labels, constrain source labels FailurePolicy::label() and ScriptPolicy::report_label() (Default included, unlike the settable-input label()) replace inline matches in overrides_report, matching fallback/on_mismatch's existing .label() use. Overrides.prefer_sources/task_source_pins now $ref TaskSourceLabel (same closed set SourceEntry.kind already uses) instead of plain strings. Skipped: turning failure_policy/script_policy into typed schema enums. Overrides has 6 other equally-closed-set label fields (fallback, on_mismatch, pm, runner, prefer_runners, install_pms) that stay plain strings by existing convention; enum-typing 2 of 8 would make the struct's own schema inconsistent rather than fix anything. A full switch is a separate, deliberate call, not a minimal patch. --- schemas/doctor.schema.json | 4 +-- src/chain/mod.rs | 11 ++++++++ src/cmd/schema.rs | 53 ++++++++++++++++++++++++++++++++++++++ src/resolver/types.rs | 12 +++++++++ src/schema/doctor.rs | 15 +++-------- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 35fce356..03b2cf3a 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -393,7 +393,7 @@ "prefer_sources": { "type": "array", "items": { - "type": "string" + "$ref": "#/$defs/TaskSourceLabel" } }, "quiet": { @@ -413,7 +413,7 @@ "additionalProperties": { "type": "array", "items": { - "type": "string" + "$ref": "#/$defs/TaskSourceLabel" } } } diff --git a/src/chain/mod.rs b/src/chain/mod.rs index b44ad673..316a5615 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -87,6 +87,17 @@ pub(crate) enum FailurePolicy { KillOnFail, } +impl FailurePolicy { + /// The user-facing label — same convention as [`crate::resolver::FallbackPolicy::label`]. + pub(crate) const fn label(self) -> &'static str { + match self { + Self::FailFast => "fail-fast", + Self::KeepGoing => "keep-going", + Self::KillOnFail => "kill-on-fail", + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/cmd/schema.rs b/src/cmd/schema.rs index 1e337415..9bf5c6e5 100644 --- a/src/cmd/schema.rs +++ b/src/cmd/schema.rs @@ -590,6 +590,18 @@ 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); +} + +/// `Overrides.prefer_sources`/`task_source_pins` hold the same structured +/// source labels as `SourceEntry.kind` — constrain them to `TaskSourceLabel` +/// too instead of leaving them generic strings. +fn patch_overrides_source_labels(defs: &mut Map) { + if !defs.contains_key("Overrides") { + return; + } + 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) { @@ -628,6 +640,47 @@ fn patch_def_field( *field_schema = json!({ "$ref": format!("#/$defs/{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, +) { + let Some(items) = 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)) + .and_then(|field_schema| field_schema.get_mut("items")) + else { + return; + }; + *items = json!({ "$ref": format!("#/$defs/{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, +) { + let Some(items) = 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)) + .and_then(|field_schema| field_schema.get_mut("additionalProperties")) + .and_then(|additional| additional.get_mut("items")) + else { + return; + }; + *items = json!({ "$ref": format!("#/$defs/{target_def}") }); +} + fn task_source_label_schema(command: &str) -> Value { json!({ "type": "string", "enum": source_labels(command) }) } diff --git a/src/resolver/types.rs b/src/resolver/types.rs index 43b59f26..8b08b6d9 100644 --- a/src/resolver/types.rs +++ b/src/resolver/types.rs @@ -208,6 +208,18 @@ impl ScriptPolicy { Self::Allow => Some("allow"), } } + + /// The label for every variant, including [`Self::Default`] — for + /// reporting the *effective* state (e.g. `doctor --json`), where + /// "default" is a real value to describe, unlike [`Self::label`]'s + /// settable-input vocabulary. + pub(crate) const fn report_label(self) -> &'static str { + match self { + Self::Default => "default", + Self::Deny => "deny", + Self::Allow => "allow", + } + } } /// How to react when manifest declaration (step 5) and lockfile (step 6) diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index b495398a..9ab74633 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -32,9 +32,8 @@ 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, ScriptPolicy}; +use crate::resolver::{ResolutionOverrides, ResolutionStep, Resolver}; use crate::tool::node::detect_pm_from_manifest; use crate::types::{DetectionWarning, Ecosystem, PackageManager, ProjectContext, Task, TaskSource}; @@ -526,11 +525,7 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { Overrides { explain: overrides.explain, fallback: overrides.fallback.label(), - failure_policy: match overrides.failure_policy { - FailurePolicy::FailFast => "fail-fast", - FailurePolicy::KeepGoing => "keep-going", - FailurePolicy::KillOnFail => "kill-on-fail", - }, + failure_policy: overrides.failure_policy.label(), install_pms: overrides.install_pms.iter().map(|pm| pm.label()).collect(), no_warnings: overrides.no_warnings, output_grouping: OutputGrouping { @@ -553,11 +548,7 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { .map(|&source| structured_source_label(source)) .collect(), runner: overrides.runner.as_ref().map(|o| o.runner.label()), - script_policy: match overrides.script_policy { - ScriptPolicy::Default => "default", - ScriptPolicy::Deny => "deny", - ScriptPolicy::Allow => "allow", - }, + script_policy: overrides.script_policy.report_label(), task_source_pins: overrides .task_source_overrides .iter() From 93cf0678cbe12d169a81b1ecb1aaa618c4270748 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 10:16:13 +0200 Subject: [PATCH 05/10] fix(schema): derive Overrides label schemas from real enums, not hand-built JSON Reworked the previous commit's approach. FailurePolicy, FallbackPolicy, MismatchPolicy, ScriptPolicy, PackageManager, TaskRunner now derive schemars::JsonSchema + Serialize (kebab-case, matching each .label()'s existing wire strings; TaskRunner::GoTask needs an explicit rename to "task" since kebab-case alone gives "go-task", the parse alias not the canonical label). Overrides' fields hold these enums directly instead of flattened &'static str/Option<&'static str>/Vec<&'static str>, so schemars derives each field's real oneOf/enum constraint automatically -- no hand-written json!() schema fragments, no patch_def_* plumbing for them. Removed the now-fully-redundant FailurePolicy::label/ALL and ScriptPolicy::ALL/ report_label added in the prior commit for that purpose. TaskSourceLabel-based patching stays for prefer_sources/task_source_pins -- those vary per command (doctor/why structured vs list flat), so they still need the runtime label-function-driven $ref, unlike the other fields which are simple closed enums schemars can derive standalone. Wire format (doctor --json's overrides object) is byte-identical; verified via doctor --json and the committed example fixture (no diff). --- schemas/doctor.schema.json | 227 ++++++++++++++++++++++++++++++++++--- src/chain/mod.rs | 15 +-- src/cmd/schema.rs | 7 +- src/resolver/types.rs | 24 ++-- src/schema/doctor.rs | 45 ++++---- src/types.rs | 14 ++- 6 files changed, 265 insertions(+), 67 deletions(-) diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 03b2cf3a..051030c3 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,6 +340,26 @@ }, "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", @@ -349,45 +409,53 @@ "type": "boolean" }, "failure_policy": { - "type": "string" + "$ref": "#/$defs/FailurePolicy" }, "fallback": { - "type": "string" + "$ref": "#/$defs/FallbackPolicy" }, "install_pms": { "type": "array", "items": { - "type": "string" + "$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" + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } ] } }, "prefer_runners": { "type": "array", "items": { - "type": "string" + "$ref": "#/$defs/TaskRunner" } }, "prefer_sources": { @@ -400,13 +468,17 @@ "type": "boolean" }, "runner": { - "type": [ - "null", - "string" + "anyOf": [ + { + "$ref": "#/$defs/TaskRunner" + }, + { + "type": "null" + } ] }, "script_policy": { - "type": "string" + "$ref": "#/$defs/ScriptPolicy" }, "task_source_pins": { "type": "object", @@ -420,6 +492,71 @@ }, "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", @@ -522,6 +659,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", @@ -577,6 +734,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 316a5615..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. @@ -87,17 +89,6 @@ pub(crate) enum FailurePolicy { KillOnFail, } -impl FailurePolicy { - /// The user-facing label — same convention as [`crate::resolver::FallbackPolicy::label`]. - pub(crate) const fn label(self) -> &'static str { - match self { - Self::FailFast => "fail-fast", - Self::KeepGoing => "keep-going", - Self::KillOnFail => "kill-on-fail", - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/cmd/schema.rs b/src/cmd/schema.rs index 9bf5c6e5..9f12b5d0 100644 --- a/src/cmd/schema.rs +++ b/src/cmd/schema.rs @@ -594,8 +594,11 @@ fn patch_source_schema(schema: &mut Value, command: &str) { } /// `Overrides.prefer_sources`/`task_source_pins` hold the same structured -/// source labels as `SourceEntry.kind` — constrain them to `TaskSourceLabel` -/// too instead of leaving them generic strings. +/// 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; diff --git a/src/resolver/types.rs b/src/resolver/types.rs index 8b08b6d9..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 @@ -208,18 +212,6 @@ impl ScriptPolicy { Self::Allow => Some("allow"), } } - - /// The label for every variant, including [`Self::Default`] — for - /// reporting the *effective* state (e.g. `doctor --json`), where - /// "default" is a real value to describe, unlike [`Self::label`]'s - /// settable-input vocabulary. - pub(crate) const fn report_label(self) -> &'static str { - match self { - Self::Default => "default", - Self::Deny => "deny", - Self::Allow => "allow", - } - } } /// How to react when manifest declaration (step 5) and lockfile (step 6) @@ -231,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 9ab74633..e55ae4e1 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))] @@ -155,19 +160,19 @@ struct ProjectInfo { #[cfg_attr(feature = "schema", schemars(deny_unknown_fields))] struct Overrides { explain: bool, - fallback: &'static str, - failure_policy: &'static str, - install_pms: Vec<&'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>, + on_mismatch: MismatchPolicy, + pm: Option, + pm_by_ecosystem: BTreeMap>, + prefer_runners: Vec, prefer_sources: Vec<&'static str>, - runner: Option<&'static str>, - script_policy: &'static str, + runner: Option, + script_policy: ScriptPolicy, task_source_pins: BTreeMap>, } @@ -524,9 +529,9 @@ fn runner_info() -> RunnerInfo { fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { Overrides { explain: overrides.explain, - fallback: overrides.fallback.label(), - failure_policy: overrides.failure_policy.label(), - install_pms: overrides.install_pms.iter().map(|pm| pm.label()).collect(), + 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, @@ -534,21 +539,21 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { 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.label().to_string(), Some(o.pm))) .collect(), - prefer_runners: overrides.prefer_runners.iter().map(|r| r.label()).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.label()), - script_policy: overrides.script_policy.report_label(), + runner: overrides.runner.as_ref().map(|o| o.runner), + script_policy: overrides.script_policy, task_source_pins: overrides .task_source_overrides .iter() diff --git a/src/types.rs b/src/types.rs index 258c301d..1376dc4a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -44,7 +44,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 +75,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 +87,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, From 0fb4dffa6428b3f29b3afb557d7159354b4e97e6 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 11:03:30 +0200 Subject: [PATCH 06/10] docs(changelog): document Overrides schema tightening Missing entries for 113fd1f (quiet required) and 93cf067 (enum-typed Overrides fields instead of generic strings). --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a932425..ec7ad99a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,12 +56,23 @@ 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`, `failure_policy`, + `script_policy`, `pm`, `pm_by_ecosystem`, `runner`, `prefer_runners`, and + `install_pms` 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. + ### 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 From 624dc1191e219b6d0d9124483bf9e070cb251626 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 11:11:56 +0200 Subject: [PATCH 07/10] fix(schema): constrain pm_by_ecosystem's keys too, not just its values You were right to push on this -- schemars 1.2's BTreeMap impl does constrain keys automatically when K's schema is the flat {"type":"string","enum":[...]} shape. It wasn't kicking in because Ecosystem's per-variant /// doc comments made schemars emit oneOf+const instead (schemars_derive treats any doc'd variant as "complex"). Doc comments moved to // (kept, just not schema-visible) and Ecosystem gained the same Serialize/JsonSchema/kebab-case treatment as the other enums this session, plus Ord (needed as a BTreeMap key). Overrides.pm_by_ecosystem now keys by Ecosystem directly; doctor.rs constructs it without the .label().to_string() detour. Schema gets explicit per-ecosystem properties + additionalProperties: false instead of a bare object. Wire output unchanged (verified via doctor --json and the committed example fixture). --- schemas/doctor.schema.json | 83 +++++++++++++++++++++++++++++++++----- src/schema/doctor.rs | 4 +- src/types.rs | 22 ++++++---- 3 files changed, 89 insertions(+), 20 deletions(-) diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 051030c3..22549495 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -441,16 +441,79 @@ }, "pm_by_ecosystem": { "type": "object", - "additionalProperties": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] - } + "properties": { + "deno": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "go": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "node": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "php": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "python": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "ruby": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + }, + "rust": { + "anyOf": [ + { + "$ref": "#/$defs/PackageManager" + }, + { + "type": "null" + } + ] + } + }, + "additionalProperties": false }, "prefer_runners": { "type": "array", diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index e55ae4e1..877d2b1e 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -168,7 +168,7 @@ struct Overrides { quiet: bool, on_mismatch: MismatchPolicy, pm: Option, - pm_by_ecosystem: BTreeMap>, + pm_by_ecosystem: BTreeMap>, prefer_runners: Vec, prefer_sources: Vec<&'static str>, runner: Option, @@ -544,7 +544,7 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { pm_by_ecosystem: overrides .pm_by_ecosystem .iter() - .map(|(eco, o)| (eco.label().to_string(), Some(o.pm))) + .map(|(&eco, o)| (eco, Some(o.pm))) .collect(), prefer_runners: overrides.prefer_runners.clone(), prefer_sources: overrides diff --git a/src/types.rs b/src/types.rs index 1376dc4a..cc463a33 100644 --- a/src/types.rs +++ b/src/types.rs @@ -8,21 +8,27 @@ 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. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +/// +/// 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, PartialOrd, Ord, 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, } From 2f19b01b83e9dc228bd3f87bf823e2415ab5306d Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 11:12:16 +0200 Subject: [PATCH 08/10] docs(changelog): note pm_by_ecosystem key constraint --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec7ad99a..9752f547 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,9 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic 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. ### Fixed From 0b7408eb5baac77c767435042fdfcef21e8cec69 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 12:00:48 +0200 Subject: [PATCH 09/10] fix(schema): drift-proof the ResolutionOverrides field guard, fix changelog wording every_resolution_overrides_field_is_reported_or_excluded now destructures ResolutionOverrides::default() with no `..` -- a new struct field fails compilation right there instead of the hand-maintained RESOLUTION_OVERRIDES_FIELDS list silently staying stale. Verified via a sabotage field (E0027 fires as expected), reverted. CHANGELOG: failure_policy/script_policy/install_pms are new fields from this PR, not pre-existing generic strings tightened into enums -- moved them out of that framing. --- CHANGELOG.md | 19 ++++++++++--------- src/schema/doctor.rs | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9752f547..403d45c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,15 +56,16 @@ 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`, `failure_policy`, - `script_policy`, `pm`, `pm_by_ecosystem`, `runner`, `prefer_runners`, and - `install_pms` 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. +- `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. `failure_policy`, `script_policy`, and + `install_pms` (new fields, see Added above) get the same closed-enum + treatment from the start. ### Fixed diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index 877d2b1e..b4b9c49c 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -1396,6 +1396,30 @@ mod tests { // resolver field name -> name it's actually reported under. const RENAMED: &[(&str, &str)] = &[("task_source_overrides", "task_source_pins")]; + // Exhaustive destructure (no `..`): if ResolutionOverrides gains a + // field, this fails to compile until it's added here too, instead + // of RESOLUTION_OVERRIDES_FIELDS above silently staying stale. + let ResolutionOverrides { + 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: _, + } = ResolutionOverrides::default(); + let schema = serde_json::to_value(schemars::schema_for!(super::Overrides)) .expect("Overrides schema should serialize"); let top_properties = schema["properties"] From 9e26a946c0fea3701e4f126d00733af0747e6458 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 5 Jul 2026 12:28:46 +0200 Subject: [PATCH 10/10] fix(schema): close review gaps in Overrides reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - drift guard: one macro list now both destructures ResolutionOverrides and feeds the checked names — the array can no longer go stale behind the compile-time check (sabotage-verified both directions) - pin pm_by_ecosystem's generated schema shape (7 closed keys, additionalProperties:false) so a stray /// on an Ecosystem variant can't silently reopen the key set - pin serde kebab-case output to label() for Ecosystem/PackageManager/ TaskRunner/FallbackPolicy/MismatchPolicy/ScriptPolicy (GoTask-style drift is now caught by test, not author vigilance) - Ecosystem: manual Ord by label — doctor pm_by_ecosystem keys stay alphabetical (pre-existing order, matches info/list surface) instead of flipping to declaration order - drop the never-None Option on pm_by_ecosystem values; schema loses the bogus null branch on all seven keys - schema.rs: field_schema_mut/def_ref consolidate the three patch fns' copy-pasted navigation --- CHANGELOG.md | 7 ++- schemas/doctor.schema.json | 63 +++----------------- src/cmd/schema.rs | 55 +++++++++--------- src/schema/doctor.rs | 114 ++++++++++++++++++++----------------- src/types.rs | 69 +++++++++++++++++++++- 5 files changed, 169 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 403d45c0..52b39353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,9 +63,10 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic 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. `failure_policy`, `script_policy`, and - `install_pms` (new fields, see Added above) get the same closed-enum - treatment from the start. + 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 diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 22549495..f532f33c 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -443,74 +443,25 @@ "type": "object", "properties": { "deno": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "go": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "node": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "php": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "python": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "ruby": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" }, "rust": { - "anyOf": [ - { - "$ref": "#/$defs/PackageManager" - }, - { - "type": "null" - } - ] + "$ref": "#/$defs/PackageManager" } }, "additionalProperties": false diff --git a/src/cmd/schema.rs b/src/cmd/schema.rs index 9f12b5d0..5d9422f1 100644 --- a/src/cmd/schema.rs +++ b/src/cmd/schema.rs @@ -626,21 +626,32 @@ 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 @@ -651,16 +662,11 @@ fn patch_def_array_items( field: &'static str, target_def: &'static str, ) { - let Some(items) = 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)) + if let Some(items) = field_schema_mut(defs, def_name, field) .and_then(|field_schema| field_schema.get_mut("items")) - else { - return; - }; - *items = json!({ "$ref": format!("#/$defs/{target_def}") }); + { + *items = def_ref(target_def); + } } /// Like [`patch_def_array_items`], but for a map-of-array field — constrains @@ -671,17 +677,12 @@ fn patch_def_map_array_items( field: &'static str, target_def: &'static str, ) { - let Some(items) = 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)) + 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")) - else { - return; - }; - *items = json!({ "$ref": format!("#/$defs/{target_def}") }); + { + *items = def_ref(target_def); + } } fn task_source_label_schema(command: &str) -> Value { diff --git a/src/schema/doctor.rs b/src/schema/doctor.rs index b4b9c49c..171e6303 100644 --- a/src/schema/doctor.rs +++ b/src/schema/doctor.rs @@ -168,7 +168,7 @@ struct Overrides { quiet: bool, on_mismatch: MismatchPolicy, pm: Option, - pm_by_ecosystem: BTreeMap>, + pm_by_ecosystem: BTreeMap, prefer_runners: Vec, prefer_sources: Vec<&'static str>, runner: Option, @@ -544,7 +544,7 @@ fn overrides_report(overrides: &ResolutionOverrides) -> Overrides { pm_by_ecosystem: overrides .pm_by_ecosystem .iter() - .map(|(&eco, o)| (eco, Some(o.pm))) + .map(|(&eco, o)| (eco, o.pm)) .collect(), prefer_runners: overrides.prefer_runners.clone(), prefer_sources: overrides @@ -1352,12 +1352,10 @@ mod tests { /// Drift guard: every field on [`ResolutionOverrides`] must appear /// either in the reflected [`Overrides`] schema or in the exclusion - /// list below (with a reason). `RESOLUTION_OVERRIDES_FIELDS` is the - /// necessarily hand-maintained side — `ResolutionOverrides` has no - /// `JsonSchema` derive of its own (it's an internal resolver type, - /// not a JSON-facing one) — so it must be kept in sync with - /// `src/resolver/types.rs` by hand; this test only catches drift - /// against `Overrides`, not against the struct itself. + /// 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: @@ -1370,55 +1368,41 @@ mod tests { #[cfg(feature = "schema")] #[test] fn every_resolution_overrides_field_is_reported_or_excluded() { - const RESOLUTION_OVERRIDES_FIELDS: &[&str] = &[ - "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", - ]; // 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")]; - // Exhaustive destructure (no `..`): if ResolutionOverrides gains a - // field, this fails to compile until it's added here too, instead - // of RESOLUTION_OVERRIDES_FIELDS above silently staying stale. - let ResolutionOverrides { - 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: _, - } = ResolutionOverrides::default(); + // 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"); @@ -1441,7 +1425,7 @@ mod tests { .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 { + for field in resolution_overrides_fields { if EXCLUDED.contains(&field) { assert!( !reported.contains(field), @@ -1460,4 +1444,30 @@ mod tests { ); } } + + /// 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 cc463a33..00234e8e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -11,7 +11,7 @@ use std::path::PathBuf; /// /// 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, PartialOrd, Ord, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[derive(serde::Serialize)] #[serde(rename_all = "kebab-case")] @@ -32,7 +32,34 @@ pub(crate) enum Ecosystem { 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. @@ -970,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"));