Skip to content

Remove supportsThreading guard from app.reply()#533

Merged
corinagum merged 1 commit into
mainfrom
cg/remove-threading-guard
Apr 20, 2026
Merged

Remove supportsThreading guard from app.reply()#533
corinagum merged 1 commit into
mainfrom
cg/remove-threading-guard

Conversation

@corinagum

Copy link
Copy Markdown
Collaborator

Summary

  • Remove supportsThreading() guard from app.reply() 3-arg form -- always calls toThreadedConversationId() when messageId is provided
  • Remove supportsThreading() function and THREADING_SUFFIXES constant (dead code)
  • Remove supportsThreading tests
  • Update test manual example to remove conversation type guard
  • Threading support is now determined by the service, not the SDK

Why: Threading support is expanding (channels, 1:1, group chats). Maintaining an allowlist of conversation ID suffixes creates a maintenance burden and can block developers from using newly supported scopes until the SDK is updated.

Test plan

  • Unit tests pass
  • Build succeeds

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings April 20, 2026 20:29

Copilot AI 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.

Pull request overview

This PR removes the SDK-side threading allowlist and makes App.reply(conversationId, messageId, activity) always construct a threaded conversation ID via toThreadedConversationId(), delegating “threading supported?” decisions to the service.

Changes:

  • Remove supportsThreading() / THREADING_SUFFIXES and associated unit tests.
  • Update App.reply() (3-arg overload) to always use toThreadedConversationId() when messageId is provided.
  • Update the threading example and README to remove the conversation-type guard and adjust notes.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/apps/src/utils/thread.ts Removes suffix allowlist and supportsThreading() helper.
packages/apps/src/utils/thread.spec.ts Drops supportsThreading() tests; keeps toThreadedConversationId() coverage.
packages/apps/src/app.ts Removes supportsThreading() guard; always threads in 3-arg reply(), updates JSDoc.
packages/apps/src/app.spec.ts Updates expectation to always append ;messageid= in 3-arg reply() case.
examples/threading/src/index.ts Removes manual guard, but leaves an outdated “channels only” comment.
examples/threading/README.md Updates command description/notes to reflect service-side behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/apps/src/app.ts
Comment thread examples/threading/src/index.ts
Comment thread examples/threading/README.md
Comment thread packages/apps/src/app.ts
@corinagum
corinagum merged commit 8dd29d3 into main Apr 20, 2026
12 checks passed
@corinagum
corinagum deleted the cg/remove-threading-guard branch April 20, 2026 21:54
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