Skip to content

Commit 0ef6d47

Browse files
Claudejedudenclaude
authored
Implement MDS048 git-hook-sync rule and pre-merge-commit hook automation (#212)
* Add pre-merge-commit subcommand with install/uninstall/status - Add new `pre-merge-commit` top-level command alongside `merge-driver` - Supports `install [files...]` to install hook for specified files - Supports `uninstall` to safely remove mdsmith-managed hooks - Supports `status` to show installation state and configured files - Hook runs `mdsmith fix` then `git add` to stage resolved conflicts - Comprehensive test coverage for all subcommands - Update main.go to register new command in CLI Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/0a73443c-afb9-4783-88c8-91e27470ea9d Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Document pre-merge-commit command in CLI reference - Add pre-merge-commit to commands table - Document install/uninstall/status subcommands - Explain hook behavior: runs after per-file merges, before merge commit - Note that hook stages resolved files with git add - Auto-fix table formatting Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/0a73443c-afb9-4783-88c8-91e27470ea9d Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Remove hardcoded files from CLI subcommands Replace hardcoded default files with automatic discovery: - Add discoverFilesWithGeneratedContent() function - Scans repo for files with <?catalog?>, <?include?>, <?toc?> - Falls back to PLAN.md, README.md if nothing found - Update merge-driver install to use discovery - Update pre-merge-commit install to use discovery - Update help text to document auto-discovery - Add comprehensive tests for discovery function - All tests pass (8.859s) Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/ead687d0-ae03-4873-ae05-a8258bc7c5a6 Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Add sync detection to pre-merge-commit status command Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/d914ddb4-6726-4899-a1b4-b4ec40d9b086 Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Implement MDS047 git-hook-sync rule to detect out-of-sync hooks Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/265ff37d-15ad-45a4-84c7-22c2408c97be Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Renumber git-hook-sync from MDS047 to MDS048 to avoid PR conflict Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/1cf0592b-01df-4155-9933-fc145afe524b Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Fix MDS048 integration tests - add missing import and fixture Added githooksync package import to rules_test.go so MDS048 is registered in the rule registry. Created minimal good fixture since MDS048 is a meta rule requiring actual git repo setup (tested via unit tests in rule_test.go). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Fix CI lint and mdsmith check errors Lint fixes: - Add error checks for w.Close() calls in test cleanup - Refactor Check function to be under 60 lines (funlen) - Run gofmt on discover_test.go mdsmith check fixes: - Wrap long lines in MDS048 README.md - Fix broken link to rule.go (use ../githooksync/rule.go) - Use backticks for directive examples to avoid parsing - Increase table max-rows limit to 45 for rules index 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Fix remaining errcheck lint errors in test cleanup Add error checks for remaining w.Close() calls at lines 197 and 231 in premergecommit_test.go. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> * Address PR #212 review: align MDS048 with real installer behavior - Move discovery and hook parsing into a shared internal/githooks package so the CLI and the git-hook-sync rule cannot drift. - Read .gitattributes (the real source of truth for merge driver assignments) instead of parsing .git/config sections that mdsmith never writes; only check when merge.mdsmith.driver is registered. - Use the canonical "# mdsmith merge-driver pre-merge-commit hook" marker so the rule recognises hooks the installer writes. - Resolve hooks dir via "git rev-parse --git-path hooks" so the rule works in worktrees, submodules, and core.hooksPath repos. - Resolve the git repo root from the file's directory (not the process CWD) so linting absolute paths and the NotInGitRepo test no longer depend on where mdsmith is invoked from. - Drop FixableRule from MDS048; hook installation is a side- effecting operation outside the scope of mdsmith fix. - Stop rewriting .mdsmith.yml during install commands; print the enable-rule snippet instead, per CLAUDE.md guidance about modifying linter config. - Normalize discovered paths to forward slashes so comparisons against .gitattributes / hook content work on Windows. - Replace the brittle extractFilesFromHook parser with one that pulls the first single-quoted token after each "fix --" marker. - Track reported repos per Rule instance so duplicate diagnostics are not emitted while linting many files in the same repo. - Update tests and the rule README to match the real installer layout (.gitattributes + merge.mdsmith.driver). * Improve test coverage on pre-merge-commit and shared githooks helpers Add tests covering the install/uninstall/status dispatch, the not-in-git-repo error branches, the auto-discovery branch when install is invoked without arguments, and the unmanaged-hook path in status. Also add metadata tests for the MDS048 rule and boundary-condition tests for the firstQuotedAfter parser. * Lift coverage on changed files toward 100% Adds focused tests for the remaining uncovered branches: - githooks: empty-dir GitRepoRoot, fallback ResolveHooksDir when the directory is not a git repo, skipping non-Markdown files in DiscoverFiles, single-field lines in ExtractGitattributesFiles, and edge cases in firstQuotedAfter. - githooksync rule: merge-driver registered without .gitattributes on disk, and a pre-merge-commit hook that lacks the mdsmith marker (user-authored hook). - pre-merge-commit CLI: install refusing to overwrite a user-authored hook. - merge-driver CLI: install with no args (auto-discovery branch) and the not-in-git-repo error path. Also drops an unreachable branch in Rule.Check (filepath.Dir never returns "" for a non-empty path). Per-package coverage on the new code: - internal/githooks: 97.9% - internal/rules/githooksync: 98.0% * Cover the remaining new code paths Adds tests for: - main.go run() dispatching to pre-merge-commit - merge-driver and pre-merge-commit install: not-in-git-repo, loadConfig error (malformed .mdsmith.yml), and bad max-input-size (invalid size string) * Address Copilot round 2: bool-enable + dedupe FilesMatch - Drop the `configured` gate on Rule.Check so the zero-value Rule runs when enabled via the bool form `git-hook-sync: true` (the framework only calls ApplySettings for mapping configs, leaving the gate stuck at false otherwise). - ApplySettings still rejects unknown keys, but no longer flips a state flag the Check path depends on. - Make FilesMatch tolerant of duplicate entries on either side by comparing deduplicated sets. - Fix two integration tests that wrote `max-input-bytes` (an unknown key) to .mdsmith.yml; use the supported `max-input-size` key instead. * Address Copilot round 3: collapse diagnostics, decode shell quotes - Rule.Check now emits at most one diagnostic per repository per lint run, joining both .gitattributes and pre-merge-commit hook drift into a single message. Brings the implementation in line with the behavior the README has always claimed. - Treat "merge.mdsmith.driver registered, but .gitattributes has no merge=mdsmith entries" as drift when discovered files exist. The driver would not run for any file, so silently passing was the wrong default. - Teach firstQuotedAfter to decode the POSIX shell-quote escape shellQuote produces ('a'\''b.md' -> a'b.md), so file paths that contain a single quote round-trip correctly through the pre-merge-commit hook script. * Fix gofmt: rephrase comment to avoid doc-comment auto-typography gofmt's doc-comment formatter rewrites consecutive ASCII single quotes into typographic right-double-quote, which mangled the shell-quote example in firstQuotedAfter's doc comment. Rephrase the comment so it conveys the same idea without the offending character sequence. * Address Copilot round 4: directive parsing, deterministic order - DiscoverFiles now scans line-by-line and uses gensection.IsRawStartMarker / IsRawEndMarker so prose mentions of <?catalog?> (e.g. inside backticks) are no longer detected as directive-bearing. Avoids bloating .gitattributes / hook files and silences false drift diagnostics. - Skip non-regular files (symlinks, FIFOs, devices, sockets) in DiscoverFiles. Matches the project's secure-by-default stance on symlinks and prevents hangs on FIFO entries. - Sort and dedupe the discovered file list so the output is deterministic across platforms and filesystems. The list is printed to users and embedded into generated artifacts, so stability matters for review diffs. - Replace the non-ASCII warning glyph in `pre-merge-commit status` output with a plain "Warning:" prefix for terminal/grep compatibility. - Add regression tests covering prose-only directive mentions and sorted deduplicated discovery output. * Address Copilot round 5: normalize and validate managed paths - Add githooks.NormalizeManagedPath / NormalizeManagedPaths that convert paths to repo-relative + forward-slash form, reject empty inputs, reject whitespace (which the .gitattributes parser cannot recover), and reject paths that escape the repo. - Apply the normalizer to the explicit-args branch in both `merge-driver install` and `pre-merge-commit install`, and share the resolution logic in resolveManagedFiles so the two commands can't drift on path handling. - Document the whitespace constraint on ExtractGitattributesFiles, since the parser splits on whitespace and cannot tokenize paths that contain it. The install-time normalizer enforces the constraint, keeping the installer and the drift checker in lockstep. * Cover the path-validation error branches Both install commands now have a test that exercises the NormalizeManagedPaths failure path (whitespace in arg), covering the corresponding stderr/return-2 branches in resolveManagedFiles that codecov flagged as untested. * Cover more new branches: ReadFile errors, symlink skip, normalize paths - Add a happy-path test for NormalizeManagedPaths so the success return is exercised from the package's own tests (codecov tracks per-package coverage, so the cmd/mdsmith call sites are not counted toward githooks.go coverage). - Add a SkipsSymlinks test that creates a real .md file plus a matching symlink and asserts only the regular file is returned, exercising the non-regular file skip in DiscoverFiles. - Add ReadError tests for `pre-merge-commit uninstall` and `pre-merge-commit status` that point hookPath at a directory so os.ReadFile returns a non-IsNotExist error. - Add a RemoveError test for uninstall that exercises the os.Remove failure branch when permissions can deny it; the test self-skips when running as root, where chmod is bypassed. * Address Copilot round 6: surface read errors, render empty as (none) - Drop the duplicated ExtractGitattributesFiles doc comment. - mergeDriverDrift now surfaces non-ENOENT read errors on .gitattributes as a "cannot verify merge-driver assignments" diagnostic instead of silently passing. Permission/IO failures no longer mask real misconfiguration. - preMergeCommitHookDrift does the same for the hook script: a non-ENOENT read failure is reported as "cannot verify pre-merge-commit hook" rather than treated as "in sync". - When the mdsmith-managed hook contains no parsable `fix -- '...'` entries, the drift message renders the installed list as "(none)" rather than a blank string. - Tests cover the (none) rendering and both new read-error branches by pointing each path at a directory so ReadFile returns IsADirectory. * Move githooksync once-per-repo state to package scope The engine clones Configurable rules per file when the rule is enabled with a settings mapping (even an empty {}). With per-Rule state, each clone reset the reported set, so the "at most one diagnostic per repository" guarantee broke as soon as a user opted in via mapping syntax. - Move reportedRepos / reportedMu to package scope so all clones share one set, regardless of whether the rule is enabled with `git-hook-sync: true` or `git-hook-sync: {}`. - Add resetReportedForTest so tests do not leak repo paths into later cases. - Add a regression test that calls rule.CloneRule twice and verifies the second clone does not re-emit the diagnostic for the same repo. * Address Copilot round 7: fenced-code, normalization, hermeticity - DiscoverFiles now tracks fenced-code state (CommonMark backtick and tilde fences) and ignores directive markers inside fences. mdsmith only parses processing-instructions at the document root, so files that merely show <?catalog?> in a code example are no longer treated as directive-bearing. - NormalizeManagedPath now performs the whitespace check on the normalized repo-relative result rather than the raw input, so a repo whose own path contains spaces (Windows/macOS home dirs) accepts absolute inputs as long as the repo-relative tail is whitespace-free. - resolveManagedFiles funnels both the explicit-args branch and the auto-discovery branch through NormalizeManagedPaths, so the installer cannot write a token that the drift checker would reject (or vice versa). - Update the pre-merge-commit hook header comment to mention both install entry points (`mdsmith merge-driver install` and `mdsmith pre-merge-commit install`) so users see accurate guidance regardless of which subcommand they used. - Make the MDS048 integration fixture hermetic by writing its fixture file path inside a fresh non-repo tempdir; the rule's GitRepoRoot lookup fails there, so the fixture cannot fail based on a contributor's locally-installed merge driver / hook. * Skip directive markers in indented code blocks too CommonMark treats a line beginning with a tab or four-plus spaces as an indented code block; internal/lint.pi_parser refuses to parse a processing-instruction on such lines for the same reason. Bring hasDirectiveMarker in line with that rule so a directive shown in an indented example does not trigger discovery. Add a regression test covering both 4-space and tab-indented markers. * Address Copilot round 9: local config scope, doc accuracy - HasMdsmithMergeDriver now uses `git config --local` so a user-global merge driver cannot opt every clone into MDS048's drift checks. Documented the local-only scope. - Update the pre-merge-commit usage text to list catalog, include, *and* toc, matching what discovery actually scans for. - Align the rule's inline comment and README with the actual behavior: the once-per-repo guard lives for the lifetime of the mdsmith process, not a single lint run. * Address Copilot round 10: split rule from install fallback The shared discovery helper used to fall back to [PLAN.md, README.md] whenever a repo contained no directive- bearing files. The fallback makes sense for install commands but not for the git-hook-sync rule: the rule should not compare against fictional paths. - DiscoverFiles now returns whatever the walk produced (possibly empty) and never invents PLAN.md / README.md. - DiscoverFilesForInstall is a thin wrapper that applies the install-only fallback. Both `merge-driver install` and `pre-merge-commit install` use this variant via the existing CLI shim. - Drift messages render an empty discovered list as "(none)" so a stale .gitattributes / hook in a directive-free repo still produces a readable warning. - Update the rule README to document the rule-vs-install difference, and drop the spurious "git-hook-sync:" prefix from the example diagnostic so the snippet matches mdsmith's actual output (the rule ID/name are printed by the formatter). * Address Copilot round 11: precise closing fence + accurate README - isClosingFence now requires the line to end in whitespace after the fence run, per CommonMark. A line like "\`\`\`not-a-closing- fence" is content, not a fence terminator, so a directive marker later in the same fenced block is correctly ignored. Add a regression test that opens a fence, embeds a "\`\`\`text" line, and asserts the directive marker afterwards is not discovered. - Update the rule README to describe the actual empty-discovery behavior: when discovery is empty, the rule is silent if no managed source lists files, but reports stale entries when one still does (e.g. left over from install-time PLAN.md/README.md fallback). The previous wording wrongly implied it never fires. * Address Copilot round 12: drain stderr pipes + document fallback - Replace the five hand-rolled `os.Pipe` redirections in premergecommit_test.go with the existing captureStderr helper, which drains and closes the read end. The previous pattern left the read end open, risking FD leaks and blocked writes once the kernel buffer filled. - Update both install commands' help text to mention the install-time fallback to PLAN.md and README.md when no directive-bearing files are discovered, so users are not surprised when those entries appear in fresh repos. * Address Copilot round 13: reject glob inputs, fix test comment - NormalizeManagedPath now rejects any path containing `*`, `?`, or `[`. The pre-merge-commit hook script wraps each managed entry in a `[ -e <path> ]` guard, and `[ -e ]` treats its argument as a literal filename, so a glob like docs/*.md would always be skipped even when files match. The drift checker likewise compares exact paths, so accepting globs would create silent false matches. Reject them at install time. - Update the comment on TestRunPreMergeCommitUninstall_RemoveError to describe what actually happens: the test creates an mdsmith-marked hook file and drops write permission from the hooks directory so os.Remove fails with EACCES (not ENOTEMPTY, which the previous comment claimed). The probe-and-skip path for root unchanged. * Address Copilot round 14: env-robust tests + parser/doc tightening - Replace six hard-coded `<repo>/.git/hooks` test setups in internal/rules/githooksync/rule_test.go with `githooks.ResolveHooksDir(dir)` so the tests do not break on developer machines that set core.hooksPath. - TestResolveHooksDir_Default now asks git itself (`git rev-parse --git-path hooks`) for the expected hooks directory rather than hard-coding `.git/hooks`. A non-default global core.hooksPath no longer makes the test flap. - ExtractHookFiles skips comment and blank lines so a commented- out example in a hook script (e.g. a "see also" note) does not produce a false managed-file entry. Add a regression test. - Update the rule README to describe install behavior accurately: `pre-merge-commit install` rewrites the hook so stale entries drop, but `merge-driver install` is append-only on .gitattributes. Mention that users must edit .gitattributes by hand to remove obsolete `merge=mdsmith` lines. * Address Copilot round 15: command prefix + uniform fixture isolation - README now spells the install commands as `mdsmith pre-merge-commit install` and `mdsmith merge-driver install` consistently. Users copying the command no longer hit a shell-not-found. - Apply fixtureFilePath to bad/ and fixed/ runners too, not just good/. Future MDS048 (or any other git-aware rule) bad/fixed fixtures will use a non-repo tempdir as f.Path automatically, staying hermetic from the contributor's local git config. * Address Copilot round 16: pin core.hooksPath in test repos Add an initTestRepo helper to both cmd/mdsmith and internal/rules/githooksync test packages. The helper runs `git init` and immediately pins core.hooksPath to <repo>/.git/hooks in the repo-local config. Replace every bare `git init <dir>` in those packages' tests with the helper. The pin keeps tests hermetic: a contributor whose global git config sets core.hooksPath to a custom directory can no longer have these tests create, modify, or delete hook files in their real environment. install/uninstall/status, the drift rule, and the integration runner all read hooks via ResolveHooksDir, which honors the now-local core.hooksPath. * Skip TestDiscoverFiles_SkipsSymlinks where symlinks unsupported Use the repo's existing testutil.SkipIfSymlinkUnsupported helper so the test cleanly skips on Windows / sandboxed CI environments where os.Symlink fails without elevated privileges, matching the pattern used by other symlink-creating tests in the codebase. * Address Copilot round 18: skip rule on stdin + surface status warnings - MDS048's Check now short-circuits when f.FS == nil. RunSource (used by stdin and other in-memory inputs) leaves FS nil, and filepath.Dir("<stdin>") would otherwise resolve git repo discovery against the process working directory and emit drift diagnostics unrelated to the linted content. - Update existing rule tests to set f.FS = os.DirFS(dir) so the rule still runs against the temp repo, and add a regression test that confirms a nil-FS file is silently skipped. - pre-merge-commit status now surfaces a non-fatal "Warning: could not load config / could not resolve max-input-size" message instead of silently skipping drift detection. The user can no longer be misled into thinking the hook is in sync when the comparison was never executed. - Extract a printManagedHookStatus helper so runPreMergeCommitStatus stays under the funlen limit, and add tests for both new warning branches. * Treat <=3 spaces + tab as indented code in discovery isIndentedCodeBlock previously only flagged lines that started with a tab in column 0 or had four-plus leading spaces. A line like ' \t<?catalog?>' (up to three spaces followed by a tab) is also an indented code block per CommonMark, and mdsmith's PI parser refuses to parse a processing-instruction there. Update the gate to scan past leading spaces and treat the next tab as indented code, so discovery cannot pick those markers up either. Extend the indented-code regression test to cover the spaces-then-tab case alongside the existing 4-space and column-0 tab cases. * Address Copilot round 19: skip walk + document config bump - Add a cheap opt-in pre-check in MDS048's Check that probes the local merge driver registration and the pre-merge-commit hook before invoking the repo-wide discovery walk. When neither source is opted in (and the hook is not unreadable), the rule returns nil immediately and avoids the O(repo) walk for projects that don't use mdsmith hooks at all. - Distinguish "hook absent" (ENOENT) from "hook unreadable" via a small peekHookSource helper so the IO-error warning path in preMergeCommitHookDrift still fires even when the merge driver isn't registered. - Add a regression test that confirms the rule emits no diagnostics in a repo with directive-bearing files but no mdsmith hooks installed. - Document the .mdsmith.yml table-readability bump in the PR description (40 -> 45) so reviewers can see the rationale: the rules index table grew past 40 rows when MDS048 was added. --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 926fddc commit 0ef6d47

17 files changed

Lines changed: 3085 additions & 56 deletions

File tree

.mdsmith.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ overrides:
122122
- files: ["internal/rules/index.md"]
123123
rules:
124124
table-readability:
125-
max-rows: 40
125+
max-rows: 45
126126
- files: ["README.md"]
127127
rules:
128128
line-length:

cmd/mdsmith/discover.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package main
2+
3+
import "github.com/jeduden/mdsmith/internal/githooks"
4+
5+
// discoverFilesWithGeneratedContent is a thin shim around
6+
// githooks.DiscoverFilesForInstall so the install commands keep their
7+
// previous "fall back to PLAN.md / README.md when no directives are
8+
// found" behavior. The shared implementation lives in
9+
// internal/githooks so the CLI and the MDS048 rule cannot drift on
10+
// directive matching itself; only the install-time fallback diverges.
11+
func discoverFilesWithGeneratedContent(repoRoot string, maxBytes int64) []string {
12+
return githooks.DiscoverFilesForInstall(repoRoot, maxBytes)
13+
}

cmd/mdsmith/discover_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestDiscoverFilesWithGeneratedContent(t *testing.T) {
13+
dir := t.TempDir()
14+
15+
// Create test markdown files.
16+
files := map[string]string{
17+
"README.md": "# Test\n\n<?catalog?>\n<?/catalog?>\n",
18+
"PLAN.md": "# Plan\n\n<?include file: foo.md ?>\n",
19+
"doc.md": "# Normal file\n\nNo directives here.\n",
20+
"guide.md": "# Guide\n\n<?toc?>\n<?/toc?>\n",
21+
".hidden/secret.md": "# Hidden\n\n<?catalog?>\n",
22+
}
23+
24+
for name, content := range files {
25+
path := filepath.Join(dir, name)
26+
require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755))
27+
require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
28+
}
29+
30+
// Test discovery.
31+
discovered := discoverFilesWithGeneratedContent(dir, 1024*1024)
32+
33+
// Should find files with directives (but not hidden files).
34+
assert.Contains(t, discovered, "README.md")
35+
assert.Contains(t, discovered, "PLAN.md")
36+
assert.Contains(t, discovered, "guide.md")
37+
assert.NotContains(t, discovered, "doc.md")
38+
assert.NotContains(t, discovered, ".hidden/secret.md")
39+
}
40+
41+
func TestDiscoverFilesWithGeneratedContent_EmptyRepo(t *testing.T) {
42+
dir := t.TempDir()
43+
44+
// Empty repo with no markdown files.
45+
discovered := discoverFilesWithGeneratedContent(dir, 1024*1024)
46+
47+
// Should fall back to defaults.
48+
assert.Equal(t, []string{"PLAN.md", "README.md"}, discovered)
49+
}
50+
51+
func TestDiscoverFilesWithGeneratedContent_NoDirectives(t *testing.T) {
52+
dir := t.TempDir()
53+
54+
// Create markdown files without directives.
55+
files := map[string]string{
56+
"README.md": "# Test\n\nNormal content.\n",
57+
"doc.md": "# Doc\n\nMore content.\n",
58+
}
59+
60+
for name, content := range files {
61+
path := filepath.Join(dir, name)
62+
require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
63+
}
64+
65+
// Test discovery.
66+
discovered := discoverFilesWithGeneratedContent(dir, 1024*1024)
67+
68+
// Should fall back to defaults when no directives found.
69+
assert.Equal(t, []string{"PLAN.md", "README.md"}, discovered)
70+
}

cmd/mdsmith/main.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
_ "github.com/jeduden/mdsmith/internal/rules/fencedcodelanguage"
3737
_ "github.com/jeduden/mdsmith/internal/rules/fencedcodestyle"
3838
_ "github.com/jeduden/mdsmith/internal/rules/firstlineheading"
39+
_ "github.com/jeduden/mdsmith/internal/rules/githooksync"
3940
_ "github.com/jeduden/mdsmith/internal/rules/headingincrement"
4041
_ "github.com/jeduden/mdsmith/internal/rules/headingstyle"
4142
_ "github.com/jeduden/mdsmith/internal/rules/include"
@@ -74,16 +75,17 @@ func main() {
7475
const usageText = `Usage: mdsmith <command> [flags] [files...]
7576
7677
Commands:
77-
check Lint Markdown files (default when given file arguments)
78-
fix Auto-fix lint issues in place
79-
query Select files by CUE expression on front matter
80-
help Show help for rules and topics
81-
metrics Show and rank shared Markdown metrics
82-
merge-driver Git merge driver for regenerable sections
83-
archetypes Discover, show, and locate archetype schemas
84-
kinds Inspect declared kinds and resolve effective config per file
85-
init Generate a default .mdsmith.yml config file
86-
version Print version and exit
78+
check Lint Markdown files (default when given file arguments)
79+
fix Auto-fix lint issues in place
80+
query Select files by CUE expression on front matter
81+
help Show help for rules and topics
82+
metrics Show and rank shared Markdown metrics
83+
merge-driver Git merge driver for regenerable sections
84+
pre-merge-commit Install/manage pre-merge-commit hook
85+
archetypes Discover, show, and locate archetype schemas
86+
kinds Inspect declared kinds and resolve effective config per file
87+
init Generate a default .mdsmith.yml config file
88+
version Print version and exit
8789
8890
Global flags:
8991
-h, --help Show this help
@@ -129,6 +131,8 @@ func run() int {
129131
return runMetrics(os.Args[2:])
130132
case "merge-driver":
131133
return runMergeDriver(os.Args[2:])
134+
case "pre-merge-commit":
135+
return runPreMergeCommit(os.Args[2:])
132136
case "archetypes":
133137
return runArchetypes(os.Args[2:])
134138
case "kinds":

cmd/mdsmith/mergedriver.go

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/jeduden/mdsmith/internal/archetype/gensection"
1212
fixpkg "github.com/jeduden/mdsmith/internal/fix"
13+
"github.com/jeduden/mdsmith/internal/githooks"
1314
"github.com/jeduden/mdsmith/internal/lint"
1415
vlog "github.com/jeduden/mdsmith/internal/log"
1516
"github.com/jeduden/mdsmith/internal/rule"
@@ -27,7 +28,10 @@ Subcommands:
2728
install [files...]
2829
Register the merge driver in git config and ensure
2930
.gitattributes assigns it to the listed files.
30-
Default files: PLAN.md README.md
31+
32+
When no files are specified, automatically discovers
33+
files with generated content (catalog, include, toc),
34+
falling back to PLAN.md and README.md if none are found.
3135
3236
Git config (set by install):
3337
merge.mdsmith.driver = '/absolute/path/to/mdsmith' merge-driver run %O %A %B %P
@@ -352,9 +356,40 @@ func hasConflictMarkers(content []byte) bool {
352356
return false
353357
}
354358

355-
// defaultMergeDriverFiles are the files assigned to the merge
356-
// driver when install is run without explicit file arguments.
357-
var defaultMergeDriverFiles = []string{"PLAN.md", "README.md"}
359+
// resolveManagedFiles returns the canonical (repo-relative,
360+
// forward-slash) list of files to manage for an install command.
361+
// Both branches — explicit args and auto-discovery — go through
362+
// githooks.NormalizeManagedPaths so the on-disk artefacts and the
363+
// drift checker agree on what counts as the canonical form. The
364+
// second return is the process exit code: 0 on success, 2 on a
365+
// user-facing error (already printed to stderr).
366+
func resolveManagedFiles(repoRoot string, args []string) ([]string, int) {
367+
if len(args) > 0 {
368+
normalized, err := githooks.NormalizeManagedPaths(repoRoot, args)
369+
if err != nil {
370+
fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
371+
return nil, 2
372+
}
373+
return normalized, 0
374+
}
375+
cfg, _, err := loadConfig("")
376+
if err != nil {
377+
fmt.Fprintf(os.Stderr, "mdsmith: loading config: %v\n", err)
378+
return nil, 2
379+
}
380+
maxBytes, err := resolveMaxInputBytes(cfg, "")
381+
if err != nil {
382+
fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
383+
return nil, 2
384+
}
385+
discovered := discoverFilesWithGeneratedContent(repoRoot, maxBytes)
386+
normalized, err := githooks.NormalizeManagedPaths(repoRoot, discovered)
387+
if err != nil {
388+
fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
389+
return nil, 2
390+
}
391+
return normalized, 0
392+
}
358393

359394
// runMergeDriverInstall registers the mdsmith merge driver in
360395
// the local git config and ensures .gitattributes assigns it.
@@ -378,10 +413,9 @@ func runMergeDriverInstall(args []string) int {
378413
return 2
379414
}
380415

381-
// Determine file list: use args if given, else defaults.
382-
files := defaultMergeDriverFiles
383-
if len(args) > 0 {
384-
files = args
416+
files, rc := resolveManagedFiles(repoRoot, args)
417+
if rc != 0 {
418+
return rc
385419
}
386420

387421
attrPath := filepath.Join(repoRoot, ".gitattributes")
@@ -402,29 +436,25 @@ func runMergeDriverInstall(args []string) int {
402436
fmt.Fprintf(os.Stderr, " git config: merge.mdsmith.driver\n")
403437
fmt.Fprintf(os.Stderr, " .gitattributes: %s\n", attrPath)
404438
fmt.Fprintf(os.Stderr, " pre-merge-commit hook: %s\n", hookPath)
439+
fmt.Fprintf(os.Stderr,
440+
"\nTo also enable drift detection, add this to your .mdsmith.yml:\n\n%s\n",
441+
githooks.EnableRuleSnippet("git-hook-sync"))
405442
return 0
406443
}
407444

408445
// preMergeCommitHookMarker identifies the hook as managed by
409446
// mdsmith so re-running install can safely replace it without
410-
// stomping on a user-authored hook of the same name.
411-
const preMergeCommitHookMarker = "# mdsmith merge-driver pre-merge-commit hook"
447+
// stomping on a user-authored hook of the same name. The canonical
448+
// constant lives in internal/githooks; this alias keeps existing
449+
// references in this package and its tests stable.
450+
const preMergeCommitHookMarker = githooks.PreMergeCommitMarker
412451

413452
// resolveHooksDir returns the directory where git hooks should be
414-
// installed. It respects core.hooksPath if configured so that
415-
// installations work correctly in repos that redirect hooks to a
416-
// custom path (e.g. via git config or a repo management tool).
417-
// Falls back to .git/hooks when git cannot be queried.
453+
// installed for the repo at repoRoot. The implementation lives in
454+
// internal/githooks so the CLI and the git-hook-sync rule resolve
455+
// the same path.
418456
func resolveHooksDir(repoRoot string) string {
419-
cmd := exec.Command("git", "-C", repoRoot, "rev-parse", "--git-path", "hooks")
420-
if out, err := cmd.Output(); err == nil {
421-
p := strings.TrimSpace(string(out))
422-
if !filepath.IsAbs(p) {
423-
p = filepath.Join(repoRoot, p)
424-
}
425-
return filepath.Clean(p)
426-
}
427-
return filepath.Join(repoRoot, ".git", "hooks")
457+
return githooks.ResolveHooksDir(repoRoot)
428458
}
429459

430460
// ensurePreMergeCommitHook writes the pre-merge-commit hook so
@@ -480,8 +510,9 @@ func ensurePreMergeCommitHook(repoRoot string, files []string) error {
480510
preMergeCommitHookMarker + "\n" +
481511
"# Re-runs mdsmith fix once git has resolved every per-file\n" +
482512
"# merge, so generated sections reflect the final merged\n" +
483-
"# state of every source file. Re-install with:\n" +
513+
"# state of every source file. Re-install with either:\n" +
484514
"# mdsmith merge-driver install\n" +
515+
"# mdsmith pre-merge-commit install\n" +
485516
"set -e\n" +
486517
fixCmds.String()
487518

cmd/mdsmith/mergedriver_test.go

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,103 @@ func TestRunMergeDriverInstall_HelpFlag_ExitsZero(t *testing.T) {
299299
})
300300
}
301301

302+
func TestRunMergeDriverInstall_NotInRepo(t *testing.T) {
303+
dir := t.TempDir()
304+
require.NoError(t, os.WriteFile(filepath.Join(dir, ".git"),
305+
[]byte("not a real gitdir"), 0o644))
306+
origWd, _ := os.Getwd()
307+
require.NoError(t, os.Chdir(dir))
308+
t.Cleanup(func() { _ = os.Chdir(origWd) })
309+
310+
got := captureStderr(func() {
311+
assert.Equal(t, 2, runMergeDriverInstall(nil))
312+
})
313+
assert.Contains(t, got, "not in a git repository")
314+
}
315+
316+
func TestRunMergeDriverInstall_LoadConfigError(t *testing.T) {
317+
dir := t.TempDir()
318+
initTestRepo(t, dir)
319+
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
320+
[]byte("not: [valid: yaml\n"), 0o644))
321+
322+
orig := executableFunc
323+
t.Cleanup(func() { executableFunc = orig })
324+
executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
325+
326+
origWd, _ := os.Getwd()
327+
require.NoError(t, os.Chdir(dir))
328+
t.Cleanup(func() { _ = os.Chdir(origWd) })
329+
330+
got := captureStderr(func() {
331+
assert.Equal(t, 2, runMergeDriverInstall(nil))
332+
})
333+
assert.Contains(t, got, "loading config")
334+
}
335+
336+
func TestRunMergeDriverInstall_BadMaxInputSize(t *testing.T) {
337+
dir := t.TempDir()
338+
initTestRepo(t, dir)
339+
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
340+
[]byte("max-input-size: nonsense\n"), 0o644))
341+
342+
orig := executableFunc
343+
t.Cleanup(func() { executableFunc = orig })
344+
executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
345+
346+
origWd, _ := os.Getwd()
347+
require.NoError(t, os.Chdir(dir))
348+
t.Cleanup(func() { _ = os.Chdir(origWd) })
349+
350+
got := captureStderr(func() {
351+
assert.Equal(t, 2, runMergeDriverInstall(nil))
352+
})
353+
assert.Contains(t, got, "invalid max-input-size")
354+
}
355+
356+
func TestRunMergeDriverInstall_RejectsWhitespacePath(t *testing.T) {
357+
dir := t.TempDir()
358+
initTestRepo(t, dir)
359+
360+
orig := executableFunc
361+
t.Cleanup(func() { executableFunc = orig })
362+
executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
363+
364+
origWd, _ := os.Getwd()
365+
require.NoError(t, os.Chdir(dir))
366+
t.Cleanup(func() { _ = os.Chdir(origWd) })
367+
368+
got := captureStderr(func() {
369+
assert.Equal(t, 2, runMergeDriverInstall([]string{"bad name.md"}))
370+
})
371+
assert.Contains(t, got, "whitespace")
372+
}
373+
374+
func TestRunMergeDriverInstall_NoArgsUsesDiscovery(t *testing.T) {
375+
dir := t.TempDir()
376+
initTestRepo(t, dir)
377+
378+
// Generate a markdown file with a directive so discovery returns
379+
// it instead of the PLAN.md/README.md fallback.
380+
require.NoError(t, os.WriteFile(filepath.Join(dir, "guide.md"),
381+
[]byte("# Guide\n\n<?catalog?>\n<?/catalog?>\n"), 0o644))
382+
383+
orig := executableFunc
384+
t.Cleanup(func() { executableFunc = orig })
385+
executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
386+
387+
origWd, _ := os.Getwd()
388+
require.NoError(t, os.Chdir(dir))
389+
t.Cleanup(func() { _ = os.Chdir(origWd) })
390+
391+
got := captureStderr(func() {
392+
assert.Equal(t, 0, runMergeDriverInstall(nil))
393+
})
394+
assert.Contains(t, got, "merge driver 'mdsmith' installed")
395+
// Make sure the snippet print landed too.
396+
assert.Contains(t, got, "git-hook-sync: true")
397+
}
398+
302399
// --- resolveInstalledBinary ---
303400

304401
func TestResolveInstalledBinary_NonTemporaryExe(t *testing.T) {
@@ -704,7 +801,7 @@ func TestResolveHooksDir_DefaultGitRepo(t *testing.T) {
704801
// Derive expected path from git itself so the test is resilient
705802
// against a global core.hooksPath set in the developer's git config.
706803
dir := t.TempDir()
707-
require.NoError(t, exec.Command("git", "init", dir).Run())
804+
initTestRepo(t, dir)
708805
out, err := exec.Command("git", "-C", dir, "rev-parse", "--git-path", "hooks").Output()
709806
require.NoError(t, err)
710807
expected := strings.TrimSpace(string(out))
@@ -717,7 +814,7 @@ func TestResolveHooksDir_DefaultGitRepo(t *testing.T) {
717814

718815
func TestResolveHooksDir_CustomRelativeHooksPath(t *testing.T) {
719816
dir := t.TempDir()
720-
require.NoError(t, exec.Command("git", "init", dir).Run())
817+
initTestRepo(t, dir)
721818
require.NoError(t, exec.Command("git", "-C", dir, "config",
722819
"core.hooksPath", "custom-hooks").Run())
723820
got := resolveHooksDir(dir)
@@ -726,7 +823,7 @@ func TestResolveHooksDir_CustomRelativeHooksPath(t *testing.T) {
726823

727824
func TestResolveHooksDir_CustomAbsoluteHooksPath(t *testing.T) {
728825
dir := t.TempDir()
729-
require.NoError(t, exec.Command("git", "init", dir).Run())
826+
initTestRepo(t, dir)
730827
absPath := filepath.Join(dir, "abs-hooks")
731828
require.NoError(t, exec.Command("git", "-C", dir, "config",
732829
"core.hooksPath", absPath).Run())

0 commit comments

Comments
 (0)