feat(bot-mode): add automatic Group Chat continuity - #97846
Conversation
d10a629 to
aa4fff8
Compare
9e619b2 to
69e8bf6
Compare
69e8bf6 to
636810f
Compare
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed the Desktop continuity delta at exact head 8f8c2bdf870a3b2790a8becd013df0aefa624e95, after #97797 head 1c6bf4cc1a5294d7387bb5dd9eb020dedcef1b44. Exact-head CI, Docker, and Nix are green.
The setup cleanup journal, persistent command outbox, monotonic replay, mutation fences, partial multi-gateway rollback, and projected-room read-only boundary are coherent. One durable-retry defect remains.
P1 reliability — transient worker unavailability permanently terminalizes queued commands
apps/desktop/src/plugins/hermes-bots/hosted-room-runtime.ts::terminalCommandFailure() classifies every integer RPC code from 4100 through 4199 as terminal except 4115.
tui_gateway/methods_groups.py uses 4123 for “Group Chat worker is unavailable. Restart the Hermes gateway and try again.”, including groups.send and groups.disband. That is a transient lifecycle state: the gateway may be connected while its process-owned hosted-room worker is restarting or stopping.
Because 4123 falls inside the terminal range, a persisted send or disband attempted during that window transitions to terminal-failure and is removed from replay rather than retaining the same durable command ID for retry. The outbox therefore converts a recoverable worker restart into permanent command loss.
Required repair:
- replace the broad numeric-range rule with an explicit typed/allowlisted classification;
- make
4123retryable while preserving the existing command ID and per-room ordering; - add cold-restart tests in which a persisted send and disband each receive
4123, survive storage reload, then settle exactly once after the worker returns; - retain terminal treatment only for errors that prove the command can never become valid without user mutation.
Additional landing blockers
- This layer inherits the #97712 UTF-8 replay-bound defect and #97797 silent policy/capability auto-reauthorization defect.
- The delta modifies
gateway/hosted_rooms.pyafter it exceeds the repository's hard 2,000-physical-line ceiling.
This head is not merge-ready until the retry classification, upstream source blockers, and touched-godfile violation are repaired and re-proven on the recomposed exact head.
857e577 to
af19603
Compare
|
Exact head
Validation on this exact object: 66 Bot Mode files / 642 tests, 98 focused backend service/RPC/storage tests, all Desktop typechecks and lint, Ruff and diff checks. Two independent Codex adversarial reviews report SHIP with no P0/P1/P2. Required CI, Docker amd64/arm64, and Nix are all green. |
andrexibiza
left a comment
There was a problem hiding this comment.
Exact-head re-review on 13115dd95f92a9ade9d88ce7c1c03b704187fa30 after the reconnect commit was replaced.
The surviving train is hosted-green commit-by-commit: 2558fb7904 (CI 33378857063 / Docker 33378856120 / Nix 33378856056), 22b26d04ca (33381788568 / 33381787708 / 33381787705), 52f80019d2 (33382258610 / 33382257942 / 33382257989), and 13115dd95f (CI 33443646095 / Docker 33443645339 / Nix 33443645330). I also re-read the replacement reconnect path, durable cleanup journal, grant-fingerprint CAS, exact-grant revocation, and the new regressions. The replacement modules stay under the 2K ceiling; there are no inline review threads. Current landing edge is main@180291162ff4df0d42b5dc4fecd08005cf7cebf9; the base→main drift contains no hermes-bots or hosted_room paths, so this moved-head reconnect slice has no current-main FILE-LIST collision.
One substantive defect remains:
P2 — reconnect lookup drops the source-qualified identity and can select the wrong gateway. matchingLocalMember() in hosted-room-reauthorization.ts matches only (profile, handle) and returns the first local member. Cross-source Group Chats explicitly allow the same profile/handle to exist on different connections; the room keeps those members source-qualified. If two peer gateways both contribute (for example) builder/builder, reconnecting the second server member can pick the first local descriptor, probe that first gateway, then fail the later peerCapability.authorityId !== targetAuthority check. This fails closed, but the affected Bot can never be reauthorized even though the server member already carries the exact target installation ID. Resolve the local member by the server target authority/connection as part of the match (not only profile+handle), and add a regression with duplicate profile+handle on two peer gateways that reconnects the second member.
The PR body also correctly leaves the new packaged reconnect path as the final exact-head UAT gate; the existing field UAT was on 15afb8fb01 and predates this replacement reconnect implementation. I would not treat that older UAT as acceptance proof for 13115dd95f.
|
Addressed the duplicate-source reconnect finding at |
andrexibiza
left a comment
There was a problem hiding this comment.
Re-reviewed exact head e75e10c62a6319e907c77f4a1fcd415648292ce3, including the new source-qualification repair a0a032bfc52643b20bb4f9a3123738c1c7d3ca94, against final-checked main@5fae0d243f98a81e49663d4c48b2ed871b9a14c2.
The source P2 from my prior review is closed. matchingLocalMember() no longer accepts the first (profile, handle) match: it qualifies duplicate candidates by the server member's exact target installation authority, returns a sole candidate only when unambiguous, and otherwise fails closed. The new regressions cover the positive two-gateway duplicate-profile case, missing authority qualification, and stale cached authority; the stale path live-revalidates capabilities before invitation/grant/registration. I do not see a new P0/P1/P2 in that delta.
The release train is still not acceptable under the every-surviving-commit proof gate. The six submitted commits are 2558fb7904, 22b26d04ca, 52f80019d2, 13115dd95f, a0a032bfc5, and merge head e75e10c62a. The first four already have exact-object CI/Docker/Nix success receipts. The new repair commit a0a032bfc52643b20bb4f9a3123738c1c7d3ca94 has Docker 33589251600 success and Nix 33589251682 success, but required CI 33589250712 is failure and dispatched zero jobs. The later merge head is green — CI 33590874227, Docker 33590873674, Nix 33590873776 — but HEAD-green cannot certify a surviving red commit.
Required release repair: recompose/squash so a0a032bfc5 is no longer a surviving submitted object (while preserving source authorship/credit), then require fresh CI/Docker/Nix success on every surviving replacement commit. Do not inherit proof from e75e10c62a onto new commit identities.
The PR's own packaged reconnect UAT gate also remains explicitly outstanding; the older 15afb8fb01 field receipt predates this replacement reconnect path and cannot certify it.
Landing-edge cross-check: main is four commits ahead of the branch's merged ff7745fb0a base. Those four commits touch only agent/conversation_loop.py, tests/run_agent/test_413_compression.py, and website/docs/reference/faq.md, disjoint from this PR's FILE-LIST, and GitHub currently reports the PR mergeable. No inline review threads exist to resolve.
So: the duplicate-source reconnect defect is substantively fixed; the remaining hard acceptance defect is exact-object release proof, plus the PR's declared packaged reconnect UAT.
andrexibiza
left a comment
There was a problem hiding this comment.
Landing-edge correction to my immediately prior review only: main advanced again while the review was being settled, to 273461f1393f0be5ecd08415779804c755c731f6. Relative to #97846's merged ff7745fb0a82be914f490f14967e158347361dd6 base, main is now eight commits ahead and touches agent/conversation_loop.py, gateway/run.py, gateway/scale_to_zero.py, tests/gateway/test_scale_to_zero_dashboard_client.py, tests/run_agent/test_413_compression.py, tui_gateway/ws.py, and website/docs/reference/faq.md. That FILE-LIST remains disjoint from this PR's 40 changed files, and GitHub still reports #97846 mergeable.
The substantive conclusion is unchanged: the duplicate-source reconnect P2 is fixed, but surviving commit a0a032bfc5 still has failed required CI 33589250712 with zero dispatched jobs, so the submitted train is not every-commit green; packaged reconnect UAT also remains pending.
f32c8a6 to
240f42f
Compare
andrexibiza
left a comment
There was a problem hiding this comment.
Exact-head re-review on 240f42ff4f54428a7a4b59309d56ebd0312d42d5.
The previous source-owner qualification defect is closed on this object. matchingLocalMember() now binds each duplicate alias candidate to its cached gateway source, queries that gateway's authoritative room-member record, and accepts only the candidate whose targetInstallationId matches the current local registered installation; stale, no-exact, and multiple-exact candidates fail closed. The two-gateway regression with identical profile + handle selects only the single source whose remote row matches the current installation, with additional stale-cache and ambiguous-source regressions.
Every surviving commit is exact-SHA green:
2558fb7904— CI33378857063, Docker33378856120, Nix3337885605622b26d04ca— CI33381788568, Docker33381787708, Nix3338178770552f80019d2— CI33382258610, Docker33382257942, Nix3338225798913115dd95f— CI33443646095, Docker33443645339, Nix33443645330240f42ff4f— CI33602385323, Docker33602383494, Nix33602383484
Landing-edge cross-check: live main@bfbb34bbec25d40f4982a70748ba390ff5efb363 is 17 commits beyond submitted base fdb2e10a8e606558e10ee5305ca7b74fbb2a3f24; GitHub currently reports mergeable=true, and the current main-only FILE-LIST does not intersect this PR's touched paths.
No new P0/P1/P2 found in this repair. The source/proof train is green.
I am not transferring the older packaged-field UAT to this exact head. The PR's own acceptance record still leaves packaged reconnect UAT for the new path as the final exact-head acceptance gate.
|
Thanks for the substantial work here, and for keeping the local serial round engine intact. We're not ready to take group-chat continuity yet. Group chat on main is still settling (we reverted parallel rounds to serial this week and are working through the open Bot Mode bugs), and landing a hosted-room client of this size on top would complicate that before the base is stable. Leaving this open rather than closing it; we'll revisit in about a month once the current Bot Mode / group-chat bug cluster is cleared. Two things that would make that review tractable when we come back: keep #99107 and #96162 as their own PRs rather than carried in this train, and split the |
Thanks, that makes sense. I’ll keep #99107 and #96162 separate and recut #97846 on current main so it contains only the Desktop continuity client, with no storage relocation. I’ll keep the field stack available for end-to-end testing and revisit continuity in about a month, once the current Group Chat bugs have settled. In the meantime, I’ll see where I can help with that Bot Mode bug cluster. |
240f42f to
d5ad004
Compare
Rebuild NousResearch#100386 as an append-only composition instead of replacing contributor history with a single-author squash. The second-parent train preserves: - David Dudok de Wit's NousResearch#97846/NousResearch#98073 continuity and messaging commits through exact source head 6fb588d. - jugol's source-authored NousResearch#96162 forward port at 22b26d0. - liuhao1024's NousResearch#89180 lease-refresh fix through the adapted source-authored replay 6a74634. The lease-status extraction at e3ec744 keeps gateway/run.py from growing, keeps run_agent.py shrinking, and preserves the original import seams and behavior. This merge commit changes no bytes relative to its second parent. Its first parent is the published NousResearch#100386 head, so the correction is forward-only: no force-push, rebase, amend, or source-history rewrite. Related: NousResearch#97681 Fixes: NousResearch#89166
d5ad004 to
ad1ced2
Compare
Retain exact installation/profile and room/member identity when Desktop is connected only to the authority gateway. Preserve verified local routing through display projection, storage reload and plugin startup, and revoke stale or ambiguous routes without dropping remote participants. Keep the Desktop-only source mirror policy unchanged. Projection tests use actual conversation history; unseeded controls cover silent room discovery, storage reload and rejected verification. Stabilize the existing equal-time thread fixture with message/thread relationships rather than array order. Consolidates five David Dudok de Wit commits reviewed at 9f178a9, without changing the final tree. The original source-port sequence remains on backup/97846-member-identity-reviewed-20260903. Existing contributor commits below ad1ced2 remain unchanged. Source repair lineage: aa07b0b 5b7dcd6 e28e3ea 87cd405 Assisted-by: Codex (OpenAI)
Show membership rather than Desktop-direct peer availability for hosted Group Chats. Retain the real runtime offline/unsupported labels, warning icons and dimming, and leave classic direct-connection counts unchanged. No backend, dispatch or routing changes. Consolidates our 8b57398 and 3efd9f9 repair sequence without changing the final tree. The detailed source history is preserved in backup/98307-availability-before-fold-20260903. Both independent reviewers cleared the actual runtime-to-render cases at the final pin. All 77 Bot Mode files / 769 tests, renderer/Electron/E2E typechecks, scoped lint and formatting pass. The 1cff packaged run motivated this fix; a new packaged label retest remains pending. Speaker-handle and backend publication repairs are separate. Assisted-by: Codex (OpenAI) (cherry picked from commit f7f9769)
Carry the canonical room/member actor through Desktop replay, persistence and bounded display mirrors. Resolve handles from that exact member instead of guessing from profile names, friendly names or connection labels. Keep distinct stable event anchors separate, retain unresolved identity conflicts across repeated mirrors, and reject contradictory event/sequence or payload evidence. Only a bounded unsequenced preview can be completed from a full sequenced replay; a preview cannot extend the Bot's message. Consolidates the resolved speaker repairs; raw reviewed history is preserved. The accompanying source-format cleanup has identical emitted JavaScript. Reviewed source lineage: f6b7cc5e9a3117eba841547ed6edf047247d0786 4a88d05f6b3c49a7ae92e6665935b4b77ec7c1f0 bb3a2d23c99cd13139acaed4341e864596095773 Both original reviewers clear the bounded core repair at bb3a2d23c9. Standalone source-port and packaged validation remain separate gates. No backend, authority, prompt or dispatch contract is changed. Assisted-by: Codex (OpenAI)
Reconcile raw client IDs with accepted user events, validate receipt payloads, retain room and actor identity through import/export/replay, and keep imported display metadata from becoming canonical proof. Preserve real sequenced history and the existing outbox. Consolidates only David Dudok de Wit repair iterations fe93ef2, 86f0c92, f73e64b4b6 and f0046824d3. Raw history is retained on backup/97846-client-raw-20260903; preceding contributor commits are unchanged. Assisted-by: Codex (OpenAI)
Separate stored-history inventory from background-execution eligibility. Retain known hosted IDs, reject stale capability and replay results, preserve warm reads, and allow real recovery while an unsupported cache or first probe is pending. Keep the serial driver and routed command owners intact. Consolidates only David Dudok de Wit repairs a50c2bb302, 6198ae4b96, 83eb520 and c3534b2. Source production tree matches the independently reviewed c353 object; raw history remains on backup/97846-client-raw-20260903. Assisted-by: Codex (OpenAI)
Await the existing zero-delay Radix focus cleanup before Vitest replaces jsdom. Preserve assertions, real timers and unhandled-error reporting. This is the same separately reviewed test-only change as 63fd7ff45b. Final tree is identical to tested source67df0c5967: 82 Bot Mode files and 1017 tests pass with no unhandled errors. This metadata consolidation changes no source or dependency bytes. Assisted-by: Codex (OpenAI)
Compose the reviewed hosted-history and classic-discovery repairs from NousResearch#97846, retaining the integration append options/external flag, reciprocal-control fingerprint and enrollment lifecycle. Preserve stopping-state precedence when moving pure display helpers. Include the separately reviewed test-only dialog cleanup. All folded commits were authored by David Dudok de Wit; preceding contributor history is unchanged. Source publication checkpoint: 95cc573. Raw integration history remains on backup/98307-client-raw-20260903 with original cherry-pick references. Final tree exactly matches tested f86fa76: 86 Bot Mode files and 1065 tests pass with no unhandled errors. No backend changes relative to68e9a09c6d; real-state offline publication recovery was verified separately on that backend. Assisted-by: Codex (OpenAI)
The user problem
A Group Chat should not stop simply because its Desktop viewer closes when the
selected gateways can already own the work and its history. Desktop still needs
to choose that path safely, reconnect to the authority, and fall back without
changing how an ordinary Group Chat behaves.
This PR is now only the Desktop continuity client. It contains no gateway,
storage, replica, or cross-source creation change.
What changes for users
when every selected gateway advertises the complete compatible contract.
room stays on the existing Desktop-driven path.
reconnects, without starting a second local round driver.
when two remote gateways use the same profile name. Reopening Desktop keeps
those identities without borrowing an unrelated local Bot's route.
connections as offline Bots. Speaker details reveal the registered handle;
cached previews cannot invent an author or extend that Bot's actual message.
which group each message belongs to.
overwrite newer connection information or cancel a genuine recovery attempt.
the current room authority with clear in-room states.
and follow the same gateway authority after a connection is replaced.
The existing serial Desktop round engine is unchanged.
Note
Fresh source-package checks now pass for the duplicate-history/reopen case and actual classic two-Bot serial execution. Broader command, reconnect and field testing continues.
Desktop safety boundaries
silent storage failure cannot acknowledge Send, Stop, or disband as durable.
Ordinary commands get a bounded retry and a durable Retry receipt; Stop and
disband remain queued until acknowledged or proven obsolete.
update guidance instead of accepting work into an endless retry queue.
resolve the exact room, member, task, execution generation, and request.
Kept separate by design
This PR works for same-gateway and otherwise-creatable rooms; the common
one-Bot-per-gateway creation path still depends on fix(desktop): enable cross-source group chat creation #96162.
Desktop client stages, replays, and downloads files only after gateways
advertise that complete contract; otherwise new rooms remain Desktop-driven.
are already on main through feat(bot-mode): Group Chats survive Desktop closing — gateway-owned authority and replay (salvage #97712) #99007, feat(bot-mode): same-gateway Group Chats keep running after Desktop closes (salvage #97744) #99099, and feat(bot-mode): Group Chats span gateways — laptop, homelab, and VPS bots in one room (salvage #97797) #99244.
Per maintainer direction, continuity review can resume after the current base
Group Chat bug cluster settles. The complete field stack remains available for
end-to-end testing in draft #98307.
Validation
Head
95cc57398d1cc1a7449d586c14e1e52168e20b66has the identical final tree tested at67df0c5967:Only our unpublished repair iterations were consolidated, into three useful commits: message history, classic discovery/recovery, and test cleanup. Earlier contributor commits are unchanged. The full raw repair history is retained on
dokterdok:backup/97846-client-raw-20260903, alongside the earlier membership/display backups.Earlier packaged evidence and what remains to verify
The standalone source package at
1d17e7580epassed authority-only four-member discovery/reopen (48.5 seconds), registered speaker details (14.5 seconds), and a settled composer/file-chip check (26.9 seconds). No task or gateway was restarted. It also exposed the duplicate-display and non-persistent-host Send-stall cases that led to this repair batch. The gateway accepted the duplicated-looking message only once.Those old successes are not transferred to the new head. The fresh95cc package cases above separately close the duplicate-history and classic serial checks; broader reconnect/command and topology coverage remains open. The existing serial engine, history-only mirror policy and source-owner routing boundaries remain intact.
The earlier
ad1ced247dpassed the broader Desktop UI matrix (723 files / 7,198 tests); this is historical, not a fresh whole-renderer run. Prior Fable and bounded closure reviews remain in the preserved history.Field-stack screenshots
Light:
Dark:
Type of change