v10.0.9#38
Conversation
Closes #36. Component discovery built glob patterns with path.join, which emits "\" separators on Windows. glob treats backslashes as escape chars, so "**" recursion broke and flows in nested subdirectories were never matched. Added a toGlobPattern() helper in discover.js that always emits forward slashes, and routed all four glob pattern builders through it (discover, workflow, reset). Also refine CSX "encoding" handling in updateCodeInJson: - absent / B64 -> Base64 (default, unchanged) - NAT -> native source written as-is (stored as an escaped JSON string via the document serialization) - any other value (e.g. REF) -> left untouched (previously REF entries were incorrectly overwritten with Base64) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ly-discover-flows-in-nested-subdirectories fix(cli): recursive nested discovery on Windows + CSX encoding handling
Reviewer's GuideRefines CSX encoding semantics and updates glob path handling to be OS-agnostic (especially on Windows) by centralizing forward-slash glob pattern construction and reusing it across discovery, reset, and workflow utilities. Flow diagram for CSX encoding handling in updateCodeInJsonflowchart TD
A[updateCodeInJson] --> B[Read obj.encoding]
B --> C{encoding value}
C -->|absent or B64| D[Set obj.code = base64Code
Increment updateCount]
C -->|NAT| E[Set obj.code = nativeCode
Increment updateCount]
C -->|other e.g REF| F[Leave obj.code unchanged
Do not increment updateCount]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a toGlobPattern helper function to resolve Windows compatibility issues by ensuring glob patterns use forward slashes instead of backslashes. It also refactors CSX encoding handling in src/lib/csx.js to support 'B64' and 'NAT' encodings while skipping others. The review feedback recommends adding a defensive check when processing obj.encoding to prevent potential runtime TypeError crashes if the property is not a string.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // absent / B64 -> Base64 (default) | ||
| // NAT -> JSON-stringified native source | ||
| // anything else (e.g. REF) -> leave untouched | ||
| const encoding = obj.encoding ? obj.encoding.toUpperCase() : 'B64'; |
There was a problem hiding this comment.
If obj.encoding is not a string (for example, if it is a boolean, number, or null), calling .toUpperCase() directly will throw a TypeError and crash the process.\n\nTo prevent runtime crashes when processing unexpected JSON structures, we should defensively check that obj.encoding is a non-empty string before calling string methods on it.
| const encoding = obj.encoding ? obj.encoding.toUpperCase() : 'B64'; | |
| const encoding = typeof obj.encoding === 'string' && obj.encoding.trim() ? obj.encoding.trim().toUpperCase() : 'B64'; |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
updateCodeInJson,encodingis now taken asobj.encoding.toUpperCase()without falling back when the property exists but is an empty string; if you still intend empty-string encodings to behave like the default Base64 case, consider normalizing falsy values (e.g.const encoding = (obj.encoding || 'B64').toUpperCase();). - The glob ignore lists (e.g.
['**/.meta/**', '**/node_modules/**', '**/dist/**']) are duplicated across multiple functions; consider centralizing these patterns in a shared constant to keep them consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `updateCodeInJson`, `encoding` is now taken as `obj.encoding.toUpperCase()` without falling back when the property exists but is an empty string; if you still intend empty-string encodings to behave like the default Base64 case, consider normalizing falsy values (e.g. `const encoding = (obj.encoding || 'B64').toUpperCase();`).
- The glob ignore lists (e.g. `['**/.meta/**', '**/node_modules/**', '**/dist/**']`) are duplicated across multiple functions; consider centralizing these patterns in a shared constant to keep them consistent and easier to maintain.
## Individual Comments
### Comment 1
<location path="src/lib/discover.js" line_range="12-15" />
<code_context>
+ * with path.join (which uses "\") breaks "**" recursion and nested files are
+ * never matched. Always feed glob forward-slash separators.
+ * @param {string} dir - Base directory
+ * @param {string} suffix - Glob suffix, e.g. "**\/*.json"
+ * @returns {string} Forward-slash glob pattern
+ */
+function toGlobPattern(dir, suffix) {
+ return path.join(dir, suffix).split(path.sep).join('/');
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Normalize backslashes in `suffix` as well, not just `path.sep` from `path.join`.
The example `suffix` (`"**\/*.json"`) contains backslashes, but `toGlobPattern` only normalizes `path.sep` from `path.join(dir, suffix)`. On POSIX, `path.sep` is `/`, so any `"\\"` in `suffix` stay unmodified and may break globbing. Consider also replacing all backslashes with forward slashes (either in `suffix` before `path.join` or on the final string) so Windows-style separators in `suffix` are handled correctly on all platforms.
Suggested implementation:
```javascript
* @param {string} suffix - Glob suffix, e.g. "**/*.json"
```
```javascript
function toGlobPattern(dir, suffix) {
// Normalize all path separators (including any backslashes in `suffix`) to forward slashes
return path.join(dir, suffix).replace(/\\/g, '/');
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| * @param {string} suffix - Glob suffix, e.g. "**\/*.json" | ||
| * @returns {string} Forward-slash glob pattern | ||
| */ | ||
| function toGlobPattern(dir, suffix) { |
There was a problem hiding this comment.
suggestion (bug_risk): Normalize backslashes in suffix as well, not just path.sep from path.join.
The example suffix ("**\/*.json") contains backslashes, but toGlobPattern only normalizes path.sep from path.join(dir, suffix). On POSIX, path.sep is /, so any "\\" in suffix stay unmodified and may break globbing. Consider also replacing all backslashes with forward slashes (either in suffix before path.join or on the final string) so Windows-style separators in suffix are handled correctly on all platforms.
Suggested implementation:
* @param {string} suffix - Glob suffix, e.g. "**/*.json"function toGlobPattern(dir, suffix) {
// Normalize all path separators (including any backslashes in `suffix`) to forward slashes
return path.join(dir, suffix).replace(/\\/g, '/');
}1 participant
Summary by Sourcery
Adjust CSX JSON update logic and glob path handling for cross-platform correctness.
Bug Fixes:
Enhancements: