Uh oh!
There was an error while loading. Please reload this page.
feat(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents - #562
Merged
Conversation
…open-agents
Returns the chat's persisted UI message stream plus its current
streaming state so callers can hydrate / refresh a chat view:
{ chat: { id, modelId, activeStreamId }, isStreaming, messages }
`messages` is the raw `parts` JSON for each `chat_messages` row,
ordered by `created_at` then `id`. `isStreaming` is derived from
`active_stream_id`.
Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when
the session or chat is missing (or when the chat lives in a
different session); 403 when the session is owned by a different
account.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Warning Review limit reached
More reviews will be available in 36 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. 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 (1)
📝 WalkthroughWalkthroughThis PR implements a complete REST endpoint at ChangesSession Chat REST Endpoints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
2 tasks
There was a problem hiding this comment.
3 issues found across 6 files
Confidence score: 3/5
- There is a concrete regression risk in
lib/supabase/chat_messages/selectChatMessages.ts: returning[]on query failure can hide real database errors as successful empty chats, which can mislead users and downstream logic. lib/sessions/chats/validateGetSessionChatRequest.tscompares against the raw route parameter instead of resolvedsession.id, which can produce false "Chat not found" results when ID canonicalization/formatting differs.- Given the two medium-to-high severity, high-confidence behavior issues (6/10 and 7/10), this carries some user-facing risk and is best fixed before merge.
- Pay close attention to
lib/supabase/chat_messages/selectChatMessages.tsandlib/sessions/chats/validateGetSessionChatRequest.ts- error handling and ID comparison logic can cause incorrect chat retrieval outcomes.
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/chats/__tests__/validateGetSessionChatRequest.test.ts">
<violation number="1" location="lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts:1">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Test file exceeds the repository’s 100-line file-size limit.</violation>
</file>
<file name="lib/sessions/chats/validateGetSessionChatRequest.ts">
<violation number="1" location="lib/sessions/chats/validateGetSessionChatRequest.ts:62">
P2: Compare `chat.session_id` to the resolved `session.id` instead of the raw route parameter to avoid false "Chat not found" responses when ID formatting/canonicalization differs.</violation>
</file>
<file name="lib/supabase/chat_messages/selectChatMessages.ts">
<violation number="1" location="lib/supabase/chat_messages/selectChatMessages.ts:28">
P1: Do not return `[]` on query failure here; it masks database errors as successful empty chats.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Client (Frontend)
participant API as API Route (Next.js)
participant Auth as Auth Service
participant DB as Supabase (PostgreSQL)
Note over UI,DB: NEW: GET /api/sessions/{sessionId}/chats/{chatId}
UI->>API: GET Request
API->>Auth: NEW: validateAuthContext(request)
alt Auth Success (Privy / API Key)
Auth-->>API: AuthContext (accountId)
else Auth Failure
Auth-->>UI: 401 Unauthorized
end
API->>DB: NEW: selectSessions({ id: sessionId })
alt Session Not Found
DB-->>API: []
API-->>UI: 404 Not Found
else Session Exists
DB-->>API: session record
opt session.account_id != auth.accountId
API-->>UI: 403 Forbidden
end
end
API->>DB: NEW: selectChats({ id: chatId })
alt Chat Not Found OR Session Mismatch
DB-->>API: [] or mismatched session_id
API-->>UI: 404 Not Found
else Chat Valid
DB-->>API: chat record (incl. active_stream_id)
end
API->>DB: NEW: selectChatMessages({ chatId })
DB-->>API: List of messages (ordered by created_at, id)
Note right of API: Derive isStreaming from active_stream_id
API-->>UI: 200 OK { chat, isStreaming, messages } (CORS headers)
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Uh oh!
There was an error while loading. Please reload this page.
| const chatRows = await selectChats({ id: chatId }); | ||
| const chat = chatRows[0] ?? null; | ||
| if (!chat || chat.session_id !== sessionId) { |
There was a problem hiding this comment.
P2: Compare chat.session_id to the resolved session.id instead of the raw route parameter to avoid false "Chat not found" responses when ID formatting/canonicalization differs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/validateGetSessionChatRequest.ts, line 62:
<comment>Compare `chat.session_id` to the resolved `session.id` instead of the raw route parameter to avoid false "Chat not found" responses when ID formatting/canonicalization differs.</comment>
<file context>
@@ -0,0 +1,70 @@
+ const chatRows = await selectChats({ id: chatId });
+ const chat = chatRows[0] ?? null;
+
+ if (!chat || chat.session_id !== sessionId) {
+ return NextResponse.json(
+ { status: "error", error: "Chat not found" },
</file context>
| @@ -0,0 +1,153 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
Test file exceeds the repository’s 100-line file-size limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts, line 1:
<comment>Test file exceeds the repository’s 100-line file-size limit.</comment>
<file context>
@@ -0,0 +1,153 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow";
</file context>
6 tasks
…s/[chatId]
PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field
required; both must be non-empty after trim. modelId is stored as-is
(no model-variant sanitization until user-preferences are migrated).
DELETE returns `{ success: true }`. Refuses with 400 if the chat is
the only one in its session.
Both reuse the same auth + session-ownership + chat-belongs-to-session
gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`.
22 new vitest cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
lib/sessions/chats/validateGetSessionChatRequest.ts (1)
32-70: 🏗️ Heavy liftExtract shared auth/session/chat validation into a reusable utility.
This validator duplicates the same gating flow used by PATCH/DELETE and is already large. Centralizing it reduces drift and keeps validators focused on method-specific rules.
As per coding guidelines, "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle", "Keep functions small and focused", and "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/sessions/chats/validateGetSessionChatRequest.ts` around lines 32 - 70, This validator validateGetSessionChatRequest duplicates auth/session/chat gating used elsewhere; extract the shared flow into a reusable helper (e.g., validateAuthSessionChat or validateAuthAndResource) that calls validateAuthContext, selectSessions, and selectChats and returns a unified result or NextResponse, then replace validateGetSessionChatRequest to call that helper and apply only method-specific checks; update other PATCH/DELETE validators to use the same helper so authentication, session existence, ownership (session.account_id vs auth.accountId), and chat-session matching logic (selectSessions/selectChats) are centralized.
🤖 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/chats/deleteSessionChatHandler.ts`:
- Around line 21-27: The current “not last chat” check and delete are separate,
allowing races; change the deletion to be atomic by moving the last-chat guard
into a single DB transaction/conditional delete. Implement or update deleteChat
(or add deleteChatIfNotLast(sessionId, chatId)) so it performs a transaction
that 1) locks the session's chats (SELECT ... FOR UPDATE or equivalent), 2)
verifies count > 1, and 3) deletes the chat in the same transaction (or use a
single conditional DELETE with a subquery that requires count>1). Return false
if the chat was not deleted (because it was the last), and update
deleteSessionChatHandler to call this new atomic delete function instead of
separately validating and deleting.
In `@lib/sessions/chats/validateDeleteSessionChatRequest.ts`:
- Around line 58-73: The current TOCTOU bug arises because selectChats + length
check (siblingChats) and the deletion are separate; replace this two-step guard
with a single atomic DB operation (transaction or conditional delete RPC) that
deletes the target chat only if sessionId has more than one chat and returns
whether the delete occurred; locate the logic around selectChats, siblingChats,
chat, chatId and sessionId in validateDeleteSessionChatRequest and instead call
a transactional/conditional delete function (or implement a DB-level RPC) that
enforces "delete only if sibling count > 1" and return the appropriate 404/400
error based on the atomic operation result.
In `@lib/sessions/chats/validatePatchSessionChatRequest.ts`:
- Around line 10-20: The schema patchSessionChatBodySchema is currently
unexported while its inferred type PatchSessionChatBody is exported; export the
schema as well so validation contracts are reusable and testable. Update the
file to export patchSessionChatBodySchema (e.g., export const
patchSessionChatBodySchema) alongside the existing exported type
PatchSessionChatBody and ensure any imports elsewhere are updated to use the
exported schema where needed.
In `@lib/supabase/chat_messages/selectChatMessages.ts`:
- Around line 26-30: The function selectChatMessages currently swallows DB
errors by logging and returning [] (see the error handling branch that checks
error and returns []), which masks failures; change it to propagate the failure
instead—either throw the received error (or wrap it with context) so callers
receive an exception, or return an error Result type if your codebase uses
Results; update the branch that references error in selectChatMessages to remove
the silent return [] and rethrow or return the error so the endpoint can respond
with a server error.
In `@lib/supabase/chats/updateChat.ts`:
- Around line 17-34: The updateChat function currently returns null for both DB
errors and missing rows; change it so DB errors are surfaced (throw) and null is
only returned for "not found". Specifically, in updateChat, after the supabase
.update(...).select().maybeSingle() call, if error is truthy throw a descriptive
error (include the supabase error object/message) instead of returning null;
keep returning data (which will be null when maybeSingle yields no row) for the
not-found case, and update the function signature/return type accordingly (or
document that it throws on DB errors) so callers can distinguish 404 (null) from
500 (exception).
---
Nitpick comments:
In `@lib/sessions/chats/validateGetSessionChatRequest.ts`:
- Around line 32-70: This validator validateGetSessionChatRequest duplicates
auth/session/chat gating used elsewhere; extract the shared flow into a reusable
helper (e.g., validateAuthSessionChat or validateAuthAndResource) that calls
validateAuthContext, selectSessions, and selectChats and returns a unified
result or NextResponse, then replace validateGetSessionChatRequest to call that
helper and apply only method-specific checks; update other PATCH/DELETE
validators to use the same helper so authentication, session existence,
ownership (session.account_id vs auth.accountId), and chat-session matching
logic (selectSessions/selectChats) are centralized.
🪄 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: 33db5f2e-661f-430c-b8a7-635552330446
⛔ Files ignored due to path filters (6)
lib/sessions/chats/__tests__/deleteSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/getSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/patchSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (10)
app/api/sessions/[sessionId]/chats/[chatId]/route.tslib/sessions/chats/deleteSessionChatHandler.tslib/sessions/chats/getSessionChatHandler.tslib/sessions/chats/patchSessionChatHandler.tslib/sessions/chats/validateDeleteSessionChatRequest.tslib/sessions/chats/validateGetSessionChatRequest.tslib/sessions/chats/validatePatchSessionChatRequest.tslib/supabase/chat_messages/selectChatMessages.tslib/supabase/chats/deleteChat.tslib/supabase/chats/updateChat.ts
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+58
to
+73
| const siblingChats = await selectChats({ sessionId }); | ||
| const chat = siblingChats.find(row => row.id === chatId) ?? null; | ||
| if (!chat) { | ||
| return NextResponse.json( | ||
| { status: "error", error: "Chat not found" }, | ||
| { status: 404, headers: getCorsHeaders() }, | ||
| ); | ||
| } | ||
| if (siblingChats.length <= 1) { | ||
| return NextResponse.json( | ||
| { status: "error", error: "Cannot delete the only chat in a session" }, | ||
| { status: 400, headers: getCorsHeaders() }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The “cannot delete only chat” guard is race-prone (TOCTOU).
The sibling-count check and the actual delete happen in separate operations, so concurrent deletes can both pass the check and violate the invariant. Enforce this with one atomic DB operation (transaction/RPC with conditional delete).
🤖 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/chats/validateDeleteSessionChatRequest.ts` around lines 58 - 73,
The current TOCTOU bug arises because selectChats + length check (siblingChats)
and the deletion are separate; replace this two-step guard with a single atomic
DB operation (transaction or conditional delete RPC) that deletes the target
chat only if sessionId has more than one chat and returns whether the delete
occurred; locate the logic around selectChats, siblingChats, chat, chatId and
sessionId in validateDeleteSessionChatRequest and instead call a
transactional/conditional delete function (or implement a DB-level RPC) that
enforces "delete only if sibling count > 1" and return the appropriate 404/400
error based on the atomic operation result.
Comment on lines
+10
to
+20
| const patchSessionChatBodySchema = z | ||
| .object({ | ||
| title: z.string().trim().min(1, "title cannot be empty").optional(), | ||
| modelId: z.string().trim().min(1, "modelId cannot be empty").optional(), | ||
| }) | ||
| .refine(value => value.title !== undefined || value.modelId !== undefined, { | ||
| message: "At least one field is required", | ||
| }); | ||
| export type PatchSessionChatBody = z.infer<typeof patchSessionChatBodySchema>; | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Export the PATCH body schema alongside the inferred type.
PatchSessionChatBody is exported, but patchSessionChatBodySchema is private. Export the schema too so validation contracts stay reusable and testable per repo convention.
As per coding guidelines, "lib/**/validate*.ts: ... export both the schema and inferred TypeScript type".
🤖 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/chats/validatePatchSessionChatRequest.ts` around lines 10 - 20,
The schema patchSessionChatBodySchema is currently unexported while its inferred
type PatchSessionChatBody is exported; export the schema as well so validation
contracts are reusable and testable. Update the file to export
patchSessionChatBodySchema (e.g., export const patchSessionChatBodySchema)
alongside the existing exported type PatchSessionChatBody and ensure any imports
elsewhere are updated to use the exported schema where needed.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
5 issues found across 11 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/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts">
<violation number="1" location="lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
New test file exceeds the repository's 100-line file-size limit.</violation>
</file>
<file name="app/api/sessions/[sessionId]/chats/[chatId]/route.ts">
<violation number="1" location="app/api/sessions/[sessionId]/chats/[chatId]/route.ts:52">
P2: This route now handles PATCH and DELETE even though the migration is intended to be GET-only, expanding behavior beyond scope and potentially changing which backend implementation serves those methods.</violation>
</file>
<file name="lib/sessions/chats/patchSessionChatHandler.ts">
<violation number="1" location="lib/sessions/chats/patchSessionChatHandler.ts:26">
P2: Wrap the update call in `try/catch` and return a generic `"Internal server error"` for all 500 paths, while logging the detailed error server-side.
(Based on your team's feedback about sanitizing 500 responses and avoiding internal detail leakage.) [FEEDBACK_USED]</violation>
</file>
<file name="lib/sessions/chats/deleteSessionChatHandler.ts">
<violation number="1" location="lib/sessions/chats/deleteSessionChatHandler.ts:26">
P1: The "cannot delete the only chat" guard (sibling count check in validation) and the actual `deleteChat` call are separate, non-atomic operations. Two concurrent DELETE requests on the last two chats in a session can both pass the `siblingChats.length <= 1` check and then both proceed to delete, leaving the session with zero chats. Consider enforcing this constraint atomically — e.g., a conditional delete (`DELETE … WHERE (SELECT count(*) …) > 1`) or a Supabase RPC/transaction.</violation>
</file>
<file name="lib/supabase/chats/updateChat.ts">
<violation number="1" location="lib/supabase/chats/updateChat.ts:33">
P2: `updateChat` returns `null` for both database errors and missing rows (since `.maybeSingle()` returns `null` when no row matches). This means the caller in `patchSessionChatHandler` always returns 500, even when the row simply doesn't exist (which should be 404). Return a discriminated result (e.g., throw on DB error and reserve `null` for not-found) so callers can respond with the correct status.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return validated; | ||
| } | ||
| const ok = await deleteChat(chatId); |
There was a problem hiding this comment.
P1: The "cannot delete the only chat" guard (sibling count check in validation) and the actual deleteChat call are separate, non-atomic operations. Two concurrent DELETE requests on the last two chats in a session can both pass the siblingChats.length <= 1 check and then both proceed to delete, leaving the session with zero chats. Consider enforcing this constraint atomically — e.g., a conditional delete (DELETE … WHERE (SELECT count(*) …) > 1) or a Supabase RPC/transaction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/deleteSessionChatHandler.ts, line 26:
<comment>The "cannot delete the only chat" guard (sibling count check in validation) and the actual `deleteChat` call are separate, non-atomic operations. Two concurrent DELETE requests on the last two chats in a session can both pass the `siblingChats.length <= 1` check and then both proceed to delete, leaving the session with zero chats. Consider enforcing this constraint atomically — e.g., a conditional delete (`DELETE … WHERE (SELECT count(*) …) > 1`) or a Supabase RPC/transaction.</comment>
<file context>
@@ -0,0 +1,35 @@
+ return validated;
+ }
+
+ const ok = await deleteChat(chatId);
+ if (!ok) {
+ return NextResponse.json(
</file context>
| @@ -0,0 +1,145 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
New test file exceeds the repository's 100-line file-size limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts, line 1:
<comment>New test file exceeds the repository's 100-line file-size limit.</comment>
<file context>
@@ -0,0 +1,145 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow";
</file context>
| * @param options.params - Route params containing the session id and chat id. | ||
| * @returns A NextResponse with `{ chat }` on 200, or an error. | ||
| */ | ||
| export async function PATCH( |
There was a problem hiding this comment.
P2: This route now handles PATCH and DELETE even though the migration is intended to be GET-only, expanding behavior beyond scope and potentially changing which backend implementation serves those methods.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/sessions/[sessionId]/chats/[chatId]/route.ts, line 52:
<comment>This route now handles PATCH and DELETE even though the migration is intended to be GET-only, expanding behavior beyond scope and potentially changing which backend implementation serves those methods.</comment>
<file context>
@@ -36,6 +38,44 @@ export async function GET(
+ * @param options.params - Route params containing the session id and chat id.
+ * @returns A NextResponse with `{ chat }` on 200, or an error.
+ */
+export async function PATCH(
+ request: NextRequest,
+ options: { params: Promise<{ sessionId: string; chatId: string }> },
</file context>
| return validated; | ||
| } | ||
| const updated = await updateChat({ |
There was a problem hiding this comment.
P2: Wrap the update call in try/catch and return a generic "Internal server error" for all 500 paths, while logging the detailed error server-side.
(Based on your team's feedback about sanitizing 500 responses and avoiding internal detail leakage.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/patchSessionChatHandler.ts, line 26:
<comment>Wrap the update call in `try/catch` and return a generic `"Internal server error"` for all 500 paths, while logging the detailed error server-side.
(Based on your team's feedback about sanitizing 500 responses and avoiding internal detail leakage.) </comment>
<file context>
@@ -0,0 +1,42 @@
+ return validated;
+ }
+
+ const updated = await updateChat({
+ chatId,
+ patch: {
</file context>
Uh oh!
There was an error while loading. Please reload this page.
The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (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 8 unresolved issues from previous reviews.
Re-trigger cubic
Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (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 8 unresolved issues from previous reviews.
Re-trigger cubic
The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.
Re-trigger cubic
Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 8 files (changes from recent commits).
Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.
Re-trigger cubic
The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.
Re-trigger cubic
Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }`
to the full camelCase wire row (sessionId, title, lastAssistantMessageAt,
createdAt, updatedAt). Lets a single helper cover both initial render
and in-tab refresh on the open-agents side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>There was a problem hiding this comment.
♻️ Duplicate comments (3)
lib/supabase/chats/updateChat.ts (1)
18-35:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSeparate "not found" from "update failed" in the
updateChatcontract.
nullcurrently represents both database errors and missing rows, so callers can't return accurate statuses (404 vs 500). Return a discriminated result (or throw on DB error and reservenullfor not found).🤖 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/chats/updateChat.ts` around lines 18 - 35, The updateChat function currently conflates DB errors and "not found" by returning null; change updateChat to distinguish these cases by either throwing on DB error or returning a discriminated union. Specifically, in updateChat (the supabase .from("chats").update(...).select().maybeSingle() call), if error is truthy then throw or return { status: "error", error } so callers can treat it as a 500, and only reserve null (or { status: "not_found" }) for the case where data is undefined (no row updated); otherwise return the updated row data. Update the function signature (and any callers) to reflect the new return type and ensure the error variable and maybeSingle result are handled separately.lib/sessions/chats/validatePatchSessionChatRequest.ts (1)
8-17: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExport the PATCH body schema alongside the inferred type.
PatchSessionChatBodyis exported, butpatchSessionChatBodySchemais private. Export the schema too so validation contracts stay reusable and testable per repo convention.As per coding guidelines, "lib/**/validate*.ts: ... export both the schema and inferred TypeScript type".
🤖 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/chats/validatePatchSessionChatRequest.ts` around lines 8 - 17, The patchSessionChatBodySchema is not exported which breaks the repository convention of exporting both the Zod schema and its inferred type; update the file to export patchSessionChatBodySchema (in addition to the already exported PatchSessionChatBody type) so other modules/tests can import the schema for validation and reuse, ensuring you export the symbol named patchSessionChatBodySchema alongside the existing PatchSessionChatBody type.lib/sessions/chats/validateDeleteSessionChatRequest.ts (1)
48-63:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftThe "cannot delete only chat" guard is race-prone (TOCTOU).
The sibling-count check and the actual delete happen in separate operations, so concurrent deletes can both pass the check and violate the invariant. Enforce this with one atomic DB operation (transaction/RPC with conditional delete).
🤖 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/chats/validateDeleteSessionChatRequest.ts` around lines 48 - 63, The current two-step check (calling selectChats and then returning an error if siblingChats.length <= 1) is TOCTOU-prone; replace it with a single atomic DB operation in validateDeleteSessionChatRequest that attempts a conditional delete (or wrapped transaction/RPC) so the DB enforces "do not delete if this is the only chat in the session." Concretely, remove the siblingChats.length guard and instead perform a single delete by chatId and sessionId that only succeeds when the session has more than one chat (e.g., a DELETE ... WHERE id = :chatId AND sessionId = :sessionId AND (SELECT COUNT(*) FROM chats WHERE sessionId = :sessionId) > 1 RETURNING *), then treat a no-rows-affected result as the appropriate 400 error; use selectChats only to verify existence if needed but rely on the atomic conditional delete to prevent concurrent deletions.
🤖 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.
Duplicate comments:
In `@lib/sessions/chats/validateDeleteSessionChatRequest.ts`:
- Around line 48-63: The current two-step check (calling selectChats and then
returning an error if siblingChats.length <= 1) is TOCTOU-prone; replace it with
a single atomic DB operation in validateDeleteSessionChatRequest that attempts a
conditional delete (or wrapped transaction/RPC) so the DB enforces "do not
delete if this is the only chat in the session." Concretely, remove the
siblingChats.length guard and instead perform a single delete by chatId and
sessionId that only succeeds when the session has more than one chat (e.g., a
DELETE ... WHERE id = :chatId AND sessionId = :sessionId AND (SELECT COUNT(*)
FROM chats WHERE sessionId = :sessionId) > 1 RETURNING *), then treat a
no-rows-affected result as the appropriate 400 error; use selectChats only to
verify existence if needed but rely on the atomic conditional delete to prevent
concurrent deletions.
In `@lib/sessions/chats/validatePatchSessionChatRequest.ts`:
- Around line 8-17: The patchSessionChatBodySchema is not exported which breaks
the repository convention of exporting both the Zod schema and its inferred
type; update the file to export patchSessionChatBodySchema (in addition to the
already exported PatchSessionChatBody type) so other modules/tests can import
the schema for validation and reuse, ensuring you export the symbol named
patchSessionChatBodySchema alongside the existing PatchSessionChatBody type.
In `@lib/supabase/chats/updateChat.ts`:
- Around line 18-35: The updateChat function currently conflates DB errors and
"not found" by returning null; change updateChat to distinguish these cases by
either throwing on DB error or returning a discriminated union. Specifically, in
updateChat (the supabase .from("chats").update(...).select().maybeSingle()
call), if error is truthy then throw or return { status: "error", error } so
callers can treat it as a 500, and only reserve null (or { status: "not_found"
}) for the case where data is undefined (no row updated); otherwise return the
updated row data. Update the function signature (and any callers) to reflect the
new return type and ensure the error variable and maybeSingle result are handled
separately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0a1093d-97c7-485f-9e9d-233a4b99bc0a
⛔ Files ignored due to path filters (6)
lib/sessions/chats/__tests__/deleteSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/getSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/patchSessionChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (7)
lib/sessions/chats/deleteSessionChatHandler.tslib/sessions/chats/getSessionChatHandler.tslib/sessions/chats/patchSessionChatHandler.tslib/sessions/chats/validateDeleteSessionChatRequest.tslib/sessions/chats/validateGetSessionChatRequest.tslib/sessions/chats/validatePatchSessionChatRequest.tslib/supabase/chats/updateChat.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/sessions/chats/patchSessionChatHandler.ts
- lib/sessions/chats/getSessionChatHandler.ts
There was a problem hiding this comment.
1 issue found across 2 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/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts">
<violation number="1" location="lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts:1">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Test file exceeds the repository’s 100-line file-size limit.</violation>
</file>
<file name="lib/sessions/chats/validateGetSessionChatRequest.ts">
<violation number="1" location="lib/sessions/chats/validateGetSessionChatRequest.ts:62">
P2: Compare `chat.session_id` to the resolved `session.id` instead of the raw route parameter to avoid false "Chat not found" responses when ID formatting/canonicalization differs.</violation>
</file>
<file name="app/api/sessions/[sessionId]/chats/[chatId]/route.ts">
<violation number="1" location="app/api/sessions/[sessionId]/chats/[chatId]/route.ts:52">
P2: This route now handles PATCH and DELETE even though the migration is intended to be GET-only, expanding behavior beyond scope and potentially changing which backend implementation serves those methods.</violation>
</file>
<file name="lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts">
<violation number="1" location="lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
New test file exceeds the repository's 100-line file-size limit.</violation>
</file>
<file name="lib/sessions/chats/patchSessionChatHandler.ts">
<violation number="1" location="lib/sessions/chats/patchSessionChatHandler.ts:26">
P2: Wrap the update call in `try/catch` and return a generic `"Internal server error"` for all 500 paths, while logging the detailed error server-side.
(Based on your team's feedback about sanitizing 500 responses and avoiding internal detail leakage.) [FEEDBACK_USED]</violation>
</file>
<file name="lib/sessions/chats/deleteSessionChatHandler.ts">
<violation number="1" location="lib/sessions/chats/deleteSessionChatHandler.ts:26">
P1: The "cannot delete the only chat" guard (sibling count check in validation) and the actual `deleteChat` call are separate, non-atomic operations. Two concurrent DELETE requests on the last two chats in a session can both pass the `siblingChats.length <= 1` check and then both proceed to delete, leaving the session with zero chats. Consider enforcing this constraint atomically — e.g., a conditional delete (`DELETE … WHERE (SELECT count(*) …) > 1`) or a Supabase RPC/transaction.</violation>
</file>
<file name="lib/sessions/chats/__tests__/getSessionChatHandler.test.ts">
<violation number="1" location="lib/sessions/chats/__tests__/getSessionChatHandler.test.ts:69">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Test file exceeds the repository’s 100-line limit.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,113 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
Test file exceeds the repository’s 100-line limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sessions/chats/__tests__/getSessionChatHandler.test.ts, line 69:
<comment>Test file exceeds the repository’s 100-line limit.</comment>
<file context>
@@ -64,14 +64,28 @@ describe("getSessionChatHandler", () => {
- chat: { id: string; modelId: string | null; activeStreamId: string | null };
+ chat: {
+ id: string;
+ sessionId: string;
+ title: string;
+ modelId: string | null;
</file context>
Tip: Review your code locally with the cubic CLI to iterate faster.
arpitgupta1214
commented
May 15, 2026
CollaboratorAuthor
Smoke test results — preview deploymentTested against
Sample GET happy-path body: {
"chat": {
"id": "50f6fc8d-bdf9-4648-b9f9-7336a6c2de74",
"sessionId": "fefa7a4b-cd9e-4127-863c-14fea8a218c0",
"title": "New chat",
"modelId": "anthropic/claude-haiku-4.5",
"activeStreamId": null,
"lastAssistantMessageAt": null,
"createdAt": "2026-05-15T19:05:26.315048+00:00",
"updatedAt": "2026-05-15T19:05:26.315048+00:00"
},
"isStreaming": false,
"messages": []
} |
…t-by-id # Conflicts: # lib/supabase/chat_messages/selectChatMessages.ts # lib/supabase/chats/updateChat.ts
Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 7 unresolved issues from previous reviews.
Re-trigger cubic
sweetmantech
approved these changes
May 26, 2026
The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. Co-Authored-By: Claude Opus 4.7 (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 7 unresolved issues from previous reviews.
Re-trigger cubic
sweetmantech
approved these changes
May 26, 2026
sweetmantech
commented
May 26, 2026
Contributor
Manual verification on previewPreview:https://api-git-feat-port-session-chat-by-id-recoup.vercel.app SetupCreated a session + initial chat via Results
All 14 expectations pass. The Side noteThe |
Uh oh!
There was an error while loading. Please reload this page.
3 tasks
sweetmantech added a commit
that referenced
this pull request
May 26, 2026
…ant persistence (#616) * fix(chat-workflow): persist the assistant message after a successful run (#609) * fix(chat-workflow): persist the assistant message after a successful run Closes the silent-data-loss gap that the open-agents → recoup-api cutover introduced: the chat workflow streamed the final assistant message to the client over SSE but never wrote it to `chat_messages`, so a page refresh after a successful exchange wiped the reply. Changes: - New `lib/chat/persistAssistantMessage.ts` step (mirrors open-agents' `app/workflows/chat-post-finish.ts` helper of the same name). Fire-and-forget upsert + chat `updated_at` touch on fresh inserts; idempotent on workflow replay; never throws. - `runAgentStep` now wires an `onFinish` callback into `toUIMessageStream` to capture the assembled assistant message, and returns it alongside `finishReason` as part of the new `RunAgentStepResult` type. - `runAgentWorkflow` calls `persistAssistantMessage(chatId, responseMessage)` after a successful `runAgentStep` (in the try block, BEFORE the existing `clearChatActiveStream` + `closeChatStream` finally). On throw, no message is persisted (nothing was generated); cleanup still runs. Tests: - `persistAssistantMessage.test.ts` — 6 cases (insert + touch, duplicate skip, wrong-role guard, DB-error swallow, exception swallow, role assertion). - `runAgentStep.test.ts` — 3 new cases (onFinish wired, captured responseMessage returned, undefined when onFinish never fires). - `runAgentWorkflow.test.ts` — 3 new cases (persist called on success, not called when responseMessage undefined, not called on throw while cleanup still runs). Full suite: 3159 → 3171 passing. Tracking: #605 (Tier 1, item 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record<string, unknown>` intersection on the outer shape required an index signature that `UIMessage` from `ai` doesn't carry, so wiring runAgentStep's UIMessage return into persistAssistantMessage failed the Vercel build with TS2345. Switched to a minimal duck-typed shape (id/role/parts) — matches both UIMessage and the in-test fixtures structurally. The `chat_messages.parts` column is jsonb so persistence doesn't care about the part subtypes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): mark persistAssistantMessage as a "use step" Vercel Workflow blocks `fetch()` in workflow-body code; the Supabase JS client uses fetch under the hood, so `upsertChatMessage` inside `persistAssistantMessage` failed at runtime with: Global "fetch" is unavailable in workflow functions. Use the "fetch" step function from "workflow" to make HTTP requests. `"use step"` directive moves the function into step-context where fetch is legal. Mirrors open-agents' `persistAssistantMessage` step in `app/workflows/chat-post-finish.ts` (which carries the same directive). Caught via runtime log inspection on the PR preview before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * debug(chat-workflow): TEMP diagnostic logs in persistAssistantMessage Hard-refresh of a chat that ran on PR #609's preview showed the assistant message NOT in chat_messages — meaning silence in the existing error log is NOT the same as "row was written." Adding explicit logs at entry, after upsert, and after updateChat so the runtime tail can disambiguate: - "skip: not assistant role" branch - upsert result shape (ok / isDuplicate / rowPresent) - "persisted + touched chat" success line Will be reverted before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): pass generateMessageId to toUIMessageStream Diagnostic logs revealed every assistant message was arriving at persistAssistantMessage with `messageId: ''` — the AI SDK's default when `generateMessageId` isn't provided. Supabase's `chat_messages.id` PK then treated every workflow run after the first as a duplicate (`onConflict: "id", ignoreDuplicates: true` → isDuplicate: true, rowPresent: false) so no assistant row landed. Generating a stable id once per `runAgentStep` invocation via `generateId()` from `ai`, then plumbing it into `result.toUIMessageStream({ generateMessageId: () => ... })` so: - the streamed chunks carry the id (existing wire format), - `onFinish.responseMessage.id` carries the id, - `persistAssistantMessage` sees a real id and the upsert lands a fresh row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat-workflow): move assistantMessageId generation to workflow body Match open-agents' structural pattern instead of generating the id inline inside runAgentStep. Rationale (which I should have applied the first time, per review feedback): 1. **Multi-step support** — when the Tier 2 outer loop lands, each runAgentStep call needs the SAME assistantMessageId so chunks accumulate under one chat_messages row instead of fragmenting per tool-call iteration. Generating inside the step gives every call a fresh id; generating in the workflow body and threading through makes the upgrade path one-line. 2. **Resume-after-tool-call** — open-agents reuses the latest message's id when `latestMessage.role === "assistant"` so the in-progress assistant turn re-attaches instead of starting a new row. Ported now to avoid a future surprise. 3. **Determinism** — `generateId()` is non-deterministic; the workflow body's WDK constraint forbids that. Wrapping it in a `"use step"` (`generateAssistantMessageId.ts`) makes the value durable across workflow replays. Changes: - New `app/lib/workflows/generateAssistantMessageId.ts` step (mirrors open-agents' local `generateId` step in `apps/web/app/workflows/chat.ts`). - `RunAgentStepInput` gains `assistantMessageId: string`. The inline `generateId()` call is removed. - `runAgentWorkflow` reads `latestMessage`; reuses its id when it's an assistant message, otherwise awaits the step. Threads the result into `runAgentStep`. - Tests: 2 new for the step, 1 new for runAgentStep forwarding, 2 new for the resume-aware branch in runAgentWorkflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(chat-workflow): revert temp diagnostic logs in persistAssistantMessage Logs served their purpose — surfaced the empty-messageId bug (fixed in 8974a37 by threading a workflow-generated id through toUIMessageStream's generateMessageId). UI verification on the PR preview confirmed the assistant row now persists. Reverting the debug logs so production runtime stays quiet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): bump last_assistant_message_at on persist (unread badge parity) Match open-agents' `updateChatAssistantActivity` which sets BOTH `updated_at` and `last_assistant_message_at` to the same timestamp. The recoup-api sidebar's `hasUnread` badge is computed in `lib/sessions/chats/getChatSummaries.ts` as `lastAssistantMessageAt > lastReadAt`, mirroring open-agents' identical query in `apps/web/lib/db/sessions.ts:201`. Without this column bump, an assistant message persisted by the workflow streams to the client, lands in `chat_messages`, but never lights up the unread badge for any other tabs/devices the user has open. The column already exists in `api`'s `chats` schema and `updateChat` already accepts it via `ChatMutableFields` — this is purely a "we forgot to set it" fix. Added two new unit tests: - bumps `last_assistant_message_at` on fresh insert - uses the same timestamp for both columns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: format-fix workflow files (prettier --write) Resolves the format/lint CI failures on df312db — purely whitespace collapsing per the repo's prettier config (no behavior change). 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(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) * feat(credits): port computeCreditsDeductedCents + estimateModelUsageCost from open-agents First piece of the chat-workflow billing path. Ports the per-turn cost math from open-agents' `apps/web/lib/credits/compute-credits-deducted-cents.ts` and `apps/web/lib/models.ts:estimateModelUsageCost` so the same billing logic runs on both sides during the cutover. Resolution order matches open-agents exactly: 1. gateway-reported cost on responseMessage.metadata.totalMessageCost (the same number the chat UI shows next to the response) 2. token-based estimate against the model catalog's cost entry 3. 1c floor when no pricing is available — so a successful turn never lands as a free run Three new files (per api's one-exported-function-per-file SRP): - AvailableModelCost.ts — shape mirroring open-agents' richer cost type (input, output, cache_read, context_over_200k) so the same estimator runs against either catalog - estimateModelUsageCost.ts — token-based USD estimator including the 200k+ context tier swap and cache_read pricing - computeCreditsDeductedCents.ts — top-level orchestrator (gateway cost → token estimate → 1c floor) using api's getAvailableModels directly (no HTTP self-fetch like open-agents does) Test coverage: 27 new unit tests across the two test files. All pricing edge cases covered (NaN/Infinity/negative gateway cost, cached-tokens- exceeding-input clamping, context_over_200k tier swap with partial overrides, catalog miss / fetch failure fallbacks). Unblocks step 3 (deductCreditsWithAudit TS wrapper) of the chat credits gap in #605. Full suite: 3191 → 3205 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(credits): charge credits per chat turn (atomic wallet debit + audit) Closes the silent revenue-loss gap tracked in #605: every successful chat workflow turn now debits the account's wallet AND records a usage_events audit row, in a single atomic transaction. End-to-end flow: 1. runAgentStep's onFinish captures responseMessage.metadata ({totalMessageCost, totalMessageUsage}) — same number the chat UI shows next to the response. 2. runAgentWorkflow calls recordChatUsage(accountId, modelId, message) after persistAssistantMessage. 3. recordChatUsage → computeCreditsDeductedCents (gateway cost OR token estimate OR 1c floor) → deductCreditsWithAudit (supabase.rpc'deduct_credits_with_audit'). 4. The Postgres function (recoupable/database#26) runs the wallet UPDATE and the usage_events INSERT in one implicit transaction — either both land or neither does. Matches open-agents' db.transaction(...) atomicity guarantee. Threads accountId through RunAgentWorkflowInput from validateChatWorkflow (auth-derived; never trusted from the request body). New files: - lib/supabase/credits_usage/deductCreditsWithAudit.ts (+ tests) Thin supabase.rpc wrapper; fire-and-forget (returns ok/error instead of throwing). Lives in lib/supabase/ per CLAUDE.md SRP. - app/lib/workflows/recordChatUsage.ts (+ tests) "use step" function that ties the two together with entry/skip/ success/error logs and graceful handling of missing metadata, catalog failures, and RPC errors. Updated: - app/lib/workflows/runAgentWorkflow.ts + accountId field on RunAgentWorkflowInput + recordChatUsage call after successful persistAssistantMessage - lib/chat/handleChatWorkflowStream.ts + passes validated.accountId into start(runAgentWorkflow, ...) - app/lib/workflows/__tests__/runAgentWorkflow.test.ts + 3 new tests (records on success, skips when no responseMessage, skips when runAgentStep throws) TDD: each new file went red → minimal impl → green. Suite: 3205 → 3220 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): use flat interface for DeductCreditsWithAuditResult Next.js 16's type checker wasn't narrowing the discriminated union `{ ok: true } | { ok: false; error: string }` through `if (!result.ok)`, breaking the production build at `recordChatUsage.ts:90`. Vitest's own type config tolerated it, so this only surfaced on the preview deploy. Flat interface with optional `error?: string` avoids the narrowing requirement entirely — caller can read `result.error` directly when `result.ok` is false. Slight type-safety loss (compiler doesn't enforce that `error` is present when ok is false) is worth the build stability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): regenerate supabase RPC type for deduct_credits_with_audit The previous deploy failed because: 1. `types/database.types.ts` was stale — it didn't include the `deduct_credits_with_audit` RPC that landed in recoupable/database#26 (and was manually applied via the MCP after Supabase's GitHub App 502'd post-merge). Without that entry, `supabase.rpc("deduct_credits_with_audit", ...)` failed Next.js's stricter type check. 2. Even with the entry, the typed `Args.p_event: Json` couldn't accept our `DeductCreditsAuditEvent` interface directly — TS doesn't infer interface → index-signature assignment. Fixes: - Added the `deduct_credits_with_audit` entry to the Functions block of types/database.types.ts (matches the upstream regen via mcp__plugin_supabase_supabase__generate_typescript_types). - Cast `params.event as unknown as Json` at the supabase boundary in deductCreditsWithAudit.ts. The runtime payload is unchanged and the interface keeps its strong typing for callers. Verified locally: `pnpm exec tsc --noEmit` shows no errors in any file this PR touches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(credits): consolidate chat-workflow billing into handleChatCredits (DRY) Addresses the user's PR review: my new files duplicated existing infrastructure. Consolidates everything into the existing pattern (handleChatCredits → getCreditUsage + recordCreditDeduction) so chat workflow billing uses the SAME orchestrator that the streaming chat path (handleChatStream) already uses. Changes: 1. lib/credits/getCreditUsage.ts - Added optional `gatewayCostUsd?: number` parameter - When positive, returns it directly (skips catalog lookup) - Otherwise existing token-math path is unchanged (backwards compat) 2. lib/credits/handleChatCredits.ts - Added `gatewayCostUsd?: number` (threaded to getCreditUsage) - Added `source?: "web" | "api"` (defaults to "web" for backwards compat; chat workflow passes "api" so admin dashboards can distinguish surface in spend rollups) 3. lib/credits/recordCreditDeduction.ts - Switched from `deductCredits + insertUsageEvent` (two separate Supabase calls, non-atomic — could leave wallet/meter drifted on partial failure) to the single `deduct_credits_with_audit` RPC. - Now atomic for ALL callers (chat workflow + research handlers), not just the new chat-workflow path. - Return shape simplified: `{ success: boolean }` instead of `{ success, newBalance }` (no caller was reading newBalance). 4. app/lib/workflows/runAgentWorkflow.ts - Imports handleChatCredits instead of recordChatUsage. - Reads gatewayCostUsd + token counts from responseMessage.metadata.{totalMessageCost, totalMessageUsage}. 5. Deleted (consolidated into existing infrastructure): - app/lib/workflows/recordChatUsage.ts - lib/credits/computeCreditsDeductedCents.ts - lib/credits/estimateModelUsageCost.ts - lib/credits/AvailableModelCost.ts - lib/credits/resolveCostTier.ts - All their test files Net delta: -7 files, +0 new orchestrator function. Plus the atomicity guarantee now applies to research handlers too. TDD: each change went RED → minimum impl → GREEN, with all 3195 tests passing at the end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): mark recordCreditDeduction as 'use step' for workflow runtime Vercel Workflow's build-time detector flagged `nanoid` as a Node.js module that can't run inside the workflow body. Marking recordCreditDeduction as 'use step' moves it into the step runtime where Node modules are allowed. Backwards compatible for the existing research-handler callers (regular API routes) — 'use step' functions execute immediately when called from non-workflow contexts. Also matches open-agents' pattern: their recordWorkflowUsage (which contains the equivalent nanoid call) is a 'use step' function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): collapse inline metadata duck-type (KISS) PR review feedback: the 14-line inline type assertion for `result.responseMessage` was needless boilerplate. Replaced with: 1. Import the existing `AgentMessageMetadata` type (already used by `runAgentStep`'s `messageMetadata` callback — single source of truth for the shape). 2. Hoist a module-level `ZERO_USAGE` default so the fallback when metadata is missing is a named constant, not an inline literal. 3. Cast `result.responseMessage.metadata` once (`as AgentMessageMetadata | undefined`). Net delta: 14 lines → 5 lines inside the workflow body, no behavior change. 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(chat-workflow): auto-commit + push after natural finish (#614) * feat(chat-workflow): auto-commit + push after natural finish (#605) Ports open-agents' auto-commit flow to the chat workflow. When a turn finishes naturally (not `tool-calls`) and the session has both `repo_owner` and `repo_name`, the workflow: 1. `git status --porcelain` to check for changes (hasAutoCommitChanges) 2. Emits `data-commit { status: "pending" }` to the SSE stream so the UI can show a spinner 3. Runs the commit + push (runAutoCommit → performAutoCommit): - `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set - `git add -A` - LLM-generated commit message via `generateText` on `git diff --cached` (falls back to "chore: update repository changes" if the gateway is down or diff is empty) - `git commit -m '<message>'` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin <branch>` 4. Emits `data-commit { status: "success"/"error", url? }` with the resolved commit URL when both committed AND pushed New files (TDD, 33 tests): - `lib/chat/auto-commit/performAutoCommit.ts` (14 tests) — the sandbox.exec orchestration, with granular failure modes so the caller can distinguish "couldn't commit" from "committed but push failed". - `lib/chat/auto-commit/hasAutoCommitChanges.ts` (5 tests) — fast pre-flight, fail-open on errors so runAutoCommit reports the real issue. - `lib/chat/auto-commit/runAutoCommit.ts` (4 tests) — workflow step wrapping performAutoCommit with global error handling. - `lib/chat/auto-commit/buildCommitData.ts` (7 tests) — pure helper shaping the AutoCommitResult into the UIMessageChunk payload (status, commit url with proper URL encoding). - `lib/chat/auto-commit/sendCommitChunk.ts` (3 tests) — workflow step that writes the data-commit chunk into the workflow writable (acquires writer / releases lock). Updated: - `app/lib/workflows/runAgentWorkflow.ts` — auto-commit branch after persistAssistantMessage; wraps VercelState with the `{type: "vercel"}` discriminator before passing to SandboxState consumers. New input fields: `sessionTitle?`, `repoOwner?`, `repoName?`. - `app/lib/workflows/__tests__/runAgentWorkflow.test.ts` — 5 new cases covering the happy path, no-changes skip, missing repo identifiers, finish reason 'tool-calls' (no auto-commit on intermediate turns), and the error path. - `lib/chat/handleChatWorkflowStream.ts` — threads `session.title`, `session.repo_owner`, `session.repo_name` into the workflow input. KNOWN LIMITATION (separate follow-up): the data-commit chunks are emitted live to the SSE stream but are NOT re-persisted onto the assistant message's `parts`. The chunk disappears on page refresh. The commit itself is permanent on GitHub. Re-persistence requires either: (a) a follow-up persistAssistantMessage call with the updated message, or (b) an updateChatMessageParts helper. TDD discipline: each new file went RED → minimum impl → GREEN. Suite: 3195 → 3233 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): derive repoOwner/repoName from session.clone_url Auto-commit needs the owner + repo name to build the GitHub commit URL and to set the remote auth URL on push. The session table has `repo_owner` / `repo_name` columns but they were never populated, so the workflow was always skipping auto-commit silently. Rather than denormalize the data (populate the columns at write time), treat `clone_url` as canonical and parse it at read time. Single source of truth, no drift risk between columns and the URL. New helper `lib/github/parseGitHubRepoIdentifiers.ts` (8 tests) handles https + ssh shapes, .git suffix, trailing slashes, and the null / non-github cases. `handleChatWorkflowStream` parses `session.clone_url` once and threads `repoOwner` / `repoName` into the workflow input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): persist data-commit chunk onto assistant message Closes the "data-commit chunk disappears on refresh" limitation flagged in the initial PR description. The chunk is now merged into the assistant message's `parts` and re-persisted via a UPDATE-only helper, so the `GitDataPartCard` UI in open-agents renders the "Committed at <sha>" affordance on page load — not just during the live SSE stream. New files: - `lib/chat/upsertAssistantDataPart.ts` (5 tests) — pure helper ported from open-agents `apps/web/app/workflows/chat.ts`. Merges a data-part into a message's `parts` by `{type, id}` (replace if matched, append otherwise). Immutable; doesn't mutate the input. - `lib/supabase/chat_messages/updateChatMessageParts.ts` (3 tests) — UPDATE-only helper that bypasses the `upsertChatMessage(onConflict: "id", ignoreDuplicates: true)` no-op-on-second-call semantics. Keeps the first-insert path's replay-idempotency for `persistAssistantMessage`; this helper is specifically for "the row exists, replace its `parts`". Wired into `runAgentWorkflow`: After the resolved data-commit chunk is emitted to the writable, the chunk is merged into `result.responseMessage.parts` via `upsertAssistantDataPart`, then `updateChatMessageParts` writes the updated `parts` to the DB. Mirrors open-agents' two-persist pattern in `apps/web/app/workflows/chat.ts:didUpdateGitData`. Tests: - 2 new in `runAgentWorkflow.test.ts`: - success path now asserts `updateChatMessageParts` was called with the resolved data-commit part merged into the parts array - no-changes path asserts `updateChatMessageParts` is NOT called - +8 helper tests across the two new files Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-commit): persist whole message + use step for workflow runtime Two bugs from the first persistence attempt, both caught when the data-commit chunk failed to appear on the open-agents UI after refresh: 1. `updateChatMessageParts` wasn't marked `"use step"`. The function calls `supabase.from(...).update(...)` which uses fetch under the hood — forbidden in the workflow body. The call ran silently with no effect. Same failure mode I hit on `recordCreditDeduction.ts`. 2. The workflow was passing `messageWithCommit.parts` (the inner parts array) when `chat_messages.parts` actually stores the WHOLE message object — matching `persistAssistantMessage`'s `parts: message as never` storage convention. Pass the merged message object now. Confirmed via direct DB query that the first attempt didn't write anything (the row still had only the original message-shape from `persistAssistantMessage`'s first call). With the step boundary + correct payload shape, the persistence path now executes and stores the data-commit chunk so the open-agents `GitDataPartCard` can render after refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): address SRP / KISS / OCP review feedback Per @sweetmantech's PR review comments on #614: KISS — Renamed updateChatMessageParts → updateChatMessage and moved the "use step" boundary out of the supabase layer. Supabase wrappers now stay pure; step-bound wrappers live in lib/chat/. - lib/supabase/chat_messages/updateChatMessage.ts (no "use step") - lib/chat/persistAssistantDataPart.ts (new) — "use step" wrapper that internally calls upsertAssistantDataPart (merge) then updateChatMessage (write). Single durable step boundary at the chat-domain layer. SRP — Extracted generateCommitMessage to its own file. Was a private helper inside performAutoCommit.ts; now reusable for alternative commit-message strategies and individually testable. - lib/chat/auto-commit/generateCommitMessage.ts (+6 tests) - performAutoCommit.ts imports it instead of defining inline. OCP — Extracted the ~50-line auto-commit block from runAgentWorkflow into its own file. Workflow body shrinks to a single function call; the auto-commit flow can evolve without touching workflow code. - lib/chat/auto-commit/autoCommitChatTurn.ts (+9 tests covering every gate, the no-changes path, the happy path including pending → resolved chunks + persistence, and the error path). - runAgentWorkflow.ts: ~50 lines → 11-line invocation. - Workflow test pruned: 6 sub-step assertions → 4 wiring assertions (the flow itself is exhaustively tested in autoCommitChatTurn.test.ts). Also flattened UpdateChatMessageResult from a discriminated union to a single interface — same Next.js 16 narrowing issue I hit on DeductCreditsWithAuditResult in #612. Net file count: +5 new files, -0 deletions (renames don't count). Lines moved out of runAgentWorkflow.ts: ~50. Tests: 3233 → 3266 passing (+33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): spread input/result + use gpt-5.4-nano for commit messages Per @sweetmantech's PR review: KISS — `runAgentWorkflow` was enumerating 8 fields when constructing the autoCommitChatTurn call. Switched to `{ ...input, ...result, writable, sandboxState }`. Any future fields added to input or result get forwarded automatically; the workflow body stays tight. Updated the workflow test assertion to `expect.objectContaining(...)` since extra fields from input/result are now passed through. Model — Updated `generateCommitMessage` to use `openai/gpt-5.4-nano` instead of `anthropic/claude-haiku-4.5`. Newer, cheaper, and a better fit for the short-output commit-message task. The prompt is unchanged so behavior should be near-identical. 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(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) * feat(api): migrate GET /api/sessions/[sessionId]/chats/[chatId] from open-agents Returns the chat's persisted UI message stream plus its current streaming state so callers can hydrate / refresh a chat view: { chat: { id, modelId, activeStreamId }, isStreaming, messages } `messages` is the raw `parts` JSON for each `chat_messages` row, ordered by `created_at` then `id`. `isStreaming` is derived from `active_stream_id`. Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when the session or chat is missing (or when the chat lives in a different session); 403 when the session is owned by a different account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): also migrate PATCH + DELETE /api/sessions/[sessionId]/chats/[chatId] PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field required; both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated). DELETE returns `{ success: true }`. Refuses with 400 if the chat is the only one in its session. Both reuse the same auth + session-ownership + chat-belongs-to-session gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`. 22 new vitest cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop redundant updated_at stamp in updateChat The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop conditional spread in patch handler Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop unused payload from delete validator The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): slim get/patch validators to just what handlers use Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): patch handler returns camelCase Chat shape The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): GET handler returns full chat row via toChatResponse Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }` to the full camelCase wire row (sessionId, title, lastAssistantMessageAt, createdAt, updatedAt). Lets a single helper cover both initial render and in-tab refresh on the open-agents side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): reject unknown fields on PATCH session chat (.strict()) Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): drop unsupported message arg to zod .strict() The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Arpit Gupta <arpitgupta1214@gmail.com>
3 tasks
sweetmantech added a commit
that referenced
this pull request
May 26, 2026
* fix(chat-workflow): persist the assistant message after a successful run (#609) * fix(chat-workflow): persist the assistant message after a successful run Closes the silent-data-loss gap that the open-agents → recoup-api cutover introduced: the chat workflow streamed the final assistant message to the client over SSE but never wrote it to `chat_messages`, so a page refresh after a successful exchange wiped the reply. Changes: - New `lib/chat/persistAssistantMessage.ts` step (mirrors open-agents' `app/workflows/chat-post-finish.ts` helper of the same name). Fire-and-forget upsert + chat `updated_at` touch on fresh inserts; idempotent on workflow replay; never throws. - `runAgentStep` now wires an `onFinish` callback into `toUIMessageStream` to capture the assembled assistant message, and returns it alongside `finishReason` as part of the new `RunAgentStepResult` type. - `runAgentWorkflow` calls `persistAssistantMessage(chatId, responseMessage)` after a successful `runAgentStep` (in the try block, BEFORE the existing `clearChatActiveStream` + `closeChatStream` finally). On throw, no message is persisted (nothing was generated); cleanup still runs. Tests: - `persistAssistantMessage.test.ts` — 6 cases (insert + touch, duplicate skip, wrong-role guard, DB-error swallow, exception swallow, role assertion). - `runAgentStep.test.ts` — 3 new cases (onFinish wired, captured responseMessage returned, undefined when onFinish never fires). - `runAgentWorkflow.test.ts` — 3 new cases (persist called on success, not called when responseMessage undefined, not called on throw while cleanup still runs). Full suite: 3159 → 3171 passing. Tracking: #605 (Tier 1, item 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record<string, unknown>` intersection on the outer shape required an index signature that `UIMessage` from `ai` doesn't carry, so wiring runAgentStep's UIMessage return into persistAssistantMessage failed the Vercel build with TS2345. Switched to a minimal duck-typed shape (id/role/parts) — matches both UIMessage and the in-test fixtures structurally. The `chat_messages.parts` column is jsonb so persistence doesn't care about the part subtypes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): mark persistAssistantMessage as a "use step" Vercel Workflow blocks `fetch()` in workflow-body code; the Supabase JS client uses fetch under the hood, so `upsertChatMessage` inside `persistAssistantMessage` failed at runtime with: Global "fetch" is unavailable in workflow functions. Use the "fetch" step function from "workflow" to make HTTP requests. `"use step"` directive moves the function into step-context where fetch is legal. Mirrors open-agents' `persistAssistantMessage` step in `app/workflows/chat-post-finish.ts` (which carries the same directive). Caught via runtime log inspection on the PR preview before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * debug(chat-workflow): TEMP diagnostic logs in persistAssistantMessage Hard-refresh of a chat that ran on PR #609's preview showed the assistant message NOT in chat_messages — meaning silence in the existing error log is NOT the same as "row was written." Adding explicit logs at entry, after upsert, and after updateChat so the runtime tail can disambiguate: - "skip: not assistant role" branch - upsert result shape (ok / isDuplicate / rowPresent) - "persisted + touched chat" success line Will be reverted before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): pass generateMessageId to toUIMessageStream Diagnostic logs revealed every assistant message was arriving at persistAssistantMessage with `messageId: ''` — the AI SDK's default when `generateMessageId` isn't provided. Supabase's `chat_messages.id` PK then treated every workflow run after the first as a duplicate (`onConflict: "id", ignoreDuplicates: true` → isDuplicate: true, rowPresent: false) so no assistant row landed. Generating a stable id once per `runAgentStep` invocation via `generateId()` from `ai`, then plumbing it into `result.toUIMessageStream({ generateMessageId: () => ... })` so: - the streamed chunks carry the id (existing wire format), - `onFinish.responseMessage.id` carries the id, - `persistAssistantMessage` sees a real id and the upsert lands a fresh row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat-workflow): move assistantMessageId generation to workflow body Match open-agents' structural pattern instead of generating the id inline inside runAgentStep. Rationale (which I should have applied the first time, per review feedback): 1. **Multi-step support** — when the Tier 2 outer loop lands, each runAgentStep call needs the SAME assistantMessageId so chunks accumulate under one chat_messages row instead of fragmenting per tool-call iteration. Generating inside the step gives every call a fresh id; generating in the workflow body and threading through makes the upgrade path one-line. 2. **Resume-after-tool-call** — open-agents reuses the latest message's id when `latestMessage.role === "assistant"` so the in-progress assistant turn re-attaches instead of starting a new row. Ported now to avoid a future surprise. 3. **Determinism** — `generateId()` is non-deterministic; the workflow body's WDK constraint forbids that. Wrapping it in a `"use step"` (`generateAssistantMessageId.ts`) makes the value durable across workflow replays. Changes: - New `app/lib/workflows/generateAssistantMessageId.ts` step (mirrors open-agents' local `generateId` step in `apps/web/app/workflows/chat.ts`). - `RunAgentStepInput` gains `assistantMessageId: string`. The inline `generateId()` call is removed. - `runAgentWorkflow` reads `latestMessage`; reuses its id when it's an assistant message, otherwise awaits the step. Threads the result into `runAgentStep`. - Tests: 2 new for the step, 1 new for runAgentStep forwarding, 2 new for the resume-aware branch in runAgentWorkflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(chat-workflow): revert temp diagnostic logs in persistAssistantMessage Logs served their purpose — surfaced the empty-messageId bug (fixed in 8974a37 by threading a workflow-generated id through toUIMessageStream's generateMessageId). UI verification on the PR preview confirmed the assistant row now persists. Reverting the debug logs so production runtime stays quiet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): bump last_assistant_message_at on persist (unread badge parity) Match open-agents' `updateChatAssistantActivity` which sets BOTH `updated_at` and `last_assistant_message_at` to the same timestamp. The recoup-api sidebar's `hasUnread` badge is computed in `lib/sessions/chats/getChatSummaries.ts` as `lastAssistantMessageAt > lastReadAt`, mirroring open-agents' identical query in `apps/web/lib/db/sessions.ts:201`. Without this column bump, an assistant message persisted by the workflow streams to the client, lands in `chat_messages`, but never lights up the unread badge for any other tabs/devices the user has open. The column already exists in `api`'s `chats` schema and `updateChat` already accepts it via `ChatMutableFields` — this is purely a "we forgot to set it" fix. Added two new unit tests: - bumps `last_assistant_message_at` on fresh insert - uses the same timestamp for both columns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: format-fix workflow files (prettier --write) Resolves the format/lint CI failures on df312db — purely whitespace collapsing per the repo's prettier config (no behavior change). 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(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) * feat(credits): port computeCreditsDeductedCents + estimateModelUsageCost from open-agents First piece of the chat-workflow billing path. Ports the per-turn cost math from open-agents' `apps/web/lib/credits/compute-credits-deducted-cents.ts` and `apps/web/lib/models.ts:estimateModelUsageCost` so the same billing logic runs on both sides during the cutover. Resolution order matches open-agents exactly: 1. gateway-reported cost on responseMessage.metadata.totalMessageCost (the same number the chat UI shows next to the response) 2. token-based estimate against the model catalog's cost entry 3. 1c floor when no pricing is available — so a successful turn never lands as a free run Three new files (per api's one-exported-function-per-file SRP): - AvailableModelCost.ts — shape mirroring open-agents' richer cost type (input, output, cache_read, context_over_200k) so the same estimator runs against either catalog - estimateModelUsageCost.ts — token-based USD estimator including the 200k+ context tier swap and cache_read pricing - computeCreditsDeductedCents.ts — top-level orchestrator (gateway cost → token estimate → 1c floor) using api's getAvailableModels directly (no HTTP self-fetch like open-agents does) Test coverage: 27 new unit tests across the two test files. All pricing edge cases covered (NaN/Infinity/negative gateway cost, cached-tokens- exceeding-input clamping, context_over_200k tier swap with partial overrides, catalog miss / fetch failure fallbacks). Unblocks step 3 (deductCreditsWithAudit TS wrapper) of the chat credits gap in #605. Full suite: 3191 → 3205 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(credits): charge credits per chat turn (atomic wallet debit + audit) Closes the silent revenue-loss gap tracked in #605: every successful chat workflow turn now debits the account's wallet AND records a usage_events audit row, in a single atomic transaction. End-to-end flow: 1. runAgentStep's onFinish captures responseMessage.metadata ({totalMessageCost, totalMessageUsage}) — same number the chat UI shows next to the response. 2. runAgentWorkflow calls recordChatUsage(accountId, modelId, message) after persistAssistantMessage. 3. recordChatUsage → computeCreditsDeductedCents (gateway cost OR token estimate OR 1c floor) → deductCreditsWithAudit (supabase.rpc'deduct_credits_with_audit'). 4. The Postgres function (recoupable/database#26) runs the wallet UPDATE and the usage_events INSERT in one implicit transaction — either both land or neither does. Matches open-agents' db.transaction(...) atomicity guarantee. Threads accountId through RunAgentWorkflowInput from validateChatWorkflow (auth-derived; never trusted from the request body). New files: - lib/supabase/credits_usage/deductCreditsWithAudit.ts (+ tests) Thin supabase.rpc wrapper; fire-and-forget (returns ok/error instead of throwing). Lives in lib/supabase/ per CLAUDE.md SRP. - app/lib/workflows/recordChatUsage.ts (+ tests) "use step" function that ties the two together with entry/skip/ success/error logs and graceful handling of missing metadata, catalog failures, and RPC errors. Updated: - app/lib/workflows/runAgentWorkflow.ts + accountId field on RunAgentWorkflowInput + recordChatUsage call after successful persistAssistantMessage - lib/chat/handleChatWorkflowStream.ts + passes validated.accountId into start(runAgentWorkflow, ...) - app/lib/workflows/__tests__/runAgentWorkflow.test.ts + 3 new tests (records on success, skips when no responseMessage, skips when runAgentStep throws) TDD: each new file went red → minimal impl → green. Suite: 3205 → 3220 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): use flat interface for DeductCreditsWithAuditResult Next.js 16's type checker wasn't narrowing the discriminated union `{ ok: true } | { ok: false; error: string }` through `if (!result.ok)`, breaking the production build at `recordChatUsage.ts:90`. Vitest's own type config tolerated it, so this only surfaced on the preview deploy. Flat interface with optional `error?: string` avoids the narrowing requirement entirely — caller can read `result.error` directly when `result.ok` is false. Slight type-safety loss (compiler doesn't enforce that `error` is present when ok is false) is worth the build stability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): regenerate supabase RPC type for deduct_credits_with_audit The previous deploy failed because: 1. `types/database.types.ts` was stale — it didn't include the `deduct_credits_with_audit` RPC that landed in recoupable/database#26 (and was manually applied via the MCP after Supabase's GitHub App 502'd post-merge). Without that entry, `supabase.rpc("deduct_credits_with_audit", ...)` failed Next.js's stricter type check. 2. Even with the entry, the typed `Args.p_event: Json` couldn't accept our `DeductCreditsAuditEvent` interface directly — TS doesn't infer interface → index-signature assignment. Fixes: - Added the `deduct_credits_with_audit` entry to the Functions block of types/database.types.ts (matches the upstream regen via mcp__plugin_supabase_supabase__generate_typescript_types). - Cast `params.event as unknown as Json` at the supabase boundary in deductCreditsWithAudit.ts. The runtime payload is unchanged and the interface keeps its strong typing for callers. Verified locally: `pnpm exec tsc --noEmit` shows no errors in any file this PR touches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(credits): consolidate chat-workflow billing into handleChatCredits (DRY) Addresses the user's PR review: my new files duplicated existing infrastructure. Consolidates everything into the existing pattern (handleChatCredits → getCreditUsage + recordCreditDeduction) so chat workflow billing uses the SAME orchestrator that the streaming chat path (handleChatStream) already uses. Changes: 1. lib/credits/getCreditUsage.ts - Added optional `gatewayCostUsd?: number` parameter - When positive, returns it directly (skips catalog lookup) - Otherwise existing token-math path is unchanged (backwards compat) 2. lib/credits/handleChatCredits.ts - Added `gatewayCostUsd?: number` (threaded to getCreditUsage) - Added `source?: "web" | "api"` (defaults to "web" for backwards compat; chat workflow passes "api" so admin dashboards can distinguish surface in spend rollups) 3. lib/credits/recordCreditDeduction.ts - Switched from `deductCredits + insertUsageEvent` (two separate Supabase calls, non-atomic — could leave wallet/meter drifted on partial failure) to the single `deduct_credits_with_audit` RPC. - Now atomic for ALL callers (chat workflow + research handlers), not just the new chat-workflow path. - Return shape simplified: `{ success: boolean }` instead of `{ success, newBalance }` (no caller was reading newBalance). 4. app/lib/workflows/runAgentWorkflow.ts - Imports handleChatCredits instead of recordChatUsage. - Reads gatewayCostUsd + token counts from responseMessage.metadata.{totalMessageCost, totalMessageUsage}. 5. Deleted (consolidated into existing infrastructure): - app/lib/workflows/recordChatUsage.ts - lib/credits/computeCreditsDeductedCents.ts - lib/credits/estimateModelUsageCost.ts - lib/credits/AvailableModelCost.ts - lib/credits/resolveCostTier.ts - All their test files Net delta: -7 files, +0 new orchestrator function. Plus the atomicity guarantee now applies to research handlers too. TDD: each change went RED → minimum impl → GREEN, with all 3195 tests passing at the end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): mark recordCreditDeduction as 'use step' for workflow runtime Vercel Workflow's build-time detector flagged `nanoid` as a Node.js module that can't run inside the workflow body. Marking recordCreditDeduction as 'use step' moves it into the step runtime where Node modules are allowed. Backwards compatible for the existing research-handler callers (regular API routes) — 'use step' functions execute immediately when called from non-workflow contexts. Also matches open-agents' pattern: their recordWorkflowUsage (which contains the equivalent nanoid call) is a 'use step' function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): collapse inline metadata duck-type (KISS) PR review feedback: the 14-line inline type assertion for `result.responseMessage` was needless boilerplate. Replaced with: 1. Import the existing `AgentMessageMetadata` type (already used by `runAgentStep`'s `messageMetadata` callback — single source of truth for the shape). 2. Hoist a module-level `ZERO_USAGE` default so the fallback when metadata is missing is a named constant, not an inline literal. 3. Cast `result.responseMessage.metadata` once (`as AgentMessageMetadata | undefined`). Net delta: 14 lines → 5 lines inside the workflow body, no behavior change. 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(chat-workflow): auto-commit + push after natural finish (#614) * feat(chat-workflow): auto-commit + push after natural finish (#605) Ports open-agents' auto-commit flow to the chat workflow. When a turn finishes naturally (not `tool-calls`) and the session has both `repo_owner` and `repo_name`, the workflow: 1. `git status --porcelain` to check for changes (hasAutoCommitChanges) 2. Emits `data-commit { status: "pending" }` to the SSE stream so the UI can show a spinner 3. Runs the commit + push (runAutoCommit → performAutoCommit): - `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set - `git add -A` - LLM-generated commit message via `generateText` on `git diff --cached` (falls back to "chore: update repository changes" if the gateway is down or diff is empty) - `git commit -m '<message>'` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin <branch>` 4. Emits `data-commit { status: "success"/"error", url? }` with the resolved commit URL when both committed AND pushed New files (TDD, 33 tests): - `lib/chat/auto-commit/performAutoCommit.ts` (14 tests) — the sandbox.exec orchestration, with granular failure modes so the caller can distinguish "couldn't commit" from "committed but push failed". - `lib/chat/auto-commit/hasAutoCommitChanges.ts` (5 tests) — fast pre-flight, fail-open on errors so runAutoCommit reports the real issue. - `lib/chat/auto-commit/runAutoCommit.ts` (4 tests) — workflow step wrapping performAutoCommit with global error handling. - `lib/chat/auto-commit/buildCommitData.ts` (7 tests) — pure helper shaping the AutoCommitResult into the UIMessageChunk payload (status, commit url with proper URL encoding). - `lib/chat/auto-commit/sendCommitChunk.ts` (3 tests) — workflow step that writes the data-commit chunk into the workflow writable (acquires writer / releases lock). Updated: - `app/lib/workflows/runAgentWorkflow.ts` — auto-commit branch after persistAssistantMessage; wraps VercelState with the `{type: "vercel"}` discriminator before passing to SandboxState consumers. New input fields: `sessionTitle?`, `repoOwner?`, `repoName?`. - `app/lib/workflows/__tests__/runAgentWorkflow.test.ts` — 5 new cases covering the happy path, no-changes skip, missing repo identifiers, finish reason 'tool-calls' (no auto-commit on intermediate turns), and the error path. - `lib/chat/handleChatWorkflowStream.ts` — threads `session.title`, `session.repo_owner`, `session.repo_name` into the workflow input. KNOWN LIMITATION (separate follow-up): the data-commit chunks are emitted live to the SSE stream but are NOT re-persisted onto the assistant message's `parts`. The chunk disappears on page refresh. The commit itself is permanent on GitHub. Re-persistence requires either: (a) a follow-up persistAssistantMessage call with the updated message, or (b) an updateChatMessageParts helper. TDD discipline: each new file went RED → minimum impl → GREEN. Suite: 3195 → 3233 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): derive repoOwner/repoName from session.clone_url Auto-commit needs the owner + repo name to build the GitHub commit URL and to set the remote auth URL on push. The session table has `repo_owner` / `repo_name` columns but they were never populated, so the workflow was always skipping auto-commit silently. Rather than denormalize the data (populate the columns at write time), treat `clone_url` as canonical and parse it at read time. Single source of truth, no drift risk between columns and the URL. New helper `lib/github/parseGitHubRepoIdentifiers.ts` (8 tests) handles https + ssh shapes, .git suffix, trailing slashes, and the null / non-github cases. `handleChatWorkflowStream` parses `session.clone_url` once and threads `repoOwner` / `repoName` into the workflow input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): persist data-commit chunk onto assistant message Closes the "data-commit chunk disappears on refresh" limitation flagged in the initial PR description. The chunk is now merged into the assistant message's `parts` and re-persisted via a UPDATE-only helper, so the `GitDataPartCard` UI in open-agents renders the "Committed at <sha>" affordance on page load — not just during the live SSE stream. New files: - `lib/chat/upsertAssistantDataPart.ts` (5 tests) — pure helper ported from open-agents `apps/web/app/workflows/chat.ts`. Merges a data-part into a message's `parts` by `{type, id}` (replace if matched, append otherwise). Immutable; doesn't mutate the input. - `lib/supabase/chat_messages/updateChatMessageParts.ts` (3 tests) — UPDATE-only helper that bypasses the `upsertChatMessage(onConflict: "id", ignoreDuplicates: true)` no-op-on-second-call semantics. Keeps the first-insert path's replay-idempotency for `persistAssistantMessage`; this helper is specifically for "the row exists, replace its `parts`". Wired into `runAgentWorkflow`: After the resolved data-commit chunk is emitted to the writable, the chunk is merged into `result.responseMessage.parts` via `upsertAssistantDataPart`, then `updateChatMessageParts` writes the updated `parts` to the DB. Mirrors open-agents' two-persist pattern in `apps/web/app/workflows/chat.ts:didUpdateGitData`. Tests: - 2 new in `runAgentWorkflow.test.ts`: - success path now asserts `updateChatMessageParts` was called with the resolved data-commit part merged into the parts array - no-changes path asserts `updateChatMessageParts` is NOT called - +8 helper tests across the two new files Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-commit): persist whole message + use step for workflow runtime Two bugs from the first persistence attempt, both caught when the data-commit chunk failed to appear on the open-agents UI after refresh: 1. `updateChatMessageParts` wasn't marked `"use step"`. The function calls `supabase.from(...).update(...)` which uses fetch under the hood — forbidden in the workflow body. The call ran silently with no effect. Same failure mode I hit on `recordCreditDeduction.ts`. 2. The workflow was passing `messageWithCommit.parts` (the inner parts array) when `chat_messages.parts` actually stores the WHOLE message object — matching `persistAssistantMessage`'s `parts: message as never` storage convention. Pass the merged message object now. Confirmed via direct DB query that the first attempt didn't write anything (the row still had only the original message-shape from `persistAssistantMessage`'s first call). With the step boundary + correct payload shape, the persistence path now executes and stores the data-commit chunk so the open-agents `GitDataPartCard` can render after refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): address SRP / KISS / OCP review feedback Per @sweetmantech's PR review comments on #614: KISS — Renamed updateChatMessageParts → updateChatMessage and moved the "use step" boundary out of the supabase layer. Supabase wrappers now stay pure; step-bound wrappers live in lib/chat/. - lib/supabase/chat_messages/updateChatMessage.ts (no "use step") - lib/chat/persistAssistantDataPart.ts (new) — "use step" wrapper that internally calls upsertAssistantDataPart (merge) then updateChatMessage (write). Single durable step boundary at the chat-domain layer. SRP — Extracted generateCommitMessage to its own file. Was a private helper inside performAutoCommit.ts; now reusable for alternative commit-message strategies and individually testable. - lib/chat/auto-commit/generateCommitMessage.ts (+6 tests) - performAutoCommit.ts imports it instead of defining inline. OCP — Extracted the ~50-line auto-commit block from runAgentWorkflow into its own file. Workflow body shrinks to a single function call; the auto-commit flow can evolve without touching workflow code. - lib/chat/auto-commit/autoCommitChatTurn.ts (+9 tests covering every gate, the no-changes path, the happy path including pending → resolved chunks + persistence, and the error path). - runAgentWorkflow.ts: ~50 lines → 11-line invocation. - Workflow test pruned: 6 sub-step assertions → 4 wiring assertions (the flow itself is exhaustively tested in autoCommitChatTurn.test.ts). Also flattened UpdateChatMessageResult from a discriminated union to a single interface — same Next.js 16 narrowing issue I hit on DeductCreditsWithAuditResult in #612. Net file count: +5 new files, -0 deletions (renames don't count). Lines moved out of runAgentWorkflow.ts: ~50. Tests: 3233 → 3266 passing (+33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): spread input/result + use gpt-5.4-nano for commit messages Per @sweetmantech's PR review: KISS — `runAgentWorkflow` was enumerating 8 fields when constructing the autoCommitChatTurn call. Switched to `{ ...input, ...result, writable, sandboxState }`. Any future fields added to input or result get forwarded automatically; the workflow body stays tight. Updated the workflow test assertion to `expect.objectContaining(...)` since extra fields from input/result are now passed through. Model — Updated `generateCommitMessage` to use `openai/gpt-5.4-nano` instead of `anthropic/claude-haiku-4.5`. Newer, cheaper, and a better fit for the short-output commit-message task. The prompt is unchanged so behavior should be near-identical. 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(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) * feat(api): migrate GET /api/sessions/[sessionId]/chats/[chatId] from open-agents Returns the chat's persisted UI message stream plus its current streaming state so callers can hydrate / refresh a chat view: { chat: { id, modelId, activeStreamId }, isStreaming, messages } `messages` is the raw `parts` JSON for each `chat_messages` row, ordered by `created_at` then `id`. `isStreaming` is derived from `active_stream_id`. Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when the session or chat is missing (or when the chat lives in a different session); 403 when the session is owned by a different account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): also migrate PATCH + DELETE /api/sessions/[sessionId]/chats/[chatId] PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field required; both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated). DELETE returns `{ success: true }`. Refuses with 400 if the chat is the only one in its session. Both reuse the same auth + session-ownership + chat-belongs-to-session gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`. 22 new vitest cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop redundant updated_at stamp in updateChat The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop conditional spread in patch handler Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop unused payload from delete validator The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): slim get/patch validators to just what handlers use Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): patch handler returns camelCase Chat shape The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): GET handler returns full chat row via toChatResponse Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }` to the full camelCase wire row (sessionId, title, lastAssistantMessageAt, createdAt, updatedAt). Lets a single helper cover both initial render and in-tab refresh on the open-agents side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): reject unknown fields on PATCH session chat (.strict()) Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): drop unsupported message arg to zod .strict() The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. 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(chat-workflow): persist assistant message per step (#603) * feat(chat-workflow): persist assistant message per step Upgrade the success-only persist (#609) to per-step persistence so a stopped or crashed turn keeps the partial reply instead of dropping it. - runAgentStep streams through createUIMessageStream and persists on every onStepFinish/onFinish (toUIMessageStream exposes only onFinish); it still returns the final responseMessage so the credits path (#612) keeps billing from its metadata. - persistAssistantMessage now overwrites the row as it grows (DO UPDATE via the restored upsertChatMessage `update` flag) and bumps last_assistant_message_at/updated_at on every persist, so a partial reply still surfaces as unread. - runAgentWorkflow drops its own persist call (now per-step) and keeps the #612 credit charge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): spread input into runAgentStep (KISS) Per review feedback on PR #603 — drop the manual field-by-field destructure and forward the workflow input object directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Arpit Gupta <arpitgupta1214@gmail.com>
sweetmantech added a commit
that referenced
this pull request
May 26, 2026
* fix(chat-workflow): persist the assistant message after a successful run (#609) * fix(chat-workflow): persist the assistant message after a successful run Closes the silent-data-loss gap that the open-agents → recoup-api cutover introduced: the chat workflow streamed the final assistant message to the client over SSE but never wrote it to `chat_messages`, so a page refresh after a successful exchange wiped the reply. Changes: - New `lib/chat/persistAssistantMessage.ts` step (mirrors open-agents' `app/workflows/chat-post-finish.ts` helper of the same name). Fire-and-forget upsert + chat `updated_at` touch on fresh inserts; idempotent on workflow replay; never throws. - `runAgentStep` now wires an `onFinish` callback into `toUIMessageStream` to capture the assembled assistant message, and returns it alongside `finishReason` as part of the new `RunAgentStepResult` type. - `runAgentWorkflow` calls `persistAssistantMessage(chatId, responseMessage)` after a successful `runAgentStep` (in the try block, BEFORE the existing `clearChatActiveStream` + `closeChatStream` finally). On throw, no message is persisted (nothing was generated); cleanup still runs. Tests: - `persistAssistantMessage.test.ts` — 6 cases (insert + touch, duplicate skip, wrong-role guard, DB-error swallow, exception swallow, role assertion). - `runAgentStep.test.ts` — 3 new cases (onFinish wired, captured responseMessage returned, undefined when onFinish never fires). - `runAgentWorkflow.test.ts` — 3 new cases (persist called on success, not called when responseMessage undefined, not called on throw while cleanup still runs). Full suite: 3159 → 3171 passing. Tracking: #605 (Tier 1, item 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record<string, unknown>` intersection on the outer shape required an index signature that `UIMessage` from `ai` doesn't carry, so wiring runAgentStep's UIMessage return into persistAssistantMessage failed the Vercel build with TS2345. Switched to a minimal duck-typed shape (id/role/parts) — matches both UIMessage and the in-test fixtures structurally. The `chat_messages.parts` column is jsonb so persistence doesn't care about the part subtypes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): mark persistAssistantMessage as a "use step" Vercel Workflow blocks `fetch()` in workflow-body code; the Supabase JS client uses fetch under the hood, so `upsertChatMessage` inside `persistAssistantMessage` failed at runtime with: Global "fetch" is unavailable in workflow functions. Use the "fetch" step function from "workflow" to make HTTP requests. `"use step"` directive moves the function into step-context where fetch is legal. Mirrors open-agents' `persistAssistantMessage` step in `app/workflows/chat-post-finish.ts` (which carries the same directive). Caught via runtime log inspection on the PR preview before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * debug(chat-workflow): TEMP diagnostic logs in persistAssistantMessage Hard-refresh of a chat that ran on PR #609's preview showed the assistant message NOT in chat_messages — meaning silence in the existing error log is NOT the same as "row was written." Adding explicit logs at entry, after upsert, and after updateChat so the runtime tail can disambiguate: - "skip: not assistant role" branch - upsert result shape (ok / isDuplicate / rowPresent) - "persisted + touched chat" success line Will be reverted before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): pass generateMessageId to toUIMessageStream Diagnostic logs revealed every assistant message was arriving at persistAssistantMessage with `messageId: ''` — the AI SDK's default when `generateMessageId` isn't provided. Supabase's `chat_messages.id` PK then treated every workflow run after the first as a duplicate (`onConflict: "id", ignoreDuplicates: true` → isDuplicate: true, rowPresent: false) so no assistant row landed. Generating a stable id once per `runAgentStep` invocation via `generateId()` from `ai`, then plumbing it into `result.toUIMessageStream({ generateMessageId: () => ... })` so: - the streamed chunks carry the id (existing wire format), - `onFinish.responseMessage.id` carries the id, - `persistAssistantMessage` sees a real id and the upsert lands a fresh row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat-workflow): move assistantMessageId generation to workflow body Match open-agents' structural pattern instead of generating the id inline inside runAgentStep. Rationale (which I should have applied the first time, per review feedback): 1. **Multi-step support** — when the Tier 2 outer loop lands, each runAgentStep call needs the SAME assistantMessageId so chunks accumulate under one chat_messages row instead of fragmenting per tool-call iteration. Generating inside the step gives every call a fresh id; generating in the workflow body and threading through makes the upgrade path one-line. 2. **Resume-after-tool-call** — open-agents reuses the latest message's id when `latestMessage.role === "assistant"` so the in-progress assistant turn re-attaches instead of starting a new row. Ported now to avoid a future surprise. 3. **Determinism** — `generateId()` is non-deterministic; the workflow body's WDK constraint forbids that. Wrapping it in a `"use step"` (`generateAssistantMessageId.ts`) makes the value durable across workflow replays. Changes: - New `app/lib/workflows/generateAssistantMessageId.ts` step (mirrors open-agents' local `generateId` step in `apps/web/app/workflows/chat.ts`). - `RunAgentStepInput` gains `assistantMessageId: string`. The inline `generateId()` call is removed. - `runAgentWorkflow` reads `latestMessage`; reuses its id when it's an assistant message, otherwise awaits the step. Threads the result into `runAgentStep`. - Tests: 2 new for the step, 1 new for runAgentStep forwarding, 2 new for the resume-aware branch in runAgentWorkflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(chat-workflow): revert temp diagnostic logs in persistAssistantMessage Logs served their purpose — surfaced the empty-messageId bug (fixed in 8974a37 by threading a workflow-generated id through toUIMessageStream's generateMessageId). UI verification on the PR preview confirmed the assistant row now persists. Reverting the debug logs so production runtime stays quiet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): bump last_assistant_message_at on persist (unread badge parity) Match open-agents' `updateChatAssistantActivity` which sets BOTH `updated_at` and `last_assistant_message_at` to the same timestamp. The recoup-api sidebar's `hasUnread` badge is computed in `lib/sessions/chats/getChatSummaries.ts` as `lastAssistantMessageAt > lastReadAt`, mirroring open-agents' identical query in `apps/web/lib/db/sessions.ts:201`. Without this column bump, an assistant message persisted by the workflow streams to the client, lands in `chat_messages`, but never lights up the unread badge for any other tabs/devices the user has open. The column already exists in `api`'s `chats` schema and `updateChat` already accepts it via `ChatMutableFields` — this is purely a "we forgot to set it" fix. Added two new unit tests: - bumps `last_assistant_message_at` on fresh insert - uses the same timestamp for both columns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: format-fix workflow files (prettier --write) Resolves the format/lint CI failures on df312db — purely whitespace collapsing per the repo's prettier config (no behavior change). 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(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) * feat(credits): port computeCreditsDeductedCents + estimateModelUsageCost from open-agents First piece of the chat-workflow billing path. Ports the per-turn cost math from open-agents' `apps/web/lib/credits/compute-credits-deducted-cents.ts` and `apps/web/lib/models.ts:estimateModelUsageCost` so the same billing logic runs on both sides during the cutover. Resolution order matches open-agents exactly: 1. gateway-reported cost on responseMessage.metadata.totalMessageCost (the same number the chat UI shows next to the response) 2. token-based estimate against the model catalog's cost entry 3. 1c floor when no pricing is available — so a successful turn never lands as a free run Three new files (per api's one-exported-function-per-file SRP): - AvailableModelCost.ts — shape mirroring open-agents' richer cost type (input, output, cache_read, context_over_200k) so the same estimator runs against either catalog - estimateModelUsageCost.ts — token-based USD estimator including the 200k+ context tier swap and cache_read pricing - computeCreditsDeductedCents.ts — top-level orchestrator (gateway cost → token estimate → 1c floor) using api's getAvailableModels directly (no HTTP self-fetch like open-agents does) Test coverage: 27 new unit tests across the two test files. All pricing edge cases covered (NaN/Infinity/negative gateway cost, cached-tokens- exceeding-input clamping, context_over_200k tier swap with partial overrides, catalog miss / fetch failure fallbacks). Unblocks step 3 (deductCreditsWithAudit TS wrapper) of the chat credits gap in #605. Full suite: 3191 → 3205 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(credits): charge credits per chat turn (atomic wallet debit + audit) Closes the silent revenue-loss gap tracked in #605: every successful chat workflow turn now debits the account's wallet AND records a usage_events audit row, in a single atomic transaction. End-to-end flow: 1. runAgentStep's onFinish captures responseMessage.metadata ({totalMessageCost, totalMessageUsage}) — same number the chat UI shows next to the response. 2. runAgentWorkflow calls recordChatUsage(accountId, modelId, message) after persistAssistantMessage. 3. recordChatUsage → computeCreditsDeductedCents (gateway cost OR token estimate OR 1c floor) → deductCreditsWithAudit (supabase.rpc'deduct_credits_with_audit'). 4. The Postgres function (recoupable/database#26) runs the wallet UPDATE and the usage_events INSERT in one implicit transaction — either both land or neither does. Matches open-agents' db.transaction(...) atomicity guarantee. Threads accountId through RunAgentWorkflowInput from validateChatWorkflow (auth-derived; never trusted from the request body). New files: - lib/supabase/credits_usage/deductCreditsWithAudit.ts (+ tests) Thin supabase.rpc wrapper; fire-and-forget (returns ok/error instead of throwing). Lives in lib/supabase/ per CLAUDE.md SRP. - app/lib/workflows/recordChatUsage.ts (+ tests) "use step" function that ties the two together with entry/skip/ success/error logs and graceful handling of missing metadata, catalog failures, and RPC errors. Updated: - app/lib/workflows/runAgentWorkflow.ts + accountId field on RunAgentWorkflowInput + recordChatUsage call after successful persistAssistantMessage - lib/chat/handleChatWorkflowStream.ts + passes validated.accountId into start(runAgentWorkflow, ...) - app/lib/workflows/__tests__/runAgentWorkflow.test.ts + 3 new tests (records on success, skips when no responseMessage, skips when runAgentStep throws) TDD: each new file went red → minimal impl → green. Suite: 3205 → 3220 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): use flat interface for DeductCreditsWithAuditResult Next.js 16's type checker wasn't narrowing the discriminated union `{ ok: true } | { ok: false; error: string }` through `if (!result.ok)`, breaking the production build at `recordChatUsage.ts:90`. Vitest's own type config tolerated it, so this only surfaced on the preview deploy. Flat interface with optional `error?: string` avoids the narrowing requirement entirely — caller can read `result.error` directly when `result.ok` is false. Slight type-safety loss (compiler doesn't enforce that `error` is present when ok is false) is worth the build stability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): regenerate supabase RPC type for deduct_credits_with_audit The previous deploy failed because: 1. `types/database.types.ts` was stale — it didn't include the `deduct_credits_with_audit` RPC that landed in recoupable/database#26 (and was manually applied via the MCP after Supabase's GitHub App 502'd post-merge). Without that entry, `supabase.rpc("deduct_credits_with_audit", ...)` failed Next.js's stricter type check. 2. Even with the entry, the typed `Args.p_event: Json` couldn't accept our `DeductCreditsAuditEvent` interface directly — TS doesn't infer interface → index-signature assignment. Fixes: - Added the `deduct_credits_with_audit` entry to the Functions block of types/database.types.ts (matches the upstream regen via mcp__plugin_supabase_supabase__generate_typescript_types). - Cast `params.event as unknown as Json` at the supabase boundary in deductCreditsWithAudit.ts. The runtime payload is unchanged and the interface keeps its strong typing for callers. Verified locally: `pnpm exec tsc --noEmit` shows no errors in any file this PR touches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(credits): consolidate chat-workflow billing into handleChatCredits (DRY) Addresses the user's PR review: my new files duplicated existing infrastructure. Consolidates everything into the existing pattern (handleChatCredits → getCreditUsage + recordCreditDeduction) so chat workflow billing uses the SAME orchestrator that the streaming chat path (handleChatStream) already uses. Changes: 1. lib/credits/getCreditUsage.ts - Added optional `gatewayCostUsd?: number` parameter - When positive, returns it directly (skips catalog lookup) - Otherwise existing token-math path is unchanged (backwards compat) 2. lib/credits/handleChatCredits.ts - Added `gatewayCostUsd?: number` (threaded to getCreditUsage) - Added `source?: "web" | "api"` (defaults to "web" for backwards compat; chat workflow passes "api" so admin dashboards can distinguish surface in spend rollups) 3. lib/credits/recordCreditDeduction.ts - Switched from `deductCredits + insertUsageEvent` (two separate Supabase calls, non-atomic — could leave wallet/meter drifted on partial failure) to the single `deduct_credits_with_audit` RPC. - Now atomic for ALL callers (chat workflow + research handlers), not just the new chat-workflow path. - Return shape simplified: `{ success: boolean }` instead of `{ success, newBalance }` (no caller was reading newBalance). 4. app/lib/workflows/runAgentWorkflow.ts - Imports handleChatCredits instead of recordChatUsage. - Reads gatewayCostUsd + token counts from responseMessage.metadata.{totalMessageCost, totalMessageUsage}. 5. Deleted (consolidated into existing infrastructure): - app/lib/workflows/recordChatUsage.ts - lib/credits/computeCreditsDeductedCents.ts - lib/credits/estimateModelUsageCost.ts - lib/credits/AvailableModelCost.ts - lib/credits/resolveCostTier.ts - All their test files Net delta: -7 files, +0 new orchestrator function. Plus the atomicity guarantee now applies to research handlers too. TDD: each change went RED → minimum impl → GREEN, with all 3195 tests passing at the end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): mark recordCreditDeduction as 'use step' for workflow runtime Vercel Workflow's build-time detector flagged `nanoid` as a Node.js module that can't run inside the workflow body. Marking recordCreditDeduction as 'use step' moves it into the step runtime where Node modules are allowed. Backwards compatible for the existing research-handler callers (regular API routes) — 'use step' functions execute immediately when called from non-workflow contexts. Also matches open-agents' pattern: their recordWorkflowUsage (which contains the equivalent nanoid call) is a 'use step' function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): collapse inline metadata duck-type (KISS) PR review feedback: the 14-line inline type assertion for `result.responseMessage` was needless boilerplate. Replaced with: 1. Import the existing `AgentMessageMetadata` type (already used by `runAgentStep`'s `messageMetadata` callback — single source of truth for the shape). 2. Hoist a module-level `ZERO_USAGE` default so the fallback when metadata is missing is a named constant, not an inline literal. 3. Cast `result.responseMessage.metadata` once (`as AgentMessageMetadata | undefined`). Net delta: 14 lines → 5 lines inside the workflow body, no behavior change. 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(chat-workflow): auto-commit + push after natural finish (#614) * feat(chat-workflow): auto-commit + push after natural finish (#605) Ports open-agents' auto-commit flow to the chat workflow. When a turn finishes naturally (not `tool-calls`) and the session has both `repo_owner` and `repo_name`, the workflow: 1. `git status --porcelain` to check for changes (hasAutoCommitChanges) 2. Emits `data-commit { status: "pending" }` to the SSE stream so the UI can show a spinner 3. Runs the commit + push (runAutoCommit → performAutoCommit): - `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set - `git add -A` - LLM-generated commit message via `generateText` on `git diff --cached` (falls back to "chore: update repository changes" if the gateway is down or diff is empty) - `git commit -m '<message>'` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin <branch>` 4. Emits `data-commit { status: "success"/"error", url? }` with the resolved commit URL when both committed AND pushed New files (TDD, 33 tests): - `lib/chat/auto-commit/performAutoCommit.ts` (14 tests) — the sandbox.exec orchestration, with granular failure modes so the caller can distinguish "couldn't commit" from "committed but push failed". - `lib/chat/auto-commit/hasAutoCommitChanges.ts` (5 tests) — fast pre-flight, fail-open on errors so runAutoCommit reports the real issue. - `lib/chat/auto-commit/runAutoCommit.ts` (4 tests) — workflow step wrapping performAutoCommit with global error handling. - `lib/chat/auto-commit/buildCommitData.ts` (7 tests) — pure helper shaping the AutoCommitResult into the UIMessageChunk payload (status, commit url with proper URL encoding). - `lib/chat/auto-commit/sendCommitChunk.ts` (3 tests) — workflow step that writes the data-commit chunk into the workflow writable (acquires writer / releases lock). Updated: - `app/lib/workflows/runAgentWorkflow.ts` — auto-commit branch after persistAssistantMessage; wraps VercelState with the `{type: "vercel"}` discriminator before passing to SandboxState consumers. New input fields: `sessionTitle?`, `repoOwner?`, `repoName?`. - `app/lib/workflows/__tests__/runAgentWorkflow.test.ts` — 5 new cases covering the happy path, no-changes skip, missing repo identifiers, finish reason 'tool-calls' (no auto-commit on intermediate turns), and the error path. - `lib/chat/handleChatWorkflowStream.ts` — threads `session.title`, `session.repo_owner`, `session.repo_name` into the workflow input. KNOWN LIMITATION (separate follow-up): the data-commit chunks are emitted live to the SSE stream but are NOT re-persisted onto the assistant message's `parts`. The chunk disappears on page refresh. The commit itself is permanent on GitHub. Re-persistence requires either: (a) a follow-up persistAssistantMessage call with the updated message, or (b) an updateChatMessageParts helper. TDD discipline: each new file went RED → minimum impl → GREEN. Suite: 3195 → 3233 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): derive repoOwner/repoName from session.clone_url Auto-commit needs the owner + repo name to build the GitHub commit URL and to set the remote auth URL on push. The session table has `repo_owner` / `repo_name` columns but they were never populated, so the workflow was always skipping auto-commit silently. Rather than denormalize the data (populate the columns at write time), treat `clone_url` as canonical and parse it at read time. Single source of truth, no drift risk between columns and the URL. New helper `lib/github/parseGitHubRepoIdentifiers.ts` (8 tests) handles https + ssh shapes, .git suffix, trailing slashes, and the null / non-github cases. `handleChatWorkflowStream` parses `session.clone_url` once and threads `repoOwner` / `repoName` into the workflow input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): persist data-commit chunk onto assistant message Closes the "data-commit chunk disappears on refresh" limitation flagged in the initial PR description. The chunk is now merged into the assistant message's `parts` and re-persisted via a UPDATE-only helper, so the `GitDataPartCard` UI in open-agents renders the "Committed at <sha>" affordance on page load — not just during the live SSE stream. New files: - `lib/chat/upsertAssistantDataPart.ts` (5 tests) — pure helper ported from open-agents `apps/web/app/workflows/chat.ts`. Merges a data-part into a message's `parts` by `{type, id}` (replace if matched, append otherwise). Immutable; doesn't mutate the input. - `lib/supabase/chat_messages/updateChatMessageParts.ts` (3 tests) — UPDATE-only helper that bypasses the `upsertChatMessage(onConflict: "id", ignoreDuplicates: true)` no-op-on-second-call semantics. Keeps the first-insert path's replay-idempotency for `persistAssistantMessage`; this helper is specifically for "the row exists, replace its `parts`". Wired into `runAgentWorkflow`: After the resolved data-commit chunk is emitted to the writable, the chunk is merged into `result.responseMessage.parts` via `upsertAssistantDataPart`, then `updateChatMessageParts` writes the updated `parts` to the DB. Mirrors open-agents' two-persist pattern in `apps/web/app/workflows/chat.ts:didUpdateGitData`. Tests: - 2 new in `runAgentWorkflow.test.ts`: - success path now asserts `updateChatMessageParts` was called with the resolved data-commit part merged into the parts array - no-changes path asserts `updateChatMessageParts` is NOT called - +8 helper tests across the two new files Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-commit): persist whole message + use step for workflow runtime Two bugs from the first persistence attempt, both caught when the data-commit chunk failed to appear on the open-agents UI after refresh: 1. `updateChatMessageParts` wasn't marked `"use step"`. The function calls `supabase.from(...).update(...)` which uses fetch under the hood — forbidden in the workflow body. The call ran silently with no effect. Same failure mode I hit on `recordCreditDeduction.ts`. 2. The workflow was passing `messageWithCommit.parts` (the inner parts array) when `chat_messages.parts` actually stores the WHOLE message object — matching `persistAssistantMessage`'s `parts: message as never` storage convention. Pass the merged message object now. Confirmed via direct DB query that the first attempt didn't write anything (the row still had only the original message-shape from `persistAssistantMessage`'s first call). With the step boundary + correct payload shape, the persistence path now executes and stores the data-commit chunk so the open-agents `GitDataPartCard` can render after refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): address SRP / KISS / OCP review feedback Per @sweetmantech's PR review comments on #614: KISS — Renamed updateChatMessageParts → updateChatMessage and moved the "use step" boundary out of the supabase layer. Supabase wrappers now stay pure; step-bound wrappers live in lib/chat/. - lib/supabase/chat_messages/updateChatMessage.ts (no "use step") - lib/chat/persistAssistantDataPart.ts (new) — "use step" wrapper that internally calls upsertAssistantDataPart (merge) then updateChatMessage (write). Single durable step boundary at the chat-domain layer. SRP — Extracted generateCommitMessage to its own file. Was a private helper inside performAutoCommit.ts; now reusable for alternative commit-message strategies and individually testable. - lib/chat/auto-commit/generateCommitMessage.ts (+6 tests) - performAutoCommit.ts imports it instead of defining inline. OCP — Extracted the ~50-line auto-commit block from runAgentWorkflow into its own file. Workflow body shrinks to a single function call; the auto-commit flow can evolve without touching workflow code. - lib/chat/auto-commit/autoCommitChatTurn.ts (+9 tests covering every gate, the no-changes path, the happy path including pending → resolved chunks + persistence, and the error path). - runAgentWorkflow.ts: ~50 lines → 11-line invocation. - Workflow test pruned: 6 sub-step assertions → 4 wiring assertions (the flow itself is exhaustively tested in autoCommitChatTurn.test.ts). Also flattened UpdateChatMessageResult from a discriminated union to a single interface — same Next.js 16 narrowing issue I hit on DeductCreditsWithAuditResult in #612. Net file count: +5 new files, -0 deletions (renames don't count). Lines moved out of runAgentWorkflow.ts: ~50. Tests: 3233 → 3266 passing (+33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): spread input/result + use gpt-5.4-nano for commit messages Per @sweetmantech's PR review: KISS — `runAgentWorkflow` was enumerating 8 fields when constructing the autoCommitChatTurn call. Switched to `{ ...input, ...result, writable, sandboxState }`. Any future fields added to input or result get forwarded automatically; the workflow body stays tight. Updated the workflow test assertion to `expect.objectContaining(...)` since extra fields from input/result are now passed through. Model — Updated `generateCommitMessage` to use `openai/gpt-5.4-nano` instead of `anthropic/claude-haiku-4.5`. Newer, cheaper, and a better fit for the short-output commit-message task. The prompt is unchanged so behavior should be near-identical. 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(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) * feat(api): migrate GET /api/sessions/[sessionId]/chats/[chatId] from open-agents Returns the chat's persisted UI message stream plus its current streaming state so callers can hydrate / refresh a chat view: { chat: { id, modelId, activeStreamId }, isStreaming, messages } `messages` is the raw `parts` JSON for each `chat_messages` row, ordered by `created_at` then `id`. `isStreaming` is derived from `active_stream_id`. Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when the session or chat is missing (or when the chat lives in a different session); 403 when the session is owned by a different account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): also migrate PATCH + DELETE /api/sessions/[sessionId]/chats/[chatId] PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field required; both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated). DELETE returns `{ success: true }`. Refuses with 400 if the chat is the only one in its session. Both reuse the same auth + session-ownership + chat-belongs-to-session gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`. 22 new vitest cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop redundant updated_at stamp in updateChat The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop conditional spread in patch handler Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop unused payload from delete validator The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): slim get/patch validators to just what handlers use Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): patch handler returns camelCase Chat shape The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): GET handler returns full chat row via toChatResponse Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }` to the full camelCase wire row (sessionId, title, lastAssistantMessageAt, createdAt, updatedAt). Lets a single helper cover both initial render and in-tab refresh on the open-agents side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): reject unknown fields on PATCH session chat (.strict()) Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): drop unsupported message arg to zod .strict() The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. 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(chat-workflow): persist assistant message per step (#603) * feat(chat-workflow): persist assistant message per step Upgrade the success-only persist (#609) to per-step persistence so a stopped or crashed turn keeps the partial reply instead of dropping it. - runAgentStep streams through createUIMessageStream and persists on every onStepFinish/onFinish (toUIMessageStream exposes only onFinish); it still returns the final responseMessage so the credits path (#612) keeps billing from its metadata. - persistAssistantMessage now overwrites the row as it grows (DO UPDATE via the restored upsertChatMessage `update` flag) and bumps last_assistant_message_at/updated_at on every persist, so a partial reply still surfaces as unread. - runAgentWorkflow drops its own persist call (now per-step) and keeps the #612 credit charge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): spread input into runAgentStep (KISS) Per review feedback on PR #603 — drop the manual field-by-field destructure and forward the workflow input object directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com> * feat(sessions): ensure personal repo on POST /api/sessions (#618) * feat(sessions): ensure personal repo on POST /api/sessions When a caller hits POST /api/sessions without a cloneUrl and without an org bound to their auth context, the handler now provisions (or reuses) their personal Recoupable workspace repo at recoupable/<kebab(name)>-<account_id> before returning the session row. This unblocks chat.recoupable.com's Path C cutover (recoupable/app#1748) — previously the chat-side bootstrap had to construct the personal cloneUrl from a client-side display name (e.g. "sweetman.eth" → "sweetman-eth"), which diverged from open-agents' canonical name source (account_info.name with email-local-part fallback) and 502'd at the clone step. Ported from open-agents: - buildPersonalRepoIdentifier, buildPersonalRepoUrl, githubOwner - repositoryExists, createRepository (plain fetch, no Octokit, to match recoup-api's existing lib/github/* style) - ensurePersonalRepo (idempotent check-then-create) - toKebabCase New session-side helper: - resolveSessionCloneUrl picks bodyCloneUrl > org-no-op > ensurePersonalRepo - buildSessionInsertRow takes the resolved cloneUrl as input - createSessionHandler returns 502 if cloneUrl resolution fails Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(sessions): flatten ResolveSessionCloneUrlResult to single interface Next.js 16's next build doesn't narrow the discriminated union { ok: true; cloneUrl } | { ok: false; error } through `if (!result.ok)`, breaking the Vercel preview build with: Type error: Property 'error' does not exist on type 'ResolveSessionCloneUrlResult'. Same compile-only divergence we hit on PR #603 — vitest's tsc is more permissive than next build's. Flatten to a single interface with `cloneUrl: string | null` and `error?: string` so callers read both fields directly after checking `ok`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(repo-naming): unify workspace repos as recoupable/<accountId> Drop the <kebab(name)>-<accountId> slug — account names are mutable, so any URL that embeds the name eventually drifts. The repo URL is now just the account UUID: stable across renames, identical shape for personal and org workspaces, trivial to parse (`recoupable/<uuid>`). Provisioning flow in ensurePersonalRepo: 1. recoupable/<accountId> exists -> return URL (idempotent) 2. legacy <slug>-<accountId> exists (via GitHub search) -> rename to <accountId>. GitHub auto-redirects the old URL forever, so any sessions.clone_url rows that still reference the old name keep working without a DB backfill. 3. nothing exists -> create fresh recoupable/<accountId> extractOrgId regex now accepts both legacy `<slug>-<uuid>` and bare `<uuid>` shapes so old + new clone URLs both parse. scripts/migrate-workspace-repo-names.ts: one-time backfill. Lists all recoupable org repos, finds those matching ^.+-<uuid>$, renames each to just <uuid>. Defaults to dry-run; --apply commits. Idempotent. resolveSessionCloneUrl no longer needs to look up the account row to derive a slug — ensurePersonalRepo only needs the accountId from auth. Deleted unused lib/string/toKebabCase. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review(PR #618): drop owner/token/description/isPrivate params Per Sweet's review comments on createRepository.ts, hard-code the constants that only have one sensible value: - owner = "recoupable" (RECOUPABLE_GITHUB_OWNER) - token = read from env via getServiceGithubToken (no param plumbing) - description = dropped (GitHub doesn't render anything meaningful) - private = false (workspace repos are public) Applied the same simplification consistently to the other three new github helpers so the surface stays symmetric: - repositoryExists, renameRepository, findLegacyAccountRepo all drop owner + token params; each reads the token via getServiceGithubToken and short-circuits when missing. - ensurePersonalRepo no longer threads token/owner through; its call sites simplified to e.g. createRepository({ name }) and repositoryExists({ repo }). - migrate-workspace-repo-names.ts script likewise calls renameRepository({ repo, newName }) — token plumbing removed. Tests updated to mock getServiceGithubToken instead of passing a fake token through the call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: drop runtime legacy-rename branch (script-once is enough) The migration script renames every legacy <slug>-<accountId> repo once before merge, after which no legacy repo can exist for ensurePersonalRepo to find. The runtime self-healing branch was pure YAGNI. Removed: - lib/github/findLegacyAccountRepo.ts (only caller was the runtime-rename branch) - lib/github/renameRepository.ts (sole consumer is the migration script; PATCH-rename inlined there) - the legacy-rename branch + its tests in ensurePersonalRepo ensurePersonalRepo is now just exists -> create. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: inline repo-name + URL into ensurePersonalRepo Per Sweet's review on buildPersonalRepoIdentifier.ts: post-refactor the helpers hide nothing — the repo name IS the accountId, the URL is one string concat. ensurePersonalRepo was their only runtime caller. Removed: - lib/recoupable/buildPersonalRepoIdentifier.ts (+ test) - lib/recoupable/buildPersonalRepoUrl.ts ensurePersonalRepo now derives the two values inline at the top of the function. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: ensurePersonalRepo now returns just the clone URL string Per Sweet's review on EnsurePersonalRepoResult: the only caller (resolveSessionCloneUrl) reads `cloneUrl` and nothing else. The other three fields (repoUrl, owner, repoName) were written into the response but never consumed. Drop the interface; return Promise<string | null>. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * unify: provision org workspace repo too (cloneUrl: null branch fix) Per Sweet's review on resolveSessionCloneUrl.ts:45 — the cloneUrl: null early-return for auth.orgId was leftover hedging that contradicts the unified recoupable/<accountId> design. Organizations ARE accounts in the data model (account_organization_ids.organization joins the accounts table), so auth.orgId is itself an account_id. The fix: drop the null-return branch and always call ensurePersonalRepo, keyed on auth.orgId ?? auth.accountId. Personal and org sessions now provision the same way — at recoupable/<accountId>, where accountId is either the user's or the org's. Error message updated from "personal repository" to "workspace repository" to reflect the unified naming. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(migrate): match empty-slug -<uuid> repos too The regex used .+ for the slug, requiring at least 1 char before the dash. Accounts that had no display name at repo-creation time produced -<uuid> names (literal leading dash, empty kebab), and the first migration run skipped those as "non-workspace". 6 leading-dash repos turned up in the recoupable org after the first apply pass — 5 of them collided with already-renamed siblings (their losers were deleted manually); 1 was free and renamed. Changed .+ to .* so future runs of this script catch empty-slug names. Bare <uuid> names still don't match (no separator before the UUID). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * revert: createRepository back to private: true Smoke test of the api PR's preview surfaced an inconsistency — the 153 legacy workspace repos in the recoupable org are all private (created by old open-agents code with private: true), but my earlier review-feedback change set new repos to public. Per Sweet's follow-up, flip back to private so the entire fleet stays uniform. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: drop migration script (already applied to prod GitHub) scripts/migrate-workspace-repo-names.ts was a one-time backfill — ran against the recoupable org on 2026-05-26, renamed every legacy <slug>-<accountId> workspace repo to bare <accountId>, then verified zero pending via final dry-run. Keeping it in the codebase forever would be dead weight (per the same KISS principle we applied to findLegacyAccountRepo + renameRepository). Updated the ensurePersonalRepo docstring to reflect that the migration is historical, not an ongoing reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: slim CreateRepositoryResult to {success, repoUrl, error} Per Sweet's review — cloneUrl, owner, repoName are all derivable from repoUrl (cloneUrl = repoUrl + ".git" which git also accepts as repoUrl; owner = "recoupable"; repoName = trailing path segment). Dropped them from the interface and the parse + return shape. ensurePersonalRepo now consumes created.repoUrl directly. The existing-repo branch already returned repoUrl, so the function is now fully consistent on the no-".git" URL form. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Arpit Gupta <arpitgupta1214@gmail.com>
sweetmantech added a commit
that referenced
this pull request
May 26, 2026
* fix(chat-workflow): persist the assistant message after a successful run (#609) * fix(chat-workflow): persist the assistant message after a successful run Closes the silent-data-loss gap that the open-agents → recoup-api cutover introduced: the chat workflow streamed the final assistant message to the client over SSE but never wrote it to `chat_messages`, so a page refresh after a successful exchange wiped the reply. Changes: - New `lib/chat/persistAssistantMessage.ts` step (mirrors open-agents' `app/workflows/chat-post-finish.ts` helper of the same name). Fire-and-forget upsert + chat `updated_at` touch on fresh inserts; idempotent on workflow replay; never throws. - `runAgentStep` now wires an `onFinish` callback into `toUIMessageStream` to capture the assembled assistant message, and returns it alongside `finishReason` as part of the new `RunAgentStepResult` type. - `runAgentWorkflow` calls `persistAssistantMessage(chatId, responseMessage)` after a successful `runAgentStep` (in the try block, BEFORE the existing `clearChatActiveStream` + `closeChatStream` finally). On throw, no message is persisted (nothing was generated); cleanup still runs. Tests: - `persistAssistantMessage.test.ts` — 6 cases (insert + touch, duplicate skip, wrong-role guard, DB-error swallow, exception swallow, role assertion). - `runAgentStep.test.ts` — 3 new cases (onFinish wired, captured responseMessage returned, undefined when onFinish never fires). - `runAgentWorkflow.test.ts` — 3 new cases (persist called on success, not called when responseMessage undefined, not called on throw while cleanup still runs). Full suite: 3159 → 3171 passing. Tracking: #605 (Tier 1, item 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record<string, unknown>` intersection on the outer shape required an index signature that `UIMessage` from `ai` doesn't carry, so wiring runAgentStep's UIMessage return into persistAssistantMessage failed the Vercel build with TS2345. Switched to a minimal duck-typed shape (id/role/parts) — matches both UIMessage and the in-test fixtures structurally. The `chat_messages.parts` column is jsonb so persistence doesn't care about the part subtypes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): mark persistAssistantMessage as a "use step" Vercel Workflow blocks `fetch()` in workflow-body code; the Supabase JS client uses fetch under the hood, so `upsertChatMessage` inside `persistAssistantMessage` failed at runtime with: Global "fetch" is unavailable in workflow functions. Use the "fetch" step function from "workflow" to make HTTP requests. `"use step"` directive moves the function into step-context where fetch is legal. Mirrors open-agents' `persistAssistantMessage` step in `app/workflows/chat-post-finish.ts` (which carries the same directive). Caught via runtime log inspection on the PR preview before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * debug(chat-workflow): TEMP diagnostic logs in persistAssistantMessage Hard-refresh of a chat that ran on PR #609's preview showed the assistant message NOT in chat_messages — meaning silence in the existing error log is NOT the same as "row was written." Adding explicit logs at entry, after upsert, and after updateChat so the runtime tail can disambiguate: - "skip: not assistant role" branch - upsert result shape (ok / isDuplicate / rowPresent) - "persisted + touched chat" success line Will be reverted before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): pass generateMessageId to toUIMessageStream Diagnostic logs revealed every assistant message was arriving at persistAssistantMessage with `messageId: ''` — the AI SDK's default when `generateMessageId` isn't provided. Supabase's `chat_messages.id` PK then treated every workflow run after the first as a duplicate (`onConflict: "id", ignoreDuplicates: true` → isDuplicate: true, rowPresent: false) so no assistant row landed. Generating a stable id once per `runAgentStep` invocation via `generateId()` from `ai`, then plumbing it into `result.toUIMessageStream({ generateMessageId: () => ... })` so: - the streamed chunks carry the id (existing wire format), - `onFinish.responseMessage.id` carries the id, - `persistAssistantMessage` sees a real id and the upsert lands a fresh row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat-workflow): move assistantMessageId generation to workflow body Match open-agents' structural pattern instead of generating the id inline inside runAgentStep. Rationale (which I should have applied the first time, per review feedback): 1. **Multi-step support** — when the Tier 2 outer loop lands, each runAgentStep call needs the SAME assistantMessageId so chunks accumulate under one chat_messages row instead of fragmenting per tool-call iteration. Generating inside the step gives every call a fresh id; generating in the workflow body and threading through makes the upgrade path one-line. 2. **Resume-after-tool-call** — open-agents reuses the latest message's id when `latestMessage.role === "assistant"` so the in-progress assistant turn re-attaches instead of starting a new row. Ported now to avoid a future surprise. 3. **Determinism** — `generateId()` is non-deterministic; the workflow body's WDK constraint forbids that. Wrapping it in a `"use step"` (`generateAssistantMessageId.ts`) makes the value durable across workflow replays. Changes: - New `app/lib/workflows/generateAssistantMessageId.ts` step (mirrors open-agents' local `generateId` step in `apps/web/app/workflows/chat.ts`). - `RunAgentStepInput` gains `assistantMessageId: string`. The inline `generateId()` call is removed. - `runAgentWorkflow` reads `latestMessage`; reuses its id when it's an assistant message, otherwise awaits the step. Threads the result into `runAgentStep`. - Tests: 2 new for the step, 1 new for runAgentStep forwarding, 2 new for the resume-aware branch in runAgentWorkflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(chat-workflow): revert temp diagnostic logs in persistAssistantMessage Logs served their purpose — surfaced the empty-messageId bug (fixed in 8974a37 by threading a workflow-generated id through toUIMessageStream's generateMessageId). UI verification on the PR preview confirmed the assistant row now persists. Reverting the debug logs so production runtime stays quiet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): bump last_assistant_message_at on persist (unread badge parity) Match open-agents' `updateChatAssistantActivity` which sets BOTH `updated_at` and `last_assistant_message_at` to the same timestamp. The recoup-api sidebar's `hasUnread` badge is computed in `lib/sessions/chats/getChatSummaries.ts` as `lastAssistantMessageAt > lastReadAt`, mirroring open-agents' identical query in `apps/web/lib/db/sessions.ts:201`. Without this column bump, an assistant message persisted by the workflow streams to the client, lands in `chat_messages`, but never lights up the unread badge for any other tabs/devices the user has open. The column already exists in `api`'s `chats` schema and `updateChat` already accepts it via `ChatMutableFields` — this is purely a "we forgot to set it" fix. Added two new unit tests: - bumps `last_assistant_message_at` on fresh insert - uses the same timestamp for both columns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: format-fix workflow files (prettier --write) Resolves the format/lint CI failures on df312db — purely whitespace collapsing per the repo's prettier config (no behavior change). 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(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) * feat(credits): port computeCreditsDeductedCents + estimateModelUsageCost from open-agents First piece of the chat-workflow billing path. Ports the per-turn cost math from open-agents' `apps/web/lib/credits/compute-credits-deducted-cents.ts` and `apps/web/lib/models.ts:estimateModelUsageCost` so the same billing logic runs on both sides during the cutover. Resolution order matches open-agents exactly: 1. gateway-reported cost on responseMessage.metadata.totalMessageCost (the same number the chat UI shows next to the response) 2. token-based estimate against the model catalog's cost entry 3. 1c floor when no pricing is available — so a successful turn never lands as a free run Three new files (per api's one-exported-function-per-file SRP): - AvailableModelCost.ts — shape mirroring open-agents' richer cost type (input, output, cache_read, context_over_200k) so the same estimator runs against either catalog - estimateModelUsageCost.ts — token-based USD estimator including the 200k+ context tier swap and cache_read pricing - computeCreditsDeductedCents.ts — top-level orchestrator (gateway cost → token estimate → 1c floor) using api's getAvailableModels directly (no HTTP self-fetch like open-agents does) Test coverage: 27 new unit tests across the two test files. All pricing edge cases covered (NaN/Infinity/negative gateway cost, cached-tokens- exceeding-input clamping, context_over_200k tier swap with partial overrides, catalog miss / fetch failure fallbacks). Unblocks step 3 (deductCreditsWithAudit TS wrapper) of the chat credits gap in #605. Full suite: 3191 → 3205 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(credits): charge credits per chat turn (atomic wallet debit + audit) Closes the silent revenue-loss gap tracked in #605: every successful chat workflow turn now debits the account's wallet AND records a usage_events audit row, in a single atomic transaction. End-to-end flow: 1. runAgentStep's onFinish captures responseMessage.metadata ({totalMessageCost, totalMessageUsage}) — same number the chat UI shows next to the response. 2. runAgentWorkflow calls recordChatUsage(accountId, modelId, message) after persistAssistantMessage. 3. recordChatUsage → computeCreditsDeductedCents (gateway cost OR token estimate OR 1c floor) → deductCreditsWithAudit (supabase.rpc'deduct_credits_with_audit'). 4. The Postgres function (recoupable/database#26) runs the wallet UPDATE and the usage_events INSERT in one implicit transaction — either both land or neither does. Matches open-agents' db.transaction(...) atomicity guarantee. Threads accountId through RunAgentWorkflowInput from validateChatWorkflow (auth-derived; never trusted from the request body). New files: - lib/supabase/credits_usage/deductCreditsWithAudit.ts (+ tests) Thin supabase.rpc wrapper; fire-and-forget (returns ok/error instead of throwing). Lives in lib/supabase/ per CLAUDE.md SRP. - app/lib/workflows/recordChatUsage.ts (+ tests) "use step" function that ties the two together with entry/skip/ success/error logs and graceful handling of missing metadata, catalog failures, and RPC errors. Updated: - app/lib/workflows/runAgentWorkflow.ts + accountId field on RunAgentWorkflowInput + recordChatUsage call after successful persistAssistantMessage - lib/chat/handleChatWorkflowStream.ts + passes validated.accountId into start(runAgentWorkflow, ...) - app/lib/workflows/__tests__/runAgentWorkflow.test.ts + 3 new tests (records on success, skips when no responseMessage, skips when runAgentStep throws) TDD: each new file went red → minimal impl → green. Suite: 3205 → 3220 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): use flat interface for DeductCreditsWithAuditResult Next.js 16's type checker wasn't narrowing the discriminated union `{ ok: true } | { ok: false; error: string }` through `if (!result.ok)`, breaking the production build at `recordChatUsage.ts:90`. Vitest's own type config tolerated it, so this only surfaced on the preview deploy. Flat interface with optional `error?: string` avoids the narrowing requirement entirely — caller can read `result.error` directly when `result.ok` is false. Slight type-safety loss (compiler doesn't enforce that `error` is present when ok is false) is worth the build stability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): regenerate supabase RPC type for deduct_credits_with_audit The previous deploy failed because: 1. `types/database.types.ts` was stale — it didn't include the `deduct_credits_with_audit` RPC that landed in recoupable/database#26 (and was manually applied via the MCP after Supabase's GitHub App 502'd post-merge). Without that entry, `supabase.rpc("deduct_credits_with_audit", ...)` failed Next.js's stricter type check. 2. Even with the entry, the typed `Args.p_event: Json` couldn't accept our `DeductCreditsAuditEvent` interface directly — TS doesn't infer interface → index-signature assignment. Fixes: - Added the `deduct_credits_with_audit` entry to the Functions block of types/database.types.ts (matches the upstream regen via mcp__plugin_supabase_supabase__generate_typescript_types). - Cast `params.event as unknown as Json` at the supabase boundary in deductCreditsWithAudit.ts. The runtime payload is unchanged and the interface keeps its strong typing for callers. Verified locally: `pnpm exec tsc --noEmit` shows no errors in any file this PR touches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(credits): consolidate chat-workflow billing into handleChatCredits (DRY) Addresses the user's PR review: my new files duplicated existing infrastructure. Consolidates everything into the existing pattern (handleChatCredits → getCreditUsage + recordCreditDeduction) so chat workflow billing uses the SAME orchestrator that the streaming chat path (handleChatStream) already uses. Changes: 1. lib/credits/getCreditUsage.ts - Added optional `gatewayCostUsd?: number` parameter - When positive, returns it directly (skips catalog lookup) - Otherwise existing token-math path is unchanged (backwards compat) 2. lib/credits/handleChatCredits.ts - Added `gatewayCostUsd?: number` (threaded to getCreditUsage) - Added `source?: "web" | "api"` (defaults to "web" for backwards compat; chat workflow passes "api" so admin dashboards can distinguish surface in spend rollups) 3. lib/credits/recordCreditDeduction.ts - Switched from `deductCredits + insertUsageEvent` (two separate Supabase calls, non-atomic — could leave wallet/meter drifted on partial failure) to the single `deduct_credits_with_audit` RPC. - Now atomic for ALL callers (chat workflow + research handlers), not just the new chat-workflow path. - Return shape simplified: `{ success: boolean }` instead of `{ success, newBalance }` (no caller was reading newBalance). 4. app/lib/workflows/runAgentWorkflow.ts - Imports handleChatCredits instead of recordChatUsage. - Reads gatewayCostUsd + token counts from responseMessage.metadata.{totalMessageCost, totalMessageUsage}. 5. Deleted (consolidated into existing infrastructure): - app/lib/workflows/recordChatUsage.ts - lib/credits/computeCreditsDeductedCents.ts - lib/credits/estimateModelUsageCost.ts - lib/credits/AvailableModelCost.ts - lib/credits/resolveCostTier.ts - All their test files Net delta: -7 files, +0 new orchestrator function. Plus the atomicity guarantee now applies to research handlers too. TDD: each change went RED → minimum impl → GREEN, with all 3195 tests passing at the end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): mark recordCreditDeduction as 'use step' for workflow runtime Vercel Workflow's build-time detector flagged `nanoid` as a Node.js module that can't run inside the workflow body. Marking recordCreditDeduction as 'use step' moves it into the step runtime where Node modules are allowed. Backwards compatible for the existing research-handler callers (regular API routes) — 'use step' functions execute immediately when called from non-workflow contexts. Also matches open-agents' pattern: their recordWorkflowUsage (which contains the equivalent nanoid call) is a 'use step' function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): collapse inline metadata duck-type (KISS) PR review feedback: the 14-line inline type assertion for `result.responseMessage` was needless boilerplate. Replaced with: 1. Import the existing `AgentMessageMetadata` type (already used by `runAgentStep`'s `messageMetadata` callback — single source of truth for the shape). 2. Hoist a module-level `ZERO_USAGE` default so the fallback when metadata is missing is a named constant, not an inline literal. 3. Cast `result.responseMessage.metadata` once (`as AgentMessageMetadata | undefined`). Net delta: 14 lines → 5 lines inside the workflow body, no behavior change. 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(chat-workflow): auto-commit + push after natural finish (#614) * feat(chat-workflow): auto-commit + push after natural finish (#605) Ports open-agents' auto-commit flow to the chat workflow. When a turn finishes naturally (not `tool-calls`) and the session has both `repo_owner` and `repo_name`, the workflow: 1. `git status --porcelain` to check for changes (hasAutoCommitChanges) 2. Emits `data-commit { status: "pending" }` to the SSE stream so the UI can show a spinner 3. Runs the commit + push (runAutoCommit → performAutoCommit): - `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set - `git add -A` - LLM-generated commit message via `generateText` on `git diff --cached` (falls back to "chore: update repository changes" if the gateway is down or diff is empty) - `git commit -m '<message>'` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin <branch>` 4. Emits `data-commit { status: "success"/"error", url? }` with the resolved commit URL when both committed AND pushed New files (TDD, 33 tests): - `lib/chat/auto-commit/performAutoCommit.ts` (14 tests) — the sandbox.exec orchestration, with granular failure modes so the caller can distinguish "couldn't commit" from "committed but push failed". - `lib/chat/auto-commit/hasAutoCommitChanges.ts` (5 tests) — fast pre-flight, fail-open on errors so runAutoCommit reports the real issue. - `lib/chat/auto-commit/runAutoCommit.ts` (4 tests) — workflow step wrapping performAutoCommit with global error handling. - `lib/chat/auto-commit/buildCommitData.ts` (7 tests) — pure helper shaping the AutoCommitResult into the UIMessageChunk payload (status, commit url with proper URL encoding). - `lib/chat/auto-commit/sendCommitChunk.ts` (3 tests) — workflow step that writes the data-commit chunk into the workflow writable (acquires writer / releases lock). Updated: - `app/lib/workflows/runAgentWorkflow.ts` — auto-commit branch after persistAssistantMessage; wraps VercelState with the `{type: "vercel"}` discriminator before passing to SandboxState consumers. New input fields: `sessionTitle?`, `repoOwner?`, `repoName?`. - `app/lib/workflows/__tests__/runAgentWorkflow.test.ts` — 5 new cases covering the happy path, no-changes skip, missing repo identifiers, finish reason 'tool-calls' (no auto-commit on intermediate turns), and the error path. - `lib/chat/handleChatWorkflowStream.ts` — threads `session.title`, `session.repo_owner`, `session.repo_name` into the workflow input. KNOWN LIMITATION (separate follow-up): the data-commit chunks are emitted live to the SSE stream but are NOT re-persisted onto the assistant message's `parts`. The chunk disappears on page refresh. The commit itself is permanent on GitHub. Re-persistence requires either: (a) a follow-up persistAssistantMessage call with the updated message, or (b) an updateChatMessageParts helper. TDD discipline: each new file went RED → minimum impl → GREEN. Suite: 3195 → 3233 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): derive repoOwner/repoName from session.clone_url Auto-commit needs the owner + repo name to build the GitHub commit URL and to set the remote auth URL on push. The session table has `repo_owner` / `repo_name` columns but they were never populated, so the workflow was always skipping auto-commit silently. Rather than denormalize the data (populate the columns at write time), treat `clone_url` as canonical and parse it at read time. Single source of truth, no drift risk between columns and the URL. New helper `lib/github/parseGitHubRepoIdentifiers.ts` (8 tests) handles https + ssh shapes, .git suffix, trailing slashes, and the null / non-github cases. `handleChatWorkflowStream` parses `session.clone_url` once and threads `repoOwner` / `repoName` into the workflow input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): persist data-commit chunk onto assistant message Closes the "data-commit chunk disappears on refresh" limitation flagged in the initial PR description. The chunk is now merged into the assistant message's `parts` and re-persisted via a UPDATE-only helper, so the `GitDataPartCard` UI in open-agents renders the "Committed at <sha>" affordance on page load — not just during the live SSE stream. New files: - `lib/chat/upsertAssistantDataPart.ts` (5 tests) — pure helper ported from open-agents `apps/web/app/workflows/chat.ts`. Merges a data-part into a message's `parts` by `{type, id}` (replace if matched, append otherwise). Immutable; doesn't mutate the input. - `lib/supabase/chat_messages/updateChatMessageParts.ts` (3 tests) — UPDATE-only helper that bypasses the `upsertChatMessage(onConflict: "id", ignoreDuplicates: true)` no-op-on-second-call semantics. Keeps the first-insert path's replay-idempotency for `persistAssistantMessage`; this helper is specifically for "the row exists, replace its `parts`". Wired into `runAgentWorkflow`: After the resolved data-commit chunk is emitted to the writable, the chunk is merged into `result.responseMessage.parts` via `upsertAssistantDataPart`, then `updateChatMessageParts` writes the updated `parts` to the DB. Mirrors open-agents' two-persist pattern in `apps/web/app/workflows/chat.ts:didUpdateGitData`. Tests: - 2 new in `runAgentWorkflow.test.ts`: - success path now asserts `updateChatMessageParts` was called with the resolved data-commit part merged into the parts array - no-changes path asserts `updateChatMessageParts` is NOT called - +8 helper tests across the two new files Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-commit): persist whole message + use step for workflow runtime Two bugs from the first persistence attempt, both caught when the data-commit chunk failed to appear on the open-agents UI after refresh: 1. `updateChatMessageParts` wasn't marked `"use step"`. The function calls `supabase.from(...).update(...)` which uses fetch under the hood — forbidden in the workflow body. The call ran silently with no effect. Same failure mode I hit on `recordCreditDeduction.ts`. 2. The workflow was passing `messageWithCommit.parts` (the inner parts array) when `chat_messages.parts` actually stores the WHOLE message object — matching `persistAssistantMessage`'s `parts: message as never` storage convention. Pass the merged message object now. Confirmed via direct DB query that the first attempt didn't write anything (the row still had only the original message-shape from `persistAssistantMessage`'s first call). With the step boundary + correct payload shape, the persistence path now executes and stores the data-commit chunk so the open-agents `GitDataPartCard` can render after refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): address SRP / KISS / OCP review feedback Per @sweetmantech's PR review comments on #614: KISS — Renamed updateChatMessageParts → updateChatMessage and moved the "use step" boundary out of the supabase layer. Supabase wrappers now stay pure; step-bound wrappers live in lib/chat/. - lib/supabase/chat_messages/updateChatMessage.ts (no "use step") - lib/chat/persistAssistantDataPart.ts (new) — "use step" wrapper that internally calls upsertAssistantDataPart (merge) then updateChatMessage (write). Single durable step boundary at the chat-domain layer. SRP — Extracted generateCommitMessage to its own file. Was a private helper inside performAutoCommit.ts; now reusable for alternative commit-message strategies and individually testable. - lib/chat/auto-commit/generateCommitMessage.ts (+6 tests) - performAutoCommit.ts imports it instead of defining inline. OCP — Extracted the ~50-line auto-commit block from runAgentWorkflow into its own file. Workflow body shrinks to a single function call; the auto-commit flow can evolve without touching workflow code. - lib/chat/auto-commit/autoCommitChatTurn.ts (+9 tests covering every gate, the no-changes path, the happy path including pending → resolved chunks + persistence, and the error path). - runAgentWorkflow.ts: ~50 lines → 11-line invocation. - Workflow test pruned: 6 sub-step assertions → 4 wiring assertions (the flow itself is exhaustively tested in autoCommitChatTurn.test.ts). Also flattened UpdateChatMessageResult from a discriminated union to a single interface — same Next.js 16 narrowing issue I hit on DeductCreditsWithAuditResult in #612. Net file count: +5 new files, -0 deletions (renames don't count). Lines moved out of runAgentWorkflow.ts: ~50. Tests: 3233 → 3266 passing (+33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): spread input/result + use gpt-5.4-nano for commit messages Per @sweetmantech's PR review: KISS — `runAgentWorkflow` was enumerating 8 fields when constructing the autoCommitChatTurn call. Switched to `{ ...input, ...result, writable, sandboxState }`. Any future fields added to input or result get forwarded automatically; the workflow body stays tight. Updated the workflow test assertion to `expect.objectContaining(...)` since extra fields from input/result are now passed through. Model — Updated `generateCommitMessage` to use `openai/gpt-5.4-nano` instead of `anthropic/claude-haiku-4.5`. Newer, cheaper, and a better fit for the short-output commit-message task. The prompt is unchanged so behavior should be near-identical. 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(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) * feat(api): migrate GET /api/sessions/[sessionId]/chats/[chatId] from open-agents Returns the chat's persisted UI message stream plus its current streaming state so callers can hydrate / refresh a chat view: { chat: { id, modelId, activeStreamId }, isStreaming, messages } `messages` is the raw `parts` JSON for each `chat_messages` row, ordered by `created_at` then `id`. `isStreaming` is derived from `active_stream_id`. Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when the session or chat is missing (or when the chat lives in a different session); 403 when the session is owned by a different account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): also migrate PATCH + DELETE /api/sessions/[sessionId]/chats/[chatId] PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field required; both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated). DELETE returns `{ success: true }`. Refuses with 400 if the chat is the only one in its session. Both reuse the same auth + session-ownership + chat-belongs-to-session gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`. 22 new vitest cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop redundant updated_at stamp in updateChat The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop conditional spread in patch handler Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop unused payload from delete validator The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): slim get/patch validators to just what handlers use Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): patch handler returns camelCase Chat shape The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): GET handler returns full chat row via toChatResponse Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }` to the full camelCase wire row (sessionId, title, lastAssistantMessageAt, createdAt, updatedAt). Lets a single helper cover both initial render and in-tab refresh on the open-agents side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): reject unknown fields on PATCH session chat (.strict()) Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): drop unsupported message arg to zod .strict() The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. 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(chat-workflow): persist assistant message per step (#603) * feat(chat-workflow): persist assistant message per step Upgrade the success-only persist (#609) to per-step persistence so a stopped or crashed turn keeps the partial reply instead of dropping it. - runAgentStep streams through createUIMessageStream and persists on every onStepFinish/onFinish (toUIMessageStream exposes only onFinish); it still returns the final responseMessage so the credits path (#612) keeps billing from its metadata. - persistAssistantMessage now overwrites the row as it grows (DO UPDATE via the restored upsertChatMessage `update` flag) and bumps last_assistant_message_at/updated_at on every persist, so a partial reply still surfaces as unread. - runAgentWorkflow drops its own persist call (now per-step) and keeps the #612 credit charge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): spread input into runAgentStep (KISS) Per review feedback on PR #603 — drop the manual field-by-field destructure and forward the workflow input object directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com> * feat(sessions): ensure personal repo on POST /api/sessions (#618) * feat(sessions): ensure personal repo on POST /api/sessions When a caller hits POST /api/sessions without a cloneUrl and without an org bound to their auth context, the handler now provisions (or reuses) their personal Recoupable workspace repo at recoupable/<kebab(name)>-<account_id> before returning the session row. This unblocks chat.recoupable.com's Path C cutover (recoupable/app#1748) — previously the chat-side bootstrap had to construct the personal cloneUrl from a client-side display name (e.g. "sweetman.eth" → "sweetman-eth"), which diverged from open-agents' canonical name source (account_info.name with email-local-part fallback) and 502'd at the clone step. Ported from open-agents: - buildPersonalRepoIdentifier, buildPersonalRepoUrl, githubOwner - repositoryExists, createRepository (plain fetch, no Octokit, to match recoup-api's existing lib/github/* style) - ensurePersonalRepo (idempotent check-then-create) - toKebabCase New session-side helper: - resolveSessionCloneUrl picks bodyCloneUrl > org-no-op > ensurePersonalRepo - buildSessionInsertRow takes the resolved cloneUrl as input - createSessionHandler returns 502 if cloneUrl resolution fails Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(sessions): flatten ResolveSessionCloneUrlResult to single interface Next.js 16's next build doesn't narrow the discriminated union { ok: true; cloneUrl } | { ok: false; error } through `if (!result.ok)`, breaking the Vercel preview build with: Type error: Property 'error' does not exist on type 'ResolveSessionCloneUrlResult'. Same compile-only divergence we hit on PR #603 — vitest's tsc is more permissive than next build's. Flatten to a single interface with `cloneUrl: string | null` and `error?: string` so callers read both fields directly after checking `ok`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(repo-naming): unify workspace repos as recoupable/<accountId> Drop the <kebab(name)>-<accountId> slug — account names are mutable, so any URL that embeds the name eventually drifts. The repo URL is now just the account UUID: stable across renames, identical shape for personal and org workspaces, trivial to parse (`recoupable/<uuid>`). Provisioning flow in ensurePersonalRepo: 1. recoupable/<accountId> exists -> return URL (idempotent) 2. legacy <slug>-<accountId> exists (via GitHub search) -> rename to <accountId>. GitHub auto-redirects the old URL forever, so any sessions.clone_url rows that still reference the old name keep working without a DB backfill. 3. nothing exists -> create fresh recoupable/<accountId> extractOrgId regex now accepts both legacy `<slug>-<uuid>` and bare `<uuid>` shapes so old + new clone URLs both parse. scripts/migrate-workspace-repo-names.ts: one-time backfill. Lists all recoupable org repos, finds those matching ^.+-<uuid>$, renames each to just <uuid>. Defaults to dry-run; --apply commits. Idempotent. resolveSessionCloneUrl no longer needs to look up the account row to derive a slug — ensurePersonalRepo only needs the accountId from auth. Deleted unused lib/string/toKebabCase. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review(PR #618): drop owner/token/description/isPrivate params Per Sweet's review comments on createRepository.ts, hard-code the constants that only have one sensible value: - owner = "recoupable" (RECOUPABLE_GITHUB_OWNER) - token = read from env via getServiceGithubToken (no param plumbing) - description = dropped (GitHub doesn't render anything meaningful) - private = false (workspace repos are public) Applied the same simplification consistently to the other three new github helpers so the surface stays symmetric: - repositoryExists, renameRepository, findLegacyAccountRepo all drop owner + token params; each reads the token via getServiceGithubToken and short-circuits when missing. - ensurePersonalRepo no longer threads token/owner through; its call sites simplified to e.g. createRepository({ name }) and repositoryExists({ repo }). - migrate-workspace-repo-names.ts script likewise calls renameRepository({ repo, newName }) — token plumbing removed. Tests updated to mock getServiceGithubToken instead of passing a fake token through the call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: drop runtime legacy-rename branch (script-once is enough) The migration script renames every legacy <slug>-<accountId> repo once before merge, after which no legacy repo can exist for ensurePersonalRepo to find. The runtime self-healing branch was pure YAGNI. Removed: - lib/github/findLegacyAccountRepo.ts (only caller was the runtime-rename branch) - lib/github/renameRepository.ts (sole consumer is the migration script; PATCH-rename inlined there) - the legacy-rename branch + its tests in ensurePersonalRepo ensurePersonalRepo is now just exists -> create. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: inline repo-name + URL into ensurePersonalRepo Per Sweet's review on buildPersonalRepoIdentifier.ts: post-refactor the helpers hide nothing — the repo name IS the accountId, the URL is one string concat. ensurePersonalRepo was their only runtime caller. Removed: - lib/recoupable/buildPersonalRepoIdentifier.ts (+ test) - lib/recoupable/buildPersonalRepoUrl.ts ensurePersonalRepo now derives the two values inline at the top of the function. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: ensurePersonalRepo now returns just the clone URL string Per Sweet's review on EnsurePersonalRepoResult: the only caller (resolveSessionCloneUrl) reads `cloneUrl` and nothing else. The other three fields (repoUrl, owner, repoName) were written into the response but never consumed. Drop the interface; return Promise<string | null>. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * unify: provision org workspace repo too (cloneUrl: null branch fix) Per Sweet's review on resolveSessionCloneUrl.ts:45 — the cloneUrl: null early-return for auth.orgId was leftover hedging that contradicts the unified recoupable/<accountId> design. Organizations ARE accounts in the data model (account_organization_ids.organization joins the accounts table), so auth.orgId is itself an account_id. The fix: drop the null-return branch and always call ensurePersonalRepo, keyed on auth.orgId ?? auth.accountId. Personal and org sessions now provision the same way — at recoupable/<accountId>, where accountId is either the user's or the org's. Error message updated from "personal repository" to "workspace repository" to reflect the unified naming. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(migrate): match empty-slug -<uuid> repos too The regex used .+ for the slug, requiring at least 1 char before the dash. Accounts that had no display name at repo-creation time produced -<uuid> names (literal leading dash, empty kebab), and the first migration run skipped those as "non-workspace". 6 leading-dash repos turned up in the recoupable org after the first apply pass — 5 of them collided with already-renamed siblings (their losers were deleted manually); 1 was free and renamed. Changed .+ to .* so future runs of this script catch empty-slug names. Bare <uuid> names still don't match (no separator before the UUID). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * revert: createRepository back to private: true Smoke test of the api PR's preview surfaced an inconsistency — the 153 legacy workspace repos in the recoupable org are all private (created by old open-agents code with private: true), but my earlier review-feedback change set new repos to public. Per Sweet's follow-up, flip back to private so the entire fleet stays uniform. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: drop migration script (already applied to prod GitHub) scripts/migrate-workspace-repo-names.ts was a one-time backfill — ran against the recoupable org on 2026-05-26, renamed every legacy <slug>-<accountId> workspace repo to bare <accountId>, then verified zero pending via final dry-run. Keeping it in the codebase forever would be dead weight (per the same KISS principle we applied to findLegacyAccountRepo + renameRepository). Updated the ensurePersonalRepo docstring to reflect that the migration is historical, not an ongoing reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: slim CreateRepositoryResult to {success, repoUrl, error} Per Sweet's review — cloneUrl, owner, repoName are all derivable from repoUrl (cloneUrl = repoUrl + ".git" which git also accepts as repoUrl; owner = "recoupable"; repoName = trailing path segment). Dropped them from the interface and the parse + return shape. ensurePersonalRepo now consumes created.repoUrl directly. The existing-repo branch already returned repoUrl, so the function is now fully consistent on the no-".git" URL form. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * feat(sessions): replace body cloneUrl+branch with organizationId (#620) * feat(sessions): replace body cloneUrl+branch with organizationId POST /api/sessions's request schema is now {title?, organizationId?, sandboxType?}. Removed: - cloneUrl: api derives the workspace repo URL itself via ensurePersonalRepo (recoupable/<accountId> for personal, recoupable/<organizationId> for org sessions) - branch: was only ever piped to sessions.branch column; sandbox handler already falls back to repo default when null - repoOwner / repoName / isNewBranch: never accepted (Zod silently dropped them); pure docs drift, removed from the OpenAPI spec in recoupable/docs#226 Added: - organizationId: optional uuid; when present, validated by validateAuthContext's existing input.organizationId path and sets auth.orgId. resolveSessionCloneUrl's old `auth.orgId ?? auth.accountId` logic now derives the workspace owner from the request alone. Inlined resolveSessionCloneUrl into createSessionHandler — it was a thin wrapper around ensurePersonalRepo once bodyCloneUrl support was removed. Deleted the file and its test. Tests: - buildSessionInsertRow takes a non-null cloneUrl now (always set on session create); branch column is hard-coded null in the row. - createSessionHandler.persistence covers both personal and org branches (ensurePersonalRepo called with auth.accountId vs auth.orgId respectively) plus the 502 path when ensure fails. - validateCreateSessionBody covers the new organizationId uuid validation + forwarding to validateAuthContext. 3,322 / 3,322 tests pass; `next build` TS phase clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(sessions): drop sandboxType too — only vercel is supported z.literal("vercel") with hard-coded SandboxState as { type: "vercel" } & VercelState meant the body field had exactly one acceptable value and the column always got the same value regardless. Pure YAGNI. Also drops `body: CreateSessionBody` from buildSessionInsertRow's input — nothing in the body shape is read there anymore (title is resolved upstream, cloneUrl is passed in, sandboxType is gone). Final POST /api/sessions request shape: { title?, organizationId? }. Pairs with recoupable/docs#226 (also amended to drop sandboxType). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Arpit Gupta <arpitgupta1214@gmail.com>
sweetmantech added a commit
that referenced
this pull request
May 28, 2026
…622) * fix(chat-workflow): persist the assistant message after a successful run (#609) * fix(chat-workflow): persist the assistant message after a successful run Closes the silent-data-loss gap that the open-agents → recoup-api cutover introduced: the chat workflow streamed the final assistant message to the client over SSE but never wrote it to `chat_messages`, so a page refresh after a successful exchange wiped the reply. Changes: - New `lib/chat/persistAssistantMessage.ts` step (mirrors open-agents' `app/workflows/chat-post-finish.ts` helper of the same name). Fire-and-forget upsert + chat `updated_at` touch on fresh inserts; idempotent on workflow replay; never throws. - `runAgentStep` now wires an `onFinish` callback into `toUIMessageStream` to capture the assembled assistant message, and returns it alongside `finishReason` as part of the new `RunAgentStepResult` type. - `runAgentWorkflow` calls `persistAssistantMessage(chatId, responseMessage)` after a successful `runAgentStep` (in the try block, BEFORE the existing `clearChatActiveStream` + `closeChatStream` finally). On throw, no message is persisted (nothing was generated); cleanup still runs. Tests: - `persistAssistantMessage.test.ts` — 6 cases (insert + touch, duplicate skip, wrong-role guard, DB-error swallow, exception swallow, role assertion). - `runAgentStep.test.ts` — 3 new cases (onFinish wired, captured responseMessage returned, undefined when onFinish never fires). - `runAgentWorkflow.test.ts` — 3 new cases (persist called on success, not called when responseMessage undefined, not called on throw while cleanup still runs). Full suite: 3159 → 3171 passing. Tracking: #605 (Tier 1, item 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record<string, unknown>` intersection on the outer shape required an index signature that `UIMessage` from `ai` doesn't carry, so wiring runAgentStep's UIMessage return into persistAssistantMessage failed the Vercel build with TS2345. Switched to a minimal duck-typed shape (id/role/parts) — matches both UIMessage and the in-test fixtures structurally. The `chat_messages.parts` column is jsonb so persistence doesn't care about the part subtypes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): mark persistAssistantMessage as a "use step" Vercel Workflow blocks `fetch()` in workflow-body code; the Supabase JS client uses fetch under the hood, so `upsertChatMessage` inside `persistAssistantMessage` failed at runtime with: Global "fetch" is unavailable in workflow functions. Use the "fetch" step function from "workflow" to make HTTP requests. `"use step"` directive moves the function into step-context where fetch is legal. Mirrors open-agents' `persistAssistantMessage` step in `app/workflows/chat-post-finish.ts` (which carries the same directive). Caught via runtime log inspection on the PR preview before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * debug(chat-workflow): TEMP diagnostic logs in persistAssistantMessage Hard-refresh of a chat that ran on PR #609's preview showed the assistant message NOT in chat_messages — meaning silence in the existing error log is NOT the same as "row was written." Adding explicit logs at entry, after upsert, and after updateChat so the runtime tail can disambiguate: - "skip: not assistant role" branch - upsert result shape (ok / isDuplicate / rowPresent) - "persisted + touched chat" success line Will be reverted before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): pass generateMessageId to toUIMessageStream Diagnostic logs revealed every assistant message was arriving at persistAssistantMessage with `messageId: ''` — the AI SDK's default when `generateMessageId` isn't provided. Supabase's `chat_messages.id` PK then treated every workflow run after the first as a duplicate (`onConflict: "id", ignoreDuplicates: true` → isDuplicate: true, rowPresent: false) so no assistant row landed. Generating a stable id once per `runAgentStep` invocation via `generateId()` from `ai`, then plumbing it into `result.toUIMessageStream({ generateMessageId: () => ... })` so: - the streamed chunks carry the id (existing wire format), - `onFinish.responseMessage.id` carries the id, - `persistAssistantMessage` sees a real id and the upsert lands a fresh row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat-workflow): move assistantMessageId generation to workflow body Match open-agents' structural pattern instead of generating the id inline inside runAgentStep. Rationale (which I should have applied the first time, per review feedback): 1. **Multi-step support** — when the Tier 2 outer loop lands, each runAgentStep call needs the SAME assistantMessageId so chunks accumulate under one chat_messages row instead of fragmenting per tool-call iteration. Generating inside the step gives every call a fresh id; generating in the workflow body and threading through makes the upgrade path one-line. 2. **Resume-after-tool-call** — open-agents reuses the latest message's id when `latestMessage.role === "assistant"` so the in-progress assistant turn re-attaches instead of starting a new row. Ported now to avoid a future surprise. 3. **Determinism** — `generateId()` is non-deterministic; the workflow body's WDK constraint forbids that. Wrapping it in a `"use step"` (`generateAssistantMessageId.ts`) makes the value durable across workflow replays. Changes: - New `app/lib/workflows/generateAssistantMessageId.ts` step (mirrors open-agents' local `generateId` step in `apps/web/app/workflows/chat.ts`). - `RunAgentStepInput` gains `assistantMessageId: string`. The inline `generateId()` call is removed. - `runAgentWorkflow` reads `latestMessage`; reuses its id when it's an assistant message, otherwise awaits the step. Threads the result into `runAgentStep`. - Tests: 2 new for the step, 1 new for runAgentStep forwarding, 2 new for the resume-aware branch in runAgentWorkflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(chat-workflow): revert temp diagnostic logs in persistAssistantMessage Logs served their purpose — surfaced the empty-messageId bug (fixed in 8974a37e by threading a workflow-generated id through toUIMessageStream's generateMessageId). UI verification on the PR preview confirmed the assistant row now persists. Reverting the debug logs so production runtime stays quiet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat-workflow): bump last_assistant_message_at on persist (unread badge parity) Match open-agents' `updateChatAssistantActivity` which sets BOTH `updated_at` and `last_assistant_message_at` to the same timestamp. The recoup-api sidebar's `hasUnread` badge is computed in `lib/sessions/chats/getChatSummaries.ts` as `lastAssistantMessageAt > lastReadAt`, mirroring open-agents' identical query in `apps/web/lib/db/sessions.ts:201`. Without this column bump, an assistant message persisted by the workflow streams to the client, lands in `chat_messages`, but never lights up the unread badge for any other tabs/devices the user has open. The column already exists in `api`'s `chats` schema and `updateChat` already accepts it via `ChatMutableFields` — this is purely a "we forgot to set it" fix. Added two new unit tests: - bumps `last_assistant_message_at` on fresh insert - uses the same timestamp for both columns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: format-fix workflow files (prettier --write) Resolves the format/lint CI failures on df312dbc — purely whitespace collapsing per the repo's prettier config (no behavior change). 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(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) * feat(credits): port computeCreditsDeductedCents + estimateModelUsageCost from open-agents First piece of the chat-workflow billing path. Ports the per-turn cost math from open-agents' `apps/web/lib/credits/compute-credits-deducted-cents.ts` and `apps/web/lib/models.ts:estimateModelUsageCost` so the same billing logic runs on both sides during the cutover. Resolution order matches open-agents exactly: 1. gateway-reported cost on responseMessage.metadata.totalMessageCost (the same number the chat UI shows next to the response) 2. token-based estimate against the model catalog's cost entry 3. 1c floor when no pricing is available — so a successful turn never lands as a free run Three new files (per api's one-exported-function-per-file SRP): - AvailableModelCost.ts — shape mirroring open-agents' richer cost type (input, output, cache_read, context_over_200k) so the same estimator runs against either catalog - estimateModelUsageCost.ts — token-based USD estimator including the 200k+ context tier swap and cache_read pricing - computeCreditsDeductedCents.ts — top-level orchestrator (gateway cost → token estimate → 1c floor) using api's getAvailableModels directly (no HTTP self-fetch like open-agents does) Test coverage: 27 new unit tests across the two test files. All pricing edge cases covered (NaN/Infinity/negative gateway cost, cached-tokens- exceeding-input clamping, context_over_200k tier swap with partial overrides, catalog miss / fetch failure fallbacks). Unblocks step 3 (deductCreditsWithAudit TS wrapper) of the chat credits gap in recoupable/api#605. Full suite: 3191 → 3205 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(credits): charge credits per chat turn (atomic wallet debit + audit) Closes the silent revenue-loss gap tracked in #605: every successful chat workflow turn now debits the account's wallet AND records a usage_events audit row, in a single atomic transaction. End-to-end flow: 1. runAgentStep's onFinish captures responseMessage.metadata ({totalMessageCost, totalMessageUsage}) — same number the chat UI shows next to the response. 2. runAgentWorkflow calls recordChatUsage(accountId, modelId, message) after persistAssistantMessage. 3. recordChatUsage → computeCreditsDeductedCents (gateway cost OR token estimate OR 1c floor) → deductCreditsWithAudit (supabase.rpc'deduct_credits_with_audit'). 4. The Postgres function (recoupable/database#26) runs the wallet UPDATE and the usage_events INSERT in one implicit transaction — either both land or neither does. Matches open-agents' db.transaction(...) atomicity guarantee. Threads accountId through RunAgentWorkflowInput from validateChatWorkflow (auth-derived; never trusted from the request body). New files: - lib/supabase/credits_usage/deductCreditsWithAudit.ts (+ tests) Thin supabase.rpc wrapper; fire-and-forget (returns ok/error instead of throwing). Lives in lib/supabase/ per CLAUDE.md SRP. - app/lib/workflows/recordChatUsage.ts (+ tests) "use step" function that ties the two together with entry/skip/ success/error logs and graceful handling of missing metadata, catalog failures, and RPC errors. Updated: - app/lib/workflows/runAgentWorkflow.ts + accountId field on RunAgentWorkflowInput + recordChatUsage call after successful persistAssistantMessage - lib/chat/handleChatWorkflowStream.ts + passes validated.accountId into start(runAgentWorkflow, ...) - app/lib/workflows/__tests__/runAgentWorkflow.test.ts + 3 new tests (records on success, skips when no responseMessage, skips when runAgentStep throws) TDD: each new file went red → minimal impl → green. Suite: 3205 → 3220 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): use flat interface for DeductCreditsWithAuditResult Next.js 16's type checker wasn't narrowing the discriminated union `{ ok: true } | { ok: false; error: string }` through `if (!result.ok)`, breaking the production build at `recordChatUsage.ts:90`. Vitest's own type config tolerated it, so this only surfaced on the preview deploy. Flat interface with optional `error?: string` avoids the narrowing requirement entirely — caller can read `result.error` directly when `result.ok` is false. Slight type-safety loss (compiler doesn't enforce that `error` is present when ok is false) is worth the build stability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): regenerate supabase RPC type for deduct_credits_with_audit The previous deploy failed because: 1. `types/database.types.ts` was stale — it didn't include the `deduct_credits_with_audit` RPC that landed in recoupable/database#26 (and was manually applied via the MCP after Supabase's GitHub App 502'd post-merge). Without that entry, `supabase.rpc("deduct_credits_with_audit", ...)` failed Next.js's stricter type check. 2. Even with the entry, the typed `Args.p_event: Json` couldn't accept our `DeductCreditsAuditEvent` interface directly — TS doesn't infer interface → index-signature assignment. Fixes: - Added the `deduct_credits_with_audit` entry to the Functions block of types/database.types.ts (matches the upstream regen via mcp__plugin_supabase_supabase__generate_typescript_types). - Cast `params.event as unknown as Json` at the supabase boundary in deductCreditsWithAudit.ts. The runtime payload is unchanged and the interface keeps its strong typing for callers. Verified locally: `pnpm exec tsc --noEmit` shows no errors in any file this PR touches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(credits): consolidate chat-workflow billing into handleChatCredits (DRY) Addresses the user's PR review: my new files duplicated existing infrastructure. Consolidates everything into the existing pattern (handleChatCredits → getCreditUsage + recordCreditDeduction) so chat workflow billing uses the SAME orchestrator that the streaming chat path (handleChatStream) already uses. Changes: 1. lib/credits/getCreditUsage.ts - Added optional `gatewayCostUsd?: number` parameter - When positive, returns it directly (skips catalog lookup) - Otherwise existing token-math path is unchanged (backwards compat) 2. lib/credits/handleChatCredits.ts - Added `gatewayCostUsd?: number` (threaded to getCreditUsage) - Added `source?: "web" | "api"` (defaults to "web" for backwards compat; chat workflow passes "api" so admin dashboards can distinguish surface in spend rollups) 3. lib/credits/recordCreditDeduction.ts - Switched from `deductCredits + insertUsageEvent` (two separate Supabase calls, non-atomic — could leave wallet/meter drifted on partial failure) to the single `deduct_credits_with_audit` RPC. - Now atomic for ALL callers (chat workflow + research handlers), not just the new chat-workflow path. - Return shape simplified: `{ success: boolean }` instead of `{ success, newBalance }` (no caller was reading newBalance). 4. app/lib/workflows/runAgentWorkflow.ts - Imports handleChatCredits instead of recordChatUsage. - Reads gatewayCostUsd + token counts from responseMessage.metadata.{totalMessageCost, totalMessageUsage}. 5. Deleted (consolidated into existing infrastructure): - app/lib/workflows/recordChatUsage.ts - lib/credits/computeCreditsDeductedCents.ts - lib/credits/estimateModelUsageCost.ts - lib/credits/AvailableModelCost.ts - lib/credits/resolveCostTier.ts - All their test files Net delta: -7 files, +0 new orchestrator function. Plus the atomicity guarantee now applies to research handlers too. TDD: each change went RED → minimum impl → GREEN, with all 3195 tests passing at the end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(credits): mark recordCreditDeduction as 'use step' for workflow runtime Vercel Workflow's build-time detector flagged `nanoid` as a Node.js module that can't run inside the workflow body. Marking recordCreditDeduction as 'use step' moves it into the step runtime where Node modules are allowed. Backwards compatible for the existing research-handler callers (regular API routes) — 'use step' functions execute immediately when called from non-workflow contexts. Also matches open-agents' pattern: their recordWorkflowUsage (which contains the equivalent nanoid call) is a 'use step' function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): collapse inline metadata duck-type (KISS) PR review feedback: the 14-line inline type assertion for `result.responseMessage` was needless boilerplate. Replaced with: 1. Import the existing `AgentMessageMetadata` type (already used by `runAgentStep`'s `messageMetadata` callback — single source of truth for the shape). 2. Hoist a module-level `ZERO_USAGE` default so the fallback when metadata is missing is a named constant, not an inline literal. 3. Cast `result.responseMessage.metadata` once (`as AgentMessageMetadata | undefined`). Net delta: 14 lines → 5 lines inside the workflow body, no behavior change. 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(chat-workflow): auto-commit + push after natural finish (#614) * feat(chat-workflow): auto-commit + push after natural finish (#605) Ports open-agents' auto-commit flow to the chat workflow. When a turn finishes naturally (not `tool-calls`) and the session has both `repo_owner` and `repo_name`, the workflow: 1. `git status --porcelain` to check for changes (hasAutoCommitChanges) 2. Emits `data-commit { status: "pending" }` to the SSE stream so the UI can show a spinner 3. Runs the commit + push (runAutoCommit → performAutoCommit): - `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set - `git add -A` - LLM-generated commit message via `generateText` on `git diff --cached` (falls back to "chore: update repository changes" if the gateway is down or diff is empty) - `git commit -m '<message>'` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin <branch>` 4. Emits `data-commit { status: "success"/"error", url? }` with the resolved commit URL when both committed AND pushed New files (TDD, 33 tests): - `lib/chat/auto-commit/performAutoCommit.ts` (14 tests) — the sandbox.exec orchestration, with granular failure modes so the caller can distinguish "couldn't commit" from "committed but push failed". - `lib/chat/auto-commit/hasAutoCommitChanges.ts` (5 tests) — fast pre-flight, fail-open on errors so runAutoCommit reports the real issue. - `lib/chat/auto-commit/runAutoCommit.ts` (4 tests) — workflow step wrapping performAutoCommit with global error handling. - `lib/chat/auto-commit/buildCommitData.ts` (7 tests) — pure helper shaping the AutoCommitResult into the UIMessageChunk payload (status, commit url with proper URL encoding). - `lib/chat/auto-commit/sendCommitChunk.ts` (3 tests) — workflow step that writes the data-commit chunk into the workflow writable (acquires writer / releases lock). Updated: - `app/lib/workflows/runAgentWorkflow.ts` — auto-commit branch after persistAssistantMessage; wraps VercelState with the `{type: "vercel"}` discriminator before passing to SandboxState consumers. New input fields: `sessionTitle?`, `repoOwner?`, `repoName?`. - `app/lib/workflows/__tests__/runAgentWorkflow.test.ts` — 5 new cases covering the happy path, no-changes skip, missing repo identifiers, finish reason 'tool-calls' (no auto-commit on intermediate turns), and the error path. - `lib/chat/handleChatWorkflowStream.ts` — threads `session.title`, `session.repo_owner`, `session.repo_name` into the workflow input. KNOWN LIMITATION (separate follow-up): the data-commit chunks are emitted live to the SSE stream but are NOT re-persisted onto the assistant message's `parts`. The chunk disappears on page refresh. The commit itself is permanent on GitHub. Re-persistence requires either: (a) a follow-up persistAssistantMessage call with the updated message, or (b) an updateChatMessageParts helper. TDD discipline: each new file went RED → minimum impl → GREEN. Suite: 3195 → 3233 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): derive repoOwner/repoName from session.clone_url Auto-commit needs the owner + repo name to build the GitHub commit URL and to set the remote auth URL on push. The session table has `repo_owner` / `repo_name` columns but they were never populated, so the workflow was always skipping auto-commit silently. Rather than denormalize the data (populate the columns at write time), treat `clone_url` as canonical and parse it at read time. Single source of truth, no drift risk between columns and the URL. New helper `lib/github/parseGitHubRepoIdentifiers.ts` (8 tests) handles https + ssh shapes, .git suffix, trailing slashes, and the null / non-github cases. `handleChatWorkflowStream` parses `session.clone_url` once and threads `repoOwner` / `repoName` into the workflow input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auto-commit): persist data-commit chunk onto assistant message Closes the "data-commit chunk disappears on refresh" limitation flagged in the initial PR description. The chunk is now merged into the assistant message's `parts` and re-persisted via a UPDATE-only helper, so the `GitDataPartCard` UI in open-agents renders the "Committed at <sha>" affordance on page load — not just during the live SSE stream. New files: - `lib/chat/upsertAssistantDataPart.ts` (5 tests) — pure helper ported from open-agents `apps/web/app/workflows/chat.ts`. Merges a data-part into a message's `parts` by `{type, id}` (replace if matched, append otherwise). Immutable; doesn't mutate the input. - `lib/supabase/chat_messages/updateChatMessageParts.ts` (3 tests) — UPDATE-only helper that bypasses the `upsertChatMessage(onConflict: "id", ignoreDuplicates: true)` no-op-on-second-call semantics. Keeps the first-insert path's replay-idempotency for `persistAssistantMessage`; this helper is specifically for "the row exists, replace its `parts`". Wired into `runAgentWorkflow`: After the resolved data-commit chunk is emitted to the writable, the chunk is merged into `result.responseMessage.parts` via `upsertAssistantDataPart`, then `updateChatMessageParts` writes the updated `parts` to the DB. Mirrors open-agents' two-persist pattern in `apps/web/app/workflows/chat.ts:didUpdateGitData`. Tests: - 2 new in `runAgentWorkflow.test.ts`: - success path now asserts `updateChatMessageParts` was called with the resolved data-commit part merged into the parts array - no-changes path asserts `updateChatMessageParts` is NOT called - +8 helper tests across the two new files Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auto-commit): persist whole message + use step for workflow runtime Two bugs from the first persistence attempt, both caught when the data-commit chunk failed to appear on the open-agents UI after refresh: 1. `updateChatMessageParts` wasn't marked `"use step"`. The function calls `supabase.from(...).update(...)` which uses fetch under the hood — forbidden in the workflow body. The call ran silently with no effect. Same failure mode I hit on `recordCreditDeduction.ts`. 2. The workflow was passing `messageWithCommit.parts` (the inner parts array) when `chat_messages.parts` actually stores the WHOLE message object — matching `persistAssistantMessage`'s `parts: message as never` storage convention. Pass the merged message object now. Confirmed via direct DB query that the first attempt didn't write anything (the row still had only the original message-shape from `persistAssistantMessage`'s first call). With the step boundary + correct payload shape, the persistence path now executes and stores the data-commit chunk so the open-agents `GitDataPartCard` can render after refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): address SRP / KISS / OCP review feedback Per @sweetmantech's PR review comments on #614: KISS — Renamed updateChatMessageParts → updateChatMessage and moved the "use step" boundary out of the supabase layer. Supabase wrappers now stay pure; step-bound wrappers live in lib/chat/. - lib/supabase/chat_messages/updateChatMessage.ts (no "use step") - lib/chat/persistAssistantDataPart.ts (new) — "use step" wrapper that internally calls upsertAssistantDataPart (merge) then updateChatMessage (write). Single durable step boundary at the chat-domain layer. SRP — Extracted generateCommitMessage to its own file. Was a private helper inside performAutoCommit.ts; now reusable for alternative commit-message strategies and individually testable. - lib/chat/auto-commit/generateCommitMessage.ts (+6 tests) - performAutoCommit.ts imports it instead of defining inline. OCP — Extracted the ~50-line auto-commit block from runAgentWorkflow into its own file. Workflow body shrinks to a single function call; the auto-commit flow can evolve without touching workflow code. - lib/chat/auto-commit/autoCommitChatTurn.ts (+9 tests covering every gate, the no-changes path, the happy path including pending → resolved chunks + persistence, and the error path). - runAgentWorkflow.ts: ~50 lines → 11-line invocation. - Workflow test pruned: 6 sub-step assertions → 4 wiring assertions (the flow itself is exhaustively tested in autoCommitChatTurn.test.ts). Also flattened UpdateChatMessageResult from a discriminated union to a single interface — same Next.js 16 narrowing issue I hit on DeductCreditsWithAuditResult in #612. Net file count: +5 new files, -0 deletions (renames don't count). Lines moved out of runAgentWorkflow.ts: ~50. Tests: 3233 → 3266 passing (+33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(auto-commit): spread input/result + use gpt-5.4-nano for commit messages Per @sweetmantech's PR review: KISS — `runAgentWorkflow` was enumerating 8 fields when constructing the autoCommitChatTurn call. Switched to `{ ...input, ...result, writable, sandboxState }`. Any future fields added to input or result get forwarded automatically; the workflow body stays tight. Updated the workflow test assertion to `expect.objectContaining(...)` since extra fields from input/result are now passed through. Model — Updated `generateCommitMessage` to use `openai/gpt-5.4-nano` instead of `anthropic/claude-haiku-4.5`. Newer, cheaper, and a better fit for the short-output commit-message task. The prompt is unchanged so behavior should be near-identical. 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(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) * feat(api): migrate GET /api/sessions/[sessionId]/chats/[chatId] from open-agents Returns the chat's persisted UI message stream plus its current streaming state so callers can hydrate / refresh a chat view: { chat: { id, modelId, activeStreamId }, isStreaming, messages } `messages` is the raw `parts` JSON for each `chat_messages` row, ordered by `created_at` then `id`. `isStreaming` is derived from `active_stream_id`. Auth via `validateAuthContext` (Privy Bearer / x-api-key); 404 when the session or chat is missing (or when the chat lives in a different session); 403 when the session is owned by a different account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): also migrate PATCH + DELETE /api/sessions/[sessionId]/chats/[chatId] PATCH `{ title?, modelId? }` returns `{ chat }`. At least one field required; both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated). DELETE returns `{ success: true }`. Refuses with 400 if the chat is the only one in its session. Both reuse the same auth + session-ownership + chat-belongs-to-session gating as the GET. New supabase helpers `lib/supabase/chats/{updateChat,deleteChat}.ts`. 22 new vitest cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop redundant updated_at stamp in updateChat The `chats` table has a `set_updated_at` Postgres trigger (added in database `20260501000000_open_agents_sessions_and_chats.sql`) that auto-refreshes `updated_at` on every row update. Matches the convention of the other 6 update helpers in `lib/supabase/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop conditional spread in patch handler Supabase strips undefined values during JSON serialization, so columns with undefined patch values are simply omitted from the PostgREST UPDATE — no need to guard the spread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): drop unused payload from delete validator The delete handler only needs to know whether validation passed — it doesn't read the auth/session/chat/sibling rows the validator was previously returning. Switch to `NextResponse | null`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chats): slim get/patch validators to just what handlers use Get handler reads only the chat row; patch handler reads only the parsed body. Drop the unused auth/session payload from both validator returns. Matches the simplification just made to the delete validator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): patch handler returns camelCase Chat shape The patch handler was returning the raw Supabase row (snake_case session_id, model_id, etc.) instead of the camelCase wire format documented under the Chat schema. Wrap with toChatResponse so it matches the create endpoint and the docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): GET handler returns full chat row via toChatResponse Expands `SessionChatResponse.chat` from `{ id, modelId, activeStreamId }` to the full camelCase wire row (sessionId, title, lastAssistantMessageAt, createdAt, updatedAt). Lets a single helper cover both initial render and in-tab refresh on the open-agents side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chats): reject unknown fields on PATCH session chat (.strict()) Honors the documented `additionalProperties: false` contract for `UpdateSessionChatRequest` (docs#209). The zod object previously stripped unknown keys silently; `.strict()` now returns a 400 when the body carries any field other than `title` / `modelId`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chats): drop unsupported message arg to zod .strict() The installed zod version's `.strict()` takes no arguments — the custom-message overload broke the production `tsc` build (passed lint + vitest, which don't typecheck the same way). Unknown keys still reject with zod's default "Unrecognized key(s)" 400. 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(chat-workflow): persist assistant message per step (#603) * feat(chat-workflow): persist assistant message per step Upgrade the success-only persist (#609) to per-step persistence so a stopped or crashed turn keeps the partial reply instead of dropping it. - runAgentStep streams through createUIMessageStream and persists on every onStepFinish/onFinish (toUIMessageStream exposes only onFinish); it still returns the final responseMessage so the credits path (#612) keeps billing from its metadata. - persistAssistantMessage now overwrites the row as it grows (DO UPDATE via the restored upsertChatMessage `update` flag) and bumps last_assistant_message_at/updated_at on every persist, so a partial reply still surfaces as unread. - runAgentWorkflow drops its own persist call (now per-step) and keeps the #612 credit charge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(workflow): spread input into runAgentStep (KISS) Per review feedback on PR #603 — drop the manual field-by-field destructure and forward the workflow input object directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com> * feat(sessions): ensure personal repo on POST /api/sessions (#618) * feat(sessions): ensure personal repo on POST /api/sessions When a caller hits POST /api/sessions without a cloneUrl and without an org bound to their auth context, the handler now provisions (or reuses) their personal Recoupable workspace repo at recoupable/<kebab(name)>-<account_id> before returning the session row. This unblocks chat.recoupable.com's Path C cutover (recoupable/chat#1748) — previously the chat-side bootstrap had to construct the personal cloneUrl from a client-side display name (e.g. "sweetman.eth" → "sweetman-eth"), which diverged from open-agents' canonical name source (account_info.name with email-local-part fallback) and 502'd at the clone step. Ported from open-agents: - buildPersonalRepoIdentifier, buildPersonalRepoUrl, githubOwner - repositoryExists, createRepository (plain fetch, no Octokit, to match recoup-api's existing lib/github/* style) - ensurePersonalRepo (idempotent check-then-create) - toKebabCase New session-side helper: - resolveSessionCloneUrl picks bodyCloneUrl > org-no-op > ensurePersonalRepo - buildSessionInsertRow takes the resolved cloneUrl as input - createSessionHandler returns 502 if cloneUrl resolution fails Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(sessions): flatten ResolveSessionCloneUrlResult to single interface Next.js 16's next build doesn't narrow the discriminated union { ok: true; cloneUrl } | { ok: false; error } through `if (!result.ok)`, breaking the Vercel preview build with: Type error: Property 'error' does not exist on type 'ResolveSessionCloneUrlResult'. Same compile-only divergence we hit on PR #603 — vitest's tsc is more permissive than next build's. Flatten to a single interface with `cloneUrl: string | null` and `error?: string` so callers read both fields directly after checking `ok`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(repo-naming): unify workspace repos as recoupable/<accountId> Drop the <kebab(name)>-<accountId> slug — account names are mutable, so any URL that embeds the name eventually drifts. The repo URL is now just the account UUID: stable across renames, identical shape for personal and org workspaces, trivial to parse (`recoupable/<uuid>`). Provisioning flow in ensurePersonalRepo: 1. recoupable/<accountId> exists -> return URL (idempotent) 2. legacy <slug>-<accountId> exists (via GitHub search) -> rename to <accountId>. GitHub auto-redirects the old URL forever, so any sessions.clone_url rows that still reference the old name keep working without a DB backfill. 3. nothing exists -> create fresh recoupable/<accountId> extractOrgId regex now accepts both legacy `<slug>-<uuid>` and bare `<uuid>` shapes so old + new clone URLs both parse. scripts/migrate-workspace-repo-names.ts: one-time backfill. Lists all recoupable org repos, finds those matching ^.+-<uuid>$, renames each to just <uuid>. Defaults to dry-run; --apply commits. Idempotent. resolveSessionCloneUrl no longer needs to look up the account row to derive a slug — ensurePersonalRepo only needs the accountId from auth. Deleted unused lib/string/toKebabCase. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review(PR #618): drop owner/token/description/isPrivate params Per Sweet's review comments on createRepository.ts, hard-code the constants that only have one sensible value: - owner = "recoupable" (RECOUPABLE_GITHUB_OWNER) - token = read from env via getServiceGithubToken (no param plumbing) - description = dropped (GitHub doesn't render anything meaningful) - private = false (workspace repos are public) Applied the same simplification consistently to the other three new github helpers so the surface stays symmetric: - repositoryExists, renameRepository, findLegacyAccountRepo all drop owner + token params; each reads the token via getServiceGithubToken and short-circuits when missing. - ensurePersonalRepo no longer threads token/owner through; its call sites simplified to e.g. createRepository({ name }) and repositoryExists({ repo }). - migrate-workspace-repo-names.ts script likewise calls renameRepository({ repo, newName }) — token plumbing removed. Tests updated to mock getServiceGithubToken instead of passing a fake token through the call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: drop runtime legacy-rename branch (script-once is enough) The migration script renames every legacy <slug>-<accountId> repo once before merge, after which no legacy repo can exist for ensurePersonalRepo to find. The runtime self-healing branch was pure YAGNI. Removed: - lib/github/findLegacyAccountRepo.ts (only caller was the runtime-rename branch) - lib/github/renameRepository.ts (sole consumer is the migration script; PATCH-rename inlined there) - the legacy-rename branch + its tests in ensurePersonalRepo ensurePersonalRepo is now just exists -> create. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: inline repo-name + URL into ensurePersonalRepo Per Sweet's review on buildPersonalRepoIdentifier.ts: post-refactor the helpers hide nothing — the repo name IS the accountId, the URL is one string concat. ensurePersonalRepo was their only runtime caller. Removed: - lib/recoupable/buildPersonalRepoIdentifier.ts (+ test) - lib/recoupable/buildPersonalRepoUrl.ts ensurePersonalRepo now derives the two values inline at the top of the function. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: ensurePersonalRepo now returns just the clone URL string Per Sweet's review on EnsurePersonalRepoResult: the only caller (resolveSessionCloneUrl) reads `cloneUrl` and nothing else. The other three fields (repoUrl, owner, repoName) were written into the response but never consumed. Drop the interface; return Promise<string | null>. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * unify: provision org workspace repo too (cloneUrl: null branch fix) Per Sweet's review on resolveSessionCloneUrl.ts:45 — the cloneUrl: null early-return for auth.orgId was leftover hedging that contradicts the unified recoupable/<accountId> design. Organizations ARE accounts in the data model (account_organization_ids.organization joins the accounts table), so auth.orgId is itself an account_id. The fix: drop the null-return branch and always call ensurePersonalRepo, keyed on auth.orgId ?? auth.accountId. Personal and org sessions now provision the same way — at recoupable/<accountId>, where accountId is either the user's or the org's. Error message updated from "personal repository" to "workspace repository" to reflect the unified naming. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(migrate): match empty-slug -<uuid> repos too The regex used .+ for the slug, requiring at least 1 char before the dash. Accounts that had no display name at repo-creation time produced -<uuid> names (literal leading dash, empty kebab), and the first migration run skipped those as "non-workspace". 6 leading-dash repos turned up in the recoupable org after the first apply pass — 5 of them collided with already-renamed siblings (their losers were deleted manually); 1 was free and renamed. Changed .+ to .* so future runs of this script catch empty-slug names. Bare <uuid> names still don't match (no separator before the UUID). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * revert: createRepository back to private: true Smoke test of the api PR's preview surfaced an inconsistency — the 153 legacy workspace repos in the recoupable org are all private (created by old open-agents code with private: true), but my earlier review-feedback change set new repos to public. Per Sweet's follow-up, flip back to private so the entire fleet stays uniform. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: drop migration script (already applied to prod GitHub) scripts/migrate-workspace-repo-names.ts was a one-time backfill — ran against the recoupable org on 2026-05-26, renamed every legacy <slug>-<accountId> workspace repo to bare <accountId>, then verified zero pending via final dry-run. Keeping it in the codebase forever would be dead weight (per the same KISS principle we applied to findLegacyAccountRepo + renameRepository). Updated the ensurePersonalRepo docstring to reflect that the migration is historical, not an ongoing reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * prune: slim CreateRepositoryResult to {success, repoUrl, error} Per Sweet's review — cloneUrl, owner, repoName are all derivable from repoUrl (cloneUrl = repoUrl + ".git" which git also accepts as repoUrl; owner = "recoupable"; repoName = trailing path segment). Dropped them from the interface and the parse + return shape. ensurePersonalRepo now consumes created.repoUrl directly. The existing-repo branch already returned repoUrl, so the function is now fully consistent on the no-".git" URL form. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * feat(sessions): replace body cloneUrl+branch with organizationId (#620) * feat(sessions): replace body cloneUrl+branch with organizationId POST /api/sessions's request schema is now {title?, organizationId?, sandboxType?}. Removed: - cloneUrl: api derives the workspace repo URL itself via ensurePersonalRepo (recoupable/<accountId> for personal, recoupable/<organizationId> for org sessions) - branch: was only ever piped to sessions.branch column; sandbox handler already falls back to repo default when null - repoOwner / repoName / isNewBranch: never accepted (Zod silently dropped them); pure docs drift, removed from the OpenAPI spec in recoupable/docs#226 Added: - organizationId: optional uuid; when present, validated by validateAuthContext's existing input.organizationId path and sets auth.orgId. resolveSessionCloneUrl's old `auth.orgId ?? auth.accountId` logic now derives the workspace owner from the request alone. Inlined resolveSessionCloneUrl into createSessionHandler — it was a thin wrapper around ensurePersonalRepo once bodyCloneUrl support was removed. Deleted the file and its test. Tests: - buildSessionInsertRow takes a non-null cloneUrl now (always set on session create); branch column is hard-coded null in the row. - createSessionHandler.persistence covers both personal and org branches (ensurePersonalRepo called with auth.accountId vs auth.orgId respectively) plus the 502 path when ensure fails. - validateCreateSessionBody covers the new organizationId uuid validation + forwarding to validateAuthContext. 3,322 / 3,322 tests pass; `next build` TS phase clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(sessions): drop sandboxType too — only vercel is supported z.literal("vercel") with hard-coded SandboxState as { type: "vercel" } & VercelState meant the body field had exactly one acceptable value and the column always got the same value regardless. Pure YAGNI. Also drops `body: CreateSessionBody` from buildSessionInsertRow's input — nothing in the body shape is read there anymore (title is resolved upstream, cloneUrl is passed in, sandboxType is gone). Final POST /api/sessions request shape: { title?, organizationId? }. Pairs with recoupable/docs#226 (also amended to drop sandboxType). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * feat(sessions): enhance session patching with sandbox state management (#578) * feat(sessions): enhance session patching with sandbox state management - Added logic to handle archiving and unarchiving of sessions, ensuring proper sandbox state checks. - Introduced `stopSandboxOnArchive` function to manage sandbox teardown for archived sessions. - Updated `patchSessionByIdHandler` to include conditions for unarchiving sessions based on sandbox state. - Implemented CORS headers in response for error handling during sandbox operations. * test(sessions): enhance session route tests with sandbox management mocks - Added mocks for `next/server` and `stopSandboxOnArchive` to improve test isolation and control over sandbox state during session route tests. - Updated session test suite to utilize these mocks for better reliability and clarity in testing session behavior. * refactor(tests): simplify mock implementation for session route tests - Updated the mock for `next/server` in the session route tests to use a more concise syntax, improving readability and maintainability of the test code. * fix(sessions): improve error handling in stopSandboxOnArchive function - Added error logging for sandbox stop failures, providing clearer insights into issues during session archiving. - Updated error message for state clearing failures to enhance clarity in debugging. * fix(sessions): enhance error handling in stopSandboxOnArchive function - Added a return statement in the error handling block to prevent further execution after a sandbox stop failure, improving robustness during session archiving. * fix(sessions): refine unarchive condition for sandbox state management - Updated the unarchive condition in `patchSessionByIdHandler` to include a check for the sandbox's lifecycle state, ensuring that the sandbox is not in a hibernated state before allowing unarchiving. This enhances error handling during session management. * fix(sessions): further refine unarchive condition for sandbox state management - Enhanced the unarchive condition in `patchSessionByIdHandler` to ensure the sandbox's lifecycle state is not "archived" in addition to not being "hibernated". This improves the accuracy of session management during state transitions. * fix(sessions): add session status check before stopping sandbox on archive - Introduced a check in the `stopSandboxOnArchive` function to ensure the session's status is not "archived" before proceeding with sandbox state clearing. This enhances session management accuracy during archiving operations. * fix(sessions): optimize session state update in stopSandboxOnArchive - Refined the logic in the `stopSandboxOnArchive` function to check if the session is still archived before updating its lifecycle state. This change improves the accuracy of session state management during archiving operations. * Enhance session patching and sandbox state management - Updated the PATCH /api/sessions/[sessionId] handler to include additional lifecycle properties when archiving a session, ensuring proper state management. - Refined the hasRuntimeSandboxState function to improve checks for sandbox state validity, including handling of expiresAt and sandboxName. - Enhanced unit tests for hasRuntimeSandboxState to cover new scenarios, ensuring accurate state validation. - Improved the stopSandboxOnArchive function to handle errors more gracefully and ensure that sandbox state is cleared appropriately when archiving sessions. All changes are accompanied by passing tests, maintaining code integrity and functionality. * Refactor file path resolution in agent tools - Replaced `path` module usage with `resolveSandboxPath` and `joinSandboxPath` in multiple tools (bashTool, editFileTool, globTool, grepTool, readFileTool, skillTool, writeFileTool) to streamline path handling within the sandbox environment. - Updated `toDisplayPath` to utilize `resolveSandboxPath` and `isPathWithinSandboxDirectory` for improved path validation. - Enhanced `discoverSkills` and `findSkillFile` to use `joinSandboxPath` for directory handling, ensuring consistency across skill discovery processes. - Adjusted `recordCreditDeduction` to use `randomUUID` instead of `nanoid` for generating unique event IDs, aligning with modern practices. These changes improve code maintainability and ensure consistent path resolution across the application. * Refactor credit deduction and path handling - Replaced `randomUUID` with `nanoid` in `recordCreditDeduction` for generating unique event IDs, aligning with modern practices. - Enhanced `isPathWithinSandboxDirectory` to avoid false negatives when checking paths, ensuring accurate path validation. - Updated `discoverSkills` to normalize file paths by replacing backslashes with forward slashes, improving cross-platform compatibility. These changes improve code consistency and maintainability across the application. * Refactor sandbox path handling in agent tools - Updated imports in multiple tools (bashTool, editFileTool, globTool, grepTool, readFileTool, skillTool, writeFileTool) to use the new `resolveSandboxPath` and `joinSandboxPath` functions, enhancing path resolution consistency. - Removed the deprecated `sandboxPaths` module, consolidating path-related functions into dedicated files for better organization. - Introduced new utility functions: `dirnameSandboxPath`, `isPathWithinSandboxDirectory`, and `relativeSandboxPath` to streamline path operations and improve maintainability. These changes enhance the clarity and efficiency of path management within the sandbox environment. * Enhance path comparison in isPathWithinSandboxDirectory for Windows compatibility - Updated the isPathWithinSandboxDirectory function to convert both the resolved file path and directory to lowercase before comparison, ensuring accurate path validation on Windows systems, which are case-insensitive. - This change prevents false negatives when checking if a file path is within a specified sandbox directory. These modifications improve the reliability of path handling in the sandbox environment. * refactor(agent/tools): revert sandbox path helpers to native path module Per KISS: path resolution in agent tools is unrelated to PATCH /api/sessions/{sessionId}. Restored path.isAbsolute/path.resolve/path.join in all 8 tools, removing resolveSandboxPath, joinSandboxPath, and dirnameSandboxPath imports that were scope creep. * chore(pr-578): remove all scope creep PR now only modifies the 3 files directly tied to PATCH /api/sessions/{sessionId}: - lib/sessions/patchSessionByIdHandler.ts - lib/sessions/stopSandboxOnArchive.ts - app/api/sessions/[sessionId]/__tests__/route.test.ts Deleted: dirnameSandboxPath, isPathWithinSandboxDirectory, isPosixSandboxPath, joinSandboxPath, relativeSandboxPath, resolveSandboxPath, toPosixSegment. Reverted: hasRuntimeSandboxState.ts, discoverSkills.ts, findSkillFile.ts, and all sandbox test files to origin/test. * refactor(sessions): extract isSandboxPausing into lib/sandbox OCP fix: move isSandboxPausing predicate out of patchSessionByIdHandler into its own lib so the handler only calls lib functions, not inline logic. * fix(sessions): preserve snapshot as fallback until stop succeeds; clear stale lifecycle_error on archive - Move snapshot_url/snapshot_created_at null-out from synchronous PATCH update into stopSandboxOnArchive success branch, matching open-agents behavior. Snapshot now remains as a fallback if stop() fails. - Add lifecycle_error: null to synchronous archive update so stale errors from a prior failed run are cleared on re-archive (open-agents parity). * refactor(sessions): extract isUnarchiveConflict predicate into lib OCP: move the 409-guard predicate out of patchSessionByIdHandler and into lib/sessions/isUnarchiveConflict.ts. Handler now calls isUnarchiveConflict(row). * refactor(sessions): simplify lifecycle state updates in patchSessionByIdHandler Replaced inline lifecycle state updates with constants ARCHIVE_LIFECYCLE_PATCH and UNARCHIVE_LIFECYCLE_PATCH for better readability and maintainability. This change streamlines the handling of session lifecycle states during patch operations. * refactor(sessions): format imports for lifecycle state patches in patchSessionByIdHandler Updated the import statements for ARCHIVE_LIFECYCLE_PATCH and UNARCHIVE_LIFECYCLE_PATCH to improve readability and maintain consistency in the code structure. This change enhances the clarity of the lifecycle state management within the session patching process. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Arpit Gupta <arpitgupta1214@gmail.com> Co-authored-by: ahmednahima0-beep <ahmednahima0@gmail.com>
This was referenced Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the full chat-by-id route from open-agents'
apps/web/app/api/sessions/[sessionId]/chats/[chatId]/route.tsto api so the frontend can cut over without behavior changes.GET /api/sessions/{sessionId}/chats/{chatId}→{ chat: { id, modelId, activeStreamId }, isStreaming, messages }PATCH→{ chat }. Body{ title?, modelId? }; at least one field, both must be non-empty after trim. modelId is stored as-is (no model-variant sanitization until user-preferences are migrated).DELETE→{ success: true }. Refuses with 400 if the chat is the only one in its session.All three reuse the same gating: auth via
validateAuthContext, 404 when session/chat is missing or chat lives in a different session, 403 on cross-account.Test plan
GETwith valid auth returns 200 +{ chat, isStreaming, messages }PATCH { title }renames the chat;PATCH { modelId }updates model_idPATCH {}returns 400 "At least one field is required"DELETEsucceeds when there are 2+ chats in the sessionDELETEon the only chat returns 400 "Cannot delete the only chat in a session"🤖 Generated with Claude Code
Summary by CodeRabbit