Merge pull request #97 from actions/sgoedecke/defensive-parsing

Parse inference response format defensively

.github/workflows/licensed.yml

@@ -42,11 +42,11 @@ jobs:
 
       - name: Setup Ruby
         id: setup-ruby
-        uses: ruby/setup-ruby@v1
+        uses: ruby/setup-ruby@829114fc20da43a41d27359103ec7a63020954d4
         with:
           ruby-version: ruby
 
-      - uses: licensee/setup-licensed@v1.3.2
+      - uses: licensee/setup-licensed@0d52e575b3258417672be0dff2f115d7db8771d8
         with:
           version: 4.x
           github_token: ${{ secrets.GITHUB_TOKEN }}

.licenses/npm/@types/tmp.dep.yml

@@ -0,0 +1,32 @@
+---
+name: "@types/tmp"
+version: 0.2.6
+type: npm
+summary: TypeScript definitions for tmp
+homepage: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/tmp
+license: mit
+licenses:
+- sources: LICENSE
+  text: |2
+        MIT License
+
+        Copyright (c) Microsoft Corporation.
+
+        Permission is hereby granted, free of charge, to any person obtaining a copy
+        of this software and associated documentation files (the "Software"), to deal
+        in the Software without restriction, including without limitation the rights
+        to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+        copies of the Software, and to permit persons to whom the Software is
+        furnished to do so, subject to the following conditions:
+
+        The above copyright notice and this permission notice shall be included in all
+        copies or substantial portions of the Software.
+
+        THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+        IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+        FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+        AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+        LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+        OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+        SOFTWARE
+notices: []

.licenses/npm/tmp.dep.yml

@@ -0,0 +1,32 @@
+---
+name: tmp
+version: 0.2.5
+type: npm
+summary: Temporary file and directory creator
+homepage: http://github.com/raszi/node-tmp
+license: mit
+licenses:
+- sources: LICENSE
+  text: |
+    The MIT License (MIT)
+
+    Copyright (c) 2014 KARASZI István
+
+    Permission is hereby granted, free of charge, to any person obtaining a copy
+    of this software and associated documentation files (the "Software"), to deal
+    in the Software without restriction, including without limitation the rights
+    to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+    copies of the Software, and to permit persons to whom the Software is
+    furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included in all
+    copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+    AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+    OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+    SOFTWARE.
+notices: []

.node-version

@@ -1 +1 @@
-20.9.0
+24.4.0

__tests__/inference.test.ts

@@ -95,6 +95,49 @@ describe('inference.ts', () => {
       expect(result).toBeNull()
       expect(core.info).toHaveBeenCalledWith('Model response: No response content')
     })
+
+    it('includes response format when specified', async () => {
+      const requestWithResponseFormat = {
+        ...mockRequest,
+        responseFormat: {
+          type: 'json_schema' as const,
+          json_schema: {type: 'object'},
+        },
+      }
+
+      const mockResponse = {
+        choices: [
+          {
+            message: {
+              content: '{"result": "success"}',
+            },
+          },
+        ],
+      }
+
+      mockCreate.mockResolvedValue(mockResponse)
+
+      const result = await simpleInference(requestWithResponseFormat)
+
+      expect(result).toBe('{"result": "success"}')
+
+      // Verify response format was included in the request
+      expect(mockCreate).toHaveBeenCalledWith({
+        messages: [
+          {
+            role: 'system',
+            content: 'You are a test assistant',
+          },
+          {
+            role: 'user',
+            content: 'Hello, AI!',
+          },
+        ],
+        max_tokens: 100,
+        model: 'gpt-4',
+        response_format: requestWithResponseFormat.responseFormat,
+      })
+    })
   })
 
   describe('mcpInference', () => {
@@ -140,6 +183,7 @@ describe('inference.ts', () => {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       const callArgs = mockCreate.mock.calls[0][0] as any
       expect(callArgs.tools).toEqual(mockMcpClient.tools)
+      expect(callArgs.response_format).toBeUndefined()
       expect(callArgs.model).toBe('gpt-4')
       expect(callArgs.max_tokens).toBe(100)
     })
@@ -315,5 +359,191 @@ describe('inference.ts', () => {
 
       expect(result).toBe('Second message')
     })
+
+    it('makes additional loop with response format when no tool calls are made', async () => {
+      const requestWithResponseFormat = {
+        ...mockRequest,
+        responseFormat: {
+          type: 'json_schema' as const,
+          json_schema: {type: 'object'},
+        },
+      }
+
+      // First response without tool calls
+      const firstResponse = {
+        choices: [
+          {
+            message: {
+              content: 'First response',
+              tool_calls: null,
+            },
+          },
+        ],
+      }
+
+      // Second response with response format applied
+      const secondResponse = {
+        choices: [
+          {
+            message: {
+              content: '{"result": "formatted response"}',
+              tool_calls: null,
+            },
+          },
+        ],
+      }
+
+      mockCreate.mockResolvedValueOnce(firstResponse).mockResolvedValueOnce(secondResponse)
+
+      const result = await mcpInference(requestWithResponseFormat, mockMcpClient)
+
+      expect(result).toBe('{"result": "formatted response"}')
+      expect(mockCreate).toHaveBeenCalledTimes(2)
+      expect(core.info).toHaveBeenCalledWith('Making one more MCP loop with the requested response format...')
+
+      // First call should have tools but no response format
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const firstCall = mockCreate.mock.calls[0][0] as any
+      expect(firstCall.tools).toEqual(mockMcpClient.tools)
+      expect(firstCall.response_format).toBeUndefined()
+
+      // Second call should have response format but no tools
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const secondCall = mockCreate.mock.calls[1][0] as any
+      expect(secondCall.tools).toBeUndefined()
+      expect(secondCall.response_format).toEqual(requestWithResponseFormat.responseFormat)
+
+      // Second call should include the user message requesting JSON format
+      expect(secondCall.messages).toHaveLength(5) // system, user, assistant, user, assistant
+      expect(secondCall.messages[3].role).toBe('user')
+      expect(secondCall.messages[3].content).toContain('Please provide your response in the exact')
+    })
+
+    it('uses response format only on final iteration after tool calls', async () => {
+      const requestWithResponseFormat = {
+        ...mockRequest,
+        responseFormat: {
+          type: 'json_schema' as const,
+          json_schema: {type: 'object'},
+        },
+      }
+
+      const toolCalls = [
+        {
+          id: 'call-123',
+          function: {
+            name: 'test-tool',
+            arguments: '{"param": "value"}',
+          },
+        },
+      ]
+
+      const toolResults = [
+        {
+          tool_call_id: 'call-123',
+          role: 'tool',
+          name: 'test-tool',
+          content: 'Tool result',
+        },
+      ]
+
+      // First response with tool calls
+      const firstResponse = {
+        choices: [
+          {
+            message: {
+              content: 'Using tool',
+              tool_calls: toolCalls,
+            },
+          },
+        ],
+      }
+
+      // Second response without tool calls, but should trigger final message loop
+      const secondResponse = {
+        choices: [
+          {
+            message: {
+              content: 'Intermediate result',
+              tool_calls: null,
+            },
+          },
+        ],
+      }
+
+      // Third response with response format
+      const thirdResponse = {
+        choices: [
+          {
+            message: {
+              content: '{"final": "result"}',
+              tool_calls: null,
+            },
+          },
+        ],
+      }
+
+      mockCreate
+        .mockResolvedValueOnce(firstResponse)
+        .mockResolvedValueOnce(secondResponse)
+        .mockResolvedValueOnce(thirdResponse)
+
+      mockExecuteToolCalls.mockResolvedValue(toolResults)
+
+      const result = await mcpInference(requestWithResponseFormat, mockMcpClient)
+
+      expect(result).toBe('{"final": "result"}')
+      expect(mockCreate).toHaveBeenCalledTimes(3)
+
+      // First call: tools but no response format
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const firstCall = mockCreate.mock.calls[0][0] as any
+      expect(firstCall.tools).toEqual(mockMcpClient.tools)
+      expect(firstCall.response_format).toBeUndefined()
+
+      // Second call: tools but no response format (after tool execution)
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const secondCall = mockCreate.mock.calls[1][0] as any
+      expect(secondCall.tools).toEqual(mockMcpClient.tools)
+      expect(secondCall.response_format).toBeUndefined()
+
+      // Third call: response format but no tools (final message)
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const thirdCall = mockCreate.mock.calls[2][0] as any
+      expect(thirdCall.tools).toBeUndefined()
+      expect(thirdCall.response_format).toEqual(requestWithResponseFormat.responseFormat)
+    })
+
+    it('returns immediately when response format is set and finalMessage is already true', async () => {
+      const requestWithResponseFormat = {
+        ...mockRequest,
+        responseFormat: {
+          type: 'json_schema' as const,
+          json_schema: {type: 'object'},
+        },
+      }
+
+      // Response without tool calls on what would be the final message iteration
+      const mockResponse = {
+        choices: [
+          {
+            message: {
+              content: '{"immediate": "result"}',
+              tool_calls: null,
+            },
+          },
+        ],
+      }
+
+      mockCreate.mockResolvedValue(mockResponse)
+
+      // We need to test a scenario where finalMessage would already be true
+      // This happens when we're already in the final iteration
+      const result = await mcpInference(requestWithResponseFormat, mockMcpClient)
+
+      // The function should make two calls: one normal, then one with response format
+      expect(mockCreate).toHaveBeenCalledTimes(2)
+      expect(result).toBe('{"immediate": "result"}')
+    })
   })
 })

__tests__/main-prompt-integration.test.ts

@@ -34,6 +34,12 @@ vi.mock('../src/mcp.js', () => ({
 
 vi.mock('@actions/core', () => core)
 
+// Mock process.exit to prevent it from actually exiting during tests
+const mockProcessExit = vi.spyOn(process, 'exit').mockImplementation(() => {
+  // Prevent actual exit, but don't throw - just return
+  return undefined as never
+})
+
 // The module being tested should be imported dynamically. This ensures that the
 // mocks are used in place of any actual dependencies.
 const {run} = await import('../src/main.js')
@@ -41,6 +47,7 @@ const {run} = await import('../src/main.js')
 describe('main.ts - prompt.yml integration', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    mockProcessExit.mockClear()
 
     // Mock environment variables
     process.env['GITHUB_TOKEN'] = 'test-token'
@@ -103,8 +110,12 @@ model: openai/gpt-4o
       }
     })
 
+    // Expect the run function to complete successfully
     await run()
 
+    // Verify process.exit was called with code 0 (success)
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
+
     // Verify simpleInference was called with the correct message structure
     expect(mockSimpleInference).toHaveBeenCalledWith(
       expect.objectContaining({
@@ -171,6 +182,9 @@ model: openai/gpt-4o
         messages: [{role: 'user', content: 'Here is the data: FILE_CONTENTS'}],
       }),
     )
+
+    // Verify process.exit was called with code 0 (success)
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('should fall back to legacy format when not using prompt YAML', async () => {
@@ -215,5 +229,8 @@ model: openai/gpt-4o
         token: 'test-token',
       }),
     )
+
+    // Verify process.exit was called with code 0 (success)
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 })

__tests__/main.test.ts

@@ -66,7 +66,7 @@ function mockInputs(inputs: Record<string, string> = {}): void {
  */
 function verifyStandardResponse(): void {
   expect(core.setOutput).toHaveBeenNthCalledWith(1, 'response', 'Hello, user!')
-  expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse.txt'))
+  expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse-'))
 }
 
 vi.mock('fs', () => ({
@@ -75,6 +75,19 @@ vi.mock('fs', () => ({
   writeFileSync: mockWriteFileSync,
 }))
 
+// Mocks for tmp module to control temporary file creation and cleanup
+const mockRemoveCallback = vi.fn()
+const mockFileSync = vi.fn().mockReturnValue({
+  name: '/secure/temp/dir/modelResponse-abc123.txt',
+  removeCallback: mockRemoveCallback,
+})
+const mockSetGracefulCleanup = vi.fn()
+
+vi.mock('tmp', () => ({
+  fileSync: mockFileSync,
+  setGracefulCleanup: mockSetGracefulCleanup,
+}))
+
 // Mock MCP and inference modules
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const mockConnectToGitHubMCP = vi.fn() as MockedFunction<any>
@@ -96,7 +109,8 @@ vi.mock('@actions/core', () => core)
 
 // Mock process.exit to prevent it from actually exiting during tests
 const mockProcessExit = vi.spyOn(process, 'exit').mockImplementation(() => {
-  throw new Error('process.exit called')
+  // Prevent actual exit, but don't throw - just return
+  return undefined as never
 })
 
 // The module being tested should be imported dynamically. This ensures that the
@@ -127,6 +141,7 @@ describe('main.ts', () => {
 
     expect(core.setOutput).toHaveBeenCalled()
     verifyStandardResponse()
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('Sets a failed status when no prompt is set', async () => {
@@ -135,8 +150,7 @@ describe('main.ts', () => {
       'prompt-file': '',
     })
 
-    // Expect the run function to throw due to process.exit being mocked
-    await expect(run()).rejects.toThrow('process.exit called')
+    await run()
 
     expect(core.setFailed).toHaveBeenCalledWith('Neither prompt-file nor prompt was set')
     expect(mockProcessExit).toHaveBeenCalledWith(1)
@@ -165,6 +179,7 @@ describe('main.ts', () => {
     expect(mockConnectToGitHubMCP).not.toHaveBeenCalled()
     expect(mockMcpInference).not.toHaveBeenCalled()
     verifyStandardResponse()
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('uses MCP inference when enabled and connection succeeds', async () => {
@@ -197,6 +212,7 @@ describe('main.ts', () => {
     )
     expect(mockSimpleInference).not.toHaveBeenCalled()
     verifyStandardResponse()
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('falls back to simple inference when MCP connection fails', async () => {
@@ -215,6 +231,7 @@ describe('main.ts', () => {
     expect(mockMcpInference).not.toHaveBeenCalled()
     expect(core.warning).toHaveBeenCalledWith('MCP connection failed, falling back to simple inference')
     verifyStandardResponse()
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('properly integrates with loadContentFromFileOrInput', async () => {
@@ -248,6 +265,7 @@ describe('main.ts', () => {
       responseFormat: undefined,
     })
     verifyStandardResponse()
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
   })
 
   it('handles non-existent prompt-file with an error', async () => {
@@ -259,10 +277,48 @@ describe('main.ts', () => {
       'prompt-file': promptFile,
     })
 
-    // Expect the run function to throw due to process.exit being mocked
-    await expect(run()).rejects.toThrow('process.exit called')
+    await run()
 
     expect(core.setFailed).toHaveBeenCalledWith(`File for prompt-file was not found: ${promptFile}`)
     expect(mockProcessExit).toHaveBeenCalledWith(1)
   })
+
+  it('creates secure temporary files with proper cleanup', async () => {
+    mockInputs({
+      prompt: 'Test prompt',
+      'system-prompt': 'You are a test assistant.',
+    })
+
+    await run()
+
+    expect(mockSetGracefulCleanup).toHaveBeenCalledOnce()
+
+    expect(mockFileSync).toHaveBeenCalledWith({
+      prefix: 'modelResponse-',
+      postfix: '.txt',
+    })
+
+    expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', '/secure/temp/dir/modelResponse-abc123.txt')
+    expect(mockWriteFileSync).toHaveBeenCalledWith('/secure/temp/dir/modelResponse-abc123.txt', 'Hello, user!', 'utf-8')
+    expect(mockRemoveCallback).toHaveBeenCalledOnce()
+
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
+  })
+
+  it('handles cleanup errors gracefully', async () => {
+    mockRemoveCallback.mockImplementationOnce(() => {
+      throw new Error('Cleanup failed')
+    })
+
+    mockInputs({
+      prompt: 'Test prompt',
+      'system-prompt': 'You are a test assistant.',
+    })
+
+    await run()
+
+    expect(mockRemoveCallback).toHaveBeenCalledOnce()
+    expect(core.warning).toHaveBeenCalledWith('Failed to cleanup temporary file: Error: Cleanup failed')
+    expect(mockProcessExit).toHaveBeenCalledWith(0)
+  })
 })

action.yml

@@ -67,5 +67,5 @@ outputs:
     description: The file path where the response is saved
 
 runs:
-  using: node20
+  using: node24
   main: dist/index.js

package-lock.json

@@ -11,9 +11,11 @@
       "dependencies": {
         "@actions/core": "^1.11.1",
         "@modelcontextprotocol/sdk": "^1.15.1",
+        "@types/tmp": "^0.2.6",
         "js-yaml": "^4.1.0",
         "openai": "^5.11.0",
-        "pkce-challenge": "^5.0.0"
+        "pkce-challenge": "^5.0.0",
+        "tmp": "^0.2.4"
       },
       "devDependencies": {
         "@eslint/compat": "^1.3.0",
@@ -24,7 +26,7 @@
         "@rollup/plugin-node-resolve": "^16.0.1",
         "@rollup/plugin-typescript": "^12.1.2",
         "@types/js-yaml": "^4.0.9",
-        "@types/node": "^22.15.31",
+        "@types/node": "^24.1.0",
         "@typescript-eslint/eslint-plugin": "^8.34.0",
         "@typescript-eslint/parser": "^8.32.1",
         "eslint": "^9.29.0",
@@ -39,7 +41,7 @@
         "vitest": "^3"
       },
       "engines": {
-        "node": ">=20"
+        "node": ">=24"
       },
       "optionalDependencies": {
         "@rollup/rollup-linux-x64-gnu": "*"
@@ -2476,13 +2478,13 @@
       "license": "MIT"
     },
     "node_modules/@types/node": {
-      "version": "22.15.31",
-      "resolved": "https://registry.npmjs.org/@types/node/-/node-22.15.31.tgz",
-      "integrity": "sha512-jnVe5ULKl6tijxUhvQeNbQG/84fHfg+yMak02cT8QVhBx/F05rAVxCGBYYTh2EKz22D6JF5ktXuNwdx7b9iEGw==",
+      "version": "24.1.0",
+      "resolved": "https://registry.npmjs.org/@types/node/-/node-24.1.0.tgz",
+      "integrity": "sha512-ut5FthK5moxFKH2T1CUOC6ctR67rQRvvHdFLCD2Ql6KXmMuCrjsSsRI9UsLCm9M18BMwClv4pn327UvB7eeO1w==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
-        "undici-types": "~6.21.0"
+        "undici-types": "~7.8.0"
       }
     },
     "node_modules/@types/resolve": {
@@ -2492,6 +2494,12 @@
       "dev": true,
       "license": "MIT"
     },
+    "node_modules/@types/tmp": {
+      "version": "0.2.6",
+      "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.6.tgz",
+      "integrity": "sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA==",
+      "license": "MIT"
+    },
     "node_modules/@typescript-eslint/eslint-plugin": {
       "version": "8.34.0",
       "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.34.0.tgz",
@@ -8943,6 +8951,15 @@
         "node": ">=14.0.0"
       }
     },
+    "node_modules/tmp": {
+      "version": "0.2.5",
+      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.5.tgz",
+      "integrity": "sha512-voyz6MApa1rQGUxT3E+BK7/ROe8itEx7vD8/HEvt4xwXucvQ5G5oeEiHkmHZJuBO21RpOf+YYm9MOivj709jow==",
+      "license": "MIT",
+      "engines": {
+        "node": ">=14.14"
+      }
+    },
     "node_modules/to-regex-range": {
       "version": "5.0.1",
       "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz",
@@ -9207,9 +9224,9 @@
       }
     },
     "node_modules/undici-types": {
-      "version": "6.21.0",
-      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz",
-      "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==",
+      "version": "7.8.0",
+      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.8.0.tgz",
+      "integrity": "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw==",
       "dev": true,
       "license": "MIT"
     },

package.json

@@ -6,7 +6,7 @@
     ".": "./dist/index.js"
   },
   "engines": {
-    "node": ">=20"
+    "node": ">=24"
   },
   "scripts": {
     "bundle": "npm run format:write && npm run package",
@@ -25,9 +25,11 @@
   "dependencies": {
     "@actions/core": "^1.11.1",
     "@modelcontextprotocol/sdk": "^1.15.1",
+    "@types/tmp": "^0.2.6",
     "js-yaml": "^4.1.0",
     "openai": "^5.11.0",
-    "pkce-challenge": "^5.0.0"
+    "pkce-challenge": "^5.0.0",
+    "tmp": "^0.2.4"
   },
   "devDependencies": {
     "@eslint/compat": "^1.3.0",
@@ -38,7 +40,7 @@
     "@rollup/plugin-node-resolve": "^16.0.1",
     "@rollup/plugin-typescript": "^12.1.2",
     "@types/js-yaml": "^4.0.9",
-    "@types/node": "^22.15.31",
+    "@types/node": "^24.1.0",
     "@typescript-eslint/eslint-plugin": "^8.34.0",
     "@typescript-eslint/parser": "^8.32.1",
     "eslint": "^9.29.0",

src/inference.ts

@@ -53,21 +53,10 @@ export async function simpleInference(request: InferenceRequest): Promise<string
     chatCompletionRequest.response_format = request.responseFormat as any
   }
 
-  try {
-    const response = await client.chat.completions.create(chatCompletionRequest)
-
-    if ('choices' in response) {
-      const modelResponse = response.choices[0]?.message?.content
-      core.info(`Model response: ${modelResponse || 'No response content'}`)
-      return modelResponse || null
-    } else {
-      core.error(`Unexpected response format from API: ${JSON.stringify(response)}`)
-      return null
-    }
-  } catch (error) {
-    core.error(`API error: ${error}`)
-    throw error
-  }
+  const response = await chatCompletion(client, chatCompletionRequest, 'simpleInference')
+  const modelResponse = response.choices[0]?.message?.content
+  core.info(`Model response: ${modelResponse || 'No response content'}`)
+  return modelResponse || null
 }
 
 /**
@@ -89,6 +78,9 @@ export async function mcpInference(
 
   let iterationCount = 0
   const maxIterations = 5 // Prevent infinite loops
+  // We want to use response_format (e.g. JSON) on the last iteration only, so the model can output
+  // the final result in the expected format without interfering with tool calls
+  let finalMessage = false
 
   while (iterationCount < maxIterations) {
     iterationCount++
@@ -98,21 +90,18 @@ export async function mcpInference(
       messages: messages as OpenAI.Chat.Completions.ChatCompletionMessageParam[],
       max_tokens: request.maxTokens,
       model: request.modelName,
-      tools: githubMcpClient.tools as OpenAI.Chat.Completions.ChatCompletionTool[],
     }
 
-    // Add response format if specified (only on first iteration to avoid conflicts)
-    if (iterationCount === 1 && request.responseFormat) {
+    // Add response format if specified (only on final iteration to avoid conflicts with tool calls)
+    if (finalMessage && request.responseFormat) {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       chatCompletionRequest.response_format = request.responseFormat as any
+    } else {
+      chatCompletionRequest.tools = githubMcpClient.tools as OpenAI.Chat.Completions.ChatCompletionTool[]
     }
 
     try {
-      const response = await client.chat.completions.create(chatCompletionRequest)
-
-      if (!('choices' in response)) {
-        throw new Error(`Unexpected response format from API: ${JSON.stringify(response)}`)
-      }
+      const response = await chatCompletion(client, chatCompletionRequest, `mcpInference iteration ${iterationCount}`)
 
       const assistantMessage = response.choices[0]?.message
       const modelResponse = assistantMessage?.content
@@ -128,17 +117,23 @@ export async function mcpInference(
 
       if (!toolCalls || toolCalls.length === 0) {
         core.info('No tool calls requested, ending GitHub MCP inference loop')
-        return modelResponse || null
+
+        if (request.responseFormat && !finalMessage) {
+          core.info('Making one more MCP loop with the requested response format...')
+          messages.push({
+            role: 'user',
+            content: `Please provide your response in the exact ${request.responseFormat.type} format specified.`,
+          })
+          finalMessage = true
+          continue
+        } else {
+          return modelResponse || null
+        }
       }
 
       core.info(`Model requested ${toolCalls.length} tool calls`)
-
-      // Execute all tool calls via GitHub MCP
       const toolResults = await executeToolCalls(githubMcpClient.client, toolCalls as ToolCall[])
-
-      // Add tool results to the conversation
       messages.push(...toolResults)
-
       core.info('Tool results added, continuing conversation...')
     } catch (error) {
       core.error(`OpenAI API error: ${error}`)
@@ -156,3 +151,43 @@ export async function mcpInference(
 
   return lastAssistantMessage?.content || null
 }
+
+/**
+ * Wrapper around OpenAI chat.completions.create with defensive handling for cases where
+ * the SDK returns a raw string (e.g., unexpected content-type or streaming body) instead of
+ * a parsed object. Ensures an object with a 'choices' array is returned or throws a descriptive error.
+ */
+async function chatCompletion(
+  client: OpenAI,
+  params: OpenAI.Chat.Completions.ChatCompletionCreateParams,
+  context: string,
+): Promise<OpenAI.Chat.Completions.ChatCompletion> {
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    let response: any = await client.chat.completions.create(params)
+    core.debug(`${context}: raw response typeof=${typeof response}`)
+
+    if (typeof response === 'string') {
+      // Attempt to parse if we unexpectedly received a string
+      try {
+        response = JSON.parse(response)
+      } catch (e) {
+        const preview = response.slice(0, 400)
+        throw new Error(
+          `${context}: Chat completion response was a string and not valid JSON (${(e as Error).message}). Preview: ${preview}`,
+        )
+      }
+    }
+
+    if (!response || typeof response !== 'object' || !('choices' in response)) {
+      const preview = JSON.stringify(response)?.slice(0, 800)
+      throw new Error(`${context}: Unexpected response shape (no choices). Preview: ${preview}`)
+    }
+
+    return response as OpenAI.Chat.Completions.ChatCompletion
+  } catch (err) {
+    // Re-throw after logging for upstream handling
+    core.error(`${context}: chatCompletion failed: ${err}`)
+    throw err
+  }
+}

src/main.ts

@@ -1,7 +1,6 @@
 import * as core from '@actions/core'
 import * as fs from 'fs'
-import * as os from 'os'
-import * as path from 'path'
+import * as tmp from 'tmp'
 import {connectToGitHubMCP} from './mcp.js'
 import {simpleInference, mcpInference} from './inference.js'
 import {loadContentFromFileOrInput, buildInferenceRequest} from './helpers.js'
@@ -13,14 +12,17 @@ import {
   parseFileTemplateVariables,
 } from './prompt.js'
 
-const RESPONSE_FILE = 'modelResponse.txt'
-
 /**
  * The main function for the action.
  *
  * @returns Resolves when the action is complete.
  */
 export async function run(): Promise<void> {
+  let responseFile: tmp.FileResult | null = null
+
+  // Set up graceful cleanup for temporary files on process exit
+  tmp.setGracefulCleanup()
+
   try {
     const promptFilePath = core.getInput('prompt-file')
     const inputVariables = core.getInput('input')
@@ -93,11 +95,16 @@ export async function run(): Promise<void> {
 
     core.setOutput('response', modelResponse || '')
 
-    const responseFilePath = path.join(tempDir(), RESPONSE_FILE)
-    core.setOutput('response-file', responseFilePath)
+    // Create a secure temporary file instead of using the temp directory directly
+    responseFile = tmp.fileSync({
+      prefix: 'modelResponse-',
+      postfix: '.txt',
+    })
+
+    core.setOutput('response-file', responseFile.name)
 
     if (modelResponse && modelResponse !== '') {
-      fs.writeFileSync(responseFilePath, modelResponse, 'utf-8')
+      fs.writeFileSync(responseFile.name, modelResponse, 'utf-8')
     }
   } catch (error) {
     if (error instanceof Error) {
@@ -105,13 +112,20 @@ export async function run(): Promise<void> {
     } else {
       core.setFailed(`An unexpected error occurred: ${JSON.stringify(error, null, 2)}`)
     }
-
     // Force exit to prevent hanging on open connections
     process.exit(1)
+  } finally {
+    // Explicit cleanup of temporary file if it was created
+    if (responseFile) {
+      try {
+        responseFile.removeCallback()
+      } catch (cleanupError) {
+        // Log cleanup errors but don't fail the action
+        core.warning(`Failed to cleanup temporary file: ${cleanupError}`)
+      }
+    }
   }
-}
 
-function tempDir(): string {
-  const tempDirectory = process.env['RUNNER_TEMP'] || os.tmpdir()
-  return tempDirectory
+  // Force exit to prevent hanging on open connections
+  process.exit(0)
 }