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

Jump to bottom
Merged
karreiro merged 2 commits into from
Jan 22, 2026
Merged

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

karreiro merged 2 commits into from
Jan 22, 2026

Conversation

@karreiro
Copy link
Contributor

@karreiro karreiro commented Jan 21, 2026 •
edited
Loading

WHY are these changes introduced?

The issue came from inconsistent state during the polling flow. The poll would filter previousChecksums, then make an async call to fetch latestChecksums, and filter those afterward.

If a file was saved locally while the network request was happening, its key could be added to unsyncedFileKeys mid-poll. That meant previousChecksums was filtered with one view of unsyncedFileKeys, while latestChecksums was filtered with a newer view. The comparison could then incorrectly treat the file as deleted on the remote and delete the local copy.

WHAT is this pull request doing?

This PR fixes the issue by adopting a snapshot of unsyncedFileKeys at the start of the poll and reuse it for both filters, so filtering stays consistent across the async boundary.

How to test your changes?

  • Run shopify theme init to clone the skeleton theme
  • Run shopify theme dev --theme-editor-sync
  • Save the templates/index.json file with this content 
    /*
 * ------------------------------------------------------------
 * IMPORTANT: The contents of this file are auto-generated.
 *
 * This file may be updated by the Shopify admin theme editor
 * or related systems. Please exercise caution as any changes
 * made to this file may be overwritten.
 * ------------------------------------------------------------
 */
{
  "sections": {
    "collections_WgPePL": {
      "type": "collections",
      "name": "t:general.collections_grid",
      "settings": {
        "grid_item_width": "collections--compact",
        "grid_gap": 15
      }
    },
    "main": {
      "type": "hello-world",
      "settings": {}
    }
  },
  "order": [
    "collections_WgPePL",
    "main"
  ]
}
  • Run this script locally (it might require running it from 1 to 3 times to get the issue happening) 
    #!/usr/bin/env ruby

require 'json'

FILE_PATH = 'templates/index.json'
DURATION = 20 # seconds
INTERVAL = 0.1 # 100ms
STEPS = (0..50).step(5).to_a # [0, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50]

start_time = Time.now
step_index = 0

puts "Starting grid_gap animation for #{DURATION}s..."

while (Time.now - start_time) < DURATION
  content = File.read(FILE_PATH)
  data = JSON.parse(content)

  new_value = STEPS[step_index % STEPS.length]
  data['sections']['collections_WgPePL']['settings']['grid_gap'] = new_value

  File.write(FILE_PATH, JSON.pretty_generate(data))

  puts "grid_gap: #{new_value}"

  step_index += 1
  sleep(INTERVAL)
end

puts "Done! Ran for #{(Time.now - start_time).round(2)}s with #{step_index} updates."

Post-release steps

N/A

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@karreiro karreiro requested a review from a team January 21, 2026 10:16
@karreiro karreiro requested a review from a team as a code owner January 21, 2026 10:16
@github-actions
Copy link
Contributor

github-actions bot commented Jan 21, 2026 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟡 Statements
79.52% (+0.29% 🔼)
 14351/18048
🟡 Branches
73.73% (+0.62% 🔼)
 7085/9610
🟡 Functions
79.67% (+0.29% 🔼)
 3667/4603
🟡 Lines
79.87% (+0.29% 🔼)
 13565/16984
Show new covered files 🐣
St.❔
 File Statements Branches Functions Lines
🟢
... / admin-as-app.ts
 100% 100% 100% 100%
🟢
... / metafield_definitions.ts
 100% 100% 100% 100%
🟢
... / metaobject_definitions.ts
 100% 100% 100% 100%
🟢
... / bulk-operation-cancel.ts
 100% 100% 100% 100%
🟢
... / bulk-operation-run-mutation.ts
 100% 100% 100% 100%
🟢
... / bulk-operation-run-query.ts
 100% 100% 100% 100%
🟢
... / get-bulk-operation-by-id.ts
 100% 100% 100% 100%
🟢
... / list-bulk-operations.ts
 100% 100% 100% 100%
🟢
... / staged-uploads-create.ts
 100% 100% 100% 100%
🟢
... / fetch_store_by_domain.ts
 100% 100% 100% 100%
🔴
... / import-custom-data-definitions.ts
 0% 100% 0% 0%
🔴
... / cancel.ts
 0% 100% 0% 0%
🔴
... / execute.ts
 0% 0% 0% 0%
🔴
... / status.ts
 0% 0% 0% 0%
🔴
... / pull.ts
 0% 100% 0% 0%
🟡
... / execute-operation.ts
 75% 50% 100% 75%
🔴
... / pull.ts
 0% 0% 0% 0%
🟢
... / bulk-operation-status.ts
 96.55% 92.11% 100% 100%
🟢
... / cancel-bulk-operation.ts
 100% 100% 100% 100%
🟢
... / constants.ts
 100% 100% 100% 100%
🟢
... / download-bulk-operation-results.ts
 100% 100% 100% 100%
🟢
... / execute-bulk-operation.ts
 86.76% 83.67% 100% 88.06%
🟢
... / format-bulk-operation-status.ts
 100% 100% 100% 100%
🟢
... / run-mutation.ts
 100% 100% 100% 100%
🟢
... / run-query.ts
 100% 100% 100% 100%
🟡
... / stage-file.ts
 73.53% 62.5% 85.71% 72.73%
🟢
... / watch-bulk-operation.ts
 100% 94.74% 100% 100%
🟢
... / utilities.ts
 100% 100% 100% 100%
🟢
... / declarative-definitions.ts
 98.54% 93.18% 100% 98.51%
🟢
... / common.ts
 97.62% 95% 100% 97.06%
🟢
... / execute-command-helpers.ts
 100% 100% 100% 100%
🟢
... / file-formatter.ts
 100% 100% 100% 100%
🔴
... / promiseWithResolvers.ts
 33.33% 50% 50% 33.33%
Show files with reduced coverage 🔻
St.❔
 File Statements Branches Functions Lines
🔴
... / execute.ts
 0%
0% (-100% 🔻)
 0% 0%
🟢
... / loader.ts
94.06% (+0.2% 🔼)
86.41% (-0.42% 🔻)
97.17% (+0.11% 🔼)
94.85% (+0.18% 🔼)
🟢
... / extension-instance.ts
84.8% (+0.23% 🔼)
77.6% (-0.91% 🔻)
92.06% (+0.13% 🔼)
85.11% (+0.24% 🔼)
🟡
... / specification.ts
69.64% (+0.55% 🔼)
75.61% (+2.44% 🔼)
76.47% (-1.31% 🔻)
69.39% (+0.64% 🔼)
🟢
... / ui_extension.ts
87.9% (-6.93% 🔻)
77.19% (-4.06% 🔻)
85.19% (-14.81% 🔻)
90.07% (-6.39% 🔻)
🟢
... / store-context.ts
 100%
82.35% (-0.98% 🔻)
 100% 100%
🟢
... / Logs.tsx
 90%
90.91% (-5.97% 🔻)
 100% 90%
🟢
... / fetch.ts
84.21% (+0.88% 🔼)
82.35% (-0.98% 🔻)
 75%
85.29% (+1.42% 🔼)
🟢
... / app-event-watcher-handler.ts
86.36% (-4.11% 🔻)
 75% 86.67%
85.71% (-5.19% 🔻)
🔴
... / server.ts
1.23% (-0.02% 🔻)
 0% 0%
1.3% (-0.02% 🔻)
🟢
... / bundle.ts
 93.22%
63.33% (-3.33% 🔻)
94.12% (+5.88% 🔼)
 96.3%
🟢
... / developer-platform-client.ts
84.62% (-1.5% 🔻)
71.43% (+0.84% 🔼)
81.82% (+1.82% 🔼)
93.75% (+0.42% 🔼)
🔴
... / http-reverse-proxy.ts
58.97% (-4.91% 🔻)
37.04% (-2.96% 🔻)
58.33% (-5.3% 🔻)
59.46% (-5.25% 🔻)
🔴
... / app-management-client.ts
53.69% (-0.16% 🔻)
 47.47%
50% (-0.45% 🔻)
52.53% (-0.17% 🔻)
🟢
... / api.ts
87.07% (-0.43% 🔻)
76.71% (-0.1% 🔻)
 100%
86.49% (-0.43% 🔻)
🟢
... / device-authorization.ts
88.24% (-0.83% 🔻)
76.47% (-2.94% 🔻)
 100%
88.24% (-0.83% 🔻)
🟢
... / SingleTask.tsx
84.21% (-15.79% 🔻)
50% (-50% 🔻)
80% (-20% 🔻)
84.21% (-15.79% 🔻)
🔴
... / environment.ts
35% (-5% 🔻)
 41.18%
40% (-10% 🔻)
36.84% (-5.26% 🔻)
🔴
... / ui.tsx
50.82% (-0.79% 🔻)
42.86% (-5.53% 🔻)
54.55% (+1.42% 🔼)
50% (-0.82% 🔻)
🟢
... / console.ts
81.82% (+15.15% 🔼)
75% (-25% 🔻)
100% (+33.33% 🔼)
81.82% (+15.15% 🔼)
🟢
... / init.ts
88% (-0.89% 🔻)
71.43% (+4.76% 🔼)
86.67% (+4.85% 🔼)
88% (-0.89% 🔻)
🟢
... / storefront-renderer.ts
90.2% (-0.54% 🔻)
 78.95%
81.82% (-1.52% 🔻)
90.2% (-0.54% 🔻)
🟡
... / theme-polling.ts
67.57% (-0.49% 🔻)
 68.75% 78.57%
66.18% (-1.47% 🔻)

Test suite run success

3671 tests passing in 1430 suites.

Report generated by 🧪jest coverage report action from b78c639

EvilGenius13
EvilGenius13 approved these changes Jan 21, 2026
View reviewed changes
Copy link
Contributor

@EvilGenius13 EvilGenius13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩 went well. This could also be caused by manually changing a file too many times in quick succession without AI. I can no longer replicate it either way. Thank you for the fix @karreiro!

graygilmore
graygilmore approved these changes Jan 21, 2026
View reviewed changes
Copy link
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

Small note: this isn't caused specifically by LLMs if you want to change the title.

@karreiro karreiro changed the title Fix shopify theme dev --theme-editor-sync so it doesn’t delete files when they’re updated by AI agents Fix shopify theme dev --theme-editor-sync so it doesn’t delete files when a race condition happens Jan 22, 2026
@karreiro
Copy link
Contributor Author

karreiro commented Jan 22, 2026

👋 Hey @Shopify/app-inner-loop, could you please take a look at this when you have a moment?

@karreiro karreiro added this pull request to the merge queue Jan 22, 2026
Merged via the queue into main with commit 76cb225 Jan 22, 2026
25 checks passed
@karreiro karreiro deleted the fix-theme-editor-sync-race-condition branch January 22, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graygilmore graygilmore graygilmore approved these changes

@amcaplan amcaplan amcaplan approved these changes

@EvilGenius13 EvilGenius13 EvilGenius13 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@karreiro @graygilmore @amcaplan @EvilGenius13