Skip to content

fix: send JSON:API Accept on GitHub App manifest setup - #880

Merged
essinghigh merged 1 commit into
masterfrom
fix/github-app-accept-header
Sep 12, 2026
Merged

essinghigh merged 1 commit into
masterfrom
fix/github-app-accept-header

Conversation

@essinghigh

@essinghigh essinghigh commented Sep 12, 2026

Copy link
Copy Markdown
Member

Clicking "Create or replace with GitHub" always failed with 406 "The Accept header must allow application/vnd.api+json".

Root cause: the frontend sent Accept: application/json on /admin/github-app/manifest/setup. The endpoint returns a vnd.api+json document, so the Accept gate rejects anything else. The endpoint itself negotiates its JSON body on either media type, so requesting vnd.api+json satisfies both sides. One-line fix.

Tests: frontend header regression test (asserts the Accept value plus end-to-end navigation to the GitHub setup URL) and backend 200/406 contract tests for the endpoint.

Verified: frontend 534/534, github-app backend suites 14/14, typechecks clean, file lint clean, complexity zero repo-wide, lint budget passes (752/0 vs 762/0).

AI-assisted contribution via Hermes Agent (Nous Research), model muse-spark.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed GitHub App setup in the admin area so requests are accepted correctly by the API.
    • Successful setup now redirects administrators to the GitHub authorization page as expected.
  • Tests

    • Added coverage for GitHub App manifest setup requests, content negotiation, authorization URL handling, and redirect behavior.

The manifest setup endpoint returns a vnd.api+json document and the
Accept gate 406s plain application/json, so the create/replace flow
always failed. The endpoint negotiates JSON for either media type,
so requesting vnd.api+json satisfies both. Adds frontend header
regression test plus backend 200/406 contract tests.
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9b638946-5b22-4ac9-a8fa-f42e1c228215

📥 Commits

Reviewing files that changed from the base of the PR and between f23734e and 88d2b7b.

📒 Files selected for processing (3)
  • backend/tests/api/admin_github_app_manifest.test.ts
  • frontend/src/views/AdminGitHubApp.tsx
  • frontend/tests/admin-github-app.test.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The backend tests define the manifest setup Accept contract. The frontend sends the JSON:API media type and redirects to the returned GitHub authorization URL. Frontend tests verify the request header and redirect behavior.

Changes

GitHub App manifest setup

Layer / File(s) Summary
Manifest endpoint contract
backend/tests/api/admin_github_app_manifest.test.ts
The tests create an authenticated site-admin request, accept application/vnd.api+json, and reject application/json with status 406.
Frontend manifest request
frontend/src/views/AdminGitHubApp.tsx, frontend/tests/admin-github-app.test.tsx
The view sends the JSON:API Accept header. The test verifies the header and redirect to the authorization URL.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 88d2b

The manifest setup request now matches the endpoint’s content-negotiation contract, preventing the prior 406 response during GitHub App setup.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: sending the JSON:API Accept header for GitHub App manifest setup.
Description check ✅ Passed The description explains the failure, root cause, fix, tests, verification results, and AI assistance disclosure. It does not reproduce the template headings or explicitly check each checklist item, b…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/github-app-accept-header

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


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

A rabbit checks the headers bright
JSON:API guides the flight
GitHub opens one shining door
Tests confirm the path once more
Hoppy code now lands just right

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

@essinghigh
essinghigh merged commit 95ff43a into master Sep 12, 2026
18 checks 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.

1 participant