Skip to content

fix(bot-mode): hide sweep and chat adoption can no longer swallow ordinary sessions (salvage #89901) - #90358

Merged
teknium1 merged 1 commit into
mainfrom
fix/salvage-89901-ordinary-session-guard
Aug 20, 2026
Merged

fix(bot-mode): hide sweep and chat adoption can no longer swallow ordinary sessions (salvage #89901)#90358
teknium1 merged 1 commit into
mainfrom
fix/salvage-89901-ordinary-session-guard

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Bot Mode's hide sweep and canonical-chat adoption can no longer claim or hide a user's ordinary conversation — every hide/adopt/re-anchor path now verifies the target is actual Bot Mode plumbing (title "Bot Chat", or root title across compression) and fails closed.

Salvages PR #89901 by @Owen-narcissus onto current main with authorship preserved (clean cherry-pick, zero conflicts against the merged #90326 activity work).

Changes

  • apps/desktop/src/plugins/hermes-bots/plugin.js:
    • hideOwnedBotSessions() resolves every stored canonical pointer through profiles.list preferred_session_ids and requires the "Bot Chat" title before session.set_hidden — a stale pointer at an ordinary session is never hidden (fail-closed on old gateways too).
    • Grandfather/adopt, pin verification, and dead-pin recovery only accept Bot Chat sessions; a pin resolving to an ordinary session is treated as corrupted metadata (cleared + fresh Bot Chat created), never opened-and-hidden.
  • tui_gateway/methods_profiles.py: preferred_session gains root_title (durable pin's own title) so compression-rotated Bot Chats ("Bot Chat (continued)") still verify.
  • Tests: 4 new safety tests (stale pointer not hidden; ordinary latest not adopted; pinned ordinary replaced; dead pin doesn't steal ordinary latest) + root_title contract coverage.

Validation

Result
node --test tests/*.test.mjs (hermes-bots) 319/319 pass
pytest tests/tui_gateway/test_profiles_list_preferred_session.py 8/8 pass
node --check plugin.js clean

Infographic

Ordinary chats are safe

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on 734b535 — fix(bot-mode): protect ordinary sessions from hide sweep

⚠️ Warnings

CI timings · View report · View job

Wall time 17m47s vs 4m6s (+333.7%). 18 job(s) slower, 16 faster, 4 unchanged.

  • Python tests / Run tests slice 10/12: -24.0s
  • Python tests / Run tests slice 7/12: -22.0s
  • Python tests / Run tests slice 11/12: -21.0s
  • OS-specific tests / Windows-only tests: +18.0s
  • JS & TS checks / apps/desktop / check:test:ui:shard-1of3: +18.0s

OSV vulnerability scan · View job

7 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.

@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/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 19, 2026
@zengzheqing

Copy link
Copy Markdown

Following up from the same production repro documented on #89901 (comment 5349681846) — posting here since this is the salvage branch where the merge decision lands.

Two notes on the current diff, neither blocking:

1. Old-backend edge in verifiedCanonical (carried over from #89901). The sweep gate requires preferred.title === 'Bot Chat', but the root_title field it relies on only exists on the patched backend (methods_profiles.py in this same PR). On an older gateway, a summary row comes back without root_title while the lookup itself succeeds — and a compression-rotated canonical chat whose live tip titles Bot Chat (continued) then fails the isCanonicalBotChatHistory check and is dropped from the sweep. That direction is fail-closed (worst case an old Bot Chat stays visible — the stated tradeoff), but it degrades exactly the legitimate case across the version-skew window the desktop compatibility guidance covers. Treating an absent root_title as "older backend: fall back to tip title" would close it without weakening the safety check.

2. Recovery for sessions already hidden is still open. This PR stops new wrongful hides; rows hidden before the fix stay hidden — no surface offers a way back out of the flag today (raw SQL on state.db is the only path, which is how the repro case was restored). Companion PR #90388 adds hermes sessions unhide + sessions list --include-hidden at the CLI edge, reusing the existing lineage-flipping setter — deliberately out of this PR's scope, happy to coordinate.

teknium1 added a commit that referenced this pull request Aug 20, 2026
…tions seen in CI)

test_progress_advances_while_the_orchestrator_blocks raced its subject on
both edges within one hour of PR CI (#90358):

- Run 1: sampled right after the shim URL printed, before the orchestrator
  published its stage — caught the page boot default
  ('Hermes will open once done.' != 'Testing quiet update').
- Run 2 (rerun): with HOLD=4s on a slow runner, the second sample slid past
  the hold and caught the cleared terminal state ('' != 'Testing quiet
  update').

Fix: wait (<=10s) for the published stage to actually land before starting
the 1.5s stability window, and raise the hold to 10s so both samples land
inside it. Same assertions, same contract — just anchored to the event the
test is about instead of wall-clock luck.
@teknium1
teknium1 merged commit 8408edc into main Aug 20, 2026
164 of 168 checks passed
@teknium1
teknium1 deleted the fix/salvage-89901-ordinary-session-guard branch August 20, 2026 02:30
lisajlau pushed a commit to lisajlau/hermes-agent that referenced this pull request Aug 20, 2026
…tions seen in CI)

test_progress_advances_while_the_orchestrator_blocks raced its subject on
both edges within one hour of PR CI (NousResearch#90358):

- Run 1: sampled right after the shim URL printed, before the orchestrator
  published its stage — caught the page boot default
  ('Hermes will open once done.' != 'Testing quiet update').
- Run 2 (rerun): with HOLD=4s on a slow runner, the second sample slid past
  the hold and caught the cleared terminal state ('' != 'Testing quiet
  update').

Fix: wait (<=10s) for the published stage to actually land before starting
the 1.5s stability window, and raise the hold to 10s so both samples land
inside it. Same assertions, same contract — just anchored to the event the
test is about instead of wall-clock luck.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants