Gate finding BN10.r1.2c (Low): the inline re-entrant branch appended the
nested ctx to the outer load's scope and never removed it, so a nested
load that had already finished (and was recorded enabled under its own
key) got fenced by _abandon_load() when the outer load later timed out.
Keep the ctx in scope only while its fn() runs.
The kept nested-inline test now times the outer load out with one finished
and one hung nested load, asserting only outer + hung are abandoned; this
also pins the nested-abandon behaviour of the previous commit.
Gate finding BN10.r1.2c (High): the per-flow _loading check only stopped a
nested walk from waiting on its own parent key. A register() that re-walks
the registry on its deadline worker could still pop a SIBLING deferred
loader, whose _loader then blocks on PluginManager._discovery_lock held by
the outer walk while it joins that same worker -> PluginLoadTimeout and the
outer plugin is abandoned. A flow already inside a loader now returns
before popping a new loader; the outer walk resolves the siblings itself.
The registry test's loaders now hold a shared RLock like the real _loader,
with two deferred platforms, so it pins the sibling case (red without fix).
Nested loads now run inline on the outer deadline worker, so their own
deadline branch never runs: if the outer load timed out, only the outer
PluginContext was abandoned and a nested context stayed live for late
registrations from the stranded worker. Hold the outermost load's contexts
in the ContextVar and abandon all of them on timeout.
Co-authored-by: JerryLiu369 <953639086@qq.com>
The marker test passes counterfactually (any worker-local flag satisfies it);
test_nested_plugin_load_runs_inline_on_deadline_worker covers the headline
behaviour and goes red on BASE.
A plugin's register() runs on a plugin-load deadline worker. When it re-walks
the platform registry (i18n/YAML layering -> plugin_entries()), _resolve sees
its own parent's in-flight loader, but ownership was keyed on
threading.get_ident() -- the parent's thread, not the worker -- so the nested
walk event.wait()ed on itself until the deadline abandoned the load (#130575).
Record in-flight load keys in a per-registry ContextVar instead. Deadline
workers start from copy_context(), so a nested walk on the parent's worker
takes the existing "recursively requested while loading" return, while
unrelated concurrent loaders (fresh contexts) still wait on the event.
Last templated summarizer that ignored the payload outcome: a delegate_task tool_error
(unknown action, spawn failure) compressed into the same stub as a successful delegation.
The two parametrized invariants pinned the same contract as
TestSummarizeToolResultSkillTools (stub carries " FAILED: <error>", stays
one line) in a separate file. Move them next to it as
TestSummarizeToolResultOutcome and delete the new file.
Every row now asserts the exact stub. The old substring check let the
cronjob "create" row pass with reason="" (always true).
Gate findings BN03b r1 reuse + quality (Low).
_skill_result_failure_suffix is now the outcome probe for every templated,
browser, web, vision, cron, process and unmapped-tool stub, but its name and
docstring still said "skill", which suggests skill-specific logic. Rename it
to _result_failure_suffix (all callers are in this module; no test imports it).
Split out the dict-taking _payload_failure_suffix so _sum_cronjob_manage and
_sum_process_manage parse the result once instead of twice (a process poll
can carry a large output), and build the exit-code suffix through
_failure_suffix like every other branch. No behaviour change.
Gate findings BN03b r1 quality + efficiency (Low).
kill_process records exit_code -15 with completion_reason "killed", so a
later poll of a process the agent stopped on purpose compressed into
"[process] poll ... FAILED: exit code -15". A manual cron run the scheduler
was already firing reports execution_skipped, and was stubbed as
"FAILED: Already being fired ...", although the job may be running right
now. After compaction the agent read both as failures.
Skip the exit-code suffix when completion_reason is "killed", and mark
execution_skipped as " SKIPPED: <reason>" (_failure_suffix takes the label);
execution_error and execution_success=False still mark FAILED.
Gate findings BN03b r1 2c + quality (Low, convergent).
The outcome suffix stopped at _sum_template, so the rest of the same bug
class still compressed a failed call into a stub that reads like a
success: web_extract {"error": "403 Forbidden"}, a failed browser_* call,
a refused vision_analyze, and every unmapped tool. The unmapped fallback
covers every MCP and plugin tool, so it was the widest remaining gap.
Append the same top-level outcome suffix in _sum_web_extract, _sum_browser,
the vision_analyze summarizer and the generic fallback in
_summarize_tool_result_unguarded. Success payloads and non-JSON content
give the same stub as before.
Gate finding BN03b r1 2c (Medium).
The salvaged file carried 17 tests across four classes. Keep two
parametrized invariants: every failing templated/cron/process call is
marked FAILED in its stub, and healthy calls (including a cron job with a
stale stored error, a clean exit and a still-running poll) stay unmarked.
The patch_tool and skill_manage cases cover other summarizers already
pinned in test_context_compressor.py.
A manual cronjob run reports a dead or skipped job inside the job view while the tool
call itself succeeded, so the payload is a clean {"success": true, "job": {...}} and the
top-level outcome probe finds nothing to mark. A poll of a finished process reports
exit_code and completion_reason instead of an error, so a non-zero exit is invisible the
same way. Both compressed into the byte-identical success stub, which is the failure this
series removes.
Each tool now reads its own payload: the cronjob stub looks at this call's
execution_error / execution_skipped / execution_success and at nothing else, because the
job view also carries state left by earlier runs; the process stub marks a non-zero
exit code, while a process still running reports none and stays unmarked.
(cherry picked from commit 295e23e3bfc084f20844bf518a244d41421d3f66)
_sum_template formatted from the tool arguments and the result's length only, so every
tool routed through it dropped its outcome. A refused read, a rate-limited search, a
rejected cronjob, an unknown process session and a tts call with no voice all compressed
into the same stub as the success they never were, and the post-compaction agent went on
to report the success (#131244).
Append the failure suffix the skill summarizers already use, so the templated family keeps
its outcome from the one helper that already knows how to read it. Stubs for calls that
report no failure are unchanged.
Complementary to #131247, which gives write_file and patch summarizers of their own.
(cherry picked from commit 6302f87ff52c10db2b43521b733914748b4bcca2)
Gate finding (quality, Low): test_turn_settles_before_post_turn_trim
repeated seven of turn_env's stubs line by line only because turn_env also
swaps threading.Thread for an inline runner and this test needs a real
worker thread. turn_stubs now holds the stubs; turn_env = turn_stubs + the
Thread patch; the settle test uses turn_stubs.
Gate finding (2c, Low): _post_turn_housekeeping ran as the last statement
of the settle block, so a raising _emit_settled_session_info (its
_session_info reads config unguarded) skipped the memory trim and left the
profile HERMES_HOME override bound on the worker thread. Wrap the settle
block in try/finally so the housekeeping always runs; the audio teardown
already happens before settlement.
The settle-before-trim test gains a row where _emit_settled_session_info
raises and still requires the trim to run.
Gate finding (2c + quality, convergent): the settle-before-trim split moved
the thinking-sound stop and the TTS end-of-text sentinel after
session["running"] = False. Both are non-blocking (a lock + Event.set; an
unbounded queue put), so deferring them gained nothing, and it opened a
window where a prompt admitted on the same session during the trim had its
thinking sound stopped by the old thread and the old stream's final chunk
cut as a barge-in. They run in _release_turn_scopes again, before
settlement; _post_turn_housekeeping keeps only the trim and the
HERMES_HOME override reset. (The original commit subject says "trim/TTS";
only the trim is deferred now.)
The settle-before-trim test now also asserts both audio teardown steps ran
while the session was still running.
The prompt-turn finally ran trim_memory (gc.collect + malloc_trim, 20-50 s
on multi-GB heaps), the thinking-sound stop and the TTS sentinel BEFORE
marking the session idle and logging the "tui turn finished" bookend. A
slow or blocked trim therefore kept the session busy and the bookend
missing for its whole duration.
Split _finish_turn into _release_turn_scopes (snapshots, one-turn model
restore, approval/secret/terminal/session scopes — unchanged order and
logging) and _post_turn_housekeeping (trim, thinking sound, TTS sentinel,
then the HERMES_HOME override reset so the trim still reads the session
profile's config, #58576). The settle block runs between them verbatim.
Refs #131740 (the reported 600 s watchdog hang is inside
agent.run_conversation, so this hardening does not close it).
Salvaged from #131773.
Gate finding (BN07 quality #3): the comment above _IMPORT_FLAG_COLS restated the
test module docstring (and called the sweep's auto_archived a user decision); cut
it to one WHY line. The donor-content and retry-adoption assertions exercise
adopt_session_lineage_from behaviour that already holds on main (verified: they
pass against the merge-base prod file), so they add no teeth for this fix.
Gate finding (BN07 quality #2): auto_archived was imported independently of
archived, so a payload with archived=0, auto_archived=1 stored a live row marked
as swept, a state the idle sweep never produces (it only sets the flag on rows
it archives). Clear it unless the row is archived.
Gate finding (BN07 quality #1): the session flags were read by truthiness, so a
hand-edited export carrying the string "0" imported as pinned/hidden/archived = 1.
The same function already coerces message flags with _coerce_or(..., int, 0);
use it for the session flags too.
A legacy export without flag columns must still import as an ordinary
visible session; the adopted canonical Bot Chat must keep its rename guard;
the donor must stay untouched; and a retried adoption must skip the
already-imported row instead of restoring the donor retirement archive
over the local copy. Extends the two existing tests instead of adding a
duplicate file.
Co-authored-by: itsflownium <157689911+itsflownium@users.noreply.github.com>
import_sessions copied archived but not auto_archived, so a chat the
idle sweep had archived came back as a deliberate archive. Resume and
compression only un-hide sweep archives (_unarchive_auto_archived_lineage),
so after a restore the chat stayed hidden for good.
(cherry picked from commit 130aa6f3cddee06238dd46f9dbc50eb8acef552f)
Stranded-session adoption copies a profile bot's chat from the default store
through import_sessions, which dropped hidden. Bot Mode chats are hidden, and
hidden is what makes the canonical Bot Chat canonical: the adopted copy showed
in every session listing, lost the rename guard on its registry title and
became a stale-archive candidate. hidden joins archived and pinned as a
restored flag.
(cherry picked from commit 1deb08a9cfd08845bc1de25cc502c3bcb51e8569)
import_sessions restored archived but not pinned, so a session restored from
a backup (dashboard import of `hermes sessions export`) or adopted from the
default store came back unpinned. Pinned is what the startup prune and the
stale-archive sweep exempt: an old pinned session was deleted by the next
startup after the restore.
Ported onto the split import helpers from #76206.
(cherry picked from commit 59ea00ea713f90a1aa0487e05de79e6690be0bdf)
The legacy AUTHOR_MAP already maps this email, but the per-email file the
release tooling now reads was missing; add it so the cherry-picked commit
is credited.
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).
Quality-gate Low (BS03 r1): test_filters_usernames_and_wildcard seeded
_username_resolved_ids with "teknium" and "*", values the resolver can never
write there, so it only pinned a defensive filter against impossible state.
Fold it into the kept accessor test, which now drives the real
_resolve_allowed_usernames (wildcard, unresolved username and numeric entry
in the allowlist) and asserts only the username-resolved ID comes back. It
fails on the pre-salvage accessor, which leaked the numeric entry.
Also drop the redundant _username_resolved_ids reset in the helper (the
resolver resets it first), hoist `import asyncio` to the module imports and
restore PEP 8 two-blank-line spacing between top-level definitions.
Keep the nickname/global-name impostor case (parametrized) and the
revoked-numeric-entry-after-env-reload case; both go red on the pre-fix
adapter. The display-name hint and numeric-only snapshot tests cover
the same code paths as the kept ones.
_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.
resolved_allowlist_user_ids() returned every numeric ID in the adapter's
connect-time _allowed_user_ids, and the gateway authz layer unions that set
into the reloaded DISCORD_ALLOWED_USERS. So an ID removed from the allowlist
after connect (hermes pairing revoke from the CLI or dashboard, or a hand
edit) kept authorizing until the gateway restarted. #130920's in-place purge
only reaches adapters in the same process, and those commands run outside it.
Track the IDs _resolve_allowed_usernames actually resolved from username
entries and return only those: they are what the per-turn .env reload loses
(a0a63a1bc2); numeric entries are already read live from the env.
(cherry picked from commit 90451505545745dbb27ecad28f2c431ad5afd301)
_resolve_allowed_usernames resolved a DISCORD_ALLOWED_USERS name through member.name,
member.display_name or member.global_name, and the first match took the slot. Any member of a
shared server could set a nickname or display name to an allowlisted username, get the
resolved ID (DMs, tools, approval buttons) and lock the real user out.
Match the unique account username only, as #125034 did for SimpleX display names. An entry
that matches only a display name stays unresolved and gets a connect-time hint. Tests cover
a copied nickname and a copied display name seen before the real user, and the hint; the
Discord docs now say display names and nicknames are never matched.
(cherry picked from commit 252f0bccab39afe94f0f71254e47a8f785ec1fb0)
The channel gate ran inside _handle_slash_command, after the bolt listener had
already acked with an ephemeral "Running /x". In an ignored or non-allowed
channel the invoker was promised a run that never came, and an ignored channel
("never touched") still got a bot reply. Move both checks into
_slash_channel_gated() and consult it before the ack: gated -> bare ack().
_handle_slash_command keeps calling the same predicate for direct callers.
The listener becomes the _handle_hermes_command method so the existing
parametrized gate test drives the real ack path (BN11 gate r1 2c Low finding).
The allowed_channels guard recomputed str(channel_id).startswith("D") inline and
the next line computed it again as is_dm. Hoist is_dm above the gates and reuse it.
Same behaviour, one DM classification (BN11 gate r1 quality/reuse Low finding).
The message path drops a message from an ignored channel (_prefilter_inbound)
and, when allowed_channels is set, from any non-1:1 conversation outside it
(_channel_gate_allows). _handle_slash_command applied neither, so a slash
command ran in a channel the operator had excluded. Apply both gates by
channel id alone, before any lookup, so a failed conversations.info can never
drop a control command.
Rebuilt from 2b7cde506c (#128767). Fixes#130358.
Gate finding (BN06 r1 quality, Low): the --session-id path never reads the
export filters, so this block passes on the merge base too (probe-BN06-3) and
pins behaviour the stack does not change.
Gate finding (BN06 r1 quality, Low): the subset check rebuilt candidates with
the same list_prune_candidates kwargs _cmd_export hard-codes, so it would still
pass if prod and test both dropped include_pinned. Assert the exported ids
directly; keep the pinned/archived flag asserts.
Gate finding (BN06 r1 quality, Low): the missing-id check lived in three
places (_collect_sessions, _export_trace, _export_markdown_single), and
_collect_sessions had grown a list|int|None return with an isinstance
branch in _export_flat just to carry the exit code. Resolve the id once at
the top of _cmd_export and return _not_found's non-zero code there; the
per-format copies and the int return are now dead and reverted.
The salvaged #131075 fix makes filtered `hermes sessions export` include
pinned rows (archived were already included), so a backup taken before
`prune --include-pinned` holds every row prune can delete. Surface that in
the export --older-than help so users do not assume prune's keep rules
also narrow the backup.
Co-authored-by: Alisson Galindo <alissongalindo@users.noreply.github.com>
Co-authored-by: Yuan Li <dskwelmcy@163.com>
The stranger[0] membership assert was circular (it re-found an element
taken from the same list) and raised IndexError instead of a useful
message when the Timer did nothing. Assert the list is non-empty and
shutdown-only in one line. The trailing time.sleep(0.2) did no work:
every stranger event is recorded before woken.set(), and the owner's
close events happen before pytest.raises exits.
_attempt_stream_socket re-implemented a narrow slice of the existing
_socket_from_stream helper (get_extra_info only) behind a blanket
try/except. Delegating to the shared helper keeps one socket-discovery
path (it already suppresses get_extra_info failures and covers the
_sock / anyio backends), and the remaining lookup cannot raise.
Also trims the close_attempt_stream docstring to the essential why.
_shutdown_socket already swallows OSError itself (agent/agent_runtime_helpers.py:
"except OSError: pass"), so the extra try/except in close_attempt_stream only hid
programming errors behind a debug log.
Closes#130115.
Co-authored-by: Yuan Li <dskwelmcy@163.com>
_CodexStreamGuard._close_client_on_timeout keeps the FD-ownership contract for the client's pool sockets
(shutdown() from the Timer, close() only on the owning thread), but then called close_attempt_stream(),
and an SDK Stream.close() on an unfinished response ends in sock.close() (httpx Response.close ->
httpcore HTTP11Connection.close -> SyncStream.close). That releases the descriptor while the owner is
still inside SSL_read: the kernel hands the number to the next open() and OpenSSL writes a TLS alert
record into that file. Two SQLite files in a gateway process lost bytes 5..28 of their header this way
(b"SQLit" + 17 03 03 00 13 ...), 15 ms after "Codex auxiliary client aborted (timeout,
tcp_force_closed=1, deferred_close=stranger_thread)".
close_attempt_stream() now does a real close() only on the owning thread. From any other thread it
shuts down the socket under the stream (stream.response.extensions["network_stream"]), which wakes the
reader and keeps the FD; the owner closes the stream in its finally (release_stream). A stream with no
reachable socket (a test double, a non-httpx transport) is closed as before.
Fixes#130115. Follow-up to #70773.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 33cfb5ea6550f4a56ea0faa072ddf7f91412af49)
The header said the duplicate paints "on current main" (stale once the
fix lands) and pointed at boundary "specs below" when only the
message.start spec remains. Also apply prettier to the delta helper,
which --check flagged.
