Feishu: parse group/dm target prefixes as intended receive IDs#31594
Conversation
Greptile SummaryExtended Feishu target parsing to recognize
Confidence Score: 4/5
Last reviewed commit: 8b2d8a1 |
| it("treats explicit group targets as chat_id", () => { | ||
| expect(resolveReceiveIdType("group:oc_123")).toBe("chat_id"); | ||
| }); |
There was a problem hiding this comment.
add test case for dm: prefix handling (e.g., dm:ou_123 → open_id, dm:u_123 → user_id)
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/targets.test.ts
Line: 17-19
Comment:
add test case for `dm:` prefix handling (e.g., `dm:ou_123` → `open_id`, `dm:u_123` → `user_id`)
How can I resolve this? If you propose a fix, please make it concise.8b2d8a1 to
3bc36ce
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bc36cef7d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lowered.startsWith("user:") || lowered.startsWith("dm:")) { | ||
| const normalized = trimmed.replace(/^(user|dm):/i, "").trim(); | ||
| return normalized.startsWith(OPEN_ID_PREFIX) ? "open_id" : "user_id"; |
There was a problem hiding this comment.
Route dm-prefixed oc_ targets as chat_id
The new dm: branch forces non-ou_ IDs to user_id, so dm:oc_... now resolves to receive_id_type=user_id even though oc_ is a chat ID format (the outbound routing code explicitly documents dm:oc_xxx as a valid direct-chat target). In flows that now accept dm: prefixes, this causes Feishu sends to fail (or target the wrong principal) because the request uses a chat ID with the user-id receive type instead of chat_id.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3739ba3428
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lowered.startsWith("user:") || lowered.startsWith("dm:")) { | ||
| const normalized = trimmed.replace(/^(user|dm):/i, "").trim(); | ||
| return normalized.startsWith(OPEN_ID_PREFIX) ? "open_id" : "user_id"; |
There was a problem hiding this comment.
Route
dm:oc_ targets with chat_id receive type
The dm: branch currently maps every non-ou_ value to user_id, so dm:oc_... is sent as receive_id_type=user_id even though oc_ is a Feishu chat identifier. This breaks explicit direct-chat targets that use chat IDs (the send call will use an incompatible receive type and can fail). Fresh evidence: src/infra/outbound/outbound-session.ts documents dm:oc_xxx as a valid explicit DM form for Feishu chat IDs, so this path is expected to work.
Useful? React with 👍 / 👎.
3739ba3 to
0a05c16
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a05c16d10
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lowered.startsWith("user:") || lowered.startsWith("dm:")) { | ||
| const normalized = trimmed.replace(/^(user|dm):/i, "").trim(); | ||
| return normalized.startsWith(OPEN_ID_PREFIX) ? "open_id" : "user_id"; |
There was a problem hiding this comment.
Route dm:oc_ targets with chat_id receive type
resolveReceiveIdType now sends every dm: target that is not ou_... as user_id, so dm:oc_... gets receive_id_type=user_id even though oc_ is a Feishu chat ID; this makes explicit DM chat targets fail or hit the wrong principal when resolveFeishuSendTarget forwards the type to send APIs. Fresh evidence: src/infra/outbound/outbound-session.ts and src/infra/outbound/outbound.test.ts explicitly treat dm:oc_xxx as a valid direct-chat form, so this path is expected to work with chat IDs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cfd3f2da2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (lowered.startsWith("user:") || lowered.startsWith("dm:")) { | ||
| const normalized = trimmed.replace(/^(user|dm):/i, "").trim(); | ||
| return normalized.startsWith(OPEN_ID_PREFIX) ? "open_id" : "user_id"; |
There was a problem hiding this comment.
Route dm:oc_ targets with chat_id receive type
resolveReceiveIdType currently treats every dm: target that is not ou_... as user_id, so dm:oc_... is sent with receive_id_type=user_id even though oc_ is a Feishu chat ID. This breaks explicit DM chat targets in outbound flows that intentionally use dm:oc_xxx (see the explicit support in src/infra/outbound/outbound-session.ts and tests in src/infra/outbound/outbound.test.ts), because the send API receives a chat identifier under the wrong receive-id type.
Useful? React with 👍 / 👎.
Summary
Describe the problem and fix in 2–5 bullets:
group:/dm:prefixes, so targets likegroup:oc_...could be treated asuser_idand fail send APIs.sessions_send/ announce flows for Feishu group sessions can fail with invalid{user_id}errors when the target is actually a chat id.group:anddm:prefixes, and alignedlooksLikeFeishuIdwith the same forms.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
group:/dm:prefixes now resolve to the intended receive id type instead of defaulting touser_id.Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
feishu:group:oc_123andlark:dm:ou_123.Expected
chat_idand DM targets normalize to user/open id forms correctly.Actual
group:/dm:prefixes and resolves the correct receive id type.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
feishu:/lark:) + prefixed target combinations.Compatibility / Migration
Yes)No)No)Failure Recovery (if this breaks)
8b2d8a1c4.extensions/feishu/src/targets.ts,extensions/feishu/src/targets.test.ts.Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.