Skip to content

Conversation

@rh-rahulshetty
Copy link
Contributor

Description

The PrecommitHooksFixer was using an incorrect template path (precommit-{lang}.yaml.j2) that doesn't match the actual template structure, causing TemplateNotFound errors when running the align command from different directory contexts.

When running agentready align . on a repository without pre-commit hooks, the following template error was observed:

  File "/home/rashetty/Desktop/agentready/agentready/.venv/lib64/python3.13/site-packages/jinja2/loaders.py", line 380, in get_source
    raise TemplateNotFound(template)
jinja2.exceptions.TemplateNotFound: precommit-python.yaml.j2

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test coverage improvement

Related Issues

Fixes #
Relates to #

Changes Made

  • src/agentready/fixers/testing.py: Fixed template path from precommit-{lang}.yaml.j2 to {lang}/precommit.yaml.j2 to match actual template structure

Testing

  • Unit tests pass (pytest)
  • Integration tests pass
  • Manual testing performed
  • No new warnings or errors

Checklist

  • My code follows the project's code style
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link
Contributor

AgentReady Code Review - PR #269

Overview

PR: fix: Correct pre-commit template path in PrecommitHooksFixer
Type: Bug Fix
Complexity: Low (Single file, 3 line changes)
AgentReady Score Impact: Affects precommit_hooks attribute (Tier 2, 3% weight)


Executive Summary

APPROVED - This is a well-targeted bug fix that corrects a template path mismatch causing TemplateNotFound errors when running agentready align.

Key Findings:

  • ✅ Correctly fixes template path from precommit-{lang}.yaml.j2 to {lang}/precommit.yaml.j2
  • ✅ Matches actual directory structure in src/agentready/templates/bootstrap/
  • ⚠️ Missing unit tests for PrecommitHooksFixer
  • ⚠️ Broad exception handling could mask other issues
  • ℹ️ Impacts AgentReady's ability to auto-fix repositories missing pre-commit hooks

AgentReady Attribute Analysis

Affected Attributes

Attribute Status Impact
precommit_hooks ✅ IMPROVED This fix enables the fixer to work correctly
test_coverage ⚠️ NEEDS IMPROVEMENT No tests for PrecommitHooksFixer (line 128 in test_fixer_service.py only counts it)
type_annotations ✅ PASS All type hints present
inline_documentation ✅ PASS Good docstrings and comments added

Score Impact Calculation

Current State: When agentready align is run on a repo without pre-commit hooks:

  • Before this fix: TemplateNotFound error → No fix applied → 0 points gained
  • After this fix: Template loads correctly → Fix applied → ~3.0 points gained (Tier 2, 3% weight)

Projected Repository Score Improvement: +3.0 points (for repos missing pre-commit hooks)


Code Quality Assessment

✅ Strengths

  1. Root Cause Identified: The fix correctly identifies the mismatch between expected path format and actual directory structure
  2. Consistent Fallback: Both the try block and except block now use the same path format
  3. Verified Template Existence: All supported languages have templates at the correct paths (python, javascript, go, _base)
  4. Type Safety: All type annotations present (testing.py:46-55)
  5. Documentation: Added helpful comment explaining fallback behavior (line 52)

⚠️ Issues & Recommendations

1. CRITICAL: Missing Unit Tests (Tier 1 Impact)

Issue: No tests exist for PrecommitHooksFixer, only a count assertion in test_fixer_service.py:128-129

Priority: HIGH - Test coverage is a Tier 1 attribute (10% weight)

Recommended tests:

  • Test template path resolution for Python, JavaScript, Go
  • Test fallback behavior for unsupported languages
  • Test fix application (dry-run and real)
  • Verify language-specific hook content

2. MEDIUM: Overly Broad Exception Handling (Security & Debugging Concern)

Issue: except Exception: catches all exceptions, masking potential issues (testing.py:51)

Recommendation: Replace with specific TemplateNotFound exception from Jinja2

Benefits:

  • Prevents masking of unexpected errors (e.g., filesystem permission issues)
  • Improves debuggability
  • Adds logging for observability
  • Aligns with AgentReady's structured_logging attribute

Priority: MEDIUM

3. LOW: Missing Edge Case - What if Python template also missing?

Recommendation: Add defensive fallback to _base template

Priority: LOW (Python template exists in the codebase, but defensive programming is good practice)


Security Analysis

✅ No Security Vulnerabilities Detected

Category Status Notes
Command Injection ✅ SAFE No user input in shell commands
Path Traversal ✅ SAFE Template paths are constructed from language detection, not user input
Template Injection ✅ SAFE Jinja2 templates are static and bundled with the package

Best Practices Compliance

✅ Follows AgentReady Guidelines

  • Conventional Commits: Commit message follows fix: <description> format
  • Type Annotations: All functions have proper type hints
  • Inline Documentation: Docstrings present and descriptive
  • Separation of Concerns: Fix is localized to the fixer class
  • Minimal Change Scope: Only modifies what's necessary

⚠️ Needs Improvement


Recommendations Summary

Required Before Merge

  1. Add unit tests for PrecommitHooksFixer (HIGH priority)

Recommended Improvements

  1. Replace broad exception with specific TemplateNotFound catch (MEDIUM priority)
  2. Add defensive fallback to _base template (LOW priority)
  3. Add structured logging for template fallback events (LOW priority)

Conclusion

VERDICT: ✅ APPROVE WITH RECOMMENDATIONS

This is a solid bug fix that correctly addresses the template path mismatch. The code change is minimal, targeted, and safe. However, the lack of unit tests for PrecommitHooksFixer is a gap that should be addressed.

Recommended Merge Path:

  1. Option A (Ideal): Add unit tests, then merge
  2. Option B (Acceptable): Merge now, create follow-up issue for tests
  3. Option C (Not Recommended): Merge without tests

AgentReady Certification Impact:

  • Enables automated fixing of precommit_hooks attribute for target repositories
  • Supports AgentReady's mission to improve repository AI-readiness
  • Aligns with Tier 2 Critical attributes (pre-commit hooks prevent bad commits)

Review conducted by: AgentReady Code Review Analysis
Date: 2026-01-29
Reviewers: @claude (sonnet-4-5)

@jeremyeder
Copy link
Contributor

this is good to go - would you mind looking at why the CI tests are failing? i dont think its related to this PR but if you could take a look I would appreciate it.

Signed-off-by: Rahul Shetty <[email protected]>
@rh-rahulshetty
Copy link
Contributor Author

rh-rahulshetty commented Jan 30, 2026

It seems the earlier CI failure was due to formatting issue in couple of files. Have run the black . command to format them.

Also I might need to get some permission to run the check automatically: https://github.com/ambient-code/agentready/pull/269/checks

@github-actions
Copy link
Contributor

📈 Test Coverage Report

Branch Coverage
This PR 65.2%
Main 65.2%
Diff ✅ +0%

Coverage calculated from unit tests only

@chambridge chambridge merged commit c42a3c9 into ambient-code:main Jan 30, 2026
11 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 30, 2026
## [2.24.1](v2.24.0...v2.24.1) (2026-01-30)

### Bug Fixes

* Correct pre-commit template path in PrecommitHooksFixer ([#269](#269)) ([c42a3c9](c42a3c9))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.24.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to kami619/agentready that referenced this pull request Feb 3, 2026
# [2.14.0](v2.13.0...v2.14.0) (2026-02-03)

### Bug Fixes

* add bounded retry logic for LLM rate limit handling ([ambient-code#205](https://github.com/kami619/agentready/issues/205)) ([6ecb786](6ecb786)), closes [ambient-code#104](https://github.com/kami619/agentready/issues/104)
* **assessors:** FileSizeLimitsAssessor now respects .gitignore ([ambient-code#248](https://github.com/kami619/agentready/issues/248)) ([eaaecc2](eaaecc2)), closes [ambient-code#245](https://github.com/kami619/agentready/issues/245)
* **ci:** use gh pr view for fork PR number lookup in coverage comment ([ambient-code#253](https://github.com/kami619/agentready/issues/253)) ([1688362](1688362))
* Correct pre-commit template path in PrecommitHooksFixer ([ambient-code#269](https://github.com/kami619/agentready/issues/269)) ([c42a3c9](c42a3c9))
* downgrade docker/metadata-action to v5 and fix shellcheck warnings ([12f5509](12f5509))
* enable Harbor task filtering for smoketest support ([ambient-code#222](https://github.com/kami619/agentready/issues/222)) ([f780188](f780188))
* make E2E test timeouts configurable and add sensitive directory test ([ambient-code#206](https://github.com/kami619/agentready/issues/206)) ([27e87e5](27e87e5)), closes [ambient-code#104](https://github.com/kami619/agentready/issues/104) [ambient-code#192](https://github.com/kami619/agentready/issues/192)
* prevent unauthorized message for non-command comments ([ambient-code#262](https://github.com/kami619/agentready/issues/262)) ([84c6f69](84c6f69))
* rename research report in data directory ([b8ddfdc](b8ddfdc))
* resolve all test suite failures - achieve zero failures ([ambient-code#180](https://github.com/kami619/agentready/issues/180)) ([990fa2d](990fa2d)), closes [ambient-code#148](https://github.com/kami619/agentready/issues/148) [ambient-code#147](https://github.com/kami619/agentready/issues/147) [ambient-code#145](https://github.com/kami619/agentready/issues/145)
* resolve YAML syntax error in continuous-learning workflow ([ambient-code#172](https://github.com/kami619/agentready/issues/172)) ([3d40fcc](3d40fcc))
* resolve YAML syntax error in update-docs workflow and add actionlint ([ambient-code#173](https://github.com/kami619/agentready/issues/173)) ([97b06af](97b06af))
* update --version flag to show correct version and research report date ([ambient-code#221](https://github.com/kami619/agentready/issues/221)) ([5a85abb](5a85abb))
* **workflows:** ensure post-comment step runs after Claude Code Action ([b087e5c](b087e5c))
* **workflows:** handle all event types in agentready-dev workflow ([9b942bf](9b942bf))
* **workflows:** improve error handling and logging for comment posting ([9ea1e6b](9ea1e6b))
* **workflows:** improve issue number extraction and add debug step ([ecd896b](ecd896b))
* **workflows:** remove if:always() to test step execution ([ff0bb12](ff0bb12))
* **workflows:** simplify post-comment step condition ([1bbf40a](1bbf40a))

### Features

* add Harbor Terminal-Bench comparison for agent effectiveness ([ambient-code#199](https://github.com/kami619/agentready/issues/199)) ([a56e318](a56e318))
* add Memory MCP server allow list to repository settings ([ambient-code#203](https://github.com/kami619/agentready/issues/203)) ([41d87bb](41d87bb))
* **assessors:** support AGENTS.md and @ references in CLAUDEmdAssessor ([ambient-code#265](https://github.com/kami619/agentready/issues/265)) ([450ec25](450ec25)), closes [ambient-code#244](https://github.com/kami619/agentready/issues/244)
* consolidate GitHub Actions workflows by purpose ([ambient-code#217](https://github.com/kami619/agentready/issues/217)) ([717ca6b](717ca6b)), closes [ambient-code#221](https://github.com/kami619/agentready/issues/221)
* container support ([ambient-code#171](https://github.com/kami619/agentready/issues/171)) ([c6874ea](c6874ea))
* convert AgentReady assessment to on-demand workflow ([ambient-code#213](https://github.com/kami619/agentready/issues/213)) ([b5a1ce0](b5a1ce0)), closes [ambient-code#191](https://github.com/kami619/agentready/issues/191)
* enhance assessors with multi-language support and security ([ambient-code#200](https://github.com/kami619/agentready/issues/200)) ([85712f2](85712f2)), closes [ambient-code#10](https://github.com/kami619/agentready/issues/10)
* Harbor framework integration for Terminal-Bench evaluations ([ambient-code#202](https://github.com/kami619/agentready/issues/202)) ([d73a8c8](d73a8c8)), closes [#4](#4) [ambient-code#178](https://github.com/kami619/agentready/issues/178) [ambient-code#178](https://github.com/kami619/agentready/issues/178)
* integrate ACL file with Claude Code Action allowed_users ([ambient-code#261](https://github.com/kami619/agentready/issues/261)) ([fe52489](fe52489))
* Redesign homepage features with two-column layout and research links ([ambient-code#189](https://github.com/kami619/agentready/issues/189)) ([570087d](570087d)), closes [ambient-code#187](https://github.com/kami619/agentready/issues/187)
* replace markdown-link-check with lychee for link validation ([ambient-code#177](https://github.com/kami619/agentready/issues/177)) ([f1a4545](f1a4545))
* Terminal-Bench eval harness (MVP Phase 1) ([ambient-code#178](https://github.com/kami619/agentready/issues/178)) ([d06bab4](d06bab4)), closes [ambient-code#171](https://github.com/kami619/agentready/issues/171)
* **workflows:** add comment posting for [@agentready-dev](https://github.com/agentready-dev) agent ([5dff614](5dff614))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants