fix(cli,linker,lockfile): patch-commit destination, CRLF patches, npm-alias catalog by jdx · Pull Request #384 · endevco/aube

jdx · 2026-04-29T17:28:03Z

Summary

Three bug fixes for endevco/aube#383 plus the supporting refactors that fell out of code review.

Bug fixes

Patches against CRLF text files — tarballs published from Windows editors ship CRLF (e.g. gifuct-js@2.1.2/index.d.ts), but git/pnpm-style patches always emit LF. Diffy is byte-exact and refused to match CRLF context against LF hunks, so a clean patch failed with error applying hunk #1. Normalize the original to LF before apply and restore CRLF on write — same approach pnpm uses. The CRLF restore also collapses any \r\r\n so a patch line containing a literal \r byte mid-line doesn't gain a second carriage return.
aube patch-commit destination — previously wrote unconditionally to pnpm.patchedDependencies in package.json, even on projects already using the pnpm v10+ workspace-yaml home. Now follows the unified rule below. aube patch-remove now strips entries from every place they could live (both yaml and both pnpm.* / aube.* namespaces) and reports the actual files rewritten in its status output.
npm-aliased catalog deps from pnpm-generated lockfiles (discussioncomment) — aube install --frozen-lockfile accepted a pnpm lockfile with beamcoder: npm:beamcoder-prebuild@… (declared via pnpm-workspace.yaml#catalog) and silently produced an empty node_modules. The importer's specifier: was 'catalog:', but the alias detection only fired on specifier.starts_with("npm:"). Now detects aliases purely from the version: shape (the canonical <real>@<resolved> form pnpm always uses), with a peer-suffix strip so version: 18.2.0(react@18.2.0) doesn't get misclassified.

Refactor: unified workspace-config writer

One rule applies to every command that mutates a setting which can live in either the workspace yaml or package.json#{pnpm,aube}.<key> (aube patch-commit, aube patch-remove, aube approve-builds, install-time auto-deny seeding, future settings):

If a workspace yaml exists on disk → write there.
Otherwise, if package.json#pnpm is already declared → write pnpm.<key> (preserve the user's chosen namespace).
Otherwise → write aube.<key> (aube's native namespace; the read side already gives aube.* precedence over pnpm.*).

New helpers in aube_manifest::workspace:

ConfigWriteTarget + config_write_target(dir) — picks step 1 vs steps 2/3.
edit_workspace_yaml(path, f) — yaml writer; skips the rewrite when the closure produces no structural change, so user comments survive every no-op write. Used by allowBuilds, patchedDependencies, and catalog cleanup.
edit_setting_map(cwd, key, f) — package.json writer; picks namespace per steps 2/3, also skips no-op rewrites.
remove_setting_entry(cwd, key, entry_key) — strips an entry from both pnpm.<key> and aube.<key> so a one-namespace removal doesn't leave a stale duplicate behind on the read merge.

Migrated callers: aube approve-builds (and the install-time auto-deny seed), aube patch-commit, aube patch-remove, catalog cleanup. The PatchDestination enum and edit_patched_dependencies helper that lived in aube/src/patches.rs are gone.

Refactor: workspace-yaml file selection

Centralized the aube-workspace.yaml → pnpm-workspace.yaml precedence walk into aube_manifest::workspace::workspace_yaml_existing. workspace_yaml_target is now a one-liner over it. Removed three duplicates of the same loop (catalogs.rs, find_workspace_root, find_workspace_yaml_root).

Test plan

cargo test (1185 passing — new coverage spans CRLF apply + embedded-\r restore in aube-linker; helper-level + per-writer no-op-skip tests for allowBuilds, patchedDependencies, and catalog cleanup; ConfigWriteTarget + namespace-picking cases; pnpm-lockfile catalog alias)
cargo clippy --all-targets -- -D warnings
cargo fmt --check
End-to-end repros from the discussion: aube install against patch-application-failure succeeds with CRLF preserved; aube install --frozen-lockfile against catalog-npm-alias installs beamcoder (resolved to beamcoder-prebuild) into packages/app/node_modules/

🤖 Generated with Claude Code

Note

Medium Risk
Touches install/patch/config persistence paths and changes where settings are written (workspace YAML vs package.json), which can affect real projects’ config layout; mitigated by extensive new tests and largely additive helpers.

Overview
Fixes patch application on CRLF-backed files by normalizing input to LF for diff matching and restoring CRLF on write (with protection for embedded literal \r), plus adds regression tests.

Unifies how workspace-level settings are written: if a workspace yaml exists, write there; otherwise write to package.json under aube.* unless pnpm already exists. This refactor is applied to allowBuilds seeding/approval, patch-commit/patch-remove (including removing entries from all possible locations and reporting the actual files changed), and catalog cleanup (now skips no-op YAML rewrites to avoid stripping comments).

Fixes pnpm lockfile parsing to detect npm-aliased dependencies based on the version: field shape (including catalog-declared aliases and peer suffix handling), with new tests.

^{Reviewed by Cursor Bugbot for commit 74cbd4d. Bugbot is set up for automated code reviews on this repo. Configure here.}

Tarballs published from Windows editors ship CRLF line endings, but git/pnpm-style patches always use LF. Diffy is byte-exact and refused to match CRLF context lines against LF hunks, so a clean patch failed with "error applying hunk #1" (e.g. gifuct-js@2.1.2). Normalize the original to LF before applying and restore CRLF on write when the source had it. Matches pnpm's behavior. Reported in #383. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`aube patch-commit` previously wrote unconditionally to `pnpm.patchedDependencies` in package.json, even on projects that already declared their patches in the (pnpm v10+ canonical) workspace yaml. Pick the destination by precedence: existing declarations win, otherwise prefer the workspace yaml when one is on disk, otherwise fall back to package.json. `aube patch-remove` strips entries from both files so a patch recorded in either location can always be cleared. Reported in #383. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-04-29T17:31:42Z

Greptile Summary

Three targeted bug fixes (CRLF patch apply, patch-commit destination routing, npm-alias catalog lockfile parsing) plus a unified workspace-config writer refactor that centralizes the yaml-vs-package.json write-target decision across all aube commands. The changes are well-scoped, heavily covered by new unit and E2E tests, and the previous review concerns (unconditional YAML rewrite on absent key, `

` double-CR) are both addressed in this revision.

Confidence Score: 5/5

Safe to merge — all three bug fixes are narrow, well-tested, and the refactor is backed by 1185 passing tests plus E2E repros.

No P0 or P1 findings. The CRLF `

collapse, no-op-skip for YAML rewrites, namespace selection, and peer-suffix stripping are all correctly implemented.pnpm_patched_dependencies()already reads bothpnpm.andaube.namespaces viapnpm_aube_objects(), so the read path correctly covers entries written by the new edit_setting_map. Adding "aube"toINSTALL_SHAPE_FIELDSensures hash invalidation for the newly-usedaube.*` namespace.

No files require special attention.

Important Files Changed

Filename	Overview
crates/aube-linker/src/lib.rs	CRLF normalization + restore for patch apply; includes `\r\r\n` collapse guard and two new targeted regression tests
crates/aube-lockfile/src/pnpm.rs	Alias detection moved from specifier-shape to version-shape with peer-suffix strip; regression test covers catalog-declared npm alias
crates/aube-manifest/src/workspace.rs	New `edit_workspace_yaml` helper with structural no-op skip, `ConfigWriteTarget` enum, `edit_setting_map`, `remove_setting_entry`, `upsert/remove_workspace_patched_dependency`; all covered by tests
crates/aube/src/patches.rs	Routing updated: `upsert_patched_dependency` returns the path written; `remove_patched_dependency` strips from both yaml and package.json; `read_patched_dependencies` merges both sources
crates/aube-util/src/hash.rs	Adds `"aube"` to INSTALL_SHAPE_FIELDS so changes to aube.* in package.json correctly invalidate the install hash

_{Reviews (8): Last reviewed commit: "fix(workspace): merge namespaces on edit..." | Re-trigger Greptile}

Three call sites duplicated the `aube-workspace.yaml` ↦ `pnpm-workspace.yaml` fallback walk: `catalogs::workspace_yaml_path`, `dirs::find_workspace_root`, and `dirs::find_workspace_yaml_root`. Add `workspace_yaml_existing` next to `workspace_yaml_target` in `aube-manifest::workspace` and route every caller through it so the precedence list lives in exactly one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two review-driven follow-ups to PR #384: - aube-manifest: peek the workspace yaml's `patchedDependencies` map before opening it for a rewrite. yaml_serde's round-trip drops user comments, and `aube patch-remove` calls remove on both yaml and package.json regardless of where the patch actually lives — so the no-op call would silently strip the comments the workspace-yaml location was chosen to host. Same protection on the upsert path so an idempotent re-record (re-`patch-commit` of an unchanged patch path) doesn't churn the file either. Mirrored on the package.json side in aube/src/patches.rs to skip its needless rewrite too. - aube-linker: when restoring CRLF on patched output, follow the bare `\n` -> `\r\n` promotion with a `\r\r\n` -> `\r\n` collapse so a patch line that contains a literal `\r` byte mid-line doesn't gain a second carriage return. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`remove_patched_dependency` now returns the list of files it rewrote (workspace yaml, package.json, both, or neither), and `aube patch-remove` prints the real filename in the "Removed <key> from <file>" line. The old code hardcoded "package.json" so workspace-yaml-resident patches got a bogus status. Same generalization in the empty-state error ("no patches declared") since patches no longer have to live in package.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Every workspace-yaml mutation in the codebase now goes through one helper, `aube_manifest::workspace::edit_workspace_yaml`, which compares the parsed Value before and after the closure and skips the rewrite (and atomic_write) on structural equality. yaml_serde isn't comment-preserving, so this is the only way to keep user comments across no-op writes. Migrated callers: - allowBuilds writer (`add_to_allow_builds` / approve-builds and the install-time auto-deny seed). Re-approving an already-approved package or re-seeding a name already on the list now leaves the file untouched. - patchedDependencies writer (`upsert_workspace_patched_dependency` / `remove_workspace_patched_dependency`). Drops the per-call peeks I added earlier — the centralized check covers them. - catalog cleanup (`prune_unused_catalog_entries`). Steady-state installs under `cleanupUnusedCatalogs` no longer rewrite when every declared entry is still referenced. Tests cover the no-op-skip on each surface (allowBuilds, patches, catalogs already covered by `prune_noop_when_all_entries_used`) plus a direct test of the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

One rule for every writer of a workspace-level setting that can live in either the workspace yaml or `package.json#{pnpm,aube}.<key>`: 1. If a workspace yaml exists on disk → write there. 2. Otherwise, if `package.json#pnpm` is already declared → write `pnpm.<key>` (preserve the user's chosen namespace). 3. Otherwise → write `aube.<key>` (aube's native namespace; the read side already gives `aube.*` precedence over `pnpm.*`). New helpers in `aube_manifest::workspace`: - `ConfigWriteTarget` + `config_write_target(dir)` — picks step 1 vs steps 2/3. - `edit_setting_map(cwd, key, f)` — handles steps 2/3 generically. No-op rewrite is skipped (mirrors `edit_workspace_yaml`'s comment-preservation guarantee), and now picks the namespace based on the existing manifest. - `remove_setting_entry(cwd, key, entry_key)` — strips an entry from both `pnpm.<key>` and `aube.<key>` so a one-namespace removal doesn't leave a stale duplicate behind on the read merge. Migrated callers: - `aube approve-builds` / install auto-deny seeding (`add_to_allow_builds`): yaml when one exists, else writes `aube.allowBuilds` (or `pnpm.allowBuilds` when the user already has a `pnpm` namespace in package.json). No more silent creation of a fresh `aube-workspace.yaml` on solo-package projects. - `aube patch-commit` / `aube patch-remove`: drops the local `PatchDestination` enum and `edit_patched_dependencies` helper in favor of the centralized ones. Remove now scrubs both namespaces via `remove_setting_entry`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Detect npm-aliased importer deps purely from the shape of `version:` instead of gating on `specifier.starts_with("npm:")`. pnpm encodes the alias in `version:` regardless of declaration form: - direct: `specifier: npm:beamcoder-prebuild@0.7.1` - catalog: `specifier: 'catalog:'` The catalog flavor has the alias declared in `pnpm-workspace.yaml#catalog`, so the importer's specifier stays `'catalog:'` and the previous gate silently dropped these deps — `aube install --frozen-lockfile` returned "Already up to date" while producing an empty `node_modules`. Strip any peer suffix before parsing so `version: 18.2.0(react@18.2.0)` (a regular dep with peers) does not parse as `name="18.2.0(react"`. Reported in #383 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The unified writer rule now lands in `package.json` under the `aube` namespace when no workspace yaml exists (and no `pnpm` namespace is already present in the manifest). Update the global-install bats test to assert the new destination instead of expecting a freshly created `aube-workspace.yaml`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`INSTALL_SHAPE_FIELDS` listed `pnpm` but not `aube`, so a package.json where `aube.allowBuilds` flips from `false` to `true` (the path `aube approve-builds` now takes on projects without a workspace yaml or a `pnpm` namespace) hashed identically to its prior state. The freshness check then short-circuited and a follow-up `aube install` skipped re-running the just-approved build scripts. Add `aube` to the field list so any change under `aube.*` invalidates the install-shape digest the same way `pnpm.*` already does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Several bats tests asserted the old "creates aube-workspace.yaml" behavior or read patches from `pnpm.patchedDependencies`. Under the unified writer rule the entries land in `package.json#aube.<key>` when there is no yaml on disk and no `pnpm` namespace already in the manifest. Update the assertions to match the new destination. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b79dd60. Configure here.}

…wing `edit_setting_map` previously wrote only to the chosen namespace (`pnpm` if a pnpm namespace existed in package.json, else `aube`). Reads merge `pnpm.<key>` and `aube.<key>` with `aube.*` winning on conflict — so when a pnpm-aware tool added a `pnpm` namespace AFTER aube had already populated `aube.<key>`, a subsequent `aube patch-commit` (or any other map mutation) wrote the new value to `pnpm.<key>` while the stale `aube.<key>` entry kept winning on read. Reported by Cursor's bugbot on PR #384. Restructure so the closure receives a merged view (matching read precedence) and the result lands in the chosen namespace, with the other namespace's `<key>` dropped. Post-write invariant: each entry lives in exactly one namespace, so reads can never silently shadow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…pm lockfiles (#403) ## Summary - pnpm encodes aliased transitive deps as `<alias>: <real>@<resolved>` inside a snapshot's dependency map (e.g. `string-width-cjs: string-width@4.2.3` for the `"string-width-cjs": "npm:string-width@^4.2.0"` declared by `@isaacs/cliui@8.0.2`). The reader passed the value through unchanged, so the linker keyed the symlink against `<dep_name>@<dep_value>` and produced a broken `string-width-cjs@string-width@4.2.3` virtual store path. On a re-resolve, the resolver's lockfile-reuse path enqueued a transitive task with the literal `string-width@4.2.3` range that no `string-width-cjs` version could satisfy, surfacing as `no version of string-width-cjs matches range \`string-width@4.2.3\``. - Detect the alias shape at parse, rewrite the dep value to the bare resolved version (preserving any peer-context suffix), and feed the existing `alias_remaps` channel that the importer path already uses to synthesize an alias-keyed `LockedPackage` with `alias_of=Some(real)`. The linker then resolves the alias symlink against the synthetic dir, and the resolver's reuse path matches the synthetic entry by name+version with no malformed range. - Reported in [#345](#345 (comment)), repro at [stevelandeydescript/aube-bug-repros/npm-alias-resolution-failure](https://github.com/stevelandeydescript/aube-bug-repros/tree/main/npm-alias-resolution-failure). The earlier #384 fix covered the importer-level alias case from a pnpm-generated lockfile; this is the matching fix for the snapshot dependency map. ## Test plan - [x] `cargo test -p aube-lockfile parse_synthesizes_npm_alias_for_transitive_deps` (new) - [x] `cargo test -p aube-lockfile parse_handles_npm_alias_for_transitive_deps_with_peer_suffix` (new) - [x] `cargo test -p aube-lockfile -p aube-resolver -p aube-linker` — full lockfile/resolver/linker suites - [x] `cargo clippy -p aube-lockfile --all-targets -- -D warnings` - [x] `cargo fmt --check` - [x] End-to-end against the repro: `pnpm install` → `rm pnpm-lock.yaml node_modules` → `aube install --frozen-lockfile` (or fresh resolve) → `node -e "require('jackspeak')"` succeeds. Before the fix, `string-width-cjs` symlink dangled; after, it resolves through the synthetic `string-width-cjs@4.2.3` entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  --- > [!NOTE] > **Medium Risk** > Changes pnpm lockfile dependency rewriting and package synthesis logic, which can affect install graph correctness if alias detection is too broad or misses edge cases; coverage is improved with targeted tests. > > **Overview** > Fixes pnpm v9 lockfile parsing for **npm-aliased transitive dependencies** encoded in `snapshots` as `<alias>: <real>@<resolved>(peers…)`. The parser now rewrites those snapshot dependency values to the bare resolved version (preserving any peer-context suffix) and feeds the existing `alias_remaps` path so alias-keyed `LockedPackage` entries are synthesized consistently. > > This logic is applied both in the main snapshot loop and when absorbing snapshot deps for local (`file:`) workspace packages, and includes new unit tests covering plain transitive aliases, peer-suffixed aliases, and local-package transitives; also slightly clarifies the missing-package error message for alias synthesis. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit df2fe15. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jdx and others added 2 commits April 29, 2026 12:27

greptile-apps Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/aube-manifest/src/workspace.rs

Comment thread crates/aube-linker/src/lib.rs

jdx and others added 2 commits April 29, 2026 12:32

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/aube/src/patches.rs Outdated

jdx and others added 2 commits April 29, 2026 12:49

jdx changed the title ~~fix(linker): apply patches against CRLF files; prefer workspace yaml for patch-commit~~ fix(cli,linker): apply CRLF patches; route patch-commit by destination Apr 29, 2026

jdx and others added 2 commits April 29, 2026 13:55

jdx changed the title ~~fix(cli,linker): apply CRLF patches; route patch-commit by destination~~ fix(cli,linker,lockfile): patch-commit destination, CRLF patches, npm-alias catalog Apr 29, 2026

jdx and others added 3 commits April 29, 2026 14:00

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/aube-manifest/src/workspace.rs

jdx merged commit 2f44140 into main Apr 29, 2026
17 checks passed

jdx deleted the claude/priceless-meitner-3caf20 branch April 29, 2026 19:47

jdx mentioned this pull request Apr 30, 2026

fix(lockfile): synthesize npm-alias entries for transitive deps in pnpm lockfiles #403

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cli,linker,lockfile): patch-commit destination, CRLF patches, npm-alias catalog#384

fix(cli,linker,lockfile): patch-commit destination, CRLF patches, npm-alias catalog#384
jdx merged 12 commits intomainfrom
claude/priceless-meitner-3caf20

jdx commented Apr 29, 2026 •

edited by cursor Bot

Loading

Uh oh!

greptile-apps Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented Apr 29, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug fixes

Refactor: unified workspace-config writer

Refactor: workspace-yaml file selection

Test plan

Uh oh!

greptile-apps Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Apr 29, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented Apr 29, 2026 •

edited

Loading