refactor(copilot): tighten manage_sandbox after review

Fixes found auditing the first pass:

- updateWorkspaceSandbox treated a whitespace-only name as supplied, so it
  trimmed to empty, skipped the falsy conflict pre-check, and wrote an
  unnamed sandbox the UI cannot produce. Name normalization now runs in the
  operations layer and reuses the contract's own sandboxNameSchema, so the
  tool path — which has no schema in front of it — cannot accept a name the
  REST path rejects.
- The handler resolved permissions through ensureWorkspaceAccess while the
  route used getUserEntityPermissions, so the same refusal was worded two
  ways. Both now use the same primitive and the same SANDBOX_ADMIN_REQUIRED
  constant, and a DB failure no longer reads as a permission denial.
- list returned errorDetail, a 4KB installer log tail per failed build, on
  every call. errorMessage is the classified summary and is all the tool
  prompt advertises.
- buildSpecUpdate now returns a result instead of throwing, dropping the
  SandboxDependencyError class, the rethrowing bridge helper, and both
  try/catch blocks.
- Model-supplied dependencies are type-checked once before dispatch rather
  than per branch; the language error copy derives from SANDBOX_LANGUAGES.
- Create logging moved into the operations layer so the tool's creates are
  logged too, and the route no longer carries a logger.
- manage_sandbox was missing from the chat tool-icon map and fell through to
  the mothership Blimp fallback.

Author: waleedlatif1

apps/sim/app/api/workspaces/[id]/sandboxes/authorize.ts

@@ -26,6 +26,8 @@ export interface SandboxMutationActor {
  */
 export function sandboxFailureResponse(failure: SandboxWriteFailure): NextResponse {
   switch (failure.code) {
+    case 'invalid_name':
+      return NextResponse.json({ error: failure.message }, { status: 400 })
     case 'name_conflict':
       return NextResponse.json(
         { error: `A sandbox named "${failure.name}" already exists in this workspace` },
@@ -78,16 +80,14 @@ export async function authorizeSandboxMutation(
 }
 
 /** Reads a workspace sandbox list; any member may look, only admins may write. */
-export async function authorizeSandboxRead(
-  workspaceId: string
-): Promise<{ ok: true; userId: string } | { ok: false; response: NextResponse }> {
+export async function authorizeSandboxRead(workspaceId: string): Promise<NextResponse | null> {
   const session = await getSession()
   if (!session?.user?.id) {
-    return { ok: false, response: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) }
+    return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
   }
   const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
   if (!permission) {
-    return { ok: false, response: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) }
+    return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
   }
-  return { ok: true, userId: session.user.id }
+  return null
 }

apps/sim/app/api/workspaces/[id]/sandboxes/route.ts

@@ -1,4 +1,3 @@
-import { createLogger } from '@sim/logger'
 import { type NextRequest, NextResponse } from 'next/server'
 import { createSandboxContract } from '@/lib/api/contracts/sandboxes'
 import { parseRequest } from '@/lib/api/server'
@@ -15,14 +14,12 @@ import {
   sandboxFailureResponse,
 } from '@/app/api/workspaces/[id]/sandboxes/authorize'
 
-const logger = createLogger('WorkspaceSandboxesAPI')
-
 export const GET = withRouteHandler(
   async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
     const workspaceId = (await context.params).id
 
-    const viewer = await authorizeSandboxRead(workspaceId)
-    if (!viewer.ok) return viewer.response
+    const denied = await authorizeSandboxRead(workspaceId)
+    if (denied) return denied
 
     // The list itself is not plan-gated: a workspace that downgraded must still
     // see (and keep executing) what it already built. `entitled` drives whether
@@ -60,11 +57,6 @@ export const POST = withRouteHandler(
     })
     if (!result.ok) return sandboxFailureResponse(result.failure)
 
-    logger.info('Created workspace sandbox', {
-      workspaceId,
-      sandboxId: result.sandbox.id,
-      language,
-    })
     return NextResponse.json({ sandbox: result.sandbox })
   }
 )

apps/sim/app/workspace/[workspaceId]/home/components/message-content/utils.ts

@@ -39,6 +39,7 @@ const TOOL_ICONS: Record<string, IconComponent> = {
   get_page_contents: Search,
   search_library_docs: Library,
   manage_mcp_tool: Settings,
+  manage_sandbox: TerminalWindow,
   manage_skill: Asterisk,
   user_memory: Database,
   function_execute: TerminalWindow,

apps/sim/lib/api/contracts/sandboxes.ts

@@ -28,7 +28,7 @@ const dependencyListSchema = z
   .array(z.string().max(2000, 'a dependency line is unreasonably long'))
   .max(1000, 'too many lines — paste a shorter dependency list')
 
-const sandboxNameSchema = z
+export const sandboxNameSchema = z
   .string()
   .trim()
   .min(1, 'Name is required')

apps/sim/lib/copilot/tools/handlers/management/manage-sandbox.test.ts

@@ -6,15 +6,15 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { ExecutionContext } from '@/lib/copilot/request/types'
 
 const {
-  ensureWorkspaceAccessMock,
+  getUserEntityPermissionsMock,
   hasWorkspaceSandboxAccessMock,
   enforceWorkspaceRateLimitMock,
   createWorkspaceSandboxMock,
   updateWorkspaceSandboxMock,
   deleteWorkspaceSandboxMock,
   listWorkspaceSandboxesMock,
 } = vi.hoisted(() => ({
-  ensureWorkspaceAccessMock: vi.fn(),
+  getUserEntityPermissionsMock: vi.fn(),
   hasWorkspaceSandboxAccessMock: vi.fn(),
   enforceWorkspaceRateLimitMock: vi.fn(),
   createWorkspaceSandboxMock: vi.fn(),
@@ -23,8 +23,8 @@ const {
   listWorkspaceSandboxesMock: vi.fn(),
 }))
 
-vi.mock('@/lib/copilot/tools/handlers/access', () => ({
-  ensureWorkspaceAccess: ensureWorkspaceAccessMock,
+vi.mock('@/lib/workspaces/permissions/utils', () => ({
+  getUserEntityPermissions: getUserEntityPermissionsMock,
 }))
 
 vi.mock('@/lib/billing/core/subscription', () => ({
@@ -67,7 +67,7 @@ const sandbox = {
 describe('manage_sandbox', () => {
   beforeEach(() => {
     vi.clearAllMocks()
-    ensureWorkspaceAccessMock.mockResolvedValue({})
+    getUserEntityPermissionsMock.mockResolvedValue('admin')
     hasWorkspaceSandboxAccessMock.mockResolvedValue(true)
     enforceWorkspaceRateLimitMock.mockResolvedValue(null)
     listWorkspaceSandboxesMock.mockResolvedValue([sandbox])
@@ -84,27 +84,31 @@ describe('manage_sandbox', () => {
 
   it('ignores a model-supplied workspaceId and uses the server context', async () => {
     await executeManageSandbox({ operation: 'list', workspaceId: 'other-ws' }, context)
-    expect(ensureWorkspaceAccessMock).toHaveBeenCalledWith('ws-1', 'user-1', 'read')
+    expect(getUserEntityPermissionsMock).toHaveBeenCalledWith('user-1', 'workspace', 'ws-1')
     expect(listWorkspaceSandboxesMock).toHaveBeenCalledWith('ws-1')
   })
 
   it('lists with only read access, and does not spend the mutation budget', async () => {
+    getUserEntityPermissionsMock.mockResolvedValue('read')
+
     const result = await executeManageSandbox({ operation: 'list' }, context)
     expect(result.success).toBe(true)
     expect(result.output).toMatchObject({ count: 1, strategy: 'prebuilt' })
+    const [listed] = (result.output as { sandboxes: Record<string, unknown>[] }).sandboxes
+    expect(listed).not.toHaveProperty('errorDetail')
+    expect(listed).toMatchObject({ id: 'sb-1', buildStatus: 'pending' })
     expect(enforceWorkspaceRateLimitMock).not.toHaveBeenCalled()
     expect(hasWorkspaceSandboxAccessMock).not.toHaveBeenCalled()
   })
 
   it.each(['add', 'edit', 'delete'])('requires workspace admin to %s', async (operation) => {
-    ensureWorkspaceAccessMock.mockRejectedValue(new Error('Admin access required'))
+    getUserEntityPermissionsMock.mockResolvedValue('write')
 
     const result = await executeManageSandbox(
       { operation, name: 'x', language: 'python', sandboxId: 'sb-1' },
       context
     )
 
-    expect(ensureWorkspaceAccessMock).toHaveBeenCalledWith('ws-1', 'user-1', 'admin')
     expect(result.success).toBe(false)
     expect(result.error).toBe('Only workspace admins can manage sandboxes')
     expect(createWorkspaceSandboxMock).not.toHaveBeenCalled()
@@ -152,7 +156,7 @@ describe('manage_sandbox', () => {
     expect(createWorkspaceSandboxMock).toHaveBeenCalledWith({
       workspaceId: 'ws-1',
       userId: 'user-1',
-      name: 'data-tools',
+      name: '  data-tools  ',
       language: 'python',
       dependencies: ['requests'],
     })
@@ -167,7 +171,7 @@ describe('manage_sandbox', () => {
     )
 
     expect(result.success).toBe(false)
-    expect(result.error).toContain('javascript')
+    expect(result.error).toContain('javascript or python')
     expect(createWorkspaceSandboxMock).not.toHaveBeenCalled()
   })
 
@@ -234,6 +238,40 @@ describe('manage_sandbox', () => {
     expect(result.error).toContain('sb-9')
   })
 
+  it('forwards a whitespace-only name to the operation, which refuses it', async () => {
+    await executeManageSandbox({ operation: 'edit', sandboxId: 'sb-1', name: '   ' }, context)
+
+    expect(updateWorkspaceSandboxMock).toHaveBeenCalledWith(
+      expect.objectContaining({ name: '   ' })
+    )
+  })
+
+  it('rejects a non-string dependency list', async () => {
+    const result = await executeManageSandbox(
+      { operation: 'add', name: 'data-tools', language: 'python', dependencies: [1, 2] },
+      context
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toContain('array of strings')
+    expect(createWorkspaceSandboxMock).not.toHaveBeenCalled()
+  })
+
+  it('surfaces an invalid name refused by the operation', async () => {
+    createWorkspaceSandboxMock.mockResolvedValue({
+      ok: false,
+      failure: { code: 'invalid_name', message: 'Name must be 64 characters or fewer' },
+    })
+
+    const result = await executeManageSandbox(
+      { operation: 'add', name: 'x'.repeat(65), language: 'python' },
+      context
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toBe('Name must be 64 characters or fewer')
+  })
+
   it('rejects an unsupported operation', async () => {
     const result = await executeManageSandbox({ operation: 'rebuild' }, context)
     expect(result.success).toBe(false)