From 87ed366b5f5bd39e522eb65e5ac818d9baaf8c7f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:08:33 -0700 Subject: [PATCH 1/7] refactor(workflows): one owner for new-workflow sort order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same ~35-line query — parent condition for workflows and folders, two parallel min(sortOrder) reads, fold to a min, subtract one, fall back to 0 — existed three times: lib/workflows/utils.ts inline in createWorkflowRecord lib/workflows/orchestration/... as a file-private nextWorkflowSortOrder lib/workflows/persistence/duplicate inline, inside the duplicate transaction The first two are character-identical modulo the table alias. The third had drifted: it omits isNull(workflow.archivedAt), which the other two apply, so a folder whose lowest-sortOrder workflow is soft-deleted positioned a *duplicate* differently from a *create*. The folder-side query agrees in all three, which marks it as a copy-paste slip rather than intent. Promotes the helper to lib/workflows/sort-order.ts, taking an optional DbOrTx so the duplicate path can keep reading inside its transaction. Its own module rather than utils.ts because duplicate.test.ts and workflow-lifecycle.test.ts both mock '@/lib/workflows/utils' wholesale — from a separate module the real query still runs under those suites, so their existing sort-order assertions keep their meaning and needed no edits. Note the archived-row behavior itself is not unit-testable here: the shared dbChainMock does not evaluate WHERE predicates. The guarantee is structural — one query builder instead of three means the predicate can no longer drift. --- .../orchestration/workflow-lifecycle.ts | 48 +-------------- .../lib/workflows/persistence/duplicate.ts | 35 +---------- apps/sim/lib/workflows/sort-order.ts | 60 +++++++++++++++++++ apps/sim/lib/workflows/utils.ts | 43 +------------ 4 files changed, 68 insertions(+), 118 deletions(-) create mode 100644 apps/sim/lib/workflows/sort-order.ts diff --git a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts index f482adee979..7c5414f63ae 100644 --- a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts @@ -5,13 +5,14 @@ import { createLogger } from '@sim/logger' import { isFolderInWorkspace } from '@sim/platform-authz/workflow' import { getPostgresConstraintName, getPostgresErrorCode, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, eq, isNull, min, ne } from 'drizzle-orm' +import { and, eq, isNull, ne } from 'drizzle-orm' import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import type { DbOrTx } from '@/lib/db/types' import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults' import { archiveWorkflow, restoreWorkflow } from '@/lib/workflows/lifecycle' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' +import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order' import { deduplicateWorkflowName } from '@/lib/workflows/utils' const logger = createLogger('WorkflowLifecycle') @@ -126,51 +127,6 @@ export interface PerformRestoreWorkflowResult { workflow?: Awaited>['workflow'] } -async function nextWorkflowSortOrder( - workspaceId: string, - folderId: string | null | undefined -): Promise { - const workflowParentCondition = folderId - ? eq(workflow.folderId, folderId) - : isNull(workflow.folderId) - const folderParentCondition = folderId - ? eq(folderTable.parentId, folderId) - : isNull(folderTable.parentId) - - const [[workflowMinResult], [folderMinResult]] = await Promise.all([ - db - .select({ minOrder: min(workflow.sortOrder) }) - .from(workflow) - .where( - and( - eq(workflow.workspaceId, workspaceId), - workflowParentCondition, - isNull(workflow.archivedAt) - ) - ), - db - .select({ minOrder: min(folderTable.sortOrder) }) - .from(folderTable) - .where( - and( - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - folderParentCondition - ) - ), - ]) - - const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce< - number | null - >((currentMin, candidate) => { - if (candidate == null) return currentMin - if (currentMin == null) return candidate - return Math.min(currentMin, candidate) - }, null) - - return minSortOrder != null ? minSortOrder - 1 : 0 -} - async function workflowNameExistsInFolder(params: { workspaceId: string name: string diff --git a/apps/sim/lib/workflows/persistence/duplicate.ts b/apps/sim/lib/workflows/persistence/duplicate.ts index 31a7c342e56..3e74cc5ba2e 100644 --- a/apps/sim/lib/workflows/persistence/duplicate.ts +++ b/apps/sim/lib/workflows/persistence/duplicate.ts @@ -17,7 +17,7 @@ import { normalizeWorkflowEdgeSourceHandle, normalizeWorkflowEdgeTargetHandle, } from '@sim/workflow-types/workflow' -import { and, eq, isNull, min } from 'drizzle-orm' +import { and, eq } from 'drizzle-orm' import type { DbOrTx } from '@/lib/db/types' import { remapConditionEdgeHandle } from '@/lib/workflows/condition-ids' import { @@ -27,6 +27,7 @@ import { type SubBlockRecord, sanitizeSubBlocksForDuplicate, } from '@/lib/workflows/persistence/remap-internal-ids' +import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order' import { deduplicateWorkflowName } from '@/lib/workflows/utils' import type { Variable } from '@/stores/variables/types' import type { LoopConfig, ParallelConfig } from '@/stores/workflows/workflow/types' @@ -183,37 +184,7 @@ export async function duplicateWorkflow( const targetFolderId = folderId !== undefined ? folderId : source.folderId await assertTargetFolderMutable(tx, targetFolderId, targetWorkspaceId) - const workflowParentCondition = targetFolderId - ? eq(workflow.folderId, targetFolderId) - : isNull(workflow.folderId) - const folderParentCondition = targetFolderId - ? eq(folderTable.parentId, targetFolderId) - : isNull(folderTable.parentId) - - const [[workflowMinResult], [folderMinResult]] = await Promise.all([ - tx - .select({ minOrder: min(workflow.sortOrder) }) - .from(workflow) - .where(and(eq(workflow.workspaceId, targetWorkspaceId), workflowParentCondition)), - tx - .select({ minOrder: min(folderTable.sortOrder) }) - .from(folderTable) - .where( - and( - eq(folderTable.workspaceId, targetWorkspaceId), - eq(folderTable.resourceType, 'workflow'), - folderParentCondition - ) - ), - ]) - const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce< - number | null - >((currentMin, candidate) => { - if (candidate == null) return currentMin - if (currentMin == null) return candidate - return Math.min(currentMin, candidate) - }, null) - const sortOrder = minSortOrder != null ? minSortOrder - 1 : 0 + const sortOrder = await nextWorkflowSortOrder(targetWorkspaceId, targetFolderId, tx) // Mapping from old variable IDs to new variable IDs (populated during variable duplication) const varIdMapping = new Map() diff --git a/apps/sim/lib/workflows/sort-order.ts b/apps/sim/lib/workflows/sort-order.ts new file mode 100644 index 00000000000..6244247c868 --- /dev/null +++ b/apps/sim/lib/workflows/sort-order.ts @@ -0,0 +1,60 @@ +import { db } from '@sim/db' +import { folder as folderTable, workflow as workflowTable } from '@sim/db/schema' +import { and, eq, isNull, min } from 'drizzle-orm' +import type { DbOrTx } from '@/lib/db/types' + +/** + * Sort order placing a new workflow above everything already in its folder. + * + * Workflows and folders share one ordering, so both minimums are consulted. + * Archived workflows are excluded: a soft-deleted row must not hold a slot that + * pushes new siblings further up each time one is created. + * + * Pass `tx` when the caller is inside a transaction, so the read sees that + * transaction's uncommitted rows rather than the pre-transaction snapshot. + */ +export async function nextWorkflowSortOrder( + workspaceId: string, + folderId: string | null | undefined, + tx: DbOrTx = db +): Promise { + const workflowParentCondition = folderId + ? eq(workflowTable.folderId, folderId) + : isNull(workflowTable.folderId) + const folderParentCondition = folderId + ? eq(folderTable.parentId, folderId) + : isNull(folderTable.parentId) + + const [[workflowMinResult], [folderMinResult]] = await Promise.all([ + tx + .select({ minOrder: min(workflowTable.sortOrder) }) + .from(workflowTable) + .where( + and( + eq(workflowTable.workspaceId, workspaceId), + workflowParentCondition, + isNull(workflowTable.archivedAt) + ) + ), + tx + .select({ minOrder: min(folderTable.sortOrder) }) + .from(folderTable) + .where( + and( + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, 'workflow'), + folderParentCondition + ) + ), + ]) + + const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce< + number | null + >((currentMin, candidate) => { + if (candidate == null) return currentMin + if (currentMin == null) return candidate + return Math.min(currentMin, candidate) + }, null) + + return minSortOrder != null ? minSortOrder - 1 : 0 +} diff --git a/apps/sim/lib/workflows/utils.ts b/apps/sim/lib/workflows/utils.ts index 1eec8c95fb9..83e98bbf3cc 100644 --- a/apps/sim/lib/workflows/utils.ts +++ b/apps/sim/lib/workflows/utils.ts @@ -3,13 +3,14 @@ import { folder as folderTable, workflow as workflowTable } from '@sim/db/schema import { createLogger } from '@sim/logger' import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow' import { generateId } from '@sim/utils/id' -import { and, asc, eq, inArray, isNull, min, sql } from 'drizzle-orm' +import { and, asc, eq, inArray, isNull, sql } from 'drizzle-orm' import { NextResponse } from 'next/server' import { getSession } from '@/lib/auth' import { materializeInlineExecutionValue } from '@/lib/execution/payloads/inline-materialization.server' import type { ExecutionMaterializationContext } from '@/lib/execution/payloads/materialization.server' import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' +import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order' import { listAccessibleWorkspaceRowsForUser } from '@/lib/workspaces/utils' import type { ExecutionResult } from '@/executor/types' @@ -396,45 +397,7 @@ export async function createWorkflowRecord(params: CreateWorkflowInput) { ) } - const workflowParentCondition = folderId - ? eq(workflowTable.folderId, folderId) - : isNull(workflowTable.folderId) - const folderParentCondition = folderId - ? eq(folderTable.parentId, folderId) - : isNull(folderTable.parentId) - - const [[workflowMinResult], [folderMinResult]] = await Promise.all([ - db - .select({ minOrder: min(workflowTable.sortOrder) }) - .from(workflowTable) - .where( - and( - eq(workflowTable.workspaceId, workspaceId), - workflowParentCondition, - isNull(workflowTable.archivedAt) - ) - ), - db - .select({ minOrder: min(folderTable.sortOrder) }) - .from(folderTable) - .where( - and( - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - folderParentCondition - ) - ), - ]) - - const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce< - number | null - >((currentMin, candidate) => { - if (candidate == null) return currentMin - if (currentMin == null) return candidate - return Math.min(currentMin, candidate) - }, null) - - const sortOrder = minSortOrder != null ? minSortOrder - 1 : 0 + const sortOrder = await nextWorkflowSortOrder(workspaceId, folderId) await db.insert(workflowTable).values({ id: workflowId, From 13259cb9b885ded92abbbd7911f8d000723503d0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:08:34 -0700 Subject: [PATCH 2/7] fix(ee): case-fold the stored integration allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ee/access-control re-implemented the allowlist intersection instead of calling intersectIntegrationAllowlists, and lost the case-folding: normalization only happened on the envAllowlist !== null branch, so with ALLOWED_INTEGRATIONS unset a stored config went through untouched. Callers compare against blockType.toLowerCase(), so a stored 'Slack' failed to match 'slack' and the block was denied. The access-control UI writes block.type directly and block types are lowercase, so this is not reachable from the UI — but allowedIntegrations is a bare z.array(z.string()) on the wire, so any API client can store mixed case. Replaces the fork with the shared helper. Adds two tests; the first fails against the old code. --- .../utils/permission-check.test.ts | 14 +++++++++ .../access-control/utils/permission-check.ts | 30 ++++++++----------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/apps/sim/ee/access-control/utils/permission-check.test.ts b/apps/sim/ee/access-control/utils/permission-check.test.ts index b5eaa610496..1ba17128f8f 100644 --- a/apps/sim/ee/access-control/utils/permission-check.test.ts +++ b/apps/sim/ee/access-control/utils/permission-check.test.ts @@ -446,6 +446,20 @@ describe('validateBlockType', () => { it('always allows start_trigger', async () => { await validateBlockType(undefined, undefined, 'start_trigger') }) + + it('case-folds a stored allowlist so a mixed-case entry still matches', async () => { + queueGroupResolution([{ config: { allowedIntegrations: ['Slack'] } }]) + + await validateBlockType('user-123', 'workspace-1', 'slack') + }) + + it('still rejects a block absent from a mixed-case stored allowlist', async () => { + queueGroupResolution([{ config: { allowedIntegrations: ['Slack'] } }]) + + await expect(validateBlockType('user-123', 'workspace-1', 'discord')).rejects.toThrow( + IntegrationNotAllowedError + ) + }) }) describe('when env allowlist is configured', () => { diff --git a/apps/sim/ee/access-control/utils/permission-check.ts b/apps/sim/ee/access-control/utils/permission-check.ts index 0afd24ed462..4d6a835c264 100644 --- a/apps/sim/ee/access-control/utils/permission-check.ts +++ b/apps/sim/ee/access-control/utils/permission-check.ts @@ -12,6 +12,7 @@ import { isPublicApiDisabled, } from '@/lib/core/config/env-flags' import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access' +import { intersectIntegrationAllowlists } from '@/lib/permission-groups/integration-allowlist' import { DEFAULT_PERMISSION_GROUP_CONFIG, type PermissionGroupConfig, @@ -108,29 +109,22 @@ export class ChatDeployAuthNotAllowedError extends Error { /** * Merges the env allowlist into a permission config. - * If `config` is null and no env allowlist is set, returns null. - * If `config` is null but env allowlist is set, returns a default config with only allowedIntegrations set. - * If both are set, intersects the two allowlists. + * + * Returns null only when neither layer restricts anything. Otherwise the group's + * own allowlist is intersected with the env one by + * {@link intersectIntegrationAllowlists}, which case-folds both sides — callers + * compare against a lowercased block type, and a stored config reaches here + * straight off the wire, where the contract permits any casing. */ function mergeEnvAllowlist(config: PermissionGroupConfig | null): PermissionGroupConfig | null { const envAllowlist = getAllowedIntegrationsFromEnv() + if (config === null && envAllowlist === null) return null - if (envAllowlist === null) { - return config - } - - if (config === null) { - return { ...DEFAULT_PERMISSION_GROUP_CONFIG, allowedIntegrations: envAllowlist } + const base = config ?? DEFAULT_PERMISSION_GROUP_CONFIG + return { + ...base, + allowedIntegrations: intersectIntegrationAllowlists(base.allowedIntegrations, envAllowlist), } - - const merged = - config.allowedIntegrations === null - ? envAllowlist - : config.allowedIntegrations - .map((i) => i.toLowerCase()) - .filter((i) => envAllowlist.includes(i)) - - return { ...config, allowedIntegrations: merged } } /** From 214a19deca6e7ce233ff555bcaa2408912a8cf8d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:14:21 -0700 Subject: [PATCH 3/7] fix(queries): forward the abort signal to getFullOrganization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useOrganization destructured `signal` from the queryFn and passed it to fetchOrganization, which named the parameter `_signal` and never used it — so the org detail fetch could not be cancelled. Switching orgs rapidly left every prior request in flight, free to resolve out of order. Better Auth takes cancellation two ways and both are already used here: fetchOptions on the params object (session.ts:21) and a second argument (admin-users.ts:129). Uses the former. This was the only `_signal` under apps/sim/hooks. The existing transition test asserted the exact call shape, so it now asserts intent via objectContaining. Adds a test for the signal itself; it fails against the old code. --- apps/sim/hooks/queries/organization.test.tsx | 17 ++++++++++++++--- apps/sim/hooks/queries/organization.ts | 3 ++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/apps/sim/hooks/queries/organization.test.tsx b/apps/sim/hooks/queries/organization.test.tsx index 836ea8e559f..011a61f6401 100644 --- a/apps/sim/hooks/queries/organization.test.tsx +++ b/apps/sim/hooks/queries/organization.test.tsx @@ -189,8 +189,19 @@ describe('organization identity transitions', () => { expect(container).not.toHaveTextContent('Member A') expect(container).not.toHaveTextContent('org-a') expect(container.querySelector('button')).toBeNull() - expect(mockGetFullOrganization).toHaveBeenCalledWith({ - query: { organizationId: 'org-b' }, - }) + expect(mockGetFullOrganization).toHaveBeenCalledWith( + expect.objectContaining({ query: { organizationId: 'org-b' } }) + ) + }) + + it('forwards the query signal so an in-flight org fetch can be cancelled', async () => { + mockGetFullOrganization.mockResolvedValue({ data: ORGANIZATION_A }) + + renderOrganization('org-a') + + await flushQueries() + + const [args] = mockGetFullOrganization.mock.calls[0] + expect(args.fetchOptions?.signal).toBeInstanceOf(AbortSignal) }) }) diff --git a/apps/sim/hooks/queries/organization.ts b/apps/sim/hooks/queries/organization.ts index f811e9fb54e..684ecc67c49 100644 --- a/apps/sim/hooks/queries/organization.ts +++ b/apps/sim/hooks/queries/organization.ts @@ -154,9 +154,10 @@ export function useMemberRemovalImpact( * (no cross-org cache collision). The active-org caller passes the active org's * id, so its behavior is unchanged. */ -async function fetchOrganization(orgId: string, _signal?: AbortSignal) { +async function fetchOrganization(orgId: string, signal?: AbortSignal) { const response = await client.organization.getFullOrganization({ query: { organizationId: orgId }, + fetchOptions: { signal }, }) return response.data } From bc210f64de48b30efd5400f72e6d8b90401ac3ec Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:14:21 -0700 Subject: [PATCH 4/7] fix(canvas): select from useWorkflowRegistry instead of subscribing whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-zustand-v5-selectors matched /use[A-Z]\w*Store\(/, and exactly one Zustand store in the repo is not named with a Store suffix — useWorkflowRegistry. So the store behind the canvas went unchecked, and two bare whole-store subscriptions had accumulated in the action bar, re-rendering it on every registry mutation (clipboard, hydration, pendingSelection, activeWorkflowId). Every other call site in the repo already uses a selector. Widens the pattern to (?:Store|Registry) and fixes both call sites. The widened gate reports these two and nothing else, so there is no cleanup tail. --- .../w/[workflowId]/components/action-bar/action-bar.tsx | 4 ++-- scripts/check-zustand-v5-selectors.ts | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/action-bar/action-bar.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/action-bar/action-bar.tsx index c053ec1b50e..fb65eb03b82 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/action-bar/action-bar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/action-bar/action-bar.tsx @@ -205,7 +205,7 @@ export const ActionBar = memo( collaborativeBatchToggleBlockEnabled, collaborativeBatchToggleLocked, } = useCollaborativeWorkflow() - const { setPendingSelection } = useWorkflowRegistry() + const setPendingSelection = useWorkflowRegistry((state) => state.setPendingSelection) const { handleCancelExecution, handleRunFromBlock } = useWorkflowExecution() const handleDuplicateBlock = useCallback(() => { const { copyBlocks, preparePasteData } = useWorkflowRegistry.getState() @@ -249,7 +249,7 @@ export const ActionBar = memo( }) ) - const { activeWorkflowId } = useWorkflowRegistry() + const activeWorkflowId = useWorkflowRegistry((state) => state.activeWorkflowId) const snapshot = useLastExecutionSnapshot(activeWorkflowId) const userPermissions = useUserPermissionsContext() const edges = useWorkflowStore((state) => state.edges) diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 21cc3b86fae..7d8fe511710 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -8,7 +8,12 @@ const APP_DIR = path.join(ROOT, 'apps/sim') const SKIP_DIRS = new Set(['node_modules', '.next', '.turbo', 'coverage', 'dist', 'build']) const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.jsx']) -const STORE_HOOK_CALL_PATTERN = /\buse[A-Z][A-Za-z0-9_]*Store\s*\(/g +/** + * Zustand store hooks are named `useStore` by convention, with one + * exception: `useWorkflowRegistry`. Matching only the `Store` suffix left that + * store — one of the hottest in the canvas — entirely unchecked. + */ +const STORE_HOOK_CALL_PATTERN = /\buse[A-Z][A-Za-z0-9_]*(?:Store|Registry)\s*\(/g const SAFE_ANNOTATION = 'zustand-v5-safe:' const UNSAFE_SELECTOR_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [ { From 9cd4bbff507c933cfd0118c42b6fe21d08ecf0a8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:14:40 -0700 Subject: [PATCH 5/7] docs: correct comments that name symbols which no longer exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of these points a reader at an identifier that is not in the repo: - table/import-data.ts, table/service.ts — `acquireTablePositionLock` and `nextAutoPosition` were removed with the service.ts split; the surviving lock is `acquireRowOrderLock`, which import-data.ts already imports and calls. service.ts's mention is load-bearing: it exists to tell the reader which other lock this one mirrors, for lock-ordering. - resources/orchestration/restore-resource.ts — named `performRestoreFolder` (the callee is `restoreFolder`) and described a `'workflow'` default it falls back to. There is no such default: resourceType is required and the config lookup is a bare index. Describing a fiction is how a future reader talks themselves into relaxing the total Record to a Partial. - knowledge/search/queries.ts — cited apps/docs/app/api/chat/route.ts, deleted with Ask AI. The k=60 it pins against now lives in the docs search route. - rate-limiter/hosted-key/queue.ts — documented a `waitForHead` method the class does not have; the queue exposes `checkHead` and the polling loop is private to the consumer. - logs/log-views.ts — a "Level 1.5 / 2 / 3" scheme that appears nowhere else; the real contract is the five named views. Dropped, and the three banner rules with it (CLAUDE.md bans banner separators). Comment-only apart from the log-views banners. --- apps/sim/lib/core/rate-limiter/hosted-key/queue.ts | 2 +- apps/sim/lib/knowledge/search/queries.ts | 2 +- apps/sim/lib/logs/log-views.ts | 12 +++--------- .../lib/resources/orchestration/restore-resource.ts | 5 +++-- apps/sim/lib/table/import-data.ts | 2 +- apps/sim/lib/table/service.ts | 2 +- 6 files changed, 10 insertions(+), 15 deletions(-) diff --git a/apps/sim/lib/core/rate-limiter/hosted-key/queue.ts b/apps/sim/lib/core/rate-limiter/hosted-key/queue.ts index bab7dfc7ca3..27d3802e8b4 100644 --- a/apps/sim/lib/core/rate-limiter/hosted-key/queue.ts +++ b/apps/sim/lib/core/rate-limiter/hosted-key/queue.ts @@ -69,7 +69,7 @@ export interface EnqueueResult { /** * Per-workspace+provider FIFO queue for hosted-key acquisitions. * - * Callers `enqueue` to claim a position, then `waitForHead` until they're at + * Callers `enqueue` to claim a position, then poll `checkHead` until they're at * the head, then attempt to consume from the token bucket. On success or cap * exceeded, they `dequeue` to make room for the next caller. * diff --git a/apps/sim/lib/knowledge/search/queries.ts b/apps/sim/lib/knowledge/search/queries.ts index 9c2867ea857..9136a80a9d8 100644 --- a/apps/sim/lib/knowledge/search/queries.ts +++ b/apps/sim/lib/knowledge/search/queries.ts @@ -314,7 +314,7 @@ const FTS_CONFIG = 'english' /** * Reciprocal-rank-fusion damping constant. 60 is the value from the original RRF - * paper and matches the docs Ask-AI retriever (`apps/docs/app/api/chat/route.ts`). + * paper and matches the docs search retriever (`apps/docs/app/api/search/route.ts`). */ export const RRF_K = 60 diff --git a/apps/sim/lib/logs/log-views.ts b/apps/sim/lib/logs/log-views.ts index da306b9ed0a..07c11fdb737 100644 --- a/apps/sim/lib/logs/log-views.ts +++ b/apps/sim/lib/logs/log-views.ts @@ -45,8 +45,7 @@ const DEFAULT_MATCH_TIME_BUDGET_MS = 5_000 */ const DEFAULT_MAX_SCANNED_CHARS = 64 * 1024 * 1024 -// Overview (Level 2): block tree with timing + cost, NO input/output. - +/** Block tree with timing and cost, without input/output. */ export interface OverviewSpan { id: string blockId?: string @@ -75,10 +74,7 @@ export function toOverview(spans: TraceSpan[]): OverviewSpan[] { }) } -// --------------------------------------------------------------------------- -// Trace (Level 1.5): condensed per-block digest — names, statuses, counts. -// --------------------------------------------------------------------------- - +/** Condensed per-block digest: names, statuses, counts. */ export interface TraceDigestEntry { /** Block id when the spans carry one; the drill-in key for `full` blockIds. */ blockId?: string @@ -125,9 +121,7 @@ export function toTrace(spans: TraceSpan[]): TraceDigestEntry[] { return Array.from(byKey.values()) } -// --------------------------------------------------------------------------- -// Full (Level 3): block tree WITH materialized input/output. - +/** Block tree with materialized input/output. */ export interface FullSpan extends OverviewSpan { startTime?: string endTime?: string diff --git a/apps/sim/lib/resources/orchestration/restore-resource.ts b/apps/sim/lib/resources/orchestration/restore-resource.ts index 4be469d5ffe..651ddd1c785 100644 --- a/apps/sim/lib/resources/orchestration/restore-resource.ts +++ b/apps/sim/lib/resources/orchestration/restore-resource.ts @@ -48,8 +48,9 @@ type RestorableFolderType = 'folder' | 'knowledge_folder' | 'table_folder' /** * Deliberately a total `Record` over the folder types, not a `Partial` one: adding a tree to * `RestorableFolderType` without a mapping here has to fail the build. With a partial map the - * lookup would yield `undefined`, `performRestoreFolder` would fall back to its `'workflow'` - * default, and the restore would silently target the wrong tree. + * lookup would yield `undefined`, which `restoreFolder` types as a required + * `FolderResourceType` — so the failure would surface as an undefined folder config deep in + * the cascade rather than at the call site. */ const FOLDER_RESOURCE_TYPE_BY_RESTORABLE: Record = { folder: 'workflow', diff --git a/apps/sim/lib/table/import-data.ts b/apps/sim/lib/table/import-data.ts index fce5491a03f..808a4108f5d 100644 --- a/apps/sim/lib/table/import-data.ts +++ b/apps/sim/lib/table/import-data.ts @@ -58,7 +58,7 @@ export interface BulkImportBatch { * Inserts one batch of rows for an async import in a single committed statement. * * Differs from {@link batchInsertRowsWithTx} for the bulk-load case: caller-supplied - * contiguous positions (no `acquireTablePositionLock` / `nextAutoPosition` scan — an + * contiguous order keys (no `acquireRowOrderLock` scan — an * import owns its hidden table as the sole writer), no `RETURNING`, and **no * `fireTableTrigger` / `runWorkflowColumn`** (a 1M-row import must not dispatch a * workflow run per row). `row_count` is maintained set-based by the statement-level diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index d77c71457f1..6063ec9be9c 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -122,7 +122,7 @@ function readLocks(row: { * Uses an advisory lock (not `SELECT ... FOR UPDATE` on the definition row) so * it adds no edges to the row-lock graph — the row-count trigger (migration * 0198) locks the definition row from `insertRow`/`deleteRow`, and a FOR UPDATE - * here would invert that order. Mirrors `acquireTablePositionLock`. The lock and + * here would invert that order. Mirrors `acquireRowOrderLock`. The lock and * the read both release at COMMIT/ROLLBACK; the wait is bounded by the * `statement_timeout` set in `setTableTxTimeouts`. */ From 4befe38a128d854bc05a3a5a37f0bbb7c0952984 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:14:44 -0700 Subject: [PATCH 6/7] refactor(ui): derive three values instead of storing or memoizing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - import-modal: browserId and profileId were state corrected by two effects when a reload dropped the selection. That commits and paints one frame in which the profile still belongs to the previously selected browser — and Import is enabled during it, submitting via a `profiles.find` that searches every browser's profiles. Both now fall back during render, and `selected` searches only the current browser's profiles. Covered by the existing 'never leaves a profile selected that belongs to another browser' test. - workflow.tsx: isWorkflowEmpty was a second useMemo over the same [blocks] dep computing exactly !hasBlocks, allocating its own Object.keys array. Both feed primitives, so neither memo bought identity stability. - thinking-loader: an effect seeding cycleVariant whenever variant is defined, which `shown = variant ?? cycleVariant` can never read. On the one transition where cycleVariant becomes visible (variant going undefined) the cycling effect assigns it in every branch — settle, reduced-motion, and tick — in the same flush, so the seed was never observable. --- .../components/import-modal/import-modal.tsx | 38 +++++++++---------- .../[workspaceId]/w/[workflowId]/workflow.tsx | 2 +- apps/sim/components/ui/thinking-loader.tsx | 6 --- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/components/import-modal/import-modal.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/components/import-modal/import-modal.tsx index 88f0770a76b..b140c7c9747 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/browser/components/import-modal/import-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/browser/components/import-modal/import-modal.tsx @@ -1,6 +1,6 @@ 'use client' -import { useEffect, useMemo, useState } from 'react' +import { useMemo, useState } from 'react' import type { BrowserImportProfile } from '@sim/desktop-bridge' import { ChipModal, @@ -39,29 +39,29 @@ function browserOptions(profiles: BrowserImportProfile[]) { */ export function ImportModal({ open, onOpenChange, profiles, pending, onImport }: ImportModalProps) { const browsers = useMemo(() => browserOptions(profiles), [profiles]) - const [browserId, setBrowserId] = useState(browsers[0]?.value ?? '') + const [pickedBrowserId, setPickedBrowserId] = useState(browsers[0]?.value ?? '') + + /** + * A reload can drop the browser or profile that was picked. Falling back here + * rather than correcting in an effect matters: the effect form commits and + * paints one frame in which the profile still belongs to the previously + * selected browser, and Import is enabled during it. + */ + const browserId = browsers.some((browser) => browser.value === pickedBrowserId) + ? pickedBrowserId + : (browsers[0]?.value ?? '') const profilesForBrowser = useMemo( () => profiles.filter((profile) => profile.browserId === browserId), [browserId, profiles] ) - const [profileId, setProfileId] = useState(profilesForBrowser[0]?.id ?? '') - - // Keep the selection valid as the browser changes or the list reloads, - // rather than leaving a profile selected that belongs to another browser. - useEffect(() => { - if (!profilesForBrowser.some((profile) => profile.id === profileId)) { - setProfileId(profilesForBrowser[0]?.id ?? '') - } - }, [profileId, profilesForBrowser]) + const [pickedProfileId, setPickedProfileId] = useState(profilesForBrowser[0]?.id ?? '') - useEffect(() => { - if (!browsers.some((browser) => browser.value === browserId)) { - setBrowserId(browsers[0]?.value ?? '') - } - }, [browserId, browsers]) + const profileId = profilesForBrowser.some((profile) => profile.id === pickedProfileId) + ? pickedProfileId + : (profilesForBrowser[0]?.id ?? '') - const selected = profiles.find((profile) => profile.id === profileId) ?? null + const selected = profilesForBrowser.find((profile) => profile.id === profileId) ?? null return ( @@ -79,7 +79,7 @@ export function ImportModal({ open, onOpenChange, profiles, pending, onImport }: title='Browser' options={browsers} value={browserId} - onChange={setBrowserId} + onChange={setPickedBrowserId} placeholder='Select a browser' align='start' disabled={pending || browsers.length === 0} @@ -92,7 +92,7 @@ export function ImportModal({ open, onOpenChange, profiles, pending, onImport }: label: profile.profileLabel, }))} value={profileId} - onChange={setProfileId} + onChange={setPickedProfileId} placeholder='Select a profile' align='start' disabled={pending || profilesForBrowser.length === 0} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx index 53687ea837f..6c6edf0517c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx @@ -574,7 +574,7 @@ const WorkflowContent = React.memo( embedded, }) - const isWorkflowEmpty = useMemo(() => Object.keys(blocks).length === 0, [blocks]) + const isWorkflowEmpty = !hasBlocks /** Handles OAuth connect events dispatched by Copilot tools. */ useEffect(() => { diff --git a/apps/sim/components/ui/thinking-loader.tsx b/apps/sim/components/ui/thinking-loader.tsx index d17b27c6dee..2aafa83915d 100644 --- a/apps/sim/components/ui/thinking-loader.tsx +++ b/apps/sim/components/ui/thinking-loader.tsx @@ -320,12 +320,6 @@ export function ThinkingLoader({ const cycling = variant === undefined const [retainMorphStages, setRetainMorphStages] = useState(cycling) - useEffect(() => { - if (variant !== undefined) { - setCycleVariant(variant) - } - }, [variant]) - useEffect(() => { if (!cycling) return // Settle: stop the cycle and melt to the terminal orb (goo handles the morph). From 50058d29a9a93d29fc3660a3727b6a3f6012cc01 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 24 Aug 2026 22:48:09 -0700 Subject: [PATCH 7/7] fix(review): exclude deleted folders from sort order; make the allowlist tests real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from cubic on #7062, all valid. 1. nextWorkflowSortOrder consulted the folder minimum without excluding soft-deleted folders. Because the helper returns min - 1, a deleted folder holding the lowest slot ratchets the floor down permanently — the same class of bug as the archived-workflow one this PR set out to fix, on the other half of the query. lib/folders/orchestration.ts already documents this exact rationale for the folder-creation side of the same algorithm, and the uploads folder manager filters it too; this was the outlier. 2. The two new allowlist tests did not call setEnterpriseOrgWorkspace(), so resolution never reached the group queries and validateBlockType returned early. They passed against the unfixed code when run in isolation and only appeared to fail in a full-file run, where mock state leaked from earlier tests. Verified with 'vitest -t': both now fail without the case-folding fix and pass with it. 3. The restore-resource comment this PR rewrote was itself wrong. A Partial map does not defer the failure into the cascade — the lookup widens to FolderResourceType | undefined and the error lands on the restoreFolder call site. Reworded to say that, and why keeping the check at the mapping matters. --- .../ee/access-control/utils/permission-check.test.ts | 2 ++ .../lib/resources/orchestration/restore-resource.ts | 8 ++++---- apps/sim/lib/workflows/sort-order.ts | 12 ++++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/apps/sim/ee/access-control/utils/permission-check.test.ts b/apps/sim/ee/access-control/utils/permission-check.test.ts index 1ba17128f8f..3a47110594f 100644 --- a/apps/sim/ee/access-control/utils/permission-check.test.ts +++ b/apps/sim/ee/access-control/utils/permission-check.test.ts @@ -448,12 +448,14 @@ describe('validateBlockType', () => { }) it('case-folds a stored allowlist so a mixed-case entry still matches', async () => { + setEnterpriseOrgWorkspace() queueGroupResolution([{ config: { allowedIntegrations: ['Slack'] } }]) await validateBlockType('user-123', 'workspace-1', 'slack') }) it('still rejects a block absent from a mixed-case stored allowlist', async () => { + setEnterpriseOrgWorkspace() queueGroupResolution([{ config: { allowedIntegrations: ['Slack'] } }]) await expect(validateBlockType('user-123', 'workspace-1', 'discord')).rejects.toThrow( diff --git a/apps/sim/lib/resources/orchestration/restore-resource.ts b/apps/sim/lib/resources/orchestration/restore-resource.ts index 651ddd1c785..fd27e57afac 100644 --- a/apps/sim/lib/resources/orchestration/restore-resource.ts +++ b/apps/sim/lib/resources/orchestration/restore-resource.ts @@ -47,10 +47,10 @@ type RestorableFolderType = 'folder' | 'knowledge_folder' | 'table_folder' /** * Deliberately a total `Record` over the folder types, not a `Partial` one: adding a tree to - * `RestorableFolderType` without a mapping here has to fail the build. With a partial map the - * lookup would yield `undefined`, which `restoreFolder` types as a required - * `FolderResourceType` — so the failure would surface as an undefined folder config deep in - * the cascade rather than at the call site. + * `RestorableFolderType` without a mapping has to fail the build *here*, at the mapping. A + * `Partial` still compiles with the tree missing — the lookup widens to + * `FolderResourceType | undefined`, so the error moves to the `restoreFolder` call site, and + * suppressing it there leaves the cascade resolving an undefined folder config. */ const FOLDER_RESOURCE_TYPE_BY_RESTORABLE: Record = { folder: 'workflow', diff --git a/apps/sim/lib/workflows/sort-order.ts b/apps/sim/lib/workflows/sort-order.ts index 6244247c868..6a6025d7edb 100644 --- a/apps/sim/lib/workflows/sort-order.ts +++ b/apps/sim/lib/workflows/sort-order.ts @@ -6,9 +6,12 @@ import type { DbOrTx } from '@/lib/db/types' /** * Sort order placing a new workflow above everything already in its folder. * - * Workflows and folders share one ordering, so both minimums are consulted. - * Archived workflows are excluded: a soft-deleted row must not hold a slot that - * pushes new siblings further up each time one is created. + * Workflows and folders share one ordering, so both minimums are consulted, and + * *both* exclude soft-deleted rows. Because this returns `min - 1`, counting a + * deleted row lets every delete ratchet the floor further negative and never + * recover: a deleted sibling at -400 pins the next new workflow at -401 forever. + * `lib/folders/orchestration.ts` documents the same rule for the folder-creation + * side of this algorithm. * * Pass `tx` when the caller is inside a transaction, so the read sees that * transaction's uncommitted rows rather than the pre-transaction snapshot. @@ -43,7 +46,8 @@ export async function nextWorkflowSortOrder( and( eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, 'workflow'), - folderParentCondition + folderParentCondition, + isNull(folderTable.deletedAt) ) ), ])