diff --git a/.changeset/pink-moles-leave.md b/.changeset/pink-moles-leave.md new file mode 100644 index 0000000000..c6aabb802b --- /dev/null +++ b/.changeset/pink-moles-leave.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix `shopify theme dev --theme-editor-sync` to avoid deleting files during race conditions, especially when multiple changes come from an external process (e.g., AI coding tools) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-polling.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-polling.test.ts index 02b893d241..4576fba4a9 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-polling.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-polling.test.ts @@ -266,6 +266,33 @@ describe('pollRemoteJsonChanges', async () => { expect(fetchThemeAssets).toHaveBeenCalledWith(1, ['templates/asset1.json', 'templates/asset3.json'], adminSession) expect(fetchThemeAssets).not.toHaveBeenCalledWith(1, ['templates/asset2.json'], adminSession) }) + + test('does not delete file when it becomes unsynced during fetchChecksums call (race condition)', async () => { + // Given + const files = new Map([ + ['templates/index.json', {checksum: '1', key: 'templates/index.json', value: '{}'}], + ]) + const themeFileSystem = { + ...fakeThemeFileSystem('tmp', files), + unsyncedFileKeys: new Set(), + } + const deleteSpy = vi.spyOn(themeFileSystem, 'delete') + const remoteChecksums = [{checksum: '1', key: 'templates/index.json'}] + const updatedRemoteChecksums = [{checksum: '1', key: 'templates/index.json'}] + + // When + // Simulate the race condition: file becomes unsynced during fetchChecksums + vi.mocked(fetchChecksums).mockImplementation(async () => { + themeFileSystem.unsyncedFileKeys.add('templates/index.json') + return updatedRemoteChecksums + }) + + await pollRemoteJsonChanges(developmentTheme, adminSession, remoteChecksums, themeFileSystem, defaultOptions) + + // Then + expect(deleteSpy).not.toHaveBeenCalled() + expect(themeFileSystem.files.get('templates/index.json')).toBeDefined() + }) }) describe('deleteRemovedAssets', () => { diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts b/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts index 342e0f2a9b..c8de412668 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts @@ -80,9 +80,21 @@ export async function pollRemoteJsonChanges( localFileSystem: ThemeFileSystem, options: PollingOptions, ): Promise { - const previousChecksums = applyFileFilters(remoteChecksums, localFileSystem) + /* + * Capture the current set of unsynced file keys to ensure + * consistent filtering between previousChecksums and + * latestChecksums. + * + * This prevents a race condition where a file saved locally + * during fetchChecksums() would be filtered out of + * latestChecksums but not previousChecksums, causing the file + * to incorrectly be deleted. + */ + const currentUnsyncedKeys = new Set(localFileSystem.unsyncedFileKeys) + + const previousChecksums = applyFileFilters(remoteChecksums, localFileSystem, currentUnsyncedKeys) const latestChecksums = await fetchChecksums(targetTheme.id, currentSession).then((checksums) => - applyFileFilters(checksums, localFileSystem), + applyFileFilters(checksums, localFileSystem, currentUnsyncedKeys), ) const changedAssets = getAssetsChangedOnRemote(previousChecksums, latestChecksums) @@ -189,9 +201,7 @@ async function abortIfMultipleSourcesChange(localFileSystem: ThemeFileSystem, as } } -function applyFileFilters(files: Checksum[], localThemeFileSystem: ThemeFileSystem) { +function applyFileFilters(files: Checksum[], localThemeFileSystem: ThemeFileSystem, unsyncedKeys: Set) { const filteredFiles = localThemeFileSystem.applyIgnoreFilters(files) - return filteredFiles - .filter((file) => file.key.endsWith('.json')) - .filter((file) => !localThemeFileSystem.unsyncedFileKeys.has(file.key)) + return filteredFiles.filter((file) => file.key.endsWith('.json')).filter((file) => !unsyncedKeys.has(file.key)) }