Skip to content

Commit c926eff

Browse files
committed
fix(forks): label a failed fill honestly, count skills and documents, and surface deploy warnings on the sync row
1 parent db8a55e commit c926eff

6 files changed

Lines changed: 147 additions & 2 deletions

File tree

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,55 @@ describe('ForkActivityPanel sync report', () => {
220220
expect(container.textContent).toContain('2 tables, 1 file')
221221
})
222222

223+
it('shows a fill of only skills and documents, which carry no table or file count', () => {
224+
renderJobs([
225+
makeJob({
226+
kind: 'fork_sync',
227+
workspaceId: WORKSPACE_ID,
228+
status: 'processing',
229+
metadata: { ...syncMetadata, tables: 0, files: 0, skills: 1, documents: 2 },
230+
}),
231+
])
232+
233+
expandRow()
234+
expect(container.textContent).toContain('Copying')
235+
expect(container.textContent).toContain('2 documents, 1 skill')
236+
})
237+
238+
it('does not call a fill copied when the row failed before it finished', () => {
239+
renderJobs([
240+
makeJob({
241+
kind: 'fork_sync',
242+
workspaceId: WORKSPACE_ID,
243+
status: 'failed',
244+
error: 'Background resource copy failed',
245+
metadata: syncMetadata,
246+
}),
247+
])
248+
249+
expandRow()
250+
expect(container.textContent).toContain('Copy failed')
251+
expect(container.textContent).toContain('2 tables, 1 file')
252+
expect(container.textContent).not.toContain('Copied')
253+
})
254+
255+
it('surfaces a deploy that succeeded with its cutover still pending', () => {
256+
renderJobs([
257+
makeJob({
258+
kind: 'fork_sync',
259+
workspaceId: WORKSPACE_ID,
260+
status: 'completed_with_warnings',
261+
metadata: {
262+
...syncMetadata,
263+
deployWarnings: ['Flow A — prior workflow version remains active'],
264+
},
265+
}),
266+
])
267+
268+
expandRow()
269+
expect(container.textContent).toContain('Flow A — prior workflow version remains active')
270+
})
271+
223272
it('reports the finished copy and what it lost on the same row', () => {
224273
renderJobs([
225274
makeJob({

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,31 @@ function countList(pairs: Array<[number | undefined, string]>): string {
3838
function contentFillCounts(m: NonNullable<BackgroundWorkMetadata>): string[] {
3939
const kinds: Array<[number | undefined, string]> = [
4040
[m.knowledgeBases, 'knowledge base'],
41+
[m.documents, 'document'],
4142
[m.tables, 'table'],
4243
[m.files, 'file'],
44+
[m.skills, 'skill'],
4345
]
4446
return kinds.filter(([n]) => (n ?? 0) > 0).map(([n, noun]) => plural(n as number, noun))
4547
}
4648

49+
/**
50+
* Label for a sync's background content fill, by where the row is in it. A failed row keeps
51+
* the counts it planned, so they must not read as copied: the fill was never scheduled, or
52+
* died before finishing, and the row's error says which.
53+
*/
54+
function contentFillLabel(status: BackgroundWorkItem['status']): string {
55+
switch (status) {
56+
case 'pending':
57+
case 'processing':
58+
return 'Copying'
59+
case 'failed':
60+
return 'Copy failed'
61+
default:
62+
return 'Copied'
63+
}
64+
}
65+
4766
/** A named group (one resource kind or change action) of a job's report. */
4867
interface ReportGroup {
4968
label: string
@@ -188,7 +207,7 @@ function jobReport(job: BackgroundWorkItem): JobReport {
188207
addGroup('Created', m.createdNames)
189208
addGroup('Archived', m.archivedNames)
190209
// Resources the sync copied have their content filled in the background on this same row.
191-
addGroup(job.status === 'processing' ? 'Copying' : 'Copied', contentFillCounts(m))
210+
addGroup(contentFillLabel(job.status), contentFillCounts(m))
192211
// Pre-names entries fall back to the count summary (redeployed mirrors updated).
193212
if (groups.length === 0) {
194213
const counts = countList([
@@ -216,6 +235,7 @@ function jobReport(job: BackgroundWorkItem): JobReport {
216235
if (m.deployFailed && m.deployFailed > 0) {
217236
notes.push({ value: `${plural(m.deployFailed, 'workflow')} failed to deploy`, warning: true })
218237
}
238+
for (const warning of m.deployWarnings ?? []) notes.push({ value: warning, warning: true })
219239
addContentFillWarnings()
220240
return { groups, notes }
221241
}

‎apps/sim/ee/workspace-forking/lib/create-fork.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,8 @@ export async function createFork(params: CreateForkParams): Promise<CreateForkRe
560560
tables: contentPlan.tables.length,
561561
knowledgeBases: contentPlan.knowledgeBases.length,
562562
files: blobTasks.length,
563+
skills: contentPlan.skills.length,
564+
documents: contentPlan.documents.length,
563565
workflowNames: forkedWorkflowNames,
564566
tableNames: forkedResourceNames.tables,
565567
knowledgeBaseNames: forkedResourceNames.knowledgeBases,

‎apps/sim/ee/workspace-forking/lib/promote/promote.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
159159
}))
160160

161161
import { db } from '@sim/db'
162+
import { performFullDeploy } from '@/lib/workflows/orchestration/deploy'
162163
import { getBlock } from '@/blocks/registry'
163164
import {
164165
recordBackgroundWork,
@@ -229,6 +230,39 @@ function promoteParams() {
229230
}
230231
}
231232

233+
/** One deployed source workflow the sync writes (replace mode) and then deploys. */
234+
function arrangeWrittenWorkflow() {
235+
mockComputePlan.mockResolvedValue(
236+
makePlan({
237+
items: [
238+
{
239+
sourceWorkflowId: 'wf-src',
240+
targetWorkflowId: 'wf-tgt',
241+
targetName: 'Flow',
242+
mode: 'replace' as const,
243+
sourceMeta: { name: 'Flow', description: null, folderId: null, sortOrder: 0 },
244+
},
245+
],
246+
})
247+
)
248+
mockLoadSourceDeployedStates.mockResolvedValue({
249+
deployedWorkflows: [],
250+
sourceStates: new Map([
251+
['wf-src', { blocks: {}, edges: [], loops: {}, parallels: {}, variables: {} }],
252+
]),
253+
})
254+
vi.mocked(copyWorkflowStateIntoTarget).mockResolvedValue({
255+
targetWorkflowId: 'wf-tgt',
256+
mode: 'replace',
257+
name: 'Flow',
258+
blocksCount: 0,
259+
edgesCount: 0,
260+
subflowsCount: 0,
261+
clearedDependents: [],
262+
blockIdMapping: new Map(),
263+
})
264+
}
265+
232266
/** A copy result carrying no content/id maps, for tests that only need the copy to run. */
233267
function emptyCopyResult() {
234268
return {
@@ -830,6 +864,27 @@ describe('promoteFork trigger URLs', () => {
830864
})
831865

832866
describe('promoteFork activity', () => {
867+
it('records a deploy that succeeded with a warning as a sync with warnings', async () => {
868+
arrangeWrittenWorkflow()
869+
vi.mocked(performFullDeploy).mockResolvedValueOnce({
870+
success: true,
871+
warnings: ['prior workflow version remains active'],
872+
} as never)
873+
874+
const result = await promoteFork(promoteParams())
875+
876+
expect(result.deployWarnings).toEqual(['Flow — prior workflow version remains active'])
877+
expect(recordBackgroundWork).toHaveBeenCalledWith(
878+
db,
879+
expect.objectContaining({
880+
status: 'completed_with_warnings',
881+
metadata: expect.objectContaining({
882+
deployWarnings: ['Flow — prior workflow version remains active'],
883+
}),
884+
})
885+
)
886+
})
887+
833888
it('records the sync as one terminal Activity row when nothing is left to fill', async () => {
834889
await promoteFork(promoteParams())
835890

@@ -882,6 +937,8 @@ describe('promoteFork activity', () => {
882937
tables: 1,
883938
knowledgeBases: 0,
884939
files: 0,
940+
skills: 0,
941+
documents: 0,
885942
}),
886943
})
887944
)

‎apps/sim/ee/workspace-forking/lib/promote/promote.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ export interface PromoteForkResult {
165165
* until a redeploy. Surfaced (rather than swallowed) so the caller can warn.
166166
*/
167167
deployFailed: number
168+
/**
169+
* Deploys that succeeded but left something pending - a cutover still activating, a side
170+
* effect still queued - as `<workflow> — <warning>`. The target is not yet running what the
171+
* sync wrote, so the sync's Activity row must not read as clean.
172+
*/
173+
deployWarnings: string[]
168174
unmappedRequired: Array<Pick<ForkReference, 'kind' | 'sourceId' | 'required' | 'blockName'>>
169175
/**
170176
* References the sync would have cleared in the target, so it was blocked without writing
@@ -225,6 +231,7 @@ function buildSyncActivity(
225231
const { direction, sourceWorkspaceId, targetWorkspaceId, otherWorkspaceName, actorName } = params
226232
const warnings =
227233
result.deployFailed > 0 ||
234+
result.deployWarnings.length > 0 ||
228235
result.needsConfiguration.length > 0 ||
229236
result.clearedOptional.length > 0 ||
230237
result.droppedReferences.length > 0 ||
@@ -246,6 +253,7 @@ function buildSyncActivity(
246253
archived: result.archived,
247254
redeployed: result.redeployed,
248255
deployFailed: result.deployFailed,
256+
deployWarnings: result.deployWarnings,
249257
updatedNames: result.updatedNames,
250258
createdNames: result.createdNames,
251259
archivedNames: result.archivedNames,
@@ -1059,6 +1067,7 @@ export async function promoteFork(params: PromoteForkParams): Promise<PromoteFor
10591067
archived: 0,
10601068
redeployed: 0,
10611069
deployFailed: 0,
1070+
deployWarnings: [],
10621071
unmappedRequired: txResult.blocked === 'unmapped' ? txResult.unmappedRequired : [],
10631072
blockers: txResult.blocked === 'cleared-refs' ? txResult.blockers : [],
10641073
blocked: txResult.blocked,
@@ -1117,7 +1126,7 @@ export async function promoteFork(params: PromoteForkParams): Promise<PromoteFor
11171126
// sync). Surface those instead of swallowing them into a clean success.
11181127
if (result.warnings?.length) {
11191128
const name = txResult.writtenNames[targetWorkflowId] ?? targetWorkflowId
1120-
for (const warning of result.warnings) deployWarnings.push(`${name}: ${warning}`)
1129+
for (const warning of result.warnings) deployWarnings.push(`${name} — ${warning}`)
11211130
}
11221131
} else {
11231132
deployFailures.push(targetWorkflowId)
@@ -1142,6 +1151,7 @@ export async function promoteFork(params: PromoteForkParams): Promise<PromoteFor
11421151
archived: txResult.archived,
11431152
redeployed,
11441153
deployFailed: deployFailures.length,
1154+
deployWarnings,
11451155
unmappedRequired: [],
11461156
blockers: [],
11471157
blocked: null,
@@ -1197,6 +1207,8 @@ export async function promoteFork(params: PromoteForkParams): Promise<PromoteFor
11971207
tables: contentFill.contentPlan.tables.length,
11981208
knowledgeBases: contentFill.contentPlan.knowledgeBases.length,
11991209
files: contentFill.blobTasks.length,
1210+
skills: contentFill.contentPlan.skills.length,
1211+
documents: contentFill.contentPlan.documents.length,
12001212
},
12011213
})
12021214
} else {

‎apps/sim/lib/api/contracts/workspace-fork.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,9 @@ export const backgroundWorkMetadataSchema = z
785785
tables: z.number().int().optional(),
786786
knowledgeBases: z.number().int().optional(),
787787
files: z.number().int().optional(),
788+
skills: z.number().int().optional(),
789+
/** Documents copied into an already-mapped target knowledge base (sync only). */
790+
documents: z.number().int().optional(),
788791
copied: z.number().int().optional(),
789792
failed: z.number().int().optional(),
790793
/** Count of failed resources whose dangling references were cleared post-fork (U8). */
@@ -814,6 +817,8 @@ export const backgroundWorkMetadataSchema = z
814817
archived: z.number().int().optional(),
815818
redeployed: z.number().int().optional(),
816819
deployFailed: z.number().int().optional(),
820+
/** Deploys that succeeded with a cutover or side effect still pending, as `<workflow> — <warning>`. */
821+
deployWarnings: z.array(z.string()).optional(),
817822
restored: z.number().int().optional(),
818823
unarchived: z.number().int().optional(),
819824
removed: z.number().int().optional(),

0 commit comments

Comments
 (0)