fix(hermes): Hermes v0.20 one-shot compat — cli_close + auxiliary task rows - #4
Merged
Merged
Conversation
Hermes v0.20+ finalizes one-shot (-q/-Q) sessions via _flush_one_shot_session_store, which writes ended_at/end_reason 'cli_close' on normal CLI exit. The adapter only accepted 'completed', so every clean one-shot turn on current Hermes failed closed with 'not a clean completion'. cli_close carries the same meaning for the one-shot contract and stays gated by ended_at window consistency plus the process-exit terminal basis.
Hermes records auxiliary model calls (title generation, vision, compression, ...) in session_model_usage alongside the main turn, with a non-empty task column and often a different model/provider. The adapter counted every usage row, so a clean one-shot turn failed closed with 'observed model set is not exactly one model' whenever an auxiliary call ran (Hermes v0.20 does this routinely). Only task ''/NULL rows (main conversation) now prove model/provider identity; schemas without a task column keep the old all-rows behavior. Matrix tests rerun against both schema variants.
There was a problem hiding this comment.
Confidence Score: 4/5
Summary
Accepts Hermes v0.20 cli_close as a clean one-shot end_reason and derives model identity from main-task usage rows only. Fail-closed paths preserved, new matrix coverage is thorough, full suite green; low merge risk.
Important Files Changed
| File | Overview |
|---|---|
| src/multi_agent_dialogue/adapters/hermes.py | Adds cli_close to clean end reasons; filters usage rows to main-task only |
| tests/test_real_contracts.py | cli_close acceptance test plus task-filter subclass rerunning full terminal matrix |
Findings
- tests/test_real_contracts.py:656 matrix docstring still lists only NULL/''/'completed' as clean end_reasons; update it to include cli_close.
…pi_call_count Review follow-up: match the session_model_usage task column case-insensitively (SQLite identifier semantics) so an oddly-cased "Task" schema still triggers the main-conversation filter, and state explicitly in the proof docs that api_call_count covers main-conversation calls only on task-column schemas. Also documents cli_close in the technical reference.
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
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.
Summary
Two fail-closed rejections hit when running the
hermes-cliadapter against Hermes Agent v0.20.4 — both false negatives on clean, successful one-shot turns:cli_closeend_reason rejected. Hermes v0.20+ finalizes one-shot (-q/-Q) sessions via_flush_one_shot_session_store, writingended_at/end_reason='cli_close'on normal CLI exit. The adapter only accepted'completed', so every clean turn failed withended with reason 'cli_close', not a clean completion.cli_closecarries the same meaning for the one-shot contract and stays gated byended_atwindow consistency plus the process-exit terminal basis.Auxiliary model calls polluted identity. Hermes records auxiliary calls (title generation, vision, compression, …) in
session_model_usagealongside the main turn, with a non-emptytaskcolumn and often a different model/provider. The adapter counted every usage row, so a clean turn failed withobserved model set [...] is not exactly one model. Now onlytask ''/NULLrows (main conversation) prove identity; pre-taskschemas keep the old all-rows behavior.Evidence
Both bugs were reproduced live during a real three-Hermes-profile dialogue (
madp run --launchagainst three distinctHERMES_HOMEs), then fixed and re-run to completion withvalidate --require-git --require-runner-completionreturningok: true.Tests
test_ended_with_cli_close_reason_is_acceptedin the terminal-field matrix.HermesUsageTaskFilterTests— the entire accept/reject matrix re-runs against atask-column schema, plus: auxiliary rows ignored,NULLtask counts as main, two main-task models still rejected.scripts/verify.py→{"ok": true}.Notes
error,canceled, …) and multi-model main conversations still fail closed.