Skip to content

feat(schema): surface all ResolutionOverrides fields in doctor --json#83

Merged
kjanat merged 11 commits into
masterfrom
issue-81-overrides-fields
Jul 5, 2026
Merged

feat(schema): surface all ResolutionOverrides fields in doctor --json#83
kjanat merged 11 commits into
masterfrom
issue-81-overrides-fields

Conversation

@kjanat

@kjanat kjanat commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #81. Stacked on #82 (base branch issue-80-schema-v1-purge) — the
renamed Overrides/DoctorReport types this PR touches only exist after
that one merges.

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,
exactly the drift #77 flagged with quiet. Adds 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 field left out: internal runner-to-runner
plumbing (an inherited env marker), never a user override.

Adds 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.

Test plan

  • cargo test --all-features (881 unit + all integration suites green)
  • cargo clippy --all-targets --all-features -- -D warnings -D clippy::all clean
  • cargo fmt --check clean
  • Sabotage-then-restore verified the new drift guard test actually fails on an unreported/unexcluded field
  • cargo run --features schema -- schema --all --output schemas is idempotent against the committed files

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
@coderabbitai

This comment was marked as outdated.

kjanat added a commit that referenced this pull request Jul 5, 2026
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.
Base automatically changed from issue-80-schema-v1-purge to master July 5, 2026 06:40
@kjanat kjanat changed the title feat(schema): surface all ResolutionOverrides fields in doctor --json feat(schema): surface all ResolutionOverrides fields in doctor --json Jul 5, 2026
@kjanat kjanat added the cr:review Allow CodeRabbit review label Jul 5, 2026
@coderabbitai coderabbitai Bot added area: cli Argument parsing, commands, and CLI UX enhancement New feature or request labels Jul 5, 2026
coderabbitai[bot]

This comment was marked as resolved.

kjanat added 2 commits July 5, 2026 08:51
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.
@kjanat kjanat self-assigned this Jul 5, 2026
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.
coderabbitai[bot]

This comment was marked as outdated.

kjanat added 5 commits July 5, 2026 10:02
… 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.
…-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).
Missing entries for 113fd1f (quiet required) and 93cf067 (enum-typed
Overrides fields instead of generic strings).
You were right to push on this -- schemars 1.2's BTreeMap<K, V> 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).
@coderabbitai coderabbitai Bot added the documentation Improvements or additions to documentation label Jul 5, 2026
coderabbitai[bot]

This comment was marked as resolved.

@kjanat kjanat removed the cr:review Allow CodeRabbit review label Jul 5, 2026
kjanat added 2 commits July 5, 2026 12:00
…ngelog 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.
- 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
@kjanat kjanat merged commit 3945f34 into master Jul 5, 2026
18 checks passed
@kjanat kjanat deleted the issue-81-overrides-fields branch July 5, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Argument parsing, commands, and CLI UX documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OverridesV3 is missing 5 ResolutionOverrides fields

1 participant