Widening pass over the whole adapter following the cluster-C16 audit
(#51019, #51097, #23676, #23375): every in-memory structure that
accumulates per-message, per-user, or per-thread state is now bounded,
and every eviction is oldest-first — never arbitrary set-iteration
order, which is the #51019 failure mode (bot silently going quiet on
the most ACTIVE thread because set.pop-order eviction removed it).
Newly bounded:
- _approval_resolved / _clarify_resolved (caps 1000): unclicked
approval/clarify prompts leaked their double-click-guard entries
forever; oldest-insertion eviction via _trim_oldest_dict_entries.
- _reacting_message_ids (cap 5000): reaction lifecycle entries leaked
when an exception fired between add and finalize; oldest-ts eviction.
- _active_status_threads (cap 1000): statuses abandoned by error paths
accumulated; oldest-thread-ts eviction so the newest live status is
never cleared.
- _channel_team (cap 10000): grew with every DM channel the bot ever
saw (DM channel IDs are per-user). All four write sites now route
through _remember_channel_team; eviction is safe because entries are
re-learned from the next event and _get_client falls back to the
primary client.
- _slash_command_contexts (cap 1000): TTL cleanup only ran on lookup,
so contexts whose ephemeral replies never happened accumulated;
overflow purges expired entries first, then oldest-stash-first.
Converted from arbitrary set-order eviction to oldest-first:
- _titled_assistant_threads: keys are (team, channel, thread_ts) —
now evicts oldest thread first via _discard_oldest_by_thread_ts.
- _thread_rehydration_checked: keys are team:channel:thread_ts[:user] —
arbitrary eviction here would re-run an ACTIVE thread's restart
rehydration check and re-inject the missed-delta context; now evicts
oldest thread first.
- _reacting_message_ids uses #51097's _discard_oldest_slack_timestamps.
Deliberately NOT bounded (naturally tiny, per-workspace):
_team_clients, _team_bot_user_ids, _team_bot_names (one entry per
installed workspace), _assistant_threads / _agent_view_contexts /
_bot_message_ts / _mentioned_threads / _user_name_cache /
_thread_context_cache (already bounded), _dedup (MessageDeduplicator
has max_size + TTL internally).
New helpers: _trim_oldest_dict_entries (dicts preserve insertion
order, so oldest-first is exact) and _discard_oldest_by_thread_ts
(chronological sort on the embedded Slack ts for keyed sets).
Tests: caps hold under churn, eviction removes OLDEST not arbitrary
entries, newest/active entries survive eviction pressure (regression
shape for #51019), plus end-to-end paths through _resolve_user_name
and _handle_slash_command.
Part of the C16 cache-bounds consolidation with #51097 (markoub),
#23676 and #23375 (EloquentBrush). Fixes#51019.
_fetch_thread_context() caches per-thread Slack history under a
60-second TTL, but expired entries are never removed. On the current
adapter this is worse than when first reported: each entry now also
retains the raw conversations.replies payloads (messages) for
watermark re-formatting, so a busy multi-channel workspace accumulates
full message lists forever. _bot_message_ts, _mentioned_threads, and
_assistant_threads all enforce a MAX constant with active eviction in
the same __init__; _thread_context_cache had the TTL declared but no
matching eviction path.
Fix: add _THREAD_CACHE_MAX = 2500 and purge expired entries
(fetched_at older than _THREAD_CACHE_TTL) whenever a new write pushes
the cache past the limit. TTL-based eviction is used instead of LRU
because fresh entries are still needed; evicting them would
immediately re-trigger a rate-limited conversations.replies call.
Adds two unit tests: one verifying stale entries are purged on
overflow, one verifying fresh entries survive.
Reapplied from #23375 onto the current adapter (moved to
plugins/platforms/slack/adapter.py; cache entries now carry raw
message payloads and parent_user_id).
Companion to gateway-gateway#160 (connector absent-row default flips to
mention-gated / requireAddress=true).
relay_relevance_policy() previously returned None whenever the projected
policy was all-falsy, on the premise that 'the connector's quiet default
already matches'. Once the connector defaults requireAddress to true,
that premise inverts for one case: an agent EXPLICITLY configured with
require_mention: false would stay silent and get mention-gated anyway,
with no way out.
The nothing-to-declare check now keys on 'require_mention is unset'
rather than 'require_mention is falsy': an explicit false IS a
configured knob and is declared to the connector; a genuinely
unconfigured platform still declares nothing and inherits the
connector's default. No wire/contract change.
Cron jobs targeting a Slack user (deliver=slack:U…) failed with
channel_not_found: chat.postMessage and files_upload_v2 require a
conversation ID, and a DM must be opened first via conversations.open
to obtain a D… ID. The tool-level resolution in send_message() does not
cover cron's direct _send_to_platform → standalone_sender_fn path.
Add _resolve_slack_user_dm to the Slack plugin: resolves U…/W… targets
via conversations.open (proxy-aware, cached per token+user so repeated
cron fires don't re-open the DM), wired into _standalone_send ahead of
both the text and media delivery branches so every standalone entry
point benefits.
Adapted from #17444 by @Esther-Zhu023 — the original patched the legacy
_send_slack helper in tools/send_message_tool.py, which has since moved
into the plugin as _standalone_send (#41112).
Closes#17444
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The in_channel flat-delivery clear was gated on `runtime_adapter is not
None`, but the flat continuation session is seeded ONLY on the live-send
path, which also requires a running event loop. When an adapter is
present but the loop is absent/not-running, the live-send/seed block is
skipped and delivery falls through to the standalone path — clearing
thread_id there flattened an unseeded brief with no continuable session
behind it (and bypassed the D6 capability check).
Factor the live-send condition into `live_adapter_ready` and gate both
the thread_id clear and the live-send block on it, so the clear can no
longer drift from the seed. Add an adapter-present/no-running-loop
regression test (verified it fails against the un-scoped clear).
Addresses review r3609147550.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cron_continuable_surface=in_channel is meant to deliver a cron brief FLAT
into a channel (thread_id=None) so a plain channel reply continues the job
via the shared-channel session. The scheduler already seeds that flat
session (_seed_cron_channel_session) but never cleared the ORIGINAL
target/origin thread_id before routing the actual delivery, so a job
scheduled from inside a Slack thread still delivered into that origin
thread — the seeded continuable session then never matched where the brief
actually landed.
Clear thread_id once in_channel_surface is finalized, before it is re-read
for route_thread_id / the standalone fallback. Scope the clear to EXACTLY
the target that gets flat-seeded — the origin-continuable target
(mirror_this_target) on a live, in_channel-capable adapter
(runtime_adapter is not None):
- fan-out / broadcast / explicit-thread targets keep their thread_id — they
are not continuable and are never seeded, so flattening them would
reroute an explicit thread and could collapse two targets into duplicate
flat sends;
- the standalone no-live-adapter path keeps the origin thread — it can
never seed the flat session, and without an adapter the D6 capability
fail-safe can't run, so flattening there would both drop the brief out of
any continuable lane and bypass D6.
This keeps the clear in lockstep with the seed condition
(in_channel_surface and mirror_this_target) and the D6 fail-safe.
mirror_this_target / origin_user_id are computed earlier using the original
thread_id to match the origin conversation.
Regression tests in TestCronContinuableSurfaceInChannel:
- a job whose origin carries a thread_id must not forward that thread_id to
the live adapter (DeliveryRouter folds target.thread_id into send
metadata), and the seeded continuable session stays flat (thread_id=None);
- with no live adapter, the standalone send falls back to the origin thread
rather than flattening.
Per review (Victor): response_url failure does not mean ephemeral delivery
is impossible — chat.postEphemeral is an independent API path that keeps
the reply private. Public-channel fallback removed entirely; when both
ephemeral paths fail the reply is dropped with a logged error rather than
leaked to the channel. No config knob needed.
- test_thread_command_skips_context_prefix: post-#69320 thread context IS
fetched on first thread entry but rides channel_context; assert the
command token stays at char zero and the backfill is preserved, instead
of asserting the fetch never happens.
- test_slack_send_retry.py: main's _get_client() now takes team_id;
update the lambda stubs.
Two silent-loss modes in the ephemeral slash reply path:
1. Delivery failure was swallowed: _send_slash_ephemeral returned
success=True on any POST failure, so the user's actual command reply
vanished behind the stale 'Running /cmd…' ack. It now returns
success=False and send() falls back to normal channel delivery.
2. Long replies were truncated to the first ~39k chunk with no notice.
Replies are now chunked across response_url POSTs (first replaces
the ack, follow-ups append, all ephemeral), capped at Slack's 5-POST
response_url budget with an explicit truncation notice when exceeded.
Also updates the #55357 bounded-error-read test for the #26788 precise
context matching (ContextVar must be set) and channel fallback.
Fixes#19688
Commands typed in Slack could be mangled by every enrichment layer the
adapter applies to normal messages:
- Block Kit / unfurl / attachment-notice / text-file injection could
prepend or append content around a command, moving the command token
away from character zero or polluting its arguments. Commands are now
restored from canonical authored input after all enrichment
(final is_command_text guard before MessageEvent construction).
- @bot /cmd (typed slash behind a mention) was never classified as a
command; the mention-strip branch now re-probes for both slash and
bang forms.
- The Slack Agent-view context label ([Slack app context: ...]) was
prepended to command events too; now command-exempt.
- Native slash payload arguments were strip()ed, destroying meaningful
spacing inside/after arguments; only the command delimiter is
nonsemantic now.
- Slash payload thread identity (thread_ts/message_ts, top-level or
nested in message/container) is preserved onto SessionSource so
session-scoped commands hit the same thread session.
- /queue and /steer queued fallbacks now propagate channel_context so a
command that triggered first-entry thread backfill doesn't lose the
history when re-queued.
Adapted from PR #66310 to the post-#69320 channel_context design (thread
history already rides MessageEvent.channel_context, never text).
Slack's send() caught all exceptions and returned a bare
SendResult(success=False) — never setting retryable=True or extracting
the server's Retry-After header. When Slack returned a 429 rate-limit
error, the base _send_with_retry() layer saw retryable=False and did
not retry, silently dropping remaining message chunks.
Reuse the existing _is_retryable_upload_error() helper (which already
detects 429, 500+, and connection-type errors) to set retryable=True,
and extract the Retry-After header from the SlackApiError response
when present so the base retry layer honors Slack's backoff schedule
instead of its own default.
Sibling of the Telegram FloodWait fix (PR #46762 / commit 404b06ac4)
which added the SendResult.retry_after plumbing to the base layer.
Adds five regression tests covering 429 with/without Retry-After,
500 server errors, 403 non-retryable errors, and connection errors.
_pop_slash_context fell back to a channel-only scan when the
_slash_user_id ContextVar was unset (i.e. send() invoked from a
non-slash code path such as a cron delivery or a normal channel reply).
That scan could steal another user's pending slash reply context: the
normal message got swallowed into an ephemeral response_url POST that
replaces the invoker's ack, and the slash invoker's actual reply then
posted publicly. Remove the fallback — when the ContextVar is unset,
match nothing.
Surgical reapply of PR #26788 (originally against gateway/platforms/slack.py).
_has_active_session_for_thread() hardcoded chat_type='group', causing
session key mismatch for DM and MPIM threads. DM sessions key as
agent:main:slack:dm:{chat_id}:{thread_ts} but the lookup built
agent:main:slack:group:{chat_id}:{user_id}:{thread_ts}.
Impact: _has_active_session_for_thread always returned False for DM
threads, causing thread context to be prepended on every message. The
prepended context broke slash command detection (get_command() checks
text.startswith('/')), so /cmd and !cmd never worked in DM threads.
Fix: accept event-derived chat_type parameter instead of hardcoding
'group'. Both call sites pass chat_type='dm' if is_dm else 'group',
where is_dm is already computed from channel_type in {'im', 'mpim'}.
This correctly handles:
- IM channels (D-prefix): chat_type='dm'
- MPIM channels (G-prefix): chat_type='dm' (was missed by D-prefix heuristic)
- Channel messages (C-prefix): chat_type='group' (unchanged)
Added regression tests covering DM thread lookup, MPIM thread lookup,
and negative cases verifying the old hardcoded 'group' behavior fails.
Slack rich_text blocks mirror the original message text. When bang
commands are rewritten from !model to /model, appending block text makes
the command arguments include a duplicate payload, so the model switcher
sees spaces in the model name and rejects valid commands like:
!model qwen3.7-plus --provider opencode-go
Skip block extraction for command messages while preserving it for
normal messages. Also preserve Slack thread_ts (top-level or nested in
message/container payload shapes) on native slash-command payloads so
session-scoped commands like /model apply to the intended thread instead
of a channel+user key the next threaded message never matches.
Surgical reapply of PR #43533 (originally against gateway/platforms/slack.py,
now plugins/platforms/slack/adapter.py). Thread-shape widening credit also
to #66310.
Follow-up to the #43755 salvage:
- Update the strict _Pool doubles in tests/run_agent/test_run_agent.py to
accept api_key_hint and assert it carries the agent's failed key.
- Add a real-CredentialPool regression (no mocks) proving the hint routes
exhaustion to the entry whose key actually failed, not pool.current(),
plus the no-hint baseline (#43747 wrong-entry marking).
recover_with_credential_pool() called mark_exhausted_and_rotate() without
api_key_hint, causing it to fall back to current() or _select_unlocked().
When a prior rotation left current() as None, _select_unlocked() returned
the NEXT (healthy) entry instead of the one that actually failed — marking
the wrong credential as exhausted (#43747).
Extract the current API key from agent.api_key (or pool.current().runtime_api_key
as fallback) and pass it as api_key_hint to all 4 call sites.
* fix(ci): publish inline E2E evidence
Upload bounded screenshot evidence from E2E, then publish validated images
from a trusted workflow_run job to commit-pinned branches in the evidence repo.
Wait briefly for the live CI review comment marker before publishing, so
GitHub's read-after-write delay cannot leave an orphaned evidence branch.
* fix(ci): isolate privileged credentials from PR jobs
Keep App private keys and Docker Hub credentials out of PR-controlled
workflows. Use protected environments for trusted publishing and a public
repository variable for the App client ID.
* fix(ci): attach E2E evidence with restricted bot session
Replace the App-backed evidence repository publisher with gh-image uploads
from a dedicated bot session in the gh-image environment.
* fix(ci): publish validated E2E evidence from forks
Let the trusted default-branch publisher handle bounded, validated evidence
artifacts from fork PR CI without checking out or executing fork code.
Update the gateway's live turn projection after an accepted correction so a warm session resume does not add the stale original prompt beside the persisted correction.
Cover the inference-time correction path end to end and assert both redirect entry points refresh the live user text.
The base class spawns _keep_typing for every adapter — a 2s refresh loop
that runs for the whole turn, including while the stream consumer sends
the response — but RelayAdapter inherited BasePlatformAdapter's no-op
send_typing. The loop ran every turn and emitted nothing, so relay-fronted
chats (hosted Discord/Telegram/Signal/Slack) never showed 'is typing…'.
The rest of the pipe already existed on both sides: OutboundOp "typing"
is in the wire contract (contract_version 1), and every connector-side
sender implements it (Discord POST /channels/{id}/typing, Telegram
sendChatAction, Signal sendTyping, Slack assistant status).
This bridges the tick onto the existing outbound frame, mirroring send():
- _with_scope re-attaches the tenant discriminator (metadata.scope_id, or
user_id for DMs) — the connector's routedEgressGuard wraps ALL ops, so
an undiscriminated typing frame is declined like a bare send would be.
- the Phase 1.5 per-frame platform tag routes typing through the platform
the chat lives on for multi-platform gateways.
Best-effort by design: transport errors are swallowed (typing is cosmetic;
the next tick retries), no transport is a silent no-op, and stop_typing
stays the base no-op (platform indicators self-expire). Additive within
contract_version 1 — an older connector returns an unsupported-op result
we ignore. Contract doc §4 updated (typing now carries metadata?).
- Iterate ROUTINE_COMPRESSION_STATUS_SAMPLES (formatted from the source
constants the emit sites use) through _prepare_gateway_status_message on
every chat platform — emission-wording drift now fails the suite without
re-copying literals.
- Extend the pinned NOISY_STATUS_MESSAGES with the buffered retry chatter
and post-#69332 wordings.
- New VISIBLE_COMPRESSION_MESSAGES negative suite: manual /compress
headlines and abort/failure notices must never be swallowed by the
widened regex.
Surface audit for #64438 consistency: cli.py and acp_adapter had their
gates removed by the salvaged #63630 commit (each with a behavioral
test). The remaining manual-compress surfaces never gated on
compression.enabled — pin that contract so a gate can't regress in:
- gateway/slash_commands.py _handle_compress_command (new behavioral
test: compression_enabled=False still compresses, force=True).
- tui_gateway/server.py — all three manual routes (session.compress
RPC, command.dispatch compress branch, slash.exec mirror) plus the
compute-host slash.compress/session.compress controls converge on
_compress_session_history; new test pins the helper ignores
agent.compression_enabled and passes force=True.
Main renamed the ACP slash command from /compact to /compress after #63630
was written; update the salvaged status line, comment, and behavioral test
to dispatch the command that actually exists.
compression.enabled: false is documented (agent/conversation_loop.py
overflow path) as disabling *automatic* compaction only — the terminal
context-overflow error explicitly tells users to run /compress manually,
and the gateway handler has never gated on the flag. But the classic CLI
(_manual_compress) and the ACP adapter (/compact) refused with
'Compression is disabled', leaving users at a full context with no
manual escape hatch on those surfaces.
Remove the stale gates (they predate the overflow-path design; the CLI
gate came from the original /compress commit's boilerplate) and unify
force=True across all manual-compaction call sites: ACP /compact and the
TUI's _compress_session_history (manual-only helper) now bypass the
summary-failure cooldown exactly like the CLI and gateway already did.
Also reword the ACP /context status line so a disabled-compression agent
no longer implies /compact is unavailable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Integration test for the two compaction layers landing this sweep:
#63144's discovery-scope fix (archived rows surface from the current
session) and #69334's bookend bounding (summary exclusion + content
caps). A single compacted session exercises both: the archived FTS hit
must surface while the compaction handoff at the session tail stays out
of bookend_end and long messages stay capped.
Addresses @teknium1's second review round on #63144:
1. _is_compacted_message now checks both active=0 AND compacted=1.
Previously checked active=0 alone, which also matched rewind/undo rows
(active=0, compacted=0) that must stay hidden.
2. Added _is_compression_ended() — checks only the session's own
end_reason, not the lineage-wide has_compression_hop flag. This prevents
delegation children living under a compression continuation from leaking
through the lineage filter.
3. _discover lineage skip now uses is_ended_session (session-level check)
instead of has_compression (lineage-level flag).
New tests:
- TestRewindExclusion: rewind rows stay hidden alongside compacted rows
- TestCompressionEndedHelper: session-level end_reason checks
- TestLegacyContinuationPlusDelegation: delegate child excluded while
compression ancestors surface
78 passed (71 + 7 new).
After context compaction, pre-compaction content was invisible to
session_search — a memory black hole. The _discover() skip logic
filtered both same-session and same-lineage hits unconditionally,
without distinguishing compression-summarised content (gone from live
context) from delegation children (still visible to the parent agent).
Reworked _resolve_to_parent to return (root_id, has_compression_hop),
checking end_reason='compression' on every hop during the same
db.get_session() traversal — zero extra queries.
_discover() now has three compression-aware paths:
- In-place compaction: FTS hits on active=0 (compacted=1) rows pass
through even when raw_sid == current_session_id
- Legacy rotation: lineage hits pass through when has_compression_hop
is true on either side of the chain
- Delegation children: still excluded (no compression edge)
18 new tests covering all three scenarios + unit tests for the helpers.
Addresses Teknium's review feedback on #6256.
Closes#13840, #13841.
With compression.in_place defaulting True the cron session id never
rotates; get_compression_tip returns the input id. Pin that the
salvaged #67188 fix titles/ends the ORIGINAL cron session in that path
(and for falsy tip returns), i.e. zero behavior change when no
compression rotation happened. Also maps colingreig's contributor
email for attribution CI.
Hardening for the #59114 salvage: generate real standalone and merged
handoffs with the current compressor and assert classify_summary_content
agrees with _is_context_summary_content and is_compaction_summary_message
on every emitted shape (behavior contract, not a format snapshot), incl.
the flag-stripped DB-reload copy. Also adds the contributor email mapping
for @israellot.
A context-compaction handoff is persisted as an ordinary history message
but is not a real turn. The ACP history replay streamed it as a bare
user/agent message chunk, dropping the in-process _compressed_summary
marker, so ACP frontends (editors, vscode-hermes) rendered the entire
handoff as a regular message.
Tag replayed summary chunks under _meta.hermes (ACP's extensibility
channel), covering all three persistence shapes the compressor emits:
- standalone role="user" handoff -> compactionSummary: true
- standalone role="assistant" handoff (alternation-driven role pick)
-> compactionSummary: true
- merge-into-tail message (preserved tail content + appended summary)
-> containsCompactionSummary: true, a distinct key so clients that
collapse standalone summaries cannot hide the preserved real content
Detection honors the in-process metadata flag and falls back to a new
ContextCompressor.classify_summary_content() content classifier
(standalone/merged/None), so it also works for a DB-reloaded session
that lost the in-memory flag. _is_context_summary_content is now a thin
wrapper over the classifier, keeping existing callers unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: surface E2E screenshots in review comment
* ci: mark completed review commits in past tense
* ci: surface approved sensitive-file reviews
* ci: link sensitive files to reviewed changes
* ci: stage desktop E2E visual evidence
Track screenshots newly introduced against main and package visual diffs for a trusted publisher.
* fix(ci): pass E2E evidence output paths
Supply the manifest and staging-directory arguments required by the screenshot status helper.
* fix(ci): download the OSV SARIF artifact
Match the artifact name and result filename emitted by the pinned upstream reusable workflow.
Detect a billing wall once (agent/error_classifier → FailoverReason.billing) and
map it to a recovery link + label in one place, then carry that structured
BillingBlock to every surface instead of re-parsing free-form error text per
surface.
- agent/billing_links.py: provider-agnostic slug/host → (label, billing URL)
table (single source of truth), Nous-aware (is_nous routes to the in-app flow);
unknown providers degrade to a readable label with no invented URL.
- conversation_loop: both billing exit paths return a billing_block through one
helper; the guidance message carries the derived URL for every provider.
- gateway forwards billing_block on message.complete (it was dropped).
- @hermes/shared: BillingBlock type shared by desktop + TUI.
CLI: the VAD monitor's playback cut and the record-key interrupt both
mark the latch; the chat path prepends the note via the existing
_prepend_note_to_message channel. TUI: _tts_stream_stop() grows a
user_barge flag (False for /voice off — a mode change isn't an
interruption; no-op when speech already finished), the VAD monitor
marks on cut, prompt.submit accepts interrupted:true from clients, and
_run_prompt_submit pops the latch into the run message.
mark_speech_interrupted() / take_speech_interrupted(): a one-shot,
TTL'd (120s) latch plus SPEECH_INTERRUPTED_NOTE. Barge-in paths mark
it when they cut live speech; the next turn's submit path pops it and
prepends the note to the model-bound message — API-call local, never
persisted, so history and prompt caching are untouched.
/api/audio/speak-stream WebSocket: one socket + one Web Audio clock per
reply. The renderer feeds raw LLM deltas as they arrive; the server
cuts sentences with the shared chunker and streams int16 PCM back while
generation continues — speech overlaps generation with no per-sentence
connection or synthesis gaps. Falls back to the POST data-URL path for
old backends / non-chunked providers.
Barge-in runs a MediaRecorder on the monitor's stream the whole time
playback is live (rotated while quiet to bound pre-roll); talking over
the agent cuts playback and the complete utterance goes straight to
transcription and submit. /api/audio/transcribe returns 200/"" for
no-speech results so quiet turns re-listen instead of toasting a 400.
Turn deltas feed a per-turn TTS pipeline; the post-complete speak_text
call survives only as a fallback. session.interrupt, /voice toggles,
and new turns cut in-flight speech. VAD barge-in emits
voice.interrupted at detection, then the captured interruption goes out
as voice.transcript — the same event the TUI already submits as a
spoken turn.
The streaming gate broadens from ElevenLabs-only to any provider that
passes check_tts_requirements(). In continuous voice mode a mic monitor
runs during playback: talking over the agent cuts TTS at detection
while the monitor keeps recording, then the captured interruption is
transcribed and queued as the next turn (process_loop's auto-restart
stands down while the capture owns the mic). New voice.barge_in config
key, default true.
