-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: send user request as separate content block for slash command support #785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…upport When in tag mode with the SDK path, extracts the user's request from the trigger comment (text after @claude) and sends it as a separate content block. This enables the CLI to process slash commands like "/review-pr". - Add extract-user-request utility to parse trigger comments - Write user request to separate file during prompt generation - Send multi-block SDKUserMessage when user request file exists - Add tests for the extraction utility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
base-action/src/run-claude-sdk.ts
Outdated
| const promptContent = await readFile(promptPath, "utf-8"); | ||
|
|
||
| // Check for user request file in the same directory | ||
| const userRequestPath = join(dirname(promptPath), "claude-user-request.txt"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Quality: Magic string should be a constant
The filename "claude-user-request.txt" is hardcoded here and in src/create-prompt/index.ts:945. Define a shared constant:
export const USER_REQUEST_FILENAME = "claude-user-request.txt";
base-action/src/run-claude-sdk.ts
Outdated
|
|
||
| // User request file exists - create multi-block message | ||
| const userRequest = await readFile(userRequestPath, "utf-8"); | ||
| console.log("Using multi-block message with user request:", userRequest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Sensitive data logging
User requests may contain sensitive information. Consider conditional logging based on showFullOutput to match the pattern used elsewhere in the file (line 125-129).
Code Review SummaryI've completed a comprehensive review of PR #785 using specialized analysis agents. Overall, the implementation is well-structured and adds valuable slash command support, but there are several critical issues that need attention. Critical Issues
High Priority Issues
Positive Observations✅ Proper regex escaping prevents injection attacks Performance AssessmentThe changes add minimal overhead (2-5ms) and follow best practices for file I/O. No performance concerns identified. RecommendationsMust Fix:
Should Fix:
See inline comments for detailed recommendations and code examples. |
- Fix potential ReDoS vulnerability by using string operations instead of regex - Remove unused extractUserRequestFromEvent function and tests - Extract USER_REQUEST_FILENAME to shared constants - Conditionally log user request based on showFullOutput setting - Add JSDoc documentation to extractUserRequestFromContext
When in tag mode with the SDK path, extracts the user's request from the trigger comment (text after @claude) and sends it as a separate content block. This enables the CLI to process slash commands like "/review-pr".
Changes