fix(onboard): announce and recover declared agent forward_ports#5389
Merged
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRecovery now re-establishes every non-primary manifest-declared agent forward port during gateway-related sandbox recovery. Onboard output prints all non-primary declared ports (labeling API ports specially) with per-port forwarding URLs and ChangesAgent Forward Port Recovery and Announcement
Sequence Diagram(s)sequenceDiagram
participant checkAndRecoverSandboxProcesses
participant ensureDeclaredAgentForwardPortsHealthy
participant forwardHealthProbe
participant OpenShell
participant result
checkAndRecoverSandboxProcesses->>ensureDeclaredAgentForwardPortsHealthy: inspect active agent forward_ports
ensureDeclaredAgentForwardPortsHealthy->>forwardHealthProbe: probe each non-primary port
alt port missing or unhealthy
ensureDeclaredAgentForwardPortsHealthy->>OpenShell: forward start for port
OpenShell-->>ensureDeclaredAgentForwardPortsHealthy: forward started
end
ensureDeclaredAgentForwardPortsHealthy-->>checkAndRecoverSandboxProcesses: return true/false/null
checkAndRecoverSandboxProcesses->>result: include declaredForwardsRecovered in forwardRecovered
sequenceDiagram
participant printDashboardUi
participant printAdditionalForwardPorts
participant buildControlUiUrls
participant output
printDashboardUi->>printAdditionalForwardPorts: validate agent.forward_ports
printAdditionalForwardPorts->>buildControlUiUrls: generate per-port control UI URLs
buildControlUiUrls-->>printAdditionalForwardPorts: dashboard link(s)
alt port == agent.healthProbe.port
printAdditionalForwardPorts->>output: print "OpenAI-compatible API" block with URLs (/v1)
else
printAdditionalForwardPorts->>output: print "additional port" block with URLs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 44%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
laitingsheng
added
integration: hermes
Contributor
PR Review AdvisorFindings: 0 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 434-476: The loop in ensureDeclaredAgentForwardPortsHealthy
currently only skips primaryPort but must also skip the optional Hermes web
dashboard port so it isn't redundantly managed; retrieve the Hermes dashboard
port using the same helper/logic used elsewhere for Hermes (the code path that
ensures Hermes' dashboard port, referenced by
ensureHermesDashboardPortForwardIfEnabled) for the given sandboxName (or call
the existing helper that returns that port) and add a check in the for loop to
continue when candidate === hermesDashboardPort; keep the other validations and
return behavior unchanged (use agentRuntime.getSessionAgent,
isSandboxPortForwardHealthy, and ensureSandboxPortForwardForPort as before).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a1496e13-5b2d-4780-aec5-0d0471e68f51
📒 Files selected for processing (4)
src/lib/actions/sandbox/process-recovery.tssrc/lib/agent/onboard.test.tssrc/lib/agent/onboard.tstest/process-recovery.test.ts
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: None Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5389.docs.buildwithfern.com/nemoclaw |
…overage Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
cv
approved these changes
Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hermes onboard declares
forward_ports: [18789, 8642]but the dashboard summary only printed the primary port and process recovery only re-established the primary forward. After the OpenShell gateway restarted during policy-preset apply, the secondary OpenAI-compatible API forward on port 8642 was silently dropped and never restored.Related Issue
Fixes #5206
Changes
printDashboardUinow walksagent.forward_portsand emits a labelled block per non-primary entry.checkAndRecoverSandboxProcessesnow invokes a newensureDeclaredAgentForwardPortsHealthyhelper in all three branches.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Tests
Documentation