fix: use service GITHUB_TOKEN for git push + GitHub API calls#10
Merged
Conversation
Fixes the "Permission denied. Check your GitHub access." error when
clicking "Commit and Push" for org-backed sessions. Primary bug:
generate-pr/route.ts guarded push-auth setup behind
`sessionRecord.repoOwner && sessionRecord.repoName`, which are null
for sessions created via the OrgSelector (only cloneUrl is populated).
The guard short-circuited, remote URL stayed unauthenticated, and
GitHub returned 403.
- New helper `ensureAuthenticatedOrigin({ sandbox, cloneUrl })` parses
owner/repo from the session cloneUrl and runs `git remote set-url
origin https://x-access-token:${GITHUB_TOKEN}@github.com/owner/repo.git`
in the sandbox
- `/api/generate-pr` replaces the buggy guard with the helper; user-
token fetch is removed (fork-fallback remains dead code, cleaned up
in follow-up YAGNI PR)
- Swaps 7 GitHub-API-op sites to the service token: `/api/pr`,
`/api/check-pr`, session merge/merge-readiness/close-pr/checks/fix,
chat runtime, and archive-session
- `auto-commit-direct.ts` and `auto-pr-direct.ts` also swap to the
service token (no effect for org sessions today since they are gated
by repoOwner+repoName elsewhere, but brings the token source into
alignment with the clone + push path)
All repos are recoupable-owned per PR #7 policy; the service
`GITHUB_TOKEN` always has access, so the per-user OAuth token is no
longer needed on these paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR systematically replaces per-user GitHub token retrieval ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
sweetmantech
commented
Apr 22, 2026
|
|
||
| // 3. Check GitHub for an existing PR on this branch | ||
| const token = await getUserGitHubToken(authResult.userId); | ||
| const token = process.env.GITHUB_TOKEN?.trim() || null; |
Contributor
Author
There was a problem hiding this comment.
How could all these retrievals of process.env.GITHUB_TOKEN better follow the DRY principle?
Addresses PR #10 review: process.env.GITHUB_TOKEN?.trim() was duplicated across 13 call sites with three different fallback conventions (null, undefined, nothing). New lib/github/service-token.ts centralizes the env-var name and trim logic in one place; each call site becomes a one-liner. Call sites needing undefined convert with ?? undefined at the use site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/app/api/pr/route.ts">
<violation number="1" location="apps/web/app/api/pr/route.ts:137">
P1: Using the shared `GITHUB_TOKEN` without verifying the requested `repoUrl` matches the session’s repository enables privilege escalation across repos the service token can access.</violation>
</file>
You're on the cubic free plan with 20 free PR reviews remaining this month. Upgrade for unlimited reviews.
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| const userToken = await getUserGitHubToken(session.user.id); | ||
| const userToken = process.env.GITHUB_TOKEN?.trim() || null; |
There was a problem hiding this comment.
P1: Using the shared GITHUB_TOKEN without verifying the requested repoUrl matches the session’s repository enables privilege escalation across repos the service token can access.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/app/api/pr/route.ts, line 137:
<comment>Using the shared `GITHUB_TOKEN` without verifying the requested `repoUrl` matches the session’s repository enables privilege escalation across repos the service token can access.</comment>
<file context>
@@ -135,7 +134,7 @@ export async function POST(req: Request) {
}
- const userToken = await getUserGitHubToken(session.user.id);
+ const userToken = process.env.GITHUB_TOKEN?.trim() || null;
if (!userToken) {
return Response.json(
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
apps/web/app/api/pr/route.ts (1)
137-164:⚠️ Potential issue | 🔴 CriticalBind the requested repo to the owned session before using the service token.
repoUrlcomes from the request body. Now thatuserTokenis the shared service token, this route needs to verify thatrepoUrlmatchessessionRecord.cloneUrl/repo metadata before calling GitHub.🔒 Proposed authorization check
+ const sessionRepoUrl = + sessionRecord.cloneUrl ?? + (sessionRecord.repoOwner && sessionRecord.repoName + ? `https://github.com/${sessionRecord.repoOwner}/${sessionRecord.repoName}` + : null); + const parsedSessionRepoUrl = sessionRepoUrl + ? parseGitHubUrl(sessionRepoUrl) + : null; + + if (!parsedSessionRepoUrl) { + return Response.json( + { error: "Session is not linked to a GitHub repository" }, + { status: 400 }, + ); + } + + if ( + parsedSessionRepoUrl.owner.toLowerCase() !== + parsedRepoUrl.owner.toLowerCase() || + parsedSessionRepoUrl.repo.toLowerCase() !== parsedRepoUrl.repo.toLowerCase() + ) { + return Response.json({ error: "Repository does not match session" }, { status: 403 }); + } + const userToken = process.env.GITHUB_TOKEN?.trim() || null; if (!userToken) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/pr/route.ts` around lines 137 - 164, Before calling createPullRequest with the shared service token (userToken), verify and bind the requested repo (repoUrl) to the current sessionRecord: check that repoUrl equals sessionRecord.cloneUrl or matches sessionRecord.repo metadata (e.g., owner/name) and only proceed if it matches; if it doesn't match return a 403/error response. Perform this authorization check immediately after computing repoUrl and before constructing tokenUsedForCreation/createPullRequest, using the existing sessionRecord (or session lookup) to validate ownership.apps/web/app/api/sessions/[sessionId]/close-pr/route.ts (1)
34-38:⚠️ Potential issue | 🟠 MajorDon’t require
repoOwner/repoNamewhencloneUrlis enough.Org-backed sessions with only
cloneUrlwill still get “not linked” here, althoughclosePullRequestonly needsrepoUrl.🐛 Proposed fix
- if ( - !sessionRecord.cloneUrl || - !sessionRecord.repoOwner || - !sessionRecord.repoName - ) { + if (!sessionRecord.cloneUrl) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/sessions/`[sessionId]/close-pr/route.ts around lines 34 - 38, The current guard erroneously requires repoOwner and repoName even when cloneUrl is available; update the conditional in route.ts around sessionRecord to allow proceeding if cloneUrl exists or if both repoOwner and repoName are present — e.g. replace the check with one that errors only when neither cloneUrl nor (repoOwner && repoName) is provided so closePullRequest can work with repoUrl (reference symbols: sessionRecord.cloneUrl, sessionRecord.repoOwner, sessionRecord.repoName, closePullRequest).apps/web/lib/chat/auto-commit-direct.ts (2)
41-55:⚠️ Potential issue | 🟠 MajorFail before committing if service remote auth cannot be configured.
When
GITHUB_TOKENis missing orgit remote set-urlfails, this still creates a local commit and only reports failure at push time. Make auth setup a hard precondition.🐛 Proposed fix
// 2. Set up auth on the remote — all repos are recoupable-owned, so the // service token has access. const repoToken = process.env.GITHUB_TOKEN?.trim() || undefined; - if (repoToken) { - const authUrl = buildGitHubAuthRemoteUrl({ - token: repoToken, - owner: repoOwner, - repo: repoName, - }); - - if (authUrl) { - await sandbox.exec(`git remote set-url origin "${authUrl}"`, cwd, 10000); - } + if (!repoToken) { + return { + committed: false, + pushed: false, + error: "GITHUB_TOKEN env var is not set", + }; + } + + const authUrl = buildGitHubAuthRemoteUrl({ + token: repoToken, + owner: repoOwner, + repo: repoName, + }); + + if (!authUrl) { + return { + committed: false, + pushed: false, + error: "Repository owner or name is not supported", + }; + } + + const remoteResult = await sandbox.exec( + `git remote set-url origin "${authUrl}"`, + cwd, + 10000, + ); + if (!remoteResult.success) { + return { + committed: false, + pushed: false, + error: "Failed to configure authenticated origin", + }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/chat/auto-commit-direct.ts` around lines 41 - 55, The code currently continues to create a local commit even when remote auth isn't configured; make auth setup a hard precondition by failing early: when process.env.GITHUB_TOKEN (repoToken) is absent, throw or return an error before any commit work, and when buildGitHubAuthRemoteUrl(repoOwner, repoName, token) returns falsy or sandbox.exec(`git remote set-url origin ...`, cwd, 10000) throws/returns a failure, propagate that error (stop execution) instead of continuing; update the auth block around repoToken, buildGitHubAuthRemoteUrl, and sandbox.exec to validate inputs and surface errors immediately so commit/push do not proceed without a working authenticated remote.
106-114:⚠️ Potential issue | 🟠 MajorShell-quote the branch before
git push.
currentBranchis read from git and interpolated directly into a bash command. Quote it before pushing, especially given that the remote URL now contains the service token.The exec implementation passes the command to
bash -c, so unquoted special characters in the branch name could cause unexpected behavior or code execution. This contradicts the quoting pattern used elsewhere in the same file (lines 76, 80, 90).🔒 Proposed fix
+ const shellQuote = (value: string) => `'${value.replace(/'/g, "'\\''")}'`; + const branchResult = await sandbox.exec( "git symbolic-ref --short HEAD", cwd, 5000, ); const currentBranch = branchResult.stdout.trim() || "HEAD"; const pushResult = await sandbox.exec( - `GIT_TERMINAL_PROMPT=0 git push -u origin ${currentBranch}`, + `GIT_TERMINAL_PROMPT=0 git push -u origin ${shellQuote(currentBranch)}`, cwd, 60000, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/chat/auto-commit-direct.ts` around lines 106 - 114, The branch name read into currentBranch is interpolated into the shell command passed to sandbox.exec for the git push, so shell-quote or escape currentBranch before using it in the `sandbox.exec(\`GIT_TERMINAL_PROMPT=0 git push -u origin ${currentBranch}\`, ...)` call to prevent word-splitting or command injection; update the git push invocation in auto-commit-direct.ts to use a safely quoted/escaped version of currentBranch (matching the pattern used at lines 76/80/90) so the branch string is passed as a single literal argument to git.apps/web/app/api/sessions/[sessionId]/checks/fix/route.ts (1)
206-246:⚠️ Potential issue | 🟠 MajorDerive owner/repo from
cloneUrlbefore rejecting the session.This still blocks org-backed sessions where only
cloneUrlis populated, so the new service token path is unreachable for them.🐛 Proposed fallback
-import type { PullRequestCheckRun } from "@/lib/github/client"; +import { parseGitHubUrl, type PullRequestCheckRun } from "@/lib/github/client"; @@ - if (!sessionRecord.repoOwner || !sessionRecord.repoName) { + const parsedCloneUrl = sessionRecord.cloneUrl + ? parseGitHubUrl(sessionRecord.cloneUrl) + : null; + const owner = sessionRecord.repoOwner ?? parsedCloneUrl?.owner ?? null; + const repo = sessionRecord.repoName ?? parsedCloneUrl?.repo ?? null; + + if (!owner || !repo) { return Response.json( { error: "Session is not linked to a GitHub repository" }, { status: 400 }, ); } @@ const octokit = new Octokit({ auth: token }); - const owner = sessionRecord.repoOwner; - const repo = sessionRecord.repoName;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/sessions/`[sessionId]/checks/fix/route.ts around lines 206 - 246, The early return when sessionRecord.repoOwner or sessionRecord.repoName are missing prevents using sessionRecord.cloneUrl to derive owner/repo for org-backed sessions; change the logic so before returning the 400 error you attempt to parse sessionRecord.cloneUrl (handle git@github.com:owner/repo(.git) and https://github.com/owner/repo(.git) forms) to populate owner/repo, and only call the Response.json error if both repoOwner/repoName are absent and cloneUrl parsing fails; update the places that use owner and repo (currently assigned from sessionRecord.repoOwner/repoName) to use the derived values when present.apps/web/lib/chat/auto-pr-direct.ts (1)
150-166:⚠️ Potential issue | 🟠 MajorHandle authenticated remote setup failure explicitly.
If
git remote set-urlfails, the flow continues and can misreport the branch as missing or not pushed. Return a real error at the auth setup point.🐛 Proposed fix
- await sandbox.exec(`git remote set-url origin "${authUrl}"`, cwd, 10000); + const remoteResult = await sandbox.exec( + `git remote set-url origin "${authUrl}"`, + cwd, + 10000, + ); + if (!remoteResult.success) { + return { + created: false, + syncedExisting: false, + skipped: false, + error: "Failed to configure authenticated origin", + }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/chat/auto-pr-direct.ts` around lines 150 - 166, The authenticated remote setup can fail silently because the call to sandbox.exec("git remote set-url origin ...", cwd, 10000) isn't handled; update the code around buildGitHubAuthRemoteUrl and the sandbox.exec call so any failure is surfaced immediately: either wrap sandbox.exec in a try/catch and return/throw a clear error (e.g., throw new Error or return an object with an explicit error message) that includes the sandbox.exec error output, or check its return status and return a failure result instead of continuing; specifically modify the block that calls sandbox.exec to propagate the failure (include the error text) so downstream logic doesn't misreport missing/unpushed branches.apps/web/app/api/sessions/[sessionId]/merge-readiness/route.ts (1)
101-106:⚠️ Potential issue | 🟠 MajorResolve repo identity from
cloneUrlfor org-backed sessions.This route still rejects sessions where
repoOwner/repoNameare null, even though the PR’s target sessions may only havecloneUrl.🐛 Proposed fallback
import { getPullRequestMergeReadiness, + parseGitHubUrl, type PullRequestCheckRun, type PullRequestMergeMethod, } from "@/lib/github/client"; @@ - const repoIdentifier = - sessionRecord.repoOwner && sessionRecord.repoName - ? `${sessionRecord.repoOwner}/${sessionRecord.repoName}` - : null; + const parsedCloneUrl = sessionRecord.cloneUrl + ? parseGitHubUrl(sessionRecord.cloneUrl) + : null; + const repoOwner = sessionRecord.repoOwner ?? parsedCloneUrl?.owner ?? null; + const repoName = sessionRecord.repoName ?? parsedCloneUrl?.repo ?? null; + const repoIdentifier = repoOwner && repoName ? `${repoOwner}/${repoName}` : null; - if (!sessionRecord.cloneUrl || !repoIdentifier || !sessionRecord.repoOwner) { + if (!sessionRecord.cloneUrl || !repoIdentifier) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/sessions/`[sessionId]/merge-readiness/route.ts around lines 101 - 106, The code rejects sessions if sessionRecord.repoOwner/repoName are missing even when sessionRecord.cloneUrl contains the repo identity; update the logic around repoIdentifier to parse owner/name from cloneUrl when repoOwner or repoName are null: implement a small helper (e.g., parseRepoFromCloneUrl) that accepts sessionRecord.cloneUrl and returns {repoOwner, repoName} for common formats (https, git+ssh, ssh) and then set repoIdentifier = sessionRecord.repoOwner && sessionRecord.repoName ? `${sessionRecord.repoOwner}/${sessionRecord.repoName}` : parsed ? `${parsed.repoOwner}/${parsed.repoName}` : null; finally use this repoIdentifier (and parsed owner/name) in the existing conditional that currently checks sessionRecord.cloneUrl || repoIdentifier || sessionRecord.repoOwner so sessions with only cloneUrl succeed.apps/web/lib/sandbox/archive-session.ts (1)
47-81:⚠️ Potential issue | 🟠 MajorUse
cloneUrlfallback and require the service token before GitHub calls.Archive refresh still skips cloneUrl-only sessions, and
getPullRequestStatusreceivesundefinedwhenGITHUB_TOKENis missing. Resolve repo identity fromcloneUrl, then return local updates if the service token is unavailable.🐛 Proposed fix
import { findPullRequestByBranch, getPullRequestStatus, + parseGitHubUrl, } from "@/lib/github/client"; @@ - if (!currentSession.repoOwner || !currentSession.repoName) { + const repoUrl = getSessionRepoUrl(currentSession); + const parsedRepo = repoUrl ? parseGitHubUrl(repoUrl) : null; + if (!repoUrl || !parsedRepo) { return {}; } @@ const updates: SessionUpdateInput = {}; const branchChanged = branch !== currentSession.branch; @@ const token = process.env.GITHUB_TOKEN?.trim() || undefined; + if (!token) { + return updates; + } if (!branchChanged && currentSession.prNumber != null) { - const repoUrl = getSessionRepoUrl(currentSession); - if (repoUrl) { - const prStatusResult = await getPullRequestStatus({ - repoUrl, - prNumber: currentSession.prNumber, - token, - }); + const prStatusResult = await getPullRequestStatus({ + repoUrl, + prNumber: currentSession.prNumber, + token, + }); - if (prStatusResult.success && prStatusResult.status) { - if (prStatusResult.status !== currentSession.prStatus) { - updates.prStatus = prStatusResult.status; - } + if (prStatusResult.success && prStatusResult.status) { + if (prStatusResult.status !== currentSession.prStatus) { + updates.prStatus = prStatusResult.status; + } - return updates; - } + return updates; } } - if (!token) { - return updates; - } - const prResult = await findPullRequestByBranch({ - owner: currentSession.repoOwner, - repo: currentSession.repoName, + owner: parsedRepo.owner, + repo: parsedRepo.repo, branchName: branch, token, });Also applies to: 93-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/sandbox/archive-session.ts` around lines 47 - 81, When checking PR status, first resolve the repo identity using the session clone URL as a fallback (use currentSession.cloneUrl if getSessionRepoUrl(currentSession) returns falsy) so sessions with only cloneUrl are handled; also require the service token before making GitHub API calls — if process.env.GITHUB_TOKEN is not present, skip calling getPullRequestStatus (and any other GitHub calls) and return the local updates (e.g., the branch change) instead. Update the logic around token, getSessionRepoUrl, and the getPullRequestStatus calls (including the similar block later at lines ~93-101) to: compute repoUrl = getSessionRepoUrl(...) || derive from currentSession.cloneUrl, check token !== undefined before invoking getPullRequestStatus, and return early with updates when token is missing.apps/web/app/api/sessions/[sessionId]/merge/route.ts (1)
88-92:⚠️ Potential issue | 🟠 MajorAllow
cloneUrl-only sessions to reach the service-token merge path.Line 88 still rejects sessions where
repoOwner/repoNameare null, so org-backed sessions described in the PR never reach the newGITHUB_TOKENmerge flow. RequirecloneUrlfor merge/readiness, and only gate branch deletion on owner availability.Proposed fix
- if ( - !sessionRecord.cloneUrl || - !sessionRecord.repoOwner || - !sessionRecord.repoName - ) { + if (!sessionRecord.cloneUrl) { return Response.json( { error: "Session is not linked to a GitHub repository" }, { status: 400 }, ); }- const normalizedRepoOwner = sessionRecord.repoOwner.toLowerCase(); + const normalizedRepoOwner = sessionRecord.repoOwner?.toLowerCase() ?? null; const normalizedHeadOwner = readiness.pr.headOwner?.toLowerCase() ?? null; if (!normalizedHeadOwner) { branchDeleteError = "Source branch owner could not be determined; branch was not deleted"; + } else if (!normalizedRepoOwner) { + branchDeleteError = + "Repository owner could not be determined; branch was not deleted"; } else if (normalizedHeadOwner !== normalizedRepoOwner) { branchDeleteError = "Source branch belongs to a fork and was not deleted"; } else {Also applies to: 144-155, 235-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/sessions/`[sessionId]/merge/route.ts around lines 88 - 92, The current checks in route handlers (using sessionRecord.cloneUrl, sessionRecord.repoOwner, sessionRecord.repoName) reject sessions that only have cloneUrl and thus prevent org-backed sessions from entering the GITHUB_TOKEN merge flow; update the conditional at the merge/readiness gate (the block referencing sessionRecord.cloneUrl || sessionRecord.repoOwner || sessionRecord.repoName) to require only sessionRecord.cloneUrl for allowing merge/readiness, and move/adjust the repoOwner/repoName checks so they only gate safe branch-deletion logic (i.e., only perform branch deletion when sessionRecord.repoOwner and sessionRecord.repoName are present). Apply this same change pattern to the other similar checks referenced (around the blocks currently at the equivalent of lines 144-155 and 235-243) so clone-only sessions proceed to the service-token merge path while branch deletion remains guarded by owner/name presence.apps/web/app/api/check-pr/route.ts (1)
57-60:⚠️ Potential issue | 🟠 MajorResolve repo info from
cloneUrlbefore checking PRs.Line 58 still returns 400 for org-backed sessions that have
cloneUrlbut nullrepoOwner/repoName, so the new service token at Line 105 is never used for those sessions.Proposed fix shape
+import { parseGitHubRepositoryFromUrl } from "@/lib/github/ensure-authenticated-origin"; + ... - if (!sessionRecord.repoOwner || !sessionRecord.repoName) { + const repository = + sessionRecord.repoOwner && sessionRecord.repoName + ? { owner: sessionRecord.repoOwner, repo: sessionRecord.repoName } + : sessionRecord.cloneUrl + ? parseGitHubRepositoryFromUrl(sessionRecord.cloneUrl) + : null; + + if (!repository) { return Response.json({ error: "No repo info on session" }, { status: 400 }); } ... const prResult = await findPullRequestByBranch({ - owner: sessionRecord.repoOwner, - repo: sessionRecord.repoName, + owner: repository.owner, + repo: repository.repo, branchName: branch, token, });Also applies to: 105-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/check-pr/route.ts` around lines 57 - 60, If sessionRecord.repoOwner or sessionRecord.repoName are missing, parse them from sessionRecord.cloneUrl before returning the 400 so org-backed sessions proceed; update the early-check in the route handler to attempt resolving repoOwner/repoName from sessionRecord.cloneUrl (e.g., extract owner/name from the git URL) and populate sessionRecord.repoOwner and sessionRecord.repoName when found, then continue to the PR-check flow that uses the service token; ensure the same cloneUrl-resolution logic is applied where the service token is selected/used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/api/generate-pr/route.ts`:
- Around line 101-109: The failure reason and git stderr returned by
ensureAuthenticatedOrigin (originResult.reason) and the later fetch/logging code
can contain credential-bearing remote URLs; sanitize those before returning or
logging. Add or call a sanitizer (e.g., sanitizeRemoteUrl or sanitizeMessage)
that strips user:pass or service-token segments from any git URL (replace with
[REDACTED] or remove the credentials) and use it when constructing
Response.json({ error: ... }) for the originResult path and before logging the
fetch output (the fetch/generic git stderr logging around the later
fetchOutput/log call). Ensure sanitizeRemoteUrl is used both for
originResult.reason and the fetch stderr/logging to avoid leaking tokens.
- Around line 112-116: The fork-fallback block currently runs even though const
userToken: string | null = null, which causes service-token permission failures
to fall through and be misreported as “no linked GitHub account”; fix by
guarding the entire disabled fallback so it only executes when a real user token
exists (e.g., if (userToken) { /* existing fallback logic */ } else { rethrow or
propagate the original service-token error }), updating the fallback references
around the userToken declaration and the similar guarded block(s) in the 450-567
region so permission errors from the service token are not swallowed.
In `@apps/web/lib/github/ensure-authenticated-origin.ts`:
- Around line 34-62: Reject returning raw cloneUrl and git output in error
reasons; instead sanitize them before including in the returned reason. In the
code around parseGitHubUrl, buildGitHubAuthRemoteUrl, and after sandbox.exec,
strip credentials from cloneUrl (e.g., remove userinfo from the URL) and
truncate/replace any token-like strings from result.stderr/result.stdout (or
replace entire git output with "<redacted git output>" when it contains "http"
with userinfo or matches token patterns) before interpolating into the error
messages so no secrets are leaked.
---
Outside diff comments:
In `@apps/web/app/api/check-pr/route.ts`:
- Around line 57-60: If sessionRecord.repoOwner or sessionRecord.repoName are
missing, parse them from sessionRecord.cloneUrl before returning the 400 so
org-backed sessions proceed; update the early-check in the route handler to
attempt resolving repoOwner/repoName from sessionRecord.cloneUrl (e.g., extract
owner/name from the git URL) and populate sessionRecord.repoOwner and
sessionRecord.repoName when found, then continue to the PR-check flow that uses
the service token; ensure the same cloneUrl-resolution logic is applied where
the service token is selected/used.
In `@apps/web/app/api/pr/route.ts`:
- Around line 137-164: Before calling createPullRequest with the shared service
token (userToken), verify and bind the requested repo (repoUrl) to the current
sessionRecord: check that repoUrl equals sessionRecord.cloneUrl or matches
sessionRecord.repo metadata (e.g., owner/name) and only proceed if it matches;
if it doesn't match return a 403/error response. Perform this authorization
check immediately after computing repoUrl and before constructing
tokenUsedForCreation/createPullRequest, using the existing sessionRecord (or
session lookup) to validate ownership.
In `@apps/web/app/api/sessions/`[sessionId]/checks/fix/route.ts:
- Around line 206-246: The early return when sessionRecord.repoOwner or
sessionRecord.repoName are missing prevents using sessionRecord.cloneUrl to
derive owner/repo for org-backed sessions; change the logic so before returning
the 400 error you attempt to parse sessionRecord.cloneUrl (handle
git@github.com:owner/repo(.git) and https://github.com/owner/repo(.git) forms)
to populate owner/repo, and only call the Response.json error if both
repoOwner/repoName are absent and cloneUrl parsing fails; update the places that
use owner and repo (currently assigned from sessionRecord.repoOwner/repoName) to
use the derived values when present.
In `@apps/web/app/api/sessions/`[sessionId]/close-pr/route.ts:
- Around line 34-38: The current guard erroneously requires repoOwner and
repoName even when cloneUrl is available; update the conditional in route.ts
around sessionRecord to allow proceeding if cloneUrl exists or if both repoOwner
and repoName are present — e.g. replace the check with one that errors only when
neither cloneUrl nor (repoOwner && repoName) is provided so closePullRequest can
work with repoUrl (reference symbols: sessionRecord.cloneUrl,
sessionRecord.repoOwner, sessionRecord.repoName, closePullRequest).
In `@apps/web/app/api/sessions/`[sessionId]/merge-readiness/route.ts:
- Around line 101-106: The code rejects sessions if
sessionRecord.repoOwner/repoName are missing even when sessionRecord.cloneUrl
contains the repo identity; update the logic around repoIdentifier to parse
owner/name from cloneUrl when repoOwner or repoName are null: implement a small
helper (e.g., parseRepoFromCloneUrl) that accepts sessionRecord.cloneUrl and
returns {repoOwner, repoName} for common formats (https, git+ssh, ssh) and then
set repoIdentifier = sessionRecord.repoOwner && sessionRecord.repoName ?
`${sessionRecord.repoOwner}/${sessionRecord.repoName}` : parsed ?
`${parsed.repoOwner}/${parsed.repoName}` : null; finally use this repoIdentifier
(and parsed owner/name) in the existing conditional that currently checks
sessionRecord.cloneUrl || repoIdentifier || sessionRecord.repoOwner so sessions
with only cloneUrl succeed.
In `@apps/web/app/api/sessions/`[sessionId]/merge/route.ts:
- Around line 88-92: The current checks in route handlers (using
sessionRecord.cloneUrl, sessionRecord.repoOwner, sessionRecord.repoName) reject
sessions that only have cloneUrl and thus prevent org-backed sessions from
entering the GITHUB_TOKEN merge flow; update the conditional at the
merge/readiness gate (the block referencing sessionRecord.cloneUrl ||
sessionRecord.repoOwner || sessionRecord.repoName) to require only
sessionRecord.cloneUrl for allowing merge/readiness, and move/adjust the
repoOwner/repoName checks so they only gate safe branch-deletion logic (i.e.,
only perform branch deletion when sessionRecord.repoOwner and
sessionRecord.repoName are present). Apply this same change pattern to the other
similar checks referenced (around the blocks currently at the equivalent of
lines 144-155 and 235-243) so clone-only sessions proceed to the service-token
merge path while branch deletion remains guarded by owner/name presence.
In `@apps/web/lib/chat/auto-commit-direct.ts`:
- Around line 41-55: The code currently continues to create a local commit even
when remote auth isn't configured; make auth setup a hard precondition by
failing early: when process.env.GITHUB_TOKEN (repoToken) is absent, throw or
return an error before any commit work, and when
buildGitHubAuthRemoteUrl(repoOwner, repoName, token) returns falsy or
sandbox.exec(`git remote set-url origin ...`, cwd, 10000) throws/returns a
failure, propagate that error (stop execution) instead of continuing; update the
auth block around repoToken, buildGitHubAuthRemoteUrl, and sandbox.exec to
validate inputs and surface errors immediately so commit/push do not proceed
without a working authenticated remote.
- Around line 106-114: The branch name read into currentBranch is interpolated
into the shell command passed to sandbox.exec for the git push, so shell-quote
or escape currentBranch before using it in the
`sandbox.exec(\`GIT_TERMINAL_PROMPT=0 git push -u origin ${currentBranch}\`,
...)` call to prevent word-splitting or command injection; update the git push
invocation in auto-commit-direct.ts to use a safely quoted/escaped version of
currentBranch (matching the pattern used at lines 76/80/90) so the branch string
is passed as a single literal argument to git.
In `@apps/web/lib/chat/auto-pr-direct.ts`:
- Around line 150-166: The authenticated remote setup can fail silently because
the call to sandbox.exec("git remote set-url origin ...", cwd, 10000) isn't
handled; update the code around buildGitHubAuthRemoteUrl and the sandbox.exec
call so any failure is surfaced immediately: either wrap sandbox.exec in a
try/catch and return/throw a clear error (e.g., throw new Error or return an
object with an explicit error message) that includes the sandbox.exec error
output, or check its return status and return a failure result instead of
continuing; specifically modify the block that calls sandbox.exec to propagate
the failure (include the error text) so downstream logic doesn't misreport
missing/unpushed branches.
In `@apps/web/lib/sandbox/archive-session.ts`:
- Around line 47-81: When checking PR status, first resolve the repo identity
using the session clone URL as a fallback (use currentSession.cloneUrl if
getSessionRepoUrl(currentSession) returns falsy) so sessions with only cloneUrl
are handled; also require the service token before making GitHub API calls — if
process.env.GITHUB_TOKEN is not present, skip calling getPullRequestStatus (and
any other GitHub calls) and return the local updates (e.g., the branch change)
instead. Update the logic around token, getSessionRepoUrl, and the
getPullRequestStatus calls (including the similar block later at lines ~93-101)
to: compute repoUrl = getSessionRepoUrl(...) || derive from
currentSession.cloneUrl, check token !== undefined before invoking
getPullRequestStatus, and return early with updates when token is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 626390a1-ee29-42f6-b4db-c472815a4e83
📒 Files selected for processing (13)
apps/web/app/api/chat/_lib/runtime.tsapps/web/app/api/chat/route.tsapps/web/app/api/check-pr/route.tsapps/web/app/api/generate-pr/route.tsapps/web/app/api/pr/route.tsapps/web/app/api/sessions/[sessionId]/checks/fix/route.tsapps/web/app/api/sessions/[sessionId]/close-pr/route.tsapps/web/app/api/sessions/[sessionId]/merge-readiness/route.tsapps/web/app/api/sessions/[sessionId]/merge/route.tsapps/web/lib/chat/auto-commit-direct.tsapps/web/lib/chat/auto-pr-direct.tsapps/web/lib/github/ensure-authenticated-origin.tsapps/web/lib/sandbox/archive-session.ts
💤 Files with no reviewable changes (1)
- apps/web/app/api/chat/route.ts
Comment on lines
+101
to
+109
| const originResult = await ensureAuthenticatedOrigin({ | ||
| sandbox, | ||
| cloneUrl: sessionRecord.cloneUrl, | ||
| }); | ||
| if (!originResult.ok) { | ||
| return Response.json( | ||
| { error: `Failed to configure git remote: ${originResult.reason}` }, | ||
| { status: 500 }, | ||
| ); |
There was a problem hiding this comment.
Redact service-token remote output before returning or logging it.
After Line 101 configures an authenticated remote, helper failure reasons and git stderr can include credential-bearing URLs. Line 107 returns the reason to the client and Line 138 logs fetch output raw.
Proposed fix
if (!originResult.ok) {
return Response.json(
- { error: `Failed to configure git remote: ${originResult.reason}` },
+ {
+ error: `Failed to configure git remote: ${redactGitHubToken(
+ originResult.reason,
+ )}`,
+ },
{ status: 500 },
);
}+ const fetchStdout = redactGitHubToken(fetchResult.stdout.trim());
+ const fetchStderr = redactGitHubToken(fetchResult.stderr?.trim() ?? "");
console.log(
- `[generate-pr] Fetch result: success=${fetchResult.success}, stdout=${fetchResult.stdout.trim()}, stderr=${fetchResult.stderr?.trim() ?? ""}`,
+ `[generate-pr] Fetch result: success=${fetchResult.success}, stdout=${fetchStdout}, stderr=${fetchStderr}`,
);Also applies to: 137-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/generate-pr/route.ts` around lines 101 - 109, The failure
reason and git stderr returned by ensureAuthenticatedOrigin
(originResult.reason) and the later fetch/logging code can contain
credential-bearing remote URLs; sanitize those before returning or logging. Add
or call a sanitizer (e.g., sanitizeRemoteUrl or sanitizeMessage) that strips
user:pass or service-token segments from any git URL (replace with [REDACTED] or
remove the credentials) and use it when constructing Response.json({ error: ...
}) for the originResult path and before logging the fetch output (the
fetch/generic git stderr logging around the later fetchOutput/log call). Ensure
sanitizeRemoteUrl is used both for originResult.reason and the fetch
stderr/logging to avoid leaking tokens.
Comment on lines
+112
to
+116
| // Personal-fork push fallback from the template fork is dead code now that | ||
| // all repos are recoupable-owned and pushed with the service token. Kept as | ||
| // a guarded block so the surrounding logic compiles; scheduled for removal | ||
| // in the follow-up YAGNI cleanup. | ||
| const userToken: string | null = null; |
There was a problem hiding this comment.
Guard the disabled fork fallback so permission errors stay accurate.
userToken is always null, but service-token push permission failures can still enter the fork-fallback block and return “no linked GitHub account,” masking the real service-token access/configuration problem.
Minimal guard until the cleanup PR removes this block
if (
+ userToken &&
!gitActions.pushed &&
isPermissionError &&
sessionRecord.repoOwner &&
sessionRecord.repoName
) {
const githubAccount = await getGitHubAccount(session.user.id);
- if (userToken && githubAccount?.username) {
+ if (githubAccount?.username) {Also applies to: 450-567
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/generate-pr/route.ts` around lines 112 - 116, The
fork-fallback block currently runs even though const userToken: string | null =
null, which causes service-token permission failures to fall through and be
misreported as “no linked GitHub account”; fix by guarding the entire disabled
fallback so it only executes when a real user token exists (e.g., if (userToken)
{ /* existing fallback logic */ } else { rethrow or propagate the original
service-token error }), updating the fallback references around the userToken
declaration and the similar guarded block(s) in the 450-567 region so permission
errors from the service token are not swallowed.
Comment on lines
+34
to
+62
| const parsed = parseGitHubUrl(cloneUrl); | ||
| if (!parsed) { | ||
| return { ok: false, reason: `Unable to parse GitHub URL: ${cloneUrl}` }; | ||
| } | ||
|
|
||
| const authUrl = buildGitHubAuthRemoteUrl({ | ||
| token, | ||
| owner: parsed.owner, | ||
| repo: parsed.repo, | ||
| }); | ||
| if (!authUrl) { | ||
| return { | ||
| ok: false, | ||
| reason: `Rejected owner/repo from cloneUrl: ${parsed.owner}/${parsed.repo}`, | ||
| }; | ||
| } | ||
|
|
||
| const result = await sandbox.exec( | ||
| `git remote set-url origin "${authUrl}"`, | ||
| sandbox.workingDirectory, | ||
| 5000, | ||
| ); | ||
|
|
||
| if (!result.success) { | ||
| const stderr = (result.stderr || result.stdout || "unknown error").slice( | ||
| 0, | ||
| 200, | ||
| ); | ||
| return { ok: false, reason: `git remote set-url failed: ${stderr}` }; |
There was a problem hiding this comment.
Redact clone URLs and git output before returning failure reasons.
cloneUrl and git remote output can contain embedded credentials; returning them in reason risks leaking the service token or other auth material.
🛡️ Proposed redaction
+function redactGitCredentials(value: string): string {
+ return value.replace(
+ /https:\/\/[^/@\s]+:[^/@\s]+@github\.com/gi,
+ "https://[redacted]@github.com",
+ );
+}
+
const parsed = parseGitHubUrl(cloneUrl);
if (!parsed) {
- return { ok: false, reason: `Unable to parse GitHub URL: ${cloneUrl}` };
+ return { ok: false, reason: "Unable to parse GitHub URL from session cloneUrl" };
}
@@
if (!result.success) {
const stderr = (result.stderr || result.stdout || "unknown error").slice(
0,
200,
);
- return { ok: false, reason: `git remote set-url failed: ${stderr}` };
+ return {
+ ok: false,
+ reason: `git remote set-url failed: ${redactGitCredentials(stderr)}`,
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parsed = parseGitHubUrl(cloneUrl); | |
| if (!parsed) { | |
| return { ok: false, reason: `Unable to parse GitHub URL: ${cloneUrl}` }; | |
| } | |
| const authUrl = buildGitHubAuthRemoteUrl({ | |
| token, | |
| owner: parsed.owner, | |
| repo: parsed.repo, | |
| }); | |
| if (!authUrl) { | |
| return { | |
| ok: false, | |
| reason: `Rejected owner/repo from cloneUrl: ${parsed.owner}/${parsed.repo}`, | |
| }; | |
| } | |
| const result = await sandbox.exec( | |
| `git remote set-url origin "${authUrl}"`, | |
| sandbox.workingDirectory, | |
| 5000, | |
| ); | |
| if (!result.success) { | |
| const stderr = (result.stderr || result.stdout || "unknown error").slice( | |
| 0, | |
| 200, | |
| ); | |
| return { ok: false, reason: `git remote set-url failed: ${stderr}` }; | |
| function redactGitCredentials(value: string): string { | |
| return value.replace( | |
| /https:\/\/[^/@\s]+:[^/@\s]+@github\.com/gi, | |
| "https://[redacted]@github.com", | |
| ); | |
| } | |
| const parsed = parseGitHubUrl(cloneUrl); | |
| if (!parsed) { | |
| return { ok: false, reason: "Unable to parse GitHub URL from session cloneUrl" }; | |
| } | |
| const authUrl = buildGitHubAuthRemoteUrl({ | |
| token, | |
| owner: parsed.owner, | |
| repo: parsed.repo, | |
| }); | |
| if (!authUrl) { | |
| return { | |
| ok: false, | |
| reason: `Rejected owner/repo from cloneUrl: ${parsed.owner}/${parsed.repo}`, | |
| }; | |
| } | |
| const result = await sandbox.exec( | |
| `git remote set-url origin "${authUrl}"`, | |
| sandbox.workingDirectory, | |
| 5000, | |
| ); | |
| if (!result.success) { | |
| const stderr = (result.stderr || result.stdout || "unknown error").slice( | |
| 0, | |
| 200, | |
| ); | |
| return { | |
| ok: false, | |
| reason: `git remote set-url failed: ${redactGitCredentials(stderr)}`, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/github/ensure-authenticated-origin.ts` around lines 34 - 62,
Reject returning raw cloneUrl and git output in error reasons; instead sanitize
them before including in the returned reason. In the code around parseGitHubUrl,
buildGitHubAuthRemoteUrl, and after sandbox.exec, strip credentials from
cloneUrl (e.g., remove userinfo from the URL) and truncate/replace any
token-like strings from result.stderr/result.stdout (or replace entire git
output with "<redacted git output>" when it contains "http" with userinfo or
matches token patterns) before interpolating into the error messages so no
secrets are leaked.
Contributor
Author
Preview-deployment test — ✅ passesExercised the fix end-to-end on the PR #10 preview deploy ( Setup
Steps
OutcomeWithin the 90s wait window, the panel updated to:
Before this fixBefore the
Sandbox-side evidence
What wasn't tested
Ready to merge from my side. |
7 tasks
sweetmantech
added a commit
that referenced
this pull request
Apr 23, 2026
* refactor: remove legacy personal-GitHub-auth flows (YAGNI) Rips out the \"user connects their own GitHub, picks their own repo, pushes to their fork\" infrastructure that came from the original Open Agents template. PR #10 already swapped all repo operations to the service GITHUB_TOKEN; this PR deletes the now-unused code. Deleted (~6,900 LOC, 29 files): - lib/github/user-token.ts (getUserGitHubToken helper) - lib/github/{installation-repos,installations-sync,connection-status,auth-url,installation-url}.ts - 8 user-identity API routes under /api/github/ (user, orgs, connection-status, orgs/install-status, installations/repos, installations, branches, create-repo, app/install, app/callback) - 10 UI components: repo-selector{,-compact}, branch-selector{,-compact}, branch-picker-dialog, create-repo-dialog, create-pr-dialog, github-reconnect-{dialog,gate}, repo-selection-screen - 2 hooks: use-github-connection-status, use-installation-repos - Settings: accounts-section + /settings/accounts + /settings/connections routes - Fork-push fallback in generate-pr/route.ts (dead since PR #10) Modified: - providers.tsx: drop GitHubReconnectGate wrapper - settings/layout.tsx: remove Connections nav item + skeleton - sessions-route-shell.tsx: drop handleCreateSessionForRepo/FromBranch - inbox-sidebar.tsx: drop onCreateSessionForRepo/FromBranch props, repo-group hover buttons (Create session + Create from branch) - session-chat-content.tsx: drop repoDialogOpen state, CreateRepoDialog render - git-panel.tsx: drop onCreateRepoClick prop, supportsRepoCreation destructure, Create Repo button - lib/github/client.ts: getOctokit no longer falls back to user token What's NOT in this PR (follow-up): - Auto-commit / auto-PR flows still gated on repoOwner+repoName (never run for org-backed sessions). Re-gating on cloneUrl to activate them as the default post-turn flow is PR C. - OAuth DB tables (github_accounts) left alone — orphan rows harmless. - Fork-related helpers in generate-pr/_lib/generate-pr-helpers.ts still exist but no longer imported anywhere. Typecheck + lint clean. Test failures are pre-existing 'getChatsBySessionId' export errors unrelated to this change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: remove stale references to deleted /api/github/branches + fork fallback Audit after the main PR B sweep found runtime regressions and dead code left behind: Runtime fixes (blocking): - lib/git-flow-client.ts: drop \`fetchRepoBranches\` — it called the now- deleted /api/github/branches endpoint (would 404 at runtime) - commit-dialog.tsx: remove the base-branch <Select> dropdown (fed by fetchRepoBranches); fall back to a static \"main\" default. UI now shows a plain info line instead of a branch picker. - git-panel.tsx: remove the fetchRepoBranches effect that overwrote baseBranch on mount. Session uses the default \"main\" baseBranch. Dead code cleanup: - lib/chat-auto-commit.ts (+ test) deleted — only caller was the already- dead auto-commit path that also hit /api/github/branches. Will be rewritten in PR C. - lib/git-flow-client.test.ts deleted — exclusively tested the dropped fetchRepoBranches + pushedToFork fields. - generate-pr-helpers.ts: drop ensureForkExists, forkPushRetryConfig, isRetryableForkPushError, sleepForForkRetry, extractGitHubOwnerFromRemoteUrl and their types. Test file pruned to match. - git-flow-client.ts types: remove pushedToFork + prHeadOwner — server no longer returns them after PR B. - commit-dialog.tsx: drop prHeadOwner state + all three pushedToFork conditionals. Commit-display URL now always uses session.repoOwner. - git-panel.tsx: drop prHeadOwner state + the owner-mismatch \"open compare page instead\" branch for PR creation. All pushes target the same recoupable-owned origin so the mismatch path is unreachable. Net: −1,039 / +18 on top of the initial 45-file deletion. Typecheck + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * nit: baseBranch is a constant, not useState Addresses CodeRabbit's trivial cleanup request on PR #11. Both files just had the dropdown's onChange handler removed in the previous commit, leaving useState with no setter — replaced with a plain const. The companion "non-main default branch" P1 from both bots is domain-inapplicable: all sessions clone recoupable/org-* repos, which use main as the convention across the codebase. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
sweetmantech
added a commit
that referenced
this pull request
Apr 23, 2026
Four GitHub API routes guarded on sessionRecord.repoOwner && sessionRecord.repoName and returned "Session is not linked to a GitHub repository" when either was null. OrgSelector sessions populate cloneUrl only, so those guards always failed — users could commit and push but couldn't merge/close/fix-checks their PR. Same bug class that PR #10 fixed for push auth (generate-pr). - New helper lib/github/resolve-session-repo.ts parses owner/repo from cloneUrl via parseGitHubUrl as a fallback when repoOwner/repoName are missing. Returns null when neither source provides a usable repo. - /api/sessions/[id]/merge-readiness: uses resolveSessionRepo for repoIdentifier display + cloneUrl passed to GitHub API - /api/sessions/[id]/merge: same; also uses repo.owner for the source- branch-owner check when deleting the merged branch - /api/sessions/[id]/checks/fix: octokit calls now use repo.owner + repo.repo - /api/sessions/[id]/close-pr: uses repo.cloneUrl for closePullRequest Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the "Permission denied. Check your GitHub access." error when clicking the "Commit and Push" button for org-backed sessions.
Root cause
In
apps/web/app/api/generate-pr/route.ts, the push-auth setup was guarded by:After PR #7 switched session creation to the OrgSelector flow (
createBlankSession(cloneUrl)), sessions only havecloneUrlpopulated —repoOwner/repoNamearenull. The guard short-circuited, the remote stayed ashttps://github.com/recoupable/org-…with no credentials, and the subsequentgit pushgot "Permission denied" from GitHub.Fix
apps/web/lib/github/ensure-authenticated-origin.ts— parses owner/repo from the session'scloneUrland rewrites the sandbox'soriginremote tohttps://x-access-token:${GITHUB_TOKEN}@github.com/<owner>/<repo>.gitusing the serviceGITHUB_TOKEN./api/generate-prreplaces the buggy guard with the helper. Fork-fallback block is now dead code (userToken is always null) — will be removed in PR B./api/pr,/api/check-pr/api/sessions/[sessionId]/{merge,merge-readiness,close-pr,checks/fix}app/api/chat/_lib/runtime.ts,lib/sandbox/archive-session.tsauto-commit-direct.ts/auto-pr-direct.tsalso swap to the service token (no behavior change for org sessions today since they're gated elsewhere byrepoOwner+repoName, but brings their token source into alignment).All repos are recoupable-owned per PR #7 policy, so the service token always has access. The per-user OAuth token path is no longer needed for these operations.
What's NOT in this PR
Per scope discussion, PR A is the focused fix. PR B (follow-up) removes:
getUserGitHubToken/lib/github/user-token.tsentirely/api/github/{user,orgs,connection-status,installations/repos,branches,create-repo,app/callback,orgs/install-status})Test plan
bun run checkandturbo typecheckpass ✅Net diff
+99 / -52 across 13 files.
🤖 Generated with Claude Code
Summary by cubic
Use the service GITHUB_TOKEN for all git pushes and GitHub API calls, and set an authenticated
originfrom the sessioncloneUrl. Fixes the "Permission denied" error when clicking "Commit and Push" for org-backed sessions.Bug Fixes
/api/generate-pr, replace therepoOwner/repoNameguard withensureAuthenticatedOrigin(...)to rewriteoriginto anx-access-tokenURL parsed fromcloneUrl.Refactors
apps/web/lib/github/service-token.ts(getServiceGitHubToken()) to centralizeGITHUB_TOKENaccess; update call sites (sandbox create, org snapshot, archive, and API routes).apps/web/lib/github/ensure-authenticated-origin.tsto centralize remote auth; alignauto-commit-direct.tsandauto-pr-direct.tsto the service token and removeuserIdtoken plumbing from the chat runtime.Written for commit 2f87e93. Summary will update on new commits.
Summary by CodeRabbit