Fix OpenAI tool format overrides (Fixes #1943)

packages/cli/src/config/extensions/github.ts

@@ -560,6 +560,19 @@ export interface DownloadOptions {
   headers?: Record<string, string>;
 }
 
+function cleanupAfterFileClosed(
+  file: fs.WriteStream,
+  err: Error,
+  cleanupAndReject: (err: Error) => void,
+): void {
+  if (file.closed) {
+    cleanupAndReject(err);
+    return;
+  }
+  file.once('close', () => cleanupAndReject(err));
+  file.destroy();
+}
+
 export async function downloadFile(
   url: string,
   dest: string,
@@ -577,7 +590,14 @@ export async function downloadFile(
   }
 
   return new Promise((resolve, reject) => {
+    let settled = false;
+
     const cleanupAndReject = (error: Error) => {
+      if (settled) {
+        return;
+      }
+      settled = true;
+
       // Try to clean up the partial file, but don't mask the original error
       fs.unlink(dest, (unlinkErr) => {
         if (unlinkErr && unlinkErr.code !== 'ENOENT') {
@@ -619,17 +639,21 @@ export async function downloadFile(
         // Handle file write errors
         file.on('error', (err) => {
           res.destroy();
-          cleanupAndReject(err);
+          cleanupAfterFileClosed(file, err, cleanupAndReject);
         });
 
         // Handle response stream errors
         res.on('error', (err) => {
-          file.destroy();
-          cleanupAndReject(err);
+          cleanupAfterFileClosed(file, err, cleanupAndReject);
         });
 
         res.pipe(file);
-        file.on('finish', () => file.close(resolve as () => void));
+        file.on('finish', () => {
+          if (!settled) {
+            settled = true;
+            file.close(resolve as () => void);
+          }
+        });
       })
       .on('error', (err) => {
         cleanupAndReject(err);

packages/cli/src/runtime/providerMutations.issue1943.test.ts

@@ -0,0 +1,105 @@
+/**
+ * @issue #1943 - /toolformat is not persisted into profile ephemerals
+ *
+ * Behavioral tests for setActiveToolFormatOverride() writing to both
+ * SettingsService AND Config ephemeral settings, so the tool-format value
+ * is captured during profile saves.
+ *
+ * Before the fix, only settingsService.updateSettings() was called.
+ * After the fix, config.setEphemeralSetting('tool-format', value) is also called,
+ * mirroring the pattern used by updateActiveProviderBaseUrl.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mockConfig = {
+  setEphemeralSetting: vi.fn(),
+  getEphemeralSetting: vi.fn().mockReturnValue(undefined),
+};
+
+const mockSettingsService = {
+  updateSettings: vi.fn().mockResolvedValue(undefined),
+  getProviderSettings: vi.fn().mockReturnValue({}),
+};
+
+const mockProviderManager = {
+  getActiveProvider: vi.fn().mockReturnValue({ name: 'openai' }),
+  getActiveProviderName: vi.fn().mockReturnValue('openai'),
+};
+
+vi.mock('./runtimeAccessors.js', () => ({
+  getCliRuntimeServices: () => ({
+    config: mockConfig,
+    settingsService: mockSettingsService,
+    providerManager: mockProviderManager,
+  }),
+  _internal: {
+    getActiveProviderOrThrow: () => ({ name: 'openai' }),
+    getProviderSettingsSnapshot: () => ({}),
+  },
+}));
+
+import { setActiveToolFormatOverride } from './providerMutations.js';
+
+describe('setActiveToolFormatOverride ephemeral persistence (issue #1943)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockSettingsService.getProviderSettings.mockReturnValue({});
+  });
+
+  it('writes "openai" to config.setEphemeralSetting when setting toolFormat to "openai"', async () => {
+    await setActiveToolFormatOverride('openai');
+
+    expect(mockSettingsService.updateSettings).toHaveBeenCalledWith('openai', {
+      toolFormat: 'openai',
+    });
+    expect(mockConfig.setEphemeralSetting).toHaveBeenCalledWith(
+      'tool-format',
+      'openai',
+    );
+  });
+
+  it('writes "auto" to config.setEphemeralSetting when clearing override', async () => {
+    await setActiveToolFormatOverride(null);
+
+    expect(mockSettingsService.updateSettings).toHaveBeenCalledWith('openai', {
+      toolFormat: 'auto',
+    });
+    expect(mockConfig.setEphemeralSetting).toHaveBeenCalledWith(
+      'tool-format',
+      'auto',
+    );
+  });
+
+  it('writes "auto" to config.setEphemeralSetting when explicitly setting to "auto"', async () => {
+    await setActiveToolFormatOverride('auto');
+
+    expect(mockSettingsService.updateSettings).toHaveBeenCalledWith('openai', {
+      toolFormat: 'auto',
+    });
+    expect(mockConfig.setEphemeralSetting).toHaveBeenCalledWith(
+      'tool-format',
+      'auto',
+    );
+  });
+
+  it('writes "kimi" to config.setEphemeralSetting when setting toolFormat to "kimi"', async () => {
+    await setActiveToolFormatOverride('kimi');
+
+    expect(mockSettingsService.updateSettings).toHaveBeenCalledWith('openai', {
+      toolFormat: 'kimi',
+    });
+    expect(mockConfig.setEphemeralSetting).toHaveBeenCalledWith(
+      'tool-format',
+      'kimi',
+    );
+  });
+
+  it('calls both settingsService and config (mirrors updateActiveProviderBaseUrl pattern)', async () => {
+    await setActiveToolFormatOverride('openai');
+
+    // Both should be called exactly once
+    expect(mockSettingsService.updateSettings).toHaveBeenCalledTimes(1);
+    expect(mockConfig.setEphemeralSetting).toHaveBeenCalledTimes(1);
+  });
+});

packages/cli/src/runtime/providerMutations.ts

@@ -376,17 +376,19 @@ export async function getActiveToolFormatState(): Promise<ToolFormatState> {
 export async function setActiveToolFormatOverride(
   formatName: ToolFormatOverrideLiteral | null,
 ): Promise<ToolFormatState> {
-  const { settingsService } = getCliRuntimeServices();
+  const { config, settingsService } = getCliRuntimeServices();
   const provider = _internal.getActiveProviderOrThrow();
 
   if (!formatName || formatName === 'auto') {
     await settingsService.updateSettings(provider.name, { toolFormat: 'auto' });
+    config.setEphemeralSetting('tool-format', 'auto');
     return getActiveToolFormatState();
   }
 
   await settingsService.updateSettings(provider.name, {
     toolFormat: formatName,
   });
+  config.setEphemeralSetting('tool-format', formatName);
   return getActiveToolFormatState();
 }