feat(hermes): add Hermes Agent app#325
Conversation
📝 WalkthroughWalkthroughAdds a new Hermes personal AI agent deployment with Argo CD and Helm configuration, two containers sharing persistent storage, a hostPath-backed volume, setup documentation, and a root Automation apps table entry. ChangesHermes application deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ArgoCD
participant Helm
participant Kubernetes
participant HermesDashboard
participant HermesGateway
participant SharedPVC
ArgoCD->>Helm: Render Hermes with values.yaml
Helm->>Kubernetes: Apply workload and LoadBalancer service
Kubernetes->>SharedPVC: Mount /opt/data
HermesDashboard->>SharedPVC: Persist dashboard configuration
HermesGateway->>SharedPVC: Read persisted configuration
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hobroker
force-pushed
the
claude/hermes-app-docker-nwfm40
branch
2 times, most recently
from
38b14bf to
bd547f4
Compare
Add a MetalLB LoadBalancer service (192.168.50.205) and bind the dashboard to 0.0.0.0, matching the AdGuard/Plex pattern. The non-loopback bind engages the dashboard's fail-closed basic-auth gate; the login credentials and the DeepSeek provider key are set via the dashboard and persisted to /opt/data/.env on the PVC, so the Infisical secret is no longer needed and is removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/automation/hermes/application.yaml`:
- Around line 30-33: Update the ArgoCD Application manifest’s syncPolicy to the
repository-standard syncPolicy: {} for manual synchronization, removing the
redundant CreateNamespace=true and ServerSideApply=true sync options.
In `@apps/automation/hermes/config/pv.yaml`:
- Around line 12-14: Update the claimRef in the PV manifest to target the PVC
name generated by app-template: set claimRef.name to hermes-data for the
persistence.data volume, while keeping the namespace unchanged.
In `@apps/automation/hermes/values.yaml`:
- Around line 46-61: The dashboard exposes basic-auth credentials over plaintext
HTTP. Update the dashboard exposure to route through the repository’s Traefik
TLS-terminated ingress convention, configure the appropriate HTTPS host and
backend service, and remove or disable the direct MetalLB/LAN exposure so port
9119 is not reachable without TLS.
- Around line 74-83: Configure the Hermes service to use Traefik ingress at
hermes.hobroker.me instead of relying solely on the MetalLB LoadBalancer in the
service main configuration. Add the appropriate Traefik ingress resource and
route it to the Hermes HTTP service port, or explicitly document the intentional
LAN-only exception in the repository conventions if ingress is not appropriate.
In `@README.md`:
- Around line 147-151: Do not manually edit the auto-generated ## Apps section
in README.md. Update the underlying app metadata or source entries for the
Hermes row, then run npm run generate to regenerate the table and preserve
consistent metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 59b3db01-ed5e-492e-a477-dd50b6101fb0
📒 Files selected for processing (5)
README.mdapps/automation/hermes/README.mdapps/automation/hermes/application.yamlapps/automation/hermes/config/pv.yamlapps/automation/hermes/values.yaml
| syncPolicy: | ||
| syncOptions: | ||
| - CreateNamespace=true | ||
| - ServerSideApply=true |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
syncPolicy deviates from the repo convention — and one option is a no-op.
That's what I always say: "You miss 100% of the syncPolicies you don't configure!" But here's the thing — the coding guidelines specify syncPolicy: {} (manual sync) by default for ArgoCD Application manifests. The syncOptions block doesn't enable auto-sync (good!), but it does deviate from the {} convention. Additionally, CreateNamespace=true is a no-op since the destination namespace is default, which always exists in Kubernetes. ServerSideApply=true is a reasonable practice, but if the team wants to keep it, consider documenting why this app is an exception to the {} default.
As per coding guidelines: apps/**/application.yaml: Use syncPolicy: {} (manual sync) by default in ArgoCD Application manifests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/automation/hermes/application.yaml` around lines 30 - 33, Update the
ArgoCD Application manifest’s syncPolicy to the repository-standard syncPolicy:
{} for manual synchronization, removing the redundant CreateNamespace=true and
ServerSideApply=true sync options.
Source: Coding guidelines
| claimRef: | ||
| name: hermes | ||
| namespace: default |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the PVC naming convention in bjw-s-labs/app-template v5.0.1
# Check the app-template chart templates for PVC naming logic
helm pull bjw-s/app-template --version 5.0.1 --untar --untardir /tmp/app-template-check 2>/dev/null || \
curl -sL https://bjw-s-labs.github.io/helm-charts/app-template-5.0.1.tgz | tar xz -C /tmp/app-template-check 2>/dev/null
# Search for PVC name template patterns
rg -n 'persistentVolumeClaim|claimRef|pvc.*name|name.*pvc' /tmp/app-template-check/ --type yaml -g '**/*.yaml' -C3
# Also check how existing apps in the repo reference their PVCs
rg -n 'claimRef' apps/ --type yaml -C2Repository: hobroker/selfhosted
Length of output: 1184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Hermes app manifests =="
sed -n '1,220p' apps/automation/hermes/config/pv.yaml
echo
echo "== Hermes values / persistence config =="
rg -n 'persistence|persistentVolumeClaim|claimRef|storageClass|existingClaim|nameOverride|fullnameOverride|releaseName' apps/automation/hermes -g '!**/*.png' -C3
echo
echo "== Openclaw comparison =="
rg -n 'persistence|persistentVolumeClaim|claimRef|storageClass|existingClaim|nameOverride|fullnameOverride|releaseName' apps/automation/openclaw -g '!**/*.png' -C3
echo
echo "== Any app-template chart references in repo =="
rg -n 'app-template|bjw-s|helm-chart' . -g '!**/node_modules/**' -C2Repository: hobroker/selfhosted
Length of output: 38913
🌐 Web query:
bjw-s app-template v5.0.1 persistence PVC name template releaseName persistence key
💡 Result:
In the bjw-s app-template (v5.0.1), the name of a Persistent Volume Claim (PVC) created via the persistence configuration is typically generated using the Helm release name and the identifier key defined in your values.yaml [1][2]. By default, this results in a PVC name formatted as RELEASE-NAME-identifier [2]. You can control or override this naming behavior using the following settings within the specific persistence item: 1. forceRename: This property allows you to completely override the default resource name [1]. It is mutually exclusive with the prefix and suffix options [1]. 2. prefix: Prepends a string to the resource name [1]. 3. suffix: Appends a string to the resource name (defaults to the release name) [1]. 4. nameOverride: In some contexts within the underlying common library, this can be used to override the name suffix used for the volume [2]. If you find that the generated name does not meet your requirements, verify your values.yaml configuration to ensure you are not accidentally triggering naming conflicts—such as when an identifier key matches the Helm release name itself, which in older versions could cause unexpected naming behavior [3]. Always refer to the official documentation for the latest property definitions, as configurations are processed by the underlying common library [1][2][4].
Citations:
- 1: https://bjw-s-labs.github.io/helm-charts/docs/app-template/reference/persistence/
- 2: https://bjw-s-labs.github.io/helm-charts/docs/common-library/storage/types/persistentVolumeClaim/
- 3: Persistent volume mapping naming is incorrect when the persistence name matches the deployment name bjw-s-labs/helm-charts#345
- 4: https://bjw-s-labs.github.io/helm-charts/docs/app-template/
🌐 Web query:
site:github.com bjw-s app-template persistence data PVC name Release.Name persistence key
💡 Result:
In the bjw-s app-template (and the underlying common library chart), the persistence configuration allows you to define Persistent Volume Claims (PVCs) [1][2]. When defining a persistence item, the key you choose under the persistence block acts as the identifier for that volume [3][2]. If you do not specify an existingClaim, the chart automatically creates a PVC [4][5]. By default, this generated PVC is typically named based on the release name and the persistence key (e.g., {{.Release.Name }}-) [4]. Key points regarding PVC naming and Release.Name: 1. Automatic Naming: If you allow the chart to manage the PVC, it will generally ensure the name is scoped to the release [4]. Avoid naming your persistence key exactly the same as your Release.Name if you encounter conflicts, as there have been historical reports of naming discrepancies in such scenarios [4]. 2. Using Release.Name in existingClaim: If you are managing your own PVCs and want to point to one that includes the Release.Name, you can use the existingClaim field [3][2]. Because the values file is processed as a Helm template, you can reference the release name dynamically [6]. To use the Release.Name in the existingClaim field, format it as a Go template string in your values.yaml [6]: persistence: data: enabled: true existingClaim: '{{.Release.Name }}-my-custom-pvc' 3. Best Practices: If you have multiple persistence entries that need to point to the same underlying physical storage (e.g., a single large PVC), use the existingClaim field across all those entries rather than relying on auto-generated PVCs [3]. This prevents the chart from creating duplicate PVC resources [3]. For advanced configurations, such as mapping specific sub-paths of a shared volume to different locations within your container, the globalMounts or advancedMounts features are recommended [3][2]. These allow you to mount the same persistence key (and thus the same PVC) multiple times with different subPaths [3].
Citations:
- 1: https://github.com/bjw-s-labs/helm-charts/tree/main/charts/other/app-template
- 2: https://github.com/bjw-s-labs/helm-charts/blob/e107d9c9/charts/library/common/values.yaml
- 3: Multiple persistence entries with same existingClaim create duplicate volume entries bjw-s-labs/helm-charts#641
- 4: Persistent volume mapping naming is incorrect when the persistence name matches the deployment name bjw-s-labs/helm-charts#345
- 5: Persistence items are disabled by default but the docs say the opposite (app-template) bjw-s-labs/helm-charts#251
- 6: Postgresql helm chart does not seem to honor existingClaim Template helm/charts#13099
Fix the PVC binding target here
app-template v5.0.1 names managed PVCs from the release name plus the persistence key, so persistence.data renders to hermes-data here. claimRef.name: hermes will miss that PVC and leave Hermes stuck waiting for storage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/automation/hermes/config/pv.yaml` around lines 12 - 14, Update the
claimRef in the PV manifest to target the PVC name generated by app-template:
set claimRef.name to hermes-data for the persistence.data volume, while keeping
the namespace unchanged.
| dashboard: | ||
| image: | ||
| repository: docker.io/nousresearch/hermes-agent | ||
| tag: v2026.7.7.2 | ||
| pullPolicy: IfNotPresent | ||
| # Binding to a non-loopback host engages the dashboard's fail-closed | ||
| # auth gate: it refuses to start unless an auth provider is configured. | ||
| # The HERMES_DASHBOARD_BASIC_AUTH_* username/password vars live in | ||
| # /opt/data/.env (set via the dashboard's API Keys page, persisted on | ||
| # the PVC), so the dashboard is LAN-exposed (MetalLB, see service below) | ||
| # behind a login — never through Traefik / the public domain, never | ||
| # with --insecure. | ||
| args: | ||
| - dashboard | ||
| - --host | ||
| - 0.0.0.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Dashboard serves basic auth over plaintext HTTP — credentials travel in the clear.
The dashboard binds to 0.0.0.0:9119 and is exposed via MetalLB — but there's no TLS. Basic auth credentials (HERMES_DASHBOARD_BASIC_AUTH_USERNAME/PASSWORD) are sent base64-encoded over HTTP, which is trivially sniffable on the LAN. The fail-closed auth gate is a great defense-in-depth measure, but it doesn't protect the credential exchange itself. Routing through Traefik with TLS termination (per the repo's ingress convention) would solve this cleanly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/automation/hermes/values.yaml` around lines 46 - 61, The dashboard
exposes basic-auth credentials over plaintext HTTP. Update the dashboard
exposure to route through the repository’s Traefik TLS-terminated ingress
convention, configure the appropriate HTTPS host and backend service, and remove
or disable the direct MetalLB/LAN exposure so port 9119 is not reachable without
TLS.
| service: | ||
| main: | ||
| controller: main | ||
| type: LoadBalancer | ||
| externalTrafficPolicy: Local | ||
| annotations: | ||
| metallb.io/loadBalancerIPs: 192.168.50.205 | ||
| ports: | ||
| http: | ||
| port: 9119 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
No Traefik ingress — this is the equivalent of throwing a pizza party and forgetting the pizza.
The coding guidelines are explicit: "Configure all apps to use Traefik ingress with domain pattern <app>.hobroker.me." This app uses a MetalLB LoadBalancer directly, bypassing Traefik entirely. The PR objectives explain the LAN-only rationale, but the guideline has no exception clause. If this deviation is intentional, consider adding a Traefik ingress for hermes.hobroker.me (even if it only routes to the dashboard) or document the exception explicitly in the repo conventions.
As per coding guidelines: Configure all apps to use Traefik ingress with domain pattern <app>.hobroker.me.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/automation/hermes/values.yaml` around lines 74 - 83, Configure the
Hermes service to use Traefik ingress at hermes.hobroker.me instead of relying
solely on the MetalLB LoadBalancer in the service main configuration. Add the
appropriate Traefik ingress resource and route it to the Hermes HTTP service
port, or explicitly document the intentional LAN-only exception in the
repository conventions if ingress is not appropriate.
Source: Coding guidelines
| | App | Description | Source Code | | ||
| | ------------------------------------ | ---------------------------------------------------------------------------------- | ---------------------------------------------- | | ||
| | [hermes](apps/automation/hermes) | Personal AI agent that connects to messaging platforms and runs tasks autonomously | <https://github.com/NousResearch/hermes-agent> | | ||
| | [n8n](apps/automation/n8n) | Workflow automation platform | <https://github.com/n8n-io/n8n> | | ||
| | [openclaw](apps/automation/openclaw) | AI assistant that connects to messaging platforms and executes tasks autonomously | <https://github.com/openclaw/openclaw> | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Regenerate the Apps table instead of hand-editing it.
The Hermes row looks great—Pizza Party Friday for the documentation! However, this section is auto-generated, so these edits should be produced by npm run generate; otherwise the next generation can overwrite them or expose metadata drift.
As per coding guidelines: README.md: Never manually edit the ## Apps section in README.md — it is auto-generated; run npm run generate instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 147 - 151, Do not manually edit the auto-generated ##
Apps section in README.md. Update the underlying app metadata or source entries
for the Hermes row, then run npm run generate to regenerate the table and
preserve consistent metadata.
Source: Coding guidelines
1 participant
Summary
Adds the Hermes Agent as a new automation app, deployed via the shared
bjw-s-labs/app-templatechart and ArgoCD.gateway run) and the configuration dashboard from a single image in one pod, mirroring upstream'sdocker-compose.yml. Both containers share the/opt/dataPVC (host path/var/local/hermes) so config written by either is visible to the other and survives restarts/upgrades.127.0.0.1(no auth of its own) and off Traefik — access is viakubectl port-forwardonly, documented in the README.infisical-hermes-secretinto the gateway viaenvFromfor env-based integrations (API server, Teams, Google Chat); it may be left empty when only the interactive wizard/dashboard is used.Recreatestrategy andfsGroup: 1000(withHERMES_UID/HERMES_GIDremapping) so the single ReadWriteOnce volume stays host-owned and is never mounted by two pods at once.Type
Summary by CodeRabbit