Skip to content

fix(linker): link workspace bins into dependent packages#353

Merged
jdx merged 2 commits intomainfrom
claude/quizzical-noether-89b0f3
Apr 27, 2026
Merged

fix(linker): link workspace bins into dependent packages#353
jdx merged 2 commits intomainfrom
claude/quizzical-noether-89b0f3

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented Apr 27, 2026

Summary

When workspace package A declares bin and B depends on A via workspace:*, pnpm symlinks A's bin into B/node_modules/.bin/ so B's npm scripts can call it. aube didn't, breaking any workspace package that runs a bin provided by another workspace package.

Root cause: workspace deps have no .aube/<dep_path>/<name>/ materialization — the linker symlinks them straight into the importer's node_modules/, so link_bins_for_dep reads package.json from a path that doesn't exist and returns Ok(None) silently.

Fix: route workspace deps through a new link_bins_for_workspace_dep helper that reads package.json from the workspace package's own directory (already known via ws_dirs) and shims each bin entry. Both the root pass (link_bins) and the per-importer workspace loop pick up the change.

Reported in #352 (item 6, workspace-bin-not-linked).

Test plan

  • cargo clippy --all-targets -- -D warnings
  • cargo test --package aube install::bin_linking
  • ./test/bats/bin/bats test/workspace.bats — all 17 tests pass, including new "workspace dep bins land in dependent's node_modules/.bin"
  • Related: test/root_bin.bats, test/workspace_member_install_walks_up.bats, test/optimistic_and_bin_settings.bats all pass

Note

Medium Risk
Changes bin-linking behavior during installs by adding a separate code path for workspace: dependencies, which could affect which executables appear in .bin across workspaces. Risk is moderate due to touching core install/linking logic but is localized and covered by a new integration test.

Overview
Fixes aube installs to link executables (bin entries) from workspace: dependencies into each dependent package’s node_modules/.bin, matching pnpm behavior (previously skipped because workspace deps aren’t materialized under .aube/<dep_path>).

Adds a dedicated link_bins_for_workspace_dep helper that reads the workspace package’s own package.json, shims its bin entries, and uses a per-install WsPkgJsonCache to avoid repeated reads across multiple importers; wires this path into both the root bin-link pass and the per-workspace-importer bin-link loop, and adds a bats test asserting the bins land in multiple consumers.

Reviewed by Cursor Bugbot for commit 5c17f3e. Bugbot is set up for automated code reviews on this repo. Configure here.

When workspace package A declares `bin` and B depends on A via
`workspace:*`, pnpm symlinks A's bin into B/node_modules/.bin so
B's npm scripts can call it. aube skipped these because workspace
deps have no `.aube/<dep_path>/<name>/` materialization — the linker
symlinks them straight into the importer's `node_modules/`, so
`link_bins_for_dep` reads `package.json` from a path that doesn't
exist and returns Ok(None).

Route workspace deps through a new `link_bins_for_workspace_dep`
helper that reads `package.json` from the workspace package's own
directory (already known via `ws_dirs`) and shims each bin entry.
Both the root pass (`link_bins`) and the per-importer workspace
loop pick up the change.

Reported in #352.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

Fixes missing bin shims for workspace:* dependencies by introducing link_bins_for_workspace_dep, which reads the workspace package's own package.json directly (since workspace deps have no .aube/<dep_path> materialization) and creates the appropriate shims. A WsPkgJsonCache deduplicates reads across importers, and the change is wired into both the root and per-importer bin-linking passes. The implementation and error-handling are clean; the only concern is a test assertion portability issue with readlink -f (see inline comment).

Confidence Score: 5/5

Safe to merge; the fix is well-scoped to the install/bin-linking path and covered by a new integration test.

Only P2 findings present — no logic errors or security concerns in the implementation. The sole issue is test portability of the readlink -f assertion, which does not affect correctness of the feature.

test/workspace.bats — the readlink -f assertion should be hardened for non-symlink shim configurations.

Important Files Changed

Filename Overview
crates/aube/src/commands/install/bin_linking.rs Adds link_bins_for_workspace_dep with its own WsPkgJsonCache, correctly routing workspace deps through their on-disk package.json instead of the absent .aube/<dep_path> materialization; error handling and cache logic look correct.
crates/aube/src/commands/install/mod.rs Threads ws_dirs and ws_pkg_json_cache into both the root link_bins pass and the per-importer workspace loop; cache lifetime spans both call sites correctly.
test/workspace.bats New bats test covers the core fix and exercises the cache via two consumers, but the readlink -f assertion is only valid for symlink shims; will fail under POSIX shell shim configurations.

Fix All in Claude Code

Reviews (2): Last reviewed commit: "fix(linker): cache workspace package.jso..." | Re-trigger Greptile

Comment thread crates/aube/src/commands/install/bin_linking.rs
`link_bins_for_workspace_dep` was reading and parsing the workspace
package's `package.json` once per consumer. In a monorepo where a
shared tooling package with bins is a `devDependency` of dozens of
workspace members (the discussion #352 reporter cites ~100), that
multiplies into N redundant reads per install.

Add a per-install `WsPkgJsonCache` keyed by `ws_dir`, threaded through
`link_bins` and the per-importer loop alongside the existing
`PkgJsonCache`. Updates the bats test to install two consumers of the
same workspace bin so the cache path is exercised.
@jdx jdx merged commit 55f1db4 into main Apr 27, 2026
17 checks passed
@jdx jdx deleted the claude/quizzical-noether-89b0f3 branch April 27, 2026 22:05
@greptile-apps greptile-apps Bot mentioned this pull request Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant