Skip to content

Fix/doc sync hook automation - #15

Merged
mvoutov merged 2 commits into
mainfrom
fix/doc-sync-hook-automation
Mar 22, 2026
Merged

Fix/doc sync hook automation#15
mvoutov merged 2 commits into
mainfrom
fix/doc-sync-hook-automation

Conversation

@kzhivotov

@kzhivotov kzhivotov commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

What

Fixes the doc-sync post-commit hook (exit 0 bug, silent failures, no uninstall) and makes hook installation automatic after doc init.

Why

The hook had a bug where exit 0 in the cooldown path killed parent hooks when appended. Background sync failures were invisible (no logging). And the hook required a separate manual step that most users wouldn't discover.

Closes #

How I tested

  • npm test passes
  • Tested against a real repo:
  • --dry-run output looks correct (if applicable)

Checklist

  • Changes are focused on a single feature or fix
  • Tests added or updated for any logic changes
  • No new dependencies added (or justified in the PR description)

Summary by CodeRabbit

Release Notes

  • New Features

    • Added --no-hook option to skip git hook setup prompts during documentation initialization.
    • Added --remove-hook option to uninstall the automatic git post-commit hook.
    • Enhanced git hook management with improved logging and idempotent installation.
  • Tests

    • Added comprehensive test suite for git hook installation and removal workflows.

@coderabbitai

coderabbitai Bot commented Mar 22, 2026

Copy link
Copy Markdown

Walkthrough

These changes introduce Git hook management for automatic documentation synchronization. Users can now install a post-commit hook during doc init, configure hook behavior with CLI flags (--no-hook, --remove-hook), and manage the installed hook through dedicated commands.

Changes

Cohort / File(s) Summary
CLI Option Registration
bin/cli.js
Added two new CLI options: --no-hook flag for doc init to skip hook installation prompt, and --remove-hook flag for doc sync to remove the installed hook.
Hook Installation & Management
src/commands/doc-init.js, src/commands/doc-sync.js
doc-init.js now conditionally prompts to install a Git post-commit hook after documentation generation. doc-sync.js introduces hook removal capability (removeGitHook), replaces hook command format with new marker-based blocks (# >>> aspens doc-sync hook ...), improves hook detection to recognize both legacy and new formats, and exports two new functions (installGitHook, removeGitHook) for hook lifecycle management.
Hook Management Tests
tests/git-hook.test.js
New comprehensive test suite validating hook installation idempotency, correct shell formatting, logging configuration, executable permissions, and removal behavior for aspens-managed and legacy hooks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title directly addresses the main changes: hook automation fixes and making installation automatic after doc init.
Description check ✅ Passed Description covers what (hook fixes and auto-install), why (bug and UX), but testing checkboxes remain unchecked and lack completion details.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/doc-sync-hook-automation

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/commands/doc-sync.js (1)

348-379: Use a portable hash fallback for cross-platform compatibility.

The hook uses shasum which is standard on macOS but unavailable on many Linux systems (e.g., Alpine, Ubuntu without perl module). The existing post-tool-use-tracker.sh hook in the codebase demonstrates the correct fallback pattern already used elsewhere:

🔧 Portable hash command
-  REPO_HASH="\$(echo "\$REPO_ROOT" | shasum | cut -c1-8)"
+  REPO_HASH="\$(echo "\$REPO_ROOT" | (shasum 2>/dev/null || sha1sum) | cut -c1-8)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/doc-sync.js` around lines 348 - 379, The hookBlock uses shasum
to compute REPO_HASH which breaks on systems without shasum; update the shell
snippet inside hookBlock (function __aspens_doc_sync and variable REPO_HASH) to
use a portable fallback (try shasum, else sha1sum, else openssl sha1) when
computing the hash of REPO_ROOT so it works on macOS and Linux variants —
implement the same fallback pattern used in post-tool-use-tracker.sh so
REPO_HASH is set reliably across platforms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/commands/doc-sync.js`:
- Around line 348-379: The hookBlock uses shasum to compute REPO_HASH which
breaks on systems without shasum; update the shell snippet inside hookBlock
(function __aspens_doc_sync and variable REPO_HASH) to use a portable fallback
(try shasum, else sha1sum, else openssl sha1) when computing the hash of
REPO_ROOT so it works on macOS and Linux variants — implement the same fallback
pattern used in post-tool-use-tracker.sh so REPO_HASH is set reliably across
platforms.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72fa6493-f1db-423a-b09d-6ece5b9d7fdb

📥 Commits

Reviewing files that changed from the base of the PR and between c14ad2c and b6247bf.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • bin/cli.js
  • src/commands/doc-init.js
  • src/commands/doc-sync.js
  • tests/git-hook.test.js

@kzhivotov
kzhivotov requested a review from mvoutov March 22, 2026 04:26
@mvoutov
mvoutov merged commit 8a80120 into main Mar 22, 2026
2 checks passed
@mvoutov
mvoutov deleted the fix/doc-sync-hook-automation branch March 22, 2026 04:38
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.

2 participants