feat(ee): workflows #1932
feat(ee): workflows #1932
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a workflow routing system: DB schema and migration, execution engine and validation, Redis link sessions, OTP-based entry verification/access APIs, workflow CRUD, steps and executions endpoints, UI pages/components, feature-flag integration, and link/view handler support for WORKFLOW_LINK. Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (27)
lib/featureFlags/index.ts (1)
14-16:workflowsfeature flag wiring is correctThe new
"workflows"member is correctly added toBetaFeaturesand initialized inteamFeatures, so it will participate in the existing edge-config driven flag resolution without further code changes. Just ensure thebetaFeaturesedge-config key is updated to include any teams that should see workflows.Also applies to: 32-32
ee/features/workflows/README.md (1)
7-44: Specify a language on the code fence to satisfy markdownlintThe structure tree is inside a bare triple‑backtick fence, which triggers MD040. Consider tagging it with a neutral language, e.g.:
```text ee/features/workflows/ ├── components/ ...This keeps rendering the same while satisfying the linter. </blockquote></details> <details> <summary>ee/features/workflows/components/step-list.tsx (1)</summary><blockquote> `6-35`: **Consider using `stepOrder` and shared types for better consistency** Right now the UI label uses `index + 1` and `WorkflowStep` is re‑declared locally. To keep display aligned with persisted ordering and reduce duplication, you might: - Render `Step {step.stepOrder}` (with a fallback to `index + 1` if needed). - Import a shared workflow step type from your workflow `lib/types` instead of redefining the shape here. Not required for correctness, but it will help keep the UI in sync with backend semantics as the workflow model evolves. Also applies to: 66-79 </blockquote></details> <details> <summary>components/ui/single-select.tsx (1)</summary><blockquote> `27-38`: **Preserve caller `onClick` and consider tightening generics** Two minor improvements: 1. **Caller `onClick` is ignored** `Button` currently does: ```ts <Button ref={ref} {...props} onClick={handleTogglePopover} … >Any
onClickpassed inpropsis overwritten. To preserve it:- ( - { - options, - onValueChange, - className, + ( + { + options, + onValueChange, + className, + onClick, value, … }, @@ - <Button - ref={ref} - {...props} - onClick={handleTogglePopover} + <Button + ref={ref} + {...props} + onClick={(event) => { + onClick?.(event); + handleTogglePopover(); + }} className={cn(
- Optional: expose
TMetaat the component levelRight now
SingleSelectis typed asSingleSelectPropswithout propagatingTMeta, sometatends to beany. If you want stricter typing, you could export a generic helper or castforwardRefto a generic signature that preservesTMeta.Also applies to: 86-91, 93-163
pages/api/links/domains/[...domainSlug].ts (1)
160-178: WORKFLOW_LINK early return and brand fetch look correct; consider sharing brand lookup logicThe WORKFLOW_LINK branch correctly runs after existing access checks and returns a minimal payload the workflow entry flow can consume. To reduce duplication with other places that fetch brand data, consider extracting a small helper for
brandlookup byteamIdand reusing it here and elsewhere.ee/features/workflows/components/workflow-access-view.tsx (1)
130-133: Remove debug logging of verification resultThe
console.logblock around the verificationresultis useful during development but will add noisy logs (and may expose internal details) in production. It’s better to remove or guard it behind a debug flag.- console.log("--------------------------------"); - console.log("result", result); - console.log("--------------------------------");app/(ee)/api/workflow-entry/link/[entryLinkId]/verify/route.ts (1)
20-89: OTP issuance flow looks solid; consider aligning IP handling with access routeThe validation, workflow/link checks, rate limiting, OTP rotation, and email dispatch are all wired correctly and consistent with the rest of the workflows feature.
One small optional improvement: here
ipAddressValuecan beundefined, which will bucket all such requests underworkflow-otp:undefined. In the access route you already useipAddress(req) ?? LOCALHOST_IP; mirroring that here would make behavior consistent across verify/access and avoid weird buckets in dev or proxy-misconfigured environments.Also applies to: 91-114
app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts (1)
94-123: VerifyverificationTokenuniqueness assumptions match Prisma schemaHere you look up the OTP with
findUniqueusing{ token: code, identifier: ... }, but delete it later withdelete({ where: { token: code } }). This implicitly relies on howverificationTokenis modeled:
- If there is a composite unique/ID on
(identifier, token), thendelete({ where: { token } })might not compile or might not match the intended record.- If
tokenalone is@unique, includingidentifierinfindUniqueis unnecessary and may not be valid.Please double-check that the Prisma model’s unique/ID constraints align with this usage (and adjust the
whereclauses accordingly to avoid runtime/type issues).ee/features/workflows/components/workflow-header.tsx (2)
2-2: Router hook source & SSRF static warningBecause this component imports
useRouterfrom"next/router", it will only work when rendered from Pages Router components; if you ever mount it from App Router (app/*), you should swap tonext/navigation’s router to avoid runtime errors. Also, the CodeQL SSRF findings on thefetch("/api/workflows/${workflowId}")calls are benign here: the URL is same-origin and only the path segment is dynamic, so no external host is user‑controlled.Also applies to: 38-59, 61-79
81-84: Use shared clipboard helper instead of re‑implementingThe local
copyToClipboardalways shows a success toast and doesn’t handlenavigator.clipboard.writeTextfailures. Consider reusing the existingcopyToClipboard(text, message)fromlib/utils(e.g.,copyToClipboard(entryUrl, "Entry link copied")) so you get consistent error handling and avoid duplicated logic.app/(ee)/api/workflows/[workflowId]/route.ts (1)
193-266: DELETE handler transaction is correct; consider guarding entryLinkIdThe transactional delete (workflow delete + link archive) is structured correctly and keeps referential integrity. If there’s any chance of historical workflows without an
entryLinkId, you may want to guard theprisma.link.updatecall (or assert non‑null) to avoid unexpected 500s during cleanup.app/(ee)/api/workflows/route.ts (1)
90-259: POST /workflows: creation flow is solid with a few optional improvementsThe create flow is well structured: body validation, membership + plan checks, workflows feature flag gating, and a transaction that creates the entry link and workflow, then returns an
entryUrl. Two optional enhancements:
- Slug uniqueness is enforced only when a custom
domainis provided; if you ever rely onslugfor non‑custom‑domain workflows, consider adding a similar uniqueness check there.entryUrlconstruction logic here and in[workflowId]/route.tsis duplicated; extracting a shared helper would reduce drift if URL rules change.app/(ee)/api/workflow-entry/domains/[...domainSlug]/route.ts (1)
20-111: Domain-based verify flow mirrors link route; consider consistent IP handlingThe domain/slug/action parsing, link lookup scoped to
WORKFLOW_LINK, and active/archived checks all look good, andhandleVerifyreusesVerifyEmailRequestSchemaplus rate limiting and OTP storage similarly to the entryLink-based route. One small improvement:ipAddress(req)can be null in some environments; you already default toLOCALHOST_IPinhandleAccess, so applying the same fallback here would keep rate-limit keys consistent and avoid a literal"workflow-otp:undefined"bucket.Also applies to: 113-169
ee/features/workflows/components/workflow-list.tsx (1)
5-32: WorkflowList logic looks solid; only minor UX and typing nitsThe list rendering and
getEntryUrllogic are clear and consistent with the backend model. Two small improvements you might consider:
- The raw entry URL in the monospace span can get long and may overflow on narrow screens; adding
truncate/break-alland atitletooltip would keep the layout tighter.- If you already have a shared
Workflowtype from your API layer, reusing it here instead of a local interface would reduce drift over time.Also applies to: 57-67
app/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
70-104: N+1 queries when enriching steps with target linkFor each step you perform a separate
prisma.link.findUniquewhen arouteaction is present. This is fine for small workflows, but will become N+1 if a workflow accumulates many steps.If this becomes hot, consider:
- Extracting
targetLinkIds fromactionsfirst and doing a singlefindManyfor all links, then mapping in-memory, or- Persisting
targetLinkIdon the step model itself and using Prisma relations to join in one query.ee/features/workflows/lib/validation.ts (1)
25-40: Validation helpers are solid; reuseformatZodErrorto remove duplicationThe structure of
validateConditions,validateActions, andvalidateWorkflowStepis clean and matches the shared schemas. The only real nit is that each catch block reimplements the sameerror.errors.map(...).join(", ")logic even thoughformatZodErroris exported below.You could simplify and keep future changes to error formatting in one place by delegating, e.g.:
- if (error instanceof ZodError) { - return { - valid: false, - error: error.errors.map((e) => `${e.path.join(".")}: ${e.message}`).join(", "), - }; - } + if (error instanceof ZodError) { + return { + valid: false, + error: formatZodError(error), + }; + }Apply similarly in
validateActionsandvalidateWorkflowStep.Also applies to: 45-63, 68-85, 87-92
app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts (2)
41-75: Minor cleanup: prefer validated data and avoid redundant parsingPATCH correctly validates and guards
conditions/actions, and the overall flow looks good. Two small improvements for consistency and maintainability:
- When computing
allowListItems, you currently usevalidation.data.conditionsagain asany. Since you already ranvalidateConditions, it would be safer and more self‑documenting to reuseconditionsValidation.data(and keep it in scope) rather than the untyped cast.- Similarly,
validateActions’ parsed result is only used for validation; you later update the step withvalidation.datainstead. Using the parsedactionsfromvalidateActions(as you do in the POST handler after refactor) would make the behavior consistent and keep all transformations in one place.These are non‑blocking, but tightening them up would make future changes to the schemas less error‑prone.
Also applies to: 150-204
242-269: DELETE handler does an extra nested steps select that isn’t usedIn the initial
workflowStep.findUniqueyou selectworkflow.stepswith astepOrder: { gt: 0 }placeholder, but the subsequent logic fetchesstepsToReordervia a separatefindManyonworkflowStepand doesn’t rely on that nested selection.You can drop the
stepsselection from the first query to reduce payload size and simplify the shape:- workflow: { - select: { - id: true, - teamId: true, - team: { - select: { - users: { select: { userId: true } }, - }, - }, - steps: { - where: { - stepOrder: { gt: 0 }, // placeholder, will be updated below - }, - select: { - id: true, - stepOrder: true, - }, - }, - }, - }, + workflow: { + select: { + id: true, + teamId: true, + team: { + select: { + users: { select: { userId: true } }, + }, + }, + }, + },The deletion + reordering transaction itself looks correct.
Also applies to: 289-312
prisma/migrations/20251116000000_add_workflow/migration.sql (1)
10-37: Migration schema looks consistent; consider a default for execution statusThe new tables, enums, indexes, and FKs all look coherent with the workflows feature and existing link/team models. Two small schema niceties you might consider for ergonomics:
WorkflowExecution.statushas no default; if the vast majority of executions start as a single state (e.g.PENDING), adding a default at the DB level can simplify inserts and keep data consistent even outside Prisma.- Similarly,
updatedAtcolumns areNOT NULLwithout a default; if you aren’t already relying on Prisma’s@updatedAtbehavior, addingDEFAULT CURRENT_TIMESTAMPcan make ad‑hoc writes less error‑prone.These aren’t blockers, just potential quality‑of‑life improvements for future direct DB access.
Also applies to: 39-67, 82-100, 102-118
ee/features/workflows/lib/engine.ts (3)
53-62: Persist full context into metadata (userAgent/referrer currently dropped)
WorkflowExecutionContextexposesuserAgentandreferrer, but onlymetadatais persisted here. Unless callers are pre‑packing these intocontext.metadata, this means those fields are silently lost. Consider merging them into the stored metadata (e.g.,{ ...context.metadata, userAgent, referrer }) so the execution record actually captures the full context.
70-131: Step loop is robust, but some data is unused and condition logging could be clearerThe per‑step parsing, evaluation, and error logging are well structured. Two minor points:
targetDocumentId/targetDataroomIdare assigned but never used after the loop; either include them inactionsResult/ final result or drop the locals.conditionResultscurrently stores the config (conditions), not per‑condition booleans; if you ever need to debug which individual condition failed, consider logging the evaluation results alongside the config.
290-320: Consider makingmatchesexplicitly case‑insensitiveYou normalize
matchescase‑sensitive unless rule authors also lowercase their patterns. If the intent is case‑insensitive matching (like the other operators), consider adding theiflag or documenting that patterns must be lowercase.prisma/schema/workflow.prisma (3)
15-33: Redundant index onentryLinkId
entryLinkIdis marked@uniqueand also has an explicit@@index([entryLinkId]). The unique constraint already creates an index, so the extra index is likely redundant and can be dropped unless you specifically need a separate non‑unique index for some tooling.
35-43: Clarify comments about JSON shape forconditionsandactionsThe comments say “JSON array of conditions/actions”, but the engine and Zod schemas use a structured
ConditionsConfigobject ({ logic, items: [...] }) and an array of discriminated actions. Consider updating the comments to describe the actual JSON shape to avoid confusion when debugging data directly in the DB.
53-70: ExecutionStatus usage and default
ExecutionStatusincludesPENDINGandBLOCKED, butstatushas no default and the engine always writes"IN_PROGRESS". If other code paths ever createWorkflowExecutionrows, you may want a sensible default (e.g.,PENDING) or to explicitly document thatstatusmust always be set by application code.ee/features/workflows/lib/types.ts (2)
48-54: Step type literal should stay in sync with Prisma enum
stepType: z.literal("ROUTER")assumes the only valid step type isROUTER, matching theWorkflowStepTypeenum. If you add more enum variants later, remember to widen this schema (e.g., to az.enum([...])) to avoid mismatches between API validation and DB.
88-92: Update step actions schema to keep error messages consistent
actionshere usesz.array(ActionSchema).min(1).optional(), but the.min(1)has no custom message like the create schema. If you rely on consistent client‑facing messages, you may want to add the same"At least one action is required"text.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
app/(ee)/api/workflow-entry/domains/[...domainSlug]/route.ts(1 hunks)app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts(1 hunks)app/(ee)/api/workflow-entry/link/[entryLinkId]/verify/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/executions/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/route.ts(1 hunks)app/(ee)/api/workflows/route.ts(1 hunks)components/sidebar/app-sidebar.tsx(5 hunks)components/ui/single-select.tsx(1 hunks)ee/features/workflows/README.md(1 hunks)ee/features/workflows/components/index.ts(1 hunks)ee/features/workflows/components/step-form-dialog.tsx(1 hunks)ee/features/workflows/components/step-list.tsx(1 hunks)ee/features/workflows/components/workflow-access-view.tsx(1 hunks)ee/features/workflows/components/workflow-empty-state.tsx(1 hunks)ee/features/workflows/components/workflow-header.tsx(1 hunks)ee/features/workflows/components/workflow-list.tsx(1 hunks)ee/features/workflows/index.ts(1 hunks)ee/features/workflows/lib/engine.ts(1 hunks)ee/features/workflows/lib/types.ts(1 hunks)ee/features/workflows/lib/validation.ts(1 hunks)ee/features/workflows/pages/workflow-detail.tsx(1 hunks)ee/features/workflows/pages/workflow-new.tsx(1 hunks)ee/features/workflows/pages/workflow-overview.tsx(1 hunks)lib/auth/link-session.ts(1 hunks)lib/featureFlags/index.ts(2 hunks)pages/api/links/[id]/index.ts(1 hunks)pages/api/links/domains/[...domainSlug].ts(1 hunks)pages/api/teams/[teamId]/workflow-links.ts(1 hunks)pages/view/[linkId]/index.tsx(5 hunks)pages/view/domains/[domain]/[slug]/index.tsx(6 hunks)pages/workflows/[id].tsx(1 hunks)pages/workflows/index.tsx(1 hunks)pages/workflows/new.tsx(1 hunks)prisma/migrations/20251116000000_add_workflow/migration.sql(1 hunks)prisma/schema/link.prisma(2 hunks)prisma/schema/team.prisma(1 hunks)prisma/schema/workflow.prisma(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-19T07:46:44.421Z
Learnt from: CR
Repo: mfts/papermark PR: 0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing schema-validated tasks, use `schemaTask` from `trigger.dev/sdk/v3` and provide a schema using Zod or another supported library.
Applied to files:
ee/features/workflows/lib/validation.tsee/features/workflows/lib/types.ts
🧬 Code graph analysis (25)
pages/workflows/[id].tsx (1)
ee/features/workflows/pages/workflow-detail.tsx (1)
WorkflowDetailPage(67-192)
app/(ee)/api/workflows/route.ts (5)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (1)
CreateWorkflowRequestSchema(63-74)ee/features/workflows/lib/validation.ts (2)
CreateWorkflowRequestSchema(16-16)formatZodError(90-92)lib/featureFlags/index.ts (1)
getFeatureFlags(19-61)
ee/features/workflows/components/workflow-empty-state.tsx (1)
ee/features/workflows/components/index.ts (1)
WorkflowEmptyState(2-2)
app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts (6)
app/(ee)/api/workflow-entry/link/[entryLinkId]/verify/route.ts (1)
POST(12-127)ee/features/workflows/lib/types.ts (1)
AccessRequestSchema(107-110)lib/utils/geo.ts (1)
LOCALHOST_IP(34-34)lib/redis.ts (1)
ratelimit(15-30)ee/features/workflows/lib/engine.ts (1)
WorkflowEngine(13-348)lib/auth/link-session.ts (1)
createLinkSession(35-85)
pages/api/teams/[teamId]/workflow-links.ts (2)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)
app/(ee)/api/workflows/[workflowId]/executions/route.ts (3)
app/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
GET(17-112)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)
ee/features/workflows/components/workflow-access-view.tsx (3)
components/view/access-form/index.tsx (2)
DEFAULT_ACCESS_FORM_TYPE(21-27)AccessForm(29-228)pages/404.tsx (1)
NotFound(3-34)components/view/access-form/email-verification-form.tsx (1)
EmailVerificationMessage(18-168)
ee/features/workflows/pages/workflow-detail.tsx (7)
pages/workflows/[id].tsx (1)
WorkflowDetailPage(3-5)context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/layouts/app.tsx (1)
AppLayout(16-41)ee/features/workflows/components/workflow-header.tsx (1)
WorkflowHeader(26-171)ee/features/workflows/components/step-list.tsx (1)
StepList(37-111)ee/features/workflows/components/step-form-dialog.tsx (1)
StepFormDialog(62-356)
ee/features/workflows/components/workflow-list.tsx (1)
lib/utils.ts (1)
timeAgo(165-182)
app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts (5)
app/(ee)/api/workflows/[workflowId]/route.ts (2)
PATCH(101-191)DELETE(194-274)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)ee/features/workflows/lib/types.ts (1)
UpdateWorkflowStepRequestSchema(88-92)ee/features/workflows/lib/validation.ts (4)
UpdateWorkflowStepRequestSchema(19-19)formatZodError(90-92)validateConditions(25-40)validateActions(45-63)lib/types.ts (1)
CustomUser(17-17)
pages/view/[linkId]/index.tsx (1)
ee/features/workflows/components/workflow-access-view.tsx (1)
WorkflowAccessView(19-203)
components/sidebar/app-sidebar.tsx (2)
lib/hooks/use-feature-flags.ts (1)
useFeatureFlags(10-31)components/sidebar/nav-main.tsx (1)
NavMain(46-125)
app/(ee)/api/workflow-entry/link/[entryLinkId]/verify/route.ts (4)
ee/features/workflows/lib/types.ts (1)
VerifyEmailRequestSchema(103-105)lib/redis.ts (1)
ratelimit(15-30)lib/utils/generate-otp.ts (1)
generateOTP(1-9)lib/emails/send-email-otp-verification.ts (1)
sendOtpVerificationEmail(8-43)
app/(ee)/api/workflows/[workflowId]/steps/route.ts (4)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (2)
CreateWorkflowStepRequestSchema(82-86)ReorderStepsRequestSchema(94-101)ee/features/workflows/lib/validation.ts (4)
CreateWorkflowStepRequestSchema(18-18)formatZodError(90-92)validateConditions(25-40)validateActions(45-63)
ee/features/workflows/pages/workflow-new.tsx (3)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/layouts/app.tsx (1)
AppLayout(16-41)
ee/features/workflows/lib/engine.ts (1)
ee/features/workflows/lib/types.ts (5)
WorkflowExecutionContext(126-132)WorkflowExecutionResult(134-143)ConditionsConfigSchema(43-46)ActionSchema(36-36)Condition(24-24)
ee/features/workflows/components/step-form-dialog.tsx (2)
components/ui/single-select.tsx (1)
SingleSelect(93-250)lib/utils.ts (1)
cn(18-20)
app/(ee)/api/workflow-entry/domains/[...domainSlug]/route.ts (9)
app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts (1)
POST(16-277)app/(ee)/api/workflow-entry/link/[entryLinkId]/verify/route.ts (1)
POST(12-127)ee/features/workflows/lib/types.ts (2)
VerifyEmailRequestSchema(103-105)AccessRequestSchema(107-110)lib/redis.ts (1)
ratelimit(15-30)lib/utils/generate-otp.ts (1)
generateOTP(1-9)lib/emails/send-email-otp-verification.ts (1)
sendOtpVerificationEmail(8-43)lib/utils/geo.ts (1)
LOCALHOST_IP(34-34)ee/features/workflows/lib/engine.ts (1)
WorkflowEngine(13-348)lib/auth/link-session.ts (1)
createLinkSession(35-85)
ee/features/workflows/components/workflow-header.tsx (2)
ee/features/workflows/components/index.ts (1)
WorkflowHeader(3-3)lib/utils.ts (1)
copyToClipboard(268-277)
ee/features/workflows/lib/validation.ts (1)
ee/features/workflows/lib/types.ts (6)
ConditionsConfig(56-56)ConditionsConfigSchema(43-46)Action(39-39)ActionSchema(36-36)WorkflowStepDefinition(57-59)WorkflowStepDefinitionSchema(48-54)
pages/view/domains/[domain]/[slug]/index.tsx (1)
ee/features/workflows/components/workflow-access-view.tsx (1)
WorkflowAccessView(19-203)
app/(ee)/api/workflows/[workflowId]/route.ts (4)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (1)
UpdateWorkflowRequestSchema(76-80)ee/features/workflows/lib/validation.ts (2)
UpdateWorkflowRequestSchema(17-17)formatZodError(90-92)
components/ui/single-select.tsx (1)
lib/utils.ts (1)
cn(18-20)
ee/features/workflows/pages/workflow-overview.tsx (7)
context/team-context.tsx (1)
useTeam(87-87)lib/swr/use-billing.ts (1)
usePlan(86-136)lib/utils.ts (1)
fetcher(48-62)components/layouts/app.tsx (1)
AppLayout(16-41)components/billing/plan-badge.tsx (1)
PlanBadge(5-22)ee/features/workflows/components/workflow-empty-state.tsx (1)
WorkflowEmptyState(8-25)ee/features/workflows/components/workflow-list.tsx (1)
WorkflowList(26-74)
lib/auth/link-session.ts (2)
lib/redis.ts (1)
redis(4-7)lib/utils/geo.ts (1)
LOCALHOST_IP(34-34)
🪛 GitHub Check: CodeQL
ee/features/workflows/pages/workflow-detail.tsx
[failure] 100-105: Server-side request forgery
The URL of this request depends on a user-provided value.
ee/features/workflows/components/step-form-dialog.tsx
[failure] 186-192: Server-side request forgery
The URL of this request depends on a user-provided value.
ee/features/workflows/components/workflow-header.tsx
[failure] 40-48: Server-side request forgery
The URL of this request depends on a user-provided value.
[failure] 65-67: Server-side request forgery
The URL of this request depends on a user-provided value.
🪛 LanguageTool
ee/features/workflows/README.md
[uncategorized] ~52-~52: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ty**: OTP verification, Redis sessions, rate limiting - Audit Trail: Complete execution l...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
ee/features/workflows/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (28)
prisma/schema/team.prisma (1)
31-31: Team–Workflow relation looks consistentAdding
workflows Workflow[]cleanly models the one-to-many relation fromTeamtoWorkflow, assumingWorkflowalready has ateamIdrelation field. No further schema changes needed here.pages/api/teams/[teamId]/workflow-links.ts (1)
13-107: Workflow link source API looks solidAuth, team‑membership check, and link filtering (team‑scoped, non‑deleted, non‑archived, document/dataroom only) are all handled cleanly. The
displayNamederivation logic provides a sensible priority order and the response shape matches the needs of the step builder UI.pages/workflows/new.tsx (1)
1-5: LGTM!Clean wrapper page following standard Next.js patterns.
pages/workflows/[id].tsx (1)
1-5: LGTM!Standard Next.js dynamic route wrapper pattern, consistent with the codebase.
ee/features/workflows/index.ts (1)
1-10: LGTM!Proper barrel export pattern providing a clean public API for the workflows feature module.
pages/workflows/index.tsx (1)
1-5: LGTM!Clean page wrapper following established patterns in the codebase.
ee/features/workflows/components/workflow-empty-state.tsx (1)
1-26: LGTM!Well-structured presentational component with proper TypeScript typing and consistent styling patterns.
prisma/schema/link.prisma (2)
4-4: LGTM!Proper extension of the LinkType enum to support the new workflow feature.
91-92: LGTM!The optional workflow relation is correctly defined, allowing links to optionally reference workflows for workflow entry functionality.
components/sidebar/app-sidebar.tsx (1)
20-20: LGTM!Proper integration of feature flags for conditional workflow navigation.
Also applies to: 23-23, 77-77
ee/features/workflows/components/index.ts (1)
1-6: Centralized workflow component exports look goodThe barrel re-exports are clear and make the workflows UI components easier to consume.
ee/features/workflows/components/workflow-access-view.tsx (1)
35-66: OTP access flow and endpoint selection are coherentEndpoint construction, request/verify branching, and 403/404 handling look consistent with the intended workflow-entry API surface. The state transitions (
showVerification,showNotFound,isInvalidCode) are straightforward and should give a predictable UX.Also applies to: 107-167
ee/features/workflows/pages/workflow-detail.tsx (1)
98-116: CodeQL SSRF warning here is a false positiveThe
fetch(\/api/workflows/${workflowId}/steps/${stepId}`)call is a client-side request to a same-origin relative URL.workflowIdcomes from the route andstepIdfromworkflow.steps`, not from arbitrary user input used to choose an external host. This does not introduce server-side request forgery and can be considered safe.ee/features/workflows/pages/workflow-overview.tsx (1)
35-91: Workflows overview gating and states look consistentThe plan-based gating (
requiresUpgrade), SWR fetch of team workflows, and the conditional rendering of upgrade prompt, loading, empty, and list states are all wired cleanly and align with the rest of the UI patterns.pages/view/domains/[domain]/[slug]/index.tsx (1)
41-47: WORKFLOW_LINK integration into domain view is well-scoped and consistentThe new
WorkflowLinkDatatype,getStaticPropsWORKFLOW_LINK branch, and theViewPageruntime branch collectively look correct:
- Domain and slug are strictly validated and safely encoded before the internal API fetch, so the new path doesn’t introduce SSRF or host injection.
- For workflow links you return a minimal prop set (no Notion/page data) with sensible meta defaults and a short revalidation window, which fits the access-only use case.
- The render branch cleanly delegates to
WorkflowAccessViewwithentryLinkId,domain,slug, andbrand, matching how the linkId-based workflow view is wired elsewhere.I don’t see any functional or security issues in this new workflow-path integration.
Also applies to: 73-114, 283-314, 358-382
app/(ee)/api/workflows/[workflowId]/route.ts (2)
14-91: GET workflow handler: auth, validation, and access control look solidYou authenticate, validate
workflowIdas a cuid, enforce team membership viaworkflow.team.users, and return workflow data plusentryUrlwhile omitting team data. This is a clean, well‑scoped read handler.
100-191: PATCH handler correctly scopes updates to validated fieldsUsing
UpdateWorkflowRequestSchema.safeParseand passing onlyvalidation.dataintoprisma.workflow.updatekeeps updates constrained to the expected fields, and the team membership check mirrors the GET path. The differing response shape (noentryUrl) is fine as long as callers don’t assume it matches GET.app/(ee)/api/workflows/route.ts (1)
15-88: GET /workflows: team scoping and validation look goodThe handler cleanly validates
teamId, verifies membership viauserTeam, and scopes the query to that team before returning workflows with entryLink and counts. No obvious issues here.pages/view/[linkId]/index.tsx (2)
82-120: WORKFLOW_LINK branch in getStaticProps is lean and well isolatedShort‑circuiting on
linkType === "WORKFLOW_LINK"to return minimal props (linkData + basic meta, no Notion work) keeps this path efficient and avoids coupling workflows to document/dataroom loading. The returned object matches theWorkflowLinkDatashape.
369-388: WorkflowAccessView integration matches the expected propsBranching on
linkType === "WORKFLOW_LINK"and passingentryLinkIdandbrandintoWorkflowAccessView, while still wrapping withCustomMetaTag, integrates the new workflow access flow cleanly without impacting the existing DOCUMENT_LINK/DATAROOM_LINK rendering.app/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
285-364: Reorder endpoint is straightforward and consistentThe PUT handler’s validation, ownership check, and
$transactionupdate ofstepOrderlook correct and match the patterns used elsewhere in the workflows routes. Any invalidstepId/workflowIdcombination will naturally surface as a Prisma error, which is acceptable for now.ee/features/workflows/lib/engine.ts (3)
21-52: Workflow existence/activation guards look solidEarly returns for missing/inactive workflows and missing steps keep execution simple and predictable; this section reads clean and correct.
249-266: Condition aggregation logic is clear and correctTreating missing/empty condition lists as “always pass” and using
AND/ORwithevery/someover the evaluation results matches the expected semantics and keeps this helper easy to reason about.
325-347: Domain extraction and operators look goodDeriving the domain via
email.split("@")[1]?.toLowerCase()with a quick falsy check, and supportingequals/contains/in_list, is straightforward and behaves as expected for malformed emails (returnsfalse).prisma/schema/workflow.prisma (1)
72-88: Step log model is well‑structured for observabilityCapturing
conditionsMatched, the config (conditionResults), executed actions, duration, and error per step gives you a solid base for debugging and analytics, and matches how the engine writes logs.ee/features/workflows/lib/types.ts (3)
5-25: Condition schemas and discriminated union look consistentEmail/domain conditions are clearly separated and combined via a
type‑discriminated union, which aligns well with the engine’s condition evaluation switch.
28-40: Route action schema matches engine expectationsConstraining
targetLinkIdto a CUID and keeping document/dataroom IDs optional fits how the engine consumes actions today (single route action, primary key on link). This looks good.
126-150: Execution context/result types are aligned with engine usageThe fields here map cleanly to what
WorkflowEngine.executeconsumes and returns, which should keep the API surfaces and engine contract in sync.
| const { name, conditions, actions } = validation.data; | ||
|
|
||
| // Validate conditions and actions | ||
| const conditionsValidation = validateConditions(conditions); | ||
| if (!conditionsValidation.valid) { | ||
| return NextResponse.json( | ||
| { error: conditionsValidation.error }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| const actionsValidation = validateActions(actions); | ||
| if (!actionsValidation.valid) { | ||
| return NextResponse.json( | ||
| { error: actionsValidation.error }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Inconsistent use of validated actions and unconditional link update can cause subtle issues
In POST, you validate actions twice and then mix the two representations:
validation.data.actions(fromCreateWorkflowStepRequestSchema) is used to locate and mutaterouteAction(addingtargetDocumentId/targetDataroomId).actionsValidation.data(fromvalidateActions) is what you actually persist on the step.
Because validateActions re-parses each action via ActionSchema.parse, the objects in actionsValidation.data are not guaranteed to be the same references that you mutated when enriching routeAction. This means the enriched targetDocumentId / targetDataroomId may not be stored on the created step, while PATCH does persist them via validation.data. That asymmetry can be surprising if you later rely on those fields at execution time.
Also, the transaction always attempts a prisma.link.update using routeAction.targetLinkId, even though the earlier validation and enrichment are guarded by if (routeAction && routeAction.targetLinkId). If the schema ever allows non‑route actions or a route without targetLinkId, this will throw before Prisma sees the transaction.
I’d tighten this up by:
- Operating on a single validated
actionsarray (fromvalidateActions) for both enrichment and persistence. - Only adding the
link.updatestep to the transaction when you actually have arouteActionwith atargetLinkId.
For example:
- const { name, conditions, actions } = validation.data;
+ const { name, conditions, actions } = validation.data;
@@
- const actionsValidation = validateActions(actions);
+ const actionsValidation = validateActions(actions);
@@
- // Validate target link exists and belongs to the team
- const routeAction = actions.find((a) => a.type === "route");
- if (routeAction && routeAction.targetLinkId) {
+ // Work on validated actions and validate target link
+ const validatedActions = actionsValidation.data;
+ const routeAction = validatedActions.find((a) => a.type === "route");
+ if (routeAction && routeAction.targetLinkId) {
@@
- const allowListItems: string[] = [];
- if (conditionsValidation.data.items) {
- conditionsValidation.data.items.forEach((condition: any) => {
+ const allowListItems: string[] = [];
+ if (conditionsValidation.data.items) {
+ conditionsValidation.data.items.forEach((condition: any) => {
@@
- const [newStep] = await prisma.$transaction([
- prisma.workflowStep.create({
- data: {
- workflowId,
- name,
- stepOrder: nextStepOrder,
- stepType: "ROUTER",
- conditions: conditionsValidation.data as any,
- actions: actionsValidation.data as any,
- },
- }),
- // Update target link's allowList
- prisma.link.update({
- where: { id: routeAction.targetLinkId },
- data: {
- allowList: allowListItems,
- },
- }),
- ]);
+ const tx: any[] = [
+ prisma.workflowStep.create({
+ data: {
+ workflowId,
+ name,
+ stepOrder: nextStepOrder,
+ stepType: "ROUTER",
+ conditions: conditionsValidation.data as any,
+ actions: validatedActions as any,
+ },
+ }),
+ ];
+
+ if (routeAction?.targetLinkId) {
+ tx.push(
+ prisma.link.update({
+ where: { id: routeAction.targetLinkId },
+ data: {
+ allowList: allowListItems,
+ },
+ }),
+ );
+ }
+
+ const [newStep] = await prisma.$transaction(tx);This keeps create/update behavior aligned and avoids relying on assumptions about Zod’s object identity.
Also applies to: 205-272
🤖 Prompt for AI Agents
In app/(ee)/api/workflows/[workflowId]/steps/route.ts around lines 149-167 (and
similarly at 205-272), you validate actions twice and mutate objects from
validation.data but persist actions from validateActions(), causing enriched
fields (targetDocumentId/targetDataroomId) to be lost; also the
prisma.link.update is unconditionally added to the transaction and can throw if
routeAction or targetLinkId is absent. Fix by using the single validated actions
array returned by validateActions() for both enrichment and persistence (apply
mutations to actionsValidation.data), and only push the prisma.link.update step
into the transaction when you have a route action with a truthy targetLinkId;
mirror the same change in the PATCH section at lines 205-272 so create/update
behavior is consistent.
| { | ||
| title: "Workflows", | ||
| url: "/workflows", | ||
| icon: WorkflowIcon, | ||
| current: router.pathname.includes("/workflows"), | ||
| disabled: !features?.workflows, | ||
| trigger: "sidebar_workflows", | ||
| plan: PlanEnum.DataRoomsPlus, | ||
| highlightItem: ["workflows"], | ||
| }, |
There was a problem hiding this comment.
Remove redundant filtering logic to preserve upgrade prompt visibility.
The Workflows navigation item has redundant visibility control:
- Line 160 marks it as
disabled: !features?.workflows - Lines 221-222 filter it out entirely when
!features?.workflows
This creates an inconsistency with other navigation items (Datarooms, Visitors) that use the disabled flag to show upgrade prompts via UpgradePlanModal. The filtering at lines 218-225 prevents the upgrade prompt from displaying, which may not be the intended behavior.
Recommendation: Choose one approach:
- Option 1 (Recommended): Remove the filtering logic (lines 218-225) and rely solely on the
disabledflag to show upgrade prompts - Option 2: Remove the
disabledflag (line 160) and use only filtering if Workflows should be completely hidden
Apply this diff for Option 1 (show upgrade prompt):
- // Filter out items that should be hidden based on feature flags
- const filteredNavMain = data.navMain.filter((item) => {
- // Hide workflows if feature flag is not enabled
- if (item.title === "Workflows" && !features?.workflows) {
- return false;
- }
- return true;
- });
-
return (
<Sidebar
className="bg-gray-50 dark:bg-black"
@@ -270,7 +263,7 @@
</SidebarHeader>
<SidebarContent>
- <NavMain items={filteredNavMain} />
+ <NavMain items={data.navMain} />
</SidebarContent>Or apply this diff for Option 2 (completely hide):
{
title: "Workflows",
url: "/workflows",
icon: WorkflowIcon,
current: router.pathname.includes("/workflows"),
- disabled: !features?.workflows,
- trigger: "sidebar_workflows",
- plan: PlanEnum.DataRoomsPlus,
- highlightItem: ["workflows"],
},Also applies to: 218-225
🤖 Prompt for AI Agents
In components/sidebar/app-sidebar.tsx around lines 155-164 and 218-225, the
Workflows nav item is both marked disabled (disabled: !features?.workflows) and
also filtered out later which prevents the UpgradePlanModal from ever appearing;
remove the filtering logic that excludes the workflows entry when
!features?.workflows (i.e., stop filtering out items based on workflows feature)
so the item remains in the nav array and the disabled flag controls
visibility/upgrade prompt; alternatively, if you truly want it hidden, remove
the disabled flag from the Workflows item instead — prefer Option 1: delete the
filter that removes workflows and rely on disabled to surface the upgrade
prompt.
| linkType: z.enum(["DOCUMENT_LINK", "DATAROOM_LINK", "WORKFLOW_LINK"]), | ||
| // Security enhancements | ||
| accessCount: z.number().default(0), | ||
| maxAccesses: z.number().default(1000), | ||
| lastAccessedAt: z.number(), | ||
| createdAt: z.number(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
linkType parameter should include WORKFLOW_LINK to match the schema
LinkSessionSchema.linkType allows "DOCUMENT_LINK" | "DATAROOM_LINK" | "WORKFLOW_LINK", but createLinkSession currently restricts its linkType parameter to only "DOCUMENT_LINK" | "DATAROOM_LINK". That prevents callers from creating sessions for workflow links despite the schema (and PR) supporting WORKFLOW_LINK.
You can tighten this up and avoid divergence by reusing the schema’s type:
-export async function createLinkSession(
- linkId: string,
- linkType: "DOCUMENT_LINK" | "DATAROOM_LINK",
+export async function createLinkSession(
+ linkId: string,
+ linkType: LinkSession["linkType"],This keeps all allowed variants in one place and ensures future additions to the enum are automatically reflected in the function signature.
Also applies to: 35-46
I'll verify the review comment by inspecting the actual code in the repository.
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the link-session.ts file to see the full context including the schema and createLinkSession function
cat lib/auth/link-session.ts
Length of output: 5046
linkType parameter should include WORKFLOW_LINK to match the schema
The createLinkSession function signature at lines 35-40 restricts linkType to "DOCUMENT_LINK" | "DATAROOM_LINK", while the LinkSessionSchema enum (line 25) explicitly includes "WORKFLOW_LINK". This prevents callers from creating sessions for workflow links despite the schema and PR supporting it.
Align the function parameter with the schema using:
-export async function createLinkSession(
- linkId: string,
- linkType: "DOCUMENT_LINK" | "DATAROOM_LINK",
+export async function createLinkSession(
+ linkId: string,
+ linkType: LinkSession["linkType"],This ensures all schema variants are supported and future enum additions are automatically reflected.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/auth/link-session.ts around lines 25 to 40, the function
createLinkSession limits the linkType parameter to "DOCUMENT_LINK" |
"DATAROOM_LINK" while the LinkSessionSchema enum (line 25) also contains
"WORKFLOW_LINK"; update the function signature to accept the full schema enum
instead of a narrow union — either use the generated/defined LinkType type (or
z.infer from LinkSessionSchema) or include "WORKFLOW_LINK" in the union so the
parameter matches the schema and future enum additions are reflected
automatically.
| await redis.set(`link_session:${sessionToken}`, JSON.stringify(sessionData), { | ||
| pxat: expiresAt, | ||
| }); | ||
|
|
||
| // Track active sessions per viewer (for revocation) |
There was a problem hiding this comment.
🧩 Analysis chain
Redis session value must be JSON‑parsed before Zod validation
You JSON.stringify(sessionData) on write, but in verifyLinkSession and revokeLinkSession you pass the raw string from redis.get directly into LinkSessionSchema.parse. Since the schema expects an object, this will throw and cause verification to always fail (and delete the session).
Consider parsing the JSON first in both places:
- const session = await redis.get(`link_session:${sessionToken}`);
-
- if (!session) return null;
-
- try {
- const sessionData = LinkSessionSchema.parse(session);
+ const sessionJson = await redis.get(`link_session:${sessionToken}`);
+
+ if (!sessionJson) return null;
+
+ try {
+ const parsed = JSON.parse(sessionJson);
+ const sessionData = LinkSessionSchema.parse(parsed);
@@
- await redis.set(
- `link_session:${sessionToken}`,
- JSON.stringify(sessionData),
- { pxat: sessionData.expiresAt },
- );
+ await redis.set(
+ `link_session:${sessionToken}`,
+ JSON.stringify(sessionData),
+ { pxat: sessionData.expiresAt },
+ );And in revokeLinkSession:
- const session = await redis.get(`link_session:${sessionToken}`);
- if (session) {
- const sessionData = LinkSessionSchema.parse(session);
+ const sessionJson = await redis.get(`link_session:${sessionToken}`);
+ if (sessionJson) {
+ const parsed = JSON.parse(sessionJson);
+ const sessionData = LinkSessionSchema.parse(parsed);
await deleteLinkSession(sessionToken, sessionData.viewerId);
}This aligns the read path with how sessions are written and lets Zod validate the actual object structure.
Also applies to: 95-101, 148-153, 176-179
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the full link-session.ts file to verify the JSON parsing claims
cat lib/auth/link-session.ts
Length of output: 5046
🏁 Script executed:
#!/bin/bash
# Search for LinkSessionSchema definition
rg -n "LinkSessionSchema|link.*schema" --type=ts -A5 -B5Length of output: 4005
🏁 Script executed:
#!/bin/bash
# Check how redis.get is used throughout the file
rg -n "redis\.get.*link_session" lib/auth/link-session.ts -A3 -B1Length of output: 373
Fix critical JSON parsing bug in session verification and revoke functions, and align linkType parameter with schema
Verification confirms the review comment is accurate. The code stores sessions as JSON strings but reads them without parsing, causing LinkSessionSchema.parse() to fail immediately on the string value (not the parsed object). This causes the catch block to execute on every verification attempt, deleting the session and always returning null. Additionally, the createLinkSession function's linkType parameter excludes WORKFLOW_LINK even though the schema supports it.
Required fixes:
- verifyLinkSession (lines 95–100): Add
JSON.parse()before schema validation - revokeLinkSession (lines 176–178): Add
JSON.parse()before schema validation - createLinkSession (line 37): Expand
linkTypeparameter to include"WORKFLOW_LINK"
🤖 Prompt for AI Agents
In lib/auth/link-session.ts (context around lines 37, 71–75, 95–100 and
176–178), the code stores sessionData as JSON strings but validates the Redis
return value directly with LinkSessionSchema, causing schema.parse to fail on
the raw string and immediately trigger the catch that deletes the session; also
the createLinkSession declaration omits "WORKFLOW_LINK" from the linkType
parameter. Fix by, in verifyLinkSession (around lines 95–100) and
revokeLinkSession (around lines 176–178), calling JSON.parse(...) on the Redis
value before passing it to LinkSessionSchema.parse; and update the
createLinkSession function signature at line 37 to allow linkType to include
"WORKFLOW_LINK" (expand the union/type accordingly).
| // Handle workflow links separately | ||
| if (linkType === "WORKFLOW_LINK") { | ||
| // For workflow links, fetch brand if available | ||
| let brand: Partial<Brand> | null = null; | ||
| if (link.teamId) { | ||
| const teamBrand = await prisma.brand.findUnique({ | ||
| where: { teamId: link.teamId }, | ||
| select: { | ||
| logo: true, | ||
| brandColor: true, | ||
| accentColor: true, | ||
| }, | ||
| }); | ||
| brand = teamBrand; | ||
| } | ||
|
|
||
| return res.status(200).json({ linkType, brand }); | ||
| } |
There was a problem hiding this comment.
Verify response format consistency and ensure linkId is included.
The WORKFLOW_LINK response format appears inconsistent with other link types:
- WORKFLOW_LINK returns:
{ linkType, brand } - Other link types (Line 187) return:
{ linkType, link: returnLink, brand }
Additionally, the response is missing the linkId field that may be needed by the frontend to identify the workflow entry link. Please verify:
- Whether the frontend expects a consistent response structure across all link types
- Whether
linkId(link.id) should be included in the response for WORKFLOW_LINK
Consider applying this diff to ensure consistency:
- return res.status(200).json({ linkType, brand });
+ return res.status(200).json({
+ linkType,
+ brand,
+ linkId: link.id
+ });🤖 Prompt for AI Agents
In pages/api/links/[id]/index.ts around lines 123–140, the WORKFLOW_LINK branch
returns { linkType, brand } which is inconsistent with other branches that
return { linkType, link: returnLink, brand } and also omits linkId; update this
branch to return the same structure by constructing a returnLink object (or
reuse the existing link data) that includes at minimum link.id as linkId and any
other fields the frontend expects, then respond with res.status(200).json({
linkType, link: returnLink, brand }) so the response shape matches other link
types and includes the linkId.
Refactors PATCH and POST handlers to use validated conditions and actions for workflow step updates and creation, ensuring enrichment and mutation persist. Also improves Zod error formatting in validation utilities for better readability.
Added a useEffect hook to re-populate form state when the dialog is opened or the step changes, preventing stale values in name, targetLinkId, and allowListInput fields.
Changed the 'domain' field in CreateWorkflowRequestSchema from optional to nullish, allowing both null and undefined values to indicate papermark.com as the default.
Changed the code field validation from fixed length to a regex that enforces exactly 6 digits. This ensures the verification code consists only of digits and is 6 characters long.
When the domain is 'papermark.com', the slug field is now set to undefined instead of a trimmed value. This aligns slug handling with domain-specific requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
app/(ee)/api/workflows/[workflowId]/executions/route.ts (1)
22-31: Pagination validation properly implemented.The pagination parameter validation correctly addresses the previous concerns by:
- Using
Number.parseIntwith explicit radix- Validating for
NaNand values less than 1- Enforcing minimum of 1 and maximum of 100 for limit
- Ensuring
Math.ceil(totalCount / limit)at line 106 remains safe and finiteapp/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
205-280: Validation and transaction logic properly implemented.The POST handler correctly addresses the previous concern by:
- Using
actionsValidation.datadirectly for both enrichment (lines 224-231) and persistence (line 263)- Conditionally adding the
link.updateto the transaction only whenrouteActionandtargetLinkIdexist (lines 269-278)- Ensuring enriched fields (
targetDocumentId/targetDataroomId) are persisted on the created stepThis eliminates the inconsistency between validation and persistence.
ee/features/workflows/components/step-form-dialog.tsx (1)
97-126: State reset logic properly implemented.The
useEffecthook correctly addresses the previous concern about stale form values by:
- Resetting all form fields (
name,targetLinkId,allowListInput) whensteporopenchanges- Handling both create (no step) and edit (with step) scenarios
- Safely accessing nested properties with optional chaining
This ensures the dialog always displays the correct data for the current step being edited or created.
ee/features/workflows/lib/types.ts (2)
63-74: Schema properly updated to accept null/undefined for domain field.The
domainfield now uses.nullish()(line 68), which correctly acceptsnull,undefined, or a string value. This aligns with the comment "null or undefined means papermark.com" and resolves the previous mismatch between the schema and documentation.
107-110: Code validation properly enforces numeric-only format.The
codefield now uses.regex(/^\d{6}$/)(line 109), which correctly enforces that verification codes must be exactly 6 numeric digits. This matches the error message and provides proper validation for OTP codes, addressing the previous concern.ee/features/workflows/pages/workflow-detail.tsx (1)
107-134: Fix SSRF vulnerability by using validated values.The function validates
workflowIdandstepIdbut then uses the original, unvalidated values from parameters in the fetch URL (lines 118-119). While validation ensures the format is correct, best practice requires explicitly using the validated values to prevent potential SSRF attacks.This is inconsistent with the safer pattern used on lines 78-83 where validated values are extracted and stored.
Apply this diff to use validated values:
const handleDeleteStep = async (stepId: string) => { // Validate IDs to prevent SSRF const workflowIdValidation = z.string().cuid().safeParse(workflowId); const stepIdValidation = z.string().cuid().safeParse(stepId); if (!workflowIdValidation.success || !stepIdValidation.success) { toast.error("Invalid workflow or step ID"); return; } + + const validatedWorkflowId = workflowIdValidation.data; + const validatedStepId = stepIdValidation.data; try { const response = await fetch( - `/api/workflows/${workflowId}/steps/${stepId}`, + `/api/workflows/${validatedWorkflowId}/steps/${validatedStepId}`, { method: "DELETE", }, );
🧹 Nitpick comments (3)
pages/api/teams/[teamId]/documents/[id]/views/index.ts (1)
181-190: Robust pagination validation looks good.The improved validation logic correctly handles invalid inputs and enforces sensible constraints. The defaults and bounds (page ≥ 1, 1 ≤ limit ≤ 100) will prevent common pagination errors.
One minor simplification: since you already handle
rawLimit < 1in the condition, theMath.max(rawLimit, 1)on line 190 is redundant—when that branch executes,rawLimitis guaranteed to be ≥ 1.Apply this diff to simplify:
- const limit = Number.isNaN(rawLimit) || rawLimit < 1 - ? 10 - : Math.min(Math.max(rawLimit, 1), 100); // Min 1, Max 100 + const limit = Number.isNaN(rawLimit) || rawLimit < 1 + ? 10 + : Math.min(rawLimit, 100); // Max 100ee/features/workflows/components/workflow-header.tsx (1)
97-100: Consider using the existingcopyToClipboardutility to reduce duplication.The codebase already has a
copyToClipboardutility inlib/utils.ts(lines 267-276) that provides the same functionality with better error handling. Consider importing and reusing it:+import { copyToClipboard } from "@/lib/utils"; - const copyToClipboard = (text: string) => { - navigator.clipboard.writeText(text); - toast.success("Copied to clipboard"); - };Then call it as:
onClick={() => copyToClipboard(entryUrl, "Copied to clipboard")}app/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
76-102: Consider batching link fetches to avoid N+1 queries.The current implementation fetches target links individually within
Promise.all, which results in N queries when there are N steps. For workflows with many steps, consider collecting all uniquetargetLinkIdvalues first, then fetching them in a single query withprisma.link.findMany({ where: { id: { in: linkIds } } })and creating a lookup map.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/(ee)/api/workflow-entry/domains/[...domainSlug]/route.ts(1 hunks)app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/executions/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/route.ts(1 hunks)ee/features/workflows/components/step-form-dialog.tsx(1 hunks)ee/features/workflows/components/workflow-header.tsx(1 hunks)ee/features/workflows/lib/types.ts(1 hunks)ee/features/workflows/lib/validation.ts(1 hunks)ee/features/workflows/pages/workflow-detail.tsx(1 hunks)ee/features/workflows/pages/workflow-new.tsx(1 hunks)pages/api/teams/[teamId]/documents/[id]/views/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/(ee)/api/workflow-entry/domains/[...domainSlug]/route.ts
- ee/features/workflows/pages/workflow-new.tsx
- app/(ee)/api/workflow-entry/link/[entryLinkId]/access/route.ts
- app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-19T07:46:44.421Z
Learnt from: CR
Repo: mfts/papermark PR: 0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing schema-validated tasks, use `schemaTask` from `trigger.dev/sdk/v3` and provide a schema using Zod or another supported library.
Applied to files:
ee/features/workflows/lib/validation.tsee/features/workflows/lib/types.ts
🧬 Code graph analysis (7)
app/(ee)/api/workflows/[workflowId]/steps/route.ts (3)
lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (2)
CreateWorkflowStepRequestSchema(82-86)ReorderStepsRequestSchema(94-101)ee/features/workflows/lib/validation.ts (5)
CreateWorkflowStepRequestSchema(20-20)formatZodError(99-103)validateConditions(28-45)validateActions(50-70)ReorderStepsRequestSchema(22-22)
ee/features/workflows/pages/workflow-detail.tsx (8)
pages/workflows/[id].tsx (1)
WorkflowDetailPage(3-5)context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/layouts/app.tsx (1)
AppLayout(16-41)ee/features/workflows/components/workflow-header.tsx (1)
WorkflowHeader(27-187)ee/features/workflows/components/index.ts (3)
WorkflowHeader(3-3)StepList(4-4)StepFormDialog(5-5)ee/features/workflows/components/step-list.tsx (1)
StepList(37-111)ee/features/workflows/components/step-form-dialog.tsx (1)
StepFormDialog(63-403)
app/(ee)/api/workflows/[workflowId]/executions/route.ts (5)
app/(ee)/api/workflows/[workflowId]/steps/route.ts (1)
GET(17-112)app/(ee)/api/workflows/[workflowId]/route.ts (1)
GET(15-98)app/(ee)/api/workflows/route.ts (1)
GET(16-88)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)
ee/features/workflows/lib/validation.ts (1)
ee/features/workflows/lib/types.ts (6)
ConditionsConfig(56-56)ConditionsConfigSchema(43-46)Action(39-39)ActionSchema(36-36)WorkflowStepDefinition(57-59)WorkflowStepDefinitionSchema(48-54)
ee/features/workflows/lib/types.ts (1)
ee/features/workflows/lib/validation.ts (5)
CreateWorkflowRequestSchema(18-18)UpdateWorkflowRequestSchema(19-19)CreateWorkflowStepRequestSchema(20-20)UpdateWorkflowStepRequestSchema(21-21)ReorderStepsRequestSchema(22-22)
ee/features/workflows/components/step-form-dialog.tsx (2)
components/ui/single-select.tsx (1)
SingleSelect(93-250)lib/utils.ts (1)
cn(18-20)
ee/features/workflows/components/workflow-header.tsx (2)
ee/features/workflows/components/index.ts (1)
WorkflowHeader(3-3)lib/utils.ts (1)
copyToClipboard(268-277)
🪛 GitHub Check: CodeQL
ee/features/workflows/pages/workflow-detail.tsx
[failure] 118-123: Server-side request forgery
The URL of this request depends on a user-provided value.
ee/features/workflows/components/step-form-dialog.tsx
[failure] 233-239: Server-side request forgery
The URL of this request depends on a user-provided value.
ee/features/workflows/components/workflow-header.tsx
[failure] 48-56: Server-side request forgery
The URL of this request depends on a user-provided value.
[failure] 81-83: Server-side request forgery
The URL of this request depends on a user-provided value.
⏰ 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). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
ee/features/workflows/lib/validation.ts (1)
28-103: LGTM! Clean validation utilities with consistent patterns.The validation functions follow good practices:
- Return discriminated unions (
{ valid: true; data: T }|{ valid: false; error: string }) for type-safe error handling- Consistent error formatting via
formatZodError- Proper error instanceof checks before accessing ZodError properties
- Clear JSDoc comments describing each function's purpose
ee/features/workflows/pages/workflow-detail.tsx (4)
1-66: LGTM! Well-structured imports and type definitions.The imports are appropriate and the TypeScript interfaces for
Workflow,WorkflowStep, andLinkare well-defined with proper nullable fields that align with the backend schema.
68-83: LGTM! Proper ID validation pattern.The validation of
workflowIdandteamIdusing zod CUID checks before constructing API URLs is a good security practice that helps prevent SSRF attacks. The pattern of extracting validated values (validWorkflowId,validTeamId) for subsequent use is well-implemented.
85-105: LGTM! Proper data fetching with validated IDs.The SWR hooks correctly use
validWorkflowIdandvalidTeamIdfor conditional fetching, ensuring API requests are only made with validated identifiers. ThegetEntryUrlfunction appropriately handles both custom domain and default marketing URL scenarios.
136-209: LGTM! Well-structured error handling and component composition.The error and loading states provide appropriate user feedback, and the main render properly composes
WorkflowHeader,StepList, andStepFormDialogwith correct prop passing and state management. The conditional rendering ofStepFormDialogcorrectly usesvalidWorkflowId.
| <main className="relative mx-2 mb-10 mt-4 space-y-8 overflow-hidden px-1 sm:mx-3 md:mx-5 md:mt-5 lg:mx-7 lg:mt-8 xl:mx-10"> | ||
| <div className="max-w-4xl space-y-6"> | ||
| <WorkflowHeader | ||
| workflowId={workflowId} |
There was a problem hiding this comment.
Consider using validWorkflowId for consistency.
The component passes the raw workflowId from router.query to WorkflowHeader, while the rest of the component uses the validated validWorkflowId (e.g., line 192). Although WorkflowHeader re-validates internally (defense in depth), using validWorkflowId here would maintain consistency with the validation pattern established elsewhere in the component.
<WorkflowHeader
- workflowId={workflowId}
+ workflowId={validWorkflowId!}
name={workflow.name}Note: The non-null assertion is safe here because we know workflow exists, which means validWorkflowId must be truthy (it's required to fetch the workflow).
📝 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.
| workflowId={workflowId} | |
| <WorkflowHeader | |
| workflowId={validWorkflowId!} | |
| name={workflow.name} |
🤖 Prompt for AI Agents
In ee/features/workflows/pages/workflow-detail.tsx around line 167, the
WorkflowHeader is passed the raw router.query workflowId instead of the
already-validated validWorkflowId used elsewhere; update the prop to pass
validWorkflowId (use a non-null assertion like validWorkflowId! if needed) so
the component uses the consistent, validated id throughout the file.
All workflow-related API routes now require a teamId query parameter and validate user membership via the userTeam table. This change improves security and access control, ensuring users can only access workflows and steps belonging to their teams. Frontend components and pages have been updated to pass teamId in API requests, and the workflow creation schema no longer requires teamId in the request body.
Refactored ID validation in workflow API routes for consistency and readability. Updated workflow engine to improve action result handling. Added 'WORKFLOW_LINK' to linkType enum in webhook schema. Improved type safety and code clarity in view pages, including dataroom lastUpdatedAt calculation and link prop usage.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pages/view/[linkId]/embed.tsx (2)
77-85: Inconsistent null check compared to DATAROOM_LINK branch.Line 79 checks
!props.linkData, butprops.linkDatawas already destructured on line 74, so this check is redundant. The DATAROOM_LINK branch (line 134) correctly checks!linkafter extraction, which is the appropriate pattern.Apply this diff to align with the DATAROOM_LINK branch:
if (linkType === "DOCUMENT_LINK") { const { link } = props.linkData; - if (!props.linkData || router.isFallback) { + if (!link || router.isFallback) { return ( <div className="flex h-screen items-center justify-center"> <LoadingSpinner className="h-20 w-20" />
77-180: Add WORKFLOW_LINK support or explicit fallback to embed.tsx.The component reuses
getStaticPropsfrom index.tsx (line 15), which generatesWORKFLOW_LINKprops (confirmed: lines 91–98 in index.tsx). However, embed.tsx only handlesDOCUMENT_LINKandDATAROOM_LINK, leavingWORKFLOW_LINKunhandled. WhenlinkType === "WORKFLOW_LINK", the component returnsundefined, causing a blank page or React error.Either add a
WORKFLOW_LINKhandler to embed.tsx or add an explicit fallback after theDATAROOM_LINKblock:// After the DATAROOM_LINK block return ( <NotFound message="This content type cannot be embedded." /> );
♻️ Duplicate comments (1)
ee/features/workflows/lib/engine.ts (1)
213-215: This unsafe cast was already flagged in a previous review. The issue remains:targetLink.linkTypemay includeWORKFLOW_LINKat runtime, but the cast narrows it to"DOCUMENT_LINK" | "DATAROOM_LINK"without validation. Add a runtime guard before this line to rejectWORKFLOW_LINKexplicitly or fail the execution with a clear error.
🧹 Nitpick comments (3)
ee/features/workflows/lib/engine.ts (1)
306-310: Log regex validation errors instead of silently returning false.When a user-supplied regex pattern is invalid (line 307), the catch block silently returns
false, making it difficult for administrators to debug misconfigured workflow steps. Consider logging the error to provide visibility into configuration issues:case "matches": try { return new RegExp(condition.value as string).test(normalizedEmail); - } catch { + } catch (error) { + console.error(`Invalid regex pattern in email condition: ${condition.value}`, error); return false; }ee/features/workflows/lib/types.ts (2)
5-15: Consider refining condition schemas to enforce operator-value type compatibility.The
valuefield currently accepts bothstringandstring[]for all operators, but semantically:
"in_list"requiresstring[]"equals","contains","matches"requirestringWhile the engine handles these at runtime, the schema doesn't prevent invalid combinations like
{ operator: "equals", value: ["array"] }. Consider adding a refinement to validate operator-value pairing, or document that runtime validation is intentionally deferred to the engine.
51-51: Consider future extensibility for additional step types.Line 51 uses
z.literal("ROUTER"), which hardcodes the only supported step type. If additional step types (e.g., transformations, notifications) are planned, consider usingz.enum(["ROUTER"])now to make future additions easier. However, ifROUTERis the only intended type for the foreseeable future, the current literal is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/(ee)/api/workflows/[workflowId]/executions/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts(1 hunks)app/(ee)/api/workflows/[workflowId]/steps/route.ts(1 hunks)app/(ee)/api/workflows/route.ts(1 hunks)components/domains/add-domain-modal.tsx(2 hunks)components/links/link-sheet/domain-section.tsx(3 hunks)components/links/link-sheet/index.tsx(2 hunks)components/links/link-sheet/link-options.tsx(1 hunks)ee/features/workflows/components/step-form-dialog.tsx(1 hunks)ee/features/workflows/components/workflow-header.tsx(1 hunks)ee/features/workflows/lib/engine.ts(1 hunks)ee/features/workflows/lib/types.ts(1 hunks)ee/features/workflows/pages/workflow-detail.tsx(1 hunks)ee/features/workflows/pages/workflow-new.tsx(1 hunks)lib/zod/schemas/webhooks.ts(1 hunks)pages/view/[linkId]/embed.tsx(3 hunks)pages/view/[linkId]/index.tsx(6 hunks)pages/view/domains/[domain]/[slug]/index.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/(ee)/api/workflows/[workflowId]/executions/route.ts
- app/(ee)/api/workflows/route.ts
- ee/features/workflows/pages/workflow-detail.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-19T07:46:44.421Z
Learnt from: CR
Repo: mfts/papermark PR: 0
File: .cursor/rules/rule-trigger-typescript.mdc:0-0
Timestamp: 2025-07-19T07:46:44.421Z
Learning: Applies to **/trigger/**/*.ts : When implementing schema-validated tasks, use `schemaTask` from `trigger.dev/sdk/v3` and provide a schema using Zod or another supported library.
Applied to files:
ee/features/workflows/lib/types.ts
🧬 Code graph analysis (9)
pages/view/domains/[domain]/[slug]/index.tsx (1)
ee/features/workflows/components/workflow-access-view.tsx (1)
WorkflowAccessView(19-203)
ee/features/workflows/pages/workflow-new.tsx (3)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/layouts/app.tsx (1)
AppLayout(16-41)
app/(ee)/api/workflows/[workflowId]/route.ts (5)
app/(ee)/api/workflows/route.ts (1)
GET(16-88)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (1)
UpdateWorkflowRequestSchema(75-79)ee/features/workflows/lib/validation.ts (2)
UpdateWorkflowRequestSchema(19-19)formatZodError(99-103)
app/(ee)/api/workflows/[workflowId]/steps/route.ts (4)
pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (2)
CreateWorkflowStepRequestSchema(81-85)ReorderStepsRequestSchema(93-100)ee/features/workflows/lib/validation.ts (5)
CreateWorkflowStepRequestSchema(20-20)formatZodError(99-103)validateConditions(28-45)validateActions(50-70)ReorderStepsRequestSchema(22-22)
app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts (5)
app/(ee)/api/workflows/[workflowId]/route.ts (2)
PATCH(114-221)DELETE(224-320)pages/api/auth/[...nextauth].ts (1)
authOptions(38-194)lib/types.ts (1)
CustomUser(17-17)ee/features/workflows/lib/types.ts (1)
UpdateWorkflowStepRequestSchema(87-91)ee/features/workflows/lib/validation.ts (4)
UpdateWorkflowStepRequestSchema(21-21)formatZodError(99-103)validateConditions(28-45)validateActions(50-70)
ee/features/workflows/components/step-form-dialog.tsx (2)
components/ui/single-select.tsx (1)
SingleSelect(93-250)lib/utils.ts (1)
cn(18-20)
ee/features/workflows/lib/engine.ts (1)
ee/features/workflows/lib/types.ts (6)
WorkflowExecutionContext(125-131)WorkflowExecutionResult(133-142)ConditionsConfigSchema(43-46)ActionSchema(36-36)Action(39-39)Condition(24-24)
pages/view/[linkId]/index.tsx (1)
ee/features/workflows/components/workflow-access-view.tsx (1)
WorkflowAccessView(19-203)
ee/features/workflows/components/workflow-header.tsx (2)
ee/features/workflows/components/index.ts (1)
WorkflowHeader(3-3)lib/utils.ts (1)
copyToClipboard(268-277)
🔇 Additional comments (6)
pages/view/[linkId]/embed.tsx (2)
7-8: LGTM: Clean import organization.The centralized analytics import improves code organization.
74-74: Solid refactoring approach for link data access.The pattern of extracting
linkTypeandbrandat the top level for routing, then extractinglinkwithin each branch ensures proper scoping and reduces the risk of accessing undefined values. This aligns well with the workflow routing changes mentioned in the PR objectives.Also applies to: 78-78, 133-133
ee/features/workflows/lib/engine.ts (2)
253-255: Confirm that steps with no conditions should always pass.Line 254 returns
truewhenconditionsConfig.itemsis empty or absent, meaning a step with no conditions will match every visitor. While this may be intentional for catch-all routing rules, it could also lead to unexpected behavior if a developer accidentally creates an empty step. Consider adding a workflow-level validation that warns or prevents steps with empty conditions unless explicitly intended as a default/fallback route.
329-330: LGTM!The optional chaining on line 329 and explicit falsy check on line 330 properly guard against malformed email addresses without an
@symbol.ee/features/workflows/lib/types.ts (2)
67-67: LGTM!Both previously flagged issues have been properly addressed:
- Line 67 now uses
.nullish()to accept bothnullandundefinedfor the domain field, consistent with the comment.- Line 108 now uses
.regex(/^\d{6}$/)to enforce numeric-only 6-digit verification codes.Also applies to: 108-108
136-136: Clarify whether workflows can route to WORKFLOW_LINK targets and add runtime guard.Line 136 explicitly excludes
WORKFLOW_LINKfromtargetLinkType, but there's no runtime validation inengine.ts(line 213) to enforce this constraint before casting. If workflows are intentionally prohibited from routing to other workflows, add a runtime check in the engine to reject such configurations with a clear error. Otherwise, includeWORKFLOW_LINKin this union type.
| const updates: any[] = [ | ||
| prisma.workflowStep.update({ | ||
| where: { id: stepId }, | ||
| data: updateData as any, | ||
| }), | ||
| ]; | ||
|
|
||
| // If we have allowList updates and a target link, sync the link | ||
| if (allowListItems !== undefined && targetLinkId) { | ||
| updates.push( | ||
| prisma.link.update({ | ||
| where: { id: targetLinkId }, | ||
| data: { allowList: allowListItems }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Updating a step still erases other recipients’ access
Here we replace the target link’s allowList with allowListItems derived from the step being edited. If multiple steps route to the same link, editing one of them strips every address added by the others, locking those users out. We need to preserve (or recompute) the union of recipients before updating the link.
After you determine targetLinkId, gather the existing link allow list (or aggregate from the other steps referencing that link) and write the merged result instead of allowListItems alone. For example, mirror the union logic suggested for the create route by selecting allowList when you load the link and using:
- if (allowListItems !== undefined && targetLinkId) {
+ if (allowListItems !== undefined && targetLinkId) {
+ const targetLink = await prisma.link.findUnique({
+ where: { id: targetLinkId },
+ select: { allowList: true },
+ });
+ const mergedAllowList = Array.from(
+ new Set([...(targetLink?.allowList ?? []), ...allowListItems]),
+ );
updates.push(
prisma.link.update({
where: { id: targetLinkId },
data: { allowList: allowListItems },
}),
);
}(Replace allowList: allowListItems with the merged list.) That keeps peers’ recipients intact while still allowing this step to add/remove its own entries.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/(ee)/api/workflows/[workflowId]/steps/[stepId]/route.ts around lines 221
to 235, the code currently overwrites the target link's allowList with only the
edited step's allowListItems which erases addresses added by other steps;
instead, fetch the current link.allowList and/or aggregate allowList entries
from all other workflow steps that reference the same targetLinkId (including
the edited step's new allowListItems), compute the union (deduplicated) of those
lists, and use that merged list when pushing the prisma.link.update into the
updates array; keep the update in the same transaction so both the workflowStep
update and the merged link.allowList update happen atomically.
| }), | ||
| ]; | ||
|
|
||
| // Only update link allowList if we have a route action with a target link | ||
| if (routeAction && routeAction.targetLinkId) { | ||
| transactionSteps.push( | ||
| prisma.link.update({ | ||
| where: { id: routeAction.targetLinkId }, | ||
| data: { | ||
| allowList: allowListItems, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Don’t wipe other recipients out of the link allow list
When we push the second transaction step (Line 303 onward) we overwrite the target link’s allowList with only allowListItems from the step being created. If two workflow steps point at the same document/dataroom (e.g., one for @company-a.com, another for @company-b.com), saving the second step will drop every address added by the first. Those users immediately lose access to the asset, which defeats the routing flow.
Please merge the new entries with what’s already on the link (or, even better, recompute the union across all steps that route to this link) before calling prisma.link.update. One way to do it:
- const routeAction = actionsValidation.data.find((a) => a.type === "route");
+ const routeAction = actionsValidation.data.find((a) => a.type === "route");
+ let nextAllowList: string[] | undefined;
@@
- const targetLink = await prisma.link.findUnique({
+ const targetLink = await prisma.link.findUnique({
where: {
id: routeAction.targetLinkId,
teamId: workflow.teamId,
},
+ select: {
+ allowList: true,
+ linkType: true,
+ documentId: true,
+ dataroomId: true,
+ },
});
@@
- if (targetLink.linkType === "DOCUMENT_LINK" && targetLink.documentId) {
+ if (targetLink.linkType === "DOCUMENT_LINK" && targetLink.documentId) {
routeAction.targetDocumentId = targetLink.documentId;
} else if (
targetLink.linkType === "DATAROOM_LINK" &&
targetLink.dataroomId
) {
routeAction.targetDataroomId = targetLink.dataroomId;
}
+
+ nextAllowList = Array.from(
+ new Set([...(targetLink.allowList ?? []), ...allowListItems]),
+ );
@@
- if (routeAction && routeAction.targetLinkId) {
+ if (routeAction && routeAction.targetLinkId && nextAllowList) {
transactionSteps.push(
prisma.link.update({
where: { id: routeAction.targetLinkId },
data: {
- allowList: allowListItems,
+ allowList: nextAllowList,
},
}),
);
}Also apply the same safeguard in the step PATCH handler so edits don’t clobber other cohorts.
Committable suggestion skipped: line range outside the PR's diff.
| linkType?: Omit<LinkType, "WORKFLOW_LINK">; | ||
| children?: React.ReactNode; |
There was a problem hiding this comment.
Fix linkType prop typing
Omit cannot narrow the Prisma enum; it produces a type error. Replace it with Exclude<LinkType, "WORKFLOW_LINK"> (and align imports/usages accordingly) so TypeScript accepts the change.
- linkType?: Omit<LinkType, "WORKFLOW_LINK">;
+ linkType?: Exclude<LinkType, "WORKFLOW_LINK">;📝 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.
| linkType?: Omit<LinkType, "WORKFLOW_LINK">; | |
| children?: React.ReactNode; | |
| linkType?: Exclude<LinkType, "WORKFLOW_LINK">; | |
| children?: React.ReactNode; |
🤖 Prompt for AI Agents
In components/domains/add-domain-modal.tsx around lines 39 to 40, the prop
typing uses Omit<LinkType, "WORKFLOW_LINK"> which cannot be used on a Prisma
enum and causes a TypeScript error; replace Omit with Exclude<LinkType,
"WORKFLOW_LINK"> for linkType?: Exclude<LinkType, "WORKFLOW_LINK"> and update
any related imports/usages to expect the Exclude-based type (run TypeScript to
ensure all call sites compile and adjust any runtime checks or narrowings if
needed).
| linkType: Omit<LinkType, "WORKFLOW_LINK">; | ||
| editLink?: boolean; |
There was a problem hiding this comment.
Use Exclude instead of Omit on Prisma enums
LinkType is a string-literal union; wrapping it in Omit<...> breaks type-checking because Omit expects an object type. Swap to Exclude<LinkType, "WORKFLOW_LINK"> (and update the other occurrences in this PR) so the code compiles.
- linkType: Omit<LinkType, "WORKFLOW_LINK">;
+ linkType: Exclude<LinkType, "WORKFLOW_LINK">;📝 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.
| linkType: Omit<LinkType, "WORKFLOW_LINK">; | |
| editLink?: boolean; | |
| linkType: Exclude<LinkType, "WORKFLOW_LINK">; | |
| editLink?: boolean; |
🤖 Prompt for AI Agents
In components/links/link-sheet/domain-section.tsx around lines 39 to 40, the
prop type uses Omit<LinkType, "WORKFLOW_LINK"> which is invalid because LinkType
is a string-literal union (Prisma enum); replace Omit with Exclude (i.e.,
Exclude<LinkType, "WORKFLOW_LINK">) to correctly remove that union member, and
search the PR for other usages of Omit with Prisma enums and update them to use
Exclude as well.
| linkType: Omit<LinkType, "WORKFLOW_LINK">, | ||
| groupId: string | null = null, |
There was a problem hiding this comment.
Replace Omit with Exclude for both linkType annotations
Omit expects an object type; using it on the Prisma enum makes TypeScript unhappy. Switch both the helper function parameter and the component prop to Exclude<LinkType, "WORKFLOW_LINK"> to narrow the union without compile-time fallout.
-export const DEFAULT_LINK_PROPS = (
- linkType: Omit<LinkType, "WORKFLOW_LINK">,
+export const DEFAULT_LINK_PROPS = (
+ linkType: Exclude<LinkType, "WORKFLOW_LINK">,
@@
- linkType: Omit<LinkType, "WORKFLOW_LINK">;
+ linkType: Exclude<LinkType, "WORKFLOW_LINK">;Also applies to: 150-151
🤖 Prompt for AI Agents
In components/links/link-sheet/index.tsx around lines 52-53 (and also update
lines 150-151), the code uses Omit<LinkType, "WORKFLOW_LINK"> which is incorrect
for a union/enum type; replace Omit with Exclude so the type becomes
Exclude<LinkType, "WORKFLOW_LINK"> for both the helper function parameter and
the component prop to properly narrow the union and fix the TypeScript error.
Ensure both occurrences are updated to Exclude<LinkType, "WORKFLOW_LINK">.
| linkType: Omit<LinkType, "WORKFLOW_LINK">; | ||
| editLink?: boolean; |
There was a problem hiding this comment.
Correct the linkType union narrowing
Applying Omit to LinkType breaks compilation; use Exclude<LinkType, "WORKFLOW_LINK"> to remove the workflow literal without invalidating the type.
- linkType: Omit<LinkType, "WORKFLOW_LINK">;
+ linkType: Exclude<LinkType, "WORKFLOW_LINK">;📝 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.
| linkType: Omit<LinkType, "WORKFLOW_LINK">; | |
| editLink?: boolean; | |
| linkType: Exclude<LinkType, "WORKFLOW_LINK">; | |
| editLink?: boolean; |
🤖 Prompt for AI Agents
In components/links/link-sheet/link-options.tsx around lines 89 to 90, the prop
type uses Omit<LinkType, "WORKFLOW_LINK"> which is invalid for a union of string
literals; replace it with Exclude<LinkType, "WORKFLOW_LINK"> so the
"WORKFLOW_LINK" literal is removed from the union without breaking compilation,
and update any imports/types if necessary to ensure LinkType remains the union
type.
Summary by CodeRabbit
New Features
Documentation
Chores