refactor(cli): share the path predicate and other-spellings list

Author: waleedlatif1

apps/sim/lib/workflows/editing/validation.ts

@@ -614,8 +614,7 @@ export function validateValueForSubBlockType(
     }
 
     case 'file-upload': {
-      // A multi-file field stores the editor's list of file objects; removing its last file
-      // stores null, so an empty list clears it the same way.
+      // Removing the editor's last file stores null, so an empty list clears the field too.
       if (subBlockConfig.multiple && Array.isArray(value)) {
         if (value.length === 0) return { valid: true, value: null }
         const index = value.findIndex((file) => !isStoredFile(file))
@@ -639,9 +638,7 @@ export function validateValueForSubBlockType(
             blockType,
             field: fieldName,
             value,
-            error: subBlockConfig.multiple
-              ? `Invalid file-upload value for field "${fieldName}" - expected a list of objects with "name" and "path" properties, or null`
-              : `Invalid file-upload value for field "${fieldName}" - expected one object with "name" and "path" properties, or null`,
+            error: `Invalid file-upload value for field "${fieldName}" - expected ${subBlockConfig.multiple ? 'a list of objects' : 'one object'} with "name" and "path" properties, or null`,
           },
         }
       }

apps/sim/lib/workspace-files/application/archive-workspace-file-items.ts

@@ -7,9 +7,11 @@ import {
   bulkArchiveWorkspaceFileItems,
   loadWorkspaceFileOperationContext,
 } from '@/lib/uploads/contexts/workspace'
-import { isWorkspaceFileVfsPath } from '@/lib/vfs/path'
 import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case'
-import { resolveWorkspaceFileVfsPath } from '@/lib/workspace-files/application/file-context'
+import {
+  namesWorkspaceFileVfsPath,
+  resolveWorkspaceFileVfsPath,
+} from '@/lib/workspace-files/application/file-context'
 import { fileOperations } from '@/lib/workspace-files/application/operations'
 import {
   MAX_WORKSPACE_FILE_BULK_REQUEST_IDS,
@@ -41,10 +43,6 @@ function normalizeSelection(input: ArchiveWorkspaceFileItemsInput) {
   }
 }
 
-function namesVfsPath(input: ArchiveWorkspaceFileItemsInput, reference: string): boolean {
-  return input.acceptVfsPath === true && isWorkspaceFileVfsPath(reference)
-}
-
 /**
  * Turns each path among the selected files into its file's id, all before anything is archived,
  * so one path that names no single live file refuses the whole request. Runs only after the
@@ -53,19 +51,19 @@ function namesVfsPath(input: ArchiveWorkspaceFileItemsInput, reference: string):
  */
 async function resolveFileReferences(
   workspaceId: string,
-  input: ArchiveWorkspaceFileItemsInput,
+  acceptVfsPath: boolean | undefined,
   references: string[]
 ): Promise<string[]> {
-  const paths = references.filter((reference) => namesVfsPath(input, reference))
-  if (paths.length > MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS) {
+  const isPath = references.map((reference) => namesWorkspaceFileVfsPath(acceptVfsPath, reference))
+  if (isPath.filter(Boolean).length > MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS) {
     throw new OrchestrationError(
       'validation',
       `Bulk file operations resolve at most ${MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS} file paths per request; pass file ids for the rest`
     )
   }
   const resolved: string[] = []
-  for (const reference of references) {
-    if (!namesVfsPath(input, reference)) {
+  for (const [index, reference] of references.entries()) {
+    if (!isPath[index]) {
       resolved.push(reference)
       continue
     }
@@ -102,7 +100,7 @@ async function executeArchiveWorkspaceFileItems({
       `Bulk file operations accept at most ${MAX_WORKSPACE_FILE_BULK_REQUEST_IDS} file and folder IDs`
     )
   }
-  const fileIds = await resolveFileReferences(context.workspaceId, input, references)
+  const fileIds = await resolveFileReferences(context.workspaceId, input.acceptVfsPath, references)
 
   await assertWorkspaceFileItemsBelongToWorkspace({
     workspaceId: context.workspaceId,
@@ -134,9 +132,13 @@ async function resolveArchiveContext({ input }: { input: ArchiveWorkspaceFileIte
   const context = await loadWorkspaceFileOperationContext(input.workspaceId)
   if (!context) throw new OrchestrationError('not_found', 'Workspace not found')
   const { fileIds, folderIds } = normalizeSelection(input)
-  const sole = fileIds.length === 1 && folderIds.length === 0 ? fileIds[0] : undefined
-  // A path is not yet an id here, so it can never match a delegation scoped to one file.
-  return { ...context, fileId: sole === undefined || namesVfsPath(input, sole) ? undefined : sole }
+  const [sole] = fileIds
+  const namesOneFileById =
+    sole !== undefined &&
+    fileIds.length === 1 &&
+    folderIds.length === 0 &&
+    !namesWorkspaceFileVfsPath(input.acceptVfsPath, sole)
+  return { ...context, fileId: namesOneFileById ? sole : undefined }
 }
 
 export const archiveWorkspaceFileItemsOperation = defineAuthorizedWorkspaceFileUseCase({

apps/sim/lib/workspace-files/application/file-context.ts

@@ -87,8 +87,16 @@ function requireWorkspaceFileContext<C extends ActiveWorkspaceFileContext>(
   return canonical
 }
 
+/** Whether a caller that may have opted in with `acceptVfsPath` named a file by its VFS path. */
+export function namesWorkspaceFileVfsPath(
+  acceptVfsPath: boolean | undefined,
+  reference: string
+): boolean {
+  return acceptVfsPath === true && isWorkspaceFileVfsPath(reference)
+}
+
 function namesVfsPath(input: WorkspaceFileTargetInput): boolean {
-  return input.acceptVfsPath === true && isWorkspaceFileVfsPath(input.fileId)
+  return namesWorkspaceFileVfsPath(input.acceptVfsPath, input.fileId)
 }
 
 /**

apps/sim/lib/workspace-files/render-document-for-vision.ts

@@ -15,15 +15,17 @@ const pageResultSchema = z.object({
 
 function requestedPages(pages?: string): { first: number; last: number } | undefined {
   if (pages === undefined) return undefined
-  const match = /^(\d+)(?:-(\d+))?$/.exec(pages.trim())
-  if (!match && /^\d+(?:-\d+)?(?:\s*,\s*\d+(?:-\d+)?)+$/.test(pages.trim())) {
-    throw new OrchestrationError(
-      'validation',
-      `"${pages.trim()}" lists several pages; pass one page or one contiguous range per read, such as 5 or 2-5, and read the others separately`
-    )
-  }
-  if (!match)
+  const trimmed = pages.trim()
+  const match = /^(\d+)(?:-(\d+))?$/.exec(trimmed)
+  if (!match) {
+    if (/^\d+(?:-\d+)?(?:\s*,\s*\d+(?:-\d+)?)+$/.test(trimmed)) {
+      throw new OrchestrationError(
+        'validation',
+        `"${trimmed}" lists several pages; pass one page or one contiguous range per read, such as 5 or 2-5, and read the others separately`
+      )
+    }
     throw new OrchestrationError('validation', 'Pages must be a page number or range, such as 2-5')
+  }
   const first = Number(match[1])
   const last = Number(match[2] ?? match[1])
   if (!Number.isSafeInteger(first) || !Number.isSafeInteger(last) || first < 1 || last < first) {

packages/sim-cli/src/contract/types.ts

@@ -47,19 +47,14 @@ export interface FlagSpec {
    * Other names this flag answers to because callers reach for them: the request field's own
    * name behind a shortened flag (`--row-ids` for `--row`), or the noun a sibling command uses
    * (`--ids`, `--paths` for `--file-ids`). Only for a spelling that can mean nothing but this
-   * field on this command.
-   *
-   * Hidden from help and the generated docs like a {@link renamedFrom} spelling, but silent,
-   * since nothing was renamed. Combining one with the documented spelling is refused.
+   * field on this command. Hidden like a {@link renamedFrom} spelling, but silent.
    */
   hiddenAliases?: readonly string[]
   /**
    * The list holds workspace file ids or `files/…` paths, so an inline value written as a JSON
    * array (`["wf_a","wf_b"]`) or joined with commas (`wf_a,wf_b`) is read as its entries.
-   *
-   * Unambiguous only because of what a file reference can be: an id is `[A-Za-z0-9_-]` and a
-   * path starts with `files/`, so neither starts with `[`. A comma splits only a value with no
-   * `/`, which no id contains and no path lacks; a path keeps a comma in a file name.
+   * Unambiguous because an id is `[A-Za-z0-9_-]` and a path starts with `files/`: neither starts
+   * with `[`, and a comma splits only a value with no `/`, so a path keeps a comma in its name.
    */
   fileReferences?: true
   /**

packages/sim-cli/src/runtime/execute.ts

@@ -21,6 +21,7 @@ import {
   cursorSlot,
   flagNameFor,
   isProfileWorkspacePath,
+  otherSpellings,
   PROFILE_INJECTED_FIELD,
   trailingListField,
 } from './request'
@@ -377,11 +378,7 @@ function foldOtherSpellings(
 ): void {
   for (const [field, flag] of Object.entries(commandSpec.flags ?? {})) {
     const current = flagNameFor(operation, field)
-    const spellings = [
-      ...(flag.renamedFrom ?? []).map((name) => ({ name, renamed: true })),
-      ...(flag.hiddenAliases ?? []).map((name) => ({ name, renamed: false })),
-    ]
-    for (const { name, renamed } of spellings) {
+    for (const { name, renamed } of otherSpellings(flag)) {
       const supplied = flags[camel(name)]
       if (supplied === undefined) continue
       if (flags[camel(current)] !== undefined) {

packages/sim-cli/src/runtime/options.ts

@@ -1,11 +1,12 @@
 import { type Command, Option } from 'commander'
-import type { CommandSpec, FlagSpec } from '../contract/types'
+import type { CommandSpec } from '../contract/types'
 import type { V2OperationName } from '../generated/v2-api'
 import {
   cursorSlot,
   type FieldSpec,
   flagNameFor,
   flagSpecFor,
+  otherSpellings,
   PROFILE_INJECTED_FIELD,
   pathFlagNameFor,
   takesJson,
@@ -122,11 +123,6 @@ function withoutWireVocabulary(documented: string): string {
 const NON_PAGINATED_LIMIT_HINT =
   ' (caps a --filter match only; omit it to act on every match, and note 0 is not accepted)'
 
-/** The undocumented spellings a flag also answers to: former names, then hidden aliases. */
-function hiddenSpellings(flag: FlagSpec): readonly string[] {
-  return [...(flag.renamedFrom ?? []), ...(flag.hiddenAliases ?? [])]
-}
-
 function addFieldOption(
   command: Command,
   operation: V2OperationName,
@@ -201,7 +197,7 @@ function addFieldOption(
     if (!flag.boolean || flag.negatable) {
       command.option(`--no-${name}`, `Send --${name} as false`)
     }
-    for (const previous of hiddenSpellings(flag)) {
+    for (const { name: previous } of otherSpellings(flag)) {
       command.addOption(new Option(`--${previous}`).hideHelp())
       if (!flag.boolean || flag.negatable)
         command.addOption(new Option(`--no-${previous}`).hideHelp())
@@ -246,7 +242,7 @@ function addFieldOption(
       : ''
   }${literalNull ? literalNullHint(documented, name) : ''}`
 
-  const otherSpellings = hiddenSpellings(flag)
+  const hiddenNames = otherSpellings(flag).map(({ name }) => name)
   const option = new Option(`${short}--${name} ${placeholder}`, describe)
   if (flag.hidden) option.hideHelp()
   if (choices && !takesList) option.choices([...choices])
@@ -257,12 +253,12 @@ function addFieldOption(
   // spelling or trailing arguments onto the flag, so a required field supplied
   // either way would be rejected. The requirement is not lost: `buildRequest`
   // raises it once every spelling has had its chance.
-  if (descriptor.required && otherSpellings.length === 0 && !flag.trailing) {
+  if (descriptor.required && hiddenNames.length === 0 && !flag.trailing) {
     option.makeOptionMandatory()
   }
   command.addOption(option)
 
-  for (const previous of otherSpellings) {
+  for (const previous of hiddenNames) {
     const hidden = new Option(`--${previous} ${placeholder}`).hideHelp()
     if (choices && !takesList) hidden.choices([...choices])
     command.addOption(hidden)

packages/sim-cli/src/runtime/request.ts

@@ -61,6 +61,14 @@ export function flagSpecFor(operation: V2OperationName, field: string): FlagSpec
   return CLI_CONTRACT[operation]?.flags?.[field] ?? {}
 }
 
+/** The undocumented spellings a flag also answers to: former names, then hidden aliases. */
+export function otherSpellings(flag: FlagSpec): { name: string; renamed: boolean }[] {
+  return [
+    ...(flag.renamedFrom ?? []).map((name) => ({ name, renamed: true })),
+    ...(flag.hiddenAliases ?? []).map((name) => ({ name, renamed: false })),
+  ]
+}
+
 /** The list field whose values may also follow the command as arguments, if any. */
 export function trailingListField(commandSpec: CommandSpec): string | undefined {
   return Object.entries(commandSpec.flags ?? {}).find(([, flag]) => flag.trailing)?.[0]
@@ -278,25 +286,23 @@ export async function readArgumentSource(
  * misread a single id or path.
  */
 function fileReferenceEntries(value: string): string[] {
-  if (value.startsWith('[')) {
-    let parsed: unknown
-    try {
-      parsed = JSON.parse(value)
-    } catch {
-      return [value]
-    }
-    if (Array.isArray(parsed) && parsed.every((entry) => typeof entry === 'string')) {
-      return parsed.map((entry) => entry.trim()).filter(Boolean)
-    }
-    return [value]
-  }
-  if (value.includes(',') && !value.includes('/')) {
-    return value
-      .split(',')
-      .map((entry) => entry.trim())
-      .filter(Boolean)
+  const entries = value.startsWith('[')
+    ? jsonStringArray(value)
+    : value.includes(',') && !value.includes('/')
+      ? value.split(',')
+      : undefined
+  return entries ? entries.map((entry) => entry.trim()).filter(Boolean) : [value]
+}
+
+function jsonStringArray(value: string): string[] | undefined {
+  try {
+    const parsed: unknown = JSON.parse(value)
+    return Array.isArray(parsed) && parsed.every((entry) => typeof entry === 'string')
+      ? parsed
+      : undefined
+  } catch {
+    return undefined
   }
-  return [value]
 }
 
 /** A manifest line that carries no value: blank, or a `#` comment. */