fix(desktop/bots): keep a substantive group reply after a synthetic (pass)

[chelsealong]

What does this PR do?

Fixes a Bot Mode group chat bug: a completed, substantive member reply can be silently discarded when the Codex intent-ack continuation guard fires on it.

Sequence that triggers the bug:

1. A member gives a complete, short group answer that happens to contain a future-tense ack phrase ("I'll ...") plus an action word ("review"), and the generated group-turn prompt contains workspace/project vocabulary.
2. agent/conversation_loop.py treats that as an unfinished Codex intermediate ack and appends _CODEX_ACK_CONTINUATION_NUDGE as a synthetic follow-up user message.
3. The member has nothing left to add and returns (pass) to the nudge.
4. runGroupChatMemberTurn (and the analogous stranded-turn path, harvestStrandedGroupReply) in apps/desktop/src/plugins/hermes-bots/plugin.js scanned backward for the last assistant message and returned it as-is — which is the synthetic (pass), not the real answer. The room shows the member working, then passing, with the actual answer never appearing.

Both functions now scan the messages appended during the current turn for the last substantive (non-pass) assistant reply, and only fall back to a pass when no substantive reply exists in that window. A genuine pass-only turn (no prior substantive answer) is unaffected and still reads as silent.

This is the Bot Mode-side fix option from the two proposed in the issue — it's self-contained to the plugin and doesn't touch the agent-core intent-ack heuristic, which still needs to keep firing for genuinely unfinished action announcements in ordinary task sessions.

Related Issue

Fixes #94376

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• apps/desktop/src/plugins/hermes-bots/plugin.js
  • New helper pickGroupTurnReply(messages, before): scans messages appended since the turn's baseline newest-first, preferring the last substantive (non-pass) assistant reply over a trailing pass.
  • runGroupChatMemberTurnLeased's finished-turn branch now uses this helper instead of returning the first (i.e. last-chronological) assistant message it finds.
  • harvestStrandedGroupReply's late-reply scan uses the same helper, fixing the identical bug for replies that land after the turn timed out.
• apps/desktop/src/plugins/hermes-bots/tests/group-turn-lease.test.mjs
  • Extended the test harness with a replyMessages option to stage a multi-message turn (substantive answer → synthetic continuation → pass).
  • Added: a substantive answer followed by a synthetic continuation (pass) still surfaces the answer.
  • Added: a genuine pass-only turn still reads as silent (no regression).

How to Test

1. node --test apps/desktop/src/plugins/hermes-bots/tests/group-turn-lease.test.mjs
2. node --test apps/desktop/src/plugins/hermes-bots/tests/*.test.mjs (full plugin suite)

Proof the new test fails without the fix

Reverted plugin.js to HEAD~0 (pre-fix) while keeping the new test, then ran the targeted test file:

not ok 8 - a substantive answer followed by a synthetic continuation (pass) still surfaces the answer
  error: |-
    Expected values to be strictly equal:
    + actual - expected

    + '(pass)'
    - "Yes. I welcomed them, and I'll review their first assignments with them."

The genuine-pass regression test (a genuine pass-only turn (no prior substantive answer) still reads as silent) still passed in that same run, confirming the assertion actually depends on the fix rather than being a tautology.

Full suite with the fix applied

$ node --test apps/desktop/src/plugins/hermes-bots/tests/*.test.mjs
...
1..558
# tests 558
# pass 558
# fail 0

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — N/A, this is a desktop-plugin-only (JS) change; see the node --test output above instead
• I've added tests for my changes
• I've tested on my platform — not run in an Electron shell in this sandbox; verified via the plugin's node --test harness only

Documentation & Housekeeping

• I've updated relevant documentation — N/A, behavior-only fix, no user-facing docs affected
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact — pure renderer/plugin JS, platform-independent
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

AI assistance disclosure

This fix was authored by an AI coding agent (Claude) operating autonomously against the public issue, with the diff and test results verified before pushing.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Nice refactor-plus-fix: consolidating three copies of the assistant-message text extraction into pickGroupTurnReply, scanning newest-first, and preferring the last substantive answer over a trailing synthetic "(pass)" matches the #94376 failure mode exactly, and both new tests pin the two sides of the contract.

Two observations:

1. The harvestStrandedGroupReply rewrite quietly narrows its scan window — the old loop walked the entire log back to index 0; the new one only considers [strandedBefore, end) (plugin.js:7513). For the common case that's identical, but a stranded member whose range contains only a pass while an older substantive reply sits before strandedBefore used to get nothing (old code returned on the first assistant message regardless) — actually delivered nothing either way — still, the reachable difference is: old code could return a pass-only tail and skip delivery, new code can surface a substantive reply from earlier in the same window. That looks strictly better, just worth stating explicitly in the description since it changes harvest semantics beyond the trailing-pass case.

2. Second call site lacks direct coverage — the tests exercise runGroupChatMemberTurn; add one for harvestStrandedGroupReply with an [answer, nudge, (pass)] tail so the rescued-delivery path there is pinned too.

Minor: when only passes exist, the helper returns the newest pass text (first seen scanning backward) — fine, just document the tie-break choice in the docstring.

[chelsealong]

Addressed the two actionable review points: added the missing test coverage for harvestStrandedGroupReply's rescued-delivery path (substantive answer → synthetic continuation nudge → (pass) tail), verified it fails on pre-fix plugin.js and passes with the fix (559/559 suite), and documented the pass-only tie-break (newest wins) in pickGroupTurnReply's docstring. Left the harvest-semantics note as-is since it's already spelled out precisely in the review thread itself.

[chelsealong]

CI note on the failing JS & TS checks run (job 97760097266, commit d3a1f75): it's not this PR's diff. check:test:ui reports 5729/5729 tests passing, but the run then fails on one uncaught exception:

ReferenceError: window is not defined
 ❯ resolveUpdatePriority ../../node_modules/react-dom/cjs/react-dom-client.development.js:1308:7
 ❯ dispatchSetState ../../node_modules/react-dom/cjs/react-dom-client.development.js:9126:14
 ❯ Timeout._onTimeout src/components/assistant-ui/thread/user-edit-composer.tsx:606:9

That's the window.setTimeout(() => setSubmitting(false), 200) in user-edit-composer.tsx:605-607 firing after user-message-edit.test.tsx's jsdom environment has already torn down — a cross-test-file timer leak, not an error in that test file itself. Neither file is touched by this PR (only apps/desktop/src/plugins/hermes-bots/plugin.js and its two test files changed here), and running user-message-edit.test.tsx alone passes 9/9. I don't have push access to NousResearch/hermes-agent to re-run the job myself.

[teknium1]

Merged via #96239 (rebase-merge) — your selection fix is on main as-authored: a588685. Thanks @chelsealong! Keeping the last SUBSTANTIVE non-pass assistant message (instead of the raw last message) was the deeper half of the sentinel seam — it stops Codex intent-ack scaffolding from shadowing real answers into silent turns. Landed together with #94310's (empty) normalization as one consolidated fix; your regression tests rode along.