fix(inference): close llama bridge after listener failure by ericksoa · Pull Request #12515 · NVIDIA/NemoClaw

ericksoa · 2026-09-30T04:36:57Z

Outcome

Close every managed llama.cpp bridge listener when startup or a listener fails. Previously, one successful bind kept the bridge process alive after the other bind failed.

Reason

The bridge starts two HTTP listeners with Promise.all. If one bind rejects, the open listener prevents process exit even after the top-level handler sets a failing exit code. The lifecycle's process-existence check can then mistake failed startup for a running bridge.

Related issues

Refs #12285. This repairs a reproduced cleanup defect found during that investigation. It does not establish that the original N1x onboarding failure is resolved; keep the issue open.

Changes

Close both HTTP servers and their active connections when startup or a later listener error rejects.
Remove the failed bridge's signal handlers during cleanup.
Add a regression that forces a real socket bind failure and checks listener and signal-handler cleanup.
Merge main at 1ccec4e141b0a830229ef68c96639851d24810fd, including the audit repair in fix(ci): upgrade OpenClaw and repair audit and runtime qualification #12507. The merge had no conflicts, and the PR diff remains limited to the two bridge files.

Verification

The regression failed before the fix because one listener remained active after EADDRINUSE.
npx vitest run --project cli src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts src/lib/inference/llama-cpp/host-local-runtime.test.ts — 119 tests passed after merging main.
Normal commit and push hooks — passed, including publication validation and plugin, JavaScript, and CLI type checks.
git diff --check origin/main...HEAD — passed.
GitHub marks both PR commits as Verified. The latest commit is 4af6d2142970c88cc757c1264252ddf9d9f22100.
PR npm audit and reviewed-npm-audit — both passed on 4af6d2142970c88cc757c1264252ddf9d9f22100, clearing the prior dependency-audit failures.
Diff inspection found no secrets, API keys, or credentials.

Review notes

