fix(recovery): let Hermes auto-recovery use config port#2894
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
📝 WalkthroughWalkthroughThe PR adds a ChangesManual Recovery Command Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
Signed-off-by: Aaron Erickson <[email protected]>
Cover both #2426 recovery gaps in the superseding PR: automatic Hermes recovery now launches through config-driven port selection, and the manual fallback message now prints a persistent nohup command instead of the foreground gateway command. The manual fallback approach is adapted from #2438 by @truffle-dev. Signed-off-by: Aaron Erickson <[email protected]> (cherry picked from commit 59ceb0f)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/agent-runtime.ts`:
- Around line 247-255: The manual recovery command uses binaryName which assumes
the binary is on PATH; update buildManualRecoveryCommand so the fallback
defaultGatewayCommand uses the resolved binaryPath (not binaryName) when
agent?.gateway_command is blank, i.e. construct defaultGatewayCommand from
binaryPath (preserving any existing " gateway run" suffix behavior), so
gatewayCmd falls back to the actual resolved binary path and the printed command
is runnable when copy-pasted; adjust any spacing/quoting around envPrefix,
gatewayCmd and portFlag consistently.
🪄 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: 701556bb-92ea-4169-aff1-7db9bc861adf
📒 Files selected for processing (3)
src/lib/agent-runtime.test.tssrc/lib/agent-runtime.tssrc/nemoclaw.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/agent-runtime.test.ts
Use resolved binary paths for manual fallback commands when an agent manifest does not provide gateway_command, matching the intent of the CodeRabbit review on #2894. Also route Hermes recovery and manual fallback through the same decode proxy environment used by agents/hermes/start.sh so recovered gateways preserve credential placeholder handling. Signed-off-by: Aaron Erickson <[email protected]>
|
Thanks for picking this up. The Hermes env-prefix layering plus the omit- |
Selective E2E Results — ✅ All requested jobs passedRun: 25268465794
|
Selective E2E Results — ✅ All requested jobs passedRun: 25268675592
|
ericksoa
added
bug
…recovery Signed-off-by: Aaron Erickson <[email protected]> # Conflicts: # src/nemoclaw.ts
4 participants
Summary
nohup ... &command instead of the foreground-only gateway command.hermes gateway rundoes not receive the external forward port./sandbox/.hermes, and keeps non-Hermes agents on--port <port>.Credit
The manual fallback command approach was originally proposed in #2438 by @truffle-dev. This PR folds that work forward because #2438 is blocked by DCO and far behind current
main, while also fixing the Hermes home path and the remaining automatic-recovery port issue.Context
Addresses #2426. Hermes startup defines
/sandbox/.hermesasHERMES_HOME, configures the API server to listen on internal port18642, and exposes public port8642through the socat bridge. Passing--port 8642to Hermes bypasses that config-driven startup model.Verification
npm run build:clinpx vitest run src/lib/agent-runtime.test.tsType of Change
AI Disclosure
Summary by CodeRabbit
New Features
Tests