Skip to content

Commit 5a2b5fb

Browse files
committed
test(cmd/mdsmith): add dedicated unit tests for runCheck/runFix
The 2026-08-02 architecture audit flagged runCheck and runFix — the check and fix CLI subcommand entry points — as missing dedicated in-process unit tests, only reachable via binary-spawn e2e tests. audit-checklist.md promotes a missing test on a CLI subcommand entry to blocker severity. Adds TestRunCheck_*/TestRunFix_* covering the flag-error, stdin, explicit-files, and config-discovered branches, matching the TestRunInit_* precedent already used for the init subcommand. Also records the full audit sweep in architecture-audit.md and files plans for the two structural tax findings (query-subcommand placement, internal/githooks package split). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AbFuwvqqq2fBiYWa3ubap
1 parent 2ab4b29 commit 5a2b5fb

5 files changed

Lines changed: 395 additions & 3 deletions

File tree

PLAN.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,4 +259,6 @@ footer: |
259259
| 2607191918 || haiku | [Deduplicate isClaimed between internal/schema and requiredstructure](plan/2607191918_arch-fix-isclaimed-dedup.md) |
260260
| 2607242010 | 🔲 | sonnet | [MDS072 external-link-check: SSRF and egress hardening](plan/2607242010_mds072-ssrf-network-hardening.md) |
261261
| 2607242011 | 🔲 | haiku | [Security hardening batch — 2026-07-24](plan/2607242011_security-hardening-batch-2026-07-24.md) |
262+
| 2608021915 | 🔲 | haiku | [Move list-query subcommand logic out of cmd/mdsmith/main.go into query.go](plan/2608021915_arch-fix-query-subcommand-placement.md) |
263+
| 2608021916 | 🔲 | sonnet | [Split internal/githooks by responsibility](plan/2608021916_arch-fix-githooks-package-split.md) |
262264
<?/catalog?>

cmd/mdsmith/main_unit_test.go

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,136 @@ func TestFixDiscovered_BadMaxInputSize_ExitsTwo(t *testing.T) {
811811
assert.Contains(t, stderr, "max-input-size")
812812
}
813813

814+
// --- runCheck ---
815+
//
816+
// runCheck is the "check" subcommand entry dispatched from dispatch() in
817+
// main.go; go.md/audit-checklist.md name a CLI subcommand entry as a
818+
// public surface, so a missing dedicated test here is a blocker, not tax.
819+
// These exercise the three branches it routes to (flag error, stdin,
820+
// explicit files, config-discovered files) in-process, mirroring the
821+
// TestRunInit_* pattern in init_unit_test.go rather than relying solely
822+
// on the binary-spawn e2e tests in e2e_coverage_test.go.
823+
824+
func TestRunCheck_UnknownFlag_ExitsTwo(t *testing.T) {
825+
var code int
826+
stderr := captureStderr(func() {
827+
code = runCheck([]string{"--definitely-not-a-flag"})
828+
})
829+
assert.Equal(t, 2, code)
830+
assert.Contains(t, stderr, "unknown flag")
831+
}
832+
833+
func TestRunCheck_Stdin_ChecksSource(t *testing.T) {
834+
t.Chdir(t.TempDir())
835+
836+
oldStdin := os.Stdin
837+
r, w, err := os.Pipe()
838+
require.NoError(t, err)
839+
os.Stdin = r
840+
defer func() { os.Stdin = oldStdin }()
841+
go func() {
842+
_, _ = w.WriteString("# Title\n\nContent here.\n")
843+
_ = w.Close()
844+
}()
845+
846+
var code int
847+
captureStderr(func() {
848+
code = runCheck([]string{"-"})
849+
})
850+
assert.Equal(t, 0, code)
851+
}
852+
853+
func TestRunCheck_Files_ExitsOneOnDiagnostics(t *testing.T) {
854+
dir := t.TempDir()
855+
t.Chdir(dir)
856+
require.NoError(t, os.WriteFile(filepath.Join(dir, "dirty.md"),
857+
[]byte("# Title\n\nHello \n"), 0o644))
858+
859+
var code int
860+
stderr := captureStderr(func() {
861+
code = runCheck([]string{"dirty.md"})
862+
})
863+
assert.Equal(t, 1, code)
864+
assert.Contains(t, stderr, "dirty.md")
865+
}
866+
867+
func TestRunCheck_Discovered_ChecksConfiguredFiles(t *testing.T) {
868+
dir := t.TempDir()
869+
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
870+
[]byte("files: [\"**/*.md\"]\n"), 0o644))
871+
require.NoError(t, os.WriteFile(filepath.Join(dir, "test.md"),
872+
[]byte("# Title\n\nContent here.\n"), 0o644))
873+
t.Chdir(dir)
874+
875+
var code int
876+
captureStderr(func() {
877+
code = runCheck(nil)
878+
})
879+
assert.Equal(t, 0, code)
880+
}
881+
882+
// --- runFix ---
883+
//
884+
// runFix is the "fix" subcommand entry, the same public-surface tier as
885+
// runCheck above. These cover the stdin-rejection branch and the two
886+
// file-resolution branches (explicit files, config-discovered files) that
887+
// runCheck's tests exercise, plus the disk write that runCheck has no
888+
// equivalent of.
889+
890+
func TestRunFix_UnknownFlag_ExitsTwo(t *testing.T) {
891+
var code int
892+
stderr := captureStderr(func() {
893+
code = runFix([]string{"--definitely-not-a-flag"})
894+
})
895+
assert.Equal(t, 2, code)
896+
assert.Contains(t, stderr, "unknown flag")
897+
}
898+
899+
func TestRunFix_StdinArg_ExitsTwo(t *testing.T) {
900+
var code int
901+
stderr := captureStderr(func() {
902+
code = runFix([]string{"-"})
903+
})
904+
assert.Equal(t, 2, code)
905+
assert.Contains(t, stderr, "cannot fix stdin in place")
906+
}
907+
908+
func TestRunFix_Files_FixesGivenFile(t *testing.T) {
909+
dir := t.TempDir()
910+
t.Chdir(dir)
911+
path := filepath.Join(dir, "fixme.md")
912+
require.NoError(t, os.WriteFile(path, []byte("# Title\n\nHello \n"), 0o644))
913+
914+
var code int
915+
captureStderr(func() {
916+
code = runFix([]string{"fixme.md"})
917+
})
918+
assert.Equal(t, 0, code)
919+
920+
fixed, err := os.ReadFile(path)
921+
require.NoError(t, err)
922+
assert.Equal(t, "# Title\n\nHello\n", string(fixed))
923+
}
924+
925+
func TestRunFix_Discovered_FixesConfiguredFiles(t *testing.T) {
926+
dir := t.TempDir()
927+
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
928+
[]byte("files: [\"**/*.md\"]\n"), 0o644))
929+
path := filepath.Join(dir, "fixme.md")
930+
require.NoError(t, os.WriteFile(path, []byte("# Title\n\nHello \n"), 0o644))
931+
t.Chdir(dir)
932+
933+
var code int
934+
captureStderr(func() {
935+
code = runFix(nil)
936+
})
937+
assert.Equal(t, 0, code)
938+
939+
fixed, err := os.ReadFile(path)
940+
require.NoError(t, err)
941+
assert.Equal(t, "# Title\n\nHello\n", string(fixed))
942+
}
943+
814944
// --- printErrors ---
815945

816946
func TestPrintErrors_Empty_NoOutput(t *testing.T) {

docs/development/architecture-audit.md

Lines changed: 103 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ summary: >-
66
solid-architecture skill (audit mode)
77
appends here; blockers are also filed as
88
plans.
9-
audit-from: 6680ff53d440337d780e62d19d6eb65a4e1b6e2c
9+
audit-from: 2ab4b2949e9420b2eb73f4239091fd647c4b5e80
1010
---
1111
# Architecture audit log
1212

@@ -17,6 +17,108 @@ to [the archive](architecture-audit-archive.md)
1717
this cycle to stay under the file-length budget;
1818
every finding there is resolved.
1919

20+
## Audit 2026-08-02 (range: 6680ff5..2ab4b29)
21+
22+
134 touched files. Notable new surfaces: MDS073
23+
slide-structure and MDS060 occurrence. MDS073 was
24+
renamed from MDS072; no leftover duplication was
25+
found. Also new: foreign-managed regions and
26+
user-extensible wordlists.
27+
28+
No rule-to-rule imports. No reverse-layer imports. No
29+
Liskov breaks. The prior cycle's `isClaimed` dedup
30+
(plan 2607191918) is confirmed resolved — `schema.IsClaimed`
31+
is now exported and `requiredstructure` calls it directly.
32+
33+
### blockers (2026-08-02)
34+
35+
- `cmd/mdsmith/check.go`'s `runCheck` and
36+
`cmd/mdsmith/fix.go`'s `runFix` — the `check` and `fix`
37+
CLI subcommand entry points — had no dedicated in-process
38+
unit test; only binary-spawn e2e tests
39+
(`internal/integration`, `cmd/mdsmith/e2e_*_test.go`)
40+
exercised them.
41+
[audit-checklist.md][audit-checklist]: "blocker if the
42+
function is on a public surface ... a CLI subcommand
43+
entry." Fixed: added `TestRunCheck_UnknownFlag_ExitsTwo`,
44+
`TestRunCheck_Stdin_ChecksSource`,
45+
`TestRunCheck_Files_ExitsOneOnDiagnostics`,
46+
`TestRunCheck_Discovered_ChecksConfiguredFiles`,
47+
`TestRunFix_UnknownFlag_ExitsTwo`,
48+
`TestRunFix_StdinArg_ExitsTwo`,
49+
`TestRunFix_Files_FixesGivenFile`, and
50+
`TestRunFix_Discovered_FixesConfiguredFiles` to
51+
`cmd/mdsmith/main_unit_test.go`, matching the
52+
`TestRunInit_*` precedent from the 2026-07-19 cycle.
53+
`go test ./cmd/mdsmith/...` and
54+
`go tool golangci-lint run` are green.
55+
56+
### tax (2026-08-02)
57+
58+
- `cmd/mdsmith/main.go` still carries the `list query`
59+
subcommand's full domain logic (`parseQueryFlags`,
60+
`runQuery`, `queryFiles`, `readFrontMatterRaw`) instead
61+
of a dedicated file, even though `list.go` dispatches to
62+
it the same way it dispatches to `backlinks.go`.
63+
[go.md][go] "Clean wiring in `cmd/mdsmith`" —
64+
[plan/2608021915][2608021915].
65+
- `internal/githooks`'s package doc comment joins three
66+
responsibilities with "and" (hook-script generation,
67+
`.gitattributes` I/O, directive-file discovery) —
68+
[go.md][go] refactor-moves: "Split a package by
69+
question" — [plan/2608021916][2608021916].
70+
- `internal/lint/files.go`'s CLI-path/glob resolution
71+
overlaps the question `internal/discovery` already
72+
answers for config-glob-driven discovery. No plan filed
73+
this cycle; flagged for the next package-boundary pass.
74+
- A broad set of unexported, branching helper functions
75+
across the touched rule packages
76+
(`requiredstructure`, `crossfilereferenceintegrity`,
77+
`noreferencestyle`, `include`, `linkstyle`,
78+
`slidevstructure`, `occurrence`, `tablefmt`,
79+
`tocdirective`, `githooksync`, `build`) and
80+
`cmd/mdsmith` (`checkFiles`, `checkBatchOptions`,
81+
`batchMaxBytes`, `parseFixFlags`, `setFixUsage`,
82+
`fixFiles`, `readStdinLimited`, `regenDirectiveNames`)
83+
lack a dedicated `TestFuncName` symbol per
84+
[tests.md][tests], though each is exercised
85+
transitively through its caller's scenario tests. None
86+
sit on a public surface by themselves, so tax rather
87+
than blocker.
88+
- Trivial one-line accessors (`FixTitle` across several
89+
rule packages, plus a few `rule.Rule` capability
90+
predicates) carry only an "implements rule.X" comment,
91+
not the exemption statement [tests.md][tests] requires
92+
to distinguish "no test by design" from "no test,
93+
forgotten."
94+
- Similar untested-but-covered helper clusters exist in
95+
`internal/lsp/server_codeaction.go`,
96+
`internal/config/merge.go`, and `internal/fix/fix.go`.
97+
98+
[tests]: architecture/tests.md
99+
[go]: architecture/go.md
100+
[audit-checklist]: architecture/audit-checklist.md
101+
[2608021915]: ../../plan/2608021915_arch-fix-query-subcommand-placement.md
102+
[2608021916]: ../../plan/2608021916_arch-fix-githooks-package-split.md
103+
104+
### nice-to-have (2026-08-02)
105+
106+
- Several tests covering the flagged helper clusters use
107+
scenario names (e.g. `TestDriftParts_ResolvesHooksDirOnce`)
108+
rather than the literal `TestReceiver_Foo` binding —
109+
coverage exists, only the naming convention drifts.
110+
- `stagingHelperShellFunc` (`internal/githooks/githooks.go`)
111+
contains "Helper" in its name — go.md flags this as a
112+
smell, though the function is well-scoped. Rename on next
113+
touch.
114+
- `Override.Patterns()` / `KindAssignmentEntry.Patterns()`
115+
(`internal/config/config.go`) are two-line branching
116+
methods exercised only inside larger config tests, not by
117+
a dedicated `Test*`. Low regression risk.
118+
- `parseCheckFlags` / `parseFixFlags` cross go.md's ~50-line
119+
guidance for `cmd/mdsmith` handlers, but the body is pure
120+
flag-registration boilerplate, not domain logic.
121+
20122
## Audit 2026-07-19 (range: 834b560..6680ff5)
21123

