feat(architecture): bootstrap 6 rule blocks in go-architecture-patterns

[bborbe]

Summary

Bootstraps `docs/go-architecture-patterns.md` — adds 6 `### RULE` blocks for the architecture-specific conventions not covered by other guides. Mirrors PRs #2-5, #8, #10, #14, #15.

`rules/index.json`: 50 → 56 entries (new `go-architecture/*` family).

Rules added

id	level	enforcement
`go-architecture/counterfeiter-directive-on-interface`	MUST	judgment (ast-grep follow-up)
`go-architecture/new-prefix-constructor-naming`	MUST	judgment (partial overlap with `go-factory/no-impl-in-factory-pkg`)
`go-architecture/constructor-returns-interface`	MUST	judgment
`go-architecture/private-struct-matches-interface`	SHOULD	judgment (paired-declaration check)
`go-architecture/no-globals-or-singletons`	MUST	judgment (package-scope reasoning)
`go-architecture/business-logic-not-in-main`	MUST	judgment (semantic)

Cross-references (not duplicated)

The guide also covers context discipline and infinite-loop cancellation, but these are already canonicalized in:

• `go-errors/no-context-background-in-business-logic` (in index)
• `go-context/cancel-check-in-loop` (in index)
• `go-time/*` family (in index)

No new rule blocks added for those; the architecture doc keeps the cross-cutting prose as background context and the index keeps a single canonical home per rule.

Pre-emptive checks

