From 0be5733584cb15e001adc057dab5575c40397ca9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:23:32 -0700 Subject: [PATCH 01/57] fix(projects): share the Project lock during fork creation, archive every banned-user workspace, and drop dead membership guards - requireForkProject takes the Project lock shared, so sibling file writers and other forks no longer wait out a fork's content copy; exclusive lifecycle holders (archive, split, transfer, departure) are still fenced - disableUserResources keeps archiving after one workspace fails, logs the failed ids, and throws an AggregateError at the end - remove null guards on workspace.project_id, which is NOT NULL - name release v0.9.18 instead of a PR number in Project backfill operator errors - log workspace and workflow ids per archive repair in the 0032 script migration (cherry picked from commit 5831241070f08536eb923a4de012b0a58bdb5ff3) --- .../__integration__/foundation.integration.ts | 58 +++++++++++-------- apps/sim/lib/projects/account-deletion.ts | 3 +- .../sim/lib/projects/application/use-cases.ts | 2 +- apps/sim/lib/projects/membership.ts | 27 +++++---- apps/sim/lib/workflows/lifecycle.ts | 21 ++++++- apps/sim/scripts/backfill-projects.ts | 4 +- packages/db/maintenance/project-backfill.ts | 2 +- .../0032_project_membership.ts | 5 ++ 8 files changed, 78 insertions(+), 44 deletions(-) diff --git a/apps/sim/lib/projects/__integration__/foundation.integration.ts b/apps/sim/lib/projects/__integration__/foundation.integration.ts index 193dcccaa04..e69f1107753 100644 --- a/apps/sim/lib/projects/__integration__/foundation.integration.ts +++ b/apps/sim/lib/projects/__integration__/foundation.integration.ts @@ -47,6 +47,7 @@ import { createProjectRecord, lockProject, lockWorkspaceProject, + requireForkProject, splitForkProject, transferWorkspaceProjects, } from '@/lib/projects/membership' @@ -1279,32 +1280,39 @@ describe('Project foundation at the database and application boundary', () => { } ) - check( - 'file writers in sibling environments share the Project lock that still fences a lifecycle holder', - async () => { - const f = await fixture(false) - const held = createDeferred() - const release = createDeferred() - const writer = db.transaction(async (tx) => { - await lockWorkspaceProject(tx, f.ids[0]) - const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`) - held.resolve(connection.pid) - await release.promise - }) - const blocker = await held.promise - let lifecycle: Promise | undefined - try { - const sibling = await db.transaction((tx) => lockWorkspaceProject(tx, f.ids[1])) - expect(sibling?.id).toBe(f.projectId) - lifecycle = db.transaction((tx) => lockProject(tx, f.projectId)) - await waitUntilBlockedBy(blocker, "lock='project'") - } finally { - release.resolve() - await writer + for (const [holder, holdProject] of [ + ['file writers', lockWorkspaceProject], + ['fork creation', requireForkProject], + ] as const) { + check( + `${holder} share the Project lock with sibling environments and still fence a lifecycle holder`, + async () => { + const f = await fixture(false) + const held = createDeferred() + const release = createDeferred() + const writer = db.transaction(async (tx) => { + await holdProject(tx, f.ids[0]) + const [connection] = await tx.execute<{ pid: number }>( + sql`SELECT pg_backend_pid() AS pid` + ) + held.resolve(connection.pid) + await release.promise + }) + const blocker = await held.promise + let lifecycle: Promise | undefined + try { + const sibling = await db.transaction((tx) => lockWorkspaceProject(tx, f.ids[1])) + expect(sibling?.id).toBe(f.projectId) + lifecycle = db.transaction((tx) => lockProject(tx, f.projectId)) + await waitUntilBlockedBy(blocker, "lock='project'") + } finally { + release.resolve() + await writer + } + await lifecycle } - await lifecycle - } - ) + ) + } check( 'account teardown transfers surviving Projects and atomically removes wholly private Projects', diff --git a/apps/sim/lib/projects/account-deletion.ts b/apps/sim/lib/projects/account-deletion.ts index a8669845704..162240aaac6 100644 --- a/apps/sim/lib/projects/account-deletion.ts +++ b/apps/sim/lib/projects/account-deletion.ts @@ -27,7 +27,7 @@ async function loadRelatedProjects(executor: DbOrTx, userId: string, doomedWorks .from(workspace) .where(inArray(workspace.id, doomedWorkspaceIds)) : [] - const projectIds = doomedMemberships.flatMap((row) => (row.projectId ? [row.projectId] : [])) + const projectIds = doomedMemberships.map((row) => row.projectId) return executor .select() .from(project) @@ -65,7 +65,6 @@ async function loadProjectEnvironments(executor: DbOrTx, projectIds: string[]) { : [] const byProject = new Map() for (const { projectId, ...environment } of rows) { - if (!projectId) continue const environments = byProject.get(projectId) if (environments) environments.push(environment) else byProject.set(projectId, [environment]) diff --git a/apps/sim/lib/projects/application/use-cases.ts b/apps/sim/lib/projects/application/use-cases.ts index 70af2249d1b..2bddc64ba58 100644 --- a/apps/sim/lib/projects/application/use-cases.ts +++ b/apps/sim/lib/projects/application/use-cases.ts @@ -221,7 +221,7 @@ export const getWorkspaceProject: OperationUseCase< .from(workspace) .where(eq(workspace.id, input.workspaceId)) .limit(1) - if (!membership?.projectId) throw new OrchestrationError('not_found', 'Project not found') + if (!membership) throw new OrchestrationError('not_found', 'Project not found') return { project: presentProject( await authorizeProject( diff --git a/apps/sim/lib/projects/membership.ts b/apps/sim/lib/projects/membership.ts index a9f8e6d5fee..95bc518c063 100644 --- a/apps/sim/lib/projects/membership.ts +++ b/apps/sim/lib/projects/membership.ts @@ -147,8 +147,6 @@ async function lockWorkspaceProjectInMode(tx: DbTransaction, workspaceId: string .where(eq(workspace.id, workspaceId)) .limit(1) if (!membership) return null - if (!membership.projectId) - throw new ProjectConflictError('Project membership needs reconciliation') await acquireAdvisoryXactLocks(tx, 'project', [ { key: projectLockKey(membership.projectId), shared }, ]) @@ -176,12 +174,13 @@ async function lockWorkspaceProjectInMode(tx: DbTransaction, workspaceId: string /** * Takes the workspace’s Project lock SHARED and rechecks membership; null means the * workspace no longer exists. For writers inside one environment (workspace files, - * shares): it fences them against exclusive Project holders (fork, split, archive, - * payer transfer, departure handoff, account deletion, retention) without serializing - * writers against each other. A transaction holding it must never later take the same - * Project exclusively ({@link lockProject}, {@link lockProjects}, {@link tryLockProjects}, - * {@link lockWorkspaceProjectExclusive}): two such holders would each wait on the other's - * shared hold. Take {@link lockWorkspaceProjectExclusive} up front instead. + * shares) and fork creation: it fences them against exclusive Project holders (split, + * archive, payer transfer, departure handoff, account deletion, retention) without + * serializing them against each other. A transaction holding it must never later take + * the same Project exclusively ({@link lockProject}, {@link lockProjects}, + * {@link tryLockProjects}, {@link lockWorkspaceProjectExclusive}): two such holders would + * each wait on the other's shared hold. Take {@link lockWorkspaceProjectExclusive} up + * front instead. */ export async function lockWorkspaceProject(tx: DbTransaction, workspaceId: string) { return lockWorkspaceProjectInMode(tx, workspaceId, true) @@ -189,17 +188,21 @@ export async function lockWorkspaceProject(tx: DbTransaction, workspaceId: strin /** * Takes the workspace’s Project lock EXCLUSIVELY and rechecks membership; null means the - * workspace no longer exists. For callers that change Project membership or lifecycle - * (fork, split, environment archive), which must exclude every writer and each other. + * workspace no longer exists. For callers that move environments between Projects or + * archive them (split, environment archive), which must exclude every writer and each other. * Take it before any shared hold on the same Project in the transaction, never after. */ export async function lockWorkspaceProjectExclusive(tx: DbTransaction, workspaceId: string) { return lockWorkspaceProjectInMode(tx, workspaceId, false) } -/** Locks the parent's Project for a new fork and refuses an archived Project. */ +/** + * Locks the parent's Project SHARED for a new fork and refuses an archived Project. Adding + * an environment only needs fencing from the exclusive holders that archive, split, or + * re-home the Project, so sibling writers need not wait out the fork's content copy. + */ export async function requireForkProject(tx: DbTransaction, parentWorkspaceId: string) { - const parent = await lockWorkspaceProjectExclusive(tx, parentWorkspaceId) + const parent = await lockWorkspaceProject(tx, parentWorkspaceId) if (!parent) throw new ProjectConflictError('Source workspace no longer exists') if (parent.archivedAt) throw new ProjectConflictError('Cannot fork an archived Project') return parent diff --git a/apps/sim/lib/workflows/lifecycle.ts b/apps/sim/lib/workflows/lifecycle.ts index 183f7ee0c2f..14fef2a268e 100644 --- a/apps/sim/lib/workflows/lifecycle.ts +++ b/apps/sim/lib/workflows/lifecycle.ts @@ -11,6 +11,7 @@ import { workspace, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { and, eq, inArray, isNull } from 'drizzle-orm' import { env } from '@/lib/core/config/env' import { PlatformEvents } from '@/lib/core/telemetry' @@ -308,8 +309,26 @@ export async function disableUserResources(userId: string): Promise { .from(workspace) .where(and(eq(workspace.ownerId, userId), isNull(workspace.archivedAt))) + const failures: { workspaceId: string; error: unknown }[] = [] + // Sequential: environments of one Project contend on its exclusive lock. for (const row of ownedWorkspaces) { - await archiveWorkspace(row.id, { requestId, expectedOwnerId: userId }) + try { + await archiveWorkspace(row.id, { requestId, expectedOwnerId: userId }) + } catch (error) { + failures.push({ workspaceId: row.id, error }) + } + } + if (failures.length > 0) { + logger.error(`[${requestId}] Failed to archive workspaces for banned user ${userId}`, { + failures: failures.map(({ workspaceId, error }) => ({ + workspaceId, + error: getErrorMessage(error), + })), + }) + throw new AggregateError( + failures.map((failure) => failure.error), + `Failed to archive ${failures.length} of ${ownedWorkspaces.length} workspaces for banned user ${userId}` + ) } logger.info( diff --git a/apps/sim/scripts/backfill-projects.ts b/apps/sim/scripts/backfill-projects.ts index 82a9cf46e76..ca924b17f93 100644 --- a/apps/sim/scripts/backfill-projects.ts +++ b/apps/sim/scripts/backfill-projects.ts @@ -161,7 +161,7 @@ async function main() { Routine assignment and enforcement run through db:migrate.\n From apps/sim: bun --no-env-file scripts/backfill-projects.ts [options]\n plan Read-only discovery; writes a new --manifest PATH. Optional --review PATH carries reviewed grouping decisions.\nrepair Repair reviewed archives and explicitly selected detached families; requires --report PATH.\napply Optional maintenance: assign reviewed families from --manifest; requires --report PATH.\nverify Validate the database and repair completion; requires --manifest and --report.\nstatus Read --report; does not connect or resume.\n -Writes require MIGRATION_DATABASE_URL pointing directly to the primary and --ack-release-drained.\nDeploy #8830 and verify old servers/workers drained first. Keep the reviewed manifest and report\nin durable private job storage. Review conflicts and repairs before repair/apply. Repair requires\nthe app runtime environment (DATABASE_URL must target the same database) for provider cleanup.\n +Writes require MIGRATION_DATABASE_URL pointing directly to the primary and --ack-release-drained.\nDeploy v0.9.18 and verify old servers/workers drained first. Keep the reviewed manifest and report\nin durable private job storage. Review conflicts and repairs before repair/apply. Repair requires\nthe app runtime environment (DATABASE_URL must target the same database) for provider cleanup.\n Defaults: --batch-size 50 (1–50), --max-batches 100 (1–10000), --seconds 60 (1–3600), --pause-ms 100 (1–60000).\nA budget limits scheduling. Transactions use 3s statement limits and a 5s total limit on PG17+,\notherwise a 5s idle-transaction limit on PG16. Reuse the same report\nto resume; lost reports can be recreated safely from the same manifest. Changed families need a\nnew plan/report. Review groupings explicitly: retain a legitimate group, or select one detachRootId and keep its generated destinationProjectId for replay. After detach, rediscover. Resolve all other conflicts and complete archive/provider cleanup before #8590. The registered runner can read exact retained-group decisions from PROJECT_BACKFILL_REVIEW_PATH. @@ -199,7 +199,7 @@ The registered migration first commits the durable column-authority switch after const databaseId = projectBackfillDatabaseId(rawUrl) const live = command === 'repair' || command === 'apply' if (live && !values['ack-release-drained']) - throw new Error('Verify #8830 server/worker drainage and pass --ack-release-drained') + throw new Error('Verify v0.9.18 server/worker drainage and pass --ack-release-drained') const batchSize = boundedInteger(values['batch-size'], 50, 50) const maxBatches = boundedInteger(values['max-batches'], 100, 10000) const seconds = boundedInteger(values.seconds, 60, 3600) diff --git a/packages/db/maintenance/project-backfill.ts b/packages/db/maintenance/project-backfill.ts index dfd1d3a1899..9d775f1f80a 100644 --- a/packages/db/maintenance/project-backfill.ts +++ b/packages/db/maintenance/project-backfill.ts @@ -175,7 +175,7 @@ export async function assertProjectBackfillDatabase(sql: Sql, write: boolean): P EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'workspace' AND column_name = 'project_id') AS expanded ` - if (!state?.expanded) throw new Error('Deploy #8830 before Project preparation') + if (!state?.expanded) throw new Error('Deploy v0.9.18 before Project preparation') if (state.version < 160000) throw new Error('Project preparation requires PostgreSQL 16+') if (write && (state.replica || state.readonly === 'on')) throw new Error('Project writes require the primary database') diff --git a/packages/db/script-migrations/0032_project_membership.ts b/packages/db/script-migrations/0032_project_membership.ts index cff3104e3bc..b8489ffc933 100644 --- a/packages/db/script-migrations/0032_project_membership.ts +++ b/packages/db/script-migrations/0032_project_membership.ts @@ -80,6 +80,11 @@ export const projectMembershipMigration: ScriptMigration = { await assertSession() try { await archiveProjectRepairWorkflows(sql, repair) + // At most 1000 ids: archiveProjectRepairWorkflows refuses larger repairs. + logger.info('Archived workflows in archived workspace', { + workspaceId: repair.workspaceId, + workflowIds: repair.workflowIds, + }) break } catch (error) { if (!(error instanceof ProjectBackfillBusy || retryable(error)) || attempt >= 2) From 5cdb83683e2a4f0810dee994bca7a015cfe7c5e7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:22:53 -0700 Subject: [PATCH 02/57] fix(files): resolve the end anchor after the start, keep line endings, share one anchored edit The end anchor is now the first match after the selected start anchor, so a closing line earlier in the file no longer fails the edit. Copilot's apply_file_edit and its streaming preview route anchored patches through applyWorkspaceFileContentEdit instead of two divergent copies. Anchored edits keep each untouched line's own ending; inserted lines take the anchor's. (cherry picked from commit fb3f81a1b7810c1dc8763f31b5a1479769f60803) --- .../tools/server/files/edit-content.ts | 98 ++----------------- .../tools/server/files/file-intent-store.ts | 41 ++++++++ .../tools/server/files/file-preview.ts | 73 ++------------ .../lib/workspace-files/edit-content.test.ts | 43 +++++++- apps/sim/lib/workspace-files/edit-content.ts | 90 +++++++++++------ 5 files changed, 160 insertions(+), 185 deletions(-) diff --git a/apps/sim/lib/mothership/tools/server/files/edit-content.ts b/apps/sim/lib/mothership/tools/server/files/edit-content.ts index 3eb0fbb7fd1..7a8a7a4e68a 100644 --- a/apps/sim/lib/mothership/tools/server/files/edit-content.ts +++ b/apps/sim/lib/mothership/tools/server/files/edit-content.ts @@ -14,6 +14,7 @@ import { } from '@/lib/mothership/tools/server/base-tool' import { getE2BDocFormat } from '@/lib/uploads/documents/compile' import { updateWorkspaceFileContent } from '@/lib/workspace-files/application/update-workspace-file-content' +import { applyWorkspaceFileContentEdit, EditContentError } from '@/lib/workspace-files/edit-content' import { collectSimPageDiagnostics, HAND_WRITTEN_PAGE_MESSAGE, @@ -22,7 +23,7 @@ import { SIM_PAGE_CONTENT_TYPE, } from '@/lib/workspace-files/page-compile' import { buildEmbeddedImageRefWarning } from './embedded-image-refs' -import { waitForLatestFileIntent } from './file-intent-store' +import { toAnchoredContentEdit, waitForLatestFileIntent } from './file-intent-store' import { compileDocForWrite, getDocumentFormatInfo, inferContentType } from './workspace-file' const logger = createLogger('EditContentServerTool') @@ -164,96 +165,15 @@ export const editContentServerTool: BaseServerTool { - const trimmed = anchor.trim() - let count = 0 - for (let i = afterIndex + 1; i < lines.length; i++) { - if (lines[i].trim() === trimmed) { - count++ - if (count === occurrence) return { index: i } - } - } - if (count === 0) { - return { - index: -1, - error: `Anchor line not found in "${fileRecord.name}": "${anchor.slice(0, 100)}"`, - } - } - return { - index: -1, - error: `Anchor line occurrence ${occurrence} not found (only ${count} match${count > 1 ? 'es' : ''}) in "${fileRecord.name}": "${anchor.slice(0, 100)}"`, - } - } - - if (intent.edit.mode === 'replace_between') { - if (!intent.edit.before_anchor || !intent.edit.after_anchor) { - return { - success: false, - message: 'replace_between requires before_anchor and after_anchor', - } - } - const before = findAnchorLine(intent.edit.before_anchor) - if (before.error) return { success: false, message: `Patch failed: ${before.error}` } - const after = findAnchorLine( - intent.edit.after_anchor, - defaultOccurrence, - before.index - ) - if (after.error) return { success: false, message: `Patch failed: ${after.error}` } - if (after.index <= before.index) { - return { - success: false, - message: 'Patch failed: after_anchor must appear after before_anchor in the file', - } - } - const newLines = [ - ...lines.slice(0, before.index + 1), - ...content.split('\n'), - ...lines.slice(after.index), - ] - finalContent = newLines.join('\n') - } else if (intent.edit.mode === 'insert_after') { - if (!intent.edit.anchor) { - return { success: false, message: 'insert_after requires anchor' } - } - const found = findAnchorLine(intent.edit.anchor) - if (found.error) return { success: false, message: `Patch failed: ${found.error}` } - const newLines = [ - ...lines.slice(0, found.index + 1), - ...content.split('\n'), - ...lines.slice(found.index + 1), - ] - finalContent = newLines.join('\n') - } else if (intent.edit.mode === 'delete_between') { - if (!intent.edit.start_anchor || !intent.edit.end_anchor) { - return { - success: false, - message: 'delete_between requires start_anchor and end_anchor', - } - } - const start = findAnchorLine(intent.edit.start_anchor) - if (start.error) return { success: false, message: `Patch failed: ${start.error}` } - const end = findAnchorLine(intent.edit.end_anchor, defaultOccurrence, start.index) - if (end.error) return { success: false, message: `Patch failed: ${end.error}` } - if (end.index <= start.index) { - return { - success: false, - message: 'Patch failed: end_anchor must appear after start_anchor in the file', - } - } - const newLines = [...lines.slice(0, start.index), ...lines.slice(end.index)] - finalContent = newLines.join('\n') - } else { + const anchored = toAnchoredContentEdit(intent.edit, content) + if ('error' in anchored) return { success: false, message: anchored.error } + try { + finalContent = applyWorkspaceFileContentEdit(existing, anchored.edit) + } catch (error) { + if (!(error instanceof EditContentError)) throw error return { success: false, - message: `Unknown anchored patch mode: "${intent.edit.mode}"`, + message: `Patch failed in "${fileRecord.name}": ${error.message}`, } } } else { diff --git a/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts b/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts index 30fe0c8be58..0e5684a06a6 100644 --- a/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts +++ b/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts @@ -3,6 +3,7 @@ import { toError } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' import { getRedisClient } from '@/lib/core/config/redis' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import type { WorkspaceFileContentEdit } from '@/lib/workspace-files/edit-content' export type PendingFileIntent = { operation: 'append' | 'update' | 'patch' @@ -44,6 +45,46 @@ export type FileIntentScope = { channelId?: string } +type AnchoredContentEdit = Exclude + +/** + * Maps a staged `anchored` patch onto the shared workspace-file edit, so the + * durable write and its streamed preview apply the same anchor semantics. + */ +export function toAnchoredContentEdit( + edit: Record, + content: string +): { edit: AnchoredContentEdit } | { error: string } { + const read = (key: string) => (typeof edit[key] === 'string' ? (edit[key] as string) : '') + const occurrence = typeof edit.occurrence === 'number' ? edit.occurrence : undefined + + switch (edit.mode) { + case 'replace_between': { + const beforeAnchor = read('before_anchor') + const afterAnchor = read('after_anchor') + if (!beforeAnchor || !afterAnchor) { + return { error: 'replace_between requires before_anchor and after_anchor' } + } + return { edit: { mode: 'replace_between', beforeAnchor, afterAnchor, content, occurrence } } + } + case 'insert_after': { + const anchor = read('anchor') + if (!anchor) return { error: 'insert_after requires anchor' } + return { edit: { mode: 'insert_after', anchor, content, occurrence } } + } + case 'delete_between': { + const startAnchor = read('start_anchor') + const endAnchor = read('end_anchor') + if (!startAnchor || !endAnchor) { + return { error: 'delete_between requires start_anchor and end_anchor' } + } + return { edit: { mode: 'delete_between', startAnchor, endAnchor, occurrence } } + } + default: + return { error: `Unknown anchored patch mode: "${String(edit.mode)}"` } + } +} + const logger = createLogger('FileIntentStore') const INTENT_TTL_MS = 60 * 60 * 1000 diff --git a/apps/sim/lib/mothership/tools/server/files/file-preview.ts b/apps/sim/lib/mothership/tools/server/files/file-preview.ts index d97093614e8..eb96654e25d 100644 --- a/apps/sim/lib/mothership/tools/server/files/file-preview.ts +++ b/apps/sim/lib/mothership/tools/server/files/file-preview.ts @@ -3,7 +3,9 @@ import { toError } from '@sim/utils/errors' import { observeContentOutsideModel } from '@/lib/core/application' import { executeCopilotFileUseCase } from '@/lib/mothership/application/execute-file-use-case' import type { ExecutionContext } from '@/lib/mothership/request/types' +import { toAnchoredContentEdit } from '@/lib/mothership/tools/server/files/file-intent-store' import { readWorkspaceFileContent } from '@/lib/workspace-files/application/read-workspace-file-content' +import { applyWorkspaceFileContentEdit, EditContentError } from '@/lib/workspace-files/edit-content' const logger = createLogger('CopilotFilePreview') const MAX_PREVIEW_SOURCE_BYTES = 5 * 1024 * 1024 @@ -15,27 +17,12 @@ interface BuildFilePreviewTextOptions { edit?: Record } -function findAnchorIndex(lines: string[], anchor: string, occurrence = 1, afterIndex = -1): number { - const trimmed = anchor.trim() - let count = 0 - for (let i = afterIndex + 1; i < lines.length; i++) { - if (lines[i].trim() === trimmed) { - count++ - if (count === occurrence) return i - } - } - return -1 -} - function extractPatchPreview( streamedContent: string, existingContent: string, edit?: Record ): string | undefined { const strategy = typeof edit?.strategy === 'string' ? edit.strategy : undefined - const lines = existingContent.split('\n') - const occurrence = - typeof edit?.occurrence === 'number' && Number.isFinite(edit.occurrence) ? edit.occurrence : 1 if (strategy === 'search_replace') { const search = typeof edit?.search === 'string' ? edit.search : '' @@ -52,55 +39,15 @@ function extractPatchPreview( ) } - const mode = typeof edit?.mode === 'string' ? edit.mode : undefined - if (!mode) return undefined - - if (mode === 'replace_between') { - const beforeAnchor = typeof edit?.before_anchor === 'string' ? edit.before_anchor : undefined - const afterAnchor = typeof edit?.after_anchor === 'string' ? edit.after_anchor : undefined - if (!beforeAnchor || !afterAnchor) return undefined - - const beforeIdx = findAnchorIndex(lines, beforeAnchor, occurrence) - const afterIdx = findAnchorIndex(lines, afterAnchor, occurrence, beforeIdx) - if (beforeIdx === -1 || afterIdx === -1 || afterIdx <= beforeIdx) return undefined - - const spliced = [ - ...lines.slice(0, beforeIdx + 1), - ...(streamedContent.length > 0 ? streamedContent.split('\n') : []), - ...lines.slice(afterIdx), - ] - return spliced.join('\n') - } - - if (mode === 'insert_after') { - const anchor = typeof edit?.anchor === 'string' ? edit.anchor : undefined - if (!anchor) return undefined - - const anchorIdx = findAnchorIndex(lines, anchor, occurrence) - if (anchorIdx === -1) return undefined - - const spliced = [ - ...lines.slice(0, anchorIdx + 1), - ...(streamedContent.length > 0 ? streamedContent.split('\n') : []), - ...lines.slice(anchorIdx + 1), - ] - return spliced.join('\n') - } - - if (mode === 'delete_between') { - const startAnchor = typeof edit?.start_anchor === 'string' ? edit.start_anchor : undefined - const endAnchor = typeof edit?.end_anchor === 'string' ? edit.end_anchor : undefined - if (!startAnchor || !endAnchor) return undefined - - const startIdx = findAnchorIndex(lines, startAnchor, occurrence) - const endIdx = findAnchorIndex(lines, endAnchor, occurrence, startIdx) - if (startIdx === -1 || endIdx === -1 || endIdx <= startIdx) return undefined - - const spliced = [...lines.slice(0, startIdx), ...lines.slice(endIdx)] - return spliced.join('\n') + if (strategy !== 'anchored' || !edit) return undefined + const anchored = toAnchoredContentEdit(edit, streamedContent) + if ('error' in anchored) return undefined + try { + return applyWorkspaceFileContentEdit(existingContent, anchored.edit) + } catch (error) { + if (error instanceof EditContentError) return undefined + throw error } - - return undefined } function shouldApplyPatchPreview(streamedContent: string, edit?: Record): boolean { diff --git a/apps/sim/lib/workspace-files/edit-content.test.ts b/apps/sim/lib/workspace-files/edit-content.test.ts index 8c93ab46c38..51a2739b5a1 100644 --- a/apps/sim/lib/workspace-files/edit-content.test.ts +++ b/apps/sim/lib/workspace-files/edit-content.test.ts @@ -188,7 +188,7 @@ describe('applyWorkspaceFileContentEdit', () => { ).toThrow(/whole number/) }) - it('refuses a matching end-anchor occurrence that precedes the start anchor', () => { + it('refuses an end anchor that only appears before the start anchor', () => { expect(() => applyWorkspaceFileContentEdit('end\nstart', { mode: 'replace_between', @@ -196,7 +196,46 @@ describe('applyWorkspaceFileContentEdit', () => { afterAnchor: 'end', content: 'x', }) - ).toThrow(/end anchor must follow the start anchor/) + ).toThrow(/Anchor line not found after the start anchor/) + }) + + it('ends the range at the first end anchor after the start, wherever earlier ones sit', () => { + expect( + applyWorkspaceFileContentEdit('a {\n}\nb {\nx\n}', { + mode: 'delete_between', + startAnchor: 'b {', + endAnchor: '}', + }) + ).toBe('a {\n}\n}') + + expect( + applyWorkspaceFileContentEdit(['s', 'e', 's', 'e', 's', 'one', 'e'].join('\n'), { + mode: 'replace_between', + beforeAnchor: 's', + afterAnchor: 'e', + occurrence: 3, + content: 'two', + }) + ).toBe(['s', 'e', 's', 'e', 's', 'two', 'e'].join('\n')) + }) + + it('keeps each untouched line ending in a file with mixed endings', () => { + expect( + applyWorkspaceFileContentEdit('crlf\r\nanchor\nlf\nend\r\n', { + mode: 'insert_after', + anchor: 'anchor', + content: 'one\r\ntwo', + }) + ).toBe('crlf\r\nanchor\none\ntwo\nlf\nend\r\n') + + expect( + applyWorkspaceFileContentEdit('start\r\nold\nend\nlf\n', { + mode: 'replace_between', + beforeAnchor: 'start', + afterAnchor: 'end', + content: 'a\nb', + }) + ).toBe('start\r\na\r\nb\nend\nlf\n') }) }) diff --git a/apps/sim/lib/workspace-files/edit-content.ts b/apps/sim/lib/workspace-files/edit-content.ts index 27934028f36..10bb62ea653 100644 --- a/apps/sim/lib/workspace-files/edit-content.ts +++ b/apps/sim/lib/workspace-files/edit-content.ts @@ -5,7 +5,6 @@ export type EditContentFailure = | { reason: 'not_found' } | { reason: 'ambiguous'; lineNumbers: number[] } | { reason: 'invalid_occurrence' } - | { reason: 'invalid_anchor_order' } | { reason: 'output_too_large'; maxBytes: number } export interface EditContentOptions { @@ -186,23 +185,31 @@ function validateOccurrence(occurrence: number | undefined): number { return value } -function anchorLineIndex(lines: string[], anchor: string, occurrence: number): number { +/** Index of the `occurrence`th line at or after `fromIndex` whose trimmed text equals the anchor. */ +function anchorLineIndex( + lines: string[], + anchor: string, + occurrence: number, + fromIndex = 0 +): number { const normalizedAnchor = anchor.trim() if (!normalizedAnchor) { throw new EditContentError('Anchor cannot be empty', { reason: 'not_found' }) } let matches = 0 - for (let index = 0; index < lines.length; index++) { + for (let index = fromIndex; index < lines.length; index++) { if (lines[index].trim() !== normalizedAnchor) continue matches++ if (matches === occurrence) return index } if (matches === 0) { - throw new EditContentError(`Anchor line not found: ${JSON.stringify(anchor.slice(0, 100))}`, { - reason: 'not_found', - }) + const where = fromIndex > 0 ? ' after the start anchor' : '' + throw new EditContentError( + `Anchor line not found${where}: ${JSON.stringify(anchor.slice(0, 100))}`, + { reason: 'not_found' } + ) } throw new EditContentError( `Anchor occurrence ${occurrence} not found; only ${matches} matching line${matches === 1 ? '' : 's'} exist`, @@ -216,15 +223,42 @@ function contentLines(content: string): string[] { return lines.at(-1) === '' ? lines.slice(0, -1) : lines } +/** + * Lines plus the separator that follows each one, so an edit rewrites only + * the lines it touches. `separators[i]` joins `lines[i]` to `lines[i + 1]`. + */ +interface SeparatedLines { + lines: string[] + separators: string[] +} + +function splitSeparatedLines(text: string): SeparatedLines { + const parts = text.split(/(\r\n|\n)/) + return { + lines: parts.filter((_, index) => index % 2 === 0), + separators: parts.filter((_, index) => index % 2 === 1), + } +} + +function joinSeparatedLines({ lines, separators }: SeparatedLines): string { + let text = lines[0] + for (let index = 1; index < lines.length; index++) { + text += separators[index - 1] + lines[index] + } + return text +} + /** * Applies one deterministic in-place edit to text. * * Exact replacement operates on byte-equivalent strings and refuses ambiguity * unless `replaceAll` is explicit. Anchored edits match complete trimmed lines, - * which makes them stable when surrounding line numbers move. Boundary anchors - * remain in place for replacement and insertion. Deletion matches the - * `apply_file_edit` protocol: it removes the start anchor and everything before - * the end anchor, while preserving the end anchor. + * which makes them stable when surrounding line numbers move. `occurrence` + * selects the start anchor (or the `insert_after` anchor); the end anchor is + * the first match after it. Boundary anchors remain in place for replacement + * and insertion. Deletion removes the start anchor and everything before the + * end anchor, while preserving the end anchor. Untouched lines keep their own + * line endings; inserted lines take the ending that follows the anchor line. */ export function applyWorkspaceFileContentEdit( text: string, @@ -235,40 +269,34 @@ export function applyWorkspaceFileContentEdit( return applyStringReplacement(text, edit.search, edit.content, edit.replaceAll, options) } - const lines = splitLines(text) + const { lines, separators } = splitSeparatedLines(text) const occurrence = validateOccurrence(edit.occurrence) - const eol = detectLineEnding(text) if (edit.mode === 'insert_after') { const anchorIndex = anchorLineIndex(lines, edit.anchor, occurrence) - return [ - ...lines.slice(0, anchorIndex + 1), - ...contentLines(edit.content), - ...lines.slice(anchorIndex + 1), - ].join(eol) + const inserted = contentLines(edit.content) + const separator = separators[anchorIndex] ?? detectLineEnding(text) + lines.splice(anchorIndex + 1, 0, ...inserted) + separators.splice(anchorIndex, 0, ...inserted.map(() => separator)) + return joinSeparatedLines({ lines, separators }) } const startAnchor = edit.mode === 'replace_between' ? edit.beforeAnchor : edit.startAnchor const endAnchor = edit.mode === 'replace_between' ? edit.afterAnchor : edit.endAnchor const startIndex = anchorLineIndex(lines, startAnchor, occurrence) - const endIndex = anchorLineIndex(lines, endAnchor, occurrence) - - if (endIndex <= startIndex) { - throw new EditContentError( - `Anchor occurrence ${occurrence} is not ordered: the end anchor must follow the start anchor`, - { reason: 'invalid_anchor_order' } - ) - } + const endIndex = anchorLineIndex(lines, endAnchor, 1, startIndex + 1) if (edit.mode === 'replace_between') { - return [ - ...lines.slice(0, startIndex + 1), - ...contentLines(edit.content), - ...lines.slice(endIndex), - ].join(eol) + const inserted = contentLines(edit.content) + const separator = separators[startIndex] + lines.splice(startIndex + 1, endIndex - startIndex - 1, ...inserted) + separators.splice(startIndex, endIndex - startIndex - 1, ...inserted.map(() => separator)) + return joinSeparatedLines({ lines, separators }) } - return [...lines.slice(0, startIndex), ...lines.slice(endIndex)].join(eol) + lines.splice(startIndex, endIndex - startIndex) + separators.splice(startIndex, endIndex - startIndex) + return joinSeparatedLines({ lines, separators }) } /** From 951e6e25f4ef0337a4c2b66056cc01242cf49b4b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:27:01 -0700 Subject: [PATCH 03/57] fix(files): resolve a named file in a folder scope by exact match, not by listing the folder resolveScopedFileReference listed up to 5000 files to find one name, so a named write, append, or edit inside a larger folder failed payload_too_large. The id and name lookups are now exact SQL predicates under the same folder scope and list authorization, with the same not-found and ambiguity errors. (cherry picked from commit 63a06782eb3327a6d858bc337600d88d76ce46c8) --- apps/sim/lib/internal/file/operations.test.ts | 72 +++++++++++++----- apps/sim/lib/internal/file/operations.ts | 74 +++++++++++-------- .../workspace/workspace-file-manager.ts | 8 ++ .../application/list-workspace-files.ts | 6 ++ 4 files changed, 111 insertions(+), 49 deletions(-) diff --git a/apps/sim/lib/internal/file/operations.test.ts b/apps/sim/lib/internal/file/operations.test.ts index c622caf40c2..13d41897ff5 100644 --- a/apps/sim/lib/internal/file/operations.test.ts +++ b/apps/sim/lib/internal/file/operations.test.ts @@ -407,23 +407,34 @@ describe('file manage folder wiring', () => { { ...FOLDER_ROW, id: 'user-b', parentId: 'memory', name: 'user-b', path: 'memory/user-b' }, ] + const MEMORY_FILES = [ + { ...workspaceFile('a-self'), name: 'self.md', folderId: 'user-a' }, + { ...workspaceFile('a-people-self'), name: 'self.md', folderId: 'user-a-people' }, + { ...workspaceFile('b-self'), name: 'self.md', folderId: 'user-b' }, + ] + beforeEach(() => { mockListWorkspaceFileFolders.mockResolvedValue({ folders: MEMORY_FOLDERS }) - mockListWorkspaceFilesInFolderScope.mockImplementation( - async ({ input }: { input: { includeSubfolders?: boolean } }) => ({ - files: [ - { ...workspaceFile('a-self'), name: 'self.md', folderId: 'user-a' }, - ...(input.includeSubfolders === false - ? [] - : [ - { - ...workspaceFile('a-people-self'), - name: 'self.md', - folderId: 'user-a-people', - }, - ]), - ], - truncated: false, + mockQueryWorkspaceFilePage.mockImplementation( + async ({ + input, + }: { + input: { + folderScope?: { folderIds: Set; includeRootItems: boolean } + fileId?: string + name?: string + } + }) => ({ + files: MEMORY_FILES.filter( + (file) => + (!input.folderScope || + (file.folderId + ? input.folderScope.folderIds.has(file.folderId) + : input.folderScope.includeRootItems)) && + (input.fileId === undefined || file.id === input.fileId) && + (input.name === undefined || file.name === input.name) + ), + nextKeys: null, }) ) }) @@ -559,11 +570,11 @@ describe('file manage folder wiring', () => { * the only thing telling them apart. */ it('appends to the file in the chosen folder, not a same-named file elsewhere', async () => { - mockListWorkspaceFilesInFolderScope.mockResolvedValue({ + mockQueryWorkspaceFilePage.mockResolvedValue({ files: [ { ...workspaceFile('notes-in-reports'), name: 'notes.md', folderId: 'folder-reports' }, ], - truncated: false, + nextKeys: null, }) await POST( createMockRequest('POST', { @@ -582,6 +593,33 @@ describe('file manage folder wiring', () => { ) }) + it('resolves a named file in a folder too large to list in one selection', async () => { + mockListWorkspaceFilesInFolderScope.mockResolvedValue({ files: [], truncated: true }) + mockQueryWorkspaceFilePage.mockResolvedValue({ + files: [ + { ...workspaceFile('notes-in-reports'), name: 'notes.md', folderId: 'folder-reports' }, + ], + nextKeys: null, + }) + + const response = await POST( + createMockRequest('POST', { + operation: 'append', + workspaceId: 'workspace-1', + fileName: 'notes.md', + folderPath: '/Reports', + content: 'more', + }) + ) + + expect(response.status).not.toBe(413) + expect(mockResolveWorkspaceFileReference).toHaveBeenCalledWith( + 'workspace-1', + 'notes-in-reports', + { includeOwnedFiles: true } + ) + }) + /* * A folder genuinely named `Q3/Q4` is one level, but the joined reference the * overwrite lookup used re-read it as two — so the existing file was never diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index eebc28d1c07..7957eeab351 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -109,6 +109,7 @@ import { MAX_ZIP_DOWNLOAD_FILES, } from '@/lib/workspace-files/limits' import { MAX_WORKSPACE_FILE_CONTENT_BYTES } from '@/lib/workspace-files/orchestration' +import { resolveWorkspaceFolderScope } from '@/lib/workspace-files/resolve-folder-scope' import { parseWorkspaceFileText } from '@/lib/workspace-files/text-extraction' import { type FileTextLineRange, sliceFileTextLines } from '@/lib/workspace-files/text-lines' import { isWorkspaceAccessDeniedError } from '@/lib/workspaces/permissions/utils' @@ -918,6 +919,9 @@ async function expandFolderPathsToFileIds(args: { return (await expandFolderPathsToFiles(args)).map((file) => file.id) } +/** How many same-named candidates an ambiguous scoped reference names. */ +const MAX_LISTED_NAME_MATCHES = 10 + /** * Resolves a typed file name inside a chosen folder to a canonical id. * @@ -926,11 +930,10 @@ async function expandFolderPathsToFileIds(args: { * lookup takes the oldest match anywhere. When a folder was chosen it is the * only thing disambiguating the target, so the name is resolved inside it, by * id, so the slash-in-a-folder-name hazard of a path-shaped reference never - * arises. + * arises. Shared by every operation that writes to a named file. * - * Shared by every operation that writes to a named file, because each of them - * has the same way to go wrong and this logic has already been rewritten twice - * under review. + * The lookups filter by exact id or name in SQL, so the size of the folder + * never bounds whether one file in it can be named. */ async function resolveScopedFileReference(args: { principal: Principal @@ -940,64 +943,71 @@ async function resolveScopedFileReference(args: { folderPaths: string[] | undefined includeSubfolders: boolean | undefined }): Promise { - const { fileName } = args + const { fileName, principal, workspaceId } = args const folderPaths = args.folderPaths ?? (args.folderPath ? [args.folderPath] : []) if (folderPaths.length === 0) return fileName const scopeLabel = folderPaths.join(', ') - const scoped = await expandFolderPathsToFiles({ - principal: args.principal, - workspaceId: args.workspaceId, + const folderScope = await resolveWorkspaceFolderScope({ + principal, + workspaceId, folderPaths, includeSubfolders: args.includeSubfolders, }) - /* - * An exact id inside the scope wins outright. Matching id and name together - * let a file NAMED like an id outproduce the file that actually carries it: - * `wf_` is a legal filename prefix, so a caller passing a real id could be - * answered with a different file that merely happens to be called that. - */ - const byId = scoped.find((file) => file.id === fileName) - if (byId) return byId.id + const findInScope = (match: { fileId: string } | { name: string }, limit: number) => + queryWorkspaceFilePage.execute({ + principal, + input: { + workspaceId, + folderScope, + ...match, + sortBy: 'uploadedAt', + sortOrder: 'asc', + limit, + }, + }) /* - * A reference that IS a real file id, but for a file outside this folder, is - * out of scope — never a name match. Falling through would answer a caller - * who named one file exactly with a different file that happens to be called - * like that id, which is the same lookalike hazard pointed the other way. - * - * Decided by looking the id up rather than by its shape, because `wf_` is a - * legal filename prefix and inferring from it is what this resolution has - * twice been wrong about. + * A reference that is a real file id resolves to that file or to nothing, + * never to a name match: `wf_` is a legal filename prefix, so a file merely + * named like an id must not answer a caller who named a file exactly. Decided + * by looking the id up rather than by its shape. */ let exactIdExists = false try { await readWorkspaceFileMetadata.execute({ - principal: args.principal, - input: { fileId: fileName, assertedWorkspaceId: args.workspaceId }, + principal, + input: { fileId: fileName, assertedWorkspaceId: workspaceId }, }) exactIdExists = true } catch (error) { if (!(error instanceof OrchestrationError) || error.code !== 'not_found') throw error } if (exactIdExists) { - throw new OrchestrationError('not_found', `File ${fileName} is not in ${scopeLabel}`) + const { files } = await findInScope({ fileId: fileName }, 1) + if (files.length === 0) { + throw new OrchestrationError('not_found', `File ${fileName} is not in ${scopeLabel}`) + } + return files[0].id } - const matches = scoped.filter((file) => file.name === fileName) + const { files: matches, nextKeys } = await findInScope( + { name: fileName }, + MAX_LISTED_NAME_MATCHES + ) if (matches.length === 0) { throw new OrchestrationError('not_found', `No file named ${fileName} in ${scopeLabel}`) } /* * A recursive scope can hold the same name at several depths, and writing to - * whichever the walk happened to reach first is a silent write to an - * arbitrary file. Refusing names the candidates so the caller can pick one, - * which is the whole reason the scope exists. + * whichever comes first is a silent write to an arbitrary file. Refusing + * names the candidates so the caller can pick one. */ if (matches.length > 1) { + const count = nextKeys ? `More than ${matches.length}` : `${matches.length}` throw new OrchestrationError( 'validation', - `${matches.length} files named ${fileName} under ${scopeLabel}: ${matches + `${count} files named ${fileName} under ${scopeLabel}: ${matches .map((file) => file.id) .join(', ')}. Give the file ID, name a deeper folder, or set includeSubfolders to false.` ) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 92cf3008363..243cc42266e 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -1759,6 +1759,10 @@ export interface QueryWorkspaceFilesOptions { folderId?: string | null | readonly string[] /** A resolved union of folder ids and workspace-root files. */ folderScope?: FolderIdScope + /** Exact file id. */ + fileId?: string + /** Exact file name. */ + name?: string /** Case-insensitive substring match on the file name. */ search?: string sortBy: V2FileSortBy @@ -1833,6 +1837,8 @@ async function queryFileRecords( scope = 'active', folderId, folderScope, + fileId, + name, search, sortBy, sortOrder, @@ -1861,6 +1867,8 @@ async function queryFileRecords( : isNull(workspaceFiles.deletedAt), workspaceFileFolderCondition(folderId), workspaceFileFolderScopeCondition(folderScope), + fileId === undefined ? undefined : eq(workspaceFiles.id, fileId), + name === undefined ? undefined : eq(workspaceFiles.originalName, name), searchFilter(workspaceFiles.originalName, search), resumeAfter, ] diff --git a/apps/sim/lib/workspace-files/application/list-workspace-files.ts b/apps/sim/lib/workspace-files/application/list-workspace-files.ts index 1df52e43c46..dcb15cfe7ee 100644 --- a/apps/sim/lib/workspace-files/application/list-workspace-files.ts +++ b/apps/sim/lib/workspace-files/application/list-workspace-files.ts @@ -31,6 +31,10 @@ export interface QueryWorkspaceFilePageInput { * default — see the v2 route, where a `search` implies a recursive look. */ recursive?: boolean + /** Exact file id. */ + fileId?: string + /** Exact file name. */ + name?: string search?: string sortBy: 'name' | 'size' | 'uploadedAt' | 'updatedAt' sortOrder: 'asc' | 'desc' @@ -118,6 +122,8 @@ export const queryWorkspaceFilePage = defineAuthorizedWorkspaceFileUseCase({ scope: input.scope, folderId, folderScope: input.folderScope, + fileId: input.fileId, + name: input.name, search: input.search, sortBy: input.sortBy, sortOrder: input.sortOrder, From 74ca9e339a721f1eb4b9d4bbaf18f76284d5f434 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:28:52 -0700 Subject: [PATCH 04/57] fix(files): treat the root with its subfolders as unscoped instead of listing every folder id A root scope including subfolders expanded to every active folder id, so file listing and content search sent an IN list of up to every folder in the workspace. It now resolves to no folder scope, which selects the same files: archiving a folder archives the files in it. The two identical folder-scope predicate builders become one workspaceFileFolderScopeCondition. (cherry picked from commit 8020c141e6462cb2c97dfe2c992225ed3baad3ab) --- apps/sim/lib/internal/file/operations.ts | 2 +- .../workspace/workspace-file-manager.ts | 15 +++----- .../folder-path-selection.test.ts | 35 ++++++++++++------- .../workspace-files/folder-path-selection.ts | 20 +++++++---- apps/sim/lib/workspace-files/query-scope.ts | 21 ++++++++++- .../workspace-files/resolve-folder-scope.ts | 7 ++-- .../lib/workspace-files/search/delivery.ts | 2 +- .../lib/workspace-files/search/repository.ts | 29 +++------------ 8 files changed, 70 insertions(+), 61 deletions(-) diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index 7957eeab351..1db822f49ea 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -2196,7 +2196,7 @@ export async function executeFileManageOperation( if (selection.missingPath !== undefined) { throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`) } - rootId = [...selection.folderIds][0] ?? null + rootId = [...(selection.scope?.folderIds ?? [])][0] ?? null } const maxDepth = body.recursive ? (body.depth ?? Number.POSITIVE_INFINITY) : 1 diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 243cc42266e..b8b01bde915 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -126,7 +126,10 @@ import { MAX_SIM_PAGE_UPLOAD_SNIFF_BYTES, restoreSimPageSourceBuffer, } from '@/lib/workspace-files/page-source-embed' -import { contentWritableWorkspaceFileContextCondition } from '@/lib/workspace-files/query-scope' +import { + contentWritableWorkspaceFileContextCondition, + workspaceFileFolderScopeCondition, +} from '@/lib/workspace-files/query-scope' import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils' import { isUuid } from '@/executor/constants' import type { UserFile } from '@/executor/types' @@ -1788,16 +1791,6 @@ function workspaceFileFolderCondition( return eq(workspaceFiles.folderId, folderId as string) } -/** The SQL predicate for a folder scope that can include both ids and root files. */ -function workspaceFileFolderScopeCondition(scope: FolderIdScope | undefined): SQL | undefined { - if (!scope) return undefined - const ids = [...scope.folderIds] - const inScope = ids.length > 0 ? inArray(workspaceFiles.folderId, ids) : undefined - const atRoot = scope.includeRootItems ? isNull(workspaceFiles.folderId) : undefined - if (inScope && atRoot) return or(inScope, atRoot) - return inScope ?? atRoot ?? sql`false` -} - /** * One filtered, sorted, bounded page of a workspace's files. * diff --git a/apps/sim/lib/workspace-files/folder-path-selection.test.ts b/apps/sim/lib/workspace-files/folder-path-selection.test.ts index 4dfa6dbc384..ffb14628fe7 100644 --- a/apps/sim/lib/workspace-files/folder-path-selection.test.ts +++ b/apps/sim/lib/workspace-files/folder-path-selection.test.ts @@ -38,33 +38,33 @@ describe('resolveFolderIdsForPaths', () => { it("keeps one user's subtree out of another's scope", () => { const result = resolveFolderIdsForPaths(memory, ['/memory/user-a']) - expect([...(result.folderIds ?? [])].sort()).toEqual(['a', 'a-commitments', 'a-people']) + expect([...(result.scope?.folderIds ?? [])].sort()).toEqual(['a', 'a-commitments', 'a-people']) }) it('does not let a shared name prefix widen the scope', () => { const result = resolveFolderIdsForPaths(memory, ['/memory/user-a']) - expect(result.folderIds?.has('a2')).toBe(false) - expect(result.folderIds?.has('a2-people')).toBe(false) + expect(result.scope?.folderIds?.has('a2')).toBe(false) + expect(result.scope?.folderIds?.has('a2-people')).toBe(false) }) it('never climbs to a parent of the requested folder', () => { const result = resolveFolderIdsForPaths(memory, ['/memory/user-a/people']) - expect([...(result.folderIds ?? [])]).toEqual(['a-people']) + expect([...(result.scope?.folderIds ?? [])]).toEqual(['a-people']) }) it('matches a folder whose name contains a slash', () => { const result = resolveFolderIdsForPaths(folders, ['/Reports/Q3%2FQ4']) - expect([...(result.folderIds ?? [])]).toEqual(['slashy']) + expect([...(result.scope?.folderIds ?? [])]).toEqual(['slashy']) }) it('reports a path that matches nothing rather than reading less', () => { const result = resolveFolderIdsForPaths(folders, ['/Reports', '/Nope']) expect(result.missingPath).toBe('/Nope') - expect(result.folderIds).toBeUndefined() + expect(result.scope?.folderIds).toBeUndefined() }) }) @@ -84,7 +84,7 @@ describe('either path spelling resolves', () => { it('still tells a slash in a name from a level separator', () => { expect([ - ...(resolveFolderIdsForPaths(canonical, ['/Reports/Q3%2FQ4']).folderIds ?? []), + ...(resolveFolderIdsForPaths(canonical, ['/Reports/Q3%2FQ4']).scope?.folderIds ?? []), ]).toEqual(['slashy']) }) }) @@ -116,16 +116,27 @@ describe('isFileInFolderScope', () => { */ describe('the workspace root', () => { it('selects files that carry no folder id', () => { - const result = resolveFolderIdsForPaths(folders, ['/']) + const result = resolveFolderIdsForPaths(folders, ['/'], { includeSubfolders: false }) expect(result.missingPath).toBeUndefined() - expect(result.includeRootItems).toBe(true) + expect(result.scope?.includeRootItems).toBe(true) + }) + + it('with its subfolders is the whole workspace, not a list of every folder', () => { + const result = resolveFolderIdsForPaths(folders, ['/', '/Reports']) + + expect(result.missingPath).toBeUndefined() + expect(result.scope).toBeUndefined() + }) + + it('still reports a missing path listed beside it', () => { + expect(resolveFolderIdsForPaths(folders, ['/', '/Nope']).missingPath).toBe('/Nope') }) it('is not implied by an ordinary folder scope', () => { const result = resolveFolderIdsForPaths(folders, ['/Reports']) - expect(result.includeRootItems).toBe(false) + expect(result.scope?.includeRootItems).toBe(false) }) }) @@ -138,8 +149,8 @@ describe('isFileInFolderScope and the root', () => { it('agrees with the id resolver for the same scope', () => { const shallowRoot = resolveFolderIdsForPaths(folders, ['/'], { includeSubfolders: false }) - expect(shallowRoot.includeRootItems).toBe(true) - expect([...(shallowRoot.folderIds ?? [])]).toEqual([]) + expect(shallowRoot.scope?.includeRootItems).toBe(true) + expect([...(shallowRoot.scope?.folderIds ?? [])]).toEqual([]) expect(isFileInFolderScope('Reports', '/', { includeSubfolders: false })).toBe(false) }) }) diff --git a/apps/sim/lib/workspace-files/folder-path-selection.ts b/apps/sim/lib/workspace-files/folder-path-selection.ts index a271644ae3f..399af5fb998 100644 --- a/apps/sim/lib/workspace-files/folder-path-selection.ts +++ b/apps/sim/lib/workspace-files/folder-path-selection.ts @@ -15,9 +15,14 @@ export interface SelectableFolder { path: string } +/** + * A resolved selection. `scope` is `undefined` when the selection covers every + * file in the workspace (the root with its subfolders), matching how the query + * builders spell "no folder filter". + */ export type FolderPathSelection = - | (FolderIdScope & { missingPath?: undefined }) - | { folderIds?: undefined; includeRootItems?: undefined; missingPath: string } + | { scope: FolderIdScope | undefined; missingPath?: undefined } + | { scope?: undefined; missingPath: string } /** * Resolves canonical folder paths to the folder ids a run should read from. @@ -38,7 +43,9 @@ export type FolderPathSelection = * in `folderIds`, because the root is the *absence* of a folder id: a file at * the root carries `null`, so there is no id to match. Encoding it as a * sentinel string in a set of real ids would survive every type check and then - * match nothing the first time the set reached a SQL `in (...)`. + * match nothing the first time the set reached a SQL `in (...)`. The root with + * its subfolders is every file, so it resolves to no scope rather than to a list + * of every folder id. */ export function resolveFolderIdsForPaths( folders: readonly SelectableFolder[], @@ -49,6 +56,7 @@ export function resolveFolderIdsForPaths( const maxDepth = includeSubfolders ? undefined : 0 const folderIds = new Set() let includeRootItems = false + let coversWorkspace = false for (const folderPath of folderPaths) { const segments = parseFolderPath(folderPath) @@ -60,9 +68,7 @@ export function resolveFolderIdsForPaths( */ if (segments.length === 0) { includeRootItems = true - if (includeSubfolders) { - for (const folder of folders) folderIds.add(folder.id) - } + if (includeSubfolders) coversWorkspace = true continue } @@ -81,7 +87,7 @@ export function resolveFolderIdsForPaths( } } - return { folderIds, includeRootItems } + return { scope: coversWorkspace ? undefined : { folderIds, includeRootItems } } } /** diff --git a/apps/sim/lib/workspace-files/query-scope.ts b/apps/sim/lib/workspace-files/query-scope.ts index 4470f572ce6..428916a747e 100644 --- a/apps/sim/lib/workspace-files/query-scope.ts +++ b/apps/sim/lib/workspace-files/query-scope.ts @@ -1,5 +1,6 @@ import { workspaceFiles } from '@sim/db/schema' -import { eq, inArray, isNull } from 'drizzle-orm' +import { eq, inArray, isNull, or, type SQL, sql } from 'drizzle-orm' +import type { FolderIdScope } from '@/lib/folders/scope' import { OWNED_FILE_CONTEXTS } from '@/lib/workspace-files/owned-files' /** Durable files available to workspace resource pickers, reference mappings, and fork copies. */ @@ -16,3 +17,21 @@ export const contentWritableWorkspaceFileContextCondition = inArray(workspaceFil 'workspace', ...OWNED_FILE_CONTEXTS, ]) + +/** + * The `workspaceFiles` predicate for a resolved folder scope, or none when the + * scope is absent. A root scope selects the files that carry no folder id, so + * the two halves are OR-ed. An empty scope selects nothing: every path resolved + * to a folder holding no subtree, and answering it unrestricted would widen it + * to the whole workspace. + */ +export function workspaceFileFolderScopeCondition( + scope: FolderIdScope | undefined +): SQL | undefined { + if (!scope) return undefined + const ids = [...scope.folderIds] + const inScope = ids.length > 0 ? inArray(workspaceFiles.folderId, ids) : undefined + const atRoot = scope.includeRootItems ? isNull(workspaceFiles.folderId) : undefined + if (inScope && atRoot) return or(inScope, atRoot) + return inScope ?? atRoot ?? sql`false` +} diff --git a/apps/sim/lib/workspace-files/resolve-folder-scope.ts b/apps/sim/lib/workspace-files/resolve-folder-scope.ts index 9e17a5300e5..53cd8907f55 100644 --- a/apps/sim/lib/workspace-files/resolve-folder-scope.ts +++ b/apps/sim/lib/workspace-files/resolve-folder-scope.ts @@ -20,14 +20,15 @@ import { resolveFolderIdsForPaths } from '@/lib/workspace-files/folder-path-sele * * Folders only. Callers that need the files filter their own listing against * the result; the search path pushes it into SQL instead, which is why the - * folder half is resolved separately from the file half. + * folder half is resolved separately from the file half. Returns `undefined` + * when the paths cover the whole workspace. */ export async function resolveWorkspaceFolderScope(args: { principal: Principal workspaceId: string folderPaths: readonly string[] includeSubfolders: boolean | undefined -}): Promise { +}): Promise { const { folders } = await listWorkspaceFileFoldersOperation.execute({ principal: args.principal, input: { workspaceId: args.workspaceId }, @@ -45,5 +46,5 @@ export async function resolveWorkspaceFolderScope(args: { throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`) } - return { folderIds: selection.folderIds, includeRootItems: selection.includeRootItems } + return selection.scope } diff --git a/apps/sim/lib/workspace-files/search/delivery.ts b/apps/sim/lib/workspace-files/search/delivery.ts index b4d19e18cc8..8efb27f55b2 100644 --- a/apps/sim/lib/workspace-files/search/delivery.ts +++ b/apps/sim/lib/workspace-files/search/delivery.ts @@ -41,7 +41,7 @@ export function resolveFileSearchFolderScope( ) if (selection.missingPath !== undefined) throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`) - return { folderIds: selection.folderIds, includeRootItems: selection.includeRootItems } + return selection.scope } /** diff --git a/apps/sim/lib/workspace-files/search/repository.ts b/apps/sim/lib/workspace-files/search/repository.ts index 6d50fcd6c8b..74a7c2d49c0 100644 --- a/apps/sim/lib/workspace-files/search/repository.ts +++ b/apps/sim/lib/workspace-files/search/repository.ts @@ -1,11 +1,12 @@ import { dbFor } from '@sim/db' import { fileSearchDependency, workspaceFileSearchRevision, workspaceFiles } from '@sim/db/schema' import { getPostgresErrorCode } from '@sim/utils/errors' -import { and, eq, inArray, isNull, or, type SQL, type SQLWrapper, sql } from 'drizzle-orm' +import { and, eq, inArray, isNull, type SQLWrapper, sql } from 'drizzle-orm' import type { DbTransaction } from '@/lib/db/types' import type { FolderIdScope } from '@/lib/folders/scope' import type { WorkspaceFileSecretProvenanceIdentity } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import type { EditableFileOwner } from '@/lib/workspace-files/ownership' +import { workspaceFileFolderScopeCondition } from '@/lib/workspace-files/query-scope' import { fileSearchAdmission } from '@/lib/workspace-files/search/admission' import { probeFileSearchCandidates, @@ -150,22 +151,6 @@ function toPreviewRange( return alignToCodePoints(content, { start, end: units }) } -/** - * The `workspaceFiles` predicate for a resolved folder scope. - * - * A root scope selects the files that carry no folder id, so the two halves are - * OR-ed rather than folded into one list. An empty scope selects nothing: it - * means every path resolved to a folder holding no subtree, and answering it - * with an unrestricted search would leak the whole workspace. - */ -function buildFolderPredicate(scope: FolderIdScope): SQL | undefined { - const ids = [...scope.folderIds] - const inScope = ids.length > 0 ? inArray(workspaceFiles.folderId, ids) : undefined - const atRoot = scope.includeRootItems ? isNull(workspaceFiles.folderId) : undefined - if (inScope && atRoot) return or(inScope, atRoot) - return inScope ?? atRoot ?? sql`false` -} - type SearchLine = { lineNumber: number content: string @@ -238,14 +223,8 @@ export async function searchFileIndex({ ) } - /** - * The chunk table carries no folder id, so a scope has to travel through - * the `workspaceFiles` join both queries already make. A scope that resolved - * to nothing must match nothing: `inArray` with an empty list would be a - * SQL error, and omitting the predicate would silently search everything, - * which for a per-user folder tree is a cross-user read. - */ - const folderPredicate = folderScope ? buildFolderPredicate(folderScope) : undefined + /** The chunk table carries no folder id, so a scope travels through the `workspaceFiles` join. */ + const folderPredicate = workspaceFileFolderScopeCondition(folderScope) return fileSearchAdmission.run( `${owner.entityType}:${owner.entityId}`, From 14f8a5ec5d97fcc09365296ab9b776f5ce26f372 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:30:03 -0700 Subject: [PATCH 05/57] chore(files): drop history narration and overstated folder-scope claims, log context kind only Folder scope narrows a search; authorization is workspace-level, so the comments no longer call it an isolation boundary. A failed chat context now logs its kind rather than the whole context, which for a file selection carries the user's selected text. The overlap-dedupe docstring keeps its invariants. (cherry picked from commit 4c22ff11d982a0f57156746f6d07bce584b1d587) --- .../lib/mothership/chat/process-contents.ts | 2 +- .../resources/resolvers.test.ts | 26 ++++------- .../search-replace/resources/resolvers.ts | 46 ++++--------------- .../search-workspace-file-content.ts | 11 ++--- .../folder-path-selection.test.ts | 8 ++-- 5 files changed, 28 insertions(+), 65 deletions(-) diff --git a/apps/sim/lib/mothership/chat/process-contents.ts b/apps/sim/lib/mothership/chat/process-contents.ts index 047d571b2b6..6e4f456b83a 100644 --- a/apps/sim/lib/mothership/chat/process-contents.ts +++ b/apps/sim/lib/mothership/chat/process-contents.ts @@ -429,7 +429,7 @@ export async function processContextsServer( ? { ...resolved, content: `Workspace ${workspaceId}:\n${resolved.content}` } : resolved } catch (error) { - logger.error('Failed processing context (server)', { ctx, error }) + logger.error('Failed processing context (server)', { kind: ctx.kind, error }) return null } } diff --git a/apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts b/apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts index 13ec0bba1a1..720cacbb2cf 100644 --- a/apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts +++ b/apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts @@ -118,10 +118,9 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => { }) /** - * The bucketed dedupe replaced an O(n^2) linear rescan. This pins it to a - * transcription of the original algorithm over randomized inputs, so any - * divergence in which overlapping match wins shows up as a diff rather than - * as a subtly wrong result the fixed examples above would miss. + * Pins the bucketed dedupe to a plain linear-scan oracle over randomized + * inputs, so any divergence in which overlapping match wins shows up as a + * diff rather than as a subtly wrong result the fixed examples would miss. */ describe('bucketed dedupe matches the original linear scan', () => { function scopeKey(match: WorkflowSearchMatch): string | null { @@ -145,7 +144,7 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => { return false } - /** Straight transcription of the pre-optimization implementation. */ + /** The plain linear-scan dedupe, as an oracle. */ function referenceDedupe(matches: WorkflowSearchMatch[]): WorkflowSearchMatch[] { const deduped: WorkflowSearchMatch[] = [] for (const match of matches) { @@ -215,11 +214,7 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => { }) } - /** - * A sequential sweep, not a handful of hand-picked seeds. An earlier version - * pinned 8 seeds that happened to be clean while 7.7% of the space diverged, - * so the count is what gives this test its power - keep it wide. - */ + /** A wide sequential sweep: a handful of hand-picked seeds can all be clean while divergences hide elsewhere. */ it('agrees with the original scan across 400 seeded inputs', () => { const diverged: number[] = [] @@ -234,9 +229,9 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => { }) /** - * The exact shape that broke the `maxEnd` short-circuit: a shorter range - * evicts a longer one but ends further right, so a stale `maxEnd` let the - * next match skip the overlap scan and leak through as a duplicate. + * A shorter range evicts a longer one but ends further right; a stale + * `maxEnd` would let the next match skip the overlap scan and leak through + * as a duplicate. */ it.each([ { @@ -297,9 +292,8 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => { /** * Pins the asymptotics, not a stopwatch. 20k disjoint hits in one scope run - * in single-digit ms bucketed; the O(n^2) rescan this replaced took ~30s on - * the same input, so the bound has roughly three orders of magnitude of - * headroom and only trips on a genuine complexity regression. + * in single-digit ms; a quadratic rescan takes ~30s on the same input, so + * the bound only trips on a genuine complexity regression. */ it('stays sub-quadratic on a single scope full of disjoint ranges', () => { const matches = Array.from({ length: 20_000 }, (_, index) => diff --git a/apps/sim/lib/workflows/search-replace/resources/resolvers.ts b/apps/sim/lib/workflows/search-replace/resources/resolvers.ts index b75490b6d51..adc7f89ff9c 100644 --- a/apps/sim/lib/workflows/search-replace/resources/resolvers.ts +++ b/apps/sim/lib/workflows/search-replace/resources/resolvers.ts @@ -136,44 +136,18 @@ function widenScopeBucket(bucket: RangeMatchScopeBucket, end: number): void { } /** - * Overlap resolution is scoped to one value inside one subblock, so candidates - * are bucketed by that scope rather than rescanned. The previous `findIndex` - * over the whole accumulated list recomputed every candidate's scope key on - * every iteration - O(n^2) string builds, which cost ~1.4s on a workflow - * producing ~500 matches and froze the search field while typing. + * Collapses overlapping range matches, keeping the preferred one per overlap. * - * A bucket only ever holds entries that have both a scope key and a range, and - * those are exactly the entries the old predicate could match. Buckets keep - * insertion order and the scan stops at the first overlap, so this picks the - * same candidate the linear scan did. + * Overlap is only possible within one value of one subblock, so kept entries + * are bucketed by that scope key, in insertion order; the first overlapping + * entry in the bucket is the one compared against. * - * `maxEnd` is a monotonic high-water mark, not the exact current maximum: a - * replacement can swap in a range that ends earlier without lowering it. Only - * the upper bound is load-bearing. A match starting at or after it cannot - * overlap anything in the bucket, so the scan is skipped; a bound left too high - * only costs a scan that would have been skipped, never a wrong answer. - * - * Recomputing the exact maximum on every shrinking replacement is a net loss - - * it walks the bucket, which is the cost this is here to avoid, and staleness - * is capped at one token length because every range spans a matched token - * (`query.length`, or a reference's `rawValue.length`) rather than the field. - * Measured on the realistic overlap shape at 10k matches: 23ms as written, - * 45ms with the recompute, against 1010ms for the scan this replaced. - * - * The bound keeps a field full of disjoint hits linear instead of quadratic - * within its own bucket, to the extent its matches arrive in ascending offset - * order; out-of-order producers just fall back to scanning. - * - * It must be refreshed on the replacement path too, not only on append: - * `shouldPreferOverlappingMatch` prefers the SHORTER range, and a shorter range - * can still end further right than the one it evicts. Leaving `maxEnd` stale - * there let the short-circuit skip genuine overlaps and leak duplicates. - * - * Only `NaN` ends are ignored. `Math.max` with `NaN` would pin `maxEnd` at - * `NaN`, and since every comparison against `NaN` is false that would silently - * switch dedupe off for the rest of the scope. A `NaN`-ended range cannot - * overlap anything anyway, while positive infinity is an unbounded end that - * can overlap later ranges and therefore must widen the high-water mark. + * Each bucket's `maxEnd` is an upper bound on its entries' `range.end`, not the + * exact maximum: a match starting at or after it cannot overlap anything in the + * bucket, so the scan is skipped. It must be widened on replacement as well as + * on append, because a shorter preferred range can still end further right. + * `NaN` ends are ignored (they overlap nothing and would poison `Math.max`); + * `+Infinity` widens it. */ export function dedupeOverlappingWorkflowSearchMatches( matches: T[] diff --git a/apps/sim/lib/workspace-files/application/search-workspace-file-content.ts b/apps/sim/lib/workspace-files/application/search-workspace-file-content.ts index c132784cba7..83c3cdf52f3 100644 --- a/apps/sim/lib/workspace-files/application/search-workspace-file-content.ts +++ b/apps/sim/lib/workspace-files/application/search-workspace-file-content.ts @@ -54,14 +54,9 @@ export const searchWorkspaceFileContent = defineAuthorizedWorkspaceFileUseCase({ signal?.throwIfAborted() /** * Resolved here rather than at the surface so every caller (the File - * block, the v2 route) is confined by the same check. A folder tree - * holding one subtree per user makes this scope the isolation boundary, - * not a convenience filter. - */ - /** - * `!== undefined`, not a length check: an explicitly empty list is a scope - * that names no folder, which must match nothing. Treating it as "absent" - * would answer a request for nothing with the whole workspace. + * block, the v2 route) narrows by the same rules. `!== undefined`, not a + * length check: an explicitly empty list is a scope that names no folder, + * which must match nothing rather than the whole workspace. */ const folderScope = input.folderPaths !== undefined diff --git a/apps/sim/lib/workspace-files/folder-path-selection.test.ts b/apps/sim/lib/workspace-files/folder-path-selection.test.ts index ffb14628fe7..4ad5383eaec 100644 --- a/apps/sim/lib/workspace-files/folder-path-selection.test.ts +++ b/apps/sim/lib/workspace-files/folder-path-selection.test.ts @@ -19,10 +19,10 @@ const folders = [ describe('resolveFolderIdsForPaths', () => { /* - * A per-user memory tree makes the scope an isolation boundary rather than a - * filter, so the sibling cases below are the ones that matter: `user-a` and - * `user-a-2` share a prefix, and a resolver comparing raw strings instead of - * decoded segments would hand one user the other's notes. + * In a per-user memory tree the scope is what picks one user's notes, so the + * sibling cases below are the ones that matter: `user-a` and `user-a-2` share + * a prefix, and a resolver comparing raw strings instead of decoded segments + * would read the other user's notes. */ const memory = [ { id: 'memory', parentId: null, path: 'memory' }, From a5b5c16731aca37c44df0f24123a8b5617bb5eb6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:32:07 -0700 Subject: [PATCH 06/57] refactor(files): move the anchored-edit mapper out of the intent store, probe the id alongside folder resolution (cherry picked from commit 0df6468f6372168cfbe9f3668eae213cee091b6c) --- apps/sim/lib/internal/file/operations.ts | 51 ++++++++++--------- .../tools/server/files/anchored-edit.ts | 42 +++++++++++++++ .../tools/server/files/edit-content.ts | 3 +- .../tools/server/files/file-intent-store.ts | 41 --------------- .../tools/server/files/file-preview.ts | 2 +- .../folder-path-selection.test.ts | 4 +- 6 files changed, 75 insertions(+), 68 deletions(-) create mode 100644 apps/sim/lib/mothership/tools/server/files/anchored-edit.ts diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index 1db822f49ea..e45dbd26d19 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -948,12 +948,33 @@ async function resolveScopedFileReference(args: { if (folderPaths.length === 0) return fileName const scopeLabel = folderPaths.join(', ') - const folderScope = await resolveWorkspaceFolderScope({ - principal, - workspaceId, - folderPaths, - includeSubfolders: args.includeSubfolders, - }) + /* + * A reference that is a real file id resolves to that file or to nothing, + * never to a name match: `wf_` is a legal filename prefix, so a file merely + * named like an id must not answer a caller who named a file exactly. Decided + * by looking the id up rather than by its shape. + */ + const fileIdExists = async () => { + try { + await readWorkspaceFileMetadata.execute({ + principal, + input: { fileId: fileName, assertedWorkspaceId: workspaceId }, + }) + return true + } catch (error) { + if (error instanceof OrchestrationError && error.code === 'not_found') return false + throw error + } + } + const [folderScope, exactIdExists] = await Promise.all([ + resolveWorkspaceFolderScope({ + principal, + workspaceId, + folderPaths, + includeSubfolders: args.includeSubfolders, + }), + fileIdExists(), + ]) const findInScope = (match: { fileId: string } | { name: string }, limit: number) => queryWorkspaceFilePage.execute({ principal, @@ -967,22 +988,6 @@ async function resolveScopedFileReference(args: { }, }) - /* - * A reference that is a real file id resolves to that file or to nothing, - * never to a name match: `wf_` is a legal filename prefix, so a file merely - * named like an id must not answer a caller who named a file exactly. Decided - * by looking the id up rather than by its shape. - */ - let exactIdExists = false - try { - await readWorkspaceFileMetadata.execute({ - principal, - input: { fileId: fileName, assertedWorkspaceId: workspaceId }, - }) - exactIdExists = true - } catch (error) { - if (!(error instanceof OrchestrationError) || error.code !== 'not_found') throw error - } if (exactIdExists) { const { files } = await findInScope({ fileId: fileName }, 1) if (files.length === 0) { @@ -2196,7 +2201,7 @@ export async function executeFileManageOperation( if (selection.missingPath !== undefined) { throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`) } - rootId = [...(selection.scope?.folderIds ?? [])][0] ?? null + rootId = selection.scope?.folderIds.values().next().value ?? null } const maxDepth = body.recursive ? (body.depth ?? Number.POSITIVE_INFINITY) : 1 diff --git a/apps/sim/lib/mothership/tools/server/files/anchored-edit.ts b/apps/sim/lib/mothership/tools/server/files/anchored-edit.ts new file mode 100644 index 00000000000..80cf3425c1c --- /dev/null +++ b/apps/sim/lib/mothership/tools/server/files/anchored-edit.ts @@ -0,0 +1,42 @@ +import { toNumberOrNull, toStringOrNull } from '@sim/utils/coerce' +import type { WorkspaceFileContentEdit } from '@/lib/workspace-files/edit-content' + +type AnchoredContentEdit = Exclude + +/** + * Maps a staged `anchored` patch onto the shared workspace-file edit, so the + * durable write and its streamed preview apply the same anchor semantics. + */ +export function toAnchoredContentEdit( + edit: Record, + content: string +): { edit: AnchoredContentEdit } | { error: string } { + const read = (key: string) => toStringOrNull(edit[key]) ?? '' + const occurrence = toNumberOrNull(edit.occurrence) ?? undefined + + switch (edit.mode) { + case 'replace_between': { + const beforeAnchor = read('before_anchor') + const afterAnchor = read('after_anchor') + if (!beforeAnchor || !afterAnchor) { + return { error: 'replace_between requires before_anchor and after_anchor' } + } + return { edit: { mode: 'replace_between', beforeAnchor, afterAnchor, content, occurrence } } + } + case 'insert_after': { + const anchor = read('anchor') + if (!anchor) return { error: 'insert_after requires anchor' } + return { edit: { mode: 'insert_after', anchor, content, occurrence } } + } + case 'delete_between': { + const startAnchor = read('start_anchor') + const endAnchor = read('end_anchor') + if (!startAnchor || !endAnchor) { + return { error: 'delete_between requires start_anchor and end_anchor' } + } + return { edit: { mode: 'delete_between', startAnchor, endAnchor, occurrence } } + } + default: + return { error: `Unknown anchored patch mode: "${String(edit.mode)}"` } + } +} diff --git a/apps/sim/lib/mothership/tools/server/files/edit-content.ts b/apps/sim/lib/mothership/tools/server/files/edit-content.ts index 7a8a7a4e68a..8dcf62ae32a 100644 --- a/apps/sim/lib/mothership/tools/server/files/edit-content.ts +++ b/apps/sim/lib/mothership/tools/server/files/edit-content.ts @@ -12,6 +12,7 @@ import { type BaseServerTool, type ServerToolContext, } from '@/lib/mothership/tools/server/base-tool' +import { toAnchoredContentEdit } from '@/lib/mothership/tools/server/files/anchored-edit' import { getE2BDocFormat } from '@/lib/uploads/documents/compile' import { updateWorkspaceFileContent } from '@/lib/workspace-files/application/update-workspace-file-content' import { applyWorkspaceFileContentEdit, EditContentError } from '@/lib/workspace-files/edit-content' @@ -23,7 +24,7 @@ import { SIM_PAGE_CONTENT_TYPE, } from '@/lib/workspace-files/page-compile' import { buildEmbeddedImageRefWarning } from './embedded-image-refs' -import { toAnchoredContentEdit, waitForLatestFileIntent } from './file-intent-store' +import { waitForLatestFileIntent } from './file-intent-store' import { compileDocForWrite, getDocumentFormatInfo, inferContentType } from './workspace-file' const logger = createLogger('EditContentServerTool') diff --git a/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts b/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts index 0e5684a06a6..30fe0c8be58 100644 --- a/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts +++ b/apps/sim/lib/mothership/tools/server/files/file-intent-store.ts @@ -3,7 +3,6 @@ import { toError } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' import { getRedisClient } from '@/lib/core/config/redis' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager' -import type { WorkspaceFileContentEdit } from '@/lib/workspace-files/edit-content' export type PendingFileIntent = { operation: 'append' | 'update' | 'patch' @@ -45,46 +44,6 @@ export type FileIntentScope = { channelId?: string } -type AnchoredContentEdit = Exclude - -/** - * Maps a staged `anchored` patch onto the shared workspace-file edit, so the - * durable write and its streamed preview apply the same anchor semantics. - */ -export function toAnchoredContentEdit( - edit: Record, - content: string -): { edit: AnchoredContentEdit } | { error: string } { - const read = (key: string) => (typeof edit[key] === 'string' ? (edit[key] as string) : '') - const occurrence = typeof edit.occurrence === 'number' ? edit.occurrence : undefined - - switch (edit.mode) { - case 'replace_between': { - const beforeAnchor = read('before_anchor') - const afterAnchor = read('after_anchor') - if (!beforeAnchor || !afterAnchor) { - return { error: 'replace_between requires before_anchor and after_anchor' } - } - return { edit: { mode: 'replace_between', beforeAnchor, afterAnchor, content, occurrence } } - } - case 'insert_after': { - const anchor = read('anchor') - if (!anchor) return { error: 'insert_after requires anchor' } - return { edit: { mode: 'insert_after', anchor, content, occurrence } } - } - case 'delete_between': { - const startAnchor = read('start_anchor') - const endAnchor = read('end_anchor') - if (!startAnchor || !endAnchor) { - return { error: 'delete_between requires start_anchor and end_anchor' } - } - return { edit: { mode: 'delete_between', startAnchor, endAnchor, occurrence } } - } - default: - return { error: `Unknown anchored patch mode: "${String(edit.mode)}"` } - } -} - const logger = createLogger('FileIntentStore') const INTENT_TTL_MS = 60 * 60 * 1000 diff --git a/apps/sim/lib/mothership/tools/server/files/file-preview.ts b/apps/sim/lib/mothership/tools/server/files/file-preview.ts index eb96654e25d..44413948724 100644 --- a/apps/sim/lib/mothership/tools/server/files/file-preview.ts +++ b/apps/sim/lib/mothership/tools/server/files/file-preview.ts @@ -3,7 +3,7 @@ import { toError } from '@sim/utils/errors' import { observeContentOutsideModel } from '@/lib/core/application' import { executeCopilotFileUseCase } from '@/lib/mothership/application/execute-file-use-case' import type { ExecutionContext } from '@/lib/mothership/request/types' -import { toAnchoredContentEdit } from '@/lib/mothership/tools/server/files/file-intent-store' +import { toAnchoredContentEdit } from '@/lib/mothership/tools/server/files/anchored-edit' import { readWorkspaceFileContent } from '@/lib/workspace-files/application/read-workspace-file-content' import { applyWorkspaceFileContentEdit, EditContentError } from '@/lib/workspace-files/edit-content' diff --git a/apps/sim/lib/workspace-files/folder-path-selection.test.ts b/apps/sim/lib/workspace-files/folder-path-selection.test.ts index 4ad5383eaec..f7a1c66b7ad 100644 --- a/apps/sim/lib/workspace-files/folder-path-selection.test.ts +++ b/apps/sim/lib/workspace-files/folder-path-selection.test.ts @@ -44,8 +44,8 @@ describe('resolveFolderIdsForPaths', () => { it('does not let a shared name prefix widen the scope', () => { const result = resolveFolderIdsForPaths(memory, ['/memory/user-a']) - expect(result.scope?.folderIds?.has('a2')).toBe(false) - expect(result.scope?.folderIds?.has('a2-people')).toBe(false) + expect(result.scope?.folderIds.has('a2')).toBe(false) + expect(result.scope?.folderIds.has('a2-people')).toBe(false) }) it('never climbs to a parent of the requested folder', () => { From f79bdc8ffe56fc8376f2e1f7e39dff1f6a860ed3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 11:26:20 -0700 Subject: [PATCH 07/57] fix(files-ui): stop folder-scope pruning writes, memoize resource folders, rename untitled files on heading exit - file-upload: drop the effect that pruned out-of-scope selections from every open editor; flag them for removal instead - use-resource-folders: memoize on stable query fields; fix byPath doc - rich-markdown-editor: name an untitled file after its heading when the cursor leaves it, on blur, or on unmount (once), not after an idle pause; caller resolves the file by id - table-grid: hoist guarded context row instead of non-null assertions - drop a formula-restating test and trim a home.tsx comment (cherry picked from commit 419520a1e42c3462b31ccbf2e68f7f8d6af4ae9d) --- .../components/file-viewer/file-viewer.tsx | 7 +- .../editor-lifecycle.test.tsx | 30 +++++++- .../rich-markdown-editor.tsx | 76 ++++++++++--------- .../rich-markdown-editor/title-heading.ts | 7 ++ .../workspace/[workspaceId]/files/files.tsx | 10 ++- .../app/workspace/[workspaceId]/home/home.tsx | 5 +- .../components/table-grid/table-grid.tsx | 26 +++---- .../components/table-grid/utils.test.ts | 13 +--- .../components/file-upload/file-upload.tsx | 47 ++++++------ .../sub-block/hooks/use-resource-folders.ts | 32 +++++--- scripts/check-explicit-any.baseline.json | 2 +- 11 files changed, 145 insertions(+), 110 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx index 5638255d2e0..c1e9e519f12 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx @@ -142,10 +142,11 @@ interface FileViewerProps { */ collaborative?: boolean /** - * Called (debounced) with the markdown document's leading-heading text while the file is still - * untitled, so the caller can name the file after it. Only wired for the editable markdown editor. + * Called with the file id and the markdown document's leading-heading text once the user finishes + * that heading while the file is still untitled, so the caller can name the file after it. Only wired + * for the editable markdown editor. */ - onDeriveTitleFromHeading?: (headingText: string) => void + onDeriveTitleFromHeading?: (fileId: string, headingText: string) => void /** * Let an open markdown file claim Cmd/Ctrl+F for find-in-document. Set wherever the file is the * whole pane the user is reading — the Files page, the mothership file view, the public share diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx index 95d66c457af..1919f492aa0 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx @@ -150,6 +150,8 @@ class FakeFileDocProvider { } interface RenderOptions { + file?: WorkspaceFileRecord + onDeriveTitleFromHeading?: (fileId: string, headingText: string) => void collaborative?: boolean downloadSourceRef?: { current: FileDownloadSource | null } isStreaming?: boolean @@ -170,7 +172,7 @@ async function render( root.render( @@ -471,4 +474,29 @@ describe('loaded rich editor lifecycle', () => { expect(abandonedOnChange).not.toHaveBeenCalled() expect(abandonedSave).not.toHaveBeenCalled() }) + + it('names an untitled file after its heading only once the cursor leaves the heading', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + try { + const onDeriveTitleFromHeading = vi.fn() + await render('# My Pro', '# My Pro', true, { + file: { ...FILE, name: 'untitled.md' }, + onDeriveTitleFromHeading, + }) + const editor = getEditor() + await act(async () => { + editor.commands.setTextSelection(editor.state.doc.content.size - 1) + editor.commands.insertContent('ject') + }) + await act(async () => vi.advanceTimersByTime(5_000)) + expect(onDeriveTitleFromHeading).not.toHaveBeenCalled() + + await act(async () => editor.commands.splitBlock()) + await act(async () => editor.commands.insertContent('Body')) + expect(onDeriveTitleFromHeading).toHaveBeenCalledOnce() + expect(onDeriveTitleFromHeading).toHaveBeenCalledWith(FILE.id, 'My Project') + } finally { + vi.useRealTimers() + } + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index cfed0501286..24df90a59ba 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -75,7 +75,10 @@ import { LinkHoverCard } from '@/app/workspace/[workspaceId]/files/components/fi import { TableBubbleMenu } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/table-menu' import { normalizeMarkdownContent } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/normalize-content' import { isRoundTripSafe } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip-safety' -import { firstHeadingTitle } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading' +import { + firstHeadingTitle, + isSelectionInLeadingHeading, +} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading' import { TextEditor } from '@/app/workspace/[workspaceId]/files/components/file-viewer/text-editor' import { useEditableFileContent } from '@/app/workspace/[workspaceId]/files/components/file-viewer/use-editable-file-content' import { useSelectionCopyBridge } from '@/app/workspace/[workspaceId]/files/components/file-viewer/use-selection-copy-bridge' @@ -100,9 +103,6 @@ const EXTENSIONS = createMarkdownEditorExtensions({ const STREAM_REPARSE_THROTTLE_THRESHOLD = 40_000 const STREAM_REPARSE_THROTTLE_MS = 120 -/** Debounce before naming a still-untitled file after its leading heading, so it fires once typing settles. */ -const DERIVE_TITLE_DEBOUNCE_MS = 600 - function warnRichMarkdownPasteLimit(reason?: 'paste' | 'formatting') { if (reason === 'formatting') { toast.warning('Pasted text kept without automatic formatting', { @@ -245,11 +245,12 @@ interface RichMarkdownEditorProps { */ collaborative?: boolean /** - * Called (debounced) with the document's leading-heading text while the file is still untitled, so the - * caller can name the file after it. Omitted on read-only/non-editable surfaces. See - * {@link isUntitledName}. + * Called at most once per mount with the file id and the document's leading-heading text, when the + * local user finishes that heading (the cursor leaves it, the editor blurs, or it unmounts) while the + * file is still untitled, so the caller can name the file after it. Omitted on read-only/non-editable + * surfaces. See {@link isUntitledName}. */ - onDeriveTitleFromHeading?: (headingText: string) => void + onDeriveTitleFromHeading?: (fileId: string, headingText: string) => void /** * Claim Cmd/Ctrl+F for find-in-document. Off by default, because this editor also renders as a * preview pane beside something that owns the shortcut itself. Every find surface binds its own @@ -424,7 +425,7 @@ interface LoadedRichMarkdownEditorProps { /** Reports client autosave eligibility; collaborative documents are persisted by the relay. */ onClientAutosaveChange: (canAutosave: boolean) => void /** See {@link RichMarkdownEditorProps.onDeriveTitleFromHeading}. */ - onDeriveTitleFromHeading?: (headingText: string) => void + onDeriveTitleFromHeading?: (fileId: string, headingText: string) => void /** See {@link RichMarkdownEditorProps.enableFind}. */ enableFind: boolean onEditSource?: () => void @@ -586,13 +587,25 @@ export function LoadedRichMarkdownEditor({ const saveFrontmatterResolverRef = useRef(resolveSaveFrontmatter) /** - * While the file is still unnamed, name it after its leading heading: `onDeriveTitleFromHeading` is - * called (debounced) so the caller can rename the file, and `fileNameRef` lets the onUpdate handler - * read the current name without re-subscribing. See {@link isUntitledName}. + * While the file is still unnamed, name it after its leading heading once the local user finishes + * typing it — never on an idle pause, which would name the file after half a heading. A local edit + * marks the title pending; leaving the heading, blurring, or unmounting commits it, at most once per + * mount. `fileNameRef` lets the editor callbacks read the current name without re-subscribing. See + * {@link isUntitledName}. */ const onDeriveTitleFromHeadingRef = useRef(onDeriveTitleFromHeading) const fileNameRef = useRef(file.name) - const deriveTitleTimerRef = useRef | null>(null) + const headingTitlePendingRef = useRef(false) + const headingTitleCommittedRef = useRef(false) + const commitHeadingTitle = (editor: Editor) => { + if (!headingTitlePendingRef.current || headingTitleCommittedRef.current) return + headingTitlePendingRef.current = false + if (!editor.isEditable || !isUntitledName(fileNameRef.current)) return + const title = firstHeadingTitle(editor.state.doc) + if (!title) return + headingTitleCommittedRef.current = true + onDeriveTitleFromHeadingRef.current?.(file.id, title) + } /** * Read in the RAF tick so an already-scheduled tick still sees the latest edit kind (it can change * between sessions within one turn, e.g. an append followed by a rewrite). @@ -809,36 +822,25 @@ export function LoadedRichMarkdownEditor({ lastSyncedFrontmatterRef.current = saveFrontmatterResolverRef.current() onChangeRef.current(applyFrontmatter(lastSyncedFrontmatterRef.current, md)) } - // While the file is still untitled, name it after its leading heading once typing settles — but - // only for the LOCAL user's own edits. `isChangeOrigin` is true for a remote Yjs change (a peer - // typing); bail BEFORE touching the timer so a remote edit never cancels or reschedules the local - // user's pending rename (and every client doesn't schedule the same rename from a peer's - // not-yet-synced heading). It is false for local edits and non-collaborative surfaces. - if (isChangeOrigin(transaction)) return - // Local edit: restart the debounce. Clearing first cancels a stale rename if the heading was - // removed/changed before it fired; the timer re-derives the title from the live doc rather than a - // value captured now, so it can never name the file after a heading the user has since changed. + // Only the LOCAL user's edits mark the heading title pending: `isChangeOrigin` is true for a + // remote Yjs change, so every peer doesn't rename from someone else's half-synced heading. // `editor.isEditable` is the autosave gate (canEdit + settled + collab-ready), so a view-only - // viewer or the not-yet-editable mount seed never schedules a rename. - if (deriveTitleTimerRef.current) clearTimeout(deriveTitleTimerRef.current) - if ( - !editor.isEditable || - !isUntitledName(fileNameRef.current) || - firstHeadingTitle(editor.state.doc) === null - ) - return - deriveTitleTimerRef.current = setTimeout(() => { - const liveEditor = editorInstanceRef.current - if (!liveEditor || !liveEditor.isEditable || !isUntitledName(fileNameRef.current)) return - const title = firstHeadingTitle(liveEditor.state.doc) - if (title) onDeriveTitleFromHeadingRef.current?.(title) - }, DERIVE_TITLE_DEBOUNCE_MS) + // viewer or the not-yet-editable mount seed never marks it. The commit re-checks the name and + // heading. + if (isChangeOrigin(transaction)) return + if (editor.isEditable) headingTitlePendingRef.current = true + if (!isSelectionInLeadingHeading(editor.state)) commitHeadingTitle(editor) + }, + onSelectionUpdate: ({ editor }) => { + if (!isSelectionInLeadingHeading(editor.state)) commitHeadingTitle(editor) }, + onBlur: ({ editor }) => commitHeadingTitle(editor), }) useEffect( () => () => { - if (deriveTitleTimerRef.current) clearTimeout(deriveTitleTimerRef.current) + const liveEditor = editorInstanceRef.current + if (liveEditor && !liveEditor.isDestroyed) commitHeadingTitle(liveEditor) }, [] ) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading.ts index 3877519ca97..b36dfc94a36 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/title-heading.ts @@ -1,4 +1,5 @@ import type { Node as ProseMirrorNode } from '@tiptap/pm/model' +import type { EditorState } from '@tiptap/pm/state' /** The text of the document's leading heading (any level), or null when the first block isn't a heading. */ export function firstHeadingTitle(doc: ProseMirrorNode): string | null { @@ -7,3 +8,9 @@ export function firstHeadingTitle(doc: ProseMirrorNode): string | null { const text = first.textContent.trim() return text.length > 0 ? text : null } + +/** True while the selection head sits inside the document's leading heading. */ +export function isSelectionInLeadingHeading(state: EditorState): boolean { + const { $head } = state.selection + return $head.depth > 0 && $head.index(0) === 0 && state.doc.firstChild?.type.name === 'heading' +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index e786ee23518..b2e8d34cd92 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -550,12 +550,14 @@ function FilesContent() { /** * While a file is still untitled, name it after the leading heading the user types in its editor. The - * editor reports the heading text (debounced); here we re-check the file is still untitled, derive a - * unique `.md` name among its folder siblings, and rename. A no-op once the file has a real name. + * editor reports the heading once the user finishes it — possibly on unmount, after the route has + * already moved to another file, so the file is looked up by the id it reports rather than read from + * the current selection. Re-checks the file is still untitled, derives a unique `.md` name among its + * folder siblings, and renames. A no-op once the file has a real name. */ const handleDeriveTitleFromHeading = useCallback( - (headingText: string) => { - const currentFile = selectedFileRef.current + (fileId: string, headingText: string) => { + const currentFile = filesRef.current.find((f) => f.id === fileId) if (!currentFile || !isUntitledName(currentFile.name)) return const derived = deriveMarkdownFileName(headingText) if (!derived) return diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 59e7c41599a..9cbebd085ea 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -294,10 +294,7 @@ function HomeContent({ chatId, userName, userId }: HomeProps) { const contexts = handoff.contexts ?? [] for (const context of contexts) handleContextAdd(context) addMothershipContexts(contexts) - // `handleContextAdd` is a body function, so it is a new value every render; - // listing it would re-run this drain on every render. Omitted deliberately to - // keep it one-shot — and harmless either way, since `consume` clears the entry - // atomically and any re-run would find nothing. + // `handleContextAdd` is omitted: it is a new function every render. }, [chatId, workspaceId, prepareResourceViewForAgentTurn, sendMessage]) function resolveResourceFromContext( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 051f9cb9660..88681d3a975 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -4251,20 +4251,18 @@ export function TableGrid({ * count label and the multi-row "Run workflows" action. */ const contextMenuRowIds = useMemo(() => { - if (!contextMenu.isOpen || !contextMenu.row) return [] - if ( - !rowSelectionIsEmpty(rowSelection) && - rowSelectionIncludes(rowSelection, contextMenu.row.id) - ) { + const contextRow = contextMenu.row + if (!contextMenu.isOpen || !contextRow) return [] + if (!rowSelectionIsEmpty(rowSelection) && rowSelectionIncludes(rowSelection, contextRow.id)) { const ids: string[] = [] for (const row of rows) { if (rowSelectionIncludes(rowSelection, row.id)) ids.push(row.id) } - return ids.length > 0 ? ids : [contextMenu.row.id] + return ids.length > 0 ? ids : [contextRow.id] } const sel = normalizedSelection if (sel) { - const contextRowArrayIndex = rows.findIndex((r) => r.id === contextMenu.row!.id) + const contextRowArrayIndex = rows.findIndex((r) => r.id === contextRow.id) const isInSelection = contextRowArrayIndex >= sel.startRow && contextRowArrayIndex <= sel.endRow if (isInSelection) { @@ -4275,10 +4273,10 @@ export function TableGrid({ const row = rows[r] if (row) ids.push(row.id) } - return ids.length > 0 ? ids : [contextMenu.row.id] + return ids.length > 0 ? ids : [contextRow.id] } } - return [contextMenu.row.id] + return [contextRow.id] }, [contextMenu.isOpen, contextMenu.row, rowSelection, normalizedSelection, rows]) /** @@ -4328,16 +4326,14 @@ export function TableGrid({ * send every column (undefined). */ const contextMenuColumnIds = useMemo(() => { - if (!contextMenu.isOpen || !contextMenu.row) return undefined - if ( - !rowSelectionIsEmpty(rowSelection) && - rowSelectionIncludes(rowSelection, contextMenu.row.id) - ) { + const contextRow = contextMenu.row + if (!contextMenu.isOpen || !contextRow) return undefined + if (!rowSelectionIsEmpty(rowSelection) && rowSelectionIncludes(rowSelection, contextRow.id)) { return undefined } const sel = normalizedSelection if (!sel) return undefined - const contextRowArrayIndex = rows.findIndex((r) => r.id === contextMenu.row!.id) + const contextRowArrayIndex = rows.findIndex((r) => r.id === contextRow.id) if (contextRowArrayIndex < sel.startRow || contextRowArrayIndex > sel.endRow) return undefined // Collapsed here too (not only in buildTableSelectionContext) because this // also decides whether the menu item reads "cell range" or "rows". diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts index 45050a5f0ff..c539f3b1501 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { MAX_TABLE_SELECTION_ROWS } from '@/lib/mothership/chat/selection-context' -import { buildTableSelectionContext, drainTargetForChip } from './utils' +import { buildTableSelectionContext } from './utils' const rowIds = (count: number) => Array.from({ length: count }, (_, i) => `r${i}`) @@ -33,14 +33,3 @@ describe('buildTableSelectionContext', () => { expect(context.columnIds).toEqual(['c0', 'c1', 'c2']) }) }) - -describe('drainTargetForChip', () => { - it('still yields a full cap when every exclusion lands in the loaded prefix', () => { - // The worst case for a gutter select-all: exclusions are filtered out AFTER - // loading, so loading only the cap would leave the chip short of the count - // the menu already advertised. - const excluded = 30 - - expect(drainTargetForChip(excluded) - excluded).toBe(MAX_TABLE_SELECTION_ROWS) - }) -}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx index 4db94709dab..4565d9df1be 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useMemo, useRef, useState } from 'react' import { Button, Combobox, cn } from '@sim/emcn' import { X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' @@ -429,33 +429,27 @@ export function FileUpload({ [filesArray, workspaceFiles] ) - useEffect(() => { - if ( - !folderScope || - !hasConcreteFolderScope || - loadingWorkspaceFiles || - workspaceFilesArePlaceholderData || - isPreview - ) { - return + /* + * Selections the folder scope no longer covers — the folder changed, or + * someone moved the file. The run refuses them, so they are flagged for the + * user to remove rather than pruned here: pruning would write to the shared + * workflow from every open editor at once. + */ + const outOfScopeFilePaths = useMemo(() => { + if (!hasConcreteFolderScope || loadingWorkspaceFiles || workspaceFilesArePlaceholderData) { + return new Set() } - const scopedIds = new Set(scopedWorkspaceFiles.map((file) => file.id)) - const nextFiles = filesArray.filter((file) => { - const workspaceFile = findSelectedWorkspaceFile(file, workspaceFiles) - return !workspaceFile || scopedIds.has(workspaceFile.id) - }) - if (nextFiles.length === filesArray.length) return - - commitValue(multiple ? (nextFiles.length > 0 ? nextFiles : null) : (nextFiles[0] ?? null)) + return new Set( + filesArray.flatMap((file) => { + const match = findSelectedWorkspaceFile(file, workspaceFiles) + return match && !scopedIds.has(match.id) ? [file.path] : [] + }) + ) }, [ - commitValue, filesArray, - folderScope, hasConcreteFolderScope, - isPreview, loadingWorkspaceFiles, - multiple, scopedWorkspaceFiles, workspaceFiles, workspaceFilesArePlaceholderData, @@ -756,6 +750,9 @@ export function FileUpload({ ({workspaceFileSizeLabel(file.size)}) + {outOfScopeFilePaths.has(file.path) && ( + Outside folder + )}