Skip to content

pm: add per-invocation minimumReleaseAge CLI flags - #645

Open
colinhacks wants to merge 1 commit into
mainfrom
age-gate-flag
Open

pm: add per-invocation minimumReleaseAge CLI flags#645
colinhacks wants to merge 1 commit into
mainfrom
age-gate-flag

Conversation

@colinhacks

@colinhacks colinhacks commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Clearing the age gate previously required an .npmrc or env-var edit — global and persistent, to get past one command.

Adds --minimum-release-age <DURATION> and --minimum-release-age-exclude <PKG> to install/ci, the engine verbs, and nubx.

Each mirrors both surfaces the setting already has: pnpm 11.9.0's spellings, and the values the matching nub.jsonc field accepts. The duration calls the same parser as install.minimumReleaseAge, so file and flag cannot drift to two grammars. A bare number means minutes (pnpm's contract; nub.jsonc rejects the bare form on purpose). Sub-minute values round up, so tightening the gate can never silently disable it.

No strictness flag, and none in nub.jsonc: under Nub the gate is strict, and --minimum-release-age=0 is the opt-out — turn the window off rather than keep one and quietly install versions that fail it. Verified 0 disables completely, including alongside minimumReleaseAgeStrict=true and paranoid=true.

Also fixes: nub add's age-gate error carried no help text at all, and install --minimum-release-age 0 misrouted 0 as a package.

Copilot AI review requested due to automatic review settings July 31, 2026 22:34
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview Aug 1, 2026 3:33am

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 interactive waiver prints "cooling window skipped for this run" unconditionally, including on the paths where the OnceLock is already latched and the waiver is a no-op. nubx --no-minimum-release-age-strict <tool> gets the message with the window still fully in force.

Reviewed changes — the full single-commit diff (dd6b988, 10 files), plus the settings-resolution, managed-policy, dlx, and consent-gate code the change depends on.

  • AgeGateFlags clap struct--minimum-release-age <MINUTES>, --minimum-release-age-strict, --no-minimum-release-age-strict, published into aube_settings::set_global_cli_overrides by apply().
  • Flag surface — flattened into Command::Nubx, Command::Install, Command::Ci, and EngineGlobals (so every engine verb, not only the registry-resolving ones).
  • Interactive waivernubx_consent::Decision::Proceed gains confirmed_interactively; a yes at the prompt calls waive_for_interactive_consent(), which publishes minimumReleaseAge=0. -y, a ledger hit, CI, and non-TTY are excluded.
  • Error help — the resolver's age-gate help now leads with the one-shot flags, and nub add's ERR_AUBE_NO_MATURE_MATCHING_VERSION gains a help field it previously lacked entirely.
  • Argv routing--minimum-release-age registered as value-consuming in install_to_add_args and value_consuming_flags("nubx") so its MINUTES token isn't read as a package or a bin.
  • Docs — the flag pair on the install page, and a waiver + "nothing else waives it" table on the runner page.

Three claims in the PR body check out against the code: managedPolicy = "max" really does clamp a 0 back up (apply_managed_u64 runs after the override bag), dlx really does force ignore-scripts (allow_build is hardcoded empty for the nubx fallback), and the waiver's blast radius really is confined to the one transient fetch — the bag is keyed per setting, in-memory only, and the global virtual store is disabled on that path.

⚠️ The consent prompt establishes identity, not freshness

The waiver's stated premise is that "a human named the tool, read it back off the prompt, and said run it." What the prompt actually shows is The <bin> bin is not installed locally. / Install and run from the remote registry? (nubx_consent.rs:387-391) — no publish date, no age, nothing that distinguishes a tool published two years ago from one published four minutes ago. The user is consenting to the package's identity, and the waiver spends that consent on its freshness.

Before this change, ERR_NUB_NO_MATURE_MATCHING_VERSION was the one place that fact surfaced. After it, the only signal a nubx user gets that they are about to run a minutes-old publish is the cooling window skipped for this run line — printed after they already answered.

