Skip to content

promote: test → main (GET /api/sessions/[sessionId])#517

Merged
sweetmantech merged 7 commits intomainfrom
test
May 5, 2026
Merged

promote: test → main (GET /api/sessions/[sessionId])#517
sweetmantech merged 7 commits intomainfrom
test

Conversation

@sweetmantech
Copy link
Copy Markdown
Contributor

@sweetmantech sweetmantech commented May 5, 2026

Summary

Promotes the GET endpoint into main:

Test plan


Summary by cubic

Promotes the GET session-by-id endpoint to main, adding /api/sessions/[sessionId] with auth and ownership checks. Returns the same camelCase session shape as POST to support open-agents cutover without frontend changes.

  • New Features
    • Added route with OPTIONS preflight and cache controls (dynamic="force-dynamic", fetchCache="force-no-store", revalidate=0).
    • Implemented getSessionByIdHandler using validateAuthContext and selectSessions({ id }); returns 401/403/404 with { status: "error", error } and CORS headers; 200 returns { session }.
    • Added tests for 401, 404, 403, 200, and OPTIONS.

Written for commit b2dd017. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • New Features
    • Added API endpoint to retrieve session details with secure authentication validation and proper error handling for unauthorized or missing sessions.

sweetmantech and others added 7 commits May 4, 2026 08:54
)

* refactor(sandbox): callers use open-agents abstraction (Phase 2.2)

Replaces direct @vercel/sandbox SDK calls with the open-agents sandbox
abstraction layer (inlined in Phase 2.1) for sandbox lifecycle (create
+ reconnect). HTTP response shapes preserved exactly.

Per the agreed Option B (hybrid): only the lifecycle creator helpers
get refactored. installClaudeCode / runClaudeCode / getSandboxStatus
stay on the SDK directly because the abstraction does not cover their
needs (sudo, stdout/stderr streaming, simple status reads). Those
two install/run files are also dead orphans (defined but never called)
and will be removed entirely after the full migration.

Production refactor:
  createSandbox.ts            Sandbox.create(...) -> VercelSandbox.create(...)
                              Input: VercelSandboxConfig (was SDK params)
                              Snapshot trigger: restoreSnapshotId field
                                (was source: { type: "snapshot", ... })
                              Returns VercelSandbox (was SDK Sandbox)
  createSandboxWithFallback.ts cascade — passes restoreSnapshotId to createSandbox
  createSandboxFromSnapshot.ts type cascade only (Sandbox -> VercelSandbox)
  getActiveSandbox.ts         Sandbox.get({name}) -> VercelSandbox.connect(name, {})
                              Status check: sandbox.status -> sandbox.sdkStatus
  getOrCreateSandbox.ts       no code change — type cascades automatically
  processCreateSandbox.ts     reads sandbox.sdkStatus instead of sandbox.status
                              defensive nullish on createdAt

Abstraction extension:
  vercel/sandbox/VercelSandbox.ts adds two readonly getters following
  the existing host/environmentDetails/expiresAt pattern:
    get sdkStatus(): string  — raw SDK session status (running/pending/
                                stopped/failed/aborted/snapshotting),
                                distinct from the abstraction's normalized
                                status getter
    get createdAt(): Date | undefined  — SDK session.createdAt

  These give api callers what they need to construct the existing
  HTTP response shape without breaking the abstraction's interface.

Tests updated:
  createSandbox.test.ts            mocks VercelSandbox.create instead of
                                    Sandbox.create; mock object uses
                                    sdkStatus instead of status
  createSandboxWithFallback.test.ts asserts restoreSnapshotId pass-through
  getActiveSandbox.test.ts         mocks VercelSandbox.connect; sdkStatus
                                    on mock objects
  processCreateSandbox.test.ts     mockSandbox uses sdkStatus

Verification:
  - pnpm lint:check: clean
  - pnpm test: 2391/2391 pass
  - HTTP response shape unchanged: same fields, same enum values for
    sandboxStatus (sourced from the SDK now via sdkStatus, was directly
    via SDK Sandbox.status before — identical strings either way)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address PR #509 review feedback

Three real issues from CodeRabbit + cubic:

1. createdAt staleness (CodeRabbit minor)
   The new `createdAt` getter on VercelSandbox skipped the
   `refreshStateFromCurrentSession()` step that `sdkStatus` uses, so
   readers right after a reconnect could see stale session metadata.
   Add the refresh.

2. Fabricated createdAt (cubic P2)
   Both createSandbox.ts and processCreateSandbox.ts had a
   `?? new Date().toISOString()` fallback that fabricated creation
   metadata when sandbox.createdAt was missing. The SDK guarantees
   createdAt is populated for any reachable instance, so the fallback
   was both wrong (fabricates data) and unnecessary.

   Tighten the getter to return `Date` (not `Date | undefined`) and
   throw with an explicit "SDK contract violation" message if the
   field is missing — fail-fast surfaces a real contract bug instead
   of silently lying.

   Drop the `?? new Date()` fallbacks at both call sites.

3. Misleading snapshot-restore branching (CodeRabbit major)
   createSandbox.ts had two paths — a "snapshot" branch that omitted
   DEFAULT_VCPUS/DEFAULT_RUNTIME (intent: let snapshot dictate), and
   a "fresh" branch that applied defaults. But VercelSandbox.create
   internally defaults vcpus=4 and runtime="node22" regardless, so
   the omission was a no-op — the abstraction always forwarded those
   to the SDK.

   Drop the misleading branching. Document the actual behavior at
   the top of createSandbox: "VercelSandbox.create applies its own
   defaults regardless of source — those apply to the runtime
   resources of the new sandbox even when restoring from a snapshot."

   Updated the snapshot-restore test to assert the actual call shape
   (vcpus + runtime + timeout + restoreSnapshotId) instead of just
   the original SDK-style truncated args.

Verification:
- pnpm lint:check: clean
- pnpm test: 2391/2391 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(sandbox): delete dead Claude Code helpers (Phase 2.3)

installClaudeCode and runClaudeCode were defined but never imported
anywhere in api production code — confirmed by grep on main:

  $ grep -rn "installClaudeCode\b\|runClaudeCode\b" lib/ app/
  lib/sandbox/installClaudeCode.ts:9: export async function installClaudeCode(...)
  lib/sandbox/runClaudeCode.ts:10:    export async function runClaudeCode(...)

Both files were skipped during the Phase 2.2 abstraction refactor
(per the agreed Option B — they used SDK features the abstraction
doesn't expose: sudo, stdout/stderr streaming, batched writes). With
the broader migration moving to Vercel Workflow + open-agents' agent
package for sandbox bootstrap, these orphans have no path to being
called again.

Removed:
  lib/sandbox/installClaudeCode.ts                (32 lines)
  lib/sandbox/runClaudeCode.ts                    (29 lines)
  lib/sandbox/__tests__/installClaudeCode.test.ts (4 tests)
  lib/sandbox/__tests__/runClaudeCode.test.ts     (6 tests)

Verification:
  - pnpm lint:check: clean
  - pnpm test: 2381/2381 pass (was 2391 — net -10 tests from the
    two deleted test files)

Note: getOrCreateSandbox.ts also has zero importers per the audit
and is similarly dead, but is intentionally NOT deleted in this PR
since it was not explicitly flagged as orphan in the migration plan.
Worth a separate follow-up decision.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(sandbox): also delete getOrCreateSandbox + getActiveSandbox (YAGNI)

Cascade audit found two more truly-dead helpers per YAGNI:

  getOrCreateSandbox.ts    0 importers (self-only references)
  getActiveSandbox.ts      only called by getOrCreateSandbox — orphan
                            once that goes

Removed:
  lib/sandbox/getOrCreateSandbox.ts                (39 lines)
  lib/sandbox/getActiveSandbox.ts                  (33 lines)
  lib/sandbox/__tests__/getOrCreateSandbox.test.ts (3 tests)
  lib/sandbox/__tests__/getActiveSandbox.test.ts   (4 tests)

Live consumers of related helpers preserved:
  - createSandboxFromSnapshot still used by processCreateSandbox
  - selectAccountSandboxes still used by aggregateAccountSandboxStats,
    buildGetSandboxesParams, getSandboxesHandler, validateGetSandboxesRequest

Verification:
  - pnpm lint:check: clean
  - pnpm test: 2374/2374 pass (was 2381 — net -7 from the two deleted
    test files; -3 from getOrCreateSandbox.test.ts + -4 from
    getActiveSandbox.test.ts)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(sessions): port GET /api/sessions/[sessionId] from open-agents (Phase 2.4 — first route)

First route in the route-by-route cutover plan. Strategy: open-agents
frontend stays unchanged in shape; api ports each route it calls in
priority order (simplest first), and the open-agents frontend gets
cut over to api one route at a time.

Why this route first:
- Pure DB read (single-row select by id) — no agent runner, no Vercel
  Workflow, no sandbox runtime
- Hits sessions table already migrated in database PR #20
- Frontend usage: agents-frontend hits /api/sessions/{id} on session
  detail page navigation
- Smallest possible blast radius for proving the cutover pattern

Files added:
  lib/supabase/sessions/selectSession.ts  Single-row helper + SessionRow
                                          type (hand-typed; database.types.ts
                                          regen pending — flagged in code
                                          comment)
  app/api/sessions/[sessionId]/route.ts   GET handler matching open-agents
                                          response shape exactly (camelCase
                                          fields, "userId" preserved on the
                                          wire even though stored as
                                          account_id internally)
  app/api/sessions/[sessionId]/__tests__/route.test.ts (5 tests)

Auth: validateAuthContext (Privy Bearer or x-api-key). Response codes
match open-agents: 200 happy path, 401 no auth, 403 not owner, 404 not
found.

Wire-format translation: snake_case Supabase row -> camelCase response,
with account_id surfaced as userId so the existing open-agents frontend
fetches with zero code changes. Translation lives at the route boundary
(toSessionResponse) where it is easy to remove once chat absorbs this
UI and we can switch to schema-natural naming.

Verification:
- pnpm lint:check: clean
- pnpm test: 2379/2379 pass (5 new for this route)

Up next:
- Cutover step (separate PR in open-agents): point the frontend at
  api's URL for this single route. Validate end-to-end before porting
  the next route.
- Next routes in priority order (still pure DB, no agent/workflow):
  GET /api/sessions (list with unread — needs Postgres RPC for the
  multi-table aggregation), GET /api/sessions/[id]/chats, GET
  /api/sessions/[id]/chats/[chatId].

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address PR review — SRP splits + use Tables<\"sessions\"> from regen'd types

Three review comments on PR #514:

1. SRP: extract toSessionResponse to its own file
   was: defined inline in app/api/sessions/[sessionId]/route.ts
   now: lib/sessions/toSessionResponse.ts (one exported fn per file)

2. SRP: add a handler function (mirroring api convention)
   was: GET handler logic inline in route.ts
   now: lib/sessions/getSessionByIdHandler.ts contains all the auth +
        ownership + DB lookup + response logic; route.ts is a thin
        shell that awaits options.params and delegates. Matches the
        pattern used by every other api route (e.g. socials/[id]/scrape,
        artists/[id]/...).

3. DRY: use existing db schema type
   was: hand-typed SessionRow interface in selectSession.ts
   now: Tables<\"sessions\"> from types/database.types.ts (regenerated
        via npx supabase gen types typescript --project-id ...
        --schema public)

