Skip to content

aube: sync upstream v1.35.0 - #621

Merged
colinhacks merged 6 commits into
mainfrom
aube-135
Jul 29, 2026
Merged

aube: sync upstream v1.35.0#621
colinhacks merged 6 commits into
mainfrom
aube-135

Conversation

@colinhacks

Copy link
Copy Markdown
Contributor

Merges jdx/aube v1.32.0..v1.35.0 (79 commits) into vendor/aube.

Behavior: add gains minimumPackageAge (30d default, fail-closed; TTY prompt, else ERR_AUBE_NEW_PACKAGE_NAME) plus similar-name and host-prompt gates. Scoped to add — install/dlx ungated, public npm only, lockfile entries exempt. node-gyp bootstrap runs in-process. update skips deprecated versions in range.

Second commit retires nub #394: upstream BinLaunch::Direct covers it off the same signal, honors preferSymlinkedExecutables: false, and keeps NODE_PATH. Upstream test for the case restored, passes.

Gates green both workspaces: check, clippy -D warnings, fmt; aube 2771 tests.

Overlaps #612 #609 #603 #587 #583 #577 — in-tree wins, second merger resolves.

Merges jdx/aube v1.32.0..v1.35.0 (79 commits) into the vendored tree via the
venue-less 3-way merge in .claude/skills/aube-bump. 28 files conflicted
textually; the compiler surfaced 3 more in files git merged silently.

Behavior changes users can hit:

- `add` gains three refusal surfaces from upstream: similar-name blocking,
  host-routed confirmation prompts, and minimumPackageAge, which defaults to
  43200 minutes (30 days) and fails closed. A public package name first
  registered inside that window now prompts interactively or fails with
  ERR_AUBE_NEW_PACKAGE_NAME; missing registry creation-time metadata fails
  with ERR_AUBE_PACKAGE_AGE_CHECK_FAILED.
- node-gyp bootstrap runs in-process under the project lock instead of
  re-execing. Upstream's rationale for the rewrite is the embedding case nub
  is: current_exe() names the host program, not aube.
- `update` no longer prefers a deprecated version inside a range.
- RuntimeProvenance::Embedder now renders the embedder name ("nub") where it
  previously rendered the literal "embedder".
- A fat64 Mach-O bin target is now classified native.

Fork delta that dissolved because upstream adopted it: registry-name patches
on npm-aliased installs, native-bin direct exec, and the exec.rs
is_native_executable_target gate. nub's EmbedderRuntime seam was dead code on
nub's side and gave way to upstream's; nub's EngineContext ladder is re-sited
into it and still drives the augmentation.

Two nub-side repairs:

- Gate npmrc_entry_is_supported to match its only caller, read_single, which
  is behind config-tui. Embedders building default-features = false dropped
  the caller but kept the helper, tripping dead_code under -D warnings.
- Pin nub's #394 invariant (native targets force the symlink layout even when
  preferSymlinkedExecutables is false) with a test. It was untested, and
  upstream v1.35.0 ships a test asserting the opposite behavior, which is
  removed here with a provenance note.
aube v1.35.0 classifies a native-executable target as BinLaunch::Direct and
every shim generator — posix, cmd, ps1, sh — emits a direct exec for it. That
reaches #394's goal (never hand a native binary to `node`) at the same point in
the call graph and off the same magic-byte signal, so nub's override is
redundant.

It was also worse on both counts it differed:

- It ignored an explicit `preferSymlinkedExecutables: false`, forcing a symlink
  the caller had asked not to have.
- A symlink carries no NODE_PATH. The direct-exec shim does.

Removes the Unix write_shim clause, the Windows early return, the three
nub-only *_shim_direct generators, and is_native_executable_target, which has
no remaining caller. is_native_executable stays — upstream's classifier reads
it.

The nub test that pinned the old behavior now asserts the new one, keeping its
ELF fixture. Upstream's own bin_command_executes_native_target_behind_generated
_shim, which this override made unreachable and which had to be dropped when the
tree was merged, is restored and passes.
Copilot AI review requested due to automatic review settings July 29, 2026 18:18

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 wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@vercel

