Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions api/src/unraid-api/cli/__test__/pm2-commands.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { describe, expect, it, vi } from 'vitest';

import { ECOSYSTEM_PATH } from '@app/environment.js';
import { LogService } from '@app/unraid-api/cli/log.service.js';
import { PM2Service } from '@app/unraid-api/cli/pm2.service.js';
import { RestartCommand } from '@app/unraid-api/cli/restart.command.js';
import { StartCommand } from '@app/unraid-api/cli/start.command.js';
import { StatusCommand } from '@app/unraid-api/cli/status.command.js';
import { StopCommand } from '@app/unraid-api/cli/stop.command.js';

const createLogger = (): LogService =>
({
trace: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
log: vi.fn(),
info: vi.fn(),
debug: vi.fn(),
}) as unknown as LogService;

const createPm2Service = () =>
({
run: vi.fn().mockResolvedValue({ stdout: '', stderr: '' }),
ensurePm2Dependencies: vi.fn().mockResolvedValue(undefined),
deleteDump: vi.fn().mockResolvedValue(undefined),
deletePm2Home: vi.fn().mockResolvedValue(undefined),
forceKillPm2Daemon: vi.fn().mockResolvedValue(undefined),
}) as unknown as PM2Service;

describe('PM2-backed CLI commands', () => {
it('start clears PM2 state and starts without mini-list', async () => {
const logger = createLogger();
const pm2 = createPm2Service();
const command = new StartCommand(logger, pm2);

await command.run([], { logLevel: 'info' });

expect(pm2.ensurePm2Dependencies).toHaveBeenCalledTimes(1);
expect(pm2.deleteDump).toHaveBeenCalledTimes(1);
expect(pm2.deletePm2Home).toHaveBeenCalledTimes(1);
expect(pm2.run).toHaveBeenNthCalledWith(1, { tag: 'PM2 Stop' }, 'stop', ECOSYSTEM_PATH);
expect(pm2.run).toHaveBeenNthCalledWith(2, { tag: 'PM2 Update' }, 'update');
expect(pm2.run).toHaveBeenNthCalledWith(3, { tag: 'PM2 Delete' }, 'delete', ECOSYSTEM_PATH);
expect(pm2.run).toHaveBeenNthCalledWith(4, { tag: 'PM2 Kill' }, 'kill', '--no-autorestart');
expect(pm2.run).toHaveBeenNthCalledWith(
5,
{ tag: 'PM2 Start', raw: true, extendEnv: true, env: { LOG_LEVEL: 'info' } },
'start',
ECOSYSTEM_PATH,
'--update-env'
);
expect(vi.mocked(pm2.run).mock.calls.flat()).not.toContain('--mini-list');
});

it('restart omits mini-list from the PM2 restart call', async () => {
const logger = createLogger();
const pm2 = createPm2Service();
const command = new RestartCommand(logger, pm2);

await command.run([], { logLevel: 'info' });

expect(pm2.run).toHaveBeenCalledWith(
{ tag: 'PM2 Restart', raw: true, extendEnv: true, env: { LOG_LEVEL: 'info' } },
'restart',
ECOSYSTEM_PATH,
'--update-env'
);
expect(vi.mocked(pm2.run).mock.calls.flat()).not.toContain('--mini-list');
});

it('status omits mini-list from the PM2 status call', async () => {
const pm2 = createPm2Service();
const command = new StatusCommand(pm2);

await command.run();

expect(pm2.run).toHaveBeenCalledWith(
{ tag: 'PM2 Status', stdio: 'inherit', raw: true },
'status',
'unraid-api'
);
expect(vi.mocked(pm2.run).mock.calls.flat()).not.toContain('--mini-list');
});

it('stop omits mini-list from the PM2 delete call', async () => {
const pm2 = createPm2Service();
const command = new StopCommand(pm2);

await command.run([], { delete: false });

expect(pm2.run).toHaveBeenCalledWith(
{ tag: 'PM2 Delete', stdio: 'inherit' },
'delete',
ECOSYSTEM_PATH,
'--no-autorestart'
);
expect(vi.mocked(pm2.run).mock.calls.flat()).not.toContain('--mini-list');
});
});
3 changes: 1 addition & 2 deletions api/src/unraid-api/cli/restart.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ export class RestartCommand extends CommandRunner {
{ tag: 'PM2 Restart', raw: true, extendEnv: true, env },
'restart',
ECOSYSTEM_PATH,
'--update-env',
'--mini-list'
'--update-env'
);

if (stderr) {
Expand Down
5 changes: 3 additions & 2 deletions api/src/unraid-api/cli/start.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export class StartCommand extends CommandRunner {
await this.pm2.run({ tag: 'PM2 Update' }, 'update');
await this.pm2.deleteDump();
await this.pm2.run({ tag: 'PM2 Delete' }, 'delete', ECOSYSTEM_PATH);
await this.pm2.run({ tag: 'PM2 Kill' }, 'kill', '--no-autorestart');
await this.pm2.deletePm2Home();
Comment on lines +26 to +27
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Also force-kill the PM2 daemon before deleting PM2_HOME.

The hardened stop --delete path already does pm2 kill then forceKillPm2Daemon() then deletePm2Home(). Skipping forceKillPm2Daemon() here means the new startup-recovery path can still race a stuck daemon and fail to clear the old state cleanly.

Suggested fix
         await this.pm2.run({ tag: 'PM2 Delete' }, 'delete', ECOSYSTEM_PATH);
         await this.pm2.run({ tag: 'PM2 Kill' }, 'kill', '--no-autorestart');
+        await this.pm2.forceKillPm2Daemon();
         await this.pm2.deletePm2Home();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await this.pm2.run({ tag: 'PM2 Kill' }, 'kill', '--no-autorestart');
await this.pm2.deletePm2Home();
await this.pm2.run({ tag: 'PM2 Kill' }, 'kill', '--no-autorestart');
await this.pm2.forceKillPm2Daemon();
await this.pm2.deletePm2Home();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/src/unraid-api/cli/start.command.ts` around lines 26 - 27, The code calls
await this.pm2.run({ tag: 'PM2 Kill' }, 'kill', '--no-autorestart') and then
await this.pm2.deletePm2Home() but omits forceKillPm2Daemon(), which can leave a
stuck daemon racing the delete; insert an awaited call to
this.pm2.forceKillPm2Daemon() between the pm2.run(... 'kill' ...) and
this.pm2.deletePm2Home() calls so the daemon is forcibly terminated before
deleting PM2_HOME.

}

async run(_: string[], options: LogLevelOptions): Promise<void> {
Expand All @@ -33,8 +35,7 @@ export class StartCommand extends CommandRunner {
{ tag: 'PM2 Start', raw: true, extendEnv: true, env },
'start',
ECOSYSTEM_PATH,
'--update-env',
'--mini-list'
'--update-env'
);
if (stdout) {
this.logger.log(stdout.toString());
Expand Down
7 changes: 1 addition & 6 deletions api/src/unraid-api/cli/status.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ export class StatusCommand extends CommandRunner {
super();
}
async run(): Promise<void> {
await this.pm2.run(
{ tag: 'PM2 Status', stdio: 'inherit', raw: true },
'status',
'unraid-api',
'--mini-list'
);
await this.pm2.run({ tag: 'PM2 Status', stdio: 'inherit', raw: true }, 'status', 'unraid-api');
}
}
3 changes: 1 addition & 2 deletions api/src/unraid-api/cli/stop.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ export class StopCommand extends CommandRunner {
{ tag: 'PM2 Delete', stdio: 'inherit' },
'delete',
ECOSYSTEM_PATH,
'--no-autorestart',
'--mini-list'
'--no-autorestart'
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,30 @@ describe('DiskSensorsService', () => {
});

describe('isAvailable', () => {
it('should return true when disks exist', async () => {
it('should return true without checking disks', async () => {
vi.mocked(disksService.getDisks).mockResolvedValue([
{ id: 'disk1', device: '/dev/sda', name: 'Test Disk' } as unknown as Disk,
]);

const available = await service.isAvailable();
expect(available).toBe(true);
expect(disksService.getDisks).not.toHaveBeenCalled();
});

it('should return false when no disks exist', async () => {
it('should return true when no disks exist', async () => {
vi.mocked(disksService.getDisks).mockResolvedValue([]);

const available = await service.isAvailable();
expect(available).toBe(false);
expect(available).toBe(true);
expect(disksService.getDisks).not.toHaveBeenCalled();
});

it('should return false when DisksService throws', async () => {
it('should return true when DisksService would throw', async () => {
vi.mocked(disksService.getDisks).mockRejectedValue(new Error('Failed'));

const available = await service.isAvailable();
expect(available).toBe(false);
expect(available).toBe(true);
expect(disksService.getDisks).not.toHaveBeenCalled();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@ export class DiskSensorsService implements TemperatureSensorProvider {
constructor(private readonly disksService: DisksService) {}

async isAvailable(): Promise<boolean> {
// Disks are always "available" since DisksService exists
try {
const disks = await this.disksService.getDisks();
return disks.length > 0;
} catch {
return false;
}
return true;
}

async read(): Promise<RawTemperatureSensor[]> {
Expand Down
Loading
Loading