Skip to content

fix(presets): resolve() honors manifest-declared file: for installed presets#3351

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/preset-resolver-manifest-file
Open

fix(presets): resolve() honors manifest-declared file: for installed presets#3351
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/preset-resolver-manifest-file

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Description

PresetResolver.resolve()'s tier-2 (installed presets) loop is convention-only — it checks <pack>/templates/<name>.md and <pack>/<name>.md, ignoring a preset manifest that declares the template with an explicit file: path:

for subdir in subdirs:
    candidate = pack_dir / subdir / f"{template_name}{ext}"
    if candidate.exists():
        return candidate

So when a preset's preset.yml declares e.g. {type: template, name: spec-template, file: custom/spec.md}, resolve() returns the core template and resolve_with_source() misattributes source='core' — while collect_all_layers() and resolve_content() in the same class correctly honor the declared file. It can also return a stray templates/spec-template.md the manifest deliberately points away from.

Reproduced on main @ bba473c: with a pack declaring file: custom/spec.md, resolve('spec-template') returned the core path, not the pack's.

Fix

Mirror collect_all_layers()'s manifest-first logic in the tier-2 loop: read the manifest, and if it lists the template with a file:, resolve to pack_dir/file when it exists (and skip the convention fallback when it's missing, to avoid masking typos — exactly as collect_all_layers does); only fall back to the convention subdir walk when the manifest is absent or doesn't list the template. resolve_with_source() is fixed for free since it delegates to resolve().

Testing

Two new tests in TestPresetResolver (both fail-before / pass-after, source-stash verified):

  • test_resolve_uses_manifest_declared_file_path: a pack declaring file: custom/spec.mdresolve() returns that file, resolve_with_source()['source'] names the pack, and it equals collect_all_layers()[0]['path'];
  • test_resolve_manifest_file_wins_over_undeclared_convention_file: an undeclared templates/spec-template.md in the pack does not shadow the manifest's file:.

Full tests/test_presets.py: 328 passed (no regressions — convention-following presets unaffected). uvx ruff check clean.

AI Disclosure

  • I did use AI assistance (describe below)

Found and fixed with Claude Code (Claude Fable 5) under my direction. AI identified the tier-2 convention-only divergence from collect_all_layers; I reproduced the wrong resolution, mirrored the authoritative logic, verified fail-before/pass-after and the full suite, and reviewed the diff.

…presets

PresetResolver.resolve()'s tier-2 (installed presets) loop was
convention-only: it looked for templates/<name>.md and <name>.md,
ignoring a preset manifest that declares the template with an explicit,
non-convention file: path. So resolve() returned the core template (and
resolve_with_source() misattributed source='core') while
collect_all_layers()/resolve_content() correctly used the preset's
declared file — a divergence inside the same class. It could also return
a stray convention-path file the manifest deliberately points away from.
Mirror collect_all_layers()'s manifest-first logic: use the declared
file: when present (skip convention fallback if it's missing, to avoid
masking typos), and fall back to the convention walk only when the
manifest is absent or doesn't list the template.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jawwad-ali jawwad-ali requested a review from mnriem as a code owner July 5, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant