Skip to content

Fix shopify theme dev --theme-editor-sync so it doesn’t delete files when a race condition happens #6784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karreiro merged 2 commits into from
Jan 22, 2026
+48 −6
Merged

Fix shopify theme dev --theme-editor-sync so it doesn’t delete files when a race condition happens #6784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d935cfb
Fix `shopify theme dev --theme-editor-sync` so it doesn’t delete file…
karreiro Jan 21, 2026
b78c639
Update commit message
karreiro Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pink-moles-leave.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
27 changes: 27 additions & 0 deletions packages/theme/src/cli/utilities/theme-environment/theme-polling.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, ThemeAsset>([
['templates/index.json', {checksum: '1', key: 'templates/index.json', value: '{}'}],
])
const themeFileSystem = {
...fakeThemeFileSystem('tmp', files),
unsyncedFileKeys: new Set<string>(),
}
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', () => {
Expand Down
22 changes: 16 additions & 6 deletions packages/theme/src/cli/utilities/theme-environment/theme-polling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,21 @@ export async function pollRemoteJsonChanges(
localFileSystem: ThemeFileSystem,
options: PollingOptions,
): Promise<Checksum[]> {
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)
Expand Down Expand Up @@ -189,9 +201,7 @@ async function abortIfMultipleSourcesChange(localFileSystem: ThemeFileSystem, as
}
}

function applyFileFilters(files: Checksum[], localThemeFileSystem: ThemeFileSystem) {
function applyFileFilters(files: Checksum[], localThemeFileSystem: ThemeFileSystem, unsyncedKeys: Set<string>) {
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))
}
Loading