Uh oh!
There was an error while loading. Please reload this page.
fix: handle duplicate room creation race condition (#165) - #167
Merged
Conversation
* fix: handle duplicate room creation race condition Catch 23505 unique constraint error when frontend creates room before backend's selectRoom sees it. Skip notification if room already exists. * fix: keep Promise.all, just wrap in try/catch Simpler fix - keep parallel execution, only catch the 23505 error. * fix: handle duplicate room in createNewRoom.ts This is the actual fix - createNewRoom is called during setupConversation at request START, which is where the 500 error occurs. * fix: revert try/catch bandaid - no longer needed Frontend no longer creates rooms, so no race condition. Backend is single source of truth for room creation. * fix: use upsert instead of insert for room creation - Changed insertRoom to use upsert with ignoreDuplicates: true - Reverted try/catch in createNewRoom.ts and handleChatCompletion.ts - Cleaner solution: duplicates are silently ignored at the DB level * fix: use upsert instead of insert for rooms One-line fix: change insert to upsert in insertRoom.ts. Removes try/catch workarounds since upsert handles duplicates. * refactor: rename insertRoom to upsertRoom to match method Renames the function and file from insertRoom to upsertRoom to align with the actual Supabase method being called (.upsert()). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add unit tests for upsertRoom function Adds comprehensive unit tests covering: - Basic upsert operation - Null artist_id handling - Null topic handling - Error handling on upsert failure - Upsert behavior (update on conflict) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR systematically replaces the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Braintrust eval reportCatalog Opportunity Analysis Evaluation (HEAD-1769548788)
Catalog Songs Count Evaluation (HEAD-1769548788)
First Week Album Sales Evaluation (HEAD-1769548788)
Memory & Storage Tools Evaluation (HEAD-1769548789)
Monthly Listeners Tracking Evaluation (HEAD-1769548788)
Search Web Tool Evaluation (HEAD-1769548788)
Social Scraping Evaluation (HEAD-1769548788)
Spotify Followers Evaluation (HEAD-1769548788)
Spotify Tools Evaluation (HEAD-1769548788)
TikTok Analytics Questions Evaluation (HEAD-1769548788)
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/chat/createNewRoom.ts (1)
40-45: Inconsistent handling of optionalartist_id.This file uses
artistId || undefined(line 43), whilehandleChatCompletion.tsusesartistId || null(line 59). This inconsistency could lead to different database behavior:
undefinedis typically omitted from the payload by Supabasenullexplicitly sets the column to NULLFor consistent upsert behavior, use
nullin both places to ensure the column is explicitly set.🔧 Suggested fix
upsertRoom({ account_id: accountId, topic: conversationName, - artist_id: artistId || undefined,+ artist_id: artistId || null, id: roomId, }),lib/chat/__tests__/integration/chatEndToEnd.test.ts (1)
140-147: Fix ESLintimport/firstviolation — imports must appear beforevi.mockcalls.The
import/firstrule is configured as an error and currently fails because imports on lines 140–156 come after thevi.mockblocks ending at line 138. Move all imports to the top of the file before anyvi.mockcalls to resolve this linting error.
🤖 Fix all issues with AI agents
In `@lib/chat/__tests__/handleChatCompletion.test.ts`:
- Around line 37-40: Tests currently import selectAccountEmails, selectRoom,
upsertRoom, and upsertMemory after vi.mock() calls, causing an ESLint
import/first violation; move those import statements so they appear immediately
after the initial vitest imports (before any vi.mock(...) blocks) to satisfy the
import/first rule while keeping the existing vi.mock(...) calls intact and
relying on Vitest hoisting behavior.
In `@lib/supabase/rooms/upsertRoom.ts`:
- Around line 8-9: The upsertRoom function calls
supabase.from("rooms").upsert(params) without an explicit onConflict, so update
the upsert invocation in upsertRoom to include the onConflict option (e.g., {
onConflict: "id" }) to match other upserts for consistency; locate the
upsertRoom function and change the upsert call to pass the onConflict option
while keeping the existing select("*").single() flow.
🧹 Nitpick comments (2)
lib/supabase/rooms/__tests__/upsertRoom.test.ts (1)
104-116: Misleading test description and mock error.The test simulates an error scenario but uses a "Duplicate key violation" error code (
23505), which shouldn't occur when usingupsertcorrectly since upserts handle conflicts gracefully. Consider updating the mock error to reflect a more realistic failure scenario (e.g., connection error, validation failure).💡 Suggested improvement
it("throws an error when upsert fails", async () => { - const mockError = { message: "Duplicate key violation", code: "23505" };+ const mockError = { message: "Database connection failed", code: "PGRST301" }; mockSingle.mockResolvedValue({ data: null, error: mockError });lib/rooms/__tests__/copyRoom.test.ts (1)
77-84: Minor: Update test description to reflect upsert.The test description says "room insertion fails" but should say "room upsert fails" for consistency with the new implementation.
💡 Suggested fix
- it("returns null when room insertion fails", async () => {+ it("returns null when room upsert fails", async () => {
Comment on lines
37
to
40
| import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails"; | ||
| import selectRoom from "@/lib/supabase/rooms/selectRoom"; | ||
| import { insertRoom } from "@/lib/supabase/rooms/insertRoom"; | ||
| import { upsertRoom } from "@/lib/supabase/rooms/upsertRoom"; | ||
| import upsertMemory from "@/lib/supabase/memories/upsertMemory"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/chat/__tests__/handleChatCompletion.test.ts | head -60Repository: Recoupable-com/Recoup-API
Length of output: 2783
🏁 Script executed:
sed -n '1,50p' lib/chat/__tests__/handleChatCompletion.test.ts | tail -20Repository: Recoupable-com/Recoup-API
Length of output: 1053
Move imports above vi.mock() blocks to fix ESLint import/first violation.
The test file currently has imports on lines 37–46 appearing after vi.mock() blocks, which violates the ESLint import/first rule. Even though Vitest hoists mocks at module load time, the linter enforces import ordering. Relocate all imports to the top of the file, immediately after the initial vitest imports.
🔧 Proposed fix
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import type { UIMessage } from "ai";
+import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails";+import selectRoom from "@/lib/supabase/rooms/selectRoom";+import { upsertRoom } from "@/lib/supabase/rooms/upsertRoom";+import upsertMemory from "@/lib/supabase/memories/upsertMemory";+import { sendNewConversationNotification } from "@/lib/telegram/sendNewConversationNotification";+import { generateChatTitle } from "@/lib/chat/generateChatTitle";+import { handleSendEmailToolOutputs } from "@/lib/emails/handleSendEmailToolOutputs";+import { sendErrorNotification } from "@/lib/telegram/sendErrorNotification";+import { handleChatCompletion } from "../handleChatCompletion";+import type { ChatRequestBody } from "../validateChatRequest";+
// Mock all dependencies before importing the module under test
vi.mock("@/lib/supabase/account_emails/selectAccountEmails", () => ({
default: vi.fn(),
}));
--import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails";-import selectRoom from "@/lib/supabase/rooms/selectRoom";-import { upsertRoom } from "@/lib/supabase/rooms/upsertRoom";-import upsertMemory from "@/lib/supabase/memories/upsertMemory";-import { sendNewConversationNotification } from "@/lib/telegram/sendNewConversationNotification";-import { generateChatTitle } from "@/lib/chat/generateChatTitle";-import { handleSendEmailToolOutputs } from "@/lib/emails/handleSendEmailToolOutputs";-import { sendErrorNotification } from "@/lib/telegram/sendErrorNotification";-import { handleChatCompletion } from "../handleChatCompletion";-import type { ChatRequestBody } from "../validateChatRequest";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| importselectAccountEmailsfrom"@/lib/supabase/account_emails/selectAccountEmails"; | |
| importselectRoomfrom"@/lib/supabase/rooms/selectRoom"; | |
| import{insertRoom}from"@/lib/supabase/rooms/insertRoom"; | |
| import{upsertRoom}from"@/lib/supabase/rooms/upsertRoom"; | |
| importupsertMemoryfrom"@/lib/supabase/memories/upsertMemory"; | |
| import{describe,it,expect,vi,beforeEach,afterEach}from"vitest"; | |
| importtype{UIMessage}from"ai"; | |
| importselectAccountEmailsfrom"@/lib/supabase/account_emails/selectAccountEmails"; | |
| importselectRoomfrom"@/lib/supabase/rooms/selectRoom"; | |
| import{upsertRoom}from"@/lib/supabase/rooms/upsertRoom"; | |
| importupsertMemoryfrom"@/lib/supabase/memories/upsertMemory"; | |
| import{sendNewConversationNotification}from"@/lib/telegram/sendNewConversationNotification"; | |
| import{generateChatTitle}from"@/lib/chat/generateChatTitle"; | |
| import{handleSendEmailToolOutputs}from"@/lib/emails/handleSendEmailToolOutputs"; | |
| import{sendErrorNotification}from"@/lib/telegram/sendErrorNotification"; | |
| import{handleChatCompletion}from"../handleChatCompletion"; | |
| importtype{ChatRequestBody}from"../validateChatRequest"; | |
| // Mock all dependencies before importing the module under test | |
| vi.mock("@/lib/supabase/account_emails/selectAccountEmails",()=>({ | |
| default: vi.fn(), | |
| })); | |
| // ... rest of vi.mock() blocks ... |
🧰 Tools
🪛 ESLint
[error] 37-37: Import in body of module; reorder to top.
(import/first)
[error] 38-38: Import in body of module; reorder to top.
(import/first)
[error] 39-39: Import in body of module; reorder to top.
(import/first)
[error] 40-40: Import in body of module; reorder to top.
(import/first)
🤖 Prompt for AI Agents
In `@lib/chat/__tests__/handleChatCompletion.test.ts` around lines 37 - 40, Tests
currently import selectAccountEmails, selectRoom, upsertRoom, and upsertMemory
after vi.mock() calls, causing an ESLint import/first violation; move those
import statements so they appear immediately after the initial vitest imports
(before any vi.mock(...) blocks) to satisfy the import/first rule while keeping
the existing vi.mock(...) calls intact and relying on Vitest hoisting behavior.
Comment on lines
+8
to
+9
| export const upsertRoom = async (params: CreateRoomParams): Promise<Room> => { | ||
| const { data, error } = await supabase.from("rooms").upsert(params).select("*").single(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Supabase upsert ignoreDuplicates option behavior documentation
💡 Result:
In Supabase, the upsert(..., { ignoreDuplicates }) option controls what happens when a row conflicts with an existing row (as determined by onConflict or, if omitted, the table’s primary key):
ignoreDuplicates: true→ skip/ignore conflicting rows (PostgRESTPrefer: resolution=ignore-duplicates, similar toON CONFLICT DO NOTHING). Supabase docs describe this as “duplicate rows are ignored.” [1] PostgREST documents the same behavior. [2]ignoreDuplicates: false(default) → merge/update conflicting rows (PostgRESTPrefer: resolution=merge-duplicates, similar toON CONFLICT DO UPDATE). Supabase docs describe this as “duplicate rows are merged with existing rows.” [1][2]
How “duplicate/conflict” is determined:
- With
onConflict: 'col1,col2', conflicts are based on those UNIQUE column(s). [1][2] - Without
onConflict, PostgREST upsert operates on the primary key columns. [2]
Return behavior note:
- With
resolution=ignore-duplicates+return=representation(what Supabase does when you chain.select()), PostgREST may return an empty array for ignored rows rather than returning the existing row; this is discussed as an issue/behavior in PostgREST. [3]
Sources:
[1] Supabase JS upsert reference (ignoreDuplicates/onConflict descriptions) (docs-451de2d9m-supabase.vercel.app)
[2] PostgREST “Upsert” docs (merge-duplicates / ignore-duplicates, PK default, on_conflict) (postgrest.org)
[3] PostgREST issue about ignore-duplicates + return=representation returning empty array (github.com)
Citations:
- 1: https://docs-451de2d9m-supabase.vercel.app/docs/reference/javascript/upsert?utm_source=openai
- 2: https://postgrest.org/en/v11/references/api/tables_views.html?utm_source=openai
- 3: Return ignored rows when
resolution=ignore-duplicatesis combined withreturn=representationPostgREST/postgrest#3976
🏁 Script executed:
# Find and read the upsertRoom.ts file to understand full context
find . -name "upsertRoom.ts" -type f | head -5Repository: Recoupable-com/Recoup-API
Length of output: 103
🏁 Script executed:
# Search for CreateRoomParams and Room type definitions
rg "type CreateRoomParams|interface CreateRoomParams" -A 5Repository: Recoupable-com/Recoup-API
Length of output: 546
🏁 Script executed:
# Search for Room type definition
rg "type Room|interface Room" -A 10Repository: Recoupable-com/Recoup-API
Length of output: 2161
🏁 Script executed:
# Look for other upsert/insert/update operations in lib/supabase to see patterns
rg "\.upsert\(|\.insert\(|\.update\(" lib/supabase -A 2 | head -40Repository: Recoupable-com/Recoup-API
Length of output: 2316
🏁 Script executed:
# Check git log or PR context for the objective
git log --oneline --all --grep="ignore duplicates" -i 2>/dev/null | head -10Repository: Recoupable-com/Recoup-API
Length of output: 51
Ensure explicit onConflict configuration for consistency.
The upsert() call omits the onConflict option and relies on implicit primary key conflict resolution. All other upsert operations in the codebase (e.g., upsertSongs.ts, upsertPulseAccount.ts) explicitly specify onConflict. For consistency and clarity, add { onConflict: "id" }:
- const { data, error } = await supabase.from("rooms").upsert(params).select("*").single();+ const { data, error } = await supabase.from("rooms").upsert(params, { onConflict: "id" }).select("*").single();The current default behavior (merging/updating on conflict) matches the "upsert" semantics and is correct for this use case.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| exportconstupsertRoom=async(params: CreateRoomParams): Promise<Room>=>{ | |
| const{ data, error }=awaitsupabase.from("rooms").upsert(params).select("*").single(); | |
| exportconstupsertRoom=async(params: CreateRoomParams): Promise<Room>=>{ | |
| const{ data, error }=awaitsupabase.from("rooms").upsert(params,{onConflict: "id"}).select("*").single(); |
🤖 Prompt for AI Agents
In `@lib/supabase/rooms/upsertRoom.ts` around lines 8 - 9, The upsertRoom function
calls supabase.from("rooms").upsert(params) without an explicit onConflict, so
update the upsert invocation in upsertRoom to include the onConflict option
(e.g., { onConflict: "id" }) to match other upserts for consistency; locate the
upsertRoom function and change the upsert call to pass the onConflict option
while keeping the existing select("*").single() flow.
4 tasks
sweetmantech added a commit
that referenced
this pull request
Apr 27, 2026
* feat(connectors): implement GET + POST /api/connectors/actions Implements the contract from recoupable/docs PR #167 (merged) — gives sandbox agents (open-agents) HTTP access to the connector action layer the chat agent uses internally. Same auth pattern as the existing /api/connectors family (validateAuthContext + RECOUP_ACCESS_TOKEN). Built TDD: each new file has its test written first, run failing, then implemented to green. 40 new tests across 8 new test files; all 113 connectors-suite tests pass. - GET /api/connectors/actions — lists every executable action across the account's supported toolkits. Each action returns slug (UPPERCASE_SNAKE_CASE), name, description, parameters JSON Schema, parent connectorSlug derived from slug prefix, and isConnected derived from the parent toolkit's connection state. Optional account_id query param mirrors GET /api/connectors. - POST /api/connectors/actions — executes an action by slug with parameters via the Vercel-AI-SDK-wrapped Composio tool.execute(). Composio validates parameters and connection state internally; the ConnectorActionNotFoundError class maps the slug-not-found case to 404. Other failures surface as 500 with the underlying message (granular 400/409/502 mapping deferred to a follow-up once we see real Composio error shapes in QA). Files: - app/api/connectors/actions/route.ts — Next.js route exporting GET + POST + OPTIONS, all delegating to the handlers. - lib/composio/connectors/getConnectorActions.ts — Composio fetch logic reusing the SUPPORTED_TOOLKITS list + buildAuthConfigs() pattern. - lib/composio/connectors/executeConnectorAction.ts — execute logic + ConnectorActionNotFoundError exception class. - lib/composio/connectors/getConnectorActionsHandler.ts — orchestrator for GET, returns flat { success, actions } per the project's flat-response convention. - lib/composio/connectors/executeConnectorActionHandler.ts — orchestrator for POST, returns flat { success, result, executedAt }; maps ConnectorActionNotFoundError to 404. - lib/composio/connectors/validate{GetConnectorActions,ExecuteConnectorAction}{Query,Body,Request}.ts — Zod-based input validation + auth + access-check orchestration, matching the existing pattern used by getConnectors / authorizeConnector. * fix(connectors): convert Zod inputSchema to JSON Schema in catalog response Smoke-testing the preview deployment surfaced that Composio's VercelProvider hands tools' inputSchema back as live Zod schema instances. Spreading those directly into the API response leaked internal Zod fields (_def, ~standard, _cached, ZodObject typeName, ZodNever catchall, etc.) into the public catalog — violating the OpenAPI contract that says `parameters` is JSON Schema. - New lib/composio/connectors/toJsonSchema.ts helper detects Zod schemas via the _def marker and runs them through Zod 4's z.toJSONSchema(). Plain JSON Schema objects pass through unchanged so a future provider that already returns JSON Schema works without code changes. Null/undefined and unrecognized inputs return {} rather than throwing. - getConnectorActions now routes tool.inputSchema through toJsonSchema before returning. - 5 new tests for the helper (TDD red→green) covering: undefined, null, Zod conversion, plain JSON Schema pass-through, and malformed-Zod fallback. - 1 new integration test in getConnectorActions.test.ts asserting Zod inputs come out as JSON Schema with no Zod internals leaked. - All 119 connector tests pass; lint clean. * fix(connectors): handle Zod v3 schemas (Composio's bundled version) First-pass fix used Zod 4's z.toJSONSchema() which only recognises Zod v4 schemas. Live preview testing revealed Composio actually bundles Zod v3 — symptom: parameters came back as {} (the converter threw, hit the fallback) instead of leaking Zod internals. Switch to a dual-converter strategy: - Zod v4 schemas (via our top-level zod@^4) → z.toJSONSchema() - Zod v3 schemas (Composio's bundled version) → zod-to-json-schema Detection: Zod v4 has _def.type, Zod v3 has _def.typeName. Try v4 first, fall back to v3, then bail to {} only on double failure. - Add `zod-to-json-schema` dependency. - Update toJsonSchema.ts to dispatch based on _def shape. - Update tests to reflect the more permissive behavior of zod-to-json-schema (it produces best-effort output rather than throwing on malformed Zod-like inputs); test still asserts no Zod internals leak through. * fix(connectors): merge shared + artist toolkits into the actions catalog The first cut of getConnectorActions only resolved the customer's own Composio session, so the catalog missed everything the chat agent gets via Recoupable's shared platform-level connections (SHARED_ACCOUNT_ID = "recoup-shared-...") plus any artist-scoped connections. End result: catalog returned only the 6 Composio meta-tools when the test account had no personal connections, even though the chat agent has full access to e.g. Google Drive through the shared account. Fix: delegate to the existing getComposioTools() — same function the chat agent uses — which merges customer > artist > shared in priority order via resolveSessionToolkits + fetchOwnerTools. The catalog now mirrors what chat actually has access to. Anything returned is executable, so isConnected is true for every action. - getConnectorActions.ts: rewritten as a thin wrapper around getComposioTools() + per-tool transformation. Drops the duplicated session/toolkit/buildAuthConfigs logic. - getConnectorActions.test.ts: rewritten to mock getComposioTools instead of the raw Composio client. 6 tests covering: full mapping with two toolkits, slug-prefix derivation, missing description/inputSchema defaults, Zod→JSON Schema conversion, and empty-tools edge case. Limitation noted: actions for *unconnected* connectors are not listed (Composio's session filters them out). If a sandbox needs to discover what it could authorize, that's a future enhancement. * fix(connectors): merge shared toolkits into the execute path too Smoke-test against the preview hit a real bug: GET catalog listed GOOGLEDOCS_INSERT_TEXT_ACTION (since the shared platform connection exposes Google Docs to every account), but POST execute returned 404 "Connector action not found". The catalog and execute paths were resolving different tool sets — catalog used getComposioTools() (merged customer→artist→shared), execute used composio.create() directly which only resolved the customer session. Fix: executeConnectorAction now also delegates to getComposioTools(), so the executable surface matches the catalog 1:1. Anything GET lists is now actually executable via POST. Side cleanup: extract ConnectorActionNotFoundError into its own file (connectorActionErrors.ts) so the handler test can import it without pulling executeConnectorAction's deep dep chain (getComposioTools → checkAccountArtistAccess → Supabase) at test load time. - executeConnectorAction.ts: rewritten as a thin wrapper around getComposioTools() — finds the tool by slug in the merged set, calls tool.execute(parameters). SUPPORTED_TOOLKITS / authConfigs duplication deleted. - connectorActionErrors.ts: new file holding ConnectorActionNotFoundError. - executeConnectorActionHandler.ts: imports the error from the new file (handler test no longer needs vi.importActual). - executeConnectorAction.test.ts: rewritten to mock getComposioTools. - executeConnectorActionHandler.test.ts: simpler vi.mock without importActual.
sweetmantech added a commit
that referenced
this pull request
Apr 27, 2026
…#485) * feat(connectors): implement GET + POST /api/connectors/actions Implements the contract from recoupable/docs PR #167 (merged) — gives sandbox agents (open-agents) HTTP access to the connector action layer the chat agent uses internally. Same auth pattern as the existing /api/connectors family (validateAuthContext + RECOUP_ACCESS_TOKEN). Built TDD: each new file has its test written first, run failing, then implemented to green. 40 new tests across 8 new test files; all 113 connectors-suite tests pass. - GET /api/connectors/actions — lists every executable action across the account's supported toolkits. Each action returns slug (UPPERCASE_SNAKE_CASE), name, description, parameters JSON Schema, parent connectorSlug derived from slug prefix, and isConnected derived from the parent toolkit's connection state. Optional account_id query param mirrors GET /api/connectors. - POST /api/connectors/actions — executes an action by slug with parameters via the Vercel-AI-SDK-wrapped Composio tool.execute(). Composio validates parameters and connection state internally; the ConnectorActionNotFoundError class maps the slug-not-found case to 404. Other failures surface as 500 with the underlying message (granular 400/409/502 mapping deferred to a follow-up once we see real Composio error shapes in QA). Files: - app/api/connectors/actions/route.ts — Next.js route exporting GET + POST + OPTIONS, all delegating to the handlers. - lib/composio/connectors/getConnectorActions.ts — Composio fetch logic reusing the SUPPORTED_TOOLKITS list + buildAuthConfigs() pattern. - lib/composio/connectors/executeConnectorAction.ts — execute logic + ConnectorActionNotFoundError exception class. - lib/composio/connectors/getConnectorActionsHandler.ts — orchestrator for GET, returns flat { success, actions } per the project's flat-response convention. - lib/composio/connectors/executeConnectorActionHandler.ts — orchestrator for POST, returns flat { success, result, executedAt }; maps ConnectorActionNotFoundError to 404. - lib/composio/connectors/validate{GetConnectorActions,ExecuteConnectorAction}{Query,Body,Request}.ts — Zod-based input validation + auth + access-check orchestration, matching the existing pattern used by getConnectors / authorizeConnector. * fix(connectors): convert Zod inputSchema to JSON Schema in catalog response Smoke-testing the preview deployment surfaced that Composio's VercelProvider hands tools' inputSchema back as live Zod schema instances. Spreading those directly into the API response leaked internal Zod fields (_def, ~standard, _cached, ZodObject typeName, ZodNever catchall, etc.) into the public catalog — violating the OpenAPI contract that says `parameters` is JSON Schema. - New lib/composio/connectors/toJsonSchema.ts helper detects Zod schemas via the _def marker and runs them through Zod 4's z.toJSONSchema(). Plain JSON Schema objects pass through unchanged so a future provider that already returns JSON Schema works without code changes. Null/undefined and unrecognized inputs return {} rather than throwing. - getConnectorActions now routes tool.inputSchema through toJsonSchema before returning. - 5 new tests for the helper (TDD red→green) covering: undefined, null, Zod conversion, plain JSON Schema pass-through, and malformed-Zod fallback. - 1 new integration test in getConnectorActions.test.ts asserting Zod inputs come out as JSON Schema with no Zod internals leaked. - All 119 connector tests pass; lint clean. * fix(connectors): handle Zod v3 schemas (Composio's bundled version) First-pass fix used Zod 4's z.toJSONSchema() which only recognises Zod v4 schemas. Live preview testing revealed Composio actually bundles Zod v3 — symptom: parameters came back as {} (the converter threw, hit the fallback) instead of leaking Zod internals. Switch to a dual-converter strategy: - Zod v4 schemas (via our top-level zod@^4) → z.toJSONSchema() - Zod v3 schemas (Composio's bundled version) → zod-to-json-schema Detection: Zod v4 has _def.type, Zod v3 has _def.typeName. Try v4 first, fall back to v3, then bail to {} only on double failure. - Add `zod-to-json-schema` dependency. - Update toJsonSchema.ts to dispatch based on _def shape. - Update tests to reflect the more permissive behavior of zod-to-json-schema (it produces best-effort output rather than throwing on malformed Zod-like inputs); test still asserts no Zod internals leak through. * fix(connectors): merge shared + artist toolkits into the actions catalog The first cut of getConnectorActions only resolved the customer's own Composio session, so the catalog missed everything the chat agent gets via Recoupable's shared platform-level connections (SHARED_ACCOUNT_ID = "recoup-shared-...") plus any artist-scoped connections. End result: catalog returned only the 6 Composio meta-tools when the test account had no personal connections, even though the chat agent has full access to e.g. Google Drive through the shared account. Fix: delegate to the existing getComposioTools() — same function the chat agent uses — which merges customer > artist > shared in priority order via resolveSessionToolkits + fetchOwnerTools. The catalog now mirrors what chat actually has access to. Anything returned is executable, so isConnected is true for every action. - getConnectorActions.ts: rewritten as a thin wrapper around getComposioTools() + per-tool transformation. Drops the duplicated session/toolkit/buildAuthConfigs logic. - getConnectorActions.test.ts: rewritten to mock getComposioTools instead of the raw Composio client. 6 tests covering: full mapping with two toolkits, slug-prefix derivation, missing description/inputSchema defaults, Zod→JSON Schema conversion, and empty-tools edge case. Limitation noted: actions for *unconnected* connectors are not listed (Composio's session filters them out). If a sandbox needs to discover what it could authorize, that's a future enhancement. * fix(connectors): merge shared toolkits into the execute path too Smoke-test against the preview hit a real bug: GET catalog listed GOOGLEDOCS_INSERT_TEXT_ACTION (since the shared platform connection exposes Google Docs to every account), but POST execute returned 404 "Connector action not found". The catalog and execute paths were resolving different tool sets — catalog used getComposioTools() (merged customer→artist→shared), execute used composio.create() directly which only resolved the customer session. Fix: executeConnectorAction now also delegates to getComposioTools(), so the executable surface matches the catalog 1:1. Anything GET lists is now actually executable via POST. Side cleanup: extract ConnectorActionNotFoundError into its own file (connectorActionErrors.ts) so the handler test can import it without pulling executeConnectorAction's deep dep chain (getComposioTools → checkAccountArtistAccess → Supabase) at test load time. - executeConnectorAction.ts: rewritten as a thin wrapper around getComposioTools() — finds the tool by slug in the merged set, calls tool.execute(parameters). SUPPORTED_TOOLKITS / authConfigs duplication deleted. - connectorActionErrors.ts: new file holding ConnectorActionNotFoundError. - executeConnectorActionHandler.ts: imports the error from the new file (handler test no longer needs vi.importActual). - executeConnectorAction.test.ts: rewritten to mock getComposioTools. - executeConnectorActionHandler.test.ts: simpler vi.mock without importActual.
sweetmantech added a commit
that referenced
this pull request
Apr 28, 2026
* feat(connectors): implement GET + POST /api/connectors/actions (#484) * feat(connectors): implement GET + POST /api/connectors/actions Implements the contract from recoupable/docs PR #167 (merged) — gives sandbox agents (open-agents) HTTP access to the connector action layer the chat agent uses internally. Same auth pattern as the existing /api/connectors family (validateAuthContext + RECOUP_ACCESS_TOKEN). Built TDD: each new file has its test written first, run failing, then implemented to green. 40 new tests across 8 new test files; all 113 connectors-suite tests pass. - GET /api/connectors/actions — lists every executable action across the account's supported toolkits. Each action returns slug (UPPERCASE_SNAKE_CASE), name, description, parameters JSON Schema, parent connectorSlug derived from slug prefix, and isConnected derived from the parent toolkit's connection state. Optional account_id query param mirrors GET /api/connectors. - POST /api/connectors/actions — executes an action by slug with parameters via the Vercel-AI-SDK-wrapped Composio tool.execute(). Composio validates parameters and connection state internally; the ConnectorActionNotFoundError class maps the slug-not-found case to 404. Other failures surface as 500 with the underlying message (granular 400/409/502 mapping deferred to a follow-up once we see real Composio error shapes in QA). Files: - app/api/connectors/actions/route.ts — Next.js route exporting GET + POST + OPTIONS, all delegating to the handlers. - lib/composio/connectors/getConnectorActions.ts — Composio fetch logic reusing the SUPPORTED_TOOLKITS list + buildAuthConfigs() pattern. - lib/composio/connectors/executeConnectorAction.ts — execute logic + ConnectorActionNotFoundError exception class. - lib/composio/connectors/getConnectorActionsHandler.ts — orchestrator for GET, returns flat { success, actions } per the project's flat-response convention. - lib/composio/connectors/executeConnectorActionHandler.ts — orchestrator for POST, returns flat { success, result, executedAt }; maps ConnectorActionNotFoundError to 404. - lib/composio/connectors/validate{GetConnectorActions,ExecuteConnectorAction}{Query,Body,Request}.ts — Zod-based input validation + auth + access-check orchestration, matching the existing pattern used by getConnectors / authorizeConnector. * fix(connectors): convert Zod inputSchema to JSON Schema in catalog response Smoke-testing the preview deployment surfaced that Composio's VercelProvider hands tools' inputSchema back as live Zod schema instances. Spreading those directly into the API response leaked internal Zod fields (_def, ~standard, _cached, ZodObject typeName, ZodNever catchall, etc.) into the public catalog — violating the OpenAPI contract that says `parameters` is JSON Schema. - New lib/composio/connectors/toJsonSchema.ts helper detects Zod schemas via the _def marker and runs them through Zod 4's z.toJSONSchema(). Plain JSON Schema objects pass through unchanged so a future provider that already returns JSON Schema works without code changes. Null/undefined and unrecognized inputs return {} rather than throwing. - getConnectorActions now routes tool.inputSchema through toJsonSchema before returning. - 5 new tests for the helper (TDD red→green) covering: undefined, null, Zod conversion, plain JSON Schema pass-through, and malformed-Zod fallback. - 1 new integration test in getConnectorActions.test.ts asserting Zod inputs come out as JSON Schema with no Zod internals leaked. - All 119 connector tests pass; lint clean. * fix(connectors): handle Zod v3 schemas (Composio's bundled version) First-pass fix used Zod 4's z.toJSONSchema() which only recognises Zod v4 schemas. Live preview testing revealed Composio actually bundles Zod v3 — symptom: parameters came back as {} (the converter threw, hit the fallback) instead of leaking Zod internals. Switch to a dual-converter strategy: - Zod v4 schemas (via our top-level zod@^4) → z.toJSONSchema() - Zod v3 schemas (Composio's bundled version) → zod-to-json-schema Detection: Zod v4 has _def.type, Zod v3 has _def.typeName. Try v4 first, fall back to v3, then bail to {} only on double failure. - Add `zod-to-json-schema` dependency. - Update toJsonSchema.ts to dispatch based on _def shape. - Update tests to reflect the more permissive behavior of zod-to-json-schema (it produces best-effort output rather than throwing on malformed Zod-like inputs); test still asserts no Zod internals leak through. * fix(connectors): merge shared + artist toolkits into the actions catalog The first cut of getConnectorActions only resolved the customer's own Composio session, so the catalog missed everything the chat agent gets via Recoupable's shared platform-level connections (SHARED_ACCOUNT_ID = "recoup-shared-...") plus any artist-scoped connections. End result: catalog returned only the 6 Composio meta-tools when the test account had no personal connections, even though the chat agent has full access to e.g. Google Drive through the shared account. Fix: delegate to the existing getComposioTools() — same function the chat agent uses — which merges customer > artist > shared in priority order via resolveSessionToolkits + fetchOwnerTools. The catalog now mirrors what chat actually has access to. Anything returned is executable, so isConnected is true for every action. - getConnectorActions.ts: rewritten as a thin wrapper around getComposioTools() + per-tool transformation. Drops the duplicated session/toolkit/buildAuthConfigs logic. - getConnectorActions.test.ts: rewritten to mock getComposioTools instead of the raw Composio client. 6 tests covering: full mapping with two toolkits, slug-prefix derivation, missing description/inputSchema defaults, Zod→JSON Schema conversion, and empty-tools edge case. Limitation noted: actions for *unconnected* connectors are not listed (Composio's session filters them out). If a sandbox needs to discover what it could authorize, that's a future enhancement. * fix(connectors): merge shared toolkits into the execute path too Smoke-test against the preview hit a real bug: GET catalog listed GOOGLEDOCS_INSERT_TEXT_ACTION (since the shared platform connection exposes Google Docs to every account), but POST execute returned 404 "Connector action not found". The catalog and execute paths were resolving different tool sets — catalog used getComposioTools() (merged customer→artist→shared), execute used composio.create() directly which only resolved the customer session. Fix: executeConnectorAction now also delegates to getComposioTools(), so the executable surface matches the catalog 1:1. Anything GET lists is now actually executable via POST. Side cleanup: extract ConnectorActionNotFoundError into its own file (connectorActionErrors.ts) so the handler test can import it without pulling executeConnectorAction's deep dep chain (getComposioTools → checkAccountArtistAccess → Supabase) at test load time. - executeConnectorAction.ts: rewritten as a thin wrapper around getComposioTools() — finds the tool by slug in the merged set, calls tool.execute(parameters). SUPPORTED_TOOLKITS / authConfigs duplication deleted. - connectorActionErrors.ts: new file holding ConnectorActionNotFoundError. - executeConnectorActionHandler.ts: imports the error from the new file (handler test no longer needs vi.importActual). - executeConnectorAction.test.ts: rewritten to mock getComposioTools. - executeConnectorActionHandler.test.ts: simpler vi.mock without importActual. * refactor(tasks): enhance PATCH /api/tasks with improved validation and access control (#486) * refactor(tasks): enhance PATCH /api/tasks with improved validation and access control - Updated the PATCH endpoint to require authentication and validate the task ID as a UUID. - Enhanced the request body to support merging optional fields while rejecting unknown keys. - Improved error handling to return 403 for unauthorized access and 404 for missing tasks. - Refactored the updateTask function to enforce ownership checks against the resolved account ID. - Removed the outdated validateUpdateTaskBody function in favor of a new validation approach. - Updated documentation to reflect changes in request and response structures. * refactor(tests): clean up imports in validateUpdateTaskRequest.auth.test.ts - Removed unnecessary import statements to streamline the test file. - Consolidated imports for better readability and maintainability. * fix(tasks): improve error handling in updateTask tool - Enhanced error handling in the updateTask tool to return specific error messages for task not found and access denied scenarios. - Updated the logic to log internal server errors while providing a generic error message for unexpected failures. - Adjusted the updateTask function to ensure it correctly handles the enabled state of tasks, defaulting to existing values when necessary. * fix(tasks): clarify error responses in updateTask documentation - Updated the documentation for the POST /api/tasks endpoint to specify that a missing or invalid task ID returns a 400 error, while a non-existent task returns a 404 error. This enhances clarity on the expected error handling for users and developers. --------- Co-authored-by: pradipthaadhi <yulius.upwork@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Catch 23505 unique constraint error when frontend creates room before backend's selectRoom sees it. Skip notification if room already exists.
Simpler fix - keep parallel execution, only catch the 23505 error.
This is the actual fix - createNewRoom is called during setupConversation at request START, which is where the 500 error occurs.
Frontend no longer creates rooms, so no race condition. Backend is single source of truth for room creation.
One-line fix: change insert to upsert in insertRoom.ts. Removes try/catch workarounds since upsert handles duplicates.
Renames the function and file from insertRoom to upsertRoom to align with the actual Supabase method being called (.upsert()).
Adds comprehensive unit tests covering:
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.