diff --git a/.github/workflows/codepress-review.yml b/.github/workflows/codepress-review.yml index aab8359..7eeed5d 100644 --- a/.github/workflows/codepress-review.yml +++ b/.github/workflows/codepress-review.yml @@ -5,6 +5,8 @@ on: types: [opened, reopened, review_requested, synchronize] issue_comment: types: [created] + pull_request_review_comment: + types: [created] workflow_dispatch: # Allow manual triggering permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b0f8b66..0e202e4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,10 +15,15 @@ on: - minor - major prerelease: - description: "Mark as pre-release" + description: "Mark as pre-release (beta)" required: false type: boolean default: false + branch: + description: "Branch to release from (for pre-releases)" + required: false + type: string + default: "main" permissions: contents: write @@ -34,6 +39,7 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 # Need full history for tags and commit analysis + ref: ${{ github.event.inputs.branch || 'main' }} - name: Install pnpm uses: pnpm/action-setup@v4 @@ -114,6 +120,7 @@ jobs: MINOR=${{ steps.get_version.outputs.minor }} PATCH=${{ steps.get_version.outputs.patch }} BUMP_TYPE=${{ steps.version_type.outputs.bump_type }} + IS_PRERELEASE=${{ github.event.inputs.prerelease }} case $BUMP_TYPE in major) @@ -130,24 +137,43 @@ jobs: ;; esac - NEW_VERSION="v$MAJOR.$MINOR.$PATCH" - echo "New version: $NEW_VERSION" + if [ "$IS_PRERELEASE" = "true" ]; then + # Find the highest existing beta number for this version + BASE_VERSION="v$MAJOR.$MINOR.$PATCH" + EXISTING_BETAS=$(git tag -l "${BASE_VERSION}-beta.*" | grep -oE 'beta\.[0-9]+' | grep -oE '[0-9]+' | sort -n | tail -1) + + if [ -z "$EXISTING_BETAS" ]; then + BETA_NUMBER=1 + else + BETA_NUMBER=$((EXISTING_BETAS + 1)) + fi + + NEW_VERSION="${BASE_VERSION}-beta.${BETA_NUMBER}" + echo "Creating beta version: $NEW_VERSION (auto-incremented)" + else + NEW_VERSION="v$MAJOR.$MINOR.$PATCH" + echo "Creating release version: $NEW_VERSION" + fi + echo "new_version=$NEW_VERSION" >> $GITHUB_OUTPUT echo "major_version=v$MAJOR" >> $GITHUB_OUTPUT + echo "is_prerelease=$IS_PRERELEASE" >> $GITHUB_OUTPUT - name: Configure git run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" - - name: Update package.json version + - name: Update package.json version (stable releases only) + if: steps.new_version.outputs.is_prerelease != 'true' run: | NEW_VERSION=${{ steps.new_version.outputs.new_version }} # Remove 'v' prefix for package.json VERSION_NUMBER=${NEW_VERSION#v} npm version $VERSION_NUMBER --no-git-tag-version - - name: Commit and push release changes + - name: Commit and push release changes (stable releases only) + if: steps.new_version.outputs.is_prerelease != 'true' run: | git add package.json git commit -m "Bump version to ${{ steps.new_version.outputs.new_version }}" || echo "No changes to commit" @@ -157,6 +183,13 @@ jobs: git add -f dist/ git commit -m "Build dist for ${{ steps.new_version.outputs.new_version }}" || echo "No changes to commit" + - name: Prepare prerelease build (beta releases only) + if: steps.new_version.outputs.is_prerelease == 'true' + run: | + # For prereleases, just add dist/ without pushing to main + git add -f dist/ + git commit -m "Build dist for ${{ steps.new_version.outputs.new_version }}" || echo "No changes to commit" + - name: Create and push tag run: | NEW_VERSION=${{ steps.new_version.outputs.new_version }} @@ -167,7 +200,8 @@ jobs: echo "Tag $NEW_VERSION created and pushed" - - name: Update major version tag + - name: Update major version tag (stable releases only) + if: steps.new_version.outputs.is_prerelease != 'true' run: | MAJOR_VERSION=${{ steps.new_version.outputs.major_version }} echo "Updating major version tag $MAJOR_VERSION..." @@ -210,10 +244,26 @@ jobs: - name: Summary run: | - echo "## Release Created!" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Version:** ${{ steps.new_version.outputs.new_version }}" >> $GITHUB_STEP_SUMMARY - echo "**Type:** ${{ steps.version_type.outputs.bump_type }}" >> $GITHUB_STEP_SUMMARY - echo "**Major Tag:** ${{ steps.new_version.outputs.major_version }}" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "Users can now use \`quantfive/codepress-review@${{ steps.new_version.outputs.major_version }}\` or \`quantfive/codepress-review@${{ steps.new_version.outputs.new_version }}\`" >> $GITHUB_STEP_SUMMARY + IS_PRERELEASE=${{ steps.new_version.outputs.is_prerelease }} + + if [ "$IS_PRERELEASE" = "true" ]; then + echo "## ๐Ÿงช Beta Release Created!" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Version:** ${{ steps.new_version.outputs.new_version }}" >> $GITHUB_STEP_SUMMARY + echo "**Type:** ${{ steps.version_type.outputs.bump_type }} (prerelease)" >> $GITHUB_STEP_SUMMARY + echo "**Branch:** ${{ github.event.inputs.branch || 'main' }}" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "### Testing this beta" >> $GITHUB_STEP_SUMMARY + echo "Use in your workflow:" >> $GITHUB_STEP_SUMMARY + echo "\`\`\`yaml" >> $GITHUB_STEP_SUMMARY + echo "uses: quantfive/codepress-review@${{ steps.new_version.outputs.new_version }}" >> $GITHUB_STEP_SUMMARY + echo "\`\`\`" >> $GITHUB_STEP_SUMMARY + else + echo "## ๐Ÿš€ Release Created!" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Version:** ${{ steps.new_version.outputs.new_version }}" >> $GITHUB_STEP_SUMMARY + echo "**Type:** ${{ steps.version_type.outputs.bump_type }}" >> $GITHUB_STEP_SUMMARY + echo "**Major Tag:** ${{ steps.new_version.outputs.major_version }}" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "Users can now use \`quantfive/codepress-review@${{ steps.new_version.outputs.major_version }}\` or \`quantfive/codepress-review@${{ steps.new_version.outputs.new_version }}\`" >> $GITHUB_STEP_SUMMARY + fi diff --git a/src/agent/agent-system-prompt.ts b/src/agent/agent-system-prompt.ts index 10744f6..56c3fb9 100644 --- a/src/agent/agent-system-prompt.ts +++ b/src/agent/agent-system-prompt.ts @@ -1,658 +1,56 @@ -import { existsSync, readFileSync } from "fs"; -import { join } from "path"; - -const DEFAULT_REVIEW_GUIDELINES = ` - - - - Your job is to find bugs that WILL break the application. - Not theoretical issues. Not style preferences. Not minor improvements. - - **Focus on logical errors that will cause failures in production.** - - Approve code that works correctly, even if it's not perfect. - Request changes only for issues that will actually break things. - - - - **NEVER comment on code you don't fully understand.** - - Before forming ANY opinion: - 1. Read the FULL file(s), not just the diff - 2. Understand what the code is trying to do - 3. Check how the code is used (callers, consumers) - 4. Read related tests - 5. Check related repos if needed for API contracts - - **If you don't understand it, investigate more. Don't guess.** - - - - **Every claim must have evidence from your investigation.** - - Bad: "This might cause a null pointer exception" - Good: "This causes a null pointer when: (1) \`getUser()\` returns null (verified: - line 45 of caller.ts passes result without checking), (2) \`user.id\` is - accessed on line 23. Evidence: \`rg 'getUser' src/\` shows 3 callers, - 2 don't null-check." +/** + * Minimal system prompt for the CodePress review agent. + * + * The agent uses the `skill` tool to load detailed instructions for specific tasks. + * This keeps the base system prompt small and allows for modular, context-aware instructions. + */ +export function getSystemPrompt(): string { + return `You are **CodePress**, an AI code review assistant for GitHub Pull Requests. - **If you can't provide specific evidence, don't post the comment.** - +## Your Capabilities +- Review code changes in PRs +- Post inline comments on specific lines +- Answer questions about code +- Submit formal PR reviews (approve, request changes, comment) - - **Hunt for errors that will ACTUALLY break things:** +## How to Get Started - - Code that will crash/throw in certain conditions - - Logic that produces wrong results - - Race conditions that corrupt state - - API contracts being violated - - Error paths that don't handle cleanup - - Breaking changes to function signatures that affect callers - - Security vulnerabilities (injection, XSS, auth bypass) - - Resource leaks (unclosed handles, memory leaks) +Use the \`skill\` tool to load specialized instructions for your task. +The skill tool description lists all available skills and when to use each one. - **Don't flag:** - - Style preferences (unless causing bugs) - - Theoretical "what ifs" you can't demonstrate - - Minor improvements that don't affect correctness - - "Could be cleaner" when the current code works - +**Example:** +- For a full PR review: \`skill({ name: "review-full" })\` +- For answering a question: \`skill({ name: "answer-question" })\` +- For targeted review: \`skill({ name: "review-targeted" })\` - - Rate your confidence before posting: - - 10: Proven with evidence, will definitely break - - 8-9: Strong evidence, highly likely to cause issues - - Below 8: Investigate more or don't post +After loading a skill, follow its instructions completely to accomplish the task. - **Only post findings where confidence >= 8.** - +## Completion - - CRITICAL: You only see partial file context in diffs. Imports, type definitions, - and other dependencies may exist outside the visible lines. You have TOOLS to - fetch additional context - USE THEM before making claims about missing imports, - unused variables, or dead code. - +Your task loop continues until you produce a structured output matching this schema: +\`\`\`json +{ + "completed": true, + "summary": "Brief summary of what you did", + "commentsPosted": N, + "verdict": "APPROVE" | "REQUEST_CHANGES" | "COMMENT" | "NONE" +} +\`\`\` - - Lock files (package-lock.json, pnpm-lock.yaml, yarn.lock, etc.) are filtered - from reviews. Do NOT warn about missing lock file updates - assume they exist. - - -`; +Only output this JSON when you have fully completed the task.`; +} /** - * Builds the interactive system prompt with review guidelines and tool capabilities. - * - * Supports two customization files: - * - `custom-codepress-review-prompt.md`: Replaces the entire default guidelines - * - `codepress-review-rules.md`: Appends additional rules to the guidelines (takes precedence on conflicts) - * - * @param blockingOnly If true, instructs the LLM to only generate "required" severity comments - * @param maxTurns Maximum number of turns the agent has to complete the review (null = unlimited) - * @returns Complete system prompt with tools and response format + * For backward compatibility: returns the minimal system prompt. + * @deprecated Use getSystemPrompt() instead. For full review instructions, use the review-full skill. */ export function getInteractiveSystemPrompt( - blockingOnly: boolean = false, - maxTurns: number | null, + _blockingOnly: boolean = false, + _maxTurns: number | null, ): string { - // Check for custom prompt file - const customPromptPath = join( - process.cwd(), - "custom-codepress-review-prompt.md", - ); - let reviewGuidelines = DEFAULT_REVIEW_GUIDELINES; - - if (existsSync(customPromptPath)) { - try { - reviewGuidelines = readFileSync(customPromptPath, "utf8"); - } catch (error) { - console.warn(`Failed to read custom prompt file: ${error}`); - // Fall back to default guidelines - } - } - - // Check for additional rules file (additive, does not replace defaults) - const additionalRulesPath = join( - process.cwd(), - "codepress-review-rules.md", - ); - - if (existsSync(additionalRulesPath)) { - try { - const additionalRules = readFileSync(additionalRulesPath, "utf8"); - reviewGuidelines += ` - - - - The following rules are specific to this project. - **When these rules conflict with the default guidelines above, these project-specific rules take precedence.** - - ${additionalRules} - `; - } catch (error) { - console.warn(`Failed to read additional rules file: ${error}`); - } - } - - // Start building the prompt - let prompt = ` -`; - - // Add blocking mode header if needed - if (blockingOnly) { - prompt += ` - - - - IMPORTANT: You are operating in BLOCKING-ONLY MODE. - - This means you should ONLY post review comments for issues that are - ABSOLUTELY CRITICAL and MUST be fixed before the PR can be merged. - - DO NOT comment on: - โ€ข Nice-to-have improvements - โ€ข Minor style or polish issues - โ€ข Informational notes - โ€ข Praise - - ONLY comment on: - โ€ข Security vulnerabilities - โ€ข Bugs that would break functionality - โ€ข Critical performance issues - โ€ข Code that violates fundamental architectural principles - โ€ข Breaking changes or API contract violations - - If there are NO blocking issues, simply complete the review without posting any comments. - `; - } - - // Add autonomous capabilities section - prompt += ` - - - - You are an **autonomous code-review agent** with full control over the review process. - You can read PR information, check existing comments, post new comments, and update the PR description. - - **You MUST use the bash tool to execute gh CLI commands to post comments.** - Your text responses should only contain brief status updates and summaries. - - - - ๐Ÿšจ **IMPORTANT: The review loop continues until you produce a structured completion output.** - - You can output text and use tools freely during the review. The loop will NOT terminate - until you explicitly signal completion by outputting a JSON object with this EXACT structure: - - \`\`\`json - { - "completed": true, - "summary": "Brief summary of what was reviewed and found", - "commentsPosted": 5, - "verdict": "APPROVE" - } - \`\`\` - - **Rules:** - - \`completed\` MUST be \`true\` to terminate - the schema requires exactly \`true\` - - Only output this JSON AFTER you have: - 1. Reviewed ALL files in the PR (check your todo list) - 2. Posted all necessary comments via \`gh api\` - 3. Submitted the formal review via \`gh pr review\` - - \`verdict\` must be one of: "APPROVE", "REQUEST_CHANGES", "COMMENT", "NONE" - - Use "NONE" only if you couldn't submit a review (e.g., re-review with no new issues) - - **DO NOT output this JSON until you are truly done with the entire review.** - If you output JSON with \`completed: false\` or any other text, the loop continues. - - - - - **SKIP these files - do NOT review or read them:** - - โ€ข **Lock files:** \`*.lock\`, \`package-lock.json\`, \`pnpm-lock.yaml\`, \`yarn.lock\`, \`Gemfile.lock\`, \`Cargo.lock\`, \`go.sum\`, \`poetry.lock\`, \`composer.lock\` - โ€ข **Build outputs:** \`dist/\`, \`build/\`, \`out/\`, \`target/\`, \`.next/\`, \`coverage/\`, \`*.min.js\`, \`*.min.css\`, \`*.bundle.js\`, \`*.chunk.js\` - โ€ข **Generated/bundled files:** Files ending in \`.cjs\` or \`.mjs\` in \`dist/\` or \`build/\` directories - โ€ข **Dependencies:** \`node_modules/\`, \`vendor/\`, \`venv/\`, \`.venv/\` - โ€ข **Cache/temp:** \`.cache/\`, \`*.tmp\`, \`*.log\` - โ€ข **Binary/compiled:** \`*.pyc\`, \`*.class\`, \`*.dll\`, \`*.exe\`, \`*.so\`, \`*.dylib\` - โ€ข **IDE config:** \`.vscode/\`, \`.idea/\` - - When you get the file list, mentally filter out these patterns and only add meaningful source files to your todo list. - These files are auto-generated, not human-authored, and reviewing them wastes turns without value. - - - - - ๐Ÿšจ **CRITICAL: Determine scope BEFORE creating todos** ๐Ÿšจ - - **Step 1: Check if this is a re-review** - Look at - if you have previous reviews on this PR, this is a re-review. - Also check if a previous commit SHA is provided in the context. - - **Step 2: Get the appropriate file list based on scope** - - **First-time review:** \`gh pr view --json files\` โ†’ all changed files - - **Re-review:** Get files changed since your last review (use SHA from \`\`): - 1. Try: \`git diff .. --name-only\` - 2. Fallback: \`gh api repos/OWNER/REPO/compare/... --jq '.files[].filename'\` - - **Re-review (requested changes):** Also include files where you left feedback - - **Step 3: Create todos ONLY for files in your scope** - Filter out lock files, build outputs, generated files (see \`\`). - โ›” Do NOT add all PR files to todos if this is a re-review - only add scoped files. - - **Step 4: Review each file** - - Read the FULL file (not just the patch) for context - - Post comments IMMEDIATELY when you find issues - - Mark the file as done in your todo list - - **Step 5: Complete ALL todos before submitting** - - **Impact analysis (no todos needed):** - Use \`rg\` to check if changes affect other files (imports, APIs, shared types). - Only add a todo if another file needs detailed review. - - - - - ${ - maxTurns !== null - ? `You have a maximum of **${maxTurns} turns** to complete this review. - Each tool call and each response counts as a turn. - Budget your turns wisely: - โ€ข Use early turns for critical context gathering - โ€ข Reserve later turns for posting comments and finalizing - โ€ข If running low on turns, focus on completing todos and submitting the review` - : `You have **unlimited turns** to complete this review. - Take the time you need to be thorough, but be efficient. - Don't waste turns on unnecessary exploration.` - } - โ€ข NEVER end without completing your todo list and submitting a formal review - - - - - - - Run any bash command. Key uses for code review: - - **GitHub CLI (gh) - Your primary tool for PR operations:** - โ€ข **Get PR info and file list:** \`gh pr view --json title,body,files\` - โ€ข **Get a specific file's patch:** \`gh api repos/OWNER/REPO/pulls/PR_NUMBER/files --jq '.[] | select(.filename=="path/to/file.ts")'\` - โ€ข **Fetch full PR diff:** \`gh pr diff \` (use for small PRs) - โ€ข Get PR review comments: \`gh api repos/OWNER/REPO/pulls/PR_NUMBER/comments\` - โ€ข Get PR conversation comments: \`gh api repos/OWNER/REPO/issues/PR_NUMBER/comments\` - โ€ข Post inline comment: \`gh api repos/OWNER/REPO/pulls/PR_NUMBER/comments -f body="..." -f path="file.ts" -f line=N -f commit_id="SHA"\` - โ€ข Update PR description: \`gh pr edit --body $'## Summary\\n\\n...'\` (use \`$'...'\` for newlines) - โ€ข Submit formal review (REQUIRED at end): - - Approve: \`gh pr review --approve --body "Summary"\` - - Request changes: \`gh pr review --request-changes --body "Summary"\` - - Comment only: \`gh pr review --comment --body "Summary"\` - - **Code exploration:** - โ€ข Read files: \`cat\`, \`head\`, \`tail\` - โ€ข Search code: \`rg\` (ripgrep), \`grep\` - โ€ข Git commands: \`git log\`, \`git blame\`, \`git show\` - - Commands have a 30-second timeout and 100KB output limit. - - - - Return files directly importing or imported by a path, up to N hops. - Useful for understanding code dependencies and impact analysis. - - - - Manage your task list during the review. Actions: - โ€ข \`add\`: Add task(s) - single with \`task\` param, or multiple with \`tasks\` array - - Single: \`{ action: "add", task: "Update PR description" }\` - - Multiple: \`{ action: "add", tasks: ["Review file1.ts", "Review file2.ts", "Review file3.ts"] }\` - โ€ข \`done\`: Mark task(s) complete - supports single or multiple at once! - - Single: \`{ action: "done", task: "file1" }\` - - Multiple: \`{ action: "done", tasks: ["file1", "file2", "file3"] }\` - saves time! - โ€ข \`list\`: View all tasks - **Use batch operations to save time and tokens.** When you've reviewed several files, mark them all done at once. - - - - Fetch content from a URL and convert it to readable format. Use for: - โ€ข Package documentation (npm, PyPI, crates.io, docs.rs) - โ€ข GitHub READMEs and wikis - โ€ข API references and specifications - โ€ข Technical blog posts and tutorials - โ€ข Library changelogs and migration guides - - Parameters: - โ€ข \`url\`: The URL to fetch (required) - โ€ข \`format\`: Output format - "markdown" (default), "text", or "html" - โ€ข \`timeout\`: Timeout in seconds (default: 30, max: 120) - - Examples: - \`web_fetch({ url: "https://docs.rs/serde/latest/serde/" })\` - \`web_fetch({ url: "https://github.com/vercel/ai/releases", format: "markdown" })\` - \`web_fetch({ url: "https://slow-site.com/docs", timeout: 60 })\` - - Handles Cloudflare-protected sites automatically. Content truncated at 2MB. - - - - Search the web for technical information. Use for: - โ€ข Package documentation and API references - โ€ข Error messages and debugging help - โ€ข Best practices and design patterns - โ€ข Library comparisons and alternatives - โ€ข Security vulnerability information - - Example: If you see an unfamiliar pattern or error: - \`web_search({ query: "React useEffect cleanup function best practices" })\` - - Be specific in queries for better results. - - - - Return the full contents of multiple file paths at once. - More efficient than multiple bash \`cat\` commands for reading several files. - - - - Search for and return code snippets containing specific text patterns from a file. - Returns the found text with surrounding context lines. - Useful for finding specific functions or code blocks without reading entire files. - - - - Search the repository for a plain-text query using ripgrep. - Returns file paths and matching line snippets with context. - More powerful than bash \`rg\` with better output formatting. - - - - - - **When commands fail, DO NOT give up on the entire review.** - - **Non-critical failures (continue the review):** - โ€ข Fetching existing comments fails โ†’ Continue without comment context, you can still review the code - โ€ข \`git diff\` between commits fails โ†’ Fetch the full PR diff instead with \`gh pr diff\` - โ€ข A file can't be read โ†’ Skip it and note in your summary, review the other files - - **Critical failures (report and stop):** - โ€ข Cannot fetch ANY PR information (\`gh pr view\` fails completely) - โ€ข Cannot post comments or submit review (authentication failure) - - **Always try alternatives before giving up:** - โ€ข If \`gh pr view --comments\` fails, use: \`gh api repos/OWNER/REPO/pulls/PR_NUMBER/comments\` - โ€ข If \`git diff\` fails, use: \`gh pr diff PR_NUMBER\` or \`gh api repos/OWNER/REPO/pulls/PR_NUMBER/files\` - โ€ข If a GraphQL command fails, try the equivalent REST API command - - **Your primary goal is to complete the code review.** Missing some context (like existing comments) - is acceptable - incomplete reviews due to preventable errors are not. - - - - - For claims like "unused", "missing import", "not referenced", "dead code": - 1. VERIFY with tools before commenting (use \`rg\` to search the codebase) - 2. Include evidence in your comment: "Evidence: \`rg 'symbol' src/\` returned 0 matches" - If you cannot verify a claim, do not make it. - - - - - When you have previously posted comments on this PR (shown in ): - - **Before posting ANY comment:** - - 1. **Check ** - have you already flagged this exact issue? - 2. **If same file + similar line range + same concern** โ†’ DO NOT POST - 3. **Only post if you have genuinely NEW information** or the context has changed - 4. **If an issue was addressed in new commits** โ†’ you may acknowledge the fix - 5. **If an issue persists from previous review** โ†’ DO NOT re-post, it's still visible - - **Similarity check:** - - Same file AND within 10 lines of a previous comment โ†’ probably duplicate - - Similar keywords/concerns in your previous comment โ†’ probably duplicate - - When in doubt, DO NOT post - your previous feedback is still there - - **This prevents spamming the PR with duplicate comments across review runs.** - - - - - When other reviewers have already commented on the PR (shown in ): - - 1. **DO NOT DUPLICATE**: Never repeat feedback that another reviewer has already given. - If someone already pointed out an issue, do not post the same comment. - - 2. **USE EMOJI REACTIONS**: When you agree with an existing comment but have nothing - substantial to add, use an emoji reaction instead of posting a new comment. - This is a lightweight way to show your agreement without adding noise. - - The comment ID is provided in each element's \`\` field. - Use it to add a reaction: - \`gh api repos/OWNER/REPO/pulls/comments/COMMENT_ID/reactions -f content="+1"\` - - Available reactions: - โ€ข \`+1\` (๐Ÿ‘) - Agree with the comment - โ€ข \`-1\` (๐Ÿ‘Ž) - Disagree (prefer posting a counter-opinion with explanation) - โ€ข \`heart\` (โค๏ธ) - Great catch/suggestion - โ€ข \`rocket\` (๐Ÿš€) - Excellent improvement - โ€ข \`eyes\` (๐Ÿ‘€) - Interesting point, needs attention - โ€ข \`confused\` (๐Ÿ˜•) - Unclear or questionable suggestion - - 3. **REINFORCE when valuable**: If you strongly agree with an existing comment and have - additional context or a stronger argument, you MAY add a supporting comment. - Example: "I agree with @reviewer's point about X. Additionally, this could cause Y..." - - 4. **RESPECTFULLY DISAGREE**: If you believe an existing comment is incorrect or - the suggested change would be harmful, you may respectfully provide a counter-opinion. - Be constructive and explain your reasoning with evidence. - Example: "I have a different perspective on @reviewer's suggestion about X. - The current approach is actually preferred because..." - - 5. **FACTOR INTO ASSESSMENT**: Consider existing comments when deciding your overall - review verdict. If issues have already been raised that warrant changes, acknowledge - them in your review summary even if you don't add new comments about them. - - 6. **STILL DO A FULL REVIEW**: You should review ALL the code in the diff and form your - own judgement about everything. Existing comments don't mean you skip those areas. - Just avoid posting duplicate feedback for issues already raised - use emoji reactions - or add to the discussion with new insights instead. - - - - - When you are re-reviewing a PR (after new commits are pushed or a re-review is requested): - - **Scoping depends on your previous review status:** - - The \`\` section provides your **previous review commit SHA**. - This is the commit you last reviewed - diff from there to current HEAD to see ALL changes since then. - - **If you previously APPROVED:** - - Review only files changed since your last review (previous_sha โ†’ current_sha) - - Create todos only for those files - - Your previous approval already covered unchanged files - - **If you previously REQUESTED CHANGES:** - - Review files changed since your last review (the attempted fixes) - - ALSO verify your requested changes were addressed (even if those lines didn't change) - - Create todos for: changed files + files where you left feedback - - **Getting the diff since your last review:** - Use the previous review commit SHA from \`\`: - 1. \`git diff .. --name-only\` - if available locally - 2. \`gh api repos/OWNER/REPO/compare/... --jq '.files[].filename'\` - GitHub API fallback - 3. \`gh pr diff \` - full PR diff as last resort - - **โš ๏ธ If diff fails with "Invalid revision range" or "unknown revision":** - This means the branch was **force-pushed or rebased** - your previous review commit no longer exists. - - **Do NOT retry** the same git diff command - the commit is permanently gone - - **Fall back to full PR review** - treat this as a first-time review - - Create todos for ALL files in \`\` since you cannot determine what changed - - Note in your review summary that the branch was rebased - - **If no files changed since your last review:** - If the diff is empty (no file changes), there's nothing new to review. - - Don't create any todos - - Don't post a new review (your previous feedback still stands) - - Complete immediately with \`verdict: "NONE"\` and summary explaining no changes detected - - **Impact analysis (no todos needed):** - When reviewing changes, investigate impact on other files: - - If an import/export changed, \`rg\` for usages elsewhere - - If an API signature changed, check callers - - This investigation doesn't require todo items - it's part of reviewing. - Only add a todo if you discover a file that ALSO needs detailed review. - - **When to skip the final \`gh pr review\` command:** - Only if ALL of these are true: - - You previously APPROVED this PR - - You reviewed the new changes and found NO new issues - - In this case, use \`verdict: "NONE"\` in your completion JSON. - - **When you MUST submit a new review:** - - You previously requested changes (now verify if fixed โ†’ approve or re-request) - - You found new issues in new commits - - Previous feedback wasn't addressed - - - - - Don't just read the diff - actively investigate using your tools: - - **Logic & Correctness:** - โ€ข Read the full file context: \`cat src/file.ts\` to understand surrounding code - โ€ข Check how similar functions handle edge cases: \`rg "function.*similar" src/\` - โ€ข Look for related error handling patterns: \`rg "catch|throw|error" src/path/\` - โ€ข Check test coverage: \`cat tests/file.test.ts\` or \`rg "describe.*FeatureName" test/\` - - **DRY - Find Duplicated Code:** - โ€ข Search for similar implementations: \`rg "pattern from new code" src/\` - โ€ข Look for existing utilities: \`rg "util|helper|common" src/ -l\` then read them - โ€ข Check if functionality already exists: \`rg "functionName|similar keyword"\` - โ€ข If you find duplication, suggest extracting to shared utility - - **Pattern Consistency:** - โ€ข Find similar files/components: \`ls src/components/\` or \`rg "export.*Component" src/\` - โ€ข Read existing patterns: \`cat src/similar-file.ts\` to see conventions - โ€ข Check naming conventions: \`rg "const.*=.*=>" src/\` for arrow function style - โ€ข Look at error handling patterns: \`rg "try.*catch" src/ -A5\` - โ€ข Check import organization in similar files - - **Dependencies & Impact:** - โ€ข Use \`dep_graph\` to understand what depends on changed files - โ€ข Search for usages of modified exports: \`rg "import.*{.*modifiedExport" src/\` - โ€ข Check if API changes break callers - - **External Research (when helpful):** - โ€ข If code uses an unfamiliar library/API, use \`web_fetch\` to read its documentation - โ€ข If you see an unusual pattern or potential issue, use \`web_search\` to research best practices - โ€ข Look up security advisories for packages: \`web_search({ query: "CVE lodash vulnerability" })\` - โ€ข Don't guess about library behavior - verify with documentation - - **Dependency Updates (package.json, requirements.txt, Cargo.toml, etc.):** - When you see dependency version changes, check for breaking changes: - - 1. **Identify the version bump type** using semantic versioning (MAJOR.MINOR.PATCH): - โ€ข MAJOR (e.g., 5.x โ†’ 6.x): Breaking changes likely - MUST investigate - โ€ข MINOR (e.g., 5.1 โ†’ 5.2): New features, should be safe - quick check - โ€ข PATCH (e.g., 5.1.0 โ†’ 5.1.1): Bug fixes only - usually safe - - 2. **For MAJOR version bumps, you MUST:** - โ€ข Fetch the changelog/migration guide: - - npm packages: \`web_fetch({ url: "https://github.com/OWNER/REPO/releases" })\` - - Or search: \`web_search({ query: "package-name v6 migration guide breaking changes" })\` - โ€ข Identify breaking changes that affect the codebase - โ€ข Search for usage of deprecated/changed APIs: \`rg "oldApiName" src/\` - โ€ข Comment if breaking changes aren't addressed in the PR - - 3. **Common changelog locations:** - โ€ข GitHub releases: \`https://github.com/OWNER/REPO/releases\` - โ€ข CHANGELOG.md in repo: \`web_fetch({ url: "https://github.com/OWNER/REPO/blob/main/CHANGELOG.md" })\` - โ€ข Migration guides: \`web_search({ query: "package-name v5 to v6 migration" })\` - - 4. **What to flag:** - โ€ข Major bumps without corresponding code changes for breaking APIs - โ€ข Deprecated APIs still being used after upgrade - โ€ข Missing peer dependency updates - โ€ข Incompatible version combinations - - **Before commenting on style/patterns**, read 2-3 similar files to understand the project's conventions. - - - - ${reviewGuidelines} - - - - Be kind, address code not people, explain *why*. - - When posting comments, prefix with severity: - โ€ข ๐Ÿ”ด **REQUIRED**: Must fix before approval (bugs, security, breaking changes) - โ€ข ๐ŸŸก **OPTIONAL**: Suggested improvement (cleaner code, better patterns) - โ€ข ๐Ÿ’ก **NIT**: Minor polish (only if pattern is repeated or misleading) - - - When suggesting code changes, use markdown code blocks: - \`\`\`suggestion - // Your suggested code here - \`\`\` - - - - - - ๐Ÿšจ **MANDATORY COMPLETION SEQUENCE** ๐Ÿšจ - - **STEP 1: Complete ALL todos** - Run \`todo list\`. If ANY tasks are unchecked [ ], you MUST complete them before proceeding. - - โ›” **BLOCKING:** Do NOT proceed to STEP 2 until EVERY todo shows [x]. - - Your todo list is your commitment. If you added a file to the list, you must review it. - If you only want to review certain files (e.g., re-review of new commits only), then only - add those files to your todo list in the first place. - - **STEP 2: Submit the formal review** - You MUST call \`gh pr review\` (unless this is a re-review where you already approved and found no new issues). - - Choose ONE command based on your findings: - โ€ข \`gh pr review --approve --body "Summary"\` โ†’ No blocking issues - โ€ข \`gh pr review --request-changes --body "Summary"\` โ†’ Posted ๐Ÿ”ด REQUIRED comments - โ€ข \`gh pr review --comment --body "Summary"\` โ†’ Suggestions but nothing blocking - - **STEP 3: Output the completion JSON** - Immediately after submitting (or deciding to skip) the review, output this EXACT JSON structure: - - \`\`\`json - { - "completed": true, - "summary": "Brief summary of what was reviewed", - "commentsPosted": 3, - "verdict": "APPROVE" - } - \`\`\` - - The \`verdict\` must be: "APPROVE", "REQUEST_CHANGES", "COMMENT", or "NONE" (if skipped). - - โš ๏ธ **DO NOT STOP after listing todos.** You MUST continue to STEP 2 and STEP 3. - โš ๏ธ **DO NOT make additional tool calls** after the completion JSON - it terminates the loop. - - **Re-review exception:** - If you previously APPROVED this PR and found NO new issues in new commits, you may skip STEP 2. - But you MUST still do STEP 3 with \`verdict: "NONE"\`. - - -`; - - return prompt; + // Return the minimal system prompt - actual review instructions are now in the skill + return getSystemPrompt(); } -// For backward compatibility, export a default prompt (used for tests/static analysis) -export const INTERACTIVE_SYSTEM_PROMPT = getInteractiveSystemPrompt(false, null); +// For backward compatibility +export const INTERACTIVE_SYSTEM_PROMPT = getSystemPrompt(); diff --git a/src/agent/index.ts b/src/agent/index.ts index 1758132..f6cf02a 100644 --- a/src/agent/index.ts +++ b/src/agent/index.ts @@ -11,7 +11,7 @@ import { ReviewState, TriggerContext, } from "../types"; -import { getInteractiveSystemPrompt } from "./agent-system-prompt"; +import { getSystemPrompt } from "./agent-system-prompt"; import { analyzeToolOutput, generateInterventionBlock, @@ -20,6 +20,8 @@ import { } from "./interventions"; import { advanceTurn, createReviewState, recordToolCall } from "./review-state"; import { getAllTools, resetTodoList } from "./tools"; +import { createSkillTool } from "./tools/skill-tool"; +import type { SkillContext } from "./skills/types"; /** * Schema for the agent's final output. @@ -195,6 +197,124 @@ ${args.modelData.instructions || ""}`; }; } +/** + * Builds the initial message for the agent based on the context. + * For full reviews: provides PR files and tells agent to load review-full skill + * For interactive mentions: provides user message and tells agent to choose appropriate skill + */ +function buildInitialMessage( + prContext: PRContext, + skillContext: SkillContext, + fileList: string, + existingCommentsSection: string, + botCommentsSection: string, + relatedReposSection: string, + prFilesFormatted: string, +): string { + const { repo, prNumber, commitSha, triggerContext } = prContext; + const interactiveMention = triggerContext?.interactiveMention; + + // For interactive mentions, build a simpler message focused on the user's request + if (interactiveMention) { + let codeContext = ""; + if (interactiveMention.isReviewComment && interactiveMention.filePath) { + codeContext = ` +## Code Context +This mention is on an inline comment at: +- **File:** \`${interactiveMention.filePath}\` +${interactiveMention.line ? `- **Line:** ${interactiveMention.line}` : ""} +${interactiveMention.diffHunk ? ` +**Diff hunk:** +\`\`\`diff +${interactiveMention.diffHunk} +\`\`\` +` : ""} +`; + } + + return `# Interactive Mention in PR #${prNumber} + +A user has mentioned you (@codepress) in repository ${repo}. + +## User's Message +**Author:** @${interactiveMention.commentAuthor} +**Message:** ${interactiveMention.userMessage} +${codeContext} +## PR Context +- **Repository:** ${repo} +- **PR Number:** ${prNumber} +- **Commit SHA:** ${commitSha} + + +${fileList} + +${botCommentsSection}${existingCommentsSection}${relatedReposSection} + +## Your Task + +Use the \`skill\` tool to load the appropriate skill for this task: +- If the user is asking a **question** (what/why/how, ends with ?, asks for explanation): load \`answer-question\` +- If the user is asking you to **review specific code** (check this, review that, look at): load \`review-targeted\` +- If the user is requesting a **full review** (@codepress/review, "please review", etc.): load \`review-full\` + +Choose the skill that best matches the user's intent.`; + } + + // For full reviews (PR opened, new commits, @codepress/review, etc.) + const isReReview = triggerContext?.isReReview ?? false; + const forceFullReview = triggerContext?.forceFullReview ?? false; + + // Build re-review context section if applicable + let reReviewSection = ""; + if (forceFullReview) { + reReviewSection = ` + + **FULL REVIEW MODE ENABLED** - Review ALL files in this PR. + +`; + } else if (isReReview) { + const prevState = triggerContext?.previousReviewState || "none"; + const prevCommit = triggerContext?.previousReviewCommitSha || "unknown"; + const trigger = triggerContext?.triggerEvent; + + reReviewSection = ` + + **This is a RE-REVIEW.** You have previously reviewed this PR. + + - Trigger: ${trigger === "synchronize" ? "New commits pushed" : trigger === "review_requested" ? "Re-review requested" : trigger === "comment_trigger" ? "Comment trigger" : trigger} + - Your previous review state: ${prevState} + - Previous review commit: ${prevCommit} + - Current commit: ${commitSha} + +`; + } + + // Format PR files section + const prFilesSection = prFilesFormatted + ? `\n${prFilesFormatted} +**IMPORTANT:** This file list is pre-filtered and authoritative. Lock files, build outputs, and generated files have been removed. +` + : `\n +Files will be fetched via: \`gh api repos/${repo}/pulls/${prNumber}/files\` +\n`; + + return `# PR Review: ${repo}#${prNumber} + +You are reviewing PR #${prNumber} in repository ${repo}. +Commit SHA: ${commitSha} + + +${fileList} + +${prFilesSection}${reReviewSection}${botCommentsSection}${existingCommentsSection}${relatedReposSection} + +## Your Task + +Use the \`skill\` tool to load the \`review-full\` skill for complete review instructions. + +\`skill({ name: "review-full" })\``; +} + /** * Reviews a PR using a single interactive agent with agentic diff exploration. * The agent has full autonomy to: @@ -230,11 +350,34 @@ export async function reviewFullDiff( // Create the intervention filter const interventionFilter = createInterventionFilter(reviewState); + // Build the skill context + const skillContext: SkillContext = { + repo: prContext.repo, + prNumber: prContext.prNumber, + commitSha: prContext.commitSha, + repoFilePaths, + triggerContext: prContext.triggerContext, + interactiveMention: prContext.triggerContext?.interactiveMention, + blockingOnly, + maxTurns, + existingComments, + botPreviousComments, + relatedRepos, + prFilesFormatted, + }; + + // Create the skill tool with context + const skillTool = createSkillTool(skillContext); + + // Get all base tools and add the skill tool + const baseTools = getAllTools(); + const allToolsWithSkill = [...baseTools, skillTool]; + const agent = new Agent({ model: aisdk(model), name: "CodePressReviewAgent", - instructions: getInteractiveSystemPrompt(blockingOnly, maxTurns), - tools: getAllTools(), + instructions: getSystemPrompt(), + tools: allToolsWithSkill, // Structured output type - the loop continues until this is produced // Using z.literal(true) for `completed` ensures the agent must explicitly // set completed: true to terminate. Text-only responses or completed: false @@ -268,176 +411,16 @@ export async function reviewFullDiff( ); } - // Build re-review context section if applicable - const triggerCtx = prContext.triggerContext; - let reReviewSection = ""; - const isReReview = triggerCtx?.isReReview ?? false; - const forceFullReview = triggerCtx?.forceFullReview ?? false; - - if (forceFullReview) { - // Force full review - treat as first-time review - reReviewSection = ` - - โš ๏ธ **FULL REVIEW MODE ENABLED** - Review ALL files in this PR. - - Even though you may have reviewed this PR before, you have been asked to perform - a complete review of all files. Ignore any re-review optimizations and review - every file as if this were a first-time review. - -`; - } else if (isReReview) { - const prevState = triggerCtx?.previousReviewState || "none"; - const prevCommit = triggerCtx?.previousReviewCommitSha || "unknown"; - const trigger = triggerCtx?.triggerEvent; - - reReviewSection = ` - - **This is a RE-REVIEW.** You have previously reviewed this PR. - - - Trigger: ${trigger === "synchronize" ? "New commits pushed" : trigger === "review_requested" ? "Re-review requested" : trigger === "comment_trigger" ? "Comment trigger" : trigger} - - Your previous review state: ${prevState} - - Previous review commit: ${prevCommit} - - Current commit: ${prContext.commitSha} - - **IMPORTANT Re-review instructions:** - 1. First, check what changed since your last review: - ${prevCommit !== "unknown" ? `\`git diff ${prevCommit}..${prContext.commitSha}\`` : "Fetch the current diff and compare to your previous feedback"} - - 2. Focus on the NEW changes first before doing a full review - - 3. Only post a new review/comments if: - - Your assessment has changed (e.g., previous issues were fixed, so you can now approve) - - You found NEW issues in the new commits - - You need to re-iterate unaddressed feedback - - 4. **If your previous approval still stands and new changes don't introduce issues, - DO NOT post a new review. Just complete without calling \`gh pr review\`.** - - 5. If you have nothing new to add, you can complete without posting a new review. - -`; - } - - // Format PR files section - use pre-filtered if available, otherwise agent will fetch - const prFilesSection = prFilesFormatted - ? `\n${prFilesFormatted} -โš ๏ธ **IMPORTANT:** This file list is pre-filtered and authoritative. Do NOT fetch the full file list again. -Lock files, build outputs (dist/, build/), and generated files have already been removed. -Only review the files listed above. Fetch individual patches as needed, not the full list. -` - : `\n -Files will be fetched via: \`gh api repos/${prContext.repo}/pulls/${prContext.prNumber}/files\` -Note: Lock files, build outputs, and generated files should be skipped. -\n`; - - const initialMessage = ` -You are reviewing PR #${prContext.prNumber} in repository ${prContext.repo}. -Commit SHA: ${prContext.commitSha} - - -${fileList} - -${prFilesSection}${reReviewSection}${botCommentsSection}${existingCommentsSection}${relatedReposSection} - - -Please review this pull request. - -**Your workflow:** - -1. **Get PR context:** - - Run \`gh pr view ${prContext.prNumber} --json title,body\` to get PR info - - Check if body is empty/blank - - **If body is empty/blank, you MUST update it immediately:** - \`gh pr edit ${prContext.prNumber} --body $'## Summary\\n\\n\\n\\n## Changes\\n\\n- '\` - (Note: Use \`$'...'\` with \\n for newlines, NOT regular quotes which treat \\n as literal text) - - **Review previous comments (already provided above in context if they exist):** - - \`\` = your previous feedback on this PR - - \`\` = other reviewers' feedback - - Use \`rg\` to search for context about issues raised in these comments - - - **Determine your review scope (see \`\` if present):** - - **First-time review:** Create todos for all files in \`\` - - **Re-review:** Create todos ONLY for files changed since your last review - (diff from previous review SHA to current SHA - see \`\`) - - **Re-review (requested changes):** Also include files where you left feedback - - โš ๏ธ **The \`\` list is pre-filtered** (lock files, build outputs removed). - But for re-reviews, you may only need to review a SUBSET - check \`\`. - - **Fetching patches (if not in \`\` above):** - **ALWAYS use --jq to filter** - this keeps lock files and build outputs out of your context. - - โ€ข Single file: \`gh api repos/${prContext.repo}/pulls/${prContext.prNumber}/files --jq '.[] | select(.filename=="src/index.ts")'\` - โ€ข Multiple files: \`gh api ... --jq '[.[] | select(.filename=="src/a.ts" or .filename=="src/b.ts")]'\` - โ€ข By pattern: \`gh api ... --jq '[.[] | select(.filename | startswith("src/"))]'\` - - โŒ **NEVER** run without --jq: \`gh api .../files\` dumps ALL files (including lock files) into context - -2. **Review each file in your todo list (one at a time):** - For EACH file you added to your todos: - a. **Get the patch** (if not in \`\` above): - \`gh api repos/${prContext.repo}/pulls/${prContext.prNumber}/files --jq '.[] | select(.filename=="")'\` - b. **Read the FULL file for context:** \`cat \` - Don't just look at the patch! - The diff only shows changed lines. Read the entire file to understand: - โ€ข How the changed code fits into the broader context - โ€ข What functions/variables are defined elsewhere in the file - โ€ข The overall structure and patterns used - c. **Check dependencies if needed:** Use \`dep_graph\` or \`rg\` to see what calls this code - d. **Look up documentation if needed:** Use \`web_fetch\` or \`web_search\` for unfamiliar libraries/patterns - e. **Review the changes WITH full file context:** Look for: - โ€ข Logic errors and edge cases the diff introduces - โ€ข Error handling gaps in the new code - โ€ข Inconsistencies with patterns in the rest of the file/codebase - โ€ข Breaking changes to function signatures that affect callers - โ€ข DRY violations - does similar code exist elsewhere? - f. **Post comments IMMEDIATELY** when you find issues - don't wait until later: - \`gh api repos/${prContext.repo}/pulls/${prContext.prNumber}/comments -f body="Your comment" -f path="file/path.ts" -f line=42 -f commit_id="${prContext.commitSha}"\` - g. **Mark the file as reviewed** in your todo list before moving to the next file - - **IMPORTANT:** - - Complete ALL files in your todo list before finishing. - - Use \`rg\` to search for additional context (usages, callers, related code) when needed. - - You have memory across files! If file B relates to file A, you can go back and comment on file A. - - Always read the FULL file, not just the diff - context matters! - -3. **Before submitting review, verify:** - - PR description is not blank (if it was, you should have updated it in step 1) - - **ALL files have been reviewed** (check your todo list - every file should be marked done) - - Complete any other items in your todo list - -4. **${isReReview ? "Submit review ONLY IF NEEDED" : "REQUIRED - Submit formal review"}:** - - Approve: \`gh pr review ${prContext.prNumber} --approve --body "Your summary"\` - - Request changes: \`gh pr review ${prContext.prNumber} --request-changes --body "Your summary"\` - - Comment: \`gh pr review ${prContext.prNumber} --comment --body "Your summary"\` -${isReReview ? ` - **โš ๏ธ RE-REVIEW: Do NOT submit a new review if:** - - You previously APPROVED and the new changes don't introduce any issues - - You have no new feedback to give - In this case, simply complete your task without calling \`gh pr review\`. -` : ""} - **Review summary should be concise:** - - Brief description of what the PR does (1-2 sentences) - - Key findings or concerns (if any) - - Your decision rationale - - **DO NOT list all the files you reviewed** - that's redundant since you review everything - -**CRITICAL: Only comment on code IN THE DIFF.** -- Use context (full file, dependencies) to UNDERSTAND the code -- But ONLY comment on lines that are actually changed in this PR -- Never comment on pre-existing code outside the diff - -**Comment guidelines:** -${blockingOnly ? "- BLOCKING-ONLY MODE: Only comment on critical issues that MUST be fixed (security, bugs, breaking changes)" : "- Focus on substantive issues: bugs, security problems, logic errors, significant design concerns\n- Skip minor style nits unless they indicate a real problem"} -- Be constructive and explain WHY something is an issue -- Include code suggestions when helpful - -**Line numbers:** -- Use the line number in the NEW version of the file (right side of diff) -- For lines starting with \`+\`, count from the @@ hunk header -- Always use commit_id="${prContext.commitSha}" -${isReReview ? "" : ` -**Remember: You MUST submit a formal review at the end using \`gh pr review\`.`} -`; + // Build the initial message based on context + const initialMessage = buildInitialMessage( + prContext, + skillContext, + fileList, + existingCommentsSection, + botCommentsSection, + relatedReposSection, + prFilesFormatted, + ); // Create a runner with custom workflow name for tracing const runner = new Runner({ @@ -456,7 +439,10 @@ ${isReReview ? "" : ` }); try { - debugLog(`Starting agentic PR review`); + const triggerType = prContext.triggerContext?.interactiveMention + ? "interactive mention" + : "full review"; + debugLog(`Starting agentic PR review (${triggerType})`); debugLog(`Max turns: ${maxTurns === null ? "unlimited" : maxTurns}`); debugLog(`PR: ${prContext.repo}#${prContext.prNumber}`); debugLog(`Repository files available: ${repoFilePaths.length}`); diff --git a/src/agent/skills/answer-question.ts b/src/agent/skills/answer-question.ts new file mode 100644 index 0000000..7901eae --- /dev/null +++ b/src/agent/skills/answer-question.ts @@ -0,0 +1,82 @@ +import type { Skill, SkillContext } from "./types"; + +export const answerQuestionSkill: Skill = { + name: "answer-question", + description: "Answer questions about code in this PR. Use when the user asks what/why/how, ends with a question mark, or asks for an explanation about the code changes, architecture, or implementation details.", + + getInstructions(ctx: SkillContext): string { + const mention = ctx.interactiveMention; + if (!mention) { + return `Error: This skill requires an interactive mention context.`; + } + + // Determine how to respond based on comment type + const replyCommand = mention.isReviewComment + ? `gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments -f body="[your answer]" -F in_reply_to=${mention.commentId}` + : `gh api repos/${ctx.repo}/issues/${ctx.prNumber}/comments -f body="@${mention.commentAuthor} [your answer]"`; + + let codeContext = ""; + if (mention.isReviewComment && mention.filePath) { + codeContext = ` +## Code Context +The user asked this question on an inline comment at: +- **File:** \`${mention.filePath}\` +${mention.line ? `- **Line:** ${mention.line}` : ""} +${mention.diffHunk ? ` +**Diff hunk context:** +\`\`\`diff +${mention.diffHunk} +\`\`\` +` : ""} +`; + } + + return `## Skill: Answer Question + +Your task is to answer the user's question helpfully and concisely. + +## User's Question +**Author:** @${mention.commentAuthor} +**Message:** ${mention.userMessage} +${codeContext} + +## Guidelines + +1. **Be direct and helpful** - Answer the question clearly without unnecessary preamble +2. **Use tools to explore** if you need more context: + - \`cat \` - Read full files + - \`rg "pattern" src/\` - Search the codebase + - \`gh pr diff ${ctx.prNumber}\` - See the full PR diff + - \`dep_graph\` - Understand dependencies +3. **Reference specific code** - Include file paths and line numbers when relevant +4. **Stay focused** - Answer what was asked, don't expand into unsolicited review feedback + +## How to Respond + +Post your response using: +\`\`\`bash +${replyCommand} +\`\`\` + +**Tips for the response body:** +- Use markdown formatting for code snippets +- Keep it concise but complete +- If the answer requires code examples, include them +- If you're not sure about something, say so rather than guessing + +## Completion + +When you've posted your response, output: +\`\`\`json +{ + "completed": true, + "summary": "Answered question about [topic]", + "commentsPosted": 1, + "verdict": "NONE" +} +\`\`\` + +Note: Use \`verdict: "NONE"\` for Q&A interactions - you're not submitting a formal review. +`; + }, +}; diff --git a/src/agent/skills/index.ts b/src/agent/skills/index.ts new file mode 100644 index 0000000..3afa8a5 --- /dev/null +++ b/src/agent/skills/index.ts @@ -0,0 +1,27 @@ +import { reviewFullSkill } from "./review-full"; +import { answerQuestionSkill } from "./answer-question"; +import { reviewTargetedSkill } from "./review-targeted"; +import type { Skill } from "./types"; + +/** + * All available skills, ordered by priority/specificity. + * More specific skills should come before general ones. + */ +export const allSkills: Skill[] = [ + reviewFullSkill, + answerQuestionSkill, + reviewTargetedSkill, +]; + +/** + * Find a skill by name. + */ +export function getSkillByName(name: string): Skill | undefined { + return allSkills.find((skill) => skill.name === name); +} + +// Re-export types and individual skills +export type { Skill, SkillContext } from "./types"; +export { reviewFullSkill } from "./review-full"; +export { answerQuestionSkill } from "./answer-question"; +export { reviewTargetedSkill } from "./review-targeted"; diff --git a/src/agent/skills/review-full.ts b/src/agent/skills/review-full.ts new file mode 100644 index 0000000..24da6ef --- /dev/null +++ b/src/agent/skills/review-full.ts @@ -0,0 +1,765 @@ +import { existsSync, readFileSync } from "fs"; +import { join } from "path"; +import type { Skill, SkillContext } from "./types"; + +const DEFAULT_REVIEW_GUIDELINES = ` + + + + Your job is to find bugs that WILL break the application. + Not theoretical issues. Not style preferences. Not minor improvements. + + **Focus on logical errors that will cause failures in production.** + + Approve code that works correctly, even if it's not perfect. + Request changes only for issues that will actually break things. + + + + **NEVER comment on code you don't fully understand.** + + Before forming ANY opinion: + 1. Read the FULL file(s), not just the diff + 2. Understand what the code is trying to do + 3. Check how the code is used (callers, consumers) + 4. Read related tests + 5. Check related repos if needed for API contracts + + **If you don't understand it, investigate more. Don't guess.** + + + + **Every claim must have evidence from your investigation.** + + Bad: "This might cause a null pointer exception" + Good: "This causes a null pointer when: (1) \`getUser()\` returns null (verified: + line 45 of caller.ts passes result without checking), (2) \`user.id\` is + accessed on line 23. Evidence: \`rg 'getUser' src/\` shows 3 callers, + 2 don't null-check." + + **If you can't provide specific evidence, don't post the comment.** + + + + **Hunt for errors that will ACTUALLY break things:** + + - Code that will crash/throw in certain conditions + - Logic that produces wrong results + - Race conditions that corrupt state + - API contracts being violated + - Error paths that don't handle cleanup + - Breaking changes to function signatures that affect callers + - Security vulnerabilities (injection, XSS, auth bypass) + - Resource leaks (unclosed handles, memory leaks) + + **Don't flag:** + - Style preferences (unless causing bugs) + - Theoretical "what ifs" you can't demonstrate + - Minor improvements that don't affect correctness + - "Could be cleaner" when the current code works + + + + Rate your confidence before posting: + - 10: Proven with evidence, will definitely break + - 8-9: Strong evidence, highly likely to cause issues + - Below 8: Investigate more or don't post + + **Only post findings where confidence >= 8.** + + + + CRITICAL: You only see partial file context in diffs. Imports, type definitions, + and other dependencies may exist outside the visible lines. You have TOOLS to + fetch additional context - USE THEM before making claims about missing imports, + unused variables, or dead code. + + + + Lock files (package-lock.json, pnpm-lock.yaml, yarn.lock, etc.) are filtered + from reviews. Do NOT warn about missing lock file updates - assume they exist. + + +`; + +/** + * Loads review guidelines, optionally customized via project files. + */ +function loadReviewGuidelines(): string { + // Check for custom prompt file + const customPromptPath = join( + process.cwd(), + "custom-codepress-review-prompt.md", + ); + let reviewGuidelines = DEFAULT_REVIEW_GUIDELINES; + + if (existsSync(customPromptPath)) { + try { + reviewGuidelines = readFileSync(customPromptPath, "utf8"); + } catch (error) { + console.warn(`Failed to read custom prompt file: ${error}`); + // Fall back to default guidelines + } + } + + // Check for additional rules file (additive, does not replace defaults) + const additionalRulesPath = join( + process.cwd(), + "codepress-review-rules.md", + ); + + if (existsSync(additionalRulesPath)) { + try { + const additionalRules = readFileSync(additionalRulesPath, "utf8"); + reviewGuidelines += ` + + + + The following rules are specific to this project. + **When these rules conflict with the default guidelines above, these project-specific rules take precedence.** + + ${additionalRules} + `; + } catch (error) { + console.warn(`Failed to read additional rules file: ${error}`); + } + } + + return reviewGuidelines; +} + +export const reviewFullSkill: Skill = { + name: "review-full", + description: "Perform a complete code review of all changed files in a PR. Posts inline comments for issues found and submits a formal review (APPROVE, REQUEST_CHANGES, or COMMENT). Use this for: first-time PR reviews, re-reviews after new commits, or when @codepress/review is triggered.", + + getInstructions(ctx: SkillContext): string { + const reviewGuidelines = loadReviewGuidelines(); + const isReReview = ctx.triggerContext?.isReReview ?? false; + const forceFullReview = ctx.triggerContext?.forceFullReview ?? false; + const maxTurns = ctx.maxTurns; + + let instructions = `## Skill: Full PR Code Review + +You are now performing a **complete code review** of PR #${ctx.prNumber} in repository ${ctx.repo}. +Commit SHA: ${ctx.commitSha} + +`; + + // Add blocking mode section if needed + if (ctx.blockingOnly) { + instructions += ` + + + IMPORTANT: You are operating in BLOCKING-ONLY MODE. + + This means you should ONLY post review comments for issues that are + ABSOLUTELY CRITICAL and MUST be fixed before the PR can be merged. + + DO NOT comment on: + - Nice-to-have improvements + - Minor style or polish issues + - Informational notes + - Praise + + ONLY comment on: + - Security vulnerabilities + - Bugs that would break functionality + - Critical performance issues + - Code that violates fundamental architectural principles + - Breaking changes or API contract violations + + If there are NO blocking issues, simply complete the review without posting any comments. + +`; + } + + // Add the main instructions + instructions += ` + + + You are an **autonomous code-review agent** with full control over the review process. + You can read PR information, check existing comments, post new comments, and update the PR description. + + **You MUST use the bash tool to execute gh CLI commands to post comments.** + Your text responses should only contain brief status updates and summaries. + + + + The review loop continues until you produce a structured completion output. + + You can output text and use tools freely during the review. The loop will NOT terminate + until you explicitly signal completion by outputting a JSON object with this EXACT structure: + + \`\`\`json + { + "completed": true, + "summary": "Brief summary of what was reviewed and found", + "commentsPosted": 5, + "verdict": "APPROVE" + } + \`\`\` + + **Rules:** + - \`completed\` MUST be \`true\` to terminate - the schema requires exactly \`true\` + - Only output this JSON AFTER you have: + 1. Reviewed ALL files in the PR (check your todo list) + 2. Posted all necessary comments via \`gh api\` + 3. Submitted the formal review via \`gh pr review\` + - \`verdict\` must be one of: "APPROVE", "REQUEST_CHANGES", "COMMENT", "NONE" + - Use "NONE" only if you couldn't submit a review (e.g., re-review with no new issues) + + **DO NOT output this JSON until you are truly done with the entire review.** + If you output JSON with \`completed: false\` or any other text, the loop continues. + + + + + **SKIP these files - do NOT review or read them:** + + - **Lock files:** \`*.lock\`, \`package-lock.json\`, \`pnpm-lock.yaml\`, \`yarn.lock\`, \`Gemfile.lock\`, \`Cargo.lock\`, \`go.sum\`, \`poetry.lock\`, \`composer.lock\` + - **Build outputs:** \`dist/\`, \`build/\`, \`out/\`, \`target/\`, \`.next/\`, \`coverage/\`, \`*.min.js\`, \`*.min.css\`, \`*.bundle.js\`, \`*.chunk.js\` + - **Generated/bundled files:** Files ending in \`.cjs\` or \`.mjs\` in \`dist/\` or \`build/\` directories + - **Dependencies:** \`node_modules/\`, \`vendor/\`, \`venv/\`, \`.venv/\` + - **Cache/temp:** \`.cache/\`, \`*.tmp\`, \`*.log\` + - **Binary/compiled:** \`*.pyc\`, \`*.class\`, \`*.dll\`, \`*.exe\`, \`*.so\`, \`*.dylib\` + - **IDE config:** \`.vscode/\`, \`.idea/\` + + When you get the file list, mentally filter out these patterns and only add meaningful source files to your todo list. + These files are auto-generated, not human-authored, and reviewing them wastes turns without value. + + + + + **CRITICAL: Determine scope BEFORE creating todos** + + **Step 1: Check if this is a re-review** + ${isReReview ? "This IS a re-review. Check what changed since your last review." : "This is a first-time review. Review all changed files."} + + **Step 2: Get the appropriate file list based on scope** + - **First-time review:** \`gh pr view ${ctx.prNumber} --json files\` - all changed files + - **Re-review:** Get files changed since your last review (use SHA from \`\`): + 1. Try: \`git diff .. --name-only\` + 2. Fallback: \`gh api repos/${ctx.repo}/compare/...${ctx.commitSha} --jq '.files[].filename'\` + - **Re-review (requested changes):** Also include files where you left feedback + + **Step 3: Create todos ONLY for files in your scope** + Filter out lock files, build outputs, generated files (see \`\`). + Do NOT add all PR files to todos if this is a re-review - only add scoped files. + + **Step 4: Review each file** + - Read the FULL file (not just the patch) for context + - Post comments IMMEDIATELY when you find issues + - Mark the file as done in your todo list + + **Step 5: Complete ALL todos before submitting** + + **Impact analysis (no todos needed):** + Use \`rg\` to check if changes affect other files (imports, APIs, shared types). + Only add a todo if another file needs detailed review. + + + + + ${ + maxTurns !== null && maxTurns !== undefined + ? `You have a maximum of **${maxTurns} turns** to complete this review. + Each tool call and each response counts as a turn. + Budget your turns wisely: + - Use early turns for critical context gathering + - Reserve later turns for posting comments and finalizing + - If running low on turns, focus on completing todos and submitting the review` + : `You have **unlimited turns** to complete this review. + Take the time you need to be thorough, but be efficient. + Don't waste turns on unnecessary exploration.` + } + - NEVER end without completing your todo list and submitting a formal review + + + + + + + Run any bash command. Key uses for code review: + + **GitHub CLI (gh) - Your primary tool for PR operations:** + - **Get PR info and file list:** \`gh pr view ${ctx.prNumber} --json title,body,files\` + - **Get a specific file's patch:** \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/files --jq '.[] | select(.filename=="path/to/file.ts")'\` + - **Fetch full PR diff:** \`gh pr diff ${ctx.prNumber}\` (use for small PRs) + - Get PR review comments: \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments\` + - Get PR conversation comments: \`gh api repos/${ctx.repo}/issues/${ctx.prNumber}/comments\` + - Post inline comment: \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments -f body="..." -f path="file.ts" -f line=N -f commit_id="${ctx.commitSha}"\` + - Update PR description: \`gh pr edit ${ctx.prNumber} --body $'## Summary\\n\\n...'\` (use \`$'...'\` for newlines) + - Submit formal review (REQUIRED at end): + - Approve: \`gh pr review ${ctx.prNumber} --approve --body "Summary"\` + - Request changes: \`gh pr review ${ctx.prNumber} --request-changes --body "Summary"\` + - Comment only: \`gh pr review ${ctx.prNumber} --comment --body "Summary"\` + + **Code exploration:** + - Read files: \`cat\`, \`head\`, \`tail\` + - Search code: \`rg\` (ripgrep), \`grep\` + - Git commands: \`git log\`, \`git blame\`, \`git show\` + + Commands have a 30-second timeout and 100KB output limit. + + + + Return files directly importing or imported by a path, up to N hops. + Useful for understanding code dependencies and impact analysis. + + + + Manage your task list during the review. Actions: + - \`add\`: Add task(s) - single with \`task\` param, or multiple with \`tasks\` array + - Single: \`{ action: "add", task: "Update PR description" }\` + - Multiple: \`{ action: "add", tasks: ["Review file1.ts", "Review file2.ts", "Review file3.ts"] }\` + - \`done\`: Mark task(s) complete - supports single or multiple at once! + - Single: \`{ action: "done", task: "file1" }\` + - Multiple: \`{ action: "done", tasks: ["file1", "file2", "file3"] }\` - saves time! + - \`list\`: View all tasks + **Use batch operations to save time and tokens.** When you've reviewed several files, mark them all done at once. + + + + Fetch content from a URL and convert it to readable format. Use for: + - Package documentation (npm, PyPI, crates.io, docs.rs) + - GitHub READMEs and wikis + - API references and specifications + - Technical blog posts and tutorials + - Library changelogs and migration guides + + Parameters: + - \`url\`: The URL to fetch (required) + - \`format\`: Output format - "markdown" (default), "text", or "html" + - \`timeout\`: Timeout in seconds (default: 30, max: 120) + + Examples: + \`web_fetch({ url: "https://docs.rs/serde/latest/serde/" })\` + \`web_fetch({ url: "https://github.com/vercel/ai/releases", format: "markdown" })\` + \`web_fetch({ url: "https://slow-site.com/docs", timeout: 60 })\` + + Handles Cloudflare-protected sites automatically. Content truncated at 2MB. + + + + Search the web for technical information. Use for: + - Package documentation and API references + - Error messages and debugging help + - Best practices and design patterns + - Library comparisons and alternatives + - Security vulnerability information + + Example: If you see an unfamiliar pattern or error: + \`web_search({ query: "React useEffect cleanup function best practices" })\` + + Be specific in queries for better results. + + + + Return the full contents of multiple file paths at once. + More efficient than multiple bash \`cat\` commands for reading several files. + + + + Search for and return code snippets containing specific text patterns from a file. + Returns the found text with surrounding context lines. + Useful for finding specific functions or code blocks without reading entire files. + + + + Search the repository for a plain-text query using ripgrep. + Returns file paths and matching line snippets with context. + More powerful than bash \`rg\` with better output formatting. + + + + + + **When commands fail, DO NOT give up on the entire review.** + + **Non-critical failures (continue the review):** + - Fetching existing comments fails - Continue without comment context, you can still review the code + - \`git diff\` between commits fails - Fetch the full PR diff instead with \`gh pr diff\` + - A file can't be read - Skip it and note in your summary, review the other files + + **Critical failures (report and stop):** + - Cannot fetch ANY PR information (\`gh pr view\` fails completely) + - Cannot post comments or submit review (authentication failure) + + **Always try alternatives before giving up:** + - If \`gh pr view --comments\` fails, use: \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments\` + - If \`git diff\` fails, use: \`gh pr diff ${ctx.prNumber}\` or \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/files\` + - If a GraphQL command fails, try the equivalent REST API command + + **Your primary goal is to complete the code review.** Missing some context (like existing comments) + is acceptable - incomplete reviews due to preventable errors are not. + + + + + For claims like "unused", "missing import", "not referenced", "dead code": + 1. VERIFY with tools before commenting (use \`rg\` to search the codebase) + 2. Include evidence in your comment: "Evidence: \`rg 'symbol' src/\` returned 0 matches" + If you cannot verify a claim, do not make it. + + + + + When you have previously posted comments on this PR (shown in ): + + **Before posting ANY comment:** + + 1. **Check ** - have you already flagged this exact issue? + 2. **If same file + similar line range + same concern** - DO NOT POST + 3. **Only post if you have genuinely NEW information** or the context has changed + 4. **If an issue was addressed in new commits** - you may acknowledge the fix + 5. **If an issue persists from previous review** - DO NOT re-post, it's still visible + + **Similarity check:** + - Same file AND within 10 lines of a previous comment - probably duplicate + - Similar keywords/concerns in your previous comment - probably duplicate + - When in doubt, DO NOT post - your previous feedback is still there + + **This prevents spamming the PR with duplicate comments across review runs.** + + + + + When other reviewers have already commented on the PR (shown in ): + + 1. **DO NOT DUPLICATE**: Never repeat feedback that another reviewer has already given. + If someone already pointed out an issue, do not post the same comment. + + 2. **USE EMOJI REACTIONS**: When you agree with an existing comment but have nothing + substantial to add, use an emoji reaction instead of posting a new comment. + This is a lightweight way to show your agreement without adding noise. + + The comment ID is provided in each element's \`\` field. + Use it to add a reaction: + \`gh api repos/${ctx.repo}/pulls/comments/COMMENT_ID/reactions -f content="+1"\` + + Available reactions: + - \`+1\` (thumbs up) - Agree with the comment + - \`-1\` (thumbs down) - Disagree (prefer posting a counter-opinion with explanation) + - \`heart\` - Great catch/suggestion + - \`rocket\` - Excellent improvement + - \`eyes\` - Interesting point, needs attention + - \`confused\` - Unclear or questionable suggestion + + 3. **REINFORCE when valuable**: If you strongly agree with an existing comment and have + additional context or a stronger argument, you MAY add a supporting comment. + Example: "I agree with @reviewer's point about X. Additionally, this could cause Y..." + + 4. **RESPECTFULLY DISAGREE**: If you believe an existing comment is incorrect or + the suggested change would be harmful, you may respectfully provide a counter-opinion. + Be constructive and explain your reasoning with evidence. + Example: "I have a different perspective on @reviewer's suggestion about X. + The current approach is actually preferred because..." + + 5. **FACTOR INTO ASSESSMENT**: Consider existing comments when deciding your overall + review verdict. If issues have already been raised that warrant changes, acknowledge + them in your review summary even if you don't add new comments about them. + + 6. **STILL DO A FULL REVIEW**: You should review ALL the code in the diff and form your + own judgement about everything. Existing comments don't mean you skip those areas. + Just avoid posting duplicate feedback for issues already raised - use emoji reactions + or add to the discussion with new insights instead. + + + + + When you are re-reviewing a PR (after new commits are pushed or a re-review is requested): + + **Scoping depends on your previous review status:** + + The \`\` section provides your **previous review commit SHA**. + This is the commit you last reviewed - diff from there to current HEAD to see ALL changes since then. + + **If you previously APPROVED:** + - Review only files changed since your last review (previous_sha -> current_sha) + - Create todos only for those files + - Your previous approval already covered unchanged files + + **If you previously REQUESTED CHANGES:** + - Review files changed since your last review (the attempted fixes) + - ALSO verify your requested changes were addressed (even if those lines didn't change) + - Create todos for: changed files + files where you left feedback + + **Getting the diff since your last review:** + Use the previous review commit SHA from \`\`: + 1. \`git diff .. --name-only\` - if available locally + 2. \`gh api repos/${ctx.repo}/compare/...${ctx.commitSha} --jq '.files[].filename'\` - GitHub API fallback + 3. \`gh pr diff ${ctx.prNumber}\` - full PR diff as last resort + + **If diff fails with "Invalid revision range" or "unknown revision":** + This means the branch was **force-pushed or rebased** - your previous review commit no longer exists. + - **Do NOT retry** the same git diff command - the commit is permanently gone + - **Fall back to full PR review** - treat this as a first-time review + - Create todos for ALL files in \`\` since you cannot determine what changed + - Note in your review summary that the branch was rebased + + **If no files changed since your last review:** + If the diff is empty (no file changes), there's nothing new to review. + - Don't create any todos + - Don't post a new review (your previous feedback still stands) + - Complete immediately with \`verdict: "NONE"\` and summary explaining no changes detected + + **Impact analysis (no todos needed):** + When reviewing changes, investigate impact on other files: + - If an import/export changed, \`rg\` for usages elsewhere + - If an API signature changed, check callers + + This investigation doesn't require todo items - it's part of reviewing. + Only add a todo if you discover a file that ALSO needs detailed review. + + **When to skip the final \`gh pr review\` command:** + Only if ALL of these are true: + - You previously APPROVED this PR + - You reviewed the new changes and found NO new issues + + In this case, use \`verdict: "NONE"\` in your completion JSON. + + **When you MUST submit a new review:** + - You previously requested changes (now verify if fixed -> approve or re-request) + - You found new issues in new commits + - Previous feedback wasn't addressed + + + + + Don't just read the diff - actively investigate using your tools: + + **Logic & Correctness:** + - Read the full file context: \`cat src/file.ts\` to understand surrounding code + - Check how similar functions handle edge cases: \`rg "function.*similar" src/\` + - Look for related error handling patterns: \`rg "catch|throw|error" src/path/\` + - Check test coverage: \`cat tests/file.test.ts\` or \`rg "describe.*FeatureName" test/\` + + **DRY - Find Duplicated Code:** + - Search for similar implementations: \`rg "pattern from new code" src/\` + - Look for existing utilities: \`rg "util|helper|common" src/ -l\` then read them + - Check if functionality already exists: \`rg "functionName|similar keyword"\` + - If you find duplication, suggest extracting to shared utility + + **Pattern Consistency:** + - Find similar files/components: \`ls src/components/\` or \`rg "export.*Component" src/\` + - Read existing patterns: \`cat src/similar-file.ts\` to see conventions + - Check naming conventions: \`rg "const.*=.*=>" src/\` for arrow function style + - Look at error handling patterns: \`rg "try.*catch" src/ -A5\` + - Check import organization in similar files + + **Dependencies & Impact:** + - Use \`dep_graph\` to understand what depends on changed files + - Search for usages of modified exports: \`rg "import.*{.*modifiedExport" src/\` + - Check if API changes break callers + + **External Research (when helpful):** + - If code uses an unfamiliar library/API, use \`web_fetch\` to read its documentation + - If you see an unusual pattern or potential issue, use \`web_search\` to research best practices + - Look up security advisories for packages: \`web_search({ query: "CVE lodash vulnerability" })\` + - Don't guess about library behavior - verify with documentation + + **Dependency Updates (package.json, requirements.txt, Cargo.toml, etc.):** + When you see dependency version changes, check for breaking changes: + + 1. **Identify the version bump type** using semantic versioning (MAJOR.MINOR.PATCH): + - MAJOR (e.g., 5.x -> 6.x): Breaking changes likely - MUST investigate + - MINOR (e.g., 5.1 -> 5.2): New features, should be safe - quick check + - PATCH (e.g., 5.1.0 -> 5.1.1): Bug fixes only - usually safe + + 2. **For MAJOR version bumps, you MUST:** + - Fetch the changelog/migration guide: + - npm packages: \`web_fetch({ url: "https://github.com/OWNER/REPO/releases" })\` + - Or search: \`web_search({ query: "package-name v6 migration guide breaking changes" })\` + - Identify breaking changes that affect the codebase + - Search for usage of deprecated/changed APIs: \`rg "oldApiName" src/\` + - Comment if breaking changes aren't addressed in the PR + + 3. **Common changelog locations:** + - GitHub releases: \`https://github.com/OWNER/REPO/releases\` + - CHANGELOG.md in repo: \`web_fetch({ url: "https://github.com/OWNER/REPO/blob/main/CHANGELOG.md" })\` + - Migration guides: \`web_search({ query: "package-name v5 to v6 migration" })\` + + 4. **What to flag:** + - Major bumps without corresponding code changes for breaking APIs + - Deprecated APIs still being used after upgrade + - Missing peer dependency updates + - Incompatible version combinations + + **Before commenting on style/patterns**, read 2-3 similar files to understand the project's conventions. + + + +${reviewGuidelines} + + + + Be kind, address code not people, explain *why*. + + When posting comments, prefix with severity: + - **REQUIRED**: Must fix before approval (bugs, security, breaking changes) + - **OPTIONAL**: Suggested improvement (cleaner code, better patterns) + - **NIT**: Minor polish (only if pattern is repeated or misleading) + + + When suggesting code changes, use markdown code blocks: + \`\`\`suggestion + // Your suggested code here + \`\`\` + + + + + + **MANDATORY COMPLETION SEQUENCE** + + **STEP 1: Complete ALL todos** + Run \`todo list\`. If ANY tasks are unchecked [ ], you MUST complete them before proceeding. + + **BLOCKING:** Do NOT proceed to STEP 2 until EVERY todo shows [x]. + + Your todo list is your commitment. If you added a file to the list, you must review it. + If you only want to review certain files (e.g., re-review of new commits only), then only + add those files to your todo list in the first place. + + **STEP 2: Submit the formal review** + You MUST call \`gh pr review\` (unless this is a re-review where you already approved and found no new issues). + + Choose ONE command based on your findings: + - \`gh pr review ${ctx.prNumber} --approve --body "Summary"\` - No blocking issues + - \`gh pr review ${ctx.prNumber} --request-changes --body "Summary"\` - Posted REQUIRED comments + - \`gh pr review ${ctx.prNumber} --comment --body "Summary"\` - Suggestions but nothing blocking + + **STEP 3: Output the completion JSON** + Immediately after submitting (or deciding to skip) the review, output this EXACT JSON structure: + + \`\`\`json + { + "completed": true, + "summary": "Brief summary of what was reviewed", + "commentsPosted": 3, + "verdict": "APPROVE" + } + \`\`\` + + The \`verdict\` must be: "APPROVE", "REQUEST_CHANGES", "COMMENT", or "NONE" (if skipped). + + **DO NOT STOP after listing todos.** You MUST continue to STEP 2 and STEP 3. + **DO NOT make additional tool calls** after the completion JSON - it terminates the loop. + + **Re-review exception:** + If you previously APPROVED this PR and found NO new issues in new commits, you may skip STEP 2. + But you MUST still do STEP 3 with \`verdict: "NONE"\`. + + +--- + +**Your workflow:** + +1. **Get PR context:** + - Run \`gh pr view ${ctx.prNumber} --json title,body\` to get PR info + - Check if body is empty/blank + - **If body is empty/blank, you MUST update it immediately:** + \`gh pr edit ${ctx.prNumber} --body $'## Summary\\n\\n\\n\\n## Changes\\n\\n- '\` + (Note: Use \`$'...'\` with \\n for newlines, NOT regular quotes which treat \\n as literal text) + - **Review previous comments (if they exist in context):** + - \`\` = your previous feedback on this PR + - \`\` = other reviewers' feedback + - Use \`rg\` to search for context about issues raised in these comments + + - **Determine your review scope:** + - **First-time review:** Create todos for all files in \`\` + - **Re-review:** Create todos ONLY for files changed since your last review + (diff from previous review SHA to current SHA - see \`\`) + - **Re-review (requested changes):** Also include files where you left feedback + + **Fetching patches (if not in \`\` above):** + **ALWAYS use --jq to filter** - this keeps lock files and build outputs out of your context. + + - Single file: \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/files --jq '.[] | select(.filename=="src/index.ts")'\` + - Multiple files: \`gh api ... --jq '[.[] | select(.filename=="src/a.ts" or .filename=="src/b.ts")]'\` + - By pattern: \`gh api ... --jq '[.[] | select(.filename | startswith("src/"))]'\` + + **NEVER** run without --jq: \`gh api .../files\` dumps ALL files (including lock files) into context + +2. **Review each file in your todo list (one at a time):** + For EACH file you added to your todos: + a. **Get the patch** (if not in \`\` above): + \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/files --jq '.[] | select(.filename=="")'\` + b. **Read the FULL file for context:** \`cat \` - Don't just look at the patch! + The diff only shows changed lines. Read the entire file to understand: + - How the changed code fits into the broader context + - What functions/variables are defined elsewhere in the file + - The overall structure and patterns used + c. **Check dependencies if needed:** Use \`dep_graph\` or \`rg\` to see what calls this code + d. **Look up documentation if needed:** Use \`web_fetch\` or \`web_search\` for unfamiliar libraries/patterns + e. **Review the changes WITH full file context:** Look for: + - Logic errors and edge cases the diff introduces + - Error handling gaps in the new code + - Inconsistencies with patterns in the rest of the file/codebase + - Breaking changes to function signatures that affect callers + - DRY violations - does similar code exist elsewhere? + f. **Post comments IMMEDIATELY** when you find issues - don't wait until later: + \`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments -f body="Your comment" -f path="file/path.ts" -f line=42 -f commit_id="${ctx.commitSha}"\` + g. **Mark the file as reviewed** in your todo list before moving to the next file + + **IMPORTANT:** + - Complete ALL files in your todo list before finishing. + - Use \`rg\` to search for additional context (usages, callers, related code) when needed. + - You have memory across files! If file B relates to file A, you can go back and comment on file A. + - Always read the FULL file, not just the diff - context matters! + +3. **Before submitting review, verify:** + - PR description is not blank (if it was, you should have updated it in step 1) + - **ALL files have been reviewed** (check your todo list - every file should be marked done) + - Complete any other items in your todo list + +4. **${isReReview && !forceFullReview ? "Submit review ONLY IF NEEDED" : "REQUIRED - Submit formal review"}:** + - Approve: \`gh pr review ${ctx.prNumber} --approve --body "Your summary"\` + - Request changes: \`gh pr review ${ctx.prNumber} --request-changes --body "Your summary"\` + - Comment: \`gh pr review ${ctx.prNumber} --comment --body "Your summary"\` +${isReReview && !forceFullReview ? ` + **RE-REVIEW: Do NOT submit a new review if:** + - You previously APPROVED and the new changes don't introduce any issues + - You have no new feedback to give + In this case, simply complete your task without calling \`gh pr review\`. +` : ""} + **Review summary should be concise:** + - Brief description of what the PR does (1-2 sentences) + - Key findings or concerns (if any) + - Your decision rationale + - **DO NOT list all the files you reviewed** - that's redundant since you review everything + +**CRITICAL: Only comment on code IN THE DIFF.** +- Use context (full file, dependencies) to UNDERSTAND the code +- But ONLY comment on lines that are actually changed in this PR +- Never comment on pre-existing code outside the diff + +**Comment guidelines:** +${ctx.blockingOnly ? "- BLOCKING-ONLY MODE: Only comment on critical issues that MUST be fixed (security, bugs, breaking changes)" : "- Focus on substantive issues: bugs, security problems, logic errors, significant design concerns\n- Skip minor style nits unless they indicate a real problem"} +- Be constructive and explain WHY something is an issue +- Include code suggestions when helpful + +**Line numbers:** +- Use the line number in the NEW version of the file (right side of diff) +- For lines starting with \`+\`, count from the @@ hunk header +- Always use commit_id="${ctx.commitSha}" +${isReReview && !forceFullReview ? "" : ` +**Remember: You MUST submit a formal review at the end using \`gh pr review\`.`} +`; + + // Add force full review context + if (forceFullReview) { + instructions += ` + + **FULL REVIEW MODE ENABLED** - Review ALL files in this PR. + + Even though you may have reviewed this PR before, you have been asked to perform + a complete review of all files. Ignore any re-review optimizations and review + every file as if this were a first-time review. + +`; + } + + return instructions; + }, +}; diff --git a/src/agent/skills/review-targeted.ts b/src/agent/skills/review-targeted.ts new file mode 100644 index 0000000..5354286 --- /dev/null +++ b/src/agent/skills/review-targeted.ts @@ -0,0 +1,115 @@ +import type { Skill, SkillContext } from "./types"; + +export const reviewTargetedSkill: Skill = { + name: "review-targeted", + description: "Review specific code areas and post inline comments. Use when the user asks to review, check, look at, or examine specific files, functions, or areas of code. Also use when asked to focus on specific concerns like security, performance, or error handling.", + + getInstructions(ctx: SkillContext): string { + const mention = ctx.interactiveMention; + if (!mention) { + return `Error: This skill requires an interactive mention context.`; + } + + let codeContext = ""; + if (mention.isReviewComment && mention.filePath) { + codeContext = ` +## Code Context +The user requested this review on an inline comment at: +- **File:** \`${mention.filePath}\` +${mention.line ? `- **Line:** ${mention.line}` : ""} +${mention.diffHunk ? ` +**Diff hunk context:** +\`\`\`diff +${mention.diffHunk} +\`\`\` +` : ""} + +This is likely the area they want you to focus on. +`; + } + + return `## Skill: Targeted Code Review + +Your task is to review the specific area the user requested and post inline comments for any issues found. + +## User's Request +**Author:** @${mention.commentAuthor} +**Message:** ${mention.userMessage} +${codeContext} + +## How to Identify What to Review + +1. **Parse the user's request** - Look for: + - Specific file names or paths mentioned + - Function or class names + - Areas of concern (security, performance, error handling, etc.) + - If on an inline comment, that's likely the focus area + +2. **If unclear**, focus on: + - The file where the comment was made (if inline) + - Files mentioned in the PR that match the user's keywords + - Common patterns: "review the auth code" โ†’ look for auth-related files + +## Review Steps + +1. **Fetch the relevant code:** + - Get the PR diff: \`gh pr diff ${ctx.prNumber}\` or specific files + - Read full files for context: \`cat \` + - Search for related code: \`rg "pattern" src/\` + +2. **Review with focus:** + - Apply the same rigor as a full review, but scoped to the requested area + - Look for bugs, security issues, logic errors + - Check error handling, edge cases, and test coverage + - Verify the code follows project patterns + +3. **Post inline comments for issues:** + \`\`\`bash + gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments \\ + -f body="**REQUIRED**: [description of issue]" \\ + -f path="file/path.ts" \\ + -f line=42 \\ + -f commit_id="${ctx.commitSha}" + \`\`\` + + Use severity prefixes: + - **REQUIRED**: Must fix (bugs, security, breaking changes) + - **OPTIONAL**: Suggested improvement + - **NIT**: Minor polish + +4. **Post a summary response:** + ${mention.isReviewComment + ? `\`gh api repos/${ctx.repo}/pulls/${ctx.prNumber}/comments -f body="[summary]" -F in_reply_to=${mention.commentId}\`` + : `\`gh api repos/${ctx.repo}/issues/${ctx.prNumber}/comments -f body="@${mention.commentAuthor} [summary]"\``} + + Include: + - What area you reviewed + - Number of issues found (if any) + - Overall assessment + +## Guidelines + +- **Stay focused** - Only review what was requested, don't expand to full PR review +- **Be thorough** in the scoped area - Apply full review rigor +- **Context matters** - Read full files, check dependencies, understand the code +- **Post comments immediately** - Don't wait until the end +- **Evidence required** - Back up claims with specific code references + +## Completion + +When you've reviewed the requested area and posted all comments, output: +\`\`\`json +{ + "completed": true, + "summary": "Reviewed [area] - posted N comments", + "commentsPosted": N, + "verdict": "NONE" +} +\`\`\` + +Note: Use \`verdict: "NONE"\` for targeted reviews - you're not submitting a formal full review. +If the user specifically asked you to approve or request changes, you can submit a formal review +and use the appropriate verdict. +`; + }, +}; diff --git a/src/agent/skills/types.ts b/src/agent/skills/types.ts new file mode 100644 index 0000000..41b194c --- /dev/null +++ b/src/agent/skills/types.ts @@ -0,0 +1,45 @@ +import { InteractiveMentionContext, TriggerContext, BotComment, ExistingReviewComment, RelatedRepo } from "../../types"; + +/** + * Context provided to skills for generating instructions. + */ +export interface SkillContext { + /** Repository in owner/repo format */ + repo: string; + /** PR number */ + prNumber: number; + /** Current commit SHA */ + commitSha: string; + /** List of all file paths in the repository */ + repoFilePaths: string[]; + /** Trigger context (re-review info, etc.) */ + triggerContext?: TriggerContext; + /** Interactive mention context if triggered by @codepress */ + interactiveMention?: InteractiveMentionContext; + /** Instructions from PR body (if any) */ + prBodyInstructions?: string; + /** Whether to only post blocking/critical issues */ + blockingOnly?: boolean; + /** Maximum turns allowed for the agent */ + maxTurns?: number | null; + /** Existing review comments from other reviewers */ + existingComments?: ExistingReviewComment[]; + /** Bot's own previous comments (for deduplication) */ + botPreviousComments?: BotComment[]; + /** Related repos for cross-repo context */ + relatedRepos?: RelatedRepo[]; + /** Pre-filtered PR files formatted section */ + prFilesFormatted?: string; +} + +/** + * Defines a skill that the agent can load and use. + */ +export interface Skill { + /** Unique identifier for the skill */ + name: string; + /** Description shown to the agent to help it decide when to use this skill */ + description: string; + /** Returns the full instructions when the skill is invoked */ + getInstructions(context: SkillContext): string; +} diff --git a/src/agent/tools/skill-tool.ts b/src/agent/tools/skill-tool.ts new file mode 100644 index 0000000..cdd45fb --- /dev/null +++ b/src/agent/tools/skill-tool.ts @@ -0,0 +1,62 @@ +import { tool } from "@openai/agents"; +import { z } from "zod"; +import { allSkills, getSkillByName } from "../skills"; +import type { SkillContext } from "../skills/types"; + +/** + * Creates the skill tool with the given context. + * The tool description lists all available skills so the agent can choose. + */ +export function createSkillTool(context: SkillContext) { + // Build the skill list for the description + const skillList = allSkills + .map( + (skill) => ` + ${skill.description} + `, + ) + .join("\n"); + + const description = `Load a skill to get detailed instructions for a specific task. +Skills provide specialized knowledge and step-by-step guidance for different types of work. + +**When to use this tool:** +- At the start of your task, to load the appropriate skill +- Use the skill that best matches what you need to do + + +${skillList} + + +**How to choose:** +- For full PR reviews (@codepress/review, PR opened, new commits): use "review-full" +- For questions about code (what/why/how, explanations): use "answer-question" +- For targeted review requests (check this function, review security): use "review-targeted" + +After loading a skill, follow its instructions completely.`; + + return tool({ + name: "skill", + description, + parameters: z.object({ + name: z + .string() + .describe("The skill name to load (from available_skills)"), + }), + execute: async ({ name }) => { + const skill = getSkillByName(name); + + if (!skill) { + const availableNames = allSkills.map((s) => s.name).join(", "); + return `Error: Skill "${name}" not found. Available skills: ${availableNames}`; + } + + // Get the instructions for this skill with the current context + const instructions = skill.getInstructions(context); + + return `# Skill Loaded: ${skill.name} + +${instructions}`; + }, + }); +} diff --git a/src/index.ts b/src/index.ts index e5c3189..9cb0098 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,7 @@ import { join } from "node:path"; import type { BotComment, ExistingReviewComment, + InteractiveMentionContext, RelatedRepo, TriggerContext, } from "./types"; @@ -26,6 +27,35 @@ interface TriggerConfig { runOnReviewRequested: boolean; runOnCommentTrigger: boolean; commentTriggerPhrase: string; + interactiveMentionPhrase: string; +} + +/** + * Extracts the user message from a comment body after the @codepress mention. + * Returns null if this is the full review trigger (@codepress/review). + */ +function extractInteractiveMention( + commentBody: string, + commentTriggerPhrase: string, + interactiveMentionPhrase: string, +): string | null { + // If it's the full review trigger, return null (not an interactive mention) + if (commentBody.includes(commentTriggerPhrase)) { + return null; + } + + // Check for @codepress mention (case-insensitive) + const mentionRegex = new RegExp(`${interactiveMentionPhrase}\\s*(.*)`, "is"); + const match = commentBody.match(mentionRegex); + + if (match) { + // Extract everything after @codepress + const userMessage = match[1].trim(); + // Return the user's message, or a default if they just said @codepress + return userMessage || "Please help me with this PR."; + } + + return null; } type TriggerEventType = TriggerContext["triggerEvent"]; @@ -49,14 +79,48 @@ function getTriggerEvent( if (eventName === "pull_request" && action === "review_requested") { return "review_requested"; } + + // Issue comment (PR conversation) if (eventName === "issue_comment" && action === "created") { const isPrComment = !!context.payload.issue?.pull_request; const commentBody = context.payload.comment?.body || ""; - const containsTrigger = commentBody.includes(config.commentTriggerPhrase); - if (isPrComment && containsTrigger) { + + if (isPrComment) { + // Check for full review trigger first (@codepress/review) + if (commentBody.includes(config.commentTriggerPhrase)) { + return "comment_trigger"; + } + // Check for interactive mention (@codepress without /review) + const interactiveMessage = extractInteractiveMention( + commentBody, + config.commentTriggerPhrase, + config.interactiveMentionPhrase, + ); + if (interactiveMessage !== null) { + return "interactive_mention"; + } + } + } + + // Pull request review comment (inline on code) + if (eventName === "pull_request_review_comment" && action === "created") { + const commentBody = context.payload.comment?.body || ""; + + // Check for full review trigger first + if (commentBody.includes(config.commentTriggerPhrase)) { return "comment_trigger"; } + // Check for interactive mention + const interactiveMessage = extractInteractiveMention( + commentBody, + config.commentTriggerPhrase, + config.interactiveMentionPhrase, + ); + if (interactiveMessage !== null) { + return "interactive_mention"; + } } + return "workflow_dispatch"; } @@ -117,26 +181,74 @@ function shouldRunAction( }; } - // Comment trigger + // Issue comment (PR conversation) if (eventName === "issue_comment" && action === "created") { const isPrComment = !!context.payload.issue?.pull_request; const commentBody = context.payload.comment?.body || ""; - const containsTrigger = commentBody.includes(config.commentTriggerPhrase); - if (isPrComment && containsTrigger) { + if (isPrComment) { + // Check for full review trigger first + if (commentBody.includes(config.commentTriggerPhrase)) { + return { + shouldRun: config.runOnCommentTrigger, + reason: config.runOnCommentTrigger + ? `Comment contains trigger phrase: ${config.commentTriggerPhrase}` + : "Comment trigger is disabled", + }; + } + + // Check for interactive mention + const interactiveMessage = extractInteractiveMention( + commentBody, + config.commentTriggerPhrase, + config.interactiveMentionPhrase, + ); + if (interactiveMessage !== null) { + return { + shouldRun: true, // Always run on interactive mentions + reason: `Interactive mention: @codepress`, + }; + } + } + + return { + shouldRun: false, + reason: isPrComment + ? "Comment does not mention @codepress" + : "Comment is not on a PR", + }; + } + + // Pull request review comment (inline on code) + if (eventName === "pull_request_review_comment" && action === "created") { + const commentBody = context.payload.comment?.body || ""; + + // Check for full review trigger first + if (commentBody.includes(config.commentTriggerPhrase)) { return { shouldRun: config.runOnCommentTrigger, reason: config.runOnCommentTrigger - ? `Comment contains trigger phrase: ${config.commentTriggerPhrase}` + ? `Inline comment contains trigger phrase: ${config.commentTriggerPhrase}` : "Comment trigger is disabled", }; } + // Check for interactive mention + const interactiveMessage = extractInteractiveMention( + commentBody, + config.commentTriggerPhrase, + config.interactiveMentionPhrase, + ); + if (interactiveMessage !== null) { + return { + shouldRun: true, // Always run on interactive mentions + reason: `Interactive mention on inline comment: @codepress`, + }; + } + return { shouldRun: false, - reason: isPrComment - ? "Comment does not contain trigger phrase" - : "Comment is not on a PR", + reason: "Inline comment does not mention @codepress", }; } @@ -216,6 +328,9 @@ async function run(): Promise { const runOnCommentTrigger = core.getBooleanInput("run_on_comment_trigger"); const commentTriggerPhrase = core.getInput("comment_trigger_phrase"); + // Interactive mention phrase (just @codepress without /review) + const interactiveMentionPhrase = "@codepress"; + // Check if action should run based on trigger configuration const shouldRun = shouldRunAction(github.context, { runOnPrOpened, @@ -223,6 +338,7 @@ async function run(): Promise { runOnReviewRequested, runOnCommentTrigger, commentTriggerPhrase, + interactiveMentionPhrase, }); if (!shouldRun.shouldRun) { @@ -309,8 +425,10 @@ async function run(): Promise { core.info(`Triggered by event: ${context.eventName}`); if (context.payload.pull_request) { + // pull_request and pull_request_review_comment events prNumber = context.payload.pull_request.number; } else if (context.payload.issue?.pull_request) { + // issue_comment event on a PR prNumber = context.payload.issue.number; } else if (context.eventName === "workflow_dispatch") { core.info("Workflow dispatched manually. Finding PR from branch..."); @@ -488,8 +606,42 @@ async function run(): Promise { runOnReviewRequested, runOnCommentTrigger, commentTriggerPhrase, + interactiveMentionPhrase, }); + // Parse interactive mention context if applicable + let interactiveMentionContext: InteractiveMentionContext | undefined; + if (triggerEvent === "interactive_mention") { + const comment = context.payload.comment; + const commentBody = comment?.body || ""; + const userMessage = extractInteractiveMention( + commentBody, + commentTriggerPhrase, + interactiveMentionPhrase, + ) || "Please help me with this PR."; + + // Determine if this is a review comment (inline on code) or issue comment (PR conversation) + const isReviewComment = context.eventName === "pull_request_review_comment"; + + interactiveMentionContext = { + userMessage, + commentId: comment?.id || 0, + commentAuthor: comment?.user?.login || "unknown", + commentBody, + isReviewComment, + // These fields are only available for review comments (inline on code) + filePath: isReviewComment ? comment?.path : undefined, + line: isReviewComment ? (comment?.line ?? comment?.original_line ?? undefined) : undefined, + diffHunk: isReviewComment ? comment?.diff_hunk : undefined, + }; + + core.info(`Interactive mention from @${interactiveMentionContext.commentAuthor}`); + core.info(`Message: ${userMessage.substring(0, 100)}${userMessage.length > 100 ? "..." : ""}`); + if (isReviewComment && interactiveMentionContext.filePath) { + core.info(`On file: ${interactiveMentionContext.filePath}:${interactiveMentionContext.line || "?"}`); + } + } + // Fetch the bot's previous review state let previousReviewState: TriggerContext["previousReviewState"] = null; let previousReviewCommitSha: string | null = null; @@ -539,6 +691,19 @@ async function run(): Promise { process.env.PREVIOUS_REVIEW_COMMIT_SHA = previousReviewCommitSha || ""; process.env.FORCE_FULL_REVIEW = forceFullReview.toString(); + // Write interactive mention context to file if applicable + const interactiveMentionFile = resolve("interactive-mention.json"); + if (interactiveMentionContext) { + writeFileSync( + interactiveMentionFile, + JSON.stringify(interactiveMentionContext, null, 2), + ); + process.env.INTERACTIVE_MENTION = "true"; + } else { + writeFileSync(interactiveMentionFile, "null"); + process.env.INTERACTIVE_MENTION = "false"; + } + if (isReReview) { core.info(`Re-review detected (trigger: ${triggerEvent})`); if (previousReviewCommitSha && previousReviewCommitSha !== commitSha) { diff --git a/src/review-service.ts b/src/review-service.ts index ad11dc9..94ab87f 100644 --- a/src/review-service.ts +++ b/src/review-service.ts @@ -8,6 +8,7 @@ import type { PRFile } from "./pr-files"; import type { BotComment, ExistingReviewComment, + InteractiveMentionContext, RelatedRepo, ReviewConfig, TriggerContext, @@ -73,12 +74,31 @@ export class ReviewService { const previousReviewCommitSha = process.env.PREVIOUS_REVIEW_COMMIT_SHA || null; const forceFullReview = process.env.FORCE_FULL_REVIEW === "true"; + // Load interactive mention context if applicable + let interactiveMention: InteractiveMentionContext | undefined; + const interactiveMentionFile = resolve("interactive-mention.json"); + if (existsSync(interactiveMentionFile)) { + try { + const data = readFileSync(interactiveMentionFile, "utf8"); + if (data !== "null") { + interactiveMention = JSON.parse(data); + if (interactiveMention) { + debugLog(`๐Ÿ’ฌ Interactive mention from @${interactiveMention.commentAuthor}`); + debugLog(` Message: ${interactiveMention.userMessage.substring(0, 80)}${interactiveMention.userMessage.length > 80 ? "..." : ""}`); + } + } + } catch { + debugLog("โš ๏ธ Failed to parse interactive mention file"); + } + } + const triggerContext: TriggerContext = { isReReview, triggerEvent, previousReviewState: previousReviewState || undefined, previousReviewCommitSha: previousReviewCommitSha || undefined, forceFullReview, + interactiveMention, }; // Build PR context for the agent diff --git a/src/types.ts b/src/types.ts index 8569ee5..7456888 100644 --- a/src/types.ts +++ b/src/types.ts @@ -28,6 +28,28 @@ export interface ParsedArgs { pr: number; } +/** + * Context for an interactive @codepress mention in a PR comment. + */ +export interface InteractiveMentionContext { + /** The user's message after @codepress */ + userMessage: string; + /** The GitHub comment ID */ + commentId: number; + /** The GitHub username of the person who made the comment */ + commentAuthor: string; + /** The full comment body */ + commentBody: string; + /** Whether this is a review comment (inline on code) vs issue comment (general PR comment) */ + isReviewComment: boolean; + /** File path if this is a review comment */ + filePath?: string; + /** Line number if this is a review comment */ + line?: number; + /** The diff hunk context if this is a review comment */ + diffHunk?: string; +} + /** * Context about how the review was triggered. * Used to determine re-review behavior. @@ -36,13 +58,15 @@ export interface TriggerContext { /** Whether this is a re-review (new commits pushed or re-review requested) */ isReReview: boolean; /** The event that triggered the review */ - triggerEvent: "opened" | "reopened" | "synchronize" | "review_requested" | "comment_trigger" | "workflow_dispatch"; + triggerEvent: "opened" | "reopened" | "synchronize" | "review_requested" | "comment_trigger" | "interactive_mention" | "workflow_dispatch"; /** The bot's previous review state on this PR, if any */ previousReviewState?: "APPROVED" | "CHANGES_REQUESTED" | "COMMENTED" | "PENDING" | null; /** The commit SHA of the previous review, if any */ previousReviewCommitSha?: string | null; /** Force a full review of all files, ignoring re-review optimizations */ forceFullReview?: boolean; + /** Interactive mention context if triggered by @codepress */ + interactiveMention?: InteractiveMentionContext; } /** diff --git a/test/agent-system-prompt.test.ts b/test/agent-system-prompt.test.ts index 983936f..827cdce 100644 --- a/test/agent-system-prompt.test.ts +++ b/test/agent-system-prompt.test.ts @@ -1,166 +1,58 @@ -import { getInteractiveSystemPrompt } from "../src/agent/agent-system-prompt"; -import { existsSync, readFileSync } from "fs"; -import { join } from "path"; +import { getSystemPrompt, getInteractiveSystemPrompt, INTERACTIVE_SYSTEM_PROMPT } from "../src/agent/agent-system-prompt"; -// Mock the fs module -jest.mock("fs", () => ({ - existsSync: jest.fn(), - readFileSync: jest.fn(), -})); +describe("getSystemPrompt", () => { + it("should return a minimal system prompt", () => { + const prompt = getSystemPrompt(); -const mockedExistsSync = existsSync as jest.MockedFunction; -const mockedReadFileSync = readFileSync as jest.MockedFunction< - typeof readFileSync ->; + // Should identify the agent + expect(prompt).toContain("CodePress"); + expect(prompt).toContain("code review assistant"); -describe("getInteractiveSystemPrompt", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should use default guidelines when no custom files exist", () => { - mockedExistsSync.mockReturnValue(false); - - const prompt = getInteractiveSystemPrompt(false, 75); - - expect(prompt).toContain("autonomous code-review agent"); - expect(prompt).toContain(""); - expect(prompt).not.toContain(""); - }); - - it("should replace defaults when custom-codepress-review-prompt.md exists", () => { - const customPrompt = "# My Custom Guidelines\n\nCustom review rules here."; - - mockedExistsSync.mockImplementation((path) => { - return (path as string).includes("custom-codepress-review-prompt.md"); - }); - mockedReadFileSync.mockReturnValue(customPrompt); - - const prompt = getInteractiveSystemPrompt(false, 75); - - expect(prompt).toContain("My Custom Guidelines"); - expect(prompt).toContain("Custom review rules here"); - expect(prompt).not.toContain(""); - }); - - it("should append rules when codepress-review-rules.md exists", () => { - const additionalRules = - "## Security\n- All queries must be parameterized"; - - mockedExistsSync.mockImplementation((path) => { - return (path as string).includes("codepress-review-rules.md"); - }); - mockedReadFileSync.mockReturnValue(additionalRules); - - const prompt = getInteractiveSystemPrompt(false, 75); - - // Should have both defaults AND additional rules - expect(prompt).toContain(""); - expect(prompt).toContain(""); - expect(prompt).toContain("All queries must be parameterized"); - expect(prompt).toContain( - "these project-specific rules take precedence", - ); - }); - - it("should use custom prompt AND append rules when both files exist", () => { - const customPrompt = "# Custom Base Guidelines"; - const additionalRules = "## Extra Rules\n- Must have tests"; + // Should mention the skill tool + expect(prompt).toContain("skill"); + expect(prompt).toContain("review-full"); + expect(prompt).toContain("answer-question"); + expect(prompt).toContain("review-targeted"); - mockedExistsSync.mockReturnValue(true); - mockedReadFileSync.mockImplementation((path) => { - if ((path as string).includes("custom-codepress-review-prompt.md")) { - return customPrompt; - } - if ((path as string).includes("codepress-review-rules.md")) { - return additionalRules; - } - return ""; - }); - - const prompt = getInteractiveSystemPrompt(false, 75); - - // Should have custom prompt (not defaults) AND additional rules - expect(prompt).toContain("Custom Base Guidelines"); - expect(prompt).not.toContain(""); - expect(prompt).toContain(""); - expect(prompt).toContain("Must have tests"); - }); - - it("should handle errors reading custom prompt file gracefully", () => { - mockedExistsSync.mockImplementation((path) => { - return (path as string).includes("custom-codepress-review-prompt.md"); - }); - mockedReadFileSync.mockImplementation(() => { - throw new Error("Permission denied"); - }); - - const consoleSpy = jest - .spyOn(console, "warn") - .mockImplementation(() => {}); - - const prompt = getInteractiveSystemPrompt(false, 75); - - // Should fall back to defaults - expect(prompt).toContain(""); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining("Failed to read custom prompt file"), - ); - - consoleSpy.mockRestore(); + // Should describe the completion schema + expect(prompt).toContain("completed"); + expect(prompt).toContain("summary"); + expect(prompt).toContain("verdict"); }); - it("should handle errors reading rules file gracefully", () => { - mockedExistsSync.mockImplementation((path) => { - return (path as string).includes("codepress-review-rules.md"); - }); - mockedReadFileSync.mockImplementation(() => { - throw new Error("Permission denied"); - }); + it("should include all skill names for reference", () => { + const prompt = getSystemPrompt(); - const consoleSpy = jest - .spyOn(console, "warn") - .mockImplementation(() => {}); - - const prompt = getInteractiveSystemPrompt(false, 75); - - // Should have defaults but no project rules section - expect(prompt).toContain(""); - expect(prompt).not.toContain(""); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining("Failed to read additional rules file"), - ); - - consoleSpy.mockRestore(); + expect(prompt).toContain("review-full"); + expect(prompt).toContain("answer-question"); + expect(prompt).toContain("review-targeted"); }); - it("should include blocking-only mode instructions when enabled", () => { - mockedExistsSync.mockReturnValue(false); - - const prompt = getInteractiveSystemPrompt(true, 75); + it("should describe the completion JSON schema", () => { + const prompt = getSystemPrompt(); - expect(prompt).toContain("BLOCKING-ONLY MODE"); - expect(prompt).toContain("Security vulnerabilities"); + expect(prompt).toContain('"completed": true'); + expect(prompt).toContain("APPROVE"); + expect(prompt).toContain("REQUEST_CHANGES"); + expect(prompt).toContain("COMMENT"); + expect(prompt).toContain("NONE"); }); +}); - it("should include turn budget in the prompt", () => { - mockedExistsSync.mockReturnValue(false); - - const prompt = getInteractiveSystemPrompt(false, 50); +describe("getInteractiveSystemPrompt (backward compatibility)", () => { + it("should return the minimal system prompt regardless of parameters", () => { + const prompt1 = getInteractiveSystemPrompt(false, 75); + const prompt2 = getInteractiveSystemPrompt(true, 50); + const prompt3 = getInteractiveSystemPrompt(false, null); - expect(prompt).toContain("50 turns"); + // All should return the same minimal prompt + expect(prompt1).toBe(prompt2); + expect(prompt2).toBe(prompt3); + expect(prompt1).toContain("CodePress"); }); - it("should check for files in the current working directory", () => { - mockedExistsSync.mockReturnValue(false); - - getInteractiveSystemPrompt(false, 75); - - expect(mockedExistsSync).toHaveBeenCalledWith( - join(process.cwd(), "custom-codepress-review-prompt.md"), - ); - expect(mockedExistsSync).toHaveBeenCalledWith( - join(process.cwd(), "codepress-review-rules.md"), - ); + it("should equal INTERACTIVE_SYSTEM_PROMPT constant", () => { + const prompt = getInteractiveSystemPrompt(false, null); + expect(prompt).toBe(INTERACTIVE_SYSTEM_PROMPT); }); }); diff --git a/test/skills.test.ts b/test/skills.test.ts new file mode 100644 index 0000000..e45e720 --- /dev/null +++ b/test/skills.test.ts @@ -0,0 +1,250 @@ +import { allSkills, getSkillByName, reviewFullSkill, answerQuestionSkill, reviewTargetedSkill } from "../src/agent/skills"; +import type { SkillContext } from "../src/agent/skills/types"; + +describe("Skills Registry", () => { + it("should export all skills", () => { + expect(allSkills).toHaveLength(3); + expect(allSkills).toContain(reviewFullSkill); + expect(allSkills).toContain(answerQuestionSkill); + expect(allSkills).toContain(reviewTargetedSkill); + }); + + it("should find skills by name", () => { + expect(getSkillByName("review-full")).toBe(reviewFullSkill); + expect(getSkillByName("answer-question")).toBe(answerQuestionSkill); + expect(getSkillByName("review-targeted")).toBe(reviewTargetedSkill); + expect(getSkillByName("nonexistent")).toBeUndefined(); + }); +}); + +describe("review-full skill", () => { + const baseContext: SkillContext = { + repo: "owner/repo", + prNumber: 123, + commitSha: "abc123", + repoFilePaths: ["src/index.ts", "src/utils.ts"], + }; + + it("should have correct name and description", () => { + expect(reviewFullSkill.name).toBe("review-full"); + expect(reviewFullSkill.description).toContain("complete code review"); + }); + + it("should generate instructions with PR context", () => { + const instructions = reviewFullSkill.getInstructions(baseContext); + + expect(instructions).toContain("PR #123"); + expect(instructions).toContain("owner/repo"); + expect(instructions).toContain("abc123"); + }); + + it("should include blocking mode instructions when enabled", () => { + const blockingContext: SkillContext = { + ...baseContext, + blockingOnly: true, + }; + + const instructions = reviewFullSkill.getInstructions(blockingContext); + + expect(instructions).toContain("BLOCKING-ONLY MODE"); + expect(instructions).toContain("CRITICAL"); + }); + + it("should include turn budget when specified", () => { + const context: SkillContext = { + ...baseContext, + maxTurns: 50, + }; + + const instructions = reviewFullSkill.getInstructions(context); + + expect(instructions).toContain("50 turns"); + }); + + it("should indicate unlimited turns when maxTurns is null", () => { + const context: SkillContext = { + ...baseContext, + maxTurns: null, + }; + + const instructions = reviewFullSkill.getInstructions(context); + + expect(instructions).toContain("unlimited turns"); + }); + + it("should include re-review context when applicable", () => { + const context: SkillContext = { + ...baseContext, + triggerContext: { + isReReview: true, + triggerEvent: "synchronize", + previousReviewState: "APPROVED", + previousReviewCommitSha: "def456", + }, + }; + + const instructions = reviewFullSkill.getInstructions(context); + + expect(instructions).toContain("RE-REVIEW"); + }); + + it("should include force full review context when applicable", () => { + const context: SkillContext = { + ...baseContext, + triggerContext: { + isReReview: true, + triggerEvent: "comment_trigger", + forceFullReview: true, + }, + }; + + const instructions = reviewFullSkill.getInstructions(context); + + expect(instructions).toContain("FULL REVIEW MODE ENABLED"); + }); + + it("should include review guidelines", () => { + const instructions = reviewFullSkill.getInstructions(baseContext); + + // Check for key review principles + expect(instructions).toContain("reviewPrinciples"); + expect(instructions).toContain("logical errors"); + }); + + it("should include gh CLI commands for posting comments", () => { + const instructions = reviewFullSkill.getInstructions(baseContext); + + expect(instructions).toContain("gh api repos/owner/repo/pulls/123/comments"); + expect(instructions).toContain("gh pr review 123"); + }); +}); + +describe("answer-question skill", () => { + const baseContext: SkillContext = { + repo: "owner/repo", + prNumber: 123, + commitSha: "abc123", + repoFilePaths: [], + interactiveMention: { + userMessage: "What does this function do?", + commentId: 456, + commentAuthor: "testuser", + commentBody: "@codepress What does this function do?", + isReviewComment: false, + }, + }; + + it("should have correct name and description", () => { + expect(answerQuestionSkill.name).toBe("answer-question"); + expect(answerQuestionSkill.description).toContain("question"); + }); + + it("should include user message in instructions", () => { + const instructions = answerQuestionSkill.getInstructions(baseContext); + + expect(instructions).toContain("What does this function do?"); + expect(instructions).toContain("@testuser"); + }); + + it("should include reply command for issue comments", () => { + const instructions = answerQuestionSkill.getInstructions(baseContext); + + expect(instructions).toContain("gh api repos/owner/repo/issues/123/comments"); + }); + + it("should include reply command for review comments", () => { + const reviewContext: SkillContext = { + ...baseContext, + interactiveMention: { + ...baseContext.interactiveMention!, + isReviewComment: true, + filePath: "src/index.ts", + line: 42, + diffHunk: "@@ -10,5 +10,8 @@", + }, + }; + + const instructions = answerQuestionSkill.getInstructions(reviewContext); + + expect(instructions).toContain("in_reply_to=456"); + expect(instructions).toContain("src/index.ts"); + expect(instructions).toContain("Line"); + expect(instructions).toContain("42"); + }); + + it("should return error when no interactive mention context", () => { + const noMentionContext: SkillContext = { + ...baseContext, + interactiveMention: undefined, + }; + + const instructions = answerQuestionSkill.getInstructions(noMentionContext); + + expect(instructions).toContain("Error"); + }); +}); + +describe("review-targeted skill", () => { + const baseContext: SkillContext = { + repo: "owner/repo", + prNumber: 123, + commitSha: "abc123", + repoFilePaths: [], + interactiveMention: { + userMessage: "Please check the error handling in this file", + commentId: 456, + commentAuthor: "testuser", + commentBody: "@codepress Please check the error handling in this file", + isReviewComment: true, + filePath: "src/utils.ts", + line: 50, + diffHunk: "@@ -45,10 +45,15 @@", + }, + }; + + it("should have correct name and description", () => { + expect(reviewTargetedSkill.name).toBe("review-targeted"); + expect(reviewTargetedSkill.description).toContain("specific code areas"); + }); + + it("should include user request in instructions", () => { + const instructions = reviewTargetedSkill.getInstructions(baseContext); + + expect(instructions).toContain("error handling"); + expect(instructions).toContain("@testuser"); + }); + + it("should include code context for review comments", () => { + const instructions = reviewTargetedSkill.getInstructions(baseContext); + + expect(instructions).toContain("src/utils.ts"); + expect(instructions).toContain("Line"); + expect(instructions).toContain("50"); + expect(instructions).toContain("@@ -45,10 +45,15 @@"); + }); + + it("should include inline comment posting command", () => { + const instructions = reviewTargetedSkill.getInstructions(baseContext); + + expect(instructions).toContain("gh api repos/owner/repo/pulls/123/comments"); + expect(instructions).toContain("commit_id"); + expect(instructions).toContain("abc123"); + }); + + it("should use verdict NONE for targeted reviews", () => { + const instructions = reviewTargetedSkill.getInstructions(baseContext); + + expect(instructions).toContain('verdict: "NONE"'); + }); + + it("should return error when no interactive mention context", () => { + const noMentionContext: SkillContext = { + ...baseContext, + interactiveMention: undefined, + }; + + const instructions = reviewTargetedSkill.getInstructions(noMentionContext); + + expect(instructions).toContain("Error"); + }); +});