feat(sandbox): plumb per-account gitUser into POST /api/sandbox#534
Merged
sweetmantech merged 1 commit intotestfrom May 8, 2026
Merged
feat(sandbox): plumb per-account gitUser into POST /api/sandbox#534sweetmantech merged 1 commit intotestfrom
sweetmantech merged 1 commit intotestfrom
Conversation
Open-agents passes a per-user `gitUser = { name, email }` to
`connectSandbox`, which the sandbox runtime applies via
`git config user.name` / `user.email` so commit objects carry that
identity. The push credential (a single hardcoded service GitHub
token) is unrelated — `gitUser` is purely about commit *authorship*.
The api was the lone gap: it never resolved a `gitUser`, so commits
made inside an api-provisioned sandbox would either fail (no author)
or fall back to whatever was baked into the snapshot image.
Adds `resolveGitUser(accountId)`:
- looks up `accounts.name` for the display name
- looks up `account_emails.email` for the address
- falls back to `recoupable-{accountId.slice(0,8)}` /
`{accountId}@users.noreply.recoupable.com` when either is missing
- returns `{ name, email }` ready to forward into `connectSandbox`
Wires it into `createSandboxHandler` so each request derives its
gitUser from the validated `auth.accountId`.
TDD: red test for `resolveGitUser` (5 cases — populated values,
missing name fallback, missing email fallback, both-missing, null
email row), then green; red test for handler integration, then green.
Tests 2585 / 2585 pass. Lint + tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds account-specific git user identity resolution for sandboxes. A new ChangesGit Identity Resolution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/sandbox/createSandboxHandler.ts`:
- Around line 86-91: The call to resolveGitUser is currently executed outside
the provisioning error handling path, so any exception it throws bypasses the
structured error response; wrap the resolveGitUser call in the same try/catch
used for provisioning (or add a dedicated try/catch immediately around
resolveGitUser) inside createSandboxHandler, catch and convert failures into the
same structured error response/return used by the provisioning flow (same shape
and logging), and only proceed to call provisionSandbox (or the provisioning
logic) when resolveGitUser succeeds.
In `@lib/sandbox/resolveGitUser.ts`:
- Around line 33-37: The selected email may be whitespace-only; update the logic
that computes email (using emails, emailRow, accountId, and name) to trim and
validate the found email before using it: derive a trimmedEmail from
emailRow?.email, check that trimmedEmail is a non-empty string, and only then
use it; otherwise fall back to the default
`${accountId}@users.noreply.recoupable.com`. Ensure the existing name fallback
using account?.name?.trim() remains unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4985606-2570-4f8a-a276-99999532aa16
⛔ Files ignored due to path filters (2)
lib/sandbox/__tests__/createSandboxHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/resolveGitUser.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/sandbox/createSandboxHandler.tslib/sandbox/resolveGitUser.ts
Comment on lines
+86
to
+91
| // Per-account `gitUser` controls commit authorship inside the sandbox | ||
| // (`git config user.name` / `user.email`). The push credential is a | ||
| // separate hardcoded service token — `gitUser` is purely about who | ||
| // each commit object is *authored* by. | ||
| const gitUser = await resolveGitUser(auth.accountId); | ||
|
|
There was a problem hiding this comment.
Guard resolveGitUser failures before provisioning.
At Line 90, resolveGitUser is outside the provisioning try/catch. If account/email lookup throws, this request skips your structured error response path.
Proposed fix
- const gitUser = await resolveGitUser(auth.accountId);
+ let gitUser: { name: string; email: string };
+ try {
+ gitUser = await resolveGitUser(auth.accountId);
+ } catch (error) {
+ console.error("[createSandboxHandler] resolveGitUser failed:", error);
+ gitUser = {
+ name: `recoupable-${auth.accountId.slice(0, 8)}`,
+ email: `${auth.accountId}@users.noreply.recoupable.com`,
+ };
+ }As per coding guidelines, “Handle errors gracefully”.
📝 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
| // Per-account `gitUser` controls commit authorship inside the sandbox | |
| // (`git config user.name` / `user.email`). The push credential is a | |
| // separate hardcoded service token — `gitUser` is purely about who | |
| // each commit object is *authored* by. | |
| const gitUser = await resolveGitUser(auth.accountId); | |
| // Per-account `gitUser` controls commit authorship inside the sandbox | |
| // (`git config user.name` / `user.email`). The push credential is a | |
| // separate hardcoded service token — `gitUser` is purely about who | |
| // each commit object is *authored* by. | |
| let gitUser: { name: string; email: string }; | |
| try { | |
| gitUser = await resolveGitUser(auth.accountId); | |
| } catch (error) { | |
| console.error("[createSandboxHandler] resolveGitUser failed:", error); | |
| gitUser = { | |
| name: `recoupable-${auth.accountId.slice(0, 8)}`, | |
| email: `${auth.accountId}@users.noreply.recoupable.com`, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/sandbox/createSandboxHandler.ts` around lines 86 - 91, The call to
resolveGitUser is currently executed outside the provisioning error handling
path, so any exception it throws bypasses the structured error response; wrap
the resolveGitUser call in the same try/catch used for provisioning (or add a
dedicated try/catch immediately around resolveGitUser) inside
createSandboxHandler, catch and convert failures into the same structured error
response/return used by the provisioning flow (same shape and logging), and only
proceed to call provisionSandbox (or the provisioning logic) when resolveGitUser
succeeds.
Comment on lines
+33
to
+37
| const emailRow = emails.find(row => typeof row.email === "string" && row.email.length > 0); | ||
|
|
||
| const name = account?.name?.trim() || `recoupable-${accountId.slice(0, 8)}`; | ||
| const email = emailRow?.email ?? `${accountId}@users.noreply.recoupable.com`; | ||
|
|
There was a problem hiding this comment.
Trim and validate the selected email before use.
At Line 33/Line 36, whitespace-only values can pass and be forwarded as gitUser.email. Normalize with trim() and fallback when empty.
Proposed fix
- const emailRow = emails.find(row => typeof row.email === "string" && row.email.length > 0);
+ const emailRow = emails.find(
+ row => typeof row.email === "string" && row.email.trim().length > 0,
+ );
@@
- const email = emailRow?.email ?? `${accountId}@users.noreply.recoupable.com`;
+ const email = emailRow?.email?.trim() || `${accountId}@users.noreply.recoupable.com`;📝 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 emailRow = emails.find(row => typeof row.email === "string" && row.email.length > 0); | |
| const name = account?.name?.trim() || `recoupable-${accountId.slice(0, 8)}`; | |
| const email = emailRow?.email ?? `${accountId}@users.noreply.recoupable.com`; | |
| const emailRow = emails.find( | |
| row => typeof row.email === "string" && row.email.trim().length > 0, | |
| ); | |
| const name = account?.name?.trim() || `recoupable-${accountId.slice(0, 8)}`; | |
| const email = emailRow?.email?.trim() || `${accountId}@users.noreply.recoupable.com`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/sandbox/resolveGitUser.ts` around lines 33 - 37, The selected email may
be whitespace-only; update the logic that computes email (using emails,
emailRow, accountId, and name) to trim and validate the found email before using
it: derive a trimmedEmail from emailRow?.email, check that trimmedEmail is a
non-empty string, and only then use it; otherwise fall back to the default
`${accountId}@users.noreply.recoupable.com`. Ensure the existing name fallback
using account?.name?.trim() remains unchanged.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 3/5
- There is a concrete regression risk in
lib/sandbox/createSandboxHandler.ts:resolveGitUsermay throw before the guarded provisioning block, which can cause uncaught failures onPOST /api/sandbox. lib/sandbox/resolveGitUser.tsshould trim/normalize email input before validation; whitespace-only values can bypass fallback logic and lead to invalid git config data.- Given the high-confidence findings and user-facing failure path, this carries some merge risk until error handling and input normalization are tightened.
- Pay close attention to
lib/sandbox/createSandboxHandler.tsandlib/sandbox/resolveGitUser.ts- uncaught pre-guard exceptions and weak email normalization can break sandbox setup.
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="lib/sandbox/createSandboxHandler.ts">
<violation number="1" location="lib/sandbox/createSandboxHandler.ts:90">
P2: `resolveGitUser` can throw before your guarded sandbox-provision block, introducing an uncaught failure path for `POST /api/sandbox`.</violation>
</file>
<file name="lib/sandbox/resolveGitUser.ts">
<violation number="1" location="lib/sandbox/resolveGitUser.ts:33">
P2: Validate and normalize email with `.trim()` before accepting it; whitespace-only emails currently bypass the fallback and can produce invalid git config.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as POST /api/sandbox
participant Auth as Auth Middleware
participant GitResolver as resolveGitUser()
participant Accounts as selectAccounts()
participant Emails as selectAccountEmails()
participant Sandbox as connectSandbox()
participant Runtime as Vercel Sandbox Runtime
Note over Client,Runtime: Per-account gitUser plumbed into sandbox creation flow
Client->>API: POST /api/sandbox (with session cookie)
API->>Auth: Validate session & extract accountId
Auth-->>API: auth.accountId
Note over API,Runtime: NEW: Resolve per-account git identity
API->>GitResolver: resolveGitUser(auth.accountId)
GitResolver->>Accounts: selectAccounts(accountId)
GitResolver->>Emails: selectAccountEmails({ accountIds: accountId })
Accounts-->>GitResolver: [{ id, name }] or []
Emails-->>GitResolver: [{ email }] or []
alt Account has name AND email exists
GitResolver->>GitResolver: name = accounts[0].name, email = emailRow.email
else Name missing, email present
GitResolver->>GitResolver: name = "recoupable-{accountId[:8]}", email = emailRow.email
else Name present, no email row
GitResolver->>GitResolver: name = accounts[0].name, email = "{accountId}@users.noreply.recoupable.com"
else Both missing
GitResolver->>GitResolver: name = "recoupable-{accountId[:8]}", email = "{accountId}@users.noreply.recoupable.com"
end
GitResolver-->>API: { name, email }
Note over API,Runtime: Forward gitUser to sandbox creation
API->>Sandbox: connectSandbox({ options: { gitUser, githubToken, ... } })
Sandbox->>Runtime: Configure workspace with gitUser
Runtime->>Runtime: git config user.name = gitUser.name
Runtime->>Runtime: git config user.email = gitUser.email
Sandbox-->>API: sandbox instance
API-->>Client: 200 + sandbox details
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // (`git config user.name` / `user.email`). The push credential is a | ||
| // separate hardcoded service token — `gitUser` is purely about who | ||
| // each commit object is *authored* by. | ||
| const gitUser = await resolveGitUser(auth.accountId); |
There was a problem hiding this comment.
P2: resolveGitUser can throw before your guarded sandbox-provision block, introducing an uncaught failure path for POST /api/sandbox.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/createSandboxHandler.ts, line 90:
<comment>`resolveGitUser` can throw before your guarded sandbox-provision block, introducing an uncaught failure path for `POST /api/sandbox`.</comment>
<file context>
@@ -82,6 +83,12 @@ export async function createSandboxHandler(request: NextRequest): Promise<NextRe
+ // (`git config user.name` / `user.email`). The push credential is a
+ // separate hardcoded service token — `gitUser` is purely about who
+ // each commit object is *authored* by.
+ const gitUser = await resolveGitUser(auth.accountId);
+
let sandbox;
</file context>
Suggested change
| const gitUser = await resolveGitUser(auth.accountId); | |
| let gitUser; | |
| try { | |
| gitUser = await resolveGitUser(auth.accountId); | |
| } catch (error) { | |
| console.error("[createSandboxHandler] resolveGitUser failed:", error); | |
| gitUser = { | |
| name: `recoupable-${auth.accountId.slice(0, 8)}`, | |
| email: `${auth.accountId}@users.noreply.recoupable.com`, | |
| }; | |
| } |
| ]); | ||
|
|
||
| const account = accounts[0] ?? null; | ||
| const emailRow = emails.find(row => typeof row.email === "string" && row.email.length > 0); |
There was a problem hiding this comment.
P2: Validate and normalize email with .trim() before accepting it; whitespace-only emails currently bypass the fallback and can produce invalid git config.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/resolveGitUser.ts, line 33:
<comment>Validate and normalize email with `.trim()` before accepting it; whitespace-only emails currently bypass the fallback and can produce invalid git config.</comment>
<file context>
@@ -0,0 +1,39 @@
+ ]);
+
+ const account = accounts[0] ?? null;
+ const emailRow = emails.find(row => typeof row.email === "string" && row.email.length > 0);
+
+ const name = account?.name?.trim() || `recoupable-${accountId.slice(0, 8)}`;
</file context>
Contributor
Author
|
Smoke test on preview `bd4e9463` (`https://api-3260wobve-recoup.vercel.app\`):
``` Both values match the api-key's owning account (`accounts.name` + `account_emails.email`) — the resolver, the handler, and the sandbox runtime all forward correctly. Commit authorship is per-account as designed. |
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
TDD: red `resolveGitUser` test (5 cases) → green; red `createSandboxHandler` integration test → green.
Test plan
🤖 Generated with Claude Code
Summary by cubic
Passes a per-account Git user (name and email) into sandbox creation so commits are authored by the correct user. Adds a resolver that pulls values from
accountsandaccount_emailswith stable no-PII fallbacks.resolveGitUser(accountId)which usesaccounts.nameandaccount_emails.email, falling back torecoupable-{accountId.slice(0,8)}and{accountId}@users.noreply.recoupable.com.POST /api/sandboxnow forwardsgitUsertoconnectSandboxto setgit config user.name/user.email; push credentials viagetServiceGithubTokenare unchanged.Written for commit bd4e946. Summary will update on new commits.
Summary by CodeRabbit