feat(rules): bootstrap 3 doc families (linting, state-machine, json-error-handler)

[bborbe]

Summary

First multi-doc bootstrap PR — 3 small docs (~450 lines each) in one worktree, one bot review cycle, one walker regen. Validates the multi-doc-per-PR pattern as a faster alternative to serial PRs without parallel-worktree merge conflicts on `rules/index.json`.

`rules/index.json`: 56 → 62 entries (6 new rules across 3 new families).

Rules added

family / id	level	owner
`go-linting/complexity-limits-enforced`	MUST	go-quality-assistant
`go-linting/banned-packages-via-depguard`	MUST	go-quality-assistant
`go-state-machine/status-phase-separation`	MUST	go-architecture-assistant
`go-state-machine/forward-only-by-default`	SHOULD	go-architecture-assistant
`go-json-error-handler/structured-response-shape`	MUST	go-http-handler-assistant
`go-json-error-handler/use-error-code-constants`	MUST	go-http-handler-assistant

Why multi-doc-per-PR

Tested today after PR #16's solo flow. Trade-offs measured:

• Serial PRs (one doc per PR): 6 review cycles × ~5 min wait = ~30 min wall-clock for 3 docs. Bot bandwidth × 3.
• Parallel PRs (one worktree per doc): ~10 min wall-clock but every PR regenerates `rules/index.json` from a different `origin/master` snapshot → predictable merge conflicts on every merge.
• Multi-doc-per-PR (this approach): ~15 min wall-clock, single review cycle, single walker regen, zero index conflicts. Sweet spot.

Owner agent choices

No dedicated agents for these doc topics. Mapped each rule to the closest-purpose existing agent:

• linting → `go-quality-assistant` (broad code-quality remit)
• state-machine → `go-architecture-assistant` (cross-unit FSM design)
• json-error-handler → `go-http-handler-assistant` (HTTP handler organisation)

If the bot pushes back on any mapping, easy to update.

Pre-emptive checks

