Increase test coverage to 95% and fix merge-queue merge driver (plan 85)

.github/workflows/merge-queue.yml

@@ -25,6 +25,18 @@ jobs:
           fetch-depth: 0
           token: ${{ secrets.MERGE_QUEUE_TOKEN }}
 
+      - name: Install mdsmith merge driver
+        env:
+          MDSMITH_VERSION: v0.5.0
+          MDSMITH_SHA256: 87519781aa7b5ab147d5ab1d75d4e0a1c6213479110055972a21025b537ce171
+        run: |
+          curl -fsSL "https://github.com/jeduden/mdsmith/releases/download/${MDSMITH_VERSION}/mdsmith-linux-amd64" \
+            -o "$RUNNER_TEMP/mdsmith"
+          echo "${MDSMITH_SHA256}  $RUNNER_TEMP/mdsmith" | sha256sum -c
+          chmod +x "$RUNNER_TEMP/mdsmith"
+          "$RUNNER_TEMP/mdsmith" merge-driver install
+          echo "$RUNNER_TEMP" >> "$GITHUB_PATH"
+
       - uses: jeduden/merge-queue-action@3be8077b142e4057d2fc097635d1ab6ada2bbbf5 # v0.7.1
         with:
           token: ${{ secrets.MERGE_QUEUE_TOKEN }}

.gitignore

@@ -5,3 +5,6 @@ cover.out
 /.tmp/
 docs/research/conciseness/spikes/wasm-embedded-inference/classifier.wasm
 internal/rules/concisenessscoring/wasmclassifier/classifier.wasm
+e2e-cover/
+unit.cov
+merged.cov

PLAN.md

@@ -22,7 +22,7 @@ footer: |
 | 78  | ✅     | [Query subcommand for front-matter filtering](plan/78_query-command.md)                              |
 | 83  | ✅     | [Security hardening batch](plan/83_security-hardening-batch.md)                                      |
 | 84  | ✅     | [Symlink default-deny for file discovery](plan/84_symlink-default-deny.md)                           |
-| 85  | 🔳     | [Increase test coverage to 95% by extracting shared rule helpers](plan/85_coverage-to-95-percent.md) |
+| 85  | ✅     | [Increase test coverage to 95% by extracting shared rule helpers](plan/85_coverage-to-95-percent.md) |
 | 86  | ✅     | [Markdown flavor validation](plan/86_markdown-flavor-validation.md)                                  |
 | 89  | ✅     | [TOC generator directive and MDS035 auto-fix](plan/89_toc-generator-directive.md)                    |
 | 90  | ✅     | [Isolate corpus test git config from host signing](plan/90_corpus-test-git-config-isolation.md)      |

cmd/mdsmith/mergedriver.go

@@ -395,9 +395,15 @@ func runMergeDriverInstall(args []string) int {
 }
 
 // registerMergeDriver writes the merge.mdsmith.* keys to local
