Consolidated defensive layer for the invalid_blocks / msg_too_long bug
class (#56615, #62054, #53693): one malformed or oversized block fails
the ENTIRE chat.postMessage / chat.update call, so approval cards never
update and messages silently drop.
New block_kit.sanitize_blocks() is applied at every call site that
attaches blocks (send/edit via _maybe_blocks, send_exec_approval,
send_slash_confirm, and the approval/slash-confirm chat.update button
handlers). It:
- truncates section/context text to the 3000-char cap with an ellipsis
(covers interaction payloads where Slack's HTML-escaping of < > &
inflates text past the limit budgeted at send time)
- truncates header text to its 150-char cap
- drops empty blocks (no text / elements / rows)
- replaces null table column_settings entries with {} and trims
default trailing entries (Slack requires every entry be an object)
- caps the payload at Slack's 50-block maximum
- returns None when nothing valid remains so callers fall back to the
plain-text payload; never raises
Also registers contributor mappings for the salvaged PRs in this
cluster (tw0316, kamonspecial, sowork-skills).
Slack's chat.update enforces the same ~40k character limit as
chat.postMessage, but edit_message sent the formatted text unchunked —
an oversized edit failed outright with msg_too_long and the message
was never updated. Unlike send() we cannot split an edit into multiple
messages, so truncate to MAX_MESSAGE_LENGTH via the shared chunker
(which preserves code-block boundaries) and keep the first chunk.
Reapplied from PR #33224 (targeted the pre-plugin gateway/platforms/slack.py
path) onto plugins/platforms/slack/adapter.py.
Slack API returns `no_text` error when `chat.postMessage` is called
with empty or whitespace-only text. This happens when cron delivery
posts a response before the agent produces content (e.g. malformed
turn before [SILENT]).
Add early-return guards in both code paths:
- `_standalone_send()`: out-of-process cron delivery via Web API
- `send()`: in-process gateway adapter
Both paths now skip the API call and return success when the formatted
message is empty or whitespace-only.
Fixes#52663
Slack re-escapes HTML entities in the interaction payload
(< -> <, > -> >, & -> &), inflating the section text
past the 3000-char Block Kit limit when the button handler
rebuilds updated_blocks for chat.update.
The send path already budget-truncates, but the click handlers
used the echoed original_text verbatim. Cap it to 3000 chars
in both _handle_approval_action and _handle_slash_confirm_action.
Fixes#53693
assert statements are stripped when Python runs with -O flag. Replace
the assert cur is not None in _rich_text_list_block() with an explicit
if/continue guard. The assert is technically unreachable (first loop
iteration always sets cur), but using assert for invariant checking in
production code is fragile under optimization.
Slack rejects a rich_text_section / rich_text_preformatted / rich_text_quote
whose elements list is empty or contains a zero-length text element, and a
header whose plain_text is empty. The Block Kit renderer emits exactly those
shapes for common inputs: a markdown table with a blank cell or a ragged
(short) row, an empty fenced code block, a blank quote line, an empty list
item, and an emphasis-only header ("# ***"). Any one of them poisons the
whole payload — chat.postMessage fails with invalid_blocks ("missing element"
/ "must be more than 0 characters") and the message loses its rich rendering
entirely.
Route every rich_text builder's child elements through _nonempty_elements
(drop zero-length text elements; substitute a single space when nothing
remains) and skip headers that reduce to empty after markdown-marker
stripping. Observed in production via live chat.postMessage rejections;
regression tests cover each case plus a well-formed-content control.
Complementary to #56618 (column_settings hardening + no-blocks retry): that
change makes Block Kit rejections recoverable, this one makes the common
empty-content cases render correctly in the first place. No overlapping hunks.
Follow-up to the salvaged #28840 change: forward original timestamps in
_persist_branch_seed (TUI first-turn branch persist), add behavioral
round-trip tests for branch copy and compression-style replace, TUI
protocol tests asserting session.branch and _persist_branch_seed forward
timestamps, and register the contributor email mapping.
The salvaged deferred-finalize wiring (#65670) covered cli.py, the gateway
slash command, and the three in-process tui_gateway/server.py compress
sites — but the dashboard compute-host isolated session.compress /
slash.compress routes run the compress mirror inside the host child, where
a control-handler exception after compress_context() queued the boundary
notification would leak it: never fired on success paths, or worse, fired
by a LATER compress against a boundary the host had rejected.
- tui_gateway/compute_host.py: _handle_control discards any pending
context-engine compression notification (committed=False) when a
session.compress / slash.compress control frame errors. finalize is
exactly-once, so this is a no-op when the mirror already emitted or
discarded it.
- tui_gateway/server.py: _mirror_slash_side_effects normalizes the
/compact alias onto the compress branch so isolated-session compacts
actually compress and hit the same finalize/discard wiring instead of
silently no-oping.
- tests/tui_gateway/test_compute_host_phase1.py: compute-host route tests —
commit-then-notify ordering, discard on host commit failure, /compact
alias routing.
- tui_gateway commands.catalog: skip _TUI_EXTRA entries that collide with a
registry command or alias (the /compact class of bug, #57133; also removes
the pre-existing /sessions duplicate) — registry entry is canonical.
- apps/desktop: /compact now dispatches as an alias of /compress (matching
the registry's canonical alias) instead of dead-ending; /density (the
renamed TUI display toggle) is marked terminal-only so the desktop palette
doesn't advertise a command its dispatcher can't run.
- ui-tui/README.md: document /density instead of the old /compact toggle.
- tests: commands.catalog regression test asserting no duplicate advertised
names and no command shadowing another command's alias; desktop routing test.
- acp_adapter/server.py: rename compact -> compress for context compression command
- tui_gateway/server.py: rename /compact -> /density for display density toggle
- ui-tui/core.ts: rename compact -> density for display density toggle
- Internal config keys (tui_compact) and UI state (ctx.ui.compact) unchanged
Bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) carry
their actionable content — section text, button URLs — in Block Kit
blocks, while the plain text field holds only the alert title.
_fetch_thread_context and _fetch_thread_parent_text only read
msg.get('text'), so that content never reached the agent.
Add a _render_message_text helper that merges top-level text with
readable block content, section/header/context text, actionable URLs,
and (folded in from #61261 during conflict resolution) legacy
attachment fields, and use it for thread-context and parent-text
rendering.
Salvaged from #29541.
`_fetch_thread_context` and `_fetch_thread_parent_text` only read each
message's plain `text` field, so messages posted by apps (Alertmanager,
Grafana, PagerDuty, CI bots) — which carry their content in legacy
`attachments` or Block Kit `blocks` with an empty `text` — were dropped
entirely. When such a message *starts* a thread (e.g. an alert), a bot
mentioned mid-thread to investigate sees an empty thread and can only ask
"what should I investigate?".
Fall back to the existing `_extract_text_from_slack_blocks` and a new
`_extract_text_from_slack_attachments` helper when `text` is empty, so
app-posted alerts and notifications are visible in fetched thread history.
Adds TestThreadContextAppMessages (attachment-only, blocks-only, and
empty-message cases).
Closes#52387
Slack messages can carry the bot @mention only inside Block Kit `blocks`
(a `rich_text` section with a `user` element), with the flat top-level
`text` containing just a fallback string. Both mention gates read only the
flat `text`:
- the `allow_bots: mentions` bot-message filter, and
- the `is_mentioned` channel router (`routing_text = original_text`)
so such messages were silently dropped — `allow_bots: mentions` was
effectively non-functional for Block-Kit senders, and the same blind spot
hit `require_mention` / `strict_mention`.
Add `_collect_slack_block_mentions()` (walks blocks, recovers `<@UID>`
tokens from non-quoted rich-text `user` elements) and
`_slack_mention_detection_text()` (flat text + recovered mentions). Both
gates now use the merged detection text. Mentions nested in
`rich_text_quote` are deliberately ignored, preserving the existing
contract that quoted/forwarded content can't trick the bot. Also emit a
debug line when a bot message is dropped so silent drops are diagnosable.
Tests: 4 new cases in tests/gateway/test_slack_mention.py (Block-Kit
mention recovered, flat-text passthrough, no-mention, quoted-mention
ignored). 275 slack tests pass.
Main no longer catches TypeError to fall back — _supported_compression_kwargs
inspects the engine signature and calls once with only accepted kwargs.
Rework the salvaged test to assert a single correctly-shaped call while
keeping heartbeat start/stop and lock-release coverage.
Refresh the agent activity tracker while context compression is blocked in the auxiliary summarizer so gateway watchdogs do not report inactivity during long compactions.
Add regression coverage for successful heartbeats, exception cleanup, touch failures, and strict-signature compressor fallback.
(cherry picked from commit c09e58b7709cc60c5b454701f0ecf840e759222f)
The salvaged estimator ran a per-character Python loop on every
estimate_tokens_rough() call — a ~28,000,000x slowdown vs (len+3)//4 on a
1MB ASCII tool output (measured ~3.0s per call). Gate it:
- str.isascii() O(1) fast path keeps pure-ASCII text bit-identical to the
classic (len+3)//4 rule at ~1.3x baseline cost (0.23us vs 0.17us per
1MB call).
- Non-ASCII text counts dense CJK chars via a compiled character-class
regex in C (len(text) - len(re.sub(''))): ~352ms/1MB hangul vs ~2.1s
for the per-char loop.
- Non-ASCII-but-non-CJK text (accents, Cyrillic, emoji) keeps the classic
rule.
Also: parity tests against the per-char reference implementation, and
updated two stale expectations that encoded the old behavior (CJK now
counted ~1 token/char; short string content now ceil-divided instead of
floored to 0). The continuity test now detects merged-into-tail summaries
via _is_context_summary_content.
Follow-up to the salvaged #64010 (Kenmege) and #63870 (dombejar) commits,
making one resolved compression.max_attempts cap govern ALL per-turn
compression attempt sites:
- conversation_loop: resolve max_compression_attempts ONCE at turn start
(it was previously re-resolved inside the API-call loop) and route the
pre-API pressure gate through it — that gate still hardcoded
'compression_attempts < 3' and logged 'attempt=%s/3'.
- conversation_loop: the salvaged post-tool compaction gate now uses the
resolved cap instead of a hardcoded 3.
- turn_context: the preflight compaction loop was 'for _pass in range(3)';
it now sizes itself from the same resolved cap.
- agent_init: harden the max_attempts parser — reject booleans (bool
subclasses int; 'true' would coerce to 1), reject fractional floats
instead of truncating them, keep accepting integral floats and numeric
strings; anything else falls back to 3 (floor 1, ceiling 10 unchanged).
- tests: replace #63870's inspect.getsource source-shape test with
behavioral loop tests (post-tool compaction fires <= cap times per turn,
shares its budget with the pre-API gate, resets between turns); add an
e2e test proving a 4th preflight pass runs at config cap=6 while the
unset default still stops at 3; extend the #64010 config tests with the
bool/float parser semantics.
Salvages #64010 by @Kenmege and #63870 by @dombejar.
The conversation loop hardcodes max_compression_attempts = 3. Sessions
that legitimately need more rounds are stranded: on a restart history
reload, incompressible tool schemas can keep the per-request estimate
above the compressor threshold even though the message floor compresses
correctly, so three rounds cannot clear it and the turn dies with
"Context length exceeded: max compression attempts (3) reached" — the
same failure class as #62605, where the rough estimate similarly leaves
3 retries short.
Make the cap a config key, compression.max_attempts:
- default 3 = identical to today, so an unset key is behavior-neutral;
- parsed and validated in agent_init alongside the other compression.*
keys (>= 1, hard-capped at 10, non-integer values fall back to 3),
attached as agent.max_compression_attempts;
- the loop reads it via getattr(agent, "max_compression_attempts", 3),
so objects without the attribute keep the prior behavior;
- documented in the DEFAULT_CONFIG compression block.
Tests pin the parse/validate/attach seam: default preserved, custom
value honored, floor and ceiling enforced, garbage tolerated, and the
loop-side getattr degradation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The gateway /compress path can force a user-leading layout that leaves
the compaction summary after a retained system head, so scanning only
the leading block misses it. Preserve marker-carrying messages wherever
they sit, filling the remaining budget with the most recent other
messages, and cover the non-leading position with a test.
Rework on top of the salvaged #58133 commits:
- Remove the compression.persist_in_response_store config key — this is
a bug fix (stored transcripts must reflect what the agent will actually
replay), not behavior that should be opt-out-able.
- Drop the per-request load_config() imports the handler-level persist
blocks added.
- Dedupe the two handler-level persist blocks: the compressed-transcript
substitution already lives in _build_response_conversation_history
(via result["_compressed"]), so the handlers only need to propagate
the effective (possibly rotation-changed) session_id. The streaming
path does this via a new session_id_snapshot arg on
_persist_response_snapshot; the non-streaming path picks up
result["session_id"] directly.
- Rotation propagation no longer gates on history-from-store: the first
request in a chain can also rotate, and its stored session_id must be
the child session or the next previous_response_id request resumes the
pre-rotation session and re-compresses every turn.
Address review feedback (PR #58133): the original test mocked _run_agent
with _compressed=True directly, bypassing the detection logic.
New tests mock _create_agent instead, so _run_agent's detection path
runs naturally and reads agent.session_id / _last_compaction_in_place:
1. test_rotation_compression_exercises_detection_and_persists_rotated_session_id
- Fake agent with rotated session_id -> verifies _compressed is set,
compressed history is stored, and rotated session_id propagates to
both response_store and X-Hermes-Session-Id header.
2. test_inplace_compression_exercises_detection_and_persists_compressed_history
- Fake agent with _last_compaction_in_place=True, session_id unchanged
-> verifies _compressed is set, compressed history is stored, and
session_id does NOT rotate.
3. test_chained_rotation_propagates_effective_session_id
- Two-request chain: first request triggers rotation, second request
loads history using the rotated session_id stored by the first.
Asserts the compressed transcript is loaded correctly for chaining.
The persist logic only checked _result_sid != session_id (rotation),
missing in-place mode where session_id is unchanged but _compressed
flag is set. response_store history doubled every turn (11->26->55->110->225)
causing repeated re-compression.
Fix: detect compression via _did_compress or _rotated, and only update
_effective_session_id on actual rotation (not in-place).
Note: preflight loop break (turn_context.py) from original commit
eee64097a is excluded — it's an optimization, not a bug fix.
Cherry-picked from alidev eee64097a (api_server.py only)
- Detect when history is loaded from response_store (via previous_response_id)
- Add history_from_store parameter to distinguish history source
- When compression occurs, persist compressed messages instead of original
- Add persist_in_response_store config option (default True)
- Update session_id and response headers to reflect session rotation
Cherry-picked from alidev 2eb816f6b
Compression produces a compact transcript in result['messages'],
but _build_response_conversation_history detected a prefix mismatch
and concatenated the original conversation_history on front.
Detect compression via _last_compaction_in_place / session_id
rotation and signal through result['_compressed'] so the builder
uses the compressed transcript directly.
Adapter acceptance is not proof of delivery: the inner #55578 resolver can
still fail closed inside the message pipeline after the adapter accepted the
synthetic event, which falsely acknowledged the durable row as delivered and
silently discarded the delegation result.
Pre-flight the delivery target in _deliver_completion_notification before
adapter acceptance (adapted from #65838 by @henrynguyeninfo1):
- live parent or verified live compression tip -> deliver (inner resolver
still owns the actual route retarget)
- explicit-reset / unknown parent -> terminal 'dropped' disposition via new
drop_completion_delivery() (not falsely 'delivered', not eternally
'pending')
- transient uncertainty (DB error, mid-flight rotation without a visible
continuation) -> release the claim for retry
release_completion_delivery() now converges to a terminal 'dropped' state
once _MAX_DELIVERY_ATTEMPTS is exhausted, so an undeliverable completion
cannot replay on every gateway restart forever.
Two tightenings on /auth/native/authorize (a public pre-auth route):
- Per-IP pending cap (8): the broker store is capacity-bounded fail-closed
at 256 entries with a 600s TTL, so one unauthenticated spammer could fill
it and deny native sign-in gateway-wide for the pending window. Pending
entries now record the requester IP and each address is capped well above
any legitimate concurrent-login count; other addresses keep signing in.
- Loopback redirect_uri accepts IP literals only (127.0.0.1 / ::1):
'localhost' can be re-pointed via the hosts file or a hostile resolver
(RFC 8252 \u00a78.3 says to use loopback IP literals); the desktop always
sends 127.0.0.1, so nothing legitimate used the name.
3 new tests: per-IP cap enforced, cap frees on TTL expiry, localhost
redirect rejected at the route.
