Skip to content

Refactor Devin PR Code Review workflow #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Freedisch merged 2 commits into from
May 20, 2026
Merged

Refactor Devin PR Code Review workflow #52

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
446fece
Refactor Devin PR Code Review workflow
Freedisch May 20, 2026
f800897
Update .github/workflows/devin-code-review.yaml
Freedisch May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions .github/workflows/devin-code-review.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,43 @@
name: Devin PR Code Review

on:
pull_request:
types: [opened, synchronize, reopened]

jobs:
devin-review:
name: Run Devin Code Review
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write

steps:
- name: Checkout Code
uses: actions/checkout@v4
with:
fetch-depth: 0

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waste: checkout step is unnecessary

The only action in this workflow is a curl call to the Devin API. The checked-out code is never read, built, or tested. Moreover, fetch-depth: 0 fetches the entire git history, which is the most expensive checkout option. This adds unnecessary latency to every PR event.

Remove this step entirely, or if you anticipate needing the repo content in a future step, at minimum change fetch-depth: 0 to fetch-depth: 1.


- name: Request Devin Review Agent
- name: Request Devin Review
env:
DEVIN_API_KEY: ${{ secrets.DEVIN_API_KEY }}
run: |
curl -X POST "https://devin.ai" \
-H "Authorization: Bearer ${{ secrets.DEVIN_API_KEY }}" \
RESPONSE=$(curl -s -w "\n%{http_code}" -X POST "https://api.devin.ai/v1/sessions" \

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge case: No curl timeout — workflow can hang indefinitely

If the Devin API is slow or unresponsive, this curl will block until GitHub's own job-level timeout (default 6 hours) kills it. Add --connect-timeout and --max-time to fail fast:

Suggested change
RESPONSE=$(curl -s -w "\n%{http_code}" -X POST "https://api.devin.ai/v1/sessions" \
RESPONSE=$(curl -s -w "\n%{http_code}" --connect-timeout 10 --max-time 30 -X POST "https://api.devin.ai/v1/sessions" \

-H "Authorization: Bearer $DEVIN_API_KEY" \
-H "Content-Type: application/json" \
-d '{
"prompt": "View the diffs for PR #${{ github.event.pull_request.number }} in the repository ${{ github.repository }}. Identify logical bugs, code convention violations, or potential edge-case failures. Post your feedback as inline comments or a summarized review on this GitHub Pull Request.",
"scan_repository": true,
"env": {
"GITHUB_TOKEN": "${{ secrets.GITHUB_TOKEN }}",
"PR_NUMBER": "${{ github.event.pull_request.number }}",
"REPO": "${{ github.repository }}"
}
}'
"prompt": "Review PR #${{ github.event.pull_request.number }} in ${{ github.repository }}. Look at the diffs, identify logical bugs, code convention violations, and potential edge-case failures. Post your feedback as inline review comments on the PR.",
"idempotent": true

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge case: "idempotent": true without an idempotency key is effectively a no-op

This workflow triggers on synchronize (every push to the PR branch). Each push fires a new API call with the same "idempotent": true but no idempotency key, so the Devin API has no way to deduplicate — it will create a new review session on every push.

If the intent is to prevent duplicate reviews, you need to provide a deterministic idempotency key, e.g.:

Suggested change
"idempotent": true
"prompt": "Review PR #${{ github.event.pull_request.number }} in ${{ github.repository }}. Look at the diffs, identify logical bugs, code convention violations, and potential edge-case failures. Post your feedback as inline review comments on the PR.",
"idempotent": true,
"idempotency_key": "pr-review-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}"

}')

HTTP_CODE=$(echo "$RESPONSE" | tail -1)
BODY=$(echo "$RESPONSE" | sed '$d')

echo "Status: $HTTP_CODE"
echo "Status: $HTTP_CODE"

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Duplicate echo — copy-paste error

This line is a duplicate of line 33. The second commit (f800897) replaced echo "Response: $BODY" with another echo "Status: $HTTP_CODE" instead of simply deleting it. The status code is printed twice for no reason.

Suggested change
echo "Status: $HTTP_CODE"
echo "Status: $HTTP_CODE"



if [ "$HTTP_CODE" -ne 200 ] && [ "$HTTP_CODE" -ne 201 ]; then
echo "::error::Devin API returned $HTTP_CODE"
exit 1
fi

SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty')

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: No validation that SESSION_ID is non-empty

If the API returns a JSON structure without session_id or id (e.g., a changed response schema, or a partial error response that still returns 200/201), jq will output an empty string and the workflow will silently succeed with a blank session ID. You should guard against this:

Suggested change
SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty')
SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty')
if [ -z "$SESSION_ID" ]; then
echo "::error::Could not extract session ID from API response"
exit 1
fi

echo "Session ID: $SESSION_ID"

@devin-ai-integration devin-ai-integration Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Session ID leaked to CI logs

The original echo "Response: $BODY" was correctly removed to avoid exposing sensitive data in CI logs, but this line re-introduces essentially the same problem. The session ID is a credential-equivalent that can be used to interact with or access the Devin session. GitHub Actions logs are visible to anyone with read access to the repo.

If you need the session ID for a downstream step, use echo "::add-mask::$SESSION_ID" before logging it, or pass it via $GITHUB_OUTPUT without echoing. Better yet, if no downstream step needs it, remove this line entirely.

Suggested change
echo "Session ID: $SESSION_ID"
SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty')

Loading