fix: resolve stale targets from workspace package context#102
Merged
LadyBluenotes merged 4 commits intomainfrom Mar 31, 2026
Merged
fix: resolve stale targets from workspace package context#102LadyBluenotes merged 4 commits intomainfrom
LadyBluenotes merged 4 commits intomainfrom
Conversation
|
View your CI Pipeline Execution ↗ for commit 2cb672d
☁️ Nx Cloud last updated this comment at |
commit: |
Contributor
📝 WalkthroughWalkthroughThis PR changes Intent CLI path resolution to detect and target a specific workspace package (via project context and resolved paths) and adds monorepo-focused tests and a changeset documenting the fix. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant Resolve as resolveStaleTargets
participant Context as resolveProjectContext
participant FS as Filesystem/Path
participant Check as checkStaleness
rect rgba(100, 150, 200, 0.5)
CLI->>Resolve: invoke resolveStaleTargets(targetDir)
Resolve->>FS: resolve path (path.resolve)
Resolve->>Context: resolveProjectContext({cwd, targetPath})
Context-->>Resolve: return {packageRoot, targetSkillsDir, workspaceRoot}
alt context.packageRoot present AND (targetSkillsDir != null OR resolvedRoot != workspaceRoot)
Resolve->>Check: readPackageName(packageRoot) & checkStaleness(packageRoot)
Check-->>Resolve: single staleness report
Resolve-->>CLI: return single-item report (early exit)
else
Resolve->>FS: look for `skills` under resolvedRoot and workspace patterns
FS-->>Resolve: discovered packages/intents
Resolve->>Check: checkStaleness for discovered packages
Check-->>Resolve: aggregated staleness reports
Resolve-->>CLI: return full workspace report
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/intent/src/cli-support.ts`:
- Around line 59-71: The fast-path predicate incorrectly fires when
workspaceRoot is null or when packageRoot equals the workspace root; restrict it
so the branch only runs for explicit skills targets or true subpackages: change
the targetsResolvedPackage condition to only be true when
context.targetSkillsDir !== null OR (context.workspaceRoot !== null AND
resolvedRoot !== context.workspaceRoot), and keep the existing guard that
context.packageRoot is set before returning the single checkStaleness(report)
for that packageRoot/readPackageName; this ensures workspace-root-owned paths
and null workspaceRoot do not short-circuit the installed-package scan.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 936e7e05-4899-4835-82dc-69f5016976fe
📒 Files selected for processing (3)
.changeset/vast-bags-switch.mdpackages/intent/src/cli-support.tspackages/intent/tests/cli.test.ts
packages/intent/src/cli-support.ts
Outdated
Comment on lines
+59
to
+71
| const targetsResolvedPackage = | ||
| context.packageRoot !== null && | ||
| (context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot) | ||
|
|
||
| if (targetsResolvedPackage && context.packageRoot) { | ||
| return { | ||
| reports: [ | ||
| await checkStaleness( | ||
| context.packageRoot, | ||
| readPackageName(context.packageRoot), | ||
| ), | ||
| ], | ||
| } |
Contributor
There was a problem hiding this comment.
Narrow this fast path to real package targets.
Line 61 is always true when context.workspaceRoot is null, so intent stale from a normal app root with a package.json now skips the installed-package scan at Lines 98-105 and reports the app package instead. The same branch also misclassifies workspace-root-owned paths like packages/ as a single-package target. Please gate this branch to explicit skills targets or subpackages whose packageRoot differs from the workspace root.
Suggested predicate tightening
- const targetsResolvedPackage =
- context.packageRoot !== null &&
- (context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot)
+ const targetsResolvedPackage =
+ context.packageRoot !== null &&
+ (
+ context.targetSkillsDir !== null ||
+ (
+ context.workspaceRoot !== null &&
+ context.packageRoot !== context.workspaceRoot &&
+ resolvedRoot !== context.workspaceRoot
+ )
+ )Based on learnings, monorepo detection here intentionally needs to answer whether a package is inside a monorepo, not whether the workspace root itself should be treated as a targeted sub-package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/intent/src/cli-support.ts` around lines 59 - 71, The fast-path
predicate incorrectly fires when workspaceRoot is null or when packageRoot
equals the workspace root; restrict it so the branch only runs for explicit
skills targets or true subpackages: change the targetsResolvedPackage condition
to only be true when context.targetSkillsDir !== null OR (context.workspaceRoot
!== null AND resolvedRoot !== context.workspaceRoot), and keep the existing
guard that context.packageRoot is set before returning the single
checkStaleness(report) for that packageRoot/readPackageName; this ensures
workspace-root-owned paths and null workspaceRoot do not short-circuit the
installed-package scan.
… cases - Inline targetsResolvedPackage boolean to remove redundant null check - Add fetchSpy call count assertions to verify only targeted package is processed - Add test for targetDir without /skills suffix - Add test for absolute targetDir path (validates join→resolve fix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
KyleAMathews
approved these changes
Mar 31, 2026
Collaborator
KyleAMathews
left a comment
KyleAMathews
left a comment
There was a problem hiding this comment.
Reviewed the fix and added a follow-up commit with improvements:
- Simplified condition: Inlined the
targetsResolvedPackageboolean — the redundant re-check ofcontext.packageRootwas only needed for TypeScript narrowing, which the inlined truthy check handles directly. - Stronger test assertions: Added
fetchSpy.toHaveBeenCalledTimes(1)to both staleness tests, proving the untargeted package is never even processed (not just excluded from reports). - New test coverage: Added tests for targeting a package path without the
/skillssuffix, and for absolutetargetDirpaths (which validates thejoin→resolvefix — red/green confirmed the oldjoincode breaks with absolute paths).
All 29 tests pass.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/intent/tests/cli.test.ts (1)
490-691: Consider adding a test for non-monorepo single-package projects.The current
staletests only cover monorepo scenarios. A test for a simple project (singlepackage.json, no workspaces, withnode_modulescontaining intent packages) would help catch the predicate issue flagged incli-support.tswhere the fast path incorrectly fires whenworkspaceRootisnull.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/tests/cli.test.ts` around lines 490 - 691, Add a new test in packages/intent/tests/cli.test.ts that sets up a non-monorepo single-package project (a root with package.json that has no "workspaces"), writes a skills file (using writeSkillMd) and simulates npm resolution (optionally create node_modules or spy fetch), then chdir into that root and call main(['stale','--json']) to ensure the CLI stale fast-path in cli-support.ts does not incorrectly treat workspaceRoot === null as a monorepo: assert exit code is 0 and the returned reports include the package (e.g., library name from package.json); reference main and the stale command so the test targets the predicate in cli-support.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/intent/tests/cli.test.ts`:
- Around line 490-691: Add a new test in packages/intent/tests/cli.test.ts that
sets up a non-monorepo single-package project (a root with package.json that has
no "workspaces"), writes a skills file (using writeSkillMd) and simulates npm
resolution (optionally create node_modules or spy fetch), then chdir into that
root and call main(['stale','--json']) to ensure the CLI stale fast-path in
cli-support.ts does not incorrectly treat workspaceRoot === null as a monorepo:
assert exit code is 0 and the returned reports include the package (e.g.,
library name from package.json); reference main and the stale command so the
test targets the predicate in cli-support.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 785cb391-3454-48fc-a36f-00287f4bf38b
📒 Files selected for processing (2)
packages/intent/src/cli-support.tspackages/intent/tests/cli.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
staleso monorepo package paths resolve to the intended workspace packageNotes
intent stale packages/<pkg>/skillsnow checks only that packageintent stalefrom insidepackages/<pkg>now checks only that packagesetup-github-actionsstill writes workflows to the workspace root from a workspace packageSummary by CodeRabbit
Bug Fixes
Tests