From efb0172faa23c3610ffe1c3cb490e5462a041fb2 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Mon, 25 May 2026 13:49:36 -0500 Subject: [PATCH 1/4] fix(chat-workflow): persist the assistant message after a successful run (#609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * fix(chat-workflow): loosen AssistantMessage type to accept UIMessage The over-strict `& Record` 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../generateAssistantMessageId.test.ts | 20 +++ .../workflows/__tests__/runAgentStep.test.ts | 83 ++++++++++- .../__tests__/runAgentWorkflow.test.ts | 102 ++++++++++++- .../workflows/generateAssistantMessageId.ts | 19 +++ app/lib/workflows/runAgentStep.ts | 53 ++++++- app/lib/workflows/runAgentWorkflow.ts | 27 ++++ .../__tests__/persistAssistantMessage.test.ts | 140 ++++++++++++++++++ lib/chat/persistAssistantMessage.ts | 73 +++++++++ 8 files changed, 506 insertions(+), 11 deletions(-) create mode 100644 app/lib/workflows/__tests__/generateAssistantMessageId.test.ts create mode 100644 app/lib/workflows/generateAssistantMessageId.ts create mode 100644 lib/chat/__tests__/persistAssistantMessage.test.ts create mode 100644 lib/chat/persistAssistantMessage.ts diff --git a/app/lib/workflows/__tests__/generateAssistantMessageId.test.ts b/app/lib/workflows/__tests__/generateAssistantMessageId.test.ts new file mode 100644 index 000000000..c83cebfb1 --- /dev/null +++ b/app/lib/workflows/__tests__/generateAssistantMessageId.test.ts @@ -0,0 +1,20 @@ +import { describe, it, expect, vi } from "vitest"; +import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; + +vi.mock("ai", async () => { + const actual = await vi.importActual("ai"); + return { ...actual, generateId: vi.fn(() => "generated-by-mock") }; +}); + +describe("generateAssistantMessageId", () => { + it("returns the value from ai's generateId()", async () => { + const id = await generateAssistantMessageId(); + expect(id).toBe("generated-by-mock"); + }); + + it("returns a string", async () => { + const id = await generateAssistantMessageId(); + expect(typeof id).toBe("string"); + expect(id.length).toBeGreaterThan(0); + }); +}); diff --git a/app/lib/workflows/__tests__/runAgentStep.test.ts b/app/lib/workflows/__tests__/runAgentStep.test.ts index b2e90475b..cfb101877 100644 --- a/app/lib/workflows/__tests__/runAgentStep.test.ts +++ b/app/lib/workflows/__tests__/runAgentStep.test.ts @@ -12,15 +12,28 @@ vi.mock("@ai-sdk/gateway", () => ({ gateway: vi.fn((modelId: string) => ({ modelId, __mock: "gateway" })), })); -function makeStreamResult(opts?: { metadataCalls?: Array }) { +function makeStreamResult(opts?: { + metadataCalls?: Array; + onFinishCalls?: Array; + emittedResponseMessage?: unknown; +}) { const calls = opts?.metadataCalls ?? []; + const onFinishCalls = opts?.onFinishCalls ?? []; return { - toUIMessageStream: vi.fn((streamOpts: { messageMetadata?: unknown }) => { - // Capture the callback so tests can inspect it + toUIMessageStream: vi.fn((streamOpts: { messageMetadata?: unknown; onFinish?: unknown }) => { + // Capture the callbacks so tests can inspect (and invoke) them calls.push(streamOpts.messageMetadata); + onFinishCalls.push(streamOpts.onFinish); return (async function* () { yield { type: "start" }; yield { type: "finish" }; + // Mirror the AI SDK contract: onFinish fires after the + // generator yields its last chunk with the assembled message. + if (typeof streamOpts.onFinish === "function" && opts?.emittedResponseMessage) { + (streamOpts.onFinish as (a: { responseMessage: unknown }) => void)({ + responseMessage: opts.emittedResponseMessage, + }); + } })(); }), finishReason: Promise.resolve("stop"), @@ -49,6 +62,7 @@ const baseInput = { agentContext: { sandbox: { state: { type: "vercel" }, workingDirectory: "/sandbox/mono" }, }, + assistantMessageId: "asst-test-id", }; describe("runAgentStep", () => { @@ -174,4 +188,67 @@ describe("runAgentStep", () => { expect(cb({ part: { type: "text-delta" } })).toBeUndefined(); expect(cb({ part: { type: "start" } })).toBeUndefined(); }); + + it("wires an onFinish callback into toUIMessageStream", async () => { + const onFinishCalls: unknown[] = []; + vi.mocked(streamText).mockReturnValue(makeStreamResult({ onFinishCalls }) as never); + const { stream } = makeWritable(); + + await runAgentStep({ ...baseInput, writable: stream } as never); + + expect(onFinishCalls).toHaveLength(1); + expect(typeof onFinishCalls[0]).toBe("function"); + }); + + it("returns the responseMessage captured from onFinish", async () => { + const emittedResponseMessage = { + id: "assistant-msg-1", + role: "assistant", + parts: [{ type: "text", text: "Hello" }], + }; + vi.mocked(streamText).mockReturnValue(makeStreamResult({ emittedResponseMessage }) as never); + const { stream } = makeWritable(); + + const result = await runAgentStep({ ...baseInput, writable: stream } as never); + + expect(result.responseMessage).toEqual(emittedResponseMessage); + expect(result.finishReason).toBe("stop"); + }); + + it("returns responseMessage: undefined when onFinish never fires", async () => { + // Default makeStreamResult — no emittedResponseMessage, so onFinish is wired but never invoked + vi.mocked(streamText).mockReturnValue(makeStreamResult() as never); + const { stream } = makeWritable(); + + const result = await runAgentStep({ ...baseInput, writable: stream } as never); + + expect(result.responseMessage).toBeUndefined(); + expect(result.finishReason).toBe("stop"); + }); + + it("forwards input.assistantMessageId into toUIMessageStream's generateMessageId", async () => { + const generateMessageIdCalls: unknown[] = []; + const streamResult = makeStreamResult(); + // Spy on the options passed to toUIMessageStream to grab the generateMessageId fn. + const originalToUIMessageStream = streamResult.toUIMessageStream; + streamResult.toUIMessageStream = vi.fn((streamOpts: { generateMessageId?: unknown }) => { + generateMessageIdCalls.push(streamOpts.generateMessageId); + return (originalToUIMessageStream as unknown as (o: unknown) => AsyncGenerator)( + streamOpts, + ); + }) as never; + vi.mocked(streamText).mockReturnValue(streamResult as never); + const { stream } = makeWritable(); + + await runAgentStep({ + ...baseInput, + writable: stream, + assistantMessageId: "asst-from-workflow-xyz", + } as never); + + expect(generateMessageIdCalls).toHaveLength(1); + const gen = generateMessageIdCalls[0] as () => string; + expect(typeof gen).toBe("function"); + expect(gen()).toBe("asst-from-workflow-xyz"); + }); }); diff --git a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts index d061abbce..3e59ffc2d 100644 --- a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts +++ b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts @@ -3,6 +3,8 @@ import { runAgentWorkflow } from "@/app/lib/workflows/runAgentWorkflow"; import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; import { clearChatActiveStream } from "@/lib/chat/clearChatActiveStream"; import { closeChatStream } from "@/app/lib/workflows/closeChatStream"; +import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; +import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; vi.mock("@/app/lib/workflows/runAgentStep", () => ({ runAgentStep: vi.fn(), @@ -13,6 +15,12 @@ vi.mock("@/lib/chat/clearChatActiveStream", () => ({ vi.mock("@/app/lib/workflows/closeChatStream", () => ({ closeChatStream: vi.fn(), })); +vi.mock("@/app/lib/workflows/generateAssistantMessageId", () => ({ + generateAssistantMessageId: vi.fn(), +})); +vi.mock("@/lib/chat/persistAssistantMessage", () => ({ + persistAssistantMessage: vi.fn(), +})); // Captured writable stub so tests can assert closeChatStream got the // same instance the workflow body holds. const writableStub = new WritableStream(); @@ -26,7 +34,10 @@ vi.mock("workflow", () => ({ })), })); -beforeEach(() => vi.clearAllMocks()); +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(generateAssistantMessageId).mockResolvedValue("asst-fresh-id"); +}); const baseInput = { messages: [{ id: "m1", role: "user", parts: [{ type: "text", text: "hi" }] } as never], @@ -40,7 +51,10 @@ const baseInput = { describe("runAgentWorkflow", () => { it("clears active_stream_id after a successful run, using the workflow's own runId", async () => { - vi.mocked(runAgentStep).mockResolvedValue({ finishReason: "stop" }); + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); await runAgentWorkflow(baseInput); @@ -58,7 +72,10 @@ describe("runAgentWorkflow", () => { }); it("explicitly closes the chat writable after a successful run so SSE ends promptly", async () => { - vi.mocked(runAgentStep).mockResolvedValue({ finishReason: "stop" }); + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); await runAgentWorkflow(baseInput); @@ -74,4 +91,83 @@ describe("runAgentWorkflow", () => { expect(closeChatStream).toHaveBeenCalledTimes(1); expect(closeChatStream).toHaveBeenCalledWith(writableStub); }); + + it("persists the assistant message when runAgentStep returns one", async () => { + const responseMessage = { + id: "assistant-msg-xyz", + role: "assistant", + parts: [{ type: "text", text: "Hello!" }], + }; + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessage as never, + }); + + await runAgentWorkflow(baseInput); + + expect(persistAssistantMessage).toHaveBeenCalledTimes(1); + expect(persistAssistantMessage).toHaveBeenCalledWith("chat-1", responseMessage); + }); + + it("does NOT call persistAssistantMessage when runAgentStep returns no responseMessage", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); + + await runAgentWorkflow(baseInput); + + expect(persistAssistantMessage).not.toHaveBeenCalled(); + }); + + it("does NOT call persistAssistantMessage when runAgentStep throws (no message to persist)", async () => { + vi.mocked(runAgentStep).mockRejectedValue(new Error("model exploded")); + + await expect(runAgentWorkflow(baseInput)).rejects.toThrow("model exploded"); + + expect(persistAssistantMessage).not.toHaveBeenCalled(); + // But cleanup steps still run via the try/finally + expect(clearChatActiveStream).toHaveBeenCalledTimes(1); + expect(closeChatStream).toHaveBeenCalledTimes(1); + }); + + it("generates a fresh assistantMessageId via the step and forwards it to runAgentStep", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); + + await runAgentWorkflow(baseInput); + + expect(generateAssistantMessageId).toHaveBeenCalledTimes(1); + expect(runAgentStep).toHaveBeenCalledWith( + expect.objectContaining({ assistantMessageId: "asst-fresh-id" }), + ); + }); + + it("reuses the latest assistant message id when resuming a tool-call turn (no fresh generation)", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); + + const resumingInput = { + ...baseInput, + messages: [ + { id: "m1", role: "user", parts: [{ type: "text", text: "go" }] }, + { + id: "asst-in-progress", + role: "assistant", + parts: [{ type: "text", text: "thinking..." }], + }, + ] as never, + }; + + await runAgentWorkflow(resumingInput); + + expect(generateAssistantMessageId).not.toHaveBeenCalled(); + expect(runAgentStep).toHaveBeenCalledWith( + expect.objectContaining({ assistantMessageId: "asst-in-progress" }), + ); + }); }); diff --git a/app/lib/workflows/generateAssistantMessageId.ts b/app/lib/workflows/generateAssistantMessageId.ts new file mode 100644 index 000000000..d33d0ce65 --- /dev/null +++ b/app/lib/workflows/generateAssistantMessageId.ts @@ -0,0 +1,19 @@ +import { generateId } from "ai"; + +/** + * Vercel Workflow `"use step"` that returns a fresh message id. + * + * Wrapped as a step rather than inlined into the workflow body + * because `generateId()` is non-deterministic — calling it directly + * from workflow code would break the durable-replay contract (a + * replay would generate a *different* id and diverge from the + * captured event log). As a step, the result is captured in the + * event log once and reused on every replay. + * + * Mirrors open-agents' `generateId` step in + * `apps/web/app/workflows/chat.ts`. + */ +export async function generateAssistantMessageId(): Promise { + "use step"; + return generateId(); +} diff --git a/app/lib/workflows/runAgentStep.ts b/app/lib/workflows/runAgentStep.ts index 7ed847d5d..0520bb20a 100644 --- a/app/lib/workflows/runAgentStep.ts +++ b/app/lib/workflows/runAgentStep.ts @@ -22,6 +22,35 @@ export type RunAgentStepInput = { * is added to `experimental_context` right before each model call. */ agentContext: DurableAgentContext; + /** + * Stable id to assign to the assistant message produced by this + * step. Generated once in `runAgentWorkflow` so: + * + * - Every chunk in this step's `toUIMessageStream` carries the + * same id (the AI SDK threads it through). + * - Future multi-step iterations of the agent loop reuse the + * same id so a single conversational reply is one row in + * `chat_messages` rather than fragmenting per tool-call cycle. + * - Resume after tool-call interaction reattaches to the in- + * progress assistant message rather than spawning a new one. + * + * Mirrors open-agents' `runAgentStep(messages, originalMessages, + * messageId, ...)` signature in + * `apps/web/app/workflows/chat.ts`. + */ + assistantMessageId: string; +}; + +export type RunAgentStepResult = { + finishReason: string; + /** + * The assembled assistant message captured from + * `toUIMessageStream`'s `onFinish` callback. `undefined` if the + * stream finished without emitting one (e.g. an error path that + * short-circuits before any chunks land). Used by `runAgentWorkflow` + * to persist the final message to `chat_messages`. + */ + responseMessage: UIMessage | undefined; }; /** @@ -38,9 +67,11 @@ export type RunAgentStepInput = { * of the tool surface ports in a follow-up PR. * * @param input - Messages + selected model + writable stream + agent context. - * @returns finishReason from the model run. + * @returns `finishReason` from the model run plus the assembled + * `responseMessage` (when one was emitted) so the caller can + * persist it. */ -export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishReason: string }> { +export async function runAgentStep(input: RunAgentStepInput): Promise { "use step"; console.log("[runAgentStep] start", { @@ -91,6 +122,12 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe }), }); + // Capture the assembled assistant message via `onFinish` so the + // caller can persist it. Mirrors open-agents' `runAgentStep` which + // stashes `finishedResponseMessage` into a closure-scoped variable + // for the outer workflow body to forward into `persistAssistantMessage`. + let responseMessage: UIMessage | undefined; + // Acquire the writer once and release in `finally` so a thrown chunk // doesn't leak the lock. const writer = input.writable.getWriter(); @@ -100,7 +137,13 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe // shape so sandbox.recoupable.com sees the same metadata when cut // over to api's /api/chat/workflow. const messageMetadata = buildMessageMetadataCallback({ modelId: input.modelId }); - for await (const part of result.toUIMessageStream({ messageMetadata })) { + for await (const part of result.toUIMessageStream({ + messageMetadata, + generateMessageId: () => input.assistantMessageId, + onFinish: ({ responseMessage: finishedResponseMessage }) => { + responseMessage = finishedResponseMessage; + }, + })) { await writer.write(part); } } finally { @@ -108,6 +151,6 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe } const finishReason = await result.finishReason; - console.log("[runAgentStep] finish", { finishReason }); - return { finishReason }; + console.log("[runAgentStep] finish", { finishReason, hasResponseMessage: !!responseMessage }); + return { finishReason, responseMessage }; } diff --git a/app/lib/workflows/runAgentWorkflow.ts b/app/lib/workflows/runAgentWorkflow.ts index c9e5fdcd9..e4e628e96 100644 --- a/app/lib/workflows/runAgentWorkflow.ts +++ b/app/lib/workflows/runAgentWorkflow.ts @@ -1,8 +1,10 @@ import { getWorkflowMetadata, getWritable } from "workflow"; import type { UIMessage, UIMessageChunk } from "ai"; import { closeChatStream } from "@/app/lib/workflows/closeChatStream"; +import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; import { clearChatActiveStream } from "@/lib/chat/clearChatActiveStream"; +import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; import type { DurableAgentContext } from "@/lib/agent/tools/AgentContext"; export type RunAgentWorkflowInput = { @@ -48,14 +50,39 @@ export async function runAgentWorkflow(input: RunAgentWorkflowInput): Promise(); + // Pick or generate a stable id for the assistant message. If the + // last message in the conversation is already an assistant message + // (we're resuming an in-progress turn after a tool-call interaction) + // reuse its id so chunks append to the same `chat_messages` row. + // Otherwise generate a fresh id once via a `"use step"` so the + // value is durable across workflow replays. Mirrors open-agents' + // pattern in `apps/web/app/workflows/chat.ts` where the id is + // generated in the workflow body and threaded into every + // `runAgentStep` call. + const latestMessage = input.messages.at(-1); + const assistantMessageId = + latestMessage?.role === "assistant" ? latestMessage.id : await generateAssistantMessageId(); + try { const result = await runAgentStep({ messages: input.messages, modelId: input.modelId, writable, agentContext: input.agentContext, + assistantMessageId, }); console.log("[runAgentWorkflow] finish", { finishReason: result.finishReason }); + + // Persist the final assistant message to `chat_messages` so a page + // refresh after the stream completes still shows the reply. Without + // this, the recoup-api cutover silently drops assistant responses — + // they stream to the client over SSE but never land in the DB. + // `persistAssistantMessage` is fire-and-forget by contract; it + // swallows its own errors so a transient DB failure here doesn't + // mark the workflow run failed. + if (result.responseMessage) { + await persistAssistantMessage(input.chatId, result.responseMessage); + } } finally { // Run two cleanup steps in parallel: // 1) `clearChatActiveStream` — CAS-gated DB clear of the chat's diff --git a/lib/chat/__tests__/persistAssistantMessage.test.ts b/lib/chat/__tests__/persistAssistantMessage.test.ts new file mode 100644 index 000000000..fc6def35e --- /dev/null +++ b/lib/chat/__tests__/persistAssistantMessage.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; +import { upsertChatMessage } from "@/lib/supabase/chat_messages/upsertChatMessage"; +import { updateChat } from "@/lib/supabase/chats/updateChat"; + +vi.mock("@/lib/supabase/chat_messages/upsertChatMessage", () => ({ + upsertChatMessage: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/updateChat", () => ({ + updateChat: vi.fn(), +})); + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(updateChat).mockResolvedValue({ + ok: true, + rowsUpdated: 1, + row: null, + }); +}); + +const CHAT_ID = "11111111-1111-1111-1111-111111111111"; +const ASSISTANT_ID = "msg_abc"; + +function buildAssistantMessage(overrides: Record = {}) { + return { + id: ASSISTANT_ID, + role: "assistant", + parts: [{ type: "text", text: "Hello!" }], + ...overrides, + }; +} + +describe("persistAssistantMessage", () => { + it("upserts the assistant message row with role 'assistant'", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: { id: ASSISTANT_ID } as never, + isDuplicate: false, + }); + + await persistAssistantMessage(CHAT_ID, buildAssistantMessage()); + + expect(upsertChatMessage).toHaveBeenCalledWith( + expect.objectContaining({ + id: ASSISTANT_ID, + chat_id: CHAT_ID, + role: "assistant", + }), + ); + }); + + it("touches updated_at on the chat row on a fresh insert", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: { id: ASSISTANT_ID } as never, + isDuplicate: false, + }); + + await persistAssistantMessage(CHAT_ID, buildAssistantMessage()); + + expect(updateChat).toHaveBeenCalledWith( + { id: CHAT_ID }, + expect.objectContaining({ updated_at: expect.any(String) }), + ); + }); + + it("bumps last_assistant_message_at on a fresh insert (drives sidebar unread badge)", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: { id: ASSISTANT_ID } as never, + isDuplicate: false, + }); + + await persistAssistantMessage(CHAT_ID, buildAssistantMessage()); + + expect(updateChat).toHaveBeenCalledWith( + { id: CHAT_ID }, + expect.objectContaining({ + last_assistant_message_at: expect.any(String), + }), + ); + }); + + it("uses the same timestamp for updated_at and last_assistant_message_at (matches open-agents)", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: { id: ASSISTANT_ID } as never, + isDuplicate: false, + }); + + await persistAssistantMessage(CHAT_ID, buildAssistantMessage()); + + const updateArgs = vi.mocked(updateChat).mock.calls[0]?.[1] as { + updated_at?: string; + last_assistant_message_at?: string; + }; + expect(updateArgs.updated_at).toBeDefined(); + expect(updateArgs.last_assistant_message_at).toBe(updateArgs.updated_at); + }); + + it("does NOT touch updated_at on duplicate (workflow replay)", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: true, + row: null, + isDuplicate: true, + }); + + await persistAssistantMessage(CHAT_ID, buildAssistantMessage()); + + expect(updateChat).not.toHaveBeenCalled(); + }); + + it("silently no-ops when the message role is not 'assistant' (guard against caller mistakes)", async () => { + await persistAssistantMessage(CHAT_ID, buildAssistantMessage({ role: "user" })); + + expect(upsertChatMessage).not.toHaveBeenCalled(); + expect(updateChat).not.toHaveBeenCalled(); + }); + + it("silently no-ops when the upsert reports a DB error (fire-and-forget contract)", async () => { + vi.mocked(upsertChatMessage).mockResolvedValue({ + ok: false, + error: "transient db error", + }); + + await expect( + persistAssistantMessage(CHAT_ID, buildAssistantMessage()), + ).resolves.toBeUndefined(); + expect(updateChat).not.toHaveBeenCalled(); + }); + + it("swallows unexpected exceptions (must not bubble up)", async () => { + vi.mocked(upsertChatMessage).mockRejectedValue(new Error("boom")); + + await expect( + persistAssistantMessage(CHAT_ID, buildAssistantMessage()), + ).resolves.toBeUndefined(); + }); +}); diff --git a/lib/chat/persistAssistantMessage.ts b/lib/chat/persistAssistantMessage.ts new file mode 100644 index 000000000..13f93c27b --- /dev/null +++ b/lib/chat/persistAssistantMessage.ts @@ -0,0 +1,73 @@ +import { upsertChatMessage } from "@/lib/supabase/chat_messages/upsertChatMessage"; +import { updateChat } from "@/lib/supabase/chats/updateChat"; + +/** + * Minimal duck-type shape we read off the assistant message. Both AI + * SDK's `UIMessage` and the in-test fixtures structurally satisfy it. + * Kept intentionally loose because the row we write to + * `chat_messages.parts` is `jsonb` — Supabase persists whatever the + * message looks like. + */ +type AssistantMessage = { + id: string; + role: string; + parts: ReadonlyArray; +}; + +/** + * Fire-and-forget persistence of the final assistant message at the + * end of a chat-workflow run. Mirrors open-agents' + * `persistAssistantMessage` step in + * `apps/web/app/workflows/chat-post-finish.ts` and closes the + * silent-data-loss gap the recoup-api cutover introduced — without + * this call the assistant response is streamed to the client but + * never written to `chat_messages`, so a page refresh after the + * stream completes wipes the message. + * + * Uses `upsertChatMessage(... { onConflict: "id", ignoreDuplicates })` + * so a workflow that's restarted (replay, recovery) doesn't + * double-insert. On a fresh insert we also bump + * `last_assistant_message_at` (drives the sidebar `hasUnread` badge + * in `getChatSummaries` — `lastAssistantMessageAt > lastReadAt`) and + * touch `updated_at` so the sidebar sort surfaces the chat. Matches + * open-agents' `updateChatAssistantActivity` which sets both columns + * to the same timestamp. + * + * Title generation lives in `persistLatestUserMessage` (the first + * user message is canonical for chat titles) — this function + * deliberately does NOT update the chat title. + * + * Errors are caught and logged. Contract is "schedule it and forget" + * — never block the workflow or surface failures to the UI. + * + * @param chatId - Target chat row. + * @param message - The assembled assistant message (typically from + * `toUIMessageStream`'s `onFinish.responseMessage`). + */ +export async function persistAssistantMessage( + chatId: string, + message: AssistantMessage, +): Promise { + "use step"; + try { + if (!message || message.role !== "assistant") return; + + const inserted = await upsertChatMessage({ + id: message.id, + chat_id: chatId, + role: "assistant", + parts: message as never, + }); + + if (!inserted.ok) return; + if (inserted.isDuplicate || inserted.row === null) return; + + const activityAt = new Date().toISOString(); + await updateChat( + { id: chatId }, + { updated_at: activityAt, last_assistant_message_at: activityAt }, + ); + } catch (error) { + console.error("[persistAssistantMessage] error:", error); + } +} From d56aa318158bd826e871e086019304269916793d Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Mon, 25 May 2026 16:57:35 -0500 Subject: [PATCH 2/4] feat(credits): charge credits per chat turn (atomic wallet debit + audit) (#612) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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) * 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) * 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) * 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) * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../__tests__/runAgentWorkflow.test.ts | 83 +++++++++++++++++ app/lib/workflows/runAgentWorkflow.ts | 35 +++++++- .../integration/chatEndToEnd.test.ts | 1 + lib/chat/handleChatWorkflowStream.ts | 1 + lib/credits/__tests__/getCreditUsage.test.ts | 76 ++++++++++++++++ .../__tests__/handleChatCredits.test.ts | 49 +++++++++- .../__tests__/recordCreditDeduction.test.ts | 82 ++++++++--------- lib/credits/getCreditUsage.ts | 17 ++++ lib/credits/handleChatCredits.ts | 33 +++++-- lib/credits/recordCreditDeduction.ts | 64 +++++++------ .../__tests__/deductCreditsWithAudit.test.ts | 89 +++++++++++++++++++ .../credits_usage/deductCreditsWithAudit.ts | 73 +++++++++++++++ types/database.types.ts | 9 ++ 13 files changed, 533 insertions(+), 79 deletions(-) create mode 100644 lib/supabase/credits_usage/__tests__/deductCreditsWithAudit.test.ts create mode 100644 lib/supabase/credits_usage/deductCreditsWithAudit.ts diff --git a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts index 3e59ffc2d..19cec3b78 100644 --- a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts +++ b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts @@ -5,6 +5,7 @@ import { clearChatActiveStream } from "@/lib/chat/clearChatActiveStream"; import { closeChatStream } from "@/app/lib/workflows/closeChatStream"; import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; +import { handleChatCredits } from "@/lib/credits/handleChatCredits"; vi.mock("@/app/lib/workflows/runAgentStep", () => ({ runAgentStep: vi.fn(), @@ -21,6 +22,9 @@ vi.mock("@/app/lib/workflows/generateAssistantMessageId", () => ({ vi.mock("@/lib/chat/persistAssistantMessage", () => ({ persistAssistantMessage: vi.fn(), })); +vi.mock("@/lib/credits/handleChatCredits", () => ({ + handleChatCredits: vi.fn(), +})); // Captured writable stub so tests can assert closeChatStream got the // same instance the workflow body holds. const writableStub = new WritableStream(); @@ -43,6 +47,7 @@ const baseInput = { messages: [{ id: "m1", role: "user", parts: [{ type: "text", text: "hi" }] } as never], chatId: "chat-1", sessionId: "session-1", + accountId: "acc-1", modelId: "anthropic/claude-haiku-4.5", agentContext: { sandbox: { state: { type: "vercel" }, workingDirectory: "/sandbox/mono" }, @@ -170,4 +175,82 @@ describe("runAgentWorkflow", () => { expect.objectContaining({ assistantMessageId: "asst-in-progress" }), ); }); + + it("calls handleChatCredits with the gateway cost + token usage from responseMessage.metadata", async () => { + const responseMessage = { + id: "assistant-msg-xyz", + role: "assistant", + parts: [{ type: "text", text: "Hello!" }], + metadata: { + totalMessageCost: 0.07, + totalMessageUsage: { + inputTokens: 100, + cachedInputTokens: 10, + outputTokens: 20, + }, + }, + }; + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessage as never, + }); + + await runAgentWorkflow(baseInput); + + expect(handleChatCredits).toHaveBeenCalledTimes(1); + expect(handleChatCredits).toHaveBeenCalledWith({ + accountId: "acc-1", + model: "anthropic/claude-haiku-4.5", + source: "api", + gatewayCostUsd: 0.07, + usage: { + inputTokens: 100, + cachedInputTokens: 10, + outputTokens: 20, + }, + }); + }); + + it("calls handleChatCredits with zero usage when metadata is missing (lets the 1c floor apply)", async () => { + const responseMessage = { + id: "assistant-msg-xyz", + role: "assistant", + parts: [{ type: "text", text: "Hello!" }], + // no metadata + }; + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessage as never, + }); + + await runAgentWorkflow(baseInput); + + expect(handleChatCredits).toHaveBeenCalledTimes(1); + expect(handleChatCredits).toHaveBeenCalledWith({ + accountId: "acc-1", + model: "anthropic/claude-haiku-4.5", + source: "api", + gatewayCostUsd: undefined, + usage: { inputTokens: 0, cachedInputTokens: 0, outputTokens: 0 }, + }); + }); + + it("does NOT call handleChatCredits when runAgentStep returns no responseMessage", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); + + await runAgentWorkflow(baseInput); + + expect(handleChatCredits).not.toHaveBeenCalled(); + }); + + it("does NOT call handleChatCredits when runAgentStep throws (no message to bill)", async () => { + vi.mocked(runAgentStep).mockRejectedValue(new Error("model exploded")); + + await expect(runAgentWorkflow(baseInput)).rejects.toThrow("model exploded"); + + expect(handleChatCredits).not.toHaveBeenCalled(); + }); }); diff --git a/app/lib/workflows/runAgentWorkflow.ts b/app/lib/workflows/runAgentWorkflow.ts index e4e628e96..67eb94b24 100644 --- a/app/lib/workflows/runAgentWorkflow.ts +++ b/app/lib/workflows/runAgentWorkflow.ts @@ -1,16 +1,31 @@ import { getWorkflowMetadata, getWritable } from "workflow"; -import type { UIMessage, UIMessageChunk } from "ai"; +import type { LanguageModelUsage, UIMessage, UIMessageChunk } from "ai"; import { closeChatStream } from "@/app/lib/workflows/closeChatStream"; import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; import { clearChatActiveStream } from "@/lib/chat/clearChatActiveStream"; import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; +import { handleChatCredits } from "@/lib/credits/handleChatCredits"; +import type { AgentMessageMetadata } from "@/lib/agent/messageMetadata/AgentMessageMetadata"; import type { DurableAgentContext } from "@/lib/agent/tools/AgentContext"; +const ZERO_USAGE: LanguageModelUsage = { + inputTokens: 0, + cachedInputTokens: 0, + outputTokens: 0, +} as LanguageModelUsage; + export type RunAgentWorkflowInput = { messages: UIMessage[]; chatId: string; sessionId: string; + /** + * Authenticated account whose wallet absorbs the turn's cost. Resolved by + * the route handler via `validateChatWorkflow` so we never trust a + * caller-supplied id. Threaded into `recordChatUsage` after the assistant + * message is persisted. + */ + accountId: string; modelId: string; /** * JSON-serializable subset of AgentContext that survives the durable @@ -82,6 +97,24 @@ export async function runAgentWorkflow(input: RunAgentWorkflowInput): Promise { expect(mockGetCreditUsage).toHaveBeenCalledWith( { promptTokens: 1000, completionTokens: 500 }, "gpt-4", + undefined, ); expect(mockRecordCreditDeduction).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/lib/chat/handleChatWorkflowStream.ts b/lib/chat/handleChatWorkflowStream.ts index 20f048a5c..27961b126 100644 --- a/lib/chat/handleChatWorkflowStream.ts +++ b/lib/chat/handleChatWorkflowStream.ts @@ -122,6 +122,7 @@ export async function handleChatWorkflowStream(request: NextRequest): Promise { expect(cost).toBe(0); }); }); + + describe("gateway cost short-circuit", () => { + it("returns gatewayCostUsd directly when it is a positive number (skips catalog lookup)", async () => { + const cost = await getCreditUsage( + { inputTokens: 1000, outputTokens: 500 }, + "anthropic/claude-haiku-4.5", + 0.07, + ); + expect(cost).toBe(0.07); + expect(mockGetModel).not.toHaveBeenCalled(); + }); + + it("falls through to token math when gatewayCostUsd is undefined", async () => { + mockGetModel.mockResolvedValue({ + id: "gpt-4", + pricing: { input: "0.00003", output: "0.00006" }, + } as any); + + const cost = await getCreditUsage( + { inputTokens: 1000, outputTokens: 500 }, + "gpt-4", + undefined, + ); + // 1000 * 0.00003 + 500 * 0.00006 = 0.06 + expect(cost).toBeCloseTo(0.06); + expect(mockGetModel).toHaveBeenCalledWith("gpt-4"); + }); + + it("falls through to token math when gatewayCostUsd is 0", async () => { + mockGetModel.mockResolvedValue({ + id: "gpt-4", + pricing: { input: "0.00003", output: "0.00006" }, + } as any); + + const cost = await getCreditUsage({ inputTokens: 1000, outputTokens: 500 }, "gpt-4", 0); + expect(cost).toBeCloseTo(0.06); + }); + + it("falls through to token math when gatewayCostUsd is negative", async () => { + mockGetModel.mockResolvedValue({ + id: "gpt-4", + pricing: { input: "0.00003", output: "0.00006" }, + } as any); + + const cost = await getCreditUsage({ inputTokens: 1000, outputTokens: 500 }, "gpt-4", -1); + expect(cost).toBeCloseTo(0.06); + }); + + it("falls through to token math when gatewayCostUsd is NaN", async () => { + mockGetModel.mockResolvedValue({ + id: "gpt-4", + pricing: { input: "0.00003", output: "0.00006" }, + } as any); + + const cost = await getCreditUsage( + { inputTokens: 1000, outputTokens: 500 }, + "gpt-4", + Number.NaN, + ); + expect(cost).toBeCloseTo(0.06); + }); + + it("falls through to token math when gatewayCostUsd is Infinity", async () => { + mockGetModel.mockResolvedValue({ + id: "gpt-4", + pricing: { input: "0.00003", output: "0.00006" }, + } as any); + + const cost = await getCreditUsage( + { inputTokens: 1000, outputTokens: 500 }, + "gpt-4", + Number.POSITIVE_INFINITY, + ); + expect(cost).toBeCloseTo(0.06); + }); + }); }); diff --git a/lib/credits/__tests__/handleChatCredits.test.ts b/lib/credits/__tests__/handleChatCredits.test.ts index 0e92f352e..c1786fc8f 100644 --- a/lib/credits/__tests__/handleChatCredits.test.ts +++ b/lib/credits/__tests__/handleChatCredits.test.ts @@ -42,7 +42,7 @@ describe("handleChatCredits", () => { accountId: "account-123", }); - expect(mockGetCreditUsage).toHaveBeenCalledWith(USAGE, "gpt-4"); + expect(mockGetCreditUsage).toHaveBeenCalledWith(USAGE, "gpt-4", undefined); expect(mockRecordCreditDeduction).toHaveBeenCalledWith({ accountId: "account-123", creditsToDeduct: 5, @@ -154,4 +154,51 @@ describe("handleChatCredits", () => { expect(consoleSpy).toHaveBeenCalledWith("Failed to handle chat credits:", expect.any(Error)); }); }); + + describe("gateway cost + source extensions", () => { + it("forwards gatewayCostUsd to getCreditUsage when provided", async () => { + mockGetCreditUsage.mockResolvedValue(0.07); + mockRecordCreditDeduction.mockResolvedValue({ success: true, newBalance: 93 }); + + await handleChatCredits({ + usage: USAGE, + model: "anthropic/claude-haiku-4.5", + accountId: "account-123", + gatewayCostUsd: 0.07, + }); + + expect(mockGetCreditUsage).toHaveBeenCalledWith(USAGE, "anthropic/claude-haiku-4.5", 0.07); + }); + + it("defaults source to 'web' when not provided (backwards compatible)", async () => { + mockGetCreditUsage.mockResolvedValue(0.05); + mockRecordCreditDeduction.mockResolvedValue({ success: true, newBalance: 95 }); + + await handleChatCredits({ + usage: USAGE, + model: "gpt-4", + accountId: "account-123", + }); + + expect(mockRecordCreditDeduction).toHaveBeenCalledWith( + expect.objectContaining({ source: "web" }), + ); + }); + + it("propagates source='api' when caller is the chat workflow", async () => { + mockGetCreditUsage.mockResolvedValue(0.05); + mockRecordCreditDeduction.mockResolvedValue({ success: true, newBalance: 95 }); + + await handleChatCredits({ + usage: USAGE, + model: "anthropic/claude-haiku-4.5", + accountId: "account-123", + source: "api", + }); + + expect(mockRecordCreditDeduction).toHaveBeenCalledWith( + expect.objectContaining({ source: "api" }), + ); + }); + }); }); diff --git a/lib/credits/__tests__/recordCreditDeduction.test.ts b/lib/credits/__tests__/recordCreditDeduction.test.ts index 9cdfa4eba..95910af4e 100644 --- a/lib/credits/__tests__/recordCreditDeduction.test.ts +++ b/lib/credits/__tests__/recordCreditDeduction.test.ts @@ -1,17 +1,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { recordCreditDeduction } from "@/lib/credits/recordCreditDeduction"; -const { deductCreditsMock, insertUsageEventMock } = vi.hoisted(() => ({ - deductCreditsMock: vi.fn(), - insertUsageEventMock: vi.fn(), +const { deductCreditsWithAuditMock } = vi.hoisted(() => ({ + deductCreditsWithAuditMock: vi.fn(), })); -vi.mock("@/lib/credits/deductCredits", () => ({ - deductCredits: deductCreditsMock, -})); - -vi.mock("@/lib/supabase/usage_events/insertUsageEvent", () => ({ - insertUsageEvent: insertUsageEventMock, +vi.mock("@/lib/supabase/credits_usage/deductCreditsWithAudit", () => ({ + deductCreditsWithAudit: deductCreditsWithAuditMock, })); const ACCOUNT = "123e4567-e89b-12d3-a456-426614174000"; @@ -21,9 +16,8 @@ describe("recordCreditDeduction", () => { vi.clearAllMocks(); }); - it("deducts credits then inserts a usage_events row carrying token detail", async () => { - deductCreditsMock.mockResolvedValue({ success: true, newBalance: 100 }); - insertUsageEventMock.mockResolvedValue({ id: "abc" }); + it("calls the atomic RPC with cents + event payload carrying token detail", async () => { + deductCreditsWithAuditMock.mockResolvedValue({ ok: true }); const result = await recordCreditDeduction({ accountId: ACCOUNT, @@ -37,13 +31,13 @@ describe("recordCreditDeduction", () => { toolCallCount: 3, }); - expect(deductCreditsMock).toHaveBeenCalledWith({ - accountId: ACCOUNT, - creditsToDeduct: 250, - }); - expect(insertUsageEventMock).toHaveBeenCalledWith({ - account_id: ACCOUNT, - credits_deducted_cents: 250, + expect(deductCreditsWithAuditMock).toHaveBeenCalledTimes(1); + const args = deductCreditsWithAuditMock.mock.calls[0]?.[0]; + expect(args.accountId).toBe(ACCOUNT); + expect(args.cents).toBe(250); + expect(typeof args.eventId).toBe("string"); + expect(args.eventId.length).toBeGreaterThan(0); + expect(args.event).toEqual({ source: "web", agent_type: "main", provider: "anthropic", @@ -53,12 +47,11 @@ describe("recordCreditDeduction", () => { output_tokens: 567, tool_call_count: 3, }); - expect(result).toEqual({ success: true, newBalance: 100 }); + expect(result).toEqual({ success: true }); }); - it("applies defaults (agent_type='main', zero tokens, null model/provider) when token detail is omitted", async () => { - deductCreditsMock.mockResolvedValue({ success: true, newBalance: 95 }); - insertUsageEventMock.mockResolvedValue({ id: "abc" }); + it("applies defaults (agent_type='main', zero tokens, undefined model/provider) when token detail is omitted", async () => { + deductCreditsWithAuditMock.mockResolvedValue({ ok: true }); await recordCreditDeduction({ accountId: ACCOUNT, @@ -66,13 +59,12 @@ describe("recordCreditDeduction", () => { source: "api", }); - expect(insertUsageEventMock).toHaveBeenCalledWith({ - account_id: ACCOUNT, - credits_deducted_cents: 5, + const args = deductCreditsWithAuditMock.mock.calls[0]?.[0]; + expect(args.event).toEqual({ source: "api", agent_type: "main", - provider: null, - model_id: null, + provider: undefined, + model_id: undefined, input_tokens: 0, cached_input_tokens: 0, output_tokens: 0, @@ -80,33 +72,35 @@ describe("recordCreditDeduction", () => { }); }); - it("does not insert the audit row when the wallet deduction fails", async () => { - deductCreditsMock.mockRejectedValue(new Error("Insufficient credits")); + it("returns { success: false } and logs when the RPC reports an error (does NOT throw)", async () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + deductCreditsWithAuditMock.mockResolvedValue({ + ok: false, + error: "credits_usage row not found", + }); - await expect( - recordCreditDeduction({ - accountId: ACCOUNT, - creditsToDeduct: 50, - source: "web", - }), - ).rejects.toThrow(/Insufficient credits/); + const result = await recordCreditDeduction({ + accountId: ACCOUNT, + creditsToDeduct: 50, + source: "web", + }); - expect(insertUsageEventMock).not.toHaveBeenCalled(); + expect(result).toEqual({ success: false }); + expect(consoleSpy).toHaveBeenCalled(); + consoleSpy.mockRestore(); }); - it("returns the deduction result even if the audit insert fails (logs and swallows)", async () => { + it("does not throw when the RPC wrapper itself rejects (defense in depth)", async () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - deductCreditsMock.mockResolvedValue({ success: true, newBalance: 100 }); - insertUsageEventMock.mockRejectedValue(new Error("db down")); + deductCreditsWithAuditMock.mockRejectedValue(new Error("network blip")); const result = await recordCreditDeduction({ accountId: ACCOUNT, creditsToDeduct: 5, - source: "api", + source: "web", }); - expect(result).toEqual({ success: true, newBalance: 100 }); - expect(consoleSpy).toHaveBeenCalled(); + expect(result).toEqual({ success: false }); consoleSpy.mockRestore(); }); }); diff --git a/lib/credits/getCreditUsage.ts b/lib/credits/getCreditUsage.ts index 4e1dc5b28..26eb9180b 100644 --- a/lib/credits/getCreditUsage.ts +++ b/lib/credits/getCreditUsage.ts @@ -3,14 +3,31 @@ import { LanguageModelUsage } from "ai"; /** * Calculates the total spend in USD for a given language model usage. + * + * Resolution order: + * 1. `gatewayCostUsd` — gateway-reported actual cost from + * `responseMessage.metadata.totalMessageCost`. Used directly when + * present and positive so the wallet debit converges with the cost + * label the chat UI shows next to the assistant response. + * 2. Token-based estimate using `model.pricing.input/output` from + * the gateway catalog (`getModel`). Authoritative for token cost. + * 3. `0` when nothing prices the turn (caller floors to the 1c + * minimum via `Math.max(1, Math.round(usd * 100))`). + * * @param usage - The language model usage data * @param modelId - The ID of the model used + * @param gatewayCostUsd - Optional gateway-reported USD cost (preferred over token math) * @returns The total spend in USD or 0 if calculation fails */ export const getCreditUsage = async ( usage: LanguageModelUsage, modelId: string, + gatewayCostUsd?: number, ): Promise => { + if (typeof gatewayCostUsd === "number" && Number.isFinite(gatewayCostUsd) && gatewayCostUsd > 0) { + return gatewayCostUsd; + } + try { const model = await getModel(modelId); if (!model) { diff --git a/lib/credits/handleChatCredits.ts b/lib/credits/handleChatCredits.ts index bc2ecff9f..061043420 100644 --- a/lib/credits/handleChatCredits.ts +++ b/lib/credits/handleChatCredits.ts @@ -6,19 +6,42 @@ interface HandleChatCreditsParams { usage: LanguageModelUsage; model: string; accountId?: string; + /** + * Gateway-reported total USD cost for this turn (from + * `responseMessage.metadata.totalMessageCost`). When present and + * positive, used directly instead of the token-based estimate so + * the wallet debit converges with the cost label the chat UI shows. + */ + gatewayCostUsd?: number; + /** + * Which surface generated the turn — used to label the + * `usage_events` audit row. Defaults to `"web"` to preserve the + * existing `handleChatStream` contract; the chat-workflow path + * (`/api/chat/workflow`) passes `"api"` so admin dashboards can + * distinguish surface in spend rollups. + */ + source?: "web" | "api"; } /** * Handles credit deduction after chat completion. * Always deducts at least 1 credit when accountId is present (round up from usage cost). - * @param usage - The language model usage data - * @param model - The model ID used for the chat - * @param accountId - The account ID to deduct credits from (optional) + * + * Resolution order for the per-turn cost: + * 1. `gatewayCostUsd` — gateway-reported actual USD (preferred) + * 2. Token-based estimate via `getCreditUsage` against the gateway catalog + * 3. 1c floor — every successful turn debits at least 1 cent + * + * Wallet debit + audit row insert are atomic via + * `recordCreditDeduction` (backed by the `deduct_credits_with_audit` + * Postgres function). */ export const handleChatCredits = async ({ usage, model, accountId, + gatewayCostUsd, + source = "web", }: HandleChatCreditsParams): Promise => { if (!accountId) { console.error("No account ID provided, skipping credit deduction"); @@ -26,13 +49,13 @@ export const handleChatCredits = async ({ } try { - const usageCost = await getCreditUsage(usage, model); + const usageCost = await getCreditUsage(usage, model, gatewayCostUsd); const creditsToDeduct = Math.max(1, Math.round(usageCost * 100)); await recordCreditDeduction({ accountId, creditsToDeduct, - source: "web", + source, modelId: model, inputTokens: usage.inputTokens, outputTokens: usage.outputTokens, diff --git a/lib/credits/recordCreditDeduction.ts b/lib/credits/recordCreditDeduction.ts index b2db46803..3bf61ecce 100644 --- a/lib/credits/recordCreditDeduction.ts +++ b/lib/credits/recordCreditDeduction.ts @@ -1,5 +1,5 @@ -import { deductCredits } from "./deductCredits"; -import { insertUsageEvent } from "@/lib/supabase/usage_events/insertUsageEvent"; +import { nanoid } from "nanoid"; +import { deductCreditsWithAudit } from "@/lib/supabase/credits_usage/deductCreditsWithAudit"; interface RecordCreditDeductionParams { accountId: string; @@ -16,42 +16,50 @@ interface RecordCreditDeductionParams { interface RecordCreditDeductionResult { success: boolean; - newBalance?: number; } /** - * Wallet + meter wrapper. Debits the credits_usage balance via deductCredits, - * then writes a usage_events row recording the wallet impact alongside any - * token detail the caller has. + * Wallet + meter atomic wrapper. Calls the `deduct_credits_with_audit` + * Postgres function (recoupable/database#26) which runs the + * `credits_usage` debit and the `usage_events` insert inside a single + * transaction — either both writes commit or neither does. Eliminates + * the wallet/meter drift risk the previous (two separate Supabase + * calls) implementation had. * - * If the audit insert fails, the deduction is preserved (already committed) - * and the error is logged but not surfaced — the wallet stays authoritative - * and a reconciliation job can recover the missing audit row later. + * Errors are caught and surfaced via `{ success: false }` so the + * caller (the chat workflow or any research handler) never aborts on + * a credit-accounting hiccup. The wallet stays authoritative — if the + * RPC rejects, neither write happened, and the caller can decide + * whether to retry or move on. */ export const recordCreditDeduction = async ( params: RecordCreditDeductionParams, ): Promise => { - const result = await deductCredits({ - accountId: params.accountId, - creditsToDeduct: params.creditsToDeduct, - }); - + "use step"; try { - await insertUsageEvent({ - account_id: params.accountId, - credits_deducted_cents: params.creditsToDeduct, - source: params.source, - agent_type: params.agentType ?? "main", - provider: params.provider ?? null, - model_id: params.modelId ?? null, - input_tokens: params.inputTokens ?? 0, - cached_input_tokens: params.cachedInputTokens ?? 0, - output_tokens: params.outputTokens ?? 0, - tool_call_count: params.toolCallCount ?? 0, + const result = await deductCreditsWithAudit({ + accountId: params.accountId, + cents: params.creditsToDeduct, + eventId: nanoid(), + event: { + source: params.source, + agent_type: params.agentType ?? "main", + provider: params.provider, + model_id: params.modelId, + input_tokens: params.inputTokens ?? 0, + cached_input_tokens: params.cachedInputTokens ?? 0, + output_tokens: params.outputTokens ?? 0, + tool_call_count: params.toolCallCount ?? 0, + }, }); + + if (!result.ok) { + console.error("[recordCreditDeduction] atomic debit failed:", result.error); + return { success: false }; + } + return { success: true }; } catch (error) { - console.error("Failed to insert usage_events row (wallet was still debited):", error); + console.error("[recordCreditDeduction] unexpected error:", error); + return { success: false }; } - - return result; }; diff --git a/lib/supabase/credits_usage/__tests__/deductCreditsWithAudit.test.ts b/lib/supabase/credits_usage/__tests__/deductCreditsWithAudit.test.ts new file mode 100644 index 000000000..c2157bd88 --- /dev/null +++ b/lib/supabase/credits_usage/__tests__/deductCreditsWithAudit.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { deductCreditsWithAudit } from "@/lib/supabase/credits_usage/deductCreditsWithAudit"; +import supabase from "@/lib/supabase/serverClient"; + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { rpc: vi.fn() }, +})); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +const ACCOUNT = "11111111-1111-1111-1111-111111111111"; +const validEvent = { + source: "api" as const, + agent_type: "main" as const, + provider: "anthropic", + model_id: "anthropic/claude-haiku-4.5", + input_tokens: 42, + cached_input_tokens: 10, + output_tokens: 13, + tool_call_count: 1, +}; + +describe("deductCreditsWithAudit", () => { + it("calls the deduct_credits_with_audit RPC with the right param names", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: null, error: null } as never); + + await deductCreditsWithAudit({ + accountId: ACCOUNT, + cents: 7, + eventId: "evt_abc", + event: validEvent, + }); + + expect(supabase.rpc).toHaveBeenCalledTimes(1); + expect(supabase.rpc).toHaveBeenCalledWith("deduct_credits_with_audit", { + p_account_id: ACCOUNT, + p_amount: 7, + p_event_id: "evt_abc", + p_event: validEvent, + }); + }); + + it("returns { ok: true } when the RPC succeeds", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: null, error: null } as never); + + const result = await deductCreditsWithAudit({ + accountId: ACCOUNT, + cents: 7, + eventId: "evt_abc", + event: validEvent, + }); + + expect(result).toEqual({ ok: true }); + }); + + it("returns { ok: false, error } when the RPC returns an error (does NOT throw)", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ + data: null, + error: { message: "credits_usage row not found" }, + } as never); + + const result = await deductCreditsWithAudit({ + accountId: ACCOUNT, + cents: 7, + eventId: "evt_abc", + event: validEvent, + }); + + expect(result).toEqual({ ok: false, error: "credits_usage row not found" }); + }); + + it("returns { ok: false, error } when the rpc call throws (network failure)", async () => { + vi.mocked(supabase.rpc).mockRejectedValue(new Error("network blip")); + + const result = await deductCreditsWithAudit({ + accountId: ACCOUNT, + cents: 7, + eventId: "evt_abc", + event: validEvent, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("network blip"); + } + }); +}); diff --git a/lib/supabase/credits_usage/deductCreditsWithAudit.ts b/lib/supabase/credits_usage/deductCreditsWithAudit.ts new file mode 100644 index 000000000..871800f73 --- /dev/null +++ b/lib/supabase/credits_usage/deductCreditsWithAudit.ts @@ -0,0 +1,73 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Json } from "@/types/database.types"; + +/** + * JSON payload populating the `usage_events` audit row. Optional + * fields fall back to column defaults (`source='api'`, + * `agent_type='main'`, provider/model_id NULL, counts 0) inside the + * SQL function — see `database/supabase/migrations/20260525000000_deduct_credits_with_audit.sql`. + */ +export interface DeductCreditsAuditEvent { + source?: "api" | "web"; + agent_type?: "main" | "subagent"; + provider?: string; + model_id?: string; + input_tokens?: number; + cached_input_tokens?: number; + output_tokens?: number; + tool_call_count?: number; +} + +export interface DeductCreditsWithAuditResult { + ok: boolean; + /** + * Present only when `ok === false`. Kept optional rather than as a + * discriminated union so callers can read `result.error` without + * tripping a narrowing edge case in the Next.js 16 type checker. + */ + error?: string; +} + +/** + * Atomically debits `credits_usage.remaining_credits` and inserts the + * corresponding `usage_events` audit row via the + * `deduct_credits_with_audit` Postgres function. The SQL function + * runs both writes inside an implicit transaction so wallet/meter + * can never drift on partial failure — matching open-agents' + * `recordUsage` `db.transaction(...)` guarantee. + * + * Caller convention: `cents` is the amount to debit (integer ≥ 1). + * `eventId` should match `lib/supabase/usage_events/insertUsageEvent.ts`'s + * nanoid convention so the audit trail is consistent. + * + * Errors are never thrown — returns `{ ok: false, error }` instead so + * the caller (`recordChatUsage`) can swallow without aborting the + * chat workflow on a credit accounting hiccup. + */ +export async function deductCreditsWithAudit(params: { + accountId: string; + cents: number; + eventId: string; + event: DeductCreditsAuditEvent; +}): Promise { + try { + const { error } = await supabase.rpc("deduct_credits_with_audit", { + p_account_id: params.accountId, + p_amount: params.cents, + p_event_id: params.eventId, + // DeductCreditsAuditEvent is structurally JSON-safe, but TS can't + // infer an interface → index-signature assignment automatically. + // Cast once at this boundary; the runtime payload is unchanged. + p_event: params.event as unknown as Json, + }); + if (error) { + return { ok: false, error: error.message }; + } + return { ok: true }; + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : String(error), + }; + } +} diff --git a/types/database.types.ts b/types/database.types.ts index 20287a6a9..cc7d03e1e 100644 --- a/types/database.types.ts +++ b/types/database.types.ts @@ -3961,6 +3961,15 @@ export type Database = { Args: { account_id: string; amount: number }; Returns: undefined; }; + deduct_credits_with_audit: { + Args: { + p_account_id: string; + p_amount: number; + p_event: Json; + p_event_id: string; + }; + Returns: undefined; + }; extract_domain: { Args: { email: string }; Returns: string }; get_account_invitations: { Args: { account_slug: string }; From 86295b8bb29fa5e1569342f2ea5bae26d0a5b7a3 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Mon, 25 May 2026 18:58:27 -0500 Subject: [PATCH 3/4] feat(chat-workflow): auto-commit + push after natural finish (#614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 ''` - `git rev-parse HEAD` + `git symbolic-ref --short HEAD` - `GIT_TERMINAL_PROMPT=0 git push -u origin ` 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) * 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) * 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 " 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) * 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) * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../__tests__/runAgentWorkflow.test.ts | 89 ++++++ app/lib/workflows/runAgentWorkflow.ts | 32 +++ .../persistAssistantDataPart.test.ts | 77 ++++++ .../__tests__/upsertAssistantDataPart.test.ts | 81 ++++++ .../__tests__/autoCommitChatTurn.test.ts | 162 +++++++++++ .../__tests__/buildCommitData.test.ts | 113 ++++++++ .../__tests__/generateCommitMessage.test.ts | 79 ++++++ .../__tests__/hasAutoCommitChanges.test.ts | 58 ++++ .../__tests__/performAutoCommit.test.ts | 259 ++++++++++++++++++ .../__tests__/runAutoCommit.test.ts | 86 ++++++ .../__tests__/sendCommitChunk.test.ts | 51 ++++ lib/chat/auto-commit/autoCommitChatTurn.ts | 95 +++++++ lib/chat/auto-commit/buildCommitData.ts | 85 ++++++ lib/chat/auto-commit/generateCommitMessage.ts | 62 +++++ lib/chat/auto-commit/hasAutoCommitChanges.ts | 47 ++++ lib/chat/auto-commit/performAutoCommit.ts | 128 +++++++++ lib/chat/auto-commit/runAutoCommit.ts | 50 ++++ lib/chat/auto-commit/sendCommitChunk.ts | 25 ++ lib/chat/handleChatWorkflowStream.ts | 11 + lib/chat/persistAssistantDataPart.ts | 55 ++++ lib/chat/upsertAssistantDataPart.ts | 44 +++ .../parseGitHubRepoIdentifiers.test.ts | 49 ++++ lib/github/parseGitHubRepoIdentifiers.ts | 32 +++ .../__tests__/updateChatMessage.test.ts | 49 ++++ .../chat_messages/updateChatMessage.ts | 58 ++++ 25 files changed, 1877 insertions(+) create mode 100644 lib/chat/__tests__/persistAssistantDataPart.test.ts create mode 100644 lib/chat/__tests__/upsertAssistantDataPart.test.ts create mode 100644 lib/chat/auto-commit/__tests__/autoCommitChatTurn.test.ts create mode 100644 lib/chat/auto-commit/__tests__/buildCommitData.test.ts create mode 100644 lib/chat/auto-commit/__tests__/generateCommitMessage.test.ts create mode 100644 lib/chat/auto-commit/__tests__/hasAutoCommitChanges.test.ts create mode 100644 lib/chat/auto-commit/__tests__/performAutoCommit.test.ts create mode 100644 lib/chat/auto-commit/__tests__/runAutoCommit.test.ts create mode 100644 lib/chat/auto-commit/__tests__/sendCommitChunk.test.ts create mode 100644 lib/chat/auto-commit/autoCommitChatTurn.ts create mode 100644 lib/chat/auto-commit/buildCommitData.ts create mode 100644 lib/chat/auto-commit/generateCommitMessage.ts create mode 100644 lib/chat/auto-commit/hasAutoCommitChanges.ts create mode 100644 lib/chat/auto-commit/performAutoCommit.ts create mode 100644 lib/chat/auto-commit/runAutoCommit.ts create mode 100644 lib/chat/auto-commit/sendCommitChunk.ts create mode 100644 lib/chat/persistAssistantDataPart.ts create mode 100644 lib/chat/upsertAssistantDataPart.ts create mode 100644 lib/github/__tests__/parseGitHubRepoIdentifiers.test.ts create mode 100644 lib/github/parseGitHubRepoIdentifiers.ts create mode 100644 lib/supabase/chat_messages/__tests__/updateChatMessage.test.ts create mode 100644 lib/supabase/chat_messages/updateChatMessage.ts diff --git a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts index 19cec3b78..3697c84a9 100644 --- a/app/lib/workflows/__tests__/runAgentWorkflow.test.ts +++ b/app/lib/workflows/__tests__/runAgentWorkflow.test.ts @@ -6,6 +6,7 @@ import { closeChatStream } from "@/app/lib/workflows/closeChatStream"; import { generateAssistantMessageId } from "@/app/lib/workflows/generateAssistantMessageId"; import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; import { handleChatCredits } from "@/lib/credits/handleChatCredits"; +import { autoCommitChatTurn } from "@/lib/chat/auto-commit/autoCommitChatTurn"; vi.mock("@/app/lib/workflows/runAgentStep", () => ({ runAgentStep: vi.fn(), @@ -25,6 +26,9 @@ vi.mock("@/lib/chat/persistAssistantMessage", () => ({ vi.mock("@/lib/credits/handleChatCredits", () => ({ handleChatCredits: vi.fn(), })); +vi.mock("@/lib/chat/auto-commit/autoCommitChatTurn", () => ({ + autoCommitChatTurn: vi.fn(), +})); // Captured writable stub so tests can assert closeChatStream got the // same instance the workflow body holds. const writableStub = new WritableStream(); @@ -49,11 +53,24 @@ const baseInput = { sessionId: "session-1", accountId: "acc-1", modelId: "anthropic/claude-haiku-4.5", + sessionTitle: "test session", + repoOwner: "recoupable", + repoName: "api", agentContext: { sandbox: { state: { type: "vercel" }, workingDirectory: "/sandbox/mono" }, } as never, }; +const responseMessageWithMetadata = { + id: "asst-msg-1", + role: "assistant", + parts: [{ type: "text", text: "Hello!" }], + metadata: { + totalMessageCost: 0.07, + totalMessageUsage: { inputTokens: 100, cachedInputTokens: 10, outputTokens: 20 }, + }, +} as never; + describe("runAgentWorkflow", () => { it("clears active_stream_id after a successful run, using the workflow's own runId", async () => { vi.mocked(runAgentStep).mockResolvedValue({ @@ -253,4 +270,76 @@ describe("runAgentWorkflow", () => { expect(handleChatCredits).not.toHaveBeenCalled(); }); + + describe("auto-commit", () => { + // The auto-commit flow itself is exhaustively covered in + // `lib/chat/auto-commit/__tests__/autoCommitChatTurn.test.ts`. + // These tests only verify the workflow body wires the orchestrator + // up with the right inputs. + + it("calls autoCommitChatTurn with workflow context after persistAssistantMessage", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessageWithMetadata, + }); + + await runAgentWorkflow(baseInput); + + expect(autoCommitChatTurn).toHaveBeenCalledTimes(1); + // Workflow spreads `...input, ...result` into the call so any + // future fields are forwarded automatically. Only assert on the + // fields autoCommitChatTurn actually consumes — extra fields + // from input/result are fine to pass through. + expect(autoCommitChatTurn).toHaveBeenCalledWith( + expect.objectContaining({ + writable: writableStub, + responseMessage: responseMessageWithMetadata, + finishReason: "stop", + sessionId: "session-1", + sessionTitle: "test session", + repoOwner: "recoupable", + repoName: "api", + sandboxState: { type: "vercel", ...baseInput.agentContext.sandbox.state }, + }), + ); + }); + + it("wraps the raw VercelState with `type: 'vercel'` before forwarding", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessageWithMetadata, + }); + + await runAgentWorkflow(baseInput); + + const call = vi.mocked(autoCommitChatTurn).mock.calls[0]?.[0]; + expect(call?.sandboxState).toMatchObject({ type: "vercel" }); + }); + + it("forwards undefined sandboxState when agentContext.sandbox is missing", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: responseMessageWithMetadata, + }); + + await runAgentWorkflow({ + ...baseInput, + agentContext: {} as never, + }); + + const call = vi.mocked(autoCommitChatTurn).mock.calls[0]?.[0]; + expect(call?.sandboxState).toBeUndefined(); + }); + + it("does NOT call autoCommitChatTurn when runAgentStep returns no responseMessage", async () => { + vi.mocked(runAgentStep).mockResolvedValue({ + finishReason: "stop", + responseMessage: undefined, + }); + + await runAgentWorkflow(baseInput); + + expect(autoCommitChatTurn).not.toHaveBeenCalled(); + }); + }); }); diff --git a/app/lib/workflows/runAgentWorkflow.ts b/app/lib/workflows/runAgentWorkflow.ts index 67eb94b24..58431094c 100644 --- a/app/lib/workflows/runAgentWorkflow.ts +++ b/app/lib/workflows/runAgentWorkflow.ts @@ -6,6 +6,7 @@ import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; import { clearChatActiveStream } from "@/lib/chat/clearChatActiveStream"; import { persistAssistantMessage } from "@/lib/chat/persistAssistantMessage"; import { handleChatCredits } from "@/lib/credits/handleChatCredits"; +import { autoCommitChatTurn } from "@/lib/chat/auto-commit/autoCommitChatTurn"; import type { AgentMessageMetadata } from "@/lib/agent/messageMetadata/AgentMessageMetadata"; import type { DurableAgentContext } from "@/lib/agent/tools/AgentContext"; @@ -27,6 +28,20 @@ export type RunAgentWorkflowInput = { */ accountId: string; modelId: string; + /** + * Optional chat title — used as context for the auto-commit + * message-generation LLM call. + */ + sessionTitle?: string; + /** + * Repo identifiers from `sessions.repo_owner` / `sessions.repo_name`. + * When BOTH are present and the sandbox is reachable, the workflow + * runs auto-commit after a successful turn (git add → LLM-generated + * commit message → git commit → git push). Either being absent + * skips auto-commit silently. + */ + repoOwner?: string; + repoName?: string; /** * JSON-serializable subset of AgentContext that survives the durable * workflow input. `runAgentStep` attaches the constructed `model` @@ -115,6 +130,23 @@ export async function runAgentWorkflow(input: RunAgentWorkflowInput): Promise ({ + updateChatMessage: vi.fn(), +})); + +const baseMessage = { + id: "msg_1", + role: "assistant" as const, + parts: [{ type: "text", text: "Hello" }] as never[], +}; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(updateChatMessage).mockResolvedValue({ ok: true }); +}); + +describe("persistAssistantDataPart", () => { + it("merges the part into the message and writes the WHOLE merged message via updateChatMessage", async () => { + const part = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "success" as const }, + }; + + await persistAssistantDataPart(baseMessage, part); + + expect(updateChatMessage).toHaveBeenCalledTimes(1); + const [id, written] = vi.mocked(updateChatMessage).mock.calls[0]!; + expect(id).toBe("msg_1"); + const message = written as typeof baseMessage; + expect(message.id).toBe("msg_1"); + expect(message.role).toBe("assistant"); + expect(message.parts).toHaveLength(2); + expect(message.parts[1]).toEqual(part); + }); + + it("replaces an existing data-part with matching {type, id} (pending → success transition)", async () => { + const messageWithPending = { + ...baseMessage, + parts: [ + ...baseMessage.parts, + { + type: "data-commit", + id: "msg_1:commit", + data: { status: "pending" }, + }, + ], + }; + const success = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "success" as const, commitSha: "abc" }, + }; + + await persistAssistantDataPart(messageWithPending, success); + + const [, written] = vi.mocked(updateChatMessage).mock.calls[0]!; + const message = written as typeof messageWithPending; + // Still 2 parts (text + commit), not 3 + expect(message.parts).toHaveLength(2); + expect(message.parts[1]).toEqual(success); + }); + + it("does not throw when updateChatMessage rejects (caller decides what to do)", async () => { + vi.mocked(updateChatMessage).mockResolvedValue({ ok: false, error: "boom" }); + await expect( + persistAssistantDataPart(baseMessage, { + type: "data-commit", + id: "msg_1:commit", + data: { status: "success" }, + }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/lib/chat/__tests__/upsertAssistantDataPart.test.ts b/lib/chat/__tests__/upsertAssistantDataPart.test.ts new file mode 100644 index 000000000..28a832407 --- /dev/null +++ b/lib/chat/__tests__/upsertAssistantDataPart.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from "vitest"; +import { upsertAssistantDataPart } from "@/lib/chat/upsertAssistantDataPart"; + +const baseMessage = { + id: "msg_1", + role: "assistant" as const, + parts: [{ type: "text", text: "Hello" }, { type: "step-finish" }] as never[], +}; + +describe("upsertAssistantDataPart", () => { + it("appends a new part when no part with the same {type, id} exists", () => { + const part = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const result = upsertAssistantDataPart(baseMessage, part); + expect(result.parts).toHaveLength(3); + expect(result.parts[2]).toEqual(part); + }); + + it("replaces the existing part when {type, id} matches (pending → success)", () => { + const pendingPart = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const successPart = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { + status: "success" as const, + committed: true, + pushed: true, + commitSha: "abc123", + url: "https://github.com/owner/repo/commit/abc123", + }, + }; + const afterPending = upsertAssistantDataPart(baseMessage, pendingPart); + const afterSuccess = upsertAssistantDataPart(afterPending, successPart); + expect(afterSuccess.parts).toHaveLength(3); + expect(afterSuccess.parts[2]).toEqual(successPart); + }); + + it("does NOT mutate the input message", () => { + const part = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const result = upsertAssistantDataPart(baseMessage, part); + expect(baseMessage.parts).toHaveLength(2); + expect(result.parts).not.toBe(baseMessage.parts); + }); + + it("preserves the other message fields (id, role, etc.)", () => { + const part = { + type: "data-commit" as const, + id: "msg_1:commit", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const result = upsertAssistantDataPart(baseMessage, part); + expect(result.id).toBe(baseMessage.id); + expect(result.role).toBe(baseMessage.role); + }); + + it("treats different ids as different parts (appends a second one)", () => { + const partA = { + type: "data-commit" as const, + id: "msg_1:commit-a", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const partB = { + type: "data-commit" as const, + id: "msg_1:commit-b", + data: { status: "pending" as const, committed: false, pushed: false }, + }; + const result = upsertAssistantDataPart(upsertAssistantDataPart(baseMessage, partA), partB); + expect(result.parts).toHaveLength(4); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/autoCommitChatTurn.test.ts b/lib/chat/auto-commit/__tests__/autoCommitChatTurn.test.ts new file mode 100644 index 000000000..d0ab5ec96 --- /dev/null +++ b/lib/chat/auto-commit/__tests__/autoCommitChatTurn.test.ts @@ -0,0 +1,162 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { autoCommitChatTurn } from "@/lib/chat/auto-commit/autoCommitChatTurn"; +import { hasAutoCommitChanges } from "@/lib/chat/auto-commit/hasAutoCommitChanges"; +import { runAutoCommit } from "@/lib/chat/auto-commit/runAutoCommit"; +import { sendCommitChunk } from "@/lib/chat/auto-commit/sendCommitChunk"; +import { persistAssistantDataPart } from "@/lib/chat/persistAssistantDataPart"; +import type { UIMessageChunk } from "ai"; + +vi.mock("@/lib/chat/auto-commit/hasAutoCommitChanges", () => ({ + hasAutoCommitChanges: vi.fn(), +})); +vi.mock("@/lib/chat/auto-commit/runAutoCommit", () => ({ + runAutoCommit: vi.fn(), +})); +vi.mock("@/lib/chat/auto-commit/sendCommitChunk", () => ({ + sendCommitChunk: vi.fn(), +})); +vi.mock("@/lib/chat/persistAssistantDataPart", () => ({ + persistAssistantDataPart: vi.fn(), +})); + +const writable = new WritableStream(); +const baseMessage = { + id: "asst-msg-1", + role: "assistant" as const, + parts: [{ type: "text", text: "ok" }], +}; + +const baseInput = { + writable, + responseMessage: baseMessage as never, + finishReason: "stop", + sessionId: "session-1", + sessionTitle: "test", + repoOwner: "recoupable", + repoName: "api", + sandboxState: { type: "vercel" as const, sandboxId: "sb_123" }, +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("autoCommitChatTurn", () => { + describe("gating (canAutoCommit)", () => { + it("skips entirely when finishReason is 'tool-calls' (intermediate turn)", async () => { + await autoCommitChatTurn({ ...baseInput, finishReason: "tool-calls" }); + expect(hasAutoCommitChanges).not.toHaveBeenCalled(); + expect(runAutoCommit).not.toHaveBeenCalled(); + expect(sendCommitChunk).not.toHaveBeenCalled(); + expect(persistAssistantDataPart).not.toHaveBeenCalled(); + }); + + it("skips entirely when repoOwner is undefined", async () => { + await autoCommitChatTurn({ ...baseInput, repoOwner: undefined }); + expect(hasAutoCommitChanges).not.toHaveBeenCalled(); + }); + + it("skips entirely when repoName is undefined", async () => { + await autoCommitChatTurn({ ...baseInput, repoName: undefined }); + expect(hasAutoCommitChanges).not.toHaveBeenCalled(); + }); + + it("skips entirely when sandboxState is undefined", async () => { + await autoCommitChatTurn({ ...baseInput, sandboxState: undefined }); + expect(hasAutoCommitChanges).not.toHaveBeenCalled(); + }); + }); + + describe("no-changes path", () => { + it("checks for changes but emits no chunks and does not persist", async () => { + vi.mocked(hasAutoCommitChanges).mockResolvedValue(false); + await autoCommitChatTurn(baseInput); + expect(hasAutoCommitChanges).toHaveBeenCalledTimes(1); + expect(runAutoCommit).not.toHaveBeenCalled(); + expect(sendCommitChunk).not.toHaveBeenCalled(); + expect(persistAssistantDataPart).not.toHaveBeenCalled(); + }); + }); + + describe("happy path", () => { + beforeEach(() => { + vi.mocked(hasAutoCommitChanges).mockResolvedValue(true); + vi.mocked(runAutoCommit).mockResolvedValue({ + committed: true, + pushed: true, + commitSha: "abc123", + commitMessage: "feat: thing", + }); + }); + + it("emits pending → resolved chunks and persists the resolved data-commit part", async () => { + await autoCommitChatTurn(baseInput); + + expect(sendCommitChunk).toHaveBeenCalledTimes(2); + expect(sendCommitChunk).toHaveBeenNthCalledWith( + 1, + writable, + "asst-msg-1:commit", + expect.objectContaining({ status: "pending" }), + ); + expect(sendCommitChunk).toHaveBeenNthCalledWith( + 2, + writable, + "asst-msg-1:commit", + expect.objectContaining({ + status: "success", + commitSha: "abc123", + url: "https://github.com/recoupable/api/commit/abc123", + }), + ); + + expect(persistAssistantDataPart).toHaveBeenCalledTimes(1); + const [calledMessage, calledPart] = vi.mocked(persistAssistantDataPart).mock.calls[0]!; + expect((calledMessage as { id: string }).id).toBe("asst-msg-1"); + expect(calledPart).toMatchObject({ + type: "data-commit", + id: "asst-msg-1:commit", + data: { status: "success", commitSha: "abc123" }, + }); + }); + + it("forwards sessionId, sessionTitle, repos, sandboxState to runAutoCommit", async () => { + await autoCommitChatTurn(baseInput); + expect(runAutoCommit).toHaveBeenCalledWith({ + sessionId: "session-1", + sessionTitle: "test", + repoOwner: "recoupable", + repoName: "api", + sandboxState: baseInput.sandboxState, + }); + }); + + it("defaults sessionTitle to empty string when undefined", async () => { + await autoCommitChatTurn({ ...baseInput, sessionTitle: undefined }); + expect(runAutoCommit).toHaveBeenCalledWith(expect.objectContaining({ sessionTitle: "" })); + }); + }); + + describe("error path (commit failed)", () => { + it("emits resolved chunk with status='error' but does NOT skip persistence", async () => { + vi.mocked(hasAutoCommitChanges).mockResolvedValue(true); + vi.mocked(runAutoCommit).mockResolvedValue({ + committed: false, + pushed: false, + error: "Failed to stage changes", + }); + + await autoCommitChatTurn(baseInput); + + expect(sendCommitChunk).toHaveBeenNthCalledWith( + 2, + writable, + "asst-msg-1:commit", + expect.objectContaining({ status: "error", error: "Failed to stage changes" }), + ); + // We still persist the error state so the GitDataPartCard + // renders "Commit failed" on refresh. + expect(persistAssistantDataPart).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/buildCommitData.test.ts b/lib/chat/auto-commit/__tests__/buildCommitData.test.ts new file mode 100644 index 000000000..e7577497d --- /dev/null +++ b/lib/chat/auto-commit/__tests__/buildCommitData.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from "vitest"; +import { buildCommitData } from "@/lib/chat/auto-commit/buildCommitData"; + +describe("buildCommitData", () => { + describe("success path", () => { + it("returns status='success' with the commit url when the commit was pushed", () => { + const data = buildCommitData( + { + committed: true, + pushed: true, + commitSha: "abc123", + commitMessage: "feat: thing", + }, + "recoupable", + "api", + ); + expect(data).toEqual({ + status: "success", + committed: true, + pushed: true, + commitMessage: "feat: thing", + commitSha: "abc123", + url: "https://github.com/recoupable/api/commit/abc123", + }); + }); + + it("omits the url when the commit landed locally but wasn't pushed", () => { + const data = buildCommitData( + { + committed: true, + pushed: false, + commitSha: "abc123", + commitMessage: "feat: thing", + }, + "recoupable", + "api", + ); + expect(data.url).toBeUndefined(); + expect(data.status).toBe("success"); + }); + + it("omits the url when commitSha is missing (paranoia)", () => { + const data = buildCommitData( + { committed: true, pushed: true, commitMessage: "feat: thing" }, + "recoupable", + "api", + ); + expect(data.url).toBeUndefined(); + }); + }); + + describe("error path", () => { + it("returns status='error' with the error message", () => { + const data = buildCommitData( + { + committed: false, + pushed: false, + error: "Failed to stage changes", + }, + "recoupable", + "api", + ); + expect(data).toEqual({ + status: "error", + committed: false, + pushed: false, + commitMessage: undefined, + commitSha: undefined, + url: undefined, + error: "Failed to stage changes", + }); + }); + + it("still includes the url when commit pushed but result was marked error (partial success edge)", () => { + // hypothetical: commit pushed, then a later step set error + const data = buildCommitData( + { + committed: true, + pushed: true, + commitSha: "abc123", + commitMessage: "feat: thing", + error: "post-push hook failed", + }, + "recoupable", + "api", + ); + expect(data.status).toBe("error"); + expect(data.url).toBe("https://github.com/recoupable/api/commit/abc123"); + }); + }); + + describe("skipped path", () => { + it("returns status='skipped' when nothing was committed (no changes)", () => { + const data = buildCommitData({ committed: false, pushed: false }, "recoupable", "api"); + expect(data).toEqual({ + status: "skipped", + committed: false, + pushed: false, + }); + }); + }); + + describe("url encoding", () => { + it("encodes owner / repo / sha in the URL path", () => { + const data = buildCommitData( + { committed: true, pushed: true, commitSha: "abc/def" }, + "owner with space", + "repo+name", + ); + expect(data.url).toBe("https://github.com/owner%20with%20space/repo%2Bname/commit/abc%2Fdef"); + }); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/generateCommitMessage.test.ts b/lib/chat/auto-commit/__tests__/generateCommitMessage.test.ts new file mode 100644 index 000000000..34e9942d9 --- /dev/null +++ b/lib/chat/auto-commit/__tests__/generateCommitMessage.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { generateCommitMessage } from "@/lib/chat/auto-commit/generateCommitMessage"; +import generateText from "@/lib/ai/generateText"; +import type { ExecResult, Sandbox } from "@/lib/sandbox/abstraction"; + +vi.mock("@/lib/ai/generateText", () => ({ default: vi.fn() })); + +const ok = (stdout = "", stderr = ""): ExecResult => ({ + success: true, + exitCode: 0, + stdout, + stderr, + truncated: false, +}); + +function makeSandbox(handlers: Record = {}) { + const exec = vi.fn((cmd: string) => { + for (const [pattern, result] of Object.entries(handlers)) { + if (cmd.includes(pattern)) return Promise.resolve(result); + } + return Promise.resolve(ok()); + }); + return { exec } as unknown as Sandbox; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("generateCommitMessage", () => { + it("returns the LLM-generated message when the gateway succeeds", async () => { + vi.mocked(generateText).mockResolvedValue({ + text: "feat: add hello world endpoint", + } as never); + const sandbox = makeSandbox({ + "git diff --cached": ok("some diff content"), + }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "test session"); + expect(msg).toBe("feat: add hello world endpoint"); + }); + + it("falls back to default message when staged diff is empty", async () => { + const sandbox = makeSandbox({ "git diff --cached": ok("") }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "session"); + expect(msg).toBe("chore: update repository changes"); + expect(generateText).not.toHaveBeenCalled(); + }); + + it("falls back to default message when generateText rejects", async () => { + vi.mocked(generateText).mockRejectedValue(new Error("gateway down")); + const sandbox = makeSandbox({ "git diff --cached": ok("diff") }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "session"); + expect(msg).toBe("chore: update repository changes"); + }); + + it("truncates the LLM-generated message to 72 chars", async () => { + const longMessage = "feat: " + "x".repeat(200); + vi.mocked(generateText).mockResolvedValue({ text: longMessage } as never); + const sandbox = makeSandbox({ "git diff --cached": ok("diff") }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "session"); + expect(msg.length).toBeLessThanOrEqual(72); + }); + + it("takes only the first line of the LLM output (strips trailing prose)", async () => { + vi.mocked(generateText).mockResolvedValue({ + text: "feat: clean header\n\nMore details about the change.", + } as never); + const sandbox = makeSandbox({ "git diff --cached": ok("diff") }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "session"); + expect(msg).toBe("feat: clean header"); + }); + + it("falls back when LLM returns an empty string", async () => { + vi.mocked(generateText).mockResolvedValue({ text: "" } as never); + const sandbox = makeSandbox({ "git diff --cached": ok("diff") }); + const msg = await generateCommitMessage(sandbox, "/sandbox", "session"); + expect(msg).toBe("chore: update repository changes"); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/hasAutoCommitChanges.test.ts b/lib/chat/auto-commit/__tests__/hasAutoCommitChanges.test.ts new file mode 100644 index 000000000..d0e075eb8 --- /dev/null +++ b/lib/chat/auto-commit/__tests__/hasAutoCommitChanges.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { hasAutoCommitChanges } from "@/lib/chat/auto-commit/hasAutoCommitChanges"; +import { connectSandbox } from "@/lib/sandbox/factory"; + +vi.mock("@/lib/sandbox/factory", () => ({ + connectSandbox: vi.fn(), +})); + +function buildSandbox(execResult: { success: boolean; stdout: string; stderr?: string }) { + return { + type: "vercel" as const, + workingDirectory: "/sandbox/repo", + exec: vi.fn().mockResolvedValue({ + success: execResult.success, + stdout: execResult.stdout, + stderr: execResult.stderr ?? "", + exitCode: execResult.success ? 0 : 1, + truncated: false, + }), + } as never; +} + +const SANDBOX_STATE = { type: "vercel" as const, sandboxId: "sb_123" } as never; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("hasAutoCommitChanges", () => { + it("returns true when git status --porcelain has output (changes present)", async () => { + vi.mocked(connectSandbox).mockResolvedValue( + buildSandbox({ success: true, stdout: "M file.txt\n" }), + ); + expect(await hasAutoCommitChanges({ sandboxState: SANDBOX_STATE })).toBe(true); + }); + + it("returns false when git status --porcelain has empty output (no changes)", async () => { + vi.mocked(connectSandbox).mockResolvedValue(buildSandbox({ success: true, stdout: "" })); + expect(await hasAutoCommitChanges({ sandboxState: SANDBOX_STATE })).toBe(false); + }); + + it("returns false when git status --porcelain has only whitespace", async () => { + vi.mocked(connectSandbox).mockResolvedValue(buildSandbox({ success: true, stdout: " \n\n" })); + expect(await hasAutoCommitChanges({ sandboxState: SANDBOX_STATE })).toBe(false); + }); + + it("returns true (fail-open) when git status itself fails — lets runAutoCommit surface the real error", async () => { + vi.mocked(connectSandbox).mockResolvedValue( + buildSandbox({ success: false, stdout: "", stderr: "not a git repo" }), + ); + expect(await hasAutoCommitChanges({ sandboxState: SANDBOX_STATE })).toBe(true); + }); + + it("returns true (fail-open) when the sandbox connect rejects — same rationale", async () => { + vi.mocked(connectSandbox).mockRejectedValue(new Error("sandbox gone")); + expect(await hasAutoCommitChanges({ sandboxState: SANDBOX_STATE })).toBe(true); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/performAutoCommit.test.ts b/lib/chat/auto-commit/__tests__/performAutoCommit.test.ts new file mode 100644 index 000000000..8290430c0 --- /dev/null +++ b/lib/chat/auto-commit/__tests__/performAutoCommit.test.ts @@ -0,0 +1,259 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { performAutoCommit } from "@/lib/chat/auto-commit/performAutoCommit"; +import generateText from "@/lib/ai/generateText"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; +import type { ExecResult, Sandbox } from "@/lib/sandbox/abstraction"; + +vi.mock("@/lib/ai/generateText", () => ({ default: vi.fn() })); +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(), +})); + +const ok = (stdout = "", stderr = ""): ExecResult => ({ + success: true, + exitCode: 0, + stdout, + stderr, + truncated: false, +}); + +const fail = (stdout = "", stderr = ""): ExecResult => ({ + success: false, + exitCode: 1, + stdout, + stderr, + truncated: false, +}); + +function makeSandbox(handlers: Record = {}) { + const exec = vi.fn((cmd: string) => { + for (const [pattern, result] of Object.entries(handlers)) { + if (cmd.includes(pattern)) return Promise.resolve(result); + } + return Promise.resolve(ok()); + }); + const sandbox = { + type: "vercel" as const, + workingDirectory: "/sandbox/repo", + exec, + } as unknown as Sandbox; + return { sandbox, exec }; +} + +const baseParams = { + sessionId: "session-1", + sessionTitle: "test session", + repoOwner: "recoupable", + repoName: "api", +}; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + vi.mocked(generateText).mockResolvedValue({ + text: "feat: add example file", + } as never); +}); + +describe("performAutoCommit", () => { + describe("no changes path", () => { + it("returns { committed:false, pushed:false } when git status is empty", async () => { + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok(""), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result).toEqual({ committed: false, pushed: false }); + // Should not have called add/commit/push when nothing to stage + expect(exec).not.toHaveBeenCalledWith( + expect.stringContaining("git add -A"), + expect.any(String), + expect.any(Number), + ); + }); + + it("returns { committed:false, pushed:false } when git status itself fails", async () => { + const { sandbox } = makeSandbox({ + "git status --porcelain": fail("not a git repo"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result).toEqual({ committed: false, pushed: false }); + }); + }); + + describe("happy path", () => { + it("commits AND pushes when changes are present", async () => { + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff content"), + "git rev-parse HEAD": ok("abc123def456"), + "git symbolic-ref --short HEAD": ok("feat/branch"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + + expect(result.committed).toBe(true); + expect(result.pushed).toBe(true); + expect(result.commitMessage).toBe("feat: add example file"); + expect(result.commitSha).toBe("abc123def456"); + expect(result.error).toBeUndefined(); + + // Verify command sequence by call ordering + const calls = exec.mock.calls.map(c => c[0]); + const addIdx = calls.findIndex(c => c.includes("git add -A")); + const commitIdx = calls.findIndex(c => c.startsWith("git commit")); + const pushIdx = calls.findIndex(c => c.includes("git push")); + expect(addIdx).toBeGreaterThanOrEqual(0); + expect(commitIdx).toBeGreaterThan(addIdx); + expect(pushIdx).toBeGreaterThan(commitIdx); + }); + + it("pushes the current branch via `git push -u origin `", async () => { + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("custom-branch"), + }); + await performAutoCommit({ sandbox, ...baseParams }); + expect(exec).toHaveBeenCalledWith( + expect.stringContaining("git push -u origin custom-branch"), + expect.any(String), + expect.any(Number), + ); + }); + + it("disables interactive prompts on push (GIT_TERMINAL_PROMPT=0)", async () => { + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + await performAutoCommit({ sandbox, ...baseParams }); + const pushCall = exec.mock.calls.find(c => c[0].includes("git push")); + expect(pushCall?.[0]).toContain("GIT_TERMINAL_PROMPT=0"); + }); + }); + + describe("error paths", () => { + it("returns { committed:false, pushed:false, error } when git add fails", async () => { + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git add -A": fail("permission denied"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.committed).toBe(false); + expect(result.pushed).toBe(false); + expect(result.error).toMatch(/stage|add/i); + }); + + it("returns { committed:false, pushed:false, error } when git commit fails", async () => { + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git commit": fail("nothing to commit"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.committed).toBe(false); + expect(result.pushed).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("returns { committed:true, pushed:false, error } when push fails after commit succeeds", async () => { + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("commitsha"), + "git symbolic-ref --short HEAD": ok("main"), + "git push": fail("network unreachable"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.committed).toBe(true); + expect(result.pushed).toBe(false); + expect(result.commitSha).toBe("commitsha"); + expect(result.error).toMatch(/push/i); + }); + }); + + describe("commit message generation", () => { + it("uses the LLM-generated commit message on success", async () => { + vi.mocked(generateText).mockResolvedValue({ + text: "fix: update header logic", + } as never); + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.commitMessage).toBe("fix: update header logic"); + }); + + it("falls back to default message when generateText rejects", async () => { + vi.mocked(generateText).mockRejectedValue(new Error("gateway down")); + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.committed).toBe(true); + expect(result.commitMessage).toBe("chore: update repository changes"); + }); + + it("falls back to default message when staged diff is empty (rare race)", async () => { + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok(""), // empty diff + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.commitMessage).toBe("chore: update repository changes"); + expect(generateText).not.toHaveBeenCalled(); + }); + + it("truncates the LLM-generated message to 72 chars", async () => { + const longMessage = "feat: " + "x".repeat(200); + vi.mocked(generateText).mockResolvedValue({ text: longMessage } as never); + const { sandbox } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + const result = await performAutoCommit({ sandbox, ...baseParams }); + expect(result.commitMessage!.length).toBeLessThanOrEqual(72); + }); + }); + + describe("github auth url", () => { + it("sets `git remote set-url origin` with x-access-token URL when GITHUB_TOKEN is set", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("ghp_test_token_value"); + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + await performAutoCommit({ sandbox, ...baseParams }); + const remoteCall = exec.mock.calls.find(c => c[0].includes("git remote set-url")); + expect(remoteCall?.[0]).toContain("x-access-token:ghp_test_token_value"); + expect(remoteCall?.[0]).toContain("github.com/recoupable/api"); + }); + + it("does NOT touch the remote when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + const { sandbox, exec } = makeSandbox({ + "git status --porcelain": ok("M file.txt"), + "git diff --cached": ok("diff"), + "git rev-parse HEAD": ok("sha"), + "git symbolic-ref --short HEAD": ok("main"), + }); + await performAutoCommit({ sandbox, ...baseParams }); + const remoteCall = exec.mock.calls.find(c => c[0].includes("git remote set-url")); + expect(remoteCall).toBeUndefined(); + }); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/runAutoCommit.test.ts b/lib/chat/auto-commit/__tests__/runAutoCommit.test.ts new file mode 100644 index 000000000..16a9dac5b --- /dev/null +++ b/lib/chat/auto-commit/__tests__/runAutoCommit.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { runAutoCommit } from "@/lib/chat/auto-commit/runAutoCommit"; +import { connectSandbox } from "@/lib/sandbox/factory"; +import { performAutoCommit } from "@/lib/chat/auto-commit/performAutoCommit"; + +vi.mock("@/lib/sandbox/factory", () => ({ connectSandbox: vi.fn() })); +vi.mock("@/lib/chat/auto-commit/performAutoCommit", () => ({ + performAutoCommit: vi.fn(), +})); + +const SANDBOX_STATE = { type: "vercel", sandboxId: "sb_123" } as never; + +const baseParams = { + sessionId: "session-1", + sessionTitle: "test", + repoOwner: "recoupable", + repoName: "api", + sandboxState: SANDBOX_STATE, +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("runAutoCommit", () => { + it("forwards sandbox + caller fields to performAutoCommit", async () => { + const sandboxStub = { + type: "vercel" as const, + workingDirectory: "/sandbox/repo", + exec: vi.fn(), + } as never; + vi.mocked(connectSandbox).mockResolvedValue(sandboxStub); + vi.mocked(performAutoCommit).mockResolvedValue({ + committed: true, + pushed: true, + commitSha: "abc", + commitMessage: "feat: thing", + }); + + const result = await runAutoCommit(baseParams); + + expect(connectSandbox).toHaveBeenCalledWith(SANDBOX_STATE); + expect(performAutoCommit).toHaveBeenCalledWith({ + sandbox: sandboxStub, + sessionId: "session-1", + sessionTitle: "test", + repoOwner: "recoupable", + repoName: "api", + }); + expect(result).toEqual({ + committed: true, + pushed: true, + commitSha: "abc", + commitMessage: "feat: thing", + }); + }); + + it("returns the AutoCommitResult unchanged on success", async () => { + vi.mocked(connectSandbox).mockResolvedValue({} as never); + vi.mocked(performAutoCommit).mockResolvedValue({ + committed: false, + pushed: false, + }); + expect(await runAutoCommit(baseParams)).toEqual({ + committed: false, + pushed: false, + }); + }); + + it("returns { committed:false, pushed:false, error } when connectSandbox rejects (does NOT throw)", async () => { + vi.mocked(connectSandbox).mockRejectedValue(new Error("sandbox unreachable")); + const result = await runAutoCommit(baseParams); + expect(result.committed).toBe(false); + expect(result.pushed).toBe(false); + expect(result.error).toMatch(/sandbox unreachable|auto-commit/i); + }); + + it("returns { committed:false, pushed:false, error } when performAutoCommit rejects", async () => { + vi.mocked(connectSandbox).mockResolvedValue({} as never); + vi.mocked(performAutoCommit).mockRejectedValue(new Error("boom")); + const result = await runAutoCommit(baseParams); + expect(result.committed).toBe(false); + expect(result.pushed).toBe(false); + expect(result.error).toBeDefined(); + }); +}); diff --git a/lib/chat/auto-commit/__tests__/sendCommitChunk.test.ts b/lib/chat/auto-commit/__tests__/sendCommitChunk.test.ts new file mode 100644 index 000000000..e42b9b59d --- /dev/null +++ b/lib/chat/auto-commit/__tests__/sendCommitChunk.test.ts @@ -0,0 +1,51 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { sendCommitChunk } from "@/lib/chat/auto-commit/sendCommitChunk"; +import type { CommitData } from "@/lib/chat/auto-commit/buildCommitData"; + +const PENDING: CommitData = { + status: "skipped", + committed: false, + pushed: false, +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("sendCommitChunk", () => { + it("writes the `data-commit` chunk into the writable with id + data", async () => { + const written: unknown[] = []; + const writable = new WritableStream({ + write(chunk) { + written.push(chunk); + }, + }); + + await sendCommitChunk(writable, "msg_abc:commit", PENDING); + + expect(written).toEqual([{ type: "data-commit", id: "msg_abc:commit", data: PENDING }]); + }); + + it("releases the writer lock after writing so subsequent writes can acquire one", async () => { + const writable = new WritableStream({ write() {} }); + await sendCommitChunk(writable, "id1", PENDING); + // If lock weren't released, this getWriter() would throw + const writer = writable.getWriter(); + expect(writer).toBeDefined(); + writer.releaseLock(); + }); + + it("releases the writer lock even when the underlying write rejects", async () => { + const writable = new WritableStream({ + write() { + throw new Error("sink boom"); + }, + }); + await expect(sendCommitChunk(writable, "id1", PENDING)).rejects.toThrow(/sink boom/); + // Lock should still be released + expect(() => { + const writer = writable.getWriter(); + writer.releaseLock(); + }).not.toThrow(); + }); +}); diff --git a/lib/chat/auto-commit/autoCommitChatTurn.ts b/lib/chat/auto-commit/autoCommitChatTurn.ts new file mode 100644 index 000000000..73c22293d --- /dev/null +++ b/lib/chat/auto-commit/autoCommitChatTurn.ts @@ -0,0 +1,95 @@ +import type { UIMessageChunk } from "ai"; +import { hasAutoCommitChanges } from "@/lib/chat/auto-commit/hasAutoCommitChanges"; +import { runAutoCommit } from "@/lib/chat/auto-commit/runAutoCommit"; +import { sendCommitChunk } from "@/lib/chat/auto-commit/sendCommitChunk"; +import { buildCommitData } from "@/lib/chat/auto-commit/buildCommitData"; +import { persistAssistantDataPart } from "@/lib/chat/persistAssistantDataPart"; +import type { SandboxState } from "@/lib/sandbox/factory"; + +interface AssistantMessage { + id: string; + role: string; + parts: ReadonlyArray; +} + +export interface AutoCommitChatTurnInput { + writable: WritableStream; + responseMessage: AssistantMessage; + finishReason: string | undefined; + sessionId: string; + sessionTitle?: string; + repoOwner?: string; + repoName?: string; + /** + * Discriminated SandboxState (already wrapped with the `type` tag + * by the caller — DurableAgentContext carries the raw VercelState + * so the workflow body composes the SandboxState before calling + * here). + */ + sandboxState?: SandboxState; +} + +/** + * Runs the full auto-commit flow at the tail of a successful chat + * turn: gates on `canAutoCommit`, checks for sandbox changes, emits + * the `data-commit` chunks (pending → resolved), runs the + * sandbox-side commit + push, and persists the resolved part onto + * the assistant message so the `GitDataPartCard` UI renders on + * page refresh. + * + * Extracted from `runAgentWorkflow` so the workflow body stays a + * thin orchestrator. Mirrors open-agents' + * `apps/web/app/workflows/chat.ts:canAutoCommit` block. + * + * Skips silently when: + * - finishReason is `"tool-calls"` (intermediate turn, not natural finish) + * - either `repoOwner` or `repoName` is missing (no GitHub link to make) + * - `sandboxState` is missing (no sandbox to commit in) + * - `hasAutoCommitChanges` returns false (`git status --porcelain` empty) + * + * Never throws — the inner steps swallow their errors and surface + * the result via `AutoCommitResult.error`, which `buildCommitData` + * shapes into a `status: "error"` chunk that's still persisted so + * the UI shows "Commit failed" on refresh. + */ +export async function autoCommitChatTurn(input: AutoCommitChatTurnInput): Promise { + const canAutoCommit = + input.finishReason !== "tool-calls" && + input.repoOwner !== undefined && + input.repoName !== undefined && + input.sandboxState !== undefined; + if (!canAutoCommit) return; + + const hasChanges = await hasAutoCommitChanges({ + sandboxState: input.sandboxState!, + }); + if (!hasChanges) return; + + const commitPartId = `${input.responseMessage.id}:commit`; + + // Emit the pending chunk BEFORE the commit step so the UI can + // show a spinner while git add/commit/push run. + await sendCommitChunk(input.writable, commitPartId, { + status: "pending", + committed: false, + pushed: false, + }); + + const commitResult = await runAutoCommit({ + sessionId: input.sessionId, + sessionTitle: input.sessionTitle ?? "", + repoOwner: input.repoOwner!, + repoName: input.repoName!, + sandboxState: input.sandboxState!, + }); + const resolvedData = buildCommitData(commitResult, input.repoOwner!, input.repoName!); + await sendCommitChunk(input.writable, commitPartId, resolvedData); + + // Persist the resolved data-commit part onto the assistant + // message so the GitDataPartCard renders on page refresh. + await persistAssistantDataPart(input.responseMessage, { + type: "data-commit", + id: commitPartId, + data: resolvedData, + }); +} diff --git a/lib/chat/auto-commit/buildCommitData.ts b/lib/chat/auto-commit/buildCommitData.ts new file mode 100644 index 000000000..982522132 --- /dev/null +++ b/lib/chat/auto-commit/buildCommitData.ts @@ -0,0 +1,85 @@ +import type { AutoCommitResult } from "@/lib/chat/auto-commit/performAutoCommit"; + +/** + * Data shape carried by the `data-commit` UI chunk emitted from + * `runAgentWorkflow` after auto-commit runs. Mirrors open-agents' + * `WebAgentCommitData` byte-for-byte so the shared chat UI can render + * it without conditional logic on the source surface. + */ +export interface CommitData { + /** + * `"pending"` is set when the workflow emits the initial chunk + * before the commit step runs (so the UI can show a spinner). + * `buildCommitData` itself only ever produces the terminal three + * statuses; pending is constructed inline in `runAgentWorkflow`. + */ + status: "pending" | "success" | "error" | "skipped"; + committed: boolean; + pushed: boolean; + commitMessage?: string; + commitSha?: string; + /** + * GitHub commit URL. Set only when the commit was both committed + * AND pushed AND has a SHA — i.e., the link will actually resolve + * on GitHub. + */ + url?: string; + error?: string; +} + +function buildGitHubCommitUrl(repoOwner: string, repoName: string, commitSha: string): string { + return `https://github.com/${encodeURIComponent(repoOwner)}/${encodeURIComponent(repoName)}/commit/${encodeURIComponent(commitSha)}`; +} + +/** + * Shapes an `AutoCommitResult` into the UI chunk payload. + * + * Resolution order: + * - `result.error` set → `status: "error"` (preserves any commit/push + * metadata that landed so the UI can still link to the partial + * result) + * - `result.committed` → `status: "success"` + * - otherwise → `status: "skipped"` + * + * Mirrors open-agents' `apps/web/app/workflows/chat.ts:buildCommitData`. + */ +export function buildCommitData( + result: AutoCommitResult, + repoOwner: string, + repoName: string, +): CommitData { + if (result.error) { + return { + status: "error", + committed: result.committed, + pushed: result.pushed, + commitMessage: result.commitMessage, + commitSha: result.commitSha, + url: + result.pushed && result.commitSha + ? buildGitHubCommitUrl(repoOwner, repoName, result.commitSha) + : undefined, + error: result.error, + }; + } + + if (result.committed) { + return { + status: "success", + committed: result.committed, + pushed: result.pushed, + commitMessage: result.commitMessage, + commitSha: result.commitSha, + url: + result.pushed && result.commitSha + ? buildGitHubCommitUrl(repoOwner, repoName, result.commitSha) + : undefined, + }; + } + + return { + status: "skipped", + committed: false, + pushed: false, + }; +} diff --git a/lib/chat/auto-commit/generateCommitMessage.ts b/lib/chat/auto-commit/generateCommitMessage.ts new file mode 100644 index 000000000..93c3af1d1 --- /dev/null +++ b/lib/chat/auto-commit/generateCommitMessage.ts @@ -0,0 +1,62 @@ +import type { Sandbox } from "@/lib/sandbox/abstraction"; +import generateText from "@/lib/ai/generateText"; + +const FALLBACK_COMMIT_MESSAGE = "chore: update repository changes"; +const COMMIT_MESSAGE_MAX_LENGTH = 72; +const DIFF_PROMPT_TRUNCATE_CHARS = 8000; +const TIMEOUT_DIFF_MS = 30_000; + +/** + * Asks the gateway to produce a conventional-commit-formatted + * message describing the staged diff. Falls back to a sane default + * when: + * - the staged diff is empty (rare race; the caller has already + * verified there's something to commit, but `git diff --cached` + * can race with the actual stage) + * - the gateway call throws + * - the gateway returns an empty/whitespace string + * + * Truncates to 72 chars to fit standard commit-message-line width. + * Only the first line of the LLM output is used — the prompt asks + * for a single line but models sometimes follow with body text. + * + * Ports the inline `generateCommitMessage` helper that previously + * lived inside `performAutoCommit.ts`. Extracting it lets callers + * compose alternative commit message strategies without re-running + * the full performAutoCommit sandbox pipeline. + */ +export async function generateCommitMessage( + sandbox: Sandbox, + cwd: string, + sessionTitle: string, +): Promise { + try { + const stagedDiffResult = await sandbox.exec("git diff --cached", cwd, TIMEOUT_DIFF_MS); + const diffForCommit = stagedDiffResult.stdout; + + if (!diffForCommit.trim()) { + return FALLBACK_COMMIT_MESSAGE; + } + + const result = await generateText({ + model: "openai/gpt-5.4-nano", + prompt: `Generate a concise git commit message for these changes. Use conventional commit format (e.g., "feat:", "fix:", "refactor:"). One line only, max ${COMMIT_MESSAGE_MAX_LENGTH} characters. + +Session context: ${sessionTitle} + +Diff: +${diffForCommit.slice(0, DIFF_PROMPT_TRUNCATE_CHARS)} + +Respond with ONLY the commit message, nothing else.`, + }); + + const generated = result.text.trim().split("\n")[0]?.trim(); + if (generated && generated.length > 0) { + return generated.slice(0, COMMIT_MESSAGE_MAX_LENGTH); + } + } catch (error) { + console.warn("[auto-commit] Failed to generate commit message:", error); + } + + return FALLBACK_COMMIT_MESSAGE; +} diff --git a/lib/chat/auto-commit/hasAutoCommitChanges.ts b/lib/chat/auto-commit/hasAutoCommitChanges.ts new file mode 100644 index 000000000..bc2b86e05 --- /dev/null +++ b/lib/chat/auto-commit/hasAutoCommitChanges.ts @@ -0,0 +1,47 @@ +import { connectSandbox, type SandboxState } from "@/lib/sandbox/factory"; + +const STATUS_TIMEOUT_MS = 10_000; + +/** + * Quick pre-flight: does the sandbox have anything staged-or-unstaged + * that auto-commit should pick up? Returns true when `git status + * --porcelain` reports any output. + * + * Failure mode: fail-open. If the sandbox connect or the git command + * itself fails, return `true` and let `runAutoCommit` try anyway — + * it will surface the real error in its own `AutoCommitResult`. The + * alternative (returning false on error) would silently skip + * auto-commit on a transient sandbox blip, which we don't want. + * + * Mirrors open-agents' + * `apps/web/app/workflows/chat-post-finish.ts:hasAutoCommitChangesStep`. + */ +export async function hasAutoCommitChanges(params: { + sandboxState: SandboxState; +}): Promise { + "use step"; + console.log("[hasAutoCommitChanges] enter"); + try { + const sandbox = await connectSandbox(params.sandboxState); + const statusResult = await sandbox.exec( + "git status --porcelain", + sandbox.workingDirectory, + STATUS_TIMEOUT_MS, + ); + + if (!statusResult.success) { + console.warn( + "[hasAutoCommitChanges] git status failed; assuming changes present (fail-open)", + { stderr: statusResult.stderr }, + ); + return true; + } + + const hasChanges = statusResult.stdout.trim().length > 0; + console.log("[hasAutoCommitChanges] done", { hasChanges }); + return hasChanges; + } catch (error) { + console.error("[hasAutoCommitChanges] unexpected error (failing open):", error); + return true; + } +} diff --git a/lib/chat/auto-commit/performAutoCommit.ts b/lib/chat/auto-commit/performAutoCommit.ts new file mode 100644 index 000000000..94c918feb --- /dev/null +++ b/lib/chat/auto-commit/performAutoCommit.ts @@ -0,0 +1,128 @@ +import type { Sandbox } from "@/lib/sandbox/abstraction"; +import { generateCommitMessage } from "@/lib/chat/auto-commit/generateCommitMessage"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +export interface AutoCommitParams { + sandbox: Sandbox; + sessionId: string; + sessionTitle: string; + repoOwner: string; + repoName: string; +} + +export interface AutoCommitResult { + committed: boolean; + pushed: boolean; + commitMessage?: string; + commitSha?: string; + error?: string; +} + +const TIMEOUT_QUICK_MS = 10_000; +const TIMEOUT_PUSH_MS = 60_000; +const TIMEOUT_RESOLVE_MS = 5_000; + +/** + * Stages all changes in the sandbox, generates a commit message via + * the gateway, commits, and pushes. Ports + * `open-agents/apps/web/lib/chat/auto-commit-direct.ts:performAutoCommit`. + * + * Failure modes are deliberately granular so the caller can surface + * the right UI state: + * - `{ committed: false, pushed: false }` when nothing to commit + * (git status empty, or git status itself failed) + * - `{ committed: false, pushed: false, error }` when stage/commit + * fails + * - `{ committed: true, pushed: false, error }` when the commit + * landed locally but the push failed (caller should retry or + * surface the local sha so the user knows the changes aren't + * remote yet) + * + * Auth: if `GITHUB_TOKEN` is set (the service-account token used to + * clone), the function rewrites `origin` to an x-access-token URL so + * the subsequent push authenticates. When the token is absent the + * remote URL is left alone (public repos / pre-authed remotes still + * work). + */ +export async function performAutoCommit(params: AutoCommitParams): Promise { + const { sandbox, sessionTitle, repoOwner, repoName } = params; + const cwd = sandbox.workingDirectory; + + // 1. Check for uncommitted changes + const statusResult = await sandbox.exec("git status --porcelain", cwd, TIMEOUT_QUICK_MS); + if (!statusResult.success || !statusResult.stdout.trim()) { + return { committed: false, pushed: false }; + } + + // 2. Configure auth on origin so the push can authenticate. + const token = getServiceGithubToken(); + if (token) { + const authUrl = `https://x-access-token:${token}@github.com/${repoOwner}/${repoName}.git`; + await sandbox.exec(`git remote set-url origin "${authUrl}"`, cwd, TIMEOUT_QUICK_MS); + } + + // 3. Stage all changes + const addResult = await sandbox.exec("git add -A", cwd, TIMEOUT_QUICK_MS); + if (!addResult.success) { + return { + committed: false, + pushed: false, + error: "Failed to stage changes", + }; + } + + // 4. Generate commit message (LLM-generated from staged diff, with + // a sane fallback when the gateway fails or the diff is empty). + const commitMessage = await generateCommitMessage(sandbox, cwd, sessionTitle); + + // 5. Commit. Single-quote escaping mirrors open-agents' + // auto-commit-direct so messages containing apostrophes don't + // break the shell. + const escapedMessage = commitMessage.replace(/'/g, "'\\''"); + const commitResult = await sandbox.exec( + `git commit -m '${escapedMessage}'`, + cwd, + TIMEOUT_QUICK_MS, + ); + if (!commitResult.success) { + return { + committed: false, + pushed: false, + error: `Failed to commit: ${commitResult.stdout || commitResult.stderr}`, + }; + } + + const headResult = await sandbox.exec("git rev-parse HEAD", cwd, TIMEOUT_RESOLVE_MS); + const commitSha = headResult.stdout.trim() || undefined; + + // 6. Push. GIT_TERMINAL_PROMPT=0 so a missing/expired token fails + // fast instead of hanging on a credential prompt the workflow + // runtime can't answer. + const branchResult = await sandbox.exec("git symbolic-ref --short HEAD", cwd, TIMEOUT_RESOLVE_MS); + const currentBranch = branchResult.stdout.trim() || "HEAD"; + + const pushResult = await sandbox.exec( + `GIT_TERMINAL_PROMPT=0 git push -u origin ${currentBranch}`, + cwd, + TIMEOUT_PUSH_MS, + ); + if (!pushResult.success) { + console.warn( + `[auto-commit] Push failed for session ${params.sessionId}: ${pushResult.stderr || pushResult.stdout}`, + ); + return { + committed: true, + pushed: false, + commitMessage, + commitSha, + error: "Commit succeeded but push failed", + }; + } + + return { + committed: true, + pushed: true, + commitMessage, + commitSha, + }; +} diff --git a/lib/chat/auto-commit/runAutoCommit.ts b/lib/chat/auto-commit/runAutoCommit.ts new file mode 100644 index 000000000..ca2b73975 --- /dev/null +++ b/lib/chat/auto-commit/runAutoCommit.ts @@ -0,0 +1,50 @@ +import { connectSandbox, type SandboxState } from "@/lib/sandbox/factory"; +import { performAutoCommit, type AutoCommitResult } from "@/lib/chat/auto-commit/performAutoCommit"; + +/** + * Workflow step that runs auto-commit + push in the sandbox. Wraps + * `performAutoCommit` with sandbox connect + global error handling so + * the chat workflow never aborts on a commit-time hiccup — failures + * land in `AutoCommitResult.error`, which the caller surfaces via the + * `data-commit` UI chunk. + * + * Mirrors open-agents' + * `apps/web/app/workflows/chat-post-finish.ts:runAutoCommitStep`. + */ +export async function runAutoCommit(params: { + sessionId: string; + sessionTitle: string; + repoOwner: string; + repoName: string; + sandboxState: SandboxState; +}): Promise { + "use step"; + console.log("[runAutoCommit] enter", { + sessionId: params.sessionId, + repoOwner: params.repoOwner, + repoName: params.repoName, + }); + try { + const sandbox = await connectSandbox(params.sandboxState); + const result = await performAutoCommit({ + sandbox, + sessionId: params.sessionId, + sessionTitle: params.sessionTitle, + repoOwner: params.repoOwner, + repoName: params.repoName, + }); + console.log("[runAutoCommit] done", { + committed: result.committed, + pushed: result.pushed, + hasError: result.error !== undefined, + }); + return result; + } catch (error) { + console.error("[runAutoCommit] unexpected error:", error); + return { + committed: false, + pushed: false, + error: error instanceof Error ? error.message : "Auto-commit failed", + }; + } +} diff --git a/lib/chat/auto-commit/sendCommitChunk.ts b/lib/chat/auto-commit/sendCommitChunk.ts new file mode 100644 index 000000000..b0b60e954 --- /dev/null +++ b/lib/chat/auto-commit/sendCommitChunk.ts @@ -0,0 +1,25 @@ +import type { UIMessageChunk } from "ai"; +import type { CommitData } from "@/lib/chat/auto-commit/buildCommitData"; + +/** + * Emits a `data-commit` UIMessageChunk into the chat workflow's + * writable. Wrapped as a `"use step"` so the chunk flushes durably to + * the SSE client before the workflow moves on (e.g., the "pending" + * status appears in the UI before `runAutoCommit` starts). + * + * Mirrors open-agents' + * `apps/web/app/workflows/chat.ts:sendDataPart`. + */ +export async function sendCommitChunk( + writable: WritableStream, + id: string, + data: CommitData, +): Promise { + "use step"; + const writer = writable.getWriter(); + try { + await writer.write({ type: "data-commit", id, data } as UIMessageChunk); + } finally { + writer.releaseLock(); + } +} diff --git a/lib/chat/handleChatWorkflowStream.ts b/lib/chat/handleChatWorkflowStream.ts index 27961b126..0e7af2f3e 100644 --- a/lib/chat/handleChatWorkflowStream.ts +++ b/lib/chat/handleChatWorkflowStream.ts @@ -14,6 +14,7 @@ import { errorResponse } from "@/lib/networking/errorResponse"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { runAgentWorkflow } from "@/app/lib/workflows/runAgentWorkflow"; import { extractOrgId } from "@/lib/recoupable/extractOrgId"; +import { parseGitHubRepoIdentifiers } from "@/lib/github/parseGitHubRepoIdentifiers"; import { DEFAULT_WORKING_DIRECTORY } from "@/lib/sandbox/vercel/sandbox/constants"; import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; import type { VercelState } from "@/lib/sandbox/vercel/state"; @@ -117,6 +118,13 @@ export async function handleChatWorkflowStream(request: NextRequest): Promise; +} + +/** + * Workflow-step wrapper that merges a data-part into an assistant + * message and persists the merged message to `chat_messages.parts`. + * + * Carries the `"use step"` directive — the underlying + * `updateChatMessage` Supabase helper is intentionally kept pure so + * the step boundary lives in the chat-domain layer (this file) + * rather than the supabase layer. Mirrors the pattern used by + * `persistAssistantMessage` + `upsertChatMessage`. + * + * Use this when a `data-*` chunk lands AFTER the initial + * `persistAssistantMessage` call already wrote the message — e.g., + * the auto-commit flow's resolved `data-commit` chunk that needs to + * survive page refresh. + * + * Errors from the underlying supabase call are surfaced as logs; + * this function never throws so a transient DB blip can't mark the + * chat workflow run failed. + */ +export async function persistAssistantDataPart( + message: AssistantMessage, + part: DataPart, +): Promise { + "use step"; + console.log("[persistAssistantDataPart] enter", { + messageId: message.id, + partType: part.type, + partId: part.id, + }); + const merged = upsertAssistantDataPart(message, part); + const result = await updateChatMessage(merged.id, merged); + if (!result.ok) { + console.error("[persistAssistantDataPart] update failed:", result.error); + return; + } + console.log("[persistAssistantDataPart] persisted", { + messageId: merged.id, + partCount: merged.parts.length, + }); +} diff --git a/lib/chat/upsertAssistantDataPart.ts b/lib/chat/upsertAssistantDataPart.ts new file mode 100644 index 000000000..4516ba458 --- /dev/null +++ b/lib/chat/upsertAssistantDataPart.ts @@ -0,0 +1,44 @@ +interface DataPart { + type: string; + id: string; + data: unknown; +} + +interface AssistantMessage { + id: string; + role: string; + parts: ReadonlyArray; +} + +/** + * Returns a new assistant message with `part` merged into `parts`: + * - replaces the existing part when one with the same `{type, id}` + * is already present (e.g. pending → success transition for the + * same `data-commit` part) + * - appends otherwise + * + * Pure helper — the input message is not mutated. Mirrors + * open-agents' `upsertAssistantDataPart` in + * `apps/web/app/workflows/chat.ts`. + * + * Used by the auto-commit branch in `runAgentWorkflow` to persist the + * resolved data-commit chunk onto the assistant message so the + * `GitDataPartCard` UI renders on page refresh (not just during the + * live SSE stream). + */ +export function upsertAssistantDataPart( + message: TMessage, + part: DataPart, +): TMessage { + const nextParts = [...message.parts]; + const existingIndex = nextParts.findIndex(p => { + const candidate = p as { type?: string; id?: string }; + return candidate.type === part.type && candidate.id === part.id; + }); + if (existingIndex >= 0) { + nextParts[existingIndex] = part; + } else { + nextParts.push(part); + } + return { ...message, parts: nextParts }; +} diff --git a/lib/github/__tests__/parseGitHubRepoIdentifiers.test.ts b/lib/github/__tests__/parseGitHubRepoIdentifiers.test.ts new file mode 100644 index 000000000..72d6ded0d --- /dev/null +++ b/lib/github/__tests__/parseGitHubRepoIdentifiers.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from "vitest"; +import { parseGitHubRepoIdentifiers } from "@/lib/github/parseGitHubRepoIdentifiers"; + +describe("parseGitHubRepoIdentifiers", () => { + it("parses owner + repo from a plain https URL", () => { + expect(parseGitHubRepoIdentifiers("https://github.com/Myco-WTF/Sweetman")).toEqual({ + owner: "Myco-WTF", + repo: "Sweetman", + }); + }); + + it("strips a trailing .git suffix", () => { + expect(parseGitHubRepoIdentifiers("https://github.com/recoupable/api.git")).toEqual({ + owner: "recoupable", + repo: "api", + }); + }); + + it("strips a trailing slash", () => { + expect(parseGitHubRepoIdentifiers("https://github.com/recoupable/api/")).toEqual({ + owner: "recoupable", + repo: "api", + }); + }); + + it("accepts ssh-style URLs (git@github.com:owner/repo.git)", () => { + expect(parseGitHubRepoIdentifiers("git@github.com:Myco-WTF/Sweetman.git")).toEqual({ + owner: "Myco-WTF", + repo: "Sweetman", + }); + }); + + it("returns null for non-github URLs", () => { + expect(parseGitHubRepoIdentifiers("https://gitlab.com/owner/repo")).toBeNull(); + }); + + it("returns null for the empty string", () => { + expect(parseGitHubRepoIdentifiers("")).toBeNull(); + }); + + it("returns null for an https URL missing the repo segment", () => { + expect(parseGitHubRepoIdentifiers("https://github.com/owner")).toBeNull(); + }); + + it("returns null when input is null/undefined", () => { + expect(parseGitHubRepoIdentifiers(null)).toBeNull(); + expect(parseGitHubRepoIdentifiers(undefined)).toBeNull(); + }); +}); diff --git a/lib/github/parseGitHubRepoIdentifiers.ts b/lib/github/parseGitHubRepoIdentifiers.ts new file mode 100644 index 000000000..eb05bf925 --- /dev/null +++ b/lib/github/parseGitHubRepoIdentifiers.ts @@ -0,0 +1,32 @@ +/** + * Parses a clone URL into `{ owner, repo }`. Used as a fallback when + * a session row was created before the api started persisting + * `repo_owner` / `repo_name` directly — auto-commit needs both to + * compose the GitHub commit URL and to set the remote auth URL on + * push. Returns `null` for non-GitHub URLs and for malformed inputs. + * + * Recognized shapes: + * - `https://github.com//` + * - `https://github.com//.git` + * - `git@github.com:/.git` + * - trailing slashes are tolerated + */ +export function parseGitHubRepoIdentifiers( + cloneUrl: string | null | undefined, +): { owner: string; repo: string } | null { + if (!cloneUrl) return null; + + // ssh form: git@github.com:owner/repo[.git] + const sshMatch = cloneUrl.match(/^git@github\.com:([^/]+)\/([^/]+?)(?:\.git)?\/?$/); + if (sshMatch) { + return { owner: sshMatch[1]!, repo: sshMatch[2]! }; + } + + // https form: https://github.com/owner/repo[.git][/] + const httpsMatch = cloneUrl.match(/^https?:\/\/github\.com\/([^/]+)\/([^/]+?)(?:\.git)?\/?$/); + if (httpsMatch) { + return { owner: httpsMatch[1]!, repo: httpsMatch[2]! }; + } + + return null; +} diff --git a/lib/supabase/chat_messages/__tests__/updateChatMessage.test.ts b/lib/supabase/chat_messages/__tests__/updateChatMessage.test.ts new file mode 100644 index 000000000..95735ec18 --- /dev/null +++ b/lib/supabase/chat_messages/__tests__/updateChatMessage.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { updateChatMessage } from "@/lib/supabase/chat_messages/updateChatMessage"; + +const { fromMock, updateMock, eqMock } = vi.hoisted(() => { + const updateMock = vi.fn(); + const eqMock = vi.fn(); + const fromMock = vi.fn(); + return { fromMock, updateMock, eqMock }; +}); + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { from: fromMock }, +})); + +beforeEach(() => { + vi.clearAllMocks(); + // chained builder: from(...).update(...).eq(...) → Promise<{error}> + eqMock.mockResolvedValue({ error: null }); + updateMock.mockReturnValue({ eq: eqMock }); + fromMock.mockReturnValue({ update: updateMock }); +}); + +describe("updateChatMessage", () => { + it("UPDATEs the `parts` column on chat_messages keyed by id", async () => { + const parts = [ + { type: "text", text: "hi" }, + { type: "data-commit", id: "x", data: { status: "success" } }, + ]; + const result = await updateChatMessage("msg_abc", parts); + + expect(fromMock).toHaveBeenCalledWith("chat_messages"); + expect(updateMock).toHaveBeenCalledWith({ parts }); + expect(eqMock).toHaveBeenCalledWith("id", "msg_abc"); + expect(result).toEqual({ ok: true }); + }); + + it("returns { ok: false, error } when supabase reports an error (does NOT throw)", async () => { + eqMock.mockResolvedValue({ error: { message: "boom" } }); + const result = await updateChatMessage("msg_abc", []); + expect(result).toEqual({ ok: false, error: "boom" }); + }); + + it("returns { ok: false, error } when the supabase client itself rejects", async () => { + eqMock.mockRejectedValue(new Error("network blip")); + const result = await updateChatMessage("msg_abc", []); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toContain("network blip"); + }); +}); diff --git a/lib/supabase/chat_messages/updateChatMessage.ts b/lib/supabase/chat_messages/updateChatMessage.ts new file mode 100644 index 000000000..8f201db10 --- /dev/null +++ b/lib/supabase/chat_messages/updateChatMessage.ts @@ -0,0 +1,58 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Json } from "@/types/database.types"; + +export interface UpdateChatMessageResult { + ok: boolean; + /** + * Present only when `ok === false`. Kept optional rather than as a + * discriminated union so callers can read `result.error` without + * tripping a narrowing edge case in the Next.js 16 type checker + * (same pattern as `DeductCreditsWithAuditResult`). + */ + error?: string; +} + +/** + * Replaces the `parts` jsonb column on an existing `chat_messages` + * row. UPDATE-only — does NOT insert if the row is missing, so the + * caller must have already persisted the message via + * `upsertChatMessage` first. + * + * Pure supabase wrapper — kept free of any workflow concerns + * (no `"use step"` directive here). Callers that need durable step + * semantics wrap this in a `"use step"` function in `lib/chat/` + * (e.g. `persistAssistantDataPart`). Keeping the step boundary out + * of the supabase layer matches the rest of the codebase + * (`upsertChatMessage`, `updateChat`, etc.) — only domain wrappers + * are step-bound, supabase wrappers stay pure. + * + * Use this when a chunk lands AFTER the initial assistant message + * was already persisted (auto-commit's `data-commit`, future PR data + * parts, etc.) and you need the chunk to survive page refresh. The + * existing `upsertChatMessage` uses `onConflict: "id", + * ignoreDuplicates: true` so a second call would be a no-op — this + * helper exists specifically to bypass that. + * + * Mirrors the UPDATE branch in open-agents' + * `apps/web/lib/db/sessions.ts:upsertChatMessageScoped` (which uses + * a single INSERT-then-UPDATE atomic helper; api keeps the two paths + * separate so the first-insert path remains replay-idempotent). + */ +export async function updateChatMessage( + id: string, + parts: unknown, +): Promise { + try { + const { error } = await supabase + .from("chat_messages") + .update({ parts: parts as Json }) + .eq("id", id); + if (error) return { ok: false, error: error.message }; + return { ok: true }; + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : String(error), + }; + } +} From 9562e6b222e3757377225e02199e4e7ae5791a5f Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Tue, 26 May 2026 05:46:56 +0530 Subject: [PATCH 4/4] feat(api): migrate /api/sessions/[sessionId]/chats/[chatId] from open-agents (#562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../[sessionId]/chats/[chatId]/route.ts | 81 +++++++ .../deleteSessionChatHandler.test.ts | 60 ++++++ .../__tests__/getSessionChatHandler.test.ts | 116 ++++++++++ .../__tests__/patchSessionChatHandler.test.ts | 102 +++++++++ .../validateDeleteSessionChatRequest.test.ts | 141 ++++++++++++ .../validateGetSessionChatRequest.test.ts | 152 +++++++++++++ .../validatePatchSessionChatRequest.test.ts | 200 ++++++++++++++++++ .../chats/deleteSessionChatHandler.ts | 35 +++ lib/sessions/chats/getSessionChatHandler.ts | 48 +++++ lib/sessions/chats/patchSessionChatHandler.ts | 40 ++++ .../chats/validateDeleteSessionChatRequest.ts | 66 ++++++ .../chats/validateGetSessionChatRequest.ts | 61 ++++++ .../chats/validatePatchSessionChatRequest.ts | 91 ++++++++ lib/supabase/chats/deleteChat.ts | 19 ++ 14 files changed, 1212 insertions(+) create mode 100644 app/api/sessions/[sessionId]/chats/[chatId]/route.ts create mode 100644 lib/sessions/chats/__tests__/deleteSessionChatHandler.test.ts create mode 100644 lib/sessions/chats/__tests__/getSessionChatHandler.test.ts create mode 100644 lib/sessions/chats/__tests__/patchSessionChatHandler.test.ts create mode 100644 lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts create mode 100644 lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts create mode 100644 lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.ts create mode 100644 lib/sessions/chats/deleteSessionChatHandler.ts create mode 100644 lib/sessions/chats/getSessionChatHandler.ts create mode 100644 lib/sessions/chats/patchSessionChatHandler.ts create mode 100644 lib/sessions/chats/validateDeleteSessionChatRequest.ts create mode 100644 lib/sessions/chats/validateGetSessionChatRequest.ts create mode 100644 lib/sessions/chats/validatePatchSessionChatRequest.ts create mode 100644 lib/supabase/chats/deleteChat.ts diff --git a/app/api/sessions/[sessionId]/chats/[chatId]/route.ts b/app/api/sessions/[sessionId]/chats/[chatId]/route.ts new file mode 100644 index 000000000..082b2ad7b --- /dev/null +++ b/app/api/sessions/[sessionId]/chats/[chatId]/route.ts @@ -0,0 +1,81 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { getSessionChatHandler } from "@/lib/sessions/chats/getSessionChatHandler"; +import { patchSessionChatHandler } from "@/lib/sessions/chats/patchSessionChatHandler"; +import { deleteSessionChatHandler } from "@/lib/sessions/chats/deleteSessionChatHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * GET /api/sessions/{sessionId}/chats/{chatId} + * + * Returns the chat's persisted UI message stream plus its current + * streaming state. Authenticates via Privy Bearer token or + * `x-api-key`; 404s when the session or chat is missing (or the chat + * lives in a different session) and 403s when the session is owned by + * a different account. + * + * @param request - The incoming request. + * @param options - Route options containing the async params. + * @param options.params - Route params containing the session id and chat id. + * @returns A NextResponse with `{ chat, isStreaming, messages }` on 200, or an error. + */ +export async function GET( + request: NextRequest, + options: { params: Promise<{ sessionId: string; chatId: string }> }, +) { + const { sessionId, chatId } = await options.params; + return getSessionChatHandler(request, sessionId, chatId); +} + +/** + * PATCH /api/sessions/{sessionId}/chats/{chatId} + * + * Applies a partial update to the chat (`title` and/or `modelId`). + * Body must include at least one of those non-empty fields. + * + * @param request - The incoming request. + * @param options - Route options containing the async params. + * @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 }> }, +) { + const { sessionId, chatId } = await options.params; + return patchSessionChatHandler(request, sessionId, chatId); +} + +/** + * DELETE /api/sessions/{sessionId}/chats/{chatId} + * + * Removes the chat (cascade clears `chat_messages` / `chat_reads`). + * Refuses with 400 if the chat is the only one in its session. + * + * @param request - The incoming request. + * @param options - Route options containing the async params. + * @param options.params - Route params containing the session id and chat id. + * @returns A NextResponse with `{ success: true }` on 200, or an error. + */ +export async function DELETE( + request: NextRequest, + options: { params: Promise<{ sessionId: string; chatId: string }> }, +) { + const { sessionId, chatId } = await options.params; + return deleteSessionChatHandler(request, sessionId, chatId); +} + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; diff --git a/lib/sessions/chats/__tests__/deleteSessionChatHandler.test.ts b/lib/sessions/chats/__tests__/deleteSessionChatHandler.test.ts new file mode 100644 index 000000000..f3d8fae65 --- /dev/null +++ b/lib/sessions/chats/__tests__/deleteSessionChatHandler.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/sessions/chats/validateDeleteSessionChatRequest", () => ({ + validateDeleteSessionChatRequest: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/deleteChat", () => ({ + deleteChat: vi.fn(), +})); + +const { validateDeleteSessionChatRequest } = await import( + "@/lib/sessions/chats/validateDeleteSessionChatRequest" +); +const { deleteChat } = await import("@/lib/supabase/chats/deleteChat"); +const { deleteSessionChatHandler } = await import("@/lib/sessions/chats/deleteSessionChatHandler"); + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1", { + method: "DELETE", + }); +} + +describe("deleteSessionChatHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the NextResponse from the validator as-is", async () => { + const failure = NextResponse.json( + { error: "Cannot delete the only chat in a session" }, + { status: 400 }, + ); + vi.mocked(validateDeleteSessionChatRequest).mockResolvedValue(failure); + + const res = await deleteSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(deleteChat).not.toHaveBeenCalled(); + }); + + it("returns { success: true } on the happy path", async () => { + vi.mocked(validateDeleteSessionChatRequest).mockResolvedValue(null); + vi.mocked(deleteChat).mockResolvedValue(true); + + const res = await deleteSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(200); + expect(deleteChat).toHaveBeenCalledWith("chat_1"); + expect(await res.json()).toEqual({ success: true }); + }); + + it("returns 500 when deleteChat reports failure", async () => { + vi.mocked(validateDeleteSessionChatRequest).mockResolvedValue(null); + vi.mocked(deleteChat).mockResolvedValue(false); + + const res = await deleteSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(500); + }); +}); diff --git a/lib/sessions/chats/__tests__/getSessionChatHandler.test.ts b/lib/sessions/chats/__tests__/getSessionChatHandler.test.ts new file mode 100644 index 000000000..b6c4c2ee3 --- /dev/null +++ b/lib/sessions/chats/__tests__/getSessionChatHandler.test.ts @@ -0,0 +1,116 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/sessions/chats/validateGetSessionChatRequest", () => ({ + validateGetSessionChatRequest: vi.fn(), +})); +vi.mock("@/lib/supabase/chat_messages/selectChatMessages", () => ({ + selectChatMessages: vi.fn(), +})); + +const { validateGetSessionChatRequest } = await import( + "@/lib/sessions/chats/validateGetSessionChatRequest" +); +const { selectChatMessages } = await import("@/lib/supabase/chat_messages/selectChatMessages"); +const { getSessionChatHandler } = await import("@/lib/sessions/chats/getSessionChatHandler"); + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1"); +} + +function mockValidated(chatOverrides: Parameters[0] = {}) { + vi.mocked(validateGetSessionChatRequest).mockResolvedValue( + baseChatRow({ id: "chat_1", session_id: "sess_1", ...chatOverrides }), + ); +} + +describe("getSessionChatHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the NextResponse from validateGetSessionChatRequest as-is", async () => { + const failure = NextResponse.json({ error: "Forbidden" }, { status: 403 }); + vi.mocked(validateGetSessionChatRequest).mockResolvedValue(failure); + + const res = await getSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(selectChatMessages).not.toHaveBeenCalled(); + }); + + it("returns 200 with chat, isStreaming=false, and message parts", async () => { + mockValidated({ active_stream_id: null, model_id: "openai/gpt-5-mini" }); + vi.mocked(selectChatMessages).mockResolvedValue([ + { + id: "msg_1", + chat_id: "chat_1", + role: "user", + parts: [{ type: "text", text: "hi" }], + created_at: "2026-05-01T00:00:00.000Z", + }, + { + id: "msg_2", + chat_id: "chat_1", + role: "assistant", + parts: [{ type: "text", text: "hello" }], + created_at: "2026-05-01T00:00:01.000Z", + }, + ]); + + const res = await getSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + chat: { + id: string; + sessionId: string; + title: string; + modelId: string | null; + activeStreamId: string | null; + lastAssistantMessageAt: string | null; + createdAt: string; + updatedAt: string; + }; + isStreaming: boolean; + messages: unknown[]; + }; + expect(body.chat).toEqual({ + id: "chat_1", + sessionId: "sess_1", + title: "New chat", + modelId: "openai/gpt-5-mini", + activeStreamId: null, + lastAssistantMessageAt: null, + createdAt: "2026-05-04T00:00:00.000Z", + updatedAt: "2026-05-04T00:00:00.000Z", + }); + expect(body.isStreaming).toBe(false); + expect(body.messages).toEqual([ + [{ type: "text", text: "hi" }], + [{ type: "text", text: "hello" }], + ]); + expect(selectChatMessages).toHaveBeenCalledWith({ + chatId: "chat_1", + orderBy: { createdAt: "asc" }, + }); + }); + + it("derives isStreaming=true from active_stream_id", async () => { + mockValidated({ active_stream_id: "stream-xyz" }); + vi.mocked(selectChatMessages).mockResolvedValue([]); + + const res = await getSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + chat: { activeStreamId: string | null }; + isStreaming: boolean; + messages: unknown[]; + }; + expect(body.isStreaming).toBe(true); + expect(body.chat.activeStreamId).toBe("stream-xyz"); + expect(body.messages).toEqual([]); + }); +}); diff --git a/lib/sessions/chats/__tests__/patchSessionChatHandler.test.ts b/lib/sessions/chats/__tests__/patchSessionChatHandler.test.ts new file mode 100644 index 000000000..b819ea188 --- /dev/null +++ b/lib/sessions/chats/__tests__/patchSessionChatHandler.test.ts @@ -0,0 +1,102 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/sessions/chats/validatePatchSessionChatRequest", () => ({ + validatePatchSessionChatRequest: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/updateChat", () => ({ + updateChat: vi.fn(), +})); + +const { validatePatchSessionChatRequest } = await import( + "@/lib/sessions/chats/validatePatchSessionChatRequest" +); +const { updateChat } = await import("@/lib/supabase/chats/updateChat"); +const { patchSessionChatHandler } = await import("@/lib/sessions/chats/patchSessionChatHandler"); + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1", { + method: "PATCH", + }); +} + +function mockValidated(patch: { title?: string; modelId?: string }) { + vi.mocked(validatePatchSessionChatRequest).mockResolvedValue(patch); +} + +describe("patchSessionChatHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the NextResponse from the validator as-is", async () => { + const failure = NextResponse.json({ error: "Forbidden" }, { status: 403 }); + vi.mocked(validatePatchSessionChatRequest).mockResolvedValue(failure); + + const res = await patchSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(updateChat).not.toHaveBeenCalled(); + }); + + it("maps modelId to model_id and stores title verbatim", async () => { + mockValidated({ title: "Renamed", modelId: "openai/gpt-5-mini" }); + vi.mocked(updateChat).mockResolvedValue({ + ok: true, + rowsUpdated: 1, + row: baseChatRow({ + id: "chat_1", + title: "Renamed", + model_id: "openai/gpt-5-mini", + }), + }); + + const res = await patchSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(200); + expect(updateChat).toHaveBeenCalledWith( + { id: "chat_1" }, + { title: "Renamed", model_id: "openai/gpt-5-mini" }, + ); + const body = (await res.json()) as { chat: { title: string; modelId: string } }; + expect(body.chat.title).toBe("Renamed"); + expect(body.chat.modelId).toBe("openai/gpt-5-mini"); + }); + + it("only patches the field provided", async () => { + mockValidated({ title: "Just a title" }); + vi.mocked(updateChat).mockResolvedValue({ + ok: true, + rowsUpdated: 1, + row: baseChatRow({ id: "chat_1", title: "Just a title" }), + }); + + await patchSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(updateChat).toHaveBeenCalledWith( + { id: "chat_1" }, + { title: "Just a title", model_id: undefined }, + ); + }); + + it("returns 500 when updateChat fails", async () => { + mockValidated({ title: "Renamed" }); + vi.mocked(updateChat).mockResolvedValue({ ok: false, error: "db down" }); + + const res = await patchSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(500); + }); + + it("returns 500 when updateChat matches no row", async () => { + mockValidated({ title: "Renamed" }); + vi.mocked(updateChat).mockResolvedValue({ + ok: true, + rowsUpdated: 0, + row: null, + }); + + const res = await patchSessionChatHandler(makeReq(), "sess_1", "chat_1"); + expect(res.status).toBe(500); + }); +}); diff --git a/lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts b/lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts new file mode 100644 index 000000000..86a32a873 --- /dev/null +++ b/lib/sessions/chats/__tests__/validateDeleteSessionChatRequest.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/selectChats", () => ({ + selectChats: vi.fn(), +})); + +const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); +const { selectSessions } = await import("@/lib/supabase/sessions/selectSessions"); +const { selectChats } = await import("@/lib/supabase/chats/selectChats"); +const { validateDeleteSessionChatRequest } = await import( + "@/lib/sessions/chats/validateDeleteSessionChatRequest" +); + +const accountId = "acc-uuid-1"; + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1", { + method: "DELETE", + }); +} + +describe("validateDeleteSessionChatRequest", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the auth NextResponse when validateAuthContext rejects", async () => { + const failure = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(failure); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(selectSessions).not.toHaveBeenCalled(); + }); + + it("returns 404 when the session is missing", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([]); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_missing", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + } + }); + + it("returns 403 when the session belongs to a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: "acc-OTHER" }), + ]); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(403); + } + }); + + it("returns 404 when the chat is not in this session", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([ + baseChatRow({ id: "chat_OTHER", session_id: "sess_1" }), + baseChatRow({ id: "chat_OTHER_2", session_id: "sess_1" }), + ]); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + } + }); + + it("returns 400 when the chat is the only one in the session", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1", session_id: "sess_1" })]); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ + status: "error", + error: "Cannot delete the only chat in a session", + }); + } + }); + + it("returns null on the happy path", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([ + baseChatRow({ id: "chat_1", session_id: "sess_1" }), + baseChatRow({ id: "chat_2", session_id: "sess_1" }), + ]); + + const res = await validateDeleteSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeNull(); + expect(selectChats).toHaveBeenCalledWith({ sessionId: "sess_1" }); + }); +}); diff --git a/lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts b/lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts new file mode 100644 index 000000000..e3f4bc595 --- /dev/null +++ b/lib/sessions/chats/__tests__/validateGetSessionChatRequest.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/selectChats", () => ({ + selectChats: vi.fn(), +})); + +const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); +const { selectSessions } = await import("@/lib/supabase/sessions/selectSessions"); +const { selectChats } = await import("@/lib/supabase/chats/selectChats"); +const { validateGetSessionChatRequest } = await import( + "@/lib/sessions/chats/validateGetSessionChatRequest" +); + +const accountId = "acc-uuid-1"; + +function makeReq(): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1"); +} + +describe("validateGetSessionChatRequest", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the auth NextResponse when validateAuthContext rejects", async () => { + const failure = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(failure); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBe(failure); + expect(selectSessions).not.toHaveBeenCalled(); + expect(selectChats).not.toHaveBeenCalled(); + }); + + it("returns 404 when the session is missing", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([]); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_missing", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Session not found", + }); + } + expect(selectChats).not.toHaveBeenCalled(); + }); + + it("returns 403 when the session belongs to a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: "acc-OTHER" }), + ]); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ status: "error", error: "Forbidden" }); + } + expect(selectChats).not.toHaveBeenCalled(); + }); + + it("returns 404 when the chat does not exist", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([]); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_1", "chat_missing"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Chat not found", + }); + } + }); + + it("returns 404 when the chat belongs to a different session", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([ + baseChatRow({ id: "chat_1", session_id: "sess_OTHER" }), + ]); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Chat not found", + }); + } + }); + + it("returns the chat row on the happy path", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1", session_id: "sess_1" })]); + + const res = await validateGetSessionChatRequest(makeReq(), "sess_1", "chat_1"); + expect(res).not.toBeInstanceOf(NextResponse); + if (!(res instanceof NextResponse)) { + expect(res.id).toBe("chat_1"); + expect(res.session_id).toBe("sess_1"); + } + expect(selectSessions).toHaveBeenCalledWith({ id: "sess_1" }); + expect(selectChats).toHaveBeenCalledWith({ id: "chat_1" }); + }); +}); diff --git a/lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.ts b/lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.ts new file mode 100644 index 000000000..4696880bf --- /dev/null +++ b/lib/sessions/chats/__tests__/validatePatchSessionChatRequest.test.ts @@ -0,0 +1,200 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); +vi.mock("@/lib/supabase/chats/selectChats", () => ({ + selectChats: vi.fn(), +})); + +const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); +const { selectSessions } = await import("@/lib/supabase/sessions/selectSessions"); +const { selectChats } = await import("@/lib/supabase/chats/selectChats"); +const { validatePatchSessionChatRequest } = await import( + "@/lib/sessions/chats/validatePatchSessionChatRequest" +); + +const accountId = "acc-uuid-1"; + +function makeReq(body: unknown): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); +} + +function makeRawReq(rawBody: string): NextRequest { + return new NextRequest("https://example.com/api/sessions/sess_1/chats/chat_1", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: rawBody, + }); +} + +function happyPathMocks() { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([baseChatRow({ id: "chat_1", session_id: "sess_1" })]); +} + +describe("validatePatchSessionChatRequest", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards the auth NextResponse when validateAuthContext rejects", async () => { + const failure = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(failure); + + const res = await validatePatchSessionChatRequest(makeReq({ title: "x" }), "sess_1", "chat_1"); + expect(res).toBe(failure); + }); + + it("returns 404 when the session is missing", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([]); + + const res = await validatePatchSessionChatRequest( + makeReq({ title: "x" }), + "sess_missing", + "chat_1", + ); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + } + }); + + it("returns 403 when the session belongs to a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: "acc-OTHER" }), + ]); + + const res = await validatePatchSessionChatRequest(makeReq({ title: "x" }), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(403); + } + }); + + it("returns 404 when the chat does not belong to the session", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ id: "sess_1", account_id: accountId }), + ]); + vi.mocked(selectChats).mockResolvedValue([ + baseChatRow({ id: "chat_1", session_id: "sess_OTHER" }), + ]); + + const res = await validatePatchSessionChatRequest(makeReq({ title: "x" }), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(404); + } + }); + + it("returns 400 on invalid JSON", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest(makeRawReq("not json"), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ + status: "error", + error: "Invalid JSON body", + }); + } + }); + + it("returns 400 when neither title nor modelId is provided", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest(makeReq({}), "sess_1", "chat_1"); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("At least one field is required"); + } + }); + + it("returns 400 when the body contains unknown fields", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest( + makeReq({ title: "Renamed", foo: "bar" }), + "sess_1", + "chat_1", + ); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(400); + } + }); + + it("returns 400 when title is whitespace-only", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest( + makeReq({ title: " " }), + "sess_1", + "chat_1", + ); + expect(res).toBeInstanceOf(NextResponse); + if (res instanceof NextResponse) { + expect(res.status).toBe(400); + } + }); + + it("returns the validated patch with title trimmed", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest( + makeReq({ title: " Renamed " }), + "sess_1", + "chat_1", + ); + expect(res).not.toBeInstanceOf(NextResponse); + if (!(res instanceof NextResponse)) { + expect(res).toEqual({ title: "Renamed" }); + } + }); + + it("returns the validated patch when modelId is provided", async () => { + happyPathMocks(); + const res = await validatePatchSessionChatRequest( + makeReq({ modelId: "openai/gpt-5-mini" }), + "sess_1", + "chat_1", + ); + expect(res).not.toBeInstanceOf(NextResponse); + if (!(res instanceof NextResponse)) { + expect(res).toEqual({ modelId: "openai/gpt-5-mini" }); + } + }); +}); diff --git a/lib/sessions/chats/deleteSessionChatHandler.ts b/lib/sessions/chats/deleteSessionChatHandler.ts new file mode 100644 index 000000000..b245c9284 --- /dev/null +++ b/lib/sessions/chats/deleteSessionChatHandler.ts @@ -0,0 +1,35 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateDeleteSessionChatRequest } from "@/lib/sessions/chats/validateDeleteSessionChatRequest"; +import { deleteChat } from "@/lib/supabase/chats/deleteChat"; + +/** + * Handles `DELETE /api/sessions/{sessionId}/chats/{chatId}`. Removes + * the chat row (cascade clears `chat_messages` / `chat_reads`). + * Refuses with 400 if the chat is the only one in its session. + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being deleted. + * @returns A NextResponse with `{ success: true }` on 200, or an error. + */ +export async function deleteSessionChatHandler( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const failure = await validateDeleteSessionChatRequest(request, sessionId, chatId); + if (failure) { + return failure; + } + + const ok = await deleteChat(chatId); + if (!ok) { + return NextResponse.json( + { status: "error", error: "Failed to delete chat" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json({ success: true }, { status: 200, headers: getCorsHeaders() }); +} diff --git a/lib/sessions/chats/getSessionChatHandler.ts b/lib/sessions/chats/getSessionChatHandler.ts new file mode 100644 index 000000000..84661945e --- /dev/null +++ b/lib/sessions/chats/getSessionChatHandler.ts @@ -0,0 +1,48 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { toChatResponse } from "@/lib/sessions/toChatResponse"; +import { validateGetSessionChatRequest } from "@/lib/sessions/chats/validateGetSessionChatRequest"; +import { selectChatMessages } from "@/lib/supabase/chat_messages/selectChatMessages"; + +export interface SessionChatResponse { + chat: ReturnType; + isStreaming: boolean; + messages: unknown[]; +} + +/** + * Handles `GET /api/sessions/{sessionId}/chats/{chatId}`. Returns the + * chat row (camelCase wire format), its streaming state, and the + * persisted UI message stream — enough for both initial render and + * in-tab refresh. + * + * `messages` is the raw `parts` JSON for each row (chat messages are + * stored as the full UIMessage; the caller deserializes back into + * its UIMessage type). + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being fetched. + * @returns A NextResponse with `{ chat, isStreaming, messages }` on 200, or an error. + */ +export async function getSessionChatHandler( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const chat = await validateGetSessionChatRequest(request, sessionId, chatId); + if (chat instanceof NextResponse) { + return chat; + } + + const messages = (await selectChatMessages({ chatId, orderBy: { createdAt: "asc" } })) ?? []; + + return NextResponse.json( + { + chat: toChatResponse(chat), + isStreaming: chat.active_stream_id !== null, + messages: messages.map(row => row.parts), + } satisfies SessionChatResponse, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sessions/chats/patchSessionChatHandler.ts b/lib/sessions/chats/patchSessionChatHandler.ts new file mode 100644 index 000000000..d23ea4016 --- /dev/null +++ b/lib/sessions/chats/patchSessionChatHandler.ts @@ -0,0 +1,40 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validatePatchSessionChatRequest } from "@/lib/sessions/chats/validatePatchSessionChatRequest"; +import { toChatResponse } from "@/lib/sessions/toChatResponse"; +import { updateChat } from "@/lib/supabase/chats/updateChat"; + +/** + * Handles `PATCH /api/sessions/{sessionId}/chats/{chatId}`. Applies a + * partial update to the chat (`title` and/or `modelId`) and returns + * the updated row under `{ chat }`. + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being updated. + * @returns A NextResponse with `{ chat }` on 200, or an error. + */ +export async function patchSessionChatHandler( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const patch = await validatePatchSessionChatRequest(request, sessionId, chatId); + if (patch instanceof NextResponse) { + return patch; + } + + const result = await updateChat({ id: chatId }, { title: patch.title, model_id: patch.modelId }); + + if (!result.ok || !result.row) { + return NextResponse.json( + { status: "error", error: "Failed to update chat" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json( + { chat: toChatResponse(result.row) }, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sessions/chats/validateDeleteSessionChatRequest.ts b/lib/sessions/chats/validateDeleteSessionChatRequest.ts new file mode 100644 index 000000000..6d9972891 --- /dev/null +++ b/lib/sessions/chats/validateDeleteSessionChatRequest.ts @@ -0,0 +1,66 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; + +/** + * Validates a `DELETE /api/sessions/{sessionId}/chats/{chatId}` + * request end-to-end: + * 1. Authenticates the caller via Privy Bearer / x-api-key + * 2. Loads the session and confirms the caller owns it + * 3. Loads every chat in the session, confirms the target chat is + * one of them, and refuses 400 if it's the only one (sessions + * must always retain at least one chat) + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being deleted. + * @returns A NextResponse on failure, or `null` when validation passes. + */ +export async function validateDeleteSessionChatRequest( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const sessionRows = await selectSessions({ id: sessionId }); + const session = sessionRows[0] ?? null; + + if (!session) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (session.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + 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() }, + ); + } + + return null; +} diff --git a/lib/sessions/chats/validateGetSessionChatRequest.ts b/lib/sessions/chats/validateGetSessionChatRequest.ts new file mode 100644 index 000000000..f453b100d --- /dev/null +++ b/lib/sessions/chats/validateGetSessionChatRequest.ts @@ -0,0 +1,61 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; +import type { Tables } from "@/types/database.types"; + +/** + * Validates a `GET /api/sessions/{sessionId}/chats/{chatId}` request + * end-to-end: + * 1. Authenticates the caller via Privy Bearer / x-api-key + * 2. Loads the session and confirms the caller owns it + * 3. Loads the chat and confirms it belongs to the session + * + * Returns either a 401/403/404 NextResponse describing the first + * failure, or the resolved chat row for the handler to render. + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being fetched. + * @returns A NextResponse on failure, or the chat row on success. + */ +export async function validateGetSessionChatRequest( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise> { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const sessionRows = await selectSessions({ id: sessionId }); + const session = sessionRows[0] ?? null; + + if (!session) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (session.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + 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" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + return chat; +} diff --git a/lib/sessions/chats/validatePatchSessionChatRequest.ts b/lib/sessions/chats/validatePatchSessionChatRequest.ts new file mode 100644 index 000000000..77470c799 --- /dev/null +++ b/lib/sessions/chats/validatePatchSessionChatRequest.ts @@ -0,0 +1,91 @@ +import { NextRequest, NextResponse } from "next/server"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { selectChats } from "@/lib/supabase/chats/selectChats"; + +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(), + }) + .strict() + .refine(value => value.title !== undefined || value.modelId !== undefined, { + message: "At least one field is required", + }); + +export type PatchSessionChatBody = z.infer; + +/** + * Validates a `PATCH /api/sessions/{sessionId}/chats/{chatId}` request + * end-to-end: + * 1. Authenticates the caller via Privy Bearer / x-api-key + * 2. Loads the session and confirms the caller owns it + * 3. Loads the chat and confirms it belongs to the session + * 4. Parses the JSON body and ensures at least one updatable field + * is present and non-empty + * + * @param request - The incoming request. + * @param sessionId - The id of the parent session. + * @param chatId - The id of the chat being updated. + * @returns A NextResponse on failure, or the validated patch body on success. + */ +export async function validatePatchSessionChatRequest( + request: NextRequest, + sessionId: string, + chatId: string, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const sessionRows = await selectSessions({ id: sessionId }); + const session = sessionRows[0] ?? null; + + if (!session) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (session.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + 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" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + let rawBody: unknown; + try { + rawBody = await request.json(); + } catch { + return NextResponse.json( + { status: "error", error: "Invalid JSON body" }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + const parsed = patchSessionChatBodySchema.safeParse(rawBody); + if (!parsed.success) { + const firstError = parsed.error.issues[0]; + return NextResponse.json( + { status: "error", error: firstError.message }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + return parsed.data; +} diff --git a/lib/supabase/chats/deleteChat.ts b/lib/supabase/chats/deleteChat.ts new file mode 100644 index 000000000..3bc0d663e --- /dev/null +++ b/lib/supabase/chats/deleteChat.ts @@ -0,0 +1,19 @@ +import supabase from "@/lib/supabase/serverClient"; + +/** + * Deletes the `chats` row identified by `chatId`. Returns `true` on + * success and `false` on Supabase error after logging. + * + * @param chatId - The id of the chat to delete. + * @returns `true` on success, `false` on error. + */ +export async function deleteChat(chatId: string): Promise { + const { error } = await supabase.from("chats").delete().eq("id", chatId); + + if (error) { + console.error("Error deleting chat:", error); + return false; + } + + return true; +}