Skip to content

fix(author): support direct workspace paths#56

Merged
harlan-zw merged 1 commit intomainfrom
fix/direct-workspace-paths
Mar 24, 2026
Merged

fix(author): support direct workspace paths#56
harlan-zw merged 1 commit intomainfrom
fix/direct-workspace-paths

Conversation

@oritwoen
Copy link
Copy Markdown
Collaborator

@oritwoen oritwoen commented Mar 24, 2026

Description

Direct workspace entries like packages/foo were being skipped by detectMonorepoPackages(). The author flow only scanned child directories under each workspace pattern, so exact package paths never resolved and skilld author missed those packages. This checks scanDir/package.json first, then falls back to the existing child-directory scan for glob patterns like packages/*.

Linked Issues

N/A

Additional context

Added a unit test for workspaces: ['packages/foo']. Verified with pnpm vitest run test/unit/author.test.ts, pnpm typecheck, and pnpm lint.

Summary by CodeRabbit

Release Notes

  • Improvements

    • Enhanced monorepo workspace package detection to more efficiently identify packages within workspace patterns.
  • Tests

    • Added unit test coverage for monorepo workspace package detection scenarios.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

Modified monorepo package discovery to optimize workspace pattern handling. When a workspace pattern directory itself is a valid package, it's directly registered; otherwise, child directories are enumerated. Package validation checks for non-private status and presence of a name field, with repository URLs normalized from repository.url or string values.

Changes

Cohort / File(s) Summary
Monorepo Package Detection
src/commands/author.ts
Enhanced detectMonorepoPackages to check if workspace directories are packages directly before enumerating children; includes repository URL normalization by stripping git+ prefix and .git suffix.
Detection Logic Tests
test/unit/author.test.ts
Added unit test covering the new direct-package-detection path, mocking filesystem operations to validate correct package identification at workspace root level.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • harlan-zw

Poem

🐰 Workspace paths now hop more swift,
Direct packages skip the sift!
No child dirs when root's a treat,
Monorepo magic, quick and neat!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(author): support direct workspace paths' directly and clearly summarizes the main change: adding support for direct workspace paths in the monorepo package detection logic.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/direct-workspace-paths

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

@oritwoen oritwoen self-assigned this Mar 24, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/commands/author.ts`:
- Around line 105-123: The direct package.json detection inside
detectMonorepoPackages incorrectly runs for glob workspace entries and causes
parent dirs like "packages" to short-circuit child package discovery; before
calling readPackageJsonSafe(scanDir + '/package.json') add a guard that skips
the directResult check when the original workspace pattern contains glob
characters (e.g., '*' '?' or '[') — use the workspace pattern variable used to
derive scanDir (or keep the pattern in scope) and only perform the directPkg
logic (the directResult/directPkg branch that pushes to packages and continues)
when the pattern is a non-glob literal; this preserves the existing behavior for
literal workspace entries while preventing glob parents from swallowing child
packages.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fbe135a1-7fc9-4cb2-b283-612eaef799a6

📥 Commits

Reviewing files that changed from the base of the PR and between 8b54a9a and 00a8b9b.

📒 Files selected for processing (2)
  • src/commands/author.ts
  • test/unit/author.test.ts

Comment on lines +105 to +123
const directResult = readPackageJsonSafe(join(scanDir, 'package.json'))
if (directResult) {
const directPkg = directResult.parsed as Record<string, any>
if (!directPkg.private && directPkg.name) {
const repoUrl = typeof directPkg.repository === 'string'
? directPkg.repository
: directPkg.repository?.url?.replace(/^git\+/, '').replace(/\.git$/, '')

packages.push({
name: directPkg.name,
version: directPkg.version || '0.0.0',
description: directPkg.description,
repoUrl,
dir: scanDir,
})
continue
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard direct package detection to non-glob workspace entries.

Line 105 runs direct scanDir/package.json detection for every workspace pattern. For glob patterns (e.g., packages/*), if packages/package.json exists, Line 120 continues and child packages are skipped.

This can cause detectMonorepoPackages() to miss valid workspace packages.

Proposed fix
 for (const pattern of patterns) {
   // Expand simple glob: "packages/*" → scan packages/*/package.json
   const base = pattern.replace(/\/?\*+$/, '')
   const scanDir = resolve(cwd, base)
