[rubnogueira]

Summary

Per @jdx's review, this backs out the workspace-root discovery change so a nested pnpm-workspace.yaml is once again a hard workspace boundary — matching pnpm. A pnpm-workspace.yaml with no packages: list configures a single-package workspace (just the root package); it does not mean "ignore this file and keep walking to an enclosing workspace". Treating a member's settings-only yaml as member-local config was a surprising divergence from aube's drop-in pnpm compatibility story.

The per-member lockfile freshness fix — the part jdx agreed with — already landed on main via #891. That install-state member-lockfile fingerprinting is what keeps repeat installs warm under sharedWorkspaceLockfile=false, so no discovery divergence is needed.

Changes

• aube (dirs): find_workspace_root treats any pnpm-workspace.yaml as a hard boundary again — no walk-past, no settings-only fallback scan.
• aube-manifest: drop WorkspaceYamlKind / workspace_yaml_kind and the load_present split; restore WorkspaceConfig::load → load_uncached.
• aube (node-gyp bootstrap): restore the empty stub marker — any pnpm-workspace.yaml halts discovery, so it no longer needs a dummy packages: glob.
• tests: drop the now-invalid walk-past regression; unit tests assert the hard-boundary semantics (a member's settings-only yaml resolves to itself; a standalone settings-only yaml resolves to itself).

Test plan

• cargo fmt --check
• cargo clippy -p aube -p aube-manifest --all-targets -- -D warnings
• cargo test -p aube find_workspace_root (8 passed)
• cargo test -p aube-manifest workspace (48 passed)

Summary by CodeRabbit

• Refactor
  • Simplified workspace root detection to stop at the first recognized workspace YAML file name, treating its presence as a hard boundary.
  • Improved workspace configuration loading to distinguish absent YAML vs present-but-empty/whitespace YAML, while keeping typed caching behavior safe.
  • Adjusted the node-gyp bootstrap workspace marker to use an empty stub YAML.
• New Features
  • Added a public API to classify workspace YAML state: Absent, SettingsOnly, or DeclaresMembers.
• Tests
  • Updated workspace root discovery tests for the new hard-boundary behavior.
  • Removed the outdated regression test covering settings-only nested member YAML handling.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• ✅ Review completed - (🔄 Check again to review again)

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b136ce75-6bb0-4dc2-88ea-1d9a95ba2535

📥 Commits

Reviewing files that changed from the base of the PR and between 0537dcd and 75e4632.

📒 Files selected for processing (3)

• crates/aube/src/commands/install/node_gyp_bootstrap.rs
• crates/aube/src/dirs.rs
• test/shared_workspace_lockfile_warm_path.bats

---

📝 Walkthrough

Walkthrough

WorkspaceYamlKind enum and workspace_yaml_kind probe function are added to classify a directory's workspace YAML as Absent, SettingsOnly, or DeclaresMembers. find_workspace_root_uncached is simplified to treat any existing workspace YAML as an immediate hard boundary. The node-gyp bootstrap marker is changed from a packages-declaring YAML to an empty stub, and the bats test asserting the former walk-past behavior is removed.

Changes

Workspace YAML hard-boundary and kind detection

Layer / File(s)	Summary
WorkspaceYamlKind enum, probe function, and load_present refactor crates/aube-manifest/src/workspace.rs, crates/aube-manifest/src/workspace/config.rs	WorkspaceConfig::load gains a load_present helper distinguishing absent YAML from empty YAML. New WorkspaceYamlKind enum and workspace_yaml_kind function classify a directory as Absent, SettingsOnly, or DeclaresMembers without warming the typed cache. Re-export list reformatted. Tests cover all classification outcomes including a no-cache regression.
Workspace root discovery simplified to hard boundary crates/aube/src/dirs.rs, test/shared_workspace_lockfile_warm_path.bats	find_workspace_root_uncached removes settings-only/members-declaring classification and the fallback-ancestor walk; any existing workspace YAML is now an immediate stop. Unit tests and comment updated to reflect the new boundary-on-presence rule. Bats regression test asserting the old walk-past behavior is deleted.
Node-gyp bootstrap writes empty stub marker crates/aube/src/commands/install/node_gyp_bootstrap.rs	bootstrap_blocking writes b"" instead of a packages-glob YAML as the workspace boundary marker in the node-gyp tool directory, relying on the new hard-boundary rule to halt discovery. Comments updated to describe the empty-stub stopping mechanism.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• jdx/aube#891: Modifies the same WorkspaceYamlKind/workspace_yaml_kind classification logic and workspace-root discovery for sharedWorkspaceLockfile=false member install behavior.
• jdx/aube#882: Changes which directory is treated as the workspace root and which lockfile gets written under sharedWorkspaceLockfile=false, directly affected by the hard-boundary rule introduced here.

Poem

🐇 Hop, hop—no more walking past!
Every YAML file holds the line so fast.
Empty stub or full of settings deep,
The boundary is set; the root won't creep.
No more fallback ancestors to find—
One YAML found, the root's defined! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the core change: treating nested pnpm-workspace.yaml files as hard workspace boundaries, which is the main functional shift in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR replaces the walk-past-settings-only-yaml workspace-root discovery logic (which used WorkspaceYamlKind to distinguish member yamls from root yamls) with a simpler rule: any pnpm-workspace.yaml is a hard boundary, matching pnpm's own behavior. The node_gyp_bootstrap sentinel yaml is simplified to an empty file since an empty yaml now correctly stops the walk.

• find_workspace_root now stops at any yaml regardless of whether it has a packages: list; WorkspaceYamlKind, workspace_yaml_kind, and the load_present option-returning variant are all removed.
• The node_gyp_bootstrap sentinel is changed from a yaml with a never-matching packages: glob to an empty yaml, achieving the same has_workspace=false outcome more cleanly.
• The bats test that validated workspace-sibling linkage after cd member && aube install (with a settings-only nested yaml) was removed; the PR description's "Fix" section still describes the old walk-past approach, which is the inverse of what the code now does.

Confidence Score: 3/5

The core simplification and node_gyp change are internally consistent, but removal of the bats test that validated workspace-sibling linkage after a member install leaves a meaningful regression risk unvalidated.

The bats test that checked siblings stay linked after cd member && aube install was removed because it would now fail. The code and renamed unit tests confirm the member anchors on itself with no workspace context, contradicting the PR description. Without that integration test, there is no validation that workspace:* deps in such a member are handled correctly under the new approach.

test/shared_workspace_lockfile_warm_path.bats and crates/aube/src/dirs.rs

Important Files Changed

Filename	Overview
crates/aube/src/dirs.rs	Simplifies find_workspace_root_uncached to stop at any yaml (hard boundary), removing WorkspaceYamlKind walk-past logic; unit tests updated to match new semantics, but the PR description contradicts the implementation.
test/shared_workspace_lockfile_warm_path.bats	Removes the bats test that validated workspace-sibling linkage after cd member && aube install with a settings-only nested yaml, leaving that scenario with no integration coverage under the new behavior.
crates/aube-manifest/src/workspace/config.rs	Renames load_present to load_uncached and collapses the Option return to Self::default() for absent files; removes WorkspaceYamlKind, workspace_yaml_kind and their tests. Internal change is clean and consistent with removing the walk-past logic.
crates/aube/src/commands/install/node_gyp_bootstrap.rs	Replaces the fake-members yaml stub (packages list with a never-matching glob) with an empty yaml; both produce has_workspace=false, so the behavior is equivalent. Simpler and correct.
crates/aube-manifest/src/workspace.rs	Removes WorkspaceYamlKind and workspace_yaml_kind from the public re-exports; no remaining callers anywhere in the codebase.

_{Reviews (5): Last reviewed commit: "fix(install): keep nested pnpm-workspace..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aube-manifest/src/workspace/config.rs`:
- Around line 812-814: Remove the typed_cache_insert call from the Ok(None)
branch in the WorkspaceConfig::load logic to avoid negative-caching absent
workspace yamls. The current code inserts WorkspaceConfig::default() into the
typed cache when no workspace yaml is found, which causes subsequent lookups to
return stale cached results even after a workspace yaml file is created. Only
cache WorkspaceConfig when an actual yaml file is successfully loaded, not when
it is absent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b9cb03de-9ec9-41de-8794-82d7c2ca9432

📥 Commits

Reviewing files that changed from the base of the PR and between 43453dc and 61175a5.

📒 Files selected for processing (4)

• crates/aube-manifest/src/workspace.rs
• crates/aube-manifest/src/workspace/config.rs
• crates/aube/src/dirs.rs
• crates/aube/src/state.rs

✅ Files skipped from review due to trivial changes (1)

• crates/aube-manifest/src/workspace.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/aube/src/state.rs

[jdx]

I think the sharedWorkspaceLockfile=false warm-path fix makes sense: if the workspace root has no shared lockfile, tracking member lockfile hashes in install state seems like the right freshness anchor.

But I don't think we should change workspace-root discovery so a nested pnpm-workspace.yaml with no non-empty packages: list is skipped. pnpm-workspace.yaml should be the workspace boundary, especially if that is pnpm's behavior. If packages is omitted, pnpm treats that workspace as containing only the root package; it does not mean "ignore this file and keep walking to an outer workspace".

That distinction matters for aube's drop-in pnpm compatibility story. Treating a nested pnpm-workspace.yaml as member-local config instead of a boundary feels like a surprising and unnecessary divergence. I’d prefer we keep pnpm-workspace.yaml as a hard boundary and limit this PR to the per-member lockfile freshness fix.

This comment was generated by Codex.

[rubnogueira]

Thanks @jdx — reverted the workspace-root discovery change so pnpm-workspace.yaml is a hard boundary again, matching pnpm: a memberless yaml configures a single-package workspace (just the root package) rather than acting as a "keep walking" signal.

The per-member lockfile freshness fix you agreed with already landed on main via #891, so this PR is now limited to backing out the discovery divergence:

• find_workspace_root stops at any pnpm-workspace.yaml again (no walk-past, no settings-only fallback scan)
• dropped WorkspaceYamlKind / workspace_yaml_kind and the load_present split
• restored the empty node-gyp bootstrap stub marker
• removed the now-invalid walk-past regression test; unit tests now assert the hard-boundary semantics

The earlier CodeRabbit (negative-cache in config.rs) and Greptile (dirs.rs) threads are moot now — the code they pointed at was removed by this revert.

This comment was generated by Claude.