Skip to content

fix(detect): decouple node script discovery from PM detection + mirror resolver PM chain#32

Merged
kjanat merged 3 commits into
masterfrom
fix/node-detection-decouple-tc
May 19, 2026
Merged

fix(detect): decouple node script discovery from PM detection + mirror resolver PM chain#32
kjanat merged 3 commits into
masterfrom
fix/node-detection-decouple-tc

Conversation

@kjanat
Copy link
Copy Markdown
Owner

@kjanat kjanat commented May 17, 2026

Problem

A pnpm-workspace member directory (package.json with scripts, no local lockfile, no packageManager/devEngines) reported "No project detected", and runner run build fell through to a bogus bun build. Detection gated script extraction on a detected Node PM; the resolver was already smart enough — that asymmetry was the bug.

Fix

  1. Decouple script discovery from PM detection — a package.json is the Node signal. Gating is now manifest-presence based, applied to the parallel extract_tasks. A manifest-less subdirectory lists ancestor scripts only when it provably sits inside a JS monorepo (workspace-root-aware, VCS-bounded), so an unrelated outer project's package.json is never silently adopted.
  2. Mirror the resolver's PM chain in detectionpackageManagerdevEngines.packageManager → enclosing-workspace lockfile/manifest, so runner info / runner install from a member target the right tool. Corepack semantics preserved: a present-but-unparseable legacy packageManager still emits a warning and is not silently superseded by devEngines (detect_pm_from_manifest short-circuits to None on an unparseable spec — verified, regression test node::detect_pm_from_manifest_blocks_dev_engines_when_package_manager_unparseable).

Shared VCS-bounded ancestor walk extracted into a reusable generic files::find_in_ancestors.

Commits

  • fix(detect): decouple node script discovery from PM detection
  • fix(detect): mirror resolver PM chain (devEngines + workspace-upward)
  • changelog: record go.mod task name + node-detection fixes

Base: master. Rebased onto current master (post-#27 merge) — feat/task-chaining, which this was originally stacked on, is now merged. 3 linear commits, no merge commits.

Verification

Run against the actual master-merged tree (2 fix commits rebased onto current master):

  • cargo test: 554 pass, 0 fail, 1 ignored (+5 new detection tests; the 1 ignored is the long-standing env-gated test, unrelated).
  • cargo clippy --all-targets --all-features -- -D clippy::all: clean.
  • cargo fmt --check: clean. dprint check: clean.
  • Rebase onto current master was conflict-free; zero functional overlap with intervening commits.

Notes

  • CHANGELOG.md [0.11.0] updated: new ### Fixed section for this PR's behavior, plus a ### Changed entry for the go.mod-module-path task-name change that landed via Task chaining: run -s/-p chain mode + install <tasks> #27. Not bumping version as 0.11.0 is not published yet.
  • Review of the diff (detect.rs, node.rs, files.rs) surfaced no code defects; the only flagged item — the Corepack/unparseable-packageManager interaction — was traced to source and confirmed correct.

@kjanat
Copy link
Copy Markdown
Owner Author

kjanat commented May 17, 2026

Superseded — rebuilding by hand-port off feat/task-chaining instead of copy/cherry-pick.

@kjanat kjanat closed this May 17, 2026
@kjanat kjanat deleted the fix/node-detection-decouple-tc branch May 17, 2026 15:30
@kjanat kjanat restored the fix/node-detection-decouple-tc branch May 17, 2026 15:32
@kjanat kjanat deleted the fix/node-detection-decouple-tc branch May 17, 2026 15:32
@kjanat kjanat restored the fix/node-detection-decouple-tc branch May 17, 2026 15:32
@kjanat kjanat reopened this May 17, 2026
@kjanat kjanat marked this pull request as draft May 17, 2026 15:34
@kjanat kjanat marked this pull request as ready for review May 19, 2026 14:44
Base automatically changed from feat/task-chaining to master May 19, 2026 15:07
@kjanat kjanat changed the title fix(detect): decouple node script discovery from PM detection + mirror resolver PM chain (stacked on feat/task-chaining) fix(detect): decouple node script discovery from PM detection + mirror resolver PM chain May 19, 2026
@kjanat kjanat self-assigned this May 19, 2026
@kjanat

This comment was marked as outdated.

kjanat added 2 commits May 19, 2026 17:13
A package.json with scripts but no lockfile and no packageManager/
devEngines field (a typical pnpm-workspace member dir) detected zero
node package managers, so script extraction was skipped entirely and
the dir reported as "No project detected".

Script discovery is now gated on manifest presence, not on a detected
node PM — which PM dispatches the scripts is the resolver's runtime
job. A manifest-less subdirectory additionally lists the nearest
ancestor manifest's scripts when it provably sits inside a JS monorepo
(pnpm-workspace.yaml / lerna.json / package.json workspaces), so a
workspace member is never met with "No project detected".

- detect.rs: gate the parallel extract_tasks on has_local_manifest
  || workspace_member || with_deno instead of a detected node/deno PM
- node.rs: add within_workspace_upwards (workspace-root-aware guard)
- files.rs: extract reusable VCS-bounded find_in_ancestors;
  find_first_upwards now delegates to it

Tests: +3 (no-PM-signal headline case, workspace-member subdir,
non-workspace negative guard).
Detection only consulted the legacy packageManager field, so a
devEngines-only manifest, or a lockfile-less workspace member, surfaced
no package manager — info/install then fell back to the wrong tool.

Detection now mirrors the resolver's manifest chain: packageManager →
devEngines.packageManager → enclosing-workspace lockfile/manifest. The
upward walk is workspace-root-aware and VCS-bounded, reusing the same
guard as upward script discovery so an unrelated outer-project lockfile
is never adopted.

- detect.rs: devEngines fallback via detect_pm_from_manifest; new
  detect_node_pm_upwards for manifest-less / PM-less workspace members
- tests: +2 (devEngines-only PM, workspace-member upward PM)

Unparseable legacy packageManager still returns no PM (Corepack: no
substitution) and still emits the UnparseablePackageManager warning.
@kjanat kjanat force-pushed the fix/node-detection-decouple-tc branch from e2c39de to 0b41b2f Compare May 19, 2026 15:14
@coderabbitai coderabbitai Bot added the enhancement New feature or request label May 19, 2026
`[0.11.0]` was missing two user-facing changes that already merged
or land in this branch:

- Changed: root Go task name now from the `go.mod` `module` path
  (commit landed via #27), not the directory name.
- Fixed: a new `### Fixed` section for the node-script-discovery
  decoupling and resolver-PM-chain mirroring in this PR — the
  "No project detected" pnpm-workspace-member regression.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR enhances Node.js package manager detection and task extraction with workspace-aware ancestry walking. It introduces reusable upward-search infrastructure, adds workspace membership detection, enables PM fallback chains within monorepos, decouples script extraction from PM detection, and validates the new behavior with comprehensive tests.

Changes

Node.js workspace-aware package manager and task detection

Layer / File(s) Summary
Ancestor-walking infrastructure with VCS boundaries
src/tool/files.rs
find_in_ancestors centralizes upward-directory traversal with VCS-root boundary rules, accepting a predicate and returning the first Some result; find_first_upwards refactored to use this helper.
Workspace membership detection
src/tool/node.rs
within_workspace_upwards scans ancestor directories for monorepo markers (pnpm-workspace.yaml, lerna.json, or package.json with workspaces key) using the new ancestor-walking infrastructure.
Package manager upward resolution within workspaces
src/detect.rs
New detect_node_pm_upwards helper walks ancestors within JS workspaces to find governing PM via lockfile or manifest settings; integrated into detect_package_managers as final fallback in a staged chain after field-derived and manifest-derived PM options.
Task extraction decoupled from PM detection
src/detect.rs
extract_tasks now independently decides on package.json script extraction based on local manifest presence, workspace membership, and Deno presence; spawn logic selects local extraction only when a local manifest exists and Deno is absent, otherwise uses upward extraction.
Regression and unit tests for workspace-aware detection
src/detect.rs
Tests validate script extraction without lockfiles, workspace-member inheritance of ancestor scripts, prevention of adoption outside workspace boundaries, PM resolution from devEngines, and upward PM adoption for PM-less members.

Possibly related PRs

  • kjanat/runner#13: Related changes to package.json script detection and Turbo passthrough classification.
  • kjanat/runner#22: Overlapping modifications in src/detect.rs for package-manager and script detection logic.
  • kjanat/runner#15: Related task extraction flow changes affecting Node/package.json handling.

Arrr, the code be tidied and true, no barnacles left to pick—
Ancestors walked stern to bow, workspace flags in tow,
Scripts now found whether ye be docked or adrift,
Warnings spit where legacy fields be stiff,
Patch hoisted, tests green—aye, that warms this pirate's heart. 🏴‍☠️

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Semver Version Bump Validation ⚠️ Warning Source code modified (256 insertions across 3 .rs files) but Cargo.toml version unchanged at 0.11.0. PR adds new pub(crate) APIs and bug fixes—requires PATCH bump to 0.11.1. Update Cargo.toml version from "0.11.0" to "0.11.1" to reflect bug fixes and new backward-compatible functionality additions.
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed Title uses 'fix:' prefix, is descriptive of the changes (decoupling script discovery from PM detection and mirroring PM chain), but exceeds the ideal 50-character limit at 88 characters.
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.
Changelog Update ✅ Passed CHANGELOG.md updated under [0.11.0] with comprehensive Fixed entries matching the three modified source files. Version header format correct, TBD date allowed per check rules.
Agents.Md Documentation Updated ✅ Passed No AGENTS.md file exists in the repository. The custom check requires AGENTS.md to exist as a prerequisite before mandating updates. Since this file is absent, the check does not apply.
Description check ✅ Passed The PR description is directly related to the changeset, detailing the problem (Node script discovery gated on PM detection in workspace members), the fixes (decoupling script discovery from PM detection, mirroring PM resolution chain, extracting shared ancestor-walk utility), and verification results.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/node-detection-decouple-tc

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

@kjanat kjanat merged commit d5af4a9 into master May 19, 2026
12 checks passed
@kjanat kjanat deleted the fix/node-detection-decouple-tc branch May 19, 2026 15:22
kjanat added a commit that referenced this pull request May 19, 2026
Stamp the `[0.11.0]` section with its release date (2026-05-19)
now that #27/#32 have landed on `master`. Add `MD034` to the
`markdownlint`/`rumdl` disable-file directives so the inline
issue/PR URLs the changelog uses by convention stop tripping the
no-bare-URL rule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant