Skip to content

feat: add Hermes Agent connector example#78

Closed
arcabotai wants to merge 2 commits into
openclaw:mainfrom
arcabotai:feat/hermes-agent-connector
Closed

feat: add Hermes Agent connector example#78
arcabotai wants to merge 2 commits into
openclaw:mainfrom
arcabotai:feat/hermes-agent-connector

Conversation

@arcabotai

Copy link
Copy Markdown

Summary

  • add a standalone Hermes Agent connector under examples/hermes-agent using Hermes' public Runs and SSE APIs
  • support DMs, mention-triggered channel threads, bounded history, progress events, cancellation, per-conversation ordering, and bounded cross-conversation concurrency
  • add SDK support for paginated realtime events, typed agent.progress events, DM pagination, and latest bounded thread windows
  • harden cursor recovery, run/approval timeouts, final-reply retries, remote Hermes transport checks, and connector E2E process handling
  • run connector unit and real ClickClack/WebSocket integration coverage in CI

Safety and lifecycle

  • hidden reasoning, tool arguments, and tool previews are never published
  • final replies use deterministic ClickClack nonces
  • remote Hermes endpoints require an API key and HTTPS by default; remote ClickClack endpoints also require HTTPS by default
  • socket read-ahead is bounded and reconnects from the last completed in-memory cursor
  • one stalled conversation does not block unrelated DMs or threads; timed-out runs and their best-effort stop requests are independently bounded

Testing

  • pnpm check
  • connector integration proof with a real ClickClack Go server and WebSocket event path plus a mock Hermes Runs/SSE server

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 15, 2026
@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 10:12 AM ET / 14:12 UTC.

Summary
Adds a Hermes Agent connector example plus realtime-event pagination, latest-thread history, typed agent-progress support, documentation, tests, and CI coverage.

Reproducibility: not applicable. This PR adds a new Hermes integration and supporting capabilities rather than repairing a defined broken behavior.

Review metrics: 3 noteworthy metrics.

  • Patch Size: 4,125 added, 19 removed across 30 files. This is a major ongoing maintenance commitment rather than an isolated example.
  • Shared Surfaces: API server, 2 stores, SDK, docs, workflow, lockfile, and example changed. The connector expands public and operational surfaces outside its own directory.
  • Validation Signal: 52 claimed passing connector tests, 1 mocked-Hermes E2E, and 1 reported live run. Automated coverage is substantial, but the exact-head live result is not independently inspectable.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Attach redacted exact-current-head Hermes and ClickClack runtime evidence.
  • Obtain an explicit maintainer decision on first-party ownership versus an external connector.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The discussion gives detailed live-run claims but no inspectable redacted artifact and identifies d7f2fc0 plus a transported patch rather than the current 5a20a79 head; attach exact-head terminal output, logs, a recording, or a linked Crabbox artifact, redact private data, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging makes ClickClack responsible for a third-party Hermes Runs/SSE compatibility surface, a large connector implementation, and recurring CI coverage beyond the existing framework-neutral bot contract.
  • [P1] The connector owns replay, contiguous cursor persistence, deterministic nonces, retries, timeouts, cancellation, and shutdown behavior; runtime drift could duplicate or suppress messages.
  • [P1] Chat messages can start tool-capable Hermes runs, creating an ongoing security burden around authorization, target isolation, credentials, transport rules, and upstream event semantics.

Maintainer options:

  1. Split Generic Support From Connector (recommended)
    Extract independently useful realtime, thread-window, and SDK changes into a narrow patch while maintaining the Hermes bridge externally.
  2. Accept First-Party Ownership
    Merge the full branch only after maintainers explicitly accept the Hermes compatibility and operational burden and inspect exact-head live evidence.
  3. Pause the Integration
    Leave the PR unmerged or close it if no maintainer wants to own a runtime-specific connector in ClickClack.

Next step before merge

  • [P1] The next action is a maintainer product-boundary decision plus contributor-supplied exact-head proof, not an automated code repair.

Maintainer decision needed

  • Question: Should ClickClack own and continuously test this production-shaped Hermes-specific connector, or keep runtime-specific bridges external to its framework-neutral bot surface?
  • Rationale: The implementation appears credible, but accepting more than four thousand lines of third-party integration, shared API additions, and ongoing CI and support cost is a repository ownership decision that source review cannot settle.
  • Likely owner: shakkernerd — They are the current review assignee and the best available owner for deciding the intended integration boundary.
  • Options:
    • Keep Bridge External (recommended): Publish the Hermes connector separately and split out only generic ClickClack API and SDK changes with independent repository value.
    • Adopt First-Party Example: Keep the full connector in this repository with explicit ongoing ownership and exact-current-head live proof before merge.

Security
Cleared: No concrete security regression remains after the default-deny allowlists, workspace and target checks, workspace-bound cursor state, HTTPS and API-key rules, and focused denial tests.

Review details

Best possible solution:

Either keep the Hermes bridge external and land only independently justified generic API and SDK improvements, or explicitly adopt it as a first-party example with exact-head live proof and named ownership for security, delivery, CI, and upstream compatibility.

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

Not applicable; this PR adds a new Hermes integration and supporting capabilities rather than repairing a defined broken behavior.

Is this the best way to solve the issue?

Unclear pending product direction: the implementation is credible, but an external connector plus narrowly separated generic API and SDK additions is the lower-maintenance solution unless maintainers explicitly want first-party Hermes ownership.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 346c80c14413.

Label changes

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority optional integration with broad but non-urgent repository impact.
  • merge-risk: 🚨 message-delivery: The patch changes replay, ordering, cursor commits, retries, cancellation, and final-reply deduplication, where runtime drift can duplicate or suppress messages.
  • merge-risk: 🚨 security-boundary: Chat messages can start tool-capable Hermes runs, making sender and channel authorization, target isolation, credentials, and transport restrictions merge-critical.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The discussion gives detailed live-run claims but no inspectable redacted artifact and identifies d7f2fc0 plus a transported patch rather than the current 5a20a79 head; attach exact-head terminal output, logs, a recording, or a linked Crabbox artifact, redact private data, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Prior authorization blocker addressed: The follow-up commit introduces default-deny immutable user and channel allowlists before history lookup or Hermes run creation, with workspace and malformed-target checks plus focused denial coverage. (examples/hermes-agent/src/connector.ts:1, 5a20a79973c8)
  • Prior recovery blocker addressed: The follow-up adds an atomic workspace-bound file cursor store and advances persistence only over the contiguous successfully completed event prefix; aborted or undelivered work remains replayable. (examples/hermes-agent/src/cursor-store.ts:1, 5a20a79973c8)
  • Existing generic bot boundary: Current main already documents a framework-neutral long-running bot contract covering realtime backfill, WebSocket cursor recovery, filtering, and channel, thread, and DM replies; the unresolved question is whether ClickClack should additionally own this runtime-specific production example. (docs/features/bots.md, 346c80c14413)
  • Repository positioning: Current main presents the integration surface as a framework-neutral TypeScript SDK and a small bot example, while this branch adds a production-shaped third-party connector and supporting shared surfaces. (README.md, 346c80c14413)
  • Exact-head proof remains uninspectable: The contributor reports a successful live run against d7f2fc0 plus a transported patch hash, but the current PR head is 5a20a79 and the discussion contains no inspectable transcript, logs, recording, or linked artifact. (5a20a79973c8)

Likely related people:

  • shakkernerd: They were assigned after the first ClawSweeper review and are the strongest available routing signal for the repository-ownership and merge-boundary decision. (role: review assignee; confidence: medium; files: examples/hermes-agent/src/connector.ts, packages/sdk-ts/src/index.ts, apps/api/internal/httpapi/server.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-15T07:05:15.587Z sha d7f2fc0 :: needs real behavior proof before merge. :: [P1] Require authorization before starting Hermes runs | [P1] Persist the completed realtime cursor across restarts

@shakkernerd shakkernerd self-assigned this Jul 16, 2026
@arcabotai

Copy link
Copy Markdown
Author

Addressed the two review blockers:

  • Added explicit default-deny authorization before history lookup or Hermes run creation. DMs require an allowed immutable user ID; channel/thread turns require both an allowed user ID and allowed channel ID. Cross-workspace and malformed dual-target messages are denied.
  • Added an atomic, workspace-bound file cursor store and restart recovery. Cursors advance only across the contiguous successfully completed prefix. Shutdown-aborted work and exhausted final/generic reply delivery now reject completion and remain replayable.

Focused regression coverage includes unauthorized users/channels, malformed targets, cursor-store recovery, out-of-order completion, failed persistence, shutdown aborts, and exhausted delivery retries.

Verification against PR head d7f2fc07a6e2724f0dcbcc0ac7a88607ead25b35 and exact transported patch SHA-256 37e43c5f4f8caa35bb8b07c95a609f65f871f394f1eaef51e885f9b6affb01bb:

  • 52/52 Hermes connector tests passed
  • TypeScript typecheck passed
  • SDK build passed
  • Web build and asset embedding passed
  • Existing real ClickClack/mock-Hermes E2E passed
  • Disposable Blaxel VM proof through Crabbox passed with exit 0
  • Node v24.18.0, Go 1.26.5, Hermes commit bda8bd76a8c98cba07ed32fb97c236e92f443397
  • Real Hermes Runs API capabilities confirmed run_submission=true and run_events_sse=true
  • Real ClickClack + connector + Hermes proof confirmed:
    • first authorized DM reply
    • persisted cursor loaded after connector restart
    • no duplicate reply after restart
    • second authorized DM reply after restart
    • unauthorized channel denied
    • durable cursor advanced
    • six progress events observed
  • Crabbox automatically deleted the sandbox; post-run Blaxel inventory was empty

Crabbox note: its Blaxel lifecycle adapter was locally provenance-patched to use the provider's current expirationPolicies schema because the released adapter's legacy creationPolicy field is rejected. The workload itself ran through Crabbox's bounded one-shot lifecycle and command path.

Please re-review when convenient.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: feat: add Hermes Agent connector example This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete

Copy link
Copy Markdown
Contributor

Sweet! Please live test this and provide screenshots!

@steipete

Copy link
Copy Markdown
Contributor

I wonder tho - shouldn't that connector be in Hermes, and not clickclack? Just like we have a clickclack connector in OpenClaw?

@steipete

Copy link
Copy Markdown
Contributor

Thank you for putting so much care into this, especially the authorization, cursor recovery, and live validation work.

We have decided not to maintain the Hermes connector inside ClickClack. The bridge fits better on the Hermes side, or in its own repository, where its ownership, compatibility work, and release cadence can follow Hermes directly.

The generic capabilities this surfaced are valuable. We will bring the reusable realtime pagination, bounded thread-history behavior, and agent-progress SDK support into ClickClack's API and SDK, with credit to you in the changelog. We would be glad to see the Hermes connector build against those framework-neutral capabilities from its natural home.

Thanks again for the thoughtful contribution and for helping sharpen the integration boundary.

@arcabotai

Copy link
Copy Markdown
Author

Thanks so much, Pete. We really appreciate the kind words, the clear boundary, and the way you carried the generic pieces forward. Seeing the history helpers and typed agent-progress SDK land in #91 and #92 with Cad’s co-author credit and @arcabotai in the changelog is an amazing outcome for us.

We’ll explore moving the Hermes-specific connector to the Hermes side as a standalone platform plugin. Thanks again for taking the contribution seriously and getting the reusable parts into ClickClack 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants