Test#284
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR migrates video-to-Arweave processing from Trigger.dev cloud tasks to Vercel Workflows within the API. It adds video transcoding utilities, refactors network operations with retry logic, implements the orchestration workflow with sequential steps, and updates all integration points throughout the codebase to invoke the new workflow instead of the external task. ChangesMux-to-Arweave Migration via Vercel Workflows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/moment/__tests__/createMoment.test.ts (1)
125-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete test assertion - missing required payload fields.
The test assertion does not match the actual
MigrateMuxToArweavePayloadinterface. According tosrc/workflows/migrateMuxToArweave.ts, the payload requires:
moment.chainId(missing from test)uriat the top level (missing from test)This will cause the test to pass even if the implementation doesn't provide these required fields.
🔧 Proposed fix to match the actual payload structure
it('starts migrate workflow with moment location', async () => { await createMoment(makeInput()); expect(migrateMuxToArweave).toHaveBeenCalledWith( expect.objectContaining({ moment: expect.objectContaining({ collectionAddress: CONTRACT_ADDRESS, tokenId: TOKEN_ID, + chainId: 8453, }), artistAddress: ARTIST, + uri: 'ar://token-meta', }) ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/moment/__tests__/createMoment.test.ts` around lines 125 - 137, The test for createMoment must assert the full MigrateMuxToArweavePayload shape; update the expect for migrateMuxToArweave in the test to include moment.chainId and the top-level uri field in addition to existing collectionAddress/tokenId and artistAddress. Locate the assertion that calls migrateMuxToArweave (in the createMoment test using makeInput()) and add expect.objectContaining({ moment: expect.objectContaining({ chainId: expect.any(Number), collectionAddress: CONTRACT_ADDRESS, tokenId: TOKEN_ID }), uri: expect.any(String), artistAddress: ARTIST }) so the test mirrors the MigrateMuxToArweavePayload contract.
🧹 Nitpick comments (6)
src/lib/og/__tests__/validateOgArtistQuery.test.ts (1)
29-36: ⚡ Quick winAssert the concrete default
CHAIN_ID, not just a number type.The current assertion won’t catch a wrong default (e.g.,
1,10, etc.). Verify exact value.Suggested test tightening
import { NextRequest, NextResponse } from 'next/server'; +import { CHAIN_ID } from '@/lib/consts'; @@ it('defaults chainId to CHAIN_ID when omitted', () => { @@ - expect((result as any).chainId).toBeTypeOf('number'); + expect((result as any).chainId).toBe(CHAIN_ID); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/og/__tests__/validateOgArtistQuery.test.ts` around lines 29 - 36, The test currently only asserts that chainId is a number; change it to assert the concrete default by comparing to the project CHAIN_ID constant: call validateOgArtistQuery via makeRequest({ artistAddress: ARTIST }) as before and replace expect((result as any).chainId).toBeTypeOf('number') with an equality assertion against CHAIN_ID (ensure the test imports or references the same CHAIN_ID used by validateOgArtistQuery) so the test fails if the default value changes.src/lib/video/probeVideo.ts (1)
26-26: ⚡ Quick winExtract magic number to a named constant.
The
50MB threshold for H.264 re-encoding should be extracted to a named constant for clarity and maintainability.♻️ Proposed refactor
+const H264_SIZE_THRESHOLD_MB = 50; + export async function probeVideo(filePath: string): Promise<VideoProbeResult> { const output = await runFfmpegProbe(filePath); const codec = parseCodec(output); // ... existing code ... if (codec === 'h264') { const { size } = await stat(filePath); const fileSizeMB = size / (1024 * 1024); - if (fileSizeMB > 50) { + if (fileSizeMB > H264_SIZE_THRESHOLD_MB) { return { codec, needsReencode: true, reason: `large H.264 file (${fileSizeMB.toFixed(1)} MB)`, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/video/probeVideo.ts` at line 26, Replace the magic number 50 in the size check inside probeVideo (the line "if (fileSizeMB > 50)") with a module-level named constant (e.g., H264_REENCODE_THRESHOLD_MB) declared near the top of src/lib/video/probeVideo.ts; update the if to use H264_REENCODE_THRESHOLD_MB and add a short comment explaining that this constant is the MB threshold for forcing H.264 re-encoding so future maintainers can change it in one place.src/workflows/migrateMuxToArweave.ts (2)
60-64: ⚡ Quick winFire-and-forget workflow start may lose error context.
The default export starts the workflow asynchronously but only logs errors to console. In a production environment, workflow startup failures may go unnoticed. Consider whether callers should be notified of startup failures, or if additional error tracking (e.g., Sentry, monitoring) is needed.
🔍 Proposed improvements
Option 1: Return the promise to allow callers to handle errors:
export default (payload: MigrateMuxToArweavePayload): Promise<void> => { return start(migrateMuxToArweave, [payload]).catch((e) => { console.error('migrate workflow start failed', e); throw e; // Re-throw so caller can handle }); };Option 2: Add structured error logging or monitoring:
export default (payload: MigrateMuxToArweavePayload): void => { start(migrateMuxToArweave, [payload]).catch((e) => { console.error('migrate workflow start failed', { error: e, payload: { tokenId: payload.moment.tokenId, artistAddress: payload.artistAddress } }); // TODO: Send to error tracking service }); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflows/migrateMuxToArweave.ts` around lines 60 - 64, The default export currently fire-and-forgets start(migrateMuxToArweave, [payload]) and only console.error on rejection, risking lost failures; update the exported function (the default export that takes MigrateMuxToArweavePayload) to either return the start(...) Promise so callers can catch/rethrow errors (i.e., propagate the rejection) or enhance the catch to perform structured logging/monitoring (include payload identifying fields) and report to your error-tracking system; reference the start function and migrateMuxToArweave workflow when making the change so you modify the export’s error handling accordingly.
39-39: ⚡ Quick winDocument the 10-minute sleep rationale.
The 10-minute sleep is waiting for Arweave block propagation, but this is not immediately obvious from the code. Consider adding a comment to explain the reasoning and whether this duration is based on Arweave's documented propagation time.
📝 Proposed improvement
+ // Wait for Arweave transaction to propagate across the network + // Arweave typically requires 8-12 minutes for data availability await sleep('10 minutes');Or extract to a named constant:
+const ARWEAVE_PROPAGATION_WAIT = '10 minutes'; + async function migrateMuxToArweave(p: MigrateMuxToArweavePayload) { 'use workflow'; // ... - await sleep('10 minutes'); + await sleep(ARWEAVE_PROPAGATION_WAIT);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflows/migrateMuxToArweave.ts` at line 39, The implicit 10-minute wait at the await sleep('10 minutes') call is unclear—document why this delay is needed (waiting for Arweave block propagation) and whether the duration is derived from Arweave docs/empirical testing; update the code by either adding a clear inline comment above the sleep call explaining the rationale and source for "10 minutes" or extract the duration into a named constant (e.g., ARWEAVE_PROPAGATION_WAIT = '10 minutes' or ARWEAVE_PROPAGATION_MS) with a comment that cites the reason/source and use that constant in the await sleep(...) invocation for clarity.docs/migrate-trigger-to-vercel-workflows.md (1)
20-22: 💤 Low valueAdd language specifiers to fenced code blocks.
The fenced code blocks on lines 20-22 and 26-28 are missing language specifiers. Add
textor leave them as triple-backticks with an empty language tag to resolve the markdownlint warnings.📝 Proposed fix
### Before -``` +```text API → tasks.trigger('migrate-mux-to-arweave') → Trigger.dev Cloud → In-Process-TasksAfter
-
+text
API → start(migrateMuxToArweaveWorkflow) → Vercel Workflows (within API)Also applies to: 26-28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/migrate-trigger-to-vercel-workflows.md` around lines 20 - 22, Add language specifiers to the fenced code blocks containing the flow lines so markdownlint warnings are resolved: replace the triple-backtick fences around "API → tasks.trigger('migrate-mux-to-arweave') → Trigger.dev Cloud → In-Process-Tasks" and "API → start(migrateMuxToArweaveWorkflow) → Vercel Workflows (within API)" with fences that include a language tag (e.g., ```text) or an explicit empty tag, keeping the inner text unchanged; ensure both blocks on lines showing those exact strings are updated.src/lib/video/transcodeIfH265.ts (1)
38-38: ⚡ Quick winHandle filenames without extensions.
If
videoFile.namehas no extension (e.g.,"video"), the regex won't match and.mp4won't be appended, resulting in aFilewith name"video"but type'video/mp4'.🔧 Proposed fix to ensure .mp4 extension
- const transcodedName = videoFile.name.replace(/\.[^.]+$/, '.mp4'); + const transcodedName = videoFile.name.includes('.') + ? videoFile.name.replace(/\.[^.]+$/, '.mp4') + : `${videoFile.name}.mp4`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/video/transcodeIfH265.ts` at line 38, The current transcodedName assignment using videoFile.name.replace(/\.[^.]+$/, '.mp4') fails when videoFile.name has no extension; update the logic in transcodeIfH265 (the transcodedName computation) to detect whether videoFile.name has an extension (e.g., test for /\.[^/.]+$/) and if it does replace that extension with '.mp4', otherwise append '.mp4' to the name so the resulting File name always ends with .mp4 while keeping the rest of the filename intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/moment/update-uri/route.ts`:
- Around line 7-9: The route is returning the promise from
updateMomentURIHandler so thrown errors escape the local try/catch; change the
call to await the handler result instead of returning the promise directly
(i.e., await updateMomentURIHandler(validated)) after validateUpdateMomentURI
succeeds so the surrounding catch can handle rejections and return your JSON 500
response; update the code using the existing functions validateUpdateMomentURI
and updateMomentURIHandler in this route.
In `@src/app/api/og/artist/route.tsx`:
- Around line 14-18: The handler currently exposes raw exception text via the
const message / error branch before returning NextResponse.json; instead, log
the full error server-side (e.g., console.error or your app logger) and replace
the response body with a generic 500 message like "Internal server error" so
clients never see raw exception details while preserving the error for debugging
in logs; update the code around the const message / NextResponse.json call
accordingly.
In `@src/lib/collection/__tests__/updateCollectionURIHandler.test.ts`:
- Around line 56-67: The test for updateCollectionURIHandler is missing an
assertion that the migrateMuxToArweave call includes the migrated uri; update
the expectation in the test that calls updateCollectionURIHandler(baseInput) to
assert migrateMuxToArweave was called with an objectContaining a uri field
(e.g., uri: EXPECTED_URI) alongside the existing
moment.collectionAddress/tokenId/chainId and artistAddress checks so the
workflow payload includes the expected migrated URI.
In `@src/lib/getBlob.ts`:
- Around line 15-17: The fetch in the tryFn lacks a per-attempt timeout so a
hung request can block retries; wrap each fetch call with an AbortController and
a setTimeout to abort after a reasonable per-attempt timeout (e.g., 5s), pass
controller.signal into fetch({ headers, signal }), clear the timeout on success,
and ensure the abort triggers the retry logic (propagate/throw the abort error
so retrying logic can catch it) in the tryFn.
In `@src/lib/metadata/getMetadataHandler.ts`:
- Around line 10-15: Add an HTTP status guard in the metadata fetch flow: in the
code that calls fetchUri (the block using fetchUri, response.headers,
response.json/text and normalizeMetadata), check response.ok immediately after
receiving the response and before attempting to parse/normalize. If
!response.ok, throw or return a clear error including response.status and
response.statusText (and optionally the raw body) so error payloads are not
treated as metadata; reference the existing fetchUri call, the response
variable, and normalizeMetadata to locate where to insert the guard.
In `@src/lib/moment/createMoment.ts`:
- Around line 78-82: The call to migrateMuxToArweave in createMoment.ts is
fire-and-forget and can produce unhandled rejections; either await
migrateMuxToArweave(...) where the migration must complete before proceeding, or
keep it non-blocking but attach explicit error handling (e.g.,
migrateMuxToArweave(...).catch(err => /* log with context */)) so failures are
logged; reference the migrateMuxToArweave call and include contextual data
(artistAddress/collectionAddress/tokenId/chainId/uri) in the error log for
debugging.
In `@src/lib/moment/validateUpdateMomentURI.ts`:
- Around line 10-12: Wrap the call to req.json() in a try/catch in
validateUpdateMomentURI so malformed JSON yields a 400 validation response
instead of throwing; specifically, catch SyntaxError (or any error from await
req.json()), return the same shaped validation error response used elsewhere,
then pass the parsed body into validate(updateMomentURISchema, body) as
before—use the existing validate function and result.response to keep behavior
consistent with validateUpload.ts.
In `@src/lib/mux/downloadVideo.ts`:
- Around line 3-17: The function downloadVideo is currently a named export but
the repo guideline requires a default export for single-utility files; change
the export of downloadVideo to be the default export (export default
downloadVideo) and ensure any callers/import sites are updated to import the
default (e.g., import downloadVideo from '...') rather than a named import; keep
the implementation (uses getBlob and constructs a File) unchanged and verify
there are no lingering named-import references.
In `@src/lib/mux/findMuxAssetIdFromPlaybackUrl.ts`:
- Around line 12-18: The code in findMuxAssetIdFromPlaybackUrl currently wraps
both the regex match and the API call in one try-catch, converting all errors to
null; change it so the regex parse of the playbackUrl runs outside any try-catch
and returns null immediately when there's no match, then wrap only the Mux API
call (the network/client call used to resolve the asset id) in a try-catch that
rethrows the caught error (or remove the catch) instead of returning null so
transient API failures bubble up for retries/alerts; locate the regex parsing
and the API invocation inside findMuxAssetIdFromPlaybackUrl and adjust the
try-catch boundaries accordingly.
In `@src/lib/og/validateOgArtistQuery.ts`:
- Around line 13-16: The query validator in validateOgArtistQuery.ts currently
calls the shared validate() helper with ogArtistQuerySchema and the query
object; change this to call ogArtistQuerySchema.safeParse(...) directly using
the same Object.fromEntries(req.nextUrl.searchParams.entries()) input, and
update the subsequent logic to handle the safeParse result (check result.success
and use result.error or result.data) instead of relying on validate() behavior
so query param validation follows the established pattern.
In `@src/lib/video/probeVideo.ts`:
- Line 11: Change the named export of the probeVideo function to a default
export while keeping the VideoProbeResult type as a named export: export the
interface VideoProbeResult unchanged and make probeVideo the default export
(i.e., default async function probeVideo(...)). Then update all consuming
imports to use the default import for probeVideo and named import for the type
(e.g., import probeVideo, { type VideoProbeResult } from '...') so callers still
get the type. Ensure the function signature and return type remain identical and
adjust any re-exports if present to use the new default export.
In `@src/lib/video/runFfmpegProbe.ts`:
- Around line 18-20: The catch in runFfmpegProbe currently returns an empty
string for every execFile failure, which hides real spawn errors
(missing/unexecutable ffmpeg); change the catch so that if err.stderr exists
return that stderr (preserving the current behavior for ffmpeg non-zero exit),
but if stderr is undefined (spawn/permission error) rethrow or propagate the
original err instead of returning ''. This touches runFfmpegProbe and how its
execFile errors flow into parseCodec/probeVideo.
In `@src/lib/video/transcodeIfH265.ts`:
- Line 7: The function transcodeIfH265 is currently a named export but the
project convention requires a default export for utility functions; change the
export to a default export (export default transcodeIfH265) while keeping the
function signature and behavior intact, and update all callers to import it as
the default (e.g., import transcodeIfH265 from '...') instead of using named
imports; search for any references to transcodeIfH265 to update their import
statements accordingly.
In `@src/lib/video/transcodeToH264.ts`:
- Around line 6-20: The function transcodeToH264 is currently a named export but
per project convention this utility should be the file's default export; change
the export from "export function transcodeToH264(...)" to a default export
(e.g., "export default function transcodeToH264(...)") and keep the existing
signature and behavior, then update any import sites to use a default import
(e.g., import transcodeToH264 from '...') so consumers remain correct.
In `@src/workflows/steps/downloadAndTranscodeStep.ts`:
- Around line 17-29: The current code detaches
unauthTurboClient.getTokenPriceForBytes(...).then(...insertArweaveUpload...)
which creates an unawaited promise; instead make the step await the audit write
so it becomes part of the step lifecycle: call
unauthTurboClient.getTokenPriceForBytes and await its result (reference:
unauthTurboClient.getTokenPriceForBytes), then await insertArweaveUpload(...)
with the returned tokenPrice (reference: insertArweaveUpload) inside a try/catch
so errors are logged and the step only returns after the insert completes.
---
Outside diff comments:
In `@src/lib/moment/__tests__/createMoment.test.ts`:
- Around line 125-137: The test for createMoment must assert the full
MigrateMuxToArweavePayload shape; update the expect for migrateMuxToArweave in
the test to include moment.chainId and the top-level uri field in addition to
existing collectionAddress/tokenId and artistAddress. Locate the assertion that
calls migrateMuxToArweave (in the createMoment test using makeInput()) and add
expect.objectContaining({ moment: expect.objectContaining({ chainId:
expect.any(Number), collectionAddress: CONTRACT_ADDRESS, tokenId: TOKEN_ID }),
uri: expect.any(String), artistAddress: ARTIST }) so the test mirrors the
MigrateMuxToArweavePayload contract.
---
Nitpick comments:
In `@docs/migrate-trigger-to-vercel-workflows.md`:
- Around line 20-22: Add language specifiers to the fenced code blocks
containing the flow lines so markdownlint warnings are resolved: replace the
triple-backtick fences around "API → tasks.trigger('migrate-mux-to-arweave') →
Trigger.dev Cloud → In-Process-Tasks" and "API →
start(migrateMuxToArweaveWorkflow) → Vercel Workflows (within API)" with fences
that include a language tag (e.g., ```text) or an explicit empty tag, keeping
the inner text unchanged; ensure both blocks on lines showing those exact
strings are updated.
In `@src/lib/og/__tests__/validateOgArtistQuery.test.ts`:
- Around line 29-36: The test currently only asserts that chainId is a number;
change it to assert the concrete default by comparing to the project CHAIN_ID
constant: call validateOgArtistQuery via makeRequest({ artistAddress: ARTIST })
as before and replace expect((result as any).chainId).toBeTypeOf('number') with
an equality assertion against CHAIN_ID (ensure the test imports or references
the same CHAIN_ID used by validateOgArtistQuery) so the test fails if the
default value changes.
In `@src/lib/video/probeVideo.ts`:
- Line 26: Replace the magic number 50 in the size check inside probeVideo (the
line "if (fileSizeMB > 50)") with a module-level named constant (e.g.,
H264_REENCODE_THRESHOLD_MB) declared near the top of
src/lib/video/probeVideo.ts; update the if to use H264_REENCODE_THRESHOLD_MB and
add a short comment explaining that this constant is the MB threshold for
forcing H.264 re-encoding so future maintainers can change it in one place.
In `@src/lib/video/transcodeIfH265.ts`:
- Line 38: The current transcodedName assignment using
videoFile.name.replace(/\.[^.]+$/, '.mp4') fails when videoFile.name has no
extension; update the logic in transcodeIfH265 (the transcodedName computation)
to detect whether videoFile.name has an extension (e.g., test for /\.[^/.]+$/)
and if it does replace that extension with '.mp4', otherwise append '.mp4' to
the name so the resulting File name always ends with .mp4 while keeping the rest
of the filename intact.
In `@src/workflows/migrateMuxToArweave.ts`:
- Around line 60-64: The default export currently fire-and-forgets
start(migrateMuxToArweave, [payload]) and only console.error on rejection,
risking lost failures; update the exported function (the default export that
takes MigrateMuxToArweavePayload) to either return the start(...) Promise so
callers can catch/rethrow errors (i.e., propagate the rejection) or enhance the
catch to perform structured logging/monitoring (include payload identifying
fields) and report to your error-tracking system; reference the start function
and migrateMuxToArweave workflow when making the change so you modify the
export’s error handling accordingly.
- Line 39: The implicit 10-minute wait at the await sleep('10 minutes') call is
unclear—document why this delay is needed (waiting for Arweave block
propagation) and whether the duration is derived from Arweave docs/empirical
testing; update the code by either adding a clear inline comment above the sleep
call explaining the rationale and source for "10 minutes" or extract the
duration into a named constant (e.g., ARWEAVE_PROPAGATION_WAIT = '10 minutes' or
ARWEAVE_PROPAGATION_MS) with a comment that cites the reason/source and use that
constant in the await sleep(...) invocation for clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5d4f9c9-bb94-43d9-8bf6-df9159d51e9d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
docs/migrate-trigger-to-vercel-workflows.mdnext.config.tspackage.jsonsrc/app/api/moment/update-uri/route.tssrc/app/api/og/artist/route.tsxsrc/lib/arweave/uploadToArweave.tssrc/lib/collection/__tests__/updateCollectionURI.test.tssrc/lib/collection/__tests__/updateCollectionURIHandler.test.tssrc/lib/collection/updateCollectionURI.tssrc/lib/collection/updateCollectionURIHandler.tssrc/lib/getBlob.tssrc/lib/metadata/__tests__/getMetadataHandler.test.tssrc/lib/metadata/getMetadataHandler.tssrc/lib/moment/__tests__/createMoment.test.tssrc/lib/moment/__tests__/updateMomentURI.test.tssrc/lib/moment/createMoment.tssrc/lib/moment/createMoments.tssrc/lib/moment/updateMomentURI.tssrc/lib/moment/updateMomentURIHandler.tssrc/lib/moment/validateUpdateMomentURI.tssrc/lib/mux/downloadVideo.tssrc/lib/mux/findMuxAssetIdFromPlaybackUrl.tssrc/lib/og/__tests__/getOgArtistHandler.test.tssrc/lib/og/__tests__/validateOgArtistQuery.test.tssrc/lib/og/getOgArtistHandler.tsxsrc/lib/og/validateOgArtistQuery.tssrc/lib/trigger.dev/__tests__/triggerMuxMigration.test.tssrc/lib/trigger.dev/triggerMuxMigration.tssrc/lib/video/parseCodec.tssrc/lib/video/probeVideo.tssrc/lib/video/runFfmpegProbe.tssrc/lib/video/transcodeIfH265.tssrc/lib/video/transcodeToH264.tssrc/proxy.tssrc/workflows/migrateMuxToArweave.tssrc/workflows/steps/deleteMuxAssetStep.tssrc/workflows/steps/downloadAndTranscodeStep.tssrc/workflows/steps/fetchMetadataStep.tssrc/workflows/steps/updateOnChainStep.tssrc/workflows/steps/uploadMetadataStep.tsvercel.json
💤 Files with no reviewable changes (6)
- src/lib/collection/tests/updateCollectionURI.test.ts
- src/lib/trigger.dev/triggerMuxMigration.ts
- src/lib/collection/updateCollectionURI.ts
- src/lib/moment/updateMomentURI.ts
- src/lib/moment/tests/updateMomentURI.test.ts
- src/lib/trigger.dev/tests/triggerMuxMigration.test.ts
| const validated = await validateUpdateMomentURI(req); | ||
| if (validated instanceof NextResponse) return validated; | ||
| return updateMomentURIHandler(validated); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/app/api/moment/update-uri/route.tsRepository: sweetmantech/In-Process-API
Length of output: 887
🏁 Script executed:
fd updateMomentURIHandler --type f | head -5Repository: sweetmantech/In-Process-API
Length of output: 112
🏁 Script executed:
cat -n src/lib/moment/updateMomentURIHandler.tsRepository: sweetmantech/In-Process-API
Length of output: 1148
Await the handler so this route-level catch actually applies.
Line 9 returns the promise directly, so a rejection from updateMomentURIHandler skips the catch below and falls back to Next's default error handling instead of your JSON 500 response.
🐛 Suggested change
- return updateMomentURIHandler(validated);
+ return await updateMomentURIHandler(validated);📝 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 validated = await validateUpdateMomentURI(req); | |
| if (validated instanceof NextResponse) return validated; | |
| return updateMomentURIHandler(validated); | |
| const validated = await validateUpdateMomentURI(req); | |
| if (validated instanceof NextResponse) return validated; | |
| return await updateMomentURIHandler(validated); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/moment/update-uri/route.ts` around lines 7 - 9, The route is
returning the promise from updateMomentURIHandler so thrown errors escape the
local try/catch; change the call to await the handler result instead of
returning the promise directly (i.e., await updateMomentURIHandler(validated))
after validateUpdateMomentURI succeeds so the surrounding catch can handle
rejections and return your JSON 500 response; update the code using the existing
functions validateUpdateMomentURI and updateMomentURIHandler in this route.
| const message = | ||
| error instanceof Error | ||
| ? error.message | ||
| : 'Failed to render OG artist image'; | ||
| return NextResponse.json({ error: message }, { status: 500 }); |
There was a problem hiding this comment.
Don’t expose raw exception messages in the API response.
Keep detailed error data in server logs and return a generic 500 message to clients.
Safer 500 response
- const message =
- error instanceof Error
- ? error.message
- : 'Failed to render OG artist image';
- return NextResponse.json({ error: message }, { status: 500 });
+ return NextResponse.json(
+ { error: 'Failed to render OG artist image' },
+ { status: 500 }
+ );📝 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 message = | |
| error instanceof Error | |
| ? error.message | |
| : 'Failed to render OG artist image'; | |
| return NextResponse.json({ error: message }, { status: 500 }); | |
| return NextResponse.json( | |
| { error: 'Failed to render OG artist image' }, | |
| { status: 500 } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/og/artist/route.tsx` around lines 14 - 18, The handler currently
exposes raw exception text via the const message / error branch before returning
NextResponse.json; instead, log the full error server-side (e.g., console.error
or your app logger) and replace the response body with a generic 500 message
like "Internal server error" so clients never see raw exception details while
preserving the error for debugging in logs; update the code around the const
message / NextResponse.json call accordingly.
| it('starts migrate workflow with collection location', async () => { | ||
| await updateCollectionURIHandler(baseInput); | ||
| expect(migrateMuxToArweave).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| moment: expect.objectContaining({ | ||
| collectionAddress: COLLECTION_ADDRESS, | ||
| tokenId: '0', | ||
| chainId: 8453, | ||
| }), | ||
| artistAddress: ARTIST_ADDRESS, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Assert the migrated uri in the workflow call.
This test verifies location fields but misses the uri argument, which is critical for the migration flow payload.
Proposed fix
expect(migrateMuxToArweave).toHaveBeenCalledWith(
expect.objectContaining({
moment: expect.objectContaining({
collectionAddress: COLLECTION_ADDRESS,
tokenId: '0',
chainId: 8453,
}),
artistAddress: ARTIST_ADDRESS,
+ uri: NEW_URI,
})
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/collection/__tests__/updateCollectionURIHandler.test.ts` around lines
56 - 67, The test for updateCollectionURIHandler is missing an assertion that
the migrateMuxToArweave call includes the migrated uri; update the expectation
in the test that calls updateCollectionURIHandler(baseInput) to assert
migrateMuxToArweave was called with an objectContaining a uri field (e.g., uri:
EXPECTED_URI) alongside the existing moment.collectionAddress/tokenId/chainId
and artistAddress checks so the workflow payload includes the expected migrated
URI.
| tryFn: async () => { | ||
| const res = await fetch(url, { headers }); | ||
| if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read its contents
cat -n src/lib/getBlob.tsRepository: sweetmantech/In-Process-API
Length of output: 1180
🏁 Script executed:
# Find and check the retriesGeneric function
find src -name "*retries*" -type fRepository: sweetmantech/In-Process-API
Length of output: 102
🏁 Script executed:
# Read the retries file to understand retry logic
cat -n src/lib/protocolSdk/retries.tsRepository: sweetmantech/In-Process-API
Length of output: 1425
Add a timeout per fetch attempt.
At line 16, each retried fetch has no timeout, so a single hung request can block the operation indefinitely. With 3 retry attempts and 2-second backoff, this compounds the risk.
Proposed fix
const response = await retriesGeneric({
maxTries: 3,
linearBackoffMS: 2_000,
tryFn: async () => {
- const res = await fetch(url, { headers });
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), 30_000);
+ let res: Response;
+ try {
+ res = await fetch(url, { headers, signal: controller.signal });
+ } finally {
+ clearTimeout(timeout);
+ }
if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`);
return res;
},
});📝 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.
| tryFn: async () => { | |
| const res = await fetch(url, { headers }); | |
| if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`); | |
| const response = await retriesGeneric({ | |
| maxTries: 3, | |
| linearBackoffMS: 2_000, | |
| tryFn: async () => { | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 30_000); | |
| let res: Response; | |
| try { | |
| res = await fetch(url, { headers, signal: controller.signal }); | |
| } finally { | |
| clearTimeout(timeout); | |
| } | |
| if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`); | |
| return res; | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/getBlob.ts` around lines 15 - 17, The fetch in the tryFn lacks a
per-attempt timeout so a hung request can block retries; wrap each fetch call
with an AbortController and a setTimeout to abort after a reasonable per-attempt
timeout (e.g., 5s), pass controller.signal into fetch({ headers, signal }),
clear the timeout on success, and ensure the abort triggers the retry logic
(propagate/throw the abort error so retrying logic can catch it) in the tryFn.
| const response = await fetchUri(uri, { cache: 'no-store' }); | ||
| const contentType = response.headers.get('content-type'); | ||
| const data = contentType?.includes('application/json') | ||
| ? await response.json() | ||
| : JSON.parse(await response.text()); | ||
| return normalizeMetadata(data); |
There was a problem hiding this comment.
Add an HTTP status guard before metadata parsing.
Line 10 parses and normalizes the response even when the request fails (non-2xx), which can treat error payloads as metadata. Fail fast on !response.ok before parse/normalize.
Proposed fix
tryFn: async () => {
const response = await fetchUri(uri, { cache: 'no-store' });
+ if (!response.ok) {
+ throw new Error(`Failed to fetch metadata from URI: ${uri}`);
+ }
const contentType = response.headers.get('content-type');
const data = contentType?.includes('application/json')
? await response.json()
: JSON.parse(await response.text());
return normalizeMetadata(data);
},📝 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 response = await fetchUri(uri, { cache: 'no-store' }); | |
| const contentType = response.headers.get('content-type'); | |
| const data = contentType?.includes('application/json') | |
| ? await response.json() | |
| : JSON.parse(await response.text()); | |
| return normalizeMetadata(data); | |
| const response = await fetchUri(uri, { cache: 'no-store' }); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch metadata from URI: ${uri}`); | |
| } | |
| const contentType = response.headers.get('content-type'); | |
| const data = contentType?.includes('application/json') | |
| ? await response.json() | |
| : JSON.parse(await response.text()); | |
| return normalizeMetadata(data); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/metadata/getMetadataHandler.ts` around lines 10 - 15, Add an HTTP
status guard in the metadata fetch flow: in the code that calls fetchUri (the
block using fetchUri, response.headers, response.json/text and
normalizeMetadata), check response.ok immediately after receiving the response
and before attempting to parse/normalize. If !response.ok, throw or return a
clear error including response.status and response.statusText (and optionally
the raw body) so error payloads are not treated as metadata; reference the
existing fetchUri call, the response variable, and normalizeMetadata to locate
where to insert the guard.
| reason: string; | ||
| } | ||
|
|
||
| export async function probeVideo(filePath: string): Promise<VideoProbeResult> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use default export for consistency.
The coding guidelines specify that each utility function in src/lib/**/*.ts should use a default export to follow the Single Responsibility Principle.
♻️ Proposed refactor to use default export
-export async function probeVideo(filePath: string): Promise<VideoProbeResult> {
+export default async function probeVideo(filePath: string): Promise<VideoProbeResult> {Keep the interface as a named export (types are fine):
export interface VideoProbeResult { ... }
export default async function probeVideo(...) { ... }Update imports in consuming files:
import probeVideo, { type VideoProbeResult } from '@/lib/video/probeVideo';As per coding guidelines: "Each utility function should be in its own file with a default export — follow the Single Responsibility Principle (SRP)".
📝 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.
| export async function probeVideo(filePath: string): Promise<VideoProbeResult> { | |
| export default async function probeVideo(filePath: string): Promise<VideoProbeResult> { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/video/probeVideo.ts` at line 11, Change the named export of the
probeVideo function to a default export while keeping the VideoProbeResult type
as a named export: export the interface VideoProbeResult unchanged and make
probeVideo the default export (i.e., default async function probeVideo(...)).
Then update all consuming imports to use the default import for probeVideo and
named import for the type (e.g., import probeVideo, { type VideoProbeResult }
from '...') so callers still get the type. Ensure the function signature and
return type remain identical and adjust any re-exports if present to use the new
default export.
| } catch (err: any) { | ||
| // ffmpeg exits non-zero when given -i with no output — stderr still has the info | ||
| return err.stderr ?? ''; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src -name "runFfmpegProbe.ts" -type fRepository: sweetmantech/In-Process-API
Length of output: 103
🏁 Script executed:
cat -n src/lib/video/runFfmpegProbe.tsRepository: sweetmantech/In-Process-API
Length of output: 848
🏁 Script executed:
rg -A 10 -B 5 "runFfmpegProbe" src/lib --type tsRepository: sweetmantech/In-Process-API
Length of output: 2230
🏁 Script executed:
cat -n src/lib/video/parseCodec.tsRepository: sweetmantech/In-Process-API
Length of output: 277
🏁 Script executed:
rg -A 5 "runFfmpegProbe\|probeVideo" src --type ts | head -60Repository: sweetmantech/In-Process-API
Length of output: 53
🏁 Script executed:
rg "ENOENT\|spawn\|execFile" src --type tsRepository: sweetmantech/In-Process-API
Length of output: 53
🏁 Script executed:
cat -n src/lib/video/probeVideo.tsRepository: sweetmantech/In-Process-API
Length of output: 1435
🏁 Script executed:
# Check how probeVideo is used further downstream
rg "probeVideo" src --type ts -A 3 -B 1Repository: sweetmantech/In-Process-API
Length of output: 1033
🏁 Script executed:
python3 << 'EOF'
# Document Node.js promisified execFile error behavior
info = """
Node.js child_process.execFile behavior with promisify:
1. When command exits with non-zero code (normal ffmpeg behavior):
- Promise rejects with ExecFileException
- Error has: code (exit code), signal, stdout, stderr (populated)
- stderr contains the ffmpeg output
2. When command fails to spawn (ENOENT, EACCES, etc.):
- Promise rejects with error
- Error has: code ('ENOENT', 'EACCES', etc.), errno, syscall
- stderr is typically undefined or empty (never populated)
- This indicates a real failure, not expected behavior
Current code: return err.stderr ?? ''
- Both cases 1 and 2 above return a string (either populated stderr or '')
- Case 2 (startup failure) gets masked as "empty probe" → parseCodec returns null
- Downstream treats it as "no video stream" instead of actual error
"""
print(info)
EOFRepository: sweetmantech/In-Process-API
Length of output: 885
🏁 Script executed:
fd "test\|spec" src/lib/video --type fRepository: sweetmantech/In-Process-API
Length of output: 53
🏁 Script executed:
rg "runFfmpegProbe\|probeVideo" src --type ts --files-with-matchesRepository: sweetmantech/In-Process-API
Length of output: 53
Don't swallow probe startup failures.
Lines 18-20 currently turn every execFile failure into ''. That hides real probe failures like a missing/unexecutable ffmpeg binary. When execFile fails to spawn, err.stderr is undefined, so the function returns ''. This gets passed to parseCodec() which treats it as an empty probe result, and probeVideo() returns { codec: null, needsReencode: false, reason: 'no video stream' }. The actual error (missing ffmpeg, permissions issue, etc.) is silently converted into a successful "no video found" result, preventing proper error handling upstream.
🐛 Suggested change
} catch (err: any) {
// ffmpeg exits non-zero when given -i with no output — stderr still has the info
- return err.stderr ?? '';
+ if (typeof err?.stderr === 'string' && err.stderr.length > 0) {
+ return err.stderr;
+ }
+ throw err;
}
}📝 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.
| } catch (err: any) { | |
| // ffmpeg exits non-zero when given -i with no output — stderr still has the info | |
| return err.stderr ?? ''; | |
| } catch (err: any) { | |
| // ffmpeg exits non-zero when given -i with no output — stderr still has the info | |
| if (typeof err?.stderr === 'string' && err.stderr.length > 0) { | |
| return err.stderr; | |
| } | |
| throw err; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/video/runFfmpegProbe.ts` around lines 18 - 20, The catch in
runFfmpegProbe currently returns an empty string for every execFile failure,
which hides real spawn errors (missing/unexecutable ffmpeg); change the catch so
that if err.stderr exists return that stderr (preserving the current behavior
for ffmpeg non-zero exit), but if stderr is undefined (spawn/permission error)
rethrow or propagate the original err instead of returning ''. This touches
runFfmpegProbe and how its execFile errors flow into parseCodec/probeVideo.
| import { probeVideo } from './probeVideo'; | ||
| import { transcodeToH264 } from './transcodeToH264'; | ||
|
|
||
| export async function transcodeIfH265(videoFile: File): Promise<File> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use default export for consistency.
The coding guidelines specify that each utility function in src/lib/**/*.ts should use a default export to follow the Single Responsibility Principle.
♻️ Proposed refactor to use default export
-export async function transcodeIfH265(videoFile: File): Promise<File> {
+export default async function transcodeIfH265(videoFile: File): Promise<File> {Update imports in consuming files accordingly:
import transcodeIfH265 from '@/lib/video/transcodeIfH265';As per coding guidelines: "Each utility function should be in its own file with a default export — follow the Single Responsibility Principle (SRP)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/video/transcodeIfH265.ts` at line 7, The function transcodeIfH265 is
currently a named export but the project convention requires a default export
for utility functions; change the export to a default export (export default
transcodeIfH265) while keeping the function signature and behavior intact, and
update all callers to import it as the default (e.g., import transcodeIfH265
from '...') instead of using named imports; search for any references to
transcodeIfH265 to update their import statements accordingly.
| export function transcodeToH264( | ||
| inputPath: string, | ||
| outputPath: string | ||
| ): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| ffmpeg(inputPath) | ||
| .videoCodec('libx264') | ||
| .audioCodec('aac') | ||
| .outputOptions(['-crf 23', '-preset fast', '-movflags +faststart']) | ||
| .output(outputPath) | ||
| .on('end', () => resolve()) | ||
| .on('error', (err) => reject(err)) | ||
| .run(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the file’s default export for this utility.
This file already holds a single utility, so keeping it as a named export makes imports inconsistent with the src/lib convention.
♻️ Suggested change
-export function transcodeToH264(
+export default function transcodeToH264(
inputPath: string,
outputPath: string
): Promise<void> {As per coding guidelines "Each utility function should be in its own file with a default export — follow the Single Responsibility Principle (SRP)".
📝 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.
| export function transcodeToH264( | |
| inputPath: string, | |
| outputPath: string | |
| ): Promise<void> { | |
| return new Promise((resolve, reject) => { | |
| ffmpeg(inputPath) | |
| .videoCodec('libx264') | |
| .audioCodec('aac') | |
| .outputOptions(['-crf 23', '-preset fast', '-movflags +faststart']) | |
| .output(outputPath) | |
| .on('end', () => resolve()) | |
| .on('error', (err) => reject(err)) | |
| .run(); | |
| }); | |
| } | |
| export default function transcodeToH264( | |
| inputPath: string, | |
| outputPath: string | |
| ): Promise<void> { | |
| return new Promise((resolve, reject) => { | |
| ffmpeg(inputPath) | |
| .videoCodec('libx264') | |
| .audioCodec('aac') | |
| .outputOptions(['-crf 23', '-preset fast', '-movflags +faststart']) | |
| .output(outputPath) | |
| .on('end', () => resolve()) | |
| .on('error', (err) => reject(err)) | |
| .run(); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/video/transcodeToH264.ts` around lines 6 - 20, The function
transcodeToH264 is currently a named export but per project convention this
utility should be the file's default export; change the export from "export
function transcodeToH264(...)" to a default export (e.g., "export default
function transcodeToH264(...)") and keep the existing signature and behavior,
then update any import sites to use a default import (e.g., import
transcodeToH264 from '...') so consumers remain correct.
| unauthTurboClient | ||
| .getTokenPriceForBytes({ byteCount: readyFile.size }) | ||
| .then(({ tokenPrice }) => | ||
| insertArweaveUpload({ | ||
| arweave_uri: result.arweave_uri, | ||
| winc_cost: result.winc_cost, | ||
| usdc_cost: Number(tokenPrice), | ||
| file_size_bytes: readyFile.size, | ||
| content_type: readyFile.type || 'video/mp4', | ||
| artist_address: artistAddress, | ||
| }) | ||
| ) | ||
| .catch((e: unknown) => console.error('logArweaveUpload:', e)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/workflows/steps/downloadAndTranscodeStep.ts | head -50Repository: sweetmantech/In-Process-API
Length of output: 1494
🏁 Script executed:
rg -t ts 'use step' src/workflows/ -B 2 -A 10 | head -80Repository: sweetmantech/In-Process-API
Length of output: 3849
🏁 Script executed:
rg -t ts 'use step' --type-add 'ts:include:*.ts' -B 0 | wc -l && \
find src/workflows -name '*.ts' -not -path '*/steps/*' | head -5Repository: sweetmantech/In-Process-API
Length of output: 188
🏁 Script executed:
cat -n src/workflows/downloadVideoWorkflow.ts | head -60Repository: sweetmantech/In-Process-API
Length of output: 142
🏁 Script executed:
fd -t f '\.ts$' src/workflows/ | grep -v stepsRepository: sweetmantech/In-Process-API
Length of output: 108
🏁 Script executed:
cat -n src/workflows/migrateMuxToArweave.tsRepository: sweetmantech/In-Process-API
Length of output: 2299
🏁 Script executed:
cat -n package.json | grep -A 5 -B 5 workflowRepository: sweetmantech/In-Process-API
Length of output: 411
🏁 Script executed:
web_search
Vercel Workflow "use step" promise lifecycle behaviorRepository: sweetmantech/In-Process-API
Length of output: 167
🌐 Web query:
Vercel Workflow SDK "use step" detached promises lifecycle
💡 Result:
In the Vercel/Workflow SDK, the lifecycle of a function marked with "use step" is not a “normal” JS promise lifecycle (where a promise you create continues to resolve in the same process). Instead, the workflow runtime treats step handlers as durable, replayable units whose results are cached and whose execution/attempts are controlled by the workflow engine. 1) What "use step" actually means for execution - Steps run as separate, retryable units with full Node.js runtime access; their results are persisted for replay [1]. - The workflow runtime can replay workflow code from the beginning, using the event log and cached step results to resume deterministically [1]. - The compiler/runtime hook for "use step" checks whether the step already ran (from the event log). If yes, it returns the cached result; if no, it triggers suspension/enqueues the step for background execution [2]. 2) How “detached promises” behave inside a step - The docs you provided don’t describe a special “detached promise” API or semantic where an un-awaited promise continues independently and later “finishes” within the same attempt. - What is clear is that the workflow engine’s durable correctness is driven by what the workflow/step returns (and by durably supported wait primitives like awaiting a webhook or using sleep). For example, the workflow suspends while waiting on asynchronous work, such as awaiting a promise returned by createWebhook (external resumption), and it resumes when the webhook is resumed [1]. - By contrast, the SDK explicitly states that calling a step outside of a workflow (or calling in a way that makes it run like a normal function) removes retry/observability and changes behavior: “calling a step from outside a workflow … is a no-op” and “outside of a workflow function will not have retry semantics, nor will it be observable” [1]. So, if by “detached promises” you mean “starting async work and not awaiting it,” the durable guarantees are tied to the awaited/returned values and to explicit durably-supported waiting mechanisms; any work not represented in the returned/awaited control flow is not something the workflow runtime can reliably persist/replay. 3) Retry semantics for steps (how attempts are managed) - By default, steps retry up to 3 attempts on errors; you can customize retry behavior via maxRetries (and use FatalError / RetryableError to control whether/when to retry) [3][1]. - When a step fails with RetryableError, the runtime retries the step (optionally after retryAfter) [3][4]. 4) Practical guidance inferred from the above semantics - Put the side effects and the “wait for completion” inside the step function and await them so that the workflow engine has a durable representation of the work result (cached output / logged events) [2][1]. - If you need to wait for external events, prefer the SDK-supported mechanism: createWebhook + await the webhook promise, or use durable sleep, both of which are explicitly described as suspending/resuming the workflow [1]. - If you call step logic outside workflow context, do not expect the step directive to provide retry/observability; it runs like a normal function instead [1]. If you share a minimal code snippet of your “detached promises” pattern (e.g., Promise.all with missing awaits, void async IIFEs, event handlers, etc.), I can map it more precisely to what the runtime will and won’t be able to persist/replay, based on the SDK’s directive semantics and the supported suspension mechanisms.
Citations:
- 1: https://workflow-sdk.dev/docs/foundations/workflows-and-steps
- 2: https://workflow-sdk.dev/docs/how-it-works/code-transform
- 3: https://workflow-sdk.dev/docs/foundations/errors-and-retries
- 4: https://workflow-sdk.dev/docs/api-reference/workflow/retryable-error
🏁 Script executed:
rg -t ts 'then\(' src/workflows/steps/ -A 3 | grep -v BinaryRepository: sweetmantech/In-Process-API
Length of output: 399
🏁 Script executed:
rg -t ts 'catch\(' src/workflows/steps/ -B 5Repository: sweetmantech/In-Process-API
Length of output: 574
Await the audit write inside the step.
Lines 17-29 detach the pricing/insert chain from the step lifecycle. In Vercel Workflow steps, detached promises are not part of the durable execution model—the runtime cannot reliably persist or replay work that is not awaited. Once this step returns on line 31, the insertArweaveUpload call may not complete, making cost tracking nondeterministic.
🐛 Suggested change
- unauthTurboClient
- .getTokenPriceForBytes({ byteCount: readyFile.size })
- .then(({ tokenPrice }) =>
- insertArweaveUpload({
- arweave_uri: result.arweave_uri,
- winc_cost: result.winc_cost,
- usdc_cost: Number(tokenPrice),
- file_size_bytes: readyFile.size,
- content_type: readyFile.type || 'video/mp4',
- artist_address: artistAddress,
- })
- )
- .catch((e: unknown) => console.error('logArweaveUpload:', e));
+ try {
+ const { tokenPrice } = await unauthTurboClient.getTokenPriceForBytes({
+ byteCount: readyFile.size,
+ });
+
+ await insertArweaveUpload({
+ arweave_uri: result.arweave_uri,
+ winc_cost: result.winc_cost,
+ usdc_cost: Number(tokenPrice),
+ file_size_bytes: readyFile.size,
+ content_type: readyFile.type || 'video/mp4',
+ artist_address: artistAddress,
+ });
+ } catch (e: unknown) {
+ console.error('logArweaveUpload:', e);
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflows/steps/downloadAndTranscodeStep.ts` around lines 17 - 29, The
current code detaches
unauthTurboClient.getTokenPriceForBytes(...).then(...insertArweaveUpload...)
which creates an unawaited promise; instead make the step await the audit write
so it becomes part of the step lifecycle: call
unauthTurboClient.getTokenPriceForBytes and await its result (reference:
unauthTurboClient.getTokenPriceForBytes), then await insertArweaveUpload(...)
with the returned tokenPrice (reference: insertArweaveUpload) inside a try/catch
so errors are logged and the step only returns after the insert completes.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor
Tests
Chores