fix(review): isolate the background-review fork from the canonical session store - #50296
Closed
arminanton wants to merge 1 commit into
Closed
fix(review): isolate the background-review fork from the canonical session store#50296arminanton wants to merge 1 commit into
arminanton wants to merge 1 commit into
Conversation
alt-glitch
added
type/bug
Collaborator
|
Related: #27190 (merged — isolates the review fork from external memory plugins, the sibling layer) and #32858 (the underlying "background curation prompts leak into user stores" bug). This PR adds the missing session-store isolation layer ( |
teknium1
added
sweeper:risk-session-state
This was referenced Jun 22, 2026
Closed
Contributor
Author
|
Thanks @alt-glitch — agreed, this is the session-store isolation layer ( |
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…n v0.17.0 (3-way merge) Real maintainer operation (per-PR 3-way merge, not naive git apply): 37 of 39 PRs merge clean. 2 combine-conflicts: NousResearch#50457 (redundant 100-file bundle, 94 files dup other PRs -> recommend slim/close) + NousResearch#50296 (ordinary 1-file overlap). The '6 compile failures' are a new-file-absent artifact of excluding the creating PR, not breakage. Each PR also individually clean on v0.17.0 (41/41).
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
… 0 markers, 0 real compile-fail Engineering this round: slimmed NousResearch#50457 (100->4 files, removed the combinability blocker, 9d3b3b9); proved NousResearch#50296 is overlap-only (clean alone on v0.17.0). Two honest corrections: union-merge mishandled a modify/delete (reverted to normal 3-way); the '6 compile failures' were a harness bug running py_compile on files NousResearch#50064 intentionally DELETES (inventory.py). Corrected: 324 .py, 0 real fails.
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…rs) + CI-lint green - combine_and_verify_v017.sh: reproducible, pins each PR to current head SHA. - PINNED-SHAS.txt: every PR's head SHA (incl slimmed NousResearch#50457). - PER-PR-CLEAN-v017.txt: 41/41 individually CLEAN on v0.17.0, 0 compile-fail. - COMBINE-v017.txt: 39 combine via 3-way merge, NousResearch#50296 overlap, 0 markers, 0 real fail. - CI-LINT-RESULT.txt: 'ruff check .' (the blocking CI gate) PASSES on integration tree. - CI-STATUS.md: upstream fork-PR CI is maintainer-gated; local equivalent runs green.
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…uivalent + NousResearch#50296 rebase - RECONCILIATION-41-vs-39.md: 41 = 40 candidates + 1 manifest; 40 = 39 merged + NousResearch#50296. - PER-PR-STATE.md: all 41 base=main, 8 ready/33 draft, head SHAs + mergeable state. - NousResearch#50296 REBASED onto current origin/main (agent_init.py drift: kept both attrs); now GitHub-MERGEABLE (084b79bed). The genuine rebase the prior rounds missed. - CI-EQUIVALENT-RESULTS.txt: ruff check . PASS + ty check PASS + 285 PR tests PASS on the integration tree (warnings = upstream deprecations, not our code).
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…LE) + reproducible delta map - GITHUB-MERGEABLE-AUDIT.md: GitHub mergeable=40/41 (only NousResearch#50111 manifest conflicts). 6 PRs (NousResearch#50296/NousResearch#49644/NousResearch#50041/NousResearch#50073/NousResearch#50064/NousResearch#50033) genuinely conflicted on current origin/main (drifted past v0.17.0); each rebased (1-file complementary conflict), now MERGEABLE. - DELTA-MAP-v017.md: reproducible per-file map (PR diffs vs v0.17.0, fresh tips): 160 = 137 in-PR + 21 DISCARD + 2 upstream-NousResearch#29433 + 0 orphans, sum verified.
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…afety proof Addresses the Council demand for SEMANTIC correctness (tests pass after resolution), not just compile-clean. Per-PR own-test results on resolved v0.17.0 (V017-PER-PR-TEST-RESULTS.txt): NousResearch#49644 10 passed | NousResearch#50033 165 gemini passed | NousResearch#50056 218 passed NousResearch#50073 9 passed (INTENT-INFERRED resolution PROVEN correct: hygiene=400 kept) NousResearch#50296 13 passed | NousResearch#50064 61 passed, 1 FAILED The single NousResearch#50064 failure is characterized precisely (not hacked): test test_routed_client_preserves_openai_sdk_default_headers asserts pre-v0.17.0 copilot routing internals that v0.17.0 ITSELF removed (commit 8d59881 / NousResearch#2647 deleted both the test and the routed-default_headers behavior; pure v0.17.0 has 0 occurrences). Documented as a forward-compat test-removal on rebase, not a regression. NousResearch#50064's feature is intact (61/62). DISCARD ratification (NON-CONTRIBUTABLE.md): git grep across all real src code files finds 0 references to any of the 25 DISCARD files (.bak/.project-intel/transcripts) — proof they are safely droppable. v017-conflict-resolutions/README.md documents all 6 resolution strategies.
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…open questions Addresses the Council demand for platform/review state, not just local repro. Pulled directly from GitHub for all 42 PRs (PER-PR-PLATFORM-STATUS.txt): - 42/42 OPEN (8 ready-for-review, 34 draft). - head-SHA: 41/41 feature PRs' GitHub headRefOid == local PINNED-SHAS resolved SHA. NousResearch#50111 is 'self-ref' (PINNED-SHAS is committed inside it; live head = f79affa). - CI: public repo runs no fork-branch PR workflow (all 'no-checks'); local CI-equivalent green (ruff + compile + per-PR tests, see V017-PER-PR-TEST-RESULTS). - External review feedback: ONLY 4 'Related: #X' cross-refs from @alt-glitch on NousResearch#49449/NousResearch#50086/NousResearch#50155/NousResearch#50296 — each CONFIRMS the PR is distinct/non-duplicate, NOT change-requests. 0 unaddressed review threads, 0 submitted reviews. - The 6 conflict PRs are MERGEABLE on their own base (origin/main); the v0.17.0 resolutions correctly live as patches on NousResearch#50111 (NOT force-pushed to PR branches, which would break body==diff against the PR base). Two user-only ratifications stated as EXPLICIT OPEN QUESTIONS (not assumed-resolved): Q1 grouping intent; Q2 accept NousResearch#50064's v0.17.0 forward-compat test drop (NousResearch#2647).
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
…script Addresses Council items 2 & 3: Item 3 (justify every resolution against PR intent): V017-RESOLUTION-JUSTIFICATION.txt proves, by set-membership, that ALL PR-added lines (vs origin/main) are present in each of the 6 v0.17.0 resolutions, with per-PR tests on the resolved tree: NousResearch#49644 take-theirs(superset) 10 passed | NousResearch#50033 take-theirs compile+165 gemini | NousResearch#50056 keep-both 218 passed | NousResearch#50073 keep-400 9 passed (3 keys verified present, hygiene line was UNCHANGED context so keeping 400 loses no intent) | NousResearch#50296 take-theirs 13 passed | NousResearch#50064 take-theirs 17/18 (1 = v0.17.0 upstream removal NousResearch#2647, NOT lost intent = Q2). Item 2 (delivery mechanism): APPLY-RESOLUTIONS-ON-v0.17.0.sh is the SIDECAR deliverable — one command materializes all 42 PRs + the 6 documented resolutions onto v0.17.0. Verified end-to-end: 0 residual markers, 0 compile failures. Sidecar (not force-push into PR branches) is the defensible default: the 6 PRs target origin/main where they are MERGEABLE; committing v0.17.0-specific resolutions onto them would make body != diff against their own base. The user can still choose to commit-into-branches; this script makes the pull-down deterministic either way.
arminanton
added a commit
to arminanton/hermes-agent
that referenced
this pull request
Jun 22, 2026
Each PR-<n>-onto-v0.17.0.patch makes its forward-port-conflict PR independently pullable onto v0.17.0 (2bd1977): the PR's content WITH its documented conflict resolution baked in. Verified APPLIES-CLEAN on a fresh v0.17.0 checkout + tests pass (NousResearch#49644:55, NousResearch#49916:279, NousResearch#50056:218, NousResearch#50064:13, NousResearch#50073:9, NousResearch#50296:code-only). Delivered as manifest patches, NOT branch pushes — the PR branches target main where they are already conflict-free; a v0.17.0 resolution on a main-targeted branch would corrupt it against main and noise the review queue. 0 private leaks.
…n store
The background skill/memory review fork shares the parent agent's session_id
(for prompt-cache warmth), so without isolation it wrote its harness turn
("Review the conversation above and update the skill library…") plus its
curator-mode reply straight into the user's REAL session in state.db. On the
user's next live turn the agent re-read that injected user message as a
standing instruction and "became" the curator, refusing the actual task.
Two layers:
1. Prevention: an `_persist_disabled` flag on the fork hard-stops every DB
write/lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall). The review writes only to the skill/memory
stores via its tools, which is all it needs.
2. Defense-in-depth on read: _strip_background_review_harness drops any stray
harness message (and the curator reply that followed it) when loading
session history, so a session polluted by an older build resumes clean.
Adds 13 unit tests for the read-side stripper.
arminanton
force-pushed
the
fix/background-review-session-isolation
branch
from
June 23, 2026 01:03
084b79b to
15bebf3
Compare
arminanton
changed the title
kshitijk4poor
pushed a commit
that referenced
this pull request
Jul 1, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from #50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
hashbender
added a commit
to hashbender/hermes-agent
that referenced
this pull request
Jul 1, 2026
…n (salvage NousResearch#50296) (#306) Co-authored-by: qbit-mirror-bot <qbit-mirror-bot@users.noreply.github.com>
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
Jasper6439
pushed a commit
to Jasper6439/hermes-agent
that referenced
this pull request
Jul 5, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
habarmc1223-sudo
pushed a commit
to habarmc1223-sudo/hermes-agent-fluxmem
that referenced
this pull request
Jul 8, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
santhreal
pushed a commit
to santhreal/hermes-agent
that referenced
this pull request
Jul 13, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
leewenjie
pushed a commit
to leewenjie/hermes-agent
that referenced
this pull request
Aug 7, 2026
…ssion
The forked skill/memory review agent shares the parent's session_id for
prompt-cache warmth. Without isolation it wrote its harness turn ('Review the
conversation above and update the skill library…') plus its curator-mode reply
straight into the user's REAL session in state.db; the next live turn re-read
that injected user message as a standing instruction and the agent 'became' the
curator, refusing the actual task.
Root fix: a _persist_disabled flag on the fork that hard-stops every DB write
and lazy-open path (_flush_messages_to_session_db, _ensure_db_session,
_get_session_db_for_recall) — the review writes only to the skill/memory stores
via its tools. Defense-in-depth: _strip_background_review_harness drops any
stray harness message (and the assistant reply that followed) at load time in
get_messages_as_conversation, so an already-polluted session resumes clean.
Salvaged from NousResearch#50296.
Co-authored-by: arminanton <29869547+arminanton@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
If you use the background skill/memory review feature, you may have hit a strange failure where, on the turn after a review fires, the agent suddenly stops doing what you asked and instead starts acting like a librarian: "Let me review the conversation and update the skill library…", refusing the actual task. I kept hitting this and finally traced it.
Root cause: the background review runs in a forked agent that shares the parent's
session_id(deliberately, for prompt-cache warmth). But the fork also wrote its own turns into that shared session. So the harness prompt it runs under ("Review the conversation above and update the skill library…") plus its curator-mode reply landed in your real session history. On your next live turn the agent re-read that injected user message as a standing instruction and "became" the curator. The session was effectively hijacked by its own background helper.The fix (two layers)
Prevention (
_persist_disabled): a per-agent flag set on the review fork that hard-stops every path that could write to or lazily open the canonical session store._flush_messages_to_session_db,_ensure_db_session, and_get_session_db_for_recallall short-circuit when it's set. The fork still does its real job (writing to the skill and memory stores via its tools); it just can't touchstate.dbor the JSON snapshot anymore.Defense-in-depth (
_strip_background_review_harness): a load-on-read filter so any session already polluted by an older build resumes clean. It drops the harness message and the curator-mode assistant reply that immediately followed it, leaving everything else untouched and in order.Tests
tests/test_background_review_session_isolation.pycovers the matcher (skill + memory prompt variants, leading-whitespace tolerance, role/non-string guards, no false positives on normal "please review my PR…" messages) and the stripper (removes harness + its reply, preserves order, handles the empty and no-match cases). 13 tests, all green.Relation to existing work
Related: #27190 (merged) isolates the review fork from external memory plugins, the sibling layer. #32858 is the underlying "background curation prompts leak into user stores" bug report. This PR adds the missing session-store isolation layer specifically: the
_persist_disabledwrite/lazy-open hard-stop plus the read-side harness stripper. It's complementary to #27190, not a duplicate, the two cover different stores.