feat: Recoup Content Agent Slack bot#341
Open
recoup-coding-agent wants to merge 3 commits intotestfrom
Open
Conversation
Streams a complete AI-generated music release campaign given artist and song details. Returns 6 sections in real-time (press release, Spotify pitch, Instagram captions, TikTok hooks, fan newsletter, curator email) using XML-style section markers parsed by the frontend. - lib/launch/validateLaunchBody.ts — Zod validation for launch body - lib/launch/buildCampaignPrompt.ts — system + user prompt builders - lib/launch/generateCampaignHandler.ts — streamText handler with auth - lib/launch/__tests__/validateLaunchBody.test.ts — 8 passing tests - app/api/launch/route.ts — POST /api/launch route Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add content-agent following the existing coding-agent pattern: - Slack @mention handler that parses artist_account_id, template, batch, lipsync args - Triggers existing create-content pipeline via Trigger.dev - poll-content-run task monitors runs and posts video results back to Slack thread - Callback endpoint receives poll results and posts to Slack - Redis-backed thread state via Chat SDK New env vars required: SLACK_CONTENT_BOT_TOKEN, SLACK_CONTENT_SIGNING_SECRET, CONTENT_AGENT_CALLBACK_SECRET Co-Authored-By: Paperclip <noreply@paperclip.ing>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces comprehensive JSDoc documentation across ~90+ files and adds new content agent and campaign launch features, including dynamic webhook endpoints, Slack integration handlers, thread state management, and AI-powered campaign generation workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as External Callback
participant API as /api/content-agent/callback
participant Handler as handleContentAgentCallback
participant Validator as validateContentAgentCallback
participant ThreadMgr as getThread + setState
participant Slack as Slack API
Client->>API: POST (with auth header & JSON payload)
API->>Handler: Pass request
Handler->>Handler: Extract x-callback-secret header
Handler->>Handler: Verify against CONTENT_AGENT_CALLBACK_SECRET
alt Auth Invalid
Handler-->>Client: 401 Unauthorized (CORS)
else Auth Valid
Handler->>Handler: Parse request body as JSON
alt Parse Error
Handler-->>Client: 400 Invalid JSON (CORS)
else Parse Success
Handler->>Validator: validateContentAgentCallback(parsed)
alt Validation Error
Validator-->>Handler: NextResponse (error)
Handler-->>Client: Return error response
else Validation Success
Validator-->>Handler: ContentAgentCallbackBody
Handler->>ThreadMgr: getThread(validated.threadId)
ThreadMgr-->>Handler: Thread instance
Handler->>Handler: Check validated.status
alt Status: "completed"
Handler->>ThreadMgr: setState({status: "completed"})
Handler->>Slack: Post formatted video results
Handler->>Slack: Post completion summary
else Status: "failed"
Handler->>ThreadMgr: setState({status: "failed"})
Handler->>Slack: Post failure message
else Status: "timeout"
Handler->>ThreadMgr: setState({status: "timeout"})
Handler->>Slack: Post timeout notice
end
Handler-->>Client: 200 {status: "ok"} (CORS)
end
end
end
sequenceDiagram
participant User as Slack User
participant Bot as Content Agent Bot
participant Handler as handleContentAgentMention
participant Parser as parseMentionArgs
participant ArtistSvc as resolveArtistSlug / getArtistContentReadiness
participant Trigger as triggerCreateContent
participant Poller as triggerPollContentRun
participant Thread as Thread State
User->>Bot: `@content-agent` <artist_id> [template] [batch=N] [lipsync]
Bot->>Handler: onNewMention event
Handler->>Parser: parseMentionArgs(text)
Parser-->>Handler: {artistAccountId, template, batch, lipsync}
alt Missing artistAccountId
Handler->>Bot: Post usage guidance
else artistAccountId Present
Handler->>ArtistSvc: resolveArtistSlug(artistAccountId)
alt Artist Not Found
Handler->>Bot: Post "artist not found" message
else Artist Resolved
Handler->>ArtistSvc: Determine GitHub repo
Handler->>Bot: Post acknowledgment with artist/batch/lipsync/template
Handler->>Trigger: triggerCreateContent(payload) × batch
Trigger-->>Handler: Promise.allSettled results
alt No Successful Runs
Handler->>Bot: Post failure message
else Successful Runs
Handler->>Thread: setState({status: "running", artistAccountId, template, lipsync, batch, runIds})
Handler->>Poller: triggerPollContentRun({runIds, callbackThreadId})
Handler->>Bot: Async result reporting initiated
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR combines ~90+ homogeneous JSDoc-only changes with several substantial new modules introducing business logic. The repetitive documentation updates lower individual friction, but the heterogeneous mix—spanning new feature architecture (content agent bot, handlers, state management), validation schemas, API routes, and AI integration (launch campaign)—requires contextual reasoning for each functional section. Logic density in the content agent callback handler (branching on status with Slack posting) and mention handler (artist resolution, trigger polling orchestration) adds complexity. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/transcribe/route.ts (1)
12-33:⚠️ Potential issue | 🔴 CriticalDo not accept
account_idfrom request body; derive account from auth context.This allows account spoofing and bypasses the mandated auth flow. Replace body-provided
account_idwithvalidateAuthContext()output, and validate input via Zod before callingprocessAudioTranscription.As per coding guidelines: "Never use
account_idin request bodies or tool schemas; always derive the account ID from authentication" and "Always usevalidateAuthContext()for authentication in API routes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/transcribe/route.ts` around lines 12 - 33, Stop reading account_id from the request body and derive the account ID via validateAuthContext(); remove the body.account_id usage and instead call validateAuthContext() in this route, extract the authenticated account id and pass that as ownerAccountId to processAudioTranscription (instead of account_id). Also perform Zod validation on the incoming body fields (audio_url, artist_account_id, title, include_timestamps) before calling processAudioTranscription so malformed input is rejected early; ensure artist_account_id still comes from the validated body and that the validated include_timestamps is forwarded to includeTimestamps.
🟡 Minor comments (13)
lib/flamingo/getFlamingoPresetsHandler.ts-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorComplete the JSDoc parameter description.
The
@paramtag should include a description of what the parameter represents. Currently it's just@param requestwith no explanation.📝 Proposed fix to add parameter description
- * `@param` request + * `@param` request - The incoming Next.js request object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/flamingo/getFlamingoPresetsHandler.ts` at line 13, Update the JSDoc for the getFlamingoPresetsHandler function by replacing the bare "@param request" tag with a concise description of what the request argument represents (e.g., the incoming HTTP request object containing query/body headers used to determine which Flamingo presets to return), keeping the parameter name "request" so it matches the function signature; ensure the JSDoc clearly states the expected shape or purpose of the request (for example that it is the incoming API request used to read query params or body payload to filter presets).lib/prompts/getSystemPrompt.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorComplete the JSDoc parameter description.
The
@param params.orgIdentry is missing a description. All other parameters in this JSDoc block include descriptions (e.g., "The ID of the room", "The ID of the artist"), but this one only lists the parameter name. This inconsistency reduces documentation quality and maintainability.📝 Proposed fix to add parameter description
- * `@param` params.orgId + * `@param` params.orgId - The ID of the organization (optional)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/prompts/getSystemPrompt.ts` at line 16, The JSDoc for params.orgId is missing a description; update the JSDoc block in getSystemPrompt.ts (the getSystemPrompt function's comment) to provide a clear description for params.orgId (e.g., "The ID of the organization" or similar), matching the style of the other parameter descriptions so the documentation is consistent and complete.lib/coding-agent/handleMergeSuccess.ts-10-11 (1)
10-11:⚠️ Potential issue | 🟡 MinorComplete the JSDoc parameter documentation.
The
@paramtag is missing a description. JSDoc parameters should include both the parameter name and a description of what it represents.📝 Proposed fix to complete the JSDoc
* - * `@param` state + * `@param` state - The coding agent thread state containing PR and snapshot information + * `@returns` A promise that resolves when cleanup completes */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handleMergeSuccess.ts` around lines 10 - 11, The JSDoc for handleMergeSuccess has an incomplete `@param` entry — update the comment above the handleMergeSuccess function to document the state parameter by name and a short description of what it represents (e.g., "state - current agent state object containing merge context, branch info, and any relevant metadata"), ensuring the `@param` line follows JSDoc format and accurately reflects the shape/purpose of the state argument used in handleMergeSuccess.lib/slack/getSlackUserInfo.ts-19-21 (1)
19-21:⚠️ Potential issue | 🟡 MinorComplete the JSDoc parameter documentation.
The
@paramtags are missing descriptions. JSDoc best practice is to include what each parameter represents and its purpose.📝 Proposed fix to add parameter descriptions
* - * `@param` token - * `@param` userId + * `@param` token - Slack Bot OAuth token (xoxb-...) + * `@param` userId - Slack user ID to look up */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack/getSlackUserInfo.ts` around lines 19 - 21, Add missing JSDoc descriptions for the parameters of getSlackUserInfo: document what the token parameter is (e.g., Slack OAuth or bot token used to authenticate API requests) and what the userId parameter represents (the Slack user ID whose profile/info will be retrieved), and include expected types and any important format or usage notes so the `@param` tags are complete and informative.lib/content/createContentHandler.ts-12-13 (1)
12-13:⚠️ Potential issue | 🟡 MinorComplete the JSDoc parameter description.
The
@param requesttag is missing a description. JSDoc parameters should include a brief explanation of their purpose.📝 Proposed fix
/** * Handler for POST /api/content/create. * Always returns runIds array (KISS — one response shape for single and batch). * - * `@param` request + * `@param` request - The incoming HTTP request containing content creation parameters */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/content/createContentHandler.ts` around lines 12 - 13, Update the JSDoc for the createContentHandler function by adding a brief description to the existing `@param` request tag explaining what the `request` parameter represents (e.g., the incoming HTTP request or payload object, its expected shape or purpose) so the comment clearly documents the input to createContentHandler; keep the description concise and mention any important fields or types if applicable.app/api/coding-agent/[platform]/route.ts-38-40 (1)
38-40:⚠️ Potential issue | 🟡 Minor
@param params.paramsshould be removed in POST docs too.Keep only
@param paramsto match the actual signature.Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/coding-agent/`[platform]/route.ts around lines 38 - 40, Remove the incorrect JSDoc param entry "@param params.params" from the POST route JSDoc and leave only the correct "@param params" to match the actual handler signature; update the JSDoc block above the exported POST function (route handler) in app/api/coding-agent/[platform]/route.ts so it contains a single "@param params" line describing the route params.app/api/accounts/[id]/route.ts-26-27 (1)
26-27:⚠️ Potential issue | 🟡 MinorRemove redundant
@param params.paramsentry.
params.paramsis misleading here sinceparamsis already documented and destructured in the signature.Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/accounts/`[id]/route.ts around lines 26 - 27, Remove the redundant JSDoc tag "@param params.params" from the route handler's comment and keep a single clear "@param params" entry describing the route params (account ID); update the JSDoc above the exported route handler (the function in app/api/accounts/[id]/route.ts that destructures params) so it only documents "params" (or "params.id" in the description) and remove the misleading duplicate entry.app/api/coding-agent/[platform]/route.ts-13-15 (1)
13-15:⚠️ Potential issue | 🟡 Minor
@param params.paramsshould be removed in GET docs.This param path is not a real function parameter and can confuse generated docs.
Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/coding-agent/`[platform]/route.ts around lines 13 - 15, Remove the incorrect JSDoc entry "@param params.params" from the GET route's documentation; update the JSDoc for the GET handler (the exported GET function in route.ts) to only document the real parameter (e.g., "@param params - Route params containing the platform name") and any other valid params, ensuring there are no nested or duplicate param lines like "params.params" that don't exist on the function signature.app/api/admins/privy/route.ts-18-20 (1)
18-20:⚠️ Potential issue | 🟡 MinorReplace empty JSDoc on
OPTIONSwith a meaningful description.Line 18–20 currently adds an empty block, which doesn’t document the preflight behavior. Add a short summary (e.g., CORS preflight response and status 204) for maintainability.
As per coding guidelines: "All API routes should have JSDoc comments."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/admins/privy/route.ts` around lines 18 - 20, Replace the empty JSDoc block above the OPTIONS handler with a concise summary that documents this endpoint as the CORS preflight response for the Privy admin route and states it returns HTTP 204 No Content; update the JSDoc placed immediately above the OPTIONS function/handler (the block currently empty above the OPTIONS declaration) to include a one-line description like "CORS preflight response — returns 204 No Content" and mention it handles preflight checks for allowed methods and headers.lib/github/getRepoGitModules.ts-7-12 (1)
7-12:⚠️ Potential issue | 🟡 MinorRemove invalid nested
@paramtags that contradict the actual function signature.The function destructures
owner,repo, andbranchas direct string parameters, but the JSDoc includes incorrect nested entries (@param owner.owner,@param owner.repo,@param owner.branch) that suggestowneris an object. Remove these nested tags to align documentation with the actual signature.Proposed fix
- * `@param` owner.owner * `@param` owner - The GitHub repository owner * `@param` repo - The GitHub repository name * `@param` branch - The branch to fetch from - * `@param` owner.repo - * `@param` owner.branch🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/github/getRepoGitModules.ts` around lines 7 - 12, The JSDoc for getRepoGitModules contains invalid nested `@param` entries (e.g., `@param` owner.owner, `@param` owner.repo, `@param` owner.branch) that contradict the function signature which takes owner, repo, and branch as direct string parameters; remove those nested tags and ensure only top-level `@param` tags remain for owner, repo, and branch (and adjust any descriptions if needed) so the documentation matches the actual getRepoGitModules parameter list.lib/github/resolveSubmodulePath.ts-9-12 (1)
9-12:⚠️ Potential issue | 🟡 MinorRemove invalid nested
@parampaths from JSDoc.The function signature destructures parameters directly (
{ githubRepo, path }), so@param githubRepo.githubRepoand@param githubRepo.pathreference a non-existent parent parameter. These conflict with the correct parameter documentation on lines 10–11 and will confuse JSDoc tools and developers. Remove lines 9 and 12.Proposed fix
- * `@param` githubRepo.githubRepo * `@param` githubRepo - The parent GitHub repository URL * `@param` path - The file path to resolve - * `@param` githubRepo.path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/github/resolveSubmodulePath.ts` around lines 9 - 12, Remove the invalid nested JSDoc param entries that reference githubRepo.githubRepo and githubRepo.path and instead document the actual destructured parameters used by the function (githubRepo and path); update the JSDoc to contain only two `@param` tags (e.g., `@param` githubRepo - The parent GitHub repository URL and `@param` path - The file path to resolve) so the comments match the resolveSubmodulePath function signature.lib/github/expandSubmoduleEntries.ts-14-22 (1)
14-22:⚠️ Potential issue | 🟡 MinorFix incorrect JSDoc parameter nesting.
The destructured parameters
regularEntries,submoduleEntries, andrepoare top-level; they should not be nested underregularEntries.*. Remove the incorrectly scoped@param regularEntries.regularEntries,@param regularEntries.submoduleEntries, and@param regularEntries.repo.*declarations. Add proper@param repo.owner,@param repo.repo, and@param repo.branchdocumentation instead.Proposed fix
- * `@param` regularEntries.regularEntries * `@param` regularEntries - Non-submodule file tree entries * `@param` submoduleEntries - Submodule references (type "commit" from GitHub Trees API) * `@param` repo - Repository context for fetching .gitmodules - * `@param` regularEntries.submoduleEntries - * `@param` regularEntries.repo - * `@param` regularEntries.repo.owner - * `@param` regularEntries.repo.repo - * `@param` regularEntries.repo.branch + * `@param` repo.owner - The GitHub repository owner + * `@param` repo.repo - The GitHub repository name + * `@param` repo.branch - The branch to fetch from🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/github/expandSubmoduleEntries.ts` around lines 14 - 22, The JSDoc incorrectly nests parameters under regularEntries; update the comment for the function (expandSubmoduleEntries) by removing `@param` regularEntries.regularEntries, `@param` regularEntries.submoduleEntries, and `@param` regularEntries.repo (and its nested owner/repo/branch lines), and instead declare top-level params: `@param` regularEntries, `@param` submoduleEntries, and `@param` repo, plus explicit `@param` repo.owner, `@param` repo.repo, and `@param` repo.branch entries so the destructured repo fields are documented correctly.lib/content-agent/getThread.ts-10-11 (1)
10-11:⚠️ Potential issue | 🟡 MinorThread ID parsing may be fragile without validation.
The colon-delimited parsing assumes a specific format (e.g.,
slack:channelId:threadTs). IfthreadIdis malformed or has an unexpected structure,split(":")[1]could returnundefined, resulting inchannelIdof"adapterName:undefined".Consider adding defensive validation:
🛡️ Proposed defensive check
export function getThread(threadId: string) { + const parts = threadId.split(":"); + if (parts.length < 2) { + throw new Error(`[content-agent] Invalid threadId format: ${threadId}`); + } - const adapterName = threadId.split(":")[0]; - const channelId = `${adapterName}:${threadId.split(":")[1]}`; + const adapterName = parts[0]; + const channelId = `${adapterName}:${parts[1]}`; return new ThreadImpl<ContentAgentThreadState>({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/content-agent/getThread.ts` around lines 10 - 11, The threadId parsing in getThread.ts that derives adapterName and channelId from threadId.split(":") is fragile; update the logic that computes adapterName and channelId to first validate threadId has at least two colon-separated parts (e.g., parts = threadId.split(":") and parts.length >= 2), handle cases with extra parts (for formats like "slack:channelId:threadTs") by using parts[0] and parts[1] to build channelId, and if validation fails return a safe failure (throw a descriptive error or return null) and/or log the invalid threadId instead of producing "undefined" in channelId. Ensure references are to the existing adapterName and channelId variables in getThread.ts so the change is localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/content-agent/`[platform]/route.ts:
- Around line 15-59: Add automated tests for the webhook route to cover the GET
404 path, POST Slack URL-verification short-circuit, and normal dispatch via
contentAgentBot.webhooks: write unit tests that invoke the exported GET and POST
handlers (mocking the params promise to return { platform }), stub
contentAgentBot.webhooks to include a fake handler and assert it is called and
its Response returned, stub contentAgentBot.initialize to avoid startup
side-effects, stub handleUrlVerification to return a Response for the Slack
short-circuit case and assert that initialize and the webhook handler are not
called, and add a test where no handler exists to assert a 404 Response; also
verify the waitUntil callback is accepted by your fake handler to exercise the
after(() => p) path.
In `@app/api/launch/route.ts`:
- Around line 45-51: Add unit/integration tests that cover the new API route's
success and failure paths by exercising POST which calls
generateCampaignHandler: create tests for (1) successful stream response (mock
request body and any downstream services, assert streaming chunks and 200), (2)
auth failures (simulate missing/invalid auth and assert the expected 401/403
response), and (3) invalid JSON/body validation errors (send malformed/invalid
payloads and assert 400 and proper error payload). Mock or stub any external
dependencies used by generateCampaignHandler (auth middleware, validators, and
downstream APIs) so tests are deterministic, and include assertions on status
codes and response body/stream behavior for each case.
In `@lib/content-agent/bot.ts`:
- Around line 21-25: The redis.connect() call in createContentAgentBot() is
being fired-and-forgotten; await the connection so callers get a connected
client (or surface connection errors) before the state adapter is created.
Modify createContentAgentBot() (or add an async initialize() method used by
callers like the GET handler) to await redis.connect() when redis.status ===
"wait", propagate any thrown error instead of catching it in a detached promise,
and only construct the state adapter after the successful await; ensure callers
(including code that currently uses the bot directly) await the
factory/initialize promise so the bot is never used with an unconnected Redis
client.
In `@lib/content-agent/handleContentAgentCallback.ts`:
- Around line 44-61: The branch in handleContentAgentCallback that posts
"Content generation finished but no videos were produced." hides per-run
failures and timeouts; instead, when videos.length === 0 use the
already-computed failed array (and any timeout/other non-completed statuses from
validated.results) to build a detailed message: list each failed run (include
status, error/captionText/videoUrl if present) and the count of
failures/timeouts, and then post that constructed message via thread.post rather
than the generic string so callers can see which runs failed and why (refer to
variables validated.results, videos, failed and the thread.post call to locate
where to make the change).
In `@lib/content-agent/handlers/handleContentAgentMention.ts`:
- Around line 111-140: The code uses Promise.allSettled over
triggerCreateContent but keeps the original batch value when calling
thread.setState and triggerPollContentRun, which misreports runs if some
promises failed; change the logic to inspect the allSettled results, compute
successful runIds (as already done), and if runIds.length === 0 throw or return
an error (already handled), but if some succeeded and some failed either (a)
update the saved batch count to runIds.length before calling thread.setState and
then call triggerPollContentRun with those runIds, or (b) treat any failure as
fatal and bail out so no partially-started batch is recorded; implement the
chosen approach by modifying the block around Promise.allSettled / runIds /
thread.setState / triggerPollContentRun to use the adjusted batch/runIds values
and avoid telling users to retry when some runs are already live.
In `@lib/credits/handleChatCredits.ts`:
- Around line 14-20: The JSDoc for the function in
lib/credits/handleChatCredits.ts incorrectly uses nested param names like
`@param usage.usage`, `@param usage.model`, and `@param usage.accountId`; update
the comment to document the actual top-level parameters from the function
signature (e.g., `@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)`), remove the redundant `@param usage` line and any
dot‑notation entries, and ensure the JSDoc parameter names match the
destructured parameters used by the function (look for the function
name/identifier in this file to locate the signature).
In `@lib/launch/buildCampaignPrompt.ts`:
- Around line 8-25: This module currently exports multiple prompt builders
(buildCampaignSystemPrompt and buildCampaignUserPrompt); split them so each file
exports exactly one function: move either buildCampaignSystemPrompt or
buildCampaignUserPrompt into a new file (e.g., create
buildCampaignSystemPrompt.ts or buildCampaignUserPrompt.ts), keep the other in
the original file, update any imports/exports to reference the new filename, and
ensure both functions are exported as default or named per project convention
and all callers (tests/consumers) import the correct symbol name; run
type-checks and tests to confirm no import paths remain broken.
- Around line 26-35: The prompt interpolates raw user fields (artist_name,
song_name, genre, release_date, description) into a marker-sensitive template
and is vulnerable to marker injection; add a small sanitizer function (e.g.
sanitizePromptField) and call it for each symbol before building the string in
buildCampaignPrompt: the sanitizer should strip or normalize newlines to spaces,
remove or escape common section/marker tokens (e.g. lines that begin with
hashes, repeated dashes/equals, or labels like "Artist:", "Song/Release:"), trim
and optionally enforce a max length, then use the sanitized values in the
returned template to prevent downstream parsing breaks.
In `@lib/launch/generateCampaignHandler.ts`:
- Around line 22-35: Wrap the call to request.json() in a try-catch and return a
controlled JSON error response with getCorsHeaders() if parsing fails, and
likewise wrap the streamText(...) invocation (the call that uses DEFAULT_MODEL,
buildCampaignSystemPrompt(), buildCampaignUserPrompt(validated)) in its own
try-catch to catch generation errors and return a consistent NextResponse.json
error with CORS headers; keep the existing validateLaunchBody flow (checking for
NextResponse) and only call streamText after validated succeeds, and ensure any
caught error messages are included in the JSON error body so handlers like
getAccountHandler/createCatalogSongsHandler style are followed.
---
Outside diff comments:
In `@app/api/transcribe/route.ts`:
- Around line 12-33: Stop reading account_id from the request body and derive
the account ID via validateAuthContext(); remove the body.account_id usage and
instead call validateAuthContext() in this route, extract the authenticated
account id and pass that as ownerAccountId to processAudioTranscription (instead
of account_id). Also perform Zod validation on the incoming body fields
(audio_url, artist_account_id, title, include_timestamps) before calling
processAudioTranscription so malformed input is rejected early; ensure
artist_account_id still comes from the validated body and that the validated
include_timestamps is forwarded to includeTimestamps.
---
Minor comments:
In `@app/api/accounts/`[id]/route.ts:
- Around line 26-27: Remove the redundant JSDoc tag "@param params.params" from
the route handler's comment and keep a single clear "@param params" entry
describing the route params (account ID); update the JSDoc above the exported
route handler (the function in app/api/accounts/[id]/route.ts that destructures
params) so it only documents "params" (or "params.id" in the description) and
remove the misleading duplicate entry.
In `@app/api/admins/privy/route.ts`:
- Around line 18-20: Replace the empty JSDoc block above the OPTIONS handler
with a concise summary that documents this endpoint as the CORS preflight
response for the Privy admin route and states it returns HTTP 204 No Content;
update the JSDoc placed immediately above the OPTIONS function/handler (the
block currently empty above the OPTIONS declaration) to include a one-line
description like "CORS preflight response — returns 204 No Content" and mention
it handles preflight checks for allowed methods and headers.
In `@app/api/coding-agent/`[platform]/route.ts:
- Around line 38-40: Remove the incorrect JSDoc param entry "@param
params.params" from the POST route JSDoc and leave only the correct "@param
params" to match the actual handler signature; update the JSDoc block above the
exported POST function (route handler) in
app/api/coding-agent/[platform]/route.ts so it contains a single "@param params"
line describing the route params.
- Around line 13-15: Remove the incorrect JSDoc entry "@param params.params"
from the GET route's documentation; update the JSDoc for the GET handler (the
exported GET function in route.ts) to only document the real parameter (e.g.,
"@param params - Route params containing the platform name") and any other valid
params, ensuring there are no nested or duplicate param lines like
"params.params" that don't exist on the function signature.
In `@lib/coding-agent/handleMergeSuccess.ts`:
- Around line 10-11: The JSDoc for handleMergeSuccess has an incomplete `@param`
entry — update the comment above the handleMergeSuccess function to document the
state parameter by name and a short description of what it represents (e.g.,
"state - current agent state object containing merge context, branch info, and
any relevant metadata"), ensuring the `@param` line follows JSDoc format and
accurately reflects the shape/purpose of the state argument used in
handleMergeSuccess.
In `@lib/content-agent/getThread.ts`:
- Around line 10-11: The threadId parsing in getThread.ts that derives
adapterName and channelId from threadId.split(":") is fragile; update the logic
that computes adapterName and channelId to first validate threadId has at least
two colon-separated parts (e.g., parts = threadId.split(":") and parts.length >=
2), handle cases with extra parts (for formats like "slack:channelId:threadTs")
by using parts[0] and parts[1] to build channelId, and if validation fails
return a safe failure (throw a descriptive error or return null) and/or log the
invalid threadId instead of producing "undefined" in channelId. Ensure
references are to the existing adapterName and channelId variables in
getThread.ts so the change is localized.
In `@lib/content/createContentHandler.ts`:
- Around line 12-13: Update the JSDoc for the createContentHandler function by
adding a brief description to the existing `@param` request tag explaining what
the `request` parameter represents (e.g., the incoming HTTP request or payload
object, its expected shape or purpose) so the comment clearly documents the
input to createContentHandler; keep the description concise and mention any
important fields or types if applicable.
In `@lib/flamingo/getFlamingoPresetsHandler.ts`:
- Line 13: Update the JSDoc for the getFlamingoPresetsHandler function by
replacing the bare "@param request" tag with a concise description of what the
request argument represents (e.g., the incoming HTTP request object containing
query/body headers used to determine which Flamingo presets to return), keeping
the parameter name "request" so it matches the function signature; ensure the
JSDoc clearly states the expected shape or purpose of the request (for example
that it is the incoming API request used to read query params or body payload to
filter presets).
In `@lib/github/expandSubmoduleEntries.ts`:
- Around line 14-22: The JSDoc incorrectly nests parameters under
regularEntries; update the comment for the function (expandSubmoduleEntries) by
removing `@param` regularEntries.regularEntries, `@param`
regularEntries.submoduleEntries, and `@param` regularEntries.repo (and its nested
owner/repo/branch lines), and instead declare top-level params: `@param`
regularEntries, `@param` submoduleEntries, and `@param` repo, plus explicit `@param`
repo.owner, `@param` repo.repo, and `@param` repo.branch entries so the destructured
repo fields are documented correctly.
In `@lib/github/getRepoGitModules.ts`:
- Around line 7-12: The JSDoc for getRepoGitModules contains invalid nested
`@param` entries (e.g., `@param` owner.owner, `@param` owner.repo, `@param`
owner.branch) that contradict the function signature which takes owner, repo,
and branch as direct string parameters; remove those nested tags and ensure only
top-level `@param` tags remain for owner, repo, and branch (and adjust any
descriptions if needed) so the documentation matches the actual
getRepoGitModules parameter list.
In `@lib/github/resolveSubmodulePath.ts`:
- Around line 9-12: Remove the invalid nested JSDoc param entries that reference
githubRepo.githubRepo and githubRepo.path and instead document the actual
destructured parameters used by the function (githubRepo and path); update the
JSDoc to contain only two `@param` tags (e.g., `@param` githubRepo - The parent
GitHub repository URL and `@param` path - The file path to resolve) so the
comments match the resolveSubmodulePath function signature.
In `@lib/prompts/getSystemPrompt.ts`:
- Line 16: The JSDoc for params.orgId is missing a description; update the JSDoc
block in getSystemPrompt.ts (the getSystemPrompt function's comment) to provide
a clear description for params.orgId (e.g., "The ID of the organization" or
similar), matching the style of the other parameter descriptions so the
documentation is consistent and complete.
In `@lib/slack/getSlackUserInfo.ts`:
- Around line 19-21: Add missing JSDoc descriptions for the parameters of
getSlackUserInfo: document what the token parameter is (e.g., Slack OAuth or bot
token used to authenticate API requests) and what the userId parameter
represents (the Slack user ID whose profile/info will be retrieved), and include
expected types and any important format or usage notes so the `@param` tags are
complete and informative.
---
Nitpick comments:
In `@app/api/transcribe/route.ts`:
- Around line 5-8: Replace the placeholder JSDoc above the route handler
(exported POST(req) function) with a concise, descriptive comment that
summarizes the route purpose, expected request body shape, response shape, and
possible errors: e.g., state that POST accepts either multipart/form-data with a
"file" field (audio) or JSON with an "audio" base64 string and optional
"language" string, returns 200 with a JSON body { transcription: string } on
success, and returns 400 for invalid input or 500 for server/transcription
errors; include `@param` req, `@returns`, and `@throws/`@errors tags and a one-line
summary so the JSDoc sits immediately above the POST(req) export.
In `@lib/admins/privy/countNewAccounts.ts`:
- Around line 8-10: Update the JSDoc for the countNewAccounts function by adding
concise descriptions to the `@param` tags: describe what the users parameter
represents (e.g., array of User objects or user count input) and what the period
parameter represents (e.g., time window format like '7d' or start/end Date), so
the comment above countNewAccounts clearly documents the expected types and
semantics of users and period.
In `@lib/admins/privy/fetchPrivyLogins.ts`:
- Around line 23-26: The JSDoc for fetchPrivyLogins is non-informative (only
lists "@param period" with no description); update the JSDoc for the function
fetchPrivyLogins to either remove the block entirely or replace it with a short
summary of what the function does and a clear description of the "period"
parameter (expected type/format/units and effects), and document the return
value if applicable (e.g., what shape or errors) so the comment is meaningful.
In `@lib/admins/privy/getCutoffMs.ts`:
- Around line 8-9: The JSDoc for getCutoffMs is missing a description for the
`@param` period tag; update the comment above the getCutoffMs function to add a
concise description for period (what it represents, allowed values or enum
names, and units if applicable, e.g., "period — time window name
('day'|'week'|'month') used to compute cutoff in milliseconds"), so readers know
how to call getCutoffMs and what each period value means.
In `@lib/admins/privy/getLatestVerifiedAt.ts`:
- Around line 7-8: Add a short descriptive text to the JSDoc `@param` user tag for
the getLatestVerifiedAt function: specify what the `user` parameter represents
(e.g., "the user object or record to inspect" or "User model instance whose
verification timestamps will be checked"), include expected shape/type if known
(e.g., contains verifiedAt or verification history), and any important
preconditions the function requires from `user`.
In `@lib/admins/privy/toMs.ts`:
- Around line 4-5: Update the JSDoc for the toMs function so the `@param`
timestamp tag includes a clear description of expected units and format (e.g.,
"Timestamp in seconds or milliseconds; if in seconds, will be converted to
milliseconds"), referencing the toMs function name so maintainers know which
docblock to edit.
In `@lib/admins/slack/extractGithubPrUrls.ts`:
- Around line 28-30: The JSDoc for extractGithubPrUrls is missing short
descriptions for its parameters; update the comment block above the
extractGithubPrUrls function to add brief `@param` entries for text, attachments,
and blocks (e.g., what type/value each represents and how it’s used) so
IDE/tooltips show actionable info; keep each description one short sentence
mentioning expected type/role (e.g., "text: raw Slack message text to scan for
PR URLs"), and ensure the parameter names exactly match the function signature.
In `@lib/admins/slack/fetchThreadPullRequests.ts`:
- Around line 21-24: Update the JSDoc for fetchThreadPullRequests to include
one-line descriptions for each `@param`: describe token (bot token or OAuth token
used for Slack API calls), channel (Slack channel ID where the thread resides),
and threadTs (parent thread timestamp/ts of the thread); place these short
clarifications on the existing `@param` lines in
lib/admins/slack/fetchThreadPullRequests.ts so tooling and future readers
immediately understand each parameter's purpose.
In `@lib/ai/isEmbedModel.ts`:
- Line 7: Update the JSDoc for the isEmbedModel function by adding a short
description to the existing `@param` m tag (e.g., "Gateway model entry to
evaluate") so the parameter is documented; locate the JSDoc above the
isEmbedModel function and replace the bare "@param m" with "@param m - Gateway
model entry to evaluate" (or equivalent concise wording) to make the annotation
actionable for readers and tooling.
In `@lib/catalog/formatCatalogSongsAsCSV.ts`:
- Around line 5-6: Update the JSDoc for the formatCatalogSongsAsCSV function to
include a descriptive `@param` for songs: explain that songs is the array of song
objects (or the expected shape, e.g., objects containing title, artist, album,
duration, etc.) that will be formatted into CSV; ensure the description clearly
states the parameter type and purpose so IDE tooltips and generated docs display
meaningful information.
In `@lib/catalog/getCatalogDataAsCSV.ts`:
- Around line 6-7: The JSDoc for the getCatalogDataAsCSV function has an
incomplete `@param`; update the comment for the `@param` catalogId in the
getCatalogDataAsCSV JSDoc to describe what the parameter represents (e.g.,
"catalogId - the unique identifier of the catalog to export, typically a string
ID or UUID") so callers understand the expected value and purpose.
In `@lib/catalog/getCatalogSongs.ts`:
- Around line 28-34: The JSDoc for getCatalogSongs is too sparse—update the
comment block above the getCatalogSongs function to document each parameter
(catalogId: string — catalog identifier, pageSize: number — max items per page,
include valid range or default, page: number — 0-based or 1-based index and
default, artistName?: string — optional filter) and add an `@returns` describing
the returned Promise shape (e.g., paginated list of songs and metadata or error
conditions). Keep descriptions short, mention any constraints/defaults, and
ensure the param names exactly match the function signature (catalogId,
pageSize, page, artistName) and include `@returns` with the return type summary.
In `@lib/chats/processCompactChatRequest.ts`:
- Around line 20-22: Update the JSDoc for the processCompactChatRequest function
to add clear descriptions for the nested params keys (params.chatId,
params.prompt, params.accountId): state what each field represents, expected
type/format, whether it is required or optional, and any constraints (e.g.,
chatId is a UUID or string identifier, prompt is the user message text,
accountId is the tenant/user account identifier). Edit the comment block above
the processCompactChatRequest function (the `@param` params entry and its nested
`@param` params.* lines) to include these concise descriptions so callers and
maintainers understand intent and format.
In `@lib/content-agent/getThread.ts`:
- Around line 9-16: Duplicate thread reconstruction logic in getThread is
implemented in ContentAgent's getThread (using ThreadImpl and
ContentAgentThreadState) and in CodingAgent's getThread; extract a single
generic utility function getThread<TState>(threadId: string) that parses
adapterName and channelId and returns new ThreadImpl<TState>({ adapterName, id:
threadId, channelId }), place it in a shared module and update both existing
getThread implementations to import and call this generic getThread with their
specific state types (e.g., getThread<ContentAgentThreadState> and
getThread<CodingAgentThreadState>) so ThreadImpl usage and parsing logic are
centralized.
In `@lib/content-agent/handlers/registerHandlers.ts`:
- Around line 9-10: The module currently calls
registerOnNewMention(contentAgentBot) and
registerOnSubscribedMessage(contentAgentBot) at import time which causes side
effects; change this to export an explicit initialization function (e.g.,
initRegisterHandlers or registerContentAgentHandlers) that performs these calls
instead, and add an internal idempotency guard (a module-level boolean like
"registered") so repeated calls are safe; update callers/tests to invoke the new
init function during startup or test setup rather than relying on module import
side-effects, ensuring registerOnNewMention, registerOnSubscribedMessage, and
contentAgentBot are referenced inside the init function only.
In `@lib/content/contentTemplates.ts`:
- Around line 28-31: Update the JSDoc for the function in contentTemplates.ts to
include a short description of the function and a descriptive `@param` for
template that states the expected format/source (e.g., "template: string |
TemplateObject - the HTML/text template or template identifier used to render
content; may include placeholders like {{name}} and should come from the
templates directory or template service"), so developers know what type/value
and origin are expected; update any additional tags (returns, throws) if
applicable to match the implementation.
In `@lib/content/createContentHandler.ts`:
- Around line 15-87: The createContentHandler function is overlong and mixes
responsibilities; extract the GitHub repo resolution, payload construction, and
batch trigger logic into helpers to reduce size and improve testability:
implement resolveGithubRepo(params: {accountId, artistAccountId, artistSlug})
that uses getArtistContentReadiness and falls back to selectAccountSnapshots
(throwing a clear error if none), implement buildContentPayload(validated,
githubRepo) to create the payload object, and implement
executeBatchTriggers(payload, count) that calls triggerCreateContent via
Promise.allSettled and returns { runIds, failedCount }; then refactor
createContentHandler to call these helpers and keep only orchestration, error
handling, and response formatting (using the same response shape).
In `@lib/content/getArtistContentReadiness.ts`:
- Around line 25-29: The JSDoc uses an auto-generated parameter name `root0`
which is unclear; update the docblock for the function getArtistContentReadiness
to use a meaningful parameter name such as `options` (or `params`) and rename
all related JSDoc tags from `@param root0.accountId`, `@param
root0.artistAccountId`, `@param root0.artistSlug` to `@param options.accountId`,
`@param options.artistAccountId`, `@param options.artistSlug` so the docs
clearly match the function signature and intent.
In `@lib/content/validateCreateContentBody.ts`:
- Around line 43-44: Update the JSDoc for the validateCreateContentBody
function: add a descriptive `@param` for the request argument (include expected
type/shape, e.g., Request or a specific DTO and what fields are required) and
add a `@returns` tag describing the return type and possible values (e.g., boolean
or an object with validation errors). Ensure the JSDoc sits immediately above
the validateCreateContentBody declaration so IDEs pick up the types and
descriptions.
In `@lib/mcp/resolveAccountId.ts`:
- Around line 19-20: Update the JSDoc for resolveAccountId to provide clear
descriptions for the params object properties: document what params.authInfo
contains (e.g., authentication context/token, the fields used such as userId,
tenantId, or permissions) and how it is used by resolveAccountId, and describe
params.accountIdOverride (e.g., an optional account ID that, when present, takes
precedence over values derived from authInfo or defaults). Mention expected
types and whether the fields are optional/required and any validation/format
expectations so future readers understand purpose and constraints.
In `@lib/mcp/tools/transcribe/registerTranscribeAudioTool.ts`:
- Around line 18-21: Add a meaningful JSDoc for the registerTranscribeAudioTool
function: describe what the function does (registers the "transcribe_audio" tool
on the provided server), document the server parameter (type and role, e.g.,
Express or Hapi server instance used to attach routes/handlers), mention side
effects (mutates server by registering routes/tools), and include a `@returns` tag
(void or Promise<void> as appropriate) and any thrown errors; reference
registerTranscribeAudioTool, its server parameter, and the registered
"transcribe_audio" tool in the comment.
In `@lib/slack/getBotChannels.ts`:
- Line 13: Update the JSDoc for the token parameter in getBotChannels to a
concise, descriptive phrase (for example "Slack bot OAuth token") so generated
docs are clear; edit the `@param` token line in the JSDoc block above the
getBotChannels function to replace the vague token comment with the short
description mentioning it's the Slack bot OAuth token used for API calls.
In `@lib/slack/getBotUserId.ts`:
- Around line 11-12: Update the JSDoc for the token parameter in getBotUserId
(in lib/slack/getBotUserId.ts) to clearly state it is the Slack bot OAuth token
used for the auth.test call (e.g., "Slack bot OAuth token used for auth.test to
retrieve the calling bot's user ID"), so future callers know exactly what to
pass; keep the description simple and unambiguous and ensure the `@param` name
matches the function parameter name.
In `@lib/slack/getSlackUserInfo.ts`:
- Around line 23-25: The parameter name userId in getSlackUserInfo conflicts
with our repo-wide "account" terminology but is acceptable at this external-API
boundary; keep the parameter as userId to match Slack's users.info API and add a
short comment above getSlackUserInfo explaining this naming decision (e.g.,
"uses Slack 'user' terminology to match Slack API; internal code should map to
'account' after this boundary") so future readers understand the trade-off and
where mapping to 'account' should occur.
In `@lib/supabase/account_artist_ids/getAccountArtistIds.ts`:
- Around line 11-13: Update the JSDoc for the getAccountArtistIds function: add
clear descriptions for the top-level params object and its nested properties by
documenting params.artistIds and params.accountIds (e.g., "artistIds - optional
array of artist ID strings to filter results" and "accountIds - optional array
of account ID strings to filter results"), indicate whether each is optional,
the expected type, and how they affect function behavior (used as filters), and
keep the param names exactly as in the signature (params, params.artistIds,
params.accountIds) so IDEs and docs pick up the details.
- Around line 44-47: The getAccountArtistIds function currently logs errors and
returns an empty array which hides failures; change its error handling so it
doesn't swallow errors: either throw the original error (preferred for library
functions) from getAccountArtistIds after logging the detailed error, or modify
its return type to a result object (e.g., { data: string[] | null, error: Error
| null }) and return the error instead of an empty array; ensure you update any
callers of getAccountArtistIds to handle the thrown error or the new result
object accordingly and keep the processLogger/console message including the full
error details.
In `@lib/supabase/song_artists/insertSongArtists.ts`:
- Around line 8-10: Add a one-line description to the existing JSDoc `@param` tag
for songArtists in insertSongArtists (the function that upserts song-artist
relationships) explaining the expected shape and intent, e.g., "Array of
song-artist join rows to upsert (each object should contain song_id and
artist_id and optional metadata)"; update the JSDoc above the insertSongArtists
function so it follows the project's documented JSDoc pattern for
lib/supabase/**/*.ts files.
In `@lib/supabase/storage/uploadFileByKey.ts`:
- Around line 7-11: Update the JSDoc for the uploadFileByKey function to add
brief descriptions for each parameter: describe "key" as the storage object
path, "file" as the Blob/File or stream to upload, and "options" as an optional
object; explicitly document options.contentType (default: inferred from the file
or fallback to "application/octet-stream") and options.upsert (default: false,
meaning fail if object exists). Also ensure the module follows the Supabase
pattern by importing the Supabase client from "@/lib/supabase/serverClient" and
that uploadFileByKey's JSDoc includes return type and notes about thrown errors
so callers see expected return and error behavior.
In `@lib/transcribe/processAudioTranscription.ts`:
- Around line 69-72: Update the sparse JSDoc for the function handling audio
transcription (processAudioTranscription) to document expected contentType
values (e.g., "audio/mpeg", "audio/mp3", "audio/wav", "audio/x-m4a" or common
file extension equivalents) and specify fallback behavior: when contentType is
missing or unrecognized the function will treat the input as "mp3". Add a short
summary line describing the function purpose and extend the `@param` contentType
description to list accepted MIME types/variants and explicitly state the
default/fallback of "mp3".
- Around line 10-12: The JSDoc for the params argument in
processAudioTranscription is a placeholder; update the docblock for the
processAudioTranscription function to either remove the empty "@param params"
tag or replace it with a one-line description of the expected keys (e.g.,
audioBuffer/audioPath, language, model, options callback fields) and their
types/optional status so callers know what to pass; reference the params
parameter in the processAudioTranscription function signature and include
concise descriptions for each expected key.
In `@lib/transcribe/saveAudioToFiles.ts`:
- Around line 5-8: The JSDoc above the saveAudioToFiles function is empty;
either flesh it out or remove it. Locate the saveAudioToFiles function in
saveAudioToFiles.ts and either add a proper JSDoc describing the function, each
param (types/purpose) and return value, or delete the empty /** ... */ block so
TypeScript types are relied upon instead.
In `@lib/transcribe/saveTranscriptToFiles.ts`:
- Around line 5-8: The JSDoc above the function is empty and should be completed
or removed; update the JSDoc for the saveTranscriptToFiles function to include a
brief one-line description and a clear `@param` description for the params object
(and document key properties if any), or simply delete the entire /** ... */
block if the TypeScript signature is sufficient, ensuring you reference the same
params identifier used by saveTranscriptToFiles.
In `@lib/transcribe/types.ts`:
- Around line 59-60: Update the JSDoc block in lib/transcribe/types.ts to add a
clear description for the `@param` error tag: state what shape or types of errors
are expected (e.g., Error, string, null/undefined, or a custom
TranscriptionError), whether it can be optional, and how callers should
interpret its fields (message, code, or nested details). Locate the JSDoc that
contains "@param error" and expand the tag to a short sentence describing the
expected error types and any important properties or optionality so the
transcribe-related types and functions can be properly documented.
In `@lib/trigger/triggerPollContentRun.ts`:
- Around line 3-6: The type PollContentRunPayload is declared but not exported,
which prevents reuse across the content agent flow; export PollContentRunPayload
from triggerPollContentRun.ts so other modules (e.g., the poll-content-run task
definition or consumers) can import it for type-safe integration, ensuring the
declaration (type PollContentRunPayload = { runIds: string[]; callbackThreadId:
string; }) is exported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33ced150-3053-495f-b3dd-17ac6da85465
⛔ Files ignored due to path filters (28)
lib/admins/emails/__tests__/validateGetAdminEmailsQuery.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/admins/pr/__tests__/getPrMergedStatusHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/admins/slack/__tests__/getSlackTagsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/admins/slack/__tests__/validateGetSlackTagsQuery.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/artists/__tests__/createArtistPostHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/artists/__tests__/validateCreateArtistBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/auth/__tests__/validateAuthContext.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chat/__tests__/integration/chatEndToEnd.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handleGitHubWebhook.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/onMergeTestToMainAction.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/content/__tests__/validateCreateContentBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/evals/callChatFunctions.tsis excluded by!**/evals/**and included bylib/**lib/evals/callChatFunctionsWithResult.tsis excluded by!**/evals/**and included bylib/**lib/evals/createToolsCalledScorer.tsis excluded by!**/evals/**and included bylib/**lib/evals/extractTextFromResult.tsis excluded by!**/evals/**and included bylib/**lib/evals/extractTextResultFromSteps.tsis excluded by!**/evals/**and included bylib/**lib/evals/getCatalogSongsCountExpected.tsis excluded by!**/evals/**and included bylib/**lib/evals/getSpotifyFollowersExpected.tsis excluded by!**/evals/**and included bylib/**lib/evals/scorers/CatalogAvailability.tsis excluded by!**/evals/**and included bylib/**lib/evals/scorers/QuestionAnswered.tsis excluded by!**/evals/**and included bylib/**lib/evals/scorers/ToolsCalled.tsis excluded by!**/evals/**and included bylib/**lib/flamingo/__tests__/getFlamingoPresetsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/launch/__tests__/validateLaunchBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/notifications/__tests__/createNotificationHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/notifications/__tests__/validateCreateNotificationBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/enrichTaskWithTriggerInfo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/getTaskRunHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/validateGetTaskRunQuery.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (78)
app/api/accounts/[id]/route.tsapp/api/admins/coding/slack/route.tsapp/api/admins/privy/route.tsapp/api/coding-agent/[platform]/route.tsapp/api/content-agent/[platform]/route.tsapp/api/content-agent/callback/route.tsapp/api/launch/route.tsapp/api/songs/analyze/presets/route.tsapp/api/transcribe/route.tslib/admins/pr/getPrStatusHandler.tslib/admins/privy/countNewAccounts.tslib/admins/privy/fetchPrivyLogins.tslib/admins/privy/getCutoffMs.tslib/admins/privy/getLatestVerifiedAt.tslib/admins/privy/toMs.tslib/admins/slack/extractGithubPrUrls.tslib/admins/slack/fetchThreadPullRequests.tslib/ai/getModel.tslib/ai/isEmbedModel.tslib/catalog/formatCatalogSongsAsCSV.tslib/catalog/getCatalogDataAsCSV.tslib/catalog/getCatalogSongs.tslib/catalog/getCatalogs.tslib/chat/toolChains/getPrepareStepResult.tslib/chats/processCompactChatRequest.tslib/coding-agent/encodeGitHubThreadId.tslib/coding-agent/handleMergeSuccess.tslib/coding-agent/parseMergeActionId.tslib/coding-agent/parseMergeTestToMainActionId.tslib/composio/getCallbackUrl.tslib/content-agent/bot.tslib/content-agent/getThread.tslib/content-agent/handleContentAgentCallback.tslib/content-agent/handlers/handleContentAgentCallback.tslib/content-agent/handlers/handleContentAgentMention.tslib/content-agent/handlers/registerHandlers.tslib/content-agent/types.tslib/content-agent/validateContentAgentCallback.tslib/content-agent/validateEnv.tslib/content/contentTemplates.tslib/content/createContentHandler.tslib/content/getArtistContentReadiness.tslib/content/getArtistFileTree.tslib/content/getArtistRootPrefix.tslib/content/getContentValidateHandler.tslib/content/isCompletedRun.tslib/content/persistCreateContentRunVideo.tslib/content/validateCreateContentBody.tslib/content/validateGetContentEstimateQuery.tslib/content/validateGetContentValidateQuery.tslib/credits/getCreditUsage.tslib/credits/handleChatCredits.tslib/emails/processAndSendEmail.tslib/flamingo/getFlamingoPresetsHandler.tslib/github/expandSubmoduleEntries.tslib/github/getRepoGitModules.tslib/github/resolveSubmodulePath.tslib/launch/buildCampaignPrompt.tslib/launch/generateCampaignHandler.tslib/launch/validateLaunchBody.tslib/mcp/resolveAccountId.tslib/mcp/tools/transcribe/registerTranscribeAudioTool.tslib/prompts/getSystemPrompt.tslib/slack/getBotChannels.tslib/slack/getBotUserId.tslib/slack/getSlackUserInfo.tslib/spotify/getSpotifyFollowers.tslib/supabase/account_artist_ids/getAccountArtistIds.tslib/supabase/account_workspace_ids/getAccountWorkspaceIds.tslib/supabase/files/createFileRecord.tslib/supabase/song_artists/insertSongArtists.tslib/supabase/storage/uploadFileByKey.tslib/transcribe/processAudioTranscription.tslib/transcribe/saveAudioToFiles.tslib/transcribe/saveTranscriptToFiles.tslib/transcribe/types.tslib/trigger/triggerCreateContent.tslib/trigger/triggerPollContentRun.ts
Comment on lines
+15
to
+59
| export async function GET( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ platform: string }> }, | ||
| ) { | ||
| const { platform } = await params; | ||
|
|
||
| const handler = contentAgentBot.webhooks[platform as keyof typeof contentAgentBot.webhooks]; | ||
|
|
||
| if (!handler) { | ||
| return new Response("Unknown platform", { status: 404 }); | ||
| } | ||
|
|
||
| return handler(request, { waitUntil: p => after(() => p) }); | ||
| } | ||
|
|
||
| /** | ||
| * POST /api/content-agent/[platform] | ||
| * | ||
| * Webhook endpoint for the content agent bot. | ||
| * Handles Slack webhooks via dynamic [platform] segment. | ||
| * | ||
| * @param request - The incoming webhook request | ||
| * @param params - Route params containing the platform name | ||
| */ | ||
| export async function POST( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ platform: string }> }, | ||
| ) { | ||
| const { platform } = await params; | ||
|
|
||
| if (platform === "slack") { | ||
| const verification = await handleUrlVerification(request); | ||
| if (verification) return verification; | ||
| } | ||
|
|
||
| await contentAgentBot.initialize(); | ||
|
|
||
| const handler = contentAgentBot.webhooks[platform as keyof typeof contentAgentBot.webhooks]; | ||
|
|
||
| if (!handler) { | ||
| return new Response("Unknown platform", { status: 404 }); | ||
| } | ||
|
|
||
| return handler(request, { waitUntil: p => after(() => p) }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Please add automated coverage for these webhook branches.
I don't see tests in this PR for the 404 path, Slack URL-verification short-circuit, or normal dispatch through contentAgentBot.webhooks. This is internet-facing ingress, so relying on the manual Slack/Vercel checklist is pretty fragile.
As per coding guidelines "Write tests for new API endpoints covering all success and error paths".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/content-agent/`[platform]/route.ts around lines 15 - 59, Add
automated tests for the webhook route to cover the GET 404 path, POST Slack
URL-verification short-circuit, and normal dispatch via
contentAgentBot.webhooks: write unit tests that invoke the exported GET and POST
handlers (mocking the params promise to return { platform }), stub
contentAgentBot.webhooks to include a fake handler and assert it is called and
its Response returned, stub contentAgentBot.initialize to avoid startup
side-effects, stub handleUrlVerification to return a Response for the Slack
short-circuit case and assert that initialize and the webhook handler are not
called, and add a test where no handler exists to assert a 404 Response; also
verify the waitUntil callback is accepted by your fake handler to exercise the
after(() => p) path.
Comment on lines
+45
to
+51
| export async function POST(request: NextRequest): Promise<Response> { | ||
| return generateCampaignHandler(request); | ||
| } | ||
|
|
||
| export const dynamic = "force-dynamic"; | ||
| export const fetchCache = "force-no-store"; | ||
| export const revalidate = 0; |
There was a problem hiding this comment.
Add endpoint tests before merge.
This new API route needs coverage for success and failure paths (auth failures, invalid JSON/body validation errors, and successful stream response).
As per coding guidelines: app/api/**/route.ts: Write tests for new API endpoints covering all success and error paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/launch/route.ts` around lines 45 - 51, Add unit/integration tests
that cover the new API route's success and failure paths by exercising POST
which calls generateCampaignHandler: create tests for (1) successful stream
response (mock request body and any downstream services, assert streaming chunks
and 200), (2) auth failures (simulate missing/invalid auth and assert the
expected 401/403 response), and (3) invalid JSON/body validation errors (send
malformed/invalid payloads and assert 400 and proper error payload). Mock or
stub any external dependencies used by generateCampaignHandler (auth middleware,
validators, and downstream APIs) so tests are deterministic, and include
assertions on status codes and response body/stream behavior for each case.
Comment on lines
+21
to
+25
| if (redis.status === "wait") { | ||
| redis.connect().catch(() => { | ||
| throw new Error("[content-agent] Redis failed to connect"); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the file
find . -type f -name "bot.ts" -path "*/content-agent/*" | head -5Repository: recoupable/api
Length of output: 84
🏁 Script executed:
# Read the file to examine the redis connection handling
cat -n lib/content-agent/bot.tsRepository: recoupable/api
Length of output: 1870
🏁 Script executed:
# Check how redis is imported and what its type is to understand connection behavior
cat -n lib/redis/connection.ts | head -30Repository: recoupable/api
Length of output: 871
🏁 Script executed:
# Check if there are any await calls or error handling for contentAgentBot downstream
rg -n "contentAgentBot" --type ts --type tsx | head -20Repository: recoupable/api
Length of output: 85
🏁 Script executed:
# Search for contentAgentBot usage (without tsx type which causes error)
rg -n "contentAgentBot" --type tsRepository: recoupable/api
Length of output: 1084
🏁 Script executed:
# Check if createContentAgentBot is called anywhere else or if singleton is the only usage
rg -n "createContentAgentBot" --type tsRepository: recoupable/api
Length of output: 331
🏁 Script executed:
# Let's check what Chat.initialize() does and when redis connection is expected
rg -n "initialize" lib/content-agent/bot.ts -A 5Repository: recoupable/api
Length of output: 40
🏁 Script executed:
# Check if there are any other redis connection calls or error handling patterns in the codebase
rg -n "redis\.connect\(\)" --type tsRepository: recoupable/api
Length of output: 239
🏁 Script executed:
# Check the other bot files to see if they have the same pattern and how they're used
cat -n lib/slack/chat/bot.ts | sed -n '20,40p'Repository: recoupable/api
Length of output: 798
🏁 Script executed:
cat -n lib/coding-agent/bot.ts | sed -n '20,40p'Repository: recoupable/api
Length of output: 893
🏁 Script executed:
# Check the Chat SDK type to understand if initialize() handles redis readiness
rg -n "initialize" lib/slack/chat/bot.ts -B 2 -A 10 | head -30Repository: recoupable/api
Length of output: 40
🏁 Script executed:
# Look at how the state adapter interacts with redis readiness
rg -n "createIoRedisState" --type ts -B 2 -A 8 | head -40Repository: recoupable/api
Length of output: 2418
🏁 Script executed:
# Check if there's any global error handler for unhandled promise rejections
rg -n "unhandledRejection" --type tsRepository: recoupable/api
Length of output: 40
🏁 Script executed:
# Look at how initialize() is actually implemented to see if it waits for redis
rg -n "\.initialize\(\)" lib/content-agent/ -B 2 -A 5Repository: recoupable/api
Length of output: 40
🏁 Script executed:
# Check the app routes that use contentAgentBot to see the full flow
cat -n app/api/content-agent/callback/route.ts | head -30Repository: recoupable/api
Length of output: 737
🏁 Script executed:
cat -n app/api/content-agent/\[platform\]/route.ts | head -30Repository: recoupable/api
Length of output: 1180
🏁 Script executed:
# Let's check the Chat.initialize() method from the test mock to understand what it does
cat -n lib/coding-agent/__tests__/bot.test.ts | sed -n '14,25p'Repository: recoupable/api
Length of output: 361
🏁 Script executed:
# Search for any documentation about how the state adapter expects redis to be ready
rg -n "ready" lib/slack/chat/bot.ts -B 2 -A 2Repository: recoupable/api
Length of output: 272
Await Redis connection before creating the state adapter instead of using fire-and-forget error handling.
The redis.connect() call at lines 21–25 is not awaited, which means createContentAgentBot() returns before the Redis client is guaranteed to be connected. If redis.connect() rejects, the error is trapped in a detached promise and never propagates to the caller. The state adapter is then created with a potentially unconnected Redis client. Additionally, not all callers invoke initialize() before using the bot (e.g., the GET handler at app/api/content-agent/[platform]/route.ts uses it directly).
Move the connection check into an async initialization step that is awaited before the bot is used, or ensure redis.connect() is awaited directly in the bot factory function and all callers handle the promise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/content-agent/bot.ts` around lines 21 - 25, The redis.connect() call in
createContentAgentBot() is being fired-and-forgotten; await the connection so
callers get a connected client (or surface connection errors) before the state
adapter is created. Modify createContentAgentBot() (or add an async initialize()
method used by callers like the GET handler) to await redis.connect() when
redis.status === "wait", propagate any thrown error instead of catching it in a
detached promise, and only construct the state adapter after the successful
await; ensure callers (including code that currently uses the bot directly)
await the factory/initialize promise so the bot is never used with an
unconnected Redis client.
Comment on lines
+44
to
+61
| const results = validated.results ?? []; | ||
| const videos = results.filter(r => r.status === "completed" && r.videoUrl); | ||
| const failed = results.filter(r => r.status === "failed"); | ||
|
|
||
| if (videos.length > 0) { | ||
| const lines = videos.map((v, i) => { | ||
| const label = videos.length > 1 ? `**Video ${i + 1}:** ` : ""; | ||
| const caption = v.captionText ? `\n> ${v.captionText}` : ""; | ||
| return `${label}${v.videoUrl}${caption}`; | ||
| }); | ||
|
|
||
| if (failed.length > 0) { | ||
| lines.push(`\n_${failed.length} run(s) failed._`); | ||
| } | ||
|
|
||
| await thread.post(lines.join("\n\n")); | ||
| } else { | ||
| await thread.post("Content generation finished but no videos were produced."); |
There was a problem hiding this comment.
Don't collapse failed batches into a generic "no videos" message.
When videos.length === 0, this branch drops the per-run failure context you've already computed, and timeout results are never surfaced at all. A batch that fully failed can end up looking like a benign no-output case.
💡 Suggested fix
const results = validated.results ?? [];
const videos = results.filter(r => r.status === "completed" && r.videoUrl);
const failed = results.filter(r => r.status === "failed");
+ const timedOut = results.filter(r => r.status === "timeout");
if (videos.length > 0) {
const lines = videos.map((v, i) => {
const label = videos.length > 1 ? `**Video ${i + 1}:** ` : "";
const caption = v.captionText ? `\n> ${v.captionText}` : "";
return `${label}${v.videoUrl}${caption}`;
});
- if (failed.length > 0) {
- lines.push(`\n_${failed.length} run(s) failed._`);
+ if (failed.length > 0 || timedOut.length > 0) {
+ const summary = [
+ failed.length > 0 ? `${failed.length} failed` : null,
+ timedOut.length > 0 ? `${timedOut.length} timed out` : null,
+ ]
+ .filter(Boolean)
+ .join(", ");
+ lines.push(`\n_${summary}._`);
}
await thread.post(lines.join("\n\n"));
} else {
- await thread.post("Content generation finished but no videos were produced.");
+ const summary = [
+ failed.length > 0 ? `${failed.length} failed` : null,
+ timedOut.length > 0 ? `${timedOut.length} timed out` : null,
+ ]
+ .filter(Boolean)
+ .join(", ");
+ await thread.post(
+ `Content generation finished but no videos were produced${summary ? ` (${summary})` : ""}.`,
+ );
}📝 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
| const results = validated.results ?? []; | |
| const videos = results.filter(r => r.status === "completed" && r.videoUrl); | |
| const failed = results.filter(r => r.status === "failed"); | |
| if (videos.length > 0) { | |
| const lines = videos.map((v, i) => { | |
| const label = videos.length > 1 ? `**Video ${i + 1}:** ` : ""; | |
| const caption = v.captionText ? `\n> ${v.captionText}` : ""; | |
| return `${label}${v.videoUrl}${caption}`; | |
| }); | |
| if (failed.length > 0) { | |
| lines.push(`\n_${failed.length} run(s) failed._`); | |
| } | |
| await thread.post(lines.join("\n\n")); | |
| } else { | |
| await thread.post("Content generation finished but no videos were produced."); | |
| const results = validated.results ?? []; | |
| const videos = results.filter(r => r.status === "completed" && r.videoUrl); | |
| const failed = results.filter(r => r.status === "failed"); | |
| const timedOut = results.filter(r => r.status === "timeout"); | |
| if (videos.length > 0) { | |
| const lines = videos.map((v, i) => { | |
| const label = videos.length > 1 ? `**Video ${i + 1}:** ` : ""; | |
| const caption = v.captionText ? `\n> ${v.captionText}` : ""; | |
| return `${label}${v.videoUrl}${caption}`; | |
| }); | |
| if (failed.length > 0 || timedOut.length > 0) { | |
| const summary = [ | |
| failed.length > 0 ? `${failed.length} failed` : null, | |
| timedOut.length > 0 ? `${timedOut.length} timed out` : null, | |
| ] | |
| .filter(Boolean) | |
| .join(", "); | |
| lines.push(`\n_${summary}._`); | |
| } | |
| await thread.post(lines.join("\n\n")); | |
| } else { | |
| const summary = [ | |
| failed.length > 0 ? `${failed.length} failed` : null, | |
| timedOut.length > 0 ? `${timedOut.length} timed out` : null, | |
| ] | |
| .filter(Boolean) | |
| .join(", "); | |
| await thread.post( | |
| `Content generation finished but no videos were produced${summary ? ` (${summary})` : ""}.`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/content-agent/handleContentAgentCallback.ts` around lines 44 - 61, The
branch in handleContentAgentCallback that posts "Content generation finished but
no videos were produced." hides per-run failures and timeouts; instead, when
videos.length === 0 use the already-computed failed array (and any timeout/other
non-completed statuses from validated.results) to build a detailed message: list
each failed run (include status, error/captionText/videoUrl if present) and the
count of failures/timeouts, and then post that constructed message via
thread.post rather than the generic string so callers can see which runs failed
and why (refer to variables validated.results, videos, failed and the
thread.post call to locate where to make the change).
Comment on lines
+111
to
+140
| const results = await Promise.allSettled( | ||
| Array.from({ length: batch }, () => triggerCreateContent(payload)), | ||
| ); | ||
| const runIds = results | ||
| .filter(r => r.status === "fulfilled") | ||
| .map(r => (r as PromiseFulfilledResult<{ id: string }>).value.id); | ||
|
|
||
| if (runIds.length === 0) { | ||
| await thread.post("Failed to trigger content creation. Please try again."); | ||
| return; | ||
| } | ||
|
|
||
| // Set thread state | ||
| await thread.setState({ | ||
| status: "running", | ||
| artistAccountId, | ||
| template, | ||
| lipsync, | ||
| batch, | ||
| runIds, | ||
| }); | ||
|
|
||
| // Trigger polling task | ||
| await triggerPollContentRun({ | ||
| runIds, | ||
| callbackThreadId: thread.id, | ||
| }); | ||
| } catch (error) { | ||
| console.error("[content-agent] onNewMention error:", error); | ||
| await thread.post("Something went wrong starting content generation. Please try again."); |
There was a problem hiding this comment.
Don't report a partially-started batch as if all requested runs are active.
Promise.allSettled() lets this continue with fewer runIds than the requested batch, but the stored state still keeps the original batch size. If triggerPollContentRun() then throws, the thread is told to retry even though some runs may already be live, which can create duplicate content.
💡 Suggested direction
const runIds = results
.filter(r => r.status === "fulfilled")
.map(r => (r as PromiseFulfilledResult<{ id: string }>).value.id);
+ const startedBatch = runIds.length;
+ const failedToStart = batch - startedBatch;
if (runIds.length === 0) {
await thread.post("Failed to trigger content creation. Please try again.");
return;
}
+
+ if (failedToStart > 0) {
+ await thread.post(
+ `${failedToStart} of ${batch} content run(s) failed to start. I'll keep tracking the ${startedBatch} that did.`,
+ );
+ }
// Set thread state
await thread.setState({
status: "running",
artistAccountId,
template,
lipsync,
- batch,
+ batch: startedBatch,
runIds,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/content-agent/handlers/handleContentAgentMention.ts` around lines 111 -
140, The code uses Promise.allSettled over triggerCreateContent but keeps the
original batch value when calling thread.setState and triggerPollContentRun,
which misreports runs if some promises failed; change the logic to inspect the
allSettled results, compute successful runIds (as already done), and if
runIds.length === 0 throw or return an error (already handled), but if some
succeeded and some failed either (a) update the saved batch count to
runIds.length before calling thread.setState and then call triggerPollContentRun
with those runIds, or (b) treat any failure as fatal and bail out so no
partially-started batch is recorded; implement the chosen approach by modifying
the block around Promise.allSettled / runIds / thread.setState /
triggerPollContentRun to use the adjusted batch/runIds values and avoid telling
users to retry when some runs are already live.
Comment on lines
+14
to
+20
| * | ||
| * @param usage.usage | ||
| * @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) | ||
| * @param usage.model | ||
| * @param usage.accountId |
There was a problem hiding this comment.
Fix incorrect JSDoc parameter documentation.
The JSDoc uses dot notation (@param usage.usage, @param usage.model, @param usage.accountId) which suggests nested properties, but the function signature shows these are three separate top-level parameters in the destructured object, not nested within usage. Line 16's @param usage is also now redundant given line 15's @param usage.usage.
This will cause documentation generators and IDEs to display incorrect parameter information.
📝 Proposed fix for the JSDoc
*
- * `@param` usage.usage
- * `@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)
- * `@param` usage.model
- * `@param` usage.accountId
+ * `@param` params - The parameters object
+ * `@param` params.usage - The language model usage data
+ * `@param` params.model - The model ID used for the chat
+ * `@param` params.accountId - The account ID to deduct credits from (optional)
*/📝 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
| * | |
| * @param usage.usage | |
| * @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) | |
| * @param usage.model | |
| * @param usage.accountId | |
| * | |
| * `@param` params - The parameters object | |
| * `@param` params.usage - The language model usage data | |
| * `@param` params.model - The model ID used for the chat | |
| * `@param` params.accountId - The account ID to deduct credits from (optional) | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/credits/handleChatCredits.ts` around lines 14 - 20, The JSDoc for the
function in lib/credits/handleChatCredits.ts incorrectly uses nested param names
like `@param usage.usage`, `@param usage.model`, and `@param usage.accountId`;
update the comment to document the actual top-level parameters from the function
signature (e.g., `@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)`), remove the redundant `@param usage` line and any
dot‑notation entries, and ensure the JSDoc parameter names match the
destructured parameters used by the function (look for the function
name/identifier in this file to locate the signature).
Comment on lines
+8
to
+25
| export function buildCampaignSystemPrompt(): string { | ||
| return `You are an expert music industry publicist and marketing strategist with 15+ years of experience | ||
| launching indie and major label artists. You write compelling, professional, and authentic music PR content | ||
| that sounds human — never generic. Your press releases get picked up by music blogs. Your Spotify pitches | ||
| get playlisted. Your emails get replies. | ||
|
|
||
| Always write as if this artist and song are genuinely exciting. Use vivid, specific language. | ||
| Avoid clichés like "sonic journey" or "genre-defying." | ||
| Output each section EXACTLY as instructed with the section markers provided — no extra text outside the markers.`; | ||
| } | ||
|
|
||
| /** | ||
| * Builds the user prompt for the release campaign generator. | ||
| * | ||
| * @param body - Validated launch request body | ||
| * @returns The formatted user prompt | ||
| */ | ||
| export function buildCampaignUserPrompt(body: LaunchBody): string { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split this module to satisfy SRP export rule.
This file exports multiple functions; move one prompt builder to its own file so each file exposes one exported function.
As per coding guidelines: lib/**/*.ts: Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/launch/buildCampaignPrompt.ts` around lines 8 - 25, This module currently
exports multiple prompt builders (buildCampaignSystemPrompt and
buildCampaignUserPrompt); split them so each file exports exactly one function:
move either buildCampaignSystemPrompt or buildCampaignUserPrompt into a new file
(e.g., create buildCampaignSystemPrompt.ts or buildCampaignUserPrompt.ts), keep
the other in the original file, update any imports/exports to reference the new
filename, and ensure both functions are exported as default or named per project
convention and all callers (tests/consumers) import the correct symbol name; run
type-checks and tests to confirm no import paths remain broken.
Comment on lines
+26
to
+35
| const { artist_name, song_name, genre, release_date, description } = body; | ||
|
|
||
| const context = description ? `\nAdditional context: ${description}` : ""; | ||
|
|
||
| return `Generate a complete music release campaign for: | ||
| Artist: ${artist_name} | ||
| Song/Release: ${song_name} | ||
| Genre: ${genre} | ||
| Release Date: ${release_date}${context} | ||
|
|
There was a problem hiding this comment.
Guard prompt structure from marker injection in user fields.
artist_name, song_name, genre, release_date, and description are inserted raw into a marker-sensitive prompt format. Inputs containing section markers can break downstream parsing of streamed sections.
💡 Suggested hardening
import type { LaunchBody } from "./validateLaunchBody";
+function sanitizePromptField(value: string): string {
+ return value
+ .replaceAll("[SECTION:", "")
+ .replaceAll("[/SECTION:", "")
+ .trim();
+}
+
/**
* Builds the system prompt for the release campaign generator.
*
* `@returns` The system prompt string
*/
@@
export function buildCampaignUserPrompt(body: LaunchBody): string {
const { artist_name, song_name, genre, release_date, description } = body;
- const context = description ? `\nAdditional context: ${description}` : "";
+ const safeArtist = sanitizePromptField(artist_name);
+ const safeSong = sanitizePromptField(song_name);
+ const safeGenre = sanitizePromptField(genre);
+ const safeReleaseDate = sanitizePromptField(release_date);
+ const safeDescription = description ? sanitizePromptField(description) : "";
+ const context = safeDescription ? `\nAdditional context: ${safeDescription}` : "";
return `Generate a complete music release campaign for:
-Artist: ${artist_name}
-Song/Release: ${song_name}
-Genre: ${genre}
-Release Date: ${release_date}${context}
+Artist: ${safeArtist}
+Song/Release: ${safeSong}
+Genre: ${safeGenre}
+Release Date: ${safeReleaseDate}${context}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/launch/buildCampaignPrompt.ts` around lines 26 - 35, The prompt
interpolates raw user fields (artist_name, song_name, genre, release_date,
description) into a marker-sensitive template and is vulnerable to marker
injection; add a small sanitizer function (e.g. sanitizePromptField) and call it
for each symbol before building the string in buildCampaignPrompt: the sanitizer
should strip or normalize newlines to spaces, remove or escape common
section/marker tokens (e.g. lines that begin with hashes, repeated
dashes/equals, or labels like "Artist:", "Song/Release:"), trim and optionally
enforce a max length, then use the sanitized values in the returned template to
prevent downstream parsing breaks.
Comment on lines
+22
to
+35
| const json = await request.json(); | ||
| const validated = validateLaunchBody(json); | ||
| if (validated instanceof NextResponse) { | ||
| return validated; | ||
| } | ||
|
|
||
| const result = streamText({ | ||
| model: DEFAULT_MODEL, | ||
| system: buildCampaignSystemPrompt(), | ||
| prompt: buildCampaignUserPrompt(validated), | ||
| }); | ||
|
|
||
| return result.toTextStreamResponse({ headers: getCorsHeaders() }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "generateCampaignHandler.ts" -type fRepository: recoupable/api
Length of output: 97
🏁 Script executed:
wc -l lib/launch/generateCampaignHandler.tsRepository: recoupable/api
Length of output: 98
🏁 Script executed:
cat -n lib/launch/generateCampaignHandler.tsRepository: recoupable/api
Length of output: 1549
🏁 Script executed:
rg -A 5 -B 5 "getCorsHeaders" --type tsRepository: recoupable/api
Length of output: 50371
🏁 Script executed:
rg -B 3 "request\.json\(\)" --type tsRepository: recoupable/api
Length of output: 6044
🏁 Script executed:
ast-grep --pattern 'export async function generateCampaignHandler($_) {
$$$
}'Repository: recoupable/api
Length of output: 40
Add error handling for JSON parsing and stream generation failures.
request.json() and streamText() can throw unhandled exceptions. Wrap both in try-catch blocks and return controlled JSON errors with CORS headers, consistent with other handlers in the codebase (e.g., getAccountHandler, createCatalogSongsHandler).
✅ Suggested error-handling patch
export async function generateCampaignHandler(request: NextRequest): Promise<Response> {
const authResult = await validateAuthContext(request);
if (authResult instanceof NextResponse) {
return authResult;
}
- const json = await request.json();
+ let json: unknown;
+ try {
+ json = await request.json();
+ } catch {
+ return NextResponse.json(
+ { status: "error", error: "Invalid JSON body" },
+ { status: 400, headers: getCorsHeaders() },
+ );
+ }
+
const validated = validateLaunchBody(json);
if (validated instanceof NextResponse) {
return validated;
}
- const result = streamText({
- model: DEFAULT_MODEL,
- system: buildCampaignSystemPrompt(),
- prompt: buildCampaignUserPrompt(validated),
- });
-
- return result.toTextStreamResponse({ headers: getCorsHeaders() });
+ try {
+ const result = streamText({
+ model: DEFAULT_MODEL,
+ system: buildCampaignSystemPrompt(),
+ prompt: buildCampaignUserPrompt(validated),
+ });
+ return result.toTextStreamResponse({ headers: getCorsHeaders() });
+ } catch {
+ return NextResponse.json(
+ { status: "error", error: "Failed to generate campaign" },
+ { status: 500, headers: getCorsHeaders() },
+ );
+ }
}Per coding guidelines, lib/**/*.ts domain functions require proper error handling.
📝 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
| const json = await request.json(); | |
| const validated = validateLaunchBody(json); | |
| if (validated instanceof NextResponse) { | |
| return validated; | |
| } | |
| const result = streamText({ | |
| model: DEFAULT_MODEL, | |
| system: buildCampaignSystemPrompt(), | |
| prompt: buildCampaignUserPrompt(validated), | |
| }); | |
| return result.toTextStreamResponse({ headers: getCorsHeaders() }); | |
| } | |
| let json: unknown; | |
| try { | |
| json = await request.json(); | |
| } catch { | |
| return NextResponse.json( | |
| { status: "error", error: "Invalid JSON body" }, | |
| { status: 400, headers: getCorsHeaders() }, | |
| ); | |
| } | |
| const validated = validateLaunchBody(json); | |
| if (validated instanceof NextResponse) { | |
| return validated; | |
| } | |
| try { | |
| const result = streamText({ | |
| model: DEFAULT_MODEL, | |
| system: buildCampaignSystemPrompt(), | |
| prompt: buildCampaignUserPrompt(validated), | |
| }); | |
| return result.toTextStreamResponse({ headers: getCorsHeaders() }); | |
| } catch { | |
| return NextResponse.json( | |
| { status: "error", error: "Failed to generate campaign" }, | |
| { status: 500, headers: getCorsHeaders() }, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/launch/generateCampaignHandler.ts` around lines 22 - 35, Wrap the call to
request.json() in a try-catch and return a controlled JSON error response with
getCorsHeaders() if parsing fails, and likewise wrap the streamText(...)
invocation (the call that uses DEFAULT_MODEL, buildCampaignSystemPrompt(),
buildCampaignUserPrompt(validated)) in its own try-catch to catch generation
errors and return a consistent NextResponse.json error with CORS headers; keep
the existing validateLaunchBody flow (checking for NextResponse) and only call
streamText after validated succeeds, and ensure any caught error messages are
included in the JSON error body so handlers like
getAccountHandler/createCatalogSongsHandler style are followed.
Collaborator
Author
recoup-coding-agent
left a comment
recoup-coding-agent
left a comment
There was a problem hiding this comment.
Code Review: Recoup Content Agent Slack Bot
Summary
Adds a new Recoup Content Agent Slack bot that lets users trigger content generation via @RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync]. Also includes a new /api/launch streaming campaign endpoint that appears unrelated to the Slack bot feature.
Follows the existing coding-agent pattern: Chat SDK with Slack adapter, Redis-backed thread state, Trigger.dev polling, and a callback endpoint for async results.
CLEAN Code Assessment
SRP ✅
Core handlers are nicely separated: handleContentAgentMention.ts handles incoming mentions, handleContentAgentCallback.ts handles HTTP callbacks, validateContentAgentEnv.ts validates config. Each file has a clear single responsibility.
OCP ✅
Registration via registerHandlers.ts follows the open-for-extension pattern from the coding-agent. New handlers can be added without modifying existing routes.
DRY ⚠️
See naming confusion issue below.
YAGNI ⚠️
- The
/api/launchcampaign endpoint has no connection to the content-agent Slack bot. It should be a separate PR. - Many existing files received boilerplate JSDoc additions with no content (e.g.,
@param bodywith no description). These add noise without value.
Issues Found
🔴 Blocking
1. Module-level bot singleton crashes on import when env vars are absent
lib/content-agent/bot.ts exports:
export const contentAgentBot = createContentAgentBot().registerSingleton();createContentAgentBot() calls validateContentAgentEnv() which throws if SLACK_CONTENT_BOT_TOKEN, SLACK_CONTENT_SIGNING_SECRET, CONTENT_AGENT_CALLBACK_SECRET, or REDIS_URL are missing. Since this runs at import time, any file that imports from bot.ts will crash the entire Next.js build/start in environments where these vars aren't configured.
Recommend wrapping in a lazy getter:
let _contentAgentBot: ContentAgentBot | null = null;
export function getContentAgentBot() {
if (!_contentAgentBot) _contentAgentBot = createContentAgentBot().registerSingleton();
return _contentAgentBot;
}🟡 Suggestions
2. Naming collision between two handleContentAgentCallback.ts files
lib/content-agent/handleContentAgentCallback.ts— HTTP callback handler (84 lines)lib/content-agent/handlers/handleContentAgentCallback.ts— bot subscribed-message registration (17 lines)
Two files with identical names doing completely different things in sibling directories. The handler in handlers/ should be renamed to registerOnSubscribedMessage.ts (consistent with registerOnNewMention.ts next to it).
3. parseMentionArgs — use batch= prefix check consistently
} else if (!token.startsWith("batch") && token !== "lipsync") {This guard uses !token.startsWith("batch") but the intent is to skip batch=N tokens. Use !token.startsWith("batch=") to be explicit and avoid accidentally filtering tokens like batchSize if future options are added.
4. /api/launch endpoint belongs in a separate PR
The streaming campaign generator has no runtime coupling to the content-agent Slack bot. Bundling it here makes the PR harder to review and harder to revert independently.
🔵 Nits
5. Boilerplate JSDoc on existing simple functions adds noise
Many existing files received @param JSDoc tags with no actual description body. Empty JSDoc is worse than no JSDoc.
Security
✅ Callback authentication uses a shared secret (x-callback-secret) — validated before processing.
✅ Zod validation on callback and launch bodies.
✅ No hardcoded secrets.
secret !== expectedSecret comparison is not timing-safe. Consider crypto.timingSafeEqual to prevent timing attacks (low-risk for internal callback, but worth noting).
Verdict: request-changes
Core feature is well-structured. Two changes needed before merge:
- Fix module-level env validation crash (blocking)
- Rename
handlers/handleContentAgentCallback.ts→handlers/registerOnSubscribedMessage.ts(maintainability)
Collaborator
Author
recoup-coding-agent
left a comment
recoup-coding-agent
left a comment
There was a problem hiding this comment.
Summary
Scaffolds a new Recoup Content Agent Slack bot following the existing coding-agent pattern. Handles @RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync] Slack mentions, triggers the create-content pipeline, polls via Trigger.dev (companion: recoupable/tasks#109), and posts video URLs back to the thread via Redis-backed Chat SDK state.
Also bundles in a new /api/launch campaign-generation endpoint (unrelated — see below).
CLEAN Code Assessment
- SRP: Generally good. The handler chain (mention → trigger → poll → callback) is clean. One concern below.
- OCP: Well-structured — new bot follows existing adapter pattern without modifying coding-agent code.
- DRY: Shares
resolveArtistSlug,getArtistContentReadiness,contentTemplates, Supabase helpers — good reuse. - YAGNI: A few instances of empty JSDoc noise (see nits).
Issues Found
blocking
Confusing duplicate filename (lib/content-agent/handlers/handleContentAgentCallback.ts):
This file exports registerOnSubscribedMessage, but its filename is handleContentAgentCallback.ts — identical to the sibling file at lib/content-agent/handleContentAgentCallback.ts that handles the actual Trigger.dev callback. This is a real confusion risk when navigating the codebase. Rename lib/content-agent/handlers/handleContentAgentCallback.ts to registerSubscribedMessage.ts (or handleSubscribedMessage.ts) to match its export.
suggestion
/api/launch bundled in this PR (app/api/launch/route.ts + lib/launch/):
This is an unrelated campaign-generation feature mixed into the content-agent PR. Unrelated features in the same PR make review harder and rollback riskier. Please split it into its own PR.
Dual-path githubRepo resolution (lib/content-agent/handlers/handleContentAgentMention.ts lines 70–87):
The try/catch fallback from getArtistContentReadiness to selectAccountSnapshots is implicit control flow via exception. If getArtistContentReadiness fails for infrastructure reasons (network, DB), it silently falls back to snapshots. Consider checking only one authoritative source, or separating the two paths with an explicit if/else rather than exception-driven branching.
ContentAgentThreadState has required fields (lib/content-agent/types.ts):
artistAccountId, template, lipsync, batch, runIds are all required in the interface, but setState is only called with a full object after triggerPollContentRun. The status: "running" is set atomically with all other fields — this is fine as written, but the type could use Partial<> or a discriminated union to represent intermediate states more precisely.
nit
batchmax is hardcoded as30inparseMentionArgs— use a named constantMAX_CONTENT_BATCH_SIZE = 30.- Many empty
@paramJSDoc blocks were added to existing functions throughout the diff (e.g.@param url,@param body,@param requestwith no description). These add noise with no value — either add meaningful descriptions or omit them. getThreadinlib/content-agent/getThread.tsmanually parses the thread ID format (adapterName:channelId). This is a tight coupling to Chat SDK internals. A comment explaining the expected format would help future maintainers.
Security
- Callback protected by
x-callback-secretshared secret — correct pattern. - Env vars validated at startup via
validateContentAgentEnv()— good. - No hardcoded secrets.
- No SQL injection risk (Supabase client used consistently).
Verdict
request-changes — two issues need resolution before merge:
- Rename
lib/content-agent/handlers/handleContentAgentCallback.tsto avoid collision withlib/content-agent/handleContentAgentCallback.ts. - Split the
/api/launchroute into a separate PR.
Collaborator
Author
Code Review — Recoup Content Agent Slack BotSummaryAdds a Slack bot ( The blocking issue is not the feature code — it's the 90+ unrelated JSDoc-only changes bundled into this PR. CLEAN Code Assessment
Issues Found🔴 Blocking1. ~90 unrelated JSDoc-only changes must be removed Out of 106 changed files, ~90 are existing files that received only trivial JSDoc additions like: /**
* @param request
*/
export async function GET(request: NextRequest) {or even empty stubs: /**
*
*/
export async function OPTIONS() {These touch completely unrelated modules:
Action required: Remove all JSDoc-only modifications to existing files. Keep only the new 🟡 Suggestion2. Naming collision between two The PR creates two files with identical names in adjacent directories:
This is confusing and makes IDE navigation unreliable. Rename one — e.g. the HTTP handler → 3. Use timing-safe secret comparison in callback handler In // Current — vulnerable to timing attacks
if (!secret || secret !== expectedSecret) {
// Better
import { timingSafeEqual } from "crypto";
const secretBuf = Buffer.from(secret);
const expectedBuf = Buffer.from(expectedSecret);
if (secretBuf.length !== expectedBuf.length || !timingSafeEqual(secretBuf, expectedBuf)) {⚪ Nit4.
Security
Verdict: 🔴 Request ChangesThe feature code itself is clean and follows project conventions. Strip the ~90 unrelated JSDoc-only file modifications, address the naming collision, and this is ready to merge. |
Collaborator
Author
CI Status Update — PR #341Checking CI status as part of this review:
Format Check Failure (Blocking)The The author must run Vercel Deployment Failure (Blocking)The preview deployment also failed. This may be related to the format check or missing env vars. Check the Vercel logs: New required env vars to add in Vercel:
Updated verdict: 🔴 Request Changes — in addition to stripping the JSDoc noise (previous comment), the format check failure and Vercel deployment failure must also be resolved before merge. |
5 tasks
Collaborator
Author
|
Superseded by #342 — clean PR with only 17 new feature files (no JSDoc noise). All review feedback addressed:
|
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 free
to 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.
Summary
coding-agentpattern@RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync]create-contentpipeline, polls for results via Trigger.dev, and posts video URLs back to the Slack threadNew Env Vars Required
SLACK_CONTENT_BOT_TOKENSLACK_CONTENT_SIGNING_SECRETCONTENT_AGENT_CALLBACK_SECRETTest plan
app_mentions:readandchat:writescopesPOST /api/content-agent/slackSummary by CodeRabbit
Release Notes
New Features
Documentation