Address Copilot round 3: format-flag UX, terminator check, dead error paths

- runHelpPatterns now treats `-f` without a value as exit-2 usage error,
  and rejects unexpected positional arguments. Previous behavior fell
  back to text output silently for both cases.
- parseFrontMatter explicitly detects a missing closing `---` and any
  scanner error, returning a clear "unterminated front matter" error
  instead of silently scanning past the front-matter region.
- Replace the unreachable rules.ListRules error branches in
  cachedRuleInfo, handleRulePatterns, and runHelpPatterns with comments
  noting why the embedded-FS read cannot fail at runtime. showRule now
  delegates to a new ruledocs.LookupRuleInfo helper that consolidates
  the list-and-find step into a single error path.
- Drop the unreachable json.Encode error branch in runHelpPatterns
  (encoding plain string/bool fields cannot fail).
- Add tests for the new error UX, the unterminated-front-matter
  detection, and LookupRuleInfo's success and failure paths.

Author: claude

cmd/mdsmith/main.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"path/filepath"
 	"runtime/debug"
-	"strings"
 
 	flag "github.com/spf13/pflag"
 
@@ -1070,7 +1069,15 @@ func runHelp(args []string) int {
 
 func runHelpPatterns(args []string) int {
 	format := "text"
-	if len(args) >= 2 && (args[0] == "-f" || args[0] == "--format") {
+	if len(args) > 0 {
+		if args[0] != "-f" && args[0] != "--format" {
+			fmt.Fprintf(os.Stderr, "mdsmith: help patterns: unexpected argument %q\n", args[0])
+			return 2
+		}
+		if len(args) < 2 {
+			fmt.Fprintf(os.Stderr, "mdsmith: help patterns: %s requires a value (text or json)\n", args[0])
+			return 2
+		}
 		format = args[1]
 	}
 	switch format {
@@ -1079,11 +1086,9 @@ func runHelpPatterns(args []string) int {
 		fmt.Fprintf(os.Stderr, "mdsmith: help patterns: unknown format %q (valid: text, json)\n", format)
 		return 2
 	}
-	rules, err := ruledocs.ListRules()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
-		return 2
-	}
+	// ruledocs.ListRules reads an embedded FS, so its error is unreachable
+	// in a correctly built binary. Treat any failure as an empty rule list.
+	rules, _ := ruledocs.ListRules()
 	type rec struct {
 		ID            string `json:"id"`
 		Name          string `json:"name"`
@@ -1107,10 +1112,8 @@ func runHelpPatterns(args []string) int {
 	if format == "json" {
 		enc := json.NewEncoder(os.Stdout)
 		enc.SetIndent("", "  ")
-		if err := enc.Encode(items); err != nil {
-			fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
-			return 2
-		}
+		// items is a slice of plain string/bool fields, so encoding cannot fail.
+		_ = enc.Encode(items)
 		return 0
 	}
 	for _, it := range items {
@@ -1249,25 +1252,13 @@ func listAllRules() int {
 }
 
 func showRule(query string) int {
-	rules, err := ruledocs.ListRules()
+	info, err := ruledocs.LookupRuleInfo(query)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
 		return 2
 	}
-	var chosen *ruledocs.RuleInfo
-	q := strings.ToUpper(query)
-	for i := range rules {
-		if strings.ToUpper(rules[i].ID) == q || rules[i].Name == query {
-			chosen = &rules[i]
-			break
-		}
-	}
-	if chosen == nil {
-		fmt.Fprintf(os.Stderr, "mdsmith: unknown rule %q\n", query)
-		return 2
-	}
-	content := ruledocs.StripFrontMatter(chosen.Content)
-	if m := chosen.Maintainability; m != nil {
+	content := ruledocs.StripFrontMatter(info.Content)
+	if m := info.Maintainability; m != nil {
 		content += "\n\n## Maintainability pattern\n\n"
 		content += fmt.Sprintf("- Signal: %s\n- Fix: %s\n- For diagnostic: %t\n",
 			m.Signal, m.Fix, m.ForDiagnostic)

cmd/mdsmith/main_unit_test.go

@@ -734,6 +734,28 @@ func TestRunHelpPatterns_UnknownFormat_ExitsTwo(t *testing.T) {
 	assert.Contains(t, stderr, "jsno")
 }
 
+func TestRunHelpPatterns_FormatFlagWithoutValue_ExitsTwo(t *testing.T) {
+	var stderr string
+	captureStdout(func() {
+		stderr = captureStderr(func() {
+			code := runHelpPatterns([]string{"-f"})
+			assert.Equal(t, 2, code)
+		})
+	})
+	assert.Contains(t, stderr, "requires a value")
+}
+
+func TestRunHelpPatterns_UnexpectedArg_ExitsTwo(t *testing.T) {
+	var stderr string
+	captureStdout(func() {
+		stderr = captureStderr(func() {
+			code := runHelpPatterns([]string{"garbage"})
+			assert.Equal(t, 2, code)
+		})
+	})
+	assert.Contains(t, stderr, "unexpected argument")
+}
+
 func TestRunHelp_PatternsTopicDispatches(t *testing.T) {
 	out := captureStdout(func() {
 		code := runHelp([]string{"patterns", "-f", "json"})

internal/lsp/hover.go

@@ -142,13 +142,12 @@ func ruleHoverContent(d Diagnostic) string {
 // cachedRuleInfo returns the rule metadata for a given diagnostic code, or
 // (zero, false) when not found. Content is already stripped of front matter.
 // The first call loads all embedded rule READMEs; later calls are O(1) lookups.
+// rules.ListRules can only fail if the embedded FS itself is corrupt, in
+// which case the cache stays empty and every lookup returns false — the same
+// behavior as an unknown code.
 func cachedRuleInfo(code string) (rules.RuleInfo, bool) {
 	ruleInfoCache.Do(func() {
-		all, err := rules.ListRules()
-		if err != nil {
-			ruleInfoCache.infos = map[string]rules.RuleInfo{}
-			return
-		}
+		all, _ := rules.ListRules()
 		m := make(map[string]rules.RuleInfo, len(all))
 		for _, r := range all {
 			r.Content = rules.StripFrontMatter(r.Content)

internal/lsp/patterns.go

@@ -1,8 +1,6 @@
 package lsp
 
 import (
-	"fmt"
-
 	"github.com/jeduden/mdsmith/internal/rules"
 )
 
@@ -14,12 +12,13 @@ type rulePattern struct {
 	ForDiagnostic bool   `json:"for-diagnostic"`
 }
 
+// handleRulePatterns serves the `mdsmith/rulePatterns` LSP request. It returns
+// every rule with a non-null maintainability block in the same shape as
+// `mdsmith help patterns -f json`. rules.ListRules reads an embedded FS that
+// cannot fail at runtime, so any error degrades silently to an empty list —
+// the same response shape as a workspace where every rule is `maintainability: null`.
 func (s *Server) handleRulePatterns(msg *requestMessage) {
-	all, err := rules.ListRules()
-	if err != nil {
-		_ = s.t.writeError(msg.ID, codeInternalError, fmt.Sprintf("listing rules: %v", err))
-		return
-	}
+	all, _ := rules.ListRules()
 	out := make([]rulePattern, 0)
 	for _, r := range all {
 		if r.Maintainability == nil {

internal/rules/ruledocs.go

@@ -46,6 +46,27 @@ func LookupRule(query string) (string, error) {
 	return lookupRuleFromFS(rulesFS, query)
 }
 
+// LookupRuleInfo finds a rule by ID (e.g. "MDS001") or name (e.g. "line-length")
+// and returns its full metadata, including the parsed maintainability block
+// and the raw README content (front matter not stripped).
+func LookupRuleInfo(query string) (RuleInfo, error) {
+	return lookupRuleInfoFromFS(rulesFS, query)
+}
+
+func lookupRuleInfoFromFS(fsys fs.FS, query string) (RuleInfo, error) {
+	rules, err := listRulesFromFS(fsys)
+	if err != nil {
+		return RuleInfo{}, err
+	}
+	q := strings.ToUpper(query)
+	for _, r := range rules {
+		if strings.ToUpper(r.ID) == q || r.Name == query {
+			return r, nil
+		}
+	}
+	return RuleInfo{}, fmt.Errorf("unknown rule %q", query)
+}
+
 func listRulesFromFS(fsys fs.FS) ([]RuleInfo, error) {
 	entries, err := fs.ReadDir(fsys, ".")
 	if err != nil {
@@ -103,13 +124,21 @@ func parseFrontMatter(content string) (RuleInfo, error) {
 	}
 
 	var front []string
+	terminated := false
 	for scanner.Scan() {
 		line := scanner.Text()
 		if strings.TrimSpace(line) == "---" {
+			terminated = true
 			break
 		}
 		front = append(front, line)
 	}
+	if err := scanner.Err(); err != nil {
+		return RuleInfo{}, fmt.Errorf("scanning front matter: %w", err)
+	}
+	if !terminated {
+		return RuleInfo{}, fmt.Errorf("unterminated front matter")
+	}
 	var meta struct {
 		ID              string           `yaml:"id"`
 		Name            string           `yaml:"name"`

internal/rules/ruledocs_test.go

@@ -77,6 +77,33 @@ func TestLookupRule_Unknown(t *testing.T) {
 	assert.Contains(t, err.Error(), "unknown rule", "error = %q, want it to contain 'unknown rule'", err.Error())
 }
 
+func TestLookupRuleInfo_ByID(t *testing.T) {
+	info, err := LookupRuleInfo("MDS019")
+	require.NoError(t, err)
+	assert.Equal(t, "MDS019", info.ID)
+	assert.Equal(t, "catalog", info.Name)
+	require.NotNil(t, info.Maintainability)
+	assert.NotEmpty(t, info.Maintainability.Signal)
+}
+
+func TestLookupRuleInfo_ByName(t *testing.T) {
+	info, err := LookupRuleInfo("line-length")
+	require.NoError(t, err)
+	assert.Equal(t, "MDS001", info.ID)
+	assert.Nil(t, info.Maintainability)
+}
+
+func TestLookupRuleInfo_Unknown(t *testing.T) {
+	_, err := LookupRuleInfo("MDSXXX")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "unknown rule")
+}
+
+func TestLookupRuleInfoFromFS_PropagatesReadDirError(t *testing.T) {
+	_, err := lookupRuleInfoFromFS(errFS{}, "anything")
+	require.Error(t, err)
+}
+
 func TestListRulesFromFS_SkipsBadFrontMatter(t *testing.T) {
 	fsys := fstest.MapFS{
 		"good/README.md": &fstest.MapFile{
@@ -284,6 +311,20 @@ func TestParseFrontMatter_FoldsBlockScalarDescription(t *testing.T) {
 	assert.NotContains(t, info.Description, ">-")
 }
 
+// TestParseFrontMatter_UnterminatedFrontMatter verifies that a front matter
+// block without a closing `---` line fails with a clear error instead of
+// silently treating the rest of the file as YAML.
+func TestParseFrontMatter_UnterminatedFrontMatter(t *testing.T) {
+	content := "---\n" +
+		"id: MDS999\n" +
+		"name: example\n" +
+		"status: ready\n" +
+		"# body without closing delimiter\n"
+	_, err := parseFrontMatter(content)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "unterminated front matter")
+}
+
 // TestParseFrontMatter_RejectsYAMLAliases verifies that the safe-YAML wrapper
 // rejects anchor/alias usage in rule README front matter rather than silently
 // expanding aliases.