Skip to content

fix: concurrent .env writes + WS TOCTOU race + gateway guard + Desktop usage panel - #77358

Closed
eaglezzz0522-cloud wants to merge 1 commit into
NousResearch:mainfrom
eaglezzz0522-cloud:feat/77187-77192-77173-77221
Closed

fix: concurrent .env writes + WS TOCTOU race + gateway guard + Desktop usage panel#77358
eaglezzz0522-cloud wants to merge 1 commit into
NousResearch:mainfrom
eaglezzz0522-cloud:feat/77187-77192-77173-77221

Conversation

@eaglezzz0522-cloud

Copy link
Copy Markdown

Summary

Four fixes and one feature, one commit, all clean against current main (rebased on 75901a2).

fix1 (#77187) — concurrent .env writes cause token clobbering

  • Add portalocker.Lock(env_path, timeout=5, fail_when_locked=False) in save_env_value() and remove_env_value() covering the full read-modify-write path on ~/.hermes/.env
  • portalocker is already a transitive dependency (used by uv in the venv)

fix2 (#77192) — WS close_on_disconnect TOCTOU race (dup of #77192 / PR #77129)

  • _close_sessions_for_transport() now snapshots session IDs outside _sessions_lock, then re-validates each session under _session_resume_lock -> _sessions_lock before acting
  • Prevents RuntimeError from mutating _sessions dict during iteration; reconnecting session is never silently closed or detached

fix3 (#77173) — gateway lifecycle guard false-positive on full paths

  • Tighten Branch A regex from r"(?:hermes\s+gateway\s+(?:restart|stop))" to r"(?:^|[\s;&|\n])hermes\s+gateway\s+(?:restart|stop)\b"
  • Stops false positives on /usr/bin/hermes gateway restart etc.

feat1 (#77221) — Desktop Usage panel (built-in full-page view at /usage)

  • Lazy-loaded UsageView showing summary cards, daily token/cost sparkline, and per-model breakdown
  • Registers USAGE_ROUTE in routes.ts, lazy import + <Route> in surfaces.tsx, sidebar nav entry, and nav.usage keybind
  • Consumes the existing /api/analytics/usage endpoint (no new backend surface needed)

Files changed

File Change
hermes_cli/config.py +import portalocker; lock save/remove env
tui_gateway/server.py TOCTOU-safe _close_sessions_for_transport
cron/lifecycle_guard.py tighter Branch A regex
apps/desktop/src/app/routes.ts +USAGE_ROUTE / usage view types
apps/desktop/src/app/contrib/surfaces.tsx lazy UsageView + route
apps/desktop/src/app/chat/sidebar/index.tsx sidebar nav entry
apps/desktop/src/lib/keybinds/actions.ts nav.usage keybind
apps/desktop/src/app/usage/{index,sparkline,store}.ts* new usage panel

Validation

  • Python syntax check: config.py ✓, server.py ✓, lifecycle_guard.py ✓
  • Rebasing cleanly onto current main (75901a2); no conflicts against upstream changes in any of the touched files.

…p usage panel

fix1 (NousResearch#77187): add portalocker.Lock to save_env_value/remove_env_value
  - cross-process lock around full read-modify-write on ~/.hermes/.env
  - prevents token clobbering when multiple Hermes processes write .env
  - import portalocker (already a transitive dependency)

fix2 (NousResearch#77192): fix WS close_on_disconnect TOCTOU race
  - snapshot owned session ids outside _sessions_lock
  - re-lookup each session under _session_resume_lock -> _sessions_lock
    before acting, validating transport still matches
  - defer teardown work off both locks (consistent with _pop_session_by_id)
  - prevents RuntimeError from mutating dict during iteration

fix3 (NousResearch#77173): tighten gateway lifecycle guard regex (Branch A)
  - anchor on start-of-command or delimiter before "hermes gateway"
  - stops false positives on absolute paths like /usr/bin/hermes gateway restart

feat1 (NousResearch#77221): add Desktop Usage panel as a built-in full-page view
  - new /usage route: summary cards, daily sparkline, per-model breakdown
  - lazy-loaded UsageView, registered in routes + sidebar nav + keybinds
  - consumes existing /api/analytics/usage (no new backend surface)
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery comp/cron Cron scheduler and job management area/config Config system, migrations, profiles area/usage-cost Token accounting, usage reporting, billing, cost tracking needs-decision Awaiting maintainer decision before any implementation sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this omnibus mixes several independent fixes with a Desktop usage feature and overlaps the active .env/WebSocket/lifecycle-guard work. Please consider splitting the independently reviewable changes or confirming the intended combined scope.

@eaglezzz0522-cloud

Copy link
Copy Markdown
Author

Thanks for the review @alt-glitch — you're right, this omnibus was too broad. Apologies for bundling independent fixes together.

I've closed this PR and split it into 4 separate, clean PRs, each addressing one issue:

  1. fix(config): concurrent .env writes lose updates silently (RAH-02) #77187 — fix(env): concurrent .env writes with portalocker lock
  2. WS disconnect/reconnect TOCTOU fix exists only on an unmerged branch, missing from main (RAH-06) #77192 — fix(ws): TOCTOU race in close_on_disconnect
  3. Gateway terminal guard false-positives on full-path binary execution #77173 — fix(guard): lifecycle guard false-positive regex anchor
  4. feat(usage): desktop app has no local token/cost analytics surface despite full metering in core #77221 — feat(usage): Desktop usage analytics panel

Each one is rebased on current main and only touches the relevant files. Thanks for the guidance — will keep PRs focused going forward 🙏

@eaglezzz0522-cloud

Copy link
Copy Markdown
Author

Closing in favor of 4 separate PRs listed above.

@eaglezzz0522-cloud

Copy link
Copy Markdown
Author

Hi maintainers — thanks for the feedback on the original submission. I have taken it to heart and split this PR into 4 focused, single-purpose PRs:

  1. fix: prevent concurrent .env writes via portalocker cross-process lock #77568fix: prevent concurrent .env writes via portalocker cross-process lock (only hermes_cli/config.py)
  2. fix: TOCTOU race in close_sessions_for_transport #77570fix: TOCTOU race in close_sessions_for_transport (only tui_gateway/server.py)
  3. fix: tighten gateway-lifecycle regex with command-boundary anchors #77571fix: tighten gateway-lifecycle regex with command-boundary anchors (only cron/lifecycle_guard.py, 1 line changed)
  4. feat: add Usage dashboard panel to desktop app #77572feat: add Usage dashboard panel to desktop app (only apps/desktop/src/)

Each PR now contains exactly one logical change, making review much easier. I have closed this omnibus PR in favor of the individual ones above. Apologies for the initial bundling — I will keep each PR scoped to a single fix going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/usage-cost Token accounting, usage reporting, billing, cost tracking comp/cron Cron scheduler and job management comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants