Skip to content

Run Hermes adapter through local harness - #35

Open
afrog33k wants to merge 2 commits into
openclaw:mainfrom
afrog33k:main
Open

Run Hermes adapter through local harness#35
afrog33k wants to merge 2 commits into
openclaw:mainfrom
afrog33k:main

Conversation

@afrog33k

@afrog33k afrog33k commented Jul 1, 2026

Copy link
Copy Markdown

Summary

  • Wire registered non-OpenClaw adapters through the generic harness lifecycle instead of rejecting them as known-but-unwired.
  • Add Hermes local adapter configuration from environment and a local verification shim for file/execution checks.
  • Prepend explicit staged-workspace guidance to Hermes MiniSWERunner prompts so local runs inspect the task workspace first.

Validation

  • /Users/reckon/miniconda3/bin/python -m pytest tests/test_harness.py tests/test_hermes_adapter.py -q -> 21 passed
  • /Users/reckon/miniconda3/bin/python -m pytest -q -> 318 passed, 5 skipped
  • Local Hermes rerun: t2-msg-summarize-thread improved from 0.63704 to 1.000 with the workspace-guidance patch.

Notes

  • This keeps OpenClaw behavior intact and routes adapter != "openclaw" through the adapter lifecycle.
  • Memory/session/cron/browser checks still need adapter-aware verification before broad non-file task sweeps.

@afrog33k
afrog33k requested a review from a team as a code owner July 1, 2026 11:36
@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 3, 2026, 12:09 AM ET / 04:09 UTC.

ClawSweeper review

What this changes

The PR routes registered local adapters such as Hermes through ShellBench’s harness, adds Hermes local configuration and verification support, and prepends workspace guidance to Hermes prompts.

Merge readiness

Blocked by patch quality or review findings - 7 items remain

This PR remains necessary: current main still rejects Hermes and other registered adapters before the generic lifecycle can run. The branch has a concrete P1 scoring defect, however: it discards the adapter context and tears it down before scoring, then substitutes a gateway-only client, so adapter state verification is never used for memory, session, cron, or gateway assertions.

Priority: P2
Reviewed head: 67ff264005ab6b55679299ba4e7fcd7cbb1276bf

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The PR has credible local-run evidence, but its current adapter-aware scoring path has a blocking correctness gap for stateful tasks.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The PR body records a real local Hermes rerun with an improved task score and reports focused and full test-suite results; the missing stateful path is a correctness defect, not an absence of after-fix proof.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body records a real local Hermes rerun with an improved task score and reports focused and full test-suite results; the missing stateful path is a correctness defect, not an absence of after-fix proof.
Evidence reviewed 6 items Current main still blocks the requested behavior: Current main validates the adapter against a fixed executable set and raises for every registered adapter other than OpenClaw, so Hermes cannot currently use the end-to-end harness lifecycle.
Adapter contract requires adapter-owned state verification: The adapter abstraction explicitly requires verify_state_query and says adapters resolve post-run StateQuery assertions against their declared capabilities.
PR bypasses that contract during local scoring: The proposed local lifecycle tears down the adapter, then calls score_task_run with _LocalVerificationClient rather than the adapter and its AdapterContext; stateful assertions can only reach the shim’s failing gateway methods.
Findings 1 actionable finding [P1] Pass adapter state verification into scoring
Security None None.

How this fits together

ShellBench’s benchmark harness selects an agent adapter, prepares a task workspace, drives task phases, and passes the resulting transcript and state assertions to the scorer. Adapters supply their own runtime lifecycle and state-query verification; the scorer produces the benchmark result used for reports and comparisons.

flowchart LR
  A[Benchmark task] --> B[Harness selects adapter]
  B --> C[Adapter setup and task phases]
  C --> D[Workspace and transcript]
  C --> E[Adapter state queries]
  D --> F[Scorer]
  E --> F
  F --> G[Benchmark result]
Loading

Before merge

  • Pass adapter state verification into scoring (P1) - Score the local run while the adapter context is still live and route state assertions through adapter.verify_state_query. The current code tears down adapter and passes _LocalVerificationClient to score_task_run, so memory/session/cron/gateway checks never reach Hermes’s verifier and are evaluated against an unavailable gateway instead. This is the unchanged P1 blocker from the prior review.
  • Resolve merge risk (P1) - Merging as written makes supported local Hermes stateful tasks fail or score against the wrong backend because adapter-owned state queries are replaced by gateway RPC failures.
  • Resolve merge risk (P1) - The branch is based on an older harness design while current main has since changed the surrounding execution architecture; the repaired behavior needs a fresh integration review after preserving adapter-aware scoring.
  • Complete next step (P2) - A narrow mechanical repair is available: retain the adapter/context through scoring and cover adapter-owned state verification with a regression test.
  • Improve patch quality - Preserve the live adapter and context through scoring before teardown.
  • Improve patch quality - Add a focused regression test showing a local state query invokes the adapter verifier.
  • Improve patch quality - Refresh the repaired integration against current main’s harness architecture.

Findings

  • [P1] Pass adapter state verification into scoring — clawbench/harness.py:636-650
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 4 files affected; production +247/-10, tests +103/-7 The lifecycle change is concentrated but spans harness execution, Hermes behavior, and its regression coverage.

Merge-risk options

