Extension Bisect: keep eliminated extensions disabled#316792
Open
NSExceptional wants to merge 2 commits into
Open
Extension Bisect: keep eliminated extensions disabled#316792NSExceptional wants to merge 2 commits into
NSExceptional wants to merge 2 commits into
Conversation
Previously, when bisect narrowed the search range it only kept the current upper half [mid, high) disabled and re-enabled every extension outside the active range. If more than one extension was faulty, the re-enabled extensions could continue to reproduce the problem, preventing bisect from converging and yielding the misleading "no extension identified" outcome. This change keeps extensions in [high, length) - those eliminated from the search via a "still bad" answer - disabled for the remainder of the bisect run. Known-good extensions in [0, low) (eliminated via "I can't reproduce") remain enabled because they were already verified safe. The algorithm now correctly identifies one problematic extension per run; the user can repeat bisect to find additional ones. The "extension identified" confirmation now mentions running bisect again to find any additional problematic extensions. Fixes microsoft#237092
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Extension Bisect service to keep previously eliminated extensions disabled so bisect can still identify a bad extension even when multiple extensions are faulty (fix for #237092).
Changes:
- Change bisect disablement to keep all extensions with index
>= middisabled (including those eliminated by “still bad”). - Update the disabled-count shown to users to reflect the new disablement rule.
- Extend the “extension identified” confirmation message to suggest re-running bisect to find additional problematic extensions.
Covers start, "I can't reproduce", "still bad", reset, identification of a single bad extension, and the "all extensions disabled, still bad" no-extension-identified path. Asserts that disabledCount stays in sync with the actual disabled set across each step, guarding against regressions of issue microsoft#237092 (eliminated extensions must remain disabled when the search narrows via "still bad"). Exports ExtensionBisectService so it can be instantiated directly in tests. The DI singleton registration is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #237092 — Extension Bisect now keeps previously eliminated extensions disabled so that it can identify a problematic extension even when more than one extension is faulty.
Before: When bisect narrowed the search via a "still bad" answer, extensions outside the new search range were re-enabled. If any of those extensions were also bad, the user would keep answering "still bad" all the way to the end and bisect would conclude with the misleading "no extension has been identified" message.
After: Extensions in
[high, length)— those eliminated by a "still bad" answer — stay disabled for the remainder of the bisect run. Extensions in[0, low)(eliminated by "I can't reproduce") were already verified safe collectively, so they remain enabled as before. Bisect now identifies one problematic extension per run; the user repeats bisect to find additional ones, as suggested in the issue's design section.The "extension identified" confirmation also now mentions that the user may want to re-run bisect to find any additional problematic extensions.
Algorithm change
The bisect state
(low, mid, high)partitions extensions into four regions:[0, low)[low, mid)[mid, high)[high, length)This simplifies to
isDisabled = i >= midanddisabledCount = extensions.length - mid.Test plan
Authored this with AI, hope that's okay :)