Technical details
# Waiver consent is uninformed about the thing being waived

## Affected sites
- `crates/nub-cli/src/pm_engine/min_release_age.rs:365-392` — the waiver's rationale rests on the prompt having shown the user enough to judge the risk.
- `crates/nub-cli/src/nubx_consent.rs:387-391` — the prompt text carries no publish-time information.

## Required outcome
- Either the consent that waives the window is informed about the age, or the window is not waived by that consent.

## Suggested approach (optional)
- Keep the gate armed through resolution, and when it *would* block, re-prompt with the concrete fact (`cowsay@1.6.0 was published 9 minutes ago. Run it anyway?`). That is a genuinely informed waiver and it costs nothing on the overwhelmingly common path where the tool is old enough.
- A cheaper variant: keep the current blanket waiver but state the age in the notice (`cooling window skipped — cowsay@1.6.0 was published 9 minutes ago`) so the user can still abort before the tool's code runs.

## Open questions for the human
- Is "the user typed the name" intended to be sufficient consent for running arbitrary code from a publish that is minutes old? The threat model the window exists for is precisely the case where the name is one the user trusts and the *version* is not.

ℹ️ Nothing pins the canonical setting keys the flags resolve through

cli_overrides() maps the flags onto the literal strings "minimumReleaseAge" and "minimumReleaseAgeStrict". Those strings are the entire contract with cli_key_matches — a typo in either produces no compile error, no clap error, and no test failure; the flag just silently stops doing anything while still being accepted and documented. The two tests the diff does add cover confirmed_interactively, not this.

Technical details
# The flag→setting key mapping is untested

## Affected sites
- `crates/nub-cli/src/pm_engine/min_release_age.rs:330-342``cli_overrides()` returns hardcoded setting-name strings with no test.

## Required outcome
- A regression here should fail a test rather than silently disable the flags.

## Suggested approach (optional)
- One test over `cli_overrides()` covering all three flags is enough, and it is stronger if it asserts the keys actually resolve rather than just comparing strings — e.g. that each key is accepted by `aube_settings`' own name lookup (`meta::find`), so a rename on the aube side breaks the test too. Avoid touching `set_global_cli_overrides` in a test: it is a process-global `OnceLock` and would leak across the suite.

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

Comment thread crates/nub-cli/src/pm_engine/min_release_age.rs Outdated
Comment thread crates/nub-cli/src/pm_engine/min_release_age.rs Outdated
Comment thread crates/nub-cli/src/pm_engine/install_family.rs Outdated
@colinhacks colinhacks changed the title pm: per-invocation minimumReleaseAge flags, and waive the gate on interactive nubx consent pm: add per-invocation minimumReleaseAge CLI flags Aug 1, 2026

@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 in this revision. The one earlier thread still open is the EngineGlobals doc contract.

Reviewed changes — the delta since the prior pullfrog review, confirmed with git range-diff against a23054c, is a straight revert of the nubx interactive-consent waiver plus a commit-message rewrite; the flag surface, error-help rewrite, argv routing, and install-page docs are unchanged from the previously reviewed revision.

  • Dropped the interactive-consent waiverwaive_for_interactive_consent(), Decision::confirmed_interactively, the ImplicitDlx::Allow note, the two nubx_consent tests, and the runner-page waiver docs are gone. The removal is complete: Decision::Proceed { record } is back to one field and consistent at both nubx_consent.rs:59 and cli.rs:5820, and a tree-wide grep for the removed identifiers returns nothing.
  • Retired the two findings that rode on it — the unconditional "cooling window skipped" notice and the informed-consent concern both died with the code they were about.
  • Rewrote the commit message — the pnpm citation now names config/reader/src/types.ts. That checks out: pnpm 11's pnpm11/config/reader/src/types.ts declares 'minimum-release-age': Number and 'minimum-release-age-strict': Boolean in the nopt type map that feeds CLI parsing, so the earlier parity objection is withdrawn and its thread resolved.

