Skip to content

fix(workflows): match overlay file extensions case-insensitively - #4531

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/overlay-suffix-case-insensitive
Open

fix(workflows): match overlay file extensions case-insensitively#4531
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/overlay-suffix-case-insensitive

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

ProjectOverlaySource.collect matches the extension verbatim:

if not path.is_file() or path.suffix not in (".yml", ".yaml"):
    continue

So a hand-placed overlay named <id>.YML or <id>.Yaml is skipped and never applied — and nothing is reported to say the file was ignored. Overlay files are explicitly hand-authored (docs/reference/workflows.md documents the format and tells users to write them), so the extension casing is the author's choice, not something the tool generated.

Reproduction on current main (c173bf1)

Three overlays in one directory, differing only in extension case:

files on disk: ['Mixed.Yaml', 'UPPER.YML', 'lower.yml']
COLLECTED    : ['lower']
SKIPPED      : ['mixed', 'upper']

The author gets no error and no warning — the overlay simply has no effect.

Why this is the odd one out

Every other YAML discovery path in the package already lowercases before matching:

location form
workflows/engine.py:941 path.suffix.lower() in (".yml", ".yaml")
workflows/_commands.py:1325 source_path.suffix.lower() in (...)
workflows/_commands.py:1894 dev_path.suffix.lower() in (...)
workflows/_commands.py:2128 source_path.suffix.lower() in (...)
overlays/layer_sources.py:150 path.suffix in (...)this PR

Fix

if not path.is_file() or path.suffix.lower() not in (".yml", ".yaml"):

Scope note

overlays/_commands.py:115 (_find_overlay_file) carries the identical case-sensitive pattern. I have deliberately not touched it here: an open PR of mine (#4141) already modifies that file, and combining the two would create a needless conflict. Happy to follow up with it once #4141 lands, or to fold it in here if you would rather see both together.

Verification

  • Fail-before / pass-after: 4 new-vs-baseline failures with the source reverted to upstream/main → passing with the fix.
  • Parametrized over .YML, .Yaml, .YAML, .Yml.
  • A companion test asserts broadening the case did not broaden which extensions are accepted — notes.txt, backup.yml.bak, README.md and data.json are still skipped.
  • Scoped regression over tests/workflows: no new failures vs a clean-main self-baseline (10 pre-existing, Windows symlink-privilege).
  • uvx ruff@0.15.0 check src tests → clean

Behaviour change, disclosed: files previously skipped now get read. If a project already holds a .YML/.YAML file in an overlay directory that is not a valid overlay manifest, resolution will now report it rather than ignoring it — which is the intended outcome, but it is a change for such a project.


Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current main.

🤖 Generated with Claude Code

`ProjectOverlaySource.collect` matched `path.suffix` verbatim:

    if not path.is_file() or path.suffix not in (".yml", ".yaml"):
        continue

so a hand-placed overlay named `<id>.YML` or `<id>.Yaml` was skipped and never
applied, with nothing reported to say the file had been ignored. Overlay files
are explicitly hand-authored (docs/reference/workflows.md documents the format
and tells users to write them), so the casing is the author's choice.

Reproduced on main -- three overlays in one directory, differing only in
extension case:

    files on disk: ['Mixed.Yaml', 'UPPER.YML', 'lower.yml']
    COLLECTED    : ['lower']
    SKIPPED      : ['mixed', 'upper']

Every other YAML discovery path in the package already lowercases before
matching: engine.py:941, _commands.py:1325, :1894, :2128. This brings the
overlay loader in line with them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jawwad-ali
jawwad-ali requested a review from mnriem as a code owner September 11, 2026 16:02
@mnriem mnriem added the triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants