feat: add code-review workflow task #21103
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,382 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This workflow performs AI-powered code review on PRs. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # It creates a Coder Task that uses AI to analyze PR changes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # review code quality, identify issues, and post committable suggestions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # The AI agent posts a single review with inline comments using GitHub's | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # native suggestion syntax, allowing one-click commits of suggested changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Triggered by: Adding the "code-review" label to a PR, or manual dispatch. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Required secrets: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # - CODE_REVIEW_CODER_URL: URL of your Coder deployment | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # - CODE_REVIEW_CODER_SESSION_TOKEN: Session token for Coder API | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: AI Code Review | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - labeled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pr_url: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Pull Request URL to review" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| template_preset: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Template preset to use" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| code-review: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: AI Code Review | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (github.event.label.name == 'code-review' || github.event_name == 'workflow_dispatch') && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (github.event.pull_request.draft == false || github.event_name == 'workflow_dispatch') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 30 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CODER_URL: ${{ secrets.CODE_REVIEW_CODER_URL }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CODER_SESSION_TOKEN: ${{ secrets.CODE_REVIEW_CODER_SESSION_TOKEN }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contents: read # Read repository contents and PR diff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pull-requests: write # Post review comments and suggestions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| actions: write # Create workflow summaries | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Missing error handling if the secrets are not set. The workflow will fail silently or with unclear errors if
Suggested change
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Determine PR Context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: determine-context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_ACTOR: ${{ github.actor }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_EVENT_PR_HTML_URL: ${{ github.event.pull_request.html_url }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_EVENT_PR_NUMBER: ${{ github.event.pull_request.number }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_EVENT_SENDER_ID: ${{ github.event.sender.id }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GITHUB_EVENT_SENDER_LOGIN: ${{ github.event.sender.login }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTS_PR_URL: ${{ inputs.pr_url }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTS_TEMPLATE_PRESET: ${{ inputs.template_preset || '' }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| GH_TOKEN: ${{ github.token }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Issue: Missing error handling directives. Shell scripts should fail fast on errors to prevent cascading failures. Without
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Using template preset: ${INPUTS_TEMPLATE_PRESET}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "template_preset=${INPUTS_TEMPLATE_PRESET}" >> "${GITHUB_OUTPUT}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # For workflow_dispatch, use the provided PR URL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ! GITHUB_USER_ID=$(gh api "users/${GITHUB_ACTOR}" --jq '.id'); then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Issue: The GitHub API call for user ID lacks error context and doesn't validate the response. If the API returns an empty or invalid response,
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "::error::Failed to get GitHub user ID for actor ${GITHUB_ACTOR}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Issue: Error message could be more helpful by suggesting what the user should check.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Using workflow_dispatch actor: ${GITHUB_ACTOR} (ID: ${GITHUB_USER_ID})" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "github_user_id=${GITHUB_USER_ID}" >> "${GITHUB_OUTPUT}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "github_username=${GITHUB_ACTOR}" >> "${GITHUB_OUTPUT}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DevelopmentCats marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Using PR URL: ${INPUTS_PR_URL}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Convert /pull/ to /issues/ for create-task-action compatibility | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ISSUE_URL="${INPUTS_PR_URL/\/pull\//\/issues\/}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Issue: Variable should be quoted to safely handle URLs with special characters or spaces. While URLs shouldn't have spaces, defensive quoting is a shell scripting best practice.
Suggested change
Contributor
Author
There was a problem hiding this comment. Important: Variable should be quoted to safely handle special characters in URLs.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "pr_url=${ISSUE_URL}" >> "${GITHUB_OUTPUT}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Extract PR number from URL for later use | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| echo "Using PR URL: ${INPUTS_PR_URL}" | |
| # Convert /pull/ to /issues/ for create-task-action compatibility | |
| ISSUE_URL="${INPUTS_PR_URL/\/pull\//\/issues\/}" | |
| echo "pr_url=${ISSUE_URL}" >> "${GITHUB_OUTPUT}" | |
| # Extract PR number from URL for later use | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | sed -n 's/.*\/pull\/\([0-9]*\).*/\1/p') | |
| if [[ -z "${PR_NUMBER}" ]]; then | |
| echo "::error::Failed to extract PR number from URL: ${INPUTS_PR_URL}" | |
| exit 1 | |
| fi | |
| echo "pr_number=${PR_NUMBER}" >> "${GITHUB_OUTPUT}" |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| if [[ -z "${PR_NUMBER}" ]]; then | |
| echo "::error::Failed to extract PR number from URL: ${INPUTS_PR_URL}" | |
| exit 1 | |
| fi |
Outdated
There was a problem hiding this comment.
Security: Potential command injection via unvalidated user input in regex. The INPUTS_PR_URL comes directly from user input and is used in a grep pattern without validation.
Recommendation: Validate URL format before extraction to prevent unexpected behavior with malformed URLs.
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| # Validate PR URL format before processing | |
| if [[ ! "${INPUTS_PR_URL}" =~ ^https://github\.com/[^/]+/[^/]+/pull/[0-9]+$ ]]; then | |
| echo "::error::Invalid PR URL format: ${INPUTS_PR_URL}. Expected format: https://github.com/owner/repo/pull/NUMBER" | |
| exit 1 | |
| fi | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') |
Outdated
There was a problem hiding this comment.
Issue: Using grep -oP with Perl regex is not portable and may fail on macOS runners or systems without GNU grep.
The -P flag (Perl regex) is a GNU extension not available in BSD grep (default on macOS). While this workflow runs on ubuntu-latest, it's better to use portable alternatives for maintainability.
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| # Extract PR number from URL | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | sed -n 's|.*/pull/\([0-9]*\)$|\1|p') | |
| echo "pr_number=${PR_NUMBER}" >> "${GITHUB_OUTPUT}" |
Outdated
There was a problem hiding this comment.
Issue: [NITPICK] grep -oP uses Perl regex which is GNU-specific. While it works on GitHub Actions (Ubuntu), sed is more portable across all Unix systems.
This matches the portability pattern mentioned in the workflow's own examples (lines 221-226).
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | sed -n 's|.*/pull/\([0-9]*\)$|\1|p') |
Outdated
There was a problem hiding this comment.
Issue: [NITPICK] grep -oP uses Perl regex which is GNU-specific. While this works on GitHub Actions runners (ubuntu-latest), using sed would be more portable across all Unix systems.
The existing regex validation at line 79 already ensures the URL format is correct, so the extraction will succeed.
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | sed -n 's|.*/pull/\([0-9]*\)$|\1|p') |
DevelopmentCats marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Issue: [NITPICK] grep -oP uses Perl-compatible regex which is GNU-specific and not available on all systems (e.g., macOS/BSD grep). While GitHub Actions runners use Ubuntu and this will work fine, sed is more portable.
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | grep -oP '(?<=pull/)\d+') | |
| PR_NUMBER=$(echo "${INPUTS_PR_URL}" | sed -n 's|.*/pull/\([0-9]\+\)$|\1|p') |
There was a problem hiding this comment.
Issue: Variable should be quoted for consistency and safety with special characters.
| ISSUE_URL="${GITHUB_EVENT_PR_HTML_URL/\/pull\//\/issues\/}" | |
| ISSUE_URL="${GITHUB_EVENT_PR_HTML_URL/\/pull\//\/issues\/}" |
There was a problem hiding this comment.
Important: Variable should be quoted for consistency and safety.
| ISSUE_URL="${GITHUB_EVENT_PR_HTML_URL/\/pull\//\/issues\/}" | |
| ISSUE_URL="${GITHUB_EVENT_PR_HTML_URL/\/pull\//\/issues\/}" |
There was a problem hiding this comment.
Issue: Missing error handling directives for this shell script block.
| run: | | |
| run: | | |
| set -euo pipefail | |
| echo "owner=${{ github.repository_owner }}" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
Important: Missing set -euo pipefail for consistency with other shell script blocks.
| run: | | |
| run: | | |
| set -euo pipefail | |
| echo "owner=${{ github.repository_owner }}" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
Issue: Missing error handling directives. This is especially important for the heredoc block that follows.
| run: | | |
| run: | | |
| set -euo pipefail | |
| echo "Building code review prompt for PR #${PR_NUMBER}" |
There was a problem hiding this comment.
Issue: Missing set -euo pipefail at the start of the script block.
Impact: Without these flags, if PR_NUMBER is undefined or the heredoc fails, the step could succeed with an incomplete or malformed prompt, causing the Coder task to receive invalid instructions.
| echo "Building code review prompt for PR #${PR_NUMBER}" | |
| run: | | |
| set -euo pipefail | |
| echo "Building code review prompt for PR #${PR_NUMBER}" |
There was a problem hiding this comment.
[nitpick] The bash heredoc syntax is missing proper indentation handling. The <<EOF should be <<-EOF to strip leading tabs, making the code more readable:
| TASK_PROMPT=$(cat <<EOF | |
| TASK_PROMPT=$(cat <<-EOF |
| TASK_PROMPT=$(cat <<EOF | |
| TASK_PROMPT=$(cat <<-EOF |
Outdated
There was a problem hiding this comment.
The prompt mentions reviewing against "Uber Go Style Guide" and checking for "context and cancellation (Go code)" and "dbauthz patterns" which are all Go/Coder-specific. This creates a workflow that's tightly coupled to a specific tech stack. Consider either:
- Renaming the workflow to indicate it's Go-specific
- Making these guidelines configurable via inputs
- Having the AI detect the project language and apply appropriate style guides
There was a problem hiding this comment.
[nitpick] The heredoc marker at line 334 and line 336 use different markers (EOFOUTPUT). While this works, it would be more maintainable to use consistent naming. Also, the heredoc at line 119 uses EOF while this uses EOFOUTPUT, creating inconsistency:
| echo "task_prompt<<EOFOUTPUT" | |
| echo "${TASK_PROMPT}" | |
| echo "EOFOUTPUT" | |
| echo "task_prompt<<EOF_TASK_PROMPT" | |
| echo "${TASK_PROMPT}" | |
| echo "EOF_TASK_PROMPT" |
| echo "task_prompt<<EOFOUTPUT" | |
| echo "${TASK_PROMPT}" | |
| echo "EOFOUTPUT" | |
| echo "task_prompt<<EOF_TASK_PROMPT" | |
| echo "${TASK_PROMPT}" | |
| echo "EOF_TASK_PROMPT" |
There was a problem hiding this comment.
Best Practice: Consider pinning to a specific commit SHA rather than the main branch for better reproducibility and security.
Using ref: main means the workflow behavior can change unexpectedly if the action is updated. Pinning to a commit SHA ensures consistent behavior and allows you to review changes before updating.
| uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 | |
| ref: main # TODO: Pin to commit SHA for reproducibility (e.g., abc123def456...) |
There was a problem hiding this comment.
The workflow checks out an external action from the main branch (line 345), which could introduce supply chain security risks if that branch is compromised. Consider pinning to a specific commit SHA or tag version for better security:
| ref: main | |
| ref: v1.0.0 # or a specific commit SHA |
| ref: main | |
| ref: v1.0.0 # or a specific commit SHA |
There was a problem hiding this comment.
Minor: Pin to commit SHA instead of main for reproducibility. Using main means workflow behavior can change unexpectedly.
| ref: main | |
| ref: main # TODO: Pin to specific commit SHA for reproducibility |
There was a problem hiding this comment.
Minor: Missing error handling directives for consistency with other steps.
| github-user-id: ${{ steps.determine-context.outputs.github_user_id }} | |
| run: | | |
| set -euo pipefail | |
| { |
There was a problem hiding this comment.
Minor: Missing set -euo pipefail for consistency.
| run: | | |
| run: | | |
| set -euo pipefail | |
| { |
Uh oh!
There was an error while loading. Please reload this page.