feat(platform): google drive integration + workflow installations#1650
Merged
Conversation
Adds a Google Drive sync example (integration + scheduled workflow) and the platform-side support it needs: - documents: open-string sourceProvider so new integration slugs don't require a schema change; reserved values upload/agent for non-integration sources. findDocumentByExternalId gains an optional folderId scope so the same external file synced to two Tale folders gets two doc rows. - document_action: new index_in_rag operation; create now upserts on externalItemId (folder-scoped) with content-hash skip and metadata passthrough; sourceProvider is forwarded. - workflows: schemas gain requires.integrations; createScheduleInternal validates dependencies and accepts variables, which scan_and_trigger now passes as workflow input. Schedule REST endpoints accept variables. - workflow loader: enabled:false maps to 'draft' (not 'inactive') so manual test runs work; auto-triggers still gated by enabled at start. - automations UI: tester pre-fills JSON input from the start step's inputSchema (without overwriting user edits); integration manage view renders setupGuide; CollapsibleGuide opens links in a new tab.
- Schedule create/edit dialog accepts workflow variables via JSON editor
pre-filled from the workflow start node's input schema
- New `find_by_external_id` document action lets sync workflows short-circuit
the download step when the source file's md5 hasn't changed
- Google Drive sync workflow uses the lookup + content hash check to skip
unchanged files; enabled by default
- input-schema-template emits concrete `[]` / `{}` for optional collections
so the JSON tree editor's `+` button can add items in place
- i18n strings (en/de/fr) for the schedule variables editor
Remove `installed` and `enabled` from workflow JSON config; introduce the wfInstallations DB table as the single source of deployment state. Workflow Active/Inactive is now derived from "any trigger active" — no separate publish/unpublish UI. Dispatchers (scheduler/webhook/api/event) and trigger create/toggle mutations gate on installation existence to avoid silent drops on uninstalled workflows. Drop the `orgSlug` parameter from all public actions (file_actions plus chatWithAgent/arenaChat) and resolve it internally from organizationId. Frontend stops hardcoding 'default' — only organizationId flows through hooks and components, paving the way for true multi-org support.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -0,0 +1,157 @@ | |||
| 'use node'; | |||
…8n keys - workflow tool tests: mock new internal.workflows.installations.getInstallationInternal query and replace removed `enabled:false` failure path with `not installed` - unified_chat_ttft test: add components.betterAuth.adapter.findOne to api mock so resolveOrgSlug runQuery resolves - schedule-create-dialog test: mock useReadWorkflow (now used to pre-fill the variables editor) so the axe audits don't bail on missing ConvexProvider - en/de/fr.json: drop common.actions.publish/deactivate/deactivating — the publish/unpublish UI was removed in the prior commit
Contributor
📝 WalkthroughWalkthroughThis PR implements a comprehensive refactoring of workflow management, replacing hardcoded organization defaults with dynamic organization IDs, decoupling workflow enablement from JSON config into a separate Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
services/platform/convex/agent_tools/workflows/workflow_read_tool.ts (1)
43-47:⚠️ Potential issue | 🟡 MinorKeep
list_allprompt wording internally consistent.The operation description says “installed workflows,” but the best-practices bullet still says “all workflows in the organization.” Align both to avoid agent confusion.
Suggested wording fix
-• Use 'list_all' to get an overview of all workflows in the organization. +• Use 'list_all' to get an overview of installed workflows in the organization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/agent_tools/workflows/workflow_read_tool.ts` around lines 43 - 47, The wording for the 'list_all' operation is inconsistent: the operation description uses "installed workflows" while the BEST PRACTICES bullet says "all workflows in the organization"; update the BEST PRACTICES bullet under 'list_all' to match the operation description by changing "Use 'list_all' to get an overview of all workflows in the organization." to "Use 'list_all' to get an overview of all installed workflows." and keep the 'get_structure' guidance unchanged so both descriptions consistently refer to installed workflows when referencing 'list_all'.services/platform/convex/workflows/rest_api.ts (1)
160-176:⚠️ Potential issue | 🟠 MajorFail closed when workflow config read fails during schedule creation.
If
readWorkflowForExecutionfails,requiresbecomesundefinedand the schedule is still created. That can skip dependency validation for workflows that declare required integrations.Suggested fix
- const requires = workflowRead.ok ? workflowRead.config.requires : undefined; + if (!workflowRead.ok) { + return jsonError('Workflow not found or unreadable', 400); + } + const requires = workflowRead.config.requires;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/workflows/rest_api.ts` around lines 160 - 176, The code currently proceeds to create a schedule even when internal.workflows.file_actions.readWorkflowForExecution (invoked via rc.ctx.runAction) fails, causing requires to be undefined and skipping dependency validation; modify the flow so that after calling readWorkflowForExecution you check workflowRead.ok and if it's false throw or return an error (or propagate failure) instead of setting requires to undefined, and only call internal.workflows.triggers.internal_mutations.createScheduleInternal with a valid requires value when workflowRead.ok is true; reference the variables workflowRead, requires, rc.ctx.runAction and rc.ctx.runMutation/createScheduleInternal to locate and enforce the early-fail behavior.services/platform/app/features/automations/components/automation-create-dialog.tsx (1)
100-122:⚠️ Potential issue | 🟠 MajorMake create + install atomic (or handle partial success explicitly).
At Line 100 you persist the workflow, then at Line 116 you install it. If install fails, the catch path shows
createFailedeven though creation already succeeded. That leaves a hidden partial state and retrying the same name can immediately fail as duplicate.Suggested direction
- await saveWorkflow({ organizationId, workflowSlug, config: { ... } }); - await installWorkflow({ organizationId, workflowSlug }); + await createAndInstallWorkflow({ + organizationId, + workflowSlug, + config: { ... }, + });If a combined backend mutation is not available, handle install failure separately and surface a “created but not installed” state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/components/automation-create-dialog.tsx` around lines 100 - 122, The current flow calls saveWorkflow(...) then installWorkflow(...), but any install error is surfaced as a generic create failure which hides the partial state; update the logic around saveWorkflow, installWorkflow and invalidateWorkflows so that install failures are handled explicitly: after saveWorkflow succeeds, wrap installWorkflow in its own try/catch and on install failure set a distinct state/flag (e.g. createdButNotInstalled) and show a UI message offering “retry install” (or “complete installation”) rather than marking creation as failed; optionally provide a cleanup/rollback path (deleteWorkflow) if available, and ensure invalidateWorkflows and window.dispatchEvent('workflow-updated') are called only after successful installation or are accompanied by the created-but-not-installed state so callers know the real status.services/platform/convex/workflows/triggers/slug_mutations.ts (1)
34-68:⚠️ Potential issue | 🟠 MajorPublic schedule creation still skips dependency validation.
ScheduleCreateDialoggoes through the slug mutation path, butcreateScheduleBySlugonly checks installation. UnlikecreateScheduleInternal, it never runsvalidateWorkflowDependencies, so the UI can still create active schedules for workflows whose required integrations are missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/workflows/triggers/slug_mutations.ts` around lines 34 - 68, createScheduleBySlug currently skips dependency checks causing active schedules to be created for workflows with missing integrations; call await validateWorkflowDependencies(ctx, args.organizationId, args.workflowSlug) (the same validation used by createScheduleInternal) after assertWorkflowInstalled and before inserting the wfSchedules row, and handle its result the same way createScheduleInternal does (either throw a descriptive error listing missing integrations or mark the schedule inactive), so schedule creation cannot proceed as active when required integrations are missing.services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx (1)
136-147:⚠️ Potential issue | 🟠 MajorDon't reset the variables editor after async workflow data arrives.
initialVariablesJsonchanges whenuseReadWorkflowresolves, and this effect re-runs while the dialog is open. If the user starts typing before the read completes, their JSON gets overwritten by the late template/reset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx` around lines 136 - 147, The effect currently resets the variables editor whenever initialVariablesJson changes (which happens after async useReadWorkflow resolves) and overwrites user edits; remove initialVariablesJson from the dependency array and change the variables reset to only populate the editor if it is empty so late-arriving data doesn't clobber user input: in the useEffect that runs when open is true (keep dependencies [open, schedule, reset]) replace setVariablesJson(initialVariablesJson) with a functional update like setVariablesJson(prev => prev ?? initialVariablesJson) and keep the other resets (setNaturalLanguage, setCronDescription, setGenerateError, setVariablesError) as-is.services/platform/app/features/automations/components/automation-steps.tsx (1)
346-372:⚠️ Potential issue | 🔴 CriticalDisable
deleteKeyCodeuntil persistence is wired.With
deleteKeyCodeactive andonNodesChange/onEdgesChangeconnected directly touseNodesState/useEdgesState, pressing Backspace or Delete removes nodes and edges from local state. TheonEdgesDeletehandler only shows a toast and never reverts the deletion. Users can clear the entire graph without persisting any changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/components/automation-steps.tsx` around lines 346 - 372, The Delete/Backspace key currently removes nodes/edges from local state because deleteKeyCode is set while onNodesChange/onEdgesChange are wired to useNodesState/useEdgesState and onEdgesDelete only shows a toast; disable deleteKeyCode until persistence is implemented by removing or clearing the deleteKeyCode prop on the ReactFlow instance in automation-steps.tsx (where deleteKeyCode, onNodesChange, onEdgesChange, onEdgesDelete and useNodesState/useEdgesState are used) so keyboard deletion cannot mutate local state unexpectedly.services/platform/convex/workflows/file_actions.ts (2)
480-487:⚠️ Potential issue | 🔴 CriticalReplace the prefix check with
verifyPathWithinBase().
startsWith(path.resolve(historyDir))is not a safe traversal guard: a sibling path like/workflows/history-bad/...still passes the string-prefix check. A craftedtimestampcan escapehistoryDirin both read and restore flows.Proposed fix
const filePath = path.join(historyDir, `${args.timestamp}.json`); - - const resolved = path.resolve(filePath); - if (!resolved.startsWith(path.resolve(historyDir))) { - throw new Error('Path traversal detected'); - } + await verifyPathWithinBase(filePath, historyDir);const historyPath = path.join(historyDir, `${args.timestamp}.json`); - - const resolved = path.resolve(historyPath); - if (!resolved.startsWith(path.resolve(historyDir))) { - throw new Error('Path traversal detected'); - } + await verifyPathWithinBase(historyPath, historyDir);Also applies to: 518-526
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/workflows/file_actions.ts` around lines 480 - 487, The prefix string check used to guard against path traversal (resolving filePath then testing resolved.startsWith(path.resolve(historyDir))) is unsafe; replace that logic by calling the central helper verifyPathWithinBase(historyDir, filePath) (or verifyPathWithinBase(path.resolve(historyDir), resolved) depending on its API) in both places (the read/restore flow around the code that builds filePath and the similar block at lines 518-526) so the check correctly validates the resolved path is strictly inside the base directory; update the error thrown to the same behavior when verifyPathWithinBase returns false and remove the startsWith-based guard.
603-645:⚠️ Potential issue | 🔴 CriticalDon’t take
orgSlugandorganizationIdas independent inputs here.This action reads files from
args.orgSlugbut loads installation state fromargs.organizationId. If a caller passes mismatched values, it can return one org’s workflow files filtered by another org’s installation set.Proposed fix
export const listWorkflowsForAgent = internalAction({ args: { - orgSlug: v.string(), organizationId: v.string(), }, @@ - handler: async (ctx, args): Promise<any[]> => { - const dir = resolveWorkflowsDir(args.orgSlug); + handler: async (ctx, args): Promise<any[]> => { + const orgSlug = await resolveOrgSlug(ctx, args.organizationId); + const dir = resolveWorkflowsDir(orgSlug); @@ - const result = await readWorkflowFile(args.orgSlug, slug); + const result = await readWorkflowFile(orgSlug, slug);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/workflows/file_actions.ts` around lines 603 - 645, The handler accepts orgSlug and organizationId independently which allows mismatched inputs; change listWorkflowsForAgent to derive the installation scope from the provided orgSlug (or validate that organizationId matches the orgSlug-derived id) instead of using args.organizationId directly: use resolveWorkflowsDir(args.orgSlug) as the single source of truth and call internal.workflows.installations.listInstalledSlugs with the organizationId obtained from the orgSlug (or fail if args.organizationId !== derived id). Update references in this function (listWorkflowsForAgent, resolveWorkflowsDir, readWorkflowFile, and the call to internal.workflows.installations.listInstalledSlugs) so the installation lookup and file reads use the same organization identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/integrations/google_drive/connector.ts`:
- Around line 183-190: The catch block around parsing response.json() silences
JSON parse errors; update the try/catch in the response -> errorBody extraction
(the code using response, err, and errorBody) to log the caught parsing
exception before falling back to response.text() so malformed API responses are
visible during debugging — use the existing logger if available (or
console.warn) to include the error object and a short context message while
preserving the fallback behavior.
In `@services/platform/app/features/automations/components/automation-steps.tsx`:
- Around line 414-421: The dismiss icon Button lacks an accessible name; update
the Button in the automation-steps component to include a translated aria-label
(do not hardcode English) — e.g., use the project's i18n/translation helper to
pass a localized string for the common dismiss key as aria-label on the Button
that calls setShowActivityBanner(false) and contains the <X /> icon so screen
readers announce the button purpose.
In `@services/platform/app/features/automations/components/automation-tester.tsx`:
- Around line 76-79: The useEffect in automation-tester.tsx currently lists
hasUserEdited as a dependency but immediately returns when true, causing
unnecessary re-runs; change it to depend only on inputTemplate and read the
latest edited state from a ref instead: create a ref (e.g., hasUserEditedRef),
update that ref wherever hasUserEdited is set (e.g., in the input change handler
or setHasUserEdited), and in the effect check hasUserEditedRef.current and call
setTestInput(inputTemplate) only when the ref indicates no user edits; this
removes hasUserEdited from the dependency array and prevents no-op re-runs while
preserving correct behavior of setTestInput in the component.
In
`@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx`:
- Around line 90-97: The variables editor is currently shown only when
hasInputSchema (inputTemplate !== '{}'), which hides existing schedule.variables
in edit mode; update the rendering condition (where hasInputSchema is used) to
also allow showing the editor if schedule?.variables exists and has keys (e.g.,
hasInputSchema || (schedule?.variables && Object.keys(schedule.variables).length
> 0)), and ensure initialVariablesJson stays derived from schedule.variables
when present (see hasInputSchema, initialVariablesJson, and the variables editor
render block around the current lines 311-325) so existing variables remain
visible and editable even when the inputTemplate is empty.
In `@services/platform/app/features/automations/utils/input-schema-template.ts`:
- Around line 44-45: Define a recursive JSONValue type
(string|number|boolean|null|JSONValue[]|Record<string,JSONValue>) and replace
all uses of unknown in this file: change the return types of defaultForType and
defaultForProperty to JSONValue, and use Record<string, JSONValue> for
obj/template objects; update any local variable typings and helper signatures
that currently use unknown so they consistently accept/return JSONValue instead
of unknown, keeping function names defaultForType and defaultForProperty
unchanged.
In `@services/platform/app/routes/dashboard/`$id/automations/$amId.tsx:
- Around line 246-249: hasActiveTrigger coming from useWorkflowActivity defaults
to false while the query is loading, causing a transient "inactive" render and
false being passed into AutomationSteps; update usages to check the hook's
loading state (e.g., isLoading/isFetching or explicit resolved flag from
useWorkflowActivity) and only treat hasActiveTrigger as false after the query is
settled—render a loader or skip the inactive state until !isLoading, and pass a
nullable value (or omit the prop) to AutomationSteps until resolved; apply the
same guard for the other occurrences that read hasActiveTrigger (the usages
around the referenced blocks and the AutomationSteps prop).
In
`@services/platform/convex/workflow_engine/helpers/data_source/file_workflow_data_source.ts`:
- Line 30: The returned workflow definition in file_workflow_data_source.ts
currently hardcodes status: 'active'; instead inspect the source config's
enabled flag and set status to 'active' when enabled === true and to 'draft' (or
the project's non-active status) when enabled === false so disabled workflows
are represented correctly; update the status assignment in the function that
constructs the returned definition (the object containing status: 'active') to
derive status from the config.enabled property and keep existing fields
unchanged.
In
`@services/platform/convex/workflow_engine/helpers/engine/load_file_workflow.ts`:
- Line 35: The loader currently hardcodes status: 'active' which ignores
config.enabled; update the workflow creation in load_file_workflow (the object
with status: 'active') to set status based on config.enabled (e.g., status:
config.enabled ? 'active' : 'disabled' or 'inactive') so disabled workflows are
preserved as non-active; ensure you reference the same workflow object/variable
where status is assigned and no other callers assume a constant 'active' value.
In
`@services/platform/convex/workflow_engine/helpers/validation/validate_workflow_dependencies.ts`:
- Around line 76-78: In validate_workflow_dependencies.ts update the
credential-status check so any non-active state fails: replace the existing
condition that only checks for null or 'inactive' on the cred (cast to
IntegrationCredentialRow) with a check for !== 'active' (i.e., if (!cred ||
(cred as IntegrationCredentialRow).status !== 'active') ) and push the missing
entry as before so credentials in 'error' or 'testing' are treated as missing.
In `@services/platform/convex/workflows/file_actions.ts`:
- Around line 411-434: The current read/delete/upsert sequence using
ctx.runQuery(internal.workflows.installations.getInstallationInternal, ...)
followed by deleteInstallation and upsertInstallation is non-atomic and creates
a TOCTOU window; replace it with a single atomic operation (e.g., add a new
mutation like internal.workflows.installations.renameInstallation or a
transactional mutation) that takes organizationId, oldSlug, newSlug and
moves/updates the installation record in one database transaction so the
installedBy and contentHash are preserved without an intermediate delete; update
the call site to use
ctx.runMutation(internal.workflows.installations.renameInstallation, {
organizationId: args.organizationId, oldSlug: args.oldSlug, newSlug:
args.newSlug }) (or the transactional variant) instead of the read/delete/upsert
sequence.
In `@services/platform/convex/workflows/installations.ts`:
- Around line 119-120: The auth lookup should treat lookup failures as
unauthenticated: wrap the call to authComponent.getAuthUser(ctx) (used in the
isInstalled handler) in a try/catch and, on any thrown error, swallow it and
proceed as unauthenticated (return false or set authUser to null) instead of
letting the exception propagate; do not log the error—just fall back to the
unauthenticated behavior.
In `@services/platform/convex/workflows/schema.ts`:
- Around line 195-196: The compound index .index('by_org_slug',
['organizationId', 'workflowSlug']) on wfInstallations should be removed (keep
the single-field .index('by_org', ['organizationId']) only) so the schema starts
with single-field indexes per Convex guidance; delete the by_org_slug index
declaration from the wfInstallations schema to defer adding compound indexes
until profiling shows they are needed.
In `@services/platform/lib/shared/schemas/__tests__/workflows.test.ts`:
- Around line 50-81: Add a negative test that uses workflowJsonSchema.safeParse
with a malformed requires.integrations entry (e.g., integration missing required
fields or with invalid operations type) and assert that result.success is false
and the parse errors mention the integration path; locate where other tests call
workflowJsonSchema.safeParse in workflows.test.ts and add a new it block (e.g.,
"rejects invalid requires.integrations") that expects result.success toBe(false)
and inspects result.error to include the requires.integrations path or relevant
validation message.
In `@services/platform/messages/en.json`:
- Around line 1343-1345: Update the validation string for the "variablesInvalid"
key so the message reads grammatically correct; replace the current value
"Variables must be valid JSON object" with "Variables must be a valid JSON
object" by editing the variablesInvalid entry in the messages JSON (key:
variablesInvalid).
---
Outside diff comments:
In
`@services/platform/app/features/automations/components/automation-create-dialog.tsx`:
- Around line 100-122: The current flow calls saveWorkflow(...) then
installWorkflow(...), but any install error is surfaced as a generic create
failure which hides the partial state; update the logic around saveWorkflow,
installWorkflow and invalidateWorkflows so that install failures are handled
explicitly: after saveWorkflow succeeds, wrap installWorkflow in its own
try/catch and on install failure set a distinct state/flag (e.g.
createdButNotInstalled) and show a UI message offering “retry install” (or
“complete installation”) rather than marking creation as failed; optionally
provide a cleanup/rollback path (deleteWorkflow) if available, and ensure
invalidateWorkflows and window.dispatchEvent('workflow-updated') are called only
after successful installation or are accompanied by the
created-but-not-installed state so callers know the real status.
In `@services/platform/app/features/automations/components/automation-steps.tsx`:
- Around line 346-372: The Delete/Backspace key currently removes nodes/edges
from local state because deleteKeyCode is set while onNodesChange/onEdgesChange
are wired to useNodesState/useEdgesState and onEdgesDelete only shows a toast;
disable deleteKeyCode until persistence is implemented by removing or clearing
the deleteKeyCode prop on the ReactFlow instance in automation-steps.tsx (where
deleteKeyCode, onNodesChange, onEdgesChange, onEdgesDelete and
useNodesState/useEdgesState are used) so keyboard deletion cannot mutate local
state unexpectedly.
In
`@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx`:
- Around line 136-147: The effect currently resets the variables editor whenever
initialVariablesJson changes (which happens after async useReadWorkflow
resolves) and overwrites user edits; remove initialVariablesJson from the
dependency array and change the variables reset to only populate the editor if
it is empty so late-arriving data doesn't clobber user input: in the useEffect
that runs when open is true (keep dependencies [open, schedule, reset]) replace
setVariablesJson(initialVariablesJson) with a functional update like
setVariablesJson(prev => prev ?? initialVariablesJson) and keep the other resets
(setNaturalLanguage, setCronDescription, setGenerateError, setVariablesError)
as-is.
In `@services/platform/convex/agent_tools/workflows/workflow_read_tool.ts`:
- Around line 43-47: The wording for the 'list_all' operation is inconsistent:
the operation description uses "installed workflows" while the BEST PRACTICES
bullet says "all workflows in the organization"; update the BEST PRACTICES
bullet under 'list_all' to match the operation description by changing "Use
'list_all' to get an overview of all workflows in the organization." to "Use
'list_all' to get an overview of all installed workflows." and keep the
'get_structure' guidance unchanged so both descriptions consistently refer to
installed workflows when referencing 'list_all'.
In `@services/platform/convex/workflows/file_actions.ts`:
- Around line 480-487: The prefix string check used to guard against path
traversal (resolving filePath then testing
resolved.startsWith(path.resolve(historyDir))) is unsafe; replace that logic by
calling the central helper verifyPathWithinBase(historyDir, filePath) (or
verifyPathWithinBase(path.resolve(historyDir), resolved) depending on its API)
in both places (the read/restore flow around the code that builds filePath and
the similar block at lines 518-526) so the check correctly validates the
resolved path is strictly inside the base directory; update the error thrown to
the same behavior when verifyPathWithinBase returns false and remove the
startsWith-based guard.
- Around line 603-645: The handler accepts orgSlug and organizationId
independently which allows mismatched inputs; change listWorkflowsForAgent to
derive the installation scope from the provided orgSlug (or validate that
organizationId matches the orgSlug-derived id) instead of using
args.organizationId directly: use resolveWorkflowsDir(args.orgSlug) as the
single source of truth and call
internal.workflows.installations.listInstalledSlugs with the organizationId
obtained from the orgSlug (or fail if args.organizationId !== derived id).
Update references in this function (listWorkflowsForAgent, resolveWorkflowsDir,
readWorkflowFile, and the call to
internal.workflows.installations.listInstalledSlugs) so the installation lookup
and file reads use the same organization identity.
In `@services/platform/convex/workflows/rest_api.ts`:
- Around line 160-176: The code currently proceeds to create a schedule even
when internal.workflows.file_actions.readWorkflowForExecution (invoked via
rc.ctx.runAction) fails, causing requires to be undefined and skipping
dependency validation; modify the flow so that after calling
readWorkflowForExecution you check workflowRead.ok and if it's false throw or
return an error (or propagate failure) instead of setting requires to undefined,
and only call
internal.workflows.triggers.internal_mutations.createScheduleInternal with a
valid requires value when workflowRead.ok is true; reference the variables
workflowRead, requires, rc.ctx.runAction and
rc.ctx.runMutation/createScheduleInternal to locate and enforce the early-fail
behavior.
In `@services/platform/convex/workflows/triggers/slug_mutations.ts`:
- Around line 34-68: createScheduleBySlug currently skips dependency checks
causing active schedules to be created for workflows with missing integrations;
call await validateWorkflowDependencies(ctx, args.organizationId,
args.workflowSlug) (the same validation used by createScheduleInternal) after
assertWorkflowInstalled and before inserting the wfSchedules row, and handle its
result the same way createScheduleInternal does (either throw a descriptive
error listing missing integrations or mark the schedule inactive), so schedule
creation cannot proceed as active when required integrations are missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 694e1c06-04a6-44e6-9cc1-72197f758240
⛔ Files ignored due to path filters (3)
examples/integrations/google_drive/icon.svgis excluded by!**/*.svgservices/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**services/platform/convex/migrations/backfill_wf_installations.tsis excluded by!**/migrations/**
📒 Files selected for processing (80)
examples/integrations/google_drive/config.jsonexamples/integrations/google_drive/connector.tsexamples/workflows/google_drive/sync.jsonservices/platform/app/components/ui/data-display/collapsible-guide.tsxservices/platform/app/features/automations/components/automation-active-toggle.test.tsxservices/platform/app/features/automations/components/automation-active-toggle.tsxservices/platform/app/features/automations/components/automation-assistant/message-list.test.tsxservices/platform/app/features/automations/components/automation-assistant/message-list.tsxservices/platform/app/features/automations/components/automation-create-dialog.tsxservices/platform/app/features/automations/components/automation-history-diff-dialog.test.tsxservices/platform/app/features/automations/components/automation-navigation.tsxservices/platform/app/features/automations/components/automation-row-actions.test.tsxservices/platform/app/features/automations/components/automation-row-actions.tsxservices/platform/app/features/automations/components/automation-steps.tsxservices/platform/app/features/automations/components/automation-tester.tsxservices/platform/app/features/automations/components/automations-table.test.tsxservices/platform/app/features/automations/components/automations-table.tsxservices/platform/app/features/automations/components/workflow-template-grid.test.tsxservices/platform/app/features/automations/components/workflow-template-grid.tsxservices/platform/app/features/automations/hooks/file-mutations.tsservices/platform/app/features/automations/hooks/file-queries.tsservices/platform/app/features/automations/hooks/use-assistant-chat.tsservices/platform/app/features/automations/hooks/use-automations-table-config.tsxservices/platform/app/features/automations/triggers/components/event-create-dialog.tsxservices/platform/app/features/automations/triggers/components/events-section.tsxservices/platform/app/features/automations/triggers/components/schedule-create-dialog.test.tsxservices/platform/app/features/automations/triggers/components/schedule-create-dialog.tsxservices/platform/app/features/automations/triggers/components/schedules-section.tsxservices/platform/app/features/automations/triggers/components/webhooks-section.tsxservices/platform/app/features/automations/triggers/hooks/queries.tsservices/platform/app/features/automations/triggers/triggers.tsxservices/platform/app/features/automations/utils/input-schema-template.tsservices/platform/app/features/chat/hooks/use-send-message.tsservices/platform/app/features/settings/integrations/components/integration-manage/integration-active-view.tsxservices/platform/app/routes/dashboard/$id/automations/$amId.tsxservices/platform/app/routes/dashboard/$id/automations/$amId/configuration.tsxservices/platform/app/routes/dashboard/$id/automations/$amId/triggers.tsxservices/platform/app/routes/dashboard/$id/automations/index.tsxservices/platform/convex/agent_tools/workflows/create_bound_workflow_tool.tsservices/platform/convex/agent_tools/workflows/helpers/read_all_workflows.tsservices/platform/convex/agent_tools/workflows/helpers/types.tsservices/platform/convex/agent_tools/workflows/internal_actions.tsservices/platform/convex/agent_tools/workflows/run_workflow_tool.tsservices/platform/convex/agent_tools/workflows/workflow_read_tool.tsservices/platform/convex/agents/__tests__/unified_chat_ttft.test.tsservices/platform/convex/agents/arena_chat.tsservices/platform/convex/agents/unified_chat.tsservices/platform/convex/documents/find_document_by_external_id.tsservices/platform/convex/documents/internal_queries.tsservices/platform/convex/documents/query_documents.tsservices/platform/convex/documents/schema.tsservices/platform/convex/documents/update_document.tsservices/platform/convex/documents/update_document_internal.tsservices/platform/convex/documents/validators.tsservices/platform/convex/schema.tsservices/platform/convex/workflow_engine/action_defs/document/document_action.tsservices/platform/convex/workflow_engine/helpers/data_source/file_workflow_data_source.tsservices/platform/convex/workflow_engine/helpers/engine/load_file_workflow.tsservices/platform/convex/workflow_engine/helpers/engine/start_workflow_from_file.tsservices/platform/convex/workflow_engine/helpers/scheduler/get_scheduled_workflows.tsservices/platform/convex/workflow_engine/helpers/scheduler/scan_and_trigger.tsservices/platform/convex/workflow_engine/helpers/validation/validate_workflow_dependencies.tsservices/platform/convex/workflow_engine/internal_queries.tsservices/platform/convex/workflows/file_actions.tsservices/platform/convex/workflows/installations.tsservices/platform/convex/workflows/rest_api.tsservices/platform/convex/workflows/schema.tsservices/platform/convex/workflows/triggers/api_http.tsservices/platform/convex/workflows/triggers/http_actions.tsservices/platform/convex/workflows/triggers/internal_mutations.tsservices/platform/convex/workflows/triggers/process_event.tsservices/platform/convex/workflows/triggers/schema.tsservices/platform/convex/workflows/triggers/slug_mutations.tsservices/platform/convex/workflows/triggers/slug_queries.tsservices/platform/lib/shared/schemas/__tests__/workflows.test.tsservices/platform/lib/shared/schemas/workflows.tsservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.jsonservices/platform/types/documents.ts
💤 Files with no reviewable changes (12)
- services/platform/app/features/automations/components/automation-history-diff-dialog.test.tsx
- services/platform/app/features/automations/components/automation-active-toggle.test.tsx
- services/platform/app/features/automations/components/automations-table.test.tsx
- services/platform/app/features/chat/hooks/use-send-message.ts
- services/platform/convex/agent_tools/workflows/helpers/types.ts
- services/platform/app/features/automations/triggers/components/webhooks-section.tsx
- services/platform/app/features/automations/triggers/components/schedules-section.tsx
- services/platform/convex/agents/arena_chat.ts
- services/platform/app/routes/dashboard/$id/automations/$amId/triggers.tsx
- services/platform/app/features/automations/triggers/components/schedule-create-dialog.test.tsx
- services/platform/app/features/automations/components/automation-active-toggle.tsx
- services/platform/app/features/automations/triggers/triggers.tsx
Comment on lines
+183
to
+190
| try { | ||
| const err = response.json() as Record<string, Record<string, string>>; | ||
| errorBody = err.error | ||
| ? err.error.message || JSON.stringify(err.error) | ||
| : response.text(); | ||
| } catch (_e) { | ||
| errorBody = response.text(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent catch may hide parse failures.
While the fallback to response.text() is reasonable, logging the parse failure would help debug malformed API responses.
💡 Suggested improvement
try {
const err = response.json() as Record<string, Record<string, string>>;
errorBody = err.error
? err.error.message || JSON.stringify(err.error)
: response.text();
- } catch (_e) {
+ } catch (e) {
+ console.warn('Failed to parse error response as JSON:', e);
errorBody = response.text();
}📝 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.
Suggested change
| try { | |
| const err = response.json() as Record<string, Record<string, string>>; | |
| errorBody = err.error | |
| ? err.error.message || JSON.stringify(err.error) | |
| : response.text(); | |
| } catch (_e) { | |
| errorBody = response.text(); | |
| } | |
| try { | |
| const err = response.json() as Record<string, Record<string, string>>; | |
| errorBody = err.error | |
| ? err.error.message || JSON.stringify(err.error) | |
| : response.text(); | |
| } catch (e) { | |
| console.warn('Failed to parse error response as JSON:', e); | |
| errorBody = response.text(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/integrations/google_drive/connector.ts` around lines 183 - 190, The
catch block around parsing response.json() silences JSON parse errors; update
the try/catch in the response -> errorBody extraction (the code using response,
err, and errorBody) to log the caught parsing exception before falling back to
response.text() so malformed API responses are visible during debugging — use
the existing logger if available (or console.warn) to include the error object
and a short context message while preserving the fallback behavior.
Comment on lines
414
to
421
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="ml-auto size-6 shrink-0 p-1 text-blue-600 hover:bg-blue-100 hover:text-blue-700" | ||
| onClick={() => setShowDraftBanner(false)} | ||
| className="ml-auto size-6 shrink-0 p-1 text-amber-600 hover:bg-amber-100 hover:text-amber-700" | ||
| onClick={() => setShowActivityBanner(false)} | ||
| > | ||
| <X className="size-4" /> | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Label the dismiss icon button.
This button has no accessible name, so screen readers will only announce “button”. Add a translated aria-label here (the existing common dismiss key would fit).
As per coding guidelines, "Every icon-only button must have a translated aria-label — never hardcode English in ARIA."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/automations/components/automation-steps.tsx`
around lines 414 - 421, The dismiss icon Button lacks an accessible name; update
the Button in the automation-steps component to include a translated aria-label
(do not hardcode English) — e.g., use the project's i18n/translation helper to
pass a localized string for the common dismiss key as aria-label on the Button
that calls setShowActivityBanner(false) and contains the <X /> icon so screen
readers announce the button purpose.
Comment on lines
+76
to
+79
| useEffect(() => { | ||
| if (hasUserEdited) return; | ||
| setTestInput(inputTemplate); | ||
| }, [inputTemplate, hasUserEdited]); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Effect re-runs unnecessarily when hasUserEdited becomes true.
The effect depends on hasUserEdited but immediately returns when it's true. This causes a no-op re-run whenever the user edits. Consider removing hasUserEdited from dependencies and using a ref instead:
♻️ Optional: Use ref to avoid unnecessary effect runs
+import { useEffect, useMemo, useRef, useState } from 'react';
-import { useEffect, useMemo, useState } from 'react';
const [testInput, setTestInput] = useState('{}');
-const [hasUserEdited, setHasUserEdited] = useState(false);
+const hasUserEditedRef = useRef(false);
useEffect(() => {
- if (hasUserEdited) return;
+ if (hasUserEditedRef.current) return;
setTestInput(inputTemplate);
-}, [inputTemplate, hasUserEdited]);
+}, [inputTemplate]);
// In onChange:
-setHasUserEdited(true);
+hasUserEditedRef.current = true;
// After execution reset:
-setHasUserEdited(false);
+hasUserEditedRef.current = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/automations/components/automation-tester.tsx`
around lines 76 - 79, The useEffect in automation-tester.tsx currently lists
hasUserEdited as a dependency but immediately returns when true, causing
unnecessary re-runs; change it to depend only on inputTemplate and read the
latest edited state from a ref instead: create a ref (e.g., hasUserEditedRef),
update that ref wherever hasUserEdited is set (e.g., in the input change handler
or setHasUserEdited), and in the effect check hasUserEditedRef.current and call
setTestInput(inputTemplate) only when the ref indicates no user edits; this
removes hasUserEdited from the dependency array and prevents no-op re-runs while
preserving correct behavior of setTestInput in the component.
Comment on lines
+90
to
+97
| const hasInputSchema = inputTemplate !== '{}'; | ||
|
|
||
| const initialVariablesJson = useMemo(() => { | ||
| if (schedule?.variables && Object.keys(schedule.variables).length > 0) { | ||
| return JSON.stringify(schedule.variables, null, 2); | ||
| } | ||
| return inputTemplate; | ||
| }, [schedule, inputTemplate]); |
Contributor
There was a problem hiding this comment.
Show the variables editor when the schedule already has variables.
Rendering is gated only by inputTemplate !== '{}'. In edit mode, an existing schedule.variables payload becomes invisible and uneditable whenever the start schema is empty or the workflow read fails.
Also applies to: 311-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx`
around lines 90 - 97, The variables editor is currently shown only when
hasInputSchema (inputTemplate !== '{}'), which hides existing schedule.variables
in edit mode; update the rendering condition (where hasInputSchema is used) to
also allow showing the editor if schedule?.variables exists and has keys (e.g.,
hasInputSchema || (schedule?.variables && Object.keys(schedule.variables).length
> 0)), and ensure initialVariablesJson stays derived from schedule.variables
when present (see hasInputSchema, initialVariablesJson, and the variables editor
render block around the current lines 311-325) so existing variables remain
visible and editable even when the inputTemplate is empty.
Comment on lines
+44
to
+45
| function defaultForType(type: SchemaType): unknown { | ||
| if (type === 'string') return ''; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\bunknown\b' services/platform/app/features/automations/utils/input-schema-template.tsRepository: tale-project/tale
Length of output: 285
🏁 Script executed:
cat -n services/platform/app/features/automations/utils/input-schema-template.ts | head -100Repository: tale-project/tale
Length of output: 3403
🏁 Script executed:
rg -n 'type\s+JSONValue|type\s+Json|interface\s+JSONValue' --type ts --type tsxRepository: tale-project/tale
Length of output: 89
🏁 Script executed:
grep -r "defaultForType\|defaultForProperty\|buildInputTemplate" services/platform/app --include="*.ts" --include="*.tsx" | head -20Repository: tale-project/tale
Length of output: 1869
🏁 Script executed:
rg -n 'JSONValue' services/platform/app --type ts | head -20Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
rg -n 'type.*Value.*=' services/platform/app/lib --type ts | grep -i jsonRepository: tale-project/tale
Length of output: 127
🏁 Script executed:
find services/platform/app -type d -name lib | head -5Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
rg -n 'type.*Record.*string' services/platform/app --type ts | grep -v node_modules | head -10Repository: tale-project/tale
Length of output: 414
Define and use a concrete JSONValue type instead of unknown.
Return types for defaultForType (line 44) and defaultForProperty (line 52) declare unknown, but these functions return constrained JSON-like values. Similarly, lines 54 and 74 use Record<string, unknown> for template objects.
Per repository guidelines, unknown is not permitted in TypeScript except for framework-generated code or third-party gaps. Define a recursive JSONValue type and apply it consistently:
Suggested approach
type JSONValue =
| string
| number
| boolean
| null
| JSONValue[]
| Record<string, JSONValue>;
function defaultForType(type: SchemaType): JSONValue { /* ... */ }
function defaultForProperty(prop: InputSchemaProperty): JSONValue { /* ... */ }
const obj: Record<string, JSONValue> = {};
const template: Record<string, JSONValue> = {};Also applies to lines 52, 54, 74.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/automations/utils/input-schema-template.ts`
around lines 44 - 45, Define a recursive JSONValue type
(string|number|boolean|null|JSONValue[]|Record<string,JSONValue>) and replace
all uses of unknown in this file: change the return types of defaultForType and
defaultForProperty to JSONValue, and use Record<string, JSONValue> for
obj/template objects; update any local variable typings and helper signatures
that currently use unknown so they consistently accept/return JSONValue instead
of unknown, keeping function names defaultForType and defaultForProperty
unchanged.
Comment on lines
+411
to
+434
| const existingInstallation = await ctx.runQuery( | ||
| internal.workflows.installations.getInstallationInternal, | ||
| { | ||
| organizationId: args.organizationId, | ||
| workflowSlug: args.oldSlug, | ||
| }, | ||
| ); | ||
| if (existingInstallation) { | ||
| await ctx.runMutation( | ||
| internal.workflows.installations.deleteInstallation, | ||
| { | ||
| organizationId: args.organizationId, | ||
| workflowSlug: args.oldSlug, | ||
| }, | ||
| ); | ||
| await ctx.runMutation( | ||
| internal.workflows.installations.upsertInstallation, | ||
| { | ||
| organizationId: args.organizationId, | ||
| workflowSlug: args.newSlug, | ||
| installedBy: existingInstallation.installedBy, | ||
| contentHash: existingInstallation.contentHash, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Move the installation record in one mutation.
This read/delete/upsert sequence is non-atomic. If the action fails between Line 419 and Line 434, the renamed workflow loses its installation state; concurrent install/uninstall also has a TOCTOU window here.
Proposed direction
- const existingInstallation = await ctx.runQuery(
- internal.workflows.installations.getInstallationInternal,
- {
- organizationId: args.organizationId,
- workflowSlug: args.oldSlug,
- },
- );
- if (existingInstallation) {
- await ctx.runMutation(
- internal.workflows.installations.deleteInstallation,
- {
- organizationId: args.organizationId,
- workflowSlug: args.oldSlug,
- },
- );
- await ctx.runMutation(
- internal.workflows.installations.upsertInstallation,
- {
- organizationId: args.organizationId,
- workflowSlug: args.newSlug,
- installedBy: existingInstallation.installedBy,
- contentHash: existingInstallation.contentHash,
- },
- );
- }
+ await ctx.runMutation(internal.workflows.installations.renameInstallation, {
+ organizationId: args.organizationId,
+ oldWorkflowSlug: args.oldSlug,
+ newWorkflowSlug: args.newSlug,
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/workflows/file_actions.ts` around lines 411 - 434,
The current read/delete/upsert sequence using
ctx.runQuery(internal.workflows.installations.getInstallationInternal, ...)
followed by deleteInstallation and upsertInstallation is non-atomic and creates
a TOCTOU window; replace it with a single atomic operation (e.g., add a new
mutation like internal.workflows.installations.renameInstallation or a
transactional mutation) that takes organizationId, oldSlug, newSlug and
moves/updates the installation record in one database transaction so the
installedBy and contentHash are preserved without an intermediate delete; update
the call site to use
ctx.runMutation(internal.workflows.installations.renameInstallation, {
organizationId: args.organizationId, oldSlug: args.oldSlug, newSlug:
args.newSlug }) (or the transactional variant) instead of the read/delete/upsert
sequence.
Comment on lines
+119
to
+120
| const authUser = await authComponent.getAuthUser(ctx); | ||
| if (!authUser) return false; |
Contributor
There was a problem hiding this comment.
Treat auth lookup errors as unauthenticated here.
Other Convex queries in this repo swallow authComponent.getAuthUser(ctx) failures and return the unauthenticated fallback instead of surfacing a query error. Without that guard, a transient auth failure turns isInstalled into an exception path rather than a simple false.
Suggested fix
handler: async (ctx, args): Promise<boolean> => {
- const authUser = await authComponent.getAuthUser(ctx);
+ let authUser = null;
+ try {
+ authUser = await authComponent.getAuthUser(ctx);
+ } catch {
+ return false;
+ }
if (!authUser) return false;Based on learnings: "In Convex query handlers, if authComponent.getAuthUser(ctx) fails, swallow the error (empty catch) and treat as unauthenticated without logging warnings/errors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/workflows/installations.ts` around lines 119 - 120,
The auth lookup should treat lookup failures as unauthenticated: wrap the call
to authComponent.getAuthUser(ctx) (used in the isInstalled handler) in a
try/catch and, on any thrown error, swallow it and proceed as unauthenticated
(return false or set authUser to null) instead of letting the exception
propagate; do not log the error—just fall back to the unauthenticated behavior.
Comment on lines
+195
to
+196
| .index('by_org', ['organizationId']) | ||
| .index('by_org_slug', ['organizationId', 'workflowSlug']); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Drop the compound index unless you already have profiling data for it.
wfInstallations is a new forward-only schema surface, and this repo’s Convex guidance is to start with single-field indexes and add compound ones only after measured need. by_org already covers the initial access pattern, so locking in by_org_slug now makes the schema harder to evolve without evidence that the extra index is necessary.
Based on learnings: "In convex schema files under services/platform/convex, start with single-field indexes and defer compound indexes until profiling shows a concrete need."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/workflows/schema.ts` around lines 195 - 196, The
compound index .index('by_org_slug', ['organizationId', 'workflowSlug']) on
wfInstallations should be removed (keep the single-field .index('by_org',
['organizationId']) only) so the schema starts with single-field indexes per
Convex guidance; delete the by_org_slug index declaration from the
wfInstallations schema to defer adding compound indexes until profiling shows
they are needed.
Comment on lines
+50
to
+81
| it('parses a config with requires.integrations', () => { | ||
| const result = workflowJsonSchema.safeParse({ | ||
| name: 'Drive Sync', | ||
| requires: { | ||
| integrations: [ | ||
| { name: 'google_drive', operations: ['list_files', 'download_file'] }, | ||
| ], | ||
| }, | ||
| steps: [], | ||
| }); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| if (result.success) { | ||
| expect(result.data.requires?.integrations).toHaveLength(1); | ||
| expect(result.data.requires?.integrations[0]?.name).toBe('google_drive'); | ||
| expect(result.data.requires?.integrations[0]?.operations).toEqual([ | ||
| 'list_files', | ||
| 'download_file', | ||
| ]); | ||
| } | ||
| }); | ||
|
|
||
| it('omits requires when not declared', () => { | ||
| const result = workflowJsonSchema.safeParse({ | ||
| name: 'No deps', | ||
| }); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| if (result.success) { | ||
| expect(result.data.requires).toBeUndefined(); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Add one negative parse test for invalid requires.integrations.
The new schema behavior is covered for valid and omitted inputs, but there’s no explicit rejection case for malformed integration dependencies.
🧪 Suggested test addition
it('omits requires when not declared', () => {
const result = workflowJsonSchema.safeParse({
name: 'No deps',
});
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.requires).toBeUndefined();
}
});
+
+ it('rejects invalid requires.integrations payload', () => {
+ const result = workflowJsonSchema.safeParse({
+ name: 'Invalid deps',
+ requires: {
+ integrations: [
+ { name: '', operations: [123] },
+ ],
+ },
+ steps: [],
+ });
+
+ expect(result.success).toBe(false);
+ });
});📝 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.
Suggested change
| it('parses a config with requires.integrations', () => { | |
| const result = workflowJsonSchema.safeParse({ | |
| name: 'Drive Sync', | |
| requires: { | |
| integrations: [ | |
| { name: 'google_drive', operations: ['list_files', 'download_file'] }, | |
| ], | |
| }, | |
| steps: [], | |
| }); | |
| expect(result.success).toBe(true); | |
| if (result.success) { | |
| expect(result.data.requires?.integrations).toHaveLength(1); | |
| expect(result.data.requires?.integrations[0]?.name).toBe('google_drive'); | |
| expect(result.data.requires?.integrations[0]?.operations).toEqual([ | |
| 'list_files', | |
| 'download_file', | |
| ]); | |
| } | |
| }); | |
| it('omits requires when not declared', () => { | |
| const result = workflowJsonSchema.safeParse({ | |
| name: 'No deps', | |
| }); | |
| expect(result.success).toBe(true); | |
| if (result.success) { | |
| expect(result.data.requires).toBeUndefined(); | |
| } | |
| }); | |
| it('parses a config with requires.integrations', () => { | |
| const result = workflowJsonSchema.safeParse({ | |
| name: 'Drive Sync', | |
| requires: { | |
| integrations: [ | |
| { name: 'google_drive', operations: ['list_files', 'download_file'] }, | |
| ], | |
| }, | |
| steps: [], | |
| }); | |
| expect(result.success).toBe(true); | |
| if (result.success) { | |
| expect(result.data.requires?.integrations).toHaveLength(1); | |
| expect(result.data.requires?.integrations[0]?.name).toBe('google_drive'); | |
| expect(result.data.requires?.integrations[0]?.operations).toEqual([ | |
| 'list_files', | |
| 'download_file', | |
| ]); | |
| } | |
| }); | |
| it('omits requires when not declared', () => { | |
| const result = workflowJsonSchema.safeParse({ | |
| name: 'No deps', | |
| }); | |
| expect(result.success).toBe(true); | |
| if (result.success) { | |
| expect(result.data.requires).toBeUndefined(); | |
| } | |
| }); | |
| it('rejects invalid requires.integrations payload', () => { | |
| const result = workflowJsonSchema.safeParse({ | |
| name: 'Invalid deps', | |
| requires: { | |
| integrations: [ | |
| { name: '', operations: [123] }, | |
| ], | |
| }, | |
| steps: [], | |
| }); | |
| expect(result.success).toBe(false); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/lib/shared/schemas/__tests__/workflows.test.ts` around
lines 50 - 81, Add a negative test that uses workflowJsonSchema.safeParse with a
malformed requires.integrations entry (e.g., integration missing required fields
or with invalid operations type) and assert that result.success is false and the
parse errors mention the integration path; locate where other tests call
workflowJsonSchema.safeParse in workflows.test.ts and add a new it block (e.g.,
"rejects invalid requires.integrations") that expects result.success toBe(false)
and inspects result.error to include the requires.integrations path or relevant
validation message.
Comment on lines
+1343
to
+1345
| "variablesLabel": "Workflow variables", | ||
| "variablesDescription": "Input parameters passed to each scheduled run. Pre-filled from the workflow's input schema.", | ||
| "variablesInvalid": "Variables must be valid JSON object", |
Contributor
There was a problem hiding this comment.
Fix the new validation string.
"Variables must be valid JSON object" is missing an article; "Variables must be a valid JSON object" reads correctly in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 1343 - 1345, Update the
validation string for the "variablesInvalid" key so the message reads
grammatically correct; replace the current value "Variables must be valid JSON
object" with "Variables must be a valid JSON object" by editing the
variablesInvalid entry in the messages JSON (key: variablesInvalid).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wfInstallationsas the single source of deployment state: workflow Active/Inactive is derived from "any trigger active" (no separate publish/unpublish UI); dispatchers gate on installation existence.scan_and_triggerpasses them as workflow input.orgSlugfrom public actions (file_actions, chatWithAgent, arenaChat); resolve internally fromorganizationId. Frontend stops hardcoding'default'.sourceProvider;findDocumentByExternalIdgains optional folder scope so the same external file synced to two folders gets two doc rows.document_actionaddsindex_in_ragand folder-scoped upsert onexternalItemId.Test plan
bun run check(lint, typecheck, tests)examples/integrations/google_drive/and complete OAuthexamples/workflows/google_drive/sync.jsonworkflow manually; verify documents appear in target folder and are indexed in RAGbackfill_wf_installationsmigration populates installations for existing workflows'default'slug regressions in chat / automations / file actionsSummary by CodeRabbit
New Features
Enhancements
UI/UX