fix(cli): seven defects found by live-testing the CLI against staging (#7176)

* fix(catalog): resolve an unversioned tool id against the visible set

`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.

* fix(files): resolve an archived folder path through its active ancestors

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.

* fix(v2): answer a folder-list miss with an empty page

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.

* 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.

* fix(cli): name the profile in the suggestion configure prints

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.

* 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.

* fix(cli): show the -- escape for an id that opens with a dash

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.

* chore: regenerate the API reference and CLI surface

* fix(cli): quote a profile name a pasted command would otherwise split

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.
This commit is contained in:
Waleed
2026-08-27 15:14:48 -07:00
committed by GitHub
parent b6a235cc9e
commit 479ae8c138
32 changed files with 687 additions and 81 deletions
+11 -1
View File
@@ -4615,7 +4615,7 @@ Also spelled `sim workflow`.
Activate Workflow Version (personal API key required)
```bash
sim workflows activate create <workflowId> <version>
sim workflows activate create <workflowId> <version> [options]
```
**Arguments**
@@ -4629,6 +4629,16 @@ sim workflows activate create <workflowId> <version>
</CommandTable>
**Options**
<CommandTable>
| Option | Required | Description |
| --- | --- | --- |
| `-y, --yes` | Yes | Confirm this operation. |
</CommandTable>
### sim workflows operations apply
Apply Workflow Operations (personal API key required)
+11 -1
View File
@@ -12,7 +12,7 @@ Every command below also accepts the [global options](/cli/commands#global-optio
## Activate workflow version
```bash
sim workflows activate create <workflowId> <version>
sim workflows activate create <workflowId> <version> [options]
```
Activate Workflow Version (personal API key required)
@@ -28,6 +28,16 @@ Activate Workflow Version (personal API key required)
</CommandTable>
**Options**
<CommandTable>
| Option | Required | Description |
| --- | --- | --- |
| `-y, --yes` | Yes | Confirm this operation. |
</CommandTable>
## Apply workflow operations
```bash
+2 -2
View File
@@ -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"
}
},
+2 -2
View File
@@ -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"
}
},
+2 -2
View File
@@ -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"
}
},
+2 -2
View File
@@ -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"
}
},
+7 -5
View File
@@ -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' }),
})
@@ -107,6 +107,22 @@ const TOOL_METADATA: Record<string, Record<string, unknown>> = {
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,
+13 -10
View File
@@ -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<GetCatalogToolResult> => {
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 }
},
})
@@ -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>): 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.
*
+2 -2
View File
@@ -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
@@ -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<string, string> }, 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<string, string> }, 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()
})
@@ -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,
@@ -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<string, string> }, 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()
})
})
+4 -9
View File
@@ -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,
@@ -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()
@@ -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<Array<Pick<RawWorkspaceFileFolder, 'id' | 'name' | 'parentId'>>> {
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))
}
@@ -41,6 +41,12 @@ vi.mock('@/lib/folders/orchestration', () => ({
vi.mock('@/lib/folders/queries', () => ({
listActiveFolderRows: mocks.listRows,
loadActiveFolderPathIndex: mocks.loadIndex,
resolveFolderPathFilter: (index: { idByPath: Map<string, string> }, 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<string, string> }, 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({
@@ -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,
@@ -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({
@@ -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: {
@@ -42,6 +42,10 @@ function run(...args: string[]): Promise<Command> {
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,78 @@ 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 —
* 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"
)
})
/**
* 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'
)
})
it('does not print a stale stored value as if it had been set', async () => {
writeConfigProfile('default', { endpoint: 'https://staging.example' })
+30 -2
View File
@@ -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'
@@ -20,6 +25,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' },
@@ -48,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")
@@ -66,11 +90,15 @@ export function configureCommand(): Command {
command: Command
) => {
const globals = globalsOf(command)
const selectedProfile = globals.profile || process.env.SIM_PROFILE
const profileArg = selectedProfile
? ` --profile ${quoteProfileArgument(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
)
}
@@ -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', () => {
@@ -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: {
+8 -4
View File
@@ -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',
+4 -1
View File
@@ -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<V2OperationName>([
// 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<V2OperationName>([
'activateWorkflowVersion',
'addTableColumn',
'addWorkflowGroup',
'addWorkspaceFilesToKnowledgeBase',
+3 -1
View File
@@ -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
@@ -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')
+43 -6
View File
@@ -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))
}
/**
@@ -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<string, unknown>) {
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 = {
+13
View File
@@ -129,6 +129,19 @@ export const BULK_OUTCOME_CHECKS: Readonly<Partial<Record<V2OperationName, BulkO
? safeOneLine(reported)
: `Updated nothing: none of the ${requested} requested ${requested === 1 ? 'chunk' : 'chunks'} matched.`
},
/**
* Only the id-list selection is checked. The filter branch answers with a
* deleted count alone — no `requestedCount` — so the guard below self-excludes
* on it, which is right: a filter matching nothing deleted nothing because
* there was nothing to delete, and failing there would break the second run of
* an otherwise idempotent sweep.
*/
deleteTableRows: (payload) => {
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)