Skip to content

fix(slack): clamp approval message so chat_update survives HTML-escaped >3000-char sections - #61697

Closed
Peterskaronis wants to merge 1 commit into
NousResearch:mainfrom
Peterskaronis:fix/slack-approval-3000-escape
Closed

fix(slack): clamp approval message so chat_update survives HTML-escaped >3000-char sections#61697
Peterskaronis wants to merge 1 commit into
NousResearch:mainfrom
Peterskaronis:fix/slack-approval-3000-escape

Conversation

@Peterskaronis

Copy link
Copy Markdown
Contributor

Problem

Interactive command-approval buttons (Allow Once / Allow Session / Always Allow / Deny) appear dead: clicking leaves the message visually unchanged. The approval does resolve — the agent unblocks and continues — but the message never redraws to strip the buttons and stamp the decision, so from the user's seat "nothing happens" and they re-click.

Root cause

send_exec_approval budgets the command preview against Slack's 3000-char section limit measured on the raw string. But Slack HTML-escapes < > & when it stores the message (> -> &gt;, +3 chars each). Dangerous commands — the ones full of redirects / pipes / && that trigger approval in the first place — grow once escaped.

chat.postMessage accepts the escaped text, but the chat.update fired on button click (_handle_approval_action) enforces the 3000 cap strictly and fails:

invalid_blocks: must be less than 3001 characters [json-pointer:/blocks/0/text/text]

so the redraw silently fails (only a logger.warning), leaving the buttons on screen.

Observed live: a prompt budgeted to exactly 3000 raw chars was stored by Slack at 3061 (escaping added 61 chars: &gt;x7, &lt;x8, &amp;x4). Every approval update failed while the approval itself resolved normally.

Fix

  • _handle_approval_action — clamp the echoed section text to 2900 chars before chat_update. The buttons are being stripped anyway, so truncating the echoed command is harmless, and it guarantees the decision update never fails regardless of escaping.
  • send_exec_approval — target 2900 instead of 3000 so the stored (escaped) section stays under the cap, avoiding the rarer case where a metacharacter-heavy command escapes past 3000 on the initial post and loses its buttons entirely.

Cosmetic / update-path only; no change to approval semantics.

…escaped >3000-char sections

Interactive command-approval buttons appeared dead: clicking Allow/Always
left the message visually unchanged, even though the approval itself
resolved and the agent unblocked.

Root cause: send_exec_approval budgets the command preview against the RAW
3000-char section limit, but Slack HTML-escapes `< > &` when it stores the
message. Dangerous commands (redirects/pipes/`&&`) — exactly the ones that
trigger approval — grow once escaped. chat_postMessage accepts the escaped
text, but the chat_update fired on button click enforces the 3000 cap
strictly and fails with `invalid_blocks`, so the message never redraws.

Observed live: a prompt budgeted to exactly 3000 raw chars was stored by
Slack at 3061 (escaping added 61), and the approval update failed every
time while the approval still resolved.

Fix:
- _handle_approval_action: clamp the echoed section text to 2900 chars
  before chat_update (the buttons are being stripped anyway, so truncating
  the echoed command is harmless).
- send_exec_approval: target 2900 rather than 3000 to leave headroom for
  escaping so the stored section stays under the cap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/plugins Plugin system and bundled plugins comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jul 9, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #53701 — both fix the same #53693 bug (Slack chat.update fails with invalid_blocks when HTML-escaping inflates the approval section past 3000 chars, leaving the buttons on screen) via the same mechanism: truncate the echoed section text in _handle_approval_action before chat.update, in plugins/platforms/slack/adapter.py. #53701 (2026-06-27) is the earliest-open PR for this fix and is canonical; this PR (2026-07-09) additionally tightens send_exec_approval to a 2900 budget, but the core change is the same code-site + mechanism. Both are the post-decision counterpart to the merged send-time fix #43444.

@teknium1 teknium1 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 targeting the real post-decision chat.update failure.

Problems

  • The same unbounded reconstruction remains in _handle_slash_confirm_action at plugins/platforms/slack/adapter.py:3511-3539; it sends original_text unchanged at :3522, so slash-confirm cards retain this failure mode.
  • This PR adds no regression coverage. Existing approval and slash-confirm tests in tests/gateway/test_slack_approval_buttons.py:174-348 do not provide an inflated interaction payload or assert the section length sent to chat_update.

Suggested changes

  • Apply the bounded-section logic to both update handlers, ideally through a shared helper.
  • Add one regression test per handler for an over-limit echoed section and assert the outgoing update block is capped.

The earlier canonical PR #53701 already demonstrates both sibling coverage and tests. This is an automated hermes-sweeper review.

# never fails with ``invalid_blocks`` (the buttons are stripped anyway,
# so truncating the echoed command is harmless).
_SECTION_CAP = 2900
if len(original_text) > _SECTION_CAP:

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.

Please apply the same cap to _handle_slash_confirm_action: current main still rebuilds its chat_update section from unbounded original_text at plugins/platforms/slack/adapter.py:3511-3523, so slash-confirm cards retain the same failure mode.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 11, 2026
@teknium1 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #69317 (merged): the escape-inflation clamp landed via #53701 (earlier) plus the new sanitize_blocks() boundary; the 2900-char send budget became unnecessary once the update boundary clamps.

Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69317's summary.

@teknium1 teknium1 closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants