Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,4 +259,6 @@ footer: |
| 2607191918 | ✅ | haiku | [Deduplicate isClaimed between internal/schema and requiredstructure](plan/2607191918_arch-fix-isclaimed-dedup.md) |
| 2607242010 | 🔲 | sonnet | [MDS072 external-link-check: SSRF and egress hardening](plan/2607242010_mds072-ssrf-network-hardening.md) |
| 2607242011 | 🔲 | haiku | [Security hardening batch — 2026-07-24](plan/2607242011_security-hardening-batch-2026-07-24.md) |
| 2608021915 | 🔲 | haiku | [Move list-query subcommand logic out of cmd/mdsmith/main.go into query.go](plan/2608021915_arch-fix-query-subcommand-placement.md) |
| 2608021916 | 🔲 | sonnet | [Split internal/githooks by responsibility](plan/2608021916_arch-fix-githooks-package-split.md) |
<?/catalog?>
137 changes: 137 additions & 0 deletions cmd/mdsmith/main_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,143 @@ func TestFixDiscovered_BadMaxInputSize_ExitsTwo(t *testing.T) {
assert.Contains(t, stderr, "max-input-size")
}

// --- runCheck ---
//
// runCheck is the "check" subcommand entry dispatched from dispatch() in
// main.go; go.md/audit-checklist.md name a CLI subcommand entry as a
// public surface, so a missing dedicated test here is a blocker, not tax.
// These exercise the three branches it routes to (flag error, stdin,
// explicit files, config-discovered files) in-process, mirroring the
// TestRunInit_* pattern in init_unit_test.go rather than relying solely
// on the binary-spawn e2e tests in e2e_coverage_test.go.

func TestRunCheck_UnknownFlag_ExitsTwo(t *testing.T) {
var code int
stderr := captureStderr(func() {
code = runCheck([]string{"--definitely-not-a-flag"})
})
assert.Equal(t, 2, code)
assert.Contains(t, stderr, "unknown flag")
}

func TestRunCheck_Stdin_ChecksSource(t *testing.T) {
t.Chdir(t.TempDir())

Comment thread
jeduden marked this conversation as resolved.
oldStdin := os.Stdin
r, w, err := os.Pipe()
require.NoError(t, err)
defer r.Close() //nolint:errcheck // best-effort close on read-only pipe end
os.Stdin = r
defer func() { os.Stdin = oldStdin }()
go func() {
// Trailing spaces trigger a diagnostic, so a passing exit code
// alone can't mask a routing bug that skips reading stdin.
_, _ = w.WriteString("# Title\n\nHello \n")
_ = w.Close()
}()

var code int
stderr := captureStderr(func() {
code = runCheck([]string{"-"})
})
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "<stdin>")
}

func TestRunCheck_Files_ExitsOneOnDiagnostics(t *testing.T) {
dir := t.TempDir()
t.Chdir(dir)
require.NoError(t, os.WriteFile(filepath.Join(dir, "dirty.md"),
[]byte("# Title\n\nHello \n"), 0o644))

var code int
stderr := captureStderr(func() {
code = runCheck([]string{"dirty.md"})
})
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "dirty.md")
}

func TestRunCheck_Discovered_ChecksConfiguredFiles(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
[]byte("files: [\"**/*.md\"]\n"), 0o644))
// Trailing spaces trigger a diagnostic, so a passing exit code alone
// can't mask a discovery bug that silently finds zero files.
require.NoError(t, os.WriteFile(filepath.Join(dir, "test.md"),
[]byte("# Title\n\nHello \n"), 0o644))
t.Chdir(dir)

var code int
stderr := captureStderr(func() {
code = runCheck(nil)
})
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "test.md")
}

// --- runFix ---
//
// runFix is the "fix" subcommand entry, the same public-surface tier as
// runCheck above. These cover the stdin-rejection branch and the two
// file-resolution branches (explicit files, config-discovered files) that
// runCheck's tests exercise, plus the disk write that runCheck has no
// equivalent of.

func TestRunFix_UnknownFlag_ExitsTwo(t *testing.T) {
var code int
stderr := captureStderr(func() {
code = runFix([]string{"--definitely-not-a-flag"})
})
assert.Equal(t, 2, code)
assert.Contains(t, stderr, "unknown flag")
}

func TestRunFix_StdinArg_ExitsTwo(t *testing.T) {
var code int
stderr := captureStderr(func() {
code = runFix([]string{"-"})
})
assert.Equal(t, 2, code)
assert.Contains(t, stderr, "cannot fix stdin in place")
}

func TestRunFix_Files_FixesGivenFile(t *testing.T) {
dir := t.TempDir()
t.Chdir(dir)
path := filepath.Join(dir, "fixme.md")
require.NoError(t, os.WriteFile(path, []byte("# Title\n\nHello \n"), 0o644))

var code int
captureStderr(func() {
code = runFix([]string{"fixme.md"})
})
assert.Equal(t, 0, code)

fixed, err := os.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, "# Title\n\nHello\n", string(fixed))
}

func TestRunFix_Discovered_FixesConfiguredFiles(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
[]byte("files: [\"**/*.md\"]\n"), 0o644))
path := filepath.Join(dir, "fixme.md")
require.NoError(t, os.WriteFile(path, []byte("# Title\n\nHello \n"), 0o644))
t.Chdir(dir)

var code int
captureStderr(func() {
code = runFix(nil)
})
assert.Equal(t, 0, code)

fixed, err := os.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, "# Title\n\nHello\n", string(fixed))
}

// --- printErrors ---

func TestPrintErrors_Empty_NoOutput(t *testing.T) {
Expand Down
106 changes: 103 additions & 3 deletions docs/development/architecture-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ summary: >-
solid-architecture skill (audit mode)
appends here; blockers are also filed as
plans.
audit-from: 6680ff53d440337d780e62d19d6eb65a4e1b6e2c
audit-from: 2ab4b2949e9420b2eb73f4239091fd647c4b5e80
---
# Architecture audit log

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

## Audit 2026-08-02 (range: 6680ff5..2ab4b29)

148 touched files. Notable new surfaces: MDS073
slide-structure and MDS060 occurrence. MDS073 was
renamed from MDS072; no leftover duplication was
found. Also new: foreign-managed regions and
user-extensible wordlists.

No rule-to-rule imports. No reverse-layer imports. No
Liskov breaks. The prior cycle's `isClaimed` dedup
(plan 2607191918) is confirmed resolved — `schema.IsClaimed`
is now exported and `requiredstructure` calls it directly.

### blockers (2026-08-02)

- `cmd/mdsmith/check.go`'s `runCheck` and
`cmd/mdsmith/fix.go`'s `runFix` — the `check` and `fix`
CLI subcommand entry points — had no dedicated in-process
unit test; only binary-spawn e2e tests
(`internal/integration`, `cmd/mdsmith/e2e_*_test.go`)
exercised them.
[audit-checklist.md][audit-checklist]: "blocker if the
function is on a public surface ... a CLI subcommand
entry." Fixed: added `TestRunCheck_UnknownFlag_ExitsTwo`,
`TestRunCheck_Stdin_ChecksSource`,
`TestRunCheck_Files_ExitsOneOnDiagnostics`,
`TestRunCheck_Discovered_ChecksConfiguredFiles`,
`TestRunFix_UnknownFlag_ExitsTwo`,
`TestRunFix_StdinArg_ExitsTwo`,
`TestRunFix_Files_FixesGivenFile`, and
`TestRunFix_Discovered_FixesConfiguredFiles` to
`cmd/mdsmith/main_unit_test.go`, matching the
`TestRunInit_*` precedent from the 2026-07-19 cycle.
`go test ./cmd/mdsmith/...` and
`go tool golangci-lint run` are green.

### tax (2026-08-02)

- `cmd/mdsmith/main.go` still carries the `list query`
subcommand's full domain logic (`parseQueryFlags`,
`runQuery`, `queryFiles`, `readFrontMatterRaw`) instead
of a dedicated file, even though `list.go` dispatches to
it the same way it dispatches to `backlinks.go`.
[go.md][go] "Clean wiring in `cmd/mdsmith`" —
[plan/2608021915][2608021915].
- `internal/githooks`'s package doc comment joins three
responsibilities with "and" (hook-script generation,
`.gitattributes` I/O, directive-file discovery) —
[go.md][go] refactor-moves: "Split a package by
question" — [plan/2608021916][2608021916].
- `internal/lint/files.go`'s CLI-path/glob resolution
overlaps the question `internal/discovery` already
answers for config-glob-driven discovery. No plan filed
this cycle; flagged for the next package-boundary pass.
- A broad set of unexported, branching helper functions
across the touched rule packages
(`requiredstructure`, `crossfilereferenceintegrity`,
`noreferencestyle`, `include`, `linkstyle`,
`slidevstructure`, `occurrence`, `tablefmt`,
`tocdirective`, `githooksync`, `build`) and
`cmd/mdsmith` (`checkFiles`, `checkBatchOptions`,
`batchMaxBytes`, `parseFixFlags`, `setFixUsage`,
`fixFiles`, `readStdinLimited`, `regenDirectiveNames`)
lack a dedicated `TestFuncName` symbol per
[tests.md][tests], though each is exercised
transitively through its caller's scenario tests. None
sit on a public surface by themselves, so tax rather
than blocker.
- Trivial one-line accessors (`FixTitle` across several
rule packages, plus a few `rule.Rule` capability
predicates) carry only an "implements rule.X" comment,
not the exemption statement [tests.md][tests] requires
to distinguish "no test by design" from "no test,
forgotten."
- Similar untested-but-covered helper clusters exist in
`internal/lsp/server_codeaction.go`,
`internal/config/merge.go`, and `internal/fix/fix.go`.

[tests]: architecture/tests.md
[go]: architecture/go.md
[audit-checklist]: architecture/audit-checklist.md
[2608021915]: ../../plan/2608021915_arch-fix-query-subcommand-placement.md
[2608021916]: ../../plan/2608021916_arch-fix-githooks-package-split.md

### nice-to-have (2026-08-02)

- Several tests covering the flagged helper clusters use
scenario names (e.g. `TestDriftParts_ResolvesHooksDirOnce`)
rather than the literal `TestReceiver_Foo` binding —
coverage exists, only the naming convention drifts.
- `stagingHelperShellFunc` (`internal/githooks/githooks.go`)
contains "Helper" in its name — go.md flags this as a
smell, though the constant is well-scoped. Rename on
next touch.
- `Override.Patterns()` / `KindAssignmentEntry.Patterns()`
(`internal/config/config.go`) are two-line branching
methods exercised only inside larger config tests, not by
a dedicated `Test*`. Low regression risk.
- `parseCheckFlags` / `parseFixFlags` cross go.md's ~50-line
guidance for `cmd/mdsmith` handlers, but the body is pure
flag-registration boilerplate, not domain logic.

## Audit 2026-07-19 (range: 834b560..6680ff5)

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

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

Expand Down
74 changes: 74 additions & 0 deletions plan/2608021915_arch-fix-query-subcommand-placement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
id: 2608021915
title: >-
Move list-query subcommand logic out of
cmd/mdsmith/main.go into query.go
status: "🔲"
model: haiku
summary: >-
parseQueryFlags, runQuery, queryFiles, and
readFrontMatterRaw implement the `mdsmith list
query` subcommand but live in main.go instead of a
dedicated per-subcommand file. Flagged by the
2026-08-02 audit.
---
# Move list-query subcommand logic out of cmd/mdsmith/main.go into query.go

## Goal

Relocate the `list query` subcommand's implementation
into its own file. That keeps `main.go` as
dispatch-and-glue, matching the pattern already used
for every sibling subcommand.

## Background

The 2026-08-02 audit (see
[the audit log](../docs/development/architecture-audit.md))
found this placement gap:

- [main.go](../cmd/mdsmith/main.go) defines
`parseQueryFlags`, `runQuery`, `queryFiles`, and
`readFrontMatterRaw` — the full domain logic for the
`list query` subcommand (CUE-expression matching
against front matter, its own file walk).
- [list.go](../cmd/mdsmith/list.go)'s `runList`
dispatches `case "query": return runQuery(...)`, the
same way it dispatches `case "backlinks":` to
`runBacklinks`, which lives in its own
[backlinks.go](../cmd/mdsmith/backlinks.go).
- [go.md](../docs/development/architecture/go.md)'s
"Clean wiring in `cmd/mdsmith`" section: domain logic
belongs in `pkg/mdsmith`, `internal/engine`, or a
subcommand's own file — not in `main.go`.
- The 2026-07-19 audit already applied this exact move
once, relocating the `init` subcommand's logic out of
`main.go` into
[init.go](../cmd/mdsmith/init.go); this plan repeats
that move for `query`.

## Tasks

1. Create `cmd/mdsmith/query.go` and move
`parseQueryFlags`, `runQuery`, `queryFiles`, and
`readFrontMatterRaw` into it, unchanged.
2. Move the query-only tests covering those functions
out of `main_unit_test.go` into a new
`cmd/mdsmith/query_unit_test.go`, following the
`init_unit_test.go` precedent.
3. Confirm `list.go`'s `runList` still compiles against
the relocated `runQuery` with no signature change.
4. `go build ./...` passes.
5. `go test ./cmd/mdsmith/...` passes.
6. `go tool -modfile=tools/go.mod golangci-lint run`
reports no issues.

## Acceptance Criteria

- [ ] `parseQueryFlags`, `runQuery`, `queryFiles`, and
`readFrontMatterRaw` no longer appear in
`cmd/mdsmith/main.go`.
- [ ] `cmd/mdsmith/query.go` holds the relocated code
with no behavior change.
- [ ] `go test ./...` is green.
- [ ] `mdsmith check .` is green.
Loading
Loading