Refactor Devin PR Code Review workflow#52
Conversation
Signed-off-by: freedisch <freeproduc@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| BODY=$(echo "$RESPONSE" | sed '$d') | ||
|
|
||
| echo "Status: $HTTP_CODE" | ||
| echo "Status: $HTTP_CODE" |
There was a problem hiding this comment.
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.
| echo "Status: $HTTP_CODE" | |
| echo "Status: $HTTP_CODE" |
| fi | ||
|
|
||
| SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty') | ||
| echo "Session ID: $SESSION_ID" |
There was a problem hiding this comment.
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.
| echo "Session ID: $SESSION_ID" | |
| SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty') |
| exit 1 | ||
| fi | ||
|
|
||
| SESSION_ID=$(echo "$BODY" | jq -r '.session_id // .id // empty') |
There was a problem hiding this comment.
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:
| 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 |
| } | ||
| }' | ||
| "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 |
There was a problem hiding this comment.
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.:
| "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 }}" |
| - name: Checkout Code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
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.
| 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" \ |
There was a problem hiding this comment.
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:
| 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" \ |
Review SummaryI reviewed the diff for this PR (refactoring the Devin PR Code Review workflow). Found 6 issues across 3 categories: 🔴 Bugs (2)
🟡 Security (1)
🟠 Edge Cases / Improvements (3)
All issues have inline comments with suggested fixes. |
No description provided.