fix(flows): resolve a non-owner Slack DM recipient via a lookup node#4955
Conversation
…ia a lookup node The self-DM fix (tinyhumansai#4933/tinyhumansai#4941) surfaces the connected owner's own platform_user_id so "DM me" works, but gave the workflow_builder no way to resolve a NAMED recipient who isn't the account owner (e.g. "DM Alan Johnson" or an email). Extend the builder's standing prompt so a non-owner DM target is resolved via an upstream tool_call lookup node (SLACK_FIND_USERS with email + exact_match for a safe single match, or SLACK_LIST_ALL_USERS + a filter as fallback), with the resolved id bound into the send node's channel arg via an = expression. Preserves the existing owner platform_user_id fast path and the ask-when-ambiguous safety rule for unverified name matches. No new Composio action needed — SLACK_FIND_USERS/SLACK_LIST_ALL_USERS were already curated (featured) and discoverable via search_tool_catalog.
📝 WalkthroughWalkthroughThe workflow-builder prompt now explains platform-agnostic resolution of non-owner direct-message recipients, ambiguity handling, contract-validated wiring, and optional conversation setup. Regression assertions verify the guidance and reject toolkit-specific hardcoding. ChangesNon-owner DM resolution
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
agent
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e641f57ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - Bind the resolved id into the send node's `channel` arg with an `=` | ||
| expression off the lookup node (`get_tool_contract` names the exact | ||
| output field; confirm with `dry_run_workflow` rather than guessing), | ||
| same as the owner path above. |
There was a problem hiding this comment.
For non-owner Slack DMs, this tells the builder to pass the user id returned by SLACK_FIND_USERS directly into SLACK_SEND_MESSAGE.channel. I checked the captured Composio Slack contracts in tests/fixtures/composio_slack.json: SLACK_SEND_MESSAGE says DMs need the D-channel returned by SLACK_OPEN_DM and that passing a user ID directly causes channel_not_found, while SLACK_OPEN_DM accepts user IDs and returns channel.id. A flow generated for “DM alan@…” will therefore dry-run with an unverifiable binding but fail at runtime; the prompt should insert an SLACK_OPEN_DM node and bind the send node to its returned DM channel.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, and confirmed against the same fixture (tests/fixtures/composio_slack.json) — thank you. Rather than hardcoding a Slack-specific SLACK_OPEN_DM insertion (which would violate this project's platform-agnostic rule for builder prompt guidance — the non-owner-DM case has to work identically for Slack/Discord/Telegram/etc.), I generalized the fix: the guidance now has the builder get_tool_contract the send action itself and check whether it names a separate open/create-conversation action as a prerequisite before it can accept a resolved user id as the recipient. If so, the builder wires that lookup as its own tool_call node between the user-lookup node and the send node; if the send action accepts a user id directly (as some toolkits do), it skips straight to the send. This covers the exact Slack SLACK_OPEN_DM → SLACK_SEND_MESSAGE case you found without special-casing Slack in the prompt text. See the updated "Check the send action's own get_tool_contract for a required 'open conversation' step first" step in prompt.md.
The initial non-owner-DM guidance hardcoded Slack's SLACK_FIND_USERS / SLACK_LIST_ALL_USERS slugs and args, violating the project's rule that builder prompt fixes must be architectural/platform-agnostic. Rework the guidance to teach the general pattern instead: search_tool_catalog the target toolkit for its own user-lookup action, wire it as a tool_call node upstream of the send, prefer an unambiguous email/exact lookup over a name search (which may return multiple people and requires bucket-3 confirmation), and check the send action's own get_tool_contract for a required open-conversation prerequisite before wiring the send node — which generally resolves the "Open Slack DMs before sending" P2 without a Slack special case. Updates builder_prompt.rs's standing-prompt test to assert the platform-neutral pattern and to reject the previously-hardcoded Slack slugs/args.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/flows/agents/workflow_builder/builder_prompt.rs (1)
403-464: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression assertions for the contract-sensitive parts of this flow.
The test covers discovery, upstream placement, cardinality, ambiguity confirmation, and conversation setup, but not the fallback list/filter path,
dry_run_workflow, or the required tool-call envelope (=nodes...item.json.data.<id_field>). The prompt currently contains these rules, so they could regress without this test failing.Proposed assertions
+ assert!(STANDING_PROMPT.contains("Prefer an **email / exact lookup")); + assert!(STANDING_PROMPT.contains("fall back to its \"list users\" style action")); + assert!(STANDING_PROMPT.contains("dry_run_workflow")); + assert!(STANDING_PROMPT.contains("=nodes.find_alan.item.json.data.<id_field>"));🤖 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 `@src/openhuman/flows/agents/workflow_builder/builder_prompt.rs` around lines 403 - 464, Add regression assertions alongside the existing STANDING_PROMPT checks for the fallback list/filter resolution path, the required dry_run_workflow guidance, and the tool-call envelope using `=nodes...item.json.data.<id_field>`. Keep these assertions focused on the prompt contract and ensure they fail if any of those rules are removed or altered.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@src/openhuman/flows/agents/workflow_builder/builder_prompt.rs`:
- Around line 403-464: Add regression assertions alongside the existing
STANDING_PROMPT checks for the fallback list/filter resolution path, the
required dry_run_workflow guidance, and the tool-call envelope using
`=nodes...item.json.data.<id_field>`. Keep these assertions focused on the
prompt contract and ensure they fail if any of those rules are removed or
altered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6093434-1e7d-4d3a-bedf-e317dbbced9f
📒 Files selected for processing (2)
src/openhuman/flows/agents/workflow_builder/builder_prompt.rssrc/openhuman/flows/agents/workflow_builder/prompt.md
|
Addressed the nitpick on |
1 participant
Summary
The merged self-DM fix (#4933/#4941) surfaces the connected owner's own
platform_user_idonlist_flow_connections, so "DM me" wires correctly.But when the user asks the
workflow_builderto DM someone else byname or email (a teammate, "DM Alan Johnson"), the builder had no way to
resolve that person's Slack member id — no matching
platform_user_idexists for a non-owner, so it either nagged the user or built a broken
graph.
This is a prompt-only fix — no new Composio action, no new tool code.
SLACK_FIND_USERSandSLACK_LIST_ALL_USERSwere already in the curatedSlack catalog (
src/openhuman/memory_sync/composio/providers/catalogs_messaging.rs,lines 12-15 and 28-31), which means they already come back
featured: truefrom
search_tool_catalogand are fully discoverable — nothing to changethere.
Resolution mechanism (confirmed against the real Composio contract)
I pulled the live Slack tool contracts from
tests/fixtures/composio_slack.json(a captured Composio catalog snapshot) to confirm the exact input shape
rather than guessing:
SLACK_FIND_USERSaccepts anemailparameter directly — "This is aconvenience parameter that automatically performs an email-based search."
Paired with
exact_match: true, this uses "Slack's dedicated email lookupendpoint" and returns at most one match, which is what makes it safe to
bind into the send node without asking the user anything.
SLACK_FIND_USERSalso acceptssearch_queryfor a name-based search,but per its own description "Name-based queries can return multiple
matches — verify exactly one user ID before passing to downstream tools."
So a name-only ask is only auto-wired when it resolves to exactly one
match; otherwise the builder must ask the user to confirm (bucket-3
ambiguity), matching the existing "never DM an unverified same-name
person" posture.
SLACK_LIST_ALL_USERS+ a downstreamtransform/codefilter onemail/display_name/real_nameis documented as the fallback only forwhen
SLACK_FIND_USERSitself can't resolve the person (e.g. a workspacethat restricts email visibility) —
SLACK_FIND_USERSexplicitlyrecommends itself over the list+filter approach ("Prefer SLACK_FIND_USERS
for targeted lookups").
What changed
src/openhuman/flows/agents/workflow_builder/prompt.md— extended theexisting "to me / message me / DM me" bucket-2 guidance (added by fix(flows): surface connected platform_user_id so the builder wires self-DMs (no #general fallback) #4933)
with a new case: a named recipient who is not the connected owner.
Teaches the builder to wire a
tool_calllookup node(
SLACK_FIND_USERS { email, exact_match: true }, or theSLACK_LIST_ALL_USERS+ filter fallback for a name-only ask) upstream ofthe
SLACK_SEND_MESSAGEnode, then bind the resolved id intochannelvia an
=expression — plus a concise worked example (schedule →find-user-by-email → send-DM). The existing owner
platform_user_idfastpath and the "ask when ambiguous" safety rule are both preserved verbatim.
src/openhuman/flows/agents/workflow_builder/builder_prompt.rs—extended
standing_prompt_teaches_plain_language_and_readonly_memory(the existing contract test for the standing prompt) with new positive
assertions covering the non-owner case, the
SLACK_FIND_USERSemail/exact_match wiring, the
SLACK_LIST_ALL_USERSfallback, and theask-when-ambiguous safety rule — mirroring the style of the existing
self-DM assertions from fix(flows): surface connected platform_user_id so the builder wires self-DMs (no #general fallback) #4933.
Deviations from the brief
so the "only if the catalog genuinely lacks it" branch didn't apply.
"prompt-first" instruction.
Test plan
GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml— cleanGGML_NATIVE=OFF cargo test --lib openhuman::flows::— 387 passed, 0failed, including the extended
builder_prompt::tests::standing_prompt_teaches_plain_language_and_readonly_memorycargo fmt --manifest-path Cargo.toml— clean (no other diffs)Summary by CodeRabbit