• `grep -nEi "personal/|/users/bborbe|~/documents/obsidian"` → clean across all 3 docs
• Trading-term grep → clean
• All 6 rule IDs unique against the 56 existing entries
• All 3 owner agents exist at `agents/.md`
• `make build-index` regenerated; `check-index` passes (PR feat(precommit): add check-index target to catch walker-output drift #13 drift guard happy)

Test plan

• `make precommit` clean
• 6 entries in index with consistent schema, correct level + owner
• No duplicate rule IDs across the 11 doc families
• Bot review

[ben-s-pull-request-reviewer]

Now I have all the information needed for a thorough review.

---

PR Review: feat(rules): bootstrap 3 doc families (linting, state-machine, json-error-handler)

Changes Overview

4 files changed, 273 insertions. Three new guide rule blocks + corresponding rules/index.json entries.

---

Must Fix (Critical)

None. No security vulnerabilities, no critical correctness issues in the changed code itself.

---

Should Fix (Important)

docs/go-state-machine-pattern.md:168 — Operator precedence bug in status-phase-separation Good example.

caseresult.Status=="done"&&result.NextPhase==""||result.NextPhase==PhaseDone:

Due to && binding tighter than ||, this evaluates as:

(result.Status == "done" && result.NextPhase == "") || result.NextPhase == PhaseDone

So it fires when NextPhase == PhaseDone even if Status != "done" — overlapping with the next case and contradicting the "workflow terminates" comment. Should be:

caseresult.Status=="done"&& (result.NextPhase==""||result.NextPhase==PhaseDone):

---

Nice to Have (Optional)

docs/go-linting-guide.md:182 — maintidx configuration inconsistency. The rule's YAML example uses:

maintidx: { under: 20 }

But the guide's own reference config (line 79) uses:

maintidx: { min-maintainability-index: 20 }

These are different parameter names. The reference config name min-maintainability-index appears correct for golangci-lint v2. Recommend aligning the rule example to use the same parameter name.

---

Verification Checklist

Concern	Status
rules/index.json valid JSON	✅
All 6 new rule IDs unique vs existing 56 entries	✅
All owner agents exist (go-http-handler-assistant, go-quality-assistant, go-architecture-assistant)	✅
All required JSON fields present per entry	✅
libhttp package path correct in examples	✅
Go code examples syntactically valid	✅ (except operator precedence issue above)
make precommit	⚠️ Could not run (sandbox blocked cd into worktree) — but JSON parses clean and no link breakage visible

---

Concerns from Plan — Resolution

Concern	Resolution
rules/index.json valid JSON schema + entry structure	Addressed — all 6 entries well-formed
6 new rule IDs unique against existing	Addressed — all unique
Owner agents exist	Addressed — all three agents present
libhttp package path in examples	Addressed — import alias libhttp + github.com/bborbe/http consistent
Forward-only-by-default rule examples accurate	Addressed — circuit breaker pattern correctly shown

---

{
"verdict": "request-changes",
"summary": "Three new rule families added correctly with proper index.json entries and matching doc blocks. One operator precedence bug in the go-state-machine-pattern.md Good example (switch case at line 168) and one YAML config inconsistency in go-linting-guide.md (maintidx parameter name) should be fixed before merging.",
"comments": [
{
"file": "docs/go-state-machine-pattern.md",
"line": 168,
"severity": "major",
"message": "go-state-machine/status-phase-separation: operator precedence bug — case `result.Status == \"done\" && result.NextPhase == \"\" || result.NextPhase == PhaseDone` evaluates as `(done && empty) || PhaseDone` due to && binding tighter than ||. This fires when NextPhase is PhaseDone regardless of Status, overlapping with the next case and contradicting the \"workflow terminates\" comment. Fix: wrap the OR in parentheses: `result.Status == \"done\" && (result.NextPhase == \"\" || result.NextPhase == PhaseDone)`."
},
{
"file": "docs/go-linting-guide.md",
"line": 182,
"severity": "nit",
"message": "go-linting/complexity-limits-enforced: maintidx YAML setting uses `under: 20` but the guide's own reference config (line 79) uses `min-maintainability-index: 20`. Recommend aligning to `min-maintainability-index` which is the correct golangci-lint v2 parameter name."
}
],
"concerns_addressed": [
"correctness: rules/index.json entries maintain valid JSON schema and match existing entry structure — verified",
"correctness: 6 new rule IDs are unique against all existing entries — verified",
"correctness: owner agents exist (go-http-handler-assistant, go-quality-assistant, go-architecture-assistant) — verified",
"correctness: libhttp package path in go-json-error-handler-guide.md examples is correct — verified",
"correctness: forward-only-by-default rule examples are accurate (circuit breaker pattern shown) — verified"
]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

Now I have all findings. Let me compile the consolidated report and JSON verdict:

---

PR Review: feat(rules): bootstrap 3 doc families (linting, state-machine, json-error-handler)

Automated Checks

3a. LICENSE file: Present ✓

3b. make precommit: Could not execute — requires user approval for make precommit in the Bash tool. Please run manually and confirm clean exit.

---

Consolidated Report

Must Fix (Critical)

None — no security vulnerabilities or data correctness issues. This is a documentation-only PR.

Should Fix (Important)

1. [go-json-error-handler-guide.md:42] WrapWithCode called with 4 arguments — the function signature only accepts 3 args (err, code, status). The example passes a 4th map[string]string details argument. Should call WrapWithDetails instead. This is the most prominent example in the doc and will mislead readers.

2. [go-json-error-handler-guide.md] Missing CLAUDE.md doc-agent alignment entry — the guide documents two ### RULE blocks owned by go-http-handler-assistant, but CLAUDE.md's alignment table has no row for go-json-error-handler-guide.md. Required by the "Doc ↔ Agent Alignment" rule in CLAUDE.md.

3. [go-linting-guide.md:178,270] YAML v1/v2 schema inconsistency — both "Good" YAML examples under the complexity-limits and depguard rules use linters-settings: (golangci-lint v1 key) while the main config example correctly uses settings: (v2 key). A developer copying these snippets would produce invalid half-v1/half-v2 configs.

4. [go-linting-guide.md:3] Dead external reference to go-skeleton repo — links to https://github.com/bborbe/go-skeleton which is not part of this plugin and may not exist. The canonical config is templates/.golangci.yml in this repo. Violates the "Self-Contained" principle in CLAUDE.md.

5. [go-linting-guide.md:273] depguard rule name casing mismatch — main config (line 55) uses Main: (capital M), but the "Good" depguard example uses main: (lowercase m). golangci-lint v2 is case-sensitive on rule names.

6. [go-state-machine-pattern.md:137,265] Wrong owner agent — both rules list go-architecture-assistant as owner, but that agent's scope is cross-unit concerns (package boundaries, layering, naive extractions). Neither rule concerns any of these — they are semantic FSM correctness rules. The correct owner is go-quality-assistant, consistent with CLAUDE.md's mapping table where go-quality-assistant is listed as "broader review" alongside go-architecture-assistant.

7. [go-state-machine-pattern.md:296] Metadata field undeclared — the "Good" circuit-breaker example returns Result{Metadata: map[string]string{...}} but the Result struct defined at line 67 has no Metadata field.

8. [go-state-machine-pattern.md:296] Missing fmt import — fmt.Sprint(attempts + 1) called in the circuit-breaker example, but the import block at line 39 does not include fmt.

Nice to Have (Optional)

9. [go-state-machine-pattern.md:135,263] Rule IDs use two components, not three — schema at docs/rule-block-schema.md requires <lang>/<topic>/<slug> (three components). Current IDs are go-state-machine/status-phase-separation and go-state-machine/forward-only-by-default. Correct form would be go/state-machine/status-phase-separation and go/state-machine/forward-only-by-default.

10. [go-state-machine-pattern.md:484] AvailablePhases referenced but not defined — the anti-pattern example iterates over AvailablePhases, but the minimal Phase enum at line 47 has no such collection. Should add var AvailablePhases Phases = Phases{...} per go-enum-type-pattern.md.

11. [go-json-error-handler-guide.md:416] Test uses string literal instead of constant — assertion compares "VALIDATION_ERROR" string instead of libhttp.ErrorCodeValidation, contradicting the use-error-code-constants rule the guide itself documents.

12. [go-json-error-handler-guide.md:7-9] Missing cross-reference — uses github.com/bborbe/errors throughout but "See also" section does not link to go-error-wrapping-guide.md.

13. [rules/index.json] Entry count discrepancy — PR description states 56 entries expected, actual is 59. The 6 new entries are all present and correct; the pre-PR baseline was 53, not 50 as implied. Not a code bug — only a PR description inaccuracy.

---

Positive Notes

• All three new docs are well-structured with good prose, accurate rule justifications, and appropriate generic examples (User, Order, Product).
• README.md and llms.txt are correctly updated with all three new guides.
• rules/index.json has all 6 new entries with correct schema, unique IDs, and valid owner agent references.
• The state machine pattern's phase/status separation rule is a genuinely non-obvious design insight that justifies a ### RULE block.
• The fork/join section in the state machine doc correctly uses run.CancelOnFirstErrorWait with proper run.Func signatures.

---

{
"verdict": "request-changes",
"summary": "Three new coding guides are added with correct structure and good content, but contain should-fix issues: a critical API misuse in go-json-error-handler-guide.md (WrapWithCode called with wrong arg count), YAML schema inconsistencies in go-linting-guide.md (v1/v2 mixing), a wrong owner agent assignment in go-state-machine-pattern.md, and a dead external reference to a non-existent go-skeleton repo. These do not block merge but should be addressed before release.",
"comments": [
{
"file": "docs/go-json-error-handler-guide.md",
"line": 42,
"severity": "major",
"message": "Should Fix: WrapWithCode called with 4 arguments but its signature only accepts 3 (err, code, status). Pass a details map — use WrapWithDetails instead, or remove the map argument."
},
{
"file": "docs/go-json-error-handler-guide.md",
"line": 7,
"severity": "major",
"message": "Should Fix: CLAUDE.md doc-agent alignment table missing entry for go-json-error-handler-guide.md → go-http-handler-assistant. Required by Doc ↔ Agent Alignment rule."
},
{
"file": "docs/go-json-error-handler-guide.md",
"line": 416,
"severity": "nit",
"message": "Nice to Have: Test assertion uses string literal \"VALIDATION_ERROR\" instead of libhttp.ErrorCodeValidation constant, contradicting the use-error-code-constants rule this guide documents."
},
{
"file": "docs/go-json-error-handler-guide.md",
"line": 7,
"severity": "nit",
"message": "Nice to Have: \"See also\" section missing cross-reference to go-error-wrapping-guide.md despite using github.com/bborbe/errors throughout."
},
{
"file": "docs/go-linting-guide.md",
"line": 178,
"severity": "major",
"message": "Should Fix: YAML example uses linters-settings: (v1) instead of settings: (v2). Inconsistent with the main config example at line 52 which correctly uses settings:. Also appears at line 270."
},
{
"file": "docs/go-linting-guide.md",
"line": 3,
"severity": "major",
"message": "Should Fix: References external https://github.com/bborbe/go-skeleton repo that is not part of this plugin. Canonical config is templates/.golangci.yml. Violates Self-Contained principle."
},
{
"file": "docs/go-linting-guide.md",
"line": 273,
"severity": "major",
"message": "Should Fix: depguard rule name uses main: (lowercase) while main config uses Main: (capitalized). golangci-lint v2 is case-sensitive on rule names."
},
{
"file": "docs/go-linting-guide.md",
"line": 416,
"severity": "nit",
"message": "Nice to Have: Consider adding version: \"2\" to the per-rule YAML snippets so developers copying them get valid v2 configs."
},
{
"file": "docs/go-state-machine-pattern.md",
"line": 137,
"severity": "major",
"message": "Should Fix: go-architecture-assistant is not the right owner for status-phase-separation. That agent's scope is cross-unit concerns (package boundaries, layering, naive extractions). This is a semantic FSM correctness rule; correct owner is go-quality-assistant. Same issue at line 265 for forward-only-by-default."
},
{
"file": "docs/go-state-machine-pattern.md",
"line": 296,
"severity": "major",
"message": "Should Fix: Result struct (line 67) has no Metadata field, but the circuit-breaker example returns Result{Metadata: ...}. Either add the field to the struct definition or remove it from the example."
},
{
"file": "docs/go-state-machine-pattern.md",
"line": 296,
"severity": "major",
"message": "Should Fix: fmt.Sprint called but \"fmt\" is not in the import block (line 39). Example will not compile as shown."
},
{
"file": "docs/go-state-machine-pattern.md",
"line": 135,
"severity": "nit",
"message": "Nice to Have: Rule ID go-state-machine/status-phase-separation uses 2 components; schema requires 3 (go/state-machine/status-phase-separation). Same at line 265 for forward-only-by-default."
},
{
"file": "docs/go-state-machine-pattern.md",
"line": 484,
"severity": "nit",
"message": "Nice to Have: Anti-pattern references AvailablePhases but the Phase enum (line 47) has no such collection. Add var AvailablePhases per go-enum-type-pattern.md so the example is self-contained."
}
],
"concerns_addressed": [
"correctness: rules/index.json has all 6 entries with correct schema, unique IDs, valid owner agent references — verified by agent",
"correctness: go-json-error-handler-guide.md rules documented correctly but WrapWithCode API misuse at line 42 — flagged as should-fix",
"correctness: go-linting-guide.md rules documented correctly but YAML v1/v2 mixing at lines 178,270 and dead external reference at line 3 — flagged as should-fix",
"correctness: go-state-machine-pattern.md rules have correct content but wrong owner agent at lines 137,265 and undeclared Metadata field at line 296 — flagged as should-fix",
"tests: make precommit could not be executed — requires user to run manually and confirm"
]
}

[ben-s-pull-request-reviewer]

Reviewed by ben-s-pull-request-reviewer[bot] — no concerns flagged.