Skip to content

Commit 5d670e2

Browse files
committed
Address code-review findings on the init.go split
- Normalize os.WriteFile's mode literal to 0o644 in writeInitConfig, matching the sibling call in writeScaffolds in the same file. - Document the archive-sharding convention this cycle improvised (split into a numbered shard, never bump max-file-length without consent) in the audit-checklist so future audits don't reinvent it. - Fold setInitUsage's identical missing-unit-test gap (surfaced by this review, not the original audit sweep) into plan/2607191917 alongside printInitCatalog's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Req4a3SADus6KRYrmYqKg
1 parent ad1d31e commit 5d670e2

5 files changed

Lines changed: 72 additions & 24 deletions

File tree

PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,6 @@ footer: |
253253
| 2607082052 || sonnet | [SARIF output format for `mdsmith check`](plan/2607082052_check-sarif-output.md) |
254254
| 2607121915 || sonnet | [Resolve the internal/linkgraph purity-contract mismatch for wikilink resolution](plan/2607121915_arch-fix-linkgraph-wikilink-purity.md) |
255255
| 2607170527 | 🔳 | opus | [External link checking on WASM hosts (MDS072)](plan/2607170527_wasm-external-link-check.md) |
256-
| 2607191917 | 🔲 | haiku | [Add a dedicated unit test for printInitCatalog](plan/2607191917_arch-fix-printinitcatalog-unit-test.md) |
256+
| 2607191917 | 🔲 | haiku | [Add dedicated unit tests for printInitCatalog and setInitUsage](plan/2607191917_arch-fix-printinitcatalog-unit-test.md) |
257257
| 2607191918 | 🔲 | haiku | [Deduplicate isClaimed between internal/schema and requiredstructure](plan/2607191918_arch-fix-isclaimed-dedup.md) |
258258
<?/catalog?>

cmd/mdsmith/init.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func writeInitConfig(configFile, fromMarkdownlint, starterName string, force boo
140140
if err != nil {
141141
return err
142142
}
143-
if err := os.WriteFile(configFile, data, 0644); err != nil {
143+
if err := os.WriteFile(configFile, data, 0o644); err != nil {
144144
return fmt.Errorf("writing %s: %w", configFile, err)
145145
}
146146
if source != "" {

docs/development/architecture-audit.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,17 @@ None.
6565
matching `init_unit_test.go`. `main.go` is now 678 lines.
6666
No behavior change; `go test ./...` and
6767
`go tool golangci-lint run` are green.
68-
- `printInitCatalog` (`cmd/mdsmith/init.go`) has no
69-
dedicated unit test — only the e2e `mdsmith init --list`
70-
subprocess test exercises it.
68+
- `printInitCatalog` and `setInitUsage`
69+
(`cmd/mdsmith/init.go`) have no dedicated unit test —
70+
only e2e subprocess tests (`mdsmith init --list` and
71+
`--help`) exercise them.
7172
[tests.md][tests]: "A new function lands together with
7273
its dedicated unit test by name," and an e2e test
7374
reachable without the process boundary is an inverted
74-
pyramid. Not a public surface itself (a `runInit`
75-
helper), so tax not blocker —
75+
pyramid. Neither is a public surface itself (both are
76+
`runInit` helpers), so tax not blocker. The
77+
`setInitUsage` half of this surfaced during this cycle's
78+
3x code-review pass on the fix, not the original sweep —
7679
[plan/2607191917][2607191917].
7780
- `internal/rules/requiredstructure/rule.go`'s `isClaimed`
7881
is a byte-for-byte copy of

docs/development/architecture/audit-checklist.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,25 @@ budget.
5959
per-finding "suggested fix". Single
6060
sentences over ~30 words trip MDS023 /
6161
MDS024 on the audit log.
62+
- **When the archive itself crosses the
63+
300-line budget**: do not bump
64+
`max-file-length` on it — that needs
65+
explicit user consent per
66+
[CLAUDE.md](../../../CLAUDE.md)'s
67+
"Never modify `.mdsmith.yml` ...
68+
without explicit user consent" rule.
69+
Instead, split it the same way the
70+
main log split into it: move the
71+
oldest entries into a new numbered
72+
shard (`architecture-audit-archive-2.md`,
73+
then `-3.md`, ...), moved verbatim, not
74+
reworded. Each shard links to the next
75+
one it overflowed into, and the
76+
original `architecture-audit-archive.md`
77+
stays the entry point new readers find
78+
first. Re-run `mdsmith fix .` after the
79+
split so every parent catalog picks up
80+
the new shard.
6281

6382
## Initial file template
6483

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
11
---
22
id: 2607191917
33
title: >-
4-
Add a dedicated unit test for printInitCatalog
4+
Add dedicated unit tests for printInitCatalog and
5+
setInitUsage
56
status: "🔲"
67
model: haiku
78
summary: >-
8-
printInitCatalog (cmd/mdsmith/init.go) has no
9-
TestPrintInitCatalog; only an e2e subprocess test
10-
exercises `mdsmith init --list`. Flagged by the
11-
2026-07-19 audit as an inverted-pyramid tax finding.
9+
printInitCatalog and setInitUsage (cmd/mdsmith/init.go)
10+
have no dedicated unit test; only e2e subprocess tests
11+
exercise them. Flagged by the 2026-07-19 audit and its
12+
own 3x code-review pass as inverted-pyramid tax findings.
1213
---
13-
# Add a dedicated unit test for printInitCatalog
14+
# Add dedicated unit tests for printInitCatalog and setInitUsage
1415

1516
## Goal
1617

17-
Give `printInitCatalog` its own unit test so `mdsmith init
18-
--list` behavior is locked at the unit layer, not only
19-
through the e2e subprocess test.
18+
Give `printInitCatalog` and `setInitUsage` their own unit
19+
tests. Lock `mdsmith init --list` and `--help` behavior at
20+
the unit layer, not only through e2e subprocess tests.
2021

2122
## Background
2223

@@ -39,9 +40,22 @@ found this gap:
3940
writes to an `io.Writer`. It needs no subprocess to
4041
exercise.
4142

42-
This is `tax`, not `blocker`. `printInitCatalog` is a
43-
helper `runInit` calls. It is not itself a CLI subcommand
44-
entry point, an LSP handler, or a `rule.Rule` method.
43+
A follow-up 3x code-review pass on the PR that filed this
44+
plan found the identical shape one function over:
45+
46+
- `setInitUsage` in [init.go](../cmd/mdsmith/init.go) has
47+
no `TestSetInitUsage` anywhere in
48+
[init_unit_test.go](../cmd/mdsmith/init_unit_test.go).
49+
- Its only exercise is the e2e subprocess assertion for
50+
`mdsmith init --help` in
51+
[e2e_coverage_test.go](../cmd/mdsmith/e2e_coverage_test.go).
52+
- Same tests.md citation applies: `setInitUsage` installs a
53+
closure on a `*pflag.FlagSet` and needs no subprocess to
54+
exercise directly.
55+
56+
Both are `tax`, not `blocker`. Neither is itself a CLI
57+
subcommand entry point, an LSP handler, or a `rule.Rule`
58+
method — both are helpers `runInit` wires up.
4559

4660
## Tasks
4761

@@ -51,16 +65,28 @@ entry point, an LSP handler, or a `rule.Rule` method.
5165
("Starters (mdsmith init --starter <name>):" and "Packs
5266
(mdsmith init --add <name>):") plus at least one known
5367
starter name and one known pack name appear in the output.
54-
2. Leave the existing e2e `--list` test in place — it still
55-
covers the full CLI dispatch path — but do not duplicate the
56-
catalog-content assertions there beyond a smoke check.
57-
3. `go build ./...` passes.
58-
4. `go test ./cmd/mdsmith/...` passes.
68+
2. Add `TestSetInitUsage` in
69+
[init_unit_test.go](../cmd/mdsmith/init_unit_test.go),
70+
building a `*pflag.FlagSet`, calling `setInitUsage`, then
71+
invoking `fs.Usage()` with a captured `os.Stderr` (or by
72+
redirecting the closure's writer, whichever the existing
73+
test helpers in this file support) and asserting the
74+
printed text names `--starter`, `--from-markdownlint`,
75+
`--add`, `--force`, and `--list`.
76+
3. Leave the existing e2e `--list` and `--help` tests in
77+
place — they still cover the full CLI dispatch path — but
78+
do not duplicate the content assertions there beyond a
79+
smoke check.
80+
4. `go build ./...` passes.
81+
5. `go test ./cmd/mdsmith/...` passes.
5982

6083
## Acceptance Criteria
6184

6285
- [ ] `TestPrintInitCatalog` exists in
6386
[init_unit_test.go](../cmd/mdsmith/init_unit_test.go) and
6487
exercises `printInitCatalog` directly (no subprocess).
88+
- [ ] `TestSetInitUsage` exists in
89+
[init_unit_test.go](../cmd/mdsmith/init_unit_test.go) and
90+
exercises `setInitUsage` directly (no subprocess).
6591
- [ ] `go test ./...` is green.
6692
- [ ] `mdsmith check .` is green.

0 commit comments

Comments
 (0)