Conversation
|
Warning Rate limit exceeded@gabimoncha has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces a comprehensive audit feature to the Cursor Rules CLI, including new commands, documentation, shell autocompletion, and Unicode security checks. The CLI now dynamically loads rule templates, supports scanning for vulnerabilities, integrates with the external "awesome-cursorrules" repository, and improves file handling with Bun APIs. Supporting scripts, tests, and documentation were added or updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant FileSystem
participant OutOfChar
participant RegexAudit
User->>CLI: Run `cursor-rules audit`
CLI->>FileSystem: Enumerate rule files
loop For each rule file
CLI->>FileSystem: Read file content
CLI->>OutOfChar: Detect non-ASCII vulnerabilities
CLI->>RegexAudit: Apply regex pattern checks
alt Vulnerability found
CLI->>User: Log vulnerable file
end
end
CLI->>User: Prompt to clean files
alt User confirms
CLI->>FileSystem: Overwrite files with cleaned content
CLI->>User: Log cleaning result
else User cancels
CLI->>User: Abort cleaning
end
sequenceDiagram
participant User
participant CLI
participant Tabtab
participant Shell
User->>CLI: Press Tab for autocompletion
CLI->>Tabtab: Handle completion request
Tabtab->>CLI: Provide commands/options/files based on context
CLI->>Shell: Return completion suggestions
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (12)
cli/src/audit/regex.ts (1)
22-22: Fix typo in variable name.There's a typo in the variable name.
-const unassigedCodePointsRegex = regex('g')`\p{Cn}++` +const unassignedCodePointsRegex = regex('g')`\p{Cn}++`Also update the export:
- unassigedCodePointsRegex, + unassignedCodePointsRegex,cli/src/core/installRules.ts (1)
31-39: Improve variable naming and add logging for skipped files.The filename handling logic is correct but could be improved for clarity and debugging.
- let fileName; + let targetFileName; - if (file.endsWith('.md')) { - fileName = file + 'c'; - } else if (file.endsWith('.mdc')){ - fileName = file; - } else { - continue; - } + if (file.endsWith('.md')) { + targetFileName = file + 'c'; + } else if (file.endsWith('.mdc')) { + targetFileName = file; + } else { + logger.prompt.warn(`Skipping unsupported file type: ${file}`); + continue; + }This change:
- Uses a more descriptive variable name
- Adds logging for skipped files to aid debugging
- Improves readability of the conditional logic
scripts/copy-templates.ts (1)
22-22: Report the count of vulnerable files.The
countvariable tracks vulnerable files but is never reported to the user, making it unclear how many files were skipped due to vulnerabilities.Add a summary log after the loop:
+ console.log(`Skipped ${count} vulnerable files`);cli/src/audit/decodeLanguageTags.ts (1)
1-17: Add function documentation.The functions lack documentation explaining their purpose, parameters, and the Unicode ranges they work with.
+/** + * Decodes ASCII characters from Unicode private use area (Language Tags block). + * Converts code points from 0xE0001-0xE007F back to ASCII 0x01-0x7F. + * @param encoded - String containing encoded characters in the private use area + * @returns Decoded ASCII string + */ export function decodeLanguageTags(encoded: string): string {+/** + * Encodes ASCII characters into Unicode private use area (Language Tags block). + * Converts ASCII 0x01-0x7F to code points 0xE0001-0xE007F. + * @param text - ASCII text to encode + * @returns Encoded string in the private use area + */ export function encodeLanguageTags(text: string): string {cli/src/cli/cliRun.ts (1)
145-146: Fix copy-paste comment error.The comment should reflect that this is handling the audit command, not the list command.
- // List command + // Audit commandcli/src/cli/actions/initAction.ts (1)
39-39: Consider using async file operations for better performance.Using synchronous
readdirandreadFileSyncoperations can block the event loop, especially when reading multiple rule files.Consider refactoring to use async operations:
- let templateFiles = await fs.readdir(rulesDir); + const templateFiles = await fs.readdir(rulesDir);Then handle the hint extraction asynchronously within the options mapping.
cli/src/audit/matchRegex.ts (1)
19-25: Add null safety for regex match groups.While the current logic appears sound, adding explicit null checks would improve robustness.
- const range = match?.indices?.groups?.tag - if (range?.length) { - - decoded = decodeLanguageTags(text.slice(range[0], range[1])) + const range = match?.indices?.groups?.tag; + if (range?.length === 2) { + decoded = decodeLanguageTags(text.slice(range[0], range[1]));cli/src/cli/actions/auditRulesAction.ts (1)
53-67: Consider less aggressive cleaning approach.The current implementation removes ALL non-ASCII characters, which might be too aggressive and could remove legitimate Unicode content like smart quotes or accented characters in comments.
Consider providing more granular cleaning options or warning users about what will be removed:
const confirmVulnerableFiles = await confirm({ - message: `\n Do you want to clean these files? (will remove all non-ASCII characters)`, + message: `\n Do you want to clean these files? (will remove potentially dangerous Unicode characters - this may affect legitimate non-ASCII content)`, });Additionally, consider implementing targeted removal based on the specific vulnerabilities detected rather than blanket non-ASCII removal.
cli/src/audit/detectSurrogates.ts (4)
34-35: Remove debugging code from production.The
outOfCharacter.detect()call and its console.log appear to be debugging code that should be removed from the production version.- const detected = outOfCharacter.detect('nothing s͏neak឵y here'); - console.log('detected', detected);
179-179: Add missing semicolon.Missing semicolon after variable assignment.
- const asciiCodePoint = codePoint - 0xE0000 + const asciiCodePoint = codePoint - 0xE0000;
243-279: Remove commented debugging code.Large blocks of commented-out console.log statements should be removed to keep the code clean.
-// console.log("Original 1:", `"${stringWithValidPair}"`); -// console.log("Decoded 1:", `"${decodeCodeUnits(stringWithValidPair)}"`); -// console.log("---"); -// // Expected: "Hello [HIGH: U+D83D][LOW: U+DE00] World" - -// console.log("Original 2:", `"${stringWithLoneHigh}"`); -// console.log("Decoded 2:", `"${decodeCodeUnits(stringWithLoneHigh)}"`); -// console.log("---"); -// // Expected: "Invalid high: [HIGH: U+D83D] here." - -// console.log("Original 3:", `"${stringWithLoneLow}"`); -// console.log("Decoded 3:", `"${decodeCodeUnits(stringWithLoneLow)}"`); -// console.log("---"); -// // Expected: "Invalid low: [LOW: U+DE00] here." - -// console.log("Original 4:", `"${stringWithSwappedPair}"`); -// console.log("Decoded 4:", `"${decodeCodeUnits(stringWithSwappedPair)}"`); -// console.log("---"); -// // Expected: "Swapped: [LOW: U+DE00][HIGH: U+D83D] pair." - -// console.log("Original 5:", `"${bmpString}"`); -// console.log("Decoded 5:", `"${decodeCodeUnits(bmpString)}"`); -// console.log("---"); -// // Expected: "Just Basic Multilingual Plane characters." - -// console.log("Original 6:", `"${specificRangeString}"`); -// console.log("Decoded 6:", `"${decodeCodeUnits(specificRangeString)}"`); -// console.log("---"); -// // Expected: "Test: [HIGH: U+DB90][LOW: U+DC00]" - - -// console.log("Original 7:", `"${hiddenInstructions}"`); -// console.log("Decoded 7:", `"${decodeCodeUnits(hiddenInstructions)}"`); -// console.log("---"); -// // Expected: "Hello [HIGH: U+D83D][LOW: U+DE00] World"
345-361: Remove additional commented debugging code.More commented-out debugging code should be removed.
-// console.log('hiddenInstructions', hiddenInstructions) -// regexArray.forEach(regex => { -// console.log('regex', regex) -// const match = hiddenInstructions.match(regex) -// console.log('hiddenInstructionsMatch', match) -// }) - - -// const tagsMatches = hiddenInstructions.matchAll(tagsRegex) - -// for (const match of tagsMatches) { -// console.log( -// `Found ${match[0]} start=${match.index} end=${ -// match.index + match[0].length -// }.`, -// ); -// }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.gitmodules(1 hunks)README.md(2 hunks)cli/README.md(2 hunks)cli/package.json(1 hunks)cli/src/audit/decodeLanguageTags.ts(1 hunks)cli/src/audit/detectSurrogates.ts(1 hunks)cli/src/audit/matchRegex.ts(1 hunks)cli/src/audit/regex.ts(1 hunks)cli/src/cli/actions/auditRulesAction.ts(1 hunks)cli/src/cli/actions/initAction.ts(1 hunks)cli/src/cli/cliRun.ts(3 hunks)cli/src/core/checkForUpdates.ts(1 hunks)cli/src/core/installRules.ts(1 hunks)docs/CLI_COMMANDS.md(2 hunks)docs/CURSOR_RULES_GUIDE.md(1 hunks)example/.clinerules(1 hunks)example/.clinerules-agent(1 hunks)example/.cursor/rules/bad-rule.mdc(1 hunks)example/.cursorrules(1 hunks)example/.github/copilot-instructions.md(1 hunks)example/.windsurfrules(1 hunks)example/base.avanterules(1 hunks)example/html.editing.avanterules(1 hunks)package.json(1 hunks)scripts/check-awesome-cursorrules.ts(1 hunks)scripts/copy-templates.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
cli/src/audit/matchRegex.ts (3)
cli/src/shared/logger.ts (1)
logger(145-145)cli/src/audit/decodeLanguageTags.ts (1)
decodeLanguageTags(1-17)cli/src/audit/regex.ts (1)
regexTemplates(30-39)
cli/src/core/checkForUpdates.ts (3)
cli/src/shared/logger.ts (2)
logger(145-145)error(78-82)cli/src/core/packageJsonParse.ts (3)
getPackageManager(40-58)getVersion(8-22)getPackageName(24-38)cli/src/core/fileExists.ts (1)
fileExists(3-10)
cli/src/cli/cliRun.ts (1)
cli/src/cli/actions/auditRulesAction.ts (1)
runAuditRulesAction(22-83)
🪛 Biome (1.9.4)
cli/src/cli/actions/auditRulesAction.ts
[error] 64-64: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
cli/src/audit/detectSurrogates.ts
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-52: This code is unreachable
(lint/correctness/noUnreachable)
[error] 52-60: This code is unreachable
(lint/correctness/noUnreachable)
[error] 60-61: This code is unreachable
(lint/correctness/noUnreachable)
[error] 61-68: This code is unreachable
(lint/correctness/noUnreachable)
[error] 71-75: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-77: This code is unreachable
(lint/correctness/noUnreachable)
[error] 77-148: This code is unreachable
(lint/correctness/noUnreachable)
[error] 149-152: This code is unreachable
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pixeebot[bot]
- GitHub Check: pixeebot[bot]
🔇 Additional comments (27)
example/.github/copilot-instructions.md (1)
1-1: Add HTML5 best practices directive
Aligns this example with the updated guideline used across rule configurations.example/.clinerules (1)
1-1: Add HTML5 best practices directive
Ensures consistency with other example rule files.example/.clinerules-agent (1)
1-1: Add HTML5 best practices directive
Maintains uniform guidance across example configurations.example/base.avanterules (1)
1-1: Add HTML5 best practices directive
Keeps example templates aligned with project standards.example/.windsurfrules (1)
1-1: Add HTML5 best practices directive
Consistent inclusion of the HTML5 guideline across example rules..gitmodules (1)
1-3: LGTM! Standard git submodule configuration.The submodule configuration is correctly set up to integrate external cursor rules from the awesome-cursorrules repository.
cli/src/audit/regex.ts (2)
1-39: Well-implemented Unicode security audit patterns.The regex patterns are comprehensive and based on established Unicode security guidelines. The use of the
regexlibrary enables proper Unicode property escapes and set operations.
6-28: Verify regex patterns match intended Unicode ranges.The regex patterns should be tested to ensure they correctly match the intended Unicode character ranges as specified in the referenced Unicode utilities.
#!/bin/bash # Description: Verify that the regex patterns are correctly implemented # Expected: The patterns should match the character ranges as documented in the Unicode references # Check if the regex library is available and patterns compile correctly cd cli npm list regexcli/package.json (2)
51-51: Appropriate dependency for Unicode auditing functionality.The
out-of-characterlibrary is well-suited for the Unicode character analysis needed by the audit command.
51-51: Verify package version and security.Ensure the specified version is current and free from known vulnerabilities.
What is the latest version of the out-of-character npm package and are there any known security vulnerabilities?example/html.editing.avanterules (1)
1-6: Template logic is correctly implemented.The conditional project_context block uses proper template syntax and will correctly include the context when available.
docs/CURSOR_RULES_GUIDE.md (1)
103-104: Verify the external repository link and commit hash.The reference to the external "awesome-cursorrules" repository uses a specific commit hash, which is good for security and reproducibility. However, please verify that the commit hash and repository path are still valid and accessible.
#!/bin/bash # Description: Verify the external repository link and commit hash validity # Expected: The commit should exist and the path should contain rule files # Check if the repository and commit exist curl -s "https://api.github.com/repos/PatrickJS/awesome-cursorrules/commits/7e4db830d65c8951463863dd25cc39b038d34e02" | jq '.sha // "NOT_FOUND"' # Check if the rules-new directory exists at that commit curl -s "https://api.github.com/repos/PatrickJS/awesome-cursorrules/contents/rules-new?ref=7e4db830d65c8951463863dd25cc39b038d34e02" | jq 'if type == "array" then length else "NOT_FOUND" end'docs/CLI_COMMANDS.md (2)
12-12: Well-documented new command.The audit command is properly added to the commands table with a clear description.
52-54: I didn’t find any evidence of anauditcommand registration or implementation. Let’s search for common patterns to locate it:#!/bin/bash # 1. Look for registration of the audit command, e.g., program.command('audit', ...) rg -n "command\(['\"]audit['\"]" # 2. Look for any function or class named audit rg -n "function audit" rg -n "class Audit" # 3. List files with “audit” in their names fd auditcli/README.md (2)
71-73: Consistent documentation of audit command.The audit command usage is properly documented and follows the existing format pattern.
90-93: Good addition of external template source.The "Awesome Rules Templates" section provides users with information about additional rule sources. The documentation is consistent with the same addition in other files.
README.md (2)
71-73: Consistent audit command documentation.The audit command is properly documented and matches the format in other documentation files.
87-90: Consistent external template documentation.The "Awesome Rules Templates" section matches the documentation in other files, maintaining consistency across the project.
cli/src/core/installRules.ts (1)
33-39: Verify the impact of the new file extension handling.This change introduces support for
.mdcfiles and standardizes.mdfiles to.mdcextensions. Please verify that this change doesn't break existing functionality.#!/bin/bash # Description: Check for existing usage of .md and .mdc file handling # Expected: Find any existing references to these file extensions in the codebase # Search for references to .md and .mdc files in the codebase echo "=== Searching for .md file references ===" rg '\.md[^c]' --type ts --type js -A 2 echo "=== Searching for .mdc file references ===" rg '\.mdc' --type ts --type js -A 2 echo "=== Searching for rule file handling ===" rg 'endsWith.*\.md' --type ts --type js -A 2example/.cursor/rules/bad-rule.mdc (1)
1-6: This file serves as a test case for the audit functionality.This appears to be an intentionally vulnerable example file designed to test the new audit command. The long Unicode sequence after "follow HTML5 best practices" on line 6 contains encoded text using the private Unicode area (0xE0000 range) that the audit functionality is designed to detect and flag as suspicious.
scripts/copy-templates.ts (1)
19-19: Verify the directory structure assumptions.The hardcoded path
../awesome-cursorrules/rules-newassumes the awesome-cursorrules directory is a sibling to the current directory. This may not be reliable across different deployment scenarios.Consider using a more robust path resolution or making the path configurable.
#!/bin/bash # Verify the expected directory structure exists if [ -d "../awesome-cursorrules/rules-new" ]; then echo "Directory exists" ls -la ../awesome-cursorrules/rules-new | head -5 else echo "Directory not found" ficli/src/cli/cliRun.ts (1)
66-70: LGTM! Clean integration of the audit command.The audit command is properly integrated into the CLI structure, following the same patterns as existing commands. The command registration and routing logic is consistent and appropriate.
Also applies to: 147-150
cli/src/audit/matchRegex.ts (1)
5-30: LGTM! Clean regex matching implementation.The function correctly handles regex matching with named capture groups and integrates well with the decoding utilities. The debug logging will be helpful for troubleshooting audit issues.
package.json (2)
4-4: LGTM! ES module configuration added.Adding
"type": "module"is appropriate for the modern ES module setup used throughout the codebase.
36-36: Verify the out-of-character dependency version.Ensure the specified version of the
out-of-characterpackage is the latest stable version and free from security vulnerabilities.What is the latest version of the npm package "out-of-character" and are there any known security vulnerabilities?cli/src/core/checkForUpdates.ts (1)
1-13: LGTM! Import organization and formatting improvements.The import reorganization and consistent quote usage improve code readability and maintainability.
cli/src/audit/detectSurrogates.ts (1)
37-49: Fix unreachable code in decodeCodeUnits function.The function has a return statement at line 48 that makes all subsequent code unreachable. This appears to be incomplete implementation where the function should continue processing the input string.
- for (let i = 0; i < input.length; i++) { - const codePoint = input.codePointAt(i); - - if (codePoint === undefined) { - // Should not happen with valid strings, but handle defensively. - i++; - continue; - } - - result.push(`U+${codePoint.toString(16).toUpperCase().padStart(4, "0")}`); - } - return result.join(" "); - - + // This block appears to be debug code and should be removedLikely an incorrect or invalid review comment.
| @@ -0,0 +1 @@ | |||
| - follow HTML5 best practices | |||
There was a problem hiding this comment.
🧩 Verification correct
Suspicious Unicode content detected - verify this is intentional test data.
This file contains extensive non-ASCII Unicode characters that appear to encode hidden malicious content. If this is intentional test data for the audit functionality, it should be clearly documented as such.
🏁 Script executed:
#!/bin/bash
# Description: Analyze the suspicious Unicode content in the example file
# Expected: Decode and analyze what the Unicode characters represent
# Use the out-of-character library to analyze the suspicious content
cd cli
node -e "
const fs = require('fs');
const content = fs.readFileSync('../example/.cursorrules', 'utf8');
console.log('File content length:', content.length);
console.log('First line:', content.split('\n')[0]);
console.log('Character codes:', content.split('').slice(30, 50).map(c => c.charCodeAt(0).toString(16)));
"Length of output: 1656
Suspicious Unicode content detected - verify this is intentional test data.
This file contains extensive non-ASCII Unicode characters that appear to encode hidden malicious content. If this is intentional test data for the audit functionality, it should be clearly documented as such.
#!/bin/bash
# Description: Analyze the suspicious Unicode content in the example file
# Expected: Decode and analyze what the Unicode characters represent
# Use the out-of-character library to analyze the suspicious content
cd cli
node -e "
const fs = require('fs');
const content = fs.readFileSync('../example/.cursorrules', 'utf8');
console.log('File content length:', content.length);
console.log('First line:', content.split('\n')[0]);
console.log('Character codes:', content.split('').slice(30, 50).map(c => c.charCodeAt(0).toString(16)));
"🤖 Prompt for AI Agents
In example/.cursorrules at line 1, the file contains suspicious non-ASCII
Unicode characters that may encode hidden malicious content. Verify if this is
intentional test data for audit functionality; if so, add clear documentation
explaining its purpose. Otherwise, remove or sanitize the content to avoid
security risks.
| </project_context> | ||
| {%- endif %} | ||
|
|
||
| - follow HTML5 best practices |
There was a problem hiding this comment.
🧩 Verification correct
Same suspicious Unicode content as other example files.
This file contains the same suspicious Unicode characters as example/.cursorrules. If these are test files for the audit functionality, they should be clearly documented as such and potentially moved to a dedicated test directory.
Consider moving these test files to a dedicated test directory with clear documentation:
🏁 Script executed:
#!/bin/bash
# Description: Check if there are other files with similar suspicious content
# Expected: Identify all files containing similar Unicode patterns
# Search for files with similar Unicode content patterns
rg -l "" --type-add 'rules:*.{cursorrules,avanterules,clinerules}' --type rulesLength of output: 208
Same suspicious Unicode content as other example files.
This file contains the same suspicious Unicode characters as example/.cursorrules. If these are test files for the audit functionality, they should be clearly documented as such and potentially moved to a dedicated test directory.
Consider moving these test files to a dedicated test directory with clear documentation:
#!/bin/bash
# Description: Check if there are other files with similar suspicious content
# Expected: Identify all files containing similar Unicode patterns
# Search for files with similar Unicode content patterns
rg -l "" --type-add 'rules:*.{cursorrules,avanterules,clinerules}' --type rules🤖 Prompt for AI Agents
In example/html.editing.avanterules at line 7, the file contains suspicious
Unicode characters similar to other test files like example/.cursorrules. To fix
this, verify if this file is intended for audit testing, then move it to a
dedicated test directory and add clear documentation explaining its purpose.
This will improve organization and clarity for test-related files with unusual
content.
| let count = 0; | ||
|
|
||
| for (const file of rulesNewFiles) { | ||
| let text = await Bun.file(path.join(awesomeRulesNew, file)).text() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace Bun-specific API with Node.js standard library.
Using Bun.file() creates a runtime dependency on Bun, while the rest of the codebase appears to use Node.js. This could cause issues in environments where Bun is not available.
- let text = await Bun.file(path.join(awesomeRulesNew, file)).text()
+ let text = await fs.readFile(path.join(awesomeRulesNew, file), 'utf-8')📝 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.
| let text = await Bun.file(path.join(awesomeRulesNew, file)).text() | |
| let text = await fs.readFile(path.join(awesomeRulesNew, file), 'utf-8') |
🤖 Prompt for AI Agents
In scripts/copy-templates.ts at line 25, replace the Bun-specific API call
Bun.file().text() with the Node.js standard library equivalent. Use the
fs/promises module to asynchronously read the file content by importing
fs/promises and then calling fs.readFile with the full file path and encoding
'utf-8' to get the text content. This removes the dependency on Bun and ensures
compatibility with Node.js environments.
| export async function copyTemplates() { | ||
| // Create the templates directory | ||
| const templatesDir = path.join(process.cwd(), 'lib', 'templates', 'rules-default'); | ||
| await fs.mkdir(templatesDir, { recursive: true }); | ||
|
|
||
| // Copy default rules | ||
| const rulesDefault = path.join(process.cwd(), 'src', 'templates', 'rules-default') | ||
| const rulesDefaultFiles = await fs.readdir(rulesDefault, { recursive: true }); | ||
|
|
||
| for (const file of rulesDefaultFiles) { | ||
| await fs.copyFile(path.join(rulesDefault, file), path.join(templatesDir, file)); | ||
| } | ||
|
|
||
| // Copy the awesome cursor rules after checking for vulnerabilities | ||
| const awesomeRulesNew = path.join(process.cwd(), '..', 'awesome-cursorrules', 'rules-new') | ||
| const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true }); | ||
|
|
||
| let count = 0; | ||
|
|
||
| for (const file of rulesNewFiles) { | ||
| let text = await Bun.file(path.join(awesomeRulesNew, file)).text() | ||
| let result = detect(text) | ||
|
|
||
| if (result?.length > 0) { | ||
| console.log(`${'Vulnerable'} ${file}`); | ||
| count++; | ||
| } else { | ||
| await fs.copyFile(path.join(awesomeRulesNew, file), path.join(templatesDir, file)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive error handling.
The function lacks error handling for file system operations, which could lead to unhandled promise rejections if directories don't exist or files can't be read.
export async function copyTemplates() {
- // Create the templates directory
- const templatesDir = path.join(process.cwd(), 'lib', 'templates', 'rules-default');
- await fs.mkdir(templatesDir, { recursive: true });
+ try {
+ // Create the templates directory
+ const templatesDir = path.join(process.cwd(), 'lib', 'templates', 'rules-default');
+ await fs.mkdir(templatesDir, { recursive: true });
+
+ // Copy default rules
+ const rulesDefault = path.join(process.cwd(), 'src', 'templates', 'rules-default')
+ const rulesDefaultFiles = await fs.readdir(rulesDefault, { recursive: true });
+
+ for (const file of rulesDefaultFiles) {
+ await fs.copyFile(path.join(rulesDefault, file), path.join(templatesDir, file));
+ }
+
+ // Copy the awesome cursor rules after checking for vulnerabilities
+ const awesomeRulesNew = path.join(process.cwd(), '..', 'awesome-cursorrules', 'rules-new')
+
+ if (!(await fs.access(awesomeRulesNew).then(() => true).catch(() => false))) {
+ console.log('awesome-cursorrules directory not found, skipping external rules');
+ return;
+ }
+
+ const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true });
+ let count = 0;
+
+ for (const file of rulesNewFiles) {
+ const text = await fs.readFile(path.join(awesomeRulesNew, file), 'utf-8');
+ const result = detect(text);
+
+ if (result?.length > 0) {
+ console.log(`Vulnerable ${file}`);
+ count++;
+ } else {
+ await fs.copyFile(path.join(awesomeRulesNew, file), path.join(templatesDir, file));
+ }
+ }
+
+ console.log(`Skipped ${count} vulnerable files`);
+ } catch (error) {
+ console.error('Failed to copy templates:', error);
+ throw error;
+ }
}📝 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.
| export async function copyTemplates() { | |
| // Create the templates directory | |
| const templatesDir = path.join(process.cwd(), 'lib', 'templates', 'rules-default'); | |
| await fs.mkdir(templatesDir, { recursive: true }); | |
| // Copy default rules | |
| const rulesDefault = path.join(process.cwd(), 'src', 'templates', 'rules-default') | |
| const rulesDefaultFiles = await fs.readdir(rulesDefault, { recursive: true }); | |
| for (const file of rulesDefaultFiles) { | |
| await fs.copyFile(path.join(rulesDefault, file), path.join(templatesDir, file)); | |
| } | |
| // Copy the awesome cursor rules after checking for vulnerabilities | |
| const awesomeRulesNew = path.join(process.cwd(), '..', 'awesome-cursorrules', 'rules-new') | |
| const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true }); | |
| let count = 0; | |
| for (const file of rulesNewFiles) { | |
| let text = await Bun.file(path.join(awesomeRulesNew, file)).text() | |
| let result = detect(text) | |
| if (result?.length > 0) { | |
| console.log(`${'Vulnerable'} ${file}`); | |
| count++; | |
| } else { | |
| await fs.copyFile(path.join(awesomeRulesNew, file), path.join(templatesDir, file)); | |
| } | |
| } | |
| } | |
| export async function copyTemplates() { | |
| try { | |
| // Create the templates directory | |
| const templatesDir = path.join(process.cwd(), 'lib', 'templates', 'rules-default'); | |
| await fs.mkdir(templatesDir, { recursive: true }); | |
| // Copy default rules | |
| const rulesDefault = path.join(process.cwd(), 'src', 'templates', 'rules-default') | |
| const rulesDefaultFiles = await fs.readdir(rulesDefault, { recursive: true }); | |
| for (const file of rulesDefaultFiles) { | |
| await fs.copyFile(path.join(rulesDefault, file), path.join(templatesDir, file)); | |
| } | |
| // Copy the awesome cursor rules after checking for vulnerabilities | |
| const awesomeRulesNew = path.join(process.cwd(), '..', 'awesome-cursorrules', 'rules-new') | |
| if (!(await fs.access(awesomeRulesNew).then(() => true).catch(() => false))) { | |
| console.log('awesome-cursorrules directory not found, skipping external rules'); | |
| return; | |
| } | |
| const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true }); | |
| let count = 0; | |
| for (const file of rulesNewFiles) { | |
| const text = await fs.readFile(path.join(awesomeRulesNew, file), 'utf-8'); | |
| const result = detect(text); | |
| if (result?.length > 0) { | |
| console.log(`Vulnerable ${file}`); | |
| count++; | |
| } else { | |
| await fs.copyFile(path.join(awesomeRulesNew, file), path.join(templatesDir, file)); | |
| } | |
| } | |
| console.log(`Skipped ${count} vulnerable files`); | |
| } catch (error) { | |
| console.error('Failed to copy templates:', error); | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/copy-templates.ts between lines 5 and 35, the function copyTemplates
performs multiple asynchronous file system operations without any error
handling, risking unhandled promise rejections. Wrap the entire function body or
each asynchronous operation in try-catch blocks to catch and handle errors
gracefully. Log or handle errors appropriately to ensure the function fails
safely without crashing the process.
| export async function checkForVulnerability() { | ||
| let count = 0; | ||
|
|
||
| const awesomeRulesNew = path.join(process.cwd(), 'awesome-cursorrules', 'rules-new') | ||
| const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true }); | ||
|
|
||
| for(const file of rulesNewFiles) { | ||
| let text = await Bun.file(path.join(awesomeRulesNew, file)).text() | ||
| let result = detect(text) | ||
|
|
||
| if (result?.length > 0) { | ||
| console.log(`${'Vulnerable'} ${file}`); | ||
| count++; | ||
| } | ||
| } | ||
|
|
||
| const awesomeRules = path.join(process.cwd(), 'awesome-cursorrules', 'rules') | ||
| const rulesFiles = await fs.readdir(awesomeRules, { recursive: true }); | ||
|
|
||
| const rulesFilesFiltered = rulesFiles.filter(f => f.endsWith('.mdc') || f === '.cursorrules') | ||
|
|
||
| for(const file of rulesFilesFiltered) { | ||
| let text = await Bun.file(path.resolve(awesomeRules, file)).text() | ||
| let result = detect(text) | ||
|
|
||
| if (result?.length > 0) { | ||
| console.log(`${'Vulnerable'} ${file}`); | ||
| count++; | ||
| } | ||
| } | ||
|
|
||
| console.log(`Found ${count} vulnerable rules`); | ||
| if (count > 0) { | ||
| process.exit(1) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive error handling.
The function lacks error handling for file system operations and could fail silently or with unhandled errors.
export async function checkForVulnerability() {
- let count = 0;
+ try {
+ let count = 0;
- const awesomeRulesNew = path.join(process.cwd(), 'awesome-cursorrules', 'rules-new')
- const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true });
+ const awesomeRulesNew = path.join(process.cwd(), 'awesome-cursorrules', 'rules-new')
+
+ if (!(await fs.access(awesomeRulesNew).then(() => true).catch(() => false))) {
+ console.log('awesome-cursorrules/rules-new directory not found');
+ return;
+ }
+
+ const rulesNewFiles = await fs.readdir(awesomeRulesNew, { recursive: true });
+
+ // ... rest of function logic
+
+ console.log(`Found ${count} vulnerable rules`);
+ if (count > 0) {
+ process.exit(1)
+ }
+ } catch (error) {
+ console.error('Failed to check for vulnerabilities:', error);
+ process.exit(1);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/check-awesome-cursorrules.ts around lines 5 to 40, the function lacks
error handling for asynchronous file system operations which may cause unhandled
exceptions. Wrap the file reading and directory listing calls in try-catch
blocks to catch and log errors gracefully. Ensure that any caught errors are
logged with meaningful messages and the function exits or handles the error
appropriately to avoid silent failures.
| const cacheFile = path.join(getCacheDir(), "update-check.json"); | ||
| fs.writeFile(cacheFile, JSON.stringify(data)); | ||
| const cacheFile = path.join(getCacheDir(), 'update-check.json'); | ||
| fs.writeFile(cacheFile, JSON.stringify(data)); |
There was a problem hiding this comment.
Add missing await for async writeCache operation.
The writeCache function is async but not being awaited, which could cause issues with the cache not being written properly.
- fs.writeFile(cacheFile, JSON.stringify(data));
+ await fs.writeFile(cacheFile, JSON.stringify(data));📝 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.
| fs.writeFile(cacheFile, JSON.stringify(data)); | |
| await fs.writeFile(cacheFile, JSON.stringify(data)); |
🤖 Prompt for AI Agents
In cli/src/core/checkForUpdates.ts at line 170, the async function writeCache is
called without awaiting its completion, which may lead to the cache not being
written properly. Add the await keyword before the writeCache call to ensure the
operation completes before proceeding.
| const codePoint = ((highSurrogate - highSurrogateStart) * 0x400) + (lowSurrogate - lowSurrogateStart) + 0x10000; | ||
| return String.fromCharCode(codePoint); |
There was a problem hiding this comment.
Fix incorrect String.fromCharCode usage.
The function uses String.fromCharCode() for supplementary plane characters, but should use String.fromCodePoint() since the calculated code point can exceed the BMP range.
- return String.fromCharCode(codePoint);
+ return String.fromCodePoint(codePoint);📝 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.
| const codePoint = ((highSurrogate - highSurrogateStart) * 0x400) + (lowSurrogate - lowSurrogateStart) + 0x10000; | |
| return String.fromCharCode(codePoint); | |
| const codePoint = ((highSurrogate - highSurrogateStart) * 0x400) + (lowSurrogate - lowSurrogateStart) + 0x10000; | |
| - return String.fromCharCode(codePoint); | |
| + return String.fromCodePoint(codePoint); |
🤖 Prompt for AI Agents
In cli/src/audit/detectSurrogates.ts at lines 157-158, the code uses
String.fromCharCode() to convert a code point that may be outside the Basic
Multilingual Plane, which is incorrect. Replace String.fromCharCode(codePoint)
with String.fromCodePoint(codePoint) to correctly handle supplementary plane
characters.
| function decodeCodeUnits(input: string): string { | ||
| const result: string[] = []; | ||
| const highSurrogateStart = 0xd800; | ||
| const highSurrogateEnd = 0xdb7f; | ||
| const highPrivateUseStart = 0xdb80; | ||
| const highPrivateUseEnd = 0xdbff; | ||
| const lowSurrogateStart = 0xdc00; | ||
| const lowSurrogateEnd = 0xdfff; | ||
| const privateUseAreaStart = 0xe000; | ||
| const privateUseAreaEnd = 0xf8ff; | ||
| const tagsStart = 0xe0000; | ||
| const tagsEnd = 0xe007f; | ||
| const variationSelectorStart = 0xe0100; | ||
| const variationSelectorEnd = 0xe01ef; | ||
| const supplementaryPUA_AStart = 0xf0000; | ||
| const supplementaryPUA_AEnd = 0xffffd; | ||
| const supplementaryPUA_BStart = 0x100000; | ||
| const supplementaryPUA_BEnd = 0x10fffd; | ||
|
|
||
| const detected = outOfCharacter.detect('nothing s͏neak឵y here'); | ||
| console.log('detected', detected); | ||
|
|
||
| for (let i = 0; i < input.length; i++) { | ||
| const codePoint = input.codePointAt(i); | ||
|
|
||
| if (codePoint === undefined) { | ||
| // Should not happen with valid strings, but handle defensively. | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| result.push(`U+${codePoint.toString(16).toUpperCase().padStart(4, "0")}`); | ||
| } | ||
| return result.join(" "); | ||
|
|
||
|
|
||
| for (let i = 0; i < input.length; i++) { | ||
| const codeUnit = input.charCodeAt(i); | ||
|
|
||
| if (codeUnit === undefined) { | ||
| // Should not happen with valid strings, but handle defensively. | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | ||
| const decoded = decodeSurrogatePairs(codeUnit, input.charCodeAt(i + 1)); | ||
| if (decoded) { | ||
| result.push(decoded); | ||
| i++; | ||
| } else { | ||
| result.push(input[i]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result.join(""); | ||
|
|
||
|
|
||
|
|
||
| // Iterate through the string using charCodeAt to get 16-bit code units | ||
| for (let i = 0; i < input.length; i++) { | ||
| const codeUnit = input.charCodeAt(i); | ||
|
|
||
| if (codeUnit === undefined) { | ||
| // Should not happen with valid strings, but handle defensively. | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| let isSurrogate = false; | ||
|
|
||
|
|
||
| // Check if it's a high surrogate | ||
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`High Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a high surrogate | ||
| if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`High Private Use Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a low surrogate | ||
| if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Low Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Private Use Area: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a tag | ||
| if (codeUnit >= tagsStart && codeUnit <= tagsEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Tag: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a variation selector | ||
| if (codeUnit >= variationSelectorStart && codeUnit <= variationSelectorEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Variation Selector: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= supplementaryPUA_AStart && codeUnit <= supplementaryPUA_AEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Supplementary Private Use Area A: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= supplementaryPUA_BStart && codeUnit <= supplementaryPUA_BEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Supplementary Private Use Area B: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // If it wasn't a surrogate, keep the original character | ||
| if (!isSurrogate) { | ||
| // We can just push the character at index i, as it's guaranteed | ||
| // to be a single code unit character in this case. | ||
| result.push(input[i]); | ||
| } | ||
| } | ||
|
|
||
| return result.join(""); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restructure the decodeCodeUnits function properly.
The current function structure is broken and needs a complete rewrite to handle the intended functionality correctly.
Here's a suggested complete rewrite of the function:
function decodeCodeUnits(input: string): string {
const result: string[] = [];
const highSurrogateStart = 0xd800;
const highSurrogateEnd = 0xdb7f;
const highPrivateUseStart = 0xdb80;
const highPrivateUseEnd = 0xdbff;
const lowSurrogateStart = 0xdc00;
const lowSurrogateEnd = 0xdfff;
const privateUseAreaStart = 0xe000;
const privateUseAreaEnd = 0xf8ff;
// Iterate through the string using charCodeAt to get 16-bit code units
for (let i = 0; i < input.length; i++) {
const codeUnit = input.charCodeAt(i);
let isSurrogate = false;
// Check if it's a high surrogate
if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) {
const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
result.push(`High Surrogate: [U+${hexCode}]`);
isSurrogate = true;
} else if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) {
const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
result.push(`High Private Use Surrogate: [U+${hexCode}]`);
isSurrogate = true;
} else if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) {
const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
result.push(`Low Surrogate: [U+${hexCode}]`);
isSurrogate = true;
} else if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) {
const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
result.push(`Private Use Area: [U+${hexCode}]`);
isSurrogate = true;
}
// If it wasn't a surrogate, keep the original character
if (!isSurrogate) {
result.push(input[i]);
}
}
return result.join("");
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-52: This code is unreachable
(lint/correctness/noUnreachable)
[error] 52-60: This code is unreachable
(lint/correctness/noUnreachable)
[error] 60-61: This code is unreachable
(lint/correctness/noUnreachable)
[error] 61-68: This code is unreachable
(lint/correctness/noUnreachable)
[error] 71-75: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-77: This code is unreachable
(lint/correctness/noUnreachable)
[error] 77-148: This code is unreachable
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In cli/src/audit/detectSurrogates.ts around lines 15 to 150, the decodeCodeUnits
function is improperly structured with multiple unreachable code blocks and
redundant loops. Rewrite the function to have a single coherent flow that
iterates through the input string once, correctly handling surrogate pairs and
other Unicode ranges, pushing appropriate decoded values or original characters
to the result array, and returning the joined string at the end. Remove
duplicate loops and unreachable returns to ensure the function executes as
intended.
| for (let i = 0; i < input.length; i++) { | ||
| const codeUnit = input.charCodeAt(i); | ||
|
|
||
| if (codeUnit === undefined) { | ||
| // Should not happen with valid strings, but handle defensively. | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | ||
| const decoded = decodeSurrogatePairs(codeUnit, input.charCodeAt(i + 1)); | ||
| if (decoded) { | ||
| result.push(decoded); | ||
| i++; | ||
| } else { | ||
| result.push(input[i]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result.join(""); | ||
|
|
||
|
|
||
|
|
||
| // Iterate through the string using charCodeAt to get 16-bit code units | ||
| for (let i = 0; i < input.length; i++) { | ||
| const codeUnit = input.charCodeAt(i); | ||
|
|
||
| if (codeUnit === undefined) { | ||
| // Should not happen with valid strings, but handle defensively. | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| let isSurrogate = false; | ||
|
|
||
|
|
||
| // Check if it's a high surrogate | ||
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`High Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a high surrogate | ||
| if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`High Private Use Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a low surrogate | ||
| if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Low Surrogate: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Private Use Area: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a tag | ||
| if (codeUnit >= tagsStart && codeUnit <= tagsEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Tag: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // Check if it's a variation selector | ||
| if (codeUnit >= variationSelectorStart && codeUnit <= variationSelectorEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Variation Selector: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= supplementaryPUA_AStart && codeUnit <= supplementaryPUA_AEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Supplementary Private Use Area A: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| if (codeUnit >= supplementaryPUA_BStart && codeUnit <= supplementaryPUA_BEnd) { | ||
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | ||
| result.push(`Supplementary Private Use Area B: [U+${hexCode}]`); | ||
| isSurrogate = true; | ||
| } else | ||
|
|
||
| // If it wasn't a surrogate, keep the original character | ||
| if (!isSurrogate) { | ||
| // We can just push the character at index i, as it's guaranteed | ||
| // to be a single code unit character in this case. | ||
| result.push(input[i]); | ||
| } | ||
| } | ||
|
|
||
| return result.join(""); |
There was a problem hiding this comment.
Fix unreachable code and incomplete logic.
The entire block from lines 51-149 is unreachable due to the early return statement. The function logic appears incomplete and needs to be restructured to properly handle the different Unicode categories.
- for (let i = 0; i < input.length; i++) {
- const codeUnit = input.charCodeAt(i);
-
- if (codeUnit === undefined) {
- // Should not happen with valid strings, but handle defensively.
- i++;
- continue;
- }
-
- if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) {
- const decoded = decodeSurrogatePairs(codeUnit, input.charCodeAt(i + 1));
- if (decoded) {
- result.push(decoded);
- i++;
- } else {
- result.push(input[i]);
- }
- }
- }
-
- return result.join("");
-
-
-
- // Iterate through the string using charCodeAt to get 16-bit code units
- for (let i = 0; i < input.length; i++) {
- const codeUnit = input.charCodeAt(i);
-
- if (codeUnit === undefined) {
- // Should not happen with valid strings, but handle defensively.
- i++;
- continue;
- }
-
- let isSurrogate = false;
-
-
- // Check if it's a high surrogate
- if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`High Surrogate: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- // Check if it's a high surrogate
- if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`High Private Use Surrogate: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- // Check if it's a low surrogate
- if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Low Surrogate: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Private Use Area: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- // Check if it's a tag
- if (codeUnit >= tagsStart && codeUnit <= tagsEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Tag: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- // Check if it's a variation selector
- if (codeUnit >= variationSelectorStart && codeUnit <= variationSelectorEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Variation Selector: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- if (codeUnit >= supplementaryPUA_AStart && codeUnit <= supplementaryPUA_AEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Supplementary Private Use Area A: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- if (codeUnit >= supplementaryPUA_BStart && codeUnit <= supplementaryPUA_BEnd) {
- const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
- result.push(`Supplementary Private Use Area B: [U+${hexCode}]`);
- isSurrogate = true;
- } else
-
- // If it wasn't a surrogate, keep the original character
- if (!isSurrogate) {
- // We can just push the character at index i, as it's guaranteed
- // to be a single code unit character in this case.
- result.push(input[i]);
- }
- }
-
- return result.join("");
+ // Iterate through the string using charCodeAt to get 16-bit code units
+ for (let i = 0; i < input.length; i++) {
+ const codeUnit = input.charCodeAt(i);
+
+ if (codeUnit === undefined) {
+ // Should not happen with valid strings, but handle defensively.
+ i++;
+ continue;
+ }
+
+ let isSurrogate = false;
+
+ // Check if it's a high surrogate
+ if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) {
+ const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
+ result.push(`High Surrogate: [U+${hexCode}]`);
+ isSurrogate = true;
+ } else if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) {
+ const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
+ result.push(`High Private Use Surrogate: [U+${hexCode}]`);
+ isSurrogate = true;
+ } else if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) {
+ const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
+ result.push(`Low Surrogate: [U+${hexCode}]`);
+ isSurrogate = true;
+ } else if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) {
+ const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0");
+ result.push(`Private Use Area: [U+${hexCode}]`);
+ isSurrogate = true;
+ }
+
+ // Note: Tags, variation selectors, and supplementary PUA characters
+ // are outside the BMP and cannot be detected using charCodeAt alone.
+ // Consider using codePointAt for full Unicode support.
+
+ // If it wasn't a surrogate, keep the original character
+ if (!isSurrogate) {
+ result.push(input[i]);
+ }
+ }
+
+ return result.join("");📝 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.
| for (let i = 0; i < input.length; i++) { | |
| const codeUnit = input.charCodeAt(i); | |
| if (codeUnit === undefined) { | |
| // Should not happen with valid strings, but handle defensively. | |
| i++; | |
| continue; | |
| } | |
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | |
| const decoded = decodeSurrogatePairs(codeUnit, input.charCodeAt(i + 1)); | |
| if (decoded) { | |
| result.push(decoded); | |
| i++; | |
| } else { | |
| result.push(input[i]); | |
| } | |
| } | |
| } | |
| return result.join(""); | |
| // Iterate through the string using charCodeAt to get 16-bit code units | |
| for (let i = 0; i < input.length; i++) { | |
| const codeUnit = input.charCodeAt(i); | |
| if (codeUnit === undefined) { | |
| // Should not happen with valid strings, but handle defensively. | |
| i++; | |
| continue; | |
| } | |
| let isSurrogate = false; | |
| // Check if it's a high surrogate | |
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`High Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| // Check if it's a high surrogate | |
| if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`High Private Use Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| // Check if it's a low surrogate | |
| if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Low Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Private Use Area: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| // Check if it's a tag | |
| if (codeUnit >= tagsStart && codeUnit <= tagsEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Tag: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| // Check if it's a variation selector | |
| if (codeUnit >= variationSelectorStart && codeUnit <= variationSelectorEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Variation Selector: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| if (codeUnit >= supplementaryPUA_AStart && codeUnit <= supplementaryPUA_AEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Supplementary Private Use Area A: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| if (codeUnit >= supplementaryPUA_BStart && codeUnit <= supplementaryPUA_BEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Supplementary Private Use Area B: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else | |
| // If it wasn't a surrogate, keep the original character | |
| if (!isSurrogate) { | |
| // We can just push the character at index i, as it's guaranteed | |
| // to be a single code unit character in this case. | |
| result.push(input[i]); | |
| } | |
| } | |
| return result.join(""); | |
| // Iterate through the string using charCodeAt to get 16-bit code units | |
| for (let i = 0; i < input.length; i++) { | |
| const codeUnit = input.charCodeAt(i); | |
| if (codeUnit === undefined) { | |
| // Should not happen with valid strings, but handle defensively. | |
| i++; | |
| continue; | |
| } | |
| let isSurrogate = false; | |
| // Check if it's a high surrogate | |
| if (codeUnit >= highSurrogateStart && codeUnit <= highSurrogateEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`High Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else if (codeUnit >= highPrivateUseStart && codeUnit <= highPrivateUseEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`High Private Use Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else if (codeUnit >= lowSurrogateStart && codeUnit <= lowSurrogateEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Low Surrogate: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } else if (codeUnit >= privateUseAreaStart && codeUnit <= privateUseAreaEnd) { | |
| const hexCode = codeUnit.toString(16).toUpperCase().padStart(4, "0"); | |
| result.push(`Private Use Area: [U+${hexCode}]`); | |
| isSurrogate = true; | |
| } | |
| // Note: Tags, variation selectors, and supplementary PUA characters | |
| // live outside the BMP and cannot be detected with charCodeAt alone. | |
| // For full Unicode support consider using codePointAt. | |
| // If it wasn't a surrogate, keep the original character | |
| if (!isSurrogate) { | |
| result.push(input[i]); | |
| } | |
| } | |
| return result.join(""); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-51: This code is unreachable
(lint/correctness/noUnreachable)
[error] 51-52: This code is unreachable
(lint/correctness/noUnreachable)
[error] 52-60: This code is unreachable
(lint/correctness/noUnreachable)
[error] 60-61: This code is unreachable
(lint/correctness/noUnreachable)
[error] 61-68: This code is unreachable
(lint/correctness/noUnreachable)
[error] 71-75: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-76: This code is unreachable
(lint/correctness/noUnreachable)
[error] 76-77: This code is unreachable
(lint/correctness/noUnreachable)
[error] 77-148: This code is unreachable
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In cli/src/audit/detectSurrogates.ts between lines 51 and 149, there is an early
return statement that causes the subsequent code block to be unreachable,
resulting in incomplete logic for handling various Unicode categories. To fix
this, remove or reposition the early return so that the entire loop processing
different Unicode ranges executes properly. Restructure the function to ensure
all Unicode category checks are performed before returning the final joined
result string.
| console.log('deprecated', deprecated) | ||
| console.log('deprecatedMatch', deprecatedMatch) | ||
| console.log('deprecatedCharMatch', deprecatedCharMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCc%7D-%5B%5Ct%5Cn%5Cr%5D&esc=on&g=gc&i= | ||
| const controlChar = String.fromCharCode(0x0001) + 'test' + '\t' | ||
| // const controlRegex = regex('g')`[\u0000-\u0009\u000E-\u001F\u007F-\u0084\u0086-\u009F\u000B\u000C\u0085]++` | ||
| const controlCharRegex = regex('g')`[\p{Cc}--[\t\n\r]]++` | ||
| const controlCharMatch = controlChar.match(controlCharRegex) | ||
| console.log('controlChar', controlChar) | ||
| console.log('controlCharMatch', controlCharMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCf%7D-%5Cp%7Bemoji_component%7D-%5B%5Cu00AD%5Cu200b-%5Cu200d%5Cu2060%5Cu180E%5D&esc=on&g=gc&i= | ||
| const formatCharacters = [String.fromCharCode(0x00AD), String.fromCharCode(0x0600), String.fromCharCode(0x06DD), String.fromCharCode(0x0890), String.fromCharCode(0xFFFB), String.fromCodePoint(0x110BD), String.fromCodePoint(0x13437), String.fromCodePoint(0xE0001)] | ||
| const formatCharactersRegex = regex('g')`[\p{Cf}--\p{Emoji_Component}--[\u00AD\u200b-\u200d\u2060\u180e\u{e0001}]]++` | ||
| const formatCharactersMatch = formatCharacters.join(', ').match(formatCharactersRegex) | ||
| console.log('formatCharacters', formatCharacters.join(', ')) | ||
| console.log('formatCharactersMatch', formatCharactersMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCo%7D&esc=on&g=gc&i= | ||
| const privateUse = [String.fromCharCode(0xE000), String.fromCharCode(0xF8FF), String.fromCodePoint(0xF0FFF), String.fromCodePoint(0x100FFD), String.fromCodePoint(0x100FD)] | ||
| const privateUseRegex = regex('g')`\p{Co}++` | ||
| const privateUseMatch = privateUse.join(', ').match(privateUseRegex) | ||
| console.log('privateUse', privateUse.join(', ')) | ||
| console.log('privateUseMatch', privateUseMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCs%7D&esc=on&g=gc&i= | ||
| const surrogates = [String.fromCharCode(0xD800), String.fromCharCode(0xDC40), String.fromCodePoint(0xDBFF), String.fromCodePoint(0xDC00), String.fromCodePoint(0xDFFF)] | ||
| const surrogatesRegex = regex('g')`\p{Cs}++` | ||
| const surrogatesMatch = surrogates.join(', ').match(surrogatesRegex) | ||
| console.log('surrogates', surrogates.join(', ')) | ||
| console.log('surrogatesMatch', surrogatesMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCn%7D&g=gc&i= | ||
| const unassigedCodePoints = [String.fromCharCode(0x0378), String.fromCharCode(0x05CF), String.fromCodePoint(0x1127F), String.fromCodePoint(0x1E02F), String.fromCodePoint(0x10FFFF)] | ||
| const unassigedCodePointsRegex = regex('g')`\p{Cn}++` | ||
| const unassigedCodePointsMatch = unassigedCodePoints.join(', ').match(unassigedCodePointsRegex) | ||
| console.log('unassigedCodePoints', unassigedCodePoints.join(', ')) | ||
| console.log('unassigedCodePointsMatch', unassigedCodePointsMatch) | ||
|
|
||
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCn%7D&g=gc&i= | ||
| const misleadingWhitespace = [String.fromCharCode(0x000B), String.fromCodePoint(0x0020), String.fromCharCode(0x2028), "\t", String.fromCodePoint(0xFFA0)," ", String.fromCodePoint(0x00A0), String.fromCodePoint(0x3000)] | ||
| const misleadingWhitespaceRegex = regex('g')`[[\p{White_Space}[\u115F\u1160\u3164\uFFA0]]--[\u0020\t\n\r]]++` | ||
| const misleadingWhitespaceMatch = misleadingWhitespace.join(', ').match(misleadingWhitespaceRegex) | ||
| console.log('misleadingWhitespace', misleadingWhitespace.join(', ')) | ||
| console.log('misleadingWhitespaceMatch', misleadingWhitespaceMatch) | ||
|
|
||
| const tagsRegex = regex('g')`[\u{e0001}\u{e007f}]+?` | ||
|
|
||
| const variationSelectorRegex = regex('g')`[\u{e0100}-\u{e01ef}]++` | ||
|
|
||
|
|
||
| const regexArray = [deprecatedRegex, controlCharRegex, formatCharactersRegex, privateUseRegex, surrogatesRegex, unassigedCodePointsRegex, misleadingWhitespaceRegex, tagsRegex, variationSelectorRegex] | ||
|
|
||
|
|
||
|
|
||
| // console.log('hiddenInstructions', hiddenInstructions) | ||
| // regexArray.forEach(regex => { | ||
| // console.log('regex', regex) | ||
| // const match = hiddenInstructions.match(regex) | ||
| // console.log('hiddenInstructionsMatch', match) | ||
| // }) | ||
|
|
||
|
|
||
| // const tagsMatches = hiddenInstructions.matchAll(tagsRegex) | ||
|
|
||
| // for (const match of tagsMatches) { | ||
| // console.log( | ||
| // `Found ${match[0]} start=${match.index} end=${ | ||
| // match.index + match[0].length | ||
| // }.`, | ||
| // ); | ||
| // } | ||
|
|
||
|
|
||
|
|
||
| const tagRanges = regex('gd')`((?<tagStart>\u{e0001})[\u{e0002}-\u{e007d}]*(?<tagEnd>\u{e007f}))`; | ||
| // const reStart = regex('gd')`\u{e0001}+?`; | ||
| console.log('tagRanges:', tagRanges); | ||
|
|
||
| const tags = regex('gd')`(?<tag>[\u{e0000}-\u{e007d}]+)`; | ||
| console.log('tags:', tags); | ||
|
|
||
| const str = testPillarRules + "test" + testPillarRules | ||
| const matches = [...hiddenRules.matchAll(tags)] | ||
|
|
||
| for (const match of matches) { | ||
| const range = match?.indices?.groups?.tag | ||
| console.log('Indices range:', match?.indices?.groups?.tag); | ||
|
|
||
| if(range?.length) { | ||
| const decode = decodeTagCharacters(hiddenRules.slice(range[0], range[1])) | ||
| console.log(decode) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing or conditionally enabling console.log statements.
The production code contains numerous console.log statements that should either be removed or conditionally enabled for debugging.
Consider wrapping debug output in a conditional check:
+const DEBUG = process.env.NODE_ENV === 'development';
+
+function debugLog(...args: any[]) {
+ if (DEBUG) {
+ console.log(...args);
+ }
+}
+
-console.log('deprecated', deprecated)
-console.log('deprecatedMatch', deprecatedMatch)
-console.log('deprecatedCharMatch', deprecatedCharMatch)
+debugLog('deprecated', deprecated)
+debugLog('deprecatedMatch', deprecatedMatch)
+debugLog('deprecatedCharMatch', deprecatedCharMatch)📝 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.
| console.log('deprecated', deprecated) | |
| console.log('deprecatedMatch', deprecatedMatch) | |
| console.log('deprecatedCharMatch', deprecatedCharMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCc%7D-%5B%5Ct%5Cn%5Cr%5D&esc=on&g=gc&i= | |
| const controlChar = String.fromCharCode(0x0001) + 'test' + '\t' | |
| // const controlRegex = regex('g')`[\u0000-\u0009\u000E-\u001F\u007F-\u0084\u0086-\u009F\u000B\u000C\u0085]++` | |
| const controlCharRegex = regex('g')`[\p{Cc}--[\t\n\r]]++` | |
| const controlCharMatch = controlChar.match(controlCharRegex) | |
| console.log('controlChar', controlChar) | |
| console.log('controlCharMatch', controlCharMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCf%7D-%5Cp%7Bemoji_component%7D-%5B%5Cu00AD%5Cu200b-%5Cu200d%5Cu2060%5Cu180E%5D&esc=on&g=gc&i= | |
| const formatCharacters = [String.fromCharCode(0x00AD), String.fromCharCode(0x0600), String.fromCharCode(0x06DD), String.fromCharCode(0x0890), String.fromCharCode(0xFFFB), String.fromCodePoint(0x110BD), String.fromCodePoint(0x13437), String.fromCodePoint(0xE0001)] | |
| const formatCharactersRegex = regex('g')`[\p{Cf}--\p{Emoji_Component}--[\u00AD\u200b-\u200d\u2060\u180e\u{e0001}]]++` | |
| const formatCharactersMatch = formatCharacters.join(', ').match(formatCharactersRegex) | |
| console.log('formatCharacters', formatCharacters.join(', ')) | |
| console.log('formatCharactersMatch', formatCharactersMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCo%7D&esc=on&g=gc&i= | |
| const privateUse = [String.fromCharCode(0xE000), String.fromCharCode(0xF8FF), String.fromCodePoint(0xF0FFF), String.fromCodePoint(0x100FFD), String.fromCodePoint(0x100FD)] | |
| const privateUseRegex = regex('g')`\p{Co}++` | |
| const privateUseMatch = privateUse.join(', ').match(privateUseRegex) | |
| console.log('privateUse', privateUse.join(', ')) | |
| console.log('privateUseMatch', privateUseMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCs%7D&esc=on&g=gc&i= | |
| const surrogates = [String.fromCharCode(0xD800), String.fromCharCode(0xDC40), String.fromCodePoint(0xDBFF), String.fromCodePoint(0xDC00), String.fromCodePoint(0xDFFF)] | |
| const surrogatesRegex = regex('g')`\p{Cs}++` | |
| const surrogatesMatch = surrogates.join(', ').match(surrogatesRegex) | |
| console.log('surrogates', surrogates.join(', ')) | |
| console.log('surrogatesMatch', surrogatesMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCn%7D&g=gc&i= | |
| const unassigedCodePoints = [String.fromCharCode(0x0378), String.fromCharCode(0x05CF), String.fromCodePoint(0x1127F), String.fromCodePoint(0x1E02F), String.fromCodePoint(0x10FFFF)] | |
| const unassigedCodePointsRegex = regex('g')`\p{Cn}++` | |
| const unassigedCodePointsMatch = unassigedCodePoints.join(', ').match(unassigedCodePointsRegex) | |
| console.log('unassigedCodePoints', unassigedCodePoints.join(', ')) | |
| console.log('unassigedCodePointsMatch', unassigedCodePointsMatch) | |
| // https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5Cp%7BCn%7D&g=gc&i= | |
| const misleadingWhitespace = [String.fromCharCode(0x000B), String.fromCodePoint(0x0020), String.fromCharCode(0x2028), "\t", String.fromCodePoint(0xFFA0)," ", String.fromCodePoint(0x00A0), String.fromCodePoint(0x3000)] | |
| const misleadingWhitespaceRegex = regex('g')`[[\p{White_Space}[\u115F\u1160\u3164\uFFA0]]--[\u0020\t\n\r]]++` | |
| const misleadingWhitespaceMatch = misleadingWhitespace.join(', ').match(misleadingWhitespaceRegex) | |
| console.log('misleadingWhitespace', misleadingWhitespace.join(', ')) | |
| console.log('misleadingWhitespaceMatch', misleadingWhitespaceMatch) | |
| const tagsRegex = regex('g')`[\u{e0001}\u{e007f}]+?` | |
| const variationSelectorRegex = regex('g')`[\u{e0100}-\u{e01ef}]++` | |
| const regexArray = [deprecatedRegex, controlCharRegex, formatCharactersRegex, privateUseRegex, surrogatesRegex, unassigedCodePointsRegex, misleadingWhitespaceRegex, tagsRegex, variationSelectorRegex] | |
| // console.log('hiddenInstructions', hiddenInstructions) | |
| // regexArray.forEach(regex => { | |
| // console.log('regex', regex) | |
| // const match = hiddenInstructions.match(regex) | |
| // console.log('hiddenInstructionsMatch', match) | |
| // }) | |
| // const tagsMatches = hiddenInstructions.matchAll(tagsRegex) | |
| // for (const match of tagsMatches) { | |
| // console.log( | |
| // `Found ${match[0]} start=${match.index} end=${ | |
| // match.index + match[0].length | |
| // }.`, | |
| // ); | |
| // } | |
| const tagRanges = regex('gd')`((?<tagStart>\u{e0001})[\u{e0002}-\u{e007d}]*(?<tagEnd>\u{e007f}))`; | |
| // const reStart = regex('gd')`\u{e0001}+?`; | |
| console.log('tagRanges:', tagRanges); | |
| const tags = regex('gd')`(?<tag>[\u{e0000}-\u{e007d}]+)`; | |
| console.log('tags:', tags); | |
| const str = testPillarRules + "test" + testPillarRules | |
| const matches = [...hiddenRules.matchAll(tags)] | |
| for (const match of matches) { | |
| const range = match?.indices?.groups?.tag | |
| console.log('Indices range:', match?.indices?.groups?.tag); | |
| if(range?.length) { | |
| const decode = decodeTagCharacters(hiddenRules.slice(range[0], range[1])) | |
| console.log(decode) | |
| } | |
| } | |
| // Insert at the top of this block | |
| const DEBUG = process.env.NODE_ENV === 'development'; | |
| function debugLog(...args: any[]) { | |
| if (DEBUG) { | |
| console.log(...args); | |
| } | |
| } | |
| // Replace the original console.log calls: | |
| debugLog('deprecated', deprecated) | |
| debugLog('deprecatedMatch', deprecatedMatch) | |
| debugLog('deprecatedCharMatch', deprecatedCharMatch) | |
| // …the rest of your existing code… | |
| const controlChar = String.fromCharCode(0x0001) + 'test' + '\t' | |
| // const controlRegex = regex('g')`[\u0000-\u0009\u000E-\u001F\u007F-\u0084\u0086-\u009F\u000B\u000C\u0085]++` | |
| const controlCharRegex = regex('g')`[\p{Cc}--[\t\n\r]]++` | |
| const controlCharMatch = controlChar.match(controlCharRegex) | |
| console.log('controlChar', controlChar) | |
| console.log('controlCharMatch', controlCharMatch) | |
| // …continue unchanged… |
🤖 Prompt for AI Agents
In cli/src/audit/detectSurrogates.ts between lines 289 and 383, there are many
console.log statements used for debugging that should not be present in
production code. To fix this, remove all console.log calls or wrap them in a
conditional block that checks a debug flag or environment variable, so they only
execute during development or debugging sessions. This will keep the production
output clean and improve performance.
…d option completions, write unit tests for tabtab, add github test workflow, track and add Bun rules
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: Tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| # ... | ||
| - uses: actions/checkout@v4 | ||
| - uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: "latest" | ||
| - run: bun install | ||
| - run: bun test:commander No newline at end of file |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the issue, we will add a permissions block to the workflow. Since the workflow only performs read operations (e.g., checking out the repository and installing dependencies), we will set contents: read as the minimal required permission. This ensures that the workflow has only the permissions it needs to function correctly.
The permissions block will be added at the root of the workflow, applying to all jobs in the workflow.
| @@ -6,2 +6,5 @@ | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
.github/workflows/tests.yml (1)
7-18: Add explicit permissions to the workflow for security.The workflow should explicitly define permissions for the GITHUB_TOKEN to follow security best practices. This limits the token's access to only what's necessary.
Add a permissions block to the workflow:
jobs: tests: name: Tests runs-on: ubuntu-latest + permissions: + contents: read steps:
🧹 Nitpick comments (8)
scripts/copy-markdown.ts (1)
58-70: Excellent security implementation with vulnerability detection.The vulnerability scanning logic is well-implemented with clear separation of vulnerable and safe files. The logging provides good visibility into the scanning process.
Consider enhancing the vulnerability detection logging:
- if (result?.length > 0) { - console.log(`${'Vulnerable'} ${file}`); - count++; + if (result?.length > 0) { + console.log(pc.red(`Vulnerable: ${file} (${result.length} issues)`)); + count++;.github/workflows/tests.yml (1)
18-18: Add a newline character at the end of the file.YAML files should end with a newline character for POSIX compliance and to avoid issues with tools that expect it.
- run: bun install - - run: bun test:commander + - run: bun test:commander +cli/src/cli/actions/completionActions.ts (1)
10-10: Consider moving shell detection inside the functions.Getting the shell at the module level might fail in certain environments or when environment variables change. Moving it inside each function would provide better error context and ensure fresh environment reading.
-const shell = getShellFromEnv(process.env); - export const runInstallCompletionAction = async () => { try { logger.info('Installing tab completion...'); + + const shell = getShellFromEnv(process.env); if (!shell || !isShellSupported(shell)) {Apply the same change to
runUninstallCompletionAction.cli/src/cli/cliRun.ts (2)
232-236: Fix incorrect comment.The comment says "List command" but the code is handling the "audit" command.
- // List command + // Audit command if (cmd === 'audit') {
109-128: Consider extracting completion handlers to maintain consistency.While the inline async handlers work correctly, other commands use separate action files. For consistency and testability, consider extracting these handlers to
completionActions.ts.The handlers are already defined in
completionActions.ts, so you could simplify:- .action(async (options) => { - if (options.uninstall) { - await runUninstallCompletionAction(); - } else { - await runInstallCompletionAction(); - } - }); + .action(commanderActionEndpoint);Then handle the logic in
runClisimilar to other commands.cli/src/core/commander-tabtab.ts (3)
16-16: Add type annotation for better type safety.Consider adding explicit type annotation to improve code clarity.
- const option = []; + const option: CompletionItem[] = [];
33-64: Consider a more scalable approach for option conflicts.The hardcoded conflict rules make this function difficult to maintain and extend. Consider defining conflicts in a configuration object or as metadata on the options themselves.
Example approach:
const optionConflicts = { '--verbose': ['--quiet', '-q'], '--quiet': ['--verbose'], '-q': ['--verbose'], '--install': ['--uninstall'], '-i': ['--uninstall'], '--uninstall': ['--install'], '-u': ['--install'] };
77-77: Simplify the redundant condition.The condition
option.name === prefixis redundant sincestartsWithalready covers exact matches.- (option) => option.name.startsWith(prefix) || option.name === prefix + (option) => option.name.startsWith(prefix)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc(1 hunks).github/workflows/tests.yml(1 hunks).tool-versions(0 hunks)cli/package.json(2 hunks)cli/src/cli/actions/completionActions.ts(1 hunks)cli/src/cli/cliRun.ts(5 hunks)cli/src/cli/types.ts(2 hunks)cli/src/core/__tests__/commander-tabtab.test.ts(1 hunks)cli/src/core/commander-tabtab.ts(1 hunks)package.json(1 hunks)scripts/check-awesome-cursorrules.ts(1 hunks)scripts/copy-markdown.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .tool-versions
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/check-awesome-cursorrules.ts
- cli/package.json
- package.json
🧰 Additional context used
📓 Path-based instructions (4)
`cli/src/cli/types.ts`: CLI type definitions should be placed in cli/src/cli/types.ts.
cli/src/cli/types.ts: CLI type definitions should be placed in cli/src/cli/types.ts.
cli/src/cli/types.ts
`cli/src/cli/actions/*`: Command action handlers should be implemented in cli/src/cli/actions/.
cli/src/cli/actions/*: Command action handlers should be implemented in cli/src/cli/actions/.
cli/src/cli/actions/completionActions.ts
`.cursor/rules/*.mdc`: Always place rule files in .cursor/rules/ using the .mdc extension and kebab-case filenames that are descriptive of the rule's purpose. Never place rule file...
.cursor/rules/*.mdc: Always place rule files in .cursor/rules/ using the .mdc extension and kebab-case filenames that are descriptive of the rule's purpose.
Never place rule files in subdirectories outside .cursor/rules, in the project root, or in any other location.
.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc
`cli/src/cli/cliRun.ts`: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts
🧬 Code Graph Analysis (3)
cli/src/cli/actions/completionActions.ts (2)
cli/src/shared/logger.ts (2)
logger(145-145)error(78-82)cli/src/cli/types.ts (1)
SHELL_LOCATIONS(29-34)
cli/src/core/__tests__/commander-tabtab.test.ts (2)
cli/src/cli/cliRun.ts (3)
program(56-56)setupProgram(58-131)RootProgram(36-54)cli/src/core/commander-tabtab.ts (5)
getCommands(7-12)getOptions(14-21)findCommand(81-83)filterByPrevArgs(23-70)filterByPrefix(72-79)
cli/src/core/commander-tabtab.ts (1)
cli/src/cli/cliRun.ts (1)
program(56-56)
🪛 GitHub Actions: Tests
cli/src/cli/cliRun.ts
[error] 14-14: TS2307: Cannot find module './actions/scanPathAction.js' or its corresponding type declarations.
🪛 GitHub Check: CodeQL
.github/workflows/tests.yml
[warning] 9-18: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
🪛 YAMLlint (1.37.1)
.github/workflows/tests.yml
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pixeebot[bot]
- GitHub Check: pixeebot[bot]
🔇 Additional comments (20)
scripts/copy-markdown.ts (6)
1-5: LGTM! Clean import organization.The new imports are well-organized and support the enhanced functionality - vulnerability detection, shell commands, and colored output.
16-24: Good optimization: Creating directories upfront.Moving directory creation to the beginning improves code organization and ensures both target directories exist before any operations.
41-45: Good error handling for external dependency.The wget download with proper error handling and user-friendly warning message is well implemented. The
.quiet()prevents unnecessary output clutter.
93-95: Consistent refactoring to Bun API.The repomix instructions copying follows the same pattern as the rules copying, maintaining consistency in the codebase.
48-53: Verify the relative path assumption for awesome-cursorrules.The path
../awesome-cursorrulesassumes a specific directory structure. Ensure this works correctly in different execution contexts (CI, local development, different working directories).#!/bin/bash # Description: Verify the awesome-cursorrules directory structure and location # Expected: Confirm the directory exists and contains the expected rules-new subdirectory # Check if awesome-cursorrules exists as a sibling directory if [ -d "../awesome-cursorrules" ]; then echo "✓ awesome-cursorrules directory found" ls -la ../awesome-cursorrules/ if [ -d "../awesome-cursorrules/rules-new" ]; then echo "✓ rules-new subdirectory found" echo "File count in rules-new:" find ../awesome-cursorrules/rules-new -type f | wc -l else echo "✗ rules-new subdirectory not found" fi else echo "✗ awesome-cursorrules directory not found" fi # Check if this is configured as a git submodule git submodule status | grep awesome-cursorrules || echo "Not configured as git submodule"
35-39: We still need to locate whererules-defaultactually lives in the repo before we can size its contents. Let’s search for that directory name:#!/bin/bash # Find any directory named "rules-default" fd -t d rules-default # If found, list all files under it with sizes for dir in $(fd -t d rules-default); do echo "Directory: $dir" find "$dir" -type f -exec wc -c {} + | sort -n echo "Total files: $(find "$dir" -type f | wc -l)" done.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc (8)
1-5: Perfect compliance with coding guidelines.The file is correctly placed in
.cursor/rules/with the.mdcextension and uses a descriptive kebab-case filename as required by the coding guidelines.
7-15: Comprehensive and accurate CLI command mappings.The command replacements are accurate and cover the most common Node.js ecosystem tools. The automatic
.envloading note is particularly valuable.
17-25: Excellent API alternatives with specific recommendations.The API section provides clear alternatives to popular Node.js libraries, which will help developers make informed choices when migrating to Bun.
30-36: Syntactically correct test example.The Bun test example follows the correct syntax and imports, providing a clear migration path from other testing frameworks.
44-73: Comprehensive server example with modern features.The
Bun.serve()example is excellent, showcasing routes, WebSocket support, and development features. The structure follows Bun's current API correctly.
77-84: Clean HTML integration example.The HTML file correctly demonstrates how to import TypeScript/JSX files directly, which is a key Bun feature for frontend development.
88-103: Accurate React integration with CSS imports.The frontend example correctly shows React setup with direct CSS imports, demonstrating Bun's bundling capabilities effectively.
107-111: Helpful execution and documentation references.The execution command with
--hotflag and reference to local documentation are practical additions for developers.cli/src/cli/types.ts (1)
1-35: Well-structured type definitions and shell configuration mapping.The additions properly extend the CLI options interface with scan-related options and provide a comprehensive mapping of supported shells to their configuration files. This follows the project's convention of placing CLI type definitions in this file.
cli/src/cli/actions/completionActions.ts (1)
12-33: LGTM! Well-structured completion installation handler.The function properly validates shell support, handles errors gracefully, and provides clear user instructions. The implementation correctly follows the project's convention for action handlers.
cli/src/cli/cliRun.ts (2)
36-54: Well-designed command extension pattern.The
RootProgramclass elegantly extends Commander'sCommandto automatically add common options (verbose/quiet) with conflict handling to all subcommands. This promotes consistency and reduces code duplication.
13-14: ```shell
#!/bin/bashExtract lines 230-260 from cliRun.ts to verify the comment around line 238
sed -n '230,260p' cli/src/cli/cliRun.ts
</details> <details> <summary>cli/src/core/commander-tabtab.ts (2)</summary> `7-12`: **LGTM!** Clean implementation for extracting command metadata. --- `81-83`: **LGTM!** Simple and effective utility function. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| it('should return empty array when no options match prefix', () => { | ||
| const options = [ | ||
| { name: '--help', description: 'show help' }, | ||
| { name: '-h', description: 'short help' }, | ||
| ]; | ||
|
|
||
| const filtered = filterByPrefix(options, '--v'); | ||
| expect(filtered).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should filter command names by prefix', () => { | ||
| const commands = getCommands(program); | ||
|
|
||
| const filtered = filterByPrefix(commands, 'a'); | ||
| expect(filtered).toHaveLength(1); | ||
| expect(filtered[0].name).toBe('audit'); | ||
| }); | ||
|
|
||
| it('should filter all commands with specific prefixes', () => { | ||
| const commands = getCommands(program); | ||
|
|
||
| // Test filtering with 's' prefix | ||
| const sCommands = filterByPrefix(commands, 's'); | ||
| expect(sCommands).toHaveLength(1); | ||
| expect(sCommands[0].name).toBe('scan'); | ||
|
|
||
| // Test filtering with 'c' prefix | ||
| const cCommands = filterByPrefix(commands, 'c'); | ||
| expect(cCommands).toHaveLength(1); | ||
| expect(cCommands[0].name).toBe('completion'); | ||
|
|
||
| // Test filtering with 'r' prefix | ||
| const rCommands = filterByPrefix(commands, 'r'); | ||
| expect(rCommands).toHaveLength(1); | ||
| expect(rCommands[0].name).toBe('repomix'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fix duplicated and misplaced test code.
The test "should return empty array when no options match prefix" (lines 354-362) is testing filterByPrefix but is placed inside the filterByPrevArgs describe block. Additionally, this exact test is duplicated later (lines 413-421).
Remove the misplaced test from the filterByPrevArgs block:
- it('should return empty array when no options match prefix', () => {
- const options = [
- { name: '--help', description: 'show help' },
- { name: '-h', description: 'short help' },
- ];
-
- const filtered = filterByPrefix(options, '--v');
- expect(filtered).toHaveLength(0);
- });
-
- it('should filter command names by prefix', () => {The remaining tests (364-389) for filtering commands are also testing filterByPrefix functionality and should be moved to the filterByPrefix describe block.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cli/src/core/__tests__/commander-tabtab.test.ts between lines 354 and 390,
the test "should return empty array when no options match prefix" is misplaced
inside the filterByPrevArgs describe block and is duplicated later in the file.
Remove this duplicated test from lines 354-362. Also, move the subsequent tests
from lines 364-389 that test filtering commands by prefix out of the
filterByPrevArgs block and place them inside the filterByPrefix describe block
to correctly group related tests.
| @@ -0,0 +1,450 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
Remove @ts-nocheck directive.
The @ts-nocheck directive disables all TypeScript type checking for this file. This can hide potential type errors and reduce code quality. Consider fixing any type issues instead of disabling checks entirely.
-// @ts-nocheck
import { describe, it, expect, beforeEach } from 'bun:test';📝 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.
| // @ts-nocheck | |
| import { describe, it, expect, beforeEach } from 'bun:test'; |
🤖 Prompt for AI Agents
In cli/src/core/__tests__/commander-tabtab.test.ts at line 1, remove the
@ts-nocheck directive to re-enable TypeScript type checking. Then, identify and
fix any type errors present in the file to ensure proper type safety without
disabling checks globally.
| .getCompletionScript({ | ||
| name: binName, | ||
| completer: binName, | ||
| shell, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move shell detection to avoid module-level side effects.
The shell variable is used here but defined at module level. Move the shell detection inside this function.
Add at the beginning of the function:
const firstArg = process.argv.slice(2)[0];
const prevFlags = process.argv.filter((arg) => arg.startsWith('-'));
+ const shell = getShellFromEnv(process.env);And remove the module-level declaration on line 4.
Also applies to: 117-117, 133-133
🤖 Prompt for AI Agents
In cli/src/core/commander-tabtab.ts at lines 99, 117, and 133, the shell
variable is currently defined at the module level causing side effects. Move the
shell detection logic inside each function where shell is used by declaring and
initializing shell at the start of those functions. Then remove the module-level
shell declaration at line 4 to avoid global side effects.
| import tabtab, { CompletionItem, getShellFromEnv } from '@pnpm/tabtab'; | ||
| import { Command, Option } from 'commander'; | ||
|
|
||
| const shell = getShellFromEnv(process.env); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move shell detection inside the function to avoid module-level side effects.
Detecting the shell at module load time can cause issues in testing environments or when process.env is not properly initialized. Consider moving this inside the commanderTabtab function where it's actually used.
🤖 Prompt for AI Agents
In cli/src/core/commander-tabtab.ts at line 4, the shell detection using
getShellFromEnv(process.env) is done at the module level, causing side effects
during module load. Move this shell detection inside the commanderTabtab
function so that it runs only when the function is called, preventing issues in
testing or environments where process.env is not fully initialized.
| const completion = await tabtab | ||
| .getCompletionScript({ | ||
| name: binName, | ||
| completer: binName, | ||
| shell, | ||
| }) | ||
| .catch((err) => console.error('GENERATE ERROR', err)); | ||
| console.log(completion); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for completion script generation.
The current implementation swallows errors and only logs them. Consider propagating the error or returning a meaningful error state.
- const completion = await tabtab
- .getCompletionScript({
- name: binName,
- completer: binName,
- shell,
- })
- .catch((err) => console.error('GENERATE ERROR', err));
- console.log(completion);
+ try {
+ const completion = await tabtab.getCompletionScript({
+ name: binName,
+ completer: binName,
+ shell,
+ });
+ console.log(completion);
+ } catch (err) {
+ console.error('Failed to generate completion script:', err);
+ process.exit(1);
+ }📝 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.
| const completion = await tabtab | |
| .getCompletionScript({ | |
| name: binName, | |
| completer: binName, | |
| shell, | |
| }) | |
| .catch((err) => console.error('GENERATE ERROR', err)); | |
| console.log(completion); | |
| return true; | |
| } | |
| try { | |
| const completion = await tabtab.getCompletionScript({ | |
| name: binName, | |
| completer: binName, | |
| shell, | |
| }); | |
| console.log(completion); | |
| } catch (err) { | |
| console.error('Failed to generate completion script:', err); | |
| process.exit(1); | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In cli/src/core/commander-tabtab.ts around lines 95 to 104, the error handling
for generating the completion script only logs the error and does not propagate
it or return an error state. Modify the code to either throw the caught error
after logging it or return a meaningful error indicator so that the caller can
handle the failure appropriately instead of silently continuing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cli/src/core/__tests__/commander-tabtab.test.ts (2)
1-1: Remove @ts-nocheck directive.The
@ts-nocheckdirective disables all TypeScript type checking for this file. This can hide potential type errors and reduce code quality. Consider fixing any type issues instead of disabling checks entirely.
339-375: Fix duplicated and misplaced test code.The tests from lines 339-375 are testing
filterByPrefixfunctionality but are incorrectly placed inside thefilterByPrevArgsdescribe block. These same tests are duplicated in the properfilterByPrefixblock (lines 398-433).Move these tests to the correct
filterByPrefixdescribe block and remove the duplicates to maintain proper test organization.
🧹 Nitpick comments (2)
cli/src/cli/cliRun.ts (2)
213-218: Remove redundant validation for required option.The
pathoption is defined asrequiredOptionin thescancommand setup (line 96), so Commander.js will automatically validate its presence and show help if it's missing. This manual validation is redundant.- if (!options.path) { - logger.error('Path argument is required for scan command'); - command.outputHelp(); - return; - }
228-232: Fix incorrect comment.The comment says "List command" but this code block handles the
auditcommand.- // List command + // Audit command
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli/src/cli/cliRun.ts(5 hunks)cli/src/core/__tests__/commander-tabtab.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`cli/src/cli/cliRun.ts`: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts
🧬 Code Graph Analysis (1)
cli/src/core/__tests__/commander-tabtab.test.ts (2)
cli/src/cli/cliRun.ts (3)
program(56-56)setupProgram(58-129)RootProgram(36-54)cli/src/core/commander-tabtab.ts (5)
getCommands(7-12)getOptions(14-21)findCommand(81-83)filterByPrevArgs(23-70)filterByPrefix(72-79)
🪛 GitHub Actions: Tests
cli/src/cli/cliRun.ts
[error] 14-14: TypeScript error TS2307: Cannot find module './actions/scanPathAction.js' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pixeebot[bot]
- GitHub Check: pixeebot[bot]
🔇 Additional comments (2)
cli/src/cli/cliRun.ts (2)
36-54: LGTM! Well-structured class design.The
RootProgramclass extension provides a clean way to add global options with proper conflict handling between--verboseand--quietoptions.
107-126: LGTM! Proper completion command implementation.The completion command properly handles mutually exclusive install/uninstall options with appropriate conflict configuration and async action handlers.
| import { runListRulesAction } from '~/cli/actions/listRulesAction.js'; | ||
| import { checkForUpdates } from '~/core/checkForUpdates.js'; | ||
| import { runAuditRulesAction } from '~/cli/actions/auditRulesAction.js'; | ||
| import { runScanPathAction } from './actions/scanPathAction.js'; |
There was a problem hiding this comment.
Fix the missing module import.
The pipeline is failing because the module './actions/scanPathAction.js' cannot be found. This will cause a runtime error when the application starts.
Ensure the scanPathAction.js file exists in the cli/src/cli/actions/ directory, or update the import path to point to the correct location.
🧰 Tools
🪛 GitHub Actions: Tests
[error] 14-14: TypeScript error TS2307: Cannot find module './actions/scanPathAction.js' or its corresponding type declarations.
🤖 Prompt for AI Agents
In cli/src/cli/cliRun.ts at line 14, the import statement for
'./actions/scanPathAction.js' is causing a runtime error because the module
cannot be found. Verify that the file scanPathAction.js exists in the
cli/src/cli/actions/ directory; if it does not, either create the file or
correct the import path to point to the actual location of the module to resolve
the missing import issue.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cli/src/cli/cliRun.ts (1)
228-228: Fix incorrect comment.The comment says "List command" but this section handles the audit command.
- // List command + // Audit command
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
cli/package.json(2 hunks)cli/src/cli/cliRun.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/package.json
🧰 Additional context used
📓 Path-based instructions (1)
`cli/src/cli/cliRun.ts`: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts: The CLI runner functionality should be implemented in cli/src/cli/cliRun.ts.
cli/src/cli/cliRun.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pixeebot[bot]
- GitHub Check: pixeebot[bot]
🔇 Additional comments (3)
cli/src/cli/cliRun.ts (3)
14-14: The commented import addresses the previous pipeline failure.The import for
scanPathAction.jsis now properly commented out, which resolves the previous TypeScript error about the missing module.
58-129: Well-structured modularization of command setup.The
setupProgramfunction properly separates command registration from the main run logic, making the code more maintainable and testable. The command definitions are clear and follow Commander.js best practices.
139-143: Shell completion integration is properly implemented.The integration with
commanderTabtabis handled correctly with early return to prevent further argument parsing when completion is active.
| programInstance | ||
| .command('repomix') | ||
| .description('generate repomix output with recommended settings') | ||
| .action(commanderActionEndpoint); | ||
|
|
||
| // program | ||
| // .command('mcp') | ||
| // .description('run as a MCP server') | ||
| // .action(runCli); | ||
| programInstance | ||
| .command('scan') | ||
| .description('scan and check all files in the specified path') | ||
| .requiredOption('-p, --path <path>', 'path to scan') | ||
| .option( | ||
| '-i, --include-pattern <pattern>', | ||
| 'regex pattern for files to include' | ||
| ) | ||
| .option( | ||
| '-e, --exclude-pattern <pattern>', | ||
| 'regex pattern for files to exclude' | ||
| ) | ||
| .action(commanderActionEndpoint); |
There was a problem hiding this comment.
Scan command registration without implementation creates inconsistent user experience.
The scan command is registered with required options but its implementation is commented out in runCli (lines 212-226). This means users can see the command in help output and attempt to use it, but it will fall through to the default help display rather than executing scan logic or providing appropriate feedback about the unimplemented feature.
Consider one of these approaches:
# Option 1: Comment out the entire scan command registration
- programInstance
- .command('scan')
- .description('scan and check all files in the specified path')
- .requiredOption('-p, --path <path>', 'path to scan')
- .option(
- '-i, --include-pattern <pattern>',
- 'regex pattern for files to include'
- )
- .option(
- '-e, --exclude-pattern <pattern>',
- 'regex pattern for files to exclude'
- )
- .action(commanderActionEndpoint);# Option 2: Add a temporary implementation that notifies users
.action(async () => {
logger.error('Scan command is not yet implemented');
process.exit(1);
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cli/src/cli/cliRun.ts between lines 88 and 105, the 'scan' command is
registered with required options but its implementation is missing, causing
inconsistent user experience. To fix this, either implement the scan command
logic in the runCli function where it is currently commented out (lines 212-226)
or remove the 'scan' command registration entirely until the feature is ready.
This ensures users either get the expected functionality or do not see an
unusable command.
2 participants
Summary by CodeRabbit
New Features
auditcommand to the CLI for auditing existing rules for vulnerabilities.Bug Fixes
Documentation
Chores
Style/Refactor