Bläddra i källkod

Merge pull request #366 from alvinunreal/fix/interview-auto-open-tests

fix(interview): suppress browser auto-open in tests
Alvin 3 månader sedan
förälder
incheckning
d052fc11cf

+ 1 - 1
docs/configuration.md

@@ -117,7 +117,7 @@ All config files support **JSONC** (JSON with Comments):
 | `todoContinuation.autoEnableThreshold` | integer | `4` | Number of todos that triggers auto-enable (only used when `autoEnable` is true, 1–50) |
 | `interview.maxQuestions` | integer | `2` | Max questions per interview round (1–10) |
 | `interview.outputFolder` | string | `"interview"` | Directory where interview markdown files are written (relative to project root) |
-| `interview.autoOpenBrowser` | boolean | `true` | Automatically open the interview UI in your default browser |
+| `interview.autoOpenBrowser` | boolean | `true` | Automatically open the interview UI in your default browser during interactive runs; suppressed in tests and CI |
 | `interview.port` | integer | `0` | Interview server port (0–65535). `0` = OS-assigned random port (per-session mode). Any value > 0 enables [dashboard mode](interview.md#dashboard-mode) |
 | `interview.dashboard` | boolean | `false` | Enable [dashboard mode](interview.md#dashboard-mode) on the default port (43211). Setting `port` > 0 also enables dashboard mode. If both are set, `port` takes precedence |
 

+ 1 - 1
docs/interview.md

@@ -231,7 +231,7 @@ The dashboard page includes a settings panel for:
 
 - `maxQuestions` — max questions per round, `1-10`, default `2`
 - `outputFolder` — where markdown files are written, default `interview`
-- `autoOpenBrowser` — open the localhost UI in your default browser, default `true`
+- `autoOpenBrowser` — open the localhost UI in your default browser during interactive runs, default `true` (suppressed automatically in tests and CI)
 - `port` — port for the interview server, `0-65535`, default `0` (OS-assigned in per-session mode). Set a fixed port to enable dashboard mode. Note: ports 1-1023 require elevated privileges on most systems.
 - `dashboard` — enable dashboard mode on the default port (`43211`), default `false`. Setting `port` to a value greater than `0` also enables dashboard mode. If both are set, `port` takes precedence.
 

+ 1 - 0
oh-my-opencode-slim.schema.json

@@ -451,6 +451,7 @@
         },
         "autoOpenBrowser": {
           "default": true,
+          "description": "Automatically open the interview UI in your default browser during interactive runs. Disabled automatically in tests and CI.",
           "type": "boolean"
         },
         "port": {

+ 6 - 1
src/config/schema.ts

@@ -162,7 +162,12 @@ export type McpName = z.infer<typeof McpNameSchema>;
 export const InterviewConfigSchema = z.object({
   maxQuestions: z.number().int().min(1).max(10).default(2),
   outputFolder: z.string().min(1).default('interview'),
-  autoOpenBrowser: z.boolean().default(true),
+  autoOpenBrowser: z
+    .boolean()
+    .default(true)
+    .describe(
+      'Automatically open the interview UI in your default browser during interactive runs. Disabled automatically in tests and CI.',
+    ),
   port: z.number().int().min(0).max(65535).default(0),
   dashboard: z.boolean().default(false),
 });

+ 104 - 9
src/interview/interview.test.ts

@@ -80,20 +80,31 @@ function requireInterviewId(value: string | null): string {
 
 function createInterviewService(
   ctx: ReturnType<typeof createMockContext>,
-  config?: Parameters<typeof createRealInterviewService>[1],
+  config?: Partial<Parameters<typeof createRealInterviewService>[1]>,
+  deps?: Parameters<typeof createRealInterviewService>[2],
 ) {
-  return createRealInterviewService(ctx, config, {
+  const resolvedConfig = config
+    ? InterviewConfigSchema.parse(config)
+    : undefined;
+
+  return createRealInterviewService(ctx, resolvedConfig, {
     openBrowser: mock((_url: string) => {}),
+    ...deps,
   });
 }
 
 function createTestService(
   ctx: ReturnType<typeof createMockContext>,
-  config?: Parameters<typeof createRealInterviewService>[1],
+  config?: Partial<Parameters<typeof createRealInterviewService>[1]>,
+  deps?: Parameters<typeof createRealInterviewService>[2],
 ) {
   const openBrowserMock = mock((_url: string) => {});
-  const service = createRealInterviewService(ctx, config, {
+  const resolvedConfig = config
+    ? InterviewConfigSchema.parse(config)
+    : undefined;
+  const service = createRealInterviewService(ctx, resolvedConfig, {
     openBrowser: openBrowserMock,
+    ...deps,
   });
 
   return {
@@ -102,6 +113,12 @@ function createTestService(
   };
 }
 
+function createRuntimeEnv(
+  overrides: NodeJS.ProcessEnv = {},
+): NodeJS.ProcessEnv {
+  return { ...overrides };
+}
+
 describe('interview service', () => {
   describe('/interview <idea> command', () => {
     test('creates interview and sends kickoff prompt with UI notification', async () => {
@@ -1393,15 +1410,93 @@ describe('interview service', () => {
   });
 
   describe('autoOpenBrowser config', () => {
+    test('does not open a browser during automated test runtimes', async () => {
+      const tempDir = await fs.mkdtemp('/tmp/interview-test-');
+      const ctx = createMockContext({ directory: tempDir });
+
+      const { service, openBrowserMock } = createTestService(
+        ctx,
+        {
+          maxQuestions: 2,
+          outputFolder: 'interview',
+          autoOpenBrowser: true,
+        },
+        {
+          env: createRuntimeEnv({
+            NODE_ENV: 'test',
+            CI: '0',
+          }),
+        },
+      );
+      service.setBaseUrlResolver(async () => 'http://localhost:9999');
+      const output = { parts: [] as Array<{ type: string; text?: string }> };
+
+      await service.handleCommandExecuteBefore(
+        {
+          command: 'interview',
+          sessionID: 'session-browser-test-env',
+          arguments: 'Browser Test Env',
+        },
+        output,
+      );
+
+      expect(openBrowserMock).not.toHaveBeenCalled();
+
+      await fs.rm(tempDir, { recursive: true, force: true });
+    });
+
+    test('does not open a browser in CI even when auto-open is enabled', async () => {
+      const tempDir = await fs.mkdtemp('/tmp/interview-test-');
+      const ctx = createMockContext({ directory: tempDir });
+
+      const { service, openBrowserMock } = createTestService(
+        ctx,
+        {
+          maxQuestions: 2,
+          outputFolder: 'interview',
+          autoOpenBrowser: true,
+        },
+        {
+          env: createRuntimeEnv({
+            CI: 'true',
+          }),
+        },
+      );
+      service.setBaseUrlResolver(async () => 'http://localhost:9999');
+      const output = { parts: [] as Array<{ type: string; text?: string }> };
+
+      await service.handleCommandExecuteBefore(
+        {
+          command: 'interview',
+          sessionID: 'session-browser-ci-env',
+          arguments: 'Browser CI Env',
+        },
+        output,
+      );
+
+      expect(openBrowserMock).not.toHaveBeenCalled();
+
+      await fs.rm(tempDir, { recursive: true, force: true });
+    });
+
     test('uses injected browser opener instead of opening a real browser in tests', async () => {
       const tempDir = await fs.mkdtemp('/tmp/interview-test-');
       const ctx = createMockContext({ directory: tempDir });
 
-      const { service, openBrowserMock } = createTestService(ctx, {
-        maxQuestions: 2,
-        outputFolder: 'interview',
-        autoOpenBrowser: true,
-      });
+      const { service, openBrowserMock } = createTestService(
+        ctx,
+        {
+          maxQuestions: 2,
+          outputFolder: 'interview',
+          autoOpenBrowser: true,
+        },
+        {
+          env: createRuntimeEnv({
+            NODE_ENV: 'development',
+            CI: '0',
+          }),
+        },
+      );
       service.setBaseUrlResolver(async () => 'http://localhost:9999');
       const output = { parts: [] as Array<{ type: string; text?: string }> };
 

+ 31 - 2
src/interview/service.ts

@@ -40,7 +40,32 @@ import type {
 
 const COMMAND_NAME = 'interview';
 const DEFAULT_MAX_QUESTIONS = 2;
-const DEFAULT_AUTO_OPEN_BROWSER = process.env.NODE_ENV !== 'test';
+
+function isTruthyEnvFlag(value: string | undefined): boolean {
+  if (!value) {
+    return false;
+  }
+
+  return value !== '0' && value.toLowerCase() !== 'false';
+}
+
+function isAutomatedRuntime(env: NodeJS.ProcessEnv): boolean {
+  return (
+    env.NODE_ENV === 'test' ||
+    isTruthyEnvFlag(env.CI) ||
+    isTruthyEnvFlag(env.BUN_TEST) ||
+    isTruthyEnvFlag(env.VITEST) ||
+    env.JEST_WORKER_ID !== undefined
+  );
+}
+
+function shouldAutoOpenBrowser(
+  config: InterviewConfig | undefined,
+  env: NodeJS.ProcessEnv,
+): boolean {
+  const requested = config?.autoOpenBrowser ?? true;
+  return requested && !isAutomatedRuntime(env);
+}
 
 /**
  * Open a URL in the default browser.
@@ -86,6 +111,7 @@ export function createInterviewService(
   config?: InterviewConfig,
   deps?: {
     openBrowser?: (url: string) => void;
+    env?: NodeJS.ProcessEnv;
   },
 ): {
   setBaseUrlResolver: (resolver: () => Promise<string>) => void;
@@ -120,7 +146,10 @@ export function createInterviewService(
   const outputFolder = normalizeOutputFolder(
     config?.outputFolder ?? DEFAULT_OUTPUT_FOLDER,
   );
-  const autoOpenBrowser = config?.autoOpenBrowser ?? DEFAULT_AUTO_OPEN_BROWSER;
+  const autoOpenBrowser = shouldAutoOpenBrowser(
+    config,
+    deps?.env ?? process.env,
+  );
   const browserOpener = deps?.openBrowser ?? openBrowser;
   const activeInterviewIds = new Map<string, string>();
   const interviewsById = new Map<string, InterviewRecord>();