Skip to content

Add CI workflow and hermes-agent integration recipe#2

Open
RichelynScott wants to merge 2 commits into
mainfrom
claude/advance-hermes-project-3mrSE
Open

Add CI workflow and hermes-agent integration recipe#2
RichelynScott wants to merge 2 commits into
mainfrom
claude/advance-hermes-project-3mrSE

Conversation

@RichelynScott
Copy link
Copy Markdown
Owner

Summary

Two small, independent improvements that unblock downstream work on this fork:

  1. .github/workflows/ci.yml — runs npm ci, npm run build, and npm test on every push to main and every PR, across Node 22.x and 24.x. The repo already has a green 16-test suite (tests/rewriter.test.ts) but no automation was running it; a regression in the rewriter would only surface on a human's laptop.
  2. docs/integrations/hermes-agent.md — documents how to route Claude Code sessions that hermes-agent spawns through cc-gateway, so AIBC sibling repos (hermes-agent, claw-orchestrator) have a stable integration story. Pure docs.

What and why

  • CI: The README claims "16 tests passing" but there is no enforcement that PRs keep that true. Adding a .github/workflows/ci.yml is the lowest-risk way to harden the alpha. Matrix covers Node 22 (the minimum per package.json engines posture) and 24 (what config.example.yaml advertises as the canonical node_version).
  • Hermes recipe: The branch name calls out hermes-agent specifically. The doc describes the env-var contract (ANTHROPIC_BASE_URL, ANTHROPIC_API_KEY pointing at a gateway client token), shows two integration styles (PATH wrapper and explicit hermes config block), and includes a troubleshooting table sourced from the actual error strings in src/proxy.ts (401 Unauthorized - provide client token via x-api-key header, 503 OAuth token not available - gateway is refreshing, 502 Bad gateway).

Risks

  • CI workflow: If npm ci or npm test fails in clean Ubuntu (e.g., a missing dev dependency or a test that reads a host-specific path), the first PR run will go red. That is the point of adding CI, but it does mean this PR's own check could fail and need a follow-up. The risk is bounded to "CI surfaces an existing latent bug"; nothing is deleted or rewritten.
  • Docs: Zero runtime risk. The recipe references endpoints (/_health, /_verify) and error strings that I verified against src/proxy.ts at 5b6f727. The exact hermes-agent config key (claude_code.env) is marked as version-dependent — this is honest given hermes's launcher API is still in flux.
  • Upstream merge: Neither file lives in a path that motiful/cc-gateway currently uses. Future upstream CI (if added) would land at the same path; resolution is a trivial accept-theirs or merge-both.

Test plan

  • CI workflow runs on this PR; both Node 22.x and 24.x jobs go green.
  • If CI is red, root-cause: is it a YAML parse issue (fix here), a flaky test (fix here), or a real test regression on Linux (open follow-up)?
  • Docs render correctly on github.com (tables, code fences).
  • Skim the doc against scripts/add-client.sh to confirm the token-minting command is current.

Codex adversarial review checklist

  1. The name: CI workflow only triggers on pushes to main and PRs targeting main. If this fork later adopts a develop or release branch, will the matrix silently stop covering them? Should this use branches: ['**'] for push events, or is the current scoping intentional?
  2. The hermes-agent doc suggests NODE_TLS_REJECT_UNAUTHORIZED=0 as a workaround for self-signed gateway certs on private networks. Given cc-gateway is explicitly a privacy tool, is normalizing this env var in an official doc inviting users to footgun themselves? Should it be removed and replaced with "add the gateway CA to the hermes host trust store" only?
  3. The Node version matrix is 22.x and 24.x. package.json does not pin an engines.node field. If a user is on Node 20 LTS, the test job won't catch syntax/API regressions for them. Is the right move to (a) add engines.node: ">=22", (b) widen the matrix to include 20.x, or (c) leave as-is because the README already says "Node 22+"?
  4. The doc points operators at /_verify to confirm rewriting works. That endpoint exposes a synthetic before/after of the rewriter against a fake payload — it does not exercise the real hermes payload. Is a recipe that ends at /_verify giving operators false confidence?
  5. There is no caching of the npm install beyond actions/setup-node's built-in cache: npm. For a repo this small that's fine, but if tsx-driven test invocation gets slow, should we layer in actions/cache keyed on tsconfig.json for the tsx transform cache?

Generated by Claude Code

Introduce a minimal GitHub Actions workflow that runs the existing
test suite and TypeScript build on every push and pull request, so
regressions to the rewriter or proxy paths surface automatically.

Also document how to route hermes-agent-spawned Claude Code sessions
through cc-gateway, since downstream sibling projects (hermes-agent,
claw-orchestrator) need a stable integration story for the alpha.
Move the npm ci comment to the line above the step name so the YAML
parser does not see a sibling key at an unexpected indent level.
Copy link
Copy Markdown
Owner Author

Second-eye adversarial review (independent of authoring session)

Fresh-eyes review for Codex. New challenges beyond the 5-item self-authored checklist.

New substantive challenges

  1. Sibling CCR fork has a competing Hermes recipe. claude_code_router#2 documents how to point Hermes at CCR for multi-provider routing. This PR's recipe documents how to point Hermes at cc-gateway for identity normalization. Both set ANTHROPIC_BASE_URL. A user with both repos installed will pick one and ignore the other — but the docs don't reconcile. Worse: the user might think they can chain them (Hermes → cc-gateway → CCR → Anthropic). That chain would double-strip the billing header and may break OAuth, since cc-gateway injects an OAuth token via x-api-key but CCR expects its own auth. Add an explicit "Not chainable with claude_code_router" section.

  2. NODE_TLS_REJECT_UNAUTHORIZED=0 advice is already flagged by the author — but the deeper issue is trust-anchor chain. The recipe suggests "self-signed certs on private networks". For a privacy tool, the trust model matters more than the README acknowledges. If a user adds the gateway's CA to the Hermes host trust store (the alternative the author proposes), they've established a new trust anchor that signs API traffic that hermes-agent's Python TLS stack will trust. Any compromise of the gateway's CA private key becomes an MITM key for the user's API traffic. The recipe should call this out.

  3. CI matrix doesn't include Node 20 LTS. Author's checklist item 3 mentions this. New angle: package.json doesn't pin engines.node. If a downstream consumer (Hermes? claw-orchestrator? cc-gateway adopters in CN regions where 22+ may not be on apt-yet) is on Node 20, the test job won't catch syntax/API regressions affecting them. The matrix decision should be paired with an engines.node field, OR justified as "Node 22+ only, by design".

  4. First-CI-run on this PR may be red, and the PR is the only place the workflow exists. If the workflow fails in clean Ubuntu — for example, because npm ci depends on a registry that's down, or npm test reads a path that's only valid on the author's machine — the PR can't merge until the workflow is fixed, and fixing the workflow requires pushing more commits to this branch. This is fine, but the PR body should explicitly say "if the first CI run is red, it's expected — root-cause and fix here".

  5. The workflow uses npm ci, but package-lock.json may not be checked in. npm ci requires package-lock.json. If the lockfile is .gitignored or absent, the workflow will fail with "no lock file found". Quick check: search the repo for package-lock.json. If present, fine. If absent, replace npm ci with npm install --frozen-lockfile=false or commit a lockfile.

  6. Hermes recipe references /_health and /_verify endpoints. These are documented in the README. But the recipe doesn't say what to do if /_verify shows a successful rewrite for the synthetic payload but the real Hermes-spawned request still gets blocked. That's a real-world failure mode: synthetic payload passes, real payload's metadata.user_id JSON blob has an unexpected field that the rewriter doesn't cover, gateway 503s. Add a "verify with a real Hermes payload" recipe step.

  7. No mention of MCP-proxy. The cc-gateway README §Caveats says mcp-proxy.anthropic.com is hardcoded and bypasses the gateway. Hermes uses MCP heavily. If hermes-agent connects to any official Anthropic MCP server, that traffic skips the gateway and re-establishes a separate identity. The recipe should warn users that the privacy guarantee is partial when MCP servers are in use, or recommend Clash REJECT rules.

Cross-PR consistency

  • claude_code_router#2 recipe vs this one: same problem from the other side. Reconciliation needed in both repos.
  • claude-code-config#4 fleet rollup: "aligned, re-check 30d" matches — this PR adds fork-local CI + docs without touching upstream-tracked paths.
  • The fork-upstream-sync skill at claude-code-config#4 would classify this PR as not requiring an upstream sync trigger (it adds files in .github/ and docs/integrations/, both fork-local). Consistent.

Suggested Codex priority order

  1. P0: Mutual-exclusion / non-chainable warning vs CCR (challenge docs: add CLAUDE.md #1)
  2. P0: MCP-proxy bypass disclosure (challenge feat: auto-sync version from npm registry motiful/cc-gateway#7)
  3. P1: package-lock.json existence verification (challenge 这个项目最大的意义是免费帮别人找漏洞 motiful/cc-gateway#5)
  4. P1: trust-anchor chain implications of NODE_TLS_REJECT_UNAUTHORIZED and CA-add (challenge Add CI workflow and hermes-agent integration recipe #2)
  5. P2: Node 20 LTS matrix decision (challenge fix: refresh token invalid after first use motiful/cc-gateway#3)
  6. P3: real-payload verification recipe step (challenge feat: add upstream proxy support motiful/cc-gateway#6)
  7. P3: red-first-CI-run expectation (challenge 请问作者,今天用这个方案被封号了吗 motiful/cc-gateway#4)

— Posted by Claude Code (independent review session). No code or merge changes.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant