fix: quote shell interpolations and add timeouts in nim.js

[WuKongAI-CMU]

Summary

• quote Docker command arguments in bin/lib/nim.js so sandbox names and image names are shell-safe
• add --connect-timeout 5 to NIM health probes so status checks fail fast instead of hanging
• return running: false from nimStatus() when Docker is unavailable instead of throwing
• add targeted tests that cover the quoted Docker commands and timeout-based health probes

Test plan

• node --test test/nim.test.js

Summary by CodeRabbit

• Improvements
  • Tighter NIM health/readiness probes with connection and overall request timeouts.
  • Explicit Docker availability check so NIM reports not-running when Docker is missing.
• Tests
  • Added tests for health probing, shell command construction, and Docker workflows; includes a helper to simulate runner/process behavior.

[cv]

Hey @WuKongAI-CMU — nice work on the shell interpolation quoting and timeouts in nim.js. Security and robustness improvements like this are always welcome. The codebase has changed quite a bit since this was first opened though (new CI checks, features, restructuring). When you have a moment, would you be able to rebase against the latest main so we can evaluate this with the current state of things? Really appreciate you taking the time to contribute!

[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: 1c011062-d609-4d45-a6ec-4a31be40561c

📥 Commits

Reviewing files that changed from the base of the PR and between 4f706b6 and 45bc12f.

📒 Files selected for processing (2)

• bin/lib/nim.js
• test/nim.test.js

🚧 Files skipped from review as they are similar to previous changes (2)

• bin/lib/nim.js
• test/nim.test.js

---

📝 Walkthrough

Walkthrough

Tightens NIM HTTP health-probe timeouts, adds an explicit Docker availability check in nimStatus, and expands tests with a mocking helper to assert probe flags, Docker command formatting, container lifecycle commands, and probe behavior.

Changes

Cohort / File(s)	Summary
NIM runtime checks bin/lib/nim.js	Replaced plain curl -sf probes with curl -sf --connect-timeout 5 --max-time 5; added an upfront command -v docker check in nimStatus to fail fast when Docker is unavailable.
Tests & helpers test/nim.test.js	Added withMockedRunner helper that stubs ../bin/lib/runner and child_process.spawnSync; added tests verifying Docker inspect/pull/run/stop/rm command construction/quoting, readiness probe uses --connect-timeout 5 --max-time 5, and probe behavior (no spawn calls during probe).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped and timed each tiny curl so spry,
Five seconds to connect, five to try,
If Docker's gone I pause my race,
Tests record each jump and place,
A rabbit’s check keeps servers dry.

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the main changes: adding shell-safe quoting for Docker commands and timeouts to NIM health probes, which aligns directly with the file-level modifications in bin/lib/nim.js.

_{✏️ 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.}

[WuKongAI-CMU]

Rebased onto latest main. The upstream quoting changes (shellQuote) absorbed most of the original diff — the remaining delta is:

• --connect-timeout 5 on both curl health-check calls (prevents indefinite hangs)
• command -v docker guard in nimStatus() (avoids throw when docker isn't installed)
• Updated tests to match shellQuote single-quote output and mock the full runner interface

All 13 nim tests pass. Let me know if there's anything else to address.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@bin/lib/nim.js`:
- Line 155: The two curl invocations that call runCapture (the one constructing
`curl -sf --connect-timeout 5 http://localhost:${safePort}/v1/models` and the
second similar call later) only set --connect-timeout and can hang after
connection; update both invocations to include `--max-time 5` (e.g., `curl -sf
--connect-timeout 5 --max-time 5 ...`) so the total probe duration is bounded;
locate the calls to runCapture in bin/lib/nim.js (the lines constructing the
curl command) and add the `--max-time 5` flag to each curl command string.

In `@test/nim.test.js`:
- Around line 26-29: The test mock for runCapture always returns a string so the
nimStatus error/catch fallback (returning running: false when Docker is missing)
isn't exercised; update the test/nim.test.js mock and tests: make the runCapture
mock (the runCapture function and its runCaptureResults queue) simulate the
Docker-unavailable path by either shifting an undefined/empty value or throwing
an error when runCaptureResults is exhausted, then add an explicit test that
calls nimStatus and asserts it returns running: false (and any expected error
message fields). Ensure you reference the runCapture mock and the nimStatus call
in the new assertion so the catch branch is directly validated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cbf71cb6-7972-468d-9781-b87d9085b569

📥 Commits

Reviewing files that changed from the base of the PR and between d37a09f and d5e9613.

📒 Files selected for processing (2)

• bin/lib/nim.js
• test/nim.test.js

[WuKongAI-CMU]

Addressed the remaining review items in 4f706b6:

• add --max-time 5 to both curl probes so they are bounded end-to-end
• add an explicit docker-unavailable fallback test for nimStatus()

Verification: npx vitest run test/nim.test.js

[WuKongAI-CMU]

Rebased this branch onto the current main by replaying the existing commit stack onto the latest base and force-pushed the refreshed branch. The patch scope is unchanged; this is only a base refresh to keep the PR current.

[WuKongAI-CMU]

The current branch already includes the requested rebase onto latest main plus the follow-up timeout/test fixes. Triggering a fresh pass so review status is based on the current branch tip.

@coderabbitai review

[coderabbitai]

@WuKongAI-CMU: Got it! Kicking off a fresh review of the current branch tip, which includes the rebase onto main and the follow-up fixes (--max-time 5 on both curl probes and the docker-unavailable fallback test). I'll evaluate the full patch scope against the current codebase state.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cv]

On second thought, running curl through runCapture() doesn't make much sense to begin with. We should be using fetch or whatever. Closing as we rework the JS code into something more maintainable shortly.

Thanks for the contrib, @WuKongAI-CMU!