feat: add github_repo to PATCH /api/sandboxes#208
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR refactors the snapshot patch endpoint to support optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as updateSnapshotPatchHandler
participant Validator as validateSnapshotPatchBody
participant Selector as selectAccountSnapshots
participant Upsert as upsertAccountSnapshot
participant Database
Client->>Handler: PATCH with optional snapshotId/githubRepo
Handler->>Validator: Validate request body
Validator->>Validator: Parse & validate schema<br/>(snapshotId?, githubRepo?)
Validator-->>Handler: Return validated params
alt snapshotId or githubRepo provided
Handler->>Upsert: Call with conditional payload<br/>(account_id + optional fields)
Upsert->>Database: upsert(params, onConflict: "account_id")
Database-->>Upsert: Updated row
Upsert-->>Handler: Result
else Neither provided
Handler->>Selector: selectAccountSnapshots(account_id)
Selector->>Database: Query current snapshot
Database-->>Selector: Current snapshot row
Selector-->>Handler: First result
end
Handler-->>Client: Return snapshot data (200 + CORS)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/sandbox/validateSnapshotPatchBody.ts`:
- Around line 8-12: The github_repo field in snapshotPatchBodySchema currently
uses z.string().url() which rejects SSH shorthand remotes like
"git@github.com:org/repo.git"; update snapshotPatchBodySchema to accept both
URL-style remotes and the SSH shorthand by replacing github_repo:
z.string().url(...).optional() with a union or custom validator (e.g.,
z.union([z.string().url("..."),
z.string().regex(/^[\w.-]+@[\w.-]+:[\w./-]+\.git$/, "github_repo must be a valid
URL or SSH shorthand")]) .optional()) or use z.string().refine(...) to validate
either format, keeping the field optional and preserving error messages.
- Around line 14-21: Replace the manual export type SnapshotPatchBody with a Zod
schema that transforms incoming snake_case body keys to camelCase and lets
z.infer produce the exported type; create a schema (e.g., snapshotPatchSchema)
that accepts { snapshot_id: string, github_repo?: string } (do not accept
account_id in the request body) and use .transform(obj => ({ snapshotId:
obj.snapshot_id, githubRepo: obj.github_repo })) so that export type
SnapshotPatchBody = z.infer<typeof snapshotPatchSchema>; ensure the
validation/parse call uses snapshotPatchSchema.parse/parseAsync and derive
accountId from auth context elsewhere.
…ams type Removes duplicate UpsertAccountSnapshotParams interface in favor of the Supabase-generated TablesInsert type. Callers now pass snake_case fields directly, eliminating the camelCase-to-snake_case mapping layer. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
As a PATCH endpoint, all fields should be optional so callers can update github_repo without re-sending snapshotId, or vice versa. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
expires_at represents the snapshot expiration, so it should only be set/refreshed when a snapshot_id is being updated. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Returns 200 with { success: true } immediately when neither snapshotId
nor github_repo is provided, avoiding an unnecessary Supabase call that
would fail due to required columns.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
UPSERT requires all NOT NULL columns (snapshot_id, expires_at), so github_repo-only requests failed with a 400. Now the handler uses updateAccountSnapshot (plain UPDATE) when only github_repo is provided, and upsertAccountSnapshot when snapshot_id is included. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Returns the complete account_snapshots record (account_id, snapshot_id,
expires_at, github_repo, created_at) instead of just { success, snapshotId }.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/sandbox/updateSnapshotPatchHandler.ts`:
- Line 51: The handler returns inconsistent response shapes: the early return in
updateSnapshotPatchHandler (returns { success: true } at the early-exit branch)
differs from the main path which currently returns result.data (full
account_snapshots row); pick one consistent shape (prefer returning the full
resource per REST guidance) and update the handler so both branches return the
same structure (e.g., { success: true, snapshot: result.data } or simply
result.data consistently), adjust the success/snapshotId fields accordingly, and
ensure the function(s) handling the DB upsert/patch (references: result,
result.data, and the early-exit block that returns { success: true }) produce
and return that unified response format across all code paths.
🧹 Nitpick comments (2)
lib/sandbox/updateSnapshotPatchHandler.ts (2)
25-30: Early exit pattern is clean, though the response payload could be more informative.The guard clause properly handles the no-op case. Returning
{ success: true }when no fields were provided is semantically accurate but could be slightly misleading to API consumers. Consider returning a more descriptive message or the current state of the record.That said, this is a reasonable design choice for idempotency. The Single Responsibility principle is well-served here—the handler validates, determines the appropriate action, and responds accordingly.
37-37: Extract magic number to a named constant for clarity.The expression
365 * 24 * 60 * 60 * 1000represents one year in milliseconds but requires mental parsing. Extracting this to a well-named constant improves readability and maintainability (KISS principle).♻️ Suggested refactor
+const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000; + export async function updateSnapshotPatchHandler(request: NextRequest): Promise<NextResponse> {Then use it at line 37:
- expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + expires_at: new Date(Date.now() + ONE_YEAR_MS).toISOString(),
When neither snapshotId nor github_repo is provided, fetches and returns
the existing account_snapshots row instead of { success: true }, so the
response shape is consistent regardless of input.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Now that snapshot_id is nullable in Supabase, the upsert function works for all cases. Removes the separate updateAccountSnapshot function. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
github_repo(URL string) to the PATCH /api/sandboxes Zod schema andSnapshotPatchBodytypeupsertAccountSnapshotto conditionally includegithub_repoin the upsert dataupdateSnapshotPatchHandlerto passgithubRepothrough to the upsert functionTest plan
validateSnapshotPatchBodytest: github_repo is passed through in validated bodyupdateSnapshotPatchHandlertest: githubRepo is forwarded to upsertAccountSnapshot🤖 Generated with Claude Code
Summary by CodeRabbit