fix(presets): return None for an unreadable layer in resolve_content - #3959
Conversation
PresetResolver.resolve_content() reads the winning layer (and each composition layer) with a bare read_text(), so a layer file that cannot be read or decoded crashed command registration with a raw OSError/UnicodeDecodeError. The docstring already promises 'Composed content string, or None if not found', and since github#3896 collect_all_layers() deliberately tolerates a non-UTF-8 legacy layer — moving the crash here, where both callers (_register_commands and _reconcile_composed_commands) are unguarded. Return None when the winning or base layer cannot be read, treating an unreadable layer like a missing one per the documented contract. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates preset content resolution to tolerate unreadable or invalidly encoded layers.
Changes:
- Returns
Nonewhen base, winning, or composition layers cannot be read. - Adds regression coverage for an undecodable winning layer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/specify_cli/presets/__init__.py |
Handles layer read and decoding failures. |
tests/test_presets.py |
Tests an undecodable winning layer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Review follow-up: add an unreadable replace base beneath a valid composing layer, and a mocked-PermissionError composing layer over a valid base, so every new boundary and both exception types are covered. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The Posted on behalf of @marcelsafin by GitHub Copilot (model: claude-fable-5). |
|
Thank you! |
Description
PresetResolver.resolve_content()reads the winning layer (and each composition layer) with a bareread_text(), so a layer file that cannot be read or decoded crashes command registration with a rawOSError/UnicodeDecodeError. The docstring already promises "Composed content string, or None if not found" — and since #3896,collect_all_layers()deliberately keeps a non-UTF-8 legacy layer (with itsreplacedefault) so unrelated commands still resolve, which moves the crash here, where both callers (_register_commandsand_reconcile_composed_commands) are unguarded.Fix: return
Nonewhen the winning/base layer or a composition layer cannot be read, treating an unreadable layer like a missing one per the documented contract._read_layer_contentbecomesOptional[str]accordingly.Testing
uv run specify --helpuv sync && uv run pytest(6,310 passed, 176 skipped)test_resolve_content_unreadable_winning_layer_returns_none(fails on main, passes with fix)ruff check src testscleanAI Disclosure
Implemented autonomously by GitHub Copilot CLI (model: Claude Fable 5) under human direction; TDD (failing test first), full suite and lint verified locally. Commit includes
Assisted-by/Co-authored-bytrailers.