Skip to content

Commit 1e333c2

Browse files
prekshivyasPrekshi Vyasclaudecv
authored andcommitted
refactor(cli): port start-services.sh to TypeScript (NVIDIA#1307)
## Summary Closes NVIDIA#1297. Part of NVIDIA#924 shell consolidation (PR 2 scope). Ports the PID-managed service lifecycle (start/stop/status for telegram-bridge and cloudflared) from `scripts/start-services.sh` to `nemoclaw/src/lib/services.ts`: - Uses `execa` with `detached: true` + `unref()` for background service spawning (replaces `nohup`) - Native `process.kill(pid, 0)` for liveness checks (replaces `kill -0`) - File-based PID management preserved (`/tmp/nemoclaw-services-{sandbox}/`) - `bin/nemoclaw.js` start/stop/status now call the TS module directly instead of `bash scripts/start-services.sh` - `scripts/start-services.sh` kept — still used by remote deploy (`ssh ... bash scripts/start-services.sh`) and `uninstall.sh` Pattern: 1. TS source in `nemoclaw/src/lib/services.ts` with flat `ServiceOptions` interface 2. Thin CJS shim in `bin/lib/services.js` 3. Tests in `src/lib/services.test.ts` importing from `../../dist/lib/services` for coverage ## Test plan - [x] `npx vitest run --project plugin` — 201 tests pass (includes new services tests) - [x] `npx vitest run --project cli` — 336 tests pass - [x] `npx vitest run test/service-env.test.js` — 11 tests pass (bash script still works) - [x] `npx eslint nemoclaw/src/lib/services.ts` — clean - [x] Pre-commit hooks pass (prettier, eslint, gitleaks, tsc, vitest) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Service management to start, stop, monitor background services and report statuses, including public connectivity URLs when available * Installer now detects when the user’s shell needs reloading and shows a conditional reload hint * CLI start/stop/status commands now use the integrated service manager for in-process control * **Bug Fixes** * More robust handling of stale/invalid PID files and safer service shutdown behavior * **Tests** * Added comprehensive tests for service behaviors and installer reload hint detection <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Prekshi Vyas <prekshivyas@nvidia.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
1 parent 02ea637 commit 1e333c2

File tree

5 files changed

+561
-5
lines changed

5 files changed

+561
-5
lines changed

bin/lib/services.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// Thin CJS shim — implementation lives in src/lib/services.ts
5+
module.exports = require("../../dist/lib/services");

bin/nemoclaw.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -782,15 +782,19 @@ async function deploy(instanceName) {
782782
}
783783

784784
async function start() {
785+
const { startAll } = require("./lib/services");
785786
const { defaultSandbox } = registry.listSandboxes();
786787
const safeName =
787788
defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
788-
const sandboxEnv = safeName ? `SANDBOX_NAME=${shellQuote(safeName)}` : "";
789-
run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh"`);
789+
await startAll({ sandboxName: safeName || undefined });
790790
}
791791

792792
function stop() {
793-
run(`bash "${SCRIPTS}/start-services.sh" --stop`);
793+
const { stopAll } = require("./lib/services");
794+
const { defaultSandbox } = registry.listSandboxes();
795+
const safeName =
796+
defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
797+
stopAll({ sandboxName: safeName || undefined });
794798
}
795799

796800
function debug(args) {
@@ -865,7 +869,8 @@ function showStatus() {
865869
}
866870

867871
// Show service status
868-
run(`bash "${SCRIPTS}/start-services.sh" --status`);
872+
const { showStatus: showServiceStatus } = require("./lib/services");
873+
showServiceStatus({ sandboxName: defaultSandbox || undefined });
869874
}
870875

871876
async function listSandboxes() {

src/lib/services.test.ts

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
5+
import { mkdtempSync, writeFileSync, existsSync, rmSync } from "node:fs";
6+
import { join } from "node:path";
7+
import { tmpdir } from "node:os";
8+
9+
// Import from compiled dist/ so coverage is attributed correctly.
10+
import {
11+
getServiceStatuses,
12+
showStatus,
13+
stopAll,
14+
} from "../../dist/lib/services";
15+
16+
describe("getServiceStatuses", () => {
17+
let pidDir: string;
18+
19+
beforeEach(() => {
20+
pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-test-"));
21+
});
22+
23+
afterEach(() => {
24+
rmSync(pidDir, { recursive: true, force: true });
25+
});
26+
27+
it("returns stopped status when no PID files exist", () => {
28+
const statuses = getServiceStatuses({ pidDir });
29+
expect(statuses).toHaveLength(2);
30+
for (const s of statuses) {
31+
expect(s.running).toBe(false);
32+
expect(s.pid).toBeNull();
33+
}
34+
});
35+
36+
it("returns service names telegram-bridge and cloudflared", () => {
37+
const statuses = getServiceStatuses({ pidDir });
38+
const names = statuses.map((s) => s.name);
39+
expect(names).toContain("telegram-bridge");
40+
expect(names).toContain("cloudflared");
41+
});
42+
43+
it("detects a stale PID file as not running with null pid", () => {
44+
// Write a PID that doesn't correspond to a running process
45+
writeFileSync(join(pidDir, "cloudflared.pid"), "999999999");
46+
const statuses = getServiceStatuses({ pidDir });
47+
const cf = statuses.find((s) => s.name === "cloudflared");
48+
expect(cf?.running).toBe(false);
49+
// Dead processes should have pid normalized to null
50+
expect(cf?.pid).toBeNull();
51+
});
52+
53+
it("ignores invalid PID file contents", () => {
54+
writeFileSync(join(pidDir, "telegram-bridge.pid"), "not-a-number");
55+
const statuses = getServiceStatuses({ pidDir });
56+
const tg = statuses.find((s) => s.name === "telegram-bridge");
57+
expect(tg?.pid).toBeNull();
58+
expect(tg?.running).toBe(false);
59+
});
60+
61+
it("creates pidDir if it does not exist", () => {
62+
const nested = join(pidDir, "nested", "deep");
63+
const statuses = getServiceStatuses({ pidDir: nested });
64+
expect(existsSync(nested)).toBe(true);
65+
expect(statuses).toHaveLength(2);
66+
});
67+
});
68+
69+
describe("sandbox name validation", () => {
70+
it("rejects names with path traversal", () => {
71+
expect(() => getServiceStatuses({ sandboxName: "../escape" })).toThrow("Invalid sandbox name");
72+
});
73+
74+
it("rejects names with slashes", () => {
75+
expect(() => getServiceStatuses({ sandboxName: "foo/bar" })).toThrow("Invalid sandbox name");
76+
});
77+
78+
it("rejects empty names", () => {
79+
expect(() => getServiceStatuses({ sandboxName: "" })).toThrow("Invalid sandbox name");
80+
});
81+
82+
it("accepts valid alphanumeric names", () => {
83+
expect(() => getServiceStatuses({ sandboxName: "my-sandbox.1" })).not.toThrow();
84+
});
85+
});
86+
87+
describe("showStatus", () => {
88+
let pidDir: string;
89+
90+
beforeEach(() => {
91+
pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-test-"));
92+
});
93+
94+
afterEach(() => {
95+
rmSync(pidDir, { recursive: true, force: true });
96+
});
97+
98+
it("prints stopped status for all services", () => {
99+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
100+
showStatus({ pidDir });
101+
const output = logSpy.mock.calls.map((c) => c[0]).join("\n");
102+
expect(output).toContain("telegram-bridge");
103+
expect(output).toContain("cloudflared");
104+
expect(output).toContain("stopped");
105+
logSpy.mockRestore();
106+
});
107+
108+
it("does not show tunnel URL when cloudflared is not running", () => {
109+
// Write a stale log file but no running process
110+
writeFileSync(
111+
join(pidDir, "cloudflared.log"),
112+
"https://abc-def.trycloudflare.com",
113+
);
114+
writeFileSync(join(pidDir, "cloudflared.pid"), "999999999");
115+
116+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
117+
showStatus({ pidDir });
118+
const output = logSpy.mock.calls.map((c) => c[0]).join("\n");
119+
// Should NOT show the URL since cloudflared is not actually running
120+
expect(output).not.toContain("Public URL");
121+
logSpy.mockRestore();
122+
});
123+
});
124+
125+
describe("stopAll", () => {
126+
let pidDir: string;
127+
128+
beforeEach(() => {
129+
pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-test-"));
130+
});
131+
132+
afterEach(() => {
133+
rmSync(pidDir, { recursive: true, force: true });
134+
});
135+
136+
it("removes stale PID files", () => {
137+
writeFileSync(join(pidDir, "cloudflared.pid"), "999999999");
138+
writeFileSync(join(pidDir, "telegram-bridge.pid"), "999999998");
139+
140+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
141+
stopAll({ pidDir });
142+
logSpy.mockRestore();
143+
144+
expect(existsSync(join(pidDir, "cloudflared.pid"))).toBe(false);
145+
expect(existsSync(join(pidDir, "telegram-bridge.pid"))).toBe(false);
146+
});
147+
148+
it("is idempotent — calling twice does not throw", () => {
149+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
150+
stopAll({ pidDir });
151+
stopAll({ pidDir });
152+
logSpy.mockRestore();
153+
});
154+
155+
it("logs stop messages", () => {
156+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
157+
stopAll({ pidDir });
158+
const output = logSpy.mock.calls.map((c) => c[0]).join("\n");
159+
expect(output).toContain("All services stopped");
160+
logSpy.mockRestore();
161+
});
162+
});

0 commit comments

Comments
 (0)