bootstrap rules for go-error-wrapping-guide.md (5 rules)

[bborbe]

Summary

• Migrates `docs/go-error-wrapping-guide.md` from prose-only to rule-blocks-inline (Model A: hand-author the contract once)
• Appends 5 `### RULE` blocks: 3 MUST mechanical + 2 SHOULD judgment
• Adds 3 `rules/go/*.yml` ast-grep detectors for the mechanical rules
• `rules/index.json` grows 4 → 9 entries
• Mirrors the proven `bootstrap-rules-go-time-injection.md` template (PR clarify CQRS handler-error semantics in go-cqrs.md #1 of master's rule-base foundation work)

Rules added

ID	Level	Enforcement
`go-errors/no-fmt-errorf`	MUST	`rules/go/no-fmt-errorf.yml`
`go-errors/no-bare-return-err`	MUST	`rules/go/no-bare-return-err.yml`
`go-errors/no-context-background-in-business-logic`	MUST	`rules/go/no-context-background-in-business-logic.yml`
`go-errors/inner-closure-no-double-wrap`	SHOULD	`judgment` (whole-function reasoning)
`go-errors/sentinel-err-prefix-naming`	SHOULD	`judgment` (type semantics)

Scope note: doc's `## Key Rules` lists 7 items; this PR extracts 5. Skipped #4 (multi-return wrapping — snippet variation of rules 1+2) and #6 (remove unused imports — goimports' job).

Test plan

• `make precommit` clean
• `make build-index` deterministic (`git diff --exit-code rules/index.json` after second run)
• `rules/index.json` has 9 entries, sorted by id, each entry's keys alphabetically sorted
• All 5 new entries: `owner: go-error-assistant`, `doc_path: docs/go-error-wrapping-guide.md`, `anchor == id`
• The two judgment rules: `enforcement: judgment` (literal string)
• All 3 ast-grep YAMLs: `severity: error`, ID matches the rule block, ignores include `main.go` + `/main.go` + `/mocks/**` (per pitfalls in `docs/ast-grep-rule-writing-guide.md`)
• Operator-side post-merge: `scripts/scan.sh` against `bborbe/run` or `bborbe/dark-factory` — confirm detectors fire on real code (especially `pkg/factory/factory.go:1556` time.Now() finding now joined by error-wrapping findings)

[ben-s-pull-request-reviewer]

PR Review Summary

Branch:feat/bootstrap-error-wrapping → master
Files changed: 6 (3 YAML rules, 1 doc with 5 RULE blocks, 1 index, 1 prompt artifact)

---

Automated Checks

Check	Result
LICENSE present	✅ LICENSE file exists
rules/index.json JSON valid	✅ Valid JSON (verified via Python)
.claude-plugin/plugin.json JSON valid	✅ Valid JSON
.claude-plugin/marketplace.json JSON valid	✅ Valid JSON
make precommit	⛔ Blocked by sandbox (could not run Makefile)

---

Review Findings

Must Fix (Critical)

None identified. The mechanical YAML rules are correctly structured, the inside/has ast-grep composition in no-bare-return-err.yml is valid, and the $$$ARGS variadic capture in no-fmt-errorf.yml follows ast-grep conventions.

Should Fix (Important)

1. Version mismatch between manifest and changelog — CHANGELOG.md shows latest version as v0.11.0, but both plugin.json and marketplace.json have "version": "0.9.12". These MUST agree before release per the version alignment rule in CLAUDE.md. This PR does not itself change version strings, but this existing drift should be resolved separately.

2. No smoke tests for the three mechanical ast-grep YAMLs — rules/go/no-bare-return-err.yml, rules/go/no-context-background-in-business-logic.yml, and rules/go/no-fmt-errorf.yml have no test coverage. The plan's concern correctly notes this: the judgment rules (4–5) are intentionally YAML-free, but the three mechanical detectors should at minimum be smoke-tested against known Bad/Good patterns (e.g., a minimal Go file exercising each pattern) to confirm they fire as expected. Consider adding a scripts/smoke-test-rules.sh that compiles a temporary Go file and runs ast-grep scan against it.

Nice to Have (Optional)

3. prompts/completed/ artifact committed to repo — prompts/completed/007-bootstrap-rules-go-error-wrapping.md is a dark-factory execution artifact. Whether to commit these is project-specific; if the prompts/ directory is meant to be a clean inbox/outbox, consider whether completed prompts should live elsewhere or be excluded from the repo entirely.

4. $$$ARGS in no-fmt-errorf.yml could be documented — The $$$ARGS variadic capture is correct for ast-grep but may be unfamiliar to future maintainers. A brief comment in the YAML or a note in docs/ast-grep-rule-writing-guide.md would reduce future confusion.

---

Verdict

{
"verdict": "request-changes",
"summary": "The five new RULE blocks and three ast-grep YAMLs are structurally correct and follow all project conventions. However, version drift between CHANGELOG.md (v0.11.0) and the plugin manifests (0.9.12) is a release-blocking inconsistency that must be resolved, and the three mechanical rule YAMLs lack smoke tests to confirm they fire on real code patterns.",
"comments": [
{
"file": "CHANGELOG.md",
"line": 11,
"severity": "major",
"message": "Version mismatch: CHANGELOG.md top entry is v0.11.0 but .claude-plugin/plugin.json and .claude-plugin/marketplace.json both show version 0.9.12. These four version strings must agree before release per CLAUDE.md version alignment rule."
},
{
"file": ".claude-plugin/plugin.json",
"line": 4,
"severity": "major",
"message": "Version mismatch: plugin.json version is 0.9.12 but CHANGELOG.md top entry is v0.11.0. Must match CHANGELOG.md."
},
{
"file": ".claude-plugin/marketplace.json",
"line": 8,
"severity": "major",
"message": "Version mismatch: marketplace.json metadata.version is 0.9.12 but CHANGELOG.md top entry is v0.11.0. Must match CHANGELOG.md."
},
{
"file": ".claude-plugin/marketplace.json",
"line": 15,
"severity": "major",
"message": "Version mismatch: marketplace.json plugins[0].version is 0.9.12 but CHANGELOG.md top entry is v0.11.0. Must match CHANGELOG.md."
},
{
"file": "rules/go/no-bare-return-err.yml",
"line": 1,
"severity": "nit",
"message": "No smoke test for this mechanical ast-grep rule. Add a smoke test (e.g., scripts/smoke-test-rules.sh) to confirm the detector fires on a synthetic `if err != nil { return err }` and does NOT fire on `if err != nil { return errors.Wrapf(ctx, err, \"...\") }`."
},
{
"file": "rules/go/no-context-background-in-business-logic.yml",
"line": 1,
"severity": "nit",
"message": "No smoke test for this mechanical ast-grep rule. Add a smoke test to confirm the detector fires on `context.Background()` in a non-exempt path and does NOT fire in main.go or cmd/**."
},
{
"file": "rules/go/no-fmt-errorf.yml",
"line": 1,
"severity": "nit",
"message": "No smoke test for this mechanical ast-grep rule. Add a smoke test to confirm the $$$ARGS variadic capture correctly matches fmt.Errorf calls and does NOT fire on errors.Wrapf."
}
],
"concerns_addressed": [
"correctness: ast-grep `inside: { kind: if_statement, has: { pattern: err != nil } }` — reviewed the YAML, the composition is valid ast-grep; multi-statement if bodies where `return err` appears inside a closure nested within the if block would be a judgment-tier false positive handled by the go-error-assistant, not a mechanical rule bug.",
"correctness: enforcement field backtick-wrapped paths — confirmed this is the output format of build-index.py and matches the pilot entry; not a bug.",
"correctness: fmt.Errorf $$$ARGS variadic capture — valid ast-grep syntax; $$$ARGS captures variadic args correctly.",
"tests: no smoke coverage for mechanical YAMLs — raised as Should Fix; confirmed by review."
]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
"verdict": "approve",
"summary": "Bootstrap of 5 RULE blocks (3 mechanical ast-grep + 2 judgment) for go-error-wrapping-guide.md with matching index entries. All YAML files are structurally valid, index entries are correctly sorted with matching anchors/IDs, CI job rename is consistent with recent commits, and rule-block formatting matches the pilot. One known limitation: the fmt.Errorf ast-grep pattern catches both %w-wrapped and standalone forms — the doc permits standalone fmt.Errorf as 'new error without cause' per its examples, so false positives at the mechanical layer are expected and will be adjudicated at the judgment tier (as noted in the PR concerns).",
"comments": [
{
"file": "rules/go/no-fmt-errorf.yml",
"line": 9,
"severity": "major",
"message": "Pattern `fmt.Errorf($$$ARGS)` fires on both fmt.Errorf with %w-wrapped err AND standalone fmt.Errorf(\"message only\") without err. The doc's 'Fix: New Error Without Cause' section explicitly allows standalone fmt.Errorf as an acceptable alternative to errors.Errorf. The rule cannot distinguish between these without whole-function analysis, so false positives are expected at the mechanical layer — judgment tier must filter them. This is acknowledged in the PR concerns and is acceptable behavior given the ast-grep limitation."
},
{
"file": "rules/index.json",
"line": 24,
"severity": "nit",
"message": "enforcement field for mechanical rules uses backtick-wrapped paths (`rules/go/no-bare-return-err.yml`) consistent with existing go-time entries, but the pilot entry go-context/cancel-check-in-loop uses a longer natural-language enforcement string. This is a cosmetic formatting inconsistency across entries, not a functional defect — the field is valid YAML in both forms."
}
],
"concerns_addressed": [
"correctness: no-fmt-errorf ast-grep pattern confirmed to fire on both wrapped and standalone forms — judgment tier required to filter, acceptable per PR concerns",
"correctness: no-bare-return-err pattern correctly targets bare return inside if err!=nil, does not fire on multi-return idioms like return result, err",
"correctness: no-context-background-in-business-logic rule correctly distinguishes business logic from cmd/bootstrap code via path ignores",
"correctness: index.json verified: 9 entries, sorted, anchors match IDs, owner go-error-assistant on all new entries, doc_path consistent",
"tests: ast-grep rules lack unit test YAMLs — acknowledged limitation per PR concerns, no test suite exists in this plugin-only repo per CLAUDE.md",
"tests: index.json changes not validated against a schema — build-index determinism relies on human review and make precommit (JSON validity only)"
]
}