Skip to content

fix(cli): enhance TTY detection logic for better error handling #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Victor1890 wants to merge 1 commit into from
Closed

fix(cli): enhance TTY detection logic for better error handling #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/cli/cli.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ export class CliController {
this.logger.error(INFO_MSGS.MIN_CLI_CLOMUNS);
this.exitWithError();
}
if (!this.stdout.isTTY) {
if (!this.uiService.stdin.isTTY && !this.stdout.isTTY) {
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

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

The updated TTY check allows execution when only stdout is a TTY (and stdin is not). However, this will cause a runtime error when prepareScreen() is called on line 108, which invokes uiService.setRawMode(). The setRawMode() method calls stdin.setRawMode() which will throw an error if stdin is not a TTY. Consider checking if stdin is a TTY before allowing execution of the interactive UI mode, or add a guard in UiService.setRawMode() to only call stdin.setRawMode() when stdin is a TTY.

Suggested change
if (!this.uiService.stdin.isTTY && !this.stdout.isTTY) {
if (!this.uiService.stdin.isTTY || !this.stdout.isTTY) {

Copilot uses AI. Check for mistakes.
this.uiService.print(INFO_MSGS.NO_TTY);
this.logger.error(INFO_MSGS.NO_TTY);
this.exitWithError();
Expand Down
46 changes: 43 additions & 3 deletions tests/cli/cli.controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import { ConsoleService } from '../../src/cli/services/console.service.js';
import { UpdateService } from '../../src/cli/services/update.service.js';
import { UiService } from '../../src/cli/services/ui.service.js';
import { ScanService } from '../../src/cli/services/scan.service.js';
import { ERROR_MSG } from '../../src/constants/messages.constants.js';
import {
ERROR_MSG,
INFO_MSGS,
} from '../../src/constants/messages.constants.js';
import { JsonOutputService } from '../../src/cli/services/json-output.service.js';
import { ProfilesService } from '../../src/core/services/profiles.service.js';
import { DEFAULT_CONFIG } from '../../src/constants/main.constants.js';
Expand Down Expand Up @@ -124,6 +127,7 @@ describe('CliController test', () => {
setCursorVisible: jest.fn(),
clear: jest.fn(),
renderAll: jest.fn(),
stdin: { isTTY: true },
};
const scanServiceMock = {
scan: jest.fn(),
Expand Down Expand Up @@ -196,8 +200,16 @@ describe('CliController test', () => {
configServiceMock as unknown as ConfigService,
);

Object.defineProperty(process.stdout, 'columns', { value: 80 });
Object.defineProperty(process.stdout, 'isTTY', { value: true });
Object.defineProperty(process.stdout, 'columns', {
value: 80,
configurable: true,
writable: true,
});
Object.defineProperty(process.stdout, 'isTTY', {
value: true,
configurable: true,
writable: true,
});

showHelpSpy = jest
.spyOn(cliController, 'showHelp')
Expand All @@ -219,6 +231,34 @@ describe('CliController test', () => {
expect(checkVersionSpy).toHaveBeenCalledTimes(1);
});

describe('TTY detection behavior', () => {
afterEach(() => {
Object.defineProperty(process.stdout, 'isTTY', { value: true });
uiServiceMock.stdin.isTTY = true;
});

it('allows running when stdin is TTY and stdout is not', () => {
Object.defineProperty(process.stdout, 'isTTY', { value: false });
uiServiceMock.stdin.isTTY = true;

cliController.init();

expect(setupEventsListenerSpy).toHaveBeenCalledTimes(1);
expect(scanSpy).toHaveBeenCalledTimes(1);
expect(exitSpy).toHaveBeenCalledTimes(0);
});

it('exits with NO_TTY when both stdin and stdout are not TTY', () => {
Object.defineProperty(process.stdout, 'isTTY', { value: false });
uiServiceMock.stdin.isTTY = false;

// Expect graceful error exit due to NO_TTY
expect(() => cliController.init()).toThrow();
expect(uiServiceMock.print).toHaveBeenCalledWith(INFO_MSGS.NO_TTY);
expect(exitSpy).toHaveBeenCalledTimes(1);
});
});
Comment on lines +234 to +260
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

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

The test suite is missing a test case for the scenario where stdout is a TTY but stdin is not. This scenario will cause the application to crash when setRawMode() is called on stdin. Add a test case to verify the behavior when uiServiceMock.stdin.isTTY = false and process.stdout.isTTY = true to ensure this edge case is properly handled.

Copilot uses AI. Check for mistakes.

describe('#getArguments', () => {
const mockParameters = (parameters: object) => {
consoleServiceMock.getParameters = () => {
Expand Down