feat(schema)!: collapse doctor/why/list versioning to a single v1#82
Conversation
Not enough external adoption yet to justify carrying v1/v2/v3 compat simultaneously across doctor/why/list. Purge the old versions and treat the current shape as the sole, first version. - --schema-version now only accepts 1 (was 1..=3); doctor/why --json always emit the structured report that used to require --schema-version 3 (the v1/v2 flat shape is gone from both output paths). list/info --json keep the flat Project/TaskListView shape, now unversioned internally. - src/schema/v1.rs, v2.rs, v3.rs deleted; source-label dispatch collapses to labels::flat_source_label/structured_source_label (list vs doctor/why), no version parameter. - CURRENT_VERSION/DOCTOR_CURRENT_VERSION/WHY_CURRENT_VERSION collapse to a single schema::SCHEMA_VERSION = 1. - src/schema/doctor_v3.rs -> src/schema/doctor.rs; V3-suffixed structs across doctor.rs/why.rs drop the suffix (EcosystemV3 -> EcosystemEntry to avoid colliding with types::Ecosystem). - 10 superseded schemas/*.json deleted; the 3 kept files drop their version suffix (doctor.v3.schema.json -> doctor.schema.json, etc). Closes #80
📝 WalkthroughCollapse
Breaking: legacy flat JSON output shapes and versioned Validated with WalkthroughThis PR collapses the doctor, why, and list JSON schema split into a single Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as src/cli.rs
participant Lib as src/lib.rs
participant Cmd as src/cmd/doctor.rs
participant Schema as src/schema/doctor.rs
CLI->>Lib: --schema-version 1
Lib->>Cmd: doctor(ctx, overrides, json)
Cmd->>Schema: DoctorReport::build(...)
Cmd->>Cmd: serde_json::to_string_pretty(...)
sequenceDiagram
participant CmdSchema as src/cmd/schema.rs
participant Config as src/config.rs
participant Schemas as schemas/*.json
participant Template as schemas/runner.init.toml
CmdSchema->>Config: include_str!("../schemas/runner.init.toml")
CmdSchema->>Template: render_init_template()
CmdSchema->>Schemas: write committed artefacts
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: Comment |
- doctor(): fix stale doc comment claiming resolver errors propagate. Both the JSON path (DoctorReport::build) and the human path (Project::build_with_schema) embed the resolver Result in the report instead; the fn can only Err on JSON serialization failure. - schema.rs: PROVIDER_LABELS was a hand-maintained copy of cmd::why::provider_label, free to drift like the pre-#77 source_labels() array did. Derive it from provider_label instead (now pub(super)); add in-memory + committed-schema drift guard tests mirroring the existing TaskSourceLabel ones. - why.rs: the "filtered" decision reason always blamed "--runner/RUNNER_RUNNER restrictions", but a --runner/[task_runner].prefer restriction that empties the eligible set always errors out earlier via runner_constraint_error — the only reachable path to "filtered" is a qualifier (deno:x) matching no candidate's source. Thread the qualifier through build_report/decision_report so the reason names it correctly instead of blaming an override that was never set. Not fixed: the review also suggested build_report/decision_report should receive the runner/qualifier-restricted candidate set instead of the unfiltered one. Verified this would be wrong — lookup_token deliberately returns same-named tasks from other sources on a qualifier miss so why can explain *why* nothing was selected; narrowing report.candidates would hide that and collapse the "filtered" strategy into indistinguishable "exec-fallback". Fixed the actual bug (the reason text) instead.
Not just field-name coverage — the previous drift guard (#77) only checked that every RunnerConfig field's NAME appeared somewhere in the hand-typed INIT_TEMPLATE string. The accepted VALUES shown (which PMs, which policy labels) were still copy-pasted prose that could drift from what the resolver actually parses. - INIT_TEMPLATE is now include_str!'d from schemas/runner.init.toml, generated by cmd::schema::render_init_template() from RunnerConfig's schemars metadata: section order/descriptions come straight from the section structs' doc comments (RunnerConfig field order reshuffled to match the desired scaffold reading order — cosmetic, serde matches by name not position). - FIELD_TEMPLATE supplies the one thing schemars can't (which value to show commented-out) via a FieldHint enum: Static (booleans, open-ended arrays/maps), ClosedSet/Annotated (small closed vocabularies, hint generated from the real accepted-value set, not hand-typed). - accepted_labels()/broader_vocab() are the real source of truth per field: PackageManager::all() filtered by ecosystem for pm.node/ pm.python, TaskRunner::all() for task_runner.prefer, FallbackPolicy::ALL/MismatchPolicy::ALL/ScriptPolicy::SETTABLE for resolution.fallback/on_mismatch/install.scripts, task_source_labels() for tasks.prefer/overrides, PackageManager::all() (unfiltered) for install.pms. - FallbackPolicy/MismatchPolicy/ScriptPolicy gained real label()/ALL (SETTABLE for ScriptPolicy, since Default has no user-facing label) methods in resolver/types.rs. Their parse functions (resolver/policies.rs, resolver/overrides.rs) and the two display call sites (schema/project.rs, schema/doctor.rs) now delegate to them instead of four separately hardcoded copies of the same label strings. - Every FIELD_TEMPLATE example value is validated (parsed as TOML, every string leaf checked) against the real accepted/broader vocabulary — an example using a value the resolver would actually reject is now a build-time panic, not a silent lie in the scaffold. - Sabotage-verified: a variant added to a policy enum without an Annotated entry, and an example value outside the real accepted set, both fail loudly.
render_init_template() panics by design on FIELD_TEMPLATE/RunnerConfig drift (a hard failure is right for the drift-guard test that normally catches this pre-merge) — but write_all_schemas called it unconditionally, so a released binary hitting that drift would crash `runner schema --all` with a raw, unhandled panic instead of a normal error exit. checked_init_template() wraps the call in catch_unwind with the default panic hook suppressed (no backtrace noise), converting a caught panic into an anyhow::Error with the same message. Sabotage-verified: an out-of-vocabulary FIELD_TEMPLATE value now exits 1 with a clean 'Error: ...' line instead of a panic backtrace. Not fixed here: the same review also flagged doctor.rs's Overrides struct missing prefer_sources/task_source_overrides/failure_policy/ group_output/github_group_parallel/parallel_grouped/install_pms/ script_policy/parent_group_open. That's real on this branch alone, but it's exactly issue #81's scope, already fully implemented (all 9 of those fields, plus a structural drift-guard test) on the stacked branch issue-81-overrides-fields (PR #83, based on this one) — duplicating it here would conflict when #83 rebases after this merges.
…paths checked_init_template's downcast_ref chain now lives in a standalone panic_message() fn, unit-tested for &str/String/fallback payloads. .zed/settings.json: doctor/list/why schema mappings pointed at deleted v1/v2/v3 files (collapsed to unversioned in 2d397fe). Point at the current files, drop the dead entries.
…chema pragma render_init_template() now skips printing sections schemars marks deprecated (task_runner) entirely, instead of emitting an active empty table new users would never want. FIELD_TEMPLATE validation still runs against it, just not the printing. schemas/runner.init.toml keeps its own repo-relative #:schema pragma (tombi.toml now associates it via [[schemas]].include); cmd::config::init() strips that pragma and substitutes the real published URL instead of stacking both when writing a user's runner.toml. KNOWN_SCHEMA/INIT_TEMPLATE drift-guard test updated: deprecated sections are recognized by KNOWN_SCHEMA (back-compat parsing) without being in the scaffold.
Closes #80.
Not enough external adoption yet to justify carrying v1/v2/v3 compat
simultaneously across
doctor/why/list. Purges the old versions,treats today's shape as the sole, first version.
Breaking
--schema-versionnow only accepts1(was1..=3).doctor --json/why --jsonalways emit the structured report thatused to require
--schema-version 3; the flat v1/v2 shape is gonefrom both output paths.
Changes
src/schema/v1.rs/v2.rs/v3.rsdeleted; source-label dispatchcollapses to
labels::flat_source_label/structured_source_label(list vs doctor/why), no version parameter anywhere.
CURRENT_VERSION/DOCTOR_CURRENT_VERSION/WHY_CURRENT_VERSIONcollapse to a single
schema::SCHEMA_VERSION = 1.src/schema/doctor_v3.rs->src/schema/doctor.rs;V3-suffixedstructs across
doctor.rs/why.rsdrop the suffix.schemas/*.jsondeleted; the 3 kept files drop theirversion suffix (
doctor.v3.schema.json->doctor.schema.json, etc).Test plan
cargo test --all-features(879 unit + all integration suites green)cargo clippy --all-targets --all-features -- -D warnings -D clippy::allcleancargo fmt --checkcleancargo run --features schema -- schema --all --output schemasis a no-op against the committed files (drift guard)