Skip to content

Fix flaky code patch acceptance test caused by Monaco diff race#4225

Merged
habdelra merged 2 commits intomainfrom
fix-flaky-diff-editor-test
Mar 21, 2026
Merged

Fix flaky code patch acceptance test caused by Monaco diff race#4225
habdelra merged 2 commits intomainfrom
fix-flaky-diff-editor-test

Conversation

@habdelra
Copy link
Contributor

Summary

  • During test teardown, Monaco's WorkerBasedDocumentDiffProvider.computeDiff can receive a null result from the editor worker when models are disposed mid-computation. This threw Error: no diff result available which surfaced as a flaky QUnit "global failure" in CI on the "Accept All" code patch acceptance test.
  • Extended the existing Monaco patch (patches/monaco-editor@0.52.2.patch) to return an empty diff result instead of throwing — matching the pattern Monaco already uses for the disposed-models case a few lines above in the same method.

Test plan

  • Ran the specific flaky test (failure patching code when using "Accept All" button) — passes
  • Ran the full Code patches tests acceptance suite (17 tests) — all pass (15 pass, 2 skipped, 0 failures)

🤖 Generated with Claude Code

During test teardown, Monaco's WorkerBasedDocumentDiffProvider.computeDiff
can receive a null result from the editor worker when models are disposed
mid-computation. This caused an unhandled "no diff result available" error
that surfaced as a flaky QUnit global failure in CI.

Extend the existing Monaco patch to return an empty diff result instead of
throwing — matching the pattern Monaco already uses for disposed models.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a flaky Monaco-driven acceptance test by hardening Monaco’s worker-based diff computation against a null diff result during teardown/model disposal, avoiding an exception that bubbles up as a QUnit global failure.

Changes:

  • Extend the existing monaco-editor@0.52.2 patch to return an empty diff result when the worker returns null instead of throwing.
  • Update pnpm-lock.yaml patched dependency hash entries to reflect the modified Monaco patch.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
patches/monaco-editor@0.52.2.patch Changes Monaco’s WorkerBasedDocumentDiffProvider.computeDiff behavior on null results to return an empty diff instead of throwing.
pnpm-lock.yaml Updates the patch_hash/patched dependency hash for monaco-editor@0.52.2 to match the new patch content.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@habdelra habdelra requested a review from a team March 20, 2026 18:36
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fed54512df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address review feedback: instead of unconditionally returning an empty
diff when the worker returns null, only suppress the error when the
models are confirmed disposed (the teardown race). Genuine worker
failures with live models still throw.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

Host Test Results

    1 files  ±0      1 suites  ±0   2h 18m 53s ⏱️ - 3m 18s
2 030 tests ±0  2 015 ✅ ±0  15 💤 ±0  0 ❌ ±0 
2 045 runs  ±0  2 030 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 6ff8d36. ± Comparison against base commit 505f18d.

@habdelra habdelra merged commit e718677 into main Mar 21, 2026
116 of 117 checks passed
habdelra added a commit that referenced this pull request Mar 23, 2026
* Fix flaky "Accept All" code patch test caused by Monaco diff editor race

During test teardown, Monaco's WorkerBasedDocumentDiffProvider.computeDiff
can receive a null result from the editor worker when models are disposed
mid-computation. This caused an unhandled "no diff result available" error
that surfaced as a flaky QUnit global failure in CI.

Extend the existing Monaco patch to return an empty diff result instead of
throwing — matching the pattern Monaco already uses for disposed models.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Narrow Monaco patch to only suppress error when models are disposed

Address review feedback: instead of unconditionally returning an empty
diff when the worker returns null, only suppress the error when the
models are confirmed disposed (the teardown race). Genuine worker
failures with live models still throw.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants