Fix npm workspaces #451
Fix npm workspaces #451
Conversation
WalkthroughAdds workspace-aware JavaScript dependency installation to the bundler. Detects npm/pnpm/yarn/bun workspaces, determines workspace root, matches workspace patterns, selects install directory accordingly, and updates install command generation and usage. Extends function signatures to include isWorkspace/production flags. Adds tests for command selection and glob/pattern matching. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/CI
participant Bundler as Bundler
participant FS as Filesystem
participant PM as Package Manager
Dev->>Bundler: bundleJavascript(agentDir)
Bundler->>FS: detectWorkspaceRoot(start=agentDir)
FS-->>Bundler: WorkspaceConfig or nil
alt Workspace detected
Bundler->>Bundler: isAgentInWorkspace(agentDir, WorkspaceConfig)
alt Agent in workspace
Bundler->>Bundler: installDir = workspace.root
note right of Bundler: New flow: workspace-aware install
else Agent not in workspace
Bundler->>Bundler: installDir = agentDir
end
else No workspace
Bundler->>Bundler: installDir = agentDir
end
Bundler->>Bundler: getJSInstallCommand(ctx, installDir, runtime, isWorkspace)
Bundler->>PM: run install command in installDir
PM-->>Bundler: install result
Bundler->>Bundler: type check / resolve SDK using installDir
Bundler-->>Dev: bundle output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
🧹 Nitpick comments (3)
internal/bundler/bundler.go (1)
241-265: Consider adding validation for empty or malformed patterns.While the function handles pattern matching well, it could benefit from validating that patterns are not empty strings or contain invalid characters that might cause issues with glob matching.
Add validation at the beginning of the function:
func isAgentInWorkspace(logger logger.Logger, agentDir string, workspace *WorkspaceConfig) bool { + if workspace == nil || len(workspace.Patterns) == 0 { + logger.Debug("no workspace patterns to check") + return false + } // Get relative path from workspace root to agent directory relPath, err := filepath.Rel(workspace.Root, agentDir)internal/bundler/bundler_test.go (2)
163-239: Consider adding tests for workspace detection and install directory resolution.While the pattern matching is well tested, consider adding integration tests for the
detectWorkspaceRoot,isAgentInWorkspace, andfindWorkspaceInstallDirfunctions to ensure the complete workspace flow works correctly.Would you like me to generate comprehensive test cases for the workspace detection and install directory resolution functions? These tests would help ensure the entire workspace feature works correctly end-to-end.
41-53: Consider testing workspace-aware installation commands.The tests currently only verify non-workspace scenarios. Consider adding tests that verify the correct installation commands are generated when
isWorkspace=true.Add test cases for workspace scenarios:
func TestWorkspaceInstallCommands(t *testing.T) { tests := []struct { name string runtime string isWorkspace bool production bool expectedArgs []string }{ { name: "npm workspace development", runtime: "nodejs", isWorkspace: true, production: false, expectedArgs: []string{"install", "--no-audit", "--no-fund", "--ignore-scripts"}, }, { name: "pnpm workspace development", runtime: "pnpm", isWorkspace: true, production: false, expectedArgs: []string{"install", "--ignore-scripts", "--silent"}, }, // Add more test cases... } // Implementation... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/bundler/bundler.go(11 hunks)internal/bundler/bundler_test.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
internal/bundler/bundler_test.go (1)
internal/bundler/bundler.go (1)
BundleContext(40-50)
internal/bundler/bundler.go (4)
internal/util/io.go (1)
Exists(14-19)internal/project/project.go (1)
Bundler(493-502)internal/util/process_nix.go (1)
ProcessSetup(14-17)internal/util/process_windows.go (1)
ProcessSetup(22-28)
🔇 Additional comments (15)
internal/bundler/bundler.go (11)
148-152: LGTM! Well-structured workspace configuration type.The
WorkspaceConfigstruct is clearly documented and appropriately captures the essential workspace information with proper field names.
154-207: Good workspace detection implementation with proper traversal logic.The function correctly walks up the directory tree looking for workspace configurations, properly handles both npm/yarn and pnpm workspace formats, and safely stops at the root directory. The debug logging is helpful for troubleshooting.
209-238: Robust handling of npm workspace formats.The function correctly handles both array and object formats for npm workspaces, with proper type checking and error handling.
267-286: Well-implemented workspace pattern matching with cross-platform support.Good use of
doublestarlibrary for robust glob matching and proper path normalization for cross-platform compatibility. The negation pattern handling is correctly implemented.
288-308: Solid implementation of workspace install directory resolution.The function properly determines whether to use the workspace root or agent directory for installation, with appropriate error handling and debug logging.
325-354: Workspace-aware installation commands look correct.The function properly adjusts installation flags based on workspace context, correctly avoiding production-only flags in workspace development mode to ensure TypeScript types are available.
423-427: Good integration of workspace detection into the bundle flow.The workspace detection is properly integrated, determining the correct install directory and passing the workspace flag through the installation pipeline.
123-123: Type checking now correctly uses workspace-aware install directory.The change ensures TypeScript compiler is found in the correct location within workspaces.
457-457: Source map support installation correctly targets workspace root.The change ensures bun's source-map-js is installed in the correct directory for workspace scenarios.
6-6: Appropriate dependency additions for workspace support.The new dependencies (
encoding/jsonfor package.json parsing,doublestarfor glob pattern matching, andgopkg.in/yaml.v3for pnpm-workspace.yaml) are all necessary and well-chosen for implementing workspace support.Also applies to: 24-24, 26-26
491-491: Approve change — using installDir for SDK resolution is correct.
resolveAgentuity walks up parent directories from the provided startDir to find node_modules/@agentuity/sdk/package.json and returns that path, so calling resolveAgentuity(ctx.Logger, installDir) correctly supports workspace installs (impl: internal/bundler/bundler.go:712; call: internal/bundler/bundler.go:491).internal/bundler/bundler_test.go (4)
41-53: Test helper properly updated for workspace parameter.The
testPackageManagerCommandhelper function correctly accepts and passes through the newisWorkspaceparameter.
133-133: Existing tests properly updated with workspace flag.All existing test calls have been correctly updated to include the
isWorkspaceparameter set tofalse, maintaining backward compatibility.Also applies to: 147-147, 157-157
163-202: Comprehensive test coverage for workspace pattern matching.Excellent test coverage including exact matches, glob patterns, recursive patterns, negation, and cross-platform path handling.
204-239: Thorough npm-style glob pattern tests.Good additional coverage for complex npm workspace patterns including deep nesting, universal globs, and edge cases.
- Added: [AGENT-684] Check if zsh is installed before adding autocomplete in the CLI (#450) - Added: [AGENT-628] Unit tests (#441) - Added: feat: automatically add AGENTUITY_SDK_KEY and AGENTUITY_PROJECT_KEY to .env file when running dev command (#442) - Changed: Dont sort releases by commit msg (#447) - Changed: [AGENT-628] prevent local development env files from syncing to production (#440) - Fixed: Fix npm workspaces (#451) - Fixed: Fix 'Press any key to continue' to accept any key, not just Enter (#445) Co-Authored-By: unknown <>
- Added: [AGENT-684] Check if zsh is installed before adding autocomplete in the CLI (#450) - Added: [AGENT-628] Unit tests (#441) - Added: feat: automatically add AGENTUITY_SDK_KEY and AGENTUITY_PROJECT_KEY to .env file when running dev command (#442) - Changed: Dont sort releases by commit msg (#447) - Changed: [AGENT-628] prevent local development env files from syncing to production (#440) - Fixed: Fix npm workspaces (#451) - Fixed: Fix 'Press any key to continue' to accept any key, not just Enter (#445) Co-Authored-By: unknown <> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Summary by CodeRabbit