docs(agents): add development guardrails, C++ language server docs, and Claude Code automation by tianjianjiang · Pull Request #787 · openvanilla/McBopomofo

tianjianjiang · 2026-02-15T07:58:30Z

Summary

#787 (independent, base: master)

Add comprehensive AGENTS.md guardrails derived from PR Refactor: extract walk strategy with dynamic span support #779 postmortem
CRITICAL primacy zone: pre-edit checklist (branch/worktree verify, read master first)
C++17-only enforcement with explicit forbidden C++20/C++23 features list
Dual build system rules: CMakeLists.txt + .pbxproj must both be updated
Stacked branch independence: each branch must compile on its own base
Code-first verification and PR review response rules
Claude Code automation: hooks (clang-format, data file protection), skills (/engine-test, /branch-guard), context threshold
C++ language server integration docs: Serena MCP (clangd) + clangd-lsp plugin, shared compile_commands.json, .mm file limitation
/engine-test skill passes -DCMAKE_EXPORT_COMPILE_COMMANDS=ON to keep compile_commands.json fresh
.gitignore: add compile_commands.json and .cache (clangd index)

Recreated from #783, which was accidentally auto-merged into docs/algorithm_viterbi instead of master during a branch reorder.

Test plan

/engine-test passes (100 tests, compile_commands.json regenerated)
Serena find_symbol("ReadingGrid") confirms clangd works
.cache/ no longer shows as untracked

Generated with Claude Code

claude

Critical Issue

Missing trailing newline in .gitignore: The file does not end with a newline, violating POSIX text file standards. This can cause problems with git operations and text processing tools.

All other changes look solid - the guardrails, hooks, and documentation updates are well-structured and follow the repository conventions.

gemini-code-assist

Code Review

This pull request introduces significant improvements to the development guardrails and agent automation, particularly for Claude Code. The changes include new hooks for blocking edits to generated files and auto-formatting C++ code, new skills for branch safety and running engine tests, and extensive updates to AGENTS.md to document these new features and enforce development practices. The changes are well-structured and greatly enhance the automated development workflow.

My review has identified a critical issue with how CLAUDE.md is implemented—it's a regular file instead of the documented symbolic link, which will likely cause tooling to fail. I've also found a couple of high-severity inconsistencies in the documented cmake commands, which are missing a crucial flag for language server integration. Addressing these points will ensure the new automation and documentation work as intended.

I am having trouble creating individual review comments. Click here to see my feedback.

AGENTS.md (1)

This comment states that CLAUDE.md is a symlink to AGENTS.md. However, the changes in this pull request replace CLAUDE.md with a regular text file containing the string "AGENTS.md", not a symbolic link. This is misleading and may cause agent tooling to fail if it expects a symlink. Please either create a proper symlink or update this comment to reflect the actual file type.

CLAUDE.md (1)

This file appears to be intended as a pointer to AGENTS.md for AI agents, as stated in the comment in AGENTS.md. However, it has been created as a regular text file containing the string "AGENTS.md" instead of a symbolic link. Most tools will read the literal content "AGENTS.md" rather than the content of the AGENTS.md file. To fix this, you should create a symbolic link instead. You can do this by deleting this file, running ln -s AGENTS.md CLAUDE.md, and committing the resulting symlink.

AGENTS.md (107)

The cmake command in this section is missing the -DCMAKE_EXPORT_COMPILE_COMMANDS=ON flag. This is inconsistent with the new documentation about C++ language server integration, which relies on an up-to-date compile_commands.json file. Without this flag, running the tests as described here will not update the compilation database, potentially leading to issues with code intelligence tools like clangd. Please add the flag for consistency and correctness.

cmake -DENABLE_TEST=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..

AGENTS.md (490)

The cmake command in this quick reference is missing the -DCMAKE_EXPORT_COMPILE_COMMANDS=ON flag. Other parts of this document and the new /engine-test skill correctly include this flag to ensure compile_commands.json is kept up-to-date for language server integration. To avoid confusion and ensure the C++ development environment works as expected, this command should also include the flag.

- C++ tests: `cd Source/Engine && mkdir -p build && cd build && cmake -DENABLE_TEST=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .. && make && ctest`

Copilot

Pull request overview

This PR establishes comprehensive development guardrails and automation for the McBopomofo project, derived from lessons learned in PR #779. It consolidates CLAUDE.md into AGENTS.md, adds critical pre-edit checklists, enforces C++17-only development, documents dual build system requirements, and integrates Claude Code automation hooks and skills.

Changes:

Consolidated CLAUDE.md and AGENTS.md documentation with enhanced structure using semantic tags (required, forbidden, context)
Added CRITICAL primacy zone with pre-edit verification checklist and C++17 enforcement with explicit forbidden features list
Documented Claude Code automation (hooks for clang-format and data file protection, skills for engine testing and branch verification)
Added C++ language server integration documentation for clangd and Serena MCP
Updated .gitignore for compile_commands.json and clangd cache

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
CLAUDE.md	Replaced with copy of AGENTS.md content (previously redirected to AGENTS.md)
AGENTS.md	Comprehensive restructure with primacy zone, C++17 enforcement, build system guardrails, PR review protocols, and automation documentation
.gitignore	Added compile_commands.json and .cache for clangd integration
.claude/settings.json	Claude Code hook configuration for auto-formatting and data file protection
.claude/skills/engine-test/SKILL.md	Skill definition for building and running C++ engine tests with compile_commands.json generation
.claude/skills/branch-guard/SKILL.md	Skill definition for branch and worktree verification
.claude/hooks/format-cpp.sh	PostToolUse hook for automatic clang-format on C++/ObjC files
.claude/hooks/block-generated-data.sh	PreToolUse hook to prevent direct edits to generated dictionary files

claude

Security Issue

Found a command injection vulnerability in both hook scripts where unsanitized file paths from JSON could potentially be exploited. Added validation to reject paths with newlines/nulls and used -- separator for clang-format to prevent flag injection.

The rest of the documentation changes look good - the AGENTS.md restructuring with primacy/recency zones, C++17 enforcement, and build system guardrails are all sound additions.

tianjianjiang · 2026-02-15T08:56:13Z

Responses to Gemini Code Assist Review

CLAUDE.md is a regular file, not a symlink (critical)

False positive. CLAUDE.md IS a symlink — ls -la confirms CLAUDE.md -> AGENTS.md. Git stores symlinks as their target path in blobs, so the diff shows the literal text AGENTS.md. The mode change 100644 => 120000 in the diff confirms symlink mode.

cmake commands missing -DCMAKE_EXPORT_COMPILE_COMMANDS=ON (high, x2)

Fixed in e1d7bb9. Both cmake invocations (C++ Engine Tests section and ACTION quick reference) now include the flag, consistent with the Language Server Integration section.

claude

Critical Issues Found

Reviewed for security vulnerabilities, correctness, and potential bugs.

Security Issues (High Priority)

Command injection vulnerability in .claude/hooks/format-cpp.sh - unquoted variable passed to clang-format
Path traversal vulnerability in .claude/hooks/block-generated-data.sh - insufficient path validation

Correctness Issues

Runtime requirement change from macOS 10.15 to 11.0 - verify this matches actual deployment target
Blob reader class name change from KeyValueBlobReader to MemoryMappedFile - verify class exists in codebase

All issues have inline comments with suggested fixes.

claude

Security Issue: Incomplete Path Validation in Hook Scripts

Both hook scripts (.claude/hooks/block-generated-data.sh and .claude/hooks/format-cpp.sh) have incomplete path traversal validation. The current check *".."* doesn't prevent paths outside the project directory or handle symlinks securely.

Impact: Low risk in practice (input comes from Claude Code's trusted tool system), but defense-in-depth is recommended.

Fix: Add project root validation using realpath to ensure all processed paths are within the project directory. See inline comments for specific suggestions.

The rest of the changes (documentation improvements, gitignore updates, skill definitions) look good and align with repository conventions.

claude

Two correctness issues in block-generated-data.sh. The rest of the PR (AGENTS.md restructure, skill/hook additions, .gitignore updates, CLAUDE.md → symlink) looks good. The MemoryMappedFile rename in the C++ Engine section is accurate against the current codebase.

claude

Review of PR #787 (docs + Claude Code tooling). No issues in the AGENTS.md content itself — the guardrails, C++17 enforcement, and build system rules are well-structured. Four issues in the shell hooks and settings worth fixing before merge.

The stack consists of 3 stacked PRs (#779, #780, #781) plus 4 independent PRs (#784, #785, #786, #787) targeting master directly, not "a stack of 6 PRs each building on the previous". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…nd Claude Code automation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

claude

Three issues found in the hook scripts; the AGENTS.md documentation changes look correct.

Critical: The block-generated-data.sh case pattern uses | \ + indented continuation, which embeds leading spaces into the relative-path alternatives — those patterns will never match, so the protection silently fails for relative paths like Source/Data/data.txt. See inline comment for the fix.

The other two findings are in format-cpp.sh: a dead null-byte check and a missing || true that turns any clang-format failure into a noisy hook error on every C++ edit.

claude · 2026-06-10T18:54:53Z

+# Block generated data files in Source/Data/ (absolute or relative paths)
+case "$FILE_PATH" in
+  */Source/Data/data.txt|*/Source/Data/data-plain-bpmf.txt|*/Source/Data/associated-phrases-v2.txt| \
+  Source/Data/data.txt|Source/Data/data-plain-bpmf.txt|Source/Data/associated-phrases-v2.txt)


Bug: line continuation introduces leading whitespace into case patterns, silently breaking relative-path protection.

In bash, \<newline> line continuation removes the backslash and newline but keeps the indentation on the following line. So the patterns for relative paths become Source/Data/data.txt (with two leading spaces) and will never match an actual file path. Only the absolute/wildcard */... variants would fire.

Suggested change

Source/Data/data.txt|Source/Data/data-plain-bpmf.txt|Source/Data/associated-phrases-v2.txt)

# Block generated data files in Source/Data/ (absolute or relative paths)

case "$FILE_PATH" in

*/Source/Data/data.txt|\

*/Source/Data/data-plain-bpmf.txt|\

*/Source/Data/associated-phrases-v2.txt|\

Source/Data/data.txt|\

Source/Data/data-plain-bpmf.txt|\

Source/Data/associated-phrases-v2.txt)

Each alternative must start the continuation line without leading whitespace to avoid embedding spaces into the pattern.

claude · 2026-06-10T18:54:59Z

+
+# Reject paths with newlines, nulls, or path traversal
+if [[ "$FILE_PATH" =~ $'\n' ]] || [[ "$FILE_PATH" =~ $'\0' ]] || [[ "$FILE_PATH" == *".."* ]]; then
+  exit 0


Dead check: bash variables cannot contain null bytes.

$'\0' terminates a shell string; $FILE_PATH can never match it. This check is effectively unreachable and gives a false sense of security.

Suggested change

exit 0

# Reject paths with newlines or path traversal

if [[ "$FILE_PATH" =~ $'\n' ]] || [[ "$FILE_PATH" == *".."* ]]; then

Remove the null-byte check (it mirrors the guard already used in block-generated-data.sh).

claude · 2026-06-10T18:55:06Z

+  *.cpp|*.h|*.mm|*.m)
+    if [[ -f "$FILE_PATH" ]]; then
+      xcrun clang-format -i -- "$FILE_PATH"
+    fi


set -e + xcrun clang-format failure will surface as a hook error on every C++ edit.

With set -euo pipefail active, if xcrun is absent or clang-format exits non-zero (e.g., parse error in a file), the PostToolUse hook exits non-zero and the agent sees an error after every edit. Consider suppressing the exit status so a formatter failure is non-fatal:

Suggested change

fi

if [[ -f "$FILE_PATH" ]]; then

xcrun clang-format -i -- "$FILE_PATH" || true

fi

Copilot AI review requested due to automatic review settings February 15, 2026 07:58

Copilot started reviewing on behalf of tianjianjiang February 15, 2026 07:58 View session

claude Bot reviewed Feb 15, 2026

View reviewed changes

Comment thread .gitignore Outdated

gemini-code-assist Bot reviewed Feb 15, 2026

View reviewed changes

Copilot AI reviewed Feb 15, 2026

View reviewed changes

Comment thread .claude/hooks/format-cpp.sh Outdated

Comment thread AGENTS.md

Comment thread AGENTS.md

Comment thread AGENTS.md

Comment thread AGENTS.md Outdated

Comment thread AGENTS.md

Comment thread .claude/hooks/format-cpp.sh

tianjianjiang self-assigned this Feb 15, 2026

tianjianjiang force-pushed the docs/agents_guardrails branch from bd414b6 to e1d7bb9 Compare February 15, 2026 08:42

claude Bot reviewed Feb 15, 2026

View reviewed changes

Comment thread .claude/hooks/format-cpp.sh

Comment thread .claude/hooks/block-generated-data.sh

tianjianjiang force-pushed the docs/agents_guardrails branch from e1d7bb9 to a520883 Compare February 15, 2026 09:03

claude Bot reviewed Feb 15, 2026

View reviewed changes

Comment thread .claude/hooks/block-generated-data.sh

Comment thread .claude/hooks/format-cpp.sh

Comment thread AGENTS.md

Comment thread AGENTS.md

tianjianjiang force-pushed the docs/agents_guardrails branch from a520883 to 434ab66 Compare February 15, 2026 09:18

claude Bot reviewed Feb 15, 2026

View reviewed changes

Comment thread .claude/hooks/block-generated-data.sh Outdated

Comment thread .claude/hooks/format-cpp.sh

tianjianjiang marked this pull request as draft February 24, 2026 04:55

tianjianjiang force-pushed the docs/agents_guardrails branch from 434ab66 to 24f7046 Compare February 26, 2026 18:02

claude Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread .claude/hooks/block-generated-data.sh Outdated

Comment thread .claude/hooks/block-generated-data.sh

tianjianjiang force-pushed the docs/agents_guardrails branch from 24f7046 to de55c0f Compare February 26, 2026 18:36

claude Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread .claude/hooks/format-cpp.sh

Comment thread .claude/hooks/format-cpp.sh

Comment thread .claude/hooks/block-generated-data.sh

Comment thread .claude/settings.json

Comment thread AGENTS.md

docs(agents): add development guardrails, C++ language server docs, a…

7ad9224

…nd Claude Code automation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tianjianjiang force-pushed the docs/agents_guardrails branch from de55c0f to 7ad9224 Compare June 10, 2026 18:53

claude Bot reviewed Jun 10, 2026

View reviewed changes

-  Source/Data/data.txt|Source/Data/data-plain-bpmf.txt|Source/Data/associated-phrases-v2.txt)
+# Block generated data files in Source/Data/ (absolute or relative paths)
+case "$FILE_PATH" in
+  */Source/Data/data.txt|\
+  */Source/Data/data-plain-bpmf.txt|\
+  */Source/Data/associated-phrases-v2.txt|\
+  Source/Data/data.txt|\
+  Source/Data/data-plain-bpmf.txt|\
+  Source/Data/associated-phrases-v2.txt)

	exit 0
	# Reject paths with newlines or path traversal
	if [[ "$FILE_PATH" =~ $'\n' ]] \|\| [[ "$FILE_PATH" == ".." ]]; then

Uh oh!

Conversation

tianjianjiang commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Critical Issue

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

AGENTS.md (1)

CLAUDE.md (1)

AGENTS.md (107)

AGENTS.md (490)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Security Issue

Uh oh!

Uh oh!

Uh oh!

tianjianjiang commented Feb 15, 2026

Responses to Gemini Code Assist Review

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Critical Issues Found

Security Issues (High Priority)

Correctness Issues

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Security Issue: Incomplete Path Validation in Hook Scripts

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

tianjianjiang commented Feb 15, 2026 •

edited

Loading