Skip to content

fix(bot-mode): clear resolved Group Chat attention and bind prompts across rename - #105960

Merged
teknium1 merged 5 commits into
mainfrom
fix/group-attention-salvage
Sep 8, 2026
Merged

teknium1 merged 5 commits into
mainfrom
fix/group-attention-salvage

Conversation

@teknium1

@teknium1 teknium1 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Derive Group Chat question/approval attention from $groupClarify; keep $groupNeedsYou exclusively for independent mention attention. Resolving the last prompt clears the question cue without losing @user attention.
  • Keep pending prompts and their async owners on the same authoritative room across rename. Retire those owners permanently on disband, including late rejected polls and multi-member background harvests; no same-name replacement-room writes.
  • Preserve existing Bot Chat cues. No mute, pause, cooldown, new attention heuristics, or room-ordering behavior.

Refs #94726. This is a regular review PR, not authorization to merge or close the tracker/originals.

Salvage and overlap

Preserves @sprmn24's authored commit from #101452, head e5038f12ccc1a4d1fb56ecc5f0e1a884f1e8b795, then adds class-completion work. Contributor email mapping was already present and verified. Credit @kokhlo's #101591 for independently diagnosing the in-flight rename race; this uses scoped room-record bindings instead of copying its persistent alias map.

Read the full source PR thread/reviews/diff and related #101591/#93903/#93993; refreshed the open-PR duplicate sweep. #105909 owns room ordering: the attention change preserves its ordering seam, and the subsequent mechanical roster decomposition keeps every ordering predicate unchanged. No duplicate ordering implementation.

Validation

  • Final source fffacaca53eb0b906190faca82cda868381228ff: complete hermes-bots suite run serially — 62 files, 579 tests passed; renderer TypeScript, touched-file ESLint, Windows-footgun, compatibility-pointer and whitespace checks passed.
  • Exactly two new invariant tests overall, extended rather than multiplied. The lifecycle matrix now checks activity, bot needs-attention, independent mention attention and room isolation for rejected session setup and prompt submission in normal and continuation phases.
  • Independent review found the stale failed-event blocker after the earlier review. Both member catches now guard room retirement before failure publication; the scoped follow-up independent review found no remaining blocker. The supplied rejection/subscription probes pass. Its separate absent-room assertion is intentionally not adopted: direct member-turn APIs support lazy initial room creation, which it would break.
  • Native Electron A/B against pre-correction 442d95a8: all four setup/submit rejection cases formerly emitted stale failure and bot-attention into a live recreated room; final source preserves both stores. Actual disband confirmation clicked. Legacy no-roomId normal turn still delivers Completed reply; pending/resolved/mention-only roster cues remain 1 / 0 / 1.
  • Tier disclosure: actual Electron renderer and production components, scripted deterministic host RPC snapshots, not live model inference/full installed gateway. Existing canonical Bot Chat live proof remains separate and unchanged.

Follow-up evidence: exact-source report, serial suite, baseline failure, native receipts and shape metrics

Shape gate Before lines / CC After lines / CC
Round driver 432 / 75 174 / 30
Leased member turn 228 / 57 67 / 8
Extracted polling phase 118 / 28
BotsPane 835 / 85 293 / 21

Behavior correction and mechanical extraction are separate commits. All touched production functions and new siblings pass the 300-line/CC30 gates; no facade behavior, re-export shims, aliases or generic registries were added.

Evidence report, native harness, logs and before/after screenshots. Hosted screenshots were fetched back and hash-verified. Earlier rejected HMR/fixture runs are excluded.

Case Before After
Resolved prompt stale cue cleared
Pending through rename old-name prompt follows room
Late poll after disband stale prompt state no room or prompt

Review notes

The larger textual diff separates existing round-member/prompt/preparation/polling phases and roster derivation/lifecycle/rendering. Explicit typed component boundaries increase aggregate LOC; this is a shape decomposition, not a net-deletion claim. Roster parity checks preserved 169 predicates, 5 complete effect calls and 19 helper bodies. Existing Node localStorage/Vite and Python invalid-escape warnings are unchanged. No screenshots or probe harnesses are committed. No backend/wire contract change.

Infographic

Group Chat authoritative attention

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 8, 2026
sprmn24 and others added 4 commits September 8, 2026 13:29
…rify instead of duplicating it into $groupNeedsYou

$groupNeedsYou was written by two independent sources: an @user mention
(appendGroupChatEntry) and, until now, syncGroupClarify whenever a member
blocked on a clarify or approval. Nothing kept the two in sync, so every
path that consumed a clarify -- answering it, the server resolving it,
disbanding or renaming the room -- left a stale badge lit with nothing
behind it, because none of them cleared the copy syncGroupClarify had
written. A naive fix (writing false back into $groupNeedsYou on every
clarify cleanup path) would have created the opposite bug: clearing an
unrelated, still-unresolved @user mention.

$groupClarify is already the source of truth for pending clarify/
approval attention. syncGroupClarify no longer writes $groupNeedsYou; a
new groupHasPendingClarify(clarifies, group) derives the same signal
from a $groupClarify snapshot. It's intentionally pure -- the caller
(roster-pane) subscribes to $groupClarify itself via useValue and passes
the live snapshot in, so the subscription actually drives the
recalculation instead of existing only to force a re-render. A prompt
resolving, being answered, or its room being disbanded all self-correct
through the existing $groupClarify cleanup paths, with nothing left to
keep in sync.

Rename needed its own fix: the old clearGroupClarify(oldName) call on
rename dropped a clarify-only room's attention entirely, since clarify no
longer lives in $groupNeedsYou to be carried over by the existing
old->new key swap there. New renameGroupClarify(oldName, newName) re-keys
matching mirrors onto the new name in two passes -- unrelated entries
first, migrated entries last -- so a stale mirror already stranded at the
destination key (left behind by a known, separate in-flight-poll race)
can never clobber the just-migrated current prompt.

The roster now reads groupNeedsYou[group] || groupHasPendingClarify(...)
and subscribes to both stores so either one repaints the row.

Tests: multi-clarify sequencing, mention+clarify independence (resolving
one must not clear the other), server-side resolution cleanup, rename
migration, rename destination-collision (red-green verified against the
prior single-pass implementation), a GroupRow badge render test, and a
subscription harness using the real stores/useValue/helper end-to-end.

(cherry picked from commit e5038f1)
Complete the PR #101452 salvage, preserving sprmn24 authorship. Credit @kokhlo PR #101591 for independently diagnosing the in-flight rename race; use scoped authoritative room bindings rather than persistent aliases. Keep mention attention independent and retire late work after disband.
@teknium1
teknium1 force-pushed the fix/group-attention-salvage branch from fffacac to f9993d5 Compare September 8, 2026 20:29
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on bc952a7 — fix: integrate group attention with room ordering and workin

⚠️ Warnings

OSV vulnerability scan · View job

33 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.


debug info

CI timings

CI timings · View report · View job

Wall time 6m27s vs 5m36s (+15.2%). 4 job(s) slower, 11 faster, 1 unchanged.

  • OS-specific tests / Windows-only tests: +77.0s
  • JS & TS checks / JS & TS checks: -40.0s
  • OSV scan / Emit review status: -28.0s
  • OSV scan / Scan lockfiles / osv-scan: -21.0s
  • Python tests / Run tests: -12.0s

@teknium1
teknium1 merged commit f03ed94 into main Sep 8, 2026
35 checks passed
@teknium1
teknium1 deleted the fix/group-attention-salvage branch September 8, 2026 21:07
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/*) 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.

3 participants