Skip to content

fix: resolve issue #3340#3342

Closed
rly09 wants to merge 1 commit into
nesquena:masterfrom
rly09:fix-issue-3340
Closed

fix: resolve issue #3340#3342
rly09 wants to merge 1 commit into
nesquena:masterfrom
rly09:fix-issue-3340

Conversation

@rly09
Copy link
Copy Markdown
Contributor

@rly09 rly09 commented Jun 1, 2026

This pull request adds a new feature that displays a small success toast notification in the chat UI whenever an agent turn saves memory or creates/updates a skill, making persistent state changes visible to users during normal WebUI turns. The implementation includes both backend detection of persistent state changes and frontend toast display logic, with tests to ensure correct behavior.

Persistent State Change Detection and Notification:

  • The backend (api/streaming.py) now snapshots and compares key memory and skill files before and after each agent turn to detect if memory was saved or a skill was created/updated. When such changes are detected, a state_saved event is emitted via SSE with relevant details. [1] [2] [3]

  • The frontend (static/messages.js) listens for state_saved events and displays a deduplicated toast notification to the user, using user-friendly labels for memory and skill changes. It also heuristically detects persistent state changes from tool calls for additional robustness. [1] [2] [3] [4]

Testing and Documentation:

  • A new test file (tests/test_issue3340_persistent_state_toasts.py) verifies backend detection, frontend notification logic, deduplication, user-visible labels, and presence of a changelog entry.

  • The changelog (CHANGELOG.md) documents the new feature for visibility.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Summary

Read the diff at this PR against origin/master for api/streaming.py, static/messages.js, plus the agent-side memory layout. The backend half is sound: _persistent_state_snapshot (api/streaming.py) captures (mtime_ns, size) signatures for memories/MEMORY.md, memories/USER.md, SOUL.md, and skills/**/SKILL.md, then _persistent_state_changes diffs the before/after snapshots and emits state_saved SSE events. I verified the file paths match the agent contract — tools/memory_tool.py:153-154 reads _mem_dir = get_hermes_home()/"memories" then MEMORY.md/USER.md, and hermes_cli/profiles.py:65-67 lists memories/MEMORY.md, memories/USER.md, SOUL.md as the curated identity files. So the snapshot targets are correct.

One real bug: cross-turn dedupe is too aggressive for memory

static/messages.js:77 declares the dedupe set at module scope and it is never cleared:

const _persistentStateToastSeen=new Set();

The dedupe key for a memory save (_showPersistentStateToast, ~line 145) is:

const dedupeKey=[
  S&&S.session&&S.session.session_id||'',
  normalizedKind,
  itemName||'memory',
].join(':');

For kind==='memory', itemName is always '', so the key collapses to "<sid>:memory:memory" for the entire page-load lifetime. That means only the first memory save in a session ever toasts — turn 1 saves memory and you see "Memory saved"; turns 5, 12, 30 each genuinely rewrite MEMORY.md/USER.md (real state_saved events fire from the backend) but the toast is silently suppressed. Since the feature's whole point is "the user has no idea persistent state changed" (#3340), swallowing every save after the first defeats it.

Skills are keyed by name so they're less affected, but the same set still suppresses a legitimate second update to the same skill later in the session.

Recommendation

Make the dedupe per-turn, not per-session. The simplest fix is to fold the active stream/turn id into the key and let the set stay bounded by the existing 200-entry LRU, e.g.:

const turnTag = (S && S.activeStreamId) || (S && S.session && S.session.lastTurnId) || '';
const dedupeKey=[turnTag, sid, normalizedKind, itemName||'memory'].join(':');

That keeps the intra-turn dedupe (backend state_saved + the _maybeNotifyPersistentStateSaved tool-call heuristic both firing for one save → one toast) while letting each new turn's saves toast again.

Second concern: double-detection path

Both the SSE state_saved listener (~line 1826) and the tool_complete heuristic _maybeNotifyPersistentStateSaved(tc) (line 1805) can fire for the same save. Within one turn the shared dedupe set collapses them, which is the intent — but it relies on _persistentToastSkillName extracting the same itemName the backend derived from the path (Path(rel).parent.name). If the tool-call args name and the directory name differ (skill stored under a slugged dir), you get two skill toasts. Worth a test asserting the SSE path and the heuristic path produce identical dedupe keys for a representative skill save.

Test plan

The added tests/test_issue3340_persistent_state_toasts.py covers backend detection and the presence of i18n keys, but doesn't exercise the cross-turn memory case. Suggest adding a JS-string assertion (or jsdom test) that a second memory state_saved with the same sid still toasts once the turn tag changes — that's the regression that the current module-level set would silently break.

nesquena-hermes added a commit that referenced this pull request Jun 4, 2026
## Release v0.51.260 — Release IB (stage-r8)

Un-held safety fixes (author resolved my earlier hold findings; re-reviewed fresh) + a clean fix batch. 6 PRs.

### Fixed
| Issue/PR | Author | Fix |
|----------|--------|-----|
| #3535 (#3538) | @rodboev | **Self-update recovers from a stash-pop conflict without data loss.** Was a BRICK bug (`git reset --merge` + `git stash drop` discarded local mods while reporting success). Now keeps the stash, returns `ok:false` + "preserved in `stash@{0}`", no restart on conflict. *(was held — fix verified)* |
| #1909 s3 (#3562) | @rodboev | **Auth `Secure` cookie no longer locks out plain-HTTP LAN/Tailscale users.** Secure now keys only on real TLS evidence (env / TLS socket / opt-in `TRUST_FORWARDED_PROTO`); non-loopback plain-HTTP is no longer force-Secure. SameSite back to `Lax`. *(was held — fix verified)* |
| #2785 (#3559) | @franksong2702 | Clearer cron/gateway diagnostics for single-container Docker (gateway configured, no daemon → jobs silently don't fire). |
| #3555 | @lambyangzhao | Long TTS responses chunked at sentence boundaries (works around the browser's ~32K silent-truncation). |
| #3340 (#3342) | @rly09 | Persistent-state toast when a turn has saved memory / created-updated a skill. |
| #3533 | @franksong2702 | `/reload-mcp` marked `cli_only` so the WebUI doesn't dispatch it as an LLM prompt. |

### Gate
- Full pytest suite: **7681 passed, 0 failed**
- ESLint: CLEAN · ruff: CLEAN · browser-smoke: CLEAN
- Codex (regression): **SAFE TO SHIP** — confirmed the stash-conflict path never drops the stash / never restarts on conflict, auth Secure handles LAN-HTTP correctly with no header-forgery hole, `/reload-mcp` allowlisted, state-toast has a real backend writer + active-session guard, diagnostics leak no paths, TTS chunking preserves order.

Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
Co-authored-by: lambyangzhao <lambyangzhao@users.noreply.github.com>
Co-authored-by: rly09 <rly09@users.noreply.github.com>
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Shipped in v0.51.260 (Release IB) — thank you! 🙏 (persistent-state toasts (memory/skill saves visible).) Closing as merged-via-release-stage.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants