feat(please-plugins): add monorepo workspace dependency scanning#133
feat(please-plugins): add monorepo workspace dependency scanning#133
Conversation
Add resolveWorkspacePackages() and collectAllDependencies() to scan workspace package.json files in monorepos. Supports npm/yarn/Bun workspaces field and pnpm-workspace.yaml. Integrates into both scanForSetup() and detectForEvent() with workspace-aware source tracking (e.g., "apps/web: @nuxt/ui"). Closes #132
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request implements monorepo workspace dependency scanning by aggregating dependencies from all workspace packages defined in package.json or pnpm-workspace.yaml. The changes update the detection logic to attribute dependencies to their specific workspace paths. Feedback highlights several improvement opportunities: the custom glob resolution should be replaced with a standard library like Bun.Glob for better robustness, the manual YAML parsing for pnpm-workspace.yaml should be made more resilient, resolved workspace paths should be deduplicated, and test assertions should be more precise regarding expected directory counts and negation patterns.
Move resolveWorkspacePackages, collectAllDependencies, and resolveGlobPatterns to workspace-resolver.ts to keep check-dependencies.ts under 500 LOC guideline.
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- There is a concrete regression risk in
plugins/please-plugins/hooks/workspace-resolver.ts: glob resolution stops processing after the first*, which can misresolve non-trivial workspace patterns to the wrong directories. - Given the issue’s medium severity (6/10) and high confidence (8/10), this introduces user-facing behavior risk in workspace discovery, so this is not fully low-risk to merge yet.
- Pay close attention to
plugins/please-plugins/hooks/workspace-resolver.ts- wildcard parsing may produce incorrect directory matches for complex patterns.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/please-plugins/hooks/workspace-resolver.ts">
<violation number="1" location="plugins/please-plugins/hooks/workspace-resolver.ts:49">
P2: Glob resolution ignores everything after the first `*`, so non-trivial workspace patterns can match the wrong directories.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as CLI / Hook Trigger
participant Scanner as check-dependencies.ts
participant Resolver as NEW: workspace-resolver.ts
participant FS as File System
participant Maps as Plugin Mappings
UI->>Scanner: scanForSetup(cwd) or detectForEvent()
Scanner->>Resolver: NEW: collectAllDependencies(cwd)
Resolver->>FS: loadPackageJson(root)
FS-->>Resolver: root dependencies
Resolver->>Resolver: NEW: resolveWorkspacePackages()
alt npm / yarn / Bun
Resolver->>Resolver: Parse "workspaces" glob patterns
else pnpm
Resolver->>FS: NEW: Read pnpm-workspace.yaml
FS-->>Resolver: yaml patterns
end
Resolver->>FS: NEW: resolveGlobPatterns (readdirSync)
loop For each resolved workspace directory
Resolver->>FS: loadPackageJson(workspaceDir)
alt Valid package.json
FS-->>Resolver: workspace dependencies
Resolver->>Resolver: NEW: Aggregate unique dependencies
Resolver->>Resolver: NEW: Map dep to workspace source path
else Missing or Malformed
FS-->>Resolver: null / skip
end
end
Resolver-->>Scanner: Return AggregatedDeps (deps + sources map)
Scanner->>Maps: Load static mappings
Scanner->>Scanner: CHANGED: detectPackages() using aggregated deps
Scanner->>Scanner: NEW: resolvePackageSource()
Note over Scanner: If dep found in workspace map,<br/>prefix source with path (e.g. "apps/web: @nuxt/ui")
Scanner-->>UI: Return SetupOutput (Detected plugins with attribution)
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Replace custom glob resolver with Bun.Glob for accurate multi-segment pattern matching (fixes patterns like "apps/*/src" truncation bug) - Deduplicate resolved workspace paths to avoid duplicate entries when npm workspaces and pnpm-workspace.yaml share the same patterns - Improve pnpm-workspace.yaml parsing to handle inline comments and more robust top-level key detection - Tighten test assertions for deduplication (exact length 1) and negation patterns (exact length 2 with both expected dirs)
Summary
workspacesfield andpnpm-workspace.yamlapps/web: @nuxt/ui)scanForSetup()and SessionStart hookTest plan
Closes #132
Verification Checklist
resolveWorkspacePackagesresolves npm/yarn/Bunworkspacesglob patterns to directory pathsresolveWorkspacePackagesresolvespnpm-workspace.yamlpatterns to directory pathsresolveWorkspacePackagesreturns empty array when no workspaces configuredcollectAllDependenciesmerges deps from root + workspace packagescollectAllDependenciesskips missing/malformed workspace package.jsonscanForSetupdetects plugins from workspace dependencies