diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index bb6ccff5bb8..d625e72c2ec 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -66,6 +66,10 @@ When the user runs `/ship`: # Runs every audit CI runs, concurrently, and replays the output of any that fail. # The audit list is derived in scripts/run-audits.ts — do not hand-list audits here. bun run check:audits || { echo "❌ audit(s) failed — do not ship"; exit 1; } + # CI's "Verify docs manifest is in sync" step is not a `check:*` script, so the runner above + # does not cover it. (CI's "Security audit" `bun audit` step is `continue-on-error` — advisory + # only, not a gate — so it is deliberately not run here.) + bun run docs-manifest:check || { echo "❌ docs manifest out of sync — do not ship"; exit 1; } ``` If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` and `type-check` are covered by steps 5 and CI respectively and are not repeated here. 7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6 diff --git a/apps/docs/content/docs/platform/enterprise/forks.mdx b/apps/docs/content/docs/platform/enterprise/forks.mdx index 4301284fd9a..adc4df254f5 100644 --- a/apps/docs/content/docs/platform/enterprise/forks.mdx +++ b/apps/docs/content/docs/platform/enterprise/forks.mdx @@ -136,7 +136,7 @@ The setting belongs to **this workspace's copy** only. Excluding a workflow here Activity view showing Fork and Push events with expandable detail rows -Expand a row for names of workflows and resources that were created, updated, or archived, and any warnings (for example failed background copies or deploy failures). +Expand a row for names of workflows and resources that were created, updated, or archived, and any warnings (for example failed background copies or deploy failures). A push or pull that copies resources fills their content in the background, and that progress and outcome show in the same row. --- @@ -331,6 +331,8 @@ Servers that **publish workflows as MCP tools**. | **Fork** | **Values never leave the source.** Workflow text still contains `{{KEY}}` names. Create matching secrets (or the names you will map to) under the child’s **Secrets**. | | **Sync** | Map source key names to target key names. Values stay in each workspace. Unmapped required secrets block Sync. | +Notes are documentation: a `{{KEY}}` that appears only inside a Note block never needs mapping and never blocks Sync. + **Example:** Workflows use `{{OPENAI_API_KEY}}`. After fork, add that secret in the child (or map `OPENAI_API_KEY` to whatever name the child uses) before runs and syncs succeed. --- diff --git a/apps/sim/app/api/workspaces/[id]/fork/promote/route.test.ts b/apps/sim/app/api/workspaces/[id]/fork/promote/route.test.ts index 09528e0aa10..a634de5425e 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/promote/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/promote/route.test.ts @@ -1,5 +1,5 @@ /** - * Tests for the fork sync (promote) route's error projection. + * Tests for the fork sync (promote) route's error projection and input mapping. * * `promoteFork` returns its deliberate refusals as a `blocked` result, but a classified * failure raised deeper in the copy — the target workspace's folder ceiling being full — @@ -12,22 +12,19 @@ import { auditMock, authMockFns, createMockRequest, type MockUser } from '@sim/t import { beforeEach, describe, expect, it, vi } from 'vitest' import { FolderCollectionFullError } from '@/lib/folders/errors' -const { mockLogger, mockPromoteFork, mockAssertCanPromote, mockRecordBackgroundWork } = vi.hoisted( - () => ({ - mockLogger: { - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - trace: vi.fn(), - fatal: vi.fn(), - child: vi.fn(), - }, - mockPromoteFork: vi.fn(), - mockAssertCanPromote: vi.fn(), - mockRecordBackgroundWork: vi.fn(), - }) -) +const { mockLogger, mockPromoteFork, mockAssertCanPromote } = vi.hoisted(() => ({ + mockLogger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + trace: vi.fn(), + fatal: vi.fn(), + child: vi.fn(), + }, + mockPromoteFork: vi.fn(), + mockAssertCanPromote: vi.fn(), +})) vi.mock('@sim/audit', () => auditMock) vi.mock('@sim/logger', () => ({ @@ -39,9 +36,6 @@ vi.mock('@/ee/workspace-forking/lib/promote/promote', () => ({ promoteFork: mock vi.mock('@/ee/workspace-forking/lib/lineage/authz', () => ({ assertCanPromote: mockAssertCanPromote, })) -vi.mock('@/ee/workspace-forking/lib/background-work/store', () => ({ - recordBackgroundWork: mockRecordBackgroundWork, -})) import { POST } from '@/app/api/workspaces/[id]/fork/promote/route' @@ -69,8 +63,41 @@ describe('POST /api/workspaces/[id]/fork/promote', () => { edge: { childWorkspaceId: WORKSPACE_ID }, sourceWorkspaceId: WORKSPACE_ID, targetWorkspaceId: 'ws-parent', + source: { name: 'Child' }, + target: { name: 'Parent' }, + }) + }) + + /** + * The sync's Activity row is recorded by the use case, not here, so the route's job is to + * hand it the one thing only the route knows: the display name of the edge's other side. + */ + it('names the other side of the edge for promoteFork to record the sync', async () => { + mockPromoteFork.mockResolvedValue({ + promoteRunId: 'run-1', + updated: 1, + created: 0, + archived: 0, + redeployed: 1, + deployFailed: 0, + unmappedRequired: [], + blockers: [], + blocked: null, + updatedNames: ['Flow'], + createdNames: [], + archivedNames: [], + needsConfiguration: [], + clearedOptional: [], + droppedReferences: [], + triggerUrlChanges: [], }) - mockRecordBackgroundWork.mockResolvedValue(undefined) + + const response = await POST(promoteRequest(), routeContext) + + expect(response.status).toBe(200) + expect(mockPromoteFork).toHaveBeenCalledWith( + expect.objectContaining({ direction: 'push', actorName: 'A', otherWorkspaceName: 'Parent' }) + ) }) it('renders a full-folder-tree refusal as an actionable 409', async () => { diff --git a/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts b/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts index 23f128589d3..af76a8d0e01 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts @@ -1,7 +1,5 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' -import { db } from '@sim/db' import { createLogger } from '@sim/logger' -import { getErrorMessage } from '@sim/utils/errors' import { type NextRequest, NextResponse } from 'next/server' import { promoteForkContract } from '@/lib/api/contracts/workspace-fork' import { parseRequest } from '@/lib/api/server' @@ -9,7 +7,6 @@ import { getSession } from '@/lib/auth' import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { recordBackgroundWork } from '@/ee/workspace-forking/lib/background-work/store' import { assertCanPromote } from '@/ee/workspace-forking/lib/lineage/authz' import { promoteFork } from '@/ee/workspace-forking/lib/promote/promote' @@ -36,6 +33,8 @@ export const POST = withRouteHandler( } = parsed.data.body const auth = await assertCanPromote(id, otherWorkspaceId, direction, session.user.id) + const otherName = + otherWorkspaceId === auth.sourceWorkspaceId ? auth.source.name : auth.target.name let result: Awaited> try { @@ -46,6 +45,7 @@ export const POST = withRouteHandler( direction, userId: session.user.id, actorName: session.user.name ?? undefined, + otherWorkspaceName: otherName, dependentValues, copyResources, dropReferences, @@ -114,44 +114,6 @@ export const POST = withRouteHandler( request: req, }) - const otherName = - otherWorkspaceId === auth.sourceWorkspaceId ? auth.source.name : auth.target.name - await recordBackgroundWork(db, { - workspaceId: id, - kind: 'fork_sync', - status: - result.deployFailed > 0 || - result.needsConfiguration.length > 0 || - result.clearedOptional.length > 0 || - result.droppedReferences.length > 0 || - result.triggerUrlChanges.length > 0 - ? 'completed_with_warnings' - : 'completed', - message: direction === 'pull' ? `Pulled from "${otherName}"` : `Pushed to "${otherName}"`, - metadata: { - actorName: session.user.name ?? undefined, - otherWorkspaceId, - otherWorkspaceName: otherName, - direction, - updated: result.updated, - created: result.created, - archived: result.archived, - redeployed: result.redeployed, - deployFailed: result.deployFailed, - updatedNames: result.updatedNames, - createdNames: result.createdNames, - archivedNames: result.archivedNames, - needsConfiguration: result.needsConfiguration, - clearedOptional: result.clearedOptional, - droppedReferences: result.droppedReferences.length, - triggerUrlChanges: result.triggerUrlChanges.length, - }, - }).catch((error) => - logger.error(`[${requestId}] Failed to record sync activity`, { - error: getErrorMessage(error), - }) - ) - return NextResponse.json(body) } ) diff --git a/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.test.tsx b/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.test.tsx index af633e4e6cd..4d024ffdd1b 100644 --- a/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.test.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.test.tsx @@ -169,3 +169,121 @@ describe('ForkActivityPanel event badge tooltip', () => { expect(row?.getAttribute('aria-expanded')).toBe('false') }) }) + +describe('ForkActivityPanel sync report', () => { + beforeEach(() => { + vi.clearAllMocks() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + const syncMetadata = { + actorName: 'Brandon Tarr', + direction: 'push' as const, + otherWorkspaceId: PARTNER_ID, + otherWorkspaceName: 'another workspace', + updatedNames: ['Flow A'], + tables: 2, + files: 1, + } + + function expandRow() { + const row = container.querySelector('button[aria-expanded]') + if (!row) throw new Error('row is not expandable') + act(() => row.click()) + } + + /** + * The reported bug: a push that copied resources showed a second row, badged "Fork", for the + * background fill. The fill belongs to the push, so it reads inside the push's own row. + */ + it('shows the background copy inside the push row while it is still running', () => { + renderJobs([ + makeJob({ + kind: 'fork_sync', + workspaceId: WORKSPACE_ID, + status: 'processing', + metadata: syncMetadata, + }), + ]) + + expect(container.querySelectorAll('button[aria-expanded]')).toHaveLength(1) + expect(badgeElement().textContent).toBe('Push') + expandRow() + expect(container.textContent).toContain('Copying') + expect(container.textContent).toContain('2 tables, 1 file') + }) + + it('shows a fill of only skills and documents, which carry no table or file count', () => { + renderJobs([ + makeJob({ + kind: 'fork_sync', + workspaceId: WORKSPACE_ID, + status: 'processing', + metadata: { ...syncMetadata, tables: 0, files: 0, skills: 1, documents: 2 }, + }), + ]) + + expandRow() + expect(container.textContent).toContain('Copying') + expect(container.textContent).toContain('2 documents, 1 skill') + }) + + it('does not call a fill copied when the row failed before it finished', () => { + renderJobs([ + makeJob({ + kind: 'fork_sync', + workspaceId: WORKSPACE_ID, + status: 'failed', + error: 'Background resource copy failed', + metadata: syncMetadata, + }), + ]) + + expandRow() + expect(container.textContent).toContain('Copy failed') + expect(container.textContent).toContain('2 tables, 1 file') + expect(container.textContent).not.toContain('Copied') + }) + + it('surfaces a deploy that succeeded with its cutover still pending', () => { + renderJobs([ + makeJob({ + kind: 'fork_sync', + workspaceId: WORKSPACE_ID, + status: 'completed_with_warnings', + metadata: { + ...syncMetadata, + deployWarnings: ['Flow A — prior workflow version remains active'], + }, + }), + ]) + + expandRow() + expect(container.textContent).toContain('Flow A — prior workflow version remains active') + }) + + it('reports the finished copy and what it lost on the same row', () => { + renderJobs([ + makeJob({ + kind: 'fork_sync', + workspaceId: WORKSPACE_ID, + status: 'completed_with_warnings', + message: 'Copied 2 items; 1 could not be copied', + metadata: { ...syncMetadata, copied: 2, failed: 1 }, + }), + ]) + + expandRow() + expect(container.textContent).toContain('Copied') + expect(container.textContent).not.toContain('Copying') + expect(container.textContent).toContain('2 tables, 1 file') + expect(container.textContent).toContain('1 resource failed to copy') + }) +}) diff --git a/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx b/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx index d0d5d7f4ceb..305c026e154 100644 --- a/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx @@ -5,7 +5,7 @@ import { Badge, Button, Tooltip } from '@sim/emcn' import { createLogger } from '@sim/logger' import { formatDateTime } from '@sim/utils/formatting' import { truncate } from '@sim/utils/string' -import type { BackgroundWorkItem } from '@/lib/api/contracts/workspace-fork' +import type { BackgroundWorkItem, BackgroundWorkMetadata } from '@/lib/api/contracts/workspace-fork' import { ActivityLog, type ActivityLogEntry, @@ -31,6 +31,38 @@ function countList(pairs: Array<[number | undefined, string]>): string { .join(' · ') } +/** + * Per-kind counts of the heavy content a fork or sync fills in the background, as "N tables" + * segments. A sync's fill records counts only; a fork's row carries names as well. + */ +function contentFillCounts(m: NonNullable): string[] { + const kinds: Array<[number | undefined, string]> = [ + [m.knowledgeBases, 'knowledge base'], + [m.documents, 'document'], + [m.tables, 'table'], + [m.files, 'file'], + [m.skills, 'skill'], + ] + return kinds.filter(([n]) => (n ?? 0) > 0).map(([n, noun]) => plural(n as number, noun)) +} + +/** + * Label for a sync's background content fill, by where the row is in it. A failed row keeps + * the counts it planned, so they must not read as copied: the fill was never scheduled, or + * died before finishing, and the row's error says which. + */ +function contentFillLabel(status: BackgroundWorkItem['status']): string { + switch (status) { + case 'pending': + case 'processing': + return 'Copying' + case 'failed': + return 'Copy failed' + default: + return 'Copied' + } +} + /** A named group (one resource kind or change action) of a job's report. */ interface ReportGroup { label: string @@ -66,8 +98,9 @@ function jobTitle(job: BackgroundWorkItem, view: ActivityView): string { const recordedHere = job.workspaceId === view.workspaceId switch (job.kind) { case 'fork_content_copy': - // A partner-recorded copy row is either this workspace's own creation (recorded - // on the parent, carrying our id as the child) or a sync's resource fill. + // A partner-recorded copy row is this workspace's own creation (recorded on the parent, + // carrying our id as the child). A row with no child is a sync's resource fill from + // before those were folded into the sync's own row, and reads by its message. if (!recordedHere && m?.childWorkspaceId === view.workspaceId) { return `Forked from "${partnerName(job, view)}"` } @@ -160,11 +193,21 @@ function jobReport(job: BackgroundWorkItem): JobReport { const addGroup = (label: string, names: string[] | undefined) => { if (names && names.length > 0) groups.push({ label, names }) } + const addContentFillWarnings = () => { + if (m.failed && m.failed > 0) { + notes.push({ value: `${plural(m.failed, 'resource')} failed to copy`, warning: true }) + } + if (m.clearingFailed) { + notes.push({ value: 'Reference cleanup incomplete', warning: true }) + } + } if (job.kind === 'fork_sync') { addGroup('Updated', m.updatedNames) addGroup('Created', m.createdNames) addGroup('Archived', m.archivedNames) + // Resources the sync copied have their content filled in the background on this same row. + addGroup(contentFillLabel(job.status), contentFillCounts(m)) // Pre-names entries fall back to the count summary (redeployed mirrors updated). if (groups.length === 0) { const counts = countList([ @@ -192,6 +235,8 @@ function jobReport(job: BackgroundWorkItem): JobReport { if (m.deployFailed && m.deployFailed > 0) { notes.push({ value: `${plural(m.deployFailed, 'workflow')} failed to deploy`, warning: true }) } + for (const warning of m.deployWarnings ?? []) notes.push({ value: warning, warning: true }) + addContentFillWarnings() return { groups, notes } } @@ -215,26 +260,16 @@ function jobReport(job: BackgroundWorkItem): JobReport { addGroup('Skills', m.skillNames) addGroup('MCP servers', m.mcpServerNames) addGroup('Workflow MCP servers', m.workflowMcpServerNames) - // Sync content-copy rows record per-kind COUNTS only (fork rows carry names), so fall back - // to the counts when no named group rendered. + // Sync fills recorded before they folded into the sync's own row carry per-kind COUNTS only + // (fork rows carry names), so fall back to the counts when no named group rendered. if (groups.length === 0) { const counts = [ - [m.workflowsCopied, 'workflow'], - [m.knowledgeBases, 'knowledge base'], - [m.tables, 'table'], - [m.files, 'file'], - ] - .filter(([n]) => ((n as number | undefined) ?? 0) > 0) - .map(([n, noun]) => plural(n as number, noun as string)) - .join(' · ') + ...(m.workflowsCopied ? [plural(m.workflowsCopied, 'workflow')] : []), + ...contentFillCounts(m), + ].join(' · ') if (counts) notes.push({ value: counts }) } - if (m.failed && m.failed > 0) { - notes.push({ value: `${plural(m.failed, 'resource')} failed to copy`, warning: true }) - } - if (m.clearingFailed) { - notes.push({ value: 'Reference cleanup incomplete', warning: true }) - } + addContentFillWarnings() return { groups, notes } } diff --git a/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.test.ts b/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.test.ts index 90168d7ac40..59b513de74d 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.test.ts @@ -1,9 +1,35 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockFinishBackgroundWork, mockCopyForkResourceContent, mockExecuteForkFileBlobCopies } = + vi.hoisted(() => ({ + mockFinishBackgroundWork: vi.fn(), + mockCopyForkResourceContent: vi.fn(), + mockExecuteForkFileBlobCopies: vi.fn(), + })) + +vi.mock('@/lib/core/config/env-flags', () => ({ isTriggerDevEnabled: false })) +vi.mock('@/lib/core/utils/background', () => ({ runDetached: vi.fn() })) +vi.mock('@/ee/workspace-forking/lib/background-work/store', () => ({ + finishBackgroundWork: mockFinishBackgroundWork, +})) +vi.mock('@/ee/workspace-forking/lib/copy/cleanup-failed', () => ({ + clearFailedForkResourceReferences: vi.fn(async () => ({ cleared: 0, clearingFailed: false })), +})) +vi.mock('@/ee/workspace-forking/lib/copy/copy-files', () => ({ + executeForkFileBlobCopies: mockExecuteForkFileBlobCopies, +})) +vi.mock('@/ee/workspace-forking/lib/copy/copy-resources', () => ({ + copyForkResourceContent: mockCopyForkResourceContent, +})) + +import { db } from '@sim/db' import { + type ForkContentCopyPayload, hasForkContentToCopy, + runForkContentCopy, serializeContentRefMaps, } from '@/ee/workspace-forking/lib/copy/content-copy-runner' import type { BlobCopyTask } from '@/ee/workspace-forking/lib/copy/copy-files' @@ -82,3 +108,69 @@ describe('hasForkContentToCopy', () => { expect(hasForkContentToCopy(emptyPlan(), noBlobs)).toBe(false) }) }) + +describe('runForkContentCopy', () => { + const payload = (overrides: Partial = {}): ForkContentCopyPayload => ({ + contentPlan: { + sourceWorkspaceId: 'src', + childWorkspaceId: 'child', + userId: 'u', + tables: [], + knowledgeBases: [], + skills: [], + documents: [], + }, + blobTasks: [], + statusId: 'status-1', + ...overrides, + }) + + beforeEach(() => { + vi.clearAllMocks() + mockCopyForkResourceContent.mockResolvedValue({ copied: 2, failed: 0, failures: [] }) + mockExecuteForkFileBlobCopies.mockResolvedValue({ copied: 0, failed: 0, failedTargetKeys: [] }) + }) + + it('finishes a clean fill as completed when the caller asks for nothing else', async () => { + await runForkContentCopy(payload()) + + expect(mockFinishBackgroundWork).toHaveBeenCalledWith( + db, + 'status-1', + expect.objectContaining({ status: 'completed', message: 'Copied 2 items' }) + ) + }) + + /** + * A sync's row carries the sync's own warnings before the fill runs. A clean fill must finish + * the row with those warnings intact, not report a clean sync. + */ + it("finishes a clean fill with the caller's completion status", async () => { + await runForkContentCopy(payload({ completionStatus: 'completed_with_warnings' })) + + expect(mockFinishBackgroundWork).toHaveBeenCalledWith( + db, + 'status-1', + expect.objectContaining({ status: 'completed_with_warnings', message: 'Copied 2 items' }) + ) + }) + + it('finishes with warnings whenever an item is lost, whatever the caller asked for', async () => { + mockExecuteForkFileBlobCopies.mockResolvedValue({ + copied: 0, + failed: 1, + failedTargetKeys: ['child-key'], + }) + + await runForkContentCopy(payload({ completionStatus: 'completed' })) + + expect(mockFinishBackgroundWork).toHaveBeenCalledWith( + db, + 'status-1', + expect.objectContaining({ + status: 'completed_with_warnings', + message: 'Copied 2 items; 1 could not be copied', + }) + ) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.ts b/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.ts index 16672fa39cd..fb2af4ac93a 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.ts @@ -67,6 +67,13 @@ export interface ForkContentCopyPayload { * after the fork commits so it's visible immediately. */ statusId?: string + /** + * Status to finish the tracked row with when every item copies; `completed` when omitted. A + * sync whose in-request phase completed with warnings passes `completed_with_warnings`, so a + * clean fill does not clear them. A fill that loses items always finishes with warnings, and + * a crash finishes `failed`. + */ + completionStatus?: 'completed' | 'completed_with_warnings' /** * Target workflow ids this sync deployed (promote's deploy loop). When a copied resource's * fill fails, its dropped placeholder must be cleared from these workflows' DEPLOYED version @@ -150,7 +157,7 @@ export async function runForkContentCopy(payload: ForkContentCopyPayload): Promi const failed = resourceCounts.failed + fileCounts.failed if (statusId) { await finishBackgroundWork(db, statusId, { - status: failed > 0 ? 'completed_with_warnings' : 'completed', + status: failed > 0 ? 'completed_with_warnings' : (payload.completionStatus ?? 'completed'), message: failed > 0 ? `Copied ${copied} item${copied === 1 ? '' : 's'}; ${failed} could not be copied` diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts index b6fcb8a963c..0a60ffc2ec3 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts @@ -1307,6 +1307,10 @@ describe('copyForkResourceContainers table views', () => { const copiedTableId = result.idMap.get('table')?.get('table-with-view') const legacyTableId = result.idMap.get('table')?.get('legacy-table') + // A copied table's id has the same shape as a created one, so nothing downstream can + // tell which path minted it. + expect(copiedTableId).toMatch(/^tbl_[0-9a-f]{32}$/) + expect(legacyTableId).toMatch(/^tbl_[0-9a-f]{32}$/) const copiedViews = inserted.get(tableViews) expect(copiedViews).toEqual( expect.arrayContaining([ diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts index d4a97158433..4143e7d1663 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts @@ -57,6 +57,7 @@ import { replaceKnowledgeDocumentSecretProvenanceInTx, } from '@/lib/knowledge/secret-provenance' import { DEFAULT_TABLE_VIEW_NAME } from '@/lib/table/constants' +import { generateTableId } from '@/lib/table/ids' import { nKeysBetween } from '@/lib/table/order-key' import { classifyTableRowSecretProvenanceForCopy, @@ -673,7 +674,7 @@ export async function copyForkResourceContainers( const inserts: (typeof userTableDefinitions.$inferInsert)[] = [] const viewInserts: (typeof tableViews.$inferInsert)[] = [] for (const definition of definitions) { - const childTableId = generateId() + const childTableId = generateTableId() const remappedSchema = remapForkTableWorkflowGroups( definition.schema as TableSchema, workflowIdMap, diff --git a/apps/sim/ee/workspace-forking/lib/create-fork.ts b/apps/sim/ee/workspace-forking/lib/create-fork.ts index 3cf98b22941..13454b0e8ed 100644 --- a/apps/sim/ee/workspace-forking/lib/create-fork.ts +++ b/apps/sim/ee/workspace-forking/lib/create-fork.ts @@ -560,6 +560,8 @@ export async function createFork(params: CreateForkParams): Promise ({ undeployWorkflow: vi.fn(async () => ({ success: true })), })) vi.mock('@/ee/workspace-forking/lib/background-work/store', () => ({ + recordBackgroundWork: vi.fn(), startBackgroundWork: vi.fn(), })) vi.mock('@/ee/workspace-forking/lib/copy/content-copy-runner', () => ({ @@ -158,7 +159,16 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ })) import { db } from '@sim/db' +import { performFullDeploy } from '@/lib/workflows/orchestration/deploy' import { getBlock } from '@/blocks/registry' +import { + recordBackgroundWork, + startBackgroundWork, +} from '@/ee/workspace-forking/lib/background-work/store' +import { + hasForkContentToCopy, + scheduleForkContentCopy, +} from '@/ee/workspace-forking/lib/copy/content-copy-runner' import { copyWorkflowStateIntoTarget } from '@/ee/workspace-forking/lib/copy/copy-workflows' import { reconcileForkDependentValues } from '@/ee/workspace-forking/lib/mapping/dependent-value-store' import { promoteFork } from '@/ee/workspace-forking/lib/promote/promote' @@ -216,9 +226,43 @@ function promoteParams() { targetWorkspaceId: 'tgt-ws', direction: 'push' as const, userId: 'user-1', + otherWorkspaceName: 'Parent', } } +/** One deployed source workflow the sync writes (replace mode) and then deploys. */ +function arrangeWrittenWorkflow() { + mockComputePlan.mockResolvedValue( + makePlan({ + items: [ + { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + targetName: 'Flow', + mode: 'replace' as const, + sourceMeta: { name: 'Flow', description: null, folderId: null, sortOrder: 0 }, + }, + ], + }) + ) + mockLoadSourceDeployedStates.mockResolvedValue({ + deployedWorkflows: [], + sourceStates: new Map([ + ['wf-src', { blocks: {}, edges: [], loops: {}, parallels: {}, variables: {} }], + ]), + }) + vi.mocked(copyWorkflowStateIntoTarget).mockResolvedValue({ + targetWorkflowId: 'wf-tgt', + mode: 'replace', + name: 'Flow', + blocksCount: 0, + edgesCount: 0, + subflowsCount: 0, + clearedDependents: [], + blockIdMapping: new Map(), + }) +} + /** A copy result carrying no content/id maps, for tests that only need the copy to run. */ function emptyCopyResult() { return { @@ -772,6 +816,39 @@ describe('promoteFork trigger URLs', () => { expect(result.triggerUrlChanges).toEqual([{ workflowName: 'Flow', path: 'live-slack-path' }]) }) + /** + * A lost URL is a warning on the sync's Activity row. When copied resources still need their + * content filled, that row is finished later by the fill - which must keep the warning rather + * than report a clean sync once every item copies. + */ + it('records the lost URL as a sync warning that the content fill keeps', async () => { + arrangeReCreatedTrigger() + mockHasCopySelection.mockReturnValue(true) + mockCopyUnmapped.mockResolvedValue(emptyCopyResult()) + vi.mocked(hasForkContentToCopy).mockReturnValueOnce(true) + vi.mocked(startBackgroundWork).mockResolvedValueOnce('status-1') + + await promoteFork({ + ...promoteParams(), + triggerMappings: [{ sourceBlockId: 'blk-new', adoptPath: null }], + }) + + expect(startBackgroundWork).toHaveBeenCalledWith( + db, + expect.objectContaining({ + kind: 'fork_sync', + metadata: expect.objectContaining({ triggerUrlChanges: 1 }), + }) + ) + expect(scheduleForkContentCopy).toHaveBeenCalledWith( + expect.objectContaining({ + statusId: 'status-1', + completionStatus: 'completed_with_warnings', + }), + expect.anything() + ) + }) + /** The server re-derives the adoptable set, so a stale or crafted path is never honoured. */ it('ignores a mapping naming a path the plan does not offer', async () => { arrangeReCreatedTrigger() @@ -785,3 +862,89 @@ describe('promoteFork trigger URLs', () => { expect(writeParams.triggerPathByBlockId?.size).toBe(0) }) }) + +describe('promoteFork activity', () => { + it('records a deploy that succeeded with a warning as a sync with warnings', async () => { + arrangeWrittenWorkflow() + vi.mocked(performFullDeploy).mockResolvedValueOnce({ + success: true, + warnings: ['prior workflow version remains active'], + } as never) + + const result = await promoteFork(promoteParams()) + + expect(result.deployWarnings).toEqual(['Flow — prior workflow version remains active']) + expect(recordBackgroundWork).toHaveBeenCalledWith( + db, + expect.objectContaining({ + status: 'completed_with_warnings', + metadata: expect.objectContaining({ + deployWarnings: ['Flow — prior workflow version remains active'], + }), + }) + ) + }) + + it('records the sync as one terminal Activity row when nothing is left to fill', async () => { + await promoteFork(promoteParams()) + + expect(recordBackgroundWork).toHaveBeenCalledWith( + db, + expect.objectContaining({ + workspaceId: 'src-ws', + kind: 'fork_sync', + status: 'completed', + message: 'Pushed to "Parent"', + metadata: expect.objectContaining({ + direction: 'push', + otherWorkspaceId: 'tgt-ws', + otherWorkspaceName: 'Parent', + }), + }) + ) + expect(startBackgroundWork).not.toHaveBeenCalled() + expect(scheduleForkContentCopy).not.toHaveBeenCalled() + }) + + /** + * The reported bug: a sync that copied resources opened a second, "Fork"-labelled row for the + * background fill. The fill now runs on the sync's own row, which stays processing until the + * runner finishes it. + */ + it('keeps the sync row processing and hands it to the content fill instead of opening a copy row', async () => { + mockHasCopySelection.mockReturnValue(true) + mockCopyUnmapped.mockResolvedValue({ + ...emptyCopyResult(), + contentPlan: { ...emptyCopyResult().contentPlan, tables: [{} as never] }, + }) + vi.mocked(hasForkContentToCopy).mockReturnValueOnce(true) + vi.mocked(startBackgroundWork).mockResolvedValueOnce('status-1') + + await promoteFork(promoteParams()) + + expect(recordBackgroundWork).not.toHaveBeenCalled() + expect(startBackgroundWork).toHaveBeenCalledTimes(1) + expect(startBackgroundWork).toHaveBeenCalledWith( + db, + expect.objectContaining({ + workspaceId: 'src-ws', + kind: 'fork_sync', + supersede: false, + message: 'Pushed to "Parent"', + metadata: expect.objectContaining({ + direction: 'push', + otherWorkspaceName: 'Parent', + tables: 1, + knowledgeBases: 0, + files: 0, + skills: 0, + documents: 0, + }), + }) + ) + expect(scheduleForkContentCopy).toHaveBeenCalledWith( + expect.objectContaining({ statusId: 'status-1', completionStatus: 'completed' }), + expect.objectContaining({ detachedLabel: 'fork-sync-content-copy' }) + ) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.ts index 1cfb3ec7469..1087e8be65a 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.ts @@ -4,7 +4,11 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, eq, inArray, isNull } from 'drizzle-orm' -import type { ForkSyncBlocker, PromoteCopyResources } from '@/lib/api/contracts/workspace-fork' +import type { + BackgroundWorkMetadata, + ForkSyncBlocker, + PromoteCopyResources, +} from '@/lib/api/contracts/workspace-fork' import type { DbOrTx } from '@/lib/db/types' import { notifyMcpToolServers } from '@/lib/mcp/workflow-mcp-sync' import { @@ -14,7 +18,10 @@ import { import { performFullDeploy } from '@/lib/workflows/orchestration/deploy' import { undeployWorkflow } from '@/lib/workflows/persistence/utils' import { getUsersWithPermissions } from '@/lib/workspaces/permissions/utils' -import { startBackgroundWork } from '@/ee/workspace-forking/lib/background-work/store' +import { + recordBackgroundWork, + startBackgroundWork, +} from '@/ee/workspace-forking/lib/background-work/store' import { type ForkContentCopyPayload, hasForkContentToCopy, @@ -106,8 +113,10 @@ export interface PromoteForkParams { targetWorkspaceId: string direction: 'push' | 'pull' userId: string - /** Initiator's display name, stamped on the sync's content-copy Activity row. */ + /** Initiator's display name, stamped on the sync's Activity row. */ actorName?: string + /** Display name of the edge's other side, for the sync's Activity row title. */ + otherWorkspaceName: string /** * The full stored mapping of dependent-field values the caller is committing (target * workflow id + deterministic block id + subblock key -> value). Applied to the target @@ -156,6 +165,12 @@ export interface PromoteForkResult { * until a redeploy. Surfaced (rather than swallowed) so the caller can warn. */ deployFailed: number + /** + * Deploys that succeeded but left something pending - a cutover still activating, a side + * effect still queued - as ``. The target is not yet running what the + * sync wrote, so the sync's Activity row must not read as clean. + */ + deployWarnings: string[] unmappedRequired: Array> /** * References the sync would have cleared in the target, so it was blocked without writing @@ -193,6 +208,63 @@ export interface PromoteForkResult { triggerUrlChanges: ForkTriggerUrlChange[] } +interface SyncActivity { + /** The workspace the sync was initiated from, whose Manage Forks -> Activity records it. */ + workspaceId: string + status: 'completed' | 'completed_with_warnings' + message: string + metadata: NonNullable +} + +/** + * The sync's Activity row: the in-request outcome (what was written, deployed, and archived, and + * what the target still has to re-pick), phrased from the initiating workspace's side and keyed + * to the edge's other side so the partner's Activity surfaces the same row. + */ +function buildSyncActivity( + result: PromoteForkResult, + params: Pick< + PromoteForkParams, + 'direction' | 'sourceWorkspaceId' | 'targetWorkspaceId' | 'otherWorkspaceName' | 'actorName' + > +): SyncActivity { + const { direction, sourceWorkspaceId, targetWorkspaceId, otherWorkspaceName, actorName } = params + const warnings = + result.deployFailed > 0 || + result.deployWarnings.length > 0 || + result.needsConfiguration.length > 0 || + result.clearedOptional.length > 0 || + result.droppedReferences.length > 0 || + result.triggerUrlChanges.length > 0 + return { + workspaceId: direction === 'push' ? sourceWorkspaceId : targetWorkspaceId, + status: warnings ? 'completed_with_warnings' : 'completed', + message: + direction === 'pull' + ? `Pulled from "${otherWorkspaceName}"` + : `Pushed to "${otherWorkspaceName}"`, + metadata: { + actorName, + otherWorkspaceId: direction === 'push' ? targetWorkspaceId : sourceWorkspaceId, + otherWorkspaceName, + direction, + updated: result.updated, + created: result.created, + archived: result.archived, + redeployed: result.redeployed, + deployFailed: result.deployFailed, + deployWarnings: result.deployWarnings, + updatedNames: result.updatedNames, + createdNames: result.createdNames, + archivedNames: result.archivedNames, + needsConfiguration: result.needsConfiguration, + clearedOptional: result.clearedOptional, + droppedReferences: result.droppedReferences.length, + triggerUrlChanges: result.triggerUrlChanges.length, + }, + } +} + function collectCredentialPairs(plan: ForkPromotePlan): Array<[string, string]> { const pairs = new Map() for (const reference of plan.references) { @@ -995,6 +1067,7 @@ export async function promoteFork(params: PromoteForkParams): Promise ({ + workflowName, + blocks, + })), + clearedOptional: txResult.clearedOptional, + droppedReferences: txResult.droppedReferences, + triggerUrlChanges: txResult.triggerUrlChanges, + } + // Fill the heavy content (table rows, KB documents + embeddings) of resources copied into the // target this sync and rewrite copied skill bodies, off the request path. Scheduled AFTER the // deploy loop so every deployed version this sync cut already EXISTS: a failed content fill's @@ -1080,45 +1176,64 @@ export async function promoteFork(params: PromoteForkParams): Promise Activity the user is - // viewing (the one the sync was initiated from), matching where the route records the sync. - const activityWorkspaceId = direction === 'push' ? sourceWorkspaceId : targetWorkspaceId - // The sync already committed; failing to record the tracking row must not turn it into a 500. - // The runner no-ops its status updates when statusId is absent, so the copy still runs. - let statusId: string | undefined - try { + ? { contentPlan: copyContentPlan, blobTasks: copyBlobTasks } + : null + + // One Activity row per sync. The content fill is part of that row, not an entry of its own: + // when there is one, the row stays `processing` and the runner finishes it, merging the fill's + // copied/failed counts into the sync's report. The sync already committed, so failing to record + // the row must not turn it into a 500 - the runner no-ops its status updates when statusId is + // absent, and the copy still runs. + const activity = buildSyncActivity(result, { + direction, + sourceWorkspaceId, + targetWorkspaceId, + otherWorkspaceName: params.otherWorkspaceName, + actorName: params.actorName, + }) + let statusId: string | undefined + try { + if (contentFill) { statusId = await startBackgroundWork(db, { - workspaceId: activityWorkspaceId, - kind: 'fork_content_copy', - // Append-only: each sync's content fill is a distinct entry in the Activity history. + workspaceId: activity.workspaceId, + kind: 'fork_sync', + // Append-only: each sync is a distinct entry in the Activity history. supersede: false, - message: 'Copying synced resources', + message: activity.message, metadata: { - // The edge's other side, so the partner workspace's Activity surfaces this row too. - otherWorkspaceId: direction === 'push' ? targetWorkspaceId : sourceWorkspaceId, - // The content fill runs as a background worker with no session; the Activity - // actor is the user who initiated the sync, not "System". - actorName: params.actorName, - tables: copyContentPlan.tables.length, - knowledgeBases: copyContentPlan.knowledgeBases.length, - files: copyBlobTasks.length, + ...activity.metadata, + tables: contentFill.contentPlan.tables.length, + knowledgeBases: contentFill.contentPlan.knowledgeBases.length, + files: contentFill.blobTasks.length, + skills: contentFill.contentPlan.skills.length, + documents: contentFill.contentPlan.documents.length, }, }) - } catch (error) { - logger.error(`[${requestId}] Failed to record sync content-copy status`, { - targetWorkspaceId, - error: getErrorMessage(error), + } else { + await recordBackgroundWork(db, { + workspaceId: activity.workspaceId, + kind: 'fork_sync', + status: activity.status, + message: activity.message, + metadata: activity.metadata, }) } + } catch (error) { + logger.error(`[${requestId}] Failed to record sync activity`, { + targetWorkspaceId, + error: getErrorMessage(error), + }) + } + if (contentFill) { const payload: ForkContentCopyPayload = { - contentPlan: copyContentPlan, - blobTasks: copyBlobTasks, + contentPlan: contentFill.contentPlan, + blobTasks: contentFill.blobTasks, contentRefMaps: txResult.copyContentRefMaps ?? undefined, statusId, + completionStatus: activity.status, // The targets this sync wrote and deployed above, so a failed content fill can sweep the // dropped placeholder from their DEPLOYED version states too, not just drafts. deployedTargetWorkflowIds: txResult.deployTargetIds, @@ -1160,25 +1275,5 @@ export async function promoteFork(params: PromoteForkParams): Promise ({ - workflowName, - blocks, - })), - clearedOptional: txResult.clearedOptional, - droppedReferences: txResult.droppedReferences, - triggerUrlChanges: txResult.triggerUrlChanges, - } + return result } diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts index 1fd1d6b7f9b..518f2fcc17d 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts @@ -1835,6 +1835,75 @@ describe('canonical mode policy (fork/promote)', () => { expect(scan.references).toEqual([]) }) + it('does NOT detect {{ENV}} in a Note block (an annotation never executes)', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'content', title: 'Content', type: 'long-input' }]) + ) + const scan = scanWorkflowReferences( + [ + { + id: 'b1', + name: 'Setup notes', + type: 'note', + subBlocks: { + content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.'), + }, + }, + ], + () => null + ) + expect(scan.references).toEqual([]) + expect(scan.unmapped).toEqual([]) + }) + + it('still detects {{ENV}} named by both a Note and an executing block, attributed to the executing block', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { id: 'content', title: 'Content', type: 'long-input' }, + { id: 'apiKey', title: 'API Key', type: 'short-input' }, + ]) + ) + const scan = scanWorkflowReferences( + [ + { + id: 'b1', + name: 'Setup notes', + type: 'note', + subBlocks: { + content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.'), + }, + }, + { + id: 'b2', + name: 'Agent', + type: 'agent', + subBlocks: { apiKey: entry('apiKey', 'short-input', '{{OPENAI_API_KEY}}') }, + }, + ], + () => null + ) + expect( + scan.references.map((ref) => [ref.kind, ref.sourceId, ref.blockId, ref.subBlockKey]) + ).toEqual([['env-var', 'OPENAI_API_KEY', 'b2', 'apiKey']]) + expect(scan.unmapped.map((ref) => ref.sourceId)).toEqual(['OPENAI_API_KEY']) + }) + + it('still rewrites a mapped {{ENV}} inside a Note on promote, so the note names the target key', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'content', title: 'Content', type: 'long-input' }]) + ) + const result = remapForkSubBlocks( + { content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.') }, + (kind, sourceId) => + kind === 'env-var' && sourceId === 'OPENAI_API_KEY' ? 'OPENAI_KEY_PROD' : null, + 'promote', + { blockType: 'note' } + ) + expect(result.subBlocks.content.value).toBe('Set {{OPENAI_KEY_PROD}} before running.') + expect(result.references).toEqual([]) + expect(result.unmapped).toEqual([]) + }) + it('an active manual member keeps its RESOURCE-id escape hatch while its {{ENV}} is detected', () => { vi.mocked(getBlock).mockReturnValue( blockWith([ diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts index 2bd9c9d8149..51a83a6570f 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike, omit } from '@sim/utils/object' import type { SubBlockType } from '@sim/workflow-types/blocks' +import { isWorkflowAnnotationOnlyBlockType } from '@sim/workflow-types/workflow' import type { z } from 'zod' import type { forkRemapKindSchema } from '@/lib/api/contracts/workspace-fork' import { readFolderPaths, replaceFolderPath } from '@/lib/folders/selection' @@ -580,7 +581,10 @@ export function createCanonicalModeGates( export interface RemapForkContext { blockId?: string blockName?: string - /** Block type, to build the canonical index for active-member DETECTION gating (rewrite unaffected). */ + /** + * Block type, to build the canonical index for active-member DETECTION gating and to recognise + * an annotation-only block, whose values are never detected. Rewrite is unaffected by either. + */ blockType?: string /** Canonical-mode overrides (`block.data.canonicalModes`), picking the active member per pair. */ canonicalModes?: CanonicalModeOverrides @@ -1126,9 +1130,9 @@ export function remapForkSubBlocks( // active ADVANCED (manual) member - and every dependent scoped to it - passes through VERBATIM // (user-owned, never remapped, never a mapping requirement); a DORMANT member's value is // CLEARED outright (below) so no stale id ever survives in an inactive slot. A condition-hidden - // subblock is still rewritten but not detected. Needs `blockType` for the config; an unknown - // block type gets no gating (everything detected, nothing passed through - the conservative - // default). + // subblock is still rewritten but not detected, and so is every field of an annotation-only + // block (see `annotationOnly` below). Needs `blockType` for the config; an unknown block type + // gets no gating (everything detected, nothing passed through - the conservative default). const blockSubBlocks = context?.blockType ? getBlock(context.blockType)?.subBlocks : undefined const configByBaseKey = new Map( (blockSubBlocks ?? []).filter((config) => config.id).map((config) => [config.id, config]) @@ -1140,6 +1144,15 @@ export function remapForkSubBlocks( context?.triggerMode === true ) + /** + * An annotation-only block (a Note) never executes, so nothing in it is a reference. Its + * `{{KEY}}` is prose about a secret, not a use of one: it must not become a mapping entry or a + * sync blocker, and the References tab — which walks blocks through this same policy — must not + * send someone rotating a key to edit a note. The rewrite side is unchanged, exactly like a + * condition-hidden field's: a mapped key is still renamed so the note names the target's key. + */ + const annotationOnly = isWorkflowAnnotationOnlyBlockType(context?.blockType) + for (const [subBlockKey, subBlock] of Object.entries(subBlocks)) { if (!subBlock || typeof subBlock !== 'object') { result[subBlockKey] = subBlock @@ -1169,7 +1182,8 @@ export function remapForkSubBlocks( const verbatimManual = !dormant && (gates.isActiveManualMember(subBlockKey) || gates.isManualParentDependent(subBlockKey)) - const detectionSkipped = dormant || verbatimManual || gates.isConditionHidden(subBlockKey) + const detectionSkipped = + annotationOnly || dormant || verbatimManual || gates.isConditionHidden(subBlockKey) // `{{ENV}}` detection is gated on EXECUTION, not on ownership. A dormant member and a // condition-hidden field never execute, so their refs must not become sync blockers - but an // ACTIVE MANUAL member is exactly the value that DOES execute, and its `{{KEY}}` is a live @@ -1179,7 +1193,7 @@ export function remapForkSubBlocks( // missing that secret silently passed the required-env gate instead of blocking the sync. // Resource-id detection keeps `verbatimManual` (a hand-typed id stays a user-owned escape // hatch); only env refs, which are never workspace-scoped ids, are detected here. - const envDetectionSkipped = dormant || gates.isConditionHidden(subBlockKey) + const envDetectionSkipped = annotationOnly || dormant || gates.isConditionHidden(subBlockKey) if (dormant && isNonEmptyValue(value)) { value = '' } diff --git a/apps/sim/hooks/queries/credentials.ts b/apps/sim/hooks/queries/credentials.ts index 6fd44768f66..1fa3975113d 100644 --- a/apps/sim/hooks/queries/credentials.ts +++ b/apps/sim/hooks/queries/credentials.ts @@ -319,11 +319,13 @@ export function useSecretUsage({ workspaceId, name, scope }: SecretUsageParams, } /** - * References only move when someone edits a workflow, a custom tool, or an MCP server — far - * less often than the usage trail, which every run appends to. A longer window keeps the scan - * (which reads every candidate block in the workspace) off the wire on tab switches. + * Always stale: the list mirrors the canvas, and the reader has usually just come from editing + * it — deleting the block a row pointed at, then returning here to see it gone. A stale window + * served the old list for its whole length, and no invalidation can cover a workflow someone + * else changed. Cached data still shows while the scan refreshes, so a tab switch costs one + * bounded, prefiltered query rather than a blank panel. */ -export const SECRET_REFERENCES_STALE_TIME = 5 * 60 * 1000 +export const SECRET_REFERENCES_STALE_TIME = 0 interface SecretReferencesParams { workspaceId?: string @@ -334,6 +336,11 @@ interface SecretReferencesParams { * Reads where one secret is wired in. Takes no scope: a reference names a key, not a scope, and * the server authorizes against what the name resolves to. Only credential admins of a workspace * secret — or the owner of a personal one — are authorized server-side. + * + * Refetches on window focus even on the web, where the app default leaves it off: the edit that + * moves a reference happens on a canvas, often in another tab, and this panel has no other + * signal for it. `'always'`, not `true`: `true` refetches only a STALE query, which would tie + * this guarantee to `SECRET_REFERENCES_STALE_TIME` staying exactly 0. */ export function useSecretReferences({ workspaceId, name }: SecretReferencesParams, enabled = true) { return useQuery({ @@ -345,5 +352,6 @@ export function useSecretReferences({ workspaceId, name }: SecretReferencesParam }), enabled: Boolean(workspaceId && name) && enabled, staleTime: SECRET_REFERENCES_STALE_TIME, + refetchOnWindowFocus: 'always', }) } diff --git a/apps/sim/lib/api/contracts/workspace-fork.ts b/apps/sim/lib/api/contracts/workspace-fork.ts index 44fca04ac4d..8238e7dcc28 100644 --- a/apps/sim/lib/api/contracts/workspace-fork.ts +++ b/apps/sim/lib/api/contracts/workspace-fork.ts @@ -777,13 +777,17 @@ export const backgroundWorkMetadataSchema = z .object({ /** Display name of the user who performed the action (denormalized at write time). */ actorName: z.string().optional(), - // Fork content copy + // Fork content copy. The per-kind counts and copied/failed also describe the background + // fill of the resources a sync copied, which reports on the sync's own row. childWorkspaceId: z.string().optional(), childWorkspaceName: z.string().optional(), workflowsCopied: z.number().int().optional(), tables: z.number().int().optional(), knowledgeBases: z.number().int().optional(), files: z.number().int().optional(), + skills: z.number().int().optional(), + /** Documents copied into an already-mapped target knowledge base (sync only). */ + documents: z.number().int().optional(), copied: z.number().int().optional(), failed: z.number().int().optional(), /** Count of failed resources whose dangling references were cleared post-fork (U8). */ @@ -813,6 +817,8 @@ export const backgroundWorkMetadataSchema = z archived: z.number().int().optional(), redeployed: z.number().int().optional(), deployFailed: z.number().int().optional(), + /** Deploys that succeeded with a cutover or side effect still pending, as ``. */ + deployWarnings: z.array(z.string()).optional(), restored: z.number().int().optional(), unarchived: z.number().int().optional(), removed: z.number().int().optional(), diff --git a/apps/sim/lib/secrets/references/scan.test.ts b/apps/sim/lib/secrets/references/scan.test.ts index 89f0345bf14..533d6d60a15 100644 --- a/apps/sim/lib/secrets/references/scan.test.ts +++ b/apps/sim/lib/secrets/references/scan.test.ts @@ -114,6 +114,65 @@ describe('scanSecretReferences', () => { expect(scan.workflows).toEqual([]) }) + /** + * A Note is documentation on the canvas: its text never resolves at run time, so a `{{KEY}}` + * in it is prose about the secret, not a use of it. The remapper drops it for the same reason + * it drops a condition-hidden field, and the tab has to agree — rotating a key does not mean + * editing every note that mentions it. + */ + it('drops a Note block that mentions the secret', async () => { + queueTableRows(schemaMock.workflowBlocks, [ + blockRow({ + blockId: 'block-1', + blockName: 'Setup notes', + blockType: 'note', + workflowId: 'workflow-1', + workflowName: 'Nightly sync', + subBlocks: { + content: { id: 'content', type: 'long-input', value: 'Uses {{API_KEY}} for auth.' }, + }, + }), + ]) + + const scan = await scanSecretReferences({ workspaceId: 'workspace-1', name: 'API_KEY' }) + + expect(scan.workflows).toEqual([]) + }) + + it('lists only the executing block when a Note and a block both name the secret', async () => { + queueTableRows(schemaMock.workflowBlocks, [ + blockRow({ + blockId: 'block-1', + blockName: 'Setup notes', + blockType: 'note', + workflowId: 'workflow-1', + workflowName: 'Nightly sync', + subBlocks: { + content: { id: 'content', type: 'long-input', value: 'Uses {{API_KEY}} for auth.' }, + }, + }), + blockRow({ + blockId: 'block-2', + blockName: 'Fetch orders', + workflowId: 'workflow-1', + workflowName: 'Nightly sync', + subBlocks: shortInput('apiKey', '{{API_KEY}}'), + }), + ]) + + const scan = await scanSecretReferences({ workspaceId: 'workspace-1', name: 'API_KEY' }) + + expect(scan.workflows).toEqual([ + { + workflowId: 'workflow-1', + workflowName: 'Nightly sync', + blocks: [ + { blockId: 'block-2', blockName: 'Fetch orders', blockType: 'agent', field: 'apiKey' }, + ], + }, + ]) + }) + /** * The fork remapper collapses a block's references to one per `(kind, sourceId)`, so a block * naming the secret twice yields one entry, not two. Pinned here because the panel renders diff --git a/apps/sim/lib/table/ids.test.ts b/apps/sim/lib/table/ids.test.ts new file mode 100644 index 00000000000..a03821fa4f0 --- /dev/null +++ b/apps/sim/lib/table/ids.test.ts @@ -0,0 +1,15 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { generateTableId } from '@/lib/table/ids' + +describe('generateTableId', () => { + it('mints a tbl_ prefix over a dash-stripped UUID', () => { + expect(generateTableId()).toMatch(/^tbl_[0-9a-f]{32}$/) + }) + + it('mints a distinct id each time', () => { + expect(generateTableId()).not.toBe(generateTableId()) + }) +}) diff --git a/apps/sim/lib/table/ids.ts b/apps/sim/lib/table/ids.ts new file mode 100644 index 00000000000..85931337551 --- /dev/null +++ b/apps/sim/lib/table/ids.ts @@ -0,0 +1,11 @@ +import { generateId } from '@sim/utils/id' + +/** + * Mints a fresh table id: `tbl_` plus a dash-stripped v4 UUID, the shape Tables have carried + * since they shipped and the one the CLI and Copilot describe to users. Every path that + * creates a table definition — the create service and the fork copy — mints through here, so + * a table's id never reveals which path made it. + */ +export function generateTableId(): string { + return `tbl_${generateId().replace(/-/g, '')}` +} diff --git a/apps/sim/lib/table/index.ts b/apps/sim/lib/table/index.ts index 72585f3c07a..69605b9f4f6 100644 --- a/apps/sim/lib/table/index.ts +++ b/apps/sim/lib/table/index.ts @@ -12,6 +12,7 @@ export * from '@/lib/table/constants' export * from '@/lib/table/currency' export * from '@/lib/table/dates' export * from '@/lib/table/errors' +export * from '@/lib/table/ids' export * from '@/lib/table/import' export * from '@/lib/table/import-data' export * from '@/lib/table/jobs/service' diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 36328223e2a..87b6de8e544 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -43,6 +43,7 @@ import { TABLE_LIMITS, } from '@/lib/table/constants' import { appendTableEvent } from '@/lib/table/events' +import { generateTableId } from '@/lib/table/ids' import { EMPTY_JOB_FIELDS, latestJobsForTables, @@ -565,7 +566,7 @@ export async function createTable( await assertTableRowTtlEnabled() } - const tableId = `tbl_${generateId().replace(/-/g, '')}` + const tableId = generateTableId() const now = new Date() // Stamp stable ids so the table is id-keyed from its first row write.