When reply_in_thread=false, top-level channel events carry their own
message ts as metadata.thread_id for session keying. Calling
assistant.threads.setStatus on that ts activated a Slack assistant
thread ('is thinking...') before the actual response was sent, and the
flat reply then never cleared it.
send_typing now routes through the same _resolve_thread_ts synthetic-
thread guard as message sending, and the gateway threads message_id
through progress/status metadata so the adapter can distinguish real
threads from synthetic top-level session keys.
Reapplied from #18859-sibling PR #17184 by @dorukardahan (both commits:
fix + progress-metadata test) onto current main via 3-way apply — the
original patched gateway/platforms/slack.py, moved to
plugins/platforms/slack/adapter.py in the plugin migration.
Progress/status callbacks (context-pressure, compression retries,
model fallback) route through _send_or_update_status_coro, which
edits the previous bubble for the same status_key when the adapter
implements send_or_update_status — but only Telegram did. On Slack
every status event posted a fresh thread message, so a compression
retry loop spammed a dozen out-of-order bubbles into the thread
('Context too large 1/3... 2/3... 3/3', fallback switches, etc.).
Implement send_or_update_status on the Slack adapter following the
Telegram pattern (#30045): first call posts and caches the message ts
per (channel, thread, status_key); subsequent calls edit that message
via chat.update. Edit failure drops the cached ts and falls back to a
fresh send. Cache is FIFO-bounded.
Covers cluster C1-images (#69185, #32315, #66136):
- _slack_file_marker unit tests: typed markers per mimetype family,
hostile-filename sanitization (newlines/brackets can't fake context
structure).
- _render_message_text appends markers; a caption-less image post no
longer vanishes from thread context.
- Cold-start hydrate integration: prior-message images surface as
markers in channel_context; the thread root's image is downloaded,
delivered as media_urls, and upgrades message_type to PHOTO.
- Failure path: root-image download failure degrades to the marker,
never blocks the turn.
- Bounds: root delivery capped at _THREAD_ROOT_IMAGE_MAX; non-image
root attachments stay marker-only (no download).
- One-time delivery: active thread session skips the hydrate → no
re-download/re-delivery on later turns.
- Composition: the trigger's own event files still ride alongside a
delivered root image; Slack Connect stubs resolve via files.info;
the collector never issues its own conversations.replies call.
- Delta refresh (#23918 path): images in new replies past the watermark
surface as markers, with no root re-download.
A/B: 14 of 15 tests fail with the adapter fix reverted, all pass with
it applied.
When the bot is mentioned mid-thread for the first time, the thread root
is very often the artifact the mention is about ("@bot what's in this
chart?" posted as a reply under an image) — but the root's image never
reached the agent, so it answered blind.
On the cold-start hydrate path (and only there), _collect_thread_root_images
reads the root message from the thread-context cache the immediately
preceding _fetch_thread_context call just populated (zero extra Slack API
calls in the normal case), downloads its image/* attachments through the
existing authenticated _download_slack_file helper, and delivers them as
media_urls/media_types on the same MessageEvent — upgrading the message
type to PHOTO so vision routing engages.
Scope and safety:
- One-time delivery by construction: the cold-start path is guarded by
_has_active_session_for_thread, so later turns in the same session can
never re-download or re-deliver. No new gateway/session plumbing needed.
- Bounded by _THREAD_ROOT_IMAGE_MAX (4); non-image root attachments stay
text-only markers.
- Slack Connect stubs (file_access=check_file_info) resolve via files.info.
- Best-effort: a failed download degrades to the [image: ...] marker from
the thread context — never an error turn.
- Also hardens the video mimetype fallback (mimetype can be empty) so
media_types entries are always non-None strings.
Adapted from #69185 by @KCAYAAI — the original plumbed MessageEvent media
through gateway/base.py, run.py and session.py with durable one-time
delivery markers (2,441 lines); this lands the user-visible behavior
adapter-locally by reusing the session guard already on the hydrate path.
Images and files posted in a Slack thread before the bot joins were
invisible to the agent: _fetch_thread_context renders text only, and a
caption-less image post was dropped from context entirely (empty text →
skip). "@bot what do you think of the chart above?" read as a question
about nothing.
_render_message_text now appends a compact, sanitized marker per file
attachment — [image: chart.png], [video: demo.mp4], [audio: note.m4a],
[file: report.pdf (application/pdf)] — so the agent can SEE that prior
thread messages carried attachments and ask for a re-share when it needs
the bytes. Filenames are stripped of newlines/brackets so a hostile name
can't fake context structure. Because both thread-context formatting and
parent-text rendering go through _render_message_text, markers appear on
the cold-start hydrate, the explicit-mention delta refresh, restart
rehydration, and reply_to_text.
Reapplied from #32315 onto the current adapter (original patched the
pre-plugin gateway/platforms/slack.py, moved in the plugin migration;
annotation labels reworked to per-file typed markers, download side
handled separately).
#58478's caplog test predates main's unlabeled-bot users.info probe
(#69xxx wave-2 gating): events without client_msg_id now hit
_resolve_user_is_bot, which the fixture's mock client doesn't wire up
(AttributeError on _user_is_bot_cache). Real human-authored Slack
messages carry client_msg_id — add it to the fixture so the test
exercises the intended block-extraction path.
Follow-up hardening for the C13 log-noise cluster:
- plugins/platforms/slack/adapter.py: clarify button resolution logged
the full chosen option text at INFO (choice=%r). Choice text is user
content — log the choice INDEX at INFO and the (truncated, %.100r)
text at DEBUG only. Widens #58478's principle: no message content
above DEBUG level anywhere in the adapter.
- tests/gateway/test_slack_log_noise.py (new): behavioral suite pinning
the cluster's invariants:
* catch-all ack registered AFTER every named handler (registration
order is bolt's dispatch priority — no shadowing);
* catch-all fires for an unsubscribed event type (reaction_added),
logs only a DEBUG line naming the type, and never logs content;
* named handlers still dispatch (message → _handle_slack_message);
* end-to-end inbound message run leaves NO message text or block
content in any adapter log record (caplog at DEBUG);
* #30185's event-arrival diagnostic is metadata-only;
* clarify resolution: INFO carries index/user only, text is
DEBUG-only (fails with the adapter fix reverted — A/B verified).
Content-leak audit of all 139 logger call sites in the adapter found
two above-DEBUG leaks: the clarify choice line (fixed here) and none
else carrying message text; remaining sites log error strings, URLs
via safe_url_for_log, ids, and counts. The block-extraction DEBUG
preview was already removed by #58478 (chars= length only).
Regression test for the catch-all ack: a re.compile(r'.*') event matcher
must be registered (after every named handler, so it never shadows
message/app_mention/reaction/file routing) and must match unhandled
subscribed event types like member_joined_channel / channel_archive /
pin_added.
Salvaged from #64218 (test half only — its adapter-side catch-all is a
duplicate of #38847, which landed as the base commit of this cluster
with first-submitter credit). Fixes#6572.
Co-authored-by: shivasymbl <sdevinarayanan@asymbl.com>
Slack posts are durable workspace messages, not an ephemeral terminal
status area. Default long_running_notifications and busy_ack_detail to
off for Slack so long-running agent work does not leave permanent
operational breadcrumbs like 'Working — 9 min — iteration 12/90' in
channels. Both remain opt-in per platform via
display.platforms.slack.*.
Also covers the platform-generic shutdown-notification mute path with a
regression test (gateway_restart_notification=false must suppress both
the active-session interruption notice and the home-channel copy).
Salvaged from #69028 (quiet-defaults half only). The PR's other half —
the channel_session_scope_channels session-scoping feature — is a new
config feature outside this log-noise cluster and overlaps the session
scoping territory reworked by merged wave-1/2 Slack session work; it is
deliberately not taken here.
Without a catch-all handler, slack-bolt returns HTTP 404 for every
unhandled bot event (user_change, user_huddle_changed, reaction_added,
etc.) and never sends the Socket Mode ack. On active Slack workspaces
where the app is subscribed to high-volume events, this produces a
near-100% un-acked failure rate that crosses Slack's >95%/60-min
threshold and triggers automatic disabling of the app's Event
Subscriptions — silently killing all inbound event delivery.
Place a catch-all re.compile(r".*") handler AFTER the specific event
handlers so bolt's router matches those first. Truly unhandled events
are silently acked (200) and logged at DEBUG. The failure rate stays
near 0% regardless of which events the Slack app manifest subscribes to.
Fixes#6572
Treat Slack users.conversations missing_scope as an expected limited-scope app condition and fall back to session history without recurring warnings.
Add tests for not-ok and SlackApiError-like missing_scope responses.
Follow-up to the #68627 cherry-pick (cluster C15 — Slack platform
capability-note accuracy; earliest report/fix: #6545 by @daikeren):
1. Session/prompt stability: the pinned session-context render
(_pinned_session_context_prompt) is keyed by _ephemeral_change_key,
whose contract requires every rendered input to appear in the key.
The new _slack_tools_loaded() gate reads config + the live MCP
registration map, so its state is now hashed into the key exactly
like the existing Discord gate — a gate flip re-renders ONCE (a
legitimate bust); within a session the note stays byte-stable for
the life of the conversation (A/B: the new parity test fails with
this key change reverted, passes with it).
2. Derived, non-overpromising positive note: rather than hardcoding a
capability list that can drift stale again (the original bug class),
the tools-present note tells the agent to consult the actual loaded
Slack tool schemas for supported operations — the schemas ARE the
source of truth, so the note cannot overclaim ops a given Slack
toolset/MCP server doesn't expose (e.g. a read-only history server).
3. Tests: parity test proving a gate flip changes both render and key;
byte-stability test proving three consecutive turns in one Slack
session return the identical pinned object (sha256-equal); autouse
fixture pins the new gate so key<->render parity is env-independent.
Ports #63234 forward onto current main per teknium1's review.
gateway/session.py hard-coded the stale-API disclaimer for every Slack
session regardless of whether Slack tools were actually loaded. This
contradicted the system prompt when MCP or native slack tools were
present, causing the agent to refuse Slack API actions it could
actually perform (issue #6536).
Per review, the original predicate only checked the native 'slack'
toolset, missing Slack MCP servers (registered under mcp-<server> in
tools/mcp_tool.py) entirely. _slack_tools_loaded() now checks two
independent paths:
1. Native 'slack' toolset + SLACK_BOT_TOKEN (as before, but now calls
_get_platform_tools() with include_default_mcp_servers=True instead
of False, so a default-enabled MCP server also counts).
2. A connected MCP server that has ACTUALLY registered tools into the
live registry (new tools.mcp_tool.get_registered_mcp_server_names()),
whose name suggests Slack. This is session-scoped in the sense that
matters here: MCP servers connect once per gateway process (not
per-session), so checking the live per-server tool-registration map
is the correct availability-filtered signal -- unlike the earlier
get_all_tool_names() approach this replaces, which conflated ALL
built-in tool names process-wide, this only inspects the small,
purpose-built MCP server-name map.
Added a real regression test that registers a tool via the actual
tools.mcp_tool._track_mcp_tool_server() tracking function (not a mock
of the capability check) to verify a genuine Slack MCP server is
detected, plus a negative case for an unrelated MCP server.
5/5 Slack-specific tests pass; 126/126 in the full
tests/gateway/test_session.py file.
Follow-up to the salvaged PR #57320 commit: the PR bridged port/host/secret
for MSGRAPH_WEBHOOK but only tested webhook and api_server. Adds a test
covering the msgraph_webhook branch including extra-precedence, plus the
contributors/emails mapping for kjames2001.
WebhookAdapter and ApiServerAdapter read port/host from config.extra, but
PlatformConfig.from_dict only populates extra from the 'extra:' sub-key in
the YAML platform section. Top-level keys like port and host are silently
ignored, causing the adapter to fall back to DEFAULT_PORT (8644).
This causes silent port conflicts in multi-profile setups: a profile that
configures 'platforms.webhook.port: 8649' still binds 8644, colliding with
the default profile's webhook on the same port.
Fix: extend the shared-key bridging loop in load_gateway_config() to bridge
top-level port/host/secret into extra for WEBHOOK, MSGRAPH_WEBHOOK, and
API_SERVER platforms, following the same pattern already used for
dm_policy, allow_from, gateway_restart_notification, and other keys.
The extra dict takes precedence: if port is already under 'extra:', the
top-level value does not clobber it.
Documents user-facing wave-1+2 Slack behavior that had no docs coverage:
- decision table for require_mention / free_response_channels /
require_mention_channels / thread_require_mention / strict_mention /
ignore_other_user_mentions and how they compose
- 'Accepting messages from other bots' section with the post-#69483
semantics: allow_bots=mentions requires a CURRENT mention from
peer bots (text or Block Kit blocks); thread state never admits them
- clarify one-tap buttons (choice buttons + Other free-text mode,
in-place resolution, double-click guard, expiry message)
- slash replies are ephemeral: replace-ack, chunking, 5-post cap with
explicit truncation notice, postEphemeral fallback, never-public rule
- cron deliver targeting (slack -> home channel, slack:C... channel,
slack:U... resolved to DM) incl. standalone sender + MEDIA uploads
- send_message media + bare-user-ID DM resolution and caption behavior
Refs #26184.
Salvaged from #45765 by @navahc09 — kept the PR's callout structure
and placement, rewrote the content to match current behavior:
Slack blocks native slash commands in threads and never delivers them,
so Hermes recognises a leading '!' as an alternate command prefix.
Post-C3 command fixes the bang form also works behind a mention
(@Hermes !cmd) and with leading whitespace; unknown '!' tokens pass
through to the agent unchanged. Cross-linked the detailed
slack.md section.
0.15s missed by 1-8ms on busy CI shards twice today (runs 30025889952,
30026770278 — branches not touching this file). The assertion pins
'handler did not block on the timeout path' (blocking = seconds); 2.0s
keeps the contract per the loose-bounds flake policy.
A user who configured a provider only inside a MoA preset (advisor or
aggregator slot) has explicitly opted into that provider — the consent
gate (is_provider_explicitly_configured) now scans moa.reference_models,
moa.aggregator, and all moa.presets.* slots, so Claude Code OAuth pool
seeding and the auxiliary auto-fallback chain treat MoA-only Anthropic
users consistently with model.provider users.
Salvaged from PR #57778 (trimmed): the auxiliary_client fallback half of
the original PR was independently landed on main in ddd3a2d247 and is
dropped here; a secret-scrubber artifact in the gate test fixture is
restored to the real placeholder token.
_apply_env_overrides() force-set ``api_server.enabled = True`` whenever
API_SERVER_KEY (or API_SERVER_ENABLED) was present in the environment.
In multiplex mode, a secondary profile pins
``platforms.api_server.enabled: false`` in its config.yaml so that it
shares the default profile's API-server listener instead of binding its
own port. That profile still inherits the process-level env, including
API_SERVER_KEY, so the unconditional re-enable flipped api_server back on
and tripped the MultiplexConfigError check.
Honor an explicit disable, flagged by ``_enabled_explicit`` in the
platform's extra. Use ``extra.pop("_enabled_explicit", False)``: the
api_server branch is terminal (unlike the migrated plugin platforms, no
later registry pass re-enables api_server), so popping consumes the flag
in a single read and avoids the double-read hazard, while the final
per-platform cleanup remains a no-op.
Adds a regression test asserting that with API_SERVER_KEY set, a config
with api_server explicitly enabled:false + _enabled_explicit:true survives
_apply_env_overrides() as enabled=False (fails without the fix), while the
key is still wired through for the shared listener.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to #60168's salvage: aggregate_moa_context() is the third
independent MoA call path; assert its aggregator call receives the
custom-provider request_overrides.extra_body via **agg_runtime.
'auto' is a sentinel meaning "inherit from main runtime / auto-detect",
not a literal model id -- already handled for cfg_model (config-derived)
in _resolve_task_provider_model, but not for the explicit `model` kwarg.
MoA reference/aggregator slots (agent/moa_loop.py's _slot_runtime) forward
a preset's `model:` field as this explicit argument rather than through
auxiliary.<task> config, so a MoA preset configured with `model: auto`
(a natural thing to try given the existing auxiliary.*.model: auto
convention) reached this function as the explicit `model` arg and took
the `model or cfg_model` branch, bypassing the cfg_model-only sentinel
check entirely -- sending the literal string "auto" to the wire as a
model id.
Normalize both the explicit `model` and `cfg_model` the same way, fixing
this at the single chokepoint every caller (MoA included) already goes
through, rather than patching moa_loop.py separately.
Extends the desktop backend's root-cause fix (aa2ae36c3f) to all remaining
console-less parent launch paths. The Windows console-flash class
(#54220/#56747) is governed by the PARENT's console: a DETACHED_PROCESS or
pythonw.exe daemon has no console, so every console-subsystem descendant
(git, gh, cmd, node, wmic, powershell) allocates its own visible conhost —
one flash per spawn, unreachable by any per-call-site CREATE_NO_WINDOW
sweep. Worse, MSDN specifies CREATE_NO_WINDOW is IGNORED when combined
with DETACHED_PROCESS, so the hide bit in the old detach bundle was dead.
Changes:
- _subprocess_compat: drop DETACHED_PROCESS from windows_detach_flags()
and windows_detach_flags_without_breakaway(); the daemon now owns a
single hidden console (CREATE_NO_WINDOW) that all descendants inherit.
- gateway_windows: _resolve_detached_python() returns the venv console
python.exe (no pythonw/base-interpreter detour — the uv-shim flash
premise only held while DETACHED_PROCESS was masking the hide bit);
UAC handoff launches console python under SW_HIDE; cmd/vbs launchers
render console python (vbs runs it window-style 0).
- gateway/run.py: restart watcher keeps sys.executable instead of
swapping in GUI-subsystem pythonw.
- web_server: dashboard actions spawn sys.executable (already carries
windows_detach_flags()).
Tests updated to pin the new invariants, including an explicit
DETACHED_PROCESS-must-stay-out regression guard.
Follow-up to the #62614 salvage: try_refresh_matching (added by the
#69843 salvage after this PR's base) calls self.current() while already
holding the now-locking non-reentrant pool lock — a guaranteed deadlock
that git merges silently (no textual conflict). Use _current_unlocked()
and cover the method in the no-deadlock test.
Follow-up to review feedback:
- Acquire self._lock in the remaining public pool-state methods:
has_credentials, reset_statuses, remove_index, resolve_target, and
add_entry. All of them read or rebind self._entries (and the mutating
ones persist auth.json), so they now hold the same lock as select()
and the query methods. None are called from within the lock, so no
unlocked helpers are needed.
- Make the blocking test deterministic: an instrumented lock records the
acquire attempt, and the test first waits for the worker to actually
reach self._lock before asserting it blocks. Previously an unlocked
method could pass if the worker thread was scheduled late.
- Extend the lock test matrix to all nine public methods; the five newly
locked ones fail the test without this fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`has_available()`, `peek()`, `current()` and `entries()` read (and, via
`_available_entries()`, mutate and persist) `self._entries` without holding
`self._lock`, while every other entry point — `select()`,
`mark_exhausted_and_rotate()`, `acquire_lease()`, `try_refresh_current()` —
guards the exact same access with the lock.
`_available_entries()` is not read-only: it prunes aged-out DEAD manual
entries (rebinding `self._entries` at the prune step) and calls `_persist()`
(writes auth.json). The gateway runs platform adapters in threads and cron
runs jobs in a ThreadPoolExecutor, so a status probe via `has_available()`
or `peek()` can race a concurrent `select()`/rotation: torn iteration of
`self._entries`, interleaved auth.json writes, or a lost token rotation.
Fix: take `self._lock` in all four query methods. Because the lock is
non-reentrant and `peek()` composes `current()` + `_available_entries()`,
add a lock-free `_current_unlocked()` helper and route the already-locked
internal callers (`_select_unlocked`, `mark_exhausted_and_rotate`,
`_try_refresh_current_unlocked`) through it to avoid self-deadlock.
Added regression tests: a no-deadlock check (peek re-entrancy) and a
lock-held-blocks-the-call check for each of the four methods.
Decompose children inherit the root's literal workspace_path (#37172),
so every sibling of a worktree-kind root points at the SAME checkout.
_resolve_worktree_workspace's existing-checkout shortcut then reuses
that directory on whatever branch is currently checked out, ignoring
the task's own branch_name. Net effect: sibling workers — which can be
promoted and dispatched concurrently — run in one directory on the
first sibling's branch, with no lock. Work lands on the wrong task's
branch (provenance corruption) and concurrent siblings trample each
other's index/tree.
Fix, two layers:
- decompose_triage_task: worktree-kind children no longer inherit the
root's literal path; each child materializes its own
<repo>/.worktrees/<child-id> at dispatch (dir/scratch inheritance
unchanged — children legitimately share those).
- _resolve_worktree_workspace: when the requested path is an existing
checkout of a DIFFERENT branch, fall back to a fresh
<repo>/.worktrees/<task-id> instead of silently reusing it (heals
rows that already carry a shared path). Same-branch reuse and the
no-repo/own-path degenerate cases keep the legacy behaviour.
Tests: tests/hermes_cli/test_kanban_worktree_isolation.py (5); full
test_kanban_db.py + test_kanban_decompose_db.py suites pass unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to the #65844 salvage: the new anthropic pool test must stub
read_claude_code_credentials like the sibling tests, otherwise a dev
machine's live claude_code singleton seeds a third entry and the
no-benching assertion fails outside CI.
Two related races in credential-pool cooldown state:
1. Lost update across processes: write_credential_pool merged only
entries missing from the caller's snapshot; for entries present on
both sides the caller's in-memory copy won wholesale. A process
holding a snapshot taken before another process marked a key
exhausted would, on its next persist (e.g. a round-robin rotation),
write the key back as healthy — erasing the cooldown so every
process resumes hammering a rate-limited key. Merge status fields by
last_status_at recency: adopt the on-disk status only when it is
strictly newer AND still binding (DEAD, or EXHAUSTED with an
unexpired cooldown), and never onto re-authed (token-changed)
entries, so legitimate expiry-clears and fresh logins are preserved.
2. Wrong-key quarantine: when mark_exhausted_and_rotate received an
api_key_hint that matched no entry, it fell through to
current()/_select_unlocked() — on a freshly loaded pool that selects
the NEXT healthy key and benches it for the full cooldown TTL,
punishing an innocent credential. When a hint is provided but
unmatched, rotate without marking anything instead of guessing.
Includes regression tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
git stash push --include-untracked exits non-zero when it saved
everything but could not DELETE some swept untracked files from the
working tree (e.g. a root-owned packaging/ directory left behind by a
sudo'd build: 'warning: failed to remove ...: Permission denied').
The updater ran the push with check=True, so this benign partial
failure raised CalledProcessError and aborted the whole update before
it even fetched — reliably, on every run, for any user with an
undeletable untracked path in the checkout.
Fix, both ends of the class:
- _stash_local_changes_if_needed: probe refs/stash before/after the
push. Non-zero push + fresh stash entry = changes are saved; warn,
reset the tracked-side leftovers (they're in the stash), and
continue the update. Non-zero push + NO stash entry = real failure;
keep aborting.
- _restore_stashed_changes: on restore, those same undeletable files
still sit in the tree, so 'git stash apply' exits 1 with 'already
exists, no checkout' even though every tracked change applied and
nothing was lost. Classify that stderr shape (strictly — any other
error line still routes to the conflict path) as restored instead
of resetting the tree and telling the user the restore failed.
Repro'd both halves with real git; behavioral E2E test covers
stash -> checkout -> restore round-trip with an undeletable dir.
A 402/429/401 is an API-key–level failure (account out of balance,
rate-limited, or key rejected), but the same key can back more than one
pool entry — e.g. an explicit pool entry plus a `model_config` entry
auto-seeded from `model.api_key`, both carrying the identical
`runtime_api_key`.
`mark_exhausted_and_rotate(api_key_hint=...)` only marked the *first*
matching entry, leaving the sibling OK. `_select_unlocked()` then kept
handing back the same depleted key, so the billing-recovery `continue`
loop in the conversation retry path never converged: the request hung
until the client disconnected (~2.5min observed against DeepSeek),
emitting only `response.created` with no 402 ever surfaced to the user.
Mark every entry sharing the failed key so the pool can reach the
"no available entries" state and let the error propagate immediately.
Adds a regression test covering two entries backed by the same key.
The dedup-reset calls assumed a full AIAgent; gateway/loop test doubles
built via object.__new__ lack _clear_context_overflow_warn and crashed
in build_turn_context (caught by test_api_content_sidecar on CI slice 3).
getattr-guard all four call sites per the established test-double pitfall
pattern (AGENTS.md #17).
Follow-up fixes for the #62625 salvage:
- Dedup-reset gap (sweeper review): when the block clears while the
context is STILL over threshold, execution enters the compression
branch — the PR's 'else' reset never ran, so the warning stayed
suppressed forever after the first block. _clear_context_overflow_warn()
now fires on every automatic compression path: turn-context preflight,
conversation_loop pre-API gate, and the post-tool loop-compaction gate.
- should_compress_info on current main: main refactored should_compress
into _automatic_compression_blocked()/_locally(); the tuple variant now
derives its reason from the same in-memory state via
_compression_block_reason(), keeping cooldown:<s>/ineffective shapes.
- ContextEngine.should_compress_info ABC default now actually returns
(should_compress(tokens), None) — the PR's default had a docstring but
no return (returned None, would crash tuple-unpacking call sites).
- Below-threshold guard: the turn-context persisted-cooldown branch and
the conversation_loop pre-API cooldown branch no longer warn when the
estimate is under threshold (should_compress_info returns a None
reason; the preflight pre-check is not a threshold guarantee). The
pre-API guard also honors compression.max_attempts instead of a
hardcoded 3, and no longer fabricates a cooldown reason.
- Noise-filter survival (#69550 composition): warning text is now a
template constant (CONTEXT_OVERFLOW_BLOCKED_WARNING_TEMPLATE) marked
FAILURE-CLASS, pinned un-swallowed in VISIBLE_COMPRESSION_MESSAGES and
in new tests that execute the real _TELEGRAM_NOISY_STATUS_RE +
_prepare_gateway_status_message.
- Contributor mapping for stanislav@local -> sl4m3.
- ContextEngine.should_compress_info() default impl so plugin engines
(e.g. _StubEngine) don't raise AttributeError at the call site.
- Centralise warning/reset in AIAgent._warn_context_overflow_blocked /
_clear_context_overflow_warn so turn-context and conversation-loop guards
share identical dedup logic and reset on the real compression boundary.
- Cover conversation_loop.py pre-API (~L1007) and loop-compaction (~L4774)
guards, not just the turn-context preflight.
- _FakeAgent mirrors the two helpers; test suite green (219 passed).
Fixes#62708
Previously, when a session crossed the compression threshold but compression
was skipped (summary-LLM cooldown, #11529, or anti-thrashing, #40803), the
model kept accumulating context until it hit the hard provider token limit and
silently stopped answering — with no signal to the user about why.
Changes:
- context_compressor.should_compress_info() returns a (should_compress, reason)
tuple. reason is 'cooldown:<seconds>' or 'ineffective' when compression is
needed but blocked. should_compress() keeps its bool contract so existing
callers (conversation_loop.py) and regression #29335 are unaffected.
- turn_context.build_turn_context() emits a deduped _emit_warning when the
context is over threshold but compression is blocked, advising /new or
/compress. Dedup keys on the block *kind* (cooldown/ineffective), not the
ticking countdown, so a cooldown doesn't re-fire the warning every turn.
- Adds tests/agent/test_turn_context_overflow_warning.py covering the tuple
shape, both block kinds, dedup, and re-fire-after-clear.
