Skip to content

fix(slack): sanitize generated session IDs#6437

Open
anneheartrecord wants to merge 3 commits into
google:mainfrom
anneheartrecord:fix/slackrunner-session-id
Open

fix(slack): sanitize generated session IDs#6437
anneheartrecord wants to merge 3 commits into
google:mainfrom
anneheartrecord:fix/slackrunner-session-id

Conversation

@anneheartrecord

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:
SlackRunner builds ADK session IDs from Slack channel/thread identifiers. Slack timestamps include ., for example C0BARCEJJ1K-1784622590.251599, but managed session services validate session IDs against ^[A-Za-z0-9_-]+$.

Solution:
Sanitize Slack-derived session ID parts before passing them to Runner.run_async, replacing characters outside the managed-session-safe set with _. Slack API calls still use the original Slack thread_ts, so Slack threading behavior is unchanged.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Passed locally:

uv run --with pytest --with pytest-asyncio --with slack-bolt --with aiohttp pytest tests/unittests/integrations/slack/test_slack_runner.py -q
# 4 passed, 4 warnings

Also ran:

uv run --with ruff ruff check src/google/adk/integrations/slack/slack_runner.py tests/unittests/integrations/slack/test_slack_runner.py
uv run --with isort isort --check-only src/google/adk/integrations/slack/slack_runner.py tests/unittests/integrations/slack/test_slack_runner.py
uv run --with pyink pyink --check src/google/adk/integrations/slack/slack_runner.py tests/unittests/integrations/slack/test_slack_runner.py

Manual End-to-End (E2E) Tests:

Not run. The unit test covers the reported Slack thread timestamp shape and verifies that Runner.run_async receives a managed-session-safe ID while Slack replies still use the original thread_ts.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

The focused regression uses thread_ts="1234567890.123456" and expects session_id="C67890-1234567890_123456".

@rohityan rohityan added integrations needs review [Status] The PR/issue is awaiting review from the maintainer and removed web [Component] This issue will be transferred to adk-web labels Jul 22, 2026
@rohityan
rohityan requested a review from sasha-gitg July 22, 2026 18:08
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @sasha-gitg , can you please review this. LGTM

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrations needs review [Status] The PR/issue is awaiting review from the maintainer services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SlackRunner error when agent uses managed session service. ValueError: Invalid session_id 'C0BARCEJJ1K-1784622590.251599': must match ^[A-Za-z0-9_-]+$.

4 participants