diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index 6d4c2d7fb..3e94e4e0b 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -91,12 +91,12 @@ runs: role-session-name: GitHubActions-StrandsAgent-${{ github.run_id }} aws-region: us-west-2 mask-aws-account-id: true - inline_session_policy: >- + inline-session-policy: >- { "Version": "2012-10-17", "Statement": [ { - "Sid":"Bedrock Access", + "Sid":"BedrockAccess", "Effect": "Allow", "Action": [ "bedrock:InvokeModelWithResponseStream", @@ -108,16 +108,16 @@ runs: "Action": [ "s3:PutObject", "s3:GetObject", - "s3:DeleteObject", + "s3:DeleteObject" ], "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions/*", + "arn:aws:s3:::strands-typescript-project-sessions/*" ] }, { "Effect": "Allow", "Action": "s3:ListBucket", "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions", + "arn:aws:s3:::strands-typescript-project-sessions" ] } ] diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md new file mode 100644 index 000000000..7281a5119 --- /dev/null +++ b/.github/agent-sops/task-reviewer.sop.md @@ -0,0 +1,218 @@ +# Task Reviewer SOP + +## Role + +You are a Task Reviewer, and your goal is to review code changes in a pull request and provide constructive feedback to improve code quality, maintainability, and adherence to project standards. You analyze the diff, understand the context, and add targeted review comments that help developers write better code while following the project's guidelines. + +## Steps + +### 1. Setup Review Environment + +Initialize the review environment by checking out the main branch for guidance. + +**Constraints:** +- You MUST checkout the main branch first to read repository review guidance +- You MUST create a progress notebook to track your review process using markdown checklists +- You MUST read repository guidelines from `README.md`, `CONTRIBUTING.md`, and `AGENTS.md` (if present) +- You MUST create a checklist of items to review based on the repository guidelines + +### 2. Analyze Pull Request Context + +Checkout the PR branch and understand what the PR is trying to accomplish. + +**Constraints:** +- You MUST checkout the PR branch to review the actual changes +- You MUST read the pull request description and understand the purpose of the changes +- You MUST note the PR number and branch name in your notebook +- You MUST identify the type of changes (feature, bugfix, refactor, etc.) +- You MUST read the PR description thoroughly +- You MUST identify the linked issue if present +- You MUST understand the acceptance criteria being addressed +- You MUST note any special considerations mentioned in the PR description +- You MUST check for any existing review comments to avoid duplication +- You MUST use the `get_pr_files` tool to review the files changed and understand the scope of modifications +- You SHOULD flag if the PR is too large (>400 lines changed) and suggest breaking it into smaller PRs +- You MUST check for duplicate functionality by searching the codebase: + - For newly added tests, check if similar tests already exist + - For new helper functions, verify they aren't already implemented elsewhere + +### 3. Code Analysis Phase + +Perform a comprehensive analysis of the code changes. + +#### 3.1 Structural Review + +Analyze the overall structure and architecture of the changes. + +**Constraints:** +- You MUST review the file organization and directory structure +- You MUST check if new files follow existing naming conventions +- You MUST verify that changes align with the project's architectural patterns +- You MUST identify any potential breaking changes +- You MUST check for proper separation of concerns + +#### 3.2 Code Quality Review + +Examine the code for quality, readability, and maintainability issues. + +**Constraints:** +- You MUST check for language-specific best practices as defined in repository guidelines +- You MUST verify code is readable with clear variable/function names and logical structure +- You MUST check that code is maintainable with modular design and loose coupling +- You MUST check for code complexity and suggest simplifications +- You MUST identify unclear or confusing code patterns +- You MUST verify proper error handling +- You MUST check for potential performance issues +- You MUST verify design decisions are documented (why certain patterns were chosen, alternatives considered, tradeoffs made) + +#### 3.3 Testing Review + +Analyze the test coverage and quality of tests. + +**Constraints:** +- You MUST verify that new functionality has corresponding tests +- You MUST check that tests follow the patterns defined in repository documentation +- You MUST ensure tests are in the correct directories as specified in guidelines +- You MUST check for proper test organization and naming +- You MUST identify missing edge cases or error scenarios +- You MUST verify integration tests are included when appropriate + +#### 3.4 Documentation Review + +Check documentation completeness and quality. + +**Constraints:** +- You MUST verify documentation exists for all public APIs as required by repository guidelines +- You MUST check that documentation is clear, helpful, and concise +- You MAY suggest examples for complex APIs +- You MUST verify that README.md updates are included if needed +- You MUST check that development documentation is updated if patterns changed + +### 4. Generate Review Comments + +Create specific, actionable review comments for identified issues. + +**Constraints:** +- You MUST focus on the most impactful improvements first +- You MUST provide specific suggestions rather than vague feedback +- You MUST be concise in your feedback +- You MUST avoid nitpicking on minor style issues (nits) - focus on substantive problems: + - Nits include: comment wording, code organization preferences, bracket/semicolon position, filename conventions + - Substantive issues include: bugs, security vulnerabilities, performance problems, maintainability concerns +- You MUST assume positive intent from the code author +- You MUST categorize feedback as: + - **Critical**: Must be fixed (security, breaking changes, major bugs) + - **Important**: Should be fixed (quality, maintainability, standards) + - **Suggestion**: Nice to have (optimizations, style preferences) +- You MUST be constructive and educational in your feedback +- You MUST prioritize feedback that helps the developer learn and improve +- You MAY skip this step if you have no feedback to provide + +#### 4.1 Comment Structure + +Format review comments to be clear and actionable. + +**Constraints:** +- You MUST be concise - avoid verbose explanations +- You MUST provide specific suggestions +- You MAY reference documentation or standards when applicable +- You SHOULD use this format: + ``` + **Issue**: [Brief description] + **Suggestion**: [Specific recommendation] + ``` + +### 5. Post Review Comments + +Add the review comments to the pull request. + +**Constraints:** +- You MUST use the `add_pr_comment` tool for inline comments on specific lines +- You MUST use the `add_pr_comment` tool with no line number for file-level comments +- You MUST use the `reply_to_review_comment` tool to reply to existing inline comments +- You MUST group related comments when possible +- You MUST avoid overwhelming the author with too many minor comments +- You MUST prioritize the most important feedback +- You MUST be respectful and professional in all comments +- You SHOULD limit to 10-15 comments per review to avoid overwhelming the author +- You MUST focus on teaching moments that help the developer improve + +### 6. Summary Review Comment + +Provide a concise overall summary of the review. + +**Constraints:** +- You MUST create a pull request review using GitHub's review feature +- You MUST provide an overall assessment (Approve, Request Changes, Comment) +- You MUST keep the summary concise - rely on GitHub's UI to display individual comments +- You MUST highlight key themes or patterns in the feedback +- You SHOULD use this format: + ``` + **Assessment**: [Approve/Request Changes/Comment] + + **Key Themes**: [High-level patterns or areas needing attention] + + [Brief encouraging note] + ``` + +## Review Focus Areas + +### Code Quality Priorities + +Focus on substantive issues that impact code quality, not stylistic preferences: + +1. **Functionality**: Does the code work as intended? Are edge cases and error conditions handled? +2. **Readability**: Is the code clear with descriptive names and logical structure? +3. **Maintainability**: Is the code modular, loosely coupled, and easy to modify in the future? +4. **Security**: Are there vulnerabilities or data exposure risks? +5. **Performance**: Are there bottlenecks or inefficient algorithms? +6. **Testing**: Is there comprehensive test coverage including edge cases? +7. **Language Best Practices**: Does it follow language-specific best practices as defined in repository guidelines? +8. **Design Documentation**: Are design decisions, alternatives, and tradeoffs documented? + +## Best Practices + +### Review Efficiency +- Focus on the most impactful issues first +- Provide specific, actionable feedback +- Be concise and avoid verbose explanations +- Reference project standards and documentation when applicable +- Be educational and constructive + +### Communication +- Be respectful and professional +- Assume positive intent from the code author +- Acknowledge good practices +- Explain the reasoning behind feedback +- Provide learning opportunities +- Encourage the developer +- Focus on ideas for improving the system, not criticisms of the author + +### Quality Gates +- Ensure critical issues are marked as blocking +- Verify tests meet repository requirements +- Check language-specific compliance as defined in guidelines +- Validate documentation completeness + +## Troubleshooting + +### Large Pull Requests +If the PR is very large: +- Focus on architectural and design issues first +- Prioritize critical bugs and security issues +- Suggest breaking the PR into smaller pieces if appropriate +- Provide high-level feedback on structure and approach + +### Complex Changes +For complex technical changes: +- Take time to understand the full context +- Ask clarifying questions if needed +- Focus on maintainability and future extensibility +- Verify that the solution aligns with project guidelines + +### Disagreements +If you disagree with the approach: +- Explain your reasoning clearly +- Reference project guidelines and standards +- Suggest alternative approaches +- Be open to discussion and learning diff --git a/.github/scripts/javascript/process-input.cjs b/.github/scripts/javascript/process-input.cjs index b7ed29263..8e44e5241 100644 --- a/.github/scripts/javascript/process-input.cjs +++ b/.github/scripts/javascript/process-input.cjs @@ -70,7 +70,8 @@ function buildPrompts(mode, issueId, isPullRequest, command, branchName, inputs) const scriptFiles = { 'implementer': '.github/agent-sops/task-implementer.sop.md', 'refiner': '.github/agent-sops/task-refiner.sop.md', - 'release-notes': '.github/agent-sops/task-release-notes.sop.md' + 'release-notes': '.github/agent-sops/task-release-notes.sop.md', + 'reviewer': '.github/agent-sops/task-reviewer.sop.md' }; const scriptFile = scriptFiles[mode] || scriptFiles['refiner']; @@ -96,6 +97,8 @@ module.exports = async (context, github, core, inputs) => { mode = 'release-notes'; } else if (command.startsWith('implement')) { mode = 'implementer'; + } else if (command.startsWith('review')) { + mode = "reviewer"; } else if (command.startsWith('refine')) { mode = 'refiner'; } else { diff --git a/.github/scripts/python/agent_runner.py b/.github/scripts/python/agent_runner.py index fcde83b66..e131a77a4 100644 --- a/.github/scripts/python/agent_runner.py +++ b/.github/scripts/python/agent_runner.py @@ -20,11 +20,13 @@ # Import local GitHub tools we need from github_tools import ( add_issue_comment, + add_pr_comment, create_issue, create_pull_request, get_issue, get_issue_comments, get_pull_request, + get_pr_files, get_pr_review_and_comments, list_issues, list_pull_requests, @@ -69,8 +71,10 @@ def _get_all_tools() -> list[Any]: get_pull_request, update_pull_request, list_pull_requests, + get_pr_files, get_pr_review_and_comments, reply_to_review_comment, + add_pr_comment, # Agent tools notebook, diff --git a/.github/scripts/python/github_tools.py b/.github/scripts/python/github_tools.py index 8826b4611..df64628b5 100644 --- a/.github/scripts/python/github_tools.py +++ b/.github/scripts/python/github_tools.py @@ -450,6 +450,114 @@ def reply_to_review_comment(pr_number: int, comment_id: int, reply_text: str, re return message +@tool +@log_inputs +@check_should_call_write_api_or_record +def add_pr_comment(pr_number: int, body: str, path: str | None = None, line: int | None = None, repo: str | None = None) -> str: + """Adds a comment to a pull request - either inline on a specific line, file-level, or general PR comment. + + Args: + pr_number: The pull request number + body: The comment text + path: The file path to comment on (optional; if omitted, creates general PR comment) + line: The line number to comment on (optional; if omitted with path, creates file-level comment) + repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) + + Returns: + Result of the operation + """ + # If no path provided, create a general PR comment (issue comment) + if path is None: + result = _github_request("POST", f"issues/{pr_number}/comments", repo, {"body": body}) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + message = f"PR comment added: {result['html_url']}" + console.print(Panel(escape(f"Comment: {body}\nURL: {result['html_url']}"), + title="[bold green]✅ PR Comment Added", border_style="green")) + return message + + # Get the latest commit SHA for the PR + pr_result = _github_request("GET", f"pulls/{pr_number}", repo) + if isinstance(pr_result, str): + console.print(Panel(escape(pr_result), title="[bold red]Error", border_style="red")) + return pr_result + + commit_sha = pr_result['head']['sha'] + + # Create inline or file-level comment + comment_data = { + "body": body, + "commit_id": commit_sha, + "path": path + } + + # Add line number if provided (inline comment), otherwise it's a file-level comment + if line is not None: + comment_data["line"] = line + + result = _github_request("POST", f"pulls/{pr_number}/comments", repo, comment_data) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + comment_type = "Inline" if line else "File-level" + location = f"{path}:{line}" if line else path + message = f"{comment_type} comment added: {result['html_url']}" + comment_details = f"Location: {location}\nComment: {body}\nURL: {result['html_url']}" + console.print(Panel(escape(comment_details), title=f"[bold green]✅ {comment_type} Comment Added", border_style="green")) + return message + + +@tool +@log_inputs +def get_pr_files(pr_number: int, repo: str | None = None) -> str: + """Gets the list of files changed in a pull request with their diffs. + + Args: + pr_number: The pull request number + repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) + + Returns: + Formatted list of changed files with their diffs + """ + result = _github_request("GET", f"pulls/{pr_number}/files", repo) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + output = f"Files changed in PR #{pr_number}:\n\n" + + for file in result: + filename = file['filename'] + status = file['status'] + additions = file['additions'] + deletions = file['deletions'] + changes = file['changes'] + + output += f"📄 **{filename}** ({status})\n" + output += f" +{additions} -{deletions} (~{changes} changes)\n" + + if file.get('patch'): + output += f" Diff:\n" + # Limit diff size to avoid overwhelming output + patch_lines = file['patch'].split('\n') + if len(patch_lines) > 50: + output += '\n'.join(patch_lines[:50]) + output += f"\n ... (truncated, {len(patch_lines) - 50} more lines)\n" + else: + output += file['patch'] + output += "\n" + else: + output += " (Binary file or no diff available)\n" + + output += "\n" + + console.print(Panel(escape(output), title=f"[bold green]PR #{pr_number} Files", border_style="blue")) + return output + + # ============================================================================= # READ FUNCTIONS (Functions that only read GitHub resources) # ============================================================================= diff --git a/.github/scripts/python/write_executor.py b/.github/scripts/python/write_executor.py index 6d3b6b84d..648f6f73a 100755 --- a/.github/scripts/python/write_executor.py +++ b/.github/scripts/python/write_executor.py @@ -23,6 +23,7 @@ create_pull_request, update_pull_request, reply_to_review_comment, + add_pr_comment, ) # Configure structured logging @@ -43,6 +44,7 @@ def get_function_mapping() -> Dict[str, Any]: create_pull_request.tool_name: create_pull_request, update_pull_request.tool_name: update_pull_request, reply_to_review_comment.tool_name: reply_to_review_comment, + add_pr_comment.tool_name: add_pr_comment, } diff --git a/package-lock.json b/package-lock.json index 732798594..e33fa9ca0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4571,6 +4571,7 @@ "integrity": "sha512-gqkrWUsS8hcm0r44yn7/xZeV1ERva/nLgrLxFRUGb7aoNMIJfZJ3AC261zDQuOAKC7MiXai1WCpYc48jAHoShQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -4610,6 +4611,7 @@ "integrity": "sha512-N9lBGA9o9aqb1hVMc9hzySbhKibHmB+N3IpoShyV6HyQYRGIhlrO5rQgttypi+yEeKsKI4idxC8Jw6gXKD4THA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.49.0", "@typescript-eslint/types": "8.49.0", @@ -4814,6 +4816,7 @@ "integrity": "sha512-zedtczX688KehaIaAv7m25CeDLb0gBtAOa2Oi1G1cqvSO5aLSVfH6lpZMJLW8BKYuWMxLQc9/5GYoM+jgvGIrw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/mocker": "4.0.15", "@vitest/utils": "4.0.15", @@ -4837,6 +4840,7 @@ "integrity": "sha512-94yVpDbb+ykiT7mK6ToonGnq2GIHEQGBTZTAzGxBGQXcVNCh54YKC2/WkfaDzxy0m6Kgw05kq3FYHKHu+wRdIA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/browser": "4.0.15", "@vitest/mocker": "4.0.15", @@ -5017,6 +5021,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -5507,6 +5512,7 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -5961,6 +5967,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -7116,6 +7123,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -7151,6 +7159,7 @@ "integrity": "sha512-ilYQj1s8sr2ppEJ2YVadYBN0Mb3mdo9J0wQ+UuDhzYqURwSoW4n1Xs5vs7ORwgDGmyEh33tRMeS8KhdkMoLXQw==", "dev": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "playwright-core": "1.57.0" }, @@ -8376,6 +8385,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -8425,6 +8435,7 @@ "integrity": "sha512-NL8jTlbo0Tn4dUEXEsUg8KeyG/Lkmc4Fnzb8JXN/Ykm9G4HNImjtABMJgkQoVjOBN/j2WAwDTRytdqJbZsah7w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -8515,6 +8526,7 @@ "integrity": "sha512-n1RxDp8UJm6N0IbJLQo+yzLZ2sQCDyl1o0LeugbPWf8+8Fttp29GghsQBjYJVmWq3gBFfe9Hs1spR44vovn2wA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "4.0.15", "@vitest/mocker": "4.0.15", @@ -8641,6 +8653,7 @@ "integrity": "sha512-PEIGCY5tSlUt50cqyMXfCzX+oOPqN0vuGqWzbcJ2xvnkzkq46oOpz7dQaTDBdfICb4N14+GARUDw2XV2N4tvzg==", "devOptional": true, "license": "MIT", + "peer": true, "engines": { "node": ">=10.0.0" }, @@ -8675,6 +8688,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.5.tgz", "integrity": "sha512-k7Nwx6vuWx1IJ9Bjuf4Zt1PEllcwe7cls3VNzm4CQ1/hgtFUK2bRNG3rvnpPUhFjmqJKAKtjV576KnUkHocg/g==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" }