feat(platform): resolve agent from thread metadata and fix workflow output extraction#828
Conversation
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 PR adds Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/convex/agent_tools/workflows/internal_mutations.ts`:
- Around line 112-118: The loop currently assigns chatAgent conditionally then
unconditionally breaks, which is confusing; change the iteration to only break
when you've found the matching agent: iterate over agentQuery and inside the
loop check agent.organizationId === organizationId, set chatAgent and then break
(or continue for non-matching entries) so the loop intent is clear — update the
block around chatAgent, agentQuery and organizationId to perform the conditional
assignment and the break together.
In
`@services/platform/convex/workflow_engine/helpers/engine/serialize_and_complete_execution_handler.test.ts`:
- Around line 15-17: Replace the duplicated local helper with the shared
implementation: remove the local function isRecord and import the exported
isRecord from the shared type-guards module (the common "isRecord" export used
in production) so tests use the same type-guard as production; update any
references in serialize_and_complete_execution_handler.test.ts to use the
imported isRecord.
🪄 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: 765d4280-b32e-4674-b14d-81a25b0edf54
📒 Files selected for processing (10)
services/platform/convex/agent_tools/workflows/internal_mutations.tsservices/platform/convex/custom_agents/test_chat.tsservices/platform/convex/custom_agents/unified_chat.tsservices/platform/convex/custom_agents/webhooks/internal_mutations.tsservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/threads/schema.tsservices/platform/convex/workflow_engine/helpers/engine/on_workflow_complete.test.tsservices/platform/convex/workflow_engine/helpers/engine/on_workflow_complete.tsservices/platform/convex/workflow_engine/helpers/engine/serialize_and_complete_execution_handler.test.tsservices/platform/convex/workflow_engine/helpers/engine/serialize_and_complete_execution_handler.ts
…utput extraction Store customAgentId on thread metadata so workflow completion responses resolve the correct agent instead of searching for the system chat agent. Also fix output extraction to use the nested variables namespace and return null (instead of sanitized variables) when no __workflowOutput exists.
…letion Replace for-await loop with .filter().first() to fix a logical defect where the break statement was outside the org check, always exiting on the first iteration regardless of organization match.
5ed29f4 to
aacda20
Compare
…utput extraction (#828)
Summary
customAgentIdon thread metadata when starting agent chat, then resolve the correct agent from thread metadata intriggerWorkflowCompletionResponseinstead of falling back to the system default chat agent. This ensures workflow completion responses use the same agent configuration as the original conversation.__workflowOutputextraction to look in the nestedvars.variablesnamespace (wherepersistExecutionResultstores it). When no explicit output node exists, returnnullinstead of sanitized variables — variables are stored separately and should not leak as output.downloadUrl) when workflow output contains them.Test plan
serialize_and_complete_execution_handler.test.ts— covers nested variables extraction, null fallback, storage ref resolutionon_workflow_complete.test.ts— covers null output when no persisted output existsSummary by CodeRabbit
New Features
Bug Fixes
Chores