Skip to content

fix: resolve #825 — slack-primitive: persist askQuestion exchanges to workflow run record #834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SlncTrZ wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: resolve #825 — slack-primitive: persist askQuestion exchanges to workflow run record #834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb1bf37
fix: resolve #825 — slack-primitive: persist askQuestion exchanges to…
SlncTrZ May 10, 2026
7134cc9
fix: resolve #825 — slack-primitive: persist askQuestion exchanges to…
SlncTrZ May 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/runner/src/execution/slack-ask-question-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Step } from '../../types';
import { updateRunAskQuestionExchanges } from '../persistence/runs';

export async function handleSlackAskQuestion(
step: Step,
runId: string,
timeoutMs: number
): Promise<void> {
try {
const result = await step.waitForReply(timeoutMs);
const exchange = {
question: step.input.question,
timestamp: new Date(),
reply: result.reply,
timeout: result.timeout === true
};
Comment on lines +11 to +16
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Exchange object fields don't match the AskQuestionExchange interface from the same PR

The exchange object constructed in handleSlackAskQuestion uses field names and types that are completely misaligned with the AskQuestionExchange interface defined in packages/runner/src/types/workflow-run-record.ts:10-20:

  • question → should be questionText
  • timestamp (Date) → should be questionTimestamp (string)
  • reply → should be replyText
  • timeout (boolean) → should be timeoutOutcome ('timeout' | 'cancelled' | null)
  • Missing required field channel

Since updateRunAskQuestionExchanges doesn't exist yet (the persistence module is unimplemented), there is no compile-time type constraint on this object — the mismatch will silently persist until the persistence layer is wired up, at which point either this code will break or the wrong data will be stored.

Prompt for agents 
The exchange object built at slack-ask-question-handler.ts:11-16 uses field names (question, timestamp, reply, timeout) that don't match the AskQuestionExchange interface defined at workflow-run-record.ts:10-20 (questionText, questionTimestamp, replyText, timeoutOutcome, channel). The handler should import AskQuestionExchange and construct an object conforming to it. Specifically: (1) rename 'question' to 'questionText', (2) rename 'timestamp' to 'questionTimestamp' and convert it to a string, (3) rename 'reply' to 'replyText', (4) replace the boolean 'timeout' with 'timeoutOutcome' using the correct union type ('timeout' | 'cancelled' | null), (5) add the required 'channel' field (likely from step.input or step.config). The import of AskQuestionExchange should come from '../types/workflow-run-record'.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

await updateRunAskQuestionExchanges(runId, exchange);
Comment on lines +11 to +17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Persisted exchange payload does not match the new run-record schema.

On Line 11–16, the object uses question/timestamp/reply/timeout, but the new AskQuestionExchange schema expects fields like channel, questionTimestamp, questionText, replyText, replyTimestamp, and timeoutOutcome. This will either fail typing upstream or persist incomplete data that misses #825’s required forensic fields.

Suggested mapping update 
+import type { AskQuestionExchange } from '../types/workflow-run-record';

-    const exchange = {
-      question: step.input.question,
-      timestamp: new Date(),
-      reply: result.reply,
-      timeout: result.timeout === true
-    };
+    const exchange: AskQuestionExchange = {
+      channel: step.input.channel,
+      questionTimestamp: step.input.questionTs,
+      questionText: step.input.question,
+      replierUserId: result.replierUserId,
+      replyText: result.reply,
+      replyTimestamp: result.replyTs,
+      matchedChoices: result.matchedChoices,
+      matchedRegexGroups: result.matchedRegexGroups,
+      timeoutOutcome: result.timeout ? 'timeout' : null,
+    };
📝 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 exchange = {
question: step.input.question,
timestamp: new Date(),
reply: result.reply,
timeout: result.timeout === true
};
await updateRunAskQuestionExchanges(runId, exchange);
import type { AskQuestionExchange } from '../types/workflow-run-record';
const exchange: AskQuestionExchange = {
channel: step.input.channel,
questionTimestamp: step.input.questionTs,
questionText: step.input.question,
replierUserId: result.replierUserId,
replyText: result.reply,
replyTimestamp: result.replyTs,
matchedChoices: result.matchedChoices,
matchedRegexGroups: result.matchedRegexGroups,
timeoutOutcome: result.timeout ? 'timeout' : null,
};
await updateRunAskQuestionExchanges(runId, exchange);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/runner/src/execution/slack-ask-question-handler.ts` around lines 11
- 17, The persisted exchange object currently created in
slack-ask-question-handler (variable exchange) uses
question/timestamp/reply/timeout which doesn't match the AskQuestionExchange
schema; update construction to populate the schema fields expected by
updateRunAskQuestionExchanges: set channel from the Slack context (e.g.,
step.input.channel or handler context), questionTimestamp to the original
timestamp (new Date() or step.input.timestamp if available), questionText from
step.input.question, replyText from result.reply, replyTimestamp from
result.replyTimestamp or now when reply arrived, and timeoutOutcome from
result.timeout (map boolean to the enum/value expected); ensure the object you
pass to updateRunAskQuestionExchanges uses these property names (channel,
questionTimestamp, questionText, replyText, replyTimestamp, timeoutOutcome) so
typing and persisted forensic fields are complete.

} catch (error) {
throw error;
}
Comment on lines +9 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify useless catch/rethrow patterns in TS files
ast-grep --lang ts --pattern $'try { $$$BODY } catch ($ERR) { throw $ERR; }'

Repository: AgentWorkforce/relay

Length of output: 1143

Remove the no-op catch/rethrow wrapper.

The try-catch block at lines 9–20 catches and immediately rethrows the error unchanged, which violates the no-useless-catch rule and adds unnecessary noise. Remove the entire try-catch and let the error propagate naturally.

🧰 Tools
🪛 ESLint

[error] 9-20: Unnecessary try/catch wrapper.

(no-useless-catch)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/runner/src/execution/slack-ask-question-handler.ts` around lines 9 -
20, Remove the no-op try/catch wrapper around the await calls: delete the try {
... } catch (error) { throw error; } block and leave the two awaits as-is so
errors from step.waitForReply(timeoutMs) and
updateRunAskQuestionExchanges(runId, exchange) propagate naturally; keep
construction of the exchange object (question, timestamp, reply, timeout) and
preserve existing variable names (step, timeoutMs, exchange, runId).

}
20 changes: 20 additions & 0 deletions packages/runner/src/types/workflow-run-record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export interface WorkflowRunRecord {
id: string;
workflowId: string;
status: 'pending' | 'running' | 'completed' | 'failed' | 'cancelled';
createdAt: Date;
updatedAt: Date;
askQuestionExchanges?: AskQuestionExchange[];
}

export interface AskQuestionExchange {
channel: string;
questionTimestamp: string;
questionText: string;
replierUserId?: string;
replyText?: string;
replyTimestamp?: string;
matchedChoices?: string[];
matchedRegexGroups?: Record<string, string>;
timeoutOutcome?: 'timeout' | 'cancelled' | null;
}