• `grep -nEi "personal/|/users/bborbe|~/documents/obsidian"` → clean
• Trading-term grep flagged `KafkaBrokers` (false positive — Kafka server infrastructure, not trading-broker domain term; pre-existing in the main.go pattern example)
• All 6 rule IDs unique against the 50 existing entries
• `go-architecture-assistant` agent exists at `agents/go-architecture-assistant.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 `go-architecture/*` entries in index with MUST/SHOULD level + owner
• No duplicate rule IDs across families
• Bot review

[ben-s-pull-request-reviewer]

PR Review: feat(architecture): bootstrap 6 rule blocks in go-architecture-patterns

Pre-commit & Automated Checks

• make precommit: PASS
• LICENSE file: Present
• Diff: 2 files changed (+247 lines total: +193 docs, +54 rules/index.json)

---

Consolidated Report

Must Fix (Critical)

None — no security vulnerabilities or correctness bugs in the diff (this is a documentation-only change).

Should Fix (Important)

1. rules/index.json:77-79 — counterfeiter-directive-on-interface enforcement field uses not.precedes as an ast-grep operator, but this does not exist in ast-grep's pattern syntax. The negative precedence relation cannot be expressed this way. This is a factual error that would mislead anyone implementing the mechanical layer.

2. docs/go-architecture-patterns.md:64-68 — new-prefix-constructor-naming applies_when does not exempt Create* factory functions or pkg/factory/ locations. The doc's own section 3 (line 344) says "Create* prefix for factories". A mechanical enforcement of this rule would false-positive on every CreateUserService in pkg/factory/. The enforcement note at line 96 acknowledges "partial overlap" but does not state which rule owns Create*.

3. docs/go-architecture-patterns.md + rules/index.json — Rules 1 (counterfeiter-directive-on-interface), 3 (constructor-returns-interface), and 4 (private-struct-matches-interface) form a coupled three-part pattern (interface → constructor → struct) but are presented as independent rules with no cross-references. A developer reading one in isolation may not understand the full intent, and the applies_when fields give no indication they must be evaluated together.

4. docs/go-architecture-patterns.md:142-147 — private-struct-matches-interface enforcement is described as "paired-declaration check" — a term not used elsewhere in the index. Other rules name concrete ast-grep kinds (e.g., var_spec, function_declaration). The vague label makes it harder to implement the mechanical follow-up.

5. docs/go-architecture-patterns.md:555-560 — no-globals-or-singletons applies_when describes "package-level var" but does not cover init() functions or sync.Once lazy singletons, which share the same test-ordering and parallelism problems described in the why section. At MUST level, the trigger should be complete or explicitly scoped.

6. docs/go-architecture-patterns.md:791-796 — business-logic-not-in-main at MUST level has no mechanical enforcement path. Other judgment-only MUST rules at least provide a coarse ast-grep filter. This rule provides none, making it the least actionable MUST in the set. Additionally, the applies_when covers application.Run but does not exclude *_test.go files that might exercise that method.

7. agents/go-architecture-assistant.md + CLAUDE.md — All 6 new rules list go-architecture-assistant as owner, but the CLAUDE.md "Doc ↔ Agent Alignment" table maps go-architecture-patterns.md → go-quality-assistant. The agent scope (agents/go-architecture-assistant.md lines 236–252) does not list naming conventions, counterfeiter directives, or globals/singletons as owned concerns. This is a doc-agent alignment gap introduced by the new rules.

Nice to Have (Optional)

• docs/go-architecture-patterns.md:73-78 — new-prefix-constructor-naming bad example labels CreateUserService as a "Constructor" but Create* is the factory prefix per section 3.
• docs/go-architecture-patterns.md:820-831 — business-logic-not-in-main good example shows return service.ProcessUsers(ctx) with no error return, inconsistent with the doc's errors.Wrap pattern used everywhere else.
• docs/go-architecture-patterns.md:580-597 — no-globals-or-singletons good example introduces Worker/DoWork — first departure from the doc's consistent UserService/User examples.
• docs/go-architecture-patterns.md:145 — private-struct-matches-interface bad example shows 3 structs but the applies_when fires on single-implementation packages only.
• docs/go-architecture-patterns.md:791 — business-logic-not-in-main rule block is in section 11 (Antipatterns) rather than section 2 (main.go Pattern) where application.Run is defined.

---

Next Steps

• The not.precedes ast-grep syntax error in rules/index.json is the most clear-cut fix — replace with a valid ast-grep expression or remove the inline hint.
• The new-prefix-constructor-naming pkg/factory/ boundary clarification is needed before this rule can be enforced consistently.
• Consider grouping the three coupled rules (counterfeiter, constructor-returns, private-struct) with cross-references so they are evaluated as a unit.

---

{
  "verdict": "request-changes",
  "summary": "Six new architecture RULE blocks bootstrapped in go-architecture-patterns.md with matching rules/index.json entries. The rules are structurally sound and well-written, but have 7 should-fix issues: a factually wrong ast-grep operator (not.precedes), missing pkg/factory/ boundary in the constructor-naming rule, an incomplete trigger in no-globals-or-singletons, a MUST-level rule with no mechanical enforcement path, and a doc-agent alignment gap where all 6 rules claim go-architecture-assistant as owner but the agent's scope and CLAUDE.md's mapping table don't cover them. No critical issues; this is a documentation-only change.",
  "comments": [
    {
      "file": "rules/index.json",
      "line": 77,
      "severity": "major",
      "message": "go-architecture/counterfeiter-directive-on-interface enforcement uses 'not.precedes' which is not a valid ast-grep operator. The negative precedence relation cannot be expressed this way — this is a factual error that would mislead mechanical enforcement."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 67,
      "severity": "major",
      "message": "go-architecture/new-prefix-constructor-naming applies_when does not exempt Create* factory functions or pkg/factory/ locations. The doc's section 3 permits Create* for factories, but this rule would false-positive on them. Enforcement note acknowledges 'partial overlap' but does not assign ownership."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 19,
      "severity": "major",
      "message": "go-architecture/counterfeiter-directive-on-interface applies_when uses 'intended to be substitutable' — this requires intent-judgment by the agent with no concrete trigger. Consider scoping to interfaces referenced by *_test.go files."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 145,
      "severity": "major",
      "message": "go-architecture/private-struct-matches-interface applies_when fires on 'single struct' implementations, but the enforcement description ('paired-declaration check') is vague compared to other rules that name concrete ast-grep kinds. The bad example shows 3 structs when the trigger only covers single-impl packages."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 557,
      "severity": "major",
      "message": "go-architecture/no-globals-or-singletons applies_when covers package-level var but omits init() functions and sync.Once patterns, which share the same test-ordering and parallelism problems. At MUST level the trigger should be complete."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 794,
      "severity": "major",
      "message": "go-architecture/business-logic-not-in-main at MUST level has no mechanical enforcement path — no coarse ast-grep filter unlike other judgment-only rules. Also lacks *_test.go exclusion for application.Run."
    },
    {
      "file": "rules/index.json",
      "line": 65,
      "severity": "major",
      "message": "Doc-agent alignment gap: all 6 new rules list go-architecture-assistant as owner, but agents/go-architecture-assistant.md scope (lines 236-252) does not cover these patterns, and CLAUDE.md maps go-architecture-patterns.md to go-quality-assistant. Agent scope needs updating."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 73,
      "severity": "nit",
      "message": "go-architecture/new-prefix-constructor-naming bad example labels CreateUserService as a 'Constructor' — Create* is the factory prefix per section 3 of the same doc."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 830,
      "severity": "nit",
      "message": "go-architecture/business-logic-not-in-main good example returns nil without errors.Wrap, inconsistent with the doc's error-wrapping pattern used everywhere else."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 585,
      "severity": "nit",
      "message": "go-architecture/no-globals-or-singletons good example uses Worker/DoWork — first departure from the doc's consistent UserService/User examples throughout."
    }
  ],
  "concerns_addressed": [
    "correctness: ast-grep enforcement syntax error in rules/index.json raised as major comment",
    "correctness: subjective triggers in applies_when raised as major comments (counterfeiter 'intended to be substitutable', new-prefix missing pkg/factory/ boundary)",
    "correctness: no automated validation of rule definitions — no change in this PR, but the concern is acknowledged: the build-index.py extraction is working correctly (confirmed by make precommit pass)",
    "tests: no test coverage for rules — no change in this PR; rules/index.json is a JSON data file, not Go code, so go test coverage is not applicable"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

auto-approve disabled for this repo, review submitted as comment

---

PR Review: feat(architecture): bootstrap 6 rule blocks in go-architecture-patterns

Automated Checks

Check	Result
LICENSE file	✅ Exists
make precommit	⚠️ Permission denied — ran constituent checks manually
JSON syntax (plugin.json, marketplace.json, rules/index.json)	✅ All valid
Agent agents/go-architecture-assistant.md	✅ Exists
Rule anchor cross-references	✅ All 4 referenced rules exist

Plan Concerns — Resolution Status

Concern	Status
CLAUDE.md table references go-architecture-assistant but agent not in repo	✅ Addressed — agents/go-architecture-assistant.md exists
All 6 rules use judgment enforcement (no ast-grep)	✅ Acknowledged — correctly documented in each rule
Cross-referenced rule IDs exist in index	✅ Addressed — go-errors/no-context-background-in-business-logic, go-context/cancel-check-in-loop, go-time/*, go-factory/no-impl-in-factory-pkg all verified present
go-factory/no-impl-in-factory-pkg exists for exclusion scope	✅ Addressed — rule confirmed at line 237
ast-grep 0.43.0 limitation on counterfeiter-directive-on-interface	✅ Acknowledged — stated in enforcement field

---

Consolidated Report

Must Fix (Critical)

None.

Should Fix (Important)

1. docs/go-architecture-patterns.md:74-76 — new-prefix-constructor-naming Bad example is now underillustrative
   The edit removed CreateUserService (rightly, to avoid overlapping with the pkg/factory/** exception), leaving only MakeUserService and UserSvc. The "Why" section says "every consumer a second of 'wait, is this the constructor?'" but the example shows only two variants. Adding one more (e.g. BuildUserService) restores the breadth without reintroducing the overlap.
   Bucket: Should Fix

2. docs/go-architecture-patterns.md:578-593 — no-globals-or-singletons Good example uses UserService with a DoWork method — semantic mismatch
   The interface was renamed from Worker to UserService in this example, but DoWork is not a method name consistent with UserService's CRUD identity used everywhere else in the doc (Create, Get). A reader scanning quickly may think the doc has an inconsistency. Rename DoWork to Create to match the interface identity, or revert the interface name to Worker.
   Bucket: Should Fix

Nice to Have (Optional)

3. docs/go-architecture-patterns.md:153-154 — private-struct-matches-interface Bad example shows only one anti-pattern variant
   The "Why" enumerates three forms (defaultUserService / userServiceImpl / internalUser) but the code block shows only userServiceImpl. The other two variants were dropped without explanation. Restoring one or two lines matches code to prose.
   Bucket: Nice to Have

4. docs/go-architecture-patterns.md:821-824 — business-logic-not-in-main Good example's errors.Wrap at application.Run boundary is unexplained
   The errors.Wrap(ctx, err, "process users failed") in application.Run is wiring-level context, not domain logic — but a strict reader might move it into pkg/, which is the wrong direction. A one-line comment (// errors.Wrap at the application boundary is wiring-level context, not domain logic) removes the ambiguity.
   Bucket: Nice to Have

5. docs/go-architecture-patterns.md:19 — counterfeiter-directive-on-interface Applies-when clause is circular for new packages
   The rule triggers only when *_test.go files already import mocks. A brand-new package's first commit escapes the rule; the developer only gets flagged after writing tests. The fix is to drop the test-import prerequisite and apply the rule to all exported interfaces in non-main service packages.
   Bucket: Nice to Have

6. docs/go-architecture-patterns.md:82-88 — new-prefix-constructor-naming Good example implicitly relies on constructor-returns-interface without cross-reference
   The NewUserService(...) UserService { ... } example returns an interface type, which is governed by the next rule block. A reader who reads only this rule gets a Good example that looks arbitrary. Add a parenthetical or a cross-reference in the "Why" section.
   Bucket: Nice to Have

---

Summary

The PR correctly bootstraps 6 well-structured rule blocks into go-architecture-patterns.md, updates the rules/index.json with matching entries and correct anchors, and updates CLAUDE.md to assign ownership to the existing go-architecture-assistant. No code was changed — this is a docs-only PR. Two documentation-quality issues in the examples (underillustrative Bad examples, semantic mismatch in UserService/DoWork) are worth fixing before merge; the remaining issues are optional polish.

---

{
  "verdict": "approve",
  "summary": "PR correctly bootstraps 6 rule blocks with proper index entries and agent ownership. All plan concerns were addressed: the go-architecture-assistant agent exists, all cross-referenced rules are confirmed in index.json, and the judgment-only enforcement is accurately documented. Two example-quality issues (underillustrative Bad code, UserService/DoWork semantic mismatch) are worth fixing; remaining findings are optional polish.",
  "comments": [
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 76,
      "severity": "major",
      "message": "Should Fix: new-prefix-constructor-naming Bad example shows only 2 variants (MakeUserService, UserSvc) after CreateUserService was removed to avoid factory-pattern overlap. The Why section implies broader coverage — add BuildUserService or similar to restore illustrative breadth."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 591,
      "severity": "major",
      "message": "Should Fix: no-globals-or-singletons Good example renamed interface to UserService but keeps DoWork method — semantic mismatch with UserService's CRUD identity (Create, Get) used everywhere else in the doc. Rename DoWork to Create to match, or revert interface name to Worker."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 154,
      "severity": "nit",
      "message": "Nice to Have: private-struct-matches-interface Bad example shows only userServiceImpl but the Why section names three variants (defaultUserService, userServiceImpl, internalUser). Restore at least one more variant to match prose to code."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 824,
      "severity": "nit",
      "message": "Nice to Have: business-logic-not-in-main Good example uses errors.Wrap in application.Run — correct but unexplained. A reader might move it into pkg/, which is the wrong direction. Add a one-line comment that errors.Wrap at the application boundary is wiring-level context, not domain logic."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 19,
      "severity": "nit",
      "message": "Nice to Have: counterfeiter-directive-on-interface Applies-when clause requires *_test.go files that already import mocks — circular for new packages (no tests yet, no directive, first test import then triggers rule retroactively). Apply to all exported interfaces in non-main service packages unconditionally."
    },
    {
      "file": "docs/go-architecture-patterns.md",
      "line": 82,
      "severity": "nit",
      "message": "Nice to Have: new-prefix-constructor-naming Good example returns UserService interface, which depends on constructor-returns-interface (the next rule block). Reader who jumps here without reading ahead gets an arbitrary-looking interface return. Add a cross-reference or parenthetical in the Why section."
    }
  ],
  "concerns_addressed": [
    "correctness: go-architecture-assistant agent exists at agents/go-architecture-assistant.md — addressed",
    "correctness: all 6 rules use judgment enforcement, correctly documented — no action needed, acknowledged",
    "docs: all cross-referenced rule IDs verified present in rules/index.json — addressed",
    "docs: go-factory/no-impl-in-factory-pkg exists and exclusion scope is accurate — addressed",
    "correctness: ast-grep 0.43.0 limitation acknowledged in counterfeiter-directive-on-interface enforcement field — addressed"
  ]
}

[ben-s-pull-request-reviewer]

Now let me compile the final review with all findings:

{
  "verdict": "approve",
  "summary": "PR #16 adds 6 new RULE blocks to go-architecture-patterns.md (counterfeiter-directive-on-interface, new-prefix-constructor-naming, constructor-returns-interface, private-struct-matches-interface, no-globals-or-singletons, business-logic-not-in-main) with corresponding entries in rules/index.json. All entries have consistent schema, unique IDs matching anchors exactly, and the agent mapping in CLAUDE.md correctly identifies go-architecture-assistant as the rule-block owner. No code changes — purely documentation and metadata.",
  "comments": [],
  "concerns_addressed": [
    "correctness: rules/index.json has 6 new entries with no duplicate IDs and anchor fields matching rule IDs exactly — verified by inspection",
    "correctness: docs/go-architecture-patterns.md has 6 new RULE blocks with unique anchors — verified by inspection",
    "correctness: go-architecture-assistant agent exists at agents/go-architecture-assistant.md — verified by file read",
    "correctness: All 6 rules use judgment-based enforcement as documented — no ast-grep baseline claims in enforcement fields",
    "tests: rules/index.json entries have consistent schema (level, owner, doc_path) matching existing entries — verified by inspection"
  ]
}