vercel Bot commented Jul 29, 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 Jul 29, 2026 9:15pm

Request Review

Picks up #612, #609 and #613, which landed while this bump was in flight and
which the PR body flagged as overlapping.

One conflict, in commands/outdated.rs: #609 added a HashSet import and still
called max_satisfying_version, which aube v1.35.0 renames to wanted_version.
Unions the import and keeps the new name. The argument stays unreferenced —
packument binds to &Packument out of the Some(Ok(p)) arm.
aube v1.35.0 drops `vite`/`vitepress` from
`disableGlobalVirtualStoreForPackages` (jdx/aube#1106 makes the global
virtual store work under Vite, so auto-disabling for it is obsolete). The
vendored tree took that default correctly; the fork-discipline test still
pinned the pre-v1.35.0 list, so it read the change as nub having modified
aube's built-in default.

Updating the pinned list alone would have left the test weaker than it
looks: upstream's new default is now identical to the list the test
registered as the embedder override, so assertion 2 would pass whether or
not the embedder tier was consulted. The override fixture is now disjoint
from the built-in default, and named for what it is rather than implying
nub ships it — nub_setting_defaults carries only the #315 vite
diskMaterializePackages eject, no GVS list.

Also corrects the skill's step 4, which is how this reached CI: it ran
`cargo test --workspace --lib`, and `--lib` skips crates/*/tests/**, so
every fork-discipline integration test was silently unexecuted. CI runs
`cargo test --workspace` from vendor/aube. Records the invocation-directory
rule too — cargo reads .cargo/config.toml from the invocation dir, and
aube's sets RUST_TEST_THREADS=1 for its env-mutating tests.

@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

Three things to settle before merging: nub's .modules.yaml writer now overwrites the one upstream started writing, nub's Vite < 8.1 dist backport is duplicated by upstream's, and the new 30-day minimumPackageAge gate ships fail-closed on nub add with no embedder-tier decision recorded.

Reviewed changes — a 79-commit jdx/aube v1.32.0→v1.35.0 vendored-fork sync, plus one nub-side fork-delta retirement. Only three files sit outside vendor/aube, so the review focused on where upstream's new behavior meets nub's existing overrides.

  • Retire the #394 native-bin symlink override — removes nub's Unix write_shim clause, the Windows early return, the three *_shim_direct generators, and is_native_executable_target; upstream's BinLaunch::Direct reaches the same goal off the same magic-byte signal while honoring preferSymlinkedExecutables: false and keeping NODE_PATH. The nub test was rewritten against its ELF fixture and upstream's bin_command_executes_native_target_behind_generated_shim restored. Correct, and it thins the fork delta.
  • New add-time supply-chain gatesminimumPackageAge (43200 minutes, default-on) plus a similar-name gate, scoped to add; run_cli_name_gates has exactly three call sites, all under commands/add/, so install/ci/update/dlx genuinely do not reach it. Names already in the active lockfile are folded into allowedUnpopularPackages and exempt.
  • Vite/GVS compat moved upstream — aube now ships write_modules_metadata, patch_legacy_vite_copies, and a Linker::with_project_local_dep_paths that ejects legacy-vite dep paths, and drops vite/vitepress from the disableGlobalVirtualStoreForPackages default.
  • node-gyp bootstrap runs in-process — replaces re-exec'ing std::env::current_exe, which under an embedder names the host program. A genuine embedder bug fix that nub is the beneficiary of.
  • Store relocation settings — new globalVirtualStoreDir and a broadened cacheDir move the real virtual store at runtime, with WARN_AUBE_GVS_CROSS_VOLUME when it lands off storeDir's volume.
  • Dependency bumps in Cargo.lockclx 2.1.0→3.0.2 (dual-versioned with nub-cli's 2.1.0), usage-lib 2.18.2→4.0.0, clap_usage 2.0.3→4.0.0, roff 0.2.2→1.1.1.

I separately verified the brand boundary held across the merge: every new AUBE_* settings alias routes through aube-util/src/env.rs::branded_env_alias_enabled (untouched here), the two auto-merged VIRTUAL_STORE_SUBDIR hardcodes the author caught are the complete set, and the new prompts — though they bypass present.rs by writing straight to stderr — contain no aube/ERR_AUBE_*/jdx-URL strings.

⚠️ nub's Vite < 8.1 dist backport is now duplicated by upstream's, with no deletion plan

Upstream v1.35.0 brought its own copy of the mechanism crates/nub-cli/src/pm_engine/vite_compat.rs exists to provide. Both now run on every install, on the same files, and neither knows about the other:

Concern nub's vite_compat upstream install/gvs.rs
Eject legacy Vite project-local diskMaterializePackages seed, direct-dep vite only with_project_local_dep_paths via legacy_vite_project_local_closure
Write .modules.yaml write_modules_yaml, workspace root only write_modules_metadata, every physical importer
Patch the < 8.1 dist marker __nubRfs(__nubJoin VITE_COMPAT_MARKER = __aubeRfs(__aubeJoin

Because the markers differ and upstream's anchor list is a strict superset of nub's, each patcher's idempotency guard passes for the other's output, so a project-local Vite below 8.1 receives both inserts — two prepended import lines and two sniff blocks after the same anchor, pushing the same directory onto allowDirs twice. That is functionally tolerable rather than broken, which is exactly why it will go unnoticed.

Technical details
# nub's Vite compat is now redundant with upstream's

## Affected sites
- `crates/nub-cli/src/pm_engine/vite_compat.rs:301-349` — nub's `PREPEND` / `INSERT` / `ANCHORS` / `MARKER`. `ANCHORS` = `["let allowDirs = server.fs.allow;", "let allowDirs = server.fs?.allow;"]`.
- `vendor/aube/crates/aube/src/commands/install/gvs.rs:500-506` — upstream's `VITE_COMPAT_PREPEND` / `VITE_COMPAT_ANCHORS` / `VITE_COMPAT_MARKER`. `VITE_COMPAT_ANCHORS` is nub's two strings plus a third (`let allowDirs = (_a = server.fs) === null || _a === void 0 ? void 0 : _a.allow;`).
- `vendor/aube/crates/aube/src/commands/install/finalize.rs:229-248` — engine-side call order: `write_modules_metadata` whenever `!virtual_store_only`, then `patch_legacy_vite_copies` when `planned_gvs && node_linker == Isolated`.
- `crates/nub-cli/src/pm_engine/install_family.rs:892` — nub's `vite_compat::apply` runs after the engine call returns, so nub's writer/patcher is always second.
- `vendor/aube/crates/aube/src/commands/install/link.rs:212``with_project_local_dep_paths(gvs::legacy_vite_project_local_closure(graph_for_link))`, upstream's ejection.
- `crates/nub-cli/src/pm_engine/mod.rs:2225-2232` — nub's `diskMaterializePackages` vite seed, the other half of the duplicated ejection.

Why the double patch lands: upstream patches first via `patch_vite_file`, which does
`source.replacen(anchor, &format!("{anchor}{insert}"), 1)`. The anchor substring survives
verbatim, so nub's `patch_one` then finds it, sees only its own `__nubRfs(__nubJoin`
marker absent, and inserts again.

## Required outcome
- Exactly one mechanism patches a given Vite dist file, and exactly one writes `.modules.yaml`.
- Whichever is kept must still cover the cases nub's version was built for: the library-embedded (Astro/SvelteKit/VitePress) shape, and the `fs.allow`-already-set shape that motivated nub's move off the `if(!allowDirs)`-scoped anchor.

## Suggested approach
Upstream's version is the settings-aware one and covers a third anchor form nub's does
not, so the natural direction is to retire nub's `vite_compat` Units A and B and the
`diskMaterializePackages` vite seed, keeping only whatever upstream genuinely does not
cover. Before deleting, confirm against `tests/vite-compat/` that upstream's
`legacy_vite_project_local_closure` ejects the same set nub's direct-dep seed did, and
that upstream's per-importer `.modules.yaml` satisfies `searchForWorkspaceRoot`.

## Open questions for the human
- Is upstream's backport now considered sufficient for issue #315, or does nub keep its own because upstream's may drift or be feature-gated later?
- If nub's is retired, does the `__NUB_VITE_COMPAT_DISABLE` A/B seam and the `tests/vite-compat/` matrix move to driving upstream's mechanism, or get dropped?

⚠️ minimumPackageAge ships at 30 days, fail-closed, with no embedder-tier decision

nub_setting_defaults already pins the sibling supply-chain knobs at nub's embedder tier rather than inheriting whatever the engine currently defaults to — minimumReleaseAge at 1440 and minimumReleaseAgeStrict at true, with a comment explaining that Nub advertises a real 24-hour floor. This PR adds a third knob in the same family and does not extend that treatment to it, so nub silently inherits upstream's 43200-minute default.

The severity comes from what the fail-closed branch treats as a failure:

Err(error) => Err(miette!(
    code = ERR_AUBE_PACKAGE_AGE_CHECK_FAILED,
    "could not verify the package-name age for {name}; refusing to bypass `minimumPackageAge`: {error}"
)),

Every aube_registry::Error variant lands there — transport failure, timeout, 5xx, a 429 that outlived the client's retry loop, NotFound, Offline — indistinguishable from a genuinely newly-registered name. Names already in the active lockfile are exempt, so this does not touch a re-add, but any first-time nub add of an unlocked package turns a transient npm blip into a hard failure whose message is about slopsquatting.

Technical details
# `minimumPackageAge` default and fail-closed granularity

## Affected sites
- `vendor/aube/crates/aube-settings/settings.toml:322-344``[minimumPackageAge]`, `default = "43200"`.
- `vendor/aube/crates/aube/src/commands/add_supply_chain.rs:231-247``verified_package_created`; both the `Ok(missing "created")` and `Err(_)` arms produce `ERR_AUBE_PACKAGE_AGE_CHECK_FAILED` with no discrimination by error kind.
- `vendor/aube/crates/aube/src/commands/add_supply_chain.rs:202-212` — the gate loop; fetches a full packument sequentially per candidate name whenever `minimum_package_age_minutes > 0`.
- `vendor/aube/crates/aube/src/commands/add/supply_chain.rs:35-40` — the lockfile escape hatch (`locked_registry_names` folded into `allowed_unpopular`), which is what bounds the blast radius to first-time adds.
- `crates/nub-cli/src/pm_engine/mod.rs:2255-2259` — where `minimumReleaseAge` / `minimumReleaseAgeStrict` are pinned at nub's embedder tier. The precedent, and the place a `minimumPackageAge` decision belongs.
- `vendor/aube/crates/aube-codes/src/errors.rs:393-398``ERR_AUBE_PACKAGE_AGE_CHECK_FAILED` carries `exit_code: None`, unlike the sibling `ERR_AUBE_LOW_DOWNLOAD_PACKAGE` (`Some(47)`), so scripts cannot branch on it.

## Required outcome
- nub records a deliberate `minimumPackageAge` value at its embedder tier, the way it already does for `minimumReleaseAge`, rather than inheriting upstream's by omission.
- A registry transport failure does not present as, or block like, a newly-registered-name refusal. Whatever the chosen policy, the two must be distinguishable to the user and to a caller reading the exit code.

## Suggested approach
Splitting `verified_package_created`'s `Err` arm by variant is the root-cause fix and
belongs upstream: `Http`/`Offline`/timeout are availability failures, `NotFound` is a
genuinely absent package, and only a well-formed packument missing `time.created` is a
real "cannot verify" condition. If that has to wait on upstream, the embedder-tier
default is the lever nub controls today.

## Open questions for the human
- What value does nub want here? Keeping 30 days is defensible for a slopsquatting gate; the question is whether it is defensible while any npm hiccup fails the command.
- Does the AGENTS.md prohibition on volunteering upstream contributions cover filing this as a defect against `jdx/aube`, as opposed to sending a patch?

ℹ️ --silent turns the new add-time prompts into hard refusals

The three gates decide interactivity with std::io::stdin().is_terminal() && std::io::stderr().is_terminal(), falling through to a refusal when either is false. nub's quiet path calls aube::silence_own_output(), which dup2s fd 2 onto /dev/null and holds it for the duration of the engine call — including the window in which the gates run.

The consequence is a pair of commands that behave differently in the same terminal:

nub add some-new-package            # prompts, user confirms, add proceeds
nub add some-new-package --silent   # ERR_AUBE_NEW_PACKAGE_NAME, no prompt shown

The mechanism predates this PR and already affected the low-download gate, so this is surfaced for awareness rather than as a regression. What changed is the reach: minimumPackageAge is default-on, so the flag combination is now something an ordinary user hits.

Technical details
# `silence_own_output` defeats TTY detection for the add-time gates

## Affected sites
- `vendor/aube/crates/aube/src/commands/add_supply_chain.rs:952-957` — similar-name gate interactivity check.
- `vendor/aube/crates/aube/src/commands/add_supply_chain.rs:1136-1141` — package-age gate, same check.
- `vendor/aube/crates/aube/src/commands/add_supply_chain.rs:1081-1086` — low-download gate, same check (pre-existing, refactored here).
- `crates/nub-cli/src/pm_engine/install_family.rs:306-319` — nub's `--silent` / `-s` / `--reporter=silent` / `--loglevel=silent` path into `aube::silence_own_output()`.
- `vendor/aube/crates/aube/src/lib.rs:283,321-331` — the `libc::dup2(devnull, 2)` that makes `stderr().is_terminal()` report false.

## Required outcome
Either a silenced install is documented and treated as non-interactive on purpose, or the
gates' interactivity decision is made from something the silencing does not clobber.

## Suggested approach
Capturing the TTY-ness of fd 1/2 before `silence_own_output` redirects them and passing
that through to the gate would keep the prompt reachable, but it needs an upstream seam.
Deciding that `--silent` means non-interactive is also a coherent answer — it just needs
to be a decision rather than an emergent property of a `dup2`.

## Open questions for the human
- Is "silent implies non-interactive" the intended contract? If so this needs a line in the PM docs, since the failure mode reads as a bug from the terminal.

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

Comment on lines +241 to +247
/// Only the DEFAULT location is reproduced here. aube v1.35.0 added the
/// `globalVirtualStoreDir` / `cacheDir` settings, which relocate the real store
/// at runtime; the resolver for those (`commands::settings_context::
/// global_virtual_store_dir`) is `pub(crate)` to the aube crate, so nub cannot
/// consult it without widening that surface. A project that sets either setting
/// therefore gets a `.modules.yaml` naming the default path rather than the
/// relocated one, and Vite would not be told to allow the real store.

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 gap is real but the framing understates it, and the pub(crate) visibility is not the blocker. aube v1.35.0 already writes .modules.yaml itself with the settings-resolved store (install/gvs.rs::write_modules_metadata, called from install/finalize.rs:235 whenever !virtual_store_only), and when the file is absent it writes a single-key pretty-printed JSON object — which is_nub_modules_yaml accepts as nub's own stub. Since vite_compat::apply runs after the engine returns, nub does not merely fail to consult the resolver: it overwrites a correct value with the default one.

Technical details
# nub's `.modules.yaml` writer clobbers the engine's settings-resolved value

## Affected sites
- `crates/nub-cli/src/pm_engine/vite_compat.rs:248-250``global_virtual_store_dir()` reproduces only `cache_dir().join(embedder().virtual_store_subdir)`.
- `crates/nub-cli/src/pm_engine/vite_compat.rs:264-280``write_modules_yaml`; the `is_nub_modules_yaml` guard is what lets the clobber through.
- `crates/nub-cli/src/pm_engine/vite_compat.rs:286-294``is_nub_modules_yaml` returns `true` for any single-key JSON object with `virtualStoreDir`, which is exactly the shape `write_modules_metadata` emits via `serde_json::to_vec_pretty` on the absent-file path.
- `crates/nub-cli/src/pm_engine/install_family.rs:892``vite_compat::apply` runs post-engine, so nub always writes last.
- `vendor/aube/crates/aube/src/commands/install/finalize.rs:229-237` — engine-side write, using `store.virtual_store_dir()` under GVS+`Isolated` and `aube_dir` otherwise.
- `vendor/aube/crates/aube/src/commands/settings_context.rs:544` — the settings-aware resolver, which honors `globalVirtualStoreDir` then falls back to `resolved_cache_dir(cwd).join(embedder().virtual_store_subdir)`.

When the two values diverge:

| Situation | engine writes | nub overwrites with | effect |
| --- | --- | --- | --- |
| defaults | default global store | same | none |
| `cacheDir` or `globalVirtualStoreDir` set | the relocated store | the default store | Vite is told to allow a path it never serves from, and the real store stays disallowed → `403 … outside of Vite serving allow list` |
| GVS off (`next`, `react-native`, `hoisted`) | project-local `node_modules/.store` | the global store | benign; the project-local path is inside the workspace root and allowed anyway |

Also note the engine writes one file per physical importer while nub writes only the
workspace root's, so in a monorepo the root file and the member files disagree after
nub's pass.

## Required outcome
- A project that relocates its store via `cacheDir` or `globalVirtualStoreDir` ends up with a `.modules.yaml` naming the real store.
- nub never replaces a `virtualStoreDir` written by the engine with a less-informed value.

## Suggested approach
The cheapest correct change needs no visibility widening: treat an existing
`virtualStoreDir` as authoritative and leave the file alone, since the engine has
already written the resolved path by the time `apply` runs. If nub's Unit A is retired
entirely in favour of upstream's writer (see the review body), this resolves itself.

## Open questions for the human
- Is nub's Unit A still needed at all now that the engine writes the same file for every importer with the resolved path?

All four assert against aube-branded filenames nub deliberately does not
use, so they are permanent divergences rather than gaps to close.

The three `-r --latest` catalog tests declare their workspace in
aube-workspace.yaml. workspace_markers() does not consult it, so `-r` finds
no workspace root and refuses to filter. The sibling test that declares the
same catalog in package.json passes untouched, which is the discriminator.

`workspace update.ignoreDeps takes precedence` gets further — the update
itself succeeds — then greps aube-lock.yaml, and nub's canonical lockfile
is nub.lock. Same reason as the block of update.bats entries already in
that section.

@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 coverage observation inline.

Reviewed changes — three files moved since the prior review at bedde94, all test or documentation; no production code changed.

  • Retargeted the GVS embedder-seam test at v1.35.0's defaultAUBE_DEFAULT now pins ["next", "nuxt", "parcel"], matching settings.toml after upstream dropped vite/vitepress, and the embedder-side constant became a deliberately disjoint stand-in (["svelte-kit", "astro"]). Worth noting the constant it replaced was already wrong: nub_setting_defaults ships next,react-native, not next,nuxt,parcel, so the old assertion both misdescribed nub's list and would have gone vacuous the moment it matched AUBE_DEFAULT. The stand-in fixes both, and nub's real list stays pinned nub-side in pm_engine/mod.rs.
  • Skipped four update.bats tests v1.35.0 adds — three catalog-update tests whose fixtures declare the workspace in aube-workspace.yaml, which workspace_yaml_names() does not return under nub, plus the update.ignoreDeps precedence test, whose assertions grep the fixture's committed aube-lock.yaml. The stated reasons check out, and the awk stager in run.sh exits non-zero on any entry matching no test, so the four names are CI-verified exact rather than best-effort.
  • Documented the test invocation in the aube-bump skill--workspace with no --lib, run from inside vendor/aube. Both halves match reality: aube-parity.yml runs cargo test --workspace under working-directory: vendor/aube, and the RUST_TEST_THREADS = "1" rationale matches the comment in vendor/aube/.cargo/config.toml.

The update.ignoreDeps precedence the fourth skip gives up is still covered by update.rs::canonical_update_ignores_replace_package_json_legacy_values, a unit test that reads pnpm-workspace.yaml, so that one costs nothing. The three catalog skips are a different story — see inline.

ℹ️ Nitpicks

  • gvs_disable_list_embedder_default.rs:27 cites #1106 bare. Read from nub's repo that number scans as a nub issue; jdx/aube#1106 is unambiguous.

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

Comment thread tests/aube-bats/skips.txt
Comment on lines +46 to +48
update.bats|aube update -r --latest updates a shared catalog entry|catalog fixture declares its workspace in aube-workspace.yaml, which workspace_markers() does not consult, so -r has no workspace root to filter against
update.bats|aube update -r --latest --no-save leaves the catalog range unchanged|catalog fixture declares its workspace in aube-workspace.yaml, which workspace_markers() does not consult, so -r has no workspace root to filter against
update.bats|aube update -r --latest updates a named catalog and preserves its prefix|catalog fixture declares its workspace in aube-workspace.yaml, which workspace_markers() does not consult, so -r has no workspace root to filter against

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.

These three fixtures differ from nub only in the workspace file's name, so skipping them is right — but they were the only coverage catalogs.rs::update_workspace_yaml_catalog_entries had. It has no unit test, and the one new catalog test that still runs under nub exercises the package.json sibling instead. The YAML path is live under nub for any pnpm-incumbent workspace with a catalog: block, since read_branded_pnpm_config puts pnpm-workspace.yaml in workspace_yaml_names().

Technical details
# Workspace-YAML catalog rewrite loses its only test

## Affected sites
- `tests/aube-bats/skips.txt:46-48` — the three new skips.
- `vendor/aube/test/update.bats:164-219` — the skipped tests: default-catalog rewrite plus comment preservation, `--no-save` leaving the range untouched, and named-catalog prefix preservation (`^0.1.2``^3.0.1`).
- `vendor/aube/crates/aube/src/commands/catalogs.rs:265-296``update_workspace_yaml_catalog_entries`, reached from `update.rs:793`. Handles the `catalog` vs `catalogs.<name>` submap split and range-only replacement through `edit_workspace_yaml`.
- `vendor/aube/crates/aube/src/commands/catalogs.rs:566+` — the `tests` module covers `decide_add_rewrite`, `prune_unused_catalog_entries`, and the manifest upserts. Nothing calls either `update_*_catalog_entries`.
- `vendor/aube/crates/aube-manifest/src/workspace/config.rs:21-30``workspace_yaml_names()` appends `pnpm-workspace.yaml` whenever `engine_context().read_branded_pnpm_config` is set, which is what makes the YAML branch reachable under nub.
- `vendor/aube/test/update.bats:221-246``aube update -r --latest updates a package.json catalog source`, correctly left unskipped; it covers `update_manifest_catalog_entries` only.

## Required outcome
- nub CI exercises the workspace-YAML catalog rewrite — default catalog, named catalog with a preserved range prefix, and `--no-save` — against a workspace filename nub actually reads.

## Open questions for the human
- Is this path pnpm-compat-only in practice? If a nub-identity project can never reach `update_workspace_yaml_catalog_entries`, the natural home for the replacement is the compat harness rather than `tests/aube-bats/`.
- If neither home is worth the fixture, is a `known-gaps.txt` entry the honest record, given `skips.txt` self-describes as permanent *intended divergences*?

Picks up nine commits, including #616 and #618, both of which touch
vendor/aube/crates/aube-linker.

One conflict, add/add in Linker::without_global_virtual_store: this branch
carries upstream v1.35.0's project_local_dep_paths, main carries #616's
reusable_hoisted. Both fields exist and are independent, so the copy
constructor takes both.

The PR had gone mergeable=false while main moved, and a pull_request run
needs refs/pull/N/merge, so GitHub had silently stopped dispatching any
workflow for the branch. This merge restores it.

@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 — the merge itself is clean; one bookkeeping gap noted below.

Reviewed changes — one commit landed since the prior review at 1f23a7a7: a merge of main into the branch, which is what restores workflow dispatch after the PR went mergeable=false. The earlier empty dfaa04ad re-dispatch commit is no longer in the history.

  • Resolved the single add/add conflict in the linker's copy constructorLinker::without_global_virtual_store now clones both project_local_dep_paths (upstream v1.35.0) and reusable_hoisted (main's #616), and Linker::new initializes both. Struct-literal exhaustiveness is what makes this resolution self-checking.
  • Brought nine main commits forward, two of them into vendor/aube/crates/aube-linker#616's hoisted placement reuse with its link-in-progress sentinel, and #618's recursive stale-entry removal in the workspace arm of link_all.
  • Changed nothing elsegit merge-tree --write-tree 1f23a7a7 f280bd3d reports that one conflict and no other, and the head tree differs from git's own auto-merge tree in that one file alone, where the edit is exactly "drop the three markers, keep both lines".

Rather than trust the auto-merge, I ran the gates on the merged tree: cargo check --workspace --all-targets and cargo test --workspace in vendor/aube (2854 passed, 0 failed, 4 ignored) plus the root cargo clippy --all-targets --all-features --profile fast -- -D warnings, all clean. The one dead_code warning (aube-linker/src/tests.rs::diff_trees, unused off macOS) predates this branch, and the aube CI leg runs cargo test rather than clippy, so it gates nothing.

Two interactions the compiler cannot see also check out. The two new Linker fields are populated under mutually exclusive NodeLinker modes — project_local_dep_paths only when Isolated, reusable_hoisted only when Hoisted — so they never coexist on one Linker. And #616's sentinel bracket survives v1.35.0's reshaped install flow: run_link_phase and run_finalize_phase have exactly one call site each, the finalize call sits downstream of run_link_phase(...)?, every state-directory reset (--force, gvs::reset_on_mode_change) runs before the mark, and the reuse vouch folds patch_hashes into its fingerprint, so the sync's alias-aware lookup_patch change cannot let a stale patch survive a reuse.

ℹ️ The bump skill's delta inventory no longer matches the delta it guards

.claude/skills/aube-bump/SKILL.md carries the list of nub fork delta that "must survive" a bump, and this PR edits that section — but two entries now drift in opposite directions. The Linker bullet still claims direct-exec of native bins as nub delta, which fbd40ff1 in this very PR retired in favor of upstream's BinLaunch::Direct; and neither that bullet nor the grep line above it mentions the linker delta this merge just folded in from main. Given the section's stated purpose — "grep after every bump, if one vanished a resolution was wrong" — a list that names a delta which no longer exists while omitting three that do is the one artifact a future bump has no way to notice is wrong.

Technical details
# `aube-bump` delta inventory is stale in both directions after this PR

## Affected sites
- `.claude/skills/aube-bump/SKILL.md:276` — the Linker bullet ends with `direct-exec of native bins`. Commit `fbd40ff1` deleted nub's `write_shim` clause, the three `*_shim_direct` generators, and `is_native_executable_target`; a grep of `vendor/aube/crates` now finds only upstream's `BinLaunch::Direct` / `detect_bin_launch` (`aube-linker/src/sys.rs:620-717`). The capability converged upstream, so it is no longer fork delta.
- `.claude/skills/aube-bump/SKILL.md:274-276` and `:262` (the grep line) — neither names the linker delta this merge brings in from `main`: `reusable_hoisted` plus `mark_link_in_progress` / `clear_link_in_progress` / `link_completed_cleanly` (#616), the recursive stale-entry removal in `link_all`'s workspace arm (#618), and the Windows `node_modules` reclaim (#613). All three live in `vendor/aube` as fork delta and none is upstreamed.

## Required outcome
- The inventory lists exactly the fork delta the vendored tree actually carries, so the post-bump grep can distinguish a lost resolution from a converged one.

## Suggested approach
Drop `direct-exec of native bins` from the Linker bullet — optionally noting it converged on upstream's `BinLaunch::Direct` in v1.35.0, which is the same "watch for upstream superseding fork delta" case the skill already documents — and add the reuse vouch plus sentinel, the recursive stale-entry removal, and the Windows reclaim. `reusable_hoisted` and `link_in_progress` are the natural greppable symbols for the line at `:262`.

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

@colinhacks

Copy link
Copy Markdown
Contributor Author

Shipped in v0.7.0: https://github.com/nubjs/nub/releases/tag/v0.7.0

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