Maintainer options:

  1. Preserve adapter-owned state verification (recommended)
    Pass the live adapter and its context into the scoring path, run state checks before teardown, and add a focused stateful local-adapter regression test.
  2. Pause the local-adapter expansion
    Close or defer this branch if generic state-query semantics are not ready to become part of the supported local harness contract.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve the live adapter and AdapterContext through local-run scoring, invoke adapter-owned state verification before teardown, and add a regression test for a stateful query.

Technical review

Best possible solution:

Keep the generic adapter lifecycle, but score local runs through the live adapter and AdapterContext before teardown, with a regression test proving a stateful Hermes-compatible assertion uses verify_state_query rather than a gateway shim.

Do we have a high-confidence way to reproduce the issue?

Yes—source establishes a high-confidence path: run a local adapter task containing a memory, session, cron, browser, or gateway state assertion. The proposed harness tears down the adapter and supplies _LocalVerificationClient to the scorer, so the adapter’s verify_state_query cannot run; this was not executed because the review is read-only.

Is this the best way to solve the issue?

No—the generic lifecycle is directionally appropriate, but the implementation must preserve the live adapter context through scoring instead of substituting a gateway-only verifier. A stateful regression test is the narrowest way to protect the intended contract.

Full review comments:

  • [P1] Pass adapter state verification into scoring — clawbench/harness.py:636-650
    Score the local run while the adapter context is still live and route state assertions through adapter.verify_state_query. The current code tears down adapter and passes _LocalVerificationClient to score_task_run, so memory/session/cron/gateway checks never reach Hermes’s verifier and are evaluated against an unavailable gateway instead. This is the unchanged P1 blocker from the prior review.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511.

Labels

Label justifications:

  • P2: The PR adds a useful benchmark capability but its current defect affects a bounded class of local adapter runs rather than production agent availability.
  • merge-risk: 🚨 session-state: Stateful assertions are routed away from adapter-owned verification and can be scored as gateway failures or unsupported behavior after merge.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🐚 platinum hermit and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body records a real local Hermes rerun with an improved task score and reports focused and full test-suite results; the missing stateful path is a correctness defect, not an absence of after-fix proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records a real local Hermes rerun with an improved task score and reports focused and full test-suite results; the missing stateful path is a correctness defect, not an absence of after-fix proof.

Evidence

Acceptance criteria:

  • [P1] python -m pytest tests/test_harness.py tests/test_hermes_adapter.py -q.
  • [P1] python -m pytest -q.
  • [P1] Run one redacted local Hermes stateful task or an equivalent adapter-backed verification scenario and capture the scorer result.

What I checked:

  • Current main still blocks the requested behavior: Current main validates the adapter against a fixed executable set and raises for every registered adapter other than OpenClaw, so Hermes cannot currently use the end-to-end harness lifecycle. (clawbench/harness.py:124, 884dd1bb5511)
  • Adapter contract requires adapter-owned state verification: The adapter abstraction explicitly requires verify_state_query and says adapters resolve post-run StateQuery assertions against their declared capabilities. (clawbench/adapters/base.py:175, 67ff264005ab)
  • PR bypasses that contract during local scoring: The proposed local lifecycle tears down the adapter, then calls score_task_run with _LocalVerificationClient rather than the adapter and its AdapterContext; stateful assertions can only reach the shim’s failing gateway methods. (clawbench/harness.py:636, 67ff264005ab)
  • Hermes advertises stateful capability in AI-agent mode: The Hermes adapter’s AI-agent mode declares MEMORY, CRON, BROWSER, and multi-turn capabilities and implements verify_state_query, so routing all state checks through the local gateway shim is materially incorrect. (clawbench/adapters/hermes.py:250, 67ff264005ab)
  • Unchanged prior blocker: The current head is the same SHA reviewed previously, and the prior review identified this same state-verification routing defect; it remains unresolved rather than being a newly discovered concern. (clawbench/harness.py:640, 67ff264005ab)
  • Current adapter architecture provenance: Current main’s adapter canonicalization work is associated with the commit that introduced the current adapter-layer direction, making its author a useful routing candidate for the compatibility design. (clawbench/adapters/__init__.py:1, 56531fbf432b)

Likely related people:

  • scoootscooob: Recent current-main history includes the adapter canonicalization layer and OpenAI-auth adapter work, both adjacent to the generic adapter lifecycle this PR extends. (role: recent area contributor; confidence: high; commits: 56531fbf432b, 0f1b45e4674b; files: clawbench/adapters/__init__.py, clawbench/adapters/openclaw.py)
  • Robin Narsingh Ranabhat: The workspace-isolation change touched the benchmark harness’s run lifecycle, which is the code path this PR changes. (role: recent harness contributor; confidence: medium; commits: 9fe22f9233f7; files: clawbench/harness.py)
  • Vincent Koc: Recent scoring and runtime hardening commits indicate familiarity with the benchmark result and verification path affected by adapter-aware scoring. (role: recent scoring contributor; confidence: medium; commits: fb486a1ed307, ed9adf8d8451; files: clawbench/scorer.py, clawbench/harness.py)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (29 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-01T19:17:15.726Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T13:16:49.266Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T14:58:11.781Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T17:06:20.210Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T19:15:11.302Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T20:37:21.042Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-02T22:14:48.627Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier
  • reviewed 2026-08-03T01:48:51.671Z sha 67ff264 :: needs changes before merge. :: [P1] Route local state checks through the adapter verifier

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 1, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant