Waitpoint tokens page, wait.listTokens() and wait.retrieveToken() #1824
Conversation
… Queues page for now
# Conflicts: # apps/webapp/app/components/navigation/SideMenu.tsx # apps/webapp/app/presenters/v3/SpanPresenter.server.ts
# Conflicts: # apps/webapp/app/models/taskRunTag.server.ts # references/hello-world/src/trigger/tags.ts
|
WalkthroughThis pull request introduces several new features and updates related to waitpoint tokens, status, and tagging. The changes span various layers, including UI components, backend presenters, API routes, database migrations, and core SDK methods. New components and hooks have been added for improved waitpoint management and token filtering, while existing functionality has been refactored for better type safety and control flow. Schema and migration modifications support new relationships and tagging functionality across the system. Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 4
🧹 Nitpick comments (32)
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
387-387: Consider documenting the reason for the large right padding.The
pr-16class adds a significant 4rem (64px) of right padding to the tags cell. While this is likely intentional for UI design reasons, it would be helpful to add a comment explaining the reasoning behind such a large padding value to prevent future developers from modifying it without understanding its purpose.apps/webapp/app/models/waitpointTag.server.ts (3)
7-50: Add exponential backoff between retry attempts.The retry logic correctly handles database conflicts, but currently has no delay between attempts. For better resilience and to avoid hammering the database, consider implementing an exponential backoff strategy.
- let attempts = 0; + let attempts = 0; + let backoffMs = 50; // Starting with 50ms delay while (attempts < MAX_RETRIES) { try { return await prisma.waitpointTag.upsert({ // ... existing code }); } catch (error) { if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { // Handle unique constraint violation (conflict) attempts++; + if (attempts < MAX_RETRIES) { + // Exponential backoff with jitter + const jitter = Math.random() * 0.1 * backoffMs; + await new Promise(resolve => setTimeout(resolve, backoffMs + jitter)); + backoffMs *= 2; // Exponential increase for next attempt + } if (attempts >= MAX_RETRIES) {
16-16: Improve early return documentation.The early return for empty tags is a valid optimization, but it's not immediately obvious why this behavior exists. Consider adding a comment to explain the rationale.
- if (tag.trim().length === 0) return; + // Skip empty tags as they're not meaningful + if (tag.trim().length === 0) return;
41-44: Include the tag name in the error message.When throwing an error after maximum retries, including the tag name would make debugging easier.
- throw new Error( - `Failed to create waitpoint tag after ${MAX_RETRIES} attempts due to conflicts.` - ); + throw new Error( + `Failed to create waitpoint tag "${tag}" after ${MAX_RETRIES} attempts due to conflicts.` + );apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.ts (1)
13-13: Clarify the purpose of the dummy findResource functionThe
findResourcefunction is currently implemented as a dummy that always returns 1, with a comment indicating it's not needed. Consider either:
- Removing it if it's genuinely not required
- Implementing it properly if resource finding is needed
- Adding a more detailed comment explaining why this workaround is necessary
This would make the code easier to understand and maintain.
apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts (1)
57-80: Consider refactoring the timeout detection logicThe timeout detection logic is overly simplified and may not accurately detect all timeout cases.
- let isTimeout = false; - if (waitpoint.outputIsError && waitpoint.output) { - isTimeout = true; - } + // Determine if this is a timeout based on output error + const isTimeout = Boolean(waitpoint.outputIsError && waitpoint.output);Also, the
isTimeoutvariable is calculated but never used. Either remove it or use it in the return object if it's intended for future use.references/hello-world/src/trigger/waits.ts (1)
67-76: Consider optimizing duplicate token retrieval.The same token (token.id) is retrieved twice at lines 56 and 74 without any changes in between. While this might be for demonstration purposes, in production code you could optimize by reusing the first retrieval result.
- const retrievedToken2 = await wait.retrieveToken(token.id); - logger.log("Retrieved token2", retrievedToken2); + logger.log("Retrieved token2", retrievedToken);apps/webapp/app/components/primitives/AnimatedNumber.tsx (1)
1-1: Remove unused import.The
useSpringimport is no longer used in the component after your changes but is still being imported.- import { motion, useSpring, useTransform, useMotionValue, animate } from "framer-motion"; + import { motion, useTransform, useMotionValue, animate } from "framer-motion";apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tags.ts (1)
28-36: Add error handling for URI decoding.Consider adding error handling around the
decodeURIComponentcall to handle potential errors if the name parameter contains invalid URI-encoded characters.const search = new URL(request.url).searchParams; const name = search.get("name"); + let decodedName; + if (name) { + try { + decodedName = decodeURIComponent(name); + } catch (error) { + throw new Response("Invalid search parameter", { status: 400 }); + } + } + const presenter = new WaitpointTagListPresenter(); const result = await presenter.call({ environmentId: environment.id, - names: name ? [decodeURIComponent(name)] : undefined, + names: name ? [decodedName] : undefined, });apps/webapp/app/models/taskRunTag.server.ts (1)
15-16: Maintain consistentfriendlyIdusage across attempts.Reusing the same
friendlyIdduring retries ensures the tag retains a stable user-friendly identifier, but it raises the possibility of consecutive conflicts if the record truly collides. Reassessing the need for a stable ID versus using a new ID per retry may be worthwhile if collisions are frequent.apps/webapp/app/components/primitives/CopyableText.tsx (1)
10-21: Handle potential clipboard write errors.The
navigator.clipboard.writeText(value)call returns aPromise. Consider leveraging.then/.catchor usingawaitin a try-catch block to gracefully handle errors (e.g., denied permissions). This prevents silent failures and provides a better user experience.const copy = useCallback( async (e: React.MouseEvent) => { e.preventDefault(); e.stopPropagation(); - navigator.clipboard.writeText(value); + try { + await navigator.clipboard.writeText(value); + setCopied(true); + setTimeout(() => { + setCopied(false); + }, 1500); + } catch (err) { + console.error("Failed to copy text:", err); + } }, [value] );apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (1)
55-61: Ensure large outputs are handled efficiently.Using
prettyPrintPacketto render the output is fine for small to moderate payloads. For extremely large outputs, consider implementing pagination, streaming, or storing partial results to avoid potential performance bottlenecks.apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (1)
121-127: Simplify complex conditional renderingThe nested ternary operators make this section difficult to read and maintain. Consider extracting this logic to a separate function for clarity.
- {waitpoint.status === "WAITING" ? null : waitpoint.status === "TIMED_OUT" ? ( - <></> - ) : waitpoint.output ? ( - <PacketDisplay title="Output" data={waitpoint.output} dataType={waitpoint.outputType} /> - ) : waitpoint.completedAfter ? null : ( - "Completed with no output" - )} + {renderOutputSection(waitpoint)} // Add this function above the return statement: + function renderOutputSection(waitpoint: WaitpointDetail) { + if (waitpoint.status === "WAITING") { + return null; + } + + if (waitpoint.status === "TIMED_OUT") { + return null; + } + + if (waitpoint.output) { + return <PacketDisplay title="Output" data={waitpoint.output} dataType={waitpoint.outputType} />; + } + + if (waitpoint.completedAfter) { + return null; + } + + return "Completed with no output"; + }apps/webapp/app/presenters/v3/ApiWaitpointTokenListPresenter.server.ts (1)
13-62: Validate potential usability issues in filter parameters.The
filter[status]field expects multiple comma-separated statuses, which is valid. However, note that using additional filtering fields (e.g.,filter[idempotencyKey],filter[tags]) might inadvertently complicate query performance if the underlying datastore queries become too fragmented. Ensure that the underlying queries remain efficient.Consider exposing a more conventional search parameter format (e.g., repeated query parameters) or limit the complexity of multi-field filters to maintain clarity and performance.
internal-packages/run-engine/src/engine/systems/waitpointSystem.ts (1)
255-255: Validate tag usage constraints at the call site.Although the consumer likely validates the length and contents of the
tagsarray before callingcreateManualWaitpoint, adding optional server-side validation could provide more robust safeguards against unexpected usage. This helps ensure data integrity when multiple consumers call this method.apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (1)
81-95: Return a descriptive error response.The catch block handles validation errors distinctly from general errors and returns appropriate HTTP status codes, ensuring the client can differentiate the error cause. This clarity is beneficial for client-side debugging and error handling.
You might add optional structured error details to the JSON body to help the client localize exactly which request part is invalid.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx (2)
26-69: Validate user access to WaitpointPresenter outputs.The loader function correctly verifies user and environment existence before invoking
WaitpointPresenter. However, if future enhancements ofWaitpointPresenterperform sensitive operations, ensure additional checks (e.g., verifying the user’s role or permissions) are introduced to guard any extension logic or details that might leak.
71-133: Consider adding unit tests for waitpoint status transitions.The page conditionally renders the
CompleteWaitpointFormifwaitpoint.statusis"WAITING". It'd be valuable to verify that correct UI elements render (or do not render) across different statuses. This ensures robust coverage of the waitpoint lifecycle.Would you like me to propose a basic Jest/RTL test suite for this?
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (3)
321-411: Optimize tag lookups for large data sets.
TagsDropdownusesmatchSorterin the UI and calls thetagsendpoint on every keystroke. If the list of tags grows significantly, consider debouncing requests or implementing a more advanced caching mechanism.
443-529: Add user feedback if partial ID is entered.The
WaitpointIdDropdownenforces an exact 35-character ID withwaitpoint_prefix, and displays an error otherwise. Consider offering user suggestions (e.g., partial matches) or a more explicit message about the supported format for a better user experience.
561-645: Clarify error message for idempotency key length.Currently, if
idempotencyKey.length === 0, the error states it must be at least 1 character. For clarity, you may want to handle a case where the user might type only whitespace or apply more robust trimming.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (3)
53-103: Streamline search parameters.When parsing
searchParamsin the loader, consider defaulting empty arrays or strings toundefinedin one place (e.g., a utility function) to reduce repetitive checks and potential confusion about empty vs. undefined filters.
105-247: Encourage user guidance when no tokens exist.In the page UI, the blank state references
NoWaitpointTokens. If a user may be unaware of how to create or manage waitpoints, consider adding a tips or help link to direct them toward the relevant docs or next steps.
105-247: Consider concurrency or TTL usage.When tokens have
idempotencyKeyExpiresAt, the UI displays a tooltip for expiration. Ensure that relevant code or background tasks handle token cleanup or re-validation once tokens expire to avoid stale data.apps/webapp/app/components/runs/v3/RunTag.tsx (1)
65-79: Consider handling component unmount with useEffect cleanupThe setTimeout to reset the copied state could potentially cause issues if the component unmounts before the timeout completes.
- const copy = useCallback( - (e: React.MouseEvent) => { - e.preventDefault(); - e.stopPropagation(); - navigator.clipboard.writeText(textToCopy); - setCopied(true); - setTimeout(() => { - setCopied(false); - }, 1500); - }, - [textToCopy] - ); + const copy = useCallback( + (e: React.MouseEvent) => { + e.preventDefault(); + e.stopPropagation(); + navigator.clipboard.writeText(textToCopy); + setCopied(true); + }, + [textToCopy] + ); + + useEffect(() => { + if (copied) { + const timeout = setTimeout(() => { + setCopied(false); + }, 1500); + return () => clearTimeout(timeout); + } + }, [copied]);apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (2)
31-49: Consider using a mapping object for icon componentsThe switch statement works well, but as you add more statuses, a mapping object might be more maintainable.
- export function WaitpointStatusIcon({ - status, - className, - }: { - status: WaitpointTokenStatus; - className: string; - }) { - switch (status) { - case "WAITING": - return <Spinner className={cn(waitpointStatusClassNameColor(status), className)} />; - case "TIMED_OUT": - return <TimedOutIcon className={cn(waitpointStatusClassNameColor(status), className)} />; - case "COMPLETED": - return <CheckCircleIcon className={cn(waitpointStatusClassNameColor(status), className)} />; - default: { - assertNever(status); - } - } - } + export function WaitpointStatusIcon({ + status, + className, + }: { + status: WaitpointTokenStatus; + className: string; + }) { + const statusIcons: Record<WaitpointTokenStatus, React.ReactNode> = { + WAITING: <Spinner className={cn(waitpointStatusClassNameColor(status), className)} />, + TIMED_OUT: <TimedOutIcon className={cn(waitpointStatusClassNameColor(status), className)} />, + COMPLETED: <CheckCircleIcon className={cn(waitpointStatusClassNameColor(status), className)} />, + }; + + const icon = statusIcons[status]; + if (!icon) { + assertNever(status); + } + + return icon; + }
51-64: Consider consolidating color and title mappingsSimilar to the icon component, these utility functions could be more maintainable as mapping objects, especially if you expect to add more statuses in the future.
- export function waitpointStatusClassNameColor(status: WaitpointTokenStatus): string { - switch (status) { - case "WAITING": - return "text-blue-500"; - case "TIMED_OUT": - return "text-error"; - case "COMPLETED": { - return "text-success"; - } - default: { - assertNever(status); - } - } - } + const STATUS_COLORS: Record<WaitpointTokenStatus, string> = { + WAITING: "text-blue-500", + TIMED_OUT: "text-error", + COMPLETED: "text-success", + }; + + export function waitpointStatusClassNameColor(status: WaitpointTokenStatus): string { + const color = STATUS_COLORS[status]; + if (!color) { + assertNever(status); + } + return color; + }apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
78-92: Consider adding unit tests or integration tests.This method is core to listing tokens with filtering and pagination. Adding targeted tests will help safeguard against future regressions and ensure the raw SQL logic is correct.
apps/webapp/app/presenters/v3/SpanPresenter.server.ts (1)
459-467: Wrap switch-case declarations in a block to avoid scope leaks.Biom’s lint rule suggests that declarations within a switch case may be accessible in other case clauses. Wrap your case body in braces to limit scope:
switch (span.entity.type) { case "waitpoint": - if (!span.entity.id) { - logger.error(`SpanPresenter: No waitpoint id`, { ... }); - return { ...data, entity: null }; - } - const presenter = new WaitpointPresenter(); - const waitpoint = await presenter.call({...}); - ... + { + if (!span.entity.id) { + logger.error(`SpanPresenter: No waitpoint id`, { ... }); + return { ...data, entity: null }; + } + const presenter = new WaitpointPresenter(); + const waitpoint = await presenter.call({...}); + ... + } break; default: return { ...data, entity: null }; }🧰 Tools
🪛 Biome (1.9.4)
[error] 467-467: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/trigger-sdk/src/v3/wait.ts (1)
239-241: Handle potential undefineddatafor error construction.If
result.outputis missing or malformed,datamay beundefined, causing an error when accessingdata.message. Safely check fordata?.message:-if (result.outputIsError) { - error = new WaitpointTimeoutError(data.message); -} else { +if (result.outputIsError) { + if (!data?.message) { + error = new WaitpointTimeoutError("Waitpoint timed out"); + } else { + error = new WaitpointTimeoutError(data.message); + } +} else { output = data as T; }packages/core/src/v3/apiClient/index.ts (1)
728-749: Improved parameter structure with object destructuringChanging the method signature to use an object parameter with destructuring improves readability and maintainability, especially as the number of parameters grows.
Consider adopting this pattern for other methods with multiple parameters to maintain consistency across the codebase.
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsx (1)
197-216: Inconsistency between id and friendlyId propertiesWhile the
FormWaitpointtype usesid, theCompleteDateTimeWaitpointFormstill expectsfriendlyId. This creates a mapping in the code that could be confusing.Consider updating
CompleteDateTimeWaitpointFormto useidconsistently:function CompleteDateTimeWaitpointForm({ waitpoint, }: { - waitpoint: { friendlyId: string; completedAfter: Date }; + waitpoint: { id: string; completedAfter: Date }; }) { // ... return ( <Form - action={`/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/waitpoints/${waitpoint.friendlyId}/complete`} + action={`/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/waitpoints/${waitpoint.id}/complete`} method="post" // ... > // ... </Form> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
apps/webapp/public/images/logo-banner.pngis excluded by!**/*.pngapps/webapp/public/images/readme/workflow-demo.gifis excluded by!**/*.gifapps/webapp/public/images/templates/github-stars-template-bg.pngis excluded by!**/*.pngapps/webapp/public/images/templates/resend-slack-template-bg.pngis excluded by!**/*.pngapps/webapp/public/images/templates/shopify-template-bg.pngis excluded by!**/*.png
📒 Files selected for processing (55)
.gitignore(1 hunks)apps/webapp/app/assets/icons/StatusIcon.tsx(1 hunks)apps/webapp/app/assets/icons/TriggerIcon.tsx(1 hunks)apps/webapp/app/assets/icons/WaitpointTokenIcon.tsx(1 hunks)apps/webapp/app/components/BlankStatePanels.tsx(2 hunks)apps/webapp/app/components/code/CodeBlock.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(3 hunks)apps/webapp/app/components/primitives/AnimatedNumber.tsx(1 hunks)apps/webapp/app/components/primitives/CopyableText.tsx(1 hunks)apps/webapp/app/components/runs/v3/PacketDisplay.tsx(1 hunks)apps/webapp/app/components/runs/v3/RunFilters.tsx(2 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(2 hunks)apps/webapp/app/components/runs/v3/RunTag.tsx(1 hunks)apps/webapp/app/components/runs/v3/TaskRunsTable.tsx(1 hunks)apps/webapp/app/components/runs/v3/WaitpointDetails.tsx(1 hunks)apps/webapp/app/components/runs/v3/WaitpointStatus.tsx(1 hunks)apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx(1 hunks)apps/webapp/app/hooks/useNewCustomerSubscribed.ts(0 hunks)apps/webapp/app/models/taskRunTag.server.ts(1 hunks)apps/webapp/app/models/waitpointTag.server.ts(1 hunks)apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/ApiWaitpointTokenListPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/DevPresenceStream.server.ts(0 hunks)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/RunListPresenter.server.ts(5 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts(7 hunks)apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/WaitpointTagListPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam/route.tsx(0 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.complete.ts(1 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.ts(1 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.ts(2 hunks)apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts(3 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx(3 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsx(6 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tags.ts(1 hunks)apps/webapp/app/utils/pathBuilder.ts(2 hunks)apps/webapp/app/utils/zod.ts(1 hunks)internal-packages/database/prisma/migrations/20250320105905_waitpoint_add_indexes_for_dashboard/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250320152314_waitpoint_tags/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250320152806_waitpoint_renamed_tags/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250325124348_added_connected_runs_to_waitpoints/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(7 hunks)internal-packages/run-engine/src/engine/index.ts(1 hunks)internal-packages/run-engine/src/engine/systems/waitpointSystem.ts(3 hunks)packages/core/src/v3/apiClient/index.ts(6 hunks)packages/core/src/v3/apiClient/types.ts(2 hunks)packages/core/src/v3/schemas/api.ts(3 hunks)packages/trigger-sdk/src/v3/wait.ts(5 hunks)references/hello-world/src/trigger/waits.ts(2 hunks)
💤 Files with no reviewable changes (3)
- apps/webapp/app/presenters/v3/DevPresenceStream.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam/route.tsx
- apps/webapp/app/hooks/useNewCustomerSubscribed.ts
🧰 Additional context used
🧬 Code Definitions (17)
apps/webapp/app/components/BlankStatePanels.tsx (1)
apps/webapp/app/components/primitives/InfoPanel.tsx (1)
InfoPanel(30-68)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
apps/webapp/app/assets/icons/TriggerIcon.tsx (1)
TriggerIcon(3-5)
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
apps/webapp/app/components/primitives/Table.tsx (1)
TableCell(193-270)
apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (5)
apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (1)
WaitpointDetail(7-7)apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (1)
WaitpointStatusCombo(8-23)apps/webapp/app/utils/pathBuilder.ts (2)
v3WaitpointTokenPath(325-335)v3WaitpointTokensPath(314-323)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsx (1)
ForceTimeout(380-414)apps/webapp/app/components/runs/v3/PacketDisplay.tsx (1)
PacketDisplay(6-51)
apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.ts (2)
apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts (1)
ApiWaitpointPresenter(8-81)packages/core/src/v3/schemas/api.ts (2)
WaitpointRetrieveTokenResponse(973-979)WaitpointRetrieveTokenResponse(980-980)
apps/webapp/app/components/navigation/SideMenu.tsx (4)
apps/webapp/app/components/navigation/SideMenuSection.tsx (1)
SideMenuSection(15-85)apps/webapp/app/components/navigation/SideMenuItem.tsx (1)
SideMenuItem(7-55)apps/webapp/app/assets/icons/WaitpointTokenIcon.tsx (1)
WaitpointTokenIcon(1-12)apps/webapp/app/utils/pathBuilder.ts (1)
v3WaitpointTokensPath(314-323)
packages/core/src/v3/apiClient/types.ts (1)
packages/core/src/v3/schemas/api.ts (2)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)
apps/webapp/app/components/runs/v3/RunFilters.tsx (1)
apps/webapp/app/assets/icons/StatusIcon.tsx (1)
StatusIcon(3-9)
apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (3)
apps/webapp/app/presenters/v3/ApiWaitpointTokenListPresenter.server.ts (2)
ApiWaitpointTokenListSearchParams(13-62)ApiWaitpointTokenListPresenter(66-134)apps/webapp/app/models/waitpointTag.server.ts (2)
MAX_TAGS_PER_WAITPOINT(4-4)createWaitpointTag(7-50)packages/core/src/v3/schemas/api.ts (2)
CreateWaitpointTokenResponseBody(945-948)CreateWaitpointTokenResponseBody(949-949)
apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (1)
packages/core/src/v3/schemas/api.ts (2)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (3)
packages/core/src/v3/schemas/api.ts (3)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)waitpointTokenStatuses(951-951)apps/webapp/app/assets/icons/StatusIcon.tsx (1)
StatusIcon(3-9)apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (2)
waitpointStatusTitle(66-79)WaitpointStatusCombo(8-23)
apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts (2)
packages/core/src/v3/schemas/api.ts (1)
RunEngineVersion(7-7)apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
waitpointStatusToApiStatus(280-290)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (3)
apps/webapp/app/components/runs/v3/RunTag.tsx (1)
RunTag(10-63)apps/webapp/app/utils/pathBuilder.ts (1)
v3RunsPath(225-234)apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (1)
WaitpointDetailTable(15-130)
apps/webapp/app/presenters/v3/ApiWaitpointTokenListPresenter.server.ts (3)
packages/core/src/v3/schemas/api.ts (3)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)RunEngineVersion(7-7)apps/webapp/app/utils/zod.ts (1)
CoercedDate(3-22)apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (2)
WaitpointTokenListOptions(17-38)WaitpointTokenListPresenter(64-278)
apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (2)
packages/core/src/v3/schemas/api.ts (5)
RunEngineVersion(7-7)WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)WaitpointTokenItem(955-965)WaitpointTokenItem(966-966)apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
WaitpointSearchParams(63-63)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsx (1)
packages/core/src/v3/schemas/api.ts (2)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)
packages/core/src/v3/apiClient/index.ts (4)
packages/core/src/v3/apiClient/core.ts (6)
params(479-487)params(528-538)ZodFetchOptions(30-38)CursorPagePromise(459-502)zodfetchCursorPage(79-114)zodfetch(70-77)packages/core/src/v3/apiClient/types.ts (1)
ListWaitpointTokensQueryParams(46-53)packages/core/src/v3/index.ts (1)
CursorPagePromise(4-4)packages/core/src/v3/schemas/api.ts (4)
WaitpointTokenItem(955-965)WaitpointTokenItem(966-966)WaitpointRetrieveTokenResponse(973-979)WaitpointRetrieveTokenResponse(980-980)
🪛 Biome (1.9.4)
apps/webapp/app/presenters/v3/SpanPresenter.server.ts
[error] 467-467: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (107)
apps/webapp/app/assets/icons/StatusIcon.tsx (1)
1-9: Clean and well-structured component implementation.The
StatusIconcomponent follows good practices with a clean implementation:
- Uses the
cnutility for class name composition- Accepts an optional className prop for flexibility
- Implements a visually consistent circular status indicator
This will be useful for showing status visually in the new Waitpoints/Tokens page mentioned in the PR.
.gitignore (1)
63-65: Correctly ignoring source package.json files.These additions properly ignore package.json files in the source directories of the packages, which is consistent with the existing patterns in the file (like line 60 and 62).
This will prevent these automatically generated files from being tracked in version control.
internal-packages/database/prisma/schema.prisma (7)
444-444: Good addition of waitpointTags relation to RuntimeEnvironment.This addition allows environment-specific tagging of waitpoints, supporting the new filtering functionality mentioned in the PR.
510-510: Good addition of waitpointTags relation to Project.This enables project-level organization of waitpoint tags, working in conjunction with the environment-level tags.
1797-1797: Enhanced TaskRun-Waitpoint relationship model.The changes improve the relationship between TaskRuns and Waitpoints:
- Named relation for the completing run
- New
connectedWaitpointsfield to track all waitpoints that have blocked a runThis bidirectional relationship will make it easier to query related waitpoints from a task run.
Also applies to: 1802-1803
2115-2115: Improved Waitpoint-TaskRun relationship tracking.The enhanced relationship will:
- Better track which run completed a waitpoint
- Record all runs connected to a waitpoint for historical reference
This supports the dashboard functionality to view detailed waitpoint activity.
Also applies to: 2128-2129
2148-2149: Good addition of tags field to Waitpoint model.The tags field allows categorizing waitpoints, directly supporting the filtering functionality mentioned in the PR objectives.
2157-2159: Helpful indexing for performance optimization.These indexes will improve query performance for:
- Time period filtering (by creation date)
- Status filtering
This is important for efficiently displaying and filtering waitpoints in the UI.
2206-2219: Well-designed WaitpointTag model.The new WaitpointTag model:
- Has proper relations to environments and projects
- Includes a unique constraint to prevent duplicate tags in an environment
- Provides a good structure for organizing and filtering waitpoints
This implementation fully supports the tag-based filtering mentioned in the PR requirements.
apps/webapp/app/assets/icons/TriggerIcon.tsx (1)
1-5: Simple and effective icon component implementation.The
TriggerIconcomponent:
- Reuses the existing
BoltIconfrom Heroicons- Provides a consistent way to utilize this icon across the application
- Maintains the same prop pattern as other icon components
This follows the project's pattern for icon components and helps maintain UI consistency.
apps/webapp/app/components/runs/v3/RunFilters.tsx (2)
64-64: Import of StatusIcon improves component consistency.The addition of this import is part of a design pattern that centralizes icon components, making the codebase more maintainable.
152-152: Appropriate usage of StatusIcon component.Replacing the previous icon implementation with the standardized StatusIcon component improves consistency across the UI and simplifies the code in this filter type definition.
apps/webapp/app/assets/icons/WaitpointTokenIcon.tsx (1)
1-12: Well-structured icon component.This new icon component follows best practices by:
- Accepting an optional className for flexible styling
- Using currentColor to inherit color from parent elements
- Implementing proper SVG attributes and path definitions
- Following the project's component naming conventions
This component will be valuable for the new Waitpoints/Tokens page mentioned in the PR objectives.
internal-packages/database/prisma/migrations/20250320152806_waitpoint_renamed_tags/migration.sql (1)
1-10:❓ Verification inconclusive
Migration properly renames and changes the tags column structure.
The migration appropriately warns about data loss and changes the column from
waitpointTagstotagswith a TEXT[] type. This change aligns with the PR's objective to enhance tag management for waitpoint tokens.A few considerations:
- Ensure any existing code that referenced the old column name has been updated
- Verify that data migration (if needed) is handled elsewhere in the codebase
- Confirm that the new TEXT[] type is properly handled in all queries
🏁 Script executed:
#!/bin/bash # Check for any remaining references to the old column name rg -i "waitpointTags" --type ts --type tsx --type js --type jsxLength of output: 91
Action Required: Reverify "waitpointTags" References
The migration correctly renames the
waitpointTagscolumn totags(with the newTEXT[]type) and warns appropriately about data loss. Please ensure that:
- All references to the old column name have been updated across the codebase.
- Any required data migration is addressed outside this SQL migration.
- The new
TEXT[]type is correctly handled in all queries.Note: The initial search command returned an error (
unrecognized file type: tsx). To ensure a proper check for any lingering references towaitpointTags, please run the adjusted search command below:#!/bin/bash # Custom search for "waitpointTags" across TypeScript, TSX, JavaScript, and JSX files rg -i "waitpointTags" -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx"After confirming that no references remain, you may proceed with confidence.
internal-packages/database/prisma/migrations/20250320105905_waitpoint_add_indexes_for_dashboard/migration.sql (1)
1-5: Performance optimization through appropriate indexing.These indexes will improve query performance for the new Waitpoints/Tokens page by:
- Optimizing filtering by environment, type, and creation date (with DESC for newest-first sorting)
- Speeding up status-based filtering within environment and type contexts
These indexes align perfectly with the PR objectives for implementing the new tokens page with filtering capabilities.
apps/webapp/app/components/runs/v3/RunIcon.tsx (2)
16-16: Added new import for TriggerIconThis import is correctly added to support the new case in the switch statement below.
69-70: Added new case for "trigger" iconThe implementation follows the established pattern of other icon cases, using the appropriate component and styling. The orange color helps distinguish this icon type visually.
apps/webapp/app/components/BlankStatePanels.tsx (2)
38-38: Added import for WaitpointTokenIconThis import is properly added to support the new NoWaitpointTokens component.
416-438: Added NoWaitpointTokens component for empty stateThe new component follows the same pattern as other blank state panels in this file, providing consistent user experience. The explanatory text clearly describes the purpose of waitpoint tokens.
apps/webapp/app/utils/zod.ts (1)
1-22: Added utility for Date coercion with robust type handlingThis utility function properly handles different input types for date conversion, with appropriate type safety via Zod. The approach of handling different input types (null/undefined, numbers, strings) separately provides comprehensive conversion support.
The implementation correctly:
- Returns undefined for null/undefined values
- Directly converts numbers to Date objects
- Attempts to parse strings as numbers first, then falls back to direct string parsing
- Preserves the original input for other types
apps/webapp/app/components/navigation/SideMenu.tsx (3)
57-57: Added path import for waitpoint tokensThis import is correctly added to support the new navigation item for waitpoint tokens.
84-84: Added import for WaitpointTokenIconThis import is properly added to support the new sidebar menu item for waitpoint tokens.
216-223: Added new Waitpoints section with Tokens menu itemThe implementation creates a new collapsible section in the sidebar for Waitpoints, following the established pattern of other sections. The menu item uses the appropriate icon and styling, with the correct routing path.
The placement between the existing menu items and the "Manage" section is logical in the navigation hierarchy.
packages/core/src/v3/apiClient/types.ts (1)
46-53: Well-structured interface for filtering waitpoint tokens.The new
ListWaitpointTokensQueryParamsinterface is cleanly designed and follows the established pattern from the existingListRunsQueryParamsinterface. It correctly extendsCursorPageParamsfor pagination support.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx (1)
137-137: Good update for supporting multiple run IDs.The change from passing a single
runIdto an array ofrunIdsimproves the flexibility of the filtering mechanism. This allows for potential future expansion to filter by multiple run IDs simultaneously.apps/webapp/app/models/waitpointTag.server.ts (1)
4-5: LGTM: Good constants with clear naming.The constants are well-named and provide clear limits for tags per waitpoint and retry attempts.
apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.complete.ts (1)
4-4: Good use of TypeScript's type-only importUsing the
typekeyword for importingCompleteWaitpointTokenResponseBodyfollows best practices in modern TypeScript. This improves type safety, enables better tree-shaking, and clearly indicates this is only used for type checking rather than at runtime.apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (2)
29-29: Good consistency improvement with version filterAdding the
version: "V2"filter to the count query makes it consistent with the filter used in thegetQueuesWithPaginationmethod. This ensures the total count matches the actual queues that would be returned.
36-56: Enhanced error handling for engine version compatibilityThis is a good improvement to the error handling logic. The code now provides more accurate information when the engine version is V1 by checking for the existence of old V1 queues and adjusting the
totalQueuesvalue accordingly.apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts (3)
2-2: Good use of TypeScript's type-only importUsing the
typekeyword for importingWaitForWaitpointTokenResponseBodyfollows best practices in modern TypeScript. This improves type safety and clearly indicates this is only used for type checking.
16-18: Proper schema validation for new releaseConcurrency parameterThe Zod schema is correctly updated to include the optional
releaseConcurrencyboolean parameter. This ensures proper validation of the request body.
45-45: Successfully implemented releaseConcurrency featureThe new
releaseConcurrencyparameter is now properly passed to the engine method, completing the feature implementation and giving users more control over concurrency when working with waitpoints.apps/webapp/app/components/runs/v3/PacketDisplay.tsx (2)
6-14: Well-structured component with clear responsibilitiesThe
PacketDisplaycomponent has a clean interface with well-typed props, creating a reusable component that can handle different types of data display based on thedataTypeparameter.
15-50: Good conditional rendering pattern with exhaustive switch statementThe component handles three distinct rendering paths using a clean switch statement pattern with appropriate default case. The conditional logic is easy to follow and each case is properly enclosed in its own block scope.
internal-packages/run-engine/src/engine/index.ts (1)
805-828: Appropriate parameter addition for tag supportThe addition of the optional
tagsparameter tocreateManualWaitpointenables the filtering functionality described in the PR objectives. The parameter is properly typed as an optional string array and correctly passed down to the waitpoint system.apps/webapp/app/presenters/v3/ApiWaitpointPresenter.server.ts (2)
8-56: Robust API presenter with appropriate error handlingThe presenter has a well-structured
callmethod that properly handles database queries and error cases. The ServiceValidationError is appropriately thrown when a waitpoint is not found.
62-78: Good transformation of database objects to API responsesThe presenter properly transforms database fields to API response format, handling optional values appropriately with nullish coalescing operators. This creates a clean API boundary.
apps/webapp/app/components/code/CodeBlock.tsx (1)
425-449: Good enhancement to handle code highlighting loading stateThe addition of the loading state for language definitions prevents rendering issues when the PrismJS components haven't fully loaded. This improves the user experience by showing a usable fallback while language definitions are being loaded.
However, consider refactoring the loading implementation to avoid redundant imports:
useEffect(() => { // This ensures the language definitions are loaded - Promise.all([ - //@ts-ignore - import("prismjs/components/prism-json"), - //@ts-ignore - import("prismjs/components/prism-typescript"), - ]).then(() => setIsLoaded(true)); + // Since these imports are already called at file level in setup(), + // we can just set isLoaded after a short delay to ensure they've completed + const timer = setTimeout(() => setIsLoaded(true), 50); + return () => clearTimeout(timer); }, []);This would prevent importing the same modules twice, as they're already imported in the
setup()function at the top of the file.references/hello-world/src/trigger/waits.ts (4)
15-15: LGTM: Added tags parameter to waitToken task.The tags parameter is correctly added to both the function signature and type definition.
Also applies to: 22-22
30-30: LGTM: Consistently passing tags to createToken calls.The tags parameter is properly passed to both token creation calls.
Also applies to: 38-38
49-58: Good implementation of token listing and retrieval.The implementation correctly uses async iteration with
for awaitto process all tokens, which is an efficient pattern for handling potentially large result sets. The logger.trace wrapper also helps with performance by only computing the log messages when tracing is enabled.
60-60: LGTM: Added releaseConcurrency option.The releaseConcurrency flag will release running concurrency limits while waiting for the token, which can improve system throughput.
internal-packages/database/prisma/migrations/20250325124348_added_connected_runs_to_waitpoints/migration.sql (1)
1-15: LGTM: Well-designed migration for many-to-many relationship.This migration properly establishes a many-to-many relationship between TaskRun and Waitpoint tables with:
- A junction table with appropriate columns
- A unique index to prevent duplicate connections
- An index on column B for efficient lookups
- Proper foreign key constraints with cascade behavior
This design follows database best practices for relationship modeling.
apps/webapp/app/components/primitives/AnimatedNumber.tsx (2)
5-12: Good simplification of animation approach.Replacing the spring physics model with a direct animation using
animate()is a good simplification. This approach with explicit duration and easing makes the animation behavior more predictable while maintaining a smooth user experience.
13-13: Proper dependency array.The dependency array correctly includes only the
valueprop, ensuring the animation is triggered only when the value changes.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tags.ts (2)
8-12: Good use of Zod for parameter validation.Using Zod to define and validate route parameters is a good practice that ensures type safety and proper error handling.
14-27: Well-structured authentication and resource loading.The loader function correctly:
- Requires authentication
- Validates route parameters
- Loads the necessary resources (project and environment)
- Returns appropriate 404 responses when resources aren't found
This follows best practices for Remix loaders.
apps/webapp/app/models/taskRunTag.server.ts (1)
7-7: Good introduction of a retry limit.Defining a clear
MAX_RETRIES = 3is helpful to prevent infinite loops or excessively long blocking. This addition improves resilience.apps/webapp/app/components/primitives/CopyableText.tsx (1)
23-61: Excellent reusable component.The tooltip-based hover behavior and icon change on successful copy provide a clear user interface. Overall, this is a well-structured and maintainable approach for copy-to-clipboard functionality.
apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (3)
62-67: Timed-out error logic is well defined.The check for
isWaitpointOutputTimeout(output)correctly flags timeouts. This helps distinguish normal errors from those indicating a waitpoint has expired.
72-81: Good approach for linked run lookups.Fetching connected runs only if they exist, limiting to 5, avoids unnecessary database overhead. This is an efficient partial retrieval strategy.
83-101: Comprehensive return structure.The final return object offers a thorough and well-organized set of fields for consumers. This makes the waitpoint detail easy to consume by other parts of the application.
internal-packages/database/prisma/migrations/20250320152314_waitpoint_tags/migration.sql (4)
1-3: Well-structured ALTER TABLE for waitpoint tagsThe migration correctly adds a
waitpointTagscolumn of TEXT[] type to the Waitpoint table, which will store an array of tag identifiers. This approach allows for efficient tag storage without requiring a separate join table.
4-13: Properly designed WaitpointTag table schemaThe WaitpointTag table is well structured with appropriate columns:
- Primary key with
id- Required
namefield for tag content- Association columns (
environmentId,projectId)- Automatic timestamp tracking with
createdAtThis design supports the tag management requirements outlined in the PR objectives.
15-16: Good use of a composite unique indexThe unique index on (
environmentId,name) ensures that tag names are unique within each environment, preventing duplicate tags while still allowing the same tag name to exist in different environments.
18-22: Appropriate foreign key constraints with CASCADE behaviorThe foreign key constraints correctly establish the relationship between WaitpointTag and both RuntimeEnvironment and Project tables. The CASCADE behavior on DELETE and UPDATE ensures referential integrity by automatically handling dependent records when parent records are modified or removed.
apps/webapp/app/presenters/v3/RunListPresenter.server.ts (4)
27-27: Good update to support multiple run IDsAdding
runIds?: string[]to the RunListOptions type correctly extends the filtering capabilities to support multiple run identifiers.
55-55: Consistent parameter update throughout the method signatureThe parameter has been correctly updated from
runIdtorunIdsin the method signature and filter condition check, ensuring type consistency throughout the component.Also applies to: 75-75
185-185: Improved rootOnly handlingThe condition has been updated to check for
runIds?.lengthwhen determining whether to override therootOnlyparameter, which is a logical enhancement for filtering when specific runs are requested.
264-264: Effective SQL query modificationThe SQL query has been properly updated to use the
INoperator with therunIdsarray, allowing efficient filtering of multiple run identifiers. The use ofPrisma.join()ensures safe parameter binding.apps/webapp/app/components/runs/v3/WaitpointDetails.tsx (4)
15-28: Well-structured component with proper type definitionsThe WaitpointDetailTable component is well defined with appropriate props and types. The use of optional props with default values (
linkToList = false) follows React best practices. The expiration check logic is clean and readable.
29-52: Good implementation of ID and status displayThe status and ID sections are well implemented with appropriate conditional rendering. The use of TextLink for navigation when
linkToListis true provides good UX while maintaining code flexibility.
53-72: Clear presentation of idempotency key informationThe idempotency key section effectively handles various states, including displaying expiration information when available and handling user-provided keys appropriately.
73-114: Comprehensive timeout and tags implementation for manual waitpointsThe component correctly handles the special case for manual waitpoints, displaying timeout information and the force timeout action when appropriate. The tag rendering with links to filtered views is particularly useful for navigation.
apps/webapp/app/utils/pathBuilder.ts (3)
7-7: Appropriate import for WaitpointSearchParamsThe import of WaitpointSearchParams type is correctly added to support the filter parameters in the new path builder functions.
314-323: Well-implemented path builder for waitpoint tokens listThe
v3WaitpointTokensPathfunction follows the established pattern of other path builders in the codebase. It correctly handles optional filter parameters and maintains consistency with the existing URL structure.
325-335: Efficient implementation of individual token path builderThe
v3WaitpointTokenPathfunction reuses thev3WaitpointTokensPathfunction to build the base URL, which promotes code reuse and ensures consistent URL structure. It correctly appends the token ID and optional query parameters.apps/webapp/app/presenters/v3/ApiWaitpointTokenListPresenter.server.ts (1)
66-134: Structured and robust approach for listing tokens.Creating a typed options object (
WaitpointTokenListOptions) and mapping it from validated search parameters is good for maintainability and clarity. Integrating pagination and filter logic here is cleanly separated from the lower-level or database-intensive code, promoting a single point of truth for listing tokens.internal-packages/run-engine/src/engine/systems/waitpointSystem.ts (2)
248-249: Include tags in "createManualWaitpoint" parameters.Adding the
tagsparameter here aligns it with the rest of the codebase’s tagging feature. This ensures consistency and fosters future extension for advanced token or waitpoint metadata handling.
291-344:❓ Verification inconclusive
Leverage retry logic for concurrent upsert conflicts.
The introduction of a retry mechanism (up to 5 attempts) on unique constraint violations is a practical way to handle concurrency issues. However, be mindful that repeated collisions can degrade performance under high contention.
Consider searching the codebase for other upsert patterns. Harmonizing concurrency strategies across all upsert flows may improve consistency. Below is a script to locate upsert calls:
🏁 Script executed:
#!/bin/bash # Searching for "upsert(" usage across the repository to ensure consistent concurrency patterns rg 'upsert\s*\(' -A 10Length of output: 20205
Standardize Upsert Concurrency Handling
The retry mechanism in
internal-packages/run-engine/src/engine/systems/waitpointSystem.ts(lines 291–344) effectively addresses unique constraint violations by retrying up to 5 times. This approach is practical for handling concurrency issues; however, note that repeated collisions under high load might impact performance.Our repository search shows multiple upsert patterns (in tests, seed scripts, etc.) that do not incorporate such retry logic. While these cases might not face the same contention, it’s worth reviewing whether similar concurrency safeguards should be applied elsewhere for consistency.
- File under review:
internal-packages/run-engine/src/engine/systems/waitpointSystem.ts
- Implements a retry loop to handle Prisma's
P2002errors.- Other occurrences: Upsert flows in various modules (e.g., task queue, user creation, promotions) lack similar retry logic.
Please verify if additional upsert flows may benefit from a standardized retry strategy and adjust accordingly if similar high-contention scenarios are expected.
apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (4)
21-31: Loader route for listing waitpoint tokens looks good.The usage of
ApiWaitpointTokenListPresenterto centralize logic is consistent with the rest of the architecture. Error handling isn’t explicitly provided here, but the container route or the presenter can handle any exceptions as needed.
41-47: Double-check request body size limit.Enforcing
maxContentLength: 1024 * 10(10 KB) is a beneficial measure. However, consider validating overall input sizes more holistically if there's a possibility that tags or other request fields could exceed 10 KB, especially in future expansions.
48-70: Robust tag creation logic with concurrency handling.Your approach ensures that each tag is created or retrieved via
createWaitpointTag. The limit ofMAX_TAGS_PER_WAITPOINTproperly guards the system against excessive tagging. This is consistent with best practices for controlling user input scale and concurrency.
71-80: Pass tags to the engine.Attaching
bodyTagsto theengine.createManualWaitpoint()call ensures the entire waitpoint, including tags, is managed in a single place. This is well-structured and consistent with the service-locating approach.apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (2)
49-63: Great use of Zod for enforcing typed search parameters.Defining the schema via
WaitpointSearchParamsSchemaensures strong validation of filter inputs. This is beneficial for consistent data shapes, preventing invalid queries from reaching backend logic.
69-93:❓ Verification inconclusive
Ensure “Clear all” filters action is robust.
Inside
WaitpointTokenFilters, the “Clear all” button triggers a form submission without specifying anaction. Confirm that this submission effectively resets all filters in all scenarios (e.g., preserving existing pagination queries, if needed).
🏁 Script executed:
#!/bin/bash # Verify that the "Clear all" button triggers correct filter reset rg -A 5 "Clear all"Length of output: 2697
Attention: Verify “Clear all” Filters Behavior
Please confirm that the “Clear all” button in
WaitpointTokenFilters.tsxreliably resets all filter-related query parameters. The current implementation uses a<Form>without an explicitaction, following a similar pattern seen inRunFilters.tsx,ScheduleFilters.tsx, andBatchFilters.tsx. Ensure that on submission:
- All filter parameters (e.g.,
statuses,period,tags,from,to,id,idempotencyKey) are cleared.- Any non-filter query parameters (such as pagination settings, if applicable) are preserved if required.
If the desired behavior is to maintain pagination or other state, additional handling or tests may be necessary.
apps/webapp/app/presenters/v3/WaitpointTagListPresenter.server.ts (4)
1-15: Well-structured type definitions and importsThe type definitions for
TagListOptions,TagList, andTagListItemare clear and well-structured. I particularly like how you've derived theTagListandTagListItemtypes from the return type of the presenter's call method, which ensures type consistency throughout the codebase.
17-24: LGTM! Good class structureThe presenter extends BasePresenter and provides a well-structured method with appropriate parameters.
26-39: Good query implementation with filter and paginationThe query implementation is robust, with:
- Proper use of case-insensitive filtering
- Effective pagination using skip/take pattern
- Fetching one extra item to determine if there are more pages
The
ORcondition handles the case when names filter is empty which is a good defensive programming practice.
41-51: Clean and efficient response structureThe response object is well-structured, providing all necessary metadata for pagination and filtering. The slicing of the tags array to the requested page size is a clean way to handle the "hasMore" pagination pattern.
apps/webapp/app/components/runs/v3/RunTag.tsx (6)
1-7: Clean import organizationThe imports are well-organized, with React hooks first, followed by component imports. The addition of the Clipboard icons will support the new copy functionality.
10-12: Enhanced component API with new propsThe addition of
toandtooltipprops enhances the component's flexibility, allowing it to act as a navigation element and to provide contextual information.
14-38: Good extraction of rendering logicExtracting the render logic into a separate function improves code readability and maintainability. The function handles both string and object tag types appropriately.
40-55: Solid implementation of conditional Link wrappingGood implementation of conditionally wrapping the tag content in a Link component when
toprop is provided. The SimpleTooltip integration adds a nice UX touch.
57-62: Clean component compositionThe main component structure is clean, with good use of composition by including the new CopyButton component.
81-105: Excellent UX for copy buttonThe implementation of the copy button with visual feedback (changing icon and tooltip) provides a great user experience. The conditional display based on hover state is a nice touch that keeps the UI clean.
apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (2)
1-11: Good use of explicit type importsThe use of the
typekeyword for import statements clarifies that these are type imports rather than value imports, which improves code clarity and can help with tree-shaking during build.
13-13: Type import consistencyConsistent application of the
typekeyword for imports, maintaining the codebase style.apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (4)
1-7: Good import organization with type safetyThe imports are well-organized and include assertNever for exhaustive type checking, which is a great practice for enum-based switch statements.
8-23: Well-composed status componentThe WaitpointStatusCombo component nicely composes the icon and label components with clean prop drilling. The use of cn utility for class name composition is a good practice.
25-29: Simple and effective label componentThe WaitpointStatusLabel component is concise and leverages utility functions for consistency.
66-79:✅ Verification successful
Verify enum coverage for future-proofing
The current implementation handles three statuses. Verify this covers all possible WaitpointTokenStatus values, or ensure you have a process to update these functions when new statuses are added.
Run this script to verify all enum values are accounted for:
🏁 Script executed:
#!/bin/bash # Check for WaitpointTokenStatus enum values grep -A 10 "export const waitpointTokenStatuses" packages/core/src/v3/schemas/api.tsLength of output: 580
Enum Coverage Verified
The implementation correctly handles all values defined in theWaitpointTokenStatusenum:"WAITING","COMPLETED", and"TIMED_OUT". The default assertion withassertNever(status)further ensures that any future additions to the enum will be caught during development. No changes are required at this time.apps/webapp/app/presenters/v3/SpanPresenter.server.ts (1)
45-45: No concerns.Exposing
projectIdin the parent run selection looks consistent with the rest of the logic.packages/trigger-sdk/src/v3/wait.ts (1)
123-142: Great addition oflistTokens.Perfectly lays out filterable and paginated iteration for waitpoint tokens. The usage examples are clear and well documented.
packages/core/src/v3/schemas/api.ts (4)
928-941: Strong enhancement with tags for waitpointsThe addition of the tags property to
CreateWaitpointTokenRequestBodyallows better organization and filtering of waitpoints. The documentation is thorough with clear constraints and usage examples.
951-954: Well-structured waitpoint token status definitionUsing a const array with
as constalongside zod's enum and type inference creates a robust type-safe enumeration for token statuses that will provide consistency throughout the codebase.
955-980: Comprehensive token schemas with clear hierarchyThe progressive schema building with
WaitpointTokenItem→WaitpointListTokenItem→WaitpointRetrieveTokenResponsecreates a well-structured type hierarchy that supports different token retrieval use cases.
1011-1018: Helpful documentation for releaseConcurrency parameterThe added documentation explains the purpose and default value of the
releaseConcurrencyparameter clearly, helping developers understand when and why to use this feature.packages/core/src/v3/apiClient/index.ts (3)
676-697: Well-implemented token listing with pagination supportThe
listWaitpointTokensmethod follows the established patterns in the codebase for paginated API requests, properly utilizingzodfetchCursorPageand handling all query parameters.
699-709: Simple and consistent token retrieval implementationThe
retrieveWaitpointTokenmethod follows the API client's conventions for GET requests and uses the appropriate response schema.
1066-1110: Well-structured query builder for token listingThe
createSearchQueryForListWaitpointTokensfunction handles all filter cases properly, following the same pattern as other query builders in the codebase. The implementation handles array vs. single value parameters correctly.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (3)
76-78: Good component imports for waitpoint displayImporting specialized components for waitpoint display improves code organization and reusability.
623-629: Improved tag component usage with explicit propertiesThe updated code uses
RunTagwith clear and explicit properties instead of wrapping it in aSimpleTooltip, which simplifies the component hierarchy and improves readability.
953-959: Simplified waitpoint detail rendering with specialized componentReplacing custom property rendering with the dedicated
WaitpointDetailTablecomponent improves consistency and reduces code duplication. The status check update from "PENDING" to "WAITING" properly aligns with the schema definition.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsx (3)
193-195: Updated FormWaitpoint type with better structureAdding the
statusproperty with theWaitpointTokenStatustype enhances type safety and aligns with the schema changes.
287-317: Updated parameter type with id propertyThe
CompleteManualWaitpointFormcomponent has been properly updated to useidinstead offriendlyId.
380-413: Consistent updates to ForceTimeout componentThe
ForceTimeoutcomponent has been properly updated to useidinstead offriendlyIdfor waitpoint identification.
| export const loader = createLoaderApiRoute( | ||
| { | ||
| params: z.object({ | ||
| waitpointFriendlyId: z.string(), | ||
| }), | ||
| findResource: async () => 1, // This is a dummy function, we don't need to find a resource | ||
| }, | ||
| async ({ params, authentication }) => { | ||
| const presenter = new ApiWaitpointPresenter(); | ||
| const result: WaitpointRetrieveTokenResponse = await presenter.call( | ||
| authentication.environment, | ||
| WaitpointId.toId(params.waitpointFriendlyId) | ||
| ); | ||
| return json(result); | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the API loader
The implementation of this new API endpoint for retrieving waitpoint tokens looks good overall. However, it lacks error handling around the presenter call, which could lead to unhandled exceptions.
Consider adding a try/catch block to properly handle errors:
export const loader = createLoaderApiRoute(
{
params: z.object({
waitpointFriendlyId: z.string(),
}),
findResource: async () => 1, // This is a dummy function, we don't need to find a resource
},
async ({ params, authentication }) => {
+ try {
const presenter = new ApiWaitpointPresenter();
const result: WaitpointRetrieveTokenResponse = await presenter.call(
authentication.environment,
WaitpointId.toId(params.waitpointFriendlyId)
);
return json(result);
+ } catch (error) {
+ if (error instanceof ServiceValidationError) {
+ return json({ error: error.message }, { status: 404 });
+ }
+ return json({ error: "Failed to retrieve waitpoint token" }, { 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.
| export const loader = createLoaderApiRoute( | |
| { | |
| params: z.object({ | |
| waitpointFriendlyId: z.string(), | |
| }), | |
| findResource: async () => 1, // This is a dummy function, we don't need to find a resource | |
| }, | |
| async ({ params, authentication }) => { | |
| const presenter = new ApiWaitpointPresenter(); | |
| const result: WaitpointRetrieveTokenResponse = await presenter.call( | |
| authentication.environment, | |
| WaitpointId.toId(params.waitpointFriendlyId) | |
| ); | |
| return json(result); | |
| } | |
| ); | |
| export const loader = createLoaderApiRoute( | |
| { | |
| params: z.object({ | |
| waitpointFriendlyId: z.string(), | |
| }), | |
| findResource: async () => 1, // This is a dummy function, we don't need to find a resource | |
| }, | |
| async ({ params, authentication }) => { | |
| try { | |
| const presenter = new ApiWaitpointPresenter(); | |
| const result: WaitpointRetrieveTokenResponse = await presenter.call( | |
| authentication.environment, | |
| WaitpointId.toId(params.waitpointFriendlyId) | |
| ); | |
| return json(result); | |
| } catch (error) { | |
| if (error instanceof ServiceValidationError) { | |
| return json({ error: error.message }, { status: 404 }); | |
| } | |
| return json({ error: "Failed to retrieve waitpoint token" }, { status: 500 }); | |
| } | |
| } | |
| ); |
| while (attempts < MAX_RETRIES) { | ||
| try { | ||
| return await prisma.taskRunTag.upsert({ | ||
| where: { | ||
| projectId_name: { | ||
| projectId, | ||
| name: tag, | ||
| }, | ||
| }, | ||
| create: { | ||
| friendlyId, | ||
| name: tag, | ||
| projectId, | ||
| }, | ||
| update: {}, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { | ||
| // Handle unique constraint violation (conflict) | ||
| attempts++; | ||
| if (attempts >= MAX_RETRIES) { | ||
| throw new Error(`Failed to create tag after ${MAX_RETRIES} attempts due to conflicts.`); | ||
| } | ||
| } else { | ||
| throw error; // Re-throw other errors | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding a small delay or exponential backoff on retry.
While the while loop handles P2002 errors effectively to mitigate unique constraint conflicts, the retries occur immediately. Introducing a brief delay or exponential backoff can help reduce contention in multi-tenant or high-load scenarios.
+ // Example: Introducing a small delay before each retry
+ const BASE_DELAY_MS = 100;
...
while (attempts < MAX_RETRIES) {
try {
...
} catch (error) {
...
if (attempts < MAX_RETRIES) {
+ await new Promise(resolve => setTimeout(resolve, BASE_DELAY_MS * 2 ** attempts));
}
}
}📝 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.
| while (attempts < MAX_RETRIES) { | |
| try { | |
| return await prisma.taskRunTag.upsert({ | |
| where: { | |
| projectId_name: { | |
| projectId, | |
| name: tag, | |
| }, | |
| }, | |
| create: { | |
| friendlyId, | |
| name: tag, | |
| projectId, | |
| }, | |
| update: {}, | |
| }); | |
| } catch (error) { | |
| if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { | |
| // Handle unique constraint violation (conflict) | |
| attempts++; | |
| if (attempts >= MAX_RETRIES) { | |
| throw new Error(`Failed to create tag after ${MAX_RETRIES} attempts due to conflicts.`); | |
| } | |
| } else { | |
| throw error; // Re-throw other errors | |
| } | |
| } | |
| } | |
| // Example: Introducing a small delay before each retry | |
| const BASE_DELAY_MS = 100; | |
| while (attempts < MAX_RETRIES) { | |
| try { | |
| return await prisma.taskRunTag.upsert({ | |
| where: { | |
| projectId_name: { | |
| projectId, | |
| name: tag, | |
| }, | |
| }, | |
| create: { | |
| friendlyId, | |
| name: tag, | |
| projectId, | |
| }, | |
| update: {}, | |
| }); | |
| } catch (error) { | |
| if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { | |
| // Handle unique constraint violation (conflict) | |
| attempts++; | |
| if (attempts >= MAX_RETRIES) { | |
| throw new Error(`Failed to create tag after ${MAX_RETRIES} attempts due to conflicts.`); | |
| } | |
| if (attempts < MAX_RETRIES) { | |
| await new Promise(resolve => setTimeout(resolve, BASE_DELAY_MS * 2 ** attempts)); | |
| } | |
| } else { | |
| throw error; // Re-throw other errors | |
| } | |
| } | |
| } |
...izationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx
Outdated
Show resolved
Hide resolved
apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts
Outdated
Show resolved
Hide resolved
…ems and removed the useLayoutEffect
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
280-291: 🛠️ Refactor suggestionAdd a fallback case for unmapped statuses.
The switch statement only covers "PENDING" and "COMPLETED." If a future database status arises, this function may return
undefined.Apply this patch to add a default fallback:
export function waitpointStatusToApiStatus( status: WaitpointStatus, outputIsError: boolean ): WaitpointTokenStatus { switch (status) { case "PENDING": return "WAITING"; case "COMPLETED": return outputIsError ? "TIMED_OUT" : "COMPLETED"; + default: + // Fallback to an existing status (e.g., WAITING or some new placeholder). + return "WAITING"; } }
🧹 Nitpick comments (5)
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (4)
346-356: Add debouncing to tag search to prevent excessive API calls.The useEffect hook triggers an API request on every change to searchValue without debouncing. This could lead to excessive API calls during rapid typing, potentially causing performance issues.
+ const [debouncedSearchValue, setDebouncedSearchValue] = useState(searchValue); + + useEffect(() => { + const timer = setTimeout(() => { + setDebouncedSearchValue(searchValue); + }, 300); + + return () => clearTimeout(timer); + }, [searchValue]); + useEffect(() => { const searchParams = new URLSearchParams(); - if (searchValue) { - searchParams.set("name", encodeURIComponent(searchValue)); + if (debouncedSearchValue) { + searchParams.set("name", encodeURIComponent(debouncedSearchValue)); } fetcher.load( `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/waitpoints/tags?${searchParams}` ); - }, [searchValue]); + }, [debouncedSearchValue, organization.slug, project.slug, environment.slug]);
499-499: Fix inconsistent placeholder for Waitpoint ID input.The placeholder says "run_" but the validation logic on lines 473-474 checks if the ID starts with "waitpoint_". This inconsistency could confuse users about the expected format.
- placeholder="run_" + placeholder="waitpoint_"
616-616: Update placeholder for Idempotency key input.The placeholder currently shows "waitpoint_" which is misleading for an idempotency key field. It should be something more relevant to idempotency keys.
- placeholder="waitpoint_" + placeholder="my-idempotency-key"
358-371: Consider optimizing the filtered tags calculation.The current implementation of
filteredcan be optimized to avoid unnecessary array operations.const filtered = useMemo(() => { let items: string[] = []; if (searchValue === "") { items = values("tags"); } if (fetcher.data === undefined) { return matchSorter(items, searchValue); } - items.push(...fetcher.data.tags.map((t) => t.name)); - - return matchSorter(Array.from(new Set(items)), searchValue); + // Combine existing tags and fetched tags efficiently + const uniqueTags = new Set([...items, ...fetcher.data.tags.map((t) => t.name)]); + return matchSorter(Array.from(uniqueTags), searchValue); }, [searchValue, fetcher.data]);apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (1)
132-215: Solid raw SQL approach, but watch out for indexing.This raw query with multiple filters is efficient, but ensure the relevant columns (e.g.,
createdAt,environmentId,id) are properly indexed to maintain performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx(1 hunks)apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (3)
packages/core/src/v3/schemas/api.ts (3)
WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)waitpointTokenStatuses(951-951)apps/webapp/app/assets/icons/StatusIcon.tsx (1)
StatusIcon(3-9)apps/webapp/app/components/runs/v3/WaitpointStatus.tsx (2)
waitpointStatusTitle(66-79)WaitpointStatusCombo(8-23)
apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (2)
packages/core/src/v3/schemas/api.ts (5)
RunEngineVersion(7-7)WaitpointTokenStatus(952-952)WaitpointTokenStatus(953-953)WaitpointTokenItem(955-965)WaitpointTokenItem(966-966)apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
WaitpointSearchParams(63-63)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- 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
🔇 Additional comments (9)
apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
49-63: LGTM: Well-structured search parameters schema.The
WaitpointSearchParamsSchemais well-designed with appropriate types and preprocessing for different parameter types. The preprocessing for array values and handling of the "all" period value are good approaches.apps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts (8)
1-16: Imports and initial setup look good.No major issues spotted in the initial imports and constants. Good modular structure and usage of external libraries.
17-38: Configuration object is well-defined.
WaitpointTokenListOptionscomprehensively covers filter and pagination parameters. The optional fields allow flexible usage.
40-62: Robust success/failure result type.The unified structure for success and failure responses is clear and easy to consume. This pattern promotes consistent error handling.
64-92: Graceful handling of deprecated engine versions.Returning an error response when users attempt to fetch tokens on Engine V1 is transparent. The messaging is adequate to guide upgrades to the new feature set.
94-115: Logical conditional checks for statuses.The use of
filterOutputIsErrorbased on matching a single status is straightforward. If multiple statuses or more nuanced conditions are needed in the future (e.g., both "COMPLETED" and "TIMED_OUT"), consider extending this logic.
217-238: Cursor-based pagination is well-structured.Although the logic for handling forward/backward pagination with reversed arrays appears correct, it’s relatively complex. Ensure complete test coverage for edge cases (e.g., minimal items, exact page size, reversing order).
Please confirm your test suite covers backward pagination thoroughly.
240-278: Response shaping is consistent.The final transformation of raw SQL rows into the
WaitpointTokenItemformat is clear. Sorting tags alphabetically is a nice touch for readability.
117-127:✅ Verification successful
Verify mapping of "TIMED_OUT" to "COMPLETED" in DB queries.
Currently, "TIMED_OUT" maps to the database status "COMPLETED" with
outputIsError = true. While this may be intentional, please confirm this aligns with all existing waitpoint statuses and any newer statuses introduced in the system.You can run the following script to see if there are any references to alternate DB statuses that might not be mapped here:
🏁 Script executed:
#!/bin/bash # Searches the codebase for references to WaitpointStatus ast-grep --pattern $'type WaitpointStatus = $_' --jsonLength of output: 13934
DB Status Mapping Verified
The mapping of
"TIMED_OUT"to"COMPLETED"in the presenter (lines 117–127 inapps/webapp/app/presenters/v3/WaitpointTokenListPresenter.server.ts) appears consistent with the current status definitions. Our search forWaitpointStatusin the codebase (notably inpackages/core/src/v3/schemas/runEngine.ts) and a review of related DB migration files did not reveal any conflicting mappings or alternate handling that would suggest a divergence from this behavior.
- The inferred type definition for
WaitpointStatusdoes not indicate any special treatment for"TIMED_OUT"aside from mapping it to"COMPLETED".- No alternate mappings or conditions were found in SQL migrations or queries that override this logic.
Please ensure that any future additions of waitpoint statuses follow the same mapping pattern and that this behavior (including using
outputIsError = truefor the"TIMED_OUT"case) remains intentional.
| {hasFilters && ( | ||
| <Form className="h-6"> | ||
| <Button variant="minimal/small" LeadingIcon={TrashIcon}> | ||
| Clear all | ||
| </Button> | ||
| </Form> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement form submission for the "Clear all" button.
The "Clear all" button is inside a Form component, but the form has no action or submission handler. To properly clear the filters, you should implement a form submission handler that resets all search parameters.
-<Form className="h-6">
- <Button variant="minimal/small" LeadingIcon={TrashIcon}>
+<Form method="get" action="." className="h-6">
+ <Button variant="minimal/small" LeadingIcon={TrashIcon} type="submit">
Clear all
</Button>
</Form>📝 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.
| {hasFilters && ( | |
| <Form className="h-6"> | |
| <Button variant="minimal/small" LeadingIcon={TrashIcon}> | |
| Clear all | |
| </Button> | |
| </Form> | |
| )} | |
| {hasFilters && ( | |
| <Form method="get" action="." className="h-6"> | |
| <Button variant="minimal/small" LeadingIcon={TrashIcon} type="submit"> | |
| Clear all | |
| </Button> | |
| </Form> | |
| )} |
Dashboard Waitpoint Tokens page
Added a Waitpoints/Tokens page where you can see all of your waitpoint tokens. You can filter them and select one to view the details.
Completing waitpoints from the dashboard
You can complete a waitpoint from the dashboard. This is really useful when testing, or even in production where you have some workflows where you haven't built a UI yet and want to do approval flows.
You can also "timeout" a token from the dashboard. When you create a token you can provide a timeout period or date. This is useful if there is a deadline for an approval, and if that date passes it should fail (which would continue runs that are blocked so you can act accordingly using the error).
wait.listTokens() and wait.retrieveToken()
You can get waitpoint tokens using the SDK.
Misc fixes/improvements
Summary by CodeRabbit
Summary by CodeRabbit
Avatarcomponent with a size-based prop system for improved rendering.PacketDisplaycomponent for displaying various data types.WaitpointDetailTablecomponent for detailed waitpoint information.RunTagcomponent with copy functionality and improved tooltip interactions.Avatarcomponent by consolidating size management.RunFilterscomponent to simplify icon rendering.