fork of hey-api/openapi-ts because I need some additional things
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge pull request #2708 from hey-api/copilot/fix-e387455f-4997-4ca9-ae1c-c7691807966c

authored by

Lubos and committed by
GitHub
1f9056a5 c4fc4aa3

+310 -9
+5
.changeset/lovely-eels-pretend.md
··· 1 + --- 2 + "@hey-api/openapi-ts": patch 3 + --- 4 + 5 + fix(config): do not override interactive config from CLI if defined in config file
-8
packages/openapi-ts/bin/index.cjs
··· 93 93 'useOptions', 94 94 ]); 95 95 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 - 104 96 if (params.plugins === true) { 105 97 userConfig.plugins = []; 106 98 } else if (params.plugins) {
+111
packages/openapi-ts/src/__tests__/error.test.ts
··· 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 + });
+175
packages/openapi-ts/src/__tests__/interactive.test.ts
··· 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 + });
+19 -1
packages/openapi-ts/src/config/init.ts
··· 14 14 import { getPlugins } from './plugins'; 15 15 16 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 + 30 + /** 17 31 * @internal 18 32 */ 19 33 export const initConfigs = async ( ··· 59 73 dryRun = false, 60 74 experimentalParser = true, 61 75 exportCore = true, 62 - interactive = false, 63 76 name, 64 77 request, 65 78 useOptions = true, 66 79 } = userConfig; 80 + 81 + const interactive = 82 + userConfig.interactive !== undefined 83 + ? userConfig.interactive 84 + : detectInteractiveSession(); 67 85 68 86 const errors: Array<Error> = []; 69 87