diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index cbca66d6c92..762120da570 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -868,7 +868,7 @@ "get": { "operationId": "listFileVersions", "summary": "List File Versions", - "description": "List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits within ten minutes fold into one version, as do repeated workflow writes by one author. Renames and moves are not versions. An empty file is version 1 until its first content replaces it. Retention keeps the newest ten and removes older versions by plan, so numbers can have gaps.\n\nOAuth scope: `api:read`.", + "description": "List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits, and repeated workflow writes by one author, fold into a version under ten minutes old and written in the last five. Renames and moves are not versions. Retention removes older versions by age and plan but keeps the newest ten, so numbers can have gaps.\n\nOAuth scope: `api:read`.", "x-sim-operation": "files.versions.list", "x-oauth-scope": "api:read", "tags": ["Files"], diff --git a/apps/sim/background/cleanup-file-versions.ts b/apps/sim/background/cleanup-file-versions.ts index 9c3161b39a6..0564806c50d 100644 --- a/apps/sim/background/cleanup-file-versions.ts +++ b/apps/sim/background/cleanup-file-versions.ts @@ -6,6 +6,7 @@ import { task } from '@trigger.dev/sdk' import { and, count, gt, inArray, isNotNull, lt, min, or, sql } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import { + DEFAULT_BATCH_SIZE, DEFAULT_DELETE_CHUNK_SIZE, DEFAULT_MAX_BATCHES_PER_TABLE, DEFAULT_WORKSPACE_CHUNK_SIZE, @@ -22,6 +23,12 @@ const cleanupDb = dbFor('cleanup') /** Candidate files whose histories are ranked in one query. */ const FILES_PER_QUERY = 500 +/** + * Bounds one run like the other cleanup jobs: {@link DEFAULT_MAX_BATCHES_PER_TABLE} batches per + * workspace chunk and this many versions overall. The next run resumes where this one stopped. + */ +const MAX_VERSIONS_PER_RUN = DEFAULT_BATCH_SIZE * DEFAULT_MAX_BATCHES_PER_TABLE + /** * Superseded versions a free file keeps (its newest 100 with the current one); versions beyond it * are pruned whatever their age. Paid plans are bounded only by the inline write-time ceiling. @@ -73,7 +80,12 @@ async function selectCandidateFileIds( * Superseded versions of the given files past retention: older than the cutoff or beyond the plan's * count, but never among the newest {@link KEEP_SUPERSEDED} superseded versions of a file. */ -function selectExpiredVersions(fileIds: string[], cutoff: Date, maxSuperseded: number) { +function selectExpiredVersions( + fileIds: string[], + cutoff: Date, + maxSuperseded: number, + batchSize: number +) { const ranked = cleanupDb .select({ id: workspaceFileVersion.id, @@ -100,7 +112,7 @@ function selectExpiredVersions(fileIds: string[], cutoff: Date, maxSuperseded: n or(lt(ranked.supersededAt, cutoff), gt(ranked.rank, maxSuperseded)) ) ) - .limit(DEFAULT_DELETE_CHUNK_SIZE) + .limit(batchSize) } /** @@ -146,16 +158,27 @@ export async function runCleanupFileVersions(payload: CleanupJobPayload): Promis ) let deleted = 0 + let attempted = 0 for (const group of chunkArray(workspaceIds, DEFAULT_WORKSPACE_CHUNK_SIZE)) { + if (attempted >= MAX_VERSIONS_PER_RUN) break const candidates = await selectCandidateFileIds(group, cutoff, maxSuperseded) + let batches = 0 for (const fileIds of chunkArray(candidates, FILES_PER_QUERY)) { - for (let batch = 0; batch < DEFAULT_MAX_BATCHES_PER_TABLE; batch++) { - const expired = await selectExpiredVersions(fileIds, cutoff, maxSuperseded) - if (expired.length === 0) break - const removed = await deleteVersions(expired) + let exhausted = false + while ( + !exhausted && + batches < DEFAULT_MAX_BATCHES_PER_TABLE && + attempted < MAX_VERSIONS_PER_RUN + ) { + batches++ + const batchSize = Math.min(DEFAULT_DELETE_CHUNK_SIZE, MAX_VERSIONS_PER_RUN - attempted) + const expired = await selectExpiredVersions(fileIds, cutoff, maxSuperseded, batchSize) + attempted += expired.length + const removed = expired.length > 0 ? await deleteVersions(expired) : 0 deleted += removed - if (expired.length < DEFAULT_DELETE_CHUNK_SIZE || removed === 0) break + exhausted = expired.length < batchSize || removed === 0 } + if (!exhausted) break } } diff --git a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts index c6a48d17003..b4fe6ff669f 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts @@ -447,7 +447,7 @@ const declaredRoutes = [ operationId: 'listFileVersions', summary: 'List File Versions', description: - 'List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits within ten minutes fold into one version, as do repeated workflow writes by one author. Renames and moves are not versions. An empty file is version 1 until its first content replaces it. Retention keeps the newest ten and removes older versions by plan, so numbers can have gaps.', + 'List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits, and repeated workflow writes by one author, fold into a version under ten minutes old and written in the last five. Renames and moves are not versions. Retention removes older versions by age and plan but keeps the newest ten, so numbers can have gaps.', errors: RESOURCE_ERRORS, success: { description: 'A page of file versions.' }, }), diff --git a/apps/sim/lib/api/mcp/generated/v2-operations.ts b/apps/sim/lib/api/mcp/generated/v2-operations.ts index 12d7f9e393b..4cb56da0fce 100644 --- a/apps/sim/lib/api/mcp/generated/v2-operations.ts +++ b/apps/sim/lib/api/mcp/generated/v2-operations.ts @@ -1425,7 +1425,7 @@ export const V2_MCP_OPERATIONS = { contract: v2ListFileVersionsContract, summary: 'List File Versions', description: - 'List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits within ten minutes fold into one version, as do repeated workflow writes by one author. Renames and moves are not versions. An empty file is version 1 until its first content replaces it. Retention keeps the newest ten and removes older versions by plan, so numbers can have gaps.\n\nOAuth scope: `api:read`.', + 'List the versions of a file, newest first by default. Each write that changes the bytes records one; identical rewrites do not. Collaborative edits, and repeated workflow writes by one author, fold into a version under ten minutes old and written in the last five. Renames and moves are not versions. Retention removes older versions by age and plan but keeps the newest ten, so numbers can have gaps.\n\nOAuth scope: `api:read`.', handler: () => import('@/app/api/v2/files/[fileId]/versions/route').then((route) => route.GET), }, listKnowledgeBases: { diff --git a/apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts b/apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts index 0cfd8654cee..5b839aad66c 100644 --- a/apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts +++ b/apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts @@ -1,6 +1,6 @@ /** Real PostgreSQL transactions and local object storage for workspace file version history. */ import { mkdtempSync } from 'node:fs' -import { access, rm } from 'node:fs/promises' +import { access, mkdir, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import path from 'node:path' import { db, dbFor } from '@sim/db' @@ -39,6 +39,7 @@ import { import { WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT } from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox' import { getCurrentWorkspaceFileVersion, + getWorkspaceFileVersion, queryWorkspaceFileVersions, releaseWorkspaceFileVersionsForPurgeInTx, } from '@/lib/uploads/contexts/workspace/workspace-file-versions' @@ -288,16 +289,123 @@ describe('workspace file version history in PostgreSQL', () => { ) await expect(deleteWorkspaceFileVersion(fixture.workspaceId, fixture.fileId, 2)).resolves.toBe( - false + 'newest' ) await expect(deleteWorkspaceFileVersion(fixture.workspaceId, fixture.fileId, 1)).resolves.toBe( - true + 'deleted' + ) + await expect(deleteWorkspaceFileVersion(fixture.workspaceId, fixture.fileId, 1)).resolves.toBe( + 'not_found' ) expect((await versionRows(fixture.fileId)).map((row) => row.version)).toEqual([2]) expect(await objectExists(fixture.firstKey)).toBe(false) }) + it('reads bytes a write left unrecorded as the version its next write records them as', async () => { + const fixture = await seedFile('original') + const write = { source: 'api', authorUserId: fixture.aliceId } as const + await updateWorkspaceFileContent( + fixture.workspaceId, + fixture.fileId, + fixture.aliceId, + Buffer.from('second'), + undefined, + { version: write } + ) + /** A content write that replaced the bytes without recording a version, as a build predating history would. */ + const unrecordedKey = `${fixture.firstKey}-unrecorded` + const unrecordedContent = 'third, never recorded' + const unrecordedPath = path.join(fixtureStorage.root, unrecordedKey) + await mkdir(path.dirname(unrecordedPath), { recursive: true }) + await writeFile(unrecordedPath, unrecordedContent) + await db + .update(workspaceFiles) + .set({ + key: unrecordedKey, + sizeBytes: Buffer.byteLength(unrecordedContent), + contentUpdatedAt: new Date(), + }) + .where(eq(workspaceFiles.id, fixture.fileId)) + const file = await getWorkspaceFile(fixture.workspaceId, fixture.fileId) + if (!file) throw new Error('file missing') + + await expect( + getWorkspaceFileWithCurrentVersion(fixture.workspaceId, fixture.fileId) + ).resolves.toMatchObject({ key: unrecordedKey, currentVersion: 3 }) + expect(await getCurrentWorkspaceFileVersion(file)).toMatchObject({ + version: 3, + key: unrecordedKey, + isCurrent: true, + }) + const listed = await queryWorkspaceFileVersions(file, { sortOrder: 'desc', limit: 10 }) + expect(listed.versions[0].size).toBe(Buffer.byteLength(unrecordedContent)) + expect(listed.versions.map((row) => [row.version, row.isCurrent, row.source])).toEqual([ + [3, true, 'unknown'], + [2, false, 'api'], + [1, false, 'upload'], + ]) + expect(listed.versions[1].supersededAt).toEqual(file.contentUpdatedAt) + + const firstPage = await queryWorkspaceFileVersions(file, { sortOrder: 'asc', limit: 2 }) + expect(firstPage.versions.map((row) => row.version)).toEqual([1, 2]) + const lastPage = await queryWorkspaceFileVersions(file, { + sortOrder: 'asc', + limit: 2, + after: firstPage.nextKeys ?? undefined, + }) + expect(lastPage.versions.map((row) => row.version)).toEqual([3]) + expect(lastPage.nextKeys).toBeNull() + + await expect(deleteWorkspaceFileVersion(fixture.workspaceId, fixture.fileId, 2)).resolves.toBe( + 'newest' + ) + + const next = await updateWorkspaceFileContent( + fixture.workspaceId, + fixture.fileId, + fixture.aliceId, + Buffer.from('fourth'), + undefined, + { version: write } + ) + expect(next.currentVersion).toBe(4) + expect((await versionRows(fixture.fileId)).map((row) => [row.version, row.source])).toEqual([ + [1, 'upload'], + [2, 'api'], + [3, 'unknown'], + [4, 'api'], + ]) + const materialized = (await versionRows(fixture.fileId))[2] + expect(materialized.key).toBe(unrecordedKey) + expect(materialized.sizeBytes).toBe(Buffer.byteLength(unrecordedContent)) + expect(await readVersionBytes(fixture.workspaceId, fixture.fileId, materialized.key)).toBe( + unrecordedContent + ) + }) + + it('reads versions against the file as committed, not a record loaded before a write', async () => { + const fixture = await seedFile('original') + const stale = await getWorkspaceFile(fixture.workspaceId, fixture.fileId) + if (!stale) throw new Error('file missing') + await updateWorkspaceFileContent( + fixture.workspaceId, + fixture.fileId, + fixture.aliceId, + Buffer.from('second'), + undefined, + { version: { source: 'api', authorUserId: fixture.aliceId } } + ) + + const listed = await queryWorkspaceFileVersions(stale, { sortOrder: 'desc', limit: 10 }) + expect(listed.versions.map((row) => [row.version, row.isCurrent])).toEqual([ + [2, true], + [1, false], + ]) + expect((await getCurrentWorkspaceFileVersion(stale)).version).toBe(2) + await expect(getWorkspaceFileVersion(stale, 3)).resolves.toBeNull() + }) + it('never folds deliberate writes, and repoints the head for identical bytes', async () => { const fixture = await seedFile('original') const write = { source: 'api', authorUserId: fixture.aliceId } as const 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 2d49bf200ec..f04352f7adc 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -82,6 +82,7 @@ import { listWorkspaceFileVersionKeysInTx, loadWorkspaceFileVersionHead, recordWorkspaceFileVersionInTx, + type WorkspaceFileVersionDeletion, type WorkspaceFileVersionWrite, } from '@/lib/uploads/contexts/workspace/workspace-file-versions' import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key' @@ -2097,15 +2098,13 @@ export async function updateWorkspaceFileContent( /** * Deletes one superseded version of an active workspace file and releases its stored object. The * file row is locked so the delete serializes with content writes that supersede or prune history. - * Returns false when the file has no such superseded version (it never existed, retention removed - * it, or it is the current version). */ export async function deleteWorkspaceFileVersion( workspaceId: string, fileId: string, version: number -): Promise { - const cleanupEventIds = await db.transaction(async (tx) => { +): Promise { + const deletion = await db.transaction(async (tx) => { const [file] = await tx .select({ id: workspaceFiles.id }) .from(workspaceFiles) @@ -2113,17 +2112,21 @@ export async function deleteWorkspaceFileVersion( .for('update') .limit(1) if (!file) throw new OrchestrationError('not_found', 'File not found') - const key = await deleteWorkspaceFileVersionInTx(tx, fileId, version) - return key ? enqueueWorkspaceFileStorageCleanups(tx, [key]) : null + const result = await deleteWorkspaceFileVersionInTx(tx, fileId, version) + return result.status === 'deleted' + ? { + status: result.status, + cleanupEventIds: await enqueueWorkspaceFileStorageCleanups(tx, [result.key]), + } + : { status: result.status, cleanupEventIds: [] } }) - if (!cleanupEventIds) return false - await processWorkspaceFileStorageCleanupsNow(cleanupEventIds, { + await processWorkspaceFileStorageCleanupsNow(deletion.cleanupEventIds, { workspaceId, fileId, reason: 'deleted version', }) - return true + return deletion.status } /** diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts index b5095577877..1cc6cbe5e9b 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox.ts @@ -1,6 +1,6 @@ import type { db } from '@sim/db' import { createLogger } from '@sim/logger' -import { describeError, getErrorMessage } from '@sim/utils/errors' +import { describeError } from '@sim/utils/errors' import { chunkArray } from '@sim/utils/helpers' import { enqueueOutboxEvents, @@ -87,7 +87,7 @@ export async function processWorkspaceFileStorageCleanupsNow( logger.warn('Storage cleanup deferred after inline processing error', { ...logContext, eventId, - error: getErrorMessage(error), + error: describeError(error), }) } } diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts index 5b960628378..482e9fe8f8d 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts @@ -286,15 +286,22 @@ async function insertVersion( }) } +/** The outcome of deleting one version; a deleted version's key is released after commit. */ +export type WorkspaceFileVersionDeletion = + | { status: 'deleted'; key: string } + | { status: 'not_found' } + | { status: 'newest' } + /** - * Deletes one superseded version, returning its storage key for release after commit, or null when - * no such superseded version exists. The current version is never deletable: it is the file. + * Deletes one superseded version. The newest row is never deleted: it is either the current version + * or the last one recorded before bytes a later write has not recorded yet, and removing it would let + * the next write reuse its number. */ export async function deleteWorkspaceFileVersionInTx( tx: DbTransaction, fileId: string, version: number -): Promise { +): Promise { const [deleted] = await tx .delete(workspaceFileVersion) .where( @@ -305,7 +312,13 @@ export async function deleteWorkspaceFileVersionInTx( ) ) .returning({ key: workspaceFileVersion.key }) - return deleted?.key ?? null + if (deleted) return { status: 'deleted', key: deleted.key } + const [kept] = await tx + .select({ version: workspaceFileVersion.version }) + .from(workspaceFileVersion) + .where(and(eq(workspaceFileVersion.fileId, fileId), eq(workspaceFileVersion.version, version))) + .limit(1) + return kept ? { status: 'newest' } : { status: 'not_found' } } /** @@ -411,7 +424,16 @@ function toSnapshotStatus(status: string | null): WorkspaceFileSecretProvenanceS return 'unknown' } -function toVersionRecord(row: WorkspaceFileVersionSummaryRow): WorkspaceFileVersionRecord { +/** + * A stored version as readers see it. A row is current only while it still holds the file's bytes; + * a newest row a later write has replaced without recording (see {@link implicitCurrentVersion}) + * reads as superseded from the moment that write landed. + */ +function toVersionRecord( + row: WorkspaceFileVersionSummaryRow, + file: WorkspaceFileVersionSubject +): WorkspaceFileVersionRecord { + const isCurrent = row.supersededAt === null && row.key === file.key return { fileId: row.fileId, version: row.version, @@ -421,24 +443,35 @@ function toVersionRecord(row: WorkspaceFileVersionSummaryRow): WorkspaceFileVers source: row.source, authorUserIds: row.authorUserIds, restoredFromVersion: row.restoredFromVersion, - isCurrent: row.supersededAt === null, + isCurrent, createdAt: row.createdAt, updatedAt: row.updatedAt, - supersededAt: row.supersededAt, + supersededAt: isCurrent ? null : (row.supersededAt ?? contentVersionTime(file)), } } +function contentVersionTime(file: WorkspaceFileVersionSubject): Date { + return file.contentUpdatedAt ?? file.updatedAt +} + /** - * Version 1 of a file with no history yet — every file before its first content write. Attributed - * exactly as {@link recordWorkspaceFileVersionInTx} will materialize it, so the version a reader sees - * keeps its identity once it becomes a row. + * The version a file's current bytes hold while no row records them, for a caller that has found + * {@link isVersionHeadCurrent} false. That is version 1 of a file with no history, or the number + * after a newest row that describes other bytes — left by a content write that skipped recording, + * such as one from a build that predates version history. Numbered and attributed exactly as + * {@link recordWorkspaceFileVersionInTx} will materialize it on the next write, so the version a + * reader sees keeps its identity once it becomes a row. */ -function implicitFirstVersion(file: WorkspaceFileVersionSubject): WorkspaceFileVersionRecord { - const contentUpdatedAt = file.contentUpdatedAt ?? file.updatedAt - const original = isOriginalUploadContent({ uploadedAt: file.uploadedAt, contentUpdatedAt }) +function implicitCurrentVersion( + file: WorkspaceFileVersionSubject, + head: WorkspaceFileVersionSummaryRow | undefined +): WorkspaceFileVersionRecord { + const contentUpdatedAt = contentVersionTime(file) + const original = + !head && isOriginalUploadContent({ uploadedAt: file.uploadedAt, contentUpdatedAt }) return { fileId: file.id, - version: 1, + version: (head?.version ?? 0) + 1, key: file.key, size: file.size, contentType: file.type, @@ -452,6 +485,59 @@ function implicitFirstVersion(file: WorkspaceFileVersionSubject): WorkspaceFileV } } +/** + * Runs `read` in one read-only snapshot that also re-reads the file's content columns, so a content + * write committing mid-read can never pair one write's file record with another write's version + * rows. A file row deleted since the caller loaded it keeps the caller's record. + */ +function withVersionSnapshot( + file: WorkspaceFileVersionSubject, + read: (tx: DbTransaction, file: WorkspaceFileVersionSubject) => Promise +): Promise { + return db.transaction( + async (tx) => { + const [row] = await tx + .select({ + key: workspaceFiles.key, + sizeBytes: workspaceFiles.sizeBytes, + contentType: workspaceFiles.contentType, + userId: workspaceFiles.userId, + uploadedAt: workspaceFiles.uploadedAt, + updatedAt: workspaceFiles.updatedAt, + contentUpdatedAt: workspaceFiles.contentUpdatedAt, + }) + .from(workspaceFiles) + .where(eq(workspaceFiles.id, file.id)) + .limit(1) + const snapshot: WorkspaceFileVersionSubject = row + ? { + id: file.id, + key: row.key, + size: getWorkspaceFileSize(row), + type: row.contentType, + uploadedBy: row.userId, + uploadedAt: row.uploadedAt, + updatedAt: row.updatedAt, + contentUpdatedAt: row.contentUpdatedAt, + } + : file + return read(tx, snapshot) + }, + { isolationLevel: 'repeatable read', accessMode: 'read only' } + ) +} + +/** The version holding the file's current bytes, recorded or implicit, within a snapshot. */ +async function currentVersionInSnapshot( + tx: DbTransaction, + file: WorkspaceFileVersionSubject +): Promise { + const head = await loadWorkspaceFileVersionHead(file.id, tx) + return head && isVersionHeadCurrent(head, file) + ? toVersionRecord(head, file) + : implicitCurrentVersion(file, head) +} + const VERSION_KEYSET: readonly KeysetKey<{ version: number }>[] = [ numberKey(workspaceFileVersion.version, (row) => row.version), ] @@ -462,22 +548,37 @@ export async function queryWorkspaceFileVersions( options: { sortOrder: ListSortOrder; limit: number; after?: CursorKey[] } ): Promise<{ versions: WorkspaceFileVersionRecord[]; nextKeys: CursorKey[] | null }> { const resume = resumeKeyset(VERSION_KEYSET, options.after, options.sortOrder) - const rows = await db - .select(versionSummaryColumns) - .from(workspaceFileVersion) - .where(and(eq(workspaceFileVersion.fileId, file.id), resume)) - .orderBy(...listOrderBy(keysetColumns(VERSION_KEYSET), options.sortOrder)) - .limit(options.limit + 1) - const records = rows.map(toVersionRecord) - /** - * An uncursored page that is empty means the file has no rows, so it lists its implicit version 1. - * Any cursor was minted past that version already, so only the first page can carry it. - */ - if (records.length === 0 && !options.after) { - records.push(implicitFirstVersion(file)) - } - const page = keysetPage(VERSION_KEYSET, records, options.limit) - return { versions: page.data, nextKeys: page.nextCursorKeys } + return withVersionSnapshot(file, async (tx, current) => { + const rows = await tx + .select(versionSummaryColumns) + .from(workspaceFileVersion) + .where(and(eq(workspaceFileVersion.fileId, current.id), resume)) + .orderBy(...listOrderBy(keysetColumns(VERSION_KEYSET), options.sortOrder)) + .limit(options.limit + 1) + const head = await loadWorkspaceFileVersionHead(current.id, tx) + const records = rows.map((row) => toVersionRecord(row, current)) + /** + * The implicit current version numbers above every row, so it leads a descending list and ends + * an ascending one; a cursor already past it leaves it out. Over-fetching by one row still + * decides whether another page follows, since the cut keeps the first `limit` records either way. + */ + const implicit = isVersionHeadCurrent(head, current) + ? null + : implicitCurrentVersion(current, head) + const resumeAfter = options.after?.[0] + if ( + implicit && + (typeof resumeAfter !== 'number' || + (options.sortOrder === 'desc' + ? implicit.version < resumeAfter + : implicit.version > resumeAfter)) + ) { + if (options.sortOrder === 'desc') records.unshift(implicit) + else records.push(implicit) + } + const page = keysetPage(VERSION_KEYSET, records, options.limit) + return { versions: page.data, nextKeys: page.nextCursorKeys } + }) } /** @@ -495,43 +596,60 @@ export async function findWorkspaceFileVersionKeys(keys: readonly string[]): Pro return new Set(rows.map((row) => row.key)) } -/** The current version, or the implicit version 1 of a file with no history rows. */ -export async function getCurrentWorkspaceFileVersion( +/** The version holding the file's current bytes, recorded or implicit. */ +export function getCurrentWorkspaceFileVersion( file: WorkspaceFileVersionSubject ): Promise { - const head = await loadWorkspaceFileVersionHead(file.id) - return head ? toVersionRecord(head) : implicitFirstVersion(file) + return withVersionSnapshot(file, currentVersionInSnapshot) } /** * The current version number of the enclosing query's `workspace_files` row, as a correlated - * subquery so the row and its number come from one statement's snapshot. Content writes record - * their version in the transaction that replaces the bytes, so the newest row describes the current - * content; a file with no rows is on its implicit version 1. Both sides of the correlation - * are table-qualified because Drizzle renders single-table columns bare, which would bind the outer - * `id` to this subquery's own table. + * subquery so the row and its number come from one statement's snapshot. The newest row numbers + * the file's bytes while it still holds them; otherwise the bytes are on the implicit version after + * it, and a file with no rows is on version 1 — the numbering {@link implicitCurrentVersion} gives. + * Both sides of the correlation are table-qualified because Drizzle renders single-table columns + * bare, which would bind the outer columns to this subquery's own table. */ export function currentWorkspaceFileVersionNumberSql() { - const versionFileId = sql`${workspaceFileVersion}.${sql.identifier(workspaceFileVersion.fileId.name)}` - const outerFileId = sql`${workspaceFiles}.${sql.identifier(workspaceFiles.id.name)}` - return sql`coalesce((select max(${workspaceFileVersion.version}) from ${workspaceFileVersion} where ${versionFileId} = ${outerFileId}), 1)`.mapWith( - Number - ) + const qualified = (table: typeof workspaceFileVersion | typeof workspaceFiles, name: string) => + sql`${table}.${sql.identifier(name)}` + const head = { + fileId: qualified(workspaceFileVersion, workspaceFileVersion.fileId.name), + version: qualified(workspaceFileVersion, workspaceFileVersion.version.name), + key: qualified(workspaceFileVersion, workspaceFileVersion.key.name), + supersededAt: qualified(workspaceFileVersion, workspaceFileVersion.supersededAt.name), + } + const file = { + id: qualified(workspaceFiles, workspaceFiles.id.name), + key: qualified(workspaceFiles, workspaceFiles.key.name), + } + const number = sql`case when ${head.supersededAt} is null and ${head.key} = ${file.key} then ${head.version} else ${head.version} + 1 end` + return sql`coalesce(( + select ${number} from ${workspaceFileVersion} + where ${head.fileId} = ${file.id} + order by ${head.version} desc + limit 1 + ), 1)`.mapWith(Number) } /** One version of a file, or null when it never existed or retention removed it. */ -export async function getWorkspaceFileVersion( +export function getWorkspaceFileVersion( file: WorkspaceFileVersionSubject, version: number ): Promise { - const [row] = await db - .select(versionSummaryColumns) - .from(workspaceFileVersion) - .where(and(eq(workspaceFileVersion.fileId, file.id), eq(workspaceFileVersion.version, version))) - .limit(1) - if (row) return toVersionRecord(row) - if (version !== 1 || (await loadWorkspaceFileVersionHead(file.id))) return null - return implicitFirstVersion(file) + return withVersionSnapshot(file, async (tx, current) => { + const [row] = await tx + .select(versionSummaryColumns) + .from(workspaceFileVersion) + .where( + and(eq(workspaceFileVersion.fileId, current.id), eq(workspaceFileVersion.version, version)) + ) + .limit(1) + if (row) return toVersionRecord(row, current) + const latest = await currentVersionInSnapshot(tx, current) + return latest.version === version ? latest : null + }) } /** diff --git a/apps/sim/lib/workspace-files/application/file-versions.test.ts b/apps/sim/lib/workspace-files/application/file-versions.test.ts index 51fc8a3f781..aa57b831978 100644 --- a/apps/sim/lib/workspace-files/application/file-versions.test.ts +++ b/apps/sim/lib/workspace-files/application/file-versions.test.ts @@ -317,7 +317,7 @@ describe('file version use cases', () => { describe('deleteWorkspaceFileVersion', () => { it('deletes a superseded version and audits it', async () => { - mocks.deleteStored.mockResolvedValueOnce(true) + mocks.deleteStored.mockResolvedValueOnce('deleted') const result = await deleteWorkspaceFileVersion.execute({ principal, @@ -344,8 +344,20 @@ describe('file version use cases', () => { expect(mocks.deleteStored).not.toHaveBeenCalled() }) + it('refuses to delete the newest recorded version, whose number the next write would reuse', async () => { + mocks.deleteStored.mockResolvedValueOnce('newest') + + await expect( + deleteWorkspaceFileVersion.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1', version: 2 }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + expect(mocks.recordAudit).not.toHaveBeenCalled() + }) + it('answers 404 when the version disappeared before the delete committed', async () => { - mocks.deleteStored.mockResolvedValueOnce(false) + mocks.deleteStored.mockResolvedValueOnce('not_found') await expect( deleteWorkspaceFileVersion.execute({ diff --git a/apps/sim/lib/workspace-files/application/file-versions.ts b/apps/sim/lib/workspace-files/application/file-versions.ts index 1c13b904dfc..a6048200a3e 100644 --- a/apps/sim/lib/workspace-files/application/file-versions.ts +++ b/apps/sim/lib/workspace-files/application/file-versions.ts @@ -343,12 +343,20 @@ export const deleteWorkspaceFileVersion = defineAuthorizedWorkspaceFileUseCase({ `Version ${target.version} is the current version and cannot be deleted; revert to another version first` ) } - const deleted = await deleteStoredWorkspaceFileVersion( + const deletion = await deleteStoredWorkspaceFileVersion( context.workspaceId, context.fileId, target.version ) - if (!deleted) throw new OrchestrationError('not_found', `Version ${target.version} not found`) + if (deletion === 'not_found') { + throw new OrchestrationError('not_found', `Version ${target.version} not found`) + } + if (deletion === 'newest') { + throw new OrchestrationError( + 'conflict', + `Version ${target.version} is the newest recorded version and cannot be deleted` + ) + } return { file, version: target.version } }, projectAudit: ({ result }) => ({ diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 3db808390e2..c3cde4daf57 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -1133,7 +1133,7 @@ export const CLI_CONTRACT: CliContract = { { header: 'current', path: 'isCurrent', format: 'bool' }, { header: 'source' }, { header: 'size', format: 'bytes' }, - { header: 'authors', format: 'count' }, + { header: 'authors', format: 'people' }, { header: 'created', path: 'createdAt', format: 'timestamp' }, { header: 'superseded', path: 'supersededAt', format: 'timestamp' }, ], @@ -1149,7 +1149,7 @@ export const CLI_CONTRACT: CliContract = { { header: 'restored from', path: 'restoredFromVersion' }, { header: 'size', format: 'bytes' }, { header: 'type', path: 'contentType' }, - { header: 'authors', format: 'count' }, + { header: 'authors', format: 'people' }, { header: 'created', path: 'createdAt', format: 'timestamp' }, { header: 'updated', path: 'updatedAt', format: 'timestamp' }, { header: 'superseded', path: 'supersededAt', format: 'timestamp' }, @@ -1163,6 +1163,17 @@ export const CLI_CONTRACT: CliContract = { revertFileVersion: { command: 'files versions revert', describe: 'Make a previous version of a file current again', + fields: [ + { header: 'reverted', format: 'bool' }, + { header: 'file', path: 'file.id' }, + { header: 'name', path: 'file.name' }, + { header: 'version', path: 'version.version' }, + { header: 'source', path: 'version.source' }, + { header: 'restored from', path: 'version.restoredFromVersion' }, + { header: 'size', path: 'version.size', format: 'bytes' }, + { header: 'authors', path: 'version.authors', format: 'people' }, + { header: 'created', path: 'version.createdAt', format: 'timestamp' }, + ], }, deleteFileVersion: { command: 'files versions delete', diff --git a/packages/sim-cli/src/contract/types.ts b/packages/sim-cli/src/contract/types.ts index db93541445f..d62d546cfb5 100644 --- a/packages/sim-cli/src/contract/types.ts +++ b/packages/sim-cli/src/contract/types.ts @@ -189,6 +189,10 @@ export interface ColumnSpec { * `score` fixes a similarity to four decimals. The raw double arrives as * `0.2818957269585687`, a nineteen-character column whose last dozen digits * cannot separate one result from another. + * + * `people` shows a list of `{ id, email }` users by email. A user whose + * account is gone has a null email, so the id stands in rather than the + * person vanishing from the list. */ format?: | 'auto' @@ -201,6 +205,7 @@ export interface ColumnSpec { | 'trace-count' | 'folder-path' | 'score' + | 'people' } export interface BodyVariantSpec { diff --git a/packages/sim-cli/src/runtime/result.test.ts b/packages/sim-cli/src/runtime/result.test.ts index a8c0d829c88..7794964ebb6 100644 --- a/packages/sim-cli/src/runtime/result.test.ts +++ b/packages/sim-cli/src/runtime/result.test.ts @@ -197,6 +197,77 @@ describe('a similarity score, at a width a person can read', () => { }) }) +describe('file version authors and reverts', () => { + const version = { + fileId: 'f_1', + version: 4, + isCurrent: true, + size: 42, + contentType: 'text/markdown', + source: 'revert', + authors: [ + { id: 'usr_1', email: 'ada@example.com' }, + { id: 'usr_gone', email: null }, + ], + restoredFromVersion: 1, + createdAt: '2026-09-19T17:20:39.920Z', + updatedAt: '2026-09-19T17:20:39.920Z', + supersededAt: null, + } + + it('lists authors by email, keeping the id of an account that is gone', () => { + renderPage( + 'table', + { data: [version], nextCursor: null }, + CLI_CONTRACT.listFileVersions as CommandSpec + ) + const [header, row] = tableLines() + expect(header).toContain('AUTHORS') + expect(row).toContain('ada@example.com, usr_gone') + }) + + it('names the authors in a version record', () => { + renderResult('getFileVersion', 'text', version, CLI_CONTRACT.getFileVersion as CommandSpec) + expect(logged).toContain('authors\tada@example.com, usr_gone') + }) + + it('shows a version with no recorded author as empty', () => { + renderResult( + 'getFileVersion', + 'text', + { ...version, authors: [] }, + CLI_CONTRACT.getFileVersion as CommandSpec + ) + expect(logged).toContain('authors\t') + }) + + it('prints a revert as fields rather than the nested objects as JSON', () => { + renderResult( + 'revertFileVersion', + 'text', + { reverted: true, file: { id: 'f_1', name: 'notes.md' }, version }, + CLI_CONTRACT.revertFileVersion as CommandSpec + ) + expect(logged).toEqual( + expect.arrayContaining([ + 'reverted\tyes', + 'file\tf_1', + 'name\tnotes.md', + 'version\t4', + 'source\trevert', + 'restored from\t1', + 'authors\tada@example.com, usr_gone', + ]) + ) + expect(logged.join('\n')).not.toContain('{') + }) + + it('keeps the raw authors in json', () => { + renderResult('getFileVersion', 'json', version, CLI_CONTRACT.getFileVersion as CommandSpec) + expect(JSON.parse(logged[0]).authors).toEqual(version.authors) + }) +}) + describe('file-content search results', () => { const response = { results: [{ fileId: 'file_1', lineNumber: 7, text: 'quarterly revenue' }], diff --git a/packages/sim-cli/src/runtime/result.ts b/packages/sim-cli/src/runtime/result.ts index 892f70a2ee2..931675d1a38 100644 --- a/packages/sim-cli/src/runtime/result.ts +++ b/packages/sim-cli/src/runtime/result.ts @@ -72,6 +72,13 @@ export function decodeFolderPath(value: string): string { .join('/') } +/** A `{ id, email }` user by email, or by id once the account behind it is gone. */ +function personLabel(person: unknown): string { + const { id, email } = (person ?? {}) as { id?: unknown; email?: unknown } + if (typeof email === 'string' && email) return email + return typeof id === 'string' && id ? id : JSON.stringify(person) +} + function renderCell( value: unknown, format: ColumnSpec['format'], @@ -92,6 +99,10 @@ function renderCell( return typeof value === 'number' ? value.toFixed(4) : text(null) case 'count': return Array.isArray(value) ? String(value.length) : text(null) + case 'people': + return Array.isArray(value) && value.length > 0 + ? sanitize(value.map(personLabel).join(', ')) + : text(null) case 'folder-path': return typeof value === 'string' ? text(decodeFolderPath(value)) : text(value) case 'trace-count': { diff --git a/packages/testing/src/mocks/schema.mock.ts b/packages/testing/src/mocks/schema.mock.ts index 7988865ff48..4ad2bb4802c 100644 --- a/packages/testing/src/mocks/schema.mock.ts +++ b/packages/testing/src/mocks/schema.mock.ts @@ -770,7 +770,6 @@ export const schemaMock = { sizeBytes: 'workspaceFileVersion.sizeBytes', contentType: 'workspaceFileVersion.contentType', contentHash: 'workspaceFileVersion.contentHash', - contentUpdatedAt: 'workspaceFileVersion.contentUpdatedAt', supersededAt: 'workspaceFileVersion.supersededAt', source: 'workspaceFileVersion.source', authorUserIds: 'workspaceFileVersion.authorUserIds',