From 62660f229be714614635a990f28e2fb57ebb06b4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:38 -0700 Subject: [PATCH 1/9] fix(catalog): resolve an unversioned tool id against the visible set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tools get github_comment` answered NOT_FOUND while `github_comment_v2` worked, though the toolId help promises an unversioned name resolves to the newest version. A superseded tool stays in the registry, so `resolveToolId` short-circuits on the exact hit and returns it unchanged; the visibility gate then refuses it because no visible block exposes a v1 tool. 204 base names were unresolvable this way. Resolution now walks the visible set newest-first, the way blocks already do. `resolveToolId` is untouched — execution depends on an exact id returning that exact id, and none of the 5182 visible ids change under the new path. --- .../catalog/application/catalog-reads.test.ts | 45 +++++++++++++++ apps/sim/lib/catalog/application/get-tool.ts | 23 ++++---- .../sim/lib/catalog/application/tool-scope.ts | 35 ++++++++++++ packages/sim-cli/src/runtime/execute.test.ts | 55 +++++++++++++++++++ 4 files changed, 148 insertions(+), 10 deletions(-) diff --git a/apps/sim/lib/catalog/application/catalog-reads.test.ts b/apps/sim/lib/catalog/application/catalog-reads.test.ts index 9f8aa2d86fa..74a4e73e0a1 100644 --- a/apps/sim/lib/catalog/application/catalog-reads.test.ts +++ b/apps/sim/lib/catalog/application/catalog-reads.test.ts @@ -107,6 +107,22 @@ const TOOL_METADATA: Record> = { params: {}, hostedApiKey: 'always', }, + confluence_read: { + id: 'confluence_read', + name: 'Confluence Read', + description: 'Read a Confluence page.', + version: '1.0.0', + params: {}, + hostedApiKey: 'none', + }, + confluence_read_v2: { + id: 'confluence_read_v2', + name: 'Confluence Read', + description: 'Read a Confluence page.', + version: '2.0.0', + params: {}, + hostedApiKey: 'none', + }, } const WORKSPACE_ID = 'workspace-1' @@ -186,6 +202,7 @@ const confluenceV2 = block({ type: 'confluence_v2', name: 'Confluence', description: 'Read Confluence pages.', + tools: { access: ['confluence_read_v2'] }, }) interface Visibility { @@ -566,6 +583,34 @@ describe('catalog block and tool reads', () => { expect(listed.entries.map((entry) => entry.hostedApiKey)).toEqual(['none', 'none']) }) + /** + * A superseded v1 tool stays registered so execution of a stored id keeps + * working, so `resolveToolId('confluence_read')` answers with the v1 id no + * visible block exposes — and `GET /v2/tools/confluence_read` 404'd while the + * list published `confluence_read_v2`. + */ + it('resolves an unversioned tool name to its newest visible version and echoes the resolved id', async () => { + mocks.getAllBlocks.mockReturnValue([confluenceV2]) + + const { tool } = await getCatalogTool.execute({ + principal: session, + input: { workspaceId: WORKSPACE_ID, toolId: 'confluence_read' }, + }) + + expect(tool.id).toBe('confluence_read_v2') + }) + + it('echoes an exact versioned tool id unchanged', async () => { + mocks.getAllBlocks.mockReturnValue([confluenceV2]) + + const { tool } = await getCatalogTool.execute({ + principal: session, + input: { workspaceId: WORKSPACE_ID, toolId: 'confluence_read_v2' }, + }) + + expect(tool.id).toBe('confluence_read_v2') + }) + it('reads one tool with its params and outputs', async () => { const { tool } = await getCatalogTool.execute({ principal: session, diff --git a/apps/sim/lib/catalog/application/get-tool.ts b/apps/sim/lib/catalog/application/get-tool.ts index d08b181f838..62b1323a2c4 100644 --- a/apps/sim/lib/catalog/application/get-tool.ts +++ b/apps/sim/lib/catalog/application/get-tool.ts @@ -3,12 +3,11 @@ import { resolveCatalogGate, } from '@/lib/catalog/application/catalog-context' import { catalogOperations } from '@/lib/catalog/application/operations' -import { resolveVisibleToolIds } from '@/lib/catalog/application/tool-scope' +import { resolveVisibleToolId, resolveVisibleToolIds } from '@/lib/catalog/application/tool-scope' import { type CatalogToolDetail, projectToolDetail } from '@/lib/catalog/projection/tool' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { isHosted } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' -import { resolveToolId } from '@/tools/tool-ids' export interface GetCatalogToolInput { workspaceId: string @@ -22,10 +21,14 @@ export interface GetCatalogToolResult { /** * One built-in tool's parameters and outputs. * - * An unversioned name resolves to the newest version exactly as execution does, - * and the returned `id` is the resolved one so a caller can see which version - * answered. A tool the workspace's blocks do not expose answers 404 rather than - * 403, for the same enumeration reason as the block detail read. + * An unversioned name resolves to the newest version this caller can see, the + * way the block detail read does, and the returned `id` is the resolved one so + * a caller can see which version answered. Resolving through + * `@/tools/tool-ids` instead would answer with the superseded v1 that stays + * registered for execution's sake, which no visible block exposes — so every + * versioned family 404'd on the base name. A tool the workspace's blocks do not + * expose answers 404 rather than 403, for the same enumeration reason as the + * block detail read. */ export const getCatalogTool = defineAuthorizedWorkspaceUseCase({ operation: catalogOperations.readTool, @@ -33,16 +36,16 @@ export const getCatalogTool = defineAuthorizedWorkspaceUseCase({ loadCatalogWorkspaceContext(input.workspaceId), authorizationOptions: {}, execute: async ({ principal, input, context }): Promise => { - const resolvedToolId = resolveToolId(input.toolId) - const tool = projectToolDetail(resolvedToolId, { hostedKeys: isHosted }) - if (!tool) throw new OrchestrationError('not_found', 'Tool not found') - const gate = await resolveCatalogGate(principal, context) const visibleToolIds = await resolveVisibleToolIds(gate) + const resolvedToolId = resolveVisibleToolId(input.toolId, visibleToolIds) if (!visibleToolIds.has(resolvedToolId)) { throw new OrchestrationError('not_found', 'Tool not found') } + const tool = projectToolDetail(resolvedToolId, { hostedKeys: isHosted }) + if (!tool) throw new OrchestrationError('not_found', 'Tool not found') + return { tool } }, }) diff --git a/apps/sim/lib/catalog/application/tool-scope.ts b/apps/sim/lib/catalog/application/tool-scope.ts index c7c5a0c344a..77868f61a2f 100644 --- a/apps/sim/lib/catalog/application/tool-scope.ts +++ b/apps/sim/lib/catalog/application/tool-scope.ts @@ -6,6 +6,41 @@ import { import { getAllBlocks } from '@/blocks/registry' import { resolveToolId } from '@/tools/tool-ids' +const VERSION_SUFFIX = /^\d+$/ + +/** + * The newest visible version of a tool name, or the name unchanged when none is. + * + * The detail-read counterpart of {@link resolveVisibleToolIds}, and the tool + * analogue of `getLatestBlockForViewer`: "newest registered" and "newest visible + * to this caller" are different questions. `@/tools/tool-ids` answers the first + * — and superseded v1 tools stay registered so execution of a stored id keeps + * working, so `resolveToolId('github_comment')` returns `github_comment`, which + * no visible block exposes. Resolving against the visible set instead walks down + * to `github_comment_v2`, the id the tool list publishes. + * + * An id that is itself visible is returned untouched, so an exact versioned + * request never silently answers with a different version. + */ +export function resolveVisibleToolId(toolId: string, visibleToolIds: ReadonlySet): string { + if (visibleToolIds.has(toolId)) return toolId + + const prefix = `${toolId}_v` + let bestId: string | undefined + let bestVersion = 0 + for (const candidate of visibleToolIds) { + if (!candidate.startsWith(prefix)) continue + const suffix = candidate.slice(prefix.length) + if (!VERSION_SUFFIX.test(suffix)) continue + const version = Number.parseInt(suffix, 10) + if (version > bestVersion) { + bestVersion = version + bestId = candidate + } + } + return bestId ?? toolId +} + /** * The built-in tools this caller may run in this workspace. * diff --git a/packages/sim-cli/src/runtime/execute.test.ts b/packages/sim-cli/src/runtime/execute.test.ts index 8750c52ded4..ad0e715ec3c 100644 --- a/packages/sim-cli/src/runtime/execute.test.ts +++ b/packages/sim-cli/src/runtime/execute.test.ts @@ -73,6 +73,19 @@ const MOVE_WORKFLOWS: OperationSpec = { const MOVE_FLAGS = { workflow: ['wf_1'], to: '/a' } +const DELETE_TABLE_ROWS: OperationSpec = { + method: 'DELETE', + path: '/api/v2/tables/[tableId]/rows', + pathParams: ['tableId'], + body: { rowIds: { kind: 'array' }, filter: { kind: 'unknown' } }, +} + +/** Invokes a generated command that takes both a path positional and flags. */ +function invokeRowDelete(flags: Record) { + const host = new Command('leaf') + return executeOperation('deleteTableRows', {}, DELETE_TABLE_ROWS, ['tbl_1', flags, host]) +} + /** Invokes a generated command that takes its input from flags rather than positionals. */ function invokeWithFlags( operation: 'bulkDeleteTables' | 'bulkDeleteFiles' | 'moveTables' | 'moveWorkflows', @@ -339,6 +352,48 @@ describe('a bulk call that changed nothing', () => { await expect(invokeWithFlags('moveTables', MOVE_TABLES, {})).resolves.toBeUndefined() }) + + it('fails the process when no requested row was deleted', async () => { + request.mockResolvedValue({ + data: { + deletedCount: 0, + deletedRowIds: [], + requestedCount: 1, + missingRowIds: ['00000000-0000-0000-0000-000000000000'], + }, + }) + + await expect( + invokeRowDelete({ row: ['00000000-0000-0000-0000-000000000000'] }) + ).rejects.toThrow(/Deleted nothing: none of the 1 requested row was deleted\./) + }) + + it('succeeds on a partial row delete', async () => { + request.mockResolvedValue({ + data: { + deletedCount: 1, + deletedRowIds: ['row_1'], + requestedCount: 2, + missingRowIds: ['row_gone'], + }, + }) + + await expect(invokeRowDelete({ row: ['row_1', 'row_gone'] })).resolves.toBeUndefined() + }) + + /** + * The selection mode the guard must not touch. A filter answers with a deleted + * count and no `requestedCount`, and a filter that matches nothing deleted + * nothing because there was nothing left to delete — the second run of an + * idempotent sweep, not a failure. + */ + it('succeeds when a filter matched no rows', async () => { + request.mockResolvedValue({ data: { deletedCount: 0, deletedRowIds: [] } }) + + await expect( + invokeRowDelete({ filter: { all: [{ field: 'status', op: 'eq', value: 'archived' }] } }) + ).resolves.toBeUndefined() + }) }) const BULK_UPDATE_CHUNKS: OperationSpec = { From 3be0b648b4d9413cc99c633365e3aeda60ca9f6a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:38 -0700 Subject: [PATCH 2/9] fix(files): resolve an archived folder path through its active ancestors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The archived folder listing built its path map from the archived rows alone, so a folder whose parent is still active came back as its own name. Deleting `a/sub` and restoring `a/sub` therefore disagreed — restore only matched the truncated `sub` — and the path and parentPath fields were wrong. The extra read is taken only for the archived scope; active and all keep their single query, which a test now pins. Restoring by path also stopped guessing. Archiving, recreating and archiving again leaves two archived folders with the same canonical path, and the resolver took the first match, silently restoring the wrong one. It now refuses and names the folder-id form. --- .../workspace-file-folder-manager.test.ts | 49 ++++++++++++++++++- .../workspace-file-folder-manager.ts | 19 ++++++- .../workspace-file-folders.test.ts | 16 ++++++ .../application/workspace-file-folders.ts | 12 +++-- 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts index 8b6963f7382..797b5469b97 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ -import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockAcquireFolderMutationLock, mockDeduplicateFolderName } = vi.hoisted(() => ({ @@ -25,6 +25,7 @@ import { buildWorkspaceFileFolderPathMap, createWorkspaceFileFolder, ensureWorkspaceFileFolderPath, + listWorkspaceFileFolders, normalizeWorkspaceFileItemName, WorkspaceFileFolderConflictError, WorkspaceFileItemsNotFoundError, @@ -168,6 +169,52 @@ describe('workspace file folder failure classification', () => { }) }) +describe('listWorkspaceFileFolders', () => { + const now = new Date('2026-08-17T12:00:00.000Z') + const activeParent = { + id: 'parent-1', + resourceType: 'file', + workspaceId: 'workspace-1', + userId: 'user-1', + name: 'Engineering', + parentId: null, + sortOrder: 0, + deletedAt: null, + createdAt: now, + updatedAt: now, + } + const archivedChild = { + ...activeParent, + id: 'child-1', + name: 'Archive', + parentId: 'parent-1', + deletedAt: now, + } + + beforeEach(() => { + resetDbChainMock() + }) + + it('resolves an archived folder path through its still-active ancestors', async () => { + queueTableRows(schemaMock.folder, [archivedChild]) + queueTableRows(schemaMock.folder, [activeParent, archivedChild]) + + const folders = await listWorkspaceFileFolders('workspace-1', { scope: 'archived' }) + + expect(folders).toHaveLength(1) + expect(folders[0].path).toBe('Engineering/Archive') + }) + + it('does not take an extra query for the active scope', async () => { + queueTableRows(schemaMock.folder, [activeParent]) + + const folders = await listWorkspaceFileFolders('workspace-1') + + expect(folders.map((folder) => folder.path)).toEqual(['Engineering']) + expect(dbChainMockFns.from).toHaveBeenCalledOnce() + }) +}) + describe('archiveWorkspaceFileFolderIfEmpty', () => { beforeEach(() => { resetDbChainMock() diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts index 4ec715d2474..c63aa73469b 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts @@ -385,6 +385,21 @@ export async function findWorkspaceFileFolderIdByPath( return parentId } +/** + * Selects the minimal columns needed to resolve every file folder's canonical path. + * + * Includes archived rows: an archived folder can still have an active ancestor, so a + * path map built only from archived rows truncates its path to the bare folder name. + */ +async function selectFileFolderPathRows( + workspaceId: string +): Promise>> { + return db + .select({ id: folderTable.id, name: folderTable.name, parentId: folderTable.parentId }) + .from(folderTable) + .where(and(eq(folderTable.workspaceId, workspaceId), isFileFolder)) +} + /** * Lists a workspace's file folders, ordered in the database like every other folder * list so a name sort uses the same collation and the same `createdAt` tiebreak. @@ -420,7 +435,9 @@ export async function listWorkspaceFileFolders( ) .orderBy(...listOrderBy(FOLDER_SORTS[sortBy], sortOrder)) - const paths = buildWorkspaceFileFolderPathMap(rows) + const paths = buildWorkspaceFileFolderPathMap( + scope === 'archived' ? await selectFileFolderPathRows(workspaceId) : rows + ) return rows.map((row) => mapFolder(row, paths)) } diff --git a/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts b/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts index cc0d072f031..b673baaec77 100644 --- a/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts +++ b/apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts @@ -275,6 +275,22 @@ describe('workspace file folder operations', () => { expect(mockRestore).not.toHaveBeenCalled() }) + it('refuses to guess when two archived folders share the same path', async () => { + mockList.mockResolvedValueOnce([ + { id: 'folder-first', name: 'Archive', path: 'Engineering/Archive' }, + { id: 'folder-second', name: 'Archive', path: 'Engineering/Archive' }, + ]) + + await expect( + restoreWorkspaceFileFolderOperation.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: 'ws-1', path: '/Engineering/Archive' }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + + expect(mockRestore).not.toHaveBeenCalled() + }) + it('rejects restoring the workspace root', async () => { await expect( restoreWorkspaceFileFolderOperation.execute({ diff --git a/apps/sim/lib/workspace-files/application/workspace-file-folders.ts b/apps/sim/lib/workspace-files/application/workspace-file-folders.ts index 82b8b7f18e3..50f138b94fb 100644 --- a/apps/sim/lib/workspace-files/application/workspace-file-folders.ts +++ b/apps/sim/lib/workspace-files/application/workspace-file-folders.ts @@ -264,15 +264,21 @@ async function findArchivedFolderIdByPath(workspaceId: string, path: string): Pr throw new OrchestrationError('validation', 'The workspace root cannot be restored') } const archived = await listWorkspaceFileFolders(workspaceId, { scope: 'archived' }) - const match = archived.find((folder) => { + const matches = archived.filter((folder) => { const segments = parseWorkspaceFileFolderDisplayPath(folder.path) return ( segments.length === target.length && segments.every((segment, index) => segment === target[index]) ) }) - if (!match) throw new OrchestrationError('not_found', 'Folder not found') - return match.id + if (matches.length === 0) throw new OrchestrationError('not_found', 'Folder not found') + if (matches.length > 1) { + throw new OrchestrationError( + 'conflict', + 'Multiple archived folders share this path. Restore by folder id instead.' + ) + } + return matches[0].id } async function executeRestoreWorkspaceFileFolder(args: { From acd6f3ec4eeab1db63c6c2abbdd33dc1f9a8175c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:38 -0700 Subject: [PATCH 3/9] fix(v2): answer a folder-list miss with an empty page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parentPath naming no folder returned 404 on the workflow, table and knowledge folder lists, and an empty page on files. The rule the codebase already publishes is the empty page: V2_FOLDER_FILTER_MISS is appended to the folderPath filter on six list surfaces, and resolveFolderPathFilter documents why a list must not become an existence oracle — a 404 claims the collection is missing and breaks a walk when a folder is deleted mid-pagination. Both TSDocs asserted the sibling folder lists already behaved that way. They did not; that premise is corrected here too. Mutations keep every 404. The miss short-circuits before the row query, because an unfiltered parent id lists the whole workspace. --- apps/sim/lib/api/contracts/v2/shared.ts | 12 +- apps/sim/lib/folders/queries.ts | 4 +- .../lib/knowledge/application/folders.test.ts | 25 +++-- apps/sim/lib/knowledge/application/folders.ts | 12 +- .../sim/lib/table/application/folders.test.ts | 106 ++++++++++++++++++ apps/sim/lib/table/application/folders.ts | 13 +-- .../application/workflow-folders.test.ts | 37 ++++++ .../workflows/application/workflow-folders.ts | 12 +- 8 files changed, 182 insertions(+), 39 deletions(-) create mode 100644 apps/sim/lib/table/application/folders.test.ts diff --git a/apps/sim/lib/api/contracts/v2/shared.ts b/apps/sim/lib/api/contracts/v2/shared.ts index eb2ad6edb19..fd594f14f90 100644 --- a/apps/sim/lib/api/contracts/v2/shared.ts +++ b/apps/sim/lib/api/contracts/v2/shared.ts @@ -471,10 +471,10 @@ export const v2SearchSchema = z * to nothing, exactly as `workflowIds` naming no workflow does. These lists used * to answer `404 Folder not found` instead, which reported a missing collection * for a collection that exists, broke a pagination walk when a folder was - * deleted mid-walk, and made a list a folder-existence oracle. The sibling - * folder lists already answered a non-matching `parentPath` with an empty page. - * Mutations keep their 404 — creating into or moving to a folder that does not - * exist has no empty-set reading. + * deleted mid-walk, and made a list a folder-existence oracle. The folder lists + * answer a non-matching `parentPath` the same way, so one rule covers every + * folder filter in the family. Mutations keep their 404 — creating into or + * moving to a folder that does not exist has no empty-set reading. */ export const V2_FOLDER_FILTER_MISS = 'A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.' @@ -593,7 +593,9 @@ export const v2ListFoldersQuerySchema = z workspaceId: workspaceIdSchema.describe('Workspace whose folders should be listed.'), parentPath: v2FolderPathInputSchema .optional() - .describe('Restrict results to direct children of this parent path.'), + .describe( + `Restrict results to direct children of this parent path. ${V2_FOLDER_FILTER_MISS}` + ), search: v2SearchSchema.describe('Case-insensitive substring match against the folder name.'), ...v2SortFields(v2FolderSortFields, { sortBy: 'name', sortOrder: 'asc' }), }) diff --git a/apps/sim/lib/folders/queries.ts b/apps/sim/lib/folders/queries.ts index 91b000867a0..7c381e42c4b 100644 --- a/apps/sim/lib/folders/queries.ts +++ b/apps/sim/lib/folders/queries.ts @@ -266,8 +266,8 @@ export type FolderPathFilter = * event from all the others, told a caller its *collection* was missing when it * was not, turned a folder deleted mid-walk into a failed pagination loop, and * answered whether a path exists on an endpoint that was not asked. The sibling - * folder lists already answer a non-matching `parentPath` with an empty page, so - * this is the family's existing behavior applied to the resource lists too. + * folder lists answer a non-matching `parentPath` the same way, so one rule + * covers every folder filter the family accepts. * * A path that could not name a folder at all is still rejected by the contract, * as a 400, before any of this runs. Mutations keep their 404: creating into or diff --git a/apps/sim/lib/knowledge/application/folders.test.ts b/apps/sim/lib/knowledge/application/folders.test.ts index 33442e38f6d..a2aafbee769 100644 --- a/apps/sim/lib/knowledge/application/folders.test.ts +++ b/apps/sim/lib/knowledge/application/folders.test.ts @@ -42,6 +42,12 @@ vi.mock('@/lib/knowledge/application/contexts', () => ({ vi.mock('@/lib/folders/queries', () => ({ loadActiveFolderPathIndex: mocks.loadIndex, listActiveFolderRows: mocks.listRows, + resolveFolderPathFilter: (index: { idByPath: Map }, path: string | undefined) => { + if (path === undefined) return { kind: 'unfiltered' } + if (path === '/') return { kind: 'folder', folderId: null } + const folderId = index.idByPath.get(path) + return folderId === undefined ? { kind: 'noMatch' } : { kind: 'folder', folderId } + }, resolveFolderPathFromIndex: (index: { idByPath: Map }, path: string) => path === '/' ? null : index.idByPath.get(path), })) @@ -112,14 +118,19 @@ describe('knowledge folder application use cases', () => { expect(result.folders[0]).toMatchObject({ id: 'folder-1', path: '/Docs' }) }) - it('rejects a missing parent without querying folder rows', async () => { - await expect( - listKnowledgeFolders.execute({ - principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, - input: { workspaceId: 'workspace-1', parentPath: '/Missing' }, - }) - ).rejects.toMatchObject({ code: 'not_found' }) + /** + * `parentPath` is a filter, so a path naming no active folder narrows the + * result to nothing rather than reporting the collection missing. Falling + * through to `listActiveFolderRows` with an undefined `parentId` would list + * every folder in the workspace, so the miss has to short-circuit. + */ + it('answers a parent path naming no folder with an empty page', async () => { + const result = await listKnowledgeFolders.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: 'workspace-1', parentPath: '/Missing' }, + }) + expect(result.folders).toEqual([]) expect(mocks.listRows).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/knowledge/application/folders.ts b/apps/sim/lib/knowledge/application/folders.ts index b12a59c7eec..9bed04a3bbb 100644 --- a/apps/sim/lib/knowledge/application/folders.ts +++ b/apps/sim/lib/knowledge/application/folders.ts @@ -14,6 +14,7 @@ import { ROOT_FOLDER_PATH } from '@/lib/folders/paths' import { listActiveFolderRows, loadActiveFolderPathIndex, + resolveFolderPathFilter, resolveFolderPathFromIndex, } from '@/lib/folders/queries' import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case' @@ -67,15 +68,10 @@ export const listKnowledgeFolders = defineAuthorizedKnowledgeUseCase({ undefined, { maxRows: MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE } ) - const parentId = - input.parentPath === undefined - ? undefined - : resolveFolderPathFromIndex(index, input.parentPath) - if (input.parentPath !== undefined && parentId === undefined) { - throw new OrchestrationError('not_found', 'Folder not found') - } + const parentFilter = resolveFolderPathFilter(index, input.parentPath) + if (parentFilter.kind === 'noMatch') return { folders: [] } const folders = await listActiveFolderRows(context.workspaceId, 'knowledge_base', { - parentId, + parentId: parentFilter.kind === 'folder' ? parentFilter.folderId : undefined, search: input.search, sortBy: input.sortBy, sortOrder: input.sortOrder, diff --git a/apps/sim/lib/table/application/folders.test.ts b/apps/sim/lib/table/application/folders.test.ts new file mode 100644 index 00000000000..30f0b60af42 --- /dev/null +++ b/apps/sim/lib/table/application/folders.test.ts @@ -0,0 +1,106 @@ +/** + * @vitest-environment node + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + listRows: vi.fn(), + loadFolderIndex: vi.fn(), + resolvePermission: vi.fn(), + resolveWorkspaceContext: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { + FOLDER_CREATED: 'folder.created', + FOLDER_DELETED: 'folder.deleted', + FOLDER_MOVED: 'folder.moved', + FOLDER_RESTORED: 'folder.restored', + }, + AuditResourceType: { FOLDER: 'folder' }, + recordAudit: vi.fn(), +})) + +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (actual: string | null, required: string) => { + const rank = { read: 1, write: 2, admin: 3 } as const + return ( + actual !== null && rank[actual as keyof typeof rank] >= rank[required as keyof typeof rank] + ) + }, + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) + +vi.mock('@/lib/folders/orchestration', () => ({ + createFolderAtPathTransition: vi.fn(), + deleteFolderByPathTransition: vi.fn(), + relocateFolderByPathTransition: vi.fn(), + restoreFolder: vi.fn(), +})) + +vi.mock('@/lib/folders/queries', () => ({ + findArchivedFolderIdByPath: vi.fn(), + listActiveFolderRows: mocks.listRows, + loadActiveFolderPathIndex: mocks.loadFolderIndex, + resolveFolderPathFilter: (index: { idByPath: Map }, path: string | undefined) => { + if (path === undefined) return { kind: 'unfiltered' } + if (path === '/') return { kind: 'folder', folderId: null } + const folderId = index.idByPath.get(path) + return folderId === undefined ? { kind: 'noMatch' } : { kind: 'folder', folderId } + }, +})) + +vi.mock('@/lib/table/application/context', () => ({ + resolveTableWorkspaceContext: mocks.resolveWorkspaceContext, +})) + +import { listTableFoldersUseCase } from '@/lib/table/application/folders' + +const principal = { kind: 'session', userId: 'user-1', sessionId: 'session-1' } as const + +describe('listTableFoldersUseCase', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.resolveWorkspaceContext.mockResolvedValue({ + workspaceId: 'ws-1', + billedAccountUserId: 'owner-1', + }) + mocks.resolvePermission.mockResolvedValue('admin') + mocks.loadFolderIndex.mockResolvedValue({ + idByPath: new Map([['/Reports', 'folder-1']]), + pathById: new Map([['folder-1', '/Reports']]), + rowById: new Map(), + }) + mocks.listRows.mockResolvedValue([]) + }) + + it('resolves a canonical parent path before listing', async () => { + await listTableFoldersUseCase.execute({ + principal, + input: { workspaceId: 'ws-1', parentPath: '/Reports' }, + }) + + expect(mocks.listRows).toHaveBeenCalledWith( + 'ws-1', + 'table', + expect.objectContaining({ parentId: 'folder-1' }) + ) + }) + + /** + * `parentPath` is a filter, so a path naming no active folder narrows the + * result to nothing rather than reporting the collection missing. Falling + * through to `listActiveFolderRows` with an undefined `parentId` would list + * every folder in the workspace, so the miss has to short-circuit. + */ + it('answers a parent path naming no folder with an empty page', async () => { + const result = await listTableFoldersUseCase.execute({ + principal, + input: { workspaceId: 'ws-1', parentPath: '/Missing' }, + }) + + expect(result.folders).toEqual([]) + expect(mocks.listRows).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/table/application/folders.ts b/apps/sim/lib/table/application/folders.ts index 5bff78498fd..0f7568171e8 100644 --- a/apps/sim/lib/table/application/folders.ts +++ b/apps/sim/lib/table/application/folders.ts @@ -14,7 +14,7 @@ import { findArchivedFolderIdByPath, listActiveFolderRows, loadActiveFolderPathIndex, - resolveFolderPathFromIndex, + resolveFolderPathFilter, } from '@/lib/folders/queries' import { defineAuthorizedTableUseCase } from '@/lib/table/application/authorized-table-use-case' import { resolveTableWorkspaceContext } from '@/lib/table/application/context' @@ -37,15 +37,10 @@ export const listTableFoldersUseCase = defineAuthorizedTableUseCase({ const index = await loadActiveFolderPathIndex(context.workspaceId, 'table', undefined, { maxRows: MAX_FOLDERS_PER_WORKSPACE, }) - const parentId = - input.parentPath === undefined - ? undefined - : resolveFolderPathFromIndex(index, input.parentPath) - if (input.parentPath !== undefined && parentId === undefined) { - throw new OrchestrationError('not_found', 'Folder not found') - } + const parentFilter = resolveFolderPathFilter(index, input.parentPath) + if (parentFilter.kind === 'noMatch') return { folders: [], index } const folders = await listActiveFolderRows(context.workspaceId, 'table', { - parentId, + parentId: parentFilter.kind === 'folder' ? parentFilter.folderId : undefined, search: input.search, sortBy: input.sortBy, sortOrder: input.sortOrder, diff --git a/apps/sim/lib/workflows/application/workflow-folders.test.ts b/apps/sim/lib/workflows/application/workflow-folders.test.ts index 9b7e173b7c7..d7a0d67d7d3 100644 --- a/apps/sim/lib/workflows/application/workflow-folders.test.ts +++ b/apps/sim/lib/workflows/application/workflow-folders.test.ts @@ -41,6 +41,12 @@ vi.mock('@/lib/folders/orchestration', () => ({ vi.mock('@/lib/folders/queries', () => ({ listActiveFolderRows: mocks.listRows, loadActiveFolderPathIndex: mocks.loadIndex, + resolveFolderPathFilter: (index: { idByPath: Map }, path: string | undefined) => { + if (path === undefined) return { kind: 'unfiltered' } + if (path === '/') return { kind: 'folder', folderId: null } + const folderId = index.idByPath.get(path) + return folderId === undefined ? { kind: 'noMatch' } : { kind: 'folder', folderId } + }, resolveFolderPathFromIndex: (index: { idByPath: Map }, path: string) => path === '/' ? null : index.idByPath.get(path), })) @@ -146,6 +152,37 @@ describe('workflow folder application operations', () => { ) }) + /** + * `parentPath` is a filter, so a path naming no active folder narrows the + * result to nothing rather than reporting the collection missing. Falling + * through to `listActiveFolderRows` with an undefined `parentId` would list + * every folder in the workspace, so the miss has to short-circuit. + */ + it('answers a parent path naming no folder with an empty page', async () => { + const result = await listWorkflowFolders.execute({ + principal: principals[0], + input: { workspaceId: 'ws-1', parentPath: '/Missing', sortBy: 'name', sortOrder: 'asc' }, + }) + + expect(result.folders).toEqual([]) + expect(mocks.listRows).not.toHaveBeenCalled() + }) + + it('resolves a canonical parent path before listing', async () => { + mocks.listRows.mockResolvedValueOnce([folder]) + + await listWorkflowFolders.execute({ + principal: principals[0], + input: { workspaceId: 'ws-1', parentPath: '/Reports', sortBy: 'name', sortOrder: 'asc' }, + }) + + expect(mocks.listRows).toHaveBeenCalledWith( + 'ws-1', + 'workflow', + expect.objectContaining({ parentId: folder.id }) + ) + }) + it('rejects a workspace key outside the canonical workspace before mutation', async () => { await expect( createWorkflowFolder.execute({ diff --git a/apps/sim/lib/workflows/application/workflow-folders.ts b/apps/sim/lib/workflows/application/workflow-folders.ts index 64eb5141be5..5c8403d56fa 100644 --- a/apps/sim/lib/workflows/application/workflow-folders.ts +++ b/apps/sim/lib/workflows/application/workflow-folders.ts @@ -15,6 +15,7 @@ import { ROOT_FOLDER_PATH } from '@/lib/folders/paths' import { listActiveFolderRows, loadActiveFolderPathIndex, + resolveFolderPathFilter, resolveFolderPathFromIndex, } from '@/lib/folders/queries' import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case' @@ -133,15 +134,10 @@ export const listWorkflowFolders = defineAuthorizedWorkflowUseCase({ const index = await loadActiveFolderPathIndex(context.workspaceId, 'workflow', undefined, { maxRows: MAX_FOLDERS_PER_WORKSPACE, }) - const parentId = - input.parentPath === undefined - ? undefined - : resolveFolderPathFromIndex(index, input.parentPath) - if (input.parentPath !== undefined && parentId === undefined) { - throw new OrchestrationError('not_found', 'Folder not found') - } + const parentFilter = resolveFolderPathFilter(index, input.parentPath) + if (parentFilter.kind === 'noMatch') return { folders: [], index } const folders = await listActiveFolderRows(context.workspaceId, 'workflow', { - parentId, + parentId: parentFilter.kind === 'folder' ? parentFilter.folderId : undefined, search: input.search, sortBy: input.sortBy, sortOrder: input.sortOrder, From b5eb3998f0477bd2769212f39f38305743eb49a9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:38 -0700 Subject: [PATCH 4/9] fix(cli): gate activating a deployed version `workflows activate create` switched which version production serves with no confirmation, while `rollback` refused without --yes. They are the same application operation under two transitions, so gating one and not the other was an accident of naming. The destructive-operation classification in the client tests listed activate as non-destructive, which is what kept its sweep from noticing. Moved, so two independent tests now hold the gate. --- packages/sim-cli/src/contract/commands.test.ts | 8 +++++--- packages/sim-cli/src/contract/commands.ts | 7 +++++++ packages/sim-cli/src/http/client.test.ts | 5 ++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/sim-cli/src/contract/commands.test.ts b/packages/sim-cli/src/contract/commands.test.ts index b2a61123eff..cc3c41b692d 100644 --- a/packages/sim-cli/src/contract/commands.test.ts +++ b/packages/sim-cli/src/contract/commands.test.ts @@ -322,10 +322,11 @@ describe('folder-path fields', () => { }) describe('confirm gates say what is actually at stake', () => { - it('gates the two workflow writes that change what production serves', () => { + it('gates the three workflow writes that change what production serves', () => { // `undeploy` takes the workflow offline for every consumer, its published - // MCP tools included. `rollback` changes which version is live, while the - // gated `revert` only overwrites the draft. + // MCP tools included. `rollback` and `activate` are the same application + // operation under two transitions and both change which version is live, + // while the gated `revert` only overwrites the draft. const undeploy = CLI_CONTRACT.undeployWorkflow?.confirm ?? '' expect(undeploy).toContain('offline') expect(undeploy).toMatch(/MCP/) @@ -336,6 +337,7 @@ describe('confirm gates say what is actually at stake', () => { expect(undeploy).toContain('until it is deployed again') expect(undeploy).not.toMatch(/does not restore|not recoverable|cannot be undone/) expect(CLI_CONTRACT.rollbackWorkflow?.confirm).toBeTruthy() + expect(CLI_CONTRACT.activateWorkflowVersion?.confirm).toBeTruthy() }) it('does not promise irreversible loss for a recoverable delete', () => { diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 46868afb00f..c12b9ed0b87 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -531,6 +531,13 @@ export const CLI_CONTRACT: CliContract = { confirm: 'This changes which deployed version runs in production for every API and chat consumer.', }, + // The same application operation as `rollback`, under a different transition: + // both switch production away from the version the caller last chose. Gating + // one and not the other was an accident of naming, not a policy. + activateWorkflowVersion: { + confirm: + 'This changes which deployed version runs in production for every API and chat consumer.', + }, // POST derives to `... create`, which creates nothing here. Named for the // operation instead, matching the shipped `files move`. moveWorkflows: { diff --git a/packages/sim-cli/src/http/client.test.ts b/packages/sim-cli/src/http/client.test.ts index 2c55e49866d..5a6ebd79aac 100644 --- a/packages/sim-cli/src/http/client.test.ts +++ b/packages/sim-cli/src/http/client.test.ts @@ -949,6 +949,10 @@ describe('destructive operations are gated', () => { * default by being named something the old regex did not match. */ const DESTRUCTIVE_NON_DELETE = new Set([ + // The same application operation as `rollbackWorkflow`, under a different + // transition: both switch which version production serves away from the one + // the caller last chose. + 'activateWorkflowVersion', 'applyWorkflowOperations', 'applyWorkflowVariables', 'bulkDeleteFiles', @@ -966,7 +970,6 @@ describe('destructive operations are gated', () => { * decision on anything new. */ const NON_DESTRUCTIVE = new Set([ - 'activateWorkflowVersion', 'addTableColumn', 'addWorkflowGroup', 'addWorkspaceFilesToKnowledgeBase', From 0a885f7423a8f73485f27a6a7c810c9f735461f3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:38 -0700 Subject: [PATCH 5/9] fix(cli): name the profile in the suggestion configure prints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refusing a root global printed a command to save it — without --profile, so following it verbatim wrote the default profile and left the named one untouched. The neighbouring suggestions in this file already carry the flag. Resolution matches resolveProfile, so SIM_PROFILE is covered too, and the profile name is redacted like the value beside it. --- .../sim-cli/src/commands/configure.test.ts | 49 +++++++++++++++++++ packages/sim-cli/src/commands/configure.ts | 8 ++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/sim-cli/src/commands/configure.test.ts b/packages/sim-cli/src/commands/configure.test.ts index 529d7baa2b5..8fc740bd87d 100644 --- a/packages/sim-cli/src/commands/configure.test.ts +++ b/packages/sim-cli/src/commands/configure.test.ts @@ -42,6 +42,10 @@ function run(...args: string[]): Promise { beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'sim-cli-')) process.env.SIM_CONFIG_DIR = dir + // The refusal reads SIM_PROFILE the way `resolveProfile` does, so an ambient + // one would otherwise decide what these assertions see. Empty rather than + // `undefined`: assigning to process.env stringifies, and "undefined" is truthy. + process.env.SIM_PROFILE = '' mocks.profileName = 'default' mocks.profileFrom.mockClear() mocks.profileFrom.mockImplementation(() => ({ name: mocks.profileName })) @@ -52,6 +56,7 @@ afterEach(() => { vi.restoreAllMocks() rmSync(dir, { recursive: true, force: true }) process.env.SIM_CONFIG_DIR = undefined + process.env.SIM_PROFILE = '' }) describe('configure --set-endpoint', () => { @@ -190,6 +195,50 @@ describe('configure and the root globals', () => { expect(readConfigProfile('default')).toEqual({}) }) + /** + * The suggested command is meant to be pasted verbatim, so omitting the + * selected profile made it write `default` and leave the profile the caller + * was targeting untouched — silently, and reported as a success. + */ + it('carries the selected profile into the command it suggests', async () => { + await expect(run('-P', 'dev', '--output', 'json')).rejects.toThrow( + 'sim configure --profile dev --set-output json' + ) + await expect(run('-P', 'dev', '--endpoint', 'https://other.example')).rejects.toThrow( + 'sim configure --profile dev --set-endpoint https://other.example' + ) + await expect(run('-P', 'dev', '-w', 'ws_9')).rejects.toThrow( + 'sim configure --profile dev --set-workspace ws_9' + ) + }) + + it('carries a SIM_PROFILE-selected profile into the command it suggests', async () => { + process.env.SIM_PROFILE = 'dev' + + await expect(run('--output', 'json')).rejects.toThrow( + 'sim configure --profile dev --set-output json' + ) + }) + + /** + * `resolveProfile` reads `overrides.profile || process.env.SIM_PROFILE`, so + * the suggestion has to name the profile the run would actually resolve to. + */ + it('lets an explicit --profile win over SIM_PROFILE, as resolveProfile does', async () => { + process.env.SIM_PROFILE = 'staging' + + await expect(run('-P', 'dev', '--output', 'json')).rejects.toThrow( + 'sim configure --profile dev --set-output json' + ) + }) + + /** The profile name is caller-supplied, so it is redacted like the value. */ + it('redacts a control character out of the profile it suggests', async () => { + await expect(run('-P', 'dev\u2028sim login', '--output', 'json')).rejects.toThrow( + 'sim configure --profile dev sim login --set-output json' + ) + }) + it('does not print a stale stored value as if it had been set', async () => { writeConfigProfile('default', { endpoint: 'https://staging.example' }) diff --git a/packages/sim-cli/src/commands/configure.ts b/packages/sim-cli/src/commands/configure.ts index de3c881a75b..4fa097b9811 100644 --- a/packages/sim-cli/src/commands/configure.ts +++ b/packages/sim-cli/src/commands/configure.ts @@ -20,6 +20,10 @@ import { SimApiError } from '../http/client' * it did not change, which reads exactly like a confirmation. Refusing and * naming the twin is the honest answer; making the global write instead would * give one command a persistent side effect the same flag has on no other. + * + * The suggested command has to carry `--profile` whenever one is selected: + * without it, a caller who follows the advice verbatim writes the `default` + * profile and leaves the one they were targeting untouched. */ const GLOBAL_FLAG_TWINS = [ { option: 'endpoint', flag: '--endpoint', setFlag: '--set-endpoint' }, @@ -66,11 +70,13 @@ export function configureCommand(): Command { command: Command ) => { const globals = globalsOf(command) + const selectedProfile = globals.profile || process.env.SIM_PROFILE + const profileArg = selectedProfile ? ` --profile ${redact(selectedProfile)}` : '' for (const { option, flag, setFlag } of GLOBAL_FLAG_TWINS) { const value = globals[option] if (value === undefined) continue throw new SimApiError( - `${flag} applies to a single command and is not stored. To save it, run: sim configure ${setFlag} ${redact(value)}`, + `${flag} applies to a single command and is not stored. To save it, run: sim configure${profileArg} ${setFlag} ${redact(value)}`, 0 ) } From e8ded4bddb4b944f5152c412acb3639618d05391 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:39 -0700 Subject: [PATCH 6/9] fix(cli): fail a row delete that matched nothing `tables rows batch-delete` exited 0 when none of the named rows existed, while the table equivalent exited 1 on the same shape. Only the id-list selection is checked: a filter answers without a requested count, so the guard self-excludes and an idempotent sweep still exits 0 on its second run. --- packages/sim-cli/src/runtime/execute.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/sim-cli/src/runtime/execute.ts b/packages/sim-cli/src/runtime/execute.ts index 79be86a713d..031293d8288 100644 --- a/packages/sim-cli/src/runtime/execute.ts +++ b/packages/sim-cli/src/runtime/execute.ts @@ -129,6 +129,19 @@ export const BULK_OUTCOME_CHECKS: Readonly { + if (countOf(payload.deletedCount) > 0) return null + const requested = countOf(payload.requestedCount) + if (requested === 0) return null + return `Deleted nothing: none of the ${requested} requested ${requested === 1 ? 'row was' : 'rows were'} deleted.` + }, moveTables: (payload) => { if (lengthOf(payload.moved) > 0) return null const missed = lengthOf(payload.notFound) + lengthOf(payload.failed) From 50853b2d1357502e76a93ffdce9977e52cc3f7a2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:39 -0700 Subject: [PATCH 7/9] fix(cli): show the -- escape for an id that opens with a dash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Short ids draw from a 64-character alphabet containing one dash, so 1 in 64 open with one and commander reads it as an unknown option. It reaches `audit-logs get` and the custom-tool commands, and the escape was documented nowhere. The hint is appended only for a lone dash followed by two or more characters carrying an uppercase letter or digit — a shape no flag on this surface has — so a misspelt flag keeps commander's own suggestion. --- packages/sim-cli/src/program.ts | 4 +- packages/sim-cli/src/runtime/build.test.ts | 46 ++++++++++++++++++++ packages/sim-cli/src/runtime/build.ts | 49 +++++++++++++++++++--- 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/packages/sim-cli/src/program.ts b/packages/sim-cli/src/program.ts index 999350d71cd..e2e784672d4 100644 --- a/packages/sim-cli/src/program.ts +++ b/packages/sim-cli/src/program.ts @@ -21,7 +21,9 @@ Profiles work like the AWS CLI: settings live in ~/.sim/config, keys in with -P, --profile, or SIM_PROFILE. Workflow, knowledge-base and workspace IDs are bare UUIDs. Table IDs carry a -tbl_ prefix and file IDs a wf_ one, so wf_ never names a workflow. +tbl_ prefix and file IDs a wf_ one, so wf_ never names a workflow. An audit-log +or custom-tool ID can open with a dash, which reads as a flag; put -- in front +of it, as in sim audit-logs get -- -HlDcD1z76nK6R4crsUp0. Examples: $ sim login Authorize the default profile diff --git a/packages/sim-cli/src/runtime/build.test.ts b/packages/sim-cli/src/runtime/build.test.ts index d0f0bbaf72d..59468426151 100644 --- a/packages/sim-cli/src/runtime/build.test.ts +++ b/packages/sim-cli/src/runtime/build.test.ts @@ -326,6 +326,52 @@ describe('commands parsed through commander', () => { expect(errorOutput).not.toContain('--skillId') }) + it('shows the -- escape when an id argument opens with a dash', async () => { + const root = program() + const auditLogs = root.commands.find((command) => command.name() === 'audit-logs') + const get = auditLogs?.commands.find((command) => command.name() === 'get') + if (!get) throw new Error('Missing command audit-logs get') + + let errorOutput = '' + get.configureOutput({ + writeErr: (message) => { + errorOutput += message + }, + }) + + await expect( + root.parseAsync(['node', 'sim', 'audit-logs', 'get', '-HlDcD1z76nK6R4crsUp0']) + ).rejects.toMatchObject({ code: 'commander.unknownOption' }) + expect(errorOutput).toContain("error: unknown option '-HlDcD1z76nK6R4crsUp0'") + expect(errorOutput).toContain('Example: sim audit-logs get -- -HlDcD1z76nK6R4crsUp0') + }) + + it("leaves a misspelt flag with commander's own suggestion", async () => { + const root = program() + const auditLogs = root.commands.find((command) => command.name() === 'audit-logs') + const get = auditLogs?.commands.find((command) => command.name() === 'get') + if (!get) throw new Error('Missing command audit-logs get') + + let errorOutput = '' + get.configureOutput({ + writeErr: (message) => { + errorOutput += message + }, + }) + + await expect( + root.parseAsync(['node', 'sim', 'audit-logs', 'get', 'log_1', '--organisation']) + ).rejects.toMatchObject({ code: 'commander.unknownOption' }) + expect(errorOutput).toContain("error: unknown option '--organisation'") + expect(errorOutput).not.toContain('Example:') + + await expect( + root.parseAsync(['node', 'sim', 'audit-logs', 'get', 'log_1', '-organisation']) + ).rejects.toMatchObject({ code: 'commander.unknownOption' }) + expect(errorOutput).toContain("error: unknown option '-organisation'") + expect(errorOutput).not.toContain('Example:') + }) + it('dispatches generated commands through their singular resource alias', async () => { const [tablePath] = await run(['table', 'list']) expect(tablePath).toBe('/api/v2/tables') diff --git a/packages/sim-cli/src/runtime/build.ts b/packages/sim-cli/src/runtime/build.ts index b387f6420a6..3d80174fbe6 100644 --- a/packages/sim-cli/src/runtime/build.ts +++ b/packages/sim-cli/src/runtime/build.ts @@ -64,18 +64,55 @@ function commandPath(command: Command): string { return names.join(' ') } -function addMissingArgumentExample(command: Command): Command { +const UNKNOWN_OPTION_TOKEN = /^error: unknown option '(.+?)'/ + +/** + * Whether an unknown-option token reads as a resource id rather than a flag. + * + * `generateShortId` draws from a 64-character alphabet holding exactly one + * `-`, so one id in 64 opens with a dash and commander parses it as an option + * instead of the positional it was typed as — `audit-logs get` and + * `custom-tools get/update/delete` all take such an id. A flag on this surface + * is either a single-character short (`-w`) or a lowercase kebab-case long + * (`--dry-run`), so a lone dash followed by two or more characters of which at + * least one is an uppercase letter or a digit is not a flag any caller meant + * to type. `--organisation` and every other misspelt flag keeps the plain + * error and commander's own suggestion. + */ +function looksLikeAnId(token: string): boolean { + return ( + token.length > 2 && + !token.startsWith('--') && + /^-[A-Za-z0-9_-]*[A-Z0-9][A-Za-z0-9_-]*$/.test(token) + ) +} + +/** + * Appends a worked example to the parse errors a positional argument causes. + * + * Covers the argument being absent and the argument being swallowed as an + * option because its id opens with a dash; the second needs the `--` escape, + * which commander never mentions. + */ +function addArgumentExamples(command: Command): Command { const outputError = command.configureOutput().outputError if (!outputError) throw new Error('Commander output formatter is not configured') command.configureOutput({ outputError: (message, write) => { outputError(message, write) - if (!message.startsWith('error: missing required argument ')) return - const syntax = argumentSyntax(command) - const example = syntax ? `${commandPath(command)} ${syntax}` : commandPath(command) - write(`Example: ${example}\n`) + if (message.startsWith('error: missing required argument ')) { + const syntax = argumentSyntax(command) + const example = syntax ? `${commandPath(command)} ${syntax}` : commandPath(command) + write(`Example: ${example}\n`) + return + } + + if (command.registeredArguments.length === 0) return + const token = UNKNOWN_OPTION_TOKEN.exec(message)?.[1] + if (!token || !looksLikeAnId(token)) return + write(`Example: ${commandPath(command)} -- ${token}\n`) }, }) return command @@ -298,7 +335,7 @@ function configureOperation( } function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: string): Command { - return addMissingArgumentExample(configureOperation(new Command(leafName), operation, spec)) + return addArgumentExamples(configureOperation(new Command(leafName), operation, spec)) } /** From 85d4308d99feff51791572cff3840ba63a85fd82 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 14:45:39 -0700 Subject: [PATCH 8/9] chore: regenerate the API reference and CLI surface --- apps/docs/content/docs/en/cli/reference.mdx | 12 +++++++++++- apps/docs/content/docs/en/cli/workflows.mdx | 12 +++++++++++- apps/docs/openapi-v2-files-audit.json | 4 ++-- apps/docs/openapi-v2-knowledge.json | 4 ++-- apps/docs/openapi-v2-tables.json | 4 ++-- apps/docs/openapi-v2-workflows.json | 4 ++-- packages/sim-cli/src/generated/v2-api.ts | 12 ++++++++---- 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/apps/docs/content/docs/en/cli/reference.mdx b/apps/docs/content/docs/en/cli/reference.mdx index f90021ebee2..9bb1d4cf8af 100644 --- a/apps/docs/content/docs/en/cli/reference.mdx +++ b/apps/docs/content/docs/en/cli/reference.mdx @@ -4615,7 +4615,7 @@ Also spelled `sim workflow`. Activate Workflow Version (personal API key required) ```bash -sim workflows activate create +sim workflows activate create [options] ``` **Arguments** @@ -4629,6 +4629,16 @@ sim workflows activate create +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `-y, --yes` | Yes | Confirm this operation. | + + + ### sim workflows operations apply Apply Workflow Operations (personal API key required) diff --git a/apps/docs/content/docs/en/cli/workflows.mdx b/apps/docs/content/docs/en/cli/workflows.mdx index 248b9d18776..18c783a1c31 100644 --- a/apps/docs/content/docs/en/cli/workflows.mdx +++ b/apps/docs/content/docs/en/cli/workflows.mdx @@ -12,7 +12,7 @@ Every command below also accepts the [global options](/cli/commands#global-optio ## Activate workflow version ```bash -sim workflows activate create +sim workflows activate create [options] ``` Activate Workflow Version (personal API key required) @@ -28,6 +28,16 @@ Activate Workflow Version (personal API key required) +**Options** + + + +| Option | Required | Description | +| --- | --- | --- | +| `-y, --yes` | Yes | Confirm this operation. | + + + ## Apply workflow operations ```bash diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index 88dd604ab00..696ff4e3623 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -2098,9 +2098,9 @@ "name": "parentPath", "in": "query", "required": false, - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "schema": { - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "$ref": "#/components/schemas/FolderPathInput" } }, diff --git a/apps/docs/openapi-v2-knowledge.json b/apps/docs/openapi-v2-knowledge.json index d433c0ea73b..38eecd23994 100644 --- a/apps/docs/openapi-v2-knowledge.json +++ b/apps/docs/openapi-v2-knowledge.json @@ -2718,9 +2718,9 @@ "name": "parentPath", "in": "query", "required": false, - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "schema": { - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "$ref": "#/components/schemas/FolderPathInput" } }, diff --git a/apps/docs/openapi-v2-tables.json b/apps/docs/openapi-v2-tables.json index 25250fc7714..359c8e1d4b9 100644 --- a/apps/docs/openapi-v2-tables.json +++ b/apps/docs/openapi-v2-tables.json @@ -3726,9 +3726,9 @@ "name": "parentPath", "in": "query", "required": false, - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "schema": { - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "$ref": "#/components/schemas/FolderPathInput" } }, diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index b80dda73fab..ca61a2adaca 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -3206,9 +3206,9 @@ "name": "parentPath", "in": "query", "required": false, - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "schema": { - "description": "Restrict results to direct children of this parent path.", + "description": "Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.", "$ref": "#/components/schemas/FolderPathInput" } }, diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index d623c667e6f..e4a2f179863 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -11702,7 +11702,8 @@ export const V2_OPERATIONS = { }, parentPath: { kind: 'string', - describe: 'Restrict results to direct children of this parent path.', + describe: + 'Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.', }, search: { kind: 'string', @@ -12058,7 +12059,8 @@ export const V2_OPERATIONS = { }, parentPath: { kind: 'string', - describe: 'Restrict results to direct children of this parent path.', + describe: + 'Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.', }, search: { kind: 'string', @@ -12444,7 +12446,8 @@ export const V2_OPERATIONS = { }, parentPath: { kind: 'string', - describe: 'Restrict results to direct children of this parent path.', + describe: + 'Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.', }, search: { kind: 'string', @@ -12623,7 +12626,8 @@ export const V2_OPERATIONS = { }, parentPath: { kind: 'string', - describe: 'Restrict results to direct children of this parent path.', + describe: + 'Restrict results to direct children of this parent path. A path that names no folder narrows the result to nothing, so the response is an empty page rather than an error.', }, search: { kind: 'string', From 5b9b3fa3727f30f6fd18613945f7e8b747c80a75 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 15:01:06 -0700 Subject: [PATCH 9/9] fix(cli): quote a profile name a pasted command would otherwise split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suggestion configure prints is meant to be pasted, and it interpolated the profile name bare. Profile-name validation is creation-only by design — the validator says so, because a hand-written `[profile my stack]` has to keep resolving — so a name carrying whitespace, or a `;` that would end the pasted command and start another, reaches this message unchecked. Names that already satisfy the creation rule stay bare; the rest are single quoted, embedded quotes included. Redaction runs first, so a control character becomes a space and is then quoted rather than splitting the command. --- .../sim-cli/src/commands/configure.test.ts | 32 +++++++++++++++++-- packages/sim-cli/src/commands/configure.ts | 26 +++++++++++++-- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/sim-cli/src/commands/configure.test.ts b/packages/sim-cli/src/commands/configure.test.ts index 8fc740bd87d..05bb5187098 100644 --- a/packages/sim-cli/src/commands/configure.test.ts +++ b/packages/sim-cli/src/commands/configure.test.ts @@ -232,10 +232,38 @@ describe('configure and the root globals', () => { ) }) - /** The profile name is caller-supplied, so it is redacted like the value. */ + /** + * The profile name is caller-supplied, so it is redacted like the value — + * and redaction turns the separator into a space, which the suggestion then + * has to quote to stay one argument. + */ it('redacts a control character out of the profile it suggests', async () => { await expect(run('-P', 'dev\u2028sim login', '--output', 'json')).rejects.toThrow( - 'sim configure --profile dev sim login --set-output json' + "sim configure --profile 'dev sim login' --set-output json" + ) + }) + + /** + * Profile-name validation is creation-only by design, so a hand-written + * `[profile my stack]` keeps resolving and reaches this suggestion. Unquoted, + * a name carrying a `;` would end the pasted command and start another. + */ + it('quotes a profile name a pasted command would otherwise split', async () => { + await expect(run('-P', 'my stack', '--output', 'json')).rejects.toThrow( + "sim configure --profile 'my stack' --set-output json" + ) + await expect(run('-P', 'a;rm -rf x', '--output', 'json')).rejects.toThrow( + "sim configure --profile 'a;rm -rf x' --set-output json" + ) + await expect(run('-P', "it's mine", '--output', 'json')).rejects.toThrow( + "sim configure --profile 'it'\\''s mine' --set-output json" + ) + }) + + /** A name that already satisfies the creation rule needs no quoting noise. */ + it('leaves an ordinary profile name bare', async () => { + await expect(run('-P', 'dev.2_a-b', '--output', 'json')).rejects.toThrow( + 'sim configure --profile dev.2_a-b --set-output json' ) }) diff --git a/packages/sim-cli/src/commands/configure.ts b/packages/sim-cli/src/commands/configure.ts index 4fa097b9811..5e1f0c0a999 100644 --- a/packages/sim-cli/src/commands/configure.ts +++ b/packages/sim-cli/src/commands/configure.ts @@ -7,7 +7,12 @@ import { resolveAuthenticationProfileName, writeConfigProfile, } from '../config/index' -import { normalizeEndpoint, normalizeWorkspaceId, redact } from '../config/profile' +import { + normalizeEndpoint, + normalizeWorkspaceId, + PROFILE_NAME_PATTERN, + redact, +} from '../config/profile' import { globalsOf, profileFrom } from '../context' import { SimApiError } from '../http/client' @@ -52,6 +57,21 @@ function requireValue(value: string | undefined, flag: string, key: string): voi * they arrive through `sim login`, which is the only path that mints a key with * a recorded consent behind it. */ +/** + * Quotes a profile name that a pasted command would otherwise split. + * + * Profile-name validation is creation-only by design, so a hand-written + * `[profile my stack]` keeps resolving — and reaches this suggestion carrying + * whitespace, or a `;` that would end the pasted command and start another. + * Names that already match the creation rule are left bare, since quoting every + * one of them would only add noise to the common case. + */ +function quoteProfileArgument(name: string): string { + const redacted = redact(name) + if (PROFILE_NAME_PATTERN.test(redacted)) return redacted + return `'${redacted.replaceAll("'", "'\\''")}'` +} + export function configureCommand(): Command { return new Command('configure') .description("Set a profile's endpoint, default workspace, or output format") @@ -71,7 +91,9 @@ export function configureCommand(): Command { ) => { const globals = globalsOf(command) const selectedProfile = globals.profile || process.env.SIM_PROFILE - const profileArg = selectedProfile ? ` --profile ${redact(selectedProfile)}` : '' + const profileArg = selectedProfile + ? ` --profile ${quoteProfileArgument(selectedProfile)}` + : '' for (const { option, flag, setFlag } of GLOBAL_FLAG_TWINS) { const value = globals[option] if (value === undefined) continue