fix(nim): resolve host port dynamically in nimStatusByName()

[latenighthackathon]

Summary

nimStatusByName() hardcoded port 8000 for health checks, but the host port is configurable. Containers on custom ports always reported unhealthy. Now uses docker inspect to resolve the actual host port mapping.

Related Issue

Closes #1016

Changes

• Use docker inspect to read the actual host port from NetworkSettings.Ports
• Validate port is numeric via regex, fall back to 8000 for unexpected output
• Existing callers are unchanged (nimStatus, nimStatusByName)

Type of Change

• Code change for a new feature, bug fix, or refactor.
• Code change with doc updates.
• Doc only. Prose changes without code sample modifications.
• Doc only. Includes code sample changes.

Testing

• npx prek run --all-files passes (or equivalently make check).
• npm test passes.
• make docs builds without warnings. (for doc-only changes)

Checklist

General

• I have read and followed the contributing guide.

Code Changes

• Formatters applied.
• No secrets, API keys, or credentials committed.

Summary by CodeRabbit

• Bug Fixes
  • Improved health check to correctly detect service status regardless of Docker container port configuration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e4f0c4ec-ad9d-47cc-a8b7-825e5ee48e7c

📥 Commits

Reviewing files that changed from the base of the PR and between 94ce221 and 966f0b7.

📒 Files selected for processing (1)

• bin/lib/nim.js

---

📝 Walkthrough

Walkthrough

Updated nimStatusByName() to resolve the container's published host port for internal 8000/tcp via docker inspect when the container is running, defaulting to 8000 if inspection or parsing fails, and then perform the health-check curl against http://localhost:<resolvedPort>/v1/models.

Changes

Cohort / File(s)	Summary
NIM Health Check Port Resolution bin/lib/nim.js	Replaced hardcoded localhost:8000 in nimStatusByName() with a docker inspect lookup of the container's 8000/tcp HostPort, validated as numeric and falling back to 8000 if missing/unparseable; the health-check curl now targets the resolved host port.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I sniffed the ports beneath the log,
Docker whispered down the bog,
Resolved the map, hopped to the gate,
Curl met the model—healthy state! 🥕

🚥 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%.	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 accurately describes the main change: dynamically resolving the host port in nimStatusByName() instead of hardcoding port 8000.
Linked Issues check	✅ Passed	The code changes successfully address all objectives from issue #1016: deriving host port from docker inspect, validating the port numerically, and falling back to 8000 when needed.
Out of Scope Changes check	✅ Passed	Changes are focused solely on fixing the hardcoded port issue in nimStatusByName() without introducing unrelated modifications or addressing the unused _interval variable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

bin/lib/nim.js (1)

204-208: Consider adding port validation to handle unexpected docker inspect output.

The code at line 207 relies on || "8000" to handle missing port mappings, but this only catches falsy values. If docker inspect unexpectedly returns non-numeric text (though unlikely with the deterministic docker inspect --format template), Number(hostPort) could produce NaN at line 208, causing an invalid health-check URL.

Since runCapture already trims output and missing port mappings are caught by the fallback, the actual risk is low. However, adding a simple regex check makes the code more robust:

Optional hardening patch

-      const hostPort = runCapture(
+      const rawHostPort = runCapture(
         `docker inspect --format '{{(index (index .NetworkSettings.Ports "8000/tcp") 0).HostPort}}' ${shellQuote(name)} 2>/dev/null`,
         { ignoreError: true }
-      ) || "8000";
-      const health = runCapture(`curl -sf http://localhost:${Number(hostPort)}/v1/models 2>/dev/null`, {
+      );
+      const hostPort = /^\d+$/.test(rawHostPort) ? rawHostPort : "8000";
+      const health = runCapture(`curl -sf http://localhost:${hostPort}/v1/models 2>/dev/null`, {
         ignoreError: true,
       });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/lib/nim.js` around lines 204 - 208, The hostPort returned by runCapture
may be non-numeric; validate it before using Number(hostPort) in the health curl
call. After computing hostPort (from runCapture or fallback "8000"), test it
with a regex or parseInt+isFinite (e.g., ensure /^\d+$/.test(hostPort) or
Number.isFinite(parsed) ) and if invalid fall back to 8000; then use the
validated numericPort variable in the runCapture used to compute health. Update
references to hostPort/Number(hostPort) so health uses the validated port value.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@bin/lib/nim.js`:
- Around line 204-208: The hostPort returned by runCapture may be non-numeric;
validate it before using Number(hostPort) in the health curl call. After
computing hostPort (from runCapture or fallback "8000"), test it with a regex or
parseInt+isFinite (e.g., ensure /^\d+$/.test(hostPort) or
Number.isFinite(parsed) ) and if invalid fall back to 8000; then use the
validated numericPort variable in the runCapture used to compute health. Update
references to hostPort/Number(hostPort) so health uses the validated port value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6d36cc1b-4259-47d5-b516-8b2f1428a582

📥 Commits

Reviewing files that changed from the base of the PR and between eb4ba8c and 3389c1d.

📒 Files selected for processing (1)

• bin/lib/nim.js

[kjw3]

Thanks for working on this. I reviewed #1068 together with #1022, since they are both trying to fix #1016.

I don’t think we should merge both. Your PR does address the underlying hardcoded-port bug, but #1022 is the stronger version of the fix:

• it covers the same bug with a more complete implementation
• it adds targeted regression tests for the relevant cases
• it has the better validation signal overall

Because of that, I think the right outcome is to take #1022 as the fix for #1016 and close this one as the competing duplicate.

[kjw3]

Closing this in favor of #1022, which covers the same #1016 bug with stronger regression coverage and validation.