fix: resolve stale targets from workspace package context#102
fix: resolve stale targets from workspace package context#102LadyBluenotes wants to merge 2 commits intomainfrom
Conversation
|
View your CI Pipeline Execution ↗ for commit b49d1ce
☁️ Nx Cloud last updated this comment at |
commit: |
📝 WalkthroughWalkthroughThis PR enhances monorepo package path resolution in the Intent CLI by targeting the intended workspace package directly rather than scanning the entire workspace. It updates the core path resolution logic and introduces comprehensive test coverage for monorepo scenarios. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant ResolveStale as resolveStaleTargets
participant Context as resolveProjectContext
participant Check as checkStaleness
participant Workspace as Workspace Scan
rect rgba(100, 150, 200, 0.5)
note over CLI,Workspace: New Flow
CLI->>ResolveStale: invoke with targetDir
ResolveStale->>ResolveStale: resolve path with path.resolve()
ResolveStale->>Context: call with project context
Context-->>ResolveStale: return context (packageRoot, targetSkillsDir, workspaceRoot)
alt Is targeted package?
ResolveStale->>Check: call for context.packageRoot
Check-->>ResolveStale: return staleness report
ResolveStale-->>CLI: return single-item report (early exit)
else Not a targeted package
ResolveStale->>Workspace: scan workspace patterns and intents
Workspace-->>ResolveStale: discover packages
ResolveStale->>Check: check staleness across packages
Check-->>ResolveStale: return staleness reports
ResolveStale-->>CLI: return full workspace report
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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}
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
| const targetsResolvedPackage = | ||
| context.packageRoot !== null && | ||
| (context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot) | ||
|
|
||
| if (targetsResolvedPackage && context.packageRoot) { | ||
| return { | ||
| reports: [ | ||
| await checkStaleness( | ||
| context.packageRoot, | ||
| readPackageName(context.packageRoot), | ||
| ), | ||
| ], | ||
| } |
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.
1 participant
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