fold2 finding 1 (2c, Medium): the connect-time missing-pin warning was gated on
`not is_reconnect`. An account whose FIRST startup connect fails (transient
IMAP/SMTP error) is brought up by the reconnect watcher with is_reconnect=True,
so it never warned while silently dropping every sender's mail.
Dedupe per address instead: a class-level _missing_pin_warned set (the watcher
builds a fresh adapter per retry, like _seen_uids_snapshot), checked after both
IMAP and SMTP probes succeed. The existing connect test now covers a failed
first connect followed by a reconnect (one warning), a second reconnect (none),
two accounts (one each) and pinned/opted-out accounts (silent).
fold2 finding 4 (quality, Low): _verify_sender_authentication defaulted
authserv_id to "" (a silent fail-closed for any caller that forgot it) and
re-stripped/lowercased a value EmailAdapter.__init__ already normalises. Make it
a required keyword, test it with `if not authserv_id:`, and compare directly.
All callers (the adapter and the test helper) already pass normalised ids by
keyword, so no call site changes.
fold2 finding 2 (reuse+quality, Low): the per-message no-header drop hint and
the connect-time missing-authserv_id hint each spelled out the same
require_authenticated_sender / EMAIL_TRUST_FROM_HEADER opt-out sentence. Build
both from one _OPT_OUT_HINT so they cannot drift. Rendered text is byte-for-byte
unchanged (printed before/after).
fold2 finding 3 (quality, Low): the __init__ comment still said an absent pin
fails closed only "when an allowlist grants access". With sender auth on, an
unset authserv_id rejects every sender's Authentication-Results, and connect()
names the fix once per account. Comment only.
The missing-pin warning lived in the per-message drop path behind a
process-wide class bool. That hid it twice over (r1 2ab/2c Medium):
- a second unpinned mailbox in the same process (multiplex) never
warned once the first had tripped the flag;
- pairing/decline-only deployments never warned at all, because a
first-contact sender is not granted and its drop logs at debug.
A missing pin is account config, not something one sender did, so
connect() now names the fix once per account after the IMAP/SMTP
probes succeed, skipping reconnect-watcher retries (is_reconnect).
Pinned or opted-out accounts stay quiet. The per-message drop path is
back to main's rule: a listed sender's drop warns (with the reason, no
hint), an unlisted one logs at debug, so it is never noisier than main.
The guide still told users to "leave authserv_id unset" when their
server's host varies, which now drops every message that needs an
authenticated From:. State that the pin is required, how to find it
(the token before the first ';' of the topmost Authentication-Results
header your server writes), what to do when it seems to vary (RFC 8601
authserv-id names the administrative domain, so configure a stable one
and pin it), and that the only alternative is the explicit
require_authenticated_sender: false / EMAIL_TRUST_FROM_HEADER=true
opt-out. Mirror the reference tables, plugin.yaml prompt and zh-Hans
translation.
Co-authored-by: JiuYue0820 <287222957+FirmaSpring@users.noreply.github.com>
The salvaged #88560 change makes an unpinned deployment refuse every
Authentication-Results header, so each message from an allowlisted
sender is dropped. Logged per message, that warning would flood the gateway log.
The salvage gave the reason its own constant and drop hint (set
EMAIL_AUTHSERV_ID / platforms.email.authserv_id, or opt out with
require_authenticated_sender: false / EMAIL_TRUST_FROM_HEADER=true);
this logs it once per process, and later drops stay at debug.
Tests: the verifier helper now pins the topmost header's own id by
default (a correctly configured receiver), the topmost/exact-pin test
gains the unpinned forged-header row, and the dispatch warning test
checks the once-per-process warning and its text.
Co-authored-by: JiuYue0820 <287222957+FirmaSpring@users.noreply.github.com>
Trust only the topmost Authentication-Results field from the explicitly configured receiving MTA. Fail closed for allowlisted senders when no exact pin is configured, preventing injected or lower matching headers from authorizing a sender.
(cherry picked from commit e013fda3facc005e8f4d5f81e20c2d21aee698df)
The wizard's invite integer and the Discord docs' Recommended row carried
Use External Emojis (1<<18), a permission the adapter never exercises (it
only reacts with unicode emoji) and that no permission list in the docs
names. The voice-mode page and the zh-Hans Discord page already used the
correct integers, so the pages disagreed with each other.
- onboarding.INVITE_PERMISSION_BITS: drop Use External Emojis
(wizard invite 309241171008 -> 309240908864, matching voice-mode.md)
- discord.md: Recommended 309238025280 -> 309237763136; the Minimal row
names Embed Links (117760 already includes it)
- .env.example and the zh-Hans table follow
Co-authored-by: Emil <etoegersen@gmail.com>
The left-core migration's only gate was "plugin dir missing + Home Assistant in use", so after
`hermes plugins remove homeassistant` every agent start, gateway start and `hermes update`
reinstalled and re-enabled it (~7.5 s each).
The automatic install now happens at most once per home and row: `_left_core_installed` in
config.yaml is written (in the same write as the toolset-scope marker when the plugin was already
there, right after a successful install otherwise) and only once the plugin dir exists in that
home. A marked row is never installed again; a catalog miss, failed install or lazy-installs-off
start writes nothing, so those homes keep retrying. Profile clones copy plugins/ with config.yaml,
so a clone's marker always matches what its source had.
The migration converts core's per-platform Home Assistant tool scope (off on telegram: [web], acp,
webhook) into known_plugin_toolsets entries, but only on the install branch: a home whose plugin
directory already existed returned before the conversion. The official plugin stays inert on a core
that still bundles HA, so a user who installed it before updating (or a partial earlier run) kept no
exclusions, and after the update the plugin toolset came on for every platform.
Installation and the one-time policy conversion are now separate: every home that uses the feature
gets the conversion once per row, plugin present or not, and the completion is recorded under
_left_core_scoped in the same config.yaml write. A later run (update, agent/gateway start) skips
it, so a choice the user makes afterwards (`hermes tools` enabling HA on telegram) is never undone.
The Home Assistant gateway platform (plugins/platforms/homeassistant) and the
homeassistant toolset (tools/homeassistant_tool.py) now ship as the official
catalog plugin `homeassistant` (NousResearch/hermes-homeassistant), with the
same platform name, toolset/tool names, config keys and env vars.
Migration (hermes_cli/left_core_migration.py, a generic "left core" table
reusing the memory-provider migration's helpers): every profile home that
used Home Assistant (HASS_TOKEN in its .env, platforms.homeassistant enabled
or holding a token, or the homeassistant toolset selected) gets the plugin
installed from the catalog at its reviewed pin by `hermes update`, and lazily
once per process at agent start / gateway start (honours
security.allow_lazy_installs). Every outcome reaches terminal, Desktop and
chat; a gateway-start outcome is handed to that home's first agent.
Core HA special cases become generic PlatformEntry seams the plugin uses:
trusted_inbound (allowlist bypass for event-bus platforms), display_tier
(built-in display defaults) and shared_env_prefixes (profile-clone ownership
rule for env shared with the plugin's tools). Cron delivery uses the existing
cron_deliver_env_var seam; HASS_TOKEN stays stripped from subprocesses via the
installed plugin's manifest secrets.
Review findings on the onboarding wizard:
- A token pasted with curly quotes reached the HTTP header and crashed with
UnicodeEncodeError; it now goes through the same non-ASCII stripping
save_env_value applies, before the Discord check.
- Reconfiguring replaced DISCORD_ALLOWED_USERS with the owner alone; the
wizard now starts from the existing entries and only adds.
- Hermes always requests Message Content, so a disabled intent makes Discord
refuse the connection (close 4014), not deliver blank messages. Wizard,
.env template and docs now say so.
The setup test records every write, so a rejected token appearing in any
write fails it, and uses ASCII fixture tokens now that the wizard sanitizes.
Discord was the hardest gateway to set up: the .env template had no Discord
section at all, the wizard saved whatever was pasted, and users had to find
the Message Content Intent toggle, build an OAuth2 invite URL by hand and turn
on Developer Mode just to learn their own user ID.
The wizard now reads GET /applications/@me with the pasted token: a rejected
token is re-prompted instead of saved, an intent that is off gets a direct
link to the toggle and a re-check, the invite link is printed with the
permissions the adapter uses, and the allowlist defaults to the application's
owner (or accepted team members). Offline, the token is still saved with the
manual intent reminder. The wizard moves from the 7k-line adapter into
plugins/platforms/discord/onboarding.py.
The salvaged change also put notify=True on both approval sends.
On A2A, `notify` marks the turn-final reply, and A2A has no native
approval renderer. A prompt that needs a human decision would therefore
complete the caller's task with the prompt text while the turn is still
waiting (@ehz0ah's review on #132527). Telegram already pushes the
prompt through the approval-aware _notification_kwargs, so only the
is_approval_prompt marker is needed. The card send goes back to the
verbatim thread metadata: Telegram's control-message path never silences
cards.
Tests: the runner test pins that the fallback metadata stays interim and
does not carry notify. The adapter tests become a single parametrize.
Fold the _notification_kwargs change back into the original one-line
shape, so adapter.py grows by one line instead of six.
In the default Telegram notification mode ("important") the plain-text
approval fallback marked only is_approval_prompt, which no notification
decision consults, so _notification_kwargs sent the prompt with
disable_notification=True: the phone got no push, the user never learned
a dangerous command was waiting, and the tool sat the full approvals.timeout
(default 300s) before failing closed (#132516).
Mirror the cron-delivery convention on both approval sends in
_approval_notify_sync: notify=True on the button card and the text
fallback, and make _notification_kwargs itself approval-aware
(is_approval_prompt defeats disable_notification) so every surface that
marks the prompt is covered in one place. Loosens no guard and
auto-approves nothing.
(cherry picked from commit 576fec2ab6a6cdca08f96968574ba741c47568c0)
Gaps in the unanswered-/health handling, found in a second review:
- _probe_bridge_health swallowed a timeout while reading the JSON body
and reported an unparseable 200, so a bridge that sent headers but
stalled on the body still reached _kill_port_process. Timeouts now
propagate from either phase.
- The guard listed listeners with the POSIX-only lsof/ss scan, which
finds nothing on Windows, so Windows fell through to the netstat-based
kill. Both now use _port_listener_pids(); if listing fails, nothing
is killed.
- _standalone_send treated a non-200 /health as an older bridge and
sent anyway. It now sends nothing.
- Docs named a nonexistent `hermes whatsapp send`; the command is
`hermes send --to whatsapp:<chat_id>`.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
Two gaps left by the session check, both found in review:
- connect(): a port holder that does not answer /health in time fell
through to _kill_port_process, so another profile's busy bridge was
still killed. No answer is no ownership evidence: after this profile's
own bridge is reaped by its pidfile, a remaining listener is left
running and connect() fails with the retryable
whatsapp_bridge_unresponsive.
- _standalone_send (hermes whatsapp send, cron delivery) posted to the
port without asking whose bridge it was, so a profile without its own
bridge sent from another profile's WhatsApp account. It now reads
/health before every send and refuses one whose session is another
profile's. A bridge without the field (older bridge.js) is still used.
The session dir is resolved by one helper for the adapter and standalone
sends, so both compare the same absolute string.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
Follow-up to the cherry-picked session handshake, rebased onto the
multiplex bridge ownership that landed in #132518:
- connect(): the foreign-session fatal sits ahead of main's
secondary-aware pidfile/port reaping, so neither path signals a bridge
that serves another profile's session.
- Drop the separate "no session -> restart" branch. A bridge without the
field runs an older bridge.js, so its scriptHash already marks it
stale and it restarts through the existing path.
- Inline sessionIdentity() as path.resolve() in /health and drop its
node test, which only exercised path.resolve and no CI lane runs.
- Collapse the four adapter cells into one parametrized invariant (own
session adopted; another session never adopted or killed, connected or
disconnected). The read-receipt test now reports its own session, so
it still restarts for the read-receipt reason rather than a missing
field.
- Docs: fold the separate-gateway guidance into the existing "Multiple
profiles" section instead of a second multi-profile section.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
Two profiles on one host default to the same bridge_port (3000) but have
profile-scoped session paths. _reuse_running_bridge adopted any connected
bridge whose scriptHash and read-receipt config matched, so profile B
adopted profile A's bridge and served A's session; on any mismatch,
connect() fell through to the pidfile/port kill path and killed A's
running bridge.
Contract, keyed on a new /health `session` field (normalized absolute
session dir via bridge_helpers.js sessionIdentity):
1. session present and EQUAL to the adapter's own normalized session
path -> today's logic unchanged (adopt on hash + read-receipt match,
else stale/restart).
2. session present and DIFFERENT -> FOREIGN bridge: not adopted, not
killed. connect() sets a fatal error naming the port, the foreign
session path, and the fix (platforms.whatsapp.extra.bridge_port, one
distinct port per profile).
3. session ABSENT (bridge predates the handshake) -> treated as stale
and restarted, exactly like an absent scriptHash today.
Normalization uses Node path.resolve / Python os.path.abspath - absolute
and `..`-normalized WITHOUT symlink resolution. fs.realpathSync /
Path.resolve() would resolve symlinks asymmetrically (macOS
/tmp -> /private/tmp) and produce false foreign verdicts.
(cherry picked from commit 7e8418678e91fa4bc69d4b4fa6faedd66f2a80e5)
A secondary treated a bound port as its own whenever its pidfile named a
live bridge, without checking that bridge listened on that port. After a
crash left the old bridge alive and bridge_port was pointed at a port
another profile's bridge held, the secondary probed and adopted the
foreign bridge (script hash and read-receipt config do not distinguish
phones). bridge.pid now records the spawn port on line 3; ownership
requires pid, start time and port to match. Pidfiles without a port fail
closed for secondaries; the default path reads lines 1-2 as before.
A profile that recorded a port as a secondary and later launched its own
gateway started its bridge on 3000 while `whatsapp send` read the record.
The launch adapter now resolves its port like standalone send (explicit,
then record, then 3000), so profiles without a record are unchanged.
gateway/AGENTS.md no longer lists the WhatsApp bridge as default-only
shared ingress.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
A secondary allocated its bridge port by bind probe alone. A sibling whose
bridge is down (gateway stopped, profile disabled) leaves its recorded port
unbound, so a newly paired profile took it and both records claimed one port;
the second to start hit the non-retryable whatsapp_bridge_conflict on every
restart. Allocation now skips ports another profile's record claims.
A non-numeric record surfaced int()'s "invalid literal" as the fatal text;
it now names the record file and says to delete it.
The secondary connect tests stub find_node_executable: connect() now ensures
Node before anything else, so without it they exited before the secondary path.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
Co-authored-by: Enough1122 <Enough1122@users.noreply.github.com>
febd2af391 skipped WhatsApp on every secondary profile under multiplex on the
premise that "a secondary has no credential of its own to bring" because the
bridge is "one authenticated session tied to a single phone number". The
session directory is home-relative (`platforms/whatsapp/session` under the
profile's own home), so each profile already owns its own paired session.
What actually collided was the bridge port, 3000 for everyone, and the
adapter's startup path that kills whatever holds that port before spawning.
A fleet running one phone per profile lost WhatsApp on every profile but the
launching one, with a single INFO line to say so.
The credential gate for WhatsApp is now the profile's own paired session
(`creds.json` in its session directory), evaluated in that profile's scope.
An unpaired secondary is reported as `whatsapp_unpaired` with the pairing
command, once, no retry loop. A paired secondary gets an adapter of its own.
Port ownership without any probe of another profile's bridge
(`plugins/platforms/whatsapp/bridge_ownership.py`): an explicit
`platforms.whatsapp.extra.bridge_port` wins; otherwise a secondary takes the
first free port in 3001..3099 and records it in
`platforms/whatsapp/bridge_port` so it is stable across restarts. A
secondary never calls `_kill_port_process`, never adopts a running bridge
and never signals a pidfile process it did not spawn; a bound port or a
live prior process is a fatal for that profile only, naming the port and
the config key. The default profile's startup path is unchanged.
Out-of-process sends (`hermes -p X whatsapp send`) read the same record so
they reach the profile's own bridge.
Relay stays process-level shared ingress; its skip and remedy are untouched.
(cherry picked from commit f96c53df65cff45af2488bd13f22225ce1aec743)
Review and simplify pass on the guest fast-search restore.
- tools/tool_backend_helpers.py: _account_info was split out so two gates
could share one account read. Only managed_nous_tools_enabled is left, so
fold it back into its pre-salvage shape. Keep the branched force_fresh
call, because tests stub the reader with zero-arg lambdas.
- plugins/web/perplexity/provider.py: the managed= parameter on
_managed_gateway only fed the removed refusal helper. search() now calls
resolve_free_search_gateway() directly once it has the route answer. The
module docstring now names guests.
- hermes_cli/nous_account.py: the _error_info docstring described the
removed gate. The tier carried on a failed lookup still picks the "needs
a Nous account" copy over billing or re-login guidance.
- Tests: the case-insensitive tier match and the tier on error snapshots
lost their only coverage when the gate tests went, so cover them at
their consumer, format_nous_portal_entitlement_message. The fast-search
premise now asserts that the guest case really reads as the anonymous
tier. Remove the dead exp_offset parameter.
- Docs: the Perplexity section said there is no anonymous tier, which is
true for direct keys only. It now names the managed guest route.
The free managed Perplexity fast-search route exists for anonymous guest
Portal users. The salvage of #130121 (cee3000c24, via #130660) added a
registered-account gate, fast_search_entitled(), that excluded the
anonymous tier. That defeated the original change: guests fell back to
the keyless ring and never sent a gateway request.
This removes the gate, so resolve_free_search_gateway() decides on
identity alone again, as rewbs wrote it. The guest-disabled, expiry
refresh and dead-guest rules still apply through the token reader. Every
other vendor, and the billed managed-Firecrawl fallback, keep the paid
entitlement gate.
Kept from the salvage: the case-insensitive is_anonymous_tier, the
account tier carried on error snapshots, and the single route walk per
search dispatch. Tests: the anonymous identity is back in the autodetect
and rescue cases. The tests that pinned the guest exclusion are removed.
plugins.isolation is profile config, so a profile (or its agent) can set it
back to in_process. The managed scope already beats profile config and makes
`hermes config set` refuse, and the isolation reader goes through that
overlay; document it as the operator lock and pin the contract with a test
(red when isolation_mode() reads the raw profile file).
Independent review of the plugin host found eight defects; each is fixed here and
the reproducible ones are covered by two new invariant tests (red on the previous
head, green now).
- Async plugin code: `await ctx.<facade>.<async method>` works (the facade probe
reports coroutine methods and the host returns a coroutine function), and ctx
calls made from coroutines keep the caller's session. The wire origin is now a
ContextVar carried onto host event-loop tasks, and the host's fallback context
starts empty instead of copying whichever caller created the host.
- Memory / context-engine / cron providers survive a host crash: proxies record
the host generation, and a category proxy reloads its plugin into the new host
on next use (bounded by the same restart budget). A proxy from an earlier host
can no longer address a recycled object id.
- Hosted provider objects no longer leak: the host imports a category plugin's
module once and drops objects whose Hermes proxies were collected.
- Every public attribute of a hosted object is read live (class-level defaults
included), and attributes Hermes assigns reach the plugin's object unless they
hold a live handle.
- A user memory provider's config_schema.py runs in the plugin host.
- Dashboard plugin APIs resolve the plugin in the requesting profile (its copy,
its plugins.enabled, its host) and keep repeated headers such as Set-Cookie.
- platform_actions coroutines run on the gateway loop their adapters use.
- Each incoming request gets its own thread (no pool exhaustion under nested
callbacks); a late close from a replaced host no longer triggers a reload; the
model-provider extraction cache is keyed by full path.
plugins.isolation: host imports every non-bundled plugin in a host process started
on demand per profile with only that profile's environment, and hands plugins a
RemotePluginContext; Hermes registers proxies for tools, hooks, commands, skills and
provider objects (subclasses of the ABC it checks). Re-entrant: handlers may call
ctx back into Hermes while Hermes waits on them. Every user-code import path routes
to the host (general, memory/context-engine/cron, model-provider profiles via a
cached credential-free extraction, dashboard APIs via an ASGI bridge) or refuses.
One table names what cannot cross (live gateway objects); the static audit shown
by `hermes plugins validate/show` reads it. A host that dies is restarted with its
plugins (bounded). Default stays in_process: nothing changes unless opted in.
Why: shared-pool deployments must never run one tenant's plugin Python in an
interpreter holding another tenant's data, without plugins becoming tenant-aware.
The HEADER.FIELDS list hand-copied the _AUTOMATED_HEADERS keys, so a key
added there would silently miss the preflight and the sender's full body
would be fetched before being dropped. Build it from the registry (the
IMAP command is byte-identical today). The preflight also repeated the
RFC822 payload unwrap and had three identical warn/consume/continue
branches: one _imap_payload helper now serves both unwraps and the
rejections collapse to one branch (gate reuse + quality Low, convergent).
preauthorize=None kept an unsafe path on the shipped adapter: any caller
that left the gate out still RFC822-fetched unauthorized mail. The only
production caller (_check_inbox) always passes it; the None default served
only direct test calls (gate quality Medium + 2c Low, convergent). Make it
required, drop the guard, and have those tests pass an allow-all gate
(their IMAP mocks answer the BODY.PEEK preflight where needed).
The gate hand-rolled run_coroutine_threadsafe(...).result(). On a closed
loop that raised and leaked a never-awaited coroutine (gate reuse Low
finding). The shared helper closes the coroutine and returns None, which
the gate now treats as a rejection, so it still fails closed.
Every sibling signature in the adapter spells the type through the
existing email_lib import; the bare Message import served one annotation
(gate quality Low finding).
The adapter fetched the full RFC822 payload (and decoded attachments) for
every unseen UID before deciding whether the sender was allowed, and
_verify_sender_authentication searched every Authentication-Results field
for one whose authserv-id matched (or was a subdomain of) the pin. A
forged lower field or an attacker-chosen child.<authserv_id> therefore
authenticated a spoofed From:.
- Only the topmost Authentication-Results field is authoritative; a set
authserv_id must match it exactly (RFC 8601 CFWS comments allowed via
_ar_clauses). Main's _NO_AUTH_RESULTS_REASON/_UNTRUSTED_AUTHSERV_REASON
constants are kept so _DROP_HINTS still resolves.
- _check_inbox fetches a bounded BODY.PEEK[HEADER.FIELDS ...] preflight
and runs _sender_accepted on the event loop before requesting RFC822;
rejected UIDs are marked \Seen without downloading the body.
Salvaged from #129772 and rebuilt on main: the PR skipped the preflight
under open access via self._open_access(), which main removed in
c2f595d6af (open access now also authenticates From:), so the gate always
runs here. Poll-loop test mocks now authorize their unauthenticated test
senders instead of forcing open access.
Gate BR3 r1 reuse (Low): build_update_processor hand-rolled the int-or-
default parse that gateway.platforms._shared.coerce_port already does.
Neither caught OverflowError, so `max_concurrent_updates: .inf` in YAML
aborted connect(). coerce_port now treats non-finite values as unparseable
(as gateway.config._coerce_num already does), which also hardens its other
port/int callers; the telegram setting warns and falls back to 32.
Gate BR3 r1 quality (Low): PTB's SimpleUpdateProcessor already provides the
no-op initialize/shutdown the class copied, and config.extra is always a
dict, so the `(extra or {})` guard was dead. The wiring test only compared
PTB's concurrent_updates property with itself (true for any processor);
assert the PerChatUpdateProcessor type and DEFAULT_MAX_CONCURRENT_UPDATES
instead - now red when a plain SimpleUpdateProcessor(32) is wired in.
PerChatUpdateProcessor awaited the predecessor's tail future directly, so
cancelling a waiting same-chat update also cancelled that future (gate
BR3 r1 2c + quality, Medium). The running predecessor then raised
InvalidStateError from done.set_result() after its handler succeeded, and
the next waiter started while the predecessor still ran (probe order:
start A, start C). Shield the await and, for a cancelled waiter, release
its turn only once its predecessor finishes.
Plain PTB concurrency (concurrent_updates=32) unblocks other chats while one
chat's handler is slow, but also lets that chat's NEXT update dispatch and
deliver before the first one finishes (download/vision/callback work happens
inline, before text batching can merge them).
PerChatUpdateProcessor (update_admission.py, not the 7k-line adapter) chains
each update behind the previous update of the same chat while keeping the
bounded cross-chat pool. The builder keeps the instance, so the connect-retry
rebuild in _initialize_app_with_retries reuses it. An invalid
max_concurrent_updates now logs a warning instead of silently falling back
(as #74652 did), and the dispatch-stall watchdog comment no longer claims PTB
dispatches sequentially.
Co-authored-by: fortona-space <299084105+fortona-space@users.noreply.github.com>
Gate finding (quality, Low): _IngressDispatchStallError had no consumer -
every branch checks isinstance(error, _PollingStallError) - so the subclass
and its docstring were dead weight. One stall was also announced three times
(the "healthy but deaf" pre-log, the handoff warning, the fatal error line);
the sibling _check_polling_stall deliberately skips a pre-log. Drop the
subclass, drop the pre-log and carry the "healthy but deaf" counts in the
error text instead, and read the polling generation once.
Tests assert on the error handed to the stubbed handoff rather than caplog.
Gate finding (2c, Low): PTB dispatches updates sequentially (no
concurrent_updates) and counts a dispatch when the handler starts, so one
slow-but-bounded handler (sticker vision_analyze at 120s+, an awaited
slash-confirm callback) plus a queued update looked like "no dispatch
progress" after two 90s heartbeats and triggered a fatal rebuild that drops
already-acknowledged updates.
Raise _INGRESS_DISPATCH_STALL_HEARTBEATS to 4 (360s) so the window exceeds
_POLLING_ERROR_TASK_STUCK_TIMEOUT (300s) with margin. Deliberately not gated
on _inflight_update_ids: that would hide a handler that is itself wedged.
Tests drive heartbeats by count; counts updated, semantics unchanged, plus a
270s-no-escalation assertion.
The confirmed-stall fatal branch hardcoded "getUpdates made no progress",
which is wrong for _IngressDispatchStallError: there getUpdates IS
progressing and PTB's dispatcher is the wedged part. Operators reading
gateway_state.json / logs were pointed at the transport. Interpolate the
(redacted) error text instead; the existing polling-stall errors already
say "getUpdates made no progress ...", so their message is unchanged in
substance.
_check_ingress_dispatch_stall is only called from the async polling
heartbeat, where a running loop always exists; the try/except RuntimeError
and the debounce reset that made its deferred retry work existed only for a
sync unit-test call. Match the sibling _check_polling_stall (no guard) and
delete the no-loop test. Without the fallback the counter stays at the
threshold until dispatch progresses, so a stall is escalated once.
The ingress dispatch watchdog logged 'healthy but deaf' but never drove
recovery, leaving the gateway reporting connected while PTB's dispatcher
stopped draining its queue (#130407 §1).
Hand the stalled adapter to the supervisor for a rebuild via a new
_IngressDispatchStallError (_PollingStallError subclass, so the ladder
skips the in-place Updater restart that keeps the wedged dispatcher and
goes retryable-fatal). _schedule_polling_recovery also marks the adapter
degraded so gateway_state.json stops saying connected. Guarded by the
recovery-in-flight check like the polling stall watchdog; no-loop direct
calls keep the warning as the diagnostic.
Tests: new test_telegram_ingress_dispatch_recovery.py (8 tests: debounce,
scheduling, degraded+fatal handoff, in-flight/webhook/teardown guards,
re-arm, no-loop fallback); existing delivery-gap counting tests stub
recovery to isolate the log contract.
(cherry picked from commit 7767ed38e4f4508b67fc4e2bf3dc726095243732)
Quality-gate Lows (BS03 r1): resolved_count always equalled
len(self._username_resolved_ids) (each name is discarded from to_resolve on
its first match, so it adds exactly one ID), and resolved_allowlist_user_ids
filtered isdigit()/"*" out of a set that only ever holds str(member.id) and is
always initialised in __init__ and reset by the resolver. Drop the counter, the
filter, the getattr fallback (no test builds the adapter without the resolver
and then calls the accessor) and the stale "Numeric only" docstring line.
Behaviour unchanged (probe-BS03-1 output identical before and after).
_resolve_allowed_usernames reset _username_resolved_ids only after the
early return for an empty allowlist / missing client, so a reconnect
after the allowlist was emptied kept the previous connect's resolved IDs
in the set the gateway authz union reads. Reset first so the set always
reflects the current connect.