ℹ️ The deferred nubx consent handling has no tracked follow-up

The scope note says the pnpm 11.9.0 behavior (policyHandlers.ts — prompt with the full set of immature picks, persist the approval) lands separately because it needs the resolver to collect violations instead of aborting on the first. Nothing in the PR references an issue for that work, so the case the commit message opens with — nubx <just-published-tool> — is left as a manual --minimum-release-age=0 retype with no tracked path to the better shape.

Technical details
# Deferred resolver work is described only in the PR body

## Affected sites
- PR #645 body, scope note — records the deferral and the prerequisite (collect age-gate violations rather than aborting on the first) with no issue link.

## Required outcome
- The prerequisite and the follow-up are tracked somewhere that outlives the PR body, or the human confirms the deferral is deliberately untracked.

## Open questions for the human
- Is the pnpm-style consent prompt still the intended destination for `nubx`, or is the one-shot flag now considered the whole answer for this surface?

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@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 code issues in this revision — two documentation inaccuracies inline.

Reviewed changes — the delta since the prior pullfrog review (164174c), computed with git diff 164174c^{tree} HEAD^{tree} since the force-push left no common commit to range-diff against.

  • Added a third flag, --minimum-release-age-exclude <PKG> — repeatable, comma-joined into one minimumReleaseAgeExclude entry because cli_raw_for walks the override bag in reverse and takes only the last match. Verified against pnpm: pnpm11/config/reader/src/types.ts declares all three spellings (minimum-release-age-exclude as [String, Array], i.e. repeatable) in the nopt map that parse-cli-args feeds to nopt(), so the parity claim in the new doc comment holds for the exclude flag too.
  • Widened the --minimum-release-age value grammarparse_release_age_minutes takes a bare integer as minutes or delegates to project_config::parse_duration for <int><s|m|h|d|w>, rounding sub-minute values up with div_ceil(60). parse_duration is now pub(crate) so file and flag share one grammar; it stays overflow-safe via checked_mul.
  • Registered the exclude flag as value-consuminginstall_to_add_args' VALUE_FLAGS, value_consuming_flags("nubx"), and the pinned dlx-scanner test list, so its PKG token is never read as a package or a bin.
  • Added four parser tests — units, bare minutes, the 1d/1440 identity, sub-minute round-up, the rejections inherited from the config grammar, and the comma-join. All four can genuinely fail; the join test's filter uses the same literal the production code pushes, so a typo in that key trips the len == 1 assert.
  • Updated the install pageTwo flagsThree flags, plus a paragraph on the duration grammar and the round-up rule.

ℹ️ Nitpicks

  • site/content/docs/install/index.mdx lines 458-461 — the two new example lines broke the console block's comment alignment; the # now lands at columns 58, 59, 63, 59. Padding all four to one column reads better in a rendered code block.
  • crates/nub-cli/src/cli.rs lines 10225-10233 — the new install_to_add_args regression test covers --minimum-release-age but not --minimum-release-age-exclude, which is the higher-risk of the two: its value is a bare package name, so a missing VALUE_FLAGS entry would silently route nub install --minimum-release-age-exclude react to add and install react. One extra assertion pins it.

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

Comment thread site/content/docs/install/index.mdx Outdated
Comment on lines +352 to +354
/// Exempt a package from the age gate for this run (repeatable). Same entry
/// grammar as the config field: a bare name, a `*` name glob, or a name with
/// a version range.

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.

"Exempt a package … for this run" reads as additive, but the flag replaces the configured list outright: nub's project block arrives as project_config_settings, cli outranks it, and string_list_from_cli returns on the first match with no merge. So in a project that excludes @internal/*, adding --minimum-release-age-exclude=some-tool re-gates @internal/* for that run — the user asks for one more exemption and gets a failure about a different package. pnpm replaces too (Object.assign(pnpmConfig, configFromCliOpts)), so the semantics are right; only the fact that they are replacing is unstated here and on the install page.

Suggested change
/// Exempt a package from the age gate for this run (repeatable). Same entry
/// grammar as the config field: a bare name, a `*` name glob, or a name with
/// a version range.
/// Exempt a package from the age gate for this run (repeatable). Same entry
/// grammar as the config field: a bare name, a `*` name glob, or a name with
/// a version range. REPLACES the configured `minimumReleaseAgeExclude` for
/// this run rather than adding to it, matching pnpm, whose CLI value
/// overwrites the configured array.

@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

This revision deletes --no-minimum-release-age-strict, but two error messages still offer it as the one-shot remedy and a test pins that wording. parse_verb matches argv strictly, so a user who follows the age-gate help now gets error: unexpected argument and EXIT_CLI_USAGE instead of an install.

Reviewed changes — the delta since the prior pullfrog review (43200f0), computed with git diff 43200f0^{tree} HEAD^{tree} since the force-push left no common commit to range-diff against. Only two files differ; the flag plumbing, argv routing, and add help field are unchanged from the previously reviewed revision.

  • Removed the strictness flags--minimum-release-age-strict and --no-minimum-release-age-strict are gone from AgeGateFlags (fields, conflicts_with, and both cli_overrides() arms), leaving two flags instead of three. The rationale comment is sound and the removal is internally consistent: nub's embedder default is minimumReleaseAgeStrict = true, strict mode hard-aborts before a fallback pick, and resolve_minimum_release_age short-circuits to None on exactly 0, so --minimum-release-age=0 really is a complete off-switch even alongside paranoid=true.
  • Demoted AgeGateFlags' struct doc from /// to // — a rustdoc comment on a clap Args struct becomes the augmented command's about-text, and this one had clobbered nub add --help. Correct diagnosis and correct fix; EngineGlobals documents the same hazard.
  • Added zero_reaches_the_engine_as_the_off_switch — asserts cli_overrides() publishes a literal "0" rather than eliding it as a default, plus 0/0s/0d all parsing to Ok(0). It can genuinely fail: eliding zero leaves the vec empty.
  • Updated the install pageThree flagsTwo flags, the console block's comment columns aligned, and a new paragraph asserting the window is enforced rather than advisory.

ℹ️ Nothing can catch the next clap about-text leak

The ///-on-a-clap-Args-struct hazard has now bitten twice and is documented in two places (EngineGlobals and AgeGateFlags), yet no test observes it. The existing help sweep, clap_outcomes_are_rewritten_and_exit_like_the_engine, renders every wired verb's long help and asserts only that it is brand-clean and that usage names nub <verb> — neither of which changes when a flattened struct's doc replaces the verb's about-text, which is exactly why the leak shipped in 43200f0 with the sweep green.

Technical details
# The clap about-text hazard is documented but untested

## Affected sites
- `crates/nub-cli/src/pm_engine/install_family.rs:2035-2085` — the help sweep already renders every verb's long help but asserts nothing about the about-text.
- `crates/nub-cli/src/pm_engine/install_family.rs:200-201` and `crates/nub-cli/src/pm_engine/min_release_age.rs:351-353` — the hazard is documented twice, in prose only.

## Required outcome
- A flattened globals struct that clobbers a verb's about-text fails a test rather than shipping.

## Suggested approach (optional)
- The sweep already has `help_of::<A>(typed)` for every verb. One added assertion per iteration — that the rendered help still contains a fragment of the verb's own about-text — closes it without new scaffolding.

ℹ️ Nitpicks

  • crates/nub-cli/src/cli.rs lines 2129-2130 — the comment says the --[no-]minimum-release-age-strict siblings "are boolean and correctly absent" from value_consuming_flags. They are absent because they no longer exist; as written it reads as a live flag deliberately omitted from one list.
  • crates/nub-cli/src/pm_engine/min_release_age.rs line 553 — the new test's doc says "nub ships no strictness flag", which is true of the CLI but minimumReleaseAgeStrict is still a live setting read from .npmrc. "No strictness flag" would read unambiguously as "no --minimum-release-age-strict flag".

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

Comment thread vendor/aube/crates/aube-resolver/src/error.rs
Comment thread vendor/aube/crates/aube/src/commands/add/manifest.rs
Comment thread vendor/aube/crates/aube-resolver/src/tests.rs
Comment thread site/content/docs/install/index.mdx

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

ℹ️ The whole delta since the last review is one doc comment — one accuracy gap in the new text, inline. The earlier open threads are untouched by this revision.

Reviewed changes — the delta since the prior pullfrog review (c537493), computed with git diff c537493^{tree} HEAD^{tree} since the force-push left no common commit to range-diff against. One file differs and the change is comment-only: no code, test, flag-surface, or docs line moved.

  • Split the EngineGlobals struct note into two groups — the subset that mirrors aube's Cli (--dir, the workspace selectors, the output flags) and the nub-own age_gate group. Both load-bearing claims check out: aube's Cli in vendor/aube/crates/aube/src/lib.rs declares no minimum_release_age* arg at all, and the generic --config.<key> extractor really does live inside cli_main_with_defaults (lib.rs:686, entered from :621), which is dead under nub.
  • Replaced the blanket "every verb parsed through here resolves from the registry" claim with a per-verb account — and the correction lands on the right fact. why, list, licenses, outdated, and audit dispatch from info_family.rs:107-112 through that module's own parse_verb at :243, which never flattens EngineGlobals, so those five never saw the flags and never advertised them. The earlier thread's premise about them was wrong, not just the comment.

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

Comment thread crates/nub-cli/src/pm_engine/install_family.rs Outdated
colinhacks added a commit to colinhacks/fray that referenced this pull request Aug 1, 2026
A pr-watch wake on nubjs/nub#645 rendered as a raw-text "Fray" card with the
agent-facing `gh api --paginate …` line showing, instead of the hairline
divider — reported 2026-07-31 18:59 ("why did we butcher the rendering on
this?").

Nothing on `main` was broken: the maintainer's own live board renders that exact
message (`# 3202910`) as a divider on a fresh load. The tab was a build behind.
`c741fb1` gave the steer a derived review-read tail and taught the parser an
ALLOWLIST for exactly those two line shapes, and `web/api/boot.ts` adopts a new
server boot id IN PLACE rather than reloading, on purpose, so an unsent draft
survives a restart. So a promoted artifact swapped a tail-emitting server under
tabs whose parser had never heard of it, `parseGithubWakeSteer` returned null,
and every one of them fell to the raw-text card. Any steer line added later would
have done it again.

Two corrections, so it cannot:

- The SERVER parses the steer at projection time and hands the structured result
  to the chat on `TranscriptMessage.wakeSteer`. There, formatter and parser are
  the same build by construction. The browser renders the divider from that
  instead of re-deriving it from prose.
- `parseGithubWakeSteer` DROPS lines it does not recognize rather than refusing
  the message. An allowlist is wrong until it is taught each new line; dropping
  is right in advance. The header's own count stays the integrity check, so a
  truncated or padded burst still returns null — pinned by the existing tests,
  unchanged.

Neither can repair a bundle already in the wild (nothing can); both make every
build from here forward immune. A reload fixes the reported tab.

The delivery-token assertion flipped: the raw delivered text now PARSES. Its old
refusal only bought a missed strip degrading to the raw card with
`<!-- fray-wake:… -->` showing, which is the bug the strip exists to prevent.

Browser gate on the real tailer→transcript→render pipeline (adhoc stack,
`seed-review-wake-card.mjs`, +1 case): review wake, burst, control, and a steer
carrying two lines NO build has seen all draw the divider with no leaked tail at
1400px and 420px; a non-GitHub limit wake still falls back to first-party chrome.
2695 tests, typecheck green.

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

ℹ️ Run 5's blocking set has landed cleanly — two observations on the new remedy text inline.

Reviewed changes — the delta since the prior pullfrog review (acd1cec), computed with git diff acd1cec^{tree} HEAD^{tree} scoped to this PR's own files: the force-push also rebased onto a newer main, so the unscoped tree diff carries ~26 files of base noise that do not belong to the PR. Five files differ, and the delta is exactly the four threads from run 5.

  • Replaced the deleted flag in both error helpsaube-resolver/src/error.rs and add/manifest.rs now offer --minimum-release-age=0 or --minimum-release-age-exclude=<pkg>. A tree-wide grep confirms no user-facing string mentions --no-minimum-release-age-strict or minimumReleaseAgeStrict=false any more; the only survivors are comments and the two new negative assertions.
  • Inverted the assertion that pinned the broken adviceaube-resolver/src/tests.rs now asserts the removed spelling is ABSENT, plus a second negative on minimumReleaseAgeStrict=false. Both are real guards rather than decoration: re-adding either string fails the test, which is precisely what the old assertion could not do.
  • Reconciled the docs' strictness storyindex.mdx:485 now frames minimumReleaseAgeStrict=false as a third way through that Nub keeps readable for compatibility and never suggests in its own output, and :469 dropped the qualifier that implied the setting was the adjustable knob. That matches what the error strings now print.
  • Rewrote the stale cli.rs:2127-2130 comment — the strictness siblings are described as nonexistent rather than as live booleans omitted from one list.
  • Extended the undateable-version helpsformat_undated_help and add's ERR_AUBE_RELEASE_AGE_MISSING_TIME now name --minimum-release-age=0 as the this-run-only form alongside the persistent setting.

ℹ️ Two install paths print the new flag advice but cannot accept the flag

The remedies the help now leads with are only parsed where AgeGateFlags is flattened. Two paths reach the same resolver error without going through one of those command lines: nub init, which always runs an install (init.rs:152) and whose Command::Init has a hidden trailing_var_arg that would swallow --minimum-release-age=0 rather than reject it, and the engine-mismatch reinstall inside verify_deps::gate (verify_deps.rs:351), reached from nub run / nub watch / nub <file>. Both are unlikely to fire in practice — init's @nubjs/types range keeps mature versions in scope, and the repair path normally skips re-resolution on a satisfying lockfile — so this is a completeness note on the invariant the new comment states, not a defect I can demonstrate.

Technical details
# Age-gate remedy text is reachable from commands that do not parse it

## Affected sites
- `crates/nub-cli/src/init.rs:152``run_install(InstallFlags::default())` with no `age_gate.apply()`; `Command::Init` (`crates/nub-cli/src/cli.rs:957-986`) declares no `AgeGateFlags` and its hidden `trailing_var_arg` absorbs unknown flags.
- `crates/nub-cli/src/verify_deps.rs:351``run_install` from the engine-repair path, invoked under `nub run` / `nub watch` / `nub <file>` / `nubx <local-bin>`.
- `vendor/aube/crates/aube-resolver/src/error.rs:400-406` — the help text that names the flags.

## Required outcome
- A user who follows the printed remedy can act on it from the command they actually ran, or the human confirms these two paths are acceptable to leave as "go run `nub install` with the flag".

## Open questions for the human
- Is it worth flattening `AgeGateFlags` onto `nub init` (which is the one of the two that deterministically resolves fresh), or is the indirect route good enough given how rarely the gate can fire there?

ℹ️ Nitpicks

  • vendor/aube/crates/aube/src/commands/add/manifest.rs lines 511-518 — this help is a hand-duplicate of format_age_gate_help's two lines, and the new absence guard in aube-resolver/src/tests.rs only covers the resolver copy. Hand-duplication is what let the two drift apart in the first place; exporting the remedy text from aube-resolver would put both under the one test.

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

Comment thread vendor/aube/crates/aube-resolver/src/error.rs Outdated
Comment thread vendor/aube/crates/aube-resolver/src/error.rs Outdated

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

ℹ️ Both of run 7's findings landed correctly — one coverage observation inline.

Reviewed changes — the delta since the prior pullfrog review (11a7b89), computed with git diff 11a7b89^{tree} HEAD^{tree} since the force-push left no common commit to range-diff against. Three files differ, all under vendor/aube, and the delta is exactly the two inline threads from run 7.

  • Carried the exclude matcher's identity into both error structsAgeGateDetails and UndatedDetails gained a registry_name field, populated from task.registry_name(), and both exclude remedies print it while the prose and dependency chain keep name. The fix is airtight rather than merely plausible: the exempt closure in resolve/driver.rs:810-818 binds the value of that same expression, so the printed name and the matched name are identical by construction for every dep shape — plain, transitive, scoped, catalog:, npm:-aliased, and jsr: (where real_name is the npm-compat @jsr/scope__name). All four construction sites are updated, and the non-Option field means a missed one would not compile.
  • Reworked the remedies to lead with shortening--minimum-release-age=<duration> with `0` turns it off as the limiting case, and shorten \minimumReleaseAge`for the persistent form.format_undated_helpdeliberately offers no shortening, which is right: an undated version clears no window, so only an exemption or0` helps.
  • Added a formatter test for the aliased caseexclude_remedies_name_the_registry_identity_not_the_alias asserts real-pkg is offered and foo is not, on both the age-gate and undated paths. Both halves can genuinely fail, and the negative assertion is the one that catches a regression back to d.name.
  • Mirrored the wording in addadd/manifest.rs carries the same two-line remedy shape. Its spec.name really is the registry identity, so the comment justifying it holds: an aliased add keeps the user-facing key in spec.alias, and a jsr: add resolves spec.name to @jsr/scope__name (add/spec.rs:471-484). add and a later install therefore name the same identity.

I also confirmed the undated remedy is live rather than decorative: an exempt version answers only to exempt_cutoff, which is None unless time-based mode is on, and classify_version_age returns Clears on a None cutoff without consulting packument.time.

ℹ️ Nitpicks

  • vendor/aube/crates/aube-resolver/src/tests.rs lines 215-221 and 247-252 — the first comment says the assertion deliberately avoids "pinning one literal spelling of the value", then thirty lines later the test pins --minimum-release-age=<duration> exactly. The loose contains("--minimum-release-age=") can no longer fail while the exact one passes, so one of the two is dead weight and the comment contradicts the file it sits in.
  • crates/nub-cli/src/pm_engine/min_release_age.rs lines 363-366 — the flag's own --help describes an exclude entry as "a bare name, a * name glob, or a name with a version range", and the install page is silent on the same point. This revision established that for an npm:-aliased or jsr: dep the entry must name the registry identity, not the manifest key; the error output now hands users the right value, but someone composing the flag or the config list up front still has no way to know. One clause on the flag doc would close it. (Same doc comment as the still-open thread about the flag replacing rather than extending the configured list.)

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

let (dated, _) = satisfying_versions(task, packument);
AgeGateDetails {
name: task.name.clone(),
registry_name: task.registry_name().to_string(),

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.

Nothing pins this line. Both new tests hand-construct the details struct, and the only test that exercises build_age_gate (tests.rs:61-83) sets real_name: None and never asserts registry_name — so registry_name: task.name.clone() here would keep the whole suite green while restoring exactly the inert advice this revision fixed. The formatter is covered; the wiring that computes the alias distinction is not.

Technical details
# The `registry_name` builders are untested for the aliased case

## Affected sites
- `vendor/aube/crates/aube-resolver/src/error.rs:332``build_age_gate` wires `task.registry_name()`; no test observes it.
- `vendor/aube/crates/aube-resolver/src/error.rs:348` — same for `build_release_age_missing_time`.
- `vendor/aube/crates/aube-resolver/src/tests.rs:61-83` — the one `build_age_gate` test uses `real_name: None`, so `name` and `registry_name` are indistinguishable.
- `vendor/aube/crates/aube-resolver/src/tests.rs:261-294` — the new test builds `AgeGateDetails` / `UndatedDetails` literally, bypassing both builders.

## Required outcome
- A builder that populates `registry_name` from `name` instead of `registry_name()` fails a test.

## Suggested approach (optional)
- `build_age_gate_resolves_dist_tag_range` already constructs a `ResolveTask`; a sibling with `name: "foo"`, `real_name: Some("real-pkg".into())` and one `assert_eq!(d.registry_name, "real-pkg")` (plus `d.name == "foo"`, so the two stay distinguishable) covers both builders with no new scaffolding.

Clearing the supply-chain age gate previously required editing .npmrc or setting
an env var — a global, persistent change to get past a single command.
`nubx <just-published-tool>` hit this routinely with no one-shot way through.

Adds two flags, on every surface that resolves from the registry (install/ci,
the engine verbs via EngineGlobals, and nubx):

    --minimum-release-age <DURATION>
    --minimum-release-age-exclude <PKG>     (repeatable)

Each mirrors both surfaces the setting already has. The spellings are pnpm
11.9.0's. The VALUES are what the matching nub.jsonc field accepts:
`--minimum-release-age` takes the same `<integer><unit>` grammar as
`install.minimumReleaseAge` (s|m|h|d|w) by calling the same parser, so the file
and the flag cannot drift, and `--minimum-release-age-exclude` takes the entries
of `install.minimumReleaseAgeExclude`.

A bare number on the flag means MINUTES, which nub.jsonc deliberately rejects
(npm counts days, pnpm counts minutes, so an unqualified number in a config file
is a trap). On the CLI that ambiguity is already settled by the tool nub mirrors.
Sub-minute durations round UP, so tightening the gate can never silently disable
it — the same rule, and reason, as the bunfig seconds->minutes path.

No strictness flag, and no `install.minimumReleaseAgeStrict` in nub.jsonc. The
gate is enforced, and `--minimum-release-age=0` is the opt-out: turn the window
off rather than keep one and quietly install versions that fail it. pnpm's
`minimumReleaseAgeStrict` defaults to false, leaving its 24h window advisory
unless separately opted into; nub does not reproduce that. Verified 0 disables
completely — including alongside `minimumReleaseAgeStrict=true` and
`paranoid=true`, neither of which can resurrect a zero window.

Three defects in the age-gate help fixed alongside:

- `nub add` emitted its refusal with no help text at all, while the install path
  had full remedies.
- The exclude remedy named `task.name`, the user-facing alias, but the exempt
  matcher binds `task.registry_name()`. For `"foo": "npm:real-pkg@^1"` nub
  printed `--minimum-release-age-exclude=foo`, which clap and the config parser
  both accept and the matcher then silently ignores. Both detail structs now
  carry the registry identity and the remedies print it. (`add` already used the
  registry name — there the alias lives in `spec.alias` — so it is unchanged.)
- Every remedy pointed at turning the gate off, though the flag takes a duration.
  Both help paths now lead with shortening and treat `0` as the limit case.

Plus the arity hazard already documented for `--loglevel`: `nub install
--minimum-release-age 0` read `0` as a package and misrouted to `add`. Both value
flags are registered in VALUE_FLAGS, the nubx pre-positional scanner, and the dlx
scanner's pinned list (added by #587), with a regression test confirmed to fail
without the fix.
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.

2 participants