fix(extensions): guard the required manifest sections so one bad extension cannot break extension list - #3797
Open
jawwad-ali wants to merge 1 commit into
Open
Conversation
…nsion cannot break `extension list`
ExtensionManifest.REQUIRED_FIELDS only checks key PRESENCE, so a section that is
written but left empty (`provides:` -> None) or given the wrong shape
(`provides: []`) passes it and then fails on first use:
extension: null -> TypeError: argument of type 'NoneType' is not iterable
requires: null -> TypeError: argument of type 'NoneType' is not iterable
provides: null -> AttributeError: 'NoneType' object has no attribute 'get'
provides: [] -> AttributeError: 'list' object has no attribute 'get'
Neither is a ValidationError, so both escape the callers that already handle
malformed manifests. list_installed() catches ValidationError only and has a
deliberate "Corrupted extension" fallback, so a single bad extension took down
the whole command -- reproduced end-to-end:
before: specify extension list -> exit 1, raw AttributeError, no output
after: specify extension list -> exit 0, the good extension listed, the
bad one shown as "Corrupted extension"
Add an isinstance guard for each required section, mirroring the nested guards
already in this function ("Invalid provides.commands: expected a list", "Invalid
hooks: expected a mapping") and _load_yaml's document-root check. Only the three
REQUIRED sections lacked one.
`provides: {}` is unaffected: it is a well-shaped mapping, so an extension that
provides only hooks still validates, and with no hooks it keeps the pre-existing
"must provide at least one command or hook" message. Both are locked by tests.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jawwad-ali
force-pushed
the
fix/extension-manifest-section-guards
branch
from
July 28, 2026 14:57
85b1b67 to
e2cf3ee
Compare
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.
Problem
ExtensionManifest.REQUIRED_FIELDSonly checks key presence:So a section that is written but left empty (
provides:→None, which is just how YAML spells an empty field) or given the wrong shape passes that check and then fails on first use:Neither is a
ValidationError, so both escape the callers that already handle malformed manifests.list_installed()catchesValidationErroronly and has a deliberate⚠️ Corrupted extensionfallback — so a single bad extension took down the entire command. Reproduced end-to-end with one broken and one good extension installed:specify extension listAttributeError, no output at allCorrupted extensionFix
Add an
isinstanceguard for each required section. This mirrors the guards already in the same function for the nested optional keys —— and
_load_yaml's document-root check. Only the three required sections lacked one; they were the odd ones out.Not a behaviour change for valid input
provides: {}is a well-shaped mapping, so the new check does not touch it:must provide at least one command or hookBoth are locked by regression tests. I got this wrong on the first attempt — my initial test asserted
provides: {}must raise, which failed because the fixture defines hooks; the tests now assert the correct behaviour.Verification
test_required_section_not_mapping_rejected— 3 sections × 3 bad shapes (None,[],"text"): 9 fail before, all pass after.provides: {}cases above.TestExtensionManifest: 55 passed.uvx ruff@0.15.0 check src testsclean.AI-assisted: authored with Claude Code. I reproduced the
extension listexit-1 crash end-to-end (correct.registrylayout, one broken + one good extension) and confirmed the graceful fallback now engages, rather than only testing the manifest in isolation.