feat(hooks): enforce mermaid fence + facet keyword for architecture notes (#239) #240
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
ldraney/claude-custom!240
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "239-mermaid-fence-enforcement"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Hardens
hooks/check-note-template.shto verify that every architecture note contains a valid mermaid fence and — when the slug has a known facet prefix — that the first keyword inside the fence matches the facet. Closes the gap where### Diagram\n(TODO)would pass the gate.Closes #239.
Changes
hooks/check-note-template.shcheck_architecture_mermaidruns after heading validation fornote_type=architecture.```mermaidmarkdown fence<pre class="mermaid">inline HTML<pre><code class="language-mermaid">server-rendered HTMLarch-domain-*→erDiagramarch-dataflow-*→sequenceDiagramarch-deployment-*→graph TB|graph TD|flowchart TB|flowchart TD%%Mermaid comment lines before the keyword, and accepts trailing diagram config (e.g.sequenceDiagram autonumber).ARCH_GRANDFATHER_SLUGSarray (clearly commented exclusion list) skips mermaid enforcement entirely for:arch-domain-pal-e-agency— legacy org chart usinggraph TD(per spec recommendation)arch-generic-checkout— legacy prose-only note with no diagram at all (discovered during regression; see Discovered Scope below)trap 'exit 0' ERRwraps the helper and all parse failures return 0.<pre><code class="language-mermaid">erDiagram.Test Plan
Parse check
Fixture tests (12/12 pass)
sequenceDiagramunderarch-domain-*)<pre class="mermaid">+ sequenceDiagram<pre><code class="language-mermaid">graph TBflowchart TDarch-auth-*) with fencearch-domain-pal-e-agencywithgraph TD%%comments before keyworderDiagramfencenote_type=docunaffectedRegression: 27/27 existing architecture notes pass the new mermaid check
Dry-run against all 28 architecture notes enumerated in the issue body, feeding the server's
html_contentthrough the new helper astool_input.content(the round-trip shape:get_note→update_note):arch-secrets-pipelineis skipped because the API returns an emptyhtml_contentfor it. The pre-existing heading-check already denies empty content, so my change does not alter its behavior. Not a regression.Discovered Scope
During regression,
arch-generic-checkoutwas found to contain no mermaid diagram at all — it's a legacy HTML-table-only prose note predating the template. The issue lists it under "fence required, facet keyword NOT checked," but enforcing a fence would break future updates to the note. Following the spec's recommended pattern forarch-domain-pal-e-agency, I added it to the clearly-commented grandfather list.Follow-up migration ticket should be filed to either:
arch-generic-checkoutas a template-conformant note with a real diagram, orarch-dataflow-checkoutsibling that carries the diagram while the existing note becomes an SOP or doc.Same applies to
arch-secrets-pipeline(empty note) — both should be tracked as separate cleanup work, not blockers for this PR.Review Checklist
bash -n hooks/check-note-template.shpasses%%comment tolerance, unknown prefix allow/deny, and non-architecture passthroughtrap 'exit 0' ERRon all new code paths)architecture)caseRelated Notes
template-architecture— the rule being enforced by this hookconvention-architecture-ids— arch label derivation depends on valid diagramsarch-domain-pal-e-docs— reference template-conformant arch note used during testingproject-pal-e-docs— home ofstory:agent-write("Zero malformed notes in production")Related
Extends check-note-template.sh architecture-note validation to verify: 1. A mermaid fence is present in the content (accepts all three container forms: markdown triple-backtick, inline HTML <pre class="mermaid">, and server-rendered <pre><code class="language-mermaid">). 2. When the slug has a known facet prefix, the first non-comment line inside the fence matches the expected diagram keyword: arch-domain-* -> erDiagram arch-dataflow-* -> sequenceDiagram arch-deployment-* -> graph TB | graph TD | flowchart TB | flowchart TD Non-triplet slugs (arch-auth-*, arch-sitemap-*, etc.) require a fence but skip the keyword check. Keyword check tolerates leading whitespace and %% comment lines before the keyword. Grandfather list (clearly commented, exclusion by slug): - arch-domain-pal-e-agency: legacy org chart uses graph TD - arch-generic-checkout: prose-only legacy note with no diagram Preserves fail-open behavior via trap 'exit 0' ERR on any parse error. Matches existing hook style (bash + jq + trap). No template changes. Validation: - bash -n parse check: OK - 12 fixture cases (missing fence, wrong keyword, all 3 container forms, flowchart TD, unknown prefix allow/deny, grandfather, leading comments, bare erDiagram, non-arch unaffected): 12/12 pass - Regression dry-run against all 28 existing architecture notes: 27/27 mermaid-check pass (1 skipped: arch-secrets-pipeline has empty html_content at the API — pre-existing state, unaffected by this change) Closes #239 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>PR #240 Review
DOMAIN REVIEW
Tech stack: bash hook script (
check-note-template.sh), jq, awk, sed, grep -E. Enforces architecture note mermaid fence + facet keyword rule from issue #239.Fence detection (all 3 forms) — verified:
```mermaidmarkdown fence: present in regex and awk state machine (mode = "md").<pre class="mermaid">inline HTML: present (mode = "html2").<pre><code class="... language-mermaid ...">server-rendered: present (mode = "html3"). A looser<pre><code class="[^"]*mermaid[^"]*">alternative is also matched — slightly more permissive than the spec's exact wording but harmless (anyclasssubstring containingmermaidis clear render intent). Acceptable.Facet keyword mapping — correct:
arch-domain-*→^[[:space:]]*erDiagram([[:space:]]|$)arch-dataflow-*→^[[:space:]]*sequenceDiagram([[:space:]]|$)(accepts trailingautonumberetc.)arch-deployment-*→^[[:space:]]*(graph|flowchart)[[:space:]]+(TB|TD)([[:space:]]|$)Grandfather list — complete:
arch-domain-pal-e-agency✓ with migration commentarch-generic-checkout✓ with "follow-up migration ticket must be filed" commentFail-open behavior — preserved:
mktemp || return 0for temp-file allocation failure.trap 'rm -f "$arch_tmp"; exit 0' ERRwraps helper body.check_architecture_mermaidis only invoked afterMISSINGheadings pass, so heading-check semantics are unchanged.%%comment tolerance — correct:^%%, loops to next non-blank/non-comment line.Awk state machine — correct handling of the dominant server-rendered inline form:
substr($0, RSTART + RLENGTH)captures trailing text on the same line (e.g.<pre><code class="language-mermaid">erDiagram) and prints it so the keyword check sees it. Verified all three modes handle this. Nice touch.Exit-on-deny via JSON stdout +
exit 0: consistent with the existingcheck-note-template.shconvention of signalling decisions via JSON body, not shell exit code. Matches surrounding code.BLOCKERS
None.
NITS
trap 'rm -f "$arch_tmp"; exit 0' ERRinstalled insidecheck_architecture_mermaidis never reset. For the allow path the functionreturn 0s and the outer script immediatelyexit 0s from the heading block, so this is benign — but a disciplinedtrap - ERRon function exit (or a subshell wrap) would be cleaner and less surprising if the helper is ever called earlier in the script.fence_bodybefore keyword extraction. Not incorrect, just redundant; the second pass is a no-op on already-decoded content. Consider dropping the secondsedor dropping the first and decoding only the extracted body.<pre><code class="[^"]*mermaid[^"]*">alternative is broader than the spec's literallanguage-mermaid. Fine, but worth a short inline comment noting it's intentional ("accept any class containingmermaid") so a future reader doesn't tighten it.printf '%s'vs content with%:printf '%s' "$content"is safe; just confirming for the record. No action.SOP COMPLIANCE
239-mermaid-fence-enforcement(matches{issue}-{kebab}convention)hooks/check-note-template.shmodified (changed_files=1, +194/−0) — no other hooks touchedarchitecture)case was extended via theARCHITECTURE_ENFORCE_MERMAID=1flag; other case branches are untouchedarch-generic-checkoutorarch-secrets-pipelinenote content — both correctly handled as follow-up cleanup tickets, not expansion of this PR's scope%%tolerance, unknown prefix allow/deny, non-architecture passthrougharch-generic-checkout) correctly handled as grandfather + follow-up ticket note, not as unplanned expansionPROCESS OBSERVATIONS
arch-domain-pal-e-agencygrandfather), added a clearly commented entry, and flagged migration as follow-up work rather than expanding PR scope. Textbookfeedback_discovered_scope_always_tracked.round-trip(get_note → update_note)shape differs fromcreate_noteshape.arch-generic-checkoutto either a template-conformant arch note or split intoarch-dataflow-checkout+ SOP.arch-secrets-pipelineemptyhtml_contentroot cause (API returns empty — pre-existing, but worth tracking).VERDICT: APPROVED