/simplify-code reuse+quality reviewers both flagged the byte-identical
fc_->call_ synthesis blocks in the assistant and tool-result branches as
a correctness coupling — the two sites MUST stay in lockstep or pairing
breaks. Extract _canonical_call_id_from_fc() and route both through it.
Mutation check: pairing regression test fails when the tool-branch call
is stubbed out, green after restore.
The sweeper review on #49224 flagged that the assistant branch synthesizes
call_<suffix> from an fc_-only id while the tool-result branch kept the raw
fc_... string — so an oversized pair hashed to two DIFFERENT clamped
surrogates and the function_call_output arrived unmatched (HTTP 400).
Canonicalize the tool-result side to the same call_<suffix> before
clamping. Also fixes the pre-existing short-fc_ pairing mismatch
(call_short123 vs fc_short123). Regression test covers both lengths.
A degenerate tool name stored in conversation history (dots, spaces,
unicode from an earlier model degeneration) bricks every subsequent
Codex Responses turn with a non-retryable HTTP 400:
Invalid input[N].name: string does not match pattern '^[a-zA-Z0-9_-]+'
The 400 replays forever until the user manually starts a new session.
Add _sanitize_replayed_fn_name() — replaces invalid chars with '_'
(runs collapsed), degrades all-invalid names to 'fn' instead of empty
(an empty name would trade one 400 for a preflight ValueError). Applied
at both replay sites: the chat-message converter and the preflight
choke-point. Live tool-definition names are left untouched — they must
match the dispatch registry exactly. Pairing is by call_id, so
renaming a replayed function_call is safe.
call_id overflow (the sibling half of #49224) was already fixed on main
by #73492 (_clamp_responses_call_id); this commit covers the remaining
invalid-name defect.
Credit: @Morad37 (#31678 — identified the bug, the replay sites, and
the regex contract), @lubosxyz (#49224 — replace-not-strip semantics
and 'fn' fallback to avoid the empty-name trap).
Fixes#31666
The rebase re-landed pre-#74805 versions of the backend release gate,
venv-blocker rescan, mac entitlements/usage tests, and package.json from
the stale branch base — restored to main's versions (only the salvaged
enumeration/profileMetadata/profile:remember hunks kept in main.ts).
refreshActiveProfile's new bounded retry chain (#70679) is no longer
awaited inside the switch-completion barrier, so a slow/unhealthy backend
cannot hold $gatewaySwitching past the switch-ownership deadline; also
drop an unused $connection import from the earlier conflict compose.
Salvage of #94192's unique work (owner-hardening portions that overlap
the class-1 branch — #94824/#93451 seams — and out-of-cluster #94864 are
intentionally excluded):
- use-gateway-request: recognize the full transport-error family
(ECONNRESET & friends, including error.code and error.cause.code) so a
reset SSH/remote socket triggers the connection-owned reconnect instead
of surfacing as a request failure; background profiles keep the
registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
the tile's composite owner (connectionId+profile) even when the active
gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
connection + profile) instead of a bare profile name that silently
collapsed the route back to the local path; delegate/wiring resolve
owners through it.
Fixes the SSH-reconnect share of #91365-adjacent routing gaps.
Salvaged (partial) from #94192.
Never-interacted remote bots painted as bare handles because roster rows
carried only profile names: display_name/title/ui_meta/has_avatar were
fetched lazily on first interaction (#91365). Thread credential-free
profile metadata from the enumeration-time /api/profiles body through
enumerateRegistryAgentSources (main.ts) and buildAgentRoster
(connection-registry.ts), keeping it attached to the connection-qualified
row across the same-install collapse. The plugin.js botRosterMeta half of
the original PR is dropped — superseded by landed #92731.
Fixes#91365
Salvaged (partial) from #92708.
The profile rail's live workspace switch never persisted the selection,
so the Desktop always booted back into the previous startup profile
(#79886). Route the successful primary-backend activation through a new
persistence-only hermes:profile:remember IPC (validated
writeActiveDesktopProfile) that records the choice WITHOUT tearing down
the backend or reloading the window like hermes:profile:set does.
Registry-source picks name another source's profiles and do not touch
the startup preference. Reapplied semantically over three weeks of
main.ts/preload.ts drift (selectProfile now routes through
activateOnCurrentSource, #91349/#91365 seams).
Fixes#79886
Salvaged from #79888.
Global remote mode fires refreshProfiles while the remote HTTP proxy is
still routing: the one-shot fetch failed silently and the rail stayed
empty until a manual refresh. Retry with 500ms/1000ms backoff, surface
terminal failures on the console, and dedupe concurrent callers into a
single retry chain (gateway open fires useBackgroundSync and the
activeGatewayProfile effect at once). Reapplied semantically on top of
the #85731 epoch guard: a stranded epoch stops the retry chain and
invalidation detaches the single-flight slot.
Fixes#70679
Salvaged from #74500.
Addresses AI-review feedback on #94653: note where the 'connect-on-demand'
sentinel is produced, and cover the interaction between
isLocalEnumerationFailure and localRouteFallbackProfiles directly (not just
the helper in isolation).
'connect-on-demand' means local roster enumeration was intentionally
skipped to avoid spawning a local backend on a remote-only workspace,
not that it failed. The plugin-profile-routes IPC handler passed
Boolean(error) straight through, so that deferral was treated as a
genuine failure and Bot Mode re-synthesized cached local profile rows
even though local was never dialed.
Fixes#94648
Old per-profile pin caches caused the stale unpin resurrection. Prove they are ignored and that sessions.pinned repopulates the gateway-wide key without a migration PATCH.
Pin localStorage was keyed per connection and profile, so an unpin
reloaded a stale copy on switch and re-asserted pinned=true.
Scope pins by connection only so they survive rescope and stay isolated per gateway.
A pooled remote backend (Bot Mode, group chat) keeps its descriptor and SSH
forward cached in the backend pool. When the remote Desktop relaunches, the
remote process dies but the local forward stays LISTENing, so
ensureRegistryBackend() keeps returning the dead descriptor and every dispatch
to that machine fails until the app is restarted.
The background sweep cannot cover this: revalidatePooledRemoteBackends() only
runs from the renderer reconnect IPC, which never fires while the primary
connection stays healthy.
Validate the exact cached descriptor at dispatch time with a short /api/status
probe (2.5 s). On failure, retire the pool entry and its SSH forward, then
reconnect on demand. Concurrent dispatches share one retire/reconnect sequence
through a RemoteRevalidationCoordinator keyed on the cached promise, and
identity checks make a late failure from an old descriptor unable to tear
down a replacement another caller already installed.
Verified on a two-Mac setup (MacBook + Mac mini over SSH): after relaunching
the Mac mini's Desktop, a group-chat turn from the MacBook now reaches the
mini's backend and its reply lands, where it previously failed forever.
reconcileBusyStatesOnReconnect downgraded stale busy/awaiting claims by
writing the $sessionStates mirror directly. The claim has four holders —
the wiring cache, that mirror, the focused view's draft $busy /
$awaitingResponse, and busyRef — and only the write path (the delegate's
updateSessionState) keeps them in lockstep. After a reconnect that orphans
a mid-turn runtime (a respawned backend re-mints runtime ids, so the
terminal busy:false never arrives) the mirror cleared but the composer
stayed latched: Send failed isTargetSessionBusy and silently no-oped until
restart, and warm resume could OR the stale cache copy back over the
backend's running:false.
- SessionTileDelegate.retireBusyClaim?: optional twin of
invalidateRuntimeBindings; writes through updateSessionState, returns
false (and writes nothing) for a runtime the cache never held.
- reconcileBusyStatesOnReconnect routes each in-scope downgrade through it,
keeps the mirror publish as the fallback, and on a primary reconcile also
clears the focused draft latches. Scoped reconciles leave the composer
alone.
Tests: hook (real useGatewayBoot + fake socket), store (write-path route,
miss fallback, primary vs scoped), cache (real updateSessionState) and
delegate (hit/miss) — RED on main, GREEN here. Full desktop UI suite,
typecheck and lint pass.
Written with LLMs under human direction: initial report and diagnosis by
GPT-5.6 (OpenAI Codex); root-cause refinement, design and review by
Claude Fable 5; implementation and tests by Claude Opus 5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the #93892 keep-set salvage (#93916): the new
"remote tile keep-set must not pin a local same-named secondary" test
exposed a real scoping defect — route identity in the prune keep-set must
be full composite scope (connectionId + profile), never a bare profile
name inherited by accident.
setPrimaryGateway() moved g.primaryProfile without moving g.activeKey
when the active route WAS the primary. The stale bare-name activeKey
(e.g. 'default') then matched a later, unrelated LOCAL 'default'
secondary in pruneSecondaryGateways' `key === g.activeKey` spare, so a
keep-set of composite scopes like 'conn:homelab::default' appeared to
pin the local socket forever. Now the active key follows the primary
re-home, keeping the exact-scope identity contract intact.
Follow-up to the mode-switch teardown split: classify legacy secondaries
via an explicit isLegacySecondary() helper, keep open-pane gateway owners
in the boot keep-set, and cover the explicit `local` registry source not
being classified as legacy.
Salvaged from PR #94370. The PR's off-topic edit-composer changes
(user-edit-composer.tsx, user-message-edit.test.tsx — an unrelated edit
submit-cooldown tweak) were dropped from this cherry-pick.
Dropped-files: apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx, apps/desktop/src/components/assistant-ui/thread/user-message-edit.test.tsx
Bot chats stay on a secondary while chrome stays on the launch profile.
The keep-set only counted busy sessions, so idle prune closed the tile
socket and resume spun forever. Keep open tiles, route catalog reads to
the owner, and hydrate model/provider from resume.
Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes#92265 (proposed fix#2; #1 and #4 are separate follow-ups, see below).
ensureGatewayForAgent() and ensureGatewayForProfile() both decided
whether a secondary activation "succeeded" by checking Boolean(entry.connection)
alone. entry.connection is set in openSecondary() BEFORE the WebSocket
dial completes (`entry.connection = conn` happens ahead of
`await entry.gateway.connect(wsUrl)`), so a transient first-dial
failure -- caught by the surrounding try/catch and left for
scheduleReconnect's backoff retry -- still left entry.connection
truthy. Both functions then treated this as a successful activation:
applyActive() switched g.activeKey and published $gateway to the
closed socket, and publishActiveConnection() pushed the connection
descriptor to the UI. The next chat RPC then failed with "Hermes
gateway is not connected" against a route the user/desktop believed
was live.
Added an isOpen(entry.gateway) check alongside the existing
Boolean(entry.connection) check in both functions' activation/publish
conditions, gating BOTH applyActive() (which switches g.activeKey and
publishes $gateway) and publishActiveConnection() (which pushes the
connection descriptor) on the socket having actually reached 'open'.
A failed first dial now correctly returns false / leaves the previous
active route untouched, matching option 3 from the issue's own
proposed fix ("if both bounded attempts fail, keep the existing
active route") -- the existing scheduleReconnect backoff still owns
recovery for that entry going forward.
Not implemented in this PR (separate, lower-priority follow-ups):
- Proposed fix#1 (one immediate bounded reconnect attempt before
returning activation status) -- a larger behavioral change with its
own retry/timing tradeoffs; left to a separate PR.
- Proposed fix#4 (Bot Mode's own connection-ID-only guard in
plugins/hermes-bots/plugin.js) -- host.ensureAgent() calls into the
now-fixed gateway.ts functions, so this class of bug is already
closed at the root; Bot Mode's own additional profile/state
verification may still be worth adding but is a separate, narrower
hardening pass on top of this fix.
Found and fixed a genuine test-suite inconsistency while verifying:
the existing "refreshes the active connection after a pooled profile
reconnect succeeds" test in gateway-shared-remote.test.ts asserted
setConnection was called once after a SINGLE ensureGatewayForProfile()
call whose first dial failed -- i.e. it encoded the exact bug this
issue reports as the EXPECTED, correct behavior. Rewrote it to assert
the corrected contract: the failed first attempt does not call
setConnection at all, and a realistic retry (calling
ensureGatewayForProfile() again, since g.activeKey correctly never
left the primary after the failed attempt -- ensureActiveGatewayOpen()
is for reconnecting an already-active gateway that went stale, not
retrying an activation that never succeeded) succeeds and publishes
once the second dial goes through.
Added a new test file (gateway-secondary-open-check.test.ts) following
the established mocking pattern from gateway-agent-scope.test.ts,
covering both ensureGatewayForAgent and ensureGatewayForProfile: a
transient first-dial failure does not activate/publish (the exact
reported symptom), and a successful dial still activates/publishes
normally (sanity, no regression to the happy path). Verified as
genuine regressions by reverting both isOpen() checks and confirming
2 of 4 new tests fail with exactly the reported symptom (activated
resolves true / the primary gets replaced despite the failed dial).
44/44 pass across all 9 gateway-related test files (no regression).
Dupe-swarm winner for issue #92265; Biotrioo (PR #92307) was the earliest
submitter of the swarm and deserves first-report credit.
Anonymous probes (_anonymous_json) are designed to probe server identity
before disclosing credentials. Sending the Hermes version on these probes
would fingerprint the exact version to an untrusted/MITM endpoint.
Keep User-Agent on authenticated requests (_headers) and multipart uploads
(_multipart_headers), which already send credentials.
taskkill /T /F returns when termination is INITIATED, not completed, and
the pre-handoff unlock gate only probed the venv hermes.exe shim — which
the 'python.exe -m hermes_cli.main serve' backend need not hold at all.
The gate could therefore pass on its first iteration with zero dwell
while the killed pythons were still unmapping .pyd files; the
venv-blocker scan (no liveness filter) then reported those dying
processes as holders and aborted the hand-off. Every first update
attempt from the footbar failed; the manual retry succeeded because the
process table had settled by then.
The unlock gate now lives in backend-release-gate.ts (dependency-free,
backend-child.ts pattern) and requires BOTH the shim unlocked AND every
signalled PID to have actually left the process table; stragglers
collected per-pass are killed and join the watch set. On deadline the
old shim-only criterion survives as the escape hatch — lingering PIDs
past 15s are the venv-blocker re-scan's job. applyUpdates additionally
re-scans up to 2x with a 1.5s settle before aborting on 'blocked', so
untracked grandchildren an AV driver holds in teardown stop failing the
update while a REAL holder still aborts on the third scan.
Surgical reapply of PR #78037 fix 1 by @3x3xX3N0N onto the post-#87599
code shape (stopBackendTreesForUpdate extraction, stopSafeBlockers
re-scan path). The re-scan settle idea was first submitted by
@MaheshBhushan (#74831); the killed-PID tracking seam matches
@webtecnica's #74956.
Co-authored-by: MaheshBhushan <128616744+MaheshBhushan@users.noreply.github.com>
Co-authored-by: webtecnica <75556242+webtecnica@users.noreply.github.com>
Co-authored-by: Hermes <hermes@nousresearch.com>
Batch follow-ups on top of the salvaged privacy declarations:
- tests-js/desktop-mac-usage-descriptions.test.ts: EXPECTED_USAGE_DESCRIPTIONS
now pins the FINAL key set (camera + calendar x2 + reminders x2 + screen
capture + local network) so the drift-protection assertion locks the whole
batch as permanent regression coverage.
- entitlements.mac.plist: add com.apple.security.personal-information.reminders
alongside the calendars entitlement #65220 added — the reminders usage
descriptions need the matching entitlement under hardened runtime (sibling
site the original PR missed).
Address maintainer review feedback (PR #66215, comment by @teknium1):
> `tests/test_desktop_mac_entitlements.py:47` reads `apps/desktop/package.json`
> from pytest. `AGENTS.md:1319-1329` requires assertions about `package.json`
> and JS-side artifacts to be in the JS/Vitest suite; otherwise CI
> classification can skip the regression test on a JS-only change.
The CI change classifier (`scripts/ci/classify_changes.py`) marks
`apps/desktop/package.json` as `_FRONTEND` (in `_PY_SKIP`), so a Python test
that reads it would be skipped on a JS-only PR — regression goes green on
the PR, red on main.
Move the regression to `tests-js/desktop-mac-usage-descriptions.test.ts`,
following the same convention as commit dbf86b923 ("test: port macOS
entitlements test from Python to vitest"), which ports an earlier Python
entitlements regression into `tests-js/desktop-mac-entitlements.test.ts`
for the identical reason. The new file is a sibling of that one — both
pin Desktop macOS manifest contracts, but they assert against different
files (`entitlements.mac.plist` vs `build.mac.extendInfo` in package.json).
The Vitest port mirrors the original assertions 1:1: every
`NS*UsageDescription` key pinned (parametrized over key + required
substring + reason), no leading/trailing whitespace or newlines in any
`extendInfo` string, and a drift-protection assertion that fails when a
new privacy key is added to the build config without a matching row.
A runtime type guard on `extendInfo` ensures a non-string plist scalar
raises a clean assertion error here ("`X` in build.mac.extendInfo must
be a string (got boolean)") rather than crashing the test runner with
`value.trim is not a function` deep in the whitespace test — caught by
Flash + GPT-OSS cross-vendor review.
Verified:
- `cd tests-js && npm run check` → typecheck clean, 14/14 tests pass
(4 files including the new one with 5 tests).
- Mutation: removing `NSAppleMusicUsageDescription` from
`apps/desktop/package.json` flips 1 test red with the exact symptom
("Info.plist privacy usage description \`NSAppleMusicUsageDescription\`
is missing"). Restore → 14/14 green.
- Mutation: adding an unpinned `NSSpeechRecognitionUsageDescription` with
whitespace flips 2 tests red (drift-protection + whitespace).
- Mutation: adding a non-string `CFBundleBooleanTest: true` flips the
whole file red with the clean "must be a string (got boolean)"
assertion (no downstream crash).
- `apps/desktop` Electron Vitest project still passes (42 files,
432 tests + 1 skipped).
Closes the maintainer comment thread on PR #66215.
Fixes#54551
The Hermes Desktop renderer initializes Chromium's audio stack on user
gesture (completion chimes via Web Audio API in completion-sound.ts,
voice TTS via voice-playback.ts, mic capture via use-mic-recorder.ts,
and an eager AudioContext prime in haptics-provider.tsx). On macOS 26+,
that initialization registers the helper with the MediaLibrary TCC
service (kTCCServiceMediaLibrary), which surfaces to the user as a
"Hermes wants to access Music" permission prompt even though Hermes
never reads or writes the Apple Music library.
The Info.plist (built from apps/desktop/package.json's build.mac.extendInfo)
already declares NSAudioCaptureUsageDescription and
NSMicrophoneUsageDescription, but NSAppleMusicUsageDescription was missing
from the desktop app entirely. macOS therefore shows a system-default or
generic prompt for the MediaLibrary bucket instead of an honest description
from the app.
Fix
---
Add NSAppleMusicUsageDescription to build.mac.extendInfo with copy that
disclaims Music library access while explaining the system audio stack
uses voice, TTS, and completion sounds.
Add tests/test_desktop_mac_entitlements.py to pin every NS*UsageDescription
key declared in the Desktop build config. The test:
- parametrized over a (key, required_substring, reason) table
- asserts no leading/trailing whitespace and no newline chars in any usage
string (electron-builder passes them through verbatim; control chars
render as broken prompt text)
- asserts drift-protection: a new NS*UsageDescription key added to the
build config without a matching test row causes a hard failure
Pattern reference: PR #59486 ("fix(desktop): add macOS contacts privacy
strings") is the open canonical for the same shape of fix for Contacts;
PR #64582 / PR #65220 extend it for Reminders. The closed duplicate PRs
Related, not in this PR
-----------------------
- PR #62601 (sounddevice on macOS) is the gateway/CLI side of the same
kTCCServiceMediaLibrary trigger.
- PR #45952 (macOS permission broker foundation) is architectural work
for centralized TCC handling; this fix does not depend on it.
- PR #52839 (browser automation Chrome launch) mutes Chromium audio in
a different surface; the same pattern is recorded there.
Fixes#54551
Since macOS 15 (Sequoia), an app that accesses the local network without
declaring NSLocalNetworkUsageDescription in its Info.plist is denied
silently: no prompt, no entry in System Settings → Privacy & Security →
Local Network, and LAN connections are dropped at the network layer
(manifesting as 'No route to host').
The desktop's build.mac.extendInfo declares camera/microphone/audio
usage descriptions but not the local-network one, so the terminal and
SSH features inside the app could never reach LAN hosts on macOS 15+.
Add the missing declaration.
Without the purpose string, macOS shows a bare Screen Recording
prompt when the desktop app (or a plugin driving desktopCapturer /
ScreenCaptureKit) first requests screen access, and some flows deny
silently instead of prompting.
Verified: package.json parses (python json.load).
The legacy ad-hoc fallback signed and verified successfully but still
fell through to return False, contradicting the fixup's documented
contract. The success witness codified the contradiction. Return True
on the verified success path; the caller ignores the return value, so
no behavior change beyond the contract correction.
Addresses round-2 review feedback on #90961. The previous commits
scoped the keychain deletion to the legacy ad-hoc fallback, but the
reviewer correctly held the blocker: the fallback ran codesign with
check=False, ignored the result, and unconditionally deleted 'Hermes
Safe Storage' — permanently orphaning gateway and native OAuth
credentials even when signing failed or a configured identity had
failed and routed into the fallback.
This commit removes the deletion entirely:
- _desktop_macos_reset_keychain_safe_storage is gone; no code path
touches the keychain item anymore.
- The legacy fallback now checks the codesign result and runs
codesign --verify --deep --strict; any failure leaves the item
untouched and prints a warning.
- The keychain prompt after an ad-hoc re-sign is recoverable
(Always Allow updates the ACL partition list and preserves the
key); deletion is not. The durable proof-carrying migration
belongs in Electron (safeStorage can read the old key) and is
tracked as a follow-up.
Tests: 4 witnesses (stable path, default no-config success, fallback
failure, fallback success) all mutation-verified against both the
deletion regression and the ignored-codesign-result regression.
The fixup no-ops on non-macOS (sys.platform guard), so the new
regression tests must carry the same @pytest.mark.macos_only marker
as their siblings (test_relaunchable_fixup_falls_back_to_legacy_adhoc_on_failure).
Without it the legacy-adhoc test failed on the Linux CI runner where
the fixup returns True before reaching the reset path.
The previous commit deleted the 'Hermes Safe Storage' keychain item after
every successful re-sign, including the stable certificate-anchored
identity path. On that path the designated requirement is stable across
rebuilds, so after the first launch under the new identity the keychain
ACL already matches; deleting the item on every update permanently
orphaned gateway-token and native-OAuth credentials that were working
fine (both are safeStorage-backed: electron/main.ts connection config
and native-oauth-tokens.json).
Addresses review feedback on #90961:
- Rename _desktop_macos_update_keychain_acl -> _desktop_macos_reset_keychain_safe_storage (it deletes, it does not update an ACL).
- Only invoke it on the legacy ad-hoc fallback path, where every rebuild
produces a new cdhash so the ACL can never match and the alternative
is a recurring prompt. The trade-off (re-enter credentials once per
update) is documented; the durable fix is a stable signing identity.
- Add regression tests: stable path must NOT reset, ad-hoc fallback MUST.
The self-updater rebuilds the desktop app locally via electron-builder after
every update (4aa9f738ce). On macOS, the rebuilt app gets ad-hoc signed,
producing a different cdhash than the original CI-signed build. macOS ties
the 'Hermes Safe Storage' keychain item's ACL to the code signature, so
the new signature doesn't match → macOS re-prompts for keychain access on
every launch.
After re-signing, delete the existing keychain item so Electron recreates
it with the correct ACL for the newly-signed app on next launch. The
trade-off: previously encrypted tokens become unreadable (the user
re-enters the gateway token once), but the keychain prompt stops appearing
on every launch.
The delete-generic-password command doesn't require reading the secret
(no ACL check), so it runs without prompting.
kshitijk4poor