feat(platform): workflow schema validation, knowledge scoping, and approval types#826
Merged
larryro merged 5 commits intoMar 21, 2026
Merged
Conversation
…ool schema - Add an "Other" freetext option to single_select and multi_select cards so users can provide custom responses beyond predefined options - Refactor tool args to use discriminated union for stricter validation - Update tool description to guide the agent to always use interactive cards instead of plain-text option lists - Set generationStatus on human input response submission for loading UI - Add deadline to agent continuation after human input response - Skip onResponseSubmitted callback in workflow context
… and approval types Extend workflow input schema to support nested object/array validation with proper type checking at each level. Add knowledgeFileIds config to LLM nodes so rag_search is automatically scoped to specified files. Introduce additional approval types (workflow run, human input, document write, integration) in the assistant UI with collapsible system messages. Update example workflow configs to use file objects (fileId + fileName) instead of plain IDs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis pull request introduces richer input schema structures and knowledge file scoping throughout the platform. It updates two workflow configurations to use structured objects instead of simple IDs (contract-comparison adds typed file arrays, contract-generation replaces templateFileIds/baseFileId with knowledgeFiles/baseFile objects). On the UI side, it extends the chat assistant to handle additional approval and human input request types, adds a new collapsible system message component, and updates message definitions to support 'system' role messages and human input response markers. The backend gains support for knowledge file ID scoping in LLM nodes through a new knowledgeFileIds configuration field, extends workflow input validation to handle nested object and array schemas, adds placeholder support to the human input tool, and updates workflow documentation with examples of nested schema usage. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/workflows/contract-comparison/config.json`:
- Around line 23-32: The schema for the "files" array documents a minimum of 2
but doesn't enforce it; update the array definition for "files" in config.json
(the object that describes items with properties "fileId" and "fileName") to
include a minItems: 2 constraint so JSON validation will reject inputs with
fewer than two file objects.
In
`@services/platform/app/features/automations/components/automation-assistant/message-list.test.tsx`:
- Around line 34-38: Add explicit unit tests in message-list.test.tsx that
render MessageList (or the tested wrapper) with messages covering the new
branches: create one message object with workflowRunApprovals, another with
humanInputRequests (and a separate test for multiline isHumanInputResponse
content), one with documentWriteApprovals, and one with integrationApprovals;
for each test, pass the message array into the same component setup used in
existing tests and assert the DOM shows the expected approval/selection UI and
that the CollapsibleSystemMessage mock (data-testid="system-message") contains
the correct content for system-message paths. Ensure you use the same prop
shapes/keys used in message-list.tsx (workflowRunApprovals, humanInputRequests,
isHumanInputResponse, documentWriteApprovals, integrationApprovals) so the new
selection/rendering branches are directly exercised and add analogous assertions
to the other case group near the existing approval tests.
In
`@services/platform/app/features/automations/components/automation-assistant/message-list.tsx`:
- Around line 152-156: The regex used to parse human-input responses (inside the
block checking message.isHumanInputResponse && message.role === 'system') fails
on multiline questions because it uses .*? for the question capture; update the
pattern to use [\s\S]*? for the question group so it matches newlines (i.e.,
change the question capture from "(.*?)" to "([\s\S]*?)") and apply the same
change in services/platform/app/features/chat/components/chat-messages.tsx where
the identical pattern is used; ensure the variables match and that response
still falls back to message.content when match is null.
In
`@services/platform/app/features/chat/components/collapsible-system-message.tsx`:
- Around line 20-31: The current split uses string equality to find
lastPreviewIdx causing duplicate content when lines repeat; change nonEmptyLines
and previewLines to preserve original indices (e.g., map lines to objects like
{line, index}), build previewLines by slicing the array of {line,index} pairs
(use nonEmptyWithIndex = lines.map((l, i) => ({line: l, index: i})).filter(p =>
p.line.trim() !== '')), compute lastPreviewIdx from the last preview pair's
index (previewLines[previewLines.length-1].index), and then compute rest by
slicing the original lines starting at lastPreviewIdx + 1; update preview to
join previewLines.map(p => p.line) accordingly so preview and rest split by
source index not string equality.
In
`@services/platform/convex/workflow_engine/helpers/nodes/llm/execute_agent_with_tools.ts`:
- Around line 135-137: The code currently omits knowledgeFileIds when its value
is an empty array because it uses a truthy check; update the merge so it only
omits the field when knowledgeFileIds is undefined (not when it's []), e.g. in
execute_agent_with_tools.ts where _args.knowledgeFileIds is merged, replace the
conditional that tests truthiness with one that tests !== undefined or uses
Object.prototype.hasOwnProperty to preserve explicitly-resolved empty arrays so
an explicit [] remains in the resulting payload.
🪄 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: 35dcc89c-875a-41bf-b056-b4f8a172d175
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (21)
examples/workflows/contract-comparison/config.jsonexamples/workflows/contract-generation/config.jsonservices/platform/app/features/automations/components/automation-assistant.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-assistant/types.tsservices/platform/app/features/automations/hooks/use-assistant-chat.tsservices/platform/app/features/chat/components/chat-messages.tsxservices/platform/app/features/chat/components/collapsible-system-message.tsxservices/platform/convex/agent_tools/human_input/request_human_input_tool.tsservices/platform/convex/agent_tools/workflows/helpers/syntax_reference.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/execute_agent_with_tools.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/execute_llm_node.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/types.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/__tests__/resolve_knowledge_file_ids.test.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/resolve_knowledge_file_ids.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/validate_and_normalize_config.tsservices/platform/convex/workflow_engine/helpers/validation/steps/start.tsservices/platform/convex/workflow_engine/helpers/validation/validate_workflow_input.test.tsservices/platform/convex/workflow_engine/helpers/validation/validate_workflow_input.tsservices/platform/convex/workflow_engine/types/nodes.ts
Comment on lines
+23
to
+32
| "description": "File objects ordered chronologically (oldest first). Minimum 2 required.", | ||
| "items": { | ||
| "type": "object", | ||
| "properties": { | ||
| "fileId": { "type": "string", "description": "Convex storage ID" }, | ||
| "fileName": { "type": "string", "description": "Original file name" } | ||
| }, | ||
| "required": ["fileId", "fileName"] | ||
| } | ||
| }, |
There was a problem hiding this comment.
files minimum cardinality is documented but not enforced.
The schema says “Minimum 2 required,” but this segment only enforces presence/type, not count. That allows invalid single-file inputs to proceed and break the comparison intent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/workflows/contract-comparison/config.json` around lines 23 - 32, The
schema for the "files" array documents a minimum of 2 but doesn't enforce it;
update the array definition for "files" in config.json (the object that
describes items with properties "fileId" and "fileName") to include a minItems:
2 constraint so JSON validation will reject inputs with fewer than two file
objects.
Comment on lines
+34
to
+38
| vi.mock('@/app/features/chat/components/collapsible-system-message', () => ({ | ||
| CollapsibleSystemMessage: ({ content }: { content: string }) => ( | ||
| <div data-testid="system-message">{content}</div> | ||
| ), | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add direct coverage for the new approval and system-message branches.
These mocks and props are wired in, but the assertions still only exercise workflow update/creation approvals. Please add direct cases for workflowRunApprovals, humanInputRequests, documentWriteApprovals, integrationApprovals, and a multiline isHumanInputResponse message so the new selection/rendering paths in message-list.tsx are regression-tested.
Also applies to: 96-99
🤖 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-assistant/message-list.test.tsx`
around lines 34 - 38, Add explicit unit tests in message-list.test.tsx that
render MessageList (or the tested wrapper) with messages covering the new
branches: create one message object with workflowRunApprovals, another with
humanInputRequests (and a separate test for multiline isHumanInputResponse
content), one with documentWriteApprovals, and one with integrationApprovals;
for each test, pass the message array into the same component setup used in
existing tests and assert the DOM shows the expected approval/selection UI and
that the CollapsibleSystemMessage mock (data-testid="system-message") contains
the correct content for system-message paths. Ensure you use the same prop
shapes/keys used in message-list.tsx (workflowRunApprovals, humanInputRequests,
isHumanInputResponse, documentWriteApprovals, integrationApprovals) so the new
selection/rendering branches are directly exercised and add analogous assertions
to the other case group near the existing approval tests.
Comment on lines
+152
to
+156
| if (message.isHumanInputResponse && message.role === 'system') { | ||
| const match = message.content.match( | ||
| /^User responded to question "(.*?)": ([\s\S]+)$/, | ||
| ); | ||
| const response = match?.[2] ?? message.content; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import re
current = re.compile(r'^User responded to question "(.*?)": ([\s\S]+)$')
fixed = re.compile(r'^User responded to question "([\s\S]*?)": ([\s\S]*)$')
sample = 'User responded to question "Line 1\nLine 2": answer'
for name, pattern in [('current', current), ('fixed', fixed)]:
match = pattern.match(sample)
print(name, bool(match), match.group(2) if match else None)
PYRepository: tale-project/tale
Length of output: 97
🏁 Script executed:
# Check the message-list.tsx file at the specified lines
cat -n services/platform/app/features/automations/components/automation-assistant/message-list.tsx | sed -n '145,165p'Repository: tale-project/tale
Length of output: 1015
🏁 Script executed:
# Search for the duplicate pattern in chat-messages.tsx
rg "User responded to question" services/platform/app/features/chat/components/Repository: tale-project/tale
Length of output: 188
🏁 Script executed:
# Get context around the chat-messages.tsx match
rg -B 5 -A 5 "User responded to question" services/platform/app/features/chat/components/chat-messages.tsxRepository: tale-project/tale
Length of output: 502
🏁 Script executed:
# Search for any other occurrences of this regex pattern in the codebase
rg "User responded to question" services/Repository: tale-project/tale
Length of output: 936
Fix the regex to handle multiline questions in human-input responses.
The regex pattern /^User responded to question "(.*?)": ([\s\S]+)$/ uses .*? which stops at newline characters. Since multiline text_input prompts are supported, questions containing newlines won't match, causing the fallback to display the entire system message instead of just the user's answer. Change the pattern to use [\s\S]*? for the question capture group at line 154 here and in services/platform/app/features/chat/components/chat-messages.tsx.
Suggested fix
const match = message.content.match(
- /^User responded to question "(.*?)": ([\s\S]+)$/,
+ /^User responded to question "([\s\S]*?)": ([\s\S]+)$/,
);📝 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
| if (message.isHumanInputResponse && message.role === 'system') { | |
| const match = message.content.match( | |
| /^User responded to question "(.*?)": ([\s\S]+)$/, | |
| ); | |
| const response = match?.[2] ?? message.content; | |
| if (message.isHumanInputResponse && message.role === 'system') { | |
| const match = message.content.match( | |
| /^User responded to question "([\s\S]*?)": ([\s\S]+)$/, | |
| ); | |
| const response = match?.[2] ?? message.content; |
🤖 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-assistant/message-list.tsx`
around lines 152 - 156, The regex used to parse human-input responses (inside
the block checking message.isHumanInputResponse && message.role === 'system')
fails on multiline questions because it uses .*? for the question capture;
update the pattern to use [\s\S]*? for the question group so it matches newlines
(i.e., change the question capture from "(.*?)" to "([\s\S]*?)") and apply the
same change in services/platform/app/features/chat/components/chat-messages.tsx
where the identical pattern is used; ensure the variables match and that
response still falls back to message.content when match is null.
Comment on lines
+20
to
+31
| const lines = formatted.split('\n'); | ||
| const nonEmptyLines = lines.filter((l) => l.trim() !== ''); | ||
| const previewLines = nonEmptyLines.slice(0, 2); | ||
| const preview = previewLines.join(' '); | ||
| const lastPreviewIdx = | ||
| previewLines.length > 0 | ||
| ? lines.indexOf(previewLines[previewLines.length - 1]) | ||
| : 0; | ||
| const rest = lines | ||
| .slice(lastPreviewIdx + 1) | ||
| .join('\n') | ||
| .trimStart(); |
There was a problem hiding this comment.
Split preview and expanded content by source index.
If the first two non-empty lines contain the same text, rest is sliced after the first matching value and the expanded section repeats content that is already in the preview. Build the preview from { line, index } pairs so the split point comes from the original position instead of string equality.
🔧 Suggested fix
const formatted = content.replace(
/\[([A-Z][A-Z_]+)\]/g,
(_, tag: string) => `${tag.replaceAll('_', ' ')} -`,
);
const lines = formatted.split('\n');
- const nonEmptyLines = lines.filter((l) => l.trim() !== '');
- const previewLines = nonEmptyLines.slice(0, 2);
+ const previewEntries = lines
+ .map((line, index) => ({ line, index }))
+ .filter(({ line }) => line.trim() !== '')
+ .slice(0, 2);
+ const previewLines = previewEntries.map(({ line }) => line);
const preview = previewLines.join(' ');
const lastPreviewIdx =
- previewLines.length > 0
- ? lines.indexOf(previewLines[previewLines.length - 1])
- : 0;
+ previewEntries.length > 0
+ ? previewEntries[previewEntries.length - 1].index
+ : -1;
const rest = lines
.slice(lastPreviewIdx + 1)
.join('\n')
.trimStart();📝 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
| const lines = formatted.split('\n'); | |
| const nonEmptyLines = lines.filter((l) => l.trim() !== ''); | |
| const previewLines = nonEmptyLines.slice(0, 2); | |
| const preview = previewLines.join(' '); | |
| const lastPreviewIdx = | |
| previewLines.length > 0 | |
| ? lines.indexOf(previewLines[previewLines.length - 1]) | |
| : 0; | |
| const rest = lines | |
| .slice(lastPreviewIdx + 1) | |
| .join('\n') | |
| .trimStart(); | |
| const lines = formatted.split('\n'); | |
| const previewEntries = lines | |
| .map((line, index) => ({ line, index })) | |
| .filter(({ line }) => line.trim() !== '') | |
| .slice(0, 2); | |
| const previewLines = previewEntries.map(({ line }) => line); | |
| const preview = previewLines.join(' '); | |
| const lastPreviewIdx = | |
| previewEntries.length > 0 | |
| ? previewEntries[previewEntries.length - 1].index | |
| : -1; | |
| const rest = lines | |
| .slice(lastPreviewIdx + 1) | |
| .join('\n') | |
| .trimStart(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/chat/components/collapsible-system-message.tsx`
around lines 20 - 31, The current split uses string equality to find
lastPreviewIdx causing duplicate content when lines repeat; change nonEmptyLines
and previewLines to preserve original indices (e.g., map lines to objects like
{line, index}), build previewLines by slicing the array of {line,index} pairs
(use nonEmptyWithIndex = lines.map((l, i) => ({line: l, index: i})).filter(p =>
p.line.trim() !== '')), compute lastPreviewIdx from the last preview pair's
index (previewLines[previewLines.length-1].index), and then compute rest by
slicing the original lines starting at lastPreviewIdx + 1; update preview to
join previewLines.map(p => p.line) accordingly so preview and rest split by
source index not string equality.
Comment on lines
+135
to
+137
| ...(_args.knowledgeFileIds?.length | ||
| ? { knowledgeFileIds: _args.knowledgeFileIds } | ||
| : {}), |
There was a problem hiding this comment.
Do not drop an explicitly empty knowledgeFileIds array.
When resolution yields [], this branch omits the field entirely, which can unintentionally remove scoping and broaden retrieval. Preserve empty arrays if they were explicitly resolved.
🔧 Proposed fix
- ...(_args.knowledgeFileIds?.length
- ? { knowledgeFileIds: _args.knowledgeFileIds }
- : {}),
+ ...(_args.knowledgeFileIds !== undefined
+ ? { knowledgeFileIds: _args.knowledgeFileIds }
+ : {}),📝 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
| ...(_args.knowledgeFileIds?.length | |
| ? { knowledgeFileIds: _args.knowledgeFileIds } | |
| : {}), | |
| ...(_args.knowledgeFileIds !== undefined | |
| ? { knowledgeFileIds: _args.knowledgeFileIds } | |
| : {}), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/convex/workflow_engine/helpers/nodes/llm/execute_agent_with_tools.ts`
around lines 135 - 137, The code currently omits knowledgeFileIds when its value
is an empty array because it uses a truthy check; update the merge so it only
omits the field when knowledgeFileIds is undefined (not when it's []), e.g. in
execute_agent_with_tools.ts where _args.knowledgeFileIds is merged, replace the
conditional that tests truthiness with one that tests !== undefined or uses
Object.prototype.hasOwnProperty to preserve explicitly-resolved empty arrays so
an explicit [] remains in the resulting payload.
…aiting approval Workflows waiting for human approval (waitingFor field set) can legitimately be idle for extended periods. Add a 24-hour extension to the stuck recovery timeout so these executions aren't incorrectly marked as failed.
On-demand sync already covers all user-facing scenarios: page view triggers syncSingleWebsite, table load triggers syncStatuses, and new website creation schedules a delayed sync. The background cron is unnecessary.
larryro
deleted the
feat/workflow-schema-validation-and-knowledge-scoping
branch
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
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
knowledgeFileIdsconfig to LLM nodes sorag_searchis automatically scoped to specified filesfileId+fileName) instead of plain IDsTest plan
resolve_knowledge_file_idsvalidate_workflow_inputwith nested schemasmessage-listtests for new approval card typesSummary by CodeRabbit
New Features
Refactor
Documentation