Skip to content

Commit f2fbf81

Browse files
committed
refactor(copilot): classify both workspace-scope guard branches and drop call-site boilerplate
requireCopilotWorkspace now accepts an undefined context and throws a classified OrchestrationError for the missing-workspace branch too, so every caller drops the 'context ?? {}' and '|| undefined' coercions and one instanceof covers the guard. query_logs inlines its now-one-line wrapper, get_credentials drops the workspace-less special case (a workflow with no workspace asserts nothing), and publish_custom_block handles the guard locally instead of widening its catch-all — keeping its deliberate assume-not-published guidance for unrelated failures.
1 parent e5c61c2 commit f2fbf81

7 files changed

Lines changed: 26 additions & 26 deletions

File tree

apps/sim/lib/copilot/tools/handlers/deployment/custom-block.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/typ
1212
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
1313
import { canonicalizeVfsPath } from '@/lib/copilot/vfs/path-utils'
1414
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
15-
import { OrchestrationError } from '@/lib/core/orchestration/types'
1615
import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key'
1716
import { uploadFile } from '@/lib/uploads/core/storage-service'
1817
import { isImageFileType } from '@/lib/uploads/utils/file-utils'
@@ -149,11 +148,17 @@ export async function executeDeployCustomBlock(
149148
error: "Managing a custom block requires admin permission on the workflow's workspace",
150149
}
151150
}
152-
const rawWorkspaceId = workflowRecord.workspaceId
153-
if (!rawWorkspaceId) {
151+
if (!workflowRecord.workspaceId) {
154152
return { success: false, error: 'Workflow must belong to a workspace' }
155153
}
156-
const workspaceId = requireCopilotWorkspace(context, rawWorkspaceId)
154+
// A model-supplied workflowId may only re-assert the chat's workspace — it
155+
// can never publish or unpublish a custom block in another workspace.
156+
let workspaceId: string
157+
try {
158+
workspaceId = requireCopilotWorkspace(context, workflowRecord.workspaceId)
159+
} catch (error) {
160+
return { success: false, error: toError(error).message }
161+
}
157162

158163
const ws = await getWorkspaceWithOwner(workspaceId)
159164
const organizationId = ws?.organizationId
@@ -306,7 +311,7 @@ export async function executeDeployCustomBlock(
306311
})
307312
return { success: true, output: { ...customBlockOutput(block, 'deploy'), updated: false } }
308313
} catch (error) {
309-
if (error instanceof CustomBlockValidationError || error instanceof OrchestrationError) {
314+
if (error instanceof CustomBlockValidationError) {
310315
return { success: false, error: error.message }
311316
}
312317
logger.error('Custom block deployment failed', { error })

apps/sim/lib/copilot/tools/handlers/deployment/manage.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ export async function executeListWorkspaceMcpServers(
139139
context: ExecutionContext
140140
): Promise<ToolCallResult> {
141141
try {
142-
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
142+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
143143
const result = await executeCopilotMcpServerUseCase(context, listWorkflowMcpDeployments, {
144144
workspaceId,
145145
})
@@ -161,7 +161,7 @@ export async function executeCreateWorkspaceMcpServer(
161161
context: ExecutionContext
162162
): Promise<ToolCallResult> {
163163
try {
164-
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
164+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
165165

166166
const name = params.name?.trim()
167167
if (!name) {

apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export async function executeCreateWorkflow(
152152
if (name.length > 200) {
153153
return { success: false, error: 'Workflow name must be 200 characters or less' }
154154
}
155-
const workspaceId = requireCopilotWorkspace(context, params?.workspaceId || undefined)
155+
const workspaceId = requireCopilotWorkspace(context, params?.workspaceId)
156156

157157
const folderPath = typeof params?.folderPath === 'string' ? params.folderPath.trim() : ''
158158
const folderId =
@@ -362,7 +362,7 @@ export async function executeGenerateApiKey(
362362
return { success: false, error: 'API key name must be 200 characters or less' }
363363
}
364364

365-
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
365+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
366366
assertWorkflowMutationNotAborted(context)
367367

368368
const result = await executeCopilotApiKeyUseCase(context, createCopilotWorkspaceApiKey, {

apps/sim/lib/copilot/tools/server/user/get-credentials.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
5656
}
5757

5858
// A model-supplied workflowId may only re-assert the chat's workspace —
59-
// it can never steer the credential listing to another workspace. A
60-
// legacy workflow with no workspace contributes no workspace scope.
61-
workspaceId = wId ? requireCopilotWorkspace(context, wId) : undefined
59+
// it can never steer the credential listing to another workspace.
60+
workspaceId = requireCopilotWorkspace(context, wId)
6261
}
6362

6463
const userId = authenticatedUserId

apps/sim/lib/copilot/tools/server/user/set-environment-variables.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,10 @@ async function resolveWorkspaceId(
169169
`Workflow ${params.workflowId} is not associated with a workspace`
170170
)
171171
}
172-
return requireCopilotWorkspace(context ?? {}, workflow.workspaceId)
172+
return requireCopilotWorkspace(context, workflow.workspaceId)
173173
}
174174

175-
const workspaceId = requireCopilotWorkspace(context ?? {}, params.workspaceId)
175+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
176176
await ensureWorkspaceAccess(workspaceId, userId, 'write')
177177
return workspaceId
178178
}

apps/sim/lib/copilot/tools/server/workflow/query-logs.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,6 @@ const queryLogsArgsSchema = z.preprocess((value) => {
118118

119119
type QueryLogsArgs = z.infer<typeof queryLogsArgsSchema>
120120

121-
/**
122-
* Logs are always read from the chat's delegated workspace. A model-supplied
123-
* `workspaceId` may only re-assert that workspace — it can never select a
124-
* different one, even one the acting user could otherwise access.
125-
*/
126-
function resolveWorkspaceId(args: QueryLogsArgs, context?: ServerToolContext): string {
127-
return requireCopilotWorkspace(context ?? {}, args.workspaceId)
128-
}
129-
130121
function buildLogViewContext(
131122
detail: {
132123
workflowId: string | null
@@ -177,7 +168,10 @@ export const queryLogsServerTool: BaseServerTool<QueryLogsArgs, unknown> = {
177168
throw new Error('Unauthorized access')
178169
}
179170
const userId = context.userId
180-
const workspaceId = resolveWorkspaceId(args, context)
171+
// Logs are always read from the chat's delegated workspace. A model-supplied
172+
// `workspaceId` may only re-assert that workspace — it can never select a
173+
// different one, even one the acting user could otherwise access.
174+
const workspaceId = requireCopilotWorkspace(context, args.workspaceId)
181175

182176
if (args.view === 'list') {
183177
const { view: _view, title: _title, ...rest } = args

apps/sim/lib/copilot/tools/server/workspace-scope.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ interface CopilotWorkspaceScopeContext {
99
* but can never select a different workspace or trigger a default-workspace fallback.
1010
*/
1111
export function requireCopilotWorkspace(
12-
context: CopilotWorkspaceScopeContext,
12+
context: CopilotWorkspaceScopeContext | undefined,
1313
assertedWorkspaceId?: string
1414
): string {
15-
if (!context.workspaceId) throw new Error('Copilot execution workspace is required')
15+
if (!context?.workspaceId) {
16+
throw new OrchestrationError('validation', 'Copilot execution workspace is required')
17+
}
1618
if (assertedWorkspaceId && assertedWorkspaceId !== context.workspaceId) {
1719
throw new OrchestrationError(
1820
'validation',

0 commit comments

Comments
 (0)