refactor(sessions): salvage #29182 + opt-in JSON snapshot writer#29278
Merged
Conversation
state.db now stores every message field the JSON snapshot stored. Removed the method, all 7 call-sites, and ~13 test stubs that suppressed its file I/O. Body is in git history if it ever needs to come back.
The attribute no longer exists; nothing to re-point.
Only caller was the removed _save_session_log. Also removes the unused convert_scratchpad_to_think and has_incomplete_scratchpad imports from run_agent.py (both still used elsewhere via their own imports).
…tespace Adds TestNoSessionJsonSnapshot to lock the contract that session_log_file attribute, _save_session_log method, and the per-session JSON snapshot writer are gone. logs_dir is retained for request_dump_*.json. Also cleans up stray trailing whitespace in test_run_agent_codex_responses introduced when the _save_session_log stub line was deleted.
The email "jonny@nousresearch.com" belongs to @yoniebans (GitHub id 5584832, display name "jonny"), not to Jeffrey Quesnelle (@jquesnelle, id 687076, who commits as emozilla@nousresearch.com). Verified across all 60 historical commits on the repo authored from this email — every one of them was a yoniebans commit being mis-credited to jquesnelle in the changelog. Surfaced while salvaging PR #29182 (yoniebans's session-log refactor).
PR #29182 deleted the per-session JSON snapshot writer outright because state.db is canonical and the snapshots had no in-tree consumer. Some users have external tooling that reads `~/.hermes/sessions/session_{sid}.json` directly, so reintroduce the writer behind a config flag that defaults to off. - Add `sessions.write_json_snapshots` (default False) to DEFAULT_CONFIG - Restore `AIAgent._save_session_log` + `_clean_session_content` as gated methods. When the flag is off the call is a fast no-op; when on, the writer behaves as before (atomic write, truncation guard preserved, REASONING_SCRATCHPAD → think tag normalization) - Re-derive the target path from `agent.session_id` on each call so `/branch` and `/compress` re-points happen automatically — no need to restore the explicit re-point bookkeeping at call sites - Wire the single call site in `_persist_session` (the cleanup-on-exit hook). Did NOT restore the 7 intra-turn calls the original PR deleted — those were redundant writes within the same turn that doubled disk I/O without adding any persistence guarantee `_persist_session` does not already provide - Read the flag once at agent init via `load_config()`, cache as `agent._session_json_enabled` - Update `TestNoSessionJsonSnapshot` → `TestSessionJsonSnapshotOptIn` to pin behavior: default off (no file), opt-in true (file written), no-op method on default agents, logs_dir retained unconditionally - Update CONTRIBUTING.md and the bundled `hermes-agent` skill to document the flag and its default
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
1 |
First entries
run_agent.py:1544: [unresolved-attribute] unresolved-attribute: Object of type `Self@_save_session_log` has no attribute `logs_dir`
✅ Fixed issues (7):
| Rule | Count |
|---|---|
invalid-assignment |
4 |
unresolved-attribute |
1 |
invalid-argument-type |
1 |
unsupported-operator |
1 |
First entries
cli.py:6507: [invalid-assignment] invalid-assignment: Object of type `Unknown` is not assignable to attribute `session_log_file` on type `AIAgent & <Protocol with members 'session_log_file'> & <Protocol with members 'logs_dir'> & ~AlwaysFalsy`
tests/cron/test_codex_execution_paths.py:77: [invalid-assignment] invalid-assignment: Object of type `(messages) -> None` is not assignable to attribute `_save_session_log` of type `def _save_session_log(self, messages: list[dict[str, Any]] = None) -> Unknown`
run_agent.py:1575: [unresolved-attribute] unresolved-attribute: Object of type `Self@_save_session_log` has no attribute `session_log_file`
tests/run_agent/test_run_agent.py:559: [invalid-argument-type] invalid-argument-type: Argument to function `AIAgent._clean_session_content` is incorrect: Expected `str`, found `None`
cli.py:6508: [unsupported-operator] unsupported-operator: Operator `/` is not supported between objects of type `object` and `str`
tests/run_agent/test_context_token_tracking.py:55: [invalid-assignment] invalid-assignment: Object of type `(...) -> None` is not assignable to attribute `_save_session_log` of type `def _save_session_log(self, messages: list[dict[str, Any]] = None) -> Unknown`
tests/run_agent/test_run_agent_codex_responses.py:597: [invalid-assignment] invalid-assignment: Object of type `(messages) -> None` is not assignable to attribute `_save_session_log` of type `def _save_session_log(self, messages: list[dict[str, Any]] = None) -> Unknown`
Unchanged: 4724 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
1 task
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.
Salvage of #29182. The default
~/.hermes/sessions/session_{sid}.jsonsnapshot writer is gone; users with external tooling that consumes those files can opt back in viasessions.write_json_snapshots: true.Changes (vs #29182)
sessions.write_json_snapshots(defaultFalse) — DEFAULT_CONFIG entry inhermes_cli/config.py. Gates the writer per-agent.AIAgent._save_session_log+_clean_session_contentas gated methods. When the flag is off the call is a fast no-op; when on, behavior matches pre-refactor(session-log): stop writing per-session JSON snapshots #29182 (atomic write, truncation guard, REASONING_SCRATCHPAD → think tag normalization).agent.session_idon each call so/branchand/compressre-points happen automatically — no need to restore the explicit re-point bookkeeping at call sites that the original PR removed._persist_session(the cleanup-on-exit hook). The 7 intra-turn calls refactor(session-log): stop writing per-session JSON snapshots #29182 deleted are NOT restored — they were redundant writes within the same turn that doubled disk I/O without adding any persistence guarantee_persist_sessiondoes not already provide.TestNoSessionJsonSnapshot→TestSessionJsonSnapshotOptIn: pins behavior (default off → no file, opt-in true → file written, no-op method on default agents,logs_dirretained unconditionally forrequest_dump_*.json).hermes-agentskill).jonny@nousresearch.comwas mapped tojquesnelle, but that email belongs to @yoniebans (GitHub id 5584832, name "jonny"). Jeffrey Quesnelle commits asemozilla@nousresearch.com. Verified across all 60 historical commits from that email — every one was a yoniebans commit being mis-credited.Verified premise of #29182
_save_session_log+session_log_filehad zero non-self readers across the codebase (trajectory, batch_runner, mcp_serve, plugins all clean).hermes_state.py::_remove_session_filescleans up{sid}.json/{sid}.jsonl(gateway format, nosession_prefix) — different files from the agent'ssession_{sid}.json, so no cleanup-path regression.Validation
tests/run_agent/tests/agent/+tests/cli/tests/hermes_cli/ -k config_session_json_enabled=False, nosession_*.jsonwrittensessions.write_json_snapshots: true)_session_json_enabled=True,session_{sid}.jsonwritten with correct shapeCloses #29182. Credit to @yoniebans for the original refactor.