feat(api): add PATCH handler for updating session details#546
Conversation
- Implemented a new PATCH endpoint for updating a session's title or status, allowing optional fields to be modified. - Authenticates requests using a Privy Bearer token or x-api-key header. - Added comprehensive tests for the PATCH handler, covering scenarios such as unauthorized access, session not found, and successful updates. This enhancement improves the API's functionality by enabling users to modify session details dynamically.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughselectSessions now returns null on DB errors; callers were updated to handle null via ChangesSession API nullability and PATCH support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![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 some merge risk because
lib/sessions/patchSessionByIdHandler.tscurrently turns data-access failures into 404s whenselectSessionsreturns[], which can misreport transient backend/DB failures that should be 500s. - A smaller consistency issue is also present in
lib/sessions/patchSessionByIdHandler.ts: standardizing the 500 response body to "Internal server error" helps avoid leaking internal failure details and keeps error handling uniform. - Pay close attention to
lib/sessions/patchSessionByIdHandler.ts- separate missing-row handling from DB-error handling, and align 500 messages with the team standard.
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/sessions/patchSessionByIdHandler.ts">
<violation number="1" location="lib/sessions/patchSessionByIdHandler.ts:32">
P2: This handler maps data-access failures to 404 because `selectSessions` returns `[]` on error. Distinguish DB errors from missing rows so transient backend failures return 500, not “Session not found.”</violation>
<violation number="2" location="lib/sessions/patchSessionByIdHandler.ts:56">
P3: Use the standard 500 error message `"Internal server error"` for consistency and to avoid revealing internal failure details.
(Based on your team's feedback about standardized 500-response messages that avoid leaking internals.) [FEEDBACK_USED]</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as External Client
participant Route as Route Handler
participant Validator as validatePatchSessionBody
participant Auth as validateAuthContext
participant Parser as safeParseJson
participant DB as Supabase DB
participant Handler as patchSessionByIdHandler
participant Select as selectSessions
participant Update as updateSession
participant Response as toSessionResponse
Client->>Route: PATCH /api/sessions/{sessionId}
Note over Client,Route: Bearer token or x-api-key header
Route->>Validator: validatePatchSessionBody(request)
alt Auth failure
Validator->>Auth: validateAuthContext(request)
Auth-->>Validator: NextResponse (401)
Validator-->>Route: NextResponse (401)
Route-->>Client: 401 Unauthorized
else Auth success
Validator->>Auth: validateAuthContext(request)
Auth-->>Validator: AuthContext { accountId, orgId, authToken }
Validator->>Parser: safeParseJson(request)
Parser-->>Validator: parsed JSON body
Validator->>Validator: Zod validation against schema
alt Invalid body
Validator-->>Route: NextResponse (400)
Route-->>Client: 400 Bad Request
else Valid body
Validator-->>Route: ValidatedPatchSessionRequest { body, auth }
end
end
Route->>Handler: patchSessionByIdHandler(request, sessionId)
Handler->>Select: selectSessions({ id: sessionId })
Select-->>Handler: session row or null
alt Session not found
Handler-->>Route: NextResponse (404)
Route-->>Client: 404 Session not found
else Session found
alt account_id mismatch
Handler-->>Route: NextResponse (403)
Route-->>Client: 403 Forbidden
else Ownership verified
Handler->>Update: updateSession(sessionId, { title?, status? })
Update-->>Handler: updated session row
alt Update failed
Handler-->>Route: NextResponse (500)
Route-->>Client: 500 Failed to update session
else Update success
Handler->>Response: toSessionResponse(updated)
Response-->>Handler: formatted session
Handler-->>Route: NextResponse (200)
Route-->>Client: 200 { session }
end
end
end
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
- Implemented tests for the GET and PATCH session API endpoints to handle scenarios where the database returns an error, ensuring a 500 status response with appropriate error messages. - Updated the `selectSessions` function to return `null` on database errors, allowing for better error handling in the API responses. - Enhanced existing tests to verify that the API correctly responds to internal server errors, improving overall robustness and reliability of the session management functionality.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/sandbox/validateSandboxReconnectRequest.ts (1)
42-50:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDB error masked as 404 in authentication path.
The
?? []pattern on line 42 silently treats a DB failure (selectSessionsreturningnull) as "no rows found", causing the function to return404 "Session not found"when the actual issue is a transient backend error that should return500.This misrepresents server-side failures as client errors, hiding infrastructure problems and misleading API consumers.
🔧 Proposed fix: explicit null check
Match the pattern in
getSessionByIdHandler.ts:- const rows = (await selectSessions({ id: sessionId })) ?? []; + const rows = await selectSessions({ id: sessionId }); + + if (rows === null) { + return NextResponse.json( + { status: "error", error: "Internal server error" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + const row = rows[0];🤖 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/validateSandboxReconnectRequest.ts` around lines 42 - 50, The code currently masks a DB failure by using "const rows = (await selectSessions({ id: sessionId })) ?? []", treating null as an empty result; change this to explicitly detect a null result from selectSessions: call selectSessions({ id: sessionId }), if the returned value is null then respond with a 500 NextResponse.json error (include a clear server error message and getCorsHeaders()), otherwise proceed to check rows.length and return 404 "Session not found" only when rows is an empty array; refer to selectSessions, rows/row, and NextResponse.json to locate and update the logic similar to getSessionByIdHandler.ts.
🧹 Nitpick comments (4)
lib/sessions/patchSessionByIdHandler.ts (1)
57-60: 💤 Low valueConsider a more explicit pattern for conditional field updates.
The current spread pattern
...(condition && object)works but relies on implicit JavaScript truthiness. A ternary with explicit empty objects would be more readable:♻️ Optional refactor for clarity
const updated = await updateSession(sessionId, { - ...(body.title !== undefined && { title: body.title }), - ...(body.status !== undefined && { status: body.status }), + ...(body.title !== undefined ? { title: body.title } : {}), + ...(body.status !== undefined ? { status: body.status } : {}), });Alternatively, for maximum clarity:
const updates: Record<string, string> = {}; if (body.title !== undefined) updates.title = body.title; if (body.status !== undefined) updates.status = body.status; const updated = await updateSession(sessionId, updates);🤖 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/patchSessionByIdHandler.ts` around lines 57 - 60, The spread-with-&& pattern in the update payload for updateSession is unclear; instead build an explicit updates object (e.g., create a mutable updates: Record<string, any> and conditionally set updates.title = body.title and updates.status = body.status only when body.title/status are !== undefined) or use ternary expressions that return {} when absent, then call updateSession(sessionId, updates); updateSession, the updated variable and body.title/body.status are the relevant symbols to change in patchSessionByIdHandler.ts.lib/sandbox/createSandboxHandler.ts (1)
36-183: ⚡ Quick winConsider breaking down this handler into smaller focused functions.
The function spans 147 lines, significantly exceeding the 50-line guideline for
lib/**/*.ts. While the orchestration logic is cohesive, extracting helper functions for distinct phases (session validation, org snapshot resolution, sandbox provisioning, post-provision updates) would improve readability and testability. As per coding guidelines, "Keep functions under 50 lines" for domain functions.🤖 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 36 - 183, The createSandboxHandler function is too large; split it into smaller focused helpers to meet the <50-line guideline by extracting the distinct phases: (1) request/session validation (use validateCreateSandboxBody and selectSessions) into a function like validateAndLoadSession, (2) org snapshot resolution and background kick (use extractOrgRepoName, findOrgSnapshot, kickBuildOrgSnapshotWorkflow) into resolveOrgSnapshot, (3) sandbox provisioning (use resolveGitUser and connectSandbox) into provisionSandbox, and (4) post-provision actions (updateSession, installSessionGlobalSkills, kickSandboxLifecycleWorkflow) into finalizeSandbox; each helper should accept the minimal inputs (validated body, auth, sessionRow, orgSnapshotId) and return structured results so createSandboxHandler becomes a short orchestrator that calls these helpers and returns the response.lib/sandbox/evaluateSandboxLifecycle.ts (1)
34-106: ⚖️ Poor tradeoffConsider extracting state-transition logic into focused helpers.
The function spans 72 lines, exceeding the 50-line guideline for
lib/**/*.ts. The FSM evaluation contains several distinct phases (validation checks lines 41-56, active-stream checks lines 58-60, hibernation execution lines 62-92) that could be extracted. As per coding guidelines, domain functions should be kept under 50 lines for maintainability.🤖 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/evaluateSandboxLifecycle.ts` around lines 34 - 106, The evaluateSandboxLifecycle function is too long; extract its distinct phases into small helpers: move the initial validation checks (session lookup, archived checks, hasRuntimeSandboxState and sandbox type, and getLifecycleDueAtMs check) into a new helper like validateSessionAndState(sessionId) that returns the session or a skip result; move the repeated active-stream logic into checkActiveWorkflow(sessionId, sandboxState) which wraps hasActiveStreamForSession and restoreActiveLifecycleState calls; and move the hibernation sequence (updateSession to hibernating, connectSandbox, stop, clearSandboxState, and final updateSession + logging) into performHibernation(sessionId, session, sandboxState, reason) which returns the final action/result. Replace the inlined blocks in evaluateSandboxLifecycle with calls to these helpers and keep error handling in evaluateSandboxLifecycle (rethrow or translate helper errors into the existing catch logic).lib/sandbox/getSandboxStatusHandler.ts (1)
27-105: ⚡ Quick winConsider extracting helper functions to reduce handler complexity.
At 78 lines, this handler exceeds the 50-line guideline for
lib/**/*.ts. The self-healing logic (lines 64-72) and overdue lifecycle kick (lines 74-84) could be extracted into focused helper functions. As per coding guidelines, domain functions should be kept under 50 lines.🤖 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/getSandboxStatusHandler.ts` around lines 27 - 105, The getSandboxStatusHandler is over the 50-line limit; extract the self-healing and overdue-kick blocks into small helper functions to reduce handler complexity: move the "recover failed-but-still-active" logic (currently using isSandboxActive(row), updateSession(row.id, {...}), and getSandboxExpiresAtDate(row.sandbox_state)) into a helper like recoverActiveFailedSession(sessionRow) that returns the effective row, and move the "kick lifecycle if overdue" logic (checking effectiveRow.lifecycle_state, Date.now() >= getLifecycleDueAtMs(effectiveRow), and calling kickSandboxLifecycleWorkflow({...})) into a helper like scheduleLifecycleIfOverdue(effectiveRow) that returns void; then call these two helpers from getSandboxStatusHandler so the handler stays under 50 lines while reusing functions updateSession, kickSandboxLifecycleWorkflow, isSandboxActive, and getLifecycleDueAtMs.
🤖 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/workflows/computeLifecycleWakeDecision.ts`:
- Around line 29-31: The current use of "const rows = (await selectSessions({
id: sessionId })) ?? []" masks DB errors as a missing session; change this so
you do not coalesce null/undefined to an empty array—await selectSessions(...)
into a variable (rows) and explicitly handle three cases: if rows === null or
rows === undefined return/raise a distinct failure (e.g., { shouldContinue:
false, reason: "db-error" }) and log the error, if Array.isArray(rows) &&
rows.length === 0 return { shouldContinue: false, reason: "session-not-found" },
otherwise take rows[0] as session; reference selectSessions, sessionId, rows,
and session when making the checks and add a defensive log call (use existing
logger or console.error) when rows is null/undefined.
In `@lib/sessions/validatePatchSessionBody.ts`:
- Around line 42-55: The error response currently returns a misleading key named
"missing_fields" when a Zod validation failure occurs; update the JSON payload
returned in the error branch (where result.success is false, using
result.error.issues[0] / firstError) to use a generic key such as "field" or
"path" (e.g., "field": firstError.path) instead of "missing_fields", and keep
the existing "error": firstError.message and status/headers through
NextResponse.json with getCorsHeaders().
---
Outside diff comments:
In `@lib/sandbox/validateSandboxReconnectRequest.ts`:
- Around line 42-50: The code currently masks a DB failure by using "const rows
= (await selectSessions({ id: sessionId })) ?? []", treating null as an empty
result; change this to explicitly detect a null result from selectSessions: call
selectSessions({ id: sessionId }), if the returned value is null then respond
with a 500 NextResponse.json error (include a clear server error message and
getCorsHeaders()), otherwise proceed to check rows.length and return 404
"Session not found" only when rows is an empty array; refer to selectSessions,
rows/row, and NextResponse.json to locate and update the logic similar to
getSessionByIdHandler.ts.
---
Nitpick comments:
In `@lib/sandbox/createSandboxHandler.ts`:
- Around line 36-183: The createSandboxHandler function is too large; split it
into smaller focused helpers to meet the <50-line guideline by extracting the
distinct phases: (1) request/session validation (use validateCreateSandboxBody
and selectSessions) into a function like validateAndLoadSession, (2) org
snapshot resolution and background kick (use extractOrgRepoName,
findOrgSnapshot, kickBuildOrgSnapshotWorkflow) into resolveOrgSnapshot, (3)
sandbox provisioning (use resolveGitUser and connectSandbox) into
provisionSandbox, and (4) post-provision actions (updateSession,
installSessionGlobalSkills, kickSandboxLifecycleWorkflow) into finalizeSandbox;
each helper should accept the minimal inputs (validated body, auth, sessionRow,
orgSnapshotId) and return structured results so createSandboxHandler becomes a
short orchestrator that calls these helpers and returns the response.
In `@lib/sandbox/evaluateSandboxLifecycle.ts`:
- Around line 34-106: The evaluateSandboxLifecycle function is too long; extract
its distinct phases into small helpers: move the initial validation checks
(session lookup, archived checks, hasRuntimeSandboxState and sandbox type, and
getLifecycleDueAtMs check) into a new helper like
validateSessionAndState(sessionId) that returns the session or a skip result;
move the repeated active-stream logic into checkActiveWorkflow(sessionId,
sandboxState) which wraps hasActiveStreamForSession and
restoreActiveLifecycleState calls; and move the hibernation sequence
(updateSession to hibernating, connectSandbox, stop, clearSandboxState, and
final updateSession + logging) into performHibernation(sessionId, session,
sandboxState, reason) which returns the final action/result. Replace the inlined
blocks in evaluateSandboxLifecycle with calls to these helpers and keep error
handling in evaluateSandboxLifecycle (rethrow or translate helper errors into
the existing catch logic).
In `@lib/sandbox/getSandboxStatusHandler.ts`:
- Around line 27-105: The getSandboxStatusHandler is over the 50-line limit;
extract the self-healing and overdue-kick blocks into small helper functions to
reduce handler complexity: move the "recover failed-but-still-active" logic
(currently using isSandboxActive(row), updateSession(row.id, {...}), and
getSandboxExpiresAtDate(row.sandbox_state)) into a helper like
recoverActiveFailedSession(sessionRow) that returns the effective row, and move
the "kick lifecycle if overdue" logic (checking effectiveRow.lifecycle_state,
Date.now() >= getLifecycleDueAtMs(effectiveRow), and calling
kickSandboxLifecycleWorkflow({...})) into a helper like
scheduleLifecycleIfOverdue(effectiveRow) that returns void; then call these two
helpers from getSandboxStatusHandler so the handler stays under 50 lines while
reusing functions updateSession, kickSandboxLifecycleWorkflow, isSandboxActive,
and getLifecycleDueAtMs.
In `@lib/sessions/patchSessionByIdHandler.ts`:
- Around line 57-60: The spread-with-&& pattern in the update payload for
updateSession is unclear; instead build an explicit updates object (e.g., create
a mutable updates: Record<string, any> and conditionally set updates.title =
body.title and updates.status = body.status only when body.title/status are !==
undefined) or use ternary expressions that return {} when absent, then call
updateSession(sessionId, updates); updateSession, the updated variable and
body.title/body.status are the relevant symbols to change in
patchSessionByIdHandler.ts.
🪄 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: 60c35ce4-68d6-49f1-89da-1e1a2c6cfe0f
⛔ Files ignored due to path filters (1)
app/api/sessions/[sessionId]/__tests__/route.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**
📒 Files selected for processing (15)
app/api/sessions/[sessionId]/route.tsapp/workflows/clearLifecycleRunIdIfOwned.tsapp/workflows/computeLifecycleWakeDecision.tslib/sandbox/createSandboxHandler.tslib/sandbox/evaluateSandboxLifecycle.tslib/sandbox/getSandboxStatusHandler.tslib/sandbox/reclaimStaleLease.tslib/sandbox/runKick.tslib/sandbox/validateSandboxReconnectRequest.tslib/sandbox/wasLifecycleTimingExtended.tslib/sessions/getSessionByIdHandler.tslib/sessions/patchSessionByIdHandler.tslib/sessions/resolveSessionTitle.tslib/sessions/validatePatchSessionBody.tslib/supabase/sessions/selectSessions.ts
There was a problem hiding this comment.
3 issues found across 14 files (changes from recent commits).
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/evaluateSandboxLifecycle.ts">
<violation number="1" location="lib/sandbox/evaluateSandboxLifecycle.ts:38">
P1: Do not coerce `selectSessions(...)=null` to `[]` here; it masks DB failures as `session-not-found`.</violation>
<violation number="2" location="lib/sandbox/evaluateSandboxLifecycle.ts:73">
P0: Avoid `?? []` on the refresh query; failed refreshes should go through the error path, not return `not-due-yet`.</violation>
</file>
<file name="app/workflows/clearLifecycleRunIdIfOwned.ts">
<violation number="1" location="app/workflows/clearLifecycleRunIdIfOwned.ts:17">
P2: The `?? []` coalesces a DB error (`null`) into an empty array, so a transient database failure is silently misreported as `"session-not-found"`. Unlike the handler files (`getSessionByIdHandler`, `patchSessionByIdHandler`) which now properly check for `null` and return a 500, this workflow path loses the error signal entirely. Consider checking for `null` explicitly and returning a distinct reason (e.g., `"db-error"`) or at minimum logging when the coalescion fires, to preserve observability.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Updated the `selectSessions` function calls in `clearLifecycleRunIdIfOwned`, `computeLifecycleWakeDecision`, and `evaluateSandboxLifecycle` to handle potential database errors more gracefully. - Added console error logging in `clearLifecycleRunIdIfOwned` for better debugging. - Enhanced return values in `computeLifecycleWakeDecision` and `evaluateSandboxLifecycle` to provide clearer failure reasons when session queries fail. These changes enhance the robustness of session management by ensuring that errors are properly logged and handled.
- Enhanced error handling in the `evaluateSandboxLifecycle` function by ensuring that an error is thrown with a clear message when session refresh fails during lifecycle extension checks. This change improves the robustness of session management by providing better feedback in case of failures.
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: This PR modifies the return type of a core data access function (selectSessions) from an empty array to null on DB errors, requiring updates across 10+ files in critical workflows and sandbox operations, and introduces a new PATCH endpoint with business logic for session updates, which together...
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR modifies the return type of a shared utility function (selectSessions) and updates error handling across numerous files in sandbox lifecycle and workflows, which introduces a significant blast radius and risk of regression if any caller is missed or if the new error handling behaves...
…nal status values and line counters - Updated the API documentation to reflect the new optional fields for line counters and expanded status options (running, completed, failed, archived). - Modified the patchSessionByIdHandler to handle updates for linesAdded and linesRemoved, returning a 200 status when no updates are provided. - Added tests to ensure correct handling of new status values and line counter mappings in the session update process.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/sandbox/evaluateSandboxLifecycle.ts (1)
74-81:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd guard for empty refreshedRows array.
After confirming
refreshedRowsis not null, the code accessesrefreshedRows[0]without verifying the array contains an element. IfselectSessionsreturns[](successful query but session no longer exists),refreshedwill be undefined. The optional chaining on line 78 would silently skip the restore, and line 81 returns"not-due-yet", which is incorrect if the session has disappeared during the extension check.🛡️ Proposed fix to guard against missing session
const refreshedRows = await selectSessions({ id: sessionId }); if (!refreshedRows) throw new Error("Failed to refresh session during lifecycle extension check"); const refreshed = refreshedRows[0]; +if (!refreshed) + throw new Error("Session disappeared during lifecycle extension check"); if (refreshed?.sandbox_state) { await restoreActiveLifecycleState(sessionId, refreshed.sandbox_state); }🤖 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/evaluateSandboxLifecycle.ts` around lines 74 - 81, After calling selectSessions(sessionId) check that refreshedRows is a non-empty array (e.g. refreshedRows.length > 0); if it's empty, treat that as a missing session and either throw a descriptive error (e.g. "Session not found during lifecycle extension check") or return an explicit result like { action: "not-found" } instead of proceeding to access refreshedRows[0]; keep the subsequent logic that assigns const refreshed = refreshedRows[0] and conditionally calls restoreActiveLifecycleState(sessionId, refreshed.sandbox_state) only when refreshed exists.
🤖 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/sessions/validatePatchSessionBody.ts`:
- Around line 42-43: The handler currently calls safeParseJson and treats
parsing errors as an empty object so malformed JSON silently passes
patchSessionBodySchema (all-optional) — change validatePatchSessionBody.ts to
detect malformed JSON and return 400: read the raw request text (await
request.text()), if the text is non-empty attempt JSON.parse in a try/catch; on
parse throw, respond with HTTP 400 and a clear error; otherwise use the parsed
object (or empty object if text is empty) with patchSessionBodySchema.safeParse.
Alternatively, update safeParseJson to return a distinct error/sentinel on parse
failure and have validatePatchSessionBody.ts return 400 when that sentinel is
returned; refer to safeParseJson and patchSessionBodySchema to locate the
change.
---
Outside diff comments:
In `@lib/sandbox/evaluateSandboxLifecycle.ts`:
- Around line 74-81: After calling selectSessions(sessionId) check that
refreshedRows is a non-empty array (e.g. refreshedRows.length > 0); if it's
empty, treat that as a missing session and either throw a descriptive error
(e.g. "Session not found during lifecycle extension check") or return an
explicit result like { action: "not-found" } instead of proceeding to access
refreshedRows[0]; keep the subsequent logic that assigns const refreshed =
refreshedRows[0] and conditionally calls restoreActiveLifecycleState(sessionId,
refreshed.sandbox_state) only when refreshed exists.
🪄 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: 6f944f1f-f791-4c26-bd94-b6a9c99d9bd7
⛔ Files ignored due to path filters (1)
app/api/sessions/[sessionId]/__tests__/route.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**
📒 Files selected for processing (6)
app/api/sessions/[sessionId]/route.tsapp/workflows/clearLifecycleRunIdIfOwned.tsapp/workflows/computeLifecycleWakeDecision.tslib/sandbox/evaluateSandboxLifecycle.tslib/sessions/patchSessionByIdHandler.tslib/sessions/validatePatchSessionBody.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/sessions/[sessionId]/route.ts
- lib/sessions/patchSessionByIdHandler.ts
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: This PR introduces a new PATCH endpoint and modifies the global error-handling behavior of the selectSessions utility, impacting multiple critical workflow and sandbox components that require manual review.
…d JSON body - Added a new utility function to read and validate the JSON body of PATCH requests, returning an empty object for empty bodies and a 400 error for malformed JSON. - Updated the test suite to include a case for handling malformed JSON, ensuring the API responds correctly with a 400 status and an appropriate error message. - Adjusted the validation logic in `validatePatchSessionBody` to utilize the new JSON reading function, improving error handling for invalid input.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This PR introduces a new PATCH endpoint for session updates and changes the return type of selectSessions to possibly null, affecting multiple callers across sandbox and workflow files; these are not trivial changes and require a human review to ensure correctness, especially for edge cases...
Manual smoke test —
|
2 participants
This enhancement improves the API's functionality by enabling users to modify session details dynamically.
Summary by cubic
Adds PATCH /api/sessions/{sessionId} to update a session’s title, status, and optional line counters, and strengthens validation and DB error handling across reads and lifecycle flows. GET/PATCH now return consistent 4xx/5xx responses; empty bodies are allowed and malformed JSON returns a 400.
New Features
linesAdded/linesRemovedmapped tolines_added/lines_removed.x-api-key; Zod validation with clear 400s (error path/message) and "Invalid JSON body" for malformed payloads.Bug Fixes
selectSessionsreturns null on DB errors; GET/PATCH respond 500 with consistent JSON.Written for commit 7717c57. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes