refactor: Claude refactoring

.changeset/brown-months-accept.md

@@ -0,0 +1,5 @@
+---
+"github-actions-cloudflare-pages": patch
+---
+
+fix: build failure now terminate polling

.changeset/grumpy-files-dance.md

@@ -0,0 +1,5 @@
+---
+"github-actions-cloudflare-pages": patch
+---
+
+refactor: setTimeout NODE_ENV check for tests

.changeset/wicked-meals-wave.md

@@ -0,0 +1,5 @@
+---
+"github-actions-cloudflare-pages": patch
+---
+
+refactor: switch from exec to execFile. Stops risk of spaces or special characters breaking the command.

.claude/settings.json

@@ -0,0 +1,34 @@
+{
+  "hooks": {
+    "Stop": [
+      {
+        "hooks": [
+          {
+            "type": "command",
+            "command": "bash ./.github/hooks/scripts/stop-type-check.sh",
+            "timeout": 60
+          }
+        ]
+      },
+      {
+        "hooks": [
+          {
+            "type": "command",
+            "command": "bash ./.github/hooks/scripts/stop-review-agents.sh",
+            "timeout": 10
+          }
+        ]
+      }
+    ],
+    "PostToolUse": [
+      {
+        "hooks": [
+          {
+            "type": "command",
+            "command": "if [[ -n \"$TOOL_INPUT_FILE_PATH\" && -f \"$TOOL_INPUT_FILE_PATH\" ]]; then bash ./.github/hooks/scripts/pre-commit-oxc.sh \"$TOOL_INPUT_FILE_PATH\"; fi"
+          }
+        ]
+      }
+    ]
+  }
+}

.vscode/settings.json

@@ -23,5 +23,6 @@
   },
   "editor.suggest.showKeywords": false,
   "oxc.enable.oxfmt": true,
-  "oxc.enable.oxlint": true
+  "oxc.enable.oxlint": true,
+  "terminal.integrated.defaultProfile.linux": "zsh"
 }

__tests__/common/cloudflare/deployment/create.test.ts

@@ -8,7 +8,7 @@ import {
   CLOUDFLARE_API_TOKEN,
   createCloudflareDeployment
 } from '@/common/cloudflare/deployment/create.js'
-import {execAsync} from '@/common/utils.js'
+import {execFileAsync} from '@/common/utils.js'
 import {INPUT_KEY_WORKING_DIRECTORY} from '@/input-keys'
 import RESPONSE_NOT_FOUND_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments-not-found.response.json' with {type: 'json'}
 import RESPONSE_DEPLOYMENTS_IDLE from '@/responses/api.cloudflare.com/pages/deployments/deployments.idle.response.json' with {type: 'json'}
@@ -35,15 +35,18 @@ describe(createCloudflareDeployment, () => {
     afterEach(async () => {
       mockApi.mockAgent.assertNoPendingInterceptors()
       await mockApi.mockAgent.close()
-      vi.mocked(execAsync).mockReset()
+      vi.mocked(execFileAsync).mockReset()
       vi.runOnlyPendingTimers()
       vi.useRealTimers()
     })
 
     test('handles thrown error from wrangler deploy', async () => {
       expect.assertions(10)
 
-      vi.mocked(execAsync).mockRejectedValueOnce({stderr: 'Oh no!', stdout: ''})
+      vi.mocked(execFileAsync).mockRejectedValueOnce({
+        stderr: 'Oh no!',
+        stdout: ''
+      })
 
       // Expect Cloudflare Api Token and Account Id to be undefined.
       expect(process.env[CLOUDFLARE_API_TOKEN]).toBeUndefined()
@@ -57,8 +60,21 @@ describe(createCloudflareDeployment, () => {
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(`[Error: Oh no!]`)
 
-      expect(execAsync).toHaveBeenCalledWith(
-        `npx wrangler@${packageJson.devDependencies.wrangler} pages deploy mock-directory --project-name=mock-cloudflare-project-name --branch=mock-github-head-ref --commit-dirty=true --commit-hash=mock-github-sha`,
+      expect(execFileAsync).toHaveBeenCalledWith(
+        'npx',
+        [
+          `wrangler@${packageJson.devDependencies.wrangler}`,
+          'pages',
+          'deploy',
+          'mock-directory',
+          '--project-name',
+          'mock-cloudflare-project-name',
+          '--branch',
+          'mock-github-head-ref',
+          '--commit-dirty=true',
+          '--commit-hash',
+          'mock-github-sha'
+        ],
         {
           // oxlint-disable-next-line typescript/no-unsafe-assignment
           env: expect.objectContaining({
@@ -69,7 +85,7 @@ describe(createCloudflareDeployment, () => {
         }
       )
 
-      expect(execAsync).toHaveBeenCalledTimes(1)
+      expect(execFileAsync).toHaveBeenCalledTimes(1)
       expect(info).not.toHaveBeenCalled()
       expect(process.env[CLOUDFLARE_API_TOKEN]).toBe(
         'mock-cloudflare-api-token'
@@ -85,7 +101,7 @@ describe(createCloudflareDeployment, () => {
     test('handles thrown error from getDeployments', async () => {
       expect.assertions(5)
 
-      vi.mocked(execAsync).mockResolvedValueOnce({
+      vi.mocked(execFileAsync).mockResolvedValueOnce({
         stdout: 'success',
         stderr: ''
       })
@@ -105,7 +121,7 @@ describe(createCloudflareDeployment, () => {
       ).rejects.toThrowErrorMatchingInlineSnapshot(
         `[ParseError: A request to the Cloudflare API (https://api.cloudflare.com/client/v4/accounts/mock-cloudflare-account-id/pages/projects/mock-cloudflare-project-name/deployments) failed.]`
       )
-      expect(execAsync).toHaveBeenCalledTimes(1)
+      expect(execFileAsync).toHaveBeenCalledTimes(1)
       expect(info).toHaveBeenLastCalledWith('success')
       expect(setOutput).not.toHaveBeenCalled()
       expect(summary.addTable).not.toHaveBeenCalled()
@@ -116,7 +132,7 @@ describe(createCloudflareDeployment, () => {
 
       stubInputEnv(INPUT_KEY_WORKING_DIRECTORY)
 
-      vi.mocked(execAsync).mockResolvedValueOnce({
+      vi.mocked(execFileAsync).mockResolvedValueOnce({
         stdout: 'success',
         stderr: ''
       })
@@ -143,8 +159,21 @@ describe(createCloudflareDeployment, () => {
       })
       // vi.advanceTimersByTime(2000)
 
-      expect(execAsync).toHaveBeenCalledWith(
-        `npx wrangler@${packageJson.devDependencies.wrangler} pages deploy mock-directory --project-name=mock-cloudflare-project-name --branch=mock-github-head-ref --commit-dirty=true --commit-hash=mock-github-sha`,
+      expect(execFileAsync).toHaveBeenCalledWith(
+        'npx',
+        [
+          `wrangler@${packageJson.devDependencies.wrangler}`,
+          'pages',
+          'deploy',
+          'mock-directory',
+          '--project-name',
+          'mock-cloudflare-project-name',
+          '--branch',
+          'mock-github-head-ref',
+          '--commit-dirty=true',
+          '--commit-hash',
+          'mock-github-sha'
+        ],
         {
           // oxlint-disable-next-line typescript/no-unsafe-assignment
           env: expect.objectContaining({

__tests__/common/cloudflare/deployment/status.test.ts

@@ -0,0 +1,138 @@
+import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
+
+import type {PagesDeployment} from '@/common/cloudflare/types.js'
+import type {MockApi} from '@/tests/helpers/api.js'
+
+import {statusCloudflareDeployment} from '@/common/cloudflare/deployment/status.js'
+import {sleep} from '@/common/utils.js'
+import RESPONSE_DEPLOYMENTS_IDLE from '@/responses/api.cloudflare.com/pages/deployments/deployments.idle.response.json' with {type: 'json'}
+import RESPONSE_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments.response.json' with {type: 'json'}
+import {
+  MOCK_ACCOUNT_ID,
+  MOCK_API_PATH_DEPLOYMENTS,
+  MOCK_PROJECT_NAME,
+  setMockApi
+} from '@/tests/helpers/api.js'
+
+vi.mock(import('@/common/utils.js'))
+vi.mock(import('@actions/core'))
+
+const API_ENDPOINT = {
+  accountId: MOCK_ACCOUNT_ID,
+  projectName: MOCK_PROJECT_NAME
+}
+
+type LatestStage = PagesDeployment['latest_stage']
+
+const withLatestStage = (
+  name: LatestStage['name'],
+  status: LatestStage['status']
+): typeof RESPONSE_DEPLOYMENTS => ({
+  ...RESPONSE_DEPLOYMENTS,
+  result: RESPONSE_DEPLOYMENTS.result.map((deployment, index) =>
+    index === 0
+      ? {
+          ...deployment,
+          latest_stage: {
+            name,
+            status,
+            started_on: null,
+            ended_on: null
+          } as LatestStage
+        }
+      : deployment
+  ) as (typeof RESPONSE_DEPLOYMENTS)['result']
+})
+
+describe(statusCloudflareDeployment, () => {
+  let mockApi: MockApi
+
+  beforeEach(() => {
+    mockApi = setMockApi()
+  })
+
+  afterEach(async () => {
+    mockApi.mockAgent.assertNoPendingInterceptors()
+    await mockApi.mockAgent.close()
+  })
+
+  test('returns success when deploy stage succeeds', async () => {
+    expect.assertions(3)
+
+    mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
+
+    const {deployment, status} = await statusCloudflareDeployment(API_ENDPOINT)
+
+    expect(status).toBe('success')
+    expect(deployment.id).toMatchInlineSnapshot(
+      `"206e215c-33b3-4ce4-adf4-7fc6c9b65483"`
+    )
+    expect(vi.mocked(sleep)).not.toHaveBeenCalled()
+  })
+
+  test('polls until deploy stage succeeds', async () => {
+    expect.assertions(2)
+
+    mockApi
+      .interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS_IDLE)
+      .times(2)
+    mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
+
+    const {status} = await statusCloudflareDeployment(API_ENDPOINT)
+
+    expect(status).toBe('success')
+    expect(vi.mocked(sleep)).toHaveBeenCalledTimes(2)
+  })
+
+  test.for([
+    {stage: 'build', status: 'failure'},
+    {stage: 'build', status: 'canceled'},
+    {stage: 'deploy', status: 'active'}
+  ] satisfies {stage: LatestStage['name']; status: LatestStage['status']}[])(
+    'returns $status immediately without polling ($stage stage)',
+    async ({stage, status}) => {
+      expect.assertions(2)
+
+      mockApi.interceptCloudflare(
+        MOCK_API_PATH_DEPLOYMENTS,
+        withLatestStage(stage, status)
+      )
+
+      const result = await statusCloudflareDeployment(API_ENDPOINT)
+
+      expect(result.status).toBe(status)
+      expect(vi.mocked(sleep)).not.toHaveBeenCalled()
+    }
+  )
+
+  test('polls while a non-deploy stage is active', async () => {
+    expect.assertions(2)
+
+    mockApi.interceptCloudflare(
+      MOCK_API_PATH_DEPLOYMENTS,
+      withLatestStage('build', 'active')
+    )
+    mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
+
+    const {status} = await statusCloudflareDeployment(API_ENDPOINT)
+
+    expect(status).toBe('success')
+    expect(vi.mocked(sleep)).toHaveBeenCalledTimes(1)
+  })
+
+  test('throws when the api returns an error', async () => {
+    expect.assertions(1)
+
+    mockApi.interceptCloudflare(
+      MOCK_API_PATH_DEPLOYMENTS,
+      {result: null, success: false, errors: [], messages: []},
+      404
+    )
+
+    await expect(
+      statusCloudflareDeployment(API_ENDPOINT)
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `[ParseError: A request to the Cloudflare API (https://api.cloudflare.com/client/v4/accounts/mock-cloudflare-account-id/pages/projects/mock-cloudflare-project-name/deployments) failed.]`
+    )
+  })
+})

__tests__/deploy/main.test.ts

@@ -3,7 +3,7 @@ import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
 
 import type {MockApi} from '@/tests/helpers/api.js'
 
-import {execAsync} from '@/common/utils.js'
+import {execFileAsync} from '@/common/utils.js'
 import {run} from '@/deploy/main.js'
 import RESPONSE_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments.response.json' with {type: 'json'}
 import {MOCK_API_PATH_DEPLOYMENTS, setMockApi} from '@/tests/helpers/api.js'
@@ -31,7 +31,7 @@ describe('deploy', () => {
     describe(run, () => {
       describe('handles resolve', () => {
         beforeEach(() => {
-          vi.mocked(execAsync).mockResolvedValue({
+          vi.mocked(execFileAsync).mockResolvedValue({
             stdout: 'success',
             stderr: ''
           })