Skip to content

fix(gateway): preserve explicit delivery target IDs in delivery parsing - #10735

Closed
Junass1 wants to merge 1 commit into
NousResearch:mainfrom
Junass1:fix/delivery-target-parsing
Closed

fix(gateway): preserve explicit delivery target IDs in delivery parsing#10735
Junass1 wants to merge 1 commit into
NousResearch:mainfrom
Junass1:fix/delivery-target-parsing

Conversation

@Junass1

@Junass1 Junass1 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

This fixes DeliveryTarget.parse() so explicit delivery targets preserve the original ID payload instead of lowercasing and mis-splitting it.

Before this change:

  • slack:C123ABC was parsed as chat_id="c123abc"
  • matrix:!RoomABC:example.org was parsed as chat_id="!roomabc" and thread_id="example.org"
  • matrix:@Alice:example.org was parsed the same lossy way

After this change:

  • only the platform token is normalized
  • explicit target payloads are preserved verbatim
  • Matrix room/user IDs keep their full homeserver-qualified form
  • existing unambiguous thread syntax like telegram:12345:678 still works

Why

DeliveryTarget.parse() was lowercasing the entire target string and then using split(":", 2) for every platform. That works for simple numeric targets, but it breaks:

  • case-sensitive channel IDs like Slack channel IDs
  • platform IDs that legitimately contain :, especially Matrix room/user IDs

This is a small gateway bugfix with direct impact on cron delivery and explicit cross-platform routing.

Changes

  • updated gateway/delivery.py to normalize only the platform segment
  • preserved the explicit target payload as-is
  • treated Matrix !room:server and @user:server targets as full chat_id values instead of interpreting the homeserver colon as a thread separator
  • added regression coverage in tests/gateway/test_delivery.py

Tests

Passed:

  • python -m pytest tests/gateway/test_delivery.py -q
  • python -m pytest tests/gateway/ -k delivery -q

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the contribution, @Junass1!

Closing this as a duplicate of #11962 (by @alexzhu0), which targets the same fix/feature. We're consolidating on that PR for review.

If you want to help push it over the line, please jump in there — or if you think your approach is better for a specific reason that isn't covered in the other PR, let us know and we can reopen.

@teknium1 teknium1 closed this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants