feat: support file as base reference in YAML recipes

[fkondej]

No description provided.

[claude]

Claude finished @fkondej's task in 1m 35s —— View job

---

PR Review: feat: support file as base reference in YAML recipes

• Read PR diff and changed files
• Review code quality, bugs, edge cases
• Post review feedback

Summary

Good feature with solid test coverage (circular deps, multi-level chains, not-found). The core design of resolving file-based bases relative to the recipe directory and using a visited map for cycle detection is sound. A few issues to address:

Issues found (see inline comments):

1. Cycle detection bypass via symlinks/aliased paths (recipe_yaml.go:153) — filepath.Abs doesn't resolve symlinks or normalize redundant path segments, so cycles through different paths to the same file would not be caught. Use filepath.EvalSymlinks instead.

2. No recursion depth limit (recipe_yaml.go:188) — Non-circular but deeply nested chains have no bound. A simple depth limit would prevent stack issues and catch misconfigurations.

3. Validation assumes parsing invariant (cmd_validate.go:83-86) — validateYAMLRecipe unconditionally trusts that file-based bases are valid because "parsing would have failed." This couples validation to the parse call order. Checking the file exists explicitly would be more robust.

4. isYAMLBasePath heuristic (recipe_yaml.go:224-227) — Minor: any future built-in recipe name ending in .yaml/.yml would be misclassified as a file path.

[claude]

Claude finished @fkondej's task in 56s —— View job

---

Re-review after updates

• Read updated PR diff and changed files
• Check if previous review issues were addressed
• Post review feedback

All four issues from the previous review have been addressed:

1. Cycle detection — Now uses filepath.EvalSymlinks with Abs fallback. ✓
2. Recursion depth limit — Added maxBaseRecipeDepth = 10 constant and check. ✓
3. Validation independence — validateYAMLRecipe now checks file existence via os.Stat instead of relying on parse order. ✓
4. isYAMLBasePath heuristic — Acknowledged (unchanged, acceptable tradeoff).

One minor issue remains: off-by-one in the depth check — see inline comment.

[claude]

Claude finished @fkondej's task in 45s —— View job

---

Re-review after latest push

• Read updated diff and changed files
• Check code quality and correctness
• Post review feedback

Review

All four issues from previous reviews have been addressed properly:

1. Cycle detection — Uses filepath.EvalSymlinks with Abs fallback ✓
2. Depth limit — maxBaseRecipeDepth = 10 with >= check (off-by-one fixed) ✓
3. Validation independence — validateYAMLRecipe checks file existence via os.Stat ✓
4. isYAMLBasePath heuristic — Acknowledged as acceptable tradeoff ✓

Code looks clean. Test coverage is thorough (circular deps, multi-level chains, not-found, path detection). No new issues found — LGTM.