← Back to PRs

#19430: Slack: infer bare user targets before media upload

by gg2uah open 2026-02-17 19:55 View on GitHub →
channel: slack size: S
## Summary - Problem: `openclaw message send --channel slack --target U... --media ...` could fail with Slack `invalid_arguments`. - Why it matters: sending files directly to a Slack user DM is a core path and currently breaks for bare user IDs. - What changed: Slack target parsing now infers bare ID kind by prefix (`U/W -> user`, `C/G/D/Z -> channel`) and uses the correct DM open flow before upload. - What did NOT change (scope boundary): no changes to delivery payload formats, no Slack API surface expansion, no behavior changes for explicit `user:`/`channel:` targets. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [x] Integrations - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes # - Related #19251 ## User-visible / Behavior Changes Bare Slack user IDs (for example `U12345678`) now route as user targets, so media sends open a DM channel and upload successfully instead of being treated as a channel ID. ## Security Impact (required) - New permissions/capabilities? (`No`) - Secrets/tokens handling changed? (`No`) - New/changed network calls? (`No`) - Command/tool execution surface changed? (`No`) - Data access scope changed? (`No`) - If any `Yes`, explain risk + mitigation: ## Repro + Verification ### Environment - OS: macOS (local dev) - Runtime/container: Node 22 / pnpm - Model/provider: N/A - Integration/channel (if any): Slack - Relevant config (redacted): Slack bot configured; target passed as bare `U...` ### Steps 1. Configure Slack and send a media message with a bare user target: `openclaw message send --channel slack --target U12345678 --media /tmp/file.txt --message "..."`. 2. Observe Slack upload path behavior. ### Expected - Bare `U...` target is treated as a user, a DM channel is opened, and `files.uploadV2` uses the DM `channel_id`. ### Actual - Before fix, bare `U...` was treated as a channel target and Slack returned `invalid_arguments` for upload. ## Evidence - [x] Failing test/log before + passing after - [ ] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - `pnpm test src/slack/targets.test.ts src/slack/send.media.test.ts` passes. - Live smoke path (before this commit) validated DM open + file upload succeeds for bare `U...` target. - Edge cases checked: - Bare `U...`/`W...` infer as `user`; bare `C...` stays `channel`. - `resolveSlackChannelId("U...")` correctly rejects non-channel target. - What you did **not** verify: - Did not run a full Slack end-to-end with every Slack conversation type in this PR cycle. ## Compatibility / Migration - Backward compatible? (`Yes`) - Config/env changes? (`No`) - Migration needed? (`No`) - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Revert commit `91300f08e`. - Files/config to restore: - `src/slack/targets.ts` - `src/slack/targets.test.ts` - `src/slack/send.media.test.ts` - Known bad symptoms reviewers should watch for: - Bare Slack IDs no longer normalizing to expected target kinds. ## Risks and Mitigations - Risk: Slack ID prefix inference could misclassify unknown ID shapes. - Mitigation: Inference only applies to known Slack ID prefixes/patterns; existing explicit prefixes and mention parsing behavior is unchanged. ## AI-assisted + Testing - AI-assisted: Yes (Codex) - Testing level: targeted Slack tests passed; `pnpm build` and `pnpm check` passed; `pnpm test` currently fails in an unrelated existing suite (`src/cron/isolated-agent/run.skill-filter.test.ts`) due missing `DEFAULT_ACCOUNT_ID` in a module mock. - Prompts/session logs: available on request. - Confirmed understanding: Yes, the changed Slack parsing and upload code paths were reviewed and validated. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a bug where bare Slack user IDs (e.g., `U12345678`) passed as `--target` to media sends were incorrectly treated as channel IDs, causing Slack's `files.uploadV2` API to reject them with `invalid_arguments`. The fix introduces `inferSlackRawTargetKind()` in `src/slack/targets.ts` which uses Slack's well-known ID prefix conventions (`U/W` → user, `C/G/D/Z` → channel) to classify bare IDs before falling through to the default-kind path. This ensures the DM-open flow (`conversations.open`) is triggered for user targets before media upload. - Adds prefix-based inference for bare Slack IDs in `parseSlackTarget`, correctly routing user IDs through the DM channel open flow - Aligns `looksLikeSlackTargetId` with the inference function by adding the `Z` prefix (Slack Connect shared channels) - Adds targeted unit tests for inference and a media upload integration test validating the DM-open + upload path - Includes unrelated test mock signature fixes across CLI option-collision tests (aligning mock call signatures with typed expectations) <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it adds well-scoped prefix-based inference for bare Slack IDs with correct fallback behavior and good test coverage. - The core change is small and targeted: a single new function with two regex checks, inserted at the correct position in the parsing cascade (before the default-kind fallback). The logic is correct — Slack IDs are genuinely prefix-typed, and the regex patterns match known Slack ID formats. Existing explicit-prefix and mention flows are unaffected. The `looksLikeSlackTargetId` alignment is a one-character addition. All changes are backed by tests. The CLI test mock signature fixes are mechanical and risk-free. - No files require special attention. <sub>Last reviewed commit: e801d95</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs