fix: isolate per-extension failures so one bad extension can't drop the rest#2951
Open
PascalThuet wants to merge 1 commit into
Open
fix: isolate per-extension failures so one bad extension can't drop the rest#2951PascalThuet wants to merge 1 commit into
PascalThuet wants to merge 1 commit into
Conversation
…r_agent The per-extension loop had no error isolation: if registering one enabled extension raised (e.g. an OSError writing a command file), the loop aborted and the exception propagated, so every subsequent enabled extension was silently skipped. Callers wrap the whole call in a single best-effort try/except, so the wholesale abort surfaced as one warning while the command still exited 0 — leaving the agent with only a prefix of its extensions. Wrap the per-extension body in try/except: warn (naming the extension) and continue, so one bad extension can no longer drop the others. Add a regression test that forces the first-iterated extension to raise and asserts the rest still register. Closes github#2950
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
1 participant
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.
What
ExtensionManager.register_enabled_extensions_for_agentiterates over enabled extensions with no per-extension error isolation. If registering one extension raised (e.g. anOSErrorwriting a command file), the loop aborted and the exception propagated, so every subsequent enabled extension was silently skipped. The callers wrap the whole call in a single best-efforttry/except, so the wholesale abort surfaced as one warning while the command still exited 0 — leaving the agent with only a prefix of its extensions registered.This path is reached by
integration switchand (since #2949 / #2886)integration installandintegration upgrade.Fix
Wrap the per-extension loop body in
try/except: on failure, warn (naming the failing extension) andcontinueto the rest. The caller-level best-effort catch remains as a backstop.Note the loop already guards the non-raising degraded cases — a manifest that fails to load is skipped (
get_extension(...) is None → continue) and an empty registration result clears stale state — so this only adds isolation for genuine exceptions mid-loop.Test
test_one_failing_extension_does_not_abort_the_restinstalls two enabled extensions, forces the first-iterated one's registration to raise, and asserts the later extension still registers and the call does not propagate. Confirmed it fails without the fix.Full suite: 3726 passed, 45 skipped locally.
Closes #2950