-// git config.
+// git config. It uses the absolute path of the current executable
+// so the driver works regardless of whether the install directory
+// is in PATH.
 func registerMergeDriver() error {
-	driver := "mdsmith merge-driver run %O %A %B %P"
+	exe, err := resolveInstalledBinary()
+	if err != nil {
+		return fmt.Errorf("cannot locate mdsmith binary: %w", err)
+	}
+	driver := exe + " merge-driver run %O %A %B %P"
 	cmds := [][]string{
 		{"git", "config", "merge.mdsmith.name",
 			"mdsmith section-aware Markdown merge"},
@@ -411,6 +417,51 @@ func registerMergeDriver() error {
 	return nil
 }
 
+// resolveInstalledBinary returns the absolute path to the mdsmith
+// binary to use as the git merge driver. It prefers the current
+// executable when it lives outside the OS temp directory (i.e. it
+// was installed via "go install" or a release download). When the
+// current executable is a transient "go run" binary it falls back
+// to searching PATH and then $GOPATH/bin.
+func resolveInstalledBinary() (string, error) {
+	if exe, err := os.Executable(); err == nil {
+		if !isTemporaryBinary(exe) {
+			return filepath.Clean(exe), nil
+		}
+	}
+	// Transient go-run binary — try PATH first, then $GOPATH/bin.
+	if p, err := exec.LookPath("mdsmith"); err == nil {
+		return p, nil
+	}
+	gopath, err := goEnvPath()
+	if err == nil {
+		candidate := filepath.Join(gopath, "bin", "mdsmith")
+		if _, err := os.Stat(candidate); err == nil {
+			return candidate, nil
+		}
+	}
+	return "", fmt.Errorf(
+		"mdsmith not found in PATH or $GOPATH/bin; " +
+			"run \"go install ./cmd/mdsmith\" first",
+	)
+}
+
+// isTemporaryBinary reports whether path looks like a transient
+// binary created by "go run" (lives under the OS temp directory).
+func isTemporaryBinary(path string) bool {
+	tmp := os.TempDir()
+	return strings.HasPrefix(filepath.Clean(path), filepath.Clean(tmp))
+}
+
+// goEnvPath returns the value of GOPATH by running "go env GOPATH".
+func goEnvPath() (string, error) {
+	out, err := exec.Command("go", "env", "GOPATH").Output()
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(string(out)), nil
+}
+
 // ensureGitattributes reads .gitattributes, adds any missing
 // merge driver entries for the given files, and writes it back.
 func ensureGitattributes(path string, files []string) error {

internal/archetype/gensection/engine_test.go

@@ -523,3 +523,21 @@ func TestParseColumnConfig_DefaultWrap(t *testing.T) {
 		t.Errorf("expected default wrap 'truncate', got %q", cols["desc"].Wrap)
 	}
 }
+
+// =====================================================================
+// Phase 5: additional branch coverage
+// =====================================================================
+
+// TestEngine_Fix_SkipsOnInvalidYAML exercises the generateContent
+// `dir == nil || len(diags) > 0` branch in Fix.
+// When the YAML body is invalid, generateContent returns (_, false) and Fix skips.
+func TestEngine_Fix_SkipsOnInvalidYAML(t *testing.T) {
+	src := "<?mock\n: invalid : yaml ::: [\n?>\nold content\n<?/mock?>\n"
+	f := newTestFile(t, "test.md", src)
+	d := &mockDirective{content: "new content\n"}
+	e := NewEngine(d)
+	result := string(e.Fix(f))
+	// Fix should leave old content intact when YAML parsing fails.
+	assert.Contains(t, result, "old content", "expected old content preserved when YAML is invalid")
+	assert.NotContains(t, result, "new content")
+}

internal/archetype/gensection/parse_test.go

@@ -3,6 +3,7 @@ package gensection
 import (
 	"testing"
 
+	"github.com/jeduden/mdsmith/internal/lint"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -26,3 +27,123 @@ func TestParseYAMLBody_AcceptsClean(t *testing.T) {
 	assert.Empty(t, diags)
 	assert.Equal(t, "value", raw["key"])
 }
+
+func TestToStringSlice_AllStrings(t *testing.T) {
+	items := []any{"alpha", "beta", "gamma"}
+	result, err := toStringSlice(items)
+	require.NoError(t, err)
+	assert.Equal(t, []string{"alpha", "beta", "gamma"}, result)
+}
+
+func TestToStringSlice_Empty(t *testing.T) {
+	result, err := toStringSlice([]any{})
+	require.NoError(t, err)
+	assert.Empty(t, result)
+}
+
+func TestToStringSlice_NonStringElement(t *testing.T) {
+	items := []any{"ok", 42, "also-ok"}
+	_, err := toStringSlice(items)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "element 1")
+}
+
+// =====================================================================
+// Phase 5: additional branch coverage
+// =====================================================================
+
+// ValidateStringParams: []any with non-string element → toStringSlice error
+func TestValidateStringParams_ListWithNonStringElement(t *testing.T) {
+	rawMap := map[string]any{
+		"glob": []any{"docs/**", 42},
+	}
+	params, diags := ValidateStringParams("test.md", 1, rawMap, "MDS999", "test-rule")
+	assert.Nil(t, params)
+	require.Len(t, diags, 1)
+	assert.Contains(t, diags[0].Message, "non-string element")
+}
+
+// ValidateStringParams: []any all strings → joined with "\n"
+func TestValidateStringParams_ListAllStrings(t *testing.T) {
+	rawMap := map[string]any{
+		"glob": []any{"docs/**", "plan/**"},
+	}
+	params, diags := ValidateStringParams("test.md", 1, rawMap, "MDS999", "test-rule")
+	assert.Empty(t, diags)
+	assert.Equal(t, "docs/**\nplan/**", params["glob"])
+}
+
+// ValidateStringParams: float64 value → decimal string
+func TestValidateStringParams_Float64Value(t *testing.T) {
+	rawMap := map[string]any{
+		"min-level": float64(2),
+	}
+	params, diags := ValidateStringParams("test.md", 1, rawMap, "MDS999", "test-rule")
+	assert.Empty(t, diags)
+	assert.Equal(t, "2", params["min-level"])
+}
+
+// ValidateStringParams: non-string non-sequence default case
+func TestValidateStringParams_BoolValue(t *testing.T) {
+	rawMap := map[string]any{
+		"enabled": true,
+	}
+	params, diags := ValidateStringParams("test.md", 1, rawMap, "MDS999", "test-rule")
+	assert.Nil(t, params)
+	require.Len(t, diags, 1)
+}
+
+// ParseColumnConfig: non-map value for column → continue
+func TestParseColumnConfig_NonMapValue(t *testing.T) {
+	raw := map[string]any{
+		"col1": "not-a-map",
+		"col2": map[string]any{"max-width": 30},
+	}
+	result := ParseColumnConfig(raw)
+	require.NotNil(t, result)
+	// "col1" is skipped (not a map), "col2" is parsed.
+	_, hasCol1 := result["col1"]
+	assert.False(t, hasCol1, "non-map column should be skipped")
+	col2, hasCol2 := result["col2"]
+	assert.True(t, hasCol2)
+	assert.Equal(t, 30, col2.MaxWidth)
+}
+
+// ParseColumnConfig: float64 max-width
+func TestParseColumnConfig_Float64MaxWidth(t *testing.T) {
+	raw := map[string]any{
+		"col": map[string]any{"max-width": float64(50)},
+	}
+	result := ParseColumnConfig(raw)
+	require.NotNil(t, result)
+	col := result["col"]
+	assert.Equal(t, 50, col.MaxWidth)
+}
+
+// ExtractContent: empty range when ContentFrom > ContentTo
+func TestExtractContent_EmptyRange(t *testing.T) {
+	mp := MarkerPair{ContentFrom: 5, ContentTo: 3}
+	result := ExtractContent(nil, mp)
+	assert.Equal(t, "", result)
+}
+
+// ExtractContent: range produces no lines (ContentFrom==ContentTo but all past file)
+func TestExtractContent_EmptyLines(t *testing.T) {
+	f := &lint.File{Path: "test.md", Lines: [][]byte{}}
+	mp := MarkerPair{ContentFrom: 1, ContentTo: 1}
+	result := ExtractContent(f, mp)
+	assert.Equal(t, "", result)
+}
+
+// ExtractColumnsRaw: "columns" key exists but is not a map[string]any → return nil
+func TestExtractColumnsRaw_NonMapValue(t *testing.T) {
+	rawMap := map[string]any{
+		"columns": "not-a-map",
+		"other":   "keep",
+	}
+	result := ExtractColumnsRaw(rawMap)
+	assert.Nil(t, result, "non-map columns value should return nil")
+	// The "columns" key should have been deleted even when the type is wrong.
+	_, hasColumns := rawMap["columns"]
+	assert.False(t, hasColumns, "columns key should be deleted from rawMap")
+}

internal/config/config_test.go

@@ -1520,3 +1520,81 @@ func TestInjectArchetypeRoots_CreatesSettingsMapIfNil(t *testing.T) {
 	got := cfg.Rules["required-structure"].Settings["archetype-roots"]
 	assert.Equal(t, []any{"r"}, got)
 }
+
+// TestMergeNilLoadedWithCategories exercises copyCategories with a non-nil
+// categories map, covering the branch at merge.go copyCategories.
+func TestMergeNilLoadedWithCategories(t *testing.T) {
+	defaults := &Config{
+		Rules: map[string]RuleCfg{
+			"line-length": {Enabled: true},
+		},
+		Categories: map[string]bool{
+			"heading":    false,
+			"whitespace": true,
+		},
+	}
+	merged := Merge(defaults, nil)
+	require.NotNil(t, merged.Categories, "expected non-nil categories")
+	assert.Equal(t, false, merged.Categories["heading"])
+	assert.Equal(t, true, merged.Categories["whitespace"])
+	// Verify it's a copy, not the same map.
+	defaults.Categories["heading"] = true
+	assert.Equal(t, false, merged.Categories["heading"], "merged categories should be independent copy")
+}
+
+// =====================================================================
+// Phase 5: additional branch coverage
+// =====================================================================
+
+// TestMergeNilLoaded_CopiesExplicitRules exercises the ExplicitRules loop
+// inside copyConfig when Merge is called with loaded == nil.
+func TestMergeNilLoaded_CopiesExplicitRules(t *testing.T) {
+	defaults := &Config{
+		Rules: map[string]RuleCfg{
+			"line-length": {Enabled: true},
+		},
+		ExplicitRules: map[string]bool{
+			"line-length": true,
+		},
+	}
+	merged := Merge(defaults, nil)
+	require.NotNil(t, merged.ExplicitRules, "expected non-nil ExplicitRules")
+	assert.True(t, merged.ExplicitRules["line-length"], "expected line-length to be explicit")
+	// Verify it's a copy.
+	defaults.ExplicitRules["heading-style"] = true
+	_, hasCopy := merged.ExplicitRules["heading-style"]
+	assert.False(t, hasCopy, "merged ExplicitRules should be independent copy")
+}
+
+// TestUnmarshalYAML_MappingDecodeError exercises the mapping decode error branch.
+// This is reached when a YAML mapping node cannot be decoded into map[string]any.
+// A mapping with YAML anchors triggers the RejectYAMLAliases check, so we need
+// a different invalid mapping. In practice this branch is very hard to trigger
+// since yaml.Decode on a MappingNode rarely fails; skip if not possible to test.
+// Instead test via the "non-scalar non-mapping" fallthrough branch.
+func TestUnmarshalYAML_NonScalarNonMappingValue(t *testing.T) {
+	// YAML sequence (list) as rule config → should return "rule config must be a bool or a mapping".
+	input := "rules:\n  line-length:\n    - item1\n    - item2\n"
+	var cfg Config
+	err := yaml.Unmarshal([]byte(input), &cfg)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "rule config must be a bool or a mapping")
+}
+
+// TestTopLevelKeySet_DocumentNodeEmpty exercises the
+// `node.Kind != yaml.DocumentNode || len(node.Content) == 0` branch
+// by passing YAML that produces a document node with empty content.
+func TestTopLevelKeySet_DocumentNodeEmpty(t *testing.T) {
+	// An empty YAML document produces a DocumentNode with no content.
+	result := topLevelKeySet([]byte(""))
+	assert.Empty(t, result, "empty YAML should return empty key set")
+}
+
+// TestTopLevelKeySet_NullDocument exercises the mapping.Kind != yaml.MappingNode
+// branch. yaml.Unmarshal("null") produces a DocumentNode whose first child is
+// a ScalarNode, so the mapping check fails and an empty set is returned.
+func TestTopLevelKeySet_NullDocument(t *testing.T) {
+	result := topLevelKeySet([]byte("null\n"))
+	// ScalarNode child means no keys to extract.
+	assert.Empty(t, result)
+}