feat(core): inject workflow run context into orchestrator prompt#1065
feat(core): inject workflow run context into orchestrator prompt#1065
Conversation
After a workflow completes, the AI had no awareness of results when answering follow-up questions. This adds a "Recent Workflow Results" section to the orchestrator prompt by querying persisted workflow_result messages from the conversation. Changes: - Add getRecentWorkflowResultMessages() to db/messages.ts - Add WorkflowResultContext type and formatWorkflowContextSection() to prompt-builder.ts - Extend buildFullPrompt() with optional workflowContext parameter - Fetch and inject workflow context in handleMessage() before prompt building Fixes #1055 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements workflow context injection into the orchestrator prompt by adding a database query function to retrieve recent workflow result messages, a formatter to structure them as prompt text, and orchestrator agent logic to fetch and inject this context before building the AI prompt. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Orchestrator as orchestrator-agent.ts
participant DB as messages.ts
participant Formatter as prompt-builder.ts
participant AI as buildFullPrompt
User->>Orchestrator: handleMessage(conversationId, ...)
Orchestrator->>DB: getRecentWorkflowResultMessages(conversationId, limit=3)
DB->>DB: Build database-specific SQL<br/>(PostgreSQL or SQLite JSON syntax)
DB-->>Orchestrator: Promise<MessageRow[]>
Orchestrator->>Orchestrator: Parse each message metadata<br/>Extract workflowName, runId
Orchestrator->>Formatter: formatWorkflowContextSection(results)
Formatter->>Formatter: Build markdown section<br/>if results.length > 0
Formatter-->>Orchestrator: workflowContext: string
Orchestrator->>AI: buildFullPrompt(..., workflowContext?)
AI->>AI: Inject workflow context<br/>after Thread Context block
AI-->>Orchestrator: fullPrompt: string
Orchestrator-->>User: AI response with context awareness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Comprehensive PR ReviewPR: #1065 - feat(core): inject workflow run context into orchestrator prompt SummaryThis PR introduces a clean, well-architected feature - injecting recent workflow results into the orchestrator prompt for better follow-up awareness. The non-blocking enrichment design, token budget (LIMIT 3 at SQL level), and pure formatter function are all solid. However, two runtime bugs make the feature completely non-functional in every deployment, and zero tests were added for three new testable code units. Verdict: REQUEST_CHANGES
Critical IssuesWrong metadata filter key - feature is a complete no-opLocation: packages/core/src/db/messages.ts:78-81 The query filters on metadata.category = workflow_result, but category is never persisted to the database. It exists only in the BufferedSegment struct for SSE streaming logic. The actual DB metadata for a workflow result message looks like { "workflowResult": { "workflowName": "...", "runId": "..." } }. The query returns 0 rows on every call. workflowContext is always undefined. The feature has no effect in production. Fix: Change the metadata filter to check for the presence of the workflowResult key: Evidence: persistence.ts:259-263 shows category is NOT included in the flushed metadata - only toolCalls, workflowDispatch, and workflowResult are persisted. High IssuesJSON.parse on JSONB column always throws in PostgreSQLLocation: packages/core/src/orchestrator/orchestrator-agent.ts:763 node-postgres automatically deserializes JSONB columns into JS objects. Calling JSON.parse(msg.metadata) on an object coerces it to "[object Object]" and throws SyntaxError. The inner catch silently swallows it - so even after the CRITICAL fix above, PostgreSQL users will always see workflowName: unknown and runId: unknown. Fix: Guard with typeof before parsing: formatWorkflowContextSection - pure function, zero testsLocation: packages/core/src/orchestrator/prompt-builder.ts:51-65 Pure formatter with four untested behaviors: empty-input early return, header construction, per-result formatting, trimEnd(). Any regression silently corrupts the orchestrator prompt. Zero dependencies - trivial to test. Add to prompt-builder.test.ts. getRecentWorkflowResultMessages - DB query, zero testsLocation: packages/core/src/db/messages.ts:73-97 The dialect switch (PostgreSQL JSON extraction vs SQLite json_extract) is completely untested. The non-throwing contract is load-bearing. getDatabaseType is a new import not present in the existing connection mock in messages.test.ts. Must add getDatabaseType to the mock and add tests covering both SQL dialects, parameter binding, default limit, non-throwing contract, and successful row return. Medium IssuesSilent JSON.parse failure - no structured logLocation: packages/core/src/orchestrator/orchestrator-agent.ts:762-769 The inner catch block has only an inline comment - no getLog().warn(), no conversationId, no messageId. The outer catch never sees it. A corrupt metadata row is completely undiagnosable in production. Fix: Add one line inside the catch: getLog().warn({ err: parseErr, conversationId, messageId: msg.id }, orchestrator.workflow_result_metadata_parse_failed); handleMessage workflow-context block - no integration coverageLocation: packages/core/src/orchestrator/orchestrator-agent.ts:748-785 ../db/messages is not mocked in orchestrator-agent.test.ts. formatWorkflowContextSection is missing from the ./prompt-builder mock - any test reaching the AI routing path would throw TypeError: formatWorkflowContextSection is not a function. Both mocks need to be added. Low Issues
What is Good
Next Steps
Reviewed by Archon comprehensive-pr-review workflow |
- CRITICAL: fix metadata filter in getRecentWorkflowResultMessages to check for workflowResult key presence instead of category (which is never persisted to DB); feature was completely non-functional on every call - HIGH: guard JSON.parse(msg.metadata) with typeof check to handle PostgreSQL JSONB columns returned as objects (not strings) by node-postgres - MEDIUM: add structured warn log inside inner metadata parse catch block - LOW: use SELECT id, content, metadata instead of SELECT * in new DB query - LOW: update comments in messages.ts and prompt-builder.ts for accuracy - Tests: add formatWorkflowContextSection unit tests (pure function coverage) - Tests: add getRecentWorkflowResultMessages tests (dialect switch + contract) - Tests: add getDatabaseType mock to messages.test.ts connection mock - Tests: add ../db/messages mock and formatWorkflowContextSection to prompt-builder mock in orchestrator-agent.test.ts - Tests: add handleMessage workflow context injection behavioral tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix ReportAll 11 review findings addressed. Commit: bf8bc8e CRITICAL Fixed: Wrong metadata filter key (
|
| File | New Tests |
|---|---|
prompt-builder.test.ts |
5 tests for formatWorkflowContextSection (empty, header, per-result format, multiple, trimEnd) |
messages.test.ts |
6 tests for getRecentWorkflowResultMessages (PG SQL, SQLite SQL, params, default limit=3, non-throwing, returns rows) + getDatabaseType added to connection mock |
orchestrator-agent.test.ts |
5 integration tests for handleMessage workflow context injection + formatWorkflowContextSection and ../db/messages mocks added |
Validation: type-check ✓, lint ✓, format ✓, test ✓ (0 failures)
Archon PR Validation ReportVerdict: APPROVE SummaryBug confirmed on main across all four claims — workflow result summaries are persisted to DB but never loaded into the orchestrator prompt. The fix is minimal and correct: a targeted DB query ( Bug Confirmation
IssuesNo blocking issues found. Minor suggestion (non-blocking): Type widening cast in Fix Quality: 5/5 | Confidence: HIGHValidated by archon-validate-pr workflow |
…1055 feat(core): inject workflow run context into orchestrator prompt
Summary
workflow_resultmessages, butbuildFullPrompt()never loaded them — wasted data and broken UX.getRecentWorkflowResultMessages()DB query inmessages.ts, (2)formatWorkflowContextSection()formatter inprompt-builder.ts, (3)handleMessage()fetches and injects the context intobuildFullPrompt()via a new optionalworkflowContextparameter.buildFullPrompt()output is byte-for-byte identical when no workflow results exist.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
handleMessage()messageDb.getRecentWorkflowResultMessages()handleMessage()formatWorkflowContextSection()handleMessage()buildFullPrompt()workflowContextoptional parambuildFullPrompt()Label Snapshot
risk: lowsize: Scorecore:orchestratorChange Metadata
featurecoreLinked Issue
Validation Evidence (required)
Security Impact (required)
Compatibility / Migration
workflowContextis optional; when undefined,buildFullPrompt()output is byte-for-byte identical to pre-PR behaviorremote_agent_messagesrows with existingmetadataJSON structureHuman Verification (required)
bun run validateformatWorkflowContextSection([])returns''(no empty section injected); malformed metadata JSON defaults to'unknown'names but still surfaces the summary content; DB errors return[]non-throwing with warn logSide Effects / Blast Radius (required)
@archon/coreorchestrator prompt building onlylimit=3defaultformatWorkflowContextSection([])returns''so no noise when no workflows have runRollback Plan (required)
git revert 3e3ddf25)workflowContextbeing non-empty, which requires actual workflow result rows in the DBRisks and Mitigations
buildFullPrompt()output changes for existing conversations with workflow historyworkflowContextis truthy;getRecentWorkflowResultMessages()returns[]for conversations with no workflow results (no change at all)'unknown'for workflowName/runId while still including the summary contentSummary by CodeRabbit
Release Notes