22124
89 touched files. Notable new surfaces: SARIF output,
@@ -86,8 +188,6 @@ None.
86188
to an interface... once two rules needed the same shape"
87189
[plan/2607191918][2607191918].
88190

89-
[tests]: architecture/tests.md
90-
[go]: architecture/go.md
91191
[2607191917]: ../../plan/2607191917_arch-fix-printinitcatalog-unit-test.md
92192
[2607191918]: ../../plan/2607191918_arch-fix-isclaimed-dedup.md
93193

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
id: 2608021915
3+
title: >-
4+
Move list-query subcommand logic out of
5+
cmd/mdsmith/main.go into query.go
6+
status: "🔲"
7+
model: haiku
8+
summary: >-
9+
parseQueryFlags, runQuery, queryFiles, and
10+
readFrontMatterRaw implement the `mdsmith list
11+
query` subcommand but live in main.go instead of a
12+
dedicated per-subcommand file. Flagged by the
13+
2026-08-02 audit.
14+
---
15+
# Move list-query subcommand logic out of cmd/mdsmith/main.go into query.go
16+
17+
## Goal
18+
19+
Relocate the `list query` subcommand's implementation
20+
into its own file. That keeps `main.go` as
21+
dispatch-and-glue, matching the pattern already used
22+
for every sibling subcommand.
23+
24+
## Background
25+
26+
The 2026-08-02 audit (see
27+
[the audit log](../docs/development/architecture-audit.md))
28+
found this placement gap:
29+
30+
- [main.go](../cmd/mdsmith/main.go) defines
31+
`parseQueryFlags`, `runQuery`, `queryFiles`, and
32+
`readFrontMatterRaw` — the full domain logic for the
33+
`list query` subcommand (CUE-expression matching
34+
against front matter, its own file walk).
35+
- [list.go](../cmd/mdsmith/list.go)'s `runList`
36+
dispatches `case "query": return runQuery(...)`, the
37+
same way it dispatches `case "backlinks":` to
38+
`runBacklinks`, which lives in its own
39+
[backlinks.go](../cmd/mdsmith/backlinks.go).
40+
- [go.md](../docs/development/architecture/go.md)'s
41+
"Clean wiring in `cmd/mdsmith`" section: domain logic
42+
belongs in `pkg/mdsmith`, `internal/engine`, or a
43+
subcommand's own file — not in `main.go`.
44+
- The 2026-07-19 audit already applied this exact move
45+
once, relocating the `init` subcommand's logic out of
46+
`main.go` into
47+
[init.go](../cmd/mdsmith/init.go); this plan repeats
48+
that move for `query`.
49+
50+
## Tasks
51+
52+
1. Create `cmd/mdsmith/query.go` and move
53+
`parseQueryFlags`, `runQuery`, `queryFiles`, and
54+
`readFrontMatterRaw` into it, unchanged.
55+
2. Move the query-only tests covering those functions
56+
out of `main_unit_test.go` into a new
57+
`cmd/mdsmith/query_unit_test.go`, following the
58+
`init_unit_test.go` precedent.
59+
3. Confirm `list.go`'s `runList` still compiles against
60+
the relocated `runQuery` with no signature change.
61+
4. `go build ./...` passes.
62+
5. `go test ./cmd/mdsmith/...` passes.
63+
6. `go tool -modfile=tools/go.mod golangci-lint run`
64+
reports no issues.
65+
66+
## Acceptance Criteria
67+
68+
- [ ] `parseQueryFlags`, `runQuery`, `queryFiles`, and
69+
`readFrontMatterRaw` no longer appear in
70+
`cmd/mdsmith/main.go`.
71+
- [ ] `cmd/mdsmith/query.go` holds the relocated code
72+
with no behavior change.
73+
- [ ] `go test ./...` is green.
74+
- [ ] `mdsmith check .` is green.

0 commit comments

Comments
 (0)