feat(hermes): use invoke JWT for inference - #4627
Conversation
Signed-off-by: Shannon Sands <shannon.sands.1979@gmail.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✨ Thanks for submitting this detailed PR about updating Hermes to prefer invoke JWT access tokens for inference, including changes to onboarding, token detection, and credential rotation. This proposes a new feature that enhances the inference capability and authentication in Hermes. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
agents/hermes/host/tool-gateway-broker.ts (1)
243-303: 💤 Low valueConsider extracting shared JWT utilities.
The JWT parsing helpers (
decodeBase64UrlJson,decodeJwtClaims,jwtScopes,jwtExpiresAtIso,isUsableInvokeJwt) duplicate logic fromsrc/lib/oauth-device-code.ts. While this duplication is understandable for a standalone broker script, you could consider extracting these to a shared module underagents/hermes/config/to reduce maintenance burden if the JWT inspection logic evolves.🤖 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 `@agents/hermes/host/tool-gateway-broker.ts` around lines 243 - 303, The JWT helpers in this file (decodeBase64UrlJson, decodeJwtClaims, jwtScopes, jwtExpiresAtIso, isUsableInvokeJwt) duplicate logic found in src/lib/oauth-device-code.ts; extract these utilities into a shared module (e.g., agents/hermes/config/jwt-utils) and replace the local implementations with imports from that module. Move the functions decodeBase64UrlJson, decodeJwtClaims, jwtScopes, jwtExpiresAtIso and isUsableInvokeJwt into the new module, export them, update callers here to import and use those exports, and ensure any constants used (e.g., NOUS_INFERENCE_INVOKE_SCOPE, INFERENCE_CREDENTIAL_REFRESH_SKEW_MS, timestampExpiresSoon) are either imported into the shared module or passed through so behavior remains identical.
🤖 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.
Nitpick comments:
In `@agents/hermes/host/tool-gateway-broker.ts`:
- Around line 243-303: The JWT helpers in this file (decodeBase64UrlJson,
decodeJwtClaims, jwtScopes, jwtExpiresAtIso, isUsableInvokeJwt) duplicate logic
found in src/lib/oauth-device-code.ts; extract these utilities into a shared
module (e.g., agents/hermes/config/jwt-utils) and replace the local
implementations with imports from that module. Move the functions
decodeBase64UrlJson, decodeJwtClaims, jwtScopes, jwtExpiresAtIso and
isUsableInvokeJwt into the new module, export them, update callers here to
import and use those exports, and ensure any constants used (e.g.,
NOUS_INFERENCE_INVOKE_SCOPE, INFERENCE_CREDENTIAL_REFRESH_SKEW_MS,
timestampExpiresSoon) are either imported into the shared module or passed
through so behavior remains identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b6faa2ce-8210-4e28-94ca-d134dd0c57c6
📒 Files selected for processing (6)
agents/hermes/host/tool-gateway-broker.tssrc/lib/hermes-provider-auth.test.tssrc/lib/hermes-provider-auth.tssrc/lib/oauth-device-code.test.tssrc/lib/oauth-device-code.tstest/hermes-tool-gateway-broker.test.ts
Manual PR Review Advisor resultThis PR Review Advisor analysis was run manually via Run: https://github.com/NVIDIA/NemoClaw/actions/runs/28206514386 PR Review AdvisorThe invoke-JWT path is covered for the happy path, but the credential trust boundary needs tighter base-URL/JWT validation and fallback regression coverage before this is safe. Required before merge
Resolve or justify before merge
In-scope improvements
Test follow-ups to resolve or justify
What looks good
|
|
Salvaged onto the NVIDIA-owned draft PR #6885 because maintainer pushes to the contributor fork branch are denied. The replacement preserves Shannon Sands as the commit author and carries forward the existing review context; product-scope approval and the credential-boundary findings remain unresolved. Leaving this PR open until the replacement checks settle. |
Summary
Updates Hermes Provider onboarding and the managed tool-gateway broker to prefer Nous
inference:invokeJWT access tokens forinference.local. Legacy agent-key minting remains as a fallback for opaque or non-invoke tokens.Changes
inference:invokeand legacy agent-key scopes during Hermes OAuth device-code onboarding.hermes-provider./api/oauth/agent-keyminting for non-JWT, expired, or non-invoke access tokens.Type of Change
Verification
Focused validation run in
/Users/shannon/.vulcan/projects/nemoclaw-pr-jwt-only:npm run build:clipassesnpm run typecheck:clipassesnpx vitest run src/lib/oauth-device-code.test.ts src/lib/hermes-provider-auth.test.ts test/hermes-tool-gateway-broker.test.ts --maxWorkers=1 --testTimeout=60000passesgit diff --check upstream/main...HEADpassesPre-push hooks were not run in the split worktree because it does not have its own dependency install; the branch was pushed with
--no-verifyafter the focused checks above.npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Shannon Sands shannon.sands.1979@gmail.com
Summary by CodeRabbit