Skip to content

fix(agent): detect SOUL.md drift on session restore via the shared identity resolver - #72253

Open
Sora-bluesky wants to merge 1 commit into
NousResearch:mainfrom
Sora-bluesky:fix/issue-68563-v3
Open

fix(agent): detect SOUL.md drift on session restore via the shared identity resolver#72253
Sora-bluesky wants to merge 1 commit into
NousResearch:mainfrom
Sora-bluesky:fix/issue-68563-v3

Conversation

@Sora-bluesky

@Sora-bluesky Sora-bluesky commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What changed

This PR no longer rebuilds the system prompt when it detects SOUL.md drift during session restore. agent/conversation_loop.py is back to the main behavior (the three restore-path hunks from the original commit are reverted, net diff zero).

Identity drift detection moved to the one place AGENTS.md:19-23 already allows the system prompt to change: the Hermes-native compaction keep-prompt gate in agent/conversation_compression.py. That gate used to keep the cached prompt whenever built-in memory state matched. It now also checks stored_identity_is_stale() and falls through to a full rebuild when the identity block no longer matches what's on disk. The rebuilt prompt gets persisted the same way any other compaction-time prompt already does: update_system_prompt writes it directly when compaction runs in place, and publish_compression_child carries it along when compaction rotates to a new session id.

One related fix: resolve_identity_block() in agent/system_prompt.py treated a missing HERMES_HOME directory the same as a HERMES_HOME that exists but has no SOUL.md in it, both landing on checkable=True and a default identity. Those are different failure modes. If HERMES_HOME itself isn't there, we can't say anything meaningful about identity, so that case now reports checkable=False and the staleness check fails open (no rebuild) rather than confidently concluding "identity is default."

Why not fix it at restore time

The original approach rebuilt the prompt inside _restore_or_build_system_prompt, which runs whenever agent._cached_system_prompt is unset (agent/turn_context.py:613-615). AGENTS.md:19-23 states the system prompt is never rebuilt mid-conversation except for the compression exception, because doing so busts prompt caching and multiplies request cost. Restore isn't that exception; compaction is.

Limitation: sessions running the app-server runtime

Sessions using api_mode == "codex_app_server" take a different compaction path (_compress_context_via_codex_app_server), and that path has no system-prompt write-back at all, on main or in this PR. Adding a staleness check there wouldn't do anything, because Hermes never transports a system prompt to a CAS session in the first place, new or existing: the app-server turn path skips active_system_prompt entirely (agent/conversation_loop.py:1249-1256), CodexAppServerSession.__init__ (agent/transports/codex_app_server_session.py:274-311) takes no prompt argument, and the per-turn call (agent/codex_runtime.py:698, run_turn(user_input=...)) sends only the user's message. This PR leaves that path untouched. If you're running against the app-server runtime, editing SOUL.md has no effect on the session through Hermes at all; prompt delivery for that runtime is a separate gap, out of scope here.

About issue #68563

The issue asks for SOUL.md edits to reach a running session right away. What this PR delivers is narrower: on the default (non-app-server) configuration, an edit reaches the session at the next Hermes-native compaction, not immediately, and it's persisted to the database when it does. Immediate reflection is a different feature: AGENTS.md:19-23 disallows rebuilding the system prompt mid-conversation outside the compression exception, so restore was never a place this could live. Getting from "eventually, at compaction" to "on demand" needs a command a user invokes on purpose, so the cost of rebuilding is one they choose to pay rather than one that fires on every turn regardless of the input file.

Two things could be split out as follow-ups, out of scope here:

  1. An explicit reload command (something like /reload-soul) as a real answer to "I want this applied now."
  2. Actual prompt delivery to CAS sessions, so identity drift could eventually reach sessions running that runtime too. Write-back alone wouldn't be enough: Hermes would also need a way to send a system prompt into a CAS session at all, which nothing in the codebase does today.

Neither is part of this PR. #68563 itself stays open for the maintainers to judge.

Testing

  • TestPromptStabilityInvariant and two new restore-path pins confirm identity drift never touches the cached prompt or calls update_system_prompt during restore.
  • TestIdentityStalenessRebuild's scenarios (edited, truncated, and deleted SOUL.md) now assert verbatim reuse on restore instead of rebuild, matching the behavior above.
  • New compaction-gate tests cover both directions: the keep optimization still holds when identity is unchanged, and it's suppressed (with persistence) when SOUL.md changed.
  • New pins confirm the app-server path and the live-child-adoption path both keep passing the cached prompt through unchanged, even with drift present, which documents the current limitation rather than silently relying on it.
  • TestSessionStartHookGuard moved out of this file; it belongs to a separate PR fixing the on_session_start double-fire bug, which is a pre-existing issue unrelated to SOUL.md identity.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 26, 2026
@Sora-bluesky
Sora-bluesky force-pushed the fix/issue-68563-v3 branch 3 times, most recently from 829eb1e to feac335 Compare July 30, 2026 08:06
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused SOUL.md fix. The current-main premise is real: agent/conversation_loop.py:470 restores a stored prompt after _stored_prompt_matches_runtime(), whose checks end at model/provider/cwd/platform in agent/conversation_loop.py:604-632; SOUL.md is loaded into the stable identity slot in agent/system_prompt.py:189-201.

Problems

  • The linked issue's requested contract is broader than this patch. agent/prompt_builder.py:2169-2175 also loads managed project context from .hermes.md/HERMES.md, AGENTS.md, CLAUDE.md, or .cursorrules; changes there still leave the persisted prompt reusable. The Gateway durable sessions do not refresh system prompt after SOUL.md changes #68563 discussion explicitly distinguishes this PR as SOUL-only.
  • Because the PR body says Fixes #68563, it could close the broader fingerprint issue despite not implementing its durable paired-persistence requirement.

Suggested changes

  • Either extend this into the managed-input fingerprint contract, including every prompt-snapshot write path, or re-scope it as a SOUL.md-only partial fix and remove the closing issue reference.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
@Sora-bluesky

Copy link
Copy Markdown
Contributor Author

The header is fixed. The body's first line now reads Refs #68563. and no closing keyword remains anywhere in the body, so merging this will not auto-close the issue. The scope note stands: this PR covers the SOUL.md identity slot only, and the persisted fingerprint contract for the other managed inputs stays with #68563.

@Sora-bluesky

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (30fcf95) as a single commit (4980347b, was 3 commits ending at 6f8132a6). Upstream had landed the Bot Chat capability-epoch block in agent/conversation_loop.py exactly where the first commit of this branch used to touch it; since the second commit had already moved the drift check to the compaction boundary, the net diff never touched that file, so I applied the net diff on top of main instead of replaying the intermediate step. The +/- lines are unchanged (3 files: agent/system_prompt.py, agent/conversation_compression.py, tests/agent/test_system_prompt_restore.py); the three commit messages are folded into the new one. tests/agent/test_system_prompt_restore.py 37 passed on the rebased tree. Still waiting on the #68563 decision — no action needed on that front.

…ia the shared identity resolver

Editing SOUL.md never reached a continuing session: the restore path
reuses the persisted system prompt verbatim, and
_stored_prompt_matches_runtime only rejects Model/Provider/cwd/Platform
drift, not identity content drift (NousResearch#68563).

The identity resolution (SOUL.md or the hardcoded default, with the
context-length-dependent truncation) is extracted from
build_system_prompt_parts into resolve_identity_block(), and
stored_identity_is_stale() asks that same resolver what identity a fresh
build would use. The comparison is anchored on the identity block plus
HERMES_AGENT_HELP_GUIDANCE (identity is slot NousResearch#1, so the pair must be the
stored prompt's prefix); a stored prompt without that anchor is reported
as not stale — the resolver can only assert staleness when it can locate
and compare slot NousResearch#1.

Rebuilding on session restore would break the AGENTS.md byte-stability
contract (the one allowed rebuild point is context compression), so the
drift check is applied at the compaction keep-prompt gate: a compaction
that persists its rebuilt prompt refuses to reuse the cached bytes when
the stored identity block no longer matches SOUL.md. Restore path I/O is
unchanged (never reads SOUL.md, never rewrites stored prompt bytes),
covered by regression tests.

The resolver reports provenance and checkability: SOUL.md that exists but
cannot be read is NOT treated as stale (rebuilding would persist a
default-identity downgrade), and a missing HERMES_HOME reports
checkable=False so a transient mount gap cannot demote a custom identity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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