feat: track web or api too#97
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (techengme/myc-4487-track-channel-including-web-api) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds channel detection and propagation into moment creation and messaging flows: origin-based channel inference, new Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Route<br/>/api/moment/create
participant GetChannel as getChannelFromReqHeader
participant CreateMoment as createMoment
participant ProcessMsg as processMessageMoment
participant LogMsg as logMessage
participant TaskTrigger as tasks.trigger
Client->>API: POST /api/moment/create (with Origin)
API->>GetChannel: getChannelFromReqHeader(req)
GetChannel-->>API: channel ('web' / 'api' / default)
API->>CreateMoment: createMoment({...data, channel})
CreateMoment-->>API: { contractAddress, tokenId }
CreateMoment->>ProcessMsg: processMessageMoment({contractAddress, tokenId, artistAddress, channel})
ProcessMsg->>LogMsg: logMessage(message, 'assistant', artistAddress, channel)
LogMsg-->>ProcessMsg: messageId (or null)
alt messageId exists
ProcessMsg->>TaskTrigger: tasks.trigger('process-message-moment', { messageId })
TaskTrigger-->>ProcessMsg: (success or error caught)
end
ProcessMsg-->>CreateMoment: (resolves)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/lib/telegram/chat/__tests__/handleMomentSuccess.test.ts (1)
28-34: Consider removing this redundant test case.This test is essentially a subset of the first test—if
thread.postis called with the correct arguments (verified in the first test), it will also be called exactly once. The two tests exercise the same code path with identical inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/telegram/chat/__tests__/handleMomentSuccess.test.ts` around lines 28 - 34, Remove the redundant test "still posts the success message even when thread.post is called once" from src/lib/telegram/chat/__tests__/handleMomentSuccess.test.ts; it duplicates the first test's assertions because verifying thread.post was called with the correct arguments already implies it was called once. Locate the test by its description and references to makeThread(), handleMomentSuccess, and thread.post, and delete that entire it(...) block to avoid duplicate coverage.src/lib/moment/__tests__/processMessageMoment.test.ts (1)
23-43: Consider adding a test case for undefined channel.The
channelparameter is optional inprocessMessageMoment. Adding a test case wherechannelis omitted would help verify the default behavior and ensure the type cast doesn't cause issues.it('handles undefined channel gracefully', async () => { await processMessageMoment({ contractAddress: CONTRACT, tokenId: '7', artistAddress: ARTIST, // channel omitted }); expect(logMessage).toHaveBeenCalledWith( expect.any(Array), 'assistant', ARTIST, expect.anything() // or the expected default ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/moment/__tests__/processMessageMoment.test.ts` around lines 23 - 43, Add a unit test that calls processMessageMoment without the optional channel to verify default handling and avoid the type cast issue: create a new it block that calls processMessageMoment({ contractAddress: CONTRACT, tokenId: '7', artistAddress: ARTIST }) and assert that logMessage was invoked with the expected message array, 'assistant', ARTIST, and either the concrete default channel value or expect.anything()/expect.any(String) for the channel; reference processMessageMoment and logMessage to locate where to add the test.src/lib/moment/processMessageMoment.ts (1)
19-19: Consider lowercasingcontractAddressin the URL.The coding guidelines specify normalizing addresses to lowercase. While the
Addresstype from viem may already be checksummed, lowercasing it in the URL ensures consistency with other parts of the codebase (e.g., the test filegetMomentFromMessage.test.tsverifies that parsed addresses are lowercased).♻️ Proposed fix
- const successMessage = `✅ Moment created! ${SITE_ORIGINAL_URL}/${path}/${chain}:${contractAddress}/${tokenId}`; + const successMessage = `✅ Moment created! ${SITE_ORIGINAL_URL}/${path}/${chain}:${contractAddress.toLowerCase()}/${tokenId}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/moment/processMessageMoment.ts` at line 19, The success URL embeds contractAddress without normalizing case; update the successMessage construction in processMessageMoment (the line building successMessage using SITE_ORIGINAL_URL, path, chain, contractAddress, tokenId) to use a lowercased contract address (e.g., replace contractAddress with contractAddress.toLowerCase()) so the emitted URL is normalized to lowercase like other parts of the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/moment/createMoment.ts`:
- Around line 85-90: The call to processMessageMoment is passing input.channel
which may be undefined and later is unsafely cast inside processMessageMoment
(see logMessage usage in processMessageMoment.ts), so change the call site
(createMoment -> processMessageMoment) or the log invocation to provide a safe
default channel (e.g., 'api' or 'web') when input.channel is undefined; update
references around processMessageMoment and logMessage to use the fallback value
(instead of blind casting) so the type is always one of 'sms' | 'telegram' |
'web' | 'api'.
In `@src/lib/moment/getChannelFromReqHeader.ts`:
- Around line 3-9: getChannelFromReqHeader currently classifies requests as
'web' only if origin includes 'inprocess.world' or 'stayinprocess.vercel.app',
missing the testnet frontend origin and causing misclassification; update
getChannelFromReqHeader (function) to also check for
'in-process-git-test-sweetmantechs-projects.vercel.app' OR refactor to reuse the
existing trusted-origin constant/pattern (e.g., SITE_ORIGINAL_URL or the
constant defined in src/lib/consts.ts) so the testnet domain is included in the
trusted domains check and requests from testnet are returned as 'web'.
In `@src/lib/moment/processMessageMoment.ts`:
- Around line 21-26: The code unsafely casts the optional channel to the union
type when calling logMessage; instead, validate and normalize channel before
calling logMessage (e.g., check if channel is one of
'sms'|'telegram'|'web'|'api' and otherwise fall back to the default 'sms'), then
pass that validated value to logMessage (referencing the channel variable and
the logMessage call where messageId is created) so the function never receives
undefined or an unexpected string.
In `@src/lib/schema/createMomentSchema.ts`:
- Line 58: The channel field currently allows any string; replace
z.string().optional() in the moment schema (e.g., createMomentSchema /
MomentSchema) with a constrained enum like
z.enum(['sms','telegram','web','api']).optional() so only supported channels
pass validation, and if you need custom logic add a .superRefine() on the schema
to emit a clear validation error for unsupported values or cross-field checks.
In `@src/lib/trigger.dev/__tests__/triggerMuxMigration.test.ts`:
- Line 84: The test currently asserts await
expect(triggerMuxMigration(baseInput)).resolves.not.toThrow(), which is
semantically wrong for a Promise<void>; update the assertion to await
expect(triggerMuxMigration(baseInput)).resolves.toBeUndefined() so the test
checks the resolved value of the Promise returned by triggerMuxMigration (use
the same baseInput and test file triggerMuxMigration.test.ts and the
triggerMuxMigration import to locate the test).
---
Nitpick comments:
In `@src/lib/moment/__tests__/processMessageMoment.test.ts`:
- Around line 23-43: Add a unit test that calls processMessageMoment without the
optional channel to verify default handling and avoid the type cast issue:
create a new it block that calls processMessageMoment({ contractAddress:
CONTRACT, tokenId: '7', artistAddress: ARTIST }) and assert that logMessage was
invoked with the expected message array, 'assistant', ARTIST, and either the
concrete default channel value or expect.anything()/expect.any(String) for the
channel; reference processMessageMoment and logMessage to locate where to add
the test.
In `@src/lib/moment/processMessageMoment.ts`:
- Line 19: The success URL embeds contractAddress without normalizing case;
update the successMessage construction in processMessageMoment (the line
building successMessage using SITE_ORIGINAL_URL, path, chain, contractAddress,
tokenId) to use a lowercased contract address (e.g., replace contractAddress
with contractAddress.toLowerCase()) so the emitted URL is normalized to
lowercase like other parts of the codebase.
In `@src/lib/telegram/chat/__tests__/handleMomentSuccess.test.ts`:
- Around line 28-34: Remove the redundant test "still posts the success message
even when thread.post is called once" from
src/lib/telegram/chat/__tests__/handleMomentSuccess.test.ts; it duplicates the
first test's assertions because verifying thread.post was called with the
correct arguments already implies it was called once. Locate the test by its
description and references to makeThread(), handleMomentSuccess, and
thread.post, and delete that entire it(...) block to avoid duplicate coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f69c2167-d95e-486e-8ec1-34bb422bf66a
📒 Files selected for processing (29)
CLAUDE.mdsrc/app/api/moment/create/route.tssrc/lib/messages/__tests__/getMomentFromMessage.test.tssrc/lib/messages/__tests__/logMessage.test.tssrc/lib/messages/__tests__/processMomentMessage.test.tssrc/lib/messages/getMomentFromMessage.tssrc/lib/messages/logMessage.tssrc/lib/messages/processMomentMessage.tssrc/lib/moment/__tests__/createMoment.test.tssrc/lib/moment/__tests__/getChannelFromReqHeader.test.tssrc/lib/moment/__tests__/processMessageMoment.test.tssrc/lib/moment/createMoment.tssrc/lib/moment/getChannelFromReqHeader.tssrc/lib/moment/processMessageMoment.tssrc/lib/phones/createMomentFromMedia.tssrc/lib/phones/processMmsMedia.tssrc/lib/schema/createMomentSchema.tssrc/lib/supabase/migrations/20260325000000_extend_message_client_enum.sqlsrc/lib/supabase/types.tssrc/lib/telegram/chat/__tests__/createMomentFromTelegramAttachment.test.tssrc/lib/telegram/chat/__tests__/handleMomentSuccess.test.tssrc/lib/telegram/chat/__tests__/processTelegramMedia.test.tssrc/lib/telegram/chat/createMomentFromTelegramAttachment.tssrc/lib/telegram/chat/handleMomentSuccess.tssrc/lib/telegram/chat/handlers/__tests__/onNewMention.test.tssrc/lib/telegram/chat/handlers/onNewMention.tssrc/lib/telegram/chat/processTelegramMedia.tssrc/lib/trigger.dev/__tests__/triggerMuxMigration.test.tssrc/lib/trigger.dev/triggerMuxMigration.ts
| await processMessageMoment({ | ||
| contractAddress, | ||
| tokenId, | ||
| artistAddress: input.account, | ||
| channel: input.channel, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how processMessageMoment handles undefined channel
cat src/lib/moment/processMessageMoment.tsRepository: sweetmantech/In-Process-API
Length of output: 1091
Unsafe type cast when channel is undefined.
input.channel can be undefined (per createMomentSchema), but line 25 of processMessageMoment.ts casts it directly to 'sms' | 'telegram' | 'web' | 'api' without null/undefined handling. While the function defensively checks channel === 'sms' for path logic, it unsafely casts the value when passing to logMessage.
Default to a fallback channel to ensure type safety:
Proposed fix
await processMessageMoment({
contractAddress,
tokenId,
artistAddress: input.account,
- channel: input.channel,
+ channel: input.channel ?? 'api',
});📝 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.
| await processMessageMoment({ | |
| contractAddress, | |
| tokenId, | |
| artistAddress: input.account, | |
| channel: input.channel, | |
| }); | |
| await processMessageMoment({ | |
| contractAddress, | |
| tokenId, | |
| artistAddress: input.account, | |
| channel: input.channel ?? 'api', | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/createMoment.ts` around lines 85 - 90, The call to
processMessageMoment is passing input.channel which may be undefined and later
is unsafely cast inside processMessageMoment (see logMessage usage in
processMessageMoment.ts), so change the call site (createMoment ->
processMessageMoment) or the log invocation to provide a safe default channel
(e.g., 'api' or 'web') when input.channel is undefined; update references around
processMessageMoment and logMessage to use the fallback value (instead of blind
casting) so the type is always one of 'sms' | 'telegram' | 'web' | 'api'.
| const getChannelFromReqHeader = (req: NextRequest): string => { | ||
| const origin = req.headers.get('origin') ?? ''; | ||
| const isWeb = | ||
| origin.includes('inprocess.world') || | ||
| origin.includes('stayinprocess.vercel.app'); | ||
| return isWeb ? 'web' : 'api'; | ||
| }; |
There was a problem hiding this comment.
Missing testnet origin in trusted domains check.
The testnet frontend URL (in-process-git-test-sweetmantechs-projects.vercel.app per src/lib/consts.ts) is not included in the origin check. Requests from testnet will be misclassified as 'api' instead of 'web'.
Proposed fix
const getChannelFromReqHeader = (req: NextRequest): string => {
const origin = req.headers.get('origin') ?? '';
const isWeb =
origin.includes('inprocess.world') ||
- origin.includes('stayinprocess.vercel.app');
+ origin.includes('stayinprocess.vercel.app') ||
+ origin.includes('in-process-git-test-sweetmantechs-projects.vercel.app');
return isWeb ? 'web' : 'api';
};Alternatively, consider extracting the trusted origins to a constant or reusing the SITE_ORIGINAL_URL pattern for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/getChannelFromReqHeader.ts` around lines 3 - 9,
getChannelFromReqHeader currently classifies requests as 'web' only if origin
includes 'inprocess.world' or 'stayinprocess.vercel.app', missing the testnet
frontend origin and causing misclassification; update getChannelFromReqHeader
(function) to also check for
'in-process-git-test-sweetmantechs-projects.vercel.app' OR refactor to reuse the
existing trusted-origin constant/pattern (e.g., SITE_ORIGINAL_URL or the
constant defined in src/lib/consts.ts) so the testnet domain is included in the
trusted domains check and requests from testnet are returned as 'web'.
| const messageId = await logMessage( | ||
| [{ type: 'text', text: successMessage }], | ||
| 'assistant', | ||
| artistAddress, | ||
| channel as 'sms' | 'telegram' | 'web' | 'api' | ||
| ); |
There was a problem hiding this comment.
Unsafe type cast when channel is undefined or unexpected value.
When channel is undefined (the parameter is optional) or an unexpected string value, casting it directly to 'sms' | 'telegram' | 'web' | 'api' will pass an invalid value to logMessage. The logMessage function has a default of 'sms' for the client parameter, but passing undefined explicitly bypasses that default.
🛡️ Proposed fix to handle undefined channel
+ const client = (['sms', 'telegram', 'web', 'api'] as const).includes(channel as any)
+ ? (channel as 'sms' | 'telegram' | 'web' | 'api')
+ : 'sms';
+
const messageId = await logMessage(
[{ type: 'text', text: successMessage }],
'assistant',
artistAddress,
- channel as 'sms' | 'telegram' | 'web' | 'api'
+ client
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const messageId = await logMessage( | |
| [{ type: 'text', text: successMessage }], | |
| 'assistant', | |
| artistAddress, | |
| channel as 'sms' | 'telegram' | 'web' | 'api' | |
| ); | |
| const client = (['sms', 'telegram', 'web', 'api'] as const).includes(channel as any) | |
| ? (channel as 'sms' | 'telegram' | 'web' | 'api') | |
| : 'sms'; | |
| const messageId = await logMessage( | |
| [{ type: 'text', text: successMessage }], | |
| 'assistant', | |
| artistAddress, | |
| client | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/processMessageMoment.ts` around lines 21 - 26, The code
unsafely casts the optional channel to the union type when calling logMessage;
instead, validate and normalize channel before calling logMessage (e.g., check
if channel is one of 'sms'|'telegram'|'web'|'api' and otherwise fall back to the
default 'sms'), then pass that validated value to logMessage (referencing the
channel variable and the logMessage call where messageId is created) so the
function never receives undefined or an unexpected string.
| await expect(triggerMuxMigration(baseInput)).rejects.toThrow( | ||
| 'Trigger.dev unavailable' | ||
| ); | ||
| await expect(triggerMuxMigration(baseInput)).resolves.not.toThrow(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the test file
fd "triggerMuxMigration.test.ts" --type fRepository: sweetmantech/In-Process-API
Length of output: 128
🏁 Script executed:
# Read the test file around line 84 to understand context
cat -n src/lib/trigger.dev/__tests__/triggerMuxMigration.test.ts | sed -n '70,95p'Repository: sweetmantech/In-Process-API
Length of output: 979
🏁 Script executed:
# Find the triggerMuxMigration function implementation
fd "triggerMuxMigration" --type f | grep -v ".test"Repository: sweetmantech/In-Process-API
Length of output: 113
🌐 Web query:
Vitest resolves.toThrow() vs resolves.toBeUndefined() Promise assertion API
💡 Result:
In Vitest, resolves.toThrow and resolves.toBeUndefined are part of the Promise assertion API for testing asynchronous code. expect(promise).resolves.toThrow asserts that the promise resolves (does NOT reject) AND the resolved value is a function that throws an error when called. This is primarily for Jest compatibility when passing async functions that throw, like expect(async => { throw new Error }).resolves.toThrow. However, there are known inconsistencies: .resolves does not auto-unwrap functions like .rejects does, leading to errors like "You must provide a Promise to expect when using .resolves, not 'function'" in some cases. expect(promise).resolves.toBeUndefined asserts that the promise resolves to the primitive value undefined. This is useful for functions expected to return no value asynchronously, e.g., await expect(someAsyncFn).resolves.toBeUndefined. Key differences: - toThrow after resolves expects the resolved value to be a throwable function (uncommon for direct promises). - toBeUndefined expects the resolved value to be undefined (common for void async functions). For expecting a promise to reject with a thrown error, use expect(promise).rejects.toThrow instead, which unwraps the rejection reason. All must be awaited. Official docs emphasize .resolves unwraps resolved values for chaining matchers, and .rejects unwraps rejection reasons.
Citations:
- 1: https://vitest.dev/api/expect.html
- 2: https://v2.vitest.dev/api/expect
- 3: https://v3.vitest.dev/api/expect.html
- 4: https://app.studyraid.com/en/read/11292/352289/testing-promises-and-asyncawait
- 5: "You must provide a Promise to expect()" inconsistent between
resolvesandrejectsvitest-dev/vitest#6643 - 6: https://stackoverflow.com/questions/79417274/how-to-expect-a-promise-to-be-resolved-in-vitest-with-custom-error-message
- 7: fix:
.resolvesand.rejectsexpectations vitest-dev/vitest#1300
🏁 Script executed:
# Read the triggerMuxMigration implementation to understand return type
cat -n src/lib/trigger.dev/triggerMuxMigration.ts | head -50Repository: sweetmantech/In-Process-API
Length of output: 1076
🏁 Script executed:
# Check the full function signature and return type
ast-grep --pattern $'export $$$function triggerMuxMigration($$$) {
$$$
}'Repository: sweetmantech/In-Process-API
Length of output: 53
Use a resolved-value matcher instead of resolves.not.toThrow.
triggerMuxMigration returns Promise<void>, which resolves to undefined. The toThrow matcher is for testing if a function throws; for a resolved Promise, use resolves.toBeUndefined() instead. This avoids semantically incorrect assertions.
Suggested fix
- await expect(triggerMuxMigration(baseInput)).resolves.not.toThrow();
+ await expect(triggerMuxMigration(baseInput)).resolves.toBeUndefined();📝 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.
| await expect(triggerMuxMigration(baseInput)).resolves.not.toThrow(); | |
| await expect(triggerMuxMigration(baseInput)).resolves.toBeUndefined(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/trigger.dev/__tests__/triggerMuxMigration.test.ts` at line 84, The
test currently asserts await
expect(triggerMuxMigration(baseInput)).resolves.not.toThrow(), which is
semantically wrong for a Promise<void>; update the assertion to await
expect(triggerMuxMigration(baseInput)).resolves.toBeUndefined() so the test
checks the resolved value of the Promise returned by triggerMuxMigration (use
the same baseInput and test file triggerMuxMigration.test.ts and the
triggerMuxMigration import to locate the test).
1 participant
Summary by CodeRabbit
New Features
Behavior Changes
Bug Fixes
Tests
Chores