feat: scan workers using cli - #68
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis change adds worker-file discovery and source parsing utilities, builds validated worker manifests with effective options, exposes ChangesWorker manifest and discovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant NuxtConfig
participant buildWorkerManifest
participant printWorkersManifest
CLI->>NuxtConfig: load processor configuration
CLI->>buildWorkerManifest: build filtered or complete manifest
buildWorkerManifest-->>CLI: return WorkerManifest
CLI->>printWorkersManifest: print JSON or formatted output
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils/generate-workers-entry-content.ts (1)
45-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
registeredWorkersto simplify logic.Since
registeredWorkersis already derived fromapi.workerson line 26, you can reuse it here to avoid redundant array-type checks and simplify the code.♻️ Proposed refactor
-const workersToRun = selectedWorkers - ? (Array.isArray(api.workers) ? api.workers.filter(w => w && selectedWorkers.includes(w.name)) : []) - : (Array.isArray(api.workers) ? api.workers : []) -if (selectedWorkers && workersToRun.length === 0) { - const available = (Array.isArray(api.workers) ? api.workers.map(w => w && w.name).filter(Boolean) : []) +const workersToRun = selectedWorkers + ? registeredWorkers.filter(w => w && selectedWorkers.includes(w.name)) + : registeredWorkers +if (selectedWorkers && workersToRun.length === 0) { + const available = registeredWorkers.map(w => w && w.name).filter(Boolean)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/generate-workers-entry-content.ts` around lines 45 - 50, Update the workers selection and warning logic around registeredWorkers to reuse that already-normalized collection instead of repeatedly checking and reading api.workers. Filter registeredWorkers for selectedWorkers and derive the available worker names from registeredWorkers while preserving the existing no-match warning behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/cli.spec.ts`:
- Line 285: Fix the implicit any errors in both stdoutSpy.mock.calls map
callbacks: annotate each call parameter as unknown[] at spec/cli.spec.ts lines
285-285 and 304-304, while preserving the existing String(call[0]) behavior.
In `@src/utils/parse-worker-definition.ts`:
- Around line 84-93: Update extractWorkerName and extractPropertyObjectLiteral
to locate name and options through the existing parseTopLevelObject helper
rather than regex-scanning nested source text, preserving null when the
top-level property is absent. In the related parseStringLiteral logic, adjust
the return at match[2] so it satisfies the declared string | null type,
including an appropriate null fallback for an unavailable capture.
---
Nitpick comments:
In `@src/utils/generate-workers-entry-content.ts`:
- Around line 45-50: Update the workers selection and warning logic around
registeredWorkers to reuse that already-normalized collection instead of
repeatedly checking and reading api.workers. Filter registeredWorkers for
selectedWorkers and derive the available worker names from registeredWorkers
while preserving the existing no-match warning behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d394a97-5564-4f89-9e7e-09ffb4274bd2
⛔ Files ignored due to path filters (1)
spec/utils/__snapshots__/generate-workers-entry-content.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
changelog.mddocs/api.mdspec/cli.spec.tsspec/utils/build-worker-manifest.spec.tsspec/utils/generate-workers-entry-content.spec.tsspec/utils/parse-worker-definition.spec.tsspec/utils/scan-worker-files.spec.tssrc/cli.tssrc/module.tssrc/utils/build-worker-manifest.tssrc/utils/cli/print-workers-manifest.tssrc/utils/cli/resolve-processor-options.tssrc/utils/generate-workers-entry-content.tssrc/utils/parse-worker-definition.tssrc/utils/scan-folder.tssrc/utils/scan-worker-files.ts
| function extractPropertyObjectLiteral(objectLiteral: string, propertyName: string): string | null { | ||
| const pattern = new RegExp(`\\b${propertyName}:\\s*\\{`) | ||
| const match = pattern.exec(objectLiteral) | ||
| if (!match) { | ||
| return null | ||
| } | ||
|
|
||
| const openBraceIndex = objectLiteral.indexOf('{', match.index) | ||
| return extractObjectLiteral(objectLiteral, openBraceIndex) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
extractWorkerName/extractPropertyObjectLiteral aren't scoped to top-level keys, risking wrong name/options extraction.
Both helpers run a plain regex .match()/.exec() over the entire objectLiteral text, which includes the body of nested functions/objects (e.g. processor). If a name: or options:-shaped literal appears earlier in the source than the true top-level property (e.g. a fallback object inside processor containing { name: 'unknown' }), the wrong value is picked — silently mis-extracting the worker's identity. Given that parsed.name feeds duplicate-name detection and the manifest (per build-worker-manifest.ts's collectWorkerEntries, which uses parsed.name/parsed.options), this is a correctness risk for a core PR acceptance criterion.
The file already has parseTopLevelObject, which correctly respects nesting depth — reuse it instead of blind regex search for name/options.
Additionally, line 246 (return match[2]) has the same string | undefined vs string | null TS error as flagged for parseStringLiteral.
♻️ Proposed refactor
-function extractWorkerName(objectLiteral: string): string | null {
- const match = objectLiteral.match(/\bname:\s*(['"`])([^'"`]+)\1/)
- if (!match) {
- return null
- }
- return match[2]
-}
-
export function parseWorkerDefinition(source: string): ParsedWorkerDefinition | null {
const callIndex = findDefineWorkerCallIndex(source)
if (callIndex === -1) {
return null
}
const afterCall = source.slice(callIndex)
const openParenIndex = afterCall.indexOf('(')
if (openParenIndex === -1) {
return null
}
const argsStart = afterCall.slice(openParenIndex + 1)
const openBraceIndex = argsStart.indexOf('{')
if (openBraceIndex === -1) {
return null
}
const objectLiteral = extractObjectLiteral(argsStart, openBraceIndex)
if (!objectLiteral) {
return null
}
- const name = extractWorkerName(objectLiteral)
+ const topLevel = parseTopLevelObject(objectLiteral)
+ const nameValue = topLevel.get('name')
+ const name = nameValue ? parseStringLiteral(nameValue) : null
if (!name) {
return null
}
- const optionsLiteral = extractPropertyObjectLiteral(objectLiteral, 'options')
- const options = optionsLiteral ? parseOptionsObject(optionsLiteral) : {}
+ const optionsValue = topLevel.get('options')
+ const options = optionsValue ? parseOptionsObject(optionsValue) : {}
return { name, options }
}Also applies to: 241-282
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 84-84: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${propertyName}:\\s*\\{)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🪛 OpenGrep (1.25.0)
[ERROR] 86-86: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/parse-worker-definition.ts` around lines 84 - 93, Update
extractWorkerName and extractPropertyObjectLiteral to locate name and options
through the existing parseTopLevelObject helper rather than regex-scanning
nested source text, preserving null when the top-level property is absent. In
the related parseStringLiteral logic, adjust the return at match[2] so it
satisfies the declared string | null type, including an appropriate null
fallback for an unavailable capture.
Summary
List what workers will be picked up via the cli with support for workers flag.
Changes
How to Test
Screenshots (optional)
n/a
Linked Issues
Closes #64
Checklist
npm run cipasses (ornpm run lintandnpm testat minimum)