Both changed files are sensitive paths under src/lib/onboard/**. Implementation self-review covered 4af6d2142970c88cc757c1264252ddf9d9f22100 against 1ccec4e141b0a830229ef68c96639851d24810fd. CodeRabbit skipped review while the PR was a draft; no independent review is claimed.

N1x Windows-on-Arm/WSL2/Docker Desktop validation remains outstanding. The request guard intentionally listens on 8081 and forwards to llama-server on loopback 8082. Before claiming an issue fix, verify container-local 8081/health, both bridge binds, WSL-to-container reachability, onboarding, and a real sandbox inference request. No live E2E was run for this change.

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

Bug Fixes
- Improved cleanup when bridge startup fails or its wait loop exits with an error, closing connections and restoring signal handlers.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

copy-pr-bot · 2026-09-30T04:37:00Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

coderabbitai · 2026-09-30T04:37:04Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

test/README.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4be1d087-9d6d-438a-884a-52bb4074eb88

📥 Commits

Reviewing files that changed from the base of the PR and between 1ccec4e and 4af6d21.

📒 Files selected for processing (2)

src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts
src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

Walkthrough

runLlamaCppPrivateBridge now removes signal handlers and closes bridge servers in a finally block. A test checks cleanup when the second listener fails with EADDRINUSE.

Changes

Private bridge cleanup

Layer / File(s)	Summary
Startup cleanup and failure test `src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts`, `src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts`	Startup and the error-wait loop run inside `try/finally`, which removes signal handlers and closes servers and connections. The test checks that both servers close and the original signal listeners are restored after the second listener fails with `EADDRINUSE`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: apurvvkumaria

Merge Risk: ⚪ Minimal · up to 4af6d

This change fixes the cleanup of listeners and signal handlers when the private llama.cpp bridge fails to start. It has a regression test and no identified merge-blocking risk. The original N1x onboarding issue still needs live validation, which is separate from this cleanup fix.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: closing the Llama bridge after a listener startup failure.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

_{Comment @coderabbitai help to get the list of available commands.}

github-code-quality · 2026-09-30T04:38:22Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 4af6d21 in the fix/issue-12285-llam... branch is 97%. The line coverage in commit 63002cd in the main branch is 96%.

Show a line coverage summary of the most impacted files.

File	`main` 63002cd	`fix/issue-12285-llam...` 4af6d21	+/-
`nemoclaw/src/onboard/config.ts`	98%	96%	-2%
`nemoclaw/src/index.ts`	94%	93%	-1%
`nemoclaw/src/bl...t-management.ts`	100%	100%	0%
`nemoclaw/src/co.../config-show.ts`	100%	100%	0%
`nemoclaw/src/commands/slash.ts`	100%	100%	0%
`nemoclaw/src/on...native-route.ts`	0%	100%	+100%

TypeScript / code-coverage/cli

The overall line coverage in commit 4af6d21 in the fix/issue-12285-llam... branch is 85%. The line coverage in commit 63002cd in the main branch is 84%.

Show a line coverage summary of the most impacted files.

File	`main` 63002cd	`fix/issue-12285-llam...` 4af6d21	+/-
`src/lib/actions.../status-text.ts`	84%	46%	-38%
`src/lib/onboard...al-inference.ts`	84%	90%	+6%
`src/lib/inferen...file/cleanup.ts`	73%	80%	+7%
`src/lib/state/p...l-retirement.ts`	79%	89%	+10%
`src/lib/readine...y-production.ts`	76%	90%	+14%
`src/lib/onboard.../application.ts`	55%	72%	+17%
`src/lib/onboard...mage/catalog.ts`	69%	90%	+21%
`src/lib/securit...zer-boundary.ts`	0%	85%	+85%
`src/lib/onboard...ternal-image.ts`	0%	94%	+94%
`src/lib/securit...ig-structure.ts`	0%	94%	+94%

_{Updated October 01, 2026 07:39 UTC}

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

github-actions · 2026-10-01T07:47:03Z

PR Review Advisor finished for commit 4af6d21. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

Request review only when Require no Advisor blockers is green.

All previous runs

Establish Docker isolation for non-live WSL tests and restore a healthy daemon before live testing. Recover a known unavailable systemd bus with one bounded restart of the job-owned distro, mask both Docker service and socket persistently, and recheck Docker immediately before Vitest. Remove the obsolete exact-timeout expectation from the rebuild recovery test while preserving target selection and recovery-state assertions. Validation: 42 WSL helper/workflow tests and 43 rebuild recovery tests passed locally; publication validation and CLI type checking passed. CI, Docker and Podman managed-runtime activation, and all nine Advisor specialist reports passed on 2b615c9. Actual WSL validation follows this main-only workflow update. Refs #12285 and #12515. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

fix(inference): close llama bridge after listener failure

a81ec55

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

merge: integrate main into llama bridge cleanup

4af6d21

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

ericksoa marked this pull request as ready for review October 1, 2026 14:12

ericksoa merged commit 05f21fc into main Oct 1, 2026
127 checks passed

ericksoa deleted the fix/issue-12285-llama-listener branch October 1, 2026 14:31

github-actions Bot added the v0.0.130 label Oct 1, 2026

ericksoa mentioned this pull request Oct 1, 2026

fix(ci): isolate Docker across WSL test phases #12554

Merged

Navigation Menu

Repository navigation

fix(inference): close llama bridge after listener failure - #12515

fix(inference): close llama bridge after listener failure#12515
ericksoa merged 2 commits into
mainfrom
fix/issue-12285-llama-listener

ericksoa commented Sep 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

copy-pr-bot Bot commented Sep 30, 2026

Uh oh!

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-code-quality Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ericksoa commented Sep 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Outcome

Reason

Related issues

Changes

Verification

Review notes

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Sep 30, 2026

Uh oh!

coderabbitai Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-code-quality Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

github-actions Bot commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ericksoa commented Sep 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading

github-code-quality Bot commented Sep 30, 2026 •

edited

Loading