Skip to content

slack: chat.postMessage answers a reply with the parent_user_id of its thread's root - #1168

Merged
nuriyoo merged 4 commits into
brekkylab:mainfrom
jnohclee-rgb:codex/slack-reply-parent-1162
Nov 1, 2026
Merged

nuriyoo merged 4 commits into
brekkylab:mainfrom
jnohclee-rgb:codex/slack-reply-parent-1162

Conversation

@jnohclee-rgb

@jnohclee-rgb jnohclee-rgb commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1162. A reply posted with chat.postMessage comes back with the parent_user_id that conversations.replies lists it with, which _parent_uid gives both answers.

What changed

chat.postMessage. When thread_ts names a message the caller can see, the handler looks up the thread's root by the ts the reply is threaded under, and the echoed message carries parent_user_id from _parent_uid. A top-level post, and a thread_ts that names nothing, carry none.

_parent_uid, beside _uid in backlot/routers/slack.py, returns the user the root is served with: the poster's id, or USLACKBOT for a deleted root's placeholder. conversations.replies reads it too.

Tests. test_posting_with_thread_ts_replies_in_that_thread holds a table of ten rows. Each changes one thing from the first: who posted the root, what thread_ts names, a reply_broadcast value that keeps the reply in the thread, or a deleted root (two rows). A row compares the echo's parent_user_id and the ids conversations.replies lists against a literal id.

test_replying_to_a_reply_lands_in_the_same_thread checks the echo against the root's user. test_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refused checks that a top-level post carries no parent_user_id.

Not in this PR. Real carries the root poster's parent_user_id on a reply in three more answers (the chat.update echo, reactions.get and a reactions.list item) and Backlot in none of the three, which is #1178.

Why this is what the real API does

The measurements are in _parent_uid's docstring. The 2026-10-30 one, recorded on the issue, covers 32 replies over a bot token and a user token, 19 to a root and 13 to a reply under it, with reply_broadcast absent, empty, false, FALSE or 0.

The 2026-10-31 one ran from 16:15 to 16:16 UTC in a private channel made for it. It deleted a root with one reply under it, then posted three replies: a user reply to the placeholder, a user reply to the earlier reply, and a bot reply to the placeholder.

Replay. The 2026-10-31 steps sent to Backlot in-process on the bundled corpus, comparing user, parent_user_id and subtype on 22 answers: main matches real on 7 and this head on 18. The four left are the answers under Not in this PR.

Verification

  • Tests — pytest tests/test_slack.py on a uv sync --locked --all-extras install, Python 3.14.4: 581 passed, none skipped. CI runs the whole suite on Python 3.11 to 3.15.
  • Optional surfaces that ran rather than skipped — none beyond tests/test_slack.py locally; the Slack SDK case in tests/test_sdk.py runs in CI.
  • Lint — ruff check . && ruff format --check ., ruff check --preview --select PLW1514 . and python scripts/prose_check.py: each exits 0.

At 285d7985, on main at a2b08a55.

pytest tests/test_slack.py                                  581 passed
the two tests on the first checkbox, on main's backlot/     2 failed
mutations of the lines this branch adds, plus 3 by hand     8, 7 caught

The mutation left turns if root is not None into if True, which differs only for a caller who can see a reply but not its root. A thread's messages share one channel, so the corpus has no caller like that.

  • A test fails without this change — test_posting_with_thread_ts_replies_in_that_thread, test_replying_to_a_reply_lands_in_the_same_thread.
  • A new endpoint serving corpus content is ACL-scoped, proved for both an admin and a scoped token — no new endpoint or visibility rule; the root lookup reads through the caller's visible ids.
  • Comments state what was measured, not the history of the fix

Prepared with AI assistance.

Dharma — leave the code kinder than we found it.

@nuriyoo nuriyoo added the fidelity The vendor API defines the right answer; closing this needs a measurement against it label Oct 31, 2026

@nuriyoo nuriyoo left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, and for keeping the change to the one lookup it needs: for a thread whose root is still there, this head answers what real answers, from either token and whichever message thread_ts names.

I measured one more case on slack.com on 2026-10-31, 16:15 to 16:16 UTC, in a private channel made for it, with a user token and a bot token; every message the run posted was deleted afterwards and the channel archived. Then I sent the same steps to Backlot in-process on the bundled corpus (ava@acme.com's token for the user token, the admin token for the bot token) and compared user, parent_user_id and subtype on 22 answers: main matches real on 7, this head on 11 (the same 11 merged with main), and the head with the suggestions on 18.

What has to change, each in an inline comment:

  1. A reply under a deleted root names USLACKBOT. With the root deleted and a reply still under it, the three replies I posted to that thread afterwards, and each of the four conversations.replies listed, came back with parent_user_id: USLACKBOT. This head answers the root poster's id. The suggestion on chat_post_message takes the id from a helper beside _uid, which conversations_replies shares; both are outside the diff, so they are in the diff below.
  2. main has moved under this branch. The return line and one test now conflict with the reply_broadcast change; the comment on the return line says how the two merge.
  3. The test table gets two deleted-root rows, and stops crossing the reply_broadcast values with the other two columns.

Outside the diff, in backlot/routers/slack.py:

@@ -1216,6 +1216,19 @@ def _uid(email: str) -> str:
     return SERVICE_USER_ID if email == SERVICE_EMAIL else synth.slack_user_id(email)
 
 
+def _parent_uid(root) -> str:
+    """The `parent_user_id` a reply in ``root``'s thread carries: the `user` the root is served
+    with, which is ``USLACKBOT`` once the root is deleted with a reply still under it.
+
+    Measured against slack.com on 2026-10-30 and 2026-10-31, over a user and a bot token: the
+    `chat.postMessage` answer to a reply carries the root's poster whether `thread_ts` names the
+    root or a reply under it, and `conversations.replies` lists each reply with the same id. With
+    the root deleted, a reply posted to the placeholder or to a reply under it, and each reply
+    `conversations.replies` listed, carried ``USLACKBOT`` instead.
+    """
+    return SLACKBOT_ID if root["subtype"] == "tombstone" else _uid(root["author_email"])
+
+
 def _handle(email: str) -> str:
     """A corpus email as Slack's handle — the `name` a user object carries and the `user`
     `auth.test` reports for the same person."""
@@ -1637,7 +1650,7 @@ async def conversations_replies(request: Request):
     latest = store.slack_latest_reply_ts(conn, name, root["ts"], ids) if rc else None
     ru = store.slack_reply_authors(conn, name, root["ts"], ids)
     ruids = [_uid(e) for e in ru[:5]]
-    parent_uid = _uid(root["author_email"])
+    parent_uid = _parent_uid(root)
     messages = [
         _message(
             x,

Not this PR's: real also carries parent_user_id on a reply in the chat.update echo (user and bot token), in reactions.get and in a reactions.list item, the root poster's id each time, and Backlot carries none of them, on main too. Those are the four answers still apart with the suggestions. Filed as #1178.

On the description and title, once the above is in:

  • The title's "reply echoes" reads wider than the change, since the chat.update echo of a reply is one and still has no parent_user_id. Something like slack: chat.postMessage answers a reply with the parent_user_id of its thread's root names what Backlot does once this is merged.
  • The paragraph on the other Slack PR describes a conflict this branch now has to resolve itself, so it can go once main is merged in.
  • "Why this is what the real API does" can add the 2026-10-31 measurement for the deleted root, which the helper's docstring holds.
  • The Tests line asks for the install the run used (pytest on a .[…] install), and a run on the head after merging main. The sentence after the checkboxes about 20 combinations changes with the table.

With every suggestion and the diff above applied, on a .[dev] install (Python 3.14.4): tests/test_slack.py passes (577 passed), and 581 passed once merged with main at 68cf7f24 as the return-line comment describes. ruff format --check ., ruff check ., ruff check --preview --select PLW1514 . and scripts/prose_check.py exit 0. The new table fails on this head's code at the first deleted-root row, and on main's code at the first row. I also changed each decision on the lines the suggestions add, one at a time, plus two by hand (the root looked up by the named parent's ts, and conversations_replies hashing the stored author): the tests catch 6 of 7. The one left turns if root is not None into if True, which differs only when the caller can see the reply but not its root. The same run over this head's lines catches 4 of 7: with the elif condition set to False the else finds the same row, and the other two are the guards on a missing root, the same case.

Comment thread backlot/routers/slack.py Resolved
Comment thread backlot/routers/slack.py Resolved
Comment thread tests/test_slack.py Resolved
…ser_id

`_parent_uid` beside `_uid` answers the `user` a thread's root is served
with, which is USLACKBOT once the root is deleted with a reply still under
it, and both `chat.postMessage` and `conversations.replies` read it.
`chat.postMessage` looks the root up once, by `thread_ts`. The reply test
table holds one axis per row plus two deleted-root rows.

The return line of `chat_post_message` and one assertion in
`test_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refused`
stay as the merge base has them, so `main` merges in without a conflict;
the commit after that merge writes both over `main`'s versions.
The return line wraps the message in `_with_root` as `main` does and builds
it with the root's `parent_user_id`; the top-level loop of
`test_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refused`
checks that a post outside a thread carries none.
@nuriyoo nuriyoo changed the title slack: reply echoes name the thread's root author slack: chat.postMessage answers a reply with the parent_user_id of its thread's root Nov 1, 2026

@nuriyoo nuriyoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, @jnohclee-rgb. With this in, a reply's chat.postMessage answer names its thread's root the way conversations.replies does, which is what the issue measured.

I pushed the suggestions from the review as maintainer commits so it could go in now. _parent_uid gives both answers the same id, USLACKBOT under a deleted root, and the test table changes one thing per row, with the two deleted-root rows at the end. main's reply_broadcast change is merged in, so the return line wraps the message in _with_root. The branch has three commits from that: the change with the return line kept as the merge base had it, GitHub's merge of main, and the return line written over main's. The description is rewritten to match, and the title names the chat.postMessage answer, since the other reply surfaces are filed as #1178.

On 285d7985, tests/test_slack.py passes (581) on an all-extras install and CI passes on 3.11 to 3.15.

@nuriyoo
nuriyoo merged commit f01b29b into brekkylab:main Nov 1, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fidelity The vendor API defines the right answer; closing this needs a measurement against it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

slack: chat.postMessage answers a reply with no parent_user_id, which real carries in that answer as in conversations.replies

2 participants