Skip to content

fix: preserve agent prompt fields in button models - #2000

Merged
mwbrooks merged 1 commit into
slackapi:mainfrom
samkantor123:skantor/button-agent-prompt-fields
Oct 28, 2026
Merged

mwbrooks merged 1 commit into
slackapi:mainfrom
samkantor123:skantor/button-agent-prompt-fields

Conversation

@samkantor123

@samkantor123 samkantor123 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Buttons built with SDK models currently lose their Slackbot prompt and per-user visibility fields before being sent to Slack, even though the equivalent raw JSON preserves them.

Add explicit ButtonElement support for the documented agent_prompt, agent_prompt_display, and visible_to_user_ids fields. Nested actions, section accessories, and Home views then preserve the fields through their existing parsing paths. This also validates the documented 4,000-character prompt limit; omitted fields retain their existing behavior.

This is a targeted model update, not a change to how arbitrary unknown fields are handled.

Testing

In an activated Python virtual environment, install the repository's dependencies with ./scripts/install.sh, then run:

./scripts/run_tests.sh 'tests/slack_sdk/models tests/slack_sdk/web/test_views_publish_agent_prompt.py'

The request regression compares the complete outgoing views.publish payload against raw JSON for both actions and section buttons, using explicit button models, raw dictionaries inside block models, and raw blocks inside a View. Only the HTTP transport is mocked; model parsing and WebClient serialization run normally.

The model tests also cover each field independently, omission of None, preserving a 4,000-character prompt, and rejecting 4,001 characters. The new regression checks were run against the original implementation first: the model paths lost fields while the raw-JSON controls retained them.

No live Slack workspace, app installation, or Slackbot interaction is claimed by these tests.

Category

  • slack_sdk.models (UI component builders)
  • tests/integration_tests (Automated tests for this library)

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

Local full validation completed on Python 3.14. The PR remains a draft for review.

Risks & Revert

⚪️ Low risk: explicitly supplied fields that were previously ignored will now be sent; buttons without these fields are unchanged.
🟢 The SDK change can be reverted. Already-published views would need to be republished to remove these fields.


🤖 Generated with Codex

@samkantor123
samkantor123 marked this pull request as ready for review October 28, 2026 15:43
@samkantor123
samkantor123 requested a review from a team as a code owner October 28, 2026 15:43
@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.34%. Comparing base (449cf2f) to head (7435f41).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2000      +/-   ##
==========================================
- Coverage   84.35%   84.34%   -0.02%     
==========================================
  Files         118      118              
  Lines       13793    13800       +7     
==========================================
+ Hits        11635    11639       +4     
- Misses       2158     2161       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@mwbrooks mwbrooks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✅ Thanks @samkantor123!

📝 We'll follow up with the same fields for the Node and Java SDKs.

@@ -0,0 +1,51 @@
from copy import deepcopy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thought: The model tests in test_elements.py already cover the round trip, and this one patches the private _sync_send, which our other web tests don't do. Not blocking, so I'd merge as-is and leave it to @WilliamBergamin to keep or move it in a follow-up PR.

@mwbrooks
mwbrooks merged commit c7adb46 into slackapi:main Oct 28, 2026
19 checks passed
@mwbrooks mwbrooks added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch models Version: 3x labels Oct 28, 2026
@mwbrooks mwbrooks added this to the 3.next milestone Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented models semver:patch Version: 3x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants