-
-
Notifications
You must be signed in to change notification settings - Fork 871
Disable payload editing for run replays with large payloads #2268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughSeveral UI and backend adjustments were made across the codebase. The ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (1)
37-38: Remove unused import.The
divideimport from "effect/Duration" appears to be unused in this file.import { InfoIconTooltip } from "~/components/primitives/Tooltip"; -import { divide } from "effect/Duration";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/components/primitives/Tabs.tsx(1 hunks)apps/webapp/app/components/runs/v3/PacketDisplay.tsx(1 hunks)apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx(2 hunks)apps/webapp/app/v3/services/replayTaskRun.server.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/webapp/**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/webapp.mdc
**/*.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/writing-tasks.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
{packages/core,apps/webapp}/**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
apps/webapp/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/webapp.mdc
🧠 Learnings (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (1)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-06-30T13:21:59.438Z
Learning: Applies to apps/webapp/app/**/*.test.{ts,tsx} : Tests should only import classes and functions from files matching `app/**/*.ts` of the webapp, and those files should not use environment variables; everything should be passed through as options instead.
apps/webapp/app/v3/services/replayTaskRun.server.ts (11)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : If you are able to generate an example payload for a task, do so.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.105Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them within the `task` function in Trigger.dev task files.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `runs` and `tasks` APIs from `@trigger.dev/sdk/v3` to subscribe to run updates and implement realtime features.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use the task instance's `trigger`, `batchTrigger`, `triggerAndWait`, `batchTriggerAndWait`, `batch.trigger`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` methods.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In `packages/core/src/v3/errors.ts`, within the `taskRunErrorEnhancer` function, `error.message` is always defined, so it's safe to directly call `error.message.includes("SIGTERM")` without additional checks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `@trigger.dev/sdk/v3` and NEVER use the deprecated `client.defineJob` pattern.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Scheduled tasks must use `schedules.task` from `@trigger.dev/sdk/v3` and define a `cron` property for declarative schedules.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing idempotency, use the `idempotencyKeys` API from `@trigger.dev/sdk/v3` and provide an `idempotencyKey` when triggering tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function must contain the main logic for each Trigger.dev task.
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (2)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `metadata` API from `@trigger.dev/sdk/v3` to attach, update, and access structured metadata within task runs.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
🧬 Code Graph Analysis (1)
apps/webapp/app/v3/services/replayTaskRun.server.ts (1)
packages/core/src/v3/utils/ioSerialization.ts (1)
conditionallyImportPacket(203-229)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/webapp/app/v3/services/replayTaskRun.server.ts (1)
125-130: LGTM! Correctly implements payload editing restriction for large payloads.This implementation properly disables payload editing for
application/storepayloads by immediately returning the result ofconditionallyImportPacket, which aligns with the PR objectives. The change correctly bypasses the override logic for large payloads while maintaining existing behavior for other payload types.apps/webapp/app/components/runs/v3/PacketDisplay.tsx (1)
19-19: LGTM! Appropriate styling adjustment for application/store payloads.The styling change removes border classes and adds smaller text size, which is consistent with the broader changes to handle
application/storepayloads differently in the UI.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (2)
320-320: LGTM! Improved scrollbar styling for better UI consistency.The addition of custom scrollbar classes (
scrollbar-thin,scrollbar-track-transparent,scrollbar-thumb-charcoal-600) enhances the visual appearance of the recent runs popover container.
1401-1401: LGTM! Consistent scrollbar styling applied.The same scrollbar styling classes are appropriately applied to the run templates popover container, maintaining visual consistency across the UI.
apps/webapp/app/components/primitives/Tabs.tsx (1)
97-101: LGTM! Excellent accessibility improvement for disabled tabs.The addition of conditional styling with
pointer-events-none opacity-50properly disables interaction and provides clear visual feedback when tabs are disabled. This enhances the user experience and supports the broader payload editing restrictions implemented in this PR.apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (2)
170-170: LGTM! Smart initial tab selection based on payload editability.The conditional logic correctly sets the initial tab to "payload" when the payload is editable, otherwise defaults to "metadata". This provides a better user experience by starting with the relevant tab.
262-285: LGTM! Excellent implementation of payload editing restrictions.The conditional disabling of the payload tab with an informative tooltip provides clear feedback to users about why the payload cannot be edited for large payloads. The tooltip includes helpful documentation links and the implementation correctly coordinates with the backend changes.
This PR disables the payload tab in the replay run when the original payload is of type
application/store.A couple of minor UI fixes are included too.