Skip to content

fix(workspace): keep generic-loader project roots workspace-relative#77

Open
gbleu wants to merge 1 commit into
frontops-dev:mainfrom
gbleu:fix/generic-loader-relative-roots
Open

fix(workspace): keep generic-loader project roots workspace-relative#77
gbleu wants to merge 1 commit into
frontops-dev:mainfrom
gbleu:fix/generic-loader-relative-roots

Conversation

@gbleu

@gbleu gbleu commented Jul 8, 2026

Copy link
Copy Markdown

Fixes #73.

Problem

In generic workspace mode (npm/yarn/pnpm/bun, no nx.json), the affected set is always empty when the CLI is run from an absolute cwd — i.e. every real-world invocation. Discovery and the semantic trace both work, but nothing is ever attributed.

parse_package_json (src/workspace/workspaces.rs) kept source_root/root absolute whenever cwd is absolute, while changed files parsed from the git diff headers (src/git.rs::parse_diff) are repo-relative. ProjectIndex::resolve_packages (src/utils.rs) decides ownership via file_path.starts_with(root), and a relative path never starts with an absolute root — so every lookup returns empty. The Nx loader (src/workspace/nx.rs) already strips the cwd prefix, which is why Nx-mode attribution works.

Net effect: generic mode returns a silent false-green (empty affected set) if used for CI gating.

Fix

Strip the cwd prefix unconditionally in parse_package_json, matching the Nx loader. The cwd.is_absolute() branch that preserved the absolute path is removed; the strip_prefix(cwd).unwrap_or(project_dir) fallback already handles the relative-cwd case. The one unit test asserting the old absolute-root behavior is updated to expect the workspace-relative root.

Verification

  • cargo test — all unit tests pass.
  • Verified end-to-end against nx affected on a 250-package pnpm monorepo (Nx 22): 251 projects discovered via pnpm-workspace.yaml, correct affected sets across a 14-scenario matrix (comment-only edit → source package only; body change in a shared lib → exact consumer set incl. transitive chains; type-shape change → 3 precise projects vs 16 from Nx's file-graph).

Related but distinct: #70 (Nx-mode discovery missing inferred projects).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Updated workspace path handling so project source paths are now derived more consistently from the current working directory.
    • In setups with an absolute workspace root, source paths will now resolve to cleaner relative paths instead of absolute paths when possible.

parse_package_json kept source_root absolute whenever cwd was absolute
(always the case in CLI usage), while changed files parsed from git diff
headers are repo-relative. ProjectIndex ownership checks use
file_path.starts_with(root), and a relative path never starts with an
absolute root — so every npm/yarn/pnpm/bun workspace run on an absolute
cwd attributed zero projects and returned an empty affected set, even
though discovery and the semantic trace both worked.

Strip the cwd prefix unconditionally, matching the Nx loader's behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fa6fcf30-c2f1-4b51-bee0-632bb7f2b337

📥 Commits

Reviewing files that changed from the base of the PR and between 8b64617 and c177c02.

📒 Files selected for processing (1)
  • src/workspace/workspaces.rs

📝 Walkthrough

Walkthrough

parse_package_json in src/workspace/workspaces.rs now always computes source_root by stripping cwd from project_dir, falling back to project_dir if stripping fails, replacing the prior conditional that returned an absolute path when cwd was absolute. The related unit test was updated to expect a relative source_root.

Changes

source_root Path Fix

Layer / File(s) Summary
Unconditional cwd stripping for source_root
src/workspace/workspaces.rs
parse_package_json now strips cwd from project_dir unconditionally (falling back to project_dir on failure) instead of returning an absolute path when cwd is absolute; the unit test for absolute cwd now asserts a relative source_root ("packages/utils").

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • frontops-dev/domino#65: Changes Project.source_root path computation (relative vs absolute), directly interacting with logic that attributes affected projects based on whether files fall outside sourceRoot.

Suggested reviewers: EladBezalel

🚥 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 clearly matches the main change: making generic workspace project roots workspace-relative.
Linked Issues check ✅ Passed The code change addresses the linked issue by stripping the cwd prefix unconditionally, aligning project roots with repo-relative changed paths.
Out of Scope Changes check ✅ Passed The PR only changes the workspace-root path computation and its test, with no unrelated or extra-scope modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

📦 Preview Release Available

A preview release has been published for commit c177c02.

Installation

npm install https://github.com/frontops-dev/domino/releases/download/pr-77-c177c02/front-ops-domino-1.4.0.tgz

Running the preview

npx https://github.com/frontops-dev/domino/releases/download/pr-77-c177c02/front-ops-domino-1.4.0.tgz affected

Details

@gbleu

gbleu commented Jul 8, 2026

Copy link
Copy Markdown
Author

✅ PASS (["@fix/a","@fix/b"] vs stock [])

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.

Generic workspace mode (npm/yarn/pnpm/bun) always returns an empty affected set when run from an absolute cwd

1 participant