Repository navigation
fix(bot-mode): service DMs use the active Hermes installation (salvage #100673) - #111298
Conversation
…deliveries Bot-to-bot message_agent delivery builds both transport argvs (local teammate chat and peer dm) with a bare "hermes" as argv[0]. Since #96631 the delivery runner spawns under terminal_tool's isolated host-local environment, which does not inherit the gateway's PATH — so on docker/service installs (venv at /opt/hermes/.venv) every delivery exits with FileNotFoundError: 'hermes'. Resolve the CLI with bot_relay._hermes_cli() (#93590) — the venv sibling of this interpreter, then shutil.which, then the bare name — at both argv construction sites. The turn-lock matcher in _delivery_lock() already matches argv[0] by basename, so absolute paths lock exactly as before. Fixes #100662
…ership Retain executable Electron evidence for the named-unowned and live-owner paths; stale PATH fails on base and passes with the contributor fix. Clarify that --in selects cwd rather than the profile database.
૮ >ﻌ< ა ci reviewran on bca2079 — test(bot-mode): keep the reproducible mock patch whitespace-
|
alt-glitch
added
type/bug
kvnloo
left a comment
kvnloo
left a comment
There was a problem hiding this comment.
Approve — the serialization, frozen-boundary watermarks, and failure visibility all check out, and the e2e spec covers the exact reported scenarios.
Two nits, both non-blocking:
- In
queueGroupChatDrive's catch, the drive-levelfailedevent is tagged with the closure'sthread(the send that created the drive), not thenextThreadwhose rounds actually threw — a later queued thread's failure gets attributed to the first thread. Would neednextThreadhoisted out of the loop to fix. - In the collapsed summary,
[...unresolvedFailures.values()].at(-1)follows Map first-insertion order, so A-fails, B-fails, A-fails-again shows B's older failure. Delete-then-set on re-failure would make it recency-ordered.
One design question worth your call: the only reason argv shape matters here is that _delivery_lock / _local_delivery_home parse the child argv to recover the target profile, which was already known at construction time. Would you take a follow-up that passes --profile through _delivery_command (next to the existing --profile-home) and deletes the argv parsing? Then the launcher becomes a free variable — bare name, sibling path, or sys.executable -m hermes_cli.main all work with zero parser updates.
That unlocks a cleaner launcher too: sibling hermes if present, else -m against the running interpreter, dropping the which()/bare tiers — which() can resolve to a different install than the gateway, which is the exact wrong-install failure this PR fixes. Happy to sketch the diff if the direction appeals.
|
Following up with the full triage of the hybrid, since the question above deserves a concrete answer. Proposal: stop parsing argv, resolve the launcher as a list. Two independent changes in 1. Pass 2. List-returning launcher. New Why this beats both current options:
Costs, honestly: the sibling tier exists only as a transition courtesy for the old parsers; once step 1 lands they have no consumers and could go. Tests change shape: assert on Either PR can carry this; #111298's structure (salvage authorship, native matrix evidence) is the natural base. If you want the smallest increment instead: step 1 alone, keeping |
|
Thanks @kvnloo. We landed the verified interpreter-adjacent resolver repair in b8bf484. The native stale-PATH case and nested replies passed, including a 451.947-second hold. The no-adjacent-entrypoint fallback remains the existing resolver contract; removing its PATH tiers and carrying explicit profile metadata is a separate follow-up design, not claimed by this patch. We have not closed #108632 as redundant. The two group-summary nits posted here are also on #111283 and are being verified there before its merge. |
4 participants
Bot messages no longer launch an unrelated
hermesfrom a service PATH when the sending interpreter has its own adjacent entrypoint.evals/botmode-dm-matrixand clarifies profile DB versus working-directory lookup.Live repro
Live repro: real Linux Electron + production backend and real
message_agent/quiet CLI subprocesses; deterministic loopback inference, disposable HOME/profiles/runtime. Base40f2702b22a343cbc95647efd0bdffe8e0b3d9e9.hermesand correct interpreter-adjacent launchersentACK, stale launcher exits 2, zero Beta inputsFinal rebuilt committed native run: 2 passed. Held provider response proves Beta remains a distinct CLI owner after its initial turn; retained process checkpoint is scoped to
matrix-beta, not default. Default Desktop lease remains unchanged, Gamma retainsmatrix-gamma, and opening Gamma renders the attributed message.The previous “incoming card needs reload” observation was a probe race: it tried to expand before asynchronous reconciliation mounted the card. The original final DOM already had
Message from beta/show message; bounded mount wait + expand works without reload. No speculative renderer fix.Validation
hermes.git diff --check: passed.tests/toolsrun was interrupted before completion; observed three failures also reproduce with base production source: delegate timeout cleanup and two Modal snapshot fixtures. Not claiming the full directory green.Scope and related reports
Related #105460: native default-live/named-unowned matrix refutes #105323's premise that
--in ~selects the default profile DB. It selects cwd; explicit Bot Chat title resolves in the target profile DB.Related #109750 / #109767: ordinary Linux nested quiet-CLI path works on main with correctly scoped completion ownership. Native Windows is not verified: available host is Linux, no Windows executable/mount/VM or configured native host was discovered. The long-running legacy-child 420-second failure was additionally tested and did not reproduce on current Linux (see measured probe below); Windows remains unverified.
Supersedes #100673 for the proven stale-launcher class; overlaps #108632's alternative executable selection. No merge/close requested automatically. No paid model inference.
Real-wallclock long-child control
An ordinary Desktop → quiet Beta CLI → quiet Gamma CLI chain held Gamma inference for 451.947 real seconds, with no clock acceleration or timeout override. Sequential/concurrent tool budgets remained 420 seconds; quiet notification linger was the existing 600 seconds. Native Electron test passed in 9.0 minutes. Both CLI owners remained live at 305, 425 and 450 seconds; default Desktop lease was unchanged. Default and Beta tool acknowledgements took 0.716s and 0.521s, with exactly one successful completion each after 482.376s and 466.459s respectively. Gamma had exactly one input and answer. The parent independently reran the receipt verifier over
/tmp/botmode-dm-long/native. This demonstrates asynchronousmessage_agentlifecycle, not blockinga2a_callor native Windows parity.Native screenshots
Alpha live incoming message, expanded without reload:
Gamma named canonical chat after nested CLI delivery:
Infographic
Combined campaign verification
All four exact PR heads (#111240, #111283, #111273, #111298) were locally integrated onto main
1a990f30628c25fb83d29c4d3b3d18dcb085406ein unionee20e99def270fb4c160018358a938e2a344c822; parent verified zero missing commits from every head. Additive test-fixture conflict resolution preserves both group scripting and held-response behavior. 13 native Linux Electron tests passed, covering group handoffs/queue/Stop/late/error, cron owner deferral and custom-root fallback, nested/stale-launcher delivery, parallel side chats, and busy-DM FIFO. 602 Bot Mode unit tests passed; 155 Python invariants passed, 3 skipped; full Desktop build and typecheck/lint passed (179 existing warnings, no errors). Real backend and tool execution with scripted loopback inference; late-observation clocks accelerated only in the named harvest test. Native Windows remains unverified. Evidence and conflict-resolution patch:/tmp/botmode-campaign/union/verification.json,conflicts.patch, and neighboring logs/screenshots. This is local integration proof, not a merge to main.