Conversation
|
@teknium1 @kshitijk4poor Friendly review bump when you have a slot. Still needed on main: a gateway row with Happy to rebase on request. main moves fast so I stopped auto-rebasing. Focused hermes_state tests green on head |
…arch#109073) A main gateway session (session_key set) must never carry _delegate_from — that marker hides the row from every picker while the gateway keeps routing into it. Salvage of kyssta-exe NousResearch#109081 (strip at insert/merge + startup heal) plus regression tests for insert, merge, child-keep, and heal paths.
…arch#109073) Align insert/merge empty-session_key predicates with the heal SQL, drop the defensive merge try/except in favour of row["session_key"], and lock the remaining edge cases (caller-dict immutability, sole-marker → NULL, heal leaves children alone, merge on missing row).
c00e407 to
76f9cac
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Moderate issues remain in session write paths and startup healing that can leave gateway rows polluted or unrepaired.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Fixes polluted gateway sessions so active conversations remain visible in session pickers.
Changes:
- Strips
_delegate_fromduring gateway session writes. - Adds startup healing for existing polluted rows.
- Adds regression coverage for gateway and delegate-child behavior.
| File | Summary |
|---|---|
tests/hermes_state/test_gateway_delegate_marker_strip.py |
Adds regression tests; two NULL-cleanup assertions need tightening. |
hermes_state_sessions.py |
Adds write-time safeguards; additional replacement, late-binding, and upsert paths need the same invariant. |
hermes_state_schema.py |
Adds startup healing; malformed JSON, migration ordering, and lock-retry handling require changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…Research#109073) Co-authored-by: Adrian Martén <Adrian.marten@outlook.com>
|
Ready for re-review on
The current GitHub Actions runs are |
|
@teknium1 @kshitijk4poor Gentle bump before I go quiet for a bit. Still ready for re-review on Happy to rebase if main has moved. Otherwise this one is waiting on eyes. |


Symptom
A main gateway session (
session_keyset, e.g.agent:main:telegram:dm:<user>) can end up withmodel_config._delegate_from. That marker excludes the row from every picker (list_sessions_rich,list_recent_sessions_bounded,/resume) while the gateway keeps routing into it bysession_key— chat keeps working, but the conversation vanishes from the desktop sidebar.Often observed together with
_reset_fromon the same row (contradictory pair). Opposite direction of #103789 / PR #105284 (children leaking into the sidebar).Closes #109073.
Approach (salvage of #109081)
Takes @kyssta-exe's write-time + heal approach from #109081 and rebases it onto current
main, then hardens it against the review findings:_delegate_fromfrom keyed gateway rows on insert, merge, and fullupdate_session_meta()replacement paths; emptysession_keyis treated like NULL._heal_polluted_gateway_delegate_markersruns idempotently on open, uses safe JSON extraction, guards removal withjson_valid, and collapses an empty result to SQLNULL.session_key) retain the marker; malformed sibling JSON is preserved and cannot abort healing of valid polluted rows.NULLacross merge, replacement, and startup-heal paths.Credit: original design and implementation in #109081 by @kyssta-exe. This PR adds the invariant tests and lands the same fix shape on current main so #109081 can close as superseded once this merges.
Test plan
Verified on current HEAD
671d2fedwith the canonical runner:How to verify manually
SessionDB:create_session(..., session_key=..., model_config={"_delegate_from": parent, "_reset_from": parent})→ row must not store_delegate_fromand must appear inlist_sessions_rich(min_message_count=1).patch_session_model_config(..., {"_delegate_from": parent})→ marker must not stick; row stays listable.update_session_meta(..., model_config_json=...)on a keyed row → marker must not stick; sole-marker replacement stores SQLNULL._delegate_fromon a keyed row beside a malformed-JSON row, reopenSessionDB→ valid row heals and returns to the list; malformed sibling and delegate child remain unchanged.