Skip to content

Commit 1f9056a

Browse files
mrlubosmrlubos
authored
Merge pull request #2708 from hey-api/copilot/fix-e387455f-4997-4ca9-ae1c-c7691807966c
2 parents c4fc4aa + bfae15d commit 1f9056a

File tree

5 files changed

+310
-9
lines changed

5 files changed

+310
-9
lines changed

‎.changeset/lovely-eels-pretend.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hey-api/openapi-ts": patch
3+
---
4+
5+
fix(config): do not override interactive config from CLI if defined in config file

‎packages/openapi-ts/bin/index.cjs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,6 @@ async function start() {
9393
'useOptions',
9494
]);
9595

96-
const isInteractive =
97-
process.stdin.isTTY &&
98-
process.stdout.isTTY &&
99-
!process.env.CI &&
100-
!process.env.NO_INTERACTIVE &&
101-
!process.env.NO_INTERACTION;
102-
userConfig.interactive = isInteractive;
103-
10496
if (params.plugins === true) {
10597
userConfig.plugins = [];
10698
} else if (params.plugins) {

‎packages/openapi-ts/src/__tests__/error.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
import { shouldReportCrash } from '../error';
4+
5+
describe('shouldReportCrash', () => {
6+
it('should return false when isInteractive is false', async () => {
7+
const result = await shouldReportCrash({
8+
error: new Error('test error'),
9+
isInteractive: false,
10+
});
11+
expect(result).toBe(false);
12+
});
13+
14+
it('should return false when isInteractive is undefined', async () => {
15+
const result = await shouldReportCrash({
16+
error: new Error('test error'),
17+
isInteractive: undefined,
18+
});
19+
expect(result).toBe(false);
20+
});
21+
22+
it('should not prompt when isInteractive is explicitly false', async () => {
23+
// Mock stdin/stdout to ensure we don't wait for user input
24+
const writeSpy = vi
25+
.spyOn(process.stdout, 'write')
26+
.mockImplementation(() => true);
27+
const setEncodingSpy = vi
28+
.spyOn(process.stdin, 'setEncoding')
29+
.mockImplementation(() => process.stdin as any);
30+
const onceSpy = vi
31+
.spyOn(process.stdin, 'once')
32+
.mockImplementation(() => process.stdin);
33+
34+
const result = await shouldReportCrash({
35+
error: new Error('test error'),
36+
isInteractive: false,
37+
});
38+
39+
expect(result).toBe(false);
40+
expect(writeSpy).not.toHaveBeenCalled();
41+
expect(setEncodingSpy).not.toHaveBeenCalled();
42+
expect(onceSpy).not.toHaveBeenCalled();
43+
44+
writeSpy.mockRestore();
45+
setEncodingSpy.mockRestore();
46+
onceSpy.mockRestore();
47+
});
48+
49+
it('should prompt when isInteractive is true', async () => {
50+
// Mock stdin/stdout for interactive session
51+
const writeSpy = vi
52+
.spyOn(process.stdout, 'write')
53+
.mockImplementation(() => true);
54+
const setEncodingSpy = vi
55+
.spyOn(process.stdin, 'setEncoding')
56+
.mockImplementation(() => process.stdin as any);
57+
const onceSpy = vi
58+
.spyOn(process.stdin, 'once')
59+
.mockImplementation((_event, callback) => {
60+
// Simulate user typing 'n'
61+
setTimeout(() => {
62+
(callback as any)('n');
63+
}, 0);
64+
return process.stdin;
65+
});
66+
67+
const result = await shouldReportCrash({
68+
error: new Error('test error'),
69+
isInteractive: true,
70+
});
71+
72+
expect(result).toBe(false); // User said 'n'
73+
expect(writeSpy).toHaveBeenCalledWith(
74+
expect.stringContaining('📢 Open a GitHub issue with crash details?'),
75+
);
76+
77+
writeSpy.mockRestore();
78+
setEncodingSpy.mockRestore();
79+
onceSpy.mockRestore();
80+
});
81+
82+
it('should handle user saying yes to crash report', async () => {
83+
// Mock stdin/stdout for interactive session
84+
const writeSpy = vi
85+
.spyOn(process.stdout, 'write')
86+
.mockImplementation(() => true);
87+
const setEncodingSpy = vi
88+
.spyOn(process.stdin, 'setEncoding')
89+
.mockImplementation(() => process.stdin as any);
90+
const onceSpy = vi
91+
.spyOn(process.stdin, 'once')
92+
.mockImplementation((_event, callback) => {
93+
// Simulate user typing 'y'
94+
setTimeout(() => {
95+
(callback as any)('y');
96+
}, 0);
97+
return process.stdin;
98+
});
99+
100+
const result = await shouldReportCrash({
101+
error: new Error('test error'),
102+
isInteractive: true,
103+
});
104+
105+
expect(result).toBe(true); // User said 'y'
106+
107+
writeSpy.mockRestore();
108+
setEncodingSpy.mockRestore();
109+
onceSpy.mockRestore();
110+
});
111+
});

‎packages/openapi-ts/src/__tests__/interactive.test.ts‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
import { afterEach, describe, expect, it } from 'vitest';
2+
3+
import { detectInteractiveSession, initConfigs } from '../config/init';
4+
import { mergeConfigs } from '../config/merge';
5+
6+
describe('interactive config', () => {
7+
it('should use detectInteractiveSession when not provided', async () => {
8+
const result = await initConfigs({
9+
input: 'test.json',
10+
output: './test',
11+
});
12+
13+
// In test environment, TTY is typically not available, so it should be false
14+
expect(result.results[0]?.config.interactive).toBe(false);
15+
});
16+
17+
it('should respect user config when set to true', async () => {
18+
const result = await initConfigs({
19+
input: 'test.json',
20+
interactive: true,
21+
output: './test',
22+
});
23+
24+
expect(result.results[0]?.config.interactive).toBe(true);
25+
});
26+
27+
it('should respect user config when set to false', async () => {
28+
const result = await initConfigs({
29+
input: 'test.json',
30+
interactive: false,
31+
output: './test',
32+
});
33+
34+
expect(result.results[0]?.config.interactive).toBe(false);
35+
});
36+
37+
it('should allow file config to set interactive when CLI does not provide it', () => {
38+
// This simulates what happens when:
39+
// 1. User has a config file with interactive: false
40+
// 2. CLI doesn't provide interactive (undefined)
41+
// 3. The bug was: bin script would set interactive = isInteractive (true in TTY)
42+
43+
const fileConfig = {
44+
input: 'test.json',
45+
interactive: false,
46+
output: './test',
47+
};
48+
49+
// CLI config without interactive (correct behavior after fix)
50+
const cliConfigWithoutInteractive = {
51+
input: 'test.json',
52+
output: './test',
53+
};
54+
55+
// CLI config with interactive set to true (simulating the bug)
56+
const cliConfigWithInteractiveBug = {
57+
input: 'test.json',
58+
interactive: true,
59+
output: './test', // Bug: bin script was setting this even when user didn't provide it
60+
};
61+
62+
// After fix: file config's interactive should be preserved
63+
const mergedCorrect = mergeConfigs(fileConfig, cliConfigWithoutInteractive);
64+
expect(mergedCorrect.interactive).toBe(false);
65+
66+
// Before fix: CLI's auto-detected interactive would override file config
67+
const mergedWithBug = mergeConfigs(fileConfig, cliConfigWithInteractiveBug);
68+
expect(mergedWithBug.interactive).toBe(true); // This was the bug - it overrode the file config
69+
});
70+
});
71+
72+
describe('detectInteractiveSession', () => {
73+
const originalEnv = process.env;
74+
const originalStdin = process.stdin;
75+
const originalStdout = process.stdout;
76+
77+
afterEach(() => {
78+
process.env = originalEnv;
79+
Object.defineProperty(process, 'stdin', { value: originalStdin });
80+
Object.defineProperty(process, 'stdout', { value: originalStdout });
81+
});
82+
83+
it('should return false when CI environment variable is set', () => {
84+
process.env = { ...originalEnv, CI: 'true' };
85+
Object.defineProperty(process.stdin, 'isTTY', {
86+
configurable: true,
87+
value: true,
88+
});
89+
Object.defineProperty(process.stdout, 'isTTY', {
90+
configurable: true,
91+
value: true,
92+
});
93+
94+
expect(detectInteractiveSession()).toBe(false);
95+
});
96+
97+
it('should return false when NO_INTERACTIVE environment variable is set', () => {
98+
process.env = { ...originalEnv, NO_INTERACTIVE: '1' };
99+
Object.defineProperty(process.stdin, 'isTTY', {
100+
configurable: true,
101+
value: true,
102+
});
103+
Object.defineProperty(process.stdout, 'isTTY', {
104+
configurable: true,
105+
value: true,
106+
});
107+
108+
expect(detectInteractiveSession()).toBe(false);
109+
});
110+
111+
it('should return false when NO_INTERACTION environment variable is set', () => {
112+
process.env = { ...originalEnv, NO_INTERACTION: '1' };
113+
Object.defineProperty(process.stdin, 'isTTY', {
114+
configurable: true,
115+
value: true,
116+
});
117+
Object.defineProperty(process.stdout, 'isTTY', {
118+
configurable: true,
119+
value: true,
120+
});
121+
122+
expect(detectInteractiveSession()).toBe(false);
123+
});
124+
125+
it('should return false when stdin is not TTY', () => {
126+
process.env = { ...originalEnv };
127+
delete process.env.CI;
128+
delete process.env.NO_INTERACTIVE;
129+
delete process.env.NO_INTERACTION;
130+
Object.defineProperty(process.stdin, 'isTTY', {
131+
configurable: true,
132+
value: false,
133+
});
134+
Object.defineProperty(process.stdout, 'isTTY', {
135+
configurable: true,
136+
value: true,
137+
});
138+
139+
expect(detectInteractiveSession()).toBe(false);
140+
});
141+
142+
it('should return false when stdout is not TTY', () => {
143+
process.env = { ...originalEnv };
144+
delete process.env.CI;
145+
delete process.env.NO_INTERACTIVE;
146+
delete process.env.NO_INTERACTION;
147+
Object.defineProperty(process.stdin, 'isTTY', {
148+
configurable: true,
149+
value: true,
150+
});
151+
Object.defineProperty(process.stdout, 'isTTY', {
152+
configurable: true,
153+
value: false,
154+
});
155+
156+
expect(detectInteractiveSession()).toBe(false);
157+
});
158+
159+
it('should return true when TTY is available and no blocking env vars are set', () => {
160+
process.env = { ...originalEnv };
161+
delete process.env.CI;
162+
delete process.env.NO_INTERACTIVE;
163+
delete process.env.NO_INTERACTION;
164+
Object.defineProperty(process.stdin, 'isTTY', {
165+
configurable: true,
166+
value: true,
167+
});
168+
Object.defineProperty(process.stdout, 'isTTY', {
169+
configurable: true,
170+
value: true,
171+
});
172+
173+
expect(detectInteractiveSession()).toBe(true);
174+
});
175+
});

‎packages/openapi-ts/src/config/init.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,20 @@ import { getProjectDependencies } from './packages';
1313
import { getParser } from './parser';
1414
import { getPlugins } from './plugins';
1515

16+
/**
17+
* Detect if the current session is interactive based on TTY status and environment variables.
18+
* This is used as a fallback when the user doesn't explicitly set the interactive option.
19+
* @internal
20+
*/
21+
export const detectInteractiveSession = (): boolean =>
22+
Boolean(
23+
process.stdin.isTTY &&
24+
process.stdout.isTTY &&
25+
!process.env.CI &&
26+
!process.env.NO_INTERACTIVE &&
27+
!process.env.NO_INTERACTION,
28+
);
29+
1630
/**
1731
* @internal
1832
*/
@@ -59,12 +73,16 @@ export const initConfigs = async (
5973
dryRun = false,
6074
experimentalParser = true,
6175
exportCore = true,
62-
interactive = false,
6376
name,
6477
request,
6578
useOptions = true,
6679
} = userConfig;
6780

81+
const interactive =
82+
userConfig.interactive !== undefined
83+
? userConfig.interactive
84+
: detectInteractiveSession();
85+
6886
const errors: Array<Error> = [];
6987

7088
const logs = getLogs(userConfig);

0 commit comments

Comments
 (0)