+  const isGlobPattern = pattern.includes('*')
   if (!existsSync(scanDir))
     continue

-  const directResult = readPackageJsonSafe(join(scanDir, 'package.json'))
-  if (directResult) {
-    const directPkg = directResult.parsed as Record<string, any>
-    if (!directPkg.private && directPkg.name) {
-      const repoUrl = typeof directPkg.repository === 'string'
-        ? directPkg.repository
-        : directPkg.repository?.url?.replace(/^git\+/, '').replace(/\.git$/, '')
-
-      packages.push({
-        name: directPkg.name,
-        version: directPkg.version || '0.0.0',
-        description: directPkg.description,
-        repoUrl,
-        dir: scanDir,
-      })
-      continue
-    }
-  }
+  if (!isGlobPattern) {
+    const directResult = readPackageJsonSafe(join(scanDir, 'package.json'))
+    if (directResult) {
+      const directPkg = directResult.parsed as Record<string, any>
+      if (!directPkg.private && directPkg.name) {
+        const repoUrl = typeof directPkg.repository === 'string'
+          ? directPkg.repository
+          : directPkg.repository?.url?.replace(/^git\+/, '').replace(/\.git$/, '')
+
+        packages.push({
+          name: directPkg.name,
+          version: directPkg.version || '0.0.0',
+          description: directPkg.description,
+          repoUrl,
+          dir: scanDir,
+        })
+        continue
+      }
+    }
+  }

   for (const entry of readdirSync(scanDir, { withFileTypes: true })) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const directResult = readPackageJsonSafe(join(scanDir, 'package.json'))
if (directResult) {
const directPkg = directResult.parsed as Record<string, any>
if (!directPkg.private && directPkg.name) {
const repoUrl = typeof directPkg.repository === 'string'
? directPkg.repository
: directPkg.repository?.url?.replace(/^git\+/, '').replace(/\.git$/, '')
packages.push({
name: directPkg.name,
version: directPkg.version || '0.0.0',
description: directPkg.description,
repoUrl,
dir: scanDir,
})
continue
}
}
const scanDir = resolve(cwd, base)
const isGlobPattern = pattern.includes('*')
if (!existsSync(scanDir))
continue
if (!isGlobPattern) {
const directResult = readPackageJsonSafe(join(scanDir, 'package.json'))
if (directResult) {
const directPkg = directResult.parsed as Record<string, any>
if (!directPkg.private && directPkg.name) {
const repoUrl = typeof directPkg.repository === 'string'
? directPkg.repository
: directPkg.repository?.url?.replace(/^git\+/, '').replace(/\.git$/, '')
packages.push({
name: directPkg.name,
version: directPkg.version || '0.0.0',
description: directPkg.description,
repoUrl,
dir: scanDir,
})
continue
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/author.ts` around lines 105 - 123, The direct package.json
detection inside detectMonorepoPackages incorrectly runs for glob workspace
entries and causes parent dirs like "packages" to short-circuit child package
discovery; before calling readPackageJsonSafe(scanDir + '/package.json') add a
guard that skips the directResult check when the original workspace pattern
contains glob characters (e.g., '*' '?' or '[') — use the workspace pattern
variable used to derive scanDir (or keep the pattern in scope) and only perform
the directPkg logic (the directResult/directPkg branch that pushes to packages
and continues) when the pattern is a non-glob literal; this preserves the
existing behavior for literal workspace entries while preventing glob parents
from swallowing child packages.

@harlan-zw harlan-zw merged commit 5c1c2e4 into main Mar 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants