[menil]

🧠 AI Decision Record: View Architecture & Reasoning on GitHub

Overview

Isolates sync-agent-ignore.sh to the root project-template repository as maintainer tooling, ensuring child repositories generated via Copier do not inherit the script or its task runner recipes.

Changes

• Template Isolation: Deleted template/scripts/sync-agent-ignore.sh and converted scripts/sync-agent-ignore.sh from a symlink into an executable regular file (100755) at the repository root.
• Child Project Simplification:
  • Removed sync-agent-ignore and check-agent-ignore-sync recipes from template/Justfile.
  • Simplified template/Justfile: format now runs without prerequisites, and validate: lint.
• Script Hardening & Resilience:
  • Added CLI argument validation (case "${1:-}" in) supporting --check, -h|--help, and rejecting unexpected flags.
  • Added guard for empty deny arrays to prevent Bash 3.2 unbound variable crashes under set -u and generate valid empty JSON ("deny": []).
  • Added regex line matching (^[[:space:]]*(#|$)) and whitespace trimming to safely ignore indented comments and blank lines.
  • Protected against CDPATH lookups with CDPATH='' cd -- "$(dirname "${BASH_SOURCE[0]}")/..".
• Documentation:
  • Updated template/README.md.jinja to clarify that .claude/settings.json is shipped pre-configured, and that downstream projects adding custom ignore rules should update .claude/settings.json directly.
• Verification & Test Assertions:
  • In root Justfile (test-template), added assertions that scripts/sync-agent-ignore.sh is an executable regular file, test -f "$TMPDIR/rendered-default/.claude/settings.json" is preserved, and scripts/ is not generated in rendered outputs.

[github-actions]

Code Review Summary

Needs Attention (1 issue)

1. [Quality] README documentation inconsistency - template/README.md.jinja:63
   The change from "generated .claude/settings.json" to "pre-configured .claude/settings.json" is misleading since the file is still generated from .agentignore patterns. Consider rewording to "pre-generated" or "automatically generated" to avoid confusion about whether manual updates to .agentignore are reflected.

Suggestions (2 items)

1. [Style] Complex test condition on single line - Justfile:43
   The test command chains three conditions without grouping. While it works correctly, consider breaking it into separate test statements or adding a comment explaining why these three conditions are logically combined (all must be true). This would improve readability.

2. [Quality] Script error handling improvement - scripts/sync-agent-ignore.sh:40-41
   The handling of empty deny array (generating empty deny list instead of potential failure) is a good defensive improvement over the original template version. Consider documenting this edge case behavior in comments or ensure .agentignore always has at least one pattern in CI/documentation.

All Clear

• Pass 1: Correctness & Logic - No bugs or logical flaws detected
• Pass 2: Static Analysis & Types - Bash syntax and variable handling correct
• Pass 4: Security - No credentials, injection risks, or security issues
• Pass 6: Test Quality - New assertions are concrete and behavior-focused
• Pass 7: Performance - No performance concerns
• Pass 8: Dependencies & Breaking Changes - jq removal from template appropriate; proper compatibility maintained
• Pass 10: PII Detection - No sensitive information present

Verdict: Ready to Merge

The PR successfully migrates the sync-agent-ignore script from template to root with improved error handling and clearer architecture. The minor documentation wording could be improved but doesn't block merging.

[menil]

Addressed review feedback in commit 49e5e7b: updated README wording to 'pre-generated', split Justfile test assertions with an explanatory comment, and documented the empty deny list edge case in scripts/sync-agent-ignore.sh.