Skip to content

missing or incorrect Accept headers for the GitHub API diff endpoint#53

Merged
yb175 merged 3 commits into
mainfrom
pr-fix
Feb 26, 2026
Merged

missing or incorrect Accept headers for the GitHub API diff endpoint#53
yb175 merged 3 commits into
mainfrom
pr-fix

Conversation

@yb175
Copy link
Copy Markdown
Owner

@yb175 yb175 commented Feb 26, 2026

Fixes : #52

Summary by CodeRabbit

  • Chores
    • Backend now fetches pull request diffs directly from GitHub’s API.
    • Deployment configuration no longer includes the frontend service.
    • Frontend build image/Dockerfile removed from the repository.
    • LLM service port mapping updated to explicitly publish the service port.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 26, 2026

📝 Walkthrough

Walkthrough

Replaces PR diff fetch to use GitHub's /pulls/{number}.diff endpoint with an Accept header; removes the frontend service from docker-compose and deletes the frontend Dockerfile.

Changes

Cohort / File(s) Summary
Backend — PR diff fetch
backend/services/analysis.service.js
Switches from using pr.diff_url to calling https://api.github.com/repos/{owner}/{repo}/pulls/{number}.diff, adds Accept: application/vnd.github.v3.diff header, keeps responseType: "text" and existing error handling.
Docker / Frontend removal
docker-compose.yaml, frontend/Dockerfile
Removes the frontend service from docker-compose and deletes the frontend/Dockerfile; updates llm service port mapping from expose to host port mapping (8000:8000).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • Pratham-9365

Poem

🐰 I hopped through code and found the tweak,
The frontend shed its Docker streak,
Diffs now fetched from GitHub's door,
Smaller images, simpler chore,
A rabbit pats the repo — sleek!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change in backend/services/analysis.service.js, which is the addition of the correct Accept header for the GitHub API diff endpoint.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pr-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yb175
Copy link
Copy Markdown
Owner Author

yb175 commented Feb 26, 2026

@coderabbitai full review and reevaluate all your previous comments that may have been dismissed, ignored or resolved. Evaluate whether the goals of the issue linked in the first PR comment are met. your conclusion as an easily readable comment that also includes your findings outside the diffs.
1.) can we merge this pr

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 26, 2026

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Get PR description and linked issues
echo "=== PR Description ==="
gh pr view 53 --json body,number,title,url --repo yb175/pullShark | jq -r '.body'

echo -e "\n=== Previous Review Comments by CodeRabbit ==="
# Get all review comments from this PR by coderabbitai
gh api repos/yb175/pullShark/pulls/53/comments --paginate | jq -r '.[] | select(.user.login == "coderabbitai[bot]") | "Comment ID: \(.id)\nPath: \(.path)\nLine: \(.line // "N/A")\nBody: \(.body)\nState: \(.state // "active")\n---"'

echo -e "\n=== PR Review Comments (review thread) ==="
gh api repos/yb175/pullShark/pulls/53/reviews --paginate | jq -r '.[] | select(.user.login == "coderabbitai[bot]") | "Review ID: \(.id)\nState: \(.state)\nBody: \(.body)\n---"'

echo -e "\n=== Files Changed in PR ==="
gh pr view 53 --json files --repo yb175/pullShark | jq -r '.files[] | "File: \(.path)\n  Additions: \(.additions)\n  Deletions: \(.deletions)\n---"'

Length of output: 778


🏁 Script executed:

#!/bin/bash

# Get the actual code changes in the main file
echo "=== Current backend/services/analysis.service.js (relevant section) ==="
cat backend/services/analysis.service.js

Length of output: 4785


🏁 Script executed:

#!/bin/bash

# Check if there are any related issues mentioned or linked
echo "=== Searching for issue references in recent commits ==="
gh pr view 53 --json commits --repo yb175/pullShark | jq -r '.commits[] | "Commit: \(.messageHeadline)\nMessage: \(.messageBody)\n---"'

# Check for open issues related to GitHub API or diff
echo -e "\n=== Related Issues ==="
gh issue list --repo yb175/pullShark --state all --search "diff OR github API OR accept header" --json number,title,state,url | jq -r '.[] | "Issue #\(.number): \(.title) [\(.state)]\nURL: \(.url)\n---"'

Length of output: 1144


🏁 Script executed:

#!/bin/bash

# Get issue `#52` details
echo "=== Issue `#52` Details ==="
gh issue view 52 --repo yb175/pullShark --json title,body,number,state,url,comments | jq -r '
  "Title: \(.title)",
  "State: \(.state)",
  "URL: \(.url)",
  "",
  "Body:",
  "\(.body)",
  "",
  "Comments:",
  (.comments[] | "---\nAuthor: \(.author.login)\nCreated: \(.createdAt)\n\(.body)")
'

Length of output: 2354


🏁 Script executed:

#!/bin/bash

# Check the git diff to see what was actually changed
echo "=== Git diff for backend/services/analysis.service.js ==="
gh pr diff 53 --repo yb175/pullShark -- backend/services/analysis.service.js

Length of output: 94


🏁 Script executed:

#!/bin/bash

# Check if there are any security concerns - look for sensitive data exposure
echo "=== Checking for potential security issues ==="

# Check if there are any hardcoded secrets or tokens
echo "Checking for hardcoded secrets..."
rg -i "password|secret|api[_-]?key|token" backend/services/analysis.service.js -C 2

echo -e "\n=== Checking error handling patterns ==="
# Check error handling
rg "catch.*err|throw new Error" backend/services/analysis.service.js -C 2

echo -e "\n=== Checking for any TODO or FIXME comments ==="
rg "TODO|FIXME|XXX|HACK" backend/services/analysis.service.js

Length of output: 2037


🏁 Script executed:

#!/bin/bash

# Get the actual diff using git commands
echo "=== Diff for backend/services/analysis.service.js ==="
git diff origin/main...origin/pr-fix -- backend/services/analysis.service.js

echo -e "\n=== Diff for docker-compose.yaml ==="
git diff origin/main...origin/pr-fix -- docker-compose.yaml

echo -e "\n=== Diff for frontend/Dockerfile ==="
git diff origin/main...origin/pr-fix -- frontend/Dockerfile

Length of output: 279


🏁 Script executed:

#!/bin/bash

# Check if there are any tests for the analysis service
echo "=== Looking for tests ==="
fd -e test.js -e spec.js -e test.ts -e spec.ts | head -20

echo -e "\n=== Checking if there's a test directory ==="
fd -t d test | head -10

echo -e "\n=== Check package.json for test scripts ==="
cat backend/package.json | jq '.scripts' 2>/dev/null || echo "No backend/package.json or jq parse failed"

Length of output: 121


🏁 Script executed:

#!/bin/bash

# Check the before state to understand what pr.diff_url was
echo "=== Checking previous version of analysis.service.js (main branch) ==="
git show origin/main:backend/services/analysis.service.js | grep -A 10 "fetch diff"

Length of output: 319


🏁 Script executed:

#!/bin/bash

# Check if frontend is used anywhere else in the codebase
echo "=== Checking for frontend references ==="
rg -i "frontend|5173|vite" --type yaml --type js --type json -g '!node_modules' -g '!frontend/**' | head -30

echo -e "\n=== Check if there's a separate frontend repo or service ==="
cat README.md | grep -i frontend -C 3 || echo "No frontend mention in README"

echo -e "\n=== Check git history for context on frontend removal ==="
git log --oneline --all -n 20 | grep -i frontend

Length of output: 620


🏁 Script executed:

#!/bin/bash

# Verify the actual changes made in the PR
echo "=== PR commits ==="
gh pr view 53 --repo yb175/pullShark --json commits | jq -r '.commits[] | "\(.oid[0:7]) \(.messageHeadline)"'

echo -e "\n=== Full commit messages ==="
gh pr view 53 --repo yb175/pullShark --json commits | jq -r '.commits[] | "Commit: \(.oid[0:7])\nAuthor: \(.author.name)\nDate: \(.committedDate)\nMessage: \(.messageHeadline)\n\(.messageBody)\n---"'

Length of output: 370


@yb175, I've completed a comprehensive review of PR #53. Here's my analysis:

✅ Issue #52 Goals - SUCCESSFULLY MET

The core fix correctly addresses the 404 error for contributor PRs:

What was fixed:

  • Changed from using pr.diff_url to explicit GitHub API endpoint: https://api.github.com/repos/${pr_owner}/${repo_name}/pulls/${pr_number}.diff
  • Added proper Accept header: application/vnd.github.v3.diff
  • Properly merged with existing authentication headers (...ghHeaders)

This matches exactly what issue #52 identified as the solution.

🔍 Code Quality Assessment

Security: ✅ GOOD

  • No hardcoded secrets or tokens
  • Proper authentication token generation via utility function
  • Environment variables used correctly
  • Error handling present with appropriate logging

Architecture: ✅ ACCEPTABLE

  • Clean code structure maintained
  • Error propagation follows existing patterns
  • Consistent with learnings (proper error handling, clean architecture)

⚠️ Concerns Outside the Main Fix

1. Frontend Removal - NEEDS CLARIFICATION

The PR removes the frontend service from Docker deployment:

  • docker-compose.yaml: Frontend service removed (5 lines)
  • frontend/Dockerfile: Completely emptied (16 lines)

However, the backend still expects a frontend:

  • backend/server.js: CORS configured for FRONTEND_URL
  • backend/controllers/webhook/getInstallationId.js: Redirects to FRONTEND_URL/repo
  • backend/controllers/auth/callback.js: Redirects to FRONTEND_URL
  • backend/controllers/webhook/appInstalled.js: Redirects to FRONTEND_URL/repo

Questions:

  • Is the frontend being deployed separately now?
  • Will FRONTEND_URL be set to an external service?
  • Is this change intentional or accidental?

The frontend removal is unrelated to the PR title ("missing or incorrect Accept headers") and could cause runtime issues if the backend expects frontend redirects to work.

2. Testing - NO AUTOMATED TESTS

  • No test files found in the repository
  • Cannot verify the fix works for both owner and contributor PRs
  • Manual testing required

📊 Final Assessment

Can we merge this PR?

Core Fix: YES ✅ - The GitHub API Accept header fix is correct and ready.

Overall: CONDITIONAL ⚠️ - I recommend:

  1. Either:

    • Revert the frontend removal changes (keep only the analysis.service.js fix), OR
    • Confirm the frontend is being deployed separately and FRONTEND_URL will be properly configured
  2. Then: Safe to merge

  3. Future: Consider adding tests for the analysis service (relates to open issue #32)


My recommendation: If the frontend removal was intentional and you have a separate deployment plan, this PR is safe to merge. If the frontend removal was accidental or unclear, I'd suggest splitting it into a separate PR with proper context.


🧠 Learnings used
Learnt from: yb175
Repo: yb175/pullShark PR: 0
File: :0-0
Timestamp: 2026-02-26T04:45:52.017Z
Learning: Repo yb175/pullShark: Reviewer preference — focus reviews on security, database performance, proper error handling, clean architecture, and low complexity. Feedback must be concise and actionable; avoid generic praise.
✅ Actions performed

Full review triggered.

@yb175
Copy link
Copy Markdown
Owner Author

yb175 commented Feb 26, 2026

🧩 Analysis chain
🏁 Script executed:

#!/bin/bash

# Get PR description and linked issues
echo "=== PR Description ==="
gh pr view 53 --json body,number,title,url --repo yb175/pullShark | jq -r '.body'

echo -e "\n=== Previous Review Comments by CodeRabbit ==="
# Get all review comments from this PR by coderabbitai
gh api repos/yb175/pullShark/pulls/53/comments --paginate | jq -r '.[] | select(.user.login == "coderabbitai[bot]") | "Comment ID: \(.id)\nPath: \(.path)\nLine: \(.line // "N/A")\nBody: \(.body)\nState: \(.state // "active")\n---"'

echo -e "\n=== PR Review Comments (review thread) ==="
gh api repos/yb175/pullShark/pulls/53/reviews --paginate | jq -r '.[] | select(.user.login == "coderabbitai[bot]") | "Review ID: \(.id)\nState: \(.state)\nBody: \(.body)\n---"'

echo -e "\n=== Files Changed in PR ==="
gh pr view 53 --json files --repo yb175/pullShark | jq -r '.files[] | "File: \(.path)\n  Additions: \(.additions)\n  Deletions: \(.deletions)\n---"'

Length of output: 778

🏁 Script executed:

#!/bin/bash

# Get the actual code changes in the main file
echo "=== Current backend/services/analysis.service.js (relevant section) ==="
cat backend/services/analysis.service.js

Length of output: 4785

🏁 Script executed:

#!/bin/bash

# Check if there are any related issues mentioned or linked
echo "=== Searching for issue references in recent commits ==="
gh pr view 53 --json commits --repo yb175/pullShark | jq -r '.commits[] | "Commit: \(.messageHeadline)\nMessage: \(.messageBody)\n---"'

# Check for open issues related to GitHub API or diff
echo -e "\n=== Related Issues ==="
gh issue list --repo yb175/pullShark --state all --search "diff OR github API OR accept header" --json number,title,state,url | jq -r '.[] | "Issue #\(.number): \(.title) [\(.state)]\nURL: \(.url)\n---"'

Length of output: 1144

🏁 Script executed:

#!/bin/bash

# Get issue `#52` details
echo "=== Issue `#52` Details ==="
gh issue view 52 --repo yb175/pullShark --json title,body,number,state,url,comments | jq -r '
  "Title: \(.title)",
  "State: \(.state)",
  "URL: \(.url)",
  "",
  "Body:",
  "\(.body)",
  "",
  "Comments:",
  (.comments[] | "---\nAuthor: \(.author.login)\nCreated: \(.createdAt)\n\(.body)")
'

Length of output: 2354

🏁 Script executed:

#!/bin/bash

# Check the git diff to see what was actually changed
echo "=== Git diff for backend/services/analysis.service.js ==="
gh pr diff 53 --repo yb175/pullShark -- backend/services/analysis.service.js

Length of output: 94

🏁 Script executed:

#!/bin/bash

# Check if there are any security concerns - look for sensitive data exposure
echo "=== Checking for potential security issues ==="

# Check if there are any hardcoded secrets or tokens
echo "Checking for hardcoded secrets..."
rg -i "password|secret|api[_-]?key|token" backend/services/analysis.service.js -C 2

echo -e "\n=== Checking error handling patterns ==="
# Check error handling
rg "catch.*err|throw new Error" backend/services/analysis.service.js -C 2

echo -e "\n=== Checking for any TODO or FIXME comments ==="
rg "TODO|FIXME|XXX|HACK" backend/services/analysis.service.js

Length of output: 2037

🏁 Script executed:

#!/bin/bash

# Get the actual diff using git commands
echo "=== Diff for backend/services/analysis.service.js ==="
git diff origin/main...origin/pr-fix -- backend/services/analysis.service.js

echo -e "\n=== Diff for docker-compose.yaml ==="
git diff origin/main...origin/pr-fix -- docker-compose.yaml

echo -e "\n=== Diff for frontend/Dockerfile ==="
git diff origin/main...origin/pr-fix -- frontend/Dockerfile

Length of output: 279

🏁 Script executed:

#!/bin/bash

# Check if there are any tests for the analysis service
echo "=== Looking for tests ==="
fd -e test.js -e spec.js -e test.ts -e spec.ts | head -20

echo -e "\n=== Checking if there's a test directory ==="
fd -t d test | head -10

echo -e "\n=== Check package.json for test scripts ==="
cat backend/package.json | jq '.scripts' 2>/dev/null || echo "No backend/package.json or jq parse failed"

Length of output: 121

🏁 Script executed:

#!/bin/bash

# Check the before state to understand what pr.diff_url was
echo "=== Checking previous version of analysis.service.js (main branch) ==="
git show origin/main:backend/services/analysis.service.js | grep -A 10 "fetch diff"

Length of output: 319

🏁 Script executed:

#!/bin/bash

# Check if frontend is used anywhere else in the codebase
echo "=== Checking for frontend references ==="
rg -i "frontend|5173|vite" --type yaml --type js --type json -g '!node_modules' -g '!frontend/**' | head -30

echo -e "\n=== Check if there's a separate frontend repo or service ==="
cat README.md | grep -i frontend -C 3 || echo "No frontend mention in README"

echo -e "\n=== Check git history for context on frontend removal ==="
git log --oneline --all -n 20 | grep -i frontend

Length of output: 620

🏁 Script executed:

#!/bin/bash

# Verify the actual changes made in the PR
echo "=== PR commits ==="
gh pr view 53 --repo yb175/pullShark --json commits | jq -r '.commits[] | "\(.oid[0:7]) \(.messageHeadline)"'

echo -e "\n=== Full commit messages ==="
gh pr view 53 --repo yb175/pullShark --json commits | jq -r '.commits[] | "Commit: \(.oid[0:7])\nAuthor: \(.author.name)\nDate: \(.committedDate)\nMessage: \(.messageHeadline)\n\(.messageBody)\n---"'

Length of output: 370

@yb175, I've completed a comprehensive review of PR #53. Here's my analysis:

✅ Issue #52 Goals - SUCCESSFULLY MET

The core fix correctly addresses the 404 error for contributor PRs:

What was fixed:

  • Changed from using pr.diff_url to explicit GitHub API endpoint: https://api.github.com/repos/${pr_owner}/${repo_name}/pulls/${pr_number}.diff
  • Added proper Accept header: application/vnd.github.v3.diff
  • Properly merged with existing authentication headers (...ghHeaders)

This matches exactly what issue #52 identified as the solution.

🔍 Code Quality Assessment

Security: ✅ GOOD

  • No hardcoded secrets or tokens
  • Proper authentication token generation via utility function
  • Environment variables used correctly
  • Error handling present with appropriate logging

Architecture: ✅ ACCEPTABLE

  • Clean code structure maintained
  • Error propagation follows existing patterns
  • Consistent with learnings (proper error handling, clean architecture)

⚠️ Concerns Outside the Main Fix

1. Frontend Removal - NEEDS CLARIFICATION

The PR removes the frontend service from Docker deployment:

  • docker-compose.yaml: Frontend service removed (5 lines)
  • frontend/Dockerfile: Completely emptied (16 lines)

However, the backend still expects a frontend:

  • backend/server.js: CORS configured for FRONTEND_URL
  • backend/controllers/webhook/getInstallationId.js: Redirects to FRONTEND_URL/repo
  • backend/controllers/auth/callback.js: Redirects to FRONTEND_URL
  • backend/controllers/webhook/appInstalled.js: Redirects to FRONTEND_URL/repo

Questions:

  • Is the frontend being deployed separately now?
  • Will FRONTEND_URL be set to an external service?
  • Is this change intentional or accidental?

The frontend removal is unrelated to the PR title ("missing or incorrect Accept headers") and could cause runtime issues if the backend expects frontend redirects to work.

2. Testing - NO AUTOMATED TESTS

  • No test files found in the repository
  • Cannot verify the fix works for both owner and contributor PRs
  • Manual testing required

📊 Final Assessment

Can we merge this PR?

Core Fix: YES ✅ - The GitHub API Accept header fix is correct and ready.

Overall: CONDITIONAL ⚠️ - I recommend:

  1. Either:

    • Revert the frontend removal changes (keep only the analysis.service.js fix), OR
    • Confirm the frontend is being deployed separately and FRONTEND_URL will be properly configured
  2. Then: Safe to merge

  3. Future: Consider adding tests for the analysis service (relates to open issue #32)

My recommendation: If the frontend removal was intentional and you have a separate deployment plan, this PR is safe to merge. If the frontend removal was accidental or unclear, I'd suggest splitting it into a separate PR with proper context.

🧠 Learnings used

Learnt from: yb175
Repo: yb175/pullShark PR: 0
File: :0-0
Timestamp: 2026-02-26T04:45:52.017Z
Learning: Repo yb175/pullShark: Reviewer preference — focus reviews on security, database performance, proper error handling, clean architecture, and low complexity. Feedback must be concise and actionable; avoid generic praise.

✅ Actions performed

@yb175 yb175 closed this Feb 26, 2026
@yb175 yb175 reopened this Feb 26, 2026
@yb175 yb175 closed this Feb 26, 2026
@yb175 yb175 reopened this Feb 26, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
docker-compose.yaml (1)

14-14: Avoid :latest tag — pin to a specific version.

Using :latest makes builds non-reproducible and can introduce unexpected changes. Pin to a specific image digest or semver tag.

-    image: yb175/pullshark-llm:latest
+    image: yb175/pullshark-llm:1.0.0  # or use sha256 digest

As per coding guidelines: "avoid :latest tags."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yaml` at line 14, The docker image is pinned to the floating
tag "yb175/pullshark-llm:latest", which makes builds non-reproducible; update
the image reference used where "image: yb175/pullshark-llm:latest" appears to a
specific semver tag or immutable digest (for example replace ":latest" with a
concrete tag like ":v1.2.3" or "@sha256:..."), and ensure the chosen tag/digest
is documented in the release notes or CI so future builds remain deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docker-compose.yaml`:
- Around line 15-16: The docker-compose removal of the frontend service is out
of scope and will break runtime behavior because backend code still references
FRONTEND_URL (see server.js and controller files); either restore the frontend
service and its Dockerfile in docker-compose.yaml (re-add the frontend service
block and its ports/build config) so local dev uses the same FRONTEND_URL, or if
the frontend is truly deployed separately, update deployment docs and ensure
FRONTEND_URL is set in backend environment/config and remove any hard-coded
assumptions in server.js/controllers; also verify CORS origins and redirect
logic in server.js reference the configured FRONTEND_URL rather than an assumed
local service.

---

Nitpick comments:
In `@docker-compose.yaml`:
- Line 14: The docker image is pinned to the floating tag
"yb175/pullshark-llm:latest", which makes builds non-reproducible; update the
image reference used where "image: yb175/pullshark-llm:latest" appears to a
specific semver tag or immutable digest (for example replace ":latest" with a
concrete tag like ":v1.2.3" or "@sha256:..."), and ensure the chosen tag/digest
is documented in the release notes or CI so future builds remain deterministic.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c72179d and 49a1593.

📒 Files selected for processing (1)
  • docker-compose.yaml

Comment thread docker-compose.yaml
@yb175 yb175 merged commit 5713eb6 into main Feb 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PRS in private repos or raised by contributers were throwing error 404

1 participant