Skip to content

docs: clarify slack_read_thread uses message_ts not thread_ts - #101

Draft
WilliamBergamin wants to merge 2 commits into
mainfrom
fix-search-problem
Draft

docs: clarify slack_read_thread uses message_ts not thread_ts#101
WilliamBergamin wants to merge 2 commits into
mainfrom
fix-search-problem

Conversation

@WilliamBergamin

@WilliamBergamin WilliamBergamin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

This pull request documents that slack_read_thread takes the thread timestamp under message_ts, not thread_ts, to eliminate a recurring failed-call-and-retry cycle.

  • Closes slack_read_thread parameter named message_ts instead of thread_ts #57.
  • The search tools return permalinks containing ?thread_ts=<ts>, so LLM agents pattern-match thread_ts and call slack_read_thread with the wrong parameter name; the server rejects it and the agent retries with message_ts, costing ~750ms plus tokens each time.
  • The server-side fix (accepting thread_ts as an alias) lives in the closed-source MCP server and is tracked internally; this PR addresses the experience from the open-source side.
  • Updates the slack-search skill in two spots: the Following Up on Results bullet and a new Common Pitfalls entry.
  • Adds a short message_ts-not-thread_ts reminder to the find-discussions, standup, and summarize-channel commands, which call slack_read_thread directly.
  • Includes a slack patch changeset.

Preview

N/A — documentation-only change (skill and command prompt text).

Testing

  • Read the updated skills/slack-search/SKILL.md and the three command prompts and confirm each names message_ts as the correct parameter and ties it to the ?thread_ts= permalink value.
  • Optional end-to-end sanity check: run slack_search_public_and_private, take a result permalink's thread_ts value, and confirm slack_read_thread { channel_id, message_ts } succeeds on the first call.

Notes

Intentionally does not add an eval scenario: tests/eval/test_tool_selection.py only validates which tool the model selects, not parameter values, so it cannot exercise this fix.

Verified locally with make lint and make test-unit (both pass). The eval layer of make test was not run locally as it needs GEMINI_API_KEY/SLACK_MCP_TOKEN; CI runs it.

Requirements

Search permalinks surface the thread timestamp as ?thread_ts=<ts>, but
slack_read_thread expects it under message_ts. LLM agents pattern-match
thread_ts from search output, the call is rejected, and they retry —
costing latency and tokens each time (issue #57).

Document the correct parameter name in the slack-search skill (Following
Up on Results + Common Pitfalls) and in the find-discussions, standup,
and summarize-channel commands that call slack_read_thread directly.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4f968b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slack Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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.

slack_read_thread parameter named message_ts instead of thread_ts

1 participant