feat(providers): support hermes interview driver#671
Conversation
![ouroboros-agent[bot]](https://avatars.githubusercontent.com/in/3091302?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
1f7499bfor PR #671
Review record:
9cdc6731-65c4-4c4c-a111-664ba38ec82e
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/config/models.py:120 | BLOCKING | Adding hermes as a first-class LLM backend here exposes a directly relevant existing contract gap in src/ouroboros/config/loader.py:1107-1210: Hermes is not included in the backend-safe "default" model mapping that Codex/OpenCode/Copilot/Kiro use. As a result, standard model helpers like get_clarification_model() / get_qa_model() still return literals such as claude-opus-4-6, claude-sonnet-4-20250514, and gpt-4 when Hermes is selected, and the new adapter forwards those verbatim to hermes chat --model .... That means a default Hermes setup will fail on normal interview/QA/evaluation flows unless every model knob is manually overridden, which is a runtime regression introduced by enabling this backend globally. |
Follow-up Findings
src/ouroboros/providers/hermes_cli_adapter.py:273[warning]_resolve_cli_path()calls.resolve()on any explicitcli_path. If a caller passes the common bare-command formcli_path=\"hermes\"(or a wrapper name onPATH), this turns it into an absolute path under the current working directory, e.g./repo/hermes, instead of preserving PATH lookup. The same backend's runtime adapter does not do this. So the new LLM adapter will report “not found” for valid PATH-installed binaries whenever the path is supplied explicitly as a command name.
Non-blocking Suggestions
None.
Design Notes
The PR wires Hermes through the right registry/factory surfaces, but it treats “backend enabled” as complete before matching the shared configuration contracts that other CLI backends already rely on. The two gaps above are integration issues rather than local adapter mechanics.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
1f7499b to
4cae512
Compare
|
Addressed the requested changes in
Validation after the fix:
|
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
4cae512for PR #671
Review record:
c88a2e41-a4b0-4656-9783-b5947b632303
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/providers/hermes_cli_adapter.py:70 | BLOCKING | allowed_tools is only injected into the prompt, but Hermes is also registered as a soft-enforcement backend (capabilities.py marks it soft_tool_enforcement=True). Unlike the existing Gemini/OpenCode soft backends, this adapter never inspects tool-use events or otherwise detects violations, and quiet mode only returns final text. That means a supposedly text-only or restricted-tool session can still execute out-of-envelope tools with no audit signal, which is a real contract/safety regression for callers relying on allowed_tools. |
Follow-up Findings
src/ouroboros/providers/hermes_cli_adapter.py:91[warning] The adapter storesmax_turnsin__init__, butcomplete()builds the prompt with_build_prompt(messages)and never threads eitherself._max_turnsorconfig.max_turnsinto the request. This makes the new Hermes interview driver ignore the engine’s turn budget entirely, so flows that expectmax_turns=1question generation can silently turn into multi-step/tool-using conversations.
Non-blocking Suggestions
None.
Design Notes
The wiring is mostly consistent: backend registry, config normalization, factory resolution, and adapter export all line up. The weak point is the new Hermes adapter contract itself: it is advertised as both interview-capable and tool-envelope-aware, but it currently lacks the enforcement/audit behavior and turn-budget handling that those roles imply.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
4cae512 to
84095bb
Compare
|
Addressed the latest review in
Validation:
|
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
84095bbfor PR #671
Review record:
f6f92798-cdd7-40a6-a707-946ce9ceb9f4
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/providers/factory.py:216 | BLOCKING | create_llm_adapter() now forwards allowed_tools into HermesCliLLMAdapter, but the new adapter immediately returns ProviderError whenever allowed_tools is not None (src/ouroboros/providers/hermes_cli_adapter.py:87). That makes the newly advertised Hermes interview path unusable at existing call sites on the changed contract boundary: interview flows already construct adapters with allowed_tools=[] or a read-only envelope, so selecting Hermes now fails before the first question is generated instead of acting as an interview driver. Either Hermes needs soft envelope handling like Gemini/OpenCode, or it should not be exposed as interview-capable / should drop the envelope in the factory for this backend. |
Non-blocking Suggestions
None.
Design Notes
The backend-capability plumbing is mostly consistent, and the default-model normalization work closes the obvious config mismatch for Hermes. The remaining issue is a contract mismatch between capability exposure/factory wiring and the adapter’s actual envelope support.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
84095bb to
9d7947a
Compare
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
9d7947afor PR #671
Review record:
7b4194ff-870a-4b3b-9463-de6e56c7a18f
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/providers/factory.py:216 | BLOCKING | The factory now silently drops any caller-supplied allowed_tools envelope for Hermes by hardcoding allowed_tools=None. That bypasses the contract instead of rejecting it: flows like interview/evaluation/PM creation call create_llm_adapter(..., allowed_tools=...) specifically to constrain or forbid tool use, but Hermes will now run unrestricted with no error. The adapter itself was changed to fail fast on unsupported envelopes, so the factory should preserve that failure rather than masking it. |
Follow-up Findings
src/ouroboros/providers/hermes_cli_adapter.py:305[warning]_resolve_cli_path()preserves PATH lookup only for an explicit constructor argument, not for the same bare value coming fromOUROBOROS_HERMES_CLI_PATHororchestrator.hermes_cli_path. If either config surface is set tohermes, this branch resolves it to${cwd}/hermes, causingFileNotFoundErroreven though PATH lookup should succeed. The PR added a regression test forcli_path=\"hermes\", but the configured-path path still has the same bug.
Non-blocking Suggestions
None.
Design Notes
The backend registry/config wiring is consistent, but the Hermes provider integration currently violates the factory’s tool-policy contract: unsupported constraints are being erased instead of surfaced. The adapter logic itself is otherwise straightforward and matches the existing Hermes runtime parsing model.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
9d7947a to
efaa22a
Compare
|
Updated #671 after the latest review on Changes:
Validation:
|
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
efaa22afor PR #671
Review record:
32149a3d-0162-4745-b2a2-5397419b8088
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/backends/capabilities.py:80 | BLOCKING | supports_interview_driver=True advertises Hermes as a valid interview backend, but the new adapter still hard-fails whenever allowed_tools is provided (src/ouroboros/providers/hermes_cli_adapter.py:87, passed through by src/ouroboros/providers/factory.py:219). Existing interview call sites already pass a tool envelope for Hermes-sized flows, e.g. authoring_handlers uses _interview_allowed_tools(...) and pm_handler passes allowed_tools=[], so selecting Hermes for interviews now breaks before the first question is generated. Either Hermes needs to stay off the interview-driver surface, or those interview paths need a Hermes-specific contract that avoids/handles the envelope. |
Non-blocking Suggestions
None.
Design Notes
The backend/config wiring is mostly coherent, but the capability registry now overstates Hermes support relative to the adapter’s actual envelope contract. The main architectural gap is that “interview-capable” currently implies compatibility with existing interview tool-policy plumbing, which Hermes does not satisfy yet.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
dcf37ee to
9430acf
Compare
|
Updated #671 after the latest review on Changes:
Validation:
|
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
9430acffor PR #671
Review record:
e7931bf2-90b4-47ae-8e29-8295e136ded2
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/mcp/tools/authoring_handlers.py:100 | BLOCKING | _interview_allowed_tools(None) treats an unspecified backend as envelope-capable and returns a tool list. In the common path where InterviewHandler.llm_backend is left unset and Hermes is selected via config or OUROBOROS_LLM_BACKEND/OUROBOROS_RUNTIME, line 1173 still passes allowed_tools into create_llm_adapter(...), so the new HermesCliLLMAdapter rejects the request with ProviderError and authoring interviews stop working. The new test only covers the explicit "hermes" case, so this regression is currently untested. |
| 2 | src/ouroboros/mcp/tools/pm_handler.py:372 | BLOCKING | The PM interview path has the same implicit-backend regression: backend_supports_tool_envelope(self.llm_backend) returns True when self.llm_backend is None, so allowed_tools=[] is still sent to create_llm_adapter(...). If Hermes is the configured default backend, the factory builds HermesCliLLMAdapter with that envelope and every PM interview request fails before question generation. The added regression test only exercises llm_backend="hermes", not the default-configured Hermes path. |
Non-blocking Suggestions
None.
Design Notes
The capability-registry approach is the right direction, but the handler call sites are checking the raw constructor field instead of the resolved effective backend. That leaves the explicit-Hermes path fixed while the default-configured Hermes path still breaks.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Q00
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
9430acf to
76475a5
Compare
|
Updated #671 after the latest review on Changes:
Validation:
|
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Reviewing commit
76475a5for PR #671
Review record:
238af9b5-3b60-4495-bff0-8af56c5d4b5e
Blocking Findings
No in-scope blocking findings remained after policy filtering.
Non-blocking Suggestions
None.
Design Notes
The capability-registry approach and the Hermes-specific tool-envelope gating are coherent, and moving the interview paths to consult backend capabilities is the right abstraction. The remaining gap is surface consistency: once a backend is promoted to first-class LLM support, every CLI/backend-selection layer needs to be updated in lockstep.
Policy Notes
- Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.
Recovery Notes
First recoverable review artifact generated from codex analysis log.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Re-review pingStatus
What the PR doesPromotes Hermes from a runtime-only adapter to a first-class interview LLM driver. The bulk of the work is plumbing through the capability registry introduced in #670 — every CLI / backend selection / tool-envelope decision now derives from
The stack contract ( Iterative improvements driven by the botThe current approval on
Each iteration was anchored by an Why merging is safe
cc @Q00 — please consider merging after #670 so the driver/brake selection in #672 can land on a stabilized capability registry. |
Ready for maintainer review — re-review pingThis PR (Hermes interview driver) is in a clean merge-ready state on commit
Why this is safe to merge
Q00 maintainers — could you merge the foundation chain (#670 → #671 → #672) when convenient? After that the auto driver/brake follow-up cluster ( |
shaun0927
force-pushed
the
stack/auto-capabilities/2-hermes-driver
branch
from
d61df30 to
7ef5208
Compare
2 participants
Stack
2/3 Hermes LLM/interview driver support. Review after #670.
Previous PR:
Next PR:
Changes
HermesCliLLMAdapterbacked byhermes chat -Q.llm.backend: hermesandOUROBOROS_RUNTIME=hermesto route completion through Hermes.Validation
uv run pytest tests/unit/backends/test_capabilities.py tests/unit/providers/test_factory.py tests/unit/providers/test_hermes_cli_adapter.py tests/unit/config/test_loader.py tests/unit/config/test_models.py -quv run ruff check src/ouroboros/backends src/ouroboros/providers/hermes_cli_adapter.py src/ouroboros/providers/factory.py tests/unit/backends/test_capabilities.py tests/unit/providers/test_hermes_cli_adapter.py tests/unit/providers/test_factory.py