The room-level test is the reported defect: two composer sends, two
threads, and neither backend transcript may contain the other thread's
prompt. Proven red on the unfixed code — both threads resolved to
sid-research-1.
Also pins same-thread continuity, the pre-thread adoption (first thread
continues the old conversation, later threads do not), and that the
member half of the key stays source-qualified so a remote `research` and
a local `research` never share a session.
Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
The co-keyed readers of `room.sessions` had to follow session identity or
they would split from it: the stop interrupt targeted a member's only
session regardless of which thread issued the stop, and the stranded-reply
harvest resumed by bare member key even though its marker already carries
`{before, thread}`.
The clarify mirror moves too — with per-thread sessions one member can be
blocked in two threads at once, and a room-and-member key let thread B's
question silently replace thread A's card. The sweep's owner lookup stays
a MEMBER question, so it reads the member half of the key and keeps the
`::` source-qualifier check on that half alone.
Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
A group member's hidden plumbing session was keyed by member alone, so
every thread in a room collapsed onto one backend transcript: thread B
resumed thread A's session and answered carrying A's context.
Everything else in the room engine is already thread-aware — the delta is
filtered by `groupThreadOf(e) === thread` and the watermark is
`${thread}::${memberKey}` — so only session identity was missing. Key and
title it per thread, and adopt a room's pre-thread pointer into the first
thread that speaks so an upgraded room's history is continued rather than
orphaned.
Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
The website key-features bullet (EN + zh-Hans) omitted /reset from the
retry points while the README lists it; also drops a local import in the
test file shadowed by the module-level one.
- none-returning client counts as success (sentinel contract)
- pending buffer drops oldest past the turn cap and the byte cap
- the shutdown interleaving test now pre-seeds a pending turn so the
no-resend assert discriminates on content, not timing: with the lock
removed it fails on assert 2 == 1 (duplicate send), verified by mutation
Two review findings on the salvage stack:
- _write_turns' _quietly consolidation keyed failure on a None result,
implicitly assuming add_memory never legitimately returns None. A None-
returning stub (the most common mock idiom) would mark every successful
write failed and re-append the batch forever. Module-level _FAILED
sentinel: only a raised exception re-queues.
- _pending_turns had no bound: a persistently failing service accumulated
one entry per turn for the process lifetime (gateway runs never
re-initialize), and every retry re-sent the whole accumulated payload —
O(n^2) upload bytes, and a size-rejected batch could never shrink. Cap
the buffer (50 turns / 256 KiB, drop oldest, warn once per trim).
Answers the open review ask on #109359 with the corrected contract: a hung
remote add_memory is bounded by the SDK timeout (empirically 1.03s at
timeout=1.0, max_retries=0), so shutdown's flush waiting on the capture
lock is bounded, not forever. The guard: while the worker owns the write
(blocked inside add_memory), a concurrent shutdown must wait and must not
re-send the same pending batch. Mutation-checked: lock -> nullcontext
makes this test and the session-switch interleaving test fail.
Eight tests compare a recorded custom_id against a freshly computed
_capture_custom_id; near a 4h-bucket edge the provider's write and the
test's expectation read now() in different buckets and the equality
fails. The frozen_capture_clock fixture pins the module datetime so both
reads are identical by construction.
The per-turn capture rewrite removed the raw urllib /v4/conversations
ingest, but stale references survived outside the diff hunks:
- README "Behavior" still carried the "written once via the conversations
endpoint" paragraph contradicted by the new bullets right above it.
- website memory-providers (EN + zh-Hans) still listed full-session ingest,
session-end /v4/conversations ingest, and ingest in the base-url and
api_timeout rows — the PR had updated one line per file but missed the
rest of the section.
Now every surface describes per-turn documents.add capture with retry.
Two cleanups on the salvaged per-turn capture path (review of #109359):
- _write_turns hand-rolled the try/except/log that the file's own _quietly
helper already provides (same exception class, level, exc_info shape);
the deleted _ingest path used _quietly for the identical call. Consolidate:
add_memory always returns a dict, so a None result is the failure sentinel.
- _capture_custom_id's `or 'hermes'` never fires: _sanitize_tag already
returns _DEFAULT_CONTAINER_TAG on empty input, and the literal duplicated
the constant the helper owns. Verified behavior-preserving (tests green
with the fallback artificially restored).
sync_turn runs on the MemoryManager worker, but on_session_switch and
shutdown run on the caller thread, so two _write_turns() calls could
snapshot the same pending batch and each replace the whole list (duplicate
append or lost pending turn). A capture lock now covers the
snapshot/write/replace sequence; _write_turns() reads pending turns inside
the lock instead of taking a caller-built list.
Retries are at-least-once: the documents API appends on a shared custom_id
and does not dedupe by content, so a write it accepted but whose response
was lost is appended again. Stated in the docstring and README instead of
implied.
Addresses review on #109359.
A failed flush at on_session_switch() restored the pending buffer and
then cleared it on the next line, so an unavailable service at the switch
boundary still lost every pending turn. Pending turns now carry their own
session_id; _write_turns() batches per session, so a later retry (next
turn, session end, shutdown) writes old-session turns under the old
session's custom_id even after the switch.
Addresses review on #109359.
sync_turn now writes each completed turn through the SDK's documents.add,
keyed by custom_id "<session>_<date>_b<0-5>" so all turns of a session in
one 4-hour window append to a single document. This matches the capture
shape of the other Supermemory agent integrations and removes the raw
urllib POST to /v4/conversations, which the self-hosted server does not
implement (#101270).
Failed turn writes stay pending and are retried with the next turn, at
session end, on session switch, and at shutdown. Previously a failed
session-end ingest was logged once and the whole session was lost.
Inline base64 data URIs in captured text are replaced with "[image]" so
pasted screenshots no longer land in the document as megabytes of text.
Metadata stays type/session_id/timestamp plus the existing sm_source.
Answers the P1 review on #111187: build_profile_secret_scope() held only
<profile>/.env plus that profile's external-source snapshot, never the
administrator-managed .env. The launch process applies that file LAST with
override (_apply_managed_env), so a managed key beats the user's own value in
os.environ. Under multiplex semantics get_secret() stops falling back to
os.environ on a scope miss, so inside a routed cron fire (and equally inside a
real multiplex gateway turn, which builds its scope through the same function
via gateway/run.py::_load_profile_secret_scope) a managed-only credential
resolved as absent and a managed-vs-user collision resolved to the USER value:
reversed precedence.
Fix at the source: build_profile_secret_scope() overlays load_managed_env()
last, after the profile .env and external sources, skipping process-global
names exactly as it does for the other two layers. Every multiplex-authoritative
scope (gateway turn, routed desktop fire, external worker env build) is built
here, so managed authority is composed once instead of restored per consumer.
No generic ambient-env fallback is reintroduced: only the managed file's own
keys enter the scope, and only with the managed file's values.
Regression (parametrized, two invariants): inside a routed fire a managed-only
key resolves through get_secret(); a managed-vs-user collision yields the
managed value.
_launch_external_cron_worker hand-rolled the same hydrate -> set_secret_scope ->
set_multiplex_context(routed) sequence, and the same context-then-scope reset, that
_install_fire_secret_scope/_reset_fire_secret_scope already encode for the in-process
fire. Two copies of the ordering is how the two paths drift apart; the helper is the
one place that owns it. The only observable difference (re-setting an already-active
multiplex context for the span) is a no-op the token reset undoes.
Rename _scope_token in _run_one_job_body to _fire_scope_tokens: it has held the helper's
(scope, context) tuple since the routed-fire change, not a single token.
Widening secret_source_names() to include skipped_existing names silently changed
tools/mcp_tool_config.py::_build_safe_env, an untouched consumer that forwards every
returned name into MCP stdio child envs. That consumer wants only names a source
actually APPLIED (pre-stack semantics), so secret_source_names() goes back to
tuple(_SECRET_SOURCES).
The routed-child scrub in strip_launch_profile_env is the one site that must also see
names a source supplied but lost to a pre-existing process value, so it reads the new
source_supplied_names() accessor instead. tools/mcp_tool_config.py is byte-identical to
origin/main.
_hydrate_profile_secret_sources and _apply_external_secret_sources each
rebuilt the same "applied + skipped_existing" set and pushed it into
_SOURCE_SUPPLIED_NAMES; the routed-child scrub depends on both sites
agreeing, so give them one helper.
refresh_installed_secret_scope cleared the installed scope before
refilling it, which left a window where a concurrent reader of the same
fire saw no credentials at all. Update first, then pop the names the
rebuild no longer supplies; stale values still disappear.
_launch_external_cron_worker wrapped fifty lines of dispatch, payload
write and scope hydration in the routed-fire multiplex context, though
only build_subprocess_env / strip_launch_profile_env read it. Compute
`multiplex_active` once (process flag OR routed fire), serialize it into
the payload, and set the context for exactly the env build inside the
existing secret-scope try/finally. Drop restore_managed_env on this path:
the worker re-runs load_hermes_dotenv -> _apply_managed_env at import
and strip_launch_profile_env already leaves managed keys in place.
_run_job_script now overlays the installed scope and re-applies managed
keys only under multiplex. In a single-profile process the scope is
os.environ, so the overlay could only re-sanitize values the child
already inherits; the "no-op outside multiplex" comment is now literal.
_run_job_script popped the launch profile's secret-source names in an
inline loop right above strip_launch_profile_env, so only the no_agent
child got that protection; the four other callers of the same helper
(the external cron worker, scheduler_delivery, kanban dispatch and the
byterover plugin) still handed a served profile the launch vault or
1Password names. Fold the names into the helper's residue set, which is
already gated on multiplex and on the target not being the launch
profile, and keeps administrator-managed keys.
The earlier trim dropped the only test of the `routed_profile_fire() and
not is_multiplex_active()` branch in the external-worker handoff: the
process flag is OFF (a desktop tick is not a multiplexer) yet a fire routed
to a sibling profile must serialize `multiplex_active=True` and hand the
worker an env without the launch profile's residue, and the context must
not outlive the handoff span. Without it that branch could regress to the
pre-fix behaviour unnoticed.
The PR shipped nineteen tests across six files, most of them variations of
one boundary. Keep the three that pin distinct behaviour:
- a routed desktop-ticker fire runs under multiplex semantics for exactly
its scope: a scope miss returns None instead of the launch credential and
the parent os.environ is byte-identical afterwards;
- a routed no_agent child never sees a launch-only name, whether the launch
.env defined it or a launch external source supplied it (applied or lost
to a pre-existing process value), while its own values come through;
- administrator-managed keys keep policy precedence over the routed
profile's own value.
Everything else was either a positive control of the same seam, a
set-membership check on a module-level constant, or a re-statement through
a different entry point.
The `_inner` wrapper existed only to set and reset the multiplex contextvar
around the whole launch. Every consumer of that context (the payload flag,
`hydrate_profile_secret_sources`, `strip_launch_profile_env`, the scrubbed
`build_subprocess_env`) runs before the worker is spawned, so the span now
covers exactly the handoff-environment build and ends before `Popen` and the
acknowledgement wait. One function, one try/finally, same behaviour.
Review findings on f5f88d5058. Three are defects the previous round introduced.
Managed keys were stripped as launch residue. Recording every dotenv load as
residue swept in the administrator-managed `.env`, which `_apply_managed_env`
applies LAST with override precisely so it beats the user's own `.env`. A
routed child then lost `ORG_POLICY_FLAG=managed-value` to the routed user's
`user-value`. Managed keys are now recorded separately, never enter the
residue set, and are re-applied over the routed scope in both child builders
(`scheduler_script`, the restart-safe handoff) so the child sees the same
precedence the launch process does. `kanban_db_dispatch` and
`scheduler_delivery` strip without any overlay, so for them the exclusion
alone is the guarantee; the test pins the case that exercises it — the same
key defined in both the user and the managed file.
Private hydration did not record supplied names. `_hydrate_profile_secret_sources`
now feeds `provenance` plus `skipped_existing` into the same ownership set the
process-global path uses; the provenance label map stays applied-only.
Removal cleanup cleared its marker before the fallible work. A raising
reload left the removed plugin's credential active with no retry, because the
next no-source discovery saw the flag already false. The marker is cleared
only after reset, reload and installed-scope refresh succeed.
Routed fire not multiplexed at the handoff. `run_one_job` enables the
context in `_install_fire_secret_scope`, which runs AFTER
`_launch_external_cron_worker`, so a routed desktop fire on the managed path
serialized `multiplex_active=False` and built the worker env with launch
residue and no scrub. The handoff now treats `routed_profile_fire()` as
multiplexed for exactly its own span; the worker re-establishes the state from
the payload as before.
Each fix was checked by reverting it and confirming its regression fails,
including the overlay half and the exclusion half of the managed fix
separately.
(cherry picked from commit 329cbd8963d68c45b425e95a5b11ade59f513960)
Review findings on d8c467f223, each reproduced through its production path.
Stale launch key. `strip_launch_profile_env` built its residue set from a
re-parse of the launch `.env`. A key removed or renamed in that file after
boot is still in `os.environ` with the old value (dotenv never unsets), and
the current file no longer names it, so it survived into the routed child.
`_load_dotenv_with_fallback` — the one chokepoint every dotenv load goes
through — now records the KEY names it put into the process env, additive for
the process lifetime (`launch_dotenv_keys()`), and the strip unions that record
with the current file.
Source name that lost to the process env. `_apply_external_secret_sources`
snapshots every name a source SUPPLIED (`provenance` + `skipped_existing`),
but `secret_source_names()` only exposed `_SECRET_SOURCES`, which is
provenance metadata and names applied values alone. A launch-profile source
that supplied `CUSTOM_VAULT_SECRET` while the process already had it was
therefore invisible to the scrub, and a routed child with an empty scope got
the launch value. Supplied names are tracked separately
(`_SOURCE_SUPPLIED_NAMES`) so the provenance labels stay honest, and
`secret_source_names()` returns the union.
Last plugin source removed. `_refresh_secret_sources_after_discovery`
returned before the cache reset and the installed-scope refresh whenever no
plugin source was enabled — and `discover_and_load(force=True)` unloads the
old registration first, so removing the final plugin source hit exactly that
return with the removed plugin's names still in the per-home snapshot and the
current scope. The manager now remembers that a discovery re-applied plugin
sources and, on the next discovery that finds none, reconciles once. A home
that never had a plugin source is still a no-op (pinned by the existing tests).
Regressions: the stale-key lifecycle and the skipped-existing case through
`_run_job_script` against a real routed child, and the removal case through
the manager. Each checked by reverting its fix and confirming the test fails.
(cherry picked from commit d464f5f6126a394cfb47937f683d3a5e2f141840)
The source-name strip added alongside the external-source fix ran
unconditionally. Outside multiplexing there is no other profile to leak from --
os.environ IS this profile's environment -- so popping those names relied on the
routed scope overlay putting each one back, which in turn relies on the
per-home snapshot recorded at boot. Correct today, but it made a single-profile
child's credentials depend on bookkeeping that has nothing to do with isolation.
Guard it the way strip_launch_profile_env guards itself: no multiplexing, no
strip. A single-profile no_agent child now keeps a byte-identical env even if a
source's snapshot were ever missing. Pinned by a regression that runs a real
child with a source-owned name in os.environ and no multiplex context; making
the strip unconditional fails it.
(cherry picked from commit afa429b30a1c9c9b4c011f7d2426a9f099911596)
Two credential-isolation gaps found in review of the previous head.
1. strip_launch_profile_env() only knows dotenv- and terminal-config-owned names,
but external secret sources (vault, 1Password, ...) also write their names into
the shared os.environ and are tracked in secret_source_names(). A name the LAUNCH
profile's source supplied therefore still reached a routed no_agent child. Drop
every non-global source-owned name from the base; the routed scope overlay that
follows puts back exactly the ones that profile's OWN sources supply, since
build_profile_secret_scope folds get_secret_source_values(home) in.
2. refresh_installed_secret_scope() merged the rebuild with dict.update(), so a
name a source had stopped supplying -- rotated, revoked, source removed -- kept
its old value for the rest of the fire. Replace the mapping contents instead: the
rebuild is the profile's current truth.
Regressions: a routed child sees <unset> for a launch-source name while its own
source value comes through, and a refresh whose rebuild omits a name drops it.
Both fail if the corresponding change is reverted.
(cherry picked from commit ecd51517c4828a75acb5f458ed99aea0bc3e5e9f)
The routed no_agent child env started from all of os.environ and only overwrote the
names present in the installed scope. A name defined only by the LAUNCH profile's .env
and absent from the routed profile therefore reached the routed child with the launch
value instead of unset -- the secret scrub only knows classified names, so a custom or
unclassified secret crossed the profile boundary (review finding on the first head).
strip_launch_profile_env (main, 284d220ba4) is the primitive the external-worker path
already uses for exactly this: it drops the launch profile's dotenv-owned keys and the
bridged TERMINAL_* settings, and is a no-op outside multiplex or when the target IS the
launch profile. Apply it to the base BEFORE the scope overlay (so a shared name keeps its
routed value) and BEFORE the sanitizer (so routed values still pass the scrub and
passthrough rules). Pinned by a child-process negative control: the launch-only name
arrives <unset>, the shared name arrives routed, and the parent process is unchanged.
(cherry picked from commit 69349b527b5144c1f1540a4c10035d5ec798c1db)
The desktop backend ticks EVERY local profile's cron store from one process — its own docstring
says "like a multiplex gateway" (hermes_cli/web_server.py) — but never sets the process-global
multiplex flag, and cannot: its own chat turns are unscoped and would fail closed. Every
isolation in the tree keys on that flag — the guard that keeps a routed `.env` out of the shared
`os.environ`, `get_secret`'s fail-closed miss, passthrough resolution, the MCP and kanban
subprocess scrubs — so all of it was inert for a sibling profile's fire. Verified: a secondary
profile's API keys replaced the launch profile's in `os.environ` with `override=True` and stayed
there after the tick, and a scope miss read the launch profile's tokens (#107692).
Give multiplex mode a context-local counterpart. `set_multiplex_context` (agent/secret_scope.py)
is OR'd into `is_multiplex_active()`. `_profile_cron_scope` only MARKS a fire whose home is not
the process's own (`routed_profile_fire`, decided against `get_process_hermes_home()`, the
override-immune resolver); `_install_fire_secret_scope` in cron/scheduler.py installs the
profile's hydrated secret scope and, for a marked fire, the multiplex context — for exactly that
span, dropped again before the scope by `_reset_fire_secret_scope`. Multiplex semantics are
therefore never active in cron without a scope to read: `run_one_job`'s restart-safe handoff runs
before the body's scope and keeps today's semantics (its own scope is #107413 / #106050's seam,
left untouched so this composes with whichever lands). Every existing multiplex-keyed isolation
applies inside the routed fire with no per-site patching; the launch profile's own fires and the
backend's turns keep single-profile semantics; marker and override both reach the pool worker via
`copy_context()`. `get_secret` read the raw global in its miss branch; it now goes through
`is_multiplex_active()`. The dotenv guard keeps its pinned flag-only form (#77970).
Two consequences of suppressing the write are handled rather than left as regressions:
- a `no_agent` script's env is `os.environ.copy()`, which no longer carries the routed `.env`;
the runner overlays the installed scope onto the base BEFORE sanitizing, so the same scrub /
passthrough rules apply to those values and the parent process is never mutated;
- plugin secret sources are discovered on the fire's first agent build, after the scope froze,
and the post-discovery reload is hydrate-only under multiplex semantics; the refresh now folds
the values into the installed scope in place (`refresh_installed_secret_scope`, the pattern
`_publish_env_value` already uses for `.env` writes under multiplex).
And the profile's external secret sources are hydrated before the scope is frozen, the order
gateway/run.py and the external cron worker already use.
Tests pin each direction: the marker without the semantics before the scope, the semantics on and
off exactly with it, the marker reaching a copy_context worker; the process's own profile staying
single-profile; the restart-safe handoff's child env building without raising under a routed tick
with a passthrough key registered; a real child process receiving the routed values while
`os.environ` keeps the launch value; a source registered after the freeze reaching the fire
through the real PluginManager refresh. Reverting any one direction fails a distinct test.
(cherry picked from commit 2f87677425d2cca19286ac83bc45cab23e546669)
`/*.db-wal` / `/*.db-shm` only match at the checkout root, so on a flat
install `git stash push --include-untracked` still swept
`cron/executions.db-wal` / `-shm` while the scheduler held the WAL-mode
database open (cron.executions._connect opens it via open_db in WAL mode).
The base file stayed put but its WAL vanished under a live writer, so the
next `_connect()` failed with `disk I/O error` (review finding on #111175).
Use `/cron/executions.db*` like the gateway recovery db rule already does,
covering -wal/-shm/-journal and retired-WAL dirs. Every other non-root db
rule in the block already uses the glob. The regression tuple gains the two
sidecars, and the stash test gets the reviewer's repro against the real
`_stash_local_changes_if_needed`: an open WAL connection with one committed
row must still be readable from a fresh connection afterwards.
The flat-install block listed state.db and kanban.db sidecars one by one,
so any other root-level SQLite store (response_store.db, a future ledger)
and its -wal/-shm/-journal sidecars would still be swept by the updater's
`git stash push --include-untracked` and unlinked under the running
gateway (#110648). Replace the per-file lines with root-anchored globs
(`/*.db`, `/*.db-wal`, ...). No tracked root-level *.db exists, and
`git ls-files -ci --exclude-standard` is unchanged before/after, so the
globs newly ignore nothing that is committed.
Also add the rest of the flat-install runtime roots the previous fold
missed: the credential siblings from
gateway/platforms/base.py::_ROOT_CREDENTIAL_PATHS (.anthropic_oauth.json,
google_token.json, google_oauth_pending.json, auth/,
webhook_subscriptions.json), the active pairing location platforms/
(gateway/pairing.py), kanban/, gateway_state.json, processes.json,
cron.pid, the channel directory/alias and feishu pairing stores,
pending_messages/, checkpoints/, plugin-data/, hooks/, and the Discord
message-recovery db under gateway/ (gateway/ itself is tracked, so only
that file pattern is ignored). `/.credentials/` had no producer -- the
real dir is `credentials/` (web_routers/files.py, _ROOT_CREDENTIAL_PATHS)
-- so it is replaced. `/state-snapshots/` is dropped: the existing
unanchored `*-snapshots/` rule already matches it.
The test tuple now carries one representative per ignored class and its
comment no longer claims _ROOT_CREDENTIAL_PATHS enumerates the sidecar
set (that is `_sqlite_files`).
The same `git stash push --include-untracked` sweep that took state.db
on a flat install (#110648) also takes every other untracked file at the
$HERMES_HOME root: config.yaml, auth.json/auth.lock, memories/,
profiles/, .credentials/, mcp-tokens/ and pairing/. Losing those on a
declined or failed restore strands the user's credentials and profile
config just as badly as losing the session store.
Extend the root-anchored block with those paths (none are tracked or
already ignored on main) and append them to the test's
FLAT_INSTALL_RUNTIME_STATE list so the existing stash invariant covers
them without a new test.
The per-profile job store lives at HERMES_HOME/cron/jobs.json, so a
flat install keeps it beside executions.db inside the checkout-root
stash domain. Without an ignore rule the untracked autostash of
hermes update sweeps it away with the rest of the runtime state.
Absorb the path (noted in #110670) and its regression assertion into
the runtime-state carrier.
(cherry picked from commit 66282e6dc3ac5f276cdee5b92a22856f146c9a45)
On a flat install (checkout root == $HERMES_HOME) the untracked autostash of
`hermes update` sweeps the live state.db/-wal, snapshots, cron ledger and
lock/pid files into the stash and unlinks them under the running gateway; the
restart recreates an empty store at the same path and the declined restore
leaves the profile with no transcripts (#110648).
Root-anchor the runtime state set in .gitignore, mirroring the
.hermes-bootstrap-complete (#38529) and /.install_method (#66189) precedent
and the $HERMES_HOME-root enumeration in the platforms base module, so the
stash step is never entered for runtime state alone. Regression test runs the
exact stash command against a real repo carrying the tracked .gitignore.
(cherry picked from commit 6c74c24e6131af189801e54f1e11b260a529b74a)
The 'execProbe keeps the parent event loop available' case asserts nothing
about timing; the 1s spawn budget only exists so a wedged child cannot stall
the run. A cold Windows CI runner can take longer than 1s just to start the
node child, which failed the test for reasons unrelated to what it guards.
5s keeps the safety bound without the flake.
runPoolBackendStart reused assertPoolEntryStillOwned at every cancellation
checkpoint, and that helper releases the local backend slot before it throws.
That is right before spawn (no child, nothing to wait for) but wrong at the
post-spawn checkpoints (after the port announcement, waitForHermes, token
adoption, WS probe): the lease was handed back while the superseded child was
still running, so a successor could spawn into an occupied slot, and the
caller's teardownFailedLocalBackend -> releaseLocalBackendSlotAfterExit then
found nothing left to release and became a no-op. This breaks the
pool-spawn-coordinator invariant that a lease is held until the child exits
or the start fails.
Add a `releaseSlot` option to assertPoolEntryStillOwned and pass
`{ releaseSlot: false }` at every site where `entry.process` is set. The
pre-spawn sites keep releasing. The post-spawn release now happens only via
teardownFailedLocalBackend (after the child has provably exited) or the
child's own exit handler.
No vitest case: the helper and its callers live in main.ts alongside the
module-level backendPool / localBackendLifecycle state and are not importable
from a unit test without extracting them; the lease-after-exit ordering
itself is already pinned by pool-spawn-coordinator.test.ts ('a rejected wait
keeps the slot occupied').
The serve-support resolver caches the probe outcome per resolved runtime
for the process lifetime. That is right for a genuine "unknown
subcommand" exit, but a probe that died by timeout says nothing about the
runtime — only that this machine was slow right then (cold AV scan on
Windows, first Python import after boot). Caching that as `false` routed
a modern runtime through the legacy `dashboard` form until the app was
relaunched.
Export isTimeoutError from backend-probes and evict the cache entry on a
timeout so the next check re-probes; non-timeout failures stay cached.
One vitest case covers timeout → re-probe; the existing case still pins
non-timeout failure → cached.
Also replace the last synchronous read on the discovery path
(dashboard.py fast path) with fs.promises.readFile, matching the stack's
goal of keeping runtime discovery off the main event loop.
isActiveRuntimeUsable relied on async-return flattening for the trailing
canImportHermesCli promise: correct today, but appending any further `&&`
operand would have made the expression truthy regardless of the probe.
Await the probe explicitly so the intent survives future edits, and mark
unwrapWindowsVenvHermesCommand async like its siblings since it always
returns a promise.
connectRemote used two inline isCurrentAttempt/throw pairs with the same
message that backendConnectionState.assertCurrentAttempt already emits,
and the third guard in the same function already uses the helper. Use it
in all three places so the superseded-attempt message has one source.
The fast-path/probe/cache explanation for serve support lived above the
resolver call site in main.ts while the logic lives in
backend-serve-support.ts; move it next to the code and leave a pointer.
The stack inserted the errcode table directly after _strip_reply_fallback's
return with no blank lines, which reads as if the constant belongs to the
function body and trips E305. Two blank lines restore the module-level
boundary; no behaviour change.
Fold the transient/permanent sync-loop tests into one parametrized table
and add the two classifier branches that had no loop-level coverage: a
401 whose body was rewritten to HTML by a reverse proxy (errcode dropped,
so only http_status can stop the loop) and a 429 M_LIMIT_EXCEEDED that
must be retried because neither errcode nor status is an auth signal.
Deleting the http_status fallback in _is_permanent_matrix_auth_error now
fails the 401-html case.
Hoist _sync_error to module level so parametrize can call it directly
instead of the staticmethod.__func__ workaround. Trim the
test_ws_auth_retry docstring, which still described a Matrix test class
that moved to test_matrix.py.
The inherited fixture coordinate "40.4302" does not contain the substring
"403" (the dot splits it), so the old substring classifier also passed on
it and the case proved nothing. Use a coordinate that genuinely embeds the
digits so the test is red on the pre-fix classifier.
The pinned mautrix 0.21.1 raises MatrixRequestError (carrying errcode and
http_status) from HTTPAPI._send on every non-2xx and sync() returns only
the parsed JSON dict, so the result-object auth branch in _sync_loop was
unreachable; it dated from the nio client whose SyncError objects were
real. Drop it together with the nio-mock test that pinned it.
With structured attributes guaranteed, the leading-status regex and the
bounded keyword scan over the message text were the only remaining ways
for body digits or HTML words to leak into the verdict, so drop them too:
no errcode/http_status auth signal means retry. Trim the contributor's
17 tests to the two loop-level invariants: both production repros (502
HTML body embedding "403" via an SVG coordinate; timeout echoing a since
token embedding "401") keep looping, and a 401/M_UNKNOWN_TOKEN stops.
The comment above the result-object branch in _sync_loop claimed mautrix's
Client.sync() returns an object carrying a message string for auth failures.
That is wrong. In the pinned mautrix 0.21.0, HTTPAPI._send raises
make_request_error() for any non-2xx and otherwise returns parsed JSON, so a
real M_FORBIDDEN arrives as an exception and is handled by the except branch.
The claim was introduced by this PR, which rewrote an accurate comment about
the earlier matrix-nio client (whose SyncError result objects were genuine).
The branch itself is kept as defense in depth against a future client swap,
but it now classifies with the same errcode/http_status logic as the
exception path instead of a lone "unknown_token" substring test, which
silently missed M_MISSING_TOKEN and M_FORBIDDEN and resynced forever
against a credential that can never succeed.
A structured errcode/http_status is authoritative; the message text is only
consulted when the object exposes neither, since str(object) is an opaque
repr. The text scan deliberately cannot override a structured verdict, so a
transient 502 whose HTML body contains "Forbidden" is still retried.
Adds four tests. Three are discriminating RED/GREEN cases that fail against
the old substring branch (M_MISSING_TOKEN errcode, http_status=401 with no
keyword in the message, and an unstructured object whose only signal is
.message). The fourth pins the precedence rule and passes either way.
Verified: 136 passed / 1 failed in tests/gateway/test_matrix.py; the single
failure (test_password_login_uses_device_id) fails identically at the
pristine PR head and is unrelated.
(cherry picked from commit bc9e6a8dafcf349a4e6b20a261fb2449603c0239)
