Skip to content

outdated: honor the release-age window silently - #779

Open
colinhacks wants to merge 5 commits into
mainfrom
outdated-silent
Open

outdated: honor the release-age window silently#779
colinhacks wants to merge 5 commits into
mainfrom
outdated-silent

Conversation

@colinhacks

Copy link
Copy Markdown
Contributor

The release-age window is the project's own configured policy, and install and update honor it without remark. The report announced it instead — a * on each held cell and a footer with the version and its ETA — so one command editorialized about a policy the others simply apply. Under the 24-hour default that is most runs.

Now a version inside the window is not offered, and a project whose only pending upgrade sits inside it reports nothing and exits 0.

The gating that fixes #722 is unchanged: both columns still resolve through pick_version_for_add, and the absent-dist-tag guard stays. Silence removes the wall case, so Hold, Pick, the walled flags and the publish-time arithmetic all go — net 512 lines deleted.

Refs #722

The window is the project's own configured policy, and `install` and `update`
honor it without remark. The report announced it instead — a `*` on each held
cell and a footer naming the version and its ETA — which made one command
editorialize about a policy the others simply apply. Under nub's 24-hour
default that is most runs on most projects.

Drop the marker, the footer, and the `minimumReleaseAgeHold` JSON field. A
version inside the window is no longer offered, and a project whose only
pending upgrade sits inside it reports nothing and exits 0.

The gating itself is unchanged, and is the part that fixes #722: both columns
still resolve through `pick_version_for_add`, so the report cannot name a
version the resolver would decline, and the absent-dist-tag guard still keeps
a registry that publishes no `latest` out of the drift check.

Silence also simplifies the wall case. `gated_pick` now returns `Option<String>`
and answers `AgeGated` with `None`, so `wanted` falls back to the locked version
and `latest` stays unknown; the row reports no drift on its own. That retires
the per-column `wanted_walled`/`latest_walled` pair, the `Hold` and `Pick`
types, the publish-time arithmetic, and the `format_iso8601_utc` export added
to aube-resolver for the ETA.

Refs #722
Copilot AI lite review requested due to automatic review settings August 20, 2026 01:53
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Error Error Aug 20, 2026 3:38am

Request Review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The silence is right for a version the window is merely holding for a day, but gated_pick discards AgeGateCause, so it also silences the case where the gate can never admit anything — leaving outdated reporting All dependencies up to date. on a project where install/update hard-error.

Reviewed changes — the full diff at 18979400, plus the resolver paths it calls and the surfaces that document the removed behavior.

  • outdated no longer marks or explains a held version — the * cell marker, the Note: * marks a version held back by minimumReleaseAge. footer, and the minimumReleaseAgeHold JSON key are gone; a version inside the window is simply not offered.
  • gated_pick / latest_pick collapse to Option<String>AgeGated now yields None, wanted falls back to current, latest stays unknown, so a fully-gated row produces no drift and never appears.
  • has_drift returns to its two-clause form — the per-column wanted_walled / latest_walled discount is unnecessary once a walled column reports no drift on its own.
  • Supporting machinery deletedHold, Pick, render_holds, mark/mark_width, clears_window_at, epoch_from_iso8601, days_from_civil, humanize_secs, and the four *_held/*_walled row flags.
  • format_iso8601_utc demoted to pub(crate) in aube-resolver, with a #[cfg(test)] use in lib.rs to keep tests.rs resolving it. No consumer outside the crate; still used in non-test code, so no dead-code warning.
  • Docs and the network-gated integration test follow — the install docs' outdated sample now shows All dependencies up to date., and the test asserts the marker and the version are absent.

I traced every Row {} construction site, the drift and render paths, and all references to the removed symbols — nothing is left dangling, and the docs edit from "every command that resolves from the registry" to "every command that installs" is a correction, since outdated genuinely takes no age flags.

ℹ️ Nothing is left that can answer "what is the window holding?"

Before this PR the footer named the version and its ETA. After it, there is no affordance anywhere: outdated accepts no --minimum-release-age override (age_gate_for passes None for the CLI arg), --json carries no field, and --long renders (unknown). That is a coherent position — it is pnpm's — but the code being deleted cited pnpm's own users asking for the information back (pnpm/pnpm#12662, pnpm/pnpm#11543), so it is worth deciding deliberately rather than as a side effect of the marker removal.

Technical details
# No discovery path for a withheld version

## Affected sites
- `vendor/aube/crates/aube/src/commands/outdated.rs:136``resolve_minimum_release_age(&ctx, None)`; the `None` is the CLI override slot, so no per-invocation flag can reach the report.
- `vendor/aube/crates/aube/src/commands/outdated.rs:655` — a window-suppressed `latest` renders as `(unknown)` under `--long`.

## Open questions for the human
- Is total silence the intended end state, or should `outdated` grow the `--minimum-release-age` / `--minimum-release-age-exclude` pair that `add`/`install` already accept, so `nub outdated --minimum-release-age=0` answers "what would I get without the window?"
- If a flag is the answer, the `install` docs' cooling-window section is where it belongs, beside the two flags already documented there.

ℹ️ Nitpicks

  • crates/nub-cli/tests/info_engine.rs:513-527 — every assertion in the silent test is now an absence (!contains("3.1.0"), !contains('*')) plus code == 0. A run where the packument fetch failed prints a warning to stderr, produces no rows, and passes all three. The sibling test at line 455 supplies the differential, but one assert!(stdout.contains("All dependencies up to date.")) would make this one self-contained and satisfy the repo's rule that an absence assertion carries a positive control.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube/src/commands/outdated.rs Outdated
Comment thread vendor/aube/crates/aube/src/commands/outdated.rs Outdated
Collapsing `AgeGated(_)` to "no version" silenced both causes, but they are
not the same refusal and the resolver already treats them separately: an
undeterminable age maps to `Error::ReleaseAgeMissingTime`, a too-new version
to `Error::AgeGate`, with disjoint remedies (#581).

A registry that serves no per-version `time` therefore gates every version
under the default strict window, and the report printed `All dependencies up
to date.` and exited 0 for a project where every install hard-errors — the
report disagreeing with the installer, which is the whole of #722.

Report the cause alongside the pick. A too-new version stays silent, because
that is the window doing exactly what the project configured it to do. An
undeterminable age warns once per package on stderr, beside the existing
packument-fetch warning, naming that `update` will fail for it; stdout stays
data.

Also correct the `latest_known` comment, which claimed the `(unknown)`
sentinel meant only "the registry published no `latest` dist-tag". It now
covers a column the window admits nothing for as well, visible under `--long`.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new Undeterminable warning is predicated partly on a range that plain nub update never resolves against, so it can tell a user an update will fail on a package where it succeeds.

Reviewed changes

  • Confirmed both prior threads are addressed by 1aae65f: gated_pick now splits AgeGateCause::Undeterminable out with its own arm and a 14-line rationale, and Row.latest_known's comment covers both causes plus the --long-only visibility. Replied and resolved.
  • Verified the cause split is sound against the resolver: TooNew outranks Undeterminable in pick_version's selection (semver_util.rs:369-375), so a column holding a mix of dated and undated candidates cannot spuriously report Undeterminable and suppress a real hold.
  • Verified the shared warned set is safe to reuse across the fetch-failure warning and the new one. The fetch-error branch continues and packument results are keyed per registry name, so a name is either always-Err or always-Ok — the two warnings cannot mask each other.
  • Traced the pub(crate) demotion of format_iso8601_utc: three non-test in-crate callers remain (trust.rs:277, trust.rs:1177, primer.rs:263) and the #[cfg(test)] use is consumed at tests.rs:2814/:2818, so the narrowing is dead-code clean.
  • Found one real defect in the new warning's predicate, flagged inline, plus an undocumented user-facing behavior change.

ℹ️ Nitpicks

The warning dedups per package name, but "the registry serves no publish times" is a property of the registry, not of any one package. On a dateless mirror every direct dependency trips it, and warned is local to each collect_rows call, so under -r the whole set repeats per workspace importer (run_filtered loops run_graph per package at 285-351). Given this PR's thesis is that the report should stop editorializing on every run, one summary line per registry would be quieter than N lines per importer.

an_undatable_registry_is_reported_as_such_not_as_silence is a good test and exercises the real function. Its neighbor a_real_upgrade_still_counts_as_drift (1213-1218) does not — it hand-builds Rows and never calls gated_pick, so it would still pass if gated_pick were mutated to always return None. That shape pre-dates this PR, so it is not yours to fix here, but it is worth knowing that the unit tests around has_drift do not guard the picking path.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube/src/commands/outdated.rs Outdated
Comment thread site/content/docs/install/index.mdx
The warning fired on either column, but they answer different questions.
`latest_pick` resolves the literal `latest` range, which a gated pick widens
to `<=dist-tags.latest` — a candidate set bounded by the tag and disjoint from
the manifest's range. Plain `update` resolves the manifest range, so a refusal
in the `latest` column is no evidence about the command the message names.

A stale or rolled-back `latest` tag reaches that state routinely: leave the
tagged version undated while a newer in-range version carries a real publish
time, and the report warned that `update` would fail on a package where it
resolves fine. Key on the `wanted` column alone, which is exactly what plain
`update` resolves.

Document the warning too. It is a user-visible behavior change and the section
above it promises the report is quiet, so it belongs beside the
`ERR_NUB_RELEASE_AGE_MISSING_TIME` error it mirrors.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — minor suggestions inline.

The false positive I flagged last round is fixed exactly as recommended, and the warning is now documented where I asked. Both prior threads are retired.

Reviewed changes

  • Established the whole behavioral delta by diffing the working tree against git show 1aae65f:…: the predicate at outdated.rs:677, the _latest_undated discard at 644, the rewritten 662-676 comment, one new test, and the docs block at install/index.mdx:539-549. Every other line in the file is byte-identical to 1aae65f, so nothing else needed re-reviewing.
  • Confirmed the narrowing is right for the command the message names. Plain nub update resolves the manifest range, which is exactly dep.specifier as handed to gated_pick at 650-657, so wanted_undated is a sound predictor in both directions. The --latest column stays uncovered, which is what I asked for rather than an || into this message.
  • Verified the \-continued eprintln! literal collapses to the string the new docs sample shows, byte-for-byte, so the documented output is not aspirational.
  • Verified every deleted test guarded code that is genuinely gone — a vendor/aube grep for Hold, epoch_from_iso8601, clears_window_at, render_holds, blocked_entirely and the *_held/*_walled flags returns only an unrelated diag.rs:887 HolderGuard.
  • Checked the surviving unit tests around gated_pick / latest_pick / has_drift: each catches a real mutation of the function it names.
  • Two nitpicks inline, both about the new code's guardrails rather than its behavior.

ℹ️ Nitpicks

gated_pick's doc comment still closes with "The caller warns instead, on stderr, beside the existing packument-fetch warning; stdout stays data" (192-194). That sentence is byte-identical at 1aae65f, so this commit did not write it — but it made it inaccurate. gated_pick has two callers and, after this commit, only the wanted one warns. Naming that caller is the whole edit, and it is the same distinction the new 668-676 comment spends nine lines making one screen below.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

for junk in ["", "2026-08-11", "not-a-date", "2026-13-11T00:00:00Z"] {
assert_eq!(epoch_from_iso8601(junk), None, "{junk:?}");
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pins the premise, not the decision.

The doc comment says the warning "must key on that column alone", but nothing here observes any keying — both assertions read return values of latest_pick and gated_pick. The predicate itself is line 677, inside collect_rows, whose only call sites are production (421 and 491); no test in this file reaches it and there is no mock-registry harness. Revert 677 to (wanted_undated || latest_undated) and this test still passes, as does every other test in the file, as does info_engine.rs. The PR's actual deliverable is unguarded at both levels.

What the test does establish is worth keeping: this packument is a case where the two columns genuinely diverge, which is the premise the fix rests on. The missing half is an observation of the warning. Either make collect_rows reachable from a test, or lift the decision into a small pure function over (wanted_undated, latest_undated) that a test can call and that 677 delegates to.

Per the repo's rule that a test which would still pass with the bug present is not evidence.

Comment thread vendor/aube/crates/aube/src/commands/outdated.rs Outdated
Narrowing the warning to the manifest range left `latest_pick`'s second tuple
element with no production consumer — the call site read it into a discard and
only tests looked at it. Return a plain `Option<String>` and drop the verdict
inside `latest_pick` instead.

This is what guards the narrowing. A unit test cannot reach the predicate,
which lives in `collect_rows` behind a registry fetch, so a test asserting the
`latest` column stays out of it would still pass with the bug reinstated.
With no such binding in scope, reinstating it does not compile.

Also name the caller in `gated_pick`'s doc comment. It said "the caller warns",
which went stale when the predicate narrowed: there are two callers and only
the `wanted` one warns.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues — one test-strength nit inline.

Both of last round's nitpicks are fixed: the vestigial flag is gone, and gated_pick's doc now names the caller that warns.

Reviewed changes — the delta from 8e56c18 to 78b9e37. It touches vendor/aube/crates/aube/src/commands/outdated.rs alone; the other four files in the PR are byte-identical to the version I reviewed last round.

  • latest_pick drops the undeterminable flag — it returns Option<String> rather than (Option<String>, bool) and ends with gated_pick(..).0. Behavior is unchanged, since the sole caller already read the bool into _latest_undated; only the discard is gone. A vendor/aube grep for latest_pick / latest_undated returns 8 sites, all on the new shape, so nothing is left expecting a tuple.
  • gated_pick's doc names its warning caller — "The caller warns instead" became "The wanted caller warns instead — and only that one, since only the manifest range predicts plain update", which is what the predicate at 680 actually does.
  • The 671-679 comment explains the discard from the other end — it says why latest_pick throws its own verdict away, and latest_pick's new inline comment points back to it. The two references agree rather than each half-explaining it.
  • One assertion is deleted rather than portedassert!(latest_pick(..).1) is gone from an_undatable_registry_is_reported_as_such_not_as_silence, but that test still asserts undated == true directly on gated_pick, so the Undeterminable arm stays guarded.
  • an_undated_latest_tag_does_not_predict_a_failure_of_the_manifest_range swaps .1 for == None — it can still fail (a broken <=tag widening would let 3.0.0 through), and its load-bearing !undated assertion on the wanted column is untouched. One consequence of the swap is flagged inline.

I did not compile this. The type change is mechanical, and aube-parity.yml runs cargo test --workspace under vendor/aube on ubuntu and windows for any PR touching that path.

ℹ️ Nitpicks

outdated.rs:159tagged.as_ref()?; is a guard statement whose value is discarded, and the eight-line doc above the function is spent explaining that this particular guard is load-bearing rather than defensive. Folding it into line 158 as let tagged = packument.dist_tags.get("latest").cloned()?;, with Some(tagged) at the call, says the same thing in one line and puts the early return where a reader looks for it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment on lines +1279 to +1282
latest_pick(&p, "pkg", Some(&g)),
None,
"the `latest` column genuinely admits nothing here"
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None cannot distinguish the undated refusal this case is about from an ordinary TooNew one, so the premise the test name rests on — that the tagged latest is undated — is no longer pinned by any assertion here. The sibling at 1215-1223 already guards its mechanism through pick_version_for_add directly; the same shape applies, and I checked it holds (gate(true) sets strict: true, the no-time branch of version_clears_cutoff is Undeterminable only under strict, and the <=tagged widening bounds candidates at the undated 2.0.0).

Suggested change
latest_pick(&p, "pkg", Some(&g)),
None,
"the `latest` column genuinely admits nothing here"
);
latest_pick(&p, "pkg", Some(&g)),
None,
"the `latest` column genuinely admits nothing here"
);
// Guard the premise: `None` alone cannot tell an undated refusal from
// a too-new one, and undated-ness is what this case is about.
assert!(matches!(
aube_resolver::pick_version_for_add(&p, "pkg", "latest", Some(&g)),
aube_resolver::PickResult::AgeGated(aube_resolver::AgeGateCause::Undeterminable)
));

Asserting the `latest` column resolves to `None` does not distinguish the
undated refusal this case is about from an ordinary too-new one, so the
premise the test is named for went unguarded when the flag it used to read
was removed. Assert the cause through `pick_version_for_add` instead, the
same shape the sibling test uses.

Verified it fails for the right reason: dating the tagged version turns the
refusal into a `Found` and the assertion goes red.
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.

[Feature Request] Take minimumReleaseAge into account for nub outdated

2 participants