Skip to content

fix(cli): report the updates fix declined instead of claiming none exist - #108

Open
justin13888 wants to merge 9 commits into
refactor/92-ecosystem-aware-wildcardfrom
fix/93-report-declined-updates
Open

fix(cli): report the updates fix declined instead of claiming none exist#108
justin13888 wants to merge 9 commits into
refactor/92-ecosystem-aware-wildcardfrom
fix/93-report-declined-updates

Conversation

@justin13888

@justin13888 justin13888 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

dependable check reported an available update to "lodash": "1.x" while dependable fix printed Everything is already up to date. for the same manifest, and fix --dry-run printed nothing at all. rewrite_constraint declined the wildcard, plan_fixes emitted no FixRecord, and run_fix had only two outcomes — records printed, or the flat "already up to date" line — so a declined-but-updatable item was indistinguishable from nothing to do.

Closes #93

Stacked

Base is refactor/92-ecosystem-aware-wildcard (#106), which is itself stacked on fix/stabilization-pass (#99). Neither is merged. Do not merge this before #99 and #106; retarget to master once they land.

#106 is the direct prerequisite: it made the wildcard decline conditional on the ecosystem (a Cargo serde = "1.*" is rewritten, an npm "lodash": "1.x" is not), so a note that states "the real reason" has to vary by ecosystem, and the Option<Ecosystem> it threads into rewrite_constraint is what lets it.

The shape chosen for carrying declines out

rewrite_constraint now returns Result<String, DeclineReason> instead of Option<String>, and plan_fixes returns (String, Vec<FixRecord>, Vec<Declined>).

The Option was the lossy boundary. Which guard fired is the only thing that makes a note actionable, that fact is live exactly at the continue that discarded it, and it is not recoverable afterwards: run_fix would have to duplicate is_rewritable, the has-an-update predicate, is_pinned, and target selection and every guard inside rewrite_constraint — a second copy kept in step with the first by hope. Returning it costs one enum.

PlannedFix gains a declined: Vec<Declined> (name, constraint, target version, reason). Declines are sorted and deduped inside plan_fixes, so the same crate under [dependencies] and [dev-dependencies] produces one note, not two.

The decline reasons and their exact wording

Notes go to stderr, in report_inherited_skips's register, from a sibling report_declined_fixes called immediately after fix::plan in run_fix. The line is:

note: left <name> = <constraint> alone in <manifest>: <target> is available, but <reason>
Reason Fires on Wording after "but "
CommaRange Cargo >=1.0, <2.0 a comma-separated range has two bounds and one version cannot carry both
MultiClause >=1.0.0 <2.0.0, ^1 || ^2 a space- or ||-separated range has more than one clause and one version cannot carry them all
Qualifier @dev, ^1.0@beta, npm:pkg@1.0.0 an @ qualifier — a stability flag or an alias — describes the range, not the version
DistTag latest, next a dist-tag names a release channel, not a version
WildcardOperator ^1.x, =1.*, ==1.* an operator in front of a wildcard is a range the new version would not reproduce
WildcardPins npm/Composer/Hex/pub/Poetry 1.x; unrecognized manifests a wildcard already tracks new releases, and a bare version here would pin it to one
WildcardUnbounds NuGet 1.*, Gradle 1.+ a wildcard already tracks new releases, and a bare version here would drop its upper bound
WildcardShape Cargo *, 1.2.*, 1.+ a wildcard already tracks new releases, and no bare version covers the same range
PartialVersion npm "16", "1.0" a partial version is an X-range that already tracks new releases

So the issue's reproduction now prints:

note: left lodash = 1.x alone in package.json: 1.9.0 is available, but a wildcard already tracks new releases, and a bare version here would pin it to one

The closing line changes only when something was left alone:

Nothing to rewrite. 1 available update left alone; see the notes above.

Everything is already up to date. survives verbatim for a run with no declines, which has its own test.

Where the updatable predicate lives

DependencyStatus::has_update() in dependable-core. There were three copies of the same matches!, not two: plan_fixes (fix.rs), report_inherited_skips (runner.rs), and ManifestCheck::outdated (dependable-fetch/src/check.rs). All three now call it. They have to agree — the whole defect is two commands disagreeing about what counts as an update — so the agreement is now structural rather than transcribed.

What falsifies the issue's reproduction

crates/dependable/tests/cli_fix.rs, a_declined_wildcard_is_reported_instead_of_claimed_up_to_date: a package.json whose only dependency is "lodash": "1.x", a registry serving 1.0.0/1.9.0/2.0.0, and three assertions — the note appears on stderr with its exact text, stdout does not contain Everything is already up to date., and the manifest is byte-identical afterwards. a_dry_run_reports_a_declined_wildcard_too covers the "printed nothing at all" half.

Also added: a_declined_dist_tag_is_reported (npm "latest" with a lockfile holding 1.0.0), a_declined_comma_range_is_reported (requirements.txt with requests>=1.0,<2.0), a_run_with_no_declines_still_says_everything_is_up_to_date, and, in fix.rs, a_decline_names_the_guard_that_refused_it (one assertion per reason variant) plus a_declined_constraint_leaves_a_record_of_what_was_not_done and an_up_to_date_dependency_is_not_a_decline.

These need a registry that actually offers a newer release, so cli_fix.rs gains a ~40-line loopback HTTP server built on std::net::TcpListener. No dev-dependency is added (dependable has none), the tests stay hermetic and un-#[ignore]d, and they exercise the real fetch path rather than a stub of it.

Judgement calls

  • The dist-tag test uses latest, not next. Only latest reaches the fix layer: check_version treats it as *, while next and beta fail to parse and are reported as unreadable constraints, never as available updates. latest alone still needs a lockfile pinning an older release, or it resolves to the newest and is up to date. Reversed by teaching the checker more dist-tags.
  • The compound-range test is Python, not npm. An npm space range (>=1.0.0 <2.0.0) or || alternation is an unparseable constraint to the checker and never reaches rewrite_constraint, so MultiClause is currently unreachable end to end; a PEP 440 comma range parses and does reach it. MultiClause is kept and unit-tested, because the guard is real and a parser change would make it live. Reversed by teaching to_version_req npm-native range dialects.
  • Wildcard sub-reason ordering: operator, then the ecosystem's reading, then the shape. The set declined is unchanged — it is the same three conditions, ORed — only which one gets to explain itself. An operator has to answer first or ^1.x would be blamed on pinning when the rewrite (^2.0.0) pins nothing; the reading answers before the shape because it names a concrete harm and the shape does not. Reversed by reordering the match.
  • A BareVersion reading added later declines under WildcardShape. The wildcard match has a _ arm (the enum is #[non_exhaustive]) that declines, as every non-caret reading already did, under the reason that names no particular harm. Inventing a harm for a reading this code has never seen would be worse than saying only that the shapes do not correspond.
  • Notes on stderr, count on stdout. The sibling note does the same, and for the same reason: a note is not part of the record of what fix changed, so piping stdout must neither swallow it nor mix it into that record.
  • The count replaces the closing line rather than being appended. Printing "Everything is already up to date." and a count would restate the contradiction the issue is about.
  • A constraint already at its target is not a decline. new_constraint == item.version_constraint still continues silently: the constraint would have accepted the rewrite, so there is nothing to explain.

Note on PR #97

#97 (a different unmerged stack) changes the same closing line for the uncheckable case to Nothing to rewrite. N dependencies could not be checked for a newer version…. That is a different condition from this one; the wording here — N available update(s) left alone — is deliberately distinct so the two read as separate facts if both land. The two will conflict textually in run_fix and need a human merge.

An unrelated Windows failure this PR had to absorb

The first CI run failed two dependable-report SARIF tests on windows-latest, in a crate this change otherwise does not touch. They are not caused by this change — they are deterministic failures of commit 0248ef8 ("fix(report): emit SARIF artifact URIs a consumer can actually resolve"), which lives on fix/stabilization-pass (#99), two levels down this stack.

uri_is_relative_to_report_root_and_slash_joined and spaces_are_encoded_in_relative_and_absolute_uris feed uri_for paths like /elsewhere/Cargo.toml and assert it returns file:///elsewhere/Cargo.toml. On Windows that path is not absolute — Path::is_absolute there wants a drive prefix — so uri_for takes its relative branch and returns elsewhere/Cargo.toml. The assertion tests nothing on Windows and cannot pass.

It went green on #99 and #106 because their Windows jobs reused a cached dependable_report test binary built before 0248ef8. This branch touches dependable-core, which dependable-report depends on, so the test binary was rebuilt and the assertion ran for the first time. Every currently-green PR in the repo is based on a branch that does not contain 0248ef8.

The last commit here builds the fixture path and its expected URI per platform, so the claim is made on Windows rather than gated off it. uri_for is unchanged, and the Windows-specific behaviour it does have is already covered by a_windows_path_keeps_its_drive_and_encodes_its_segments.

This fix belongs in #99, not here. It is one self-contained commit precisely so it can be moved: if #99 fixes it at the source, drop test(report): give the SARIF uri fixtures a path Windows agrees is absolute from this branch. Until it is fixed somewhere, fix/stabilization-pass and refactor/92-ecosystem-aware-wildcard will go red on Windows the moment their cache turns over — including after they merge to master.

Gates

env -u FORCE_COLOR -u COLORTERM cargo test --workspace   → 925 passed; 0 failed; 20 ignored
env -u FORCE_COLOR -u COLORTERM cargo clippy --workspace --all-targets -- -D warnings → clean
cargo fmt --all --check → clean

Baseline on the base branch was 917 passed / 0 failed / 20 ignored; the 8 new tests are the difference. No test was deleted or weakened.


Restacked on the repaired base

refactor/92-ecosystem-aware-wildcard — itself just brought forward onto the ten
stabilization commits it was missing — is merged in at fd84779. A merge, not
a rebase, so the pushed history is unchanged.

Conflicts

crates/dependable/src/fix.rs was the only conflicted file, in two hunks:

Hunk Resolution
Imports The base needs DependencyKind for the override guard; this branch had already dropped DependencyStatus when has_update() replaced the inline matches!. Kept as {CheckResult, DependencyKind} — the test module imports DependencyStatus for itself.
plan_fixes guards The base added an override skip; this branch replaced the updatable matches! with status.has_update(). Independent changes, both kept, with the override skip first so an override never reaches rewrite_constraint.

An override is skipped without recording a Declined. A Declined says a
constraint refused a rewrite, and its note invites the author to widen that
constraint. An override refuses for a reason its constraint has no part in and
that no edit to the constraint would change, so a note there would point the
author at a string that is not the problem.
fix_all_leaves_an_override_alone now asserts that emptiness alongside its
existing claims.

Everything else merged cleanly and was checked to compose rather than merely to
compile: check_version_for's translation-failure detection routes an
untranslatable constraint to Undetermined, which has_update() excludes, so
such a dependency is neither reported up to date nor rewritten. All three
has_update() call sites survive, ManifestCheck::outdated among them, as do
registry_unreachable in gate_is_answerable and report_declined_fixes.

9ced38d reverted

9ced38d ("give the SARIF uri fixtures a path Windows agrees is absolute") is
reverted in 7bd51d9. It worked around a Windows failure that 787480d has
since fixed at its cause — uri_for now asks Path::has_root rather than
Path::is_absolute, and the gap between those two predicates was the entire
reason /elsewhere/Cargo.toml took the relative branch on Windows while
asserting the absolute branch's answer.

Keeping it would have cost coverage rather than added it. On Windows its
outside_root helper substitutes a drive-absolute path, so the
rooted-but-drive-less case — precisely the one 787480d repaired — would no
longer be exercised on the only platform where it was ever broken, and the
drive-absolute form it substitutes is already covered by
a_windows_path_keeps_its_drive_and_encodes_its_segments. Its doc comment had
also become false and contradicted an assertion 787480d added a few hundred
lines below in the same file.

crates/dependable-report/src/sarif.rs is now byte-identical to its state on
fix/stabilization-pass. The SARIF tests were re-read for self-consistency: the
one #[cfg(windows)] test is gated in full, and every ungated uri_for
assertion resolves the same way on both platforms under has_root.

Gates

cargo test --workspace       941 passed / 0 failed / 20 ignored
cargo test -p dependable-report   109 + 4 passed / 0 failed / 0 ignored
cargo clippy --workspace --all-targets -- -D warnings   clean
cargo fmt --all --check      clean

Both the base (925) and this branch's pre-merge count (925) are exceeded, so no
test was lost in the resolution.

Repairs verified end to end

Run against the built binary, not inspected:

  1. fix --all on {"pnpm":{"overrides":{"foo@2>bar":"3.0.0"}}} leaves the override
    untouched and prints no decline note. The registry resolves bar — the last arrow
    segment — to an unrelated package at 0.1.2, which is the downgrade the guard
    prevents. As a control, an npm "lodash": "1.x" in the same build does still
    produce note: left lodash = 1.x alone … a bare version here would pin it to one,
    so the silence above is the override guard and not broken reporting.
  2. A dependency the registry 404s reports error and exits 0 under
    --fail-on vulnerable, with note: 1 dependency was not found in its registry, so it is not gated on.
  3. A Gradle catalog entry version = "[4.0,4.9" reports undetermined, not
    up to date.
  4. An npm "overrides": {"semver": "$semver"} resolves as a reference to the
    semver constraint and exits 0.

Restacked on the second repair round

fix/stabilization-pass received ten further commits after this branch was last brought forward — repairs for four defects that its own first round of repairs had introduced. This branch now contains them: Go's 410 Gone counted as not-found alongside 404; an ErrorOrigin (NotFound / Unanswered / Local) carried on CheckResult from the typed fetch error, splitting ScanIntegrity into unresolved (exempt from the gate, reported) and unevaluated (unanswerable); an override key carrying a version range no longer split on the > inside that range; a bare * in PEP 440 and Poetry translating to * rather than reading as a failed translation; a stderr note for undetermined dependencies; -q honoured on both notes; PackageSource::Unresolved gaining its own "unresolved" list token; and crates/dependable/tests/cli_gate.rs, a stub HTTP registry that can answer with a chosen status code and content type.

The merge was clean — no conflicted files.

Everything load-bearing across the stack survived, verified in the merged tree rather than assumed: the DependencyKind::Override exclusion in plan_fixes (which deliberately records no decline note, since an override is not a refused rewrite but a version the author forced), all three has_update() call sites, report_declined_fixes and the ErrorOrigin-based gate coexisting in runner.rs, Ecosystem::bare_version(), and translation-failure detection reaching Undetermined.

Validation

cargo test --workspace                                  → 0 failures
cargo test --workspace --no-default-features            → 0 failures
cargo clippy --workspace --all-targets -- -D warnings   → clean
cargo fmt --all --check                                 → clean

The behaviours the stack must not lose are each covered by a test that ran green in the merged tree, rather than by a claim:

fix::tests::fix_all_leaves_an_override_alone                          ok
a_range_in_an_override_key_is_not_a_parent_separator                  ok
a_scoped_override_key_names_the_package_after_the_last_arrow          ok
an_override_key_carrying_a_range_is_checked_as_its_own_package        ok
a_go_module_the_proxy_answers_410_for_does_not_break_the_gate         ok
a_package_the_registry_answers_404_for_does_not_break_the_gate        ok
an_unreadable_constraint_still_refuses_to_certify_the_build           ok
a_metadata_document_listing_no_versions_is_not_exempt_from_the_gate   ok
semver::python::tests::a_bare_wildcard_is_any_version                 ok
a_poetry_wildcard_resolves_instead_of_going_undetermined              ok
a_declined_wildcard_is_reported_instead_of_claimed_up_to_date         ok

Decisions taken

  1. What suppresses "Everything is already up to date."
    Taken: every category the run emitted a note for, not only constraint declines. An inherited skip is "left behind" in exactly the sense the line's own comment means.
    Rejected: counting only constraint declines and rewording the line — the line would still be printed over a note contradicting it, which is the defect rather than its description.
    Reverses: restore the declined == 0 condition alone.

  2. Whether a pin skipped for want of --all is a decline
    Taken: yes, with a reason that names --all as the action. A pin is a constraint, and the author can act on it; the stated rule for the list is "a rewrite a constraint refused, which the author could act on", and the only thing that differs here is which action.
    Rejected: leaving it silent — it reproduces issue fix(cli): fix reports "already up to date" when it declined an available update #93's exact symptom by default, for the single most common way a dependency is deliberately held back.
    Reverses: restore the bare continue at the pin guard.

  3. The reasons the base's new refusals report
    Taken: each of the base's three new refusals carries its own DeclineReason rather than borrowing an existing one, so the note a user reads names the actual cause — a narrowed caret bound, or an arity-sensitive tilde operator, is not the same fact as a wildcard that pins.
    Rejected: collapsing them into the nearest existing variant — the explain() text would then describe a different refusal than the one that fired, which is worse than no note.
    Reverses: map the new guards onto existing variants and delete the added ones.

Restacked again, on the ecosystem-aware wildcard repair

refactor/92-ecosystem-aware-wildcard moved from 121db8e to ca59875, carrying a repair of its own plus the whole of #99's Critical repair beneath it. Merged by merge commit 6a45d31. One conflicted file, crates/dependable/src/fix.rs, resolved on the union of properties rather than of lines: this branch had changed rewrite_constraint to return Result<String, DeclineReason>, and the base added three refusals written against the old Option signature.

Base refusal Old form Resolved as Why not an existing variant
caret_bound_survives_substitution0.*, 0, 0.0 under a caret reading return None new DeclineReason::CaretBoundNarrows the shape is one a caret reproduces; what does not survive is the bound. WildcardShape's "no bare version covers the same range" would send the author looking at the wrong half of the constraint.
tilde with a partial version — ~1, ~> 1.0, ~=1.4 return None new DeclineReason::TildeArity the arity is only a harm because the operator reads it. PartialVersion's "an X-range that already tracks new releases" describes a constraint the author did not write.
ecosystem.is_none_or(Ecosystem::bare_version_is_exact) at the partial guard return None existing DeclineReason::PartialVersion it is that refusal — the base only widened which ecosystems reach it.

The base's ordering rule is preserved and is what decides where the caret-bound guard sits: a constraint that refuses for its shape reports the shape reason. caret_bound_survives_substitution therefore moves back out of is_minor_wildcard and is asked by the caller after the shape test, so 1.2.* still reports WildcardShape and 0.* reports CaretBoundNarrows. is_minor_wildcard is shape-only again, with its doc saying so.

The base's two new tests were converted from Option to Result and now assert the reason rather than the bare refusal — which is the whole property the Result return exists to carry. No verdict was weakened in either direction: every form the base declines is still declined, and every form it permits is still permitted.

Correction: MultiClause and Qualifier are no longer unreachable

The note above headed "The compound-range test is Python, not npm" says MultiClause is unreachable end to end because an npm space range never reaches rewrite_constraint. That is no longer true after this merge. #99's repair gave several constraint dialects a real front end, and both variants are now reachable from the CLI. Verified against the built binary, not inferred:

note: left lodash = >=1.0.0 <2.0.0 alone in ./package.json: 1.9.0 is available, but a space- or `||`-separated range has more than one clause and one version cannot carry them all
note: left react = ^1.0.0 || ^2.0.0 alone in ./package.json: 2.0.0 is available, but a space- or `||`-separated range has more than one clause and one version cannot carry them all
note: left acme/lib = 2.8.*@dev alone in ./composer.json: 2.8.5 is available, but an `@` qualifier — a stability flag or an alias — describes the range, not the version

Both now have end-to-end coverage: a_declined_multi_clause_range_is_reported and a_declined_composer_stability_flag_is_reported in crates/dependable/tests/cli_fix.rs.

One @ form remains unreachable and is worth stating plainly rather than leaving implied: an npm alias ("vue": "npm:lodash@1.0.0") never produces Qualifier, because the parser resolves the alias at parse time and hands fix the name lodash with the constraint 1.0.0. The @ is gone before rewrite_constraint is asked. The guard is still correct — a constraint that does reach it carrying an @ must be declined — it is simply not the alias that reaches it.

The four remaining ways fix contradicted check

Three are plan_fixes dropping a has_update() row before any constraint is consulted; each is now a Declined with its own reason.

Path Reproducer Reason recorded
a pin without --all "lodash": "=1.0.0", 1.9.0 published Pinned — "the constraint pins one release, and only --all moves a pin"
no version resolved to write a has_update() row with neither latest_compatible nor latest_available NoTarget
the constraint already names the target "lodash": "1.0.0" under npm with 2.0.0 published; and any Vulnerable row whose only fixed release is the one already in force AlreadyAtTarget

Declined::target becomes Option<String>, because NoTarget has no version to name. The note then opens "an update was reported, but …" instead of naming a release. Every existing note is byte-identical — the tests asserting them were not touched.

The fourth is the summary line itself. It counted only the constraint declines, so a run could print note: … inherits serde from the workspace; upgrade it in /repo/Cargo.toml to stderr and Everything is already up to date. to stdout in the same breath — the inherited item is not rewritable, so the planner drops it before any constraint is consulted and it never reaches the declined list. report_inherited_skips now returns its count and the summary adds it. The line also says the notes are on stderr rather than "above": the summary is on stdout, so dependable fix > fix.log put the two in different places and "above" named nothing the reader of either stream could find.

An override stays the one silent skip, unchanged: it is not rewritable by this tool under any flag, so a note would describe a decision the author cannot change and did not make. The comment at that guard now says why it differs from the pin.

Validation

Every command run in the merged tree, verbatim, all exit 0:

cargo test -p dependable --bin dependable fix::   34 passed, 0 failed
cargo test -p dependable --test cli_fix           17 passed, 0 failed
cargo test -p dependable-core semver              80 passed, 0 failed
mise run test                                     0 failures across the workspace
mise run fmt:check                                clean
mise run lint                                     clean (clippy -D warnings)
convco check refactor/92-ecosystem-aware-wildcard..HEAD   no errors

New end-to-end tests. The four that cover this round's repair were run against the parent commit (6a45d31, source files only, test file held at this revision) and all four fail there, so each proves a behaviour change rather than restating one:

an_inherited_skip_is_not_contradicted_by_the_summary   FAILED on 6a45d31, ok here
a_pin_held_back_for_want_of_all_is_reported            FAILED on 6a45d31, ok here
a_constraint_already_at_its_target_is_reported         FAILED on 6a45d31, ok here
the_summary_names_the_stream_the_notes_went_to         FAILED on 6a45d31, ok here

The two reachability tests pass on the parent commit too, and are stated as such rather than folded in above: what made MultiClause and Qualifier reachable was the merge, not this round's repair. They are added because the behaviour was until now untested, not because it changed here.

a_declined_multi_clause_range_is_reported              ok on 6a45d31 and here
a_declined_composer_stability_flag_is_reported         ok on 6a45d31 and here

New unit tests at the planner boundary:

fix::tests::a_pin_held_back_for_want_of_all_is_recorded      ok
fix::tests::an_update_with_no_resolved_target_is_recorded    ok
fix::tests::a_constraint_already_at_its_target_is_recorded   ok

Unresolved review notes

None. All three review findings and the base merge were closed in this round. The one item that could not be closed by a code change is the stale claim in an earlier section of this description, which is corrected under "Correction: MultiClause and Qualifier are no longer unreachable" above rather than edited in place.

`fix` planned rewrites for a status set, `report_inherited_skips` reported
skips for the same set, and `ManifestCheck::outdated` iterated it again --
three hand-written copies of one `matches!`. They have to agree or the
commands built on them contradict each other, so make the agreement
structural: `DependencyStatus::has_update`, called from all three.
`rewrite_constraint` returned `Option<String>`, so `plan_fixes` learned that
a constraint could not be rewritten and immediately threw away why. That is
the lossy boundary: the reason is live only at the guard that fires, and
recovering it later would mean a second copy of every guard.

Return `Result<String, DeclineReason>` instead and carry the declined items
out of `plan_fixes` alongside the records. The wildcard guard's three
conditions are now checked in the order that makes the best explanation --
an operator answers whatever the ecosystem reads a bare version as, then
the ecosystem's reading, then the wildcard's shape -- declining exactly the
same set as the single boolean it replaces.
`dependable check` reported an update to "lodash": "1.x" and `dependable fix`
answered "Everything is already up to date." for the same manifest, because
a declined constraint and a manifest with nothing to do produced the same
empty record list. `--dry-run` printed nothing at all.

Emit a note per declined update, in the register and on the stream
`report_inherited_skips` already uses for the sibling case, and give the
closing line a count of what was left alone so it cannot claim otherwise.
Every silent decline is covered, not just the wildcard #89 widened the set
with: dist-tags and compound ranges have been silent for longer.

Closes #93
…solute

`/elsewhere/Cargo.toml` is absolute on Unix and is not on Windows, where
`Path::is_absolute` wants a drive prefix. So on Windows the two fixtures
asserting the `file:` URI branch of `uri_for` were taking its *relative*
branch and asserting the absolute branch's answer -- a deterministic
failure that a stale cached test binary had been hiding on this stack, and
that surfaced here only because touching `dependable-core` forced
`dependable-report` to rebuild.

Build the fixture path and its expected URI per platform instead, so the
claim is made on both rather than gated off one. `uri_for` itself is
unchanged: a real Windows path outside the root carries a drive, and
`a_windows_path_keeps_its_drive_and_encodes_its_segments` already covers it.
…t-declined-updates

Brings forward both the ecosystem-aware wildcard work this branch sits on and,
through it, the ten commits that repaired its base — nine of them fixes for
defects an adversarial review confirmed.

`crates/dependable/src/fix.rs` was the only conflict, in two hunks:

- The imports. The base needs `DependencyKind` for the override guard; this
  branch had already dropped `DependencyStatus` when `has_update()` replaced
  the inline `matches!`. Kept as `{CheckResult, DependencyKind}` — the test
  module imports `DependencyStatus` for itself.
- The `plan_fixes` guards. The base added an override skip and this branch
  replaced the `updatable` `matches!` with `status.has_update()`; the two
  changes are independent and both are kept. The override skip stays first, so
  an override never reaches `rewrite_constraint` at all.

The override is skipped without recording a `Declined`, and
`fix_all_leaves_an_override_alone` now asserts that emptiness alongside its
existing claims. A `Declined` reports a constraint that refused a rewrite, and
its note invites the author to widen that constraint. An override refuses for
a reason its constraint has no part in and that no edit to the constraint
would change, so a note there would point the author at a string that is not
the problem.
This reverts commit 9ced38d.

The Windows failure it worked around has since been fixed at its cause. This
branch was cut before `787480d`, which changed `uri_for` to ask
`Path::has_root` rather than `Path::is_absolute` — and the difference between
those two predicates is the entire reason `/elsewhere/Cargo.toml` took the
relative branch on Windows while asserting the absolute branch's answer. It
now takes the absolute branch on every platform, so the original fixtures make
one claim that holds everywhere.

Keeping the per-platform helper would cost coverage rather than add it. On
Windows `outside_root` substitutes `C:\elsewhere\Cargo.toml`, a drive-absolute
path, which means the rooted-but-drive-less case — precisely the case `787480d`
repaired — would no longer be exercised on the one platform where it was ever
broken. The drive-absolute form it substitutes instead is already asserted by
`a_windows_path_keeps_its_drive_and_encodes_its_segments`.

The helper's doc comment had also become false, and contradicted an assertion
in the same file: it says `/elsewhere/Cargo.toml` "takes `uri_for`'s relative
branch on Windows", while `787480d` added
`uri_for(r"D:\repo", "/elsewhere/Cargo.toml") == "file:///elsewhere/Cargo.toml"`
a few hundred lines below. Two contradictory statements about one input is
worse than either alone.

`crates/dependable-report/src/sarif.rs` is now identical to its state on the
repaired base.
@justin13888
justin13888 force-pushed the fix/93-report-declined-updates branch from 7bd51d9 to 7c8f3ab Compare September 1, 2026 22:04
…card' into fix/93-report-declined-updates

The base added three refusals to `rewrite_constraint` against the `Option`
signature this branch replaced with `Result<String, DeclineReason>`. Resolved on
the union of properties rather than of lines: every one of the three now returns
an `Err` carrying a reason, so the note a user reads names the actual cause.

- The caret-bound guard gets `DeclineReason::CaretBoundNarrows`. `0.*` clears
  every shape test a caret reading reproduces and still loses the author's upper
  bound, because a caret is minor-scoped below 1.0.0: `<1.0.0` becomes
  `<0.11.0`. `caret_bound_survives_substitution` moves out of
  `is_minor_wildcard` and back to the caller so the shape refusal and the bound
  refusal report the two different facts — the base's ordering rule, that a
  constraint refusing for its shape reports the shape reason, is what decides
  which is asked first.
- The tilde guard gets `DeclineReason::TildeArity`, not `PartialVersion`: the
  arity is only a harm because the operator reads it, and an X-range note would
  describe a constraint the author did not write.
- `ecosystem.is_none_or(Ecosystem::bare_version_is_exact)` keeps
  `DeclineReason::PartialVersion`, which is the reason it already had.

The base's two new tests assert the reason rather than the bare refusal, which
is the property the `Result` return exists to carry.
`check` reports an update and `fix` prints "Everything is already up to
date." — issue #93's symptom — was still reachable by default
configuration on four paths the declined list did not cover.

Three are `plan_fixes` dropping a `has_update()` row before any
constraint is consulted, each now recorded with its own reason:

- a pin without `--all`, whose note names `--all` as the action. A pin
  is a constraint and it refused; what differs from a wildcard is which
  action widens it, not whether the author can act on it.
- a row with no version resolved to write.
- a row whose constraint already names the target, reached most sharply
  by a `Vulnerable` row whose only fixed release is the one already in
  force.

`Declined::target` becomes `Option<String>`, because the second of those
has no version to name; the note then opens "an update was reported,
but …" rather than printing an empty one. Every existing note stays
byte-identical.

The fourth is the summary line itself, which made "Everything is already
up to date." conditional on the constraint declines alone while
`report_inherited_skips` had already written a note to stderr
contradicting it. It now counts every category the run emitted a note
for, and says the notes are on stderr rather than "above" — the summary
is on stdout, so `dependable fix > fix.log` puts the two in different
places.

An override stays the one silent skip: it is not rewritable by this tool
under any flag, so a note would describe a decision the author cannot
change and did not make.

End-to-end coverage for each, and for `DeclineReason::MultiClause` and
`DeclineReason::Qualifier`, which the constraint front end now
translates instead of rejecting and which are therefore reachable from
the CLI for the first time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant