Waitpoint token callback URLs #2025
Conversation
🦋 Changeset detectedLatest commit: d4b7135 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change introduces secure HTTP callback URLs for waitpoint tokens, allowing external systems to complete waitpoints via POST requests. The callback URL is now generated and included in token objects, exposed in API responses, UI tables, and documentation. Supporting logic for URL generation, verification, and handling of callback requests is implemented, with related types and presenter classes updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Database
participant Engine
Client->>API: Request to create waitpoint token
API->>Database: Store waitpoint token and retrieve API key
API->>API: Generate callback URL (with HMAC hash)
API-->>Client: Return token object with callback URL
Note over Client,API: External service can POST to callback URL
participant WebhookService
WebhookService->>API: POST to /api/v1/waitpoints/tokens/{id}/callback/{hash}
API->>Database: Lookup waitpoint and API key
API->>API: Verify hash matches waitpoint ID and API key
API->>Engine: Mark waitpoint as complete with POST body
API-->>WebhookService: Return JSON success response
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts (1)
69-84: 🛠️ Refactor suggestionCursor pagination uses unordered UUID – replace with
createdAtfor deterministic paging
ORDER BY w.id DESCtogether withw.id < ${cursor}assumes theidcolumn is time-ordered, which isn’t true for CUID/UUID. Users can see duplicate / skipped rows when two waitpoints share the same timestamp but unordered ids.Proposed fix (keeps the existing composite index suggested earlier):
- ORDER BY - ${direction === "forward" ? Prisma.sql`w.id DESC` : Prisma.sql`w.id ASC`} - LIMIT ${pageSize + 1}`; + ORDER BY + ${direction === "forward" + ? Prisma.sql`w."createdAt" DESC, w.id DESC` + : Prisma.sql`w."createdAt" ASC, w.id ASC`} + LIMIT ${pageSize + 1}`;Then use the tuple
(createdAt,id)for cursors.
🧹 Nitpick comments (17)
apps/webapp/app/assets/icons/HttpCallbackIcon.tsx (1)
1-22: Extend props and enhance accessibility
Consider extending the component to accept general SVG props (e.g.,aria-label,role, event handlers) by typing its props asReact.SVGProps<SVGSVGElement>and spreading...propsonto the<svg>. Also addaria-hidden="true"by default for decorative icons to improve accessibility and reusability.-export function HttpCallbackIcon({ className }: { className?: string }) { +import type { SVGProps } from "react"; + +export function HttpCallbackIcon({ className, ...props }: SVGProps<SVGSVGElement>) { return ( <svg - className={className} + className={className} + aria-hidden="true" + {...props} xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none"apps/webapp/app/components/navigation/SideMenu.tsx (2)
84-84: Import HTTP callback icon.
Bringing inHttpCallbackIconcomplements the existing icon set. Consider moving this import alongside other icon imports for grouping.
250-255: Add HTTP callbacks menu item.
Integration under "Waitpoints" looks correct. For consistency, change the label to title case ("HTTP Callbacks") and consider adding adata-actionattribute for telemetry.packages/core/src/v3/apiClient/index.ts (1)
794-810: Well-implemented API client method for HTTP callbacksThe
createWaitpointHttpCallbackmethod follows established patterns in the codebase:
- It uses the same parameter structure as similar methods
- It properly handles request options merging
- It correctly types the return value as an ApiPromise
This addition enables the SDK to create HTTP callback waitpoints via the API.
One minor note: the method accepts
CreateWaitpointTokenRequestBodyas its options parameter. While this likely works because both token and HTTP callback waitpoints share similar creation parameters, consider whether a more specific type likeCreateWaitpointHttpCallbackRequestBodymight be clearer in the future.apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts (1)
257-259: Minor optimisation – use.count()instead offindFirst()The sole purpose here is existence checking.
findFirstfetches an entire row;countis marginally cheaper and expresses the intent better:- const firstToken = await this._replica.waitpoint.findFirst({ + const tokenCount = await this._replica.waitpoint.count({ where: { environmentId: environment.id, resolver, }, }); - if (firstToken) { hasAnyTokens = true; } + hasAnyTokens = tokenCount > 0;references/hello-world/src/trigger/waits.ts (2)
155-176: Variable naming can cause confusionInside the task the identifier
predictionnow refers to the waitpoint result (wrapper withok,output, etc.), not the Replicate response returned byreplicate.predictions.create. Readers may assume otherwise.Consider renaming to make intent crystal-clear:
- const prediction = await wait.forHttpCallback<Prediction>(...); + const predictionResult = await wait.forHttpCallback<Prediction>(...);Same for
result2later on.
203-216: Handle timeout path to avoid silent failuresWhen
result.okis false, you log the error but still continue successfully. In production code you may want tothrowso the task run fails and surfaces the timeout upstream.if (!result.ok) { throw result.error; // re-throw so retries / alerts kick in }Up to you whether the example should model that.
docs/wait-for-http-callback.mdx (3)
18-26: Avoid variable shadowing in the example snippetInside the code sample you first declare
const replicate = new Replicate(...)and a few lines later re-declare
export const replicate = task({...})Using the same identifier for two different objects is confusing for readers and would be a compile-time error in a real file. Rename one of them (e.g.
replicateClient/replicateTask) to keep the example self-contained and copy-paste-able.Also applies to: 21-27
37-38: Likely typo: “Any Warhol” → “Andy Warhol”The prompt string reads:
"A painting of a cat by Any Warhol"unless this is deliberate word-play, it should say Andy Warhol.
8-11: Add comma for readability“URL. When the callback URL is requested …”
Insert a comma after requested to match conventional punctuation:
… URL is requested, the run will …Improves the flow and matches the LanguageTool hint.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...webhook) URL. When the callback URL is requested the run will continue where it left off...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
apps/webapp/app/routes/engine.v1.waitpoints.http-callback.ts (2)
52-72:tagsarray is built but never usedYou populate the local
tagsarray with the upserted tag records, yet it is not returned, logged, or passed toengine.createManualWaitpoint. This is probably vestigial from a previous iteration and may confuse future readers.- let tags: { id: string; name: string }[] = []; ... - if (tagRecord) { - tags.push(tagRecord); - }Either remove the accumulation entirely or pass the records where needed.
37-43: Consider using a dedicated request-body schemaThe body is validated with
CreateWaitpointTokenRequestBody, yet the resolver is hard-coded to"HTTP_CALLBACK". A separateCreateWaitpointHttpCallbackRequestBody(which already exists in@trigger.dev/core) would make intent explicit and prevent accidental omission of HTTP-callback-specific fields in the future.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks/route.tsx (2)
127-130: Link to the specific HTTP-callback docs pageThe docs button currently points to
/wait, which lands on the generic waitpoints page. Using the new page improves discoverability.- <LinkButton ... to={docsPath("/wait")}> + <LinkButton ... to={docsPath("/wait-for-http-callback")}>
42-47: Unused import
v3WaitpointTokenPathis imported but not referenced in this file. Consider removing it to keep imports tidy.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks.$waitpointParam/route.tsx (1)
92-98: Redundant nestedcn()call
cnalready returns a string, so wrapping it in anothercnis superfluous and just adds runtime overhead.- <div - className={cn( - cn( - "grid h-full max-h-full grid-rows-[2.5rem_1fr] overflow-hidden bg-background-bright", - waitpoint.status === "WAITING" && "grid-rows-[2.5rem_1fr_auto]" - ) - )} + <div + className={cn( + "grid h-full max-h-full grid-rows-[2.5rem_1fr] overflow-hidden bg-background-bright", + waitpoint.status === "WAITING" && "grid-rows-[2.5rem_1fr_auto]" + )}packages/trigger-sdk/src/v3/wait.ts (2)
381-402: Useless constructor inManualWaitpointPromiseExtending
Promisewithout additional state doesn’t require an explicit constructor – the default one suffices.
Removing it eliminates TS-lint (noUselessConstructor) noise and a few bytes.-class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> { - constructor( - executor: ( - resolve: ( - value: WaitpointTokenTypedResult<TOutput> | PromiseLike<WaitpointTokenTypedResult<TOutput>> - ) => void, - reject: (reason?: any) => void - ) => void - ) { - super(executor); - } +class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> {}🧰 Tools
🪛 Biome (1.9.4)
[error] 382-391: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
603-680:new Promise(async …)antipatternUsing an
asyncexecutor in thePromiseconstructor can lead to unhandled rejections if an exception is thrown after the firstawaitbut beforeresolve/rejectis called.
While the currenttry/catchmitigates most cases, refactoring to anasyncfactory function avoids the pitfall entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
apps/webapp/app/assets/icons/HttpCallbackIcon.tsx(1 hunks)apps/webapp/app/components/BlankStatePanels.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(3 hunks)apps/webapp/app/components/runs/v3/WaitpointDetails.tsx(4 hunks)apps/webapp/app/presenters/v3/ApiWaitpointListPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts(9 hunks)apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks.$waitpointParam/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx(2 hunks)apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.ts(1 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.ts(2 hunks)apps/webapp/app/routes/engine.v1.waitpoints.http-callback.ts(1 hunks)apps/webapp/app/services/apiRateLimit.server.ts(1 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)docs/docs.json(1 hunks)docs/wait-for-http-callback.mdx(1 hunks)docs/wait-for-token.mdx(1 hunks)docs/wait.mdx(1 hunks)internal-packages/database/prisma/migrations/20250502154714_waitpoint_added_resolvers/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250502155009_waitpoint_resolve_index_for_dashboard/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250505164454_waitpoint_resolve_status_index/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250505164720_waitpoint_drop_type_status_index/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250505165445_waitpoint_drop_type_index/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(3 hunks)internal-packages/run-engine/src/engine/index.ts(2 hunks)internal-packages/run-engine/src/engine/systems/waitpointSystem.ts(2 hunks)internal-packages/run-engine/src/engine/tests/checkpoints.test.ts(5 hunks)internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts(11 hunks)internal-packages/run-engine/src/engine/tests/waitpoints.test.ts(8 hunks)packages/core/src/v3/apiClient/index.ts(2 hunks)packages/core/src/v3/schemas/api.ts(1 hunks)packages/core/src/v3/types/index.ts(1 hunks)packages/core/src/v3/types/waitpoints.ts(1 hunks)packages/trigger-sdk/src/v3/wait.ts(4 hunks)references/hello-world/package.json(1 hunks)references/hello-world/src/trigger/waits.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (1)
apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts (1)
WaitpointListPresenter(69-301)
packages/core/src/v3/types/waitpoints.ts (1)
packages/core/src/v3/types/schemas.ts (2)
Schema(74-74)inferSchemaOut(96-99)
apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (1)
packages/core/src/v3/isomorphic/friendlyId.ts (1)
WaitpointId(95-95)
apps/webapp/app/components/BlankStatePanels.tsx (4)
apps/webapp/app/components/primitives/InfoPanel.tsx (1)
InfoPanel(30-68)apps/webapp/app/assets/icons/HttpCallbackIcon.tsx (1)
HttpCallbackIcon(1-22)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton(333-399)apps/webapp/app/utils/pathBuilder.ts (1)
docsPath(453-455)
references/hello-world/src/trigger/waits.ts (1)
packages/trigger-sdk/src/v3/wait.ts (1)
wait(404-836)
🪛 LanguageTool
docs/wait-for-http-callback.mdx
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...webhook) URL. When the callback URL is requested the run will continue where it left off...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[typographical] ~16-~16: It appears that a comma is missing.
Context: ... ## Usage In this example we create an image using Replicate. The...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~138-~138: Possible missing comma found.
Context: ...rn the output of the token. If an error occurs it will throw an error.
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Biome (1.9.4)
packages/trigger-sdk/src/v3/wait.ts
[error] 382-391: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (57)
apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts (1)
6-6: Import path updated correctly
ThewaitpointStatusToApiStatusimport has been updated to the new presenter file. Confirm thatWaitpointListPresenter.server.tsexports this function and remove any residual imports or exports from the old module.apps/webapp/app/services/apiRateLimit.server.ts (1)
62-62: Ensure rate-limit bypass is safe for callback endpoint
The HTTP callback route is whitelisted and bypasses rate limiting. Verify that the corresponding handler enforces proper validation (e.g., a secret token or signature) to prevent abuse by unauthenticated callers.docs/docs.json (1)
50-56: Verify documentation page existence and naming
A new navigation entry for"wait-for-http-callback"was added. Confirm that the corresponding documentation file exists (e.g.,wait-for-http-callback.md) and matches naming conventions so it renders correctly in the site.internal-packages/database/prisma/migrations/20250505164720_waitpoint_drop_type_status_index/migration.sql (1)
1-2: Concurrent drop of obsolete index
Dropping theWaitpoint_environmentId_type_status_idxconcurrently is correct to avoid table locks. TheIF EXISTSguard makes this idempotent.internal-packages/database/prisma/migrations/20250505165445_waitpoint_drop_type_index/migration.sql (1)
1-1: Correctly drop deprecated index concurrently.
DroppingWaitpoint_environmentId_type_createdAt_idxaligns the schema with the newresolver-based filters. UsingCONCURRENTLY IF EXISTSavoids downtime by preventing table locks.references/hello-world/package.json (1)
9-13: Add runtime dependencies for callback example.
Dependencies"openai","replicate", and"zod"support the newwaitHttpCallbackSDK usage and input schema validation in the Hello World sample.internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts (1)
105-105: Addresolverparameter tocreateManualWaitpointcalls.
All test invocations now explicitly setresolver: "TOKEN"to conform with the updated method signature.Also applies to: 158-158, 307-307, 361-361, 497-497, 677-677, 837-837, 1023-1023, 1197-1197, 1352-1352
internal-packages/database/prisma/migrations/20250505164454_waitpoint_resolve_status_index/migration.sql (1)
1-7: Add concurrent index for resolver & status.
CreatingWaitpoint_environmentId_resolver_status_createdAt_idxwithCONCURRENTLY IF NOT EXISTSoptimizes queries filtering by environment, resolver, and status, ordered by creation time.apps/webapp/app/components/navigation/SideMenu.tsx (1)
57-57: Import new HTTP callback route builder.
Thev3WaitpointHttpCallbacksPathis required for the new menu link. Ensure the path builder exports match your route definitions.packages/core/src/v3/types/index.ts (1)
10-10: Export added for new waitpoints functionality.This change correctly exports the waitpoint types needed for the new HTTP callback functionality. This aligns with the PR objective of enabling workflows to pause until an HTTP request is received at a generated callback URL.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (2)
39-39: Updated import to use the generalized WaitpointListPresenter.This change appropriately updates the import to use the resolver-aware
WaitpointListPresenterinstead of the token-specific presenter, supporting the new resolver-based architecture.
87-92: Updated presenter instantiation with resolver type.Good implementation of the resolver pattern. The
resolver: "TOKEN"parameter properly identifies this as a token-based waitpoint, allowing the same presenter to be used for different resolver types including the new HTTP callbacks.docs/wait.mdx (3)
7-7: Fixed import statement.Added missing semicolon to the import statement, improving code consistency.
9-9: Corrected terminology.Fixed the wording from "scheduled" to "schedule", improving clarity.
13-18: Improved table formatting and added new HTTP callback functionality.The table formatting has been improved with better column widths and clearer descriptions. The new
wait.forHttpCallback()function has been properly documented, clearly explaining its purpose.internal-packages/database/prisma/migrations/20250502155009_waitpoint_resolve_index_for_dashboard/migration.sql (1)
1-2:❓ Verification inconclusive
Added concurrent index for improved query performance.
This migration creates a concurrent index on the Waitpoint table covering
environmentId,resolver, andcreatedAtcolumns, which will optimize dashboard queries. UsingCREATE INDEX CONCURRENTLYis the correct approach to avoid locking issues in production, aligning with the PR objective of safely implementing database changes.
🏁 Script executed:
#!/bin/bash # Verify this migration isn't duplicating existing indexes # Search for existing similar indexes on the Waitpoint table rg -A 3 "CREATE.*INDEX.*Waitpoint.*environmentId.*resolver" --glob="*.sql"Length of output: 922
Confirm index necessity to avoid duplication
This migration adds a concurrent index on (
environmentId,resolver,createdAt), but there’s an existing index Waitpoint_environmentId_resolver_status_createdAt_idx on (environmentId,resolver,status,createdAt). Since the existing index already prefixes on the same columns, please confirm that the new index is required for queries that don’t filter bystatus, or consider consolidating/dropping one to avoid redundancy.• internal-packages/database/prisma/migrations/20250505164454_waitpoint_resolve_status_index/migration.sql
• internal-packages/database/prisma/migrations/20250502155009_waitpoint_resolve_index_for_dashboard/migration.sqlinternal-packages/run-engine/src/engine/tests/checkpoints.test.ts (1)
94-94: Correctly updated test cases with the new required resolver parameter.The changes properly adapt all test cases to include the new required
resolver: "TOKEN"parameter in calls toengine.createManualWaitpoint(). This ensures the tests remain compatible with the updated method signature while maintaining the original test semantics.Also applies to: 353-353, 404-404, 557-557
docs/wait-for-token.mdx (1)
273-283: Great documentation for the new unwrap() method.The added documentation clearly explains the purpose and benefits of the new
.unwrap()method, providing a concise example that demonstrates how it simplifies error handling. This is a valuable addition that will help developers write cleaner code.packages/core/src/v3/schemas/api.ts (1)
1013-1020: Well-structured schema for HTTP callback waitpoint responses.The new
CreateWaitpointHttpCallbackResponseBodyschema properly defines the response structure for creating HTTP callback waitpoints, including the essentialurlfield that external services will use to deliver results back to the system. The schema follows the existing patterns in the codebase and provides proper type safety.apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (1)
9-12: Successfully adapted API route to the new resolver-based waitpoint system.The changes correctly update this token-specific API endpoint to work with the more generic waitpoint system that supports multiple resolver types. The modifications include:
- Using the more generic presenter and search params classes
- Adding the explicit
"TOKEN"resolver parameter to maintain the original functionality- Properly updating type imports
- Adding the required resolver parameter to the engine call
These changes ensure the token API continues to function correctly while enabling the system to support additional waitpoint types like HTTP callbacks.
Also applies to: 24-25, 28-29, 78-78
internal-packages/run-engine/src/engine/tests/waitpoints.test.ts (6)
348-349: Implementation correctly adds required resolver parameter.The addition of
resolver: "TOKEN"aligns with the updated method signature in theWaitpointSystemthat now requires explicitly specifying the resolver type. This change supports the new HTTP callbacks feature while maintaining backward compatibility for existing token-based waitpoints.
489-490: Implementation correctly includes resolver parameter for timeout test case.The resolver parameter is consistently added to the waitpoint creation with timeout, maintaining the same pattern across test files.
612-613: Resolver parameter correctly added to batch test cases.The addition of the resolver parameter to waitpoints used in concurrency testing ensures all test cases properly specify the resolver type.
907-908: Idempotency test case properly updated with resolver parameter.The resolver parameter is correctly added to the waitpoint with idempotency key test, ensuring all test scenarios continue to function with the new resolver requirement.
1060-1061: TTL idempotency test cases consistently updated.Both waitpoint creation calls in the TTL idempotency test are properly updated with the resolver parameter, maintaining consistency across the test suite.
Also applies to: 1071-1072
1227-1228: Concurrency test case updated with resolver parameter.The addition of the resolver parameter to the concurrency test case ensures the test continues to function with the updated method signature.
internal-packages/database/prisma/migrations/20250502154714_waitpoint_added_resolvers/migration.sql (1)
1-6: Migration correctly implements waitpoint resolver support.This migration properly creates the necessary database schema changes to support multiple waitpoint resolver types:
- Creates a new enum type with the three resolver values needed (ENGINE, TOKEN, HTTP_CALLBACK)
- Adds a non-nullable column with a default value, which is a safe operation that won't lock tables during migration
The approach of using an enum type ensures type safety at the database level and aligns well with the PR objective of supporting HTTP callbacks as a new resolver type.
apps/webapp/app/components/BlankStatePanels.tsx (2)
39-39: Appropriate icon import for HTTP callbacks.The HttpCallbackIcon import is correctly added to support the new HTTP callbacks UI component.
435-461: Well-designed blank state component for HTTP callbacks.This component follows the established pattern for blank state panels while providing clear explanation of HTTP callbacks:
- Uses consistent styling with other blank state panels
- Provides concise explanation of the HTTP callback feature
- Includes appropriate documentation link
- Uses distinct teal color theme to differentiate from token waitpoints
The explanatory text clearly communicates the purpose and use case for HTTP callbacks, helping users understand when to use this feature.
internal-packages/run-engine/src/engine/systems/waitpointSystem.ts (2)
232-233: Properly typed resolver parameter added to method signature.The addition of the required
resolverparameter with proper typing ("TOKEN" | "HTTP_CALLBACK") ensures type safety and makes the API contract explicit. This change is foundational for supporting different waitpoint resolution mechanisms.Also applies to: 240-241
290-291: Resolver parameter correctly included in database operation.The resolver parameter is properly included in the Prisma upsert operation when creating the waitpoint, ensuring the resolver type is stored in the database. This change aligns with the database migration that added the resolver column.
internal-packages/run-engine/src/engine/index.ts (2)
836-853: Good implementation of the resolver parameter!The addition of the
resolverparameter to thecreateManualWaitpointmethod appropriately distinguishes between different waitpoint resolution mechanisms. This change aligns well with the new HTTP callback functionality described in the PR objectives.
854-863: Method implementation correctly passes the resolver parameterThe resolver parameter is properly passed through to the waitpoint system, ensuring consistent behavior throughout the codebase.
apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (6)
10-14: Path builder imports support both resolver typesThe import of both token and HTTP callback path builders enables proper navigation for different waitpoint types.
18-22: Good UI component imports for new functionalityThe additional imports for
CopyableText,ClipboardField,WaitpointResolver, and icons provide the necessary UI elements for displaying and interacting with HTTP callback waitpoints.
51-59: Well-implemented conditional routing based on resolver typeThe ternary operator effectively routes to the appropriate path based on the waitpoint resolver type, ensuring users navigate to the correct detail view.
68-73: Good addition of the resolver type displayThe addition of a dedicated property item for showing the waitpoint resolver type improves clarity for users.
74-81: Well-implemented conditional display of callback URLThe conditional rendering of the callback URL with a copy function provides excellent UX for HTTP callback waitpoints, allowing users to easily copy the URL for use with external services.
161-178: Great implementation of the WaitpointResolverCombo componentThis component effectively displays different resolver types with appropriate icons, enhancing visual distinction between TOKEN and HTTP_CALLBACK waitpoints. The consistent styling with existing components maintains UI coherence.
packages/core/src/v3/types/waitpoints.ts (1)
1-15: Clear type definitions for HTTP callbacksThe types defined here provide a solid foundation for HTTP callback waitpoints:
HttpCallbackSchemareuses the existingSchematype, maintaining consistencyHttpCallbackResultTypeFromSchemaprovides convenient type inference for outputHttpCallbackResultuses a clear discriminated union pattern for success/error handlingThese types align well with the overall system design and will provide good type safety for SDK users implementing HTTP callbacks.
apps/webapp/app/utils/pathBuilder.ts (2)
345-358: LGTM: Well-structured path function for HTTP callbacks listingThe
v3WaitpointHttpCallbacksPathfunction follows the established pattern of other path builder functions in this file, correctly handling filter parameters and using the existing utilities likeobjectToSearchParams.
360-370: LGTM: Consistent implementation for specific HTTP callback pathThe implementation properly builds on the list path function and adds the token ID parameter, maintaining consistency with other similar path builder functions like
v3WaitpointTokenPath.apps/webapp/app/presenters/v3/ApiWaitpointListPresenter.server.ts (3)
1-7: LGTM: Updated imports for the new resolver type supportThe imports have been correctly updated to include
WaitpointResolverfrom the database package and reference the renamed presenter.
9-60: LGTM: No changes to search parameter handlingThis section maintains the existing search parameter handling logic, which is appropriate since the resolver type filtering is handled separately.
62-132: LGTM: Added resolver parameter to support filtering by resolver typeThe
resolverparameter has been correctly added to the call method and is properly passed to theWaitpointListPresenter, enabling filtering waitpoints by their resolution mechanism.apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (4)
5-7: LGTM: Updated imports to reflect renamed modules and added WaitpointIdThe import changes correctly adapt to the renamed
WaitpointListPresenterand add the necessaryWaitpointIdutility for generating callback URLs.
40-40: LGTM: Added resolver field to database queryThe
resolverfield has been appropriately added to the database query to retrieve the resolver type information.
89-93: LGTM: Added resolver field and conditional callback URL to the returned objectThe implementation correctly includes the resolver type in the returned object and conditionally adds the callback URL only when the resolver is of type "HTTP_CALLBACK".
112-116: LGTM: Well-implemented callback URL generator functionThe
generateWaitpointCallbackUrlfunction properly handles environment configuration to generate the full callback URL, using either API_ORIGIN or APP_ORIGIN as the base URL.apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.ts (6)
13-15: LGTM: Clear parameter validation schemaThe params schema correctly validates the waitpoint friendly ID parameter from the route.
17-24: LGTM: Proper HTTP method validation and ID conversionThe code correctly validates that only POST requests are allowed and appropriately converts the friendly ID to an internal ID format.
25-41: LGTM: Comprehensive waitpoint existence and status checksThe implementation properly verifies that the waitpoint exists and handles the case where it's already completed, returning appropriate HTTP status codes.
43-49: LGTM: Robust request body handling and serializationThe code appropriately processes the JSON request body and prepares it for storage, including potential packet export.
51-63: LGTM: Proper waitpoint completion with engine integrationThe implementation correctly completes the waitpoint using the engine's API and returns a standardized success response.
64-67: LGTM: Appropriate error handlingThe code properly catches any errors during processing, logs them, and returns a structured error response with the correct HTTP status code.
apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts (1)
270-271: Callback URL uses internalid, UI routes expectfriendlyId
generateWaitpointCallbackUrl(token.id)passes the DBid. Most front-end & API routes (e.g./api/v1/waitpoints/http-callback/:waitpointFriendlyId/callback) are keyed byfriendlyId. Using the raw UUID will break deep-links and public-facing callbacks.- callbackUrl: generateWaitpointCallbackUrl(token.id), + callbackUrl: generateWaitpointCallbackUrl(token.friendlyId),Please verify the helper’s implementation – if it already expects a UUID this change will require adjusting route params correspondingly.
references/hello-world/src/trigger/waits.ts (1)
169-170: Timeout string exceeds Replicate SLA?
timeout: "10m"is generous. Replicate predictions for Stable-Diffusion-XL tend to finish in ~1-3 min; longer timeouts hold queue capacity longer. If this is an example, maybe reduce to"3m"to encourage best-practice.
...s.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks/route.tsx
Outdated
Show resolved
Hide resolved
...lug.projects.$projectParam.env.$envParam.waitpoints.http-callbacks.$waitpointParam/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.$hash.ts (1)
87-90: Improve error handling to preserve intentional Response objects.The current implementation treats all errors as unexpected and converts them to 500 responses, but should preserve any intentional Response objects that might be thrown.
- logger.error("Failed to complete HTTP callback", { error }); - throw json({ error: "Failed to complete HTTP callback" }, { status: 500 }); + if (error instanceof Response) { + // Forward expected Remix responses untouched + throw error; + } + + logger.error("Failed to complete HTTP callback", { error }); + throw json({ error: "Failed to complete HTTP callback" }, { status: 500 });
🧹 Nitpick comments (2)
apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.$hash.ts (2)
65-66: Consider adding schema validation for the request body.While the code gracefully handles JSON parsing errors, it doesn't validate that the request body matches an expected schema. Consider adding a Zod schema to validate the structure of the request body for more robust error handling.
// If the request body is not valid JSON, return an empty object const body = await request.json().catch(() => ({})); + +// Define a schema for the expected body structure +const bodySchema = z.object({ + // Add expected fields here +}).partial().default({}); + +// Validate and transform the body using the schema +const validatedBody = bodySchema.parse(body);
74-79: Consider checking and handling the result of the engine.completeWaitpoint call.The result of
engine.completeWaitpoint()is stored but never checked. It would be more robust to verify the completion was successful and handle any potential errors.const result = await engine.completeWaitpoint({ id: waitpointId, output: finalData.data ? { type: finalData.dataType, value: finalData.data, isError: false } : undefined, }); + +// Verify the waitpoint was completed successfully +if (!result.success) { + logger.warn("Failed to complete waitpoint", { waitpointId, result }); + return json({ error: "Failed to complete waitpoint" }, { status: 500 }); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.$hash.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/webapp/app/routes/api.v1.waitpoints.http-callback.$waitpointFriendlyId.callback.$hash.ts (6)
1-14: Imports look good and cover all required dependencies.The imports are well-structured and include all necessary dependencies for handling Remix actions, processing waitpoint data, database access, and security verification.
15-18: Parameter validation is properly implemented.Good use of Zod schema to validate route parameters, ensuring type safety and preventing invalid inputs.
20-33: HTTP method and content size validation are well-implemented.The endpoint correctly:
- Restricts to POST method with proper 405 response and Allow header
- Enforces Content-Length header presence (411 response if missing)
- Limits request body size based on environment configuration (413 response if too large)
These are good security practices for preventing abuse of the callback endpoint.
34-57: Security and error handling look good in this section.The implementation:
- Properly parses and validates route parameters
- Correctly fetches the waitpoint from a replica database (good for read performance)
- Returns appropriate 404/401 errors for missing waitpoints or invalid hashes
- Uses
return json()instead ofthrow json()as recommended in previous reviews
59-66: Waitpoint status check and body handling looks good.The code properly:
- Returns early if the waitpoint is already completed, preventing duplicate processing
- Gracefully handles invalid JSON in the request body by falling back to an empty object
68-86: Waitpoint data processing and completion is implemented correctly.The code:
- Properly processes and exports the request body data
- Correctly calls the engine to complete the waitpoint
- Returns an appropriate success response
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
51-52:⚠️ Potential issueFix unreachable code in the existing component.
This line appears to be a statement that doesn't return anything, making it unreachable code after the return statements above. This should be fixed to correctly return the icon.
- <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; + return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
♻️ Duplicate comments (2)
packages/trigger-sdk/src/v3/wait.ts (2)
480-501: Fix potential undefined access inunwrap()methodThe
unwrap()method assumesresult.error.messageis always defined whenok === false, which could cause an error ifresult.erroris undefined or doesn't have a message property.Apply this defensive approach to avoid potential errors:
unwrap(): Promise<TOutput> { return this.then((result) => { if (result.ok) { return result.output; } else { - throw new WaitpointTimeoutError(result.error.message); + const message = + typeof result.error?.message === "string" + ? result.error.message + : "Waitpoint timed out"; + throw new WaitpointTimeoutError(message); } }); }🧰 Tools
🪛 Biome (1.9.4)
[error] 481-490: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
742-743: Same undefined-access risk for HTTP callback pathSimilar to the issue in
unwrap(), this code assumesdata.messagewill always be defined.Apply the same defensive approach:
- const error = new WaitpointTimeoutError(data.message); + const error = new WaitpointTimeoutError( + typeof data?.message === "string" ? data.message : "Waitpoint timed out" + );
🧹 Nitpick comments (3)
apps/webapp/app/components/runs/v3/RunIcon.tsx (2)
83-84: Consider using consistent colors for all waitpoint types.The
wait-tokencase uses sky-500 color which differs from the teal-500 used by other wait types. For better visual consistency, consider using the same color for all waitpoint types.- return <WaitpointTokenIcon className={cn(className, "text-sky-500")} />; + return <WaitpointTokenIcon className={cn(className, "text-teal-500")} />;
84-85: Consider consolidating function-related cases.The new
functioncase is separated from related function hook cases (lines 101-112). Consider consolidating this with the existing function-related cases for better code organization.packages/trigger-sdk/src/v3/wait.ts (1)
480-490: Remove unnecessary constructorThe constructor is redundant as it simply passes arguments to the parent class without additional logic.
You can remove the constructor entirely:
class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> { - constructor( - executor: ( - resolve: ( - value: WaitpointTokenTypedResult<TOutput> | PromiseLike<WaitpointTokenTypedResult<TOutput>> - ) => void, - reject: (reason?: any) => void - ) => void - ) { - super(executor); - } unwrap(): Promise<TOutput> { // ... } }🧰 Tools
🪛 Biome (1.9.4)
[error] 481-490: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/curvy-dogs-share.md(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(2 hunks)packages/trigger-sdk/src/v3/wait.ts(5 hunks)references/hello-world/src/trigger/waits.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/curvy-dogs-share.md
🚧 Files skipped from review as they are similar to previous changes (1)
- references/hello-world/src/trigger/waits.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/webapp/app/components/runs/v3/RunIcon.tsx (3)
apps/webapp/app/assets/icons/HttpCallbackIcon.tsx (1)
HttpCallbackIcon(1-22)references/d3-chat/src/lib/utils.ts (1)
cn(4-6)apps/webapp/app/assets/icons/WaitpointTokenIcon.tsx (1)
WaitpointTokenIcon(1-12)
🪛 Biome (1.9.4)
packages/trigger-sdk/src/v3/wait.ts
[error] 481-490: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/webapp/app/components/runs/v3/RunIcon.tsx (2)
22-23: New icon imports align with feature requirements.The new imports for HttpCallbackIcon and WaitpointTokenIcon support the HTTP callback waitpoints feature described in the PR objectives.
80-82: HTTP callback waitpoint icon uses consistent styling.The
wait-http-callbackcase correctly uses the teal-500 color, matching the existing "wait" case styling for visual consistency.packages/trigger-sdk/src/v3/wait.ts (3)
84-182: Well-implemented HTTP callback functionality with comprehensive documentation!The new
createHttpCallbackfunction is well-designed, providing a clean API for integrating with external services that use webhooks. The tracing spans will ensure good observability, and the implementation follows established patterns in the codebase.
682-779: Nice refactor offorTokento utilize the new promise classThe refactoring of
forTokento return aManualWaitpointPromiseprovides a better developer experience with the.unwrap()method while maintaining the original functionality. The implementation includes proper error handling and tracing.
780-780: Correctly exposed new functionality in the exported APIGood job exporting the new
createHttpCallbackfunction through thewaitobject, making it available for users of the SDK.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docs/wait-for-http-callback.mdx (4)
16-16: Typographical: Add missing comma for clarity.It appears a comma is missing after "In this example". Update as follows:
- In this example we create an image using Replicate. + In this example, we create an image using Replicate.🧰 Tools
🪛 LanguageTool
[typographical] ~16-~16: It appears that a comma is missing.
Context: ... ## Usage In this example we create an image using Replicate. The...(DURING_THAT_TIME_COMMA)
17-17: Use code formatting for technical terms.Replace curly quotes around “webhook” with backticks for consistency in markdown:
- Their API accepts a “webhook”, which is an HTTP POST callback. + Their API accepts a `webhook`, which is an HTTP POST callback.
69-69: Enhance heading formatting for code clarity.Consider wrapping method names in code formatting for consistency with other headings:
- ## unwrap() + ## `unwrap()`
103-103: Consistent heading capitalization.Change "### returns" to title case to match other section headings:
- ### returns + ### Returns
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/wait-for-http-callback.mdx(1 hunks)docs/wait.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/wait.mdx
🧰 Additional context used
🪛 LanguageTool
docs/wait-for-http-callback.mdx
[typographical] ~16-~16: It appears that a comma is missing.
Context: ... ## Usage In this example we create an image using Replicate. The...
(DURING_THAT_TIME_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
docs/wait-for-http-callback.mdx
Outdated
| The `wait.createHttpCallback()` function gives you a callback URL and returns a token representing the waitpoint. You should call a third-party API and provide it with the callback URL so they can notify you when their work is done. | ||
|
|
||
| Then you can use `wait.forToken()` to pause the run until the callback URL is hit with a `POST` request. | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify function naming consistency.
The documentation references wait.createHttpCallback(), but the PR title mentions wait.forHttpCallback(). Please ensure the function name in the docs matches the actual SDK method.
Run the following script to verify available methods in the SDK:
🏁 Script executed:
#!/bin/bash
# Verify function names in SDK wait implementation
rg -n "createHttpCallback" -n packages/trigger-sdk/src/v3/wait.ts
rg -n "forHttpCallback" -n packages/trigger-sdk/src/v3/wait.tsLength of output: 309
Update PR title to use the correct SDK method name
The SDK only exposes wait.createHttpCallback(...)—there is no wait.forHttpCallback. Please update the PR title (and any other references) to use createHttpCallback for consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/trigger-sdk/src/v3/wait.ts (1)
419-427:unwrap()may throw with an undefined message
Same issue flagged previously: ifresult.error?.messageis undefined, the thrownWaitpointTimeoutErrorwill have an empty message, orresult.erroritself may beundefined.Consider the earlier suggested guard:
- throw new WaitpointTimeoutError(result.error.message); + const msg = + typeof result.error?.message === "string" + ? result.error.message + : "Waitpoint timed out"; + throw new WaitpointTimeoutError(msg);
🧹 Nitpick comments (2)
apps/webapp/app/presenters/v3/ApiWaitpointListPresenter.server.ts (1)
1-2: Remove unused import
WaitpointResolverisn’t referenced in this file, so the import is dead code and may trip linting rules.-import { type RunEngineVersion, type WaitpointResolver } from "@trigger.dev/database"; +import { type RunEngineVersion } from "@trigger.dev/database";packages/trigger-sdk/src/v3/wait.ts (1)
407-417: Useless constructor – simplify the subclassThe constructor does nothing beyond delegating to
super, triggering Biome’snoUselessConstructorwarning. Removing it keeps the class lean.-class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> { - constructor( - executor: ( - resolve: ( - value: WaitpointTokenTypedResult<TOutput> | PromiseLike<WaitpointTokenTypedResult<TOutput>> - ) => void, - reject: (reason?: any) => void - ) => void - ) { - super(executor); - } +class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> {}🧰 Tools
🪛 Biome (1.9.4)
[error] 408-417: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/webapp/app/components/navigation/SideMenu.tsx(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(2 hunks)apps/webapp/app/components/runs/v3/WaitpointDetails.tsx(2 hunks)apps/webapp/app/presenters/v3/ApiWaitpointListPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts(7 hunks)apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx(5 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.callback.$hash.ts(1 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.ts(2 hunks)apps/webapp/app/services/apiRateLimit.server.ts(1 hunks)apps/webapp/app/services/httpCallback.server.ts(1 hunks)docs/wait.mdx(1 hunks)packages/core/src/v3/schemas/api.ts(2 hunks)packages/trigger-sdk/src/v3/wait.ts(6 hunks)references/hello-world/src/trigger/waits.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/webapp/app/services/apiRateLimit.server.ts
- apps/webapp/app/services/httpCallback.server.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- docs/wait.mdx
- apps/webapp/app/routes/api.v1.waitpoints.tokens.ts
- apps/webapp/app/components/runs/v3/RunIcon.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx
- apps/webapp/app/components/navigation/SideMenu.tsx
- apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts
- apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts
- references/hello-world/src/trigger/waits.ts
- packages/core/src/v3/schemas/api.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (1)
apps/webapp/app/components/primitives/ClipboardField.tsx (1)
ClipboardField(73-132)
apps/webapp/app/presenters/v3/ApiWaitpointListPresenter.server.ts (1)
apps/webapp/app/presenters/v3/WaitpointListPresenter.server.ts (2)
WaitpointListOptions(19-41)WaitpointListPresenter(69-300)
🪛 Biome (1.9.4)
packages/trigger-sdk/src/v3/wait.ts
[error] 408-417: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
| <Property.Item> | ||
| <Property.Label>Callback URL</Property.Label> | ||
| <Property.Value className="my-1"> | ||
| <ClipboardField value={waitpoint.callbackUrl} variant={"secondary/small"} /> | ||
| </Property.Value> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against missing callbackUrl & consider masking secret portions
ClipboardFieldrenders whatever string it receives. Ifwaitpoint.callbackUrlisundefined(e.g. resolver ≠HTTP_CALLBACKor the presenter failed) the input will literally show the string"undefined", which is misleading.- The URL contains an HMAC-style secret. Displaying it in full may be undesirable;
ClipboardFieldalready supports asecureprop to mask until focused.
- <Property.Item>
- <Property.Label>Callback URL</Property.Label>
- <Property.Value className="my-1">
- <ClipboardField value={waitpoint.callbackUrl} variant={"secondary/small"} />
- </Property.Value>
- </Property.Item>
+ {waitpoint.callbackUrl ? (
+ <Property.Item>
+ <Property.Label>Callback URL</Property.Label>
+ <Property.Value className="my-1">
+ {/* Mask the URL until the user focuses the field */}
+ <ClipboardField
+ value={waitpoint.callbackUrl}
+ secure
+ variant="secondary/small"
+ />
+ </Property.Value>
+ </Property.Item>
+ ) : null}This prevents accidental leakage and avoids confusing "undefined" renders.
📝 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.
| <Property.Item> | |
| <Property.Label>Callback URL</Property.Label> | |
| <Property.Value className="my-1"> | |
| <ClipboardField value={waitpoint.callbackUrl} variant={"secondary/small"} /> | |
| </Property.Value> | |
| {waitpoint.callbackUrl ? ( | |
| <Property.Item> | |
| <Property.Label>Callback URL</Property.Label> | |
| <Property.Value className="my-1"> | |
| {/* Mask the URL until the user focuses the field */} | |
| <ClipboardField | |
| value={waitpoint.callbackUrl} | |
| secure | |
| variant="secondary/small" | |
| /> | |
| </Property.Value> | |
| </Property.Item> | |
| ) : null} |
apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.callback.$hash.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/trigger-sdk/src/v3/wait.ts (1)
267-271:⚠️ Potential issueAvoid crash when
data.messageis undefined
datamay beundefined, or not contain amessagefield. Safer construction:- error = new WaitpointTimeoutError(data.message); + const msg = + typeof (data as any)?.message === "string" + ? (data as any).message + : "Waitpoint timed out"; + error = new WaitpointTimeoutError(msg);
♻️ Duplicate comments (1)
packages/trigger-sdk/src/v3/wait.ts (1)
421-428: Guard against missingerror.messagein.unwrap()
(See previous review – same issue)
Accessingresult.error.messagecan throw iferrorormessageis undefined. Use a fallback:- throw new WaitpointTimeoutError(result.error.message); + const msg = + typeof result.error?.message === "string" + ? result.error.message + : "Waitpoint timed out"; + throw new WaitpointTimeoutError(msg);
🧹 Nitpick comments (4)
docs/wait-for-token.mdx (3)
57-78: Clarify external-service example & add missingreplicateimportGreat addition – the webhook example makes the callback flow much clearer.
To make the snippet copy-pasteable:
- Add
import Replicate from "replicate";(or the correct client import) so users know wherereplicatecomes from.- Consider adding a short note that the Replicate client must be initialised with an API key before use.
These small tweaks remove guess-work for readers and reduce the chance of “
replicateis not defined” errors when they try the sample.
113-117: Minor punctuation – missing comma before “the output”For readability, insert a comma after “body”:
-If there's no body the output will be an empty object `{}`. +If there's no body, the output will be an empty object `{}`.Applies to both occurrences in the createToken and listTokens property tables.
Also applies to: 373-377
🧰 Tools
🪛 LanguageTool
[uncategorized] ~116-~116: Possible missing comma found.
Context: ... the output of the token. If there's no body the output will be an empty object{}...(AI_HYDRA_LEO_MISSING_COMMA)
305-315: Mentiontry/catcharound.unwrap()for time-out handlingThe
.unwrap()helper is nice, but calling code still needs to handle the thrownWaitpointTimeoutError.
Adding a quick example:try { const prediction = await wait.forToken<Prediction>(token).unwrap(); // happy path } catch (err) { // handle timeout }will reinforce the best-practice pattern.
packages/trigger-sdk/src/v3/wait.ts (1)
410-419: Remove empty constructor – flagged by linter
ManualWaitpointPromisedoesn’t add any logic in its constructor; the default inherited one is sufficient and keeps the class smaller:-class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> { - constructor( - executor: ( - resolve: ( - value: WaitpointTokenTypedResult<TOutput> | PromiseLike<WaitpointTokenTypedResult<TOutput>> - ) => void, - reject: (reason?: any) => void - ) => void - ) { - super(executor); - } +class ManualWaitpointPromise<TOutput> extends Promise<WaitpointTokenTypedResult<TOutput>> {}🧰 Tools
🪛 Biome (1.9.4)
[error] 410-419: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/wait-for-token.mdx(6 hunks)packages/core/src/v3/apiClient/index.ts(2 hunks)packages/core/src/v3/schemas/api.ts(2 hunks)packages/trigger-sdk/src/v3/wait.ts(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/v3/apiClient/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/v3/schemas/api.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/trigger-sdk/src/v3/wait.ts
[error] 410-419: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🪛 LanguageTool
docs/wait-for-token.mdx
[uncategorized] ~116-~116: Possible missing comma found.
Context: ... the output of the token. If there's no body the output will be an empty object {}...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~376-~376: Possible missing comma found.
Context: ... the output of the token. If there's no body the output will be an empty object {}...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
When you create a Waitpoint token we now give you back a URL. You can do an HTTP POST request to it and it will complete the waitpoint with the data you send.
This allows you to start some work on another API (or one of your own services) and continue the run when the callback URL we give you is hit with the result.
In this example we start a Replicate prediction and continue the run when the prediction is complete.
Security
The URL include the waitpoint friendlyId and a HMAC of the waitpoint.id + the environment API key. E.g.
This makes these URLs safe as they're unguessable and they don't leak any private information.
Summary by CodeRabbit
New Features
Bug Fixes
Chores