The types regen also resolved the preview-build failure
(\"Type instantiation is excessively deep and possibly infinite\") on
the .from(\"sessions\") call — Supabase's type inference was choking
because the table was unknown to the generic.

Files added:
  lib/sessions/toSessionResponse.ts
  lib/sessions/getSessionByIdHandler.ts

Files modified:
  app/api/sessions/[sessionId]/route.ts        thin shell now
  app/api/sessions/[sessionId]/__tests__/
    route.test.ts                              type alias updated
  lib/supabase/sessions/selectSession.ts       Tables<\"sessions\">
  types/database.types.ts                      Supabase regen

Verification:
  - pnpm lint:check: clean
  - pnpm test: 2379/2379 pass (no test changes; same 5 route tests)
  - tsc compile clean (the local pnpm build progresses past compile
    into page-data collection where it fails on missing local env
    vars — Vercel preview will have those set, so the preview rebuild
    should now succeed)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sessions): make 404/403 errors emit status:"error" for shape consistency

The 401 returned by validateAuthContext shaped like
{status:"error", error:"..."} but 404/403 from this handler returned
{error:"..."} only. Same endpoint, two error shapes — inconsistent for
clients. Align all error responses on the validateAuthContext shape.

Tests now assert the full error body, not just the status code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sessions): port POST /api/sessions from open-agents

Implements the POST /api/sessions contract documented in
recoupable/docs PR #186 + #187. Creates a session row and an
initial chat row; rolls back the session if chat insert fails so
callers never observe an orphaned session.

Auth: validateAuthContext (Privy Bearer or x-api-key).
Validation: Zod schema + GitHub repo segment regex. Body is
optional — empty body creates a session with sensible defaults
(status=running, lifecycle_state=provisioning, sandbox_state.type=
vercel, title="New session").

Out of scope (will follow once database catches up):
  auto_commit_push_override, auto_create_pr_override, pr_number,
  pr_status — these columns don't yet exist on api's sessions
  table, so the docs spec was trimmed accordingly in docs PR #187.

TDD: 9 handler tests cover 401, 400 (sandboxType / repoOwner /
repoName), 200 happy path, branch generation, title pass-through,
500 (insertSession failure), and 500-with-rollback (insertChat
failure). Plus 1 thin test on the route shell.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sessions): add OPTIONS handler + cache directives to POST route

Match the convention from app/api/sessions/[sessionId]/route.ts:
- OPTIONS handler returning 200 + CORS headers (preflight)
- dynamic="force-dynamic", fetchCache="force-no-store", revalidate=0

POST routes that mutate DB shouldn't be cached, and browsers issuing
preflight checks (POST with JSON body + custom auth headers) need
OPTIONS to respond.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sessions): address PR review feedback

- SRP: extract insert-row construction to lib/sessions/buildSessionInsertRow.ts
- YAGNI: drop generateSessionBranchName + isNewBranch handling (sessions
  commit to whatever branch the client provides; auto-generation was
  speculative)
- Tighten isValidGitHubRepoOwner: GitHub's actual rules are alphanumeric
  + hyphen only (no `_` or `.`), 1-39 chars, no leading/trailing or
  consecutive hyphens
- Tighten isValidGitHubRepoName: reject reserved `.` and `..`, reject
  `.git` suffix, cap at 100 chars
- Add unit tests for both validators (15 cases) and for the new
  buildSessionInsertRow (4 cases)
- Split createSessionHandler tests into auth/validation + persistence
  files; share fixtures via createSessionHandlerFixtures.ts. All test
  files now under 100 lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sessions): address second round of PR review

- 500 message: "Failed to create session" → "Internal server error"
  (per cubic.dev standardized 500 envelope feedback)
- SRP: extract failedToCreateSession to lib/sessions/failedToCreateSession.ts
- YAGNI: drop repoOwner from request body and remove
  isValidGitHubRepoOwner helper entirely (recoupable is the only
  owner; no need to validate)
- YAGNI: drop repoName from request body and remove
  isValidGitHubRepoName helper (repo identity is derived server-side
  from the authenticated account, not accepted from user input)
- Single-export per file: split createSessionHandlerFixtures.ts into
  makeCreateSessionReq.ts, baseSessionRow.ts, baseChatRow.ts.
  okAuth constant inlined where used.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sessions): port random-city title fallback from open-agents

Generated session titles now match the open-agents UX — names like
"Anchorage", "Vienna", "Philadelphia" — instead of every untitled
session being called "New session". Closes a wire-shape gap with
open-agents production identified by the head-to-head test on PR.

Pieces:
- lib/sessions/cityNames.ts: ~200-city curated list (verbatim port)
- lib/sessions/getRandomCityName.ts: pick a city not in `usedNames`,
  numeric-suffix fallback when the curated list is exhausted
- lib/supabase/sessions/selectSessionTitlesByAccountId.ts: Supabase
  helper for collision avoidance
- lib/sessions/resolveSessionTitle.ts: orchestrates provided title
  (trimmed) > random city fallback. Async. Kept separate from the
  insert-row builder so that stays synchronous + pure.
- buildSessionInsertRow now takes `title` as a parameter
- createSessionHandler awaits resolveSessionTitle before building the
  row

TDD: 4 tests for getRandomCityName, 4 for resolveSessionTitle. Handler
tests updated to mock resolveSessionTitle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: remove GET-only files (scope this PR to POST)

The GET endpoint + handler + tests live in PR #514 and were
inadvertently brought in when this branch was rebased after #514's
work. This PR is scoped to POST only; GET ships in #514.

Shared infrastructure stays (types/database.types.ts regen +
lib/sessions/toSessionResponse.ts) — both are required by the POST
handler too. When either #514 or this PR merges to test first, the
other will see those files already present and resolve cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sessions): consolidate request validation + DRY supabase select

Two reviewer asks rolled into one commit:

SRP — validateCreateSessionBody now owns the full validation flow.
The handler used to call safeParseJson, validateAuthContext, and the
Zod body schema separately; that was three places to short-circuit
and three places to duplicate the error envelope. Folded them into
validateCreateSessionBody so the handler does one call → success or
NextResponse error. Returns { body, auth } on success.

DRY — replaced lib/supabase/sessions/selectSession.ts and
selectSessionTitlesByAccountId.ts with a single
selectSessions({ id?, accountId? }) that supports both call sites.
resolveSessionTitle now derives titles from the general fetch.

Tests:
- New validateCreateSessionBody.test.ts covers auth-failure / 400 /
  success / malformed-JSON tolerance (4 cases)
- Handler tests now mock validateCreateSessionBody (single mock
  surface instead of three)
- resolveSessionTitle tests mock selectSessions

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sessions): address automated review feedback

Four small fixes from the latest round:

1. Zod v4 migration: { message } → { error } on the sandboxType
   literal. v4 unified the error customization API; { message } is
   deprecated.

2. Orphan rollback observability: when insertChat fails AND the
   session-rollback delete also fails, log the session id so ops
   can detect orphaned rows. New persistence test asserts the log.

3. Defensive try/catch in selectSessions so a thrown exception
   (network-level rejection, not a Supabase {error} return) doesn't
   bubble up and 500 the entire session-creation flow.

4. Deterministic test for getRandomCityName suffix-increment: pin
   Math.random instead of looping until the random pick lands on
   baseCity. Previous test could pass without ever asserting if the
   loop cap was hit.

Skipped: cubic-dev-ai's note about logging raw sessionId in
selectSession.ts — that file was deleted earlier in this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: prettier format fix on persistence test

The new orphan-session test had a line that exceeded prettier's wrap
width. Auto-format fixed it; format-check now clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…514)

Rebased onto current main (which now has the POST endpoint + shared
infra from PR #515). Three pieces of GET-specific work:

- app/api/sessions/[sessionId]/route.ts: thin shell delegating to the
  handler, plus OPTIONS for CORS preflight + cache directives
- lib/sessions/getSessionByIdHandler.ts: validates auth via
  validateAuthContext, reads via selectSessions({id}), enforces
  ownership (403 if account_id mismatch), 404 if missing
- app/api/sessions/[sessionId]/__tests__/route.test.ts: 5 cases —
  401 / 404 / 403 / 200 happy path / OPTIONS smoke

Uses the new general selectSessions({id}) reader rather than the
deleted single-purpose selectSession helper. All other shared infra
(types, toSessionResponse) is already on main from #515.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api Ready Ready Preview May 5, 2026 0:01am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

📝 Walkthrough

Walkthrough

This PR introduces a new API endpoint GET /api/sessions/[sessionId] with CORS support. The route handler extracts the session ID from URL parameters, delegates to a business logic handler that validates authentication, fetches the session from the database, enforces ownership checks, and returns the session or appropriate error responses.

Changes

Session Fetch Endpoint

Layer / File(s) Summary
Authentication & Authorization
lib/sessions/getSessionByIdHandler.ts
Validates request authentication context and enforces account ownership by comparing session account_id with authenticated accountId.
Data Access
lib/sessions/getSessionByIdHandler.ts
Queries session by sessionId using selectSessions() and handles missing records with appropriate error responses.
Response Serialization
lib/sessions/getSessionByIdHandler.ts
Shapes database rows into client response format via toSessionResponse() and includes CORS headers in all responses.
Route Wiring
app/api/sessions/[sessionId]/route.ts
Exports GET and OPTIONS handlers; extracts async sessionId param and delegates to handler; configures route as force-dynamic with no caching.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route Handler
    participant Auth Validator
    participant Database
    participant Response Formatter

    Client->>Route Handler: GET /api/sessions/{sessionId}
    Route Handler->>Auth Validator: validateAuthContext(request)
    alt Auth Invalid
        Auth Validator-->>Route Handler: Error Response
    else Auth Valid
        Route Handler->>Database: selectSessions({ id: sessionId })
        alt Session Not Found
            Database-->>Route Handler: Empty Result
            Route Handler-->>Client: 404 { error: "Session not found" }
        else Session Found
            alt Account Mismatch
                Route Handler-->>Client: 403 { error: "Forbidden" }
            else Account Match
                Database-->>Response Formatter: Session Row
                Response Formatter-->>Route Handler: Serialized Session
                Route Handler-->>Client: 200 { session: {...} }
            end
        end
    end
Loading

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

The changes follow a straightforward, familiar authentication-and-database-lookup pattern with clear error handling. Two cohesive files, ~93 lines of code, standard Next.js 13+ App Router conventions, and no complex logic branches. Reviewers should verify authentication enforcement, database query correctness, and CORS header consistency.

Possibly Related PRs

  • recoupable/api#514: Introduces the same GET /api/sessions/[sessionId] endpoint with identical handler logic, authentication checks, and response serialization.

Poem

Sessions unlocked with a single ID, 🔑
Auth guards the gate with watchful eye,
Database whispers secrets true,
CORS headers sail the web anew! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Missing Zod parameter validation for sessionId and unhandled data-layer errors. Violates Clean Code guidelines for API validation and error handling. Add Zod sessionIdParamsSchema validation in route. Add try-catch around selectSessions() to return 500 with consistent error envelope on data failures.
✅ Passed checks (2 passed)
Check name Status Explanation
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test

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.

Copy link
Copy Markdown

@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: 2

🧹 Nitpick comments (1)
lib/sessions/getSessionByIdHandler.ts (1)

21-51: ⚡ Quick win

Split the handler into small helpers to meet function-size constraints.

This function exceeds the repository’s max-length guideline; extracting response builders/guards will keep it easier to evolve.

As per coding guidelines, “Flag functions longer than 20 lines” and “Keep functions small and focused.”

🤖 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/sessions/getSessionByIdHandler.ts` around lines 21 - 51, Split
getSessionByIdHandler into small helpers: create getSessionOr404(sessionId) that
calls selectSessions and returns the row or returns the 404 NextResponse (use
getCorsHeaders), create assertSessionOwnership(row, auth) that returns a 403
NextResponse when row.account_id !== auth.accountId or null on success, and
create okSessionResponse(row) that returns the 200 NextResponse with
toSessionResponse(row) and CORS headers; then refactor getSessionByIdHandler to
call validateAuthContext(request), then getSessionOr404(sessionId), then
assertSessionOwnership(row, auth), and finally return okSessionResponse(row),
keeping all response construction in the new helpers and leaving
validateAuthContext, selectSessions, toSessionResponse, and getCorsHeaders usage
intact.
🤖 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 `@app/api/sessions/`[sessionId]/route.ts:
- Around line 32-37: Add a Zod-based parameter validator for sessionId and call
it from the GET route before invoking getSessionByIdHandler: create a function
(e.g., validateSessionParams) modeled after validateAccountParams /
validateAddArtistBody that accepts the raw params object, defines a Zod schema
for { sessionId: string } (with any required format checks), returns the parsed
value on success or a NextResponse with a 400 error on failure, then update the
GET handler to await validateSessionParams(options.params) and only call
getSessionByIdHandler(request, sessionId) when validation passes.

In `@lib/sessions/getSessionByIdHandler.ts`:
- Around line 30-50: selectSessions can throw and currently bubbles out,
breaking the endpoint's JSON+CORS contract; wrap the call to selectSessions({
id: sessionId }) in a try/catch inside getSessionByIdHandler (or the handler
function in this file), and on any caught error return a NextResponse.json({
status: "error", error: "Internal server error" }, { status: 500, headers:
getCorsHeaders() }); optionally log the caught error before returning; keep the
rest of the control flow (not-found, forbidden, success using
toSessionResponse(row)) unchanged.

---

Nitpick comments:
In `@lib/sessions/getSessionByIdHandler.ts`:
- Around line 21-51: Split getSessionByIdHandler into small helpers: create
getSessionOr404(sessionId) that calls selectSessions and returns the row or
returns the 404 NextResponse (use getCorsHeaders), create
assertSessionOwnership(row, auth) that returns a 403 NextResponse when
row.account_id !== auth.accountId or null on success, and create
okSessionResponse(row) that returns the 200 NextResponse with
toSessionResponse(row) and CORS headers; then refactor getSessionByIdHandler to
call validateAuthContext(request), then getSessionOr404(sessionId), then
assertSessionOwnership(row, auth), and finally return okSessionResponse(row),
keeping all response construction in the new helpers and leaving
validateAuthContext, selectSessions, toSessionResponse, and getCorsHeaders usage
intact.
🪄 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: d19d18ad-6cda-47d9-b4cd-efc848be6c0d

📥 Commits

Reviewing files that changed from the base of the PR and between 6368313 and b2dd017.

⛔ Files ignored due to path filters (1)
  • app/api/sessions/[sessionId]/__tests__/route.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by app/**
📒 Files selected for processing (2)
  • app/api/sessions/[sessionId]/route.ts
  • lib/sessions/getSessionByIdHandler.ts

Comment on lines +32 to +37
export async function GET(
request: NextRequest,
options: { params: Promise<{ sessionId: string }> },
) {
const { sessionId } = await options.params;
return getSessionByIdHandler(request, sessionId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

find app/api -name "route.ts" -type f | head -5

Repository: recoupable/api

Length of output: 218


🏁 Script executed:

cat -n app/api/sessions/\[sessionId\]/route.ts

Repository: recoupable/api

Length of output: 1740


🏁 Script executed:

# Check if getSessionByIdHandler exists and review its signature
find . -name "getSessionByIdHandler*" -type f

Repository: recoupable/api

Length of output: 98


🏁 Script executed:

# Check other API routes to see validation patterns
grep -r "safeParse\|z\.object" app/api --include="*.ts" -l | head -10

Repository: recoupable/api

Length of output: 118


🏁 Script executed:

cat -n ./lib/sessions/getSessionByIdHandler.ts

Repository: recoupable/api

Length of output: 2105


🏁 Script executed:

cat -n app/api/accounts/artists/route.ts

Repository: recoupable/api

Length of output: 1634


🏁 Script executed:

cat -n app/api/accounts/route.ts

Repository: recoupable/api

Length of output: 2536


🏁 Script executed:

# Check the validation function pattern
cat -n lib/accounts/validateAddArtistBody.ts

Repository: recoupable/api

Length of output: 1376


🏁 Script executed:

# Check another validation function
cat -n lib/accounts/validateCreateAccountBody.ts

Repository: recoupable/api

Length of output: 1407


🏁 Script executed:

# Search for other route parameters that might need validation
grep -r "options.params" app/api --include="*.ts" -B 2 -A 2

Repository: recoupable/api

Length of output: 5345


🏁 Script executed:

# Check if there are any validation functions for path parameters
find lib -name "validate*" -type f | xargs grep -l "params\|Params" | head -5

Repository: recoupable/api

Length of output: 290


🏁 Script executed:

# Check the accounts/[id]/catalogs route to see how it handles params
cat -n app/api/accounts/\[id\]/catalogs/route.ts

Repository: recoupable/api

Length of output: 1149


🏁 Script executed:

# Check another parameterized route with handler
cat -n app/api/artists/\[id\]/route.ts | head -50

Repository: recoupable/api

Length of output: 1938


🏁 Script executed:

# Check the validateGetAccountParams and validateAccountParams patterns
cat -n lib/accounts/validateGetAccountParams.ts

Repository: recoupable/api

Length of output: 1725


🏁 Script executed:

cat -n lib/accounts/validateAccountParams.ts

Repository: recoupable/api

Length of output: 1324


🏁 Script executed:

# Search for how other routes with IDs validate their params
grep -r "validateGetAccountParams\|validateAccountParams" app/api --include="*.ts" -B 2 -A 2

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Search for usage of these validation functions in routes
grep -r "validateGetAccountParams\|validateAccountParams" . --include="*.ts" | grep -v node_modules

Repository: recoupable/api

Length of output: 4885


🏁 Script executed:

# Look for session parameter validation patterns
grep -r "sessionId" lib --include="*.ts" -l | head -5

Repository: recoupable/api

Length of output: 280


🏁 Script executed:

# Check if there's a pattern for validating route params across the API
grep -r "safeParse.*params\|validateGet.*Params" app/api --include="*.ts" -B 3 -A 3

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Check if there are tests for the sessions route
find . -name "*sessions*" -type f | grep -i test

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Look at the getAccountHandler to see the pattern
cat -n lib/accounts/getAccountHandler.ts | head -50

Repository: recoupable/api

Length of output: 1906


Add parameter validation for sessionId using Zod before calling the handler.

The route currently forwards the sessionId parameter without validation. Create a validation function following the established pattern in the codebase (similar to validateAccountParams or validateAddArtistBody) that validates the parameter format and returns either the validated value or a NextResponse error.

Proposed validation pattern
 import { NextRequest, NextResponse } from "next/server";
 import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
 import { getSessionByIdHandler } from "@/lib/sessions/getSessionByIdHandler";
+import { z } from "zod";
+
+const sessionIdParamsSchema = z.object({
+  sessionId: z.string().min(1, "sessionId cannot be empty"),
+});

 export async function GET(
   request: NextRequest,
   options: { params: Promise<{ sessionId: string }> },
 ) {
-  const { sessionId } = await options.params;
-  return getSessionByIdHandler(request, sessionId);
+  const parsed = sessionIdParamsSchema.safeParse(await options.params);
+  if (!parsed.success) {
+    const firstError = parsed.error.issues[0];
+    return NextResponse.json(
+      {
+        status: "error",
+        missing_fields: firstError.path,
+        error: firstError.message,
+      },
+      { status: 400, headers: getCorsHeaders() },
+    );
+  }
+  return getSessionByIdHandler(request, parsed.data.sessionId);
+}

This follows the coding guideline: "All API endpoints should use a validate function for input parsing using Zod for schema validation."

📝 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
export async function GET(
request: NextRequest,
options: { params: Promise<{ sessionId: string }> },
) {
const { sessionId } = await options.params;
return getSessionByIdHandler(request, sessionId);
import { NextRequest, NextResponse } from "next/server";
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
import { getSessionByIdHandler } from "@/lib/sessions/getSessionByIdHandler";
import { z } from "zod";
const sessionIdParamsSchema = z.object({
sessionId: z.string().min(1, "sessionId cannot be empty"),
});
export async function GET(
request: NextRequest,
options: { params: Promise<{ sessionId: string }> },
) {
const parsed = sessionIdParamsSchema.safeParse(await options.params);
if (!parsed.success) {
const firstError = parsed.error.issues[0];
return NextResponse.json(
{
status: "error",
missing_fields: firstError.path,
error: firstError.message,
},
{ status: 400, headers: getCorsHeaders() },
);
}
return getSessionByIdHandler(request, parsed.data.sessionId);
}
🤖 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 `@app/api/sessions/`[sessionId]/route.ts around lines 32 - 37, Add a Zod-based
parameter validator for sessionId and call it from the GET route before invoking
getSessionByIdHandler: create a function (e.g., validateSessionParams) modeled
after validateAccountParams / validateAddArtistBody that accepts the raw params
object, defines a Zod schema for { sessionId: string } (with any required format
checks), returns the parsed value on success or a NextResponse with a 400 error
on failure, then update the GET handler to await
validateSessionParams(options.params) and only call
getSessionByIdHandler(request, sessionId) when validation passes.

Comment on lines +30 to +50
const rows = await selectSessions({ id: sessionId });
const row = rows[0] ?? null;

if (!row) {
return NextResponse.json(
{ status: "error", error: "Session not found" },
{ status: 404, headers: getCorsHeaders() },
);
}

if (row.account_id !== auth.accountId) {
return NextResponse.json(
{ status: "error", error: "Forbidden" },
{ status: 403, headers: getCorsHeaders() },
);
}

return NextResponse.json(
{ session: toSessionResponse(row) },
{ status: 200, headers: getCorsHeaders() },
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return a consistent 500 JSON+CORS response on data-layer failures.

selectSessions() errors currently bubble out and can break your consistent envelope/CORS contract for this endpoint.

Proposed fix
 export async function getSessionByIdHandler(
   request: NextRequest,
   sessionId: string,
 ): Promise<NextResponse> {
   const auth = await validateAuthContext(request);
   if (auth instanceof NextResponse) {
     return auth;
   }

-  const rows = await selectSessions({ id: sessionId });
+  let rows;
+  try {
+    rows = await selectSessions({ id: sessionId });
+  } catch {
+    return NextResponse.json(
+      { status: "error", error: "Internal server error" },
+      { status: 500, headers: getCorsHeaders() },
+    );
+  }
   const row = rows[0] ?? null;

As per coding guidelines, “Handle errors gracefully” and for API logic “Proper error handling and validation.”

📝 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 rows = await selectSessions({ id: sessionId });
const row = rows[0] ?? null;
if (!row) {
return NextResponse.json(
{ status: "error", error: "Session not found" },
{ status: 404, headers: getCorsHeaders() },
);
}
if (row.account_id !== auth.accountId) {
return NextResponse.json(
{ status: "error", error: "Forbidden" },
{ status: 403, headers: getCorsHeaders() },
);
}
return NextResponse.json(
{ session: toSessionResponse(row) },
{ status: 200, headers: getCorsHeaders() },
);
export async function getSessionByIdHandler(
request: NextRequest,
sessionId: string,
): Promise<NextResponse> {
const auth = await validateAuthContext(request);
if (auth instanceof NextResponse) {
return auth;
}
let rows;
try {
rows = await selectSessions({ id: sessionId });
} catch {
return NextResponse.json(
{ status: "error", error: "Internal server error" },
{ status: 500, headers: getCorsHeaders() },
);
}
const row = rows[0] ?? null;
if (!row) {
return NextResponse.json(
{ status: "error", error: "Session not found" },
{ status: 404, headers: getCorsHeaders() },
);
}
if (row.account_id !== auth.accountId) {
return NextResponse.json(
{ status: "error", error: "Forbidden" },
{ status: 403, headers: getCorsHeaders() },
);
}
return NextResponse.json(
{ session: toSessionResponse(row) },
{ status: 200, headers: getCorsHeaders() },
);
}
🤖 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/sessions/getSessionByIdHandler.ts` around lines 30 - 50, selectSessions
can throw and currently bubbles out, breaking the endpoint's JSON+CORS contract;
wrap the call to selectSessions({ id: sessionId }) in a try/catch inside
getSessionByIdHandler (or the handler function in this file), and on any caught
error return a NextResponse.json({ status: "error", error: "Internal server
error" }, { status: 500, headers: getCorsHeaders() }); optionally log the caught
error before returning; keep the rest of the control flow (not-found, forbidden,
success using toSessionResponse(row)) unchanged.

@sweetmantech sweetmantech merged commit f1c2dec into main May 5, 2026
7 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