fix(tools): update supabase buckets atomically

Author: icecrasher321

apps/sim/lib/internal/supabase/operations/storage-update-bucket.test.ts

@@ -22,36 +22,39 @@ describe('executeStorageUpdateBucketOperation', () => {
     vi.unstubAllGlobals()
   })
 
-  it('passes cancellation to both bucket requests', async () => {
+  it('sends only explicitly changed fields in one non-redirecting update request', async () => {
     const controller = new AbortController()
-    fetchMock
-      .mockResolvedValueOnce(Response.json({ public: false, file_size_limit: 100 }))
-      .mockResolvedValueOnce(Response.json({ id: 'documents' }))
+    fetchMock.mockResolvedValueOnce(Response.json({ message: 'Successfully updated' }))
 
     await executeStorageUpdateBucketOperation({ ...INPUT, isPublic: true }, controller.signal)
 
-    expect(fetchMock).toHaveBeenCalledTimes(2)
-    expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ signal: controller.signal })
-    expect(fetchMock.mock.calls[1]?.[1]).toMatchObject({ signal: controller.signal })
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+    expect(fetchMock).toHaveBeenCalledWith(
+      'https://projectref.supabase.co/storage/v1/bucket/documents',
+      expect.objectContaining({
+        method: 'PUT',
+        body: JSON.stringify({ public: true }),
+        redirect: 'error',
+        signal: controller.signal,
+      })
+    )
   })
 
-  it('treats whitespace-only file limits as omitted', async () => {
-    fetchMock
-      .mockResolvedValueOnce(Response.json({ public: false, file_size_limit: 4096 }))
-      .mockResolvedValueOnce(Response.json({ id: 'documents' }))
+  it('treats whitespace-only file limits as omitted without reading the bucket', async () => {
+    fetchMock.mockResolvedValueOnce(Response.json({ message: 'Successfully updated' }))
 
     await executeStorageUpdateBucketOperation({
       ...INPUT,
+      isPublic: false,
       fileSizeLimit: '   ' as never,
     })
 
-    const payload = JSON.parse(String(fetchMock.mock.calls[1]?.[1]?.body))
-    expect(payload.file_size_limit).toBe(4096)
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+    const payload = JSON.parse(String(fetchMock.mock.calls[0]?.[1]?.body))
+    expect(payload).toEqual({ public: false })
   })
 
   it('rejects a nonnumeric file limit before updating the bucket', async () => {
-    fetchMock.mockResolvedValueOnce(Response.json({ public: false, file_size_limit: 4096 }))
-
     const result = await executeStorageUpdateBucketOperation({
       ...INPUT,
       fileSizeLimit: 'not-a-number' as never,
@@ -61,12 +64,10 @@ describe('executeStorageUpdateBucketOperation', () => {
       success: false,
       error: 'File size limit must be a finite number',
     })
-    expect(fetchMock).toHaveBeenCalledTimes(1)
+    expect(fetchMock).not.toHaveBeenCalled()
   })
 
   it.each([true, [], {}, '0x100'])('rejects a non-decimal file limit %j', async (fileSizeLimit) => {
-    fetchMock.mockResolvedValueOnce(Response.json({ public: false, file_size_limit: 4096 }))
-
     const result = await executeStorageUpdateBucketOperation({
       ...INPUT,
       fileSizeLimit: fileSizeLimit as never,
@@ -76,7 +77,29 @@ describe('executeStorageUpdateBucketOperation', () => {
       success: false,
       error: 'File size limit must be a finite number',
     })
-    expect(fetchMock).toHaveBeenCalledTimes(1)
+    expect(fetchMock).not.toHaveBeenCalled()
+  })
+
+  it('preserves a no-op update while still verifying access to the bucket', async () => {
+    fetchMock.mockResolvedValueOnce(Response.json({ id: 'documents' }))
+
+    const result = await executeStorageUpdateBucketOperation({
+      ...INPUT,
+      fileSizeLimit: '  ' as never,
+    })
+
+    expect(result).toEqual({
+      success: true,
+      output: {
+        message: 'Successfully updated storage bucket',
+        results: { message: 'Successfully updated' },
+      },
+      error: undefined,
+    })
+    expect(fetchMock).toHaveBeenCalledWith(
+      'https://projectref.supabase.co/storage/v1/bucket/documents',
+      expect.objectContaining({ method: 'GET', redirect: 'error' })
+    )
   })
 
   it('propagates cancellation instead of returning a failed tool envelope', async () => {
@@ -87,7 +110,7 @@ describe('executeStorageUpdateBucketOperation', () => {
     })
 
     await expect(
-      executeStorageUpdateBucketOperation(INPUT, controller.signal)
+      executeStorageUpdateBucketOperation({ ...INPUT, isPublic: true }, controller.signal)
     ).rejects.toMatchObject({ name: 'AbortError' })
   })
 

apps/sim/lib/internal/supabase/operations/storage-update-bucket.ts

@@ -1,4 +1,5 @@
 import { getErrorMessage } from '@sim/utils/errors'
+import { filterUndefined } from '@sim/utils/object'
 import type { InternalToolOperationImplementation } from '@/lib/internal/tool-operations/types'
 import type {
   SupabaseStorageUpdateBucketParams,
@@ -20,23 +21,6 @@ export const executeStorageUpdateBucketOperation: InternalToolOperationImplement
       Authorization: `Bearer ${params.apiKey}`,
       'Content-Type': 'application/json',
     }
-    const currentResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
-      method: 'GET',
-      headers,
-      signal,
-    })
-
-    if (!currentResponse.ok) {
-      const errorText = await currentResponse.text()
-      throw new Error(`Failed to read current bucket configuration: ${errorText}`)
-    }
-
-    const current = await currentResponse.json()
-
-    // Block subBlocks for a shared field can forward an empty string
-    // (e.g. an untouched short-input) rather than omitting the key
-    // entirely — treat that the same as "not provided" so it falls
-    // back to the bucket's current value instead of coercing to 0/false.
     const hasValue = (value: unknown): boolean =>
       value !== undefined && value !== null && (typeof value !== 'string' || value.trim() !== '')
     const rawFileSizeLimit: unknown = params.fileSizeLimit
@@ -52,20 +36,40 @@ export const executeStorageUpdateBucketOperation: InternalToolOperationImplement
       throw new Error('File size limit must be a finite number')
     }
 
-    const payload: Record<string, unknown> = {
-      id: params.bucket,
-      name: params.bucket,
-      public: hasValue(params.isPublic) ? params.isPublic : Boolean(current.public),
-      file_size_limit: fileSizeLimit ?? current.file_size_limit ?? null,
-      allowed_mime_types: hasValue(params.allowedMimeTypes)
-        ? params.allowedMimeTypes
-        : (current.allowed_mime_types ?? null),
+    const payload = filterUndefined({
+      public: hasValue(params.isPublic) ? params.isPublic : undefined,
+      file_size_limit: fileSizeLimit,
+      allowed_mime_types: hasValue(params.allowedMimeTypes) ? params.allowedMimeTypes : undefined,
+    })
+
+    if (Object.keys(payload).length === 0) {
+      const currentResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
+        method: 'GET',
+        headers,
+        redirect: 'error',
+        signal,
+      })
+      if (!currentResponse.ok) {
+        const errorText = await currentResponse.text()
+        throw new Error(`Failed to read current bucket configuration: ${errorText}`)
+      }
+      await currentResponse.body?.cancel()
+      signal?.throwIfAborted()
+      return {
+        success: true,
+        output: {
+          message: 'Successfully updated storage bucket',
+          results: { message: 'Successfully updated' },
+        },
+        error: undefined,
+      }
     }
 
     const updateResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
       method: 'PUT',
       headers,
       body: JSON.stringify(payload),
+      redirect: 'error',
       signal,
     })