Skip to content

Commit f62386d

Browse files
committed
fix(forks): report a sync's resource copy on its push or pull row instead of a separate Fork entry
1 parent d7a83ce commit f62386d

10 files changed

Lines changed: 498 additions & 136 deletions

File tree

apps/docs/content/docs/platform/enterprise/forks.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ The setting belongs to **this workspace's copy** only. Excluding a workflow here
136136

137137
<Image src="/static/enterprise/forks-activity.png" alt="Activity view showing Fork and Push events with expandable detail rows" width={900} height={614} />
138138

139-
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).
139+
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.
140140

141141
---
142142

apps/sim/app/api/workspaces/[id]/fork/promote/route.test.ts

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Tests for the fork sync (promote) route's error projection.
2+
* Tests for the fork sync (promote) route's error projection and input mapping.
33
*
44
* `promoteFork` returns its deliberate refusals as a `blocked` result, but a classified
55
* 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
1212
import { beforeEach, describe, expect, it, vi } from 'vitest'
1313
import { FolderCollectionFullError } from '@/lib/folders/errors'
1414

15-
const { mockLogger, mockPromoteFork, mockAssertCanPromote, mockRecordBackgroundWork } = vi.hoisted(
16-
() => ({
17-
mockLogger: {
18-
info: vi.fn(),
19-
warn: vi.fn(),
20-
error: vi.fn(),
21-
debug: vi.fn(),
22-
trace: vi.fn(),
23-
fatal: vi.fn(),
24-
child: vi.fn(),
25-
},
26-
mockPromoteFork: vi.fn(),
27-
mockAssertCanPromote: vi.fn(),
28-
mockRecordBackgroundWork: vi.fn(),
29-
})
30-
)
15+
const { mockLogger, mockPromoteFork, mockAssertCanPromote } = vi.hoisted(() => ({
16+
mockLogger: {
17+
info: vi.fn(),
18+
warn: vi.fn(),
19+
error: vi.fn(),
20+
debug: vi.fn(),
21+
trace: vi.fn(),
22+
fatal: vi.fn(),
23+
child: vi.fn(),
24+
},
25+
mockPromoteFork: vi.fn(),
26+
mockAssertCanPromote: vi.fn(),
27+
}))
3128

3229
vi.mock('@sim/audit', () => auditMock)
3330
vi.mock('@sim/logger', () => ({
@@ -39,9 +36,6 @@ vi.mock('@/ee/workspace-forking/lib/promote/promote', () => ({ promoteFork: mock
3936
vi.mock('@/ee/workspace-forking/lib/lineage/authz', () => ({
4037
assertCanPromote: mockAssertCanPromote,
4138
}))
42-
vi.mock('@/ee/workspace-forking/lib/background-work/store', () => ({
43-
recordBackgroundWork: mockRecordBackgroundWork,
44-
}))
4539

4640
import { POST } from '@/app/api/workspaces/[id]/fork/promote/route'
4741

@@ -69,8 +63,41 @@ describe('POST /api/workspaces/[id]/fork/promote', () => {
6963
edge: { childWorkspaceId: WORKSPACE_ID },
7064
sourceWorkspaceId: WORKSPACE_ID,
7165
targetWorkspaceId: 'ws-parent',
66+
source: { name: 'Child' },
67+
target: { name: 'Parent' },
68+
})
69+
})
70+
71+
/**
72+
* The sync's Activity row is recorded by the use case, not here, so the route's job is to
73+
* hand it the one thing only the route knows: the display name of the edge's other side.
74+
*/
75+
it('names the other side of the edge for promoteFork to record the sync', async () => {
76+
mockPromoteFork.mockResolvedValue({
77+
promoteRunId: 'run-1',
78+
updated: 1,
79+
created: 0,
80+
archived: 0,
81+
redeployed: 1,
82+
deployFailed: 0,
83+
unmappedRequired: [],
84+
blockers: [],
85+
blocked: null,
86+
updatedNames: ['Flow'],
87+
createdNames: [],
88+
archivedNames: [],
89+
needsConfiguration: [],
90+
clearedOptional: [],
91+
droppedReferences: [],
92+
triggerUrlChanges: [],
7293
})
73-
mockRecordBackgroundWork.mockResolvedValue(undefined)
94+
95+
const response = await POST(promoteRequest(), routeContext)
96+
97+
expect(response.status).toBe(200)
98+
expect(mockPromoteFork).toHaveBeenCalledWith(
99+
expect.objectContaining({ direction: 'push', actorName: 'A', otherWorkspaceName: 'Parent' })
100+
)
74101
})
75102

76103
it('renders a full-folder-tree refusal as an actionable 409', async () => {

apps/sim/app/api/workspaces/[id]/fork/promote/route.ts

Lines changed: 3 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
2-
import { db } from '@sim/db'
32
import { createLogger } from '@sim/logger'
4-
import { getErrorMessage } from '@sim/utils/errors'
53
import { type NextRequest, NextResponse } from 'next/server'
64
import { promoteForkContract } from '@/lib/api/contracts/workspace-fork'
75
import { parseRequest } from '@/lib/api/server'
86
import { getSession } from '@/lib/auth'
97
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
108
import { generateRequestId } from '@/lib/core/utils/request'
119
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
12-
import { recordBackgroundWork } from '@/ee/workspace-forking/lib/background-work/store'
1310
import { assertCanPromote } from '@/ee/workspace-forking/lib/lineage/authz'
1411
import { promoteFork } from '@/ee/workspace-forking/lib/promote/promote'
1512

@@ -36,6 +33,8 @@ export const POST = withRouteHandler(
3633
} = parsed.data.body
3734

3835
const auth = await assertCanPromote(id, otherWorkspaceId, direction, session.user.id)
36+
const otherName =
37+
otherWorkspaceId === auth.sourceWorkspaceId ? auth.source.name : auth.target.name
3938

4039
let result: Awaited<ReturnType<typeof promoteFork>>
4140
try {
@@ -46,6 +45,7 @@ export const POST = withRouteHandler(
4645
direction,
4746
userId: session.user.id,
4847
actorName: session.user.name ?? undefined,
48+
otherWorkspaceName: otherName,
4949
dependentValues,
5050
copyResources,
5151
dropReferences,
@@ -114,44 +114,6 @@ export const POST = withRouteHandler(
114114
request: req,
115115
})
116116

117-
const otherName =
118-
otherWorkspaceId === auth.sourceWorkspaceId ? auth.source.name : auth.target.name
119-
await recordBackgroundWork(db, {
120-
workspaceId: id,
121-
kind: 'fork_sync',
122-
status:
123-
result.deployFailed > 0 ||
124-
result.needsConfiguration.length > 0 ||
125-
result.clearedOptional.length > 0 ||
126-
result.droppedReferences.length > 0 ||
127-
result.triggerUrlChanges.length > 0
128-
? 'completed_with_warnings'
129-
: 'completed',
130-
message: direction === 'pull' ? `Pulled from "${otherName}"` : `Pushed to "${otherName}"`,
131-
metadata: {
132-
actorName: session.user.name ?? undefined,
133-
otherWorkspaceId,
134-
otherWorkspaceName: otherName,
135-
direction,
136-
updated: result.updated,
137-
created: result.created,
138-
archived: result.archived,
139-
redeployed: result.redeployed,
140-
deployFailed: result.deployFailed,
141-
updatedNames: result.updatedNames,
142-
createdNames: result.createdNames,
143-
archivedNames: result.archivedNames,
144-
needsConfiguration: result.needsConfiguration,
145-
clearedOptional: result.clearedOptional,
146-
droppedReferences: result.droppedReferences.length,
147-
triggerUrlChanges: result.triggerUrlChanges.length,
148-
},
149-
}).catch((error) =>
150-
logger.error(`[${requestId}] Failed to record sync activity`, {
151-
error: getErrorMessage(error),
152-
})
153-
)
154-
155117
return NextResponse.json(body)
156118
}
157119
)

apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.test.tsx

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,72 @@ describe('ForkActivityPanel event badge tooltip', () => {
169169
expect(row?.getAttribute('aria-expanded')).toBe('false')
170170
})
171171
})
172+
173+
describe('ForkActivityPanel sync report', () => {
174+
beforeEach(() => {
175+
vi.clearAllMocks()
176+
container = document.createElement('div')
177+
document.body.appendChild(container)
178+
root = createRoot(container)
179+
})
180+
181+
afterEach(() => {
182+
act(() => root.unmount())
183+
container.remove()
184+
})
185+
186+
const syncMetadata = {
187+
actorName: 'Brandon Tarr',
188+
direction: 'push' as const,
189+
otherWorkspaceId: PARTNER_ID,
190+
otherWorkspaceName: 'another workspace',
191+
updatedNames: ['Flow A'],
192+
tables: 2,
193+
files: 1,
194+
}
195+
196+
function expandRow() {
197+
const row = container.querySelector<HTMLButtonElement>('button[aria-expanded]')
198+
if (!row) throw new Error('row is not expandable')
199+
act(() => row.click())
200+
}
201+
202+
/**
203+
* The reported bug: a push that copied resources showed a second row, badged "Fork", for the
204+
* background fill. The fill belongs to the push, so it reads inside the push's own row.
205+
*/
206+
it('shows the background copy inside the push row while it is still running', () => {
207+
renderJobs([
208+
makeJob({
209+
kind: 'fork_sync',
210+
workspaceId: WORKSPACE_ID,
211+
status: 'processing',
212+
metadata: syncMetadata,
213+
}),
214+
])
215+
216+
expect(container.querySelectorAll('button[aria-expanded]')).toHaveLength(1)
217+
expect(badgeElement().textContent).toBe('Push')
218+
expandRow()
219+
expect(container.textContent).toContain('Copying')
220+
expect(container.textContent).toContain('2 tables, 1 file')
221+
})
222+
223+
it('reports the finished copy and what it lost on the same row', () => {
224+
renderJobs([
225+
makeJob({
226+
kind: 'fork_sync',
227+
workspaceId: WORKSPACE_ID,
228+
status: 'completed_with_warnings',
229+
message: 'Copied 2 items; 1 could not be copied',
230+
metadata: { ...syncMetadata, copied: 2, failed: 1 },
231+
}),
232+
])
233+
234+
expandRow()
235+
expect(container.textContent).toContain('Copied')
236+
expect(container.textContent).not.toContain('Copying')
237+
expect(container.textContent).toContain('2 tables, 1 file')
238+
expect(container.textContent).toContain('1 resource failed to copy')
239+
})
240+
})

apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Badge, Button, Tooltip } from '@sim/emcn'
55
import { createLogger } from '@sim/logger'
66
import { formatDateTime } from '@sim/utils/formatting'
77
import { truncate } from '@sim/utils/string'
8-
import type { BackgroundWorkItem } from '@/lib/api/contracts/workspace-fork'
8+
import type { BackgroundWorkItem, BackgroundWorkMetadata } from '@/lib/api/contracts/workspace-fork'
99
import {
1010
ActivityLog,
1111
type ActivityLogEntry,
@@ -31,6 +31,19 @@ function countList(pairs: Array<[number | undefined, string]>): string {
3131
.join(' · ')
3232
}
3333

34+
/**
35+
* Per-kind counts of the heavy content a fork or sync fills in the background, as "N tables"
36+
* segments. A sync's fill records counts only; a fork's row carries names as well.
37+
*/
38+
function contentFillCounts(m: NonNullable<BackgroundWorkMetadata>): string[] {
39+
const kinds: Array<[number | undefined, string]> = [
40+
[m.knowledgeBases, 'knowledge base'],
41+
[m.tables, 'table'],
42+
[m.files, 'file'],
43+
]
44+
return kinds.filter(([n]) => (n ?? 0) > 0).map(([n, noun]) => plural(n as number, noun))
45+
}
46+
3447
/** A named group (one resource kind or change action) of a job's report. */
3548
interface ReportGroup {
3649
label: string
@@ -66,8 +79,9 @@ function jobTitle(job: BackgroundWorkItem, view: ActivityView): string {
6679
const recordedHere = job.workspaceId === view.workspaceId
6780
switch (job.kind) {
6881
case 'fork_content_copy':
69-
// A partner-recorded copy row is either this workspace's own creation (recorded
70-
// on the parent, carrying our id as the child) or a sync's resource fill.
82+
// A partner-recorded copy row is this workspace's own creation (recorded on the parent,
83+
// carrying our id as the child). A row with no child is a sync's resource fill from
84+
// before those were folded into the sync's own row, and reads by its message.
7185
if (!recordedHere && m?.childWorkspaceId === view.workspaceId) {
7286
return `Forked from "${partnerName(job, view)}"`
7387
}
@@ -160,11 +174,21 @@ function jobReport(job: BackgroundWorkItem): JobReport {
160174
const addGroup = (label: string, names: string[] | undefined) => {
161175
if (names && names.length > 0) groups.push({ label, names })
162176
}
177+
const addContentFillWarnings = () => {
178+
if (m.failed && m.failed > 0) {
179+
notes.push({ value: `${plural(m.failed, 'resource')} failed to copy`, warning: true })
180+
}
181+
if (m.clearingFailed) {
182+
notes.push({ value: 'Reference cleanup incomplete', warning: true })
183+
}
184+
}
163185

164186
if (job.kind === 'fork_sync') {
165187
addGroup('Updated', m.updatedNames)
166188
addGroup('Created', m.createdNames)
167189
addGroup('Archived', m.archivedNames)
190+
// Resources the sync copied have their content filled in the background on this same row.
191+
addGroup(job.status === 'processing' ? 'Copying' : 'Copied', contentFillCounts(m))
168192
// Pre-names entries fall back to the count summary (redeployed mirrors updated).
169193
if (groups.length === 0) {
170194
const counts = countList([
@@ -192,6 +216,7 @@ function jobReport(job: BackgroundWorkItem): JobReport {
192216
if (m.deployFailed && m.deployFailed > 0) {
193217
notes.push({ value: `${plural(m.deployFailed, 'workflow')} failed to deploy`, warning: true })
194218
}
219+
addContentFillWarnings()
195220
return { groups, notes }
196221
}
197222

@@ -215,26 +240,16 @@ function jobReport(job: BackgroundWorkItem): JobReport {
215240
addGroup('Skills', m.skillNames)
216241
addGroup('MCP servers', m.mcpServerNames)
217242
addGroup('Workflow MCP servers', m.workflowMcpServerNames)
218-
// Sync content-copy rows record per-kind COUNTS only (fork rows carry names), so fall back
219-
// to the counts when no named group rendered.
243+
// Sync fills recorded before they folded into the sync's own row carry per-kind COUNTS only
244+
// (fork rows carry names), so fall back to the counts when no named group rendered.
220245
if (groups.length === 0) {
221246
const counts = [
222-
[m.workflowsCopied, 'workflow'],
223-
[m.knowledgeBases, 'knowledge base'],
224-
[m.tables, 'table'],
225-
[m.files, 'file'],
226-
]
227-
.filter(([n]) => ((n as number | undefined) ?? 0) > 0)
228-
.map(([n, noun]) => plural(n as number, noun as string))
229-
.join(' · ')
247+
...(m.workflowsCopied ? [plural(m.workflowsCopied, 'workflow')] : []),
248+
...contentFillCounts(m),
249+
].join(' · ')
230250
if (counts) notes.push({ value: counts })
231251
}
232-
if (m.failed && m.failed > 0) {
233-
notes.push({ value: `${plural(m.failed, 'resource')} failed to copy`, warning: true })
234-
}
235-
if (m.clearingFailed) {
236-
notes.push({ value: 'Reference cleanup incomplete', warning: true })
237-
}
252+
addContentFillWarnings()
238253
return { groups, notes }
239254
}
240255

0 commit comments

Comments
 (0)