Sync Claude annotations from my-claude-code#1
Conversation
This commit syncs the .claude folder from leonj1/my-claude-code to keep Claude Code configurations consistent across repos.
WalkthroughThis PR introduces a comprehensive multi-agent orchestration framework for Claude Code, defining architecture specification, BDD-TDD pipelines, code quality validation, forensic debugging protocols, coding standards across languages, and webhook-based state management to coordinate feature implementation and debugging workflows. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Note 🔄 Code Review #1 in Progress AI-powered code review is currently analyzing your changes. This comment will be updated when the review is complete. |
|
Note 🔄 Code Review #2 in Progress AI-powered code review is currently analyzing your changes. This comment will be updated when the review is complete. |
|
Note 🔄 Code Review #3 in Progress AI-powered code review is currently analyzing your changes. This comment will be updated when the review is complete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @.claude/hooks/post-coder-orchestrator-loop.py:
- Around line 26-33: Add "from __future__ import annotations" to the top of the
imports to defer PEP 585 style annotations; update count_incomplete_features and
any other functions using "tuple[int, list[str]]" to rely on deferred evaluation
rather than changing types. In count_incomplete_features, tighten the regex to
only match documented "Incomplete:" checklist items (e.g., use r"^\s*-\s*\[
\]\s*Incomplete:\s*(.+)$" for the variable that builds the pattern) instead of
matching any unchecked box. Wrap all Path.read_text() calls in try-except and
handle OSError/UnicodeDecodeError by returning/propagating a clear error or an
empty result so the hook doesn't crash on encoding/permission issues (locate
uses in count_incomplete_features and any helper functions that call
Path.read_text()).
In @.claude/hooks/post-coder-standards-check.sh:
- Around line 17-34: Current checks validate SESSION_ID but trust CWD; update
the script to harden CWD before any writes: resolve CWD to an absolute canonical
path (realpath or readlink -f), verify it is a directory (-d), confirm it
contains a ".claude" subdirectory, and ensure the canonical CWD is under the
expected workspace root (for example $PWD or a configured WORKSPACE_ROOT) to
prevent path traversal or writes outside the repo; additionally reject or fail
if the resolved path differs from the original due to symlinks (prevent symlink
traversal) and optionally verify ownership matches the current user (stat/uid)
before proceeding.
In @.claude/hooks/post-test-consistency-validator.sh:
- Around line 24-35: The CWD validation currently only rejects '..' but still
allows absolute or symlinked paths outside the workspace; update the validator
to resolve CWD to its physical path (using pwd -P or realpath) and verify that
the resolved CWD is inside the expected workspace root (or equals `pwd -P`), and
additionally check that the directory contains the required marker `.claude`
(i.e., ensure `$CWD/.claude` exists and is a directory/file); if either check
fails, print a clear error and exit non‑zero.
In @.claude/scripts/crash.py:
- Around line 31-36: The load_state function should gracefully handle corrupt
JSON in STATE_FILE: wrap json.load(f) in a try/except that catches
json.JSONDecodeError (and optionally ValueError), log an error including the
filename and exception (use existing logger or print if none), optionally
move/rename or back up the corrupted STATE_FILE for inspection, and return None
instead of letting the exception propagate; keep the function name load_state
and the STATE_FILE symbol so the change is easy to locate.
🟠 Major comments (16)
.claude/hooks/post-tester-infrastructure.sh-46-50 (1)
46-50: Make the timestamp portable (date -Isecondsis not macOS/BSD compatible) and consider atomic write.
On macOS,date -Isecondstypically fails; withset -ethis will abort the hook.Proposed fix
STATE_DIR="$CWD/.claude/.state" mkdir -p "$STATE_DIR" -echo "$(date -Iseconds)" > "$STATE_DIR/testing-completed-${SESSION_ID}" +NOW="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" +tmp="$(mktemp "${STATE_DIR}/testing-completed-${SESSION_ID}.XXXXXX")" +printf '%s\n' "$NOW" > "$tmp" +mv "$tmp" "$STATE_DIR/testing-completed-${SESSION_ID}".claude/hooks/post-tester-infrastructure.sh-7-16 (1)
7-16: Harden shell safety + JSON parsing (avoid"null"and improve jq failure behavior).
Right now missing keys become"null"(jq-r), and2>&1can smear error text into variables; alsojqisn’t validated as a dependency.Proposed fix
-set -e +set -euo pipefail + +command -v jq >/dev/null 2>&1 || { echo "❌ jq is required for this hook" >&2; exit 1; } # Parse the hook input from stdin HOOK_INPUT=$(cat) # Extract session info -SESSION_ID=$(echo "$HOOK_INPUT" | jq -r '.session_id' 2>&1) || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } -CWD=$(echo "$HOOK_INPUT" | jq -r '.cwd' 2>&1) || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } -SUBAGENT_NAME=$(echo "$HOOK_INPUT" | jq -r '.subagent_name // empty' 2>&1) || { echo "❌ Failed to parse subagent_name from input" >&2; exit 1; } +SESSION_ID=$(jq -er '.session_id' <<<"$HOOK_INPUT") || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } +CWD=$(jq -er '.cwd' <<<"$HOOK_INPUT") || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } +SUBAGENT_NAME=$(jq -r '.subagent_name // ""' <<<"$HOOK_INPUT") || { echo "❌ Failed to parse subagent_name from input" >&2; exit 1; }.claude/hooks/post-tester-infrastructure.sh-17-34 (1)
17-34: Fix validation gaps: reject empty/nullsession_id/cwd; validate CWD is an existing absolute directory.
The current checks allowSESSION_ID="null"andCWD="null"to pass, and allow relative/non-existent CWD values (risking state written to unintended locations).Proposed fix
-# Validate SESSION_ID to prevent path traversal -if [[ "$SESSION_ID" =~ [^a-zA-Z0-9_-] ]]; then - echo "❌ Invalid session_id: contains disallowed characters" >&2 - exit 1 -fi - -# Validate session_id format (alphanumeric, hyphen, underscore only) -if ! [[ "$SESSION_ID" =~ ^[a-zA-Z0-9_-]+$ ]]; then +# Validate session_id format (alphanumeric, hyphen, underscore only) +if [ -z "$SESSION_ID" ] || ! [[ "$SESSION_ID" =~ ^[a-zA-Z0-9_-]+$ ]]; then echo "❌ Invalid session_id format" >&2 exit 1 fi # Validate CWD is not empty -if [ -z "$CWD" ]; then - echo "❌ CWD is empty" >&2 +if [ -z "$CWD" ] || [ "$CWD" = "null" ]; then + echo "❌ CWD is empty/invalid" >&2 + exit 1 +fi +if [[ "$CWD" != /* ]] || [ ! -d "$CWD" ]; then + echo "❌ CWD must be an existing absolute directory: $CWD" >&2 exit 1 fi.claude/hooks/post-standards-testing.sh-29-33 (1)
29-33: Add path traversal validation for CWD.The script validates that
CWDis non-empty but doesn't check for path traversal sequences (..). This is present inpost-bdd-agent.sh(lines 29-33) and should be consistent across hooks.🔒 Proposed security fix
# Validate CWD is not empty if [ -z "$CWD" ]; then echo "❌ CWD is empty" >&2 exit 1 fi + +# Validate CWD does not contain path traversal sequences +if [[ "$CWD" =~ \.\. ]]; then + echo "❌ Invalid CWD: path traversal detected" >&2 + exit 1 +fi.claude/agents/refactorer.md-74-74 (1)
74-74: Use proper heading syntax instead of bold emphasis.Lines 74, 80, 87, 93, 99, and 105 use bold text (
**Step X:**) instead of markdown heading syntax (### Step X). This affects document structure and accessibility.Convert emphasis to proper headings
- **Step A: Structural Refactoring** + ### Step A: Structural RefactoringApply this change to all step headings throughout the workflow sections.
Also applies to: 80-80, 87-87, 93-93, 99-99, 105-105
.claude/agents/refactorer.md-22-31 (1)
22-31: Fix unordered list indentation to meet markdown standards.The list formatting has inconsistent indentation. Unordered lists should use 0 or 2-space indentation, not 3-5 spaces as currently shown.
Fix markdown list indentation
## Environment Detection -**CRITICAL**: Check if the environment variable `TERRAGON` exists and is set to `"true"`. +**CRITICAL**: Check if the environment variable `TERRAGON` exists and is set to `"true"`. -- **When `TERRAGON` is NOT set or is `false`**: Follow the **Standard Environment** workflow (see `stuck-original.md`) -- **When `TERRAGON` is set to `"true"`**: Follow the **Terragon Environment** workflow (see `stuck-terragon.md`) +- **When `TERRAGON` is NOT set or is `false`**: Follow the **Standard Environment** workflow (see `stuck-original.md`) +- **When `TERRAGON` is set to `"true"`**: Follow the **Terragon Environment** workflow (see `stuck-terragon.md`)Apply this pattern throughout the document to normalize list indentation to 0 or 2 spaces.
Also applies to: 35-41, 45-60
.claude/agents/refactorer.md-42-42 (1)
42-42: Specify language for fenced code blocks.Code blocks at lines 42, 131, 206, 215, 222, and 238 are missing language identifiers. This impacts syntax highlighting and renders inconsistently.
Add language specifiers to code blocks
- ``` + ```bash skill: "fastapi-clean-architecture" - ``` + ``` -Provide a detailed refactoring report in this format: - ``` + ```markdown **Refactoring Complete** **Files Refactored**: - ``` + ```Identify each code block's language (bash, python, markdown, makefile, etc.) and add the appropriate specifier.
Also applies to: 131-131, 206-206, 215-215, 222-222, 238-238
.claude/agents/bdd-test-runner.md-145-155 (1)
145-155: Fix Makefile syntax for fallback test commands.Lines 149 and 155 in the Makefile template use shell OR logic (
||) across mutually exclusive commands (pytest, npm, go), which won't work as intended. Thetest-localandtest-bddtargets need language-specific variants, not generic fallbacks.Fix Makefile test target syntax
# Run tests locally (faster, requires local setup) test-local: @echo "Running tests locally..." - # Framework-specific command goes here - pytest -v tests/ || npm test || go test -v ./... + # Framework-specific command - select based on detected language + @if [ -f "requirements.txt" ]; then \ + pytest -v tests/; \ + elif [ -f "package.json" ]; then \ + npm test; \ + elif [ -f "go.mod" ]; then \ + go test -v ./...; \ + else \ + echo "No recognized test framework found"; \ + exit 1; \ + fi # Run BDD tests specifically test-bdd: @echo "Running BDD tests..." - # BDD-specific command - pytest -v tests/bdd/ || npm run test:bdd || go test -v ./tests/bdd/... + @if [ -f "requirements.txt" ]; then \ + pytest -v tests/bdd/; \ + elif [ -f "package.json" ]; then \ + npm run test:bdd; \ + elif [ -f "go.mod" ]; then \ + go test -v ./tests/bdd/...; \ + fiThis ensures the template actually works in any language context rather than failing with syntax errors.
.claude/hooks/skill_discovery_hook.py-8-10 (1)
8-10: Consider validating the SKILL_API_URL scheme.The URL is read from the environment, which could allow unexpected schemes (like
file:). For security, validate thatSKILL_API_URLuseshttps://before making the request.🔒 Suggested URL scheme validation
SKILL_API_URL = os.environ.get( "SKILL_API_URL", "https://external-claude-skills-production.up.railway.app/" ) + +# Validate URL scheme for security +if not SKILL_API_URL.startswith("https://"): + raise ValueError("SKILL_API_URL must use https:// scheme").claude/scripts/crash.py-76-113 (1)
76-113: Validate confidence score range.The function accepts any float value for
confidencewithout validating that it's between 0.0 and 1.0. Invalid values (e.g.,-1,5,100) would be accepted and could produce misleading HIGH/MED/LOW indicators.🔒 Proposed fix to validate confidence range
def log_step(hypothesis, action, confidence): """Log an investigation step with hypothesis and planned action.""" state = load_state() if not state or state['status'] != 'active': print("ERROR: No active CRASH session.") print("Run 'crash.py start \"issue description\"' first.") sys.exit(1) + confidence_float = float(confidence) + if not 0.0 <= confidence_float <= 1.0: + print(f"ERROR: Confidence must be between 0.0 and 1.0, got {confidence_float}") + sys.exit(1) + step_id = len(state['steps']) + 1 new_step = { "id": step_id, "timestamp": datetime.now().isoformat(), "hypothesis": hypothesis, "action": action, - "confidence": float(confidence) + "confidence": confidence_float }.claude/commands/refactor.md-17-32 (1)
17-32: Empty-ARGUMENTS behavior is dangerously broad; consider confirmation gate.“If $ARGUMENTS is empty, refactor all code in the current project” can trigger massive, low-signal diffs; recommend requiring explicit confirmation or switching the default to “analyze + propose plan” when empty.
.claude/hooks/post-gherkin-to-test.sh-7-16 (1)
7-16: Same JSON parsing hardening needed (nullfields + jq dependency + pipefail).As written, missing
session_id/cwdcan become"null"and pass, andset -ewon’t catch pipe failures. Mirror the fixes from the other hooks (set -euo pipefail,jq ... // empty, explicit empty checks,command -v jq)..claude/hooks/post-coder-standards-check.sh-7-16 (1)
7-16: Harden bash mode + dependency/“null” handling for JSON fields.
set -ewithout-u/pipefail+jq -rreturningnullcan silently accept missing fields (SESSION_ID="null",CWD="null").2>&1inside thejqcommand substitutions can leak parse errors into captured values.Proposed patch
-set -e +set -euo pipefail # Parse the hook input from stdin HOOK_INPUT=$(cat) # Extract session info -SESSION_ID=$(echo "$HOOK_INPUT" | jq -r '.session_id' 2>&1) || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } -CWD=$(echo "$HOOK_INPUT" | jq -r '.cwd' 2>&1) || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } -SUBAGENT_NAME=$(echo "$HOOK_INPUT" | jq -r '.subagent_name // empty' 2>&1) || { echo "❌ Failed to parse subagent_name from input" >&2; exit 1; } +command -v jq >/dev/null 2>&1 || { echo "❌ jq is required but not found" >&2; exit 1; } +SESSION_ID=$(echo "$HOOK_INPUT" | jq -r '.session_id // empty' 2>/dev/null) || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } +CWD=$(echo "$HOOK_INPUT" | jq -r '.cwd // empty' 2>/dev/null) || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } +SUBAGENT_NAME=$(echo "$HOOK_INPUT" | jq -r '.subagent_name // empty' 2>/dev/null) || { echo "❌ Failed to parse subagent_name from input" >&2; exit 1; } +if [ -z "$SESSION_ID" ] || [ "$SESSION_ID" = "null" ]; then + echo "❌ session_id is missing" >&2 + exit 1 +fi +if [ -z "$CWD" ] || [ "$CWD" = "null" ]; then + echo "❌ cwd is missing" >&2 + exit 1 +fi.claude/hooks/post-test-consistency-validator.sh-7-17 (1)
7-17: Treat missing JSON fields as errors (avoidSESSION_ID="null"/CWD="null").
jq -r '.session_id'returnsnullwhen absent and your regex allows it, so missingsession_idwill pass validation.Proposed patch
-set -e +set -euo pipefail + +command -v jq >/dev/null 2>&1 || { echo "❌ jq is required but not found" >&2; exit 1; } -SESSION_ID=$(echo "$HOOK_INPUT" | jq -r '.session_id' 2>/dev/null) || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } -CWD=$(echo "$HOOK_INPUT" | jq -r '.cwd' 2>/dev/null) || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } +SESSION_ID=$(echo "$HOOK_INPUT" | jq -r '.session_id // empty' 2>/dev/null) || { echo "❌ Failed to parse session_id from input" >&2; exit 1; } +CWD=$(echo "$HOOK_INPUT" | jq -r '.cwd // empty' 2>/dev/null) || { echo "❌ Failed to parse cwd from input" >&2; exit 1; } SUBAGENT_NAME=$(echo "$HOOK_INPUT" | jq -r '.subagent_name // empty' 2>/dev/null) || { echo "❌ Failed to parse subagent_name from input" >&2; exit 1; } TRANSCRIPT=$(echo "$HOOK_INPUT" | jq -r '.transcript // empty' 2>/dev/null) || TRANSCRIPT="" + +if [ -z "$SESSION_ID" ] || [ "$SESSION_ID" = "null" ]; then + echo "❌ session_id is missing" >&2 + exit 1 +fi +if [ -z "$CWD" ] || [ "$CWD" = "null" ]; then + echo "❌ cwd is missing" >&2 + exit 1 +fi.claude/hooks/post-coder-orchestrator-loop.py-45-51 (1)
45-51:feature_list.mdmatcher is broader than the documented contract; risk of false-positive infinite loop.
Docs say it looks for"[ ] Incomplete"entries, but the regex counts any unchecked list item (- [ ] ...). Iffeature_list.mdcontains unchecked checklists unrelated to “incomplete features”, this will keep forcing exit code 1. Either tighten the regex or update the doc to match behavior.Proposed fix (tighten to “Incomplete:” only)
- # Match "[ ] Incomplete" or just "[ ]" followed by feature text - # Pattern: - [ ] Incomplete: Feature name OR - [ ] Feature name - incomplete_pattern = r"^\s*-\s*\[ \]\s*(?:Incomplete:\s*)?(.+)$" + # Match: - [ ] Incomplete: Feature name + incomplete_pattern = r"^\s*-\s*\[ \]\s*Incomplete:\s*(.+)$" matches = re.findall(incomplete_pattern, content, re.MULTILINE) - return len(matches), matches[:5] # Return first 5 for display + cleaned = [m.strip() for m in matches] + return len(cleaned), cleaned[:5] # Return first 5 for displayAlso applies to: 109-136
.claude/commands/coder.md-1-52 (1)
1-52: Filename mismatch:.claude/commands/coder.mdcontains orchestrator instructions instead of coder role guidance.When invoked as
/coder, this file will instruct the agent to orchestrate tasks rather than implement them—a functional misalignment. A separate.claude/agents/coder-orchestrator.mdfile exists with the proper orchestrator role. Either renamecoder.mdto reflect its orchestrator purpose or move/rewrite its content for the actual coder role.Additionally, lines 113, 150, and 285 contain fenced code blocks without language specifiers, violating MD040 (fenced-code-languages). The repo's markdownlint config (
coderabbit.markdownlint-cli2.jsonc) has MD040 enabled, so these will fail linting checks.
🟡 Minor comments (20)
.claude/agents/code-searcher.md-54-54 (1)
54-54: Add language specifiers to fenced code blocks.Multiple fenced code blocks are missing language specifiers, which reduces readability and prevents proper syntax highlighting. Consider adding appropriate specifiers:
- Lines 54, 128: Use
markdownfor response format templates- Lines 75, 83, 91, 98: Use
regexfor search pattern examples- Line 120: Use
textoryamlfor the example request📝 Proposed fixes
## Response Format -``` +```markdown ## Code Search Results### Python -``` +```regex class \w+Service### TypeScript/JavaScript -``` +```regex class \w+### Go -``` +```regex type \w+ struct### .NET -``` +```regex class \w+## Example Search Request -``` +```text Search for: User authentication service## Example Response -``` +```markdown ## Code Search ResultsAlso applies to: 75-75, 83-83, 91-91, 98-98, 120-120, 128-128
.claude/agents/acceptance-qa.md-47-54 (1)
47-54: Add language specifiers to example code blocks.The fenced code blocks showing example report formats should include
markdownas the language specifier for proper syntax highlighting and rendering.📝 Proposed fixes
#### If PASSED: -```markdown +```markdown # Acceptance QA Report#### If FAILED: -```markdown +```markdown # Acceptance QA ReportAlso applies to: 57-68
.claude/commands/build-and-restart.md-5-5 (1)
5-5: Improve grammar and sentence structure.The description has a minor grammar issue and runs on without proper punctuation, affecting readability.
📝 Proposed fix
-Build and restart this project first check if Makefile exists to use targets in this file, otherwise default by checking the project type to determine how to build and restart it. +Build and restart this project. First check if a Makefile exists to use targets in this file; otherwise, default to checking the project type to determine how to build and restart it..claude/agents/debugger.md-165-193 (1)
165-193: Add language specifier to code fence in example session.Line 165 contains a bare code fence without a language specifier. Per markdown standards (MD040), code blocks should specify a language. Since this example shows session output/interaction flow, consider using
textorplaintextas the language identifier.📝 Suggested fix
## Example Session - ``` + ```plaintext User: "The login API returns 500 errors intermittently".claude/agents/tech-stack-analyzer.md-64-64 (1)
64-64: Convert emphasis formatting to proper headings.Five section labels use bold emphasis (
**...**) when they should be proper markdown headings (###). This applies to: "Language & Runtime" (line 64), "Framework" (line 68), "Test Framework" (line 72), "Build Tools" (line 76), and "Key Dependencies" (line 80).📝 Proposed fixes
- **Language & Runtime** + ### Language & Runtime - **Framework** + ### Framework - **Test Framework** + ### Test Framework - **Build Tools** + ### Build Tools - **Key Dependencies** + ### Key DependenciesAlso applies to: 68-68, 72-72, 76-76, 80-80
.claude/agents/stuck-terragon.md-52-52 (1)
52-52: Add language identifiers to fenced code blocks.Four fenced code blocks are missing language specifications. This is a markdown linting requirement that should be addressed for consistency with documentation standards.
📝 Proposed fixes for code block language specifications
# Line 52 - Build error example - ``` + ```markdown # Line 79 - Test failure example - ``` + ```markdown # Line 107 - Uncertainty example - ``` + ```markdown # Line 176 - Phase 2 response format - ``` + ```markdownAlso applies to: 79-79, 107-107, 176-176
.claude/agents/tech-stack-analyzer.md-27-27 (1)
27-27: Add language identifiers to fenced code blocks.Three fenced code blocks are missing language specifications. Lines 27 and 89 contain manifest file lists, and line 137 contains a markdown example response.
📝 Proposed fixes for code block language specifications
# Line 27 - Manifest file list - ``` + ```text # Line 89 - Output format template - ``` + ```markdown # Line 137 - Example response - ``` + ```markdownAlso applies to: 89-89, 137-137
.claude/agents/forensic.md-21-21 (1)
21-21: Convert emphasis to a proper heading.Line 21 uses emphasis formatting (
**...**) for "CRITICAL: You are in FORENSIC MODE" when this should be a section heading (##) for proper markdown structure.📝 Proposed fix
- **CRITICAL: You are in FORENSIC MODE** + ## CRITICAL: You are in FORENSIC MODE.claude/agents/forensic.md-132-132 (1)
132-132: Add language identifier to code block at line 132.The fenced code block in the evidence collection format example is missing a language specification.
📝 Proposed fix
- ``` + ```markdown **Investigation**: [What was being searched for].claude/coding-standards/python.md-63-63 (1)
63-63: Use heading syntax instead of emphasis for section headers.Line 63 uses bold emphasis (
**Exception: FastAPI Optional Form Parameters**) but should use markdown heading syntax (###) to maintain proper document structure for the coding-standards documentation.Fix markdown heading syntax
-**Exception: FastAPI Optional Form Parameters** +### Exception: FastAPI Optional Form Parameters.claude/coding-standards/python.md-590-592 (1)
590-592: Specify language identifier for code block.The code block at line 590 is missing a language identifier. This helps with syntax highlighting and follows markdown best practices. The block appears to reference a skill invocation with YAML-like key-value syntax.
Add language identifier to code block
-``` +```yaml skill: "fastapi-clean-architecture" -``` +```.claude/scripts/crash.py-156-201 (1)
156-201: Add error handling for file removal in diagnose.Similar to
cancel_session, theos.remove()call can fail due to permission issues or if the file has already been deleted. Adding error handling ensures graceful degradation.🛡️ Proposed fix for robust file removal
print("---") print("") print("Session Complete. Write/Edit tools are now re-enabled.") - os.remove(STATE_FILE) + try: + os.remove(STATE_FILE) + except OSError as e: + print(f"WARNING: Could not remove state file: {e}", file=sys.stderr).claude/scripts/crash.py-142-153 (1)
142-153: Add error handling for file removal.
os.remove()can raiseOSErrorif the file doesn't exist or due to permission issues. While unlikely in normal usage, this could cause an unexpected crash.🛡️ Proposed fix for robust file removal
def cancel_session(): """Cancel the current session without diagnosis.""" state = load_state() if not state: print("No active session to cancel.") return session_id = state['session_id'] if os.path.exists(STATE_FILE): - os.remove(STATE_FILE) + try: + os.remove(STATE_FILE) + except OSError as e: + print(f"WARNING: Could not remove state file: {e}", file=sys.stderr) print(f"Session #{session_id} cancelled.") print("Write/Edit tools are now re-enabled.").claude/hooks/post-gherkin-to-test.sh-52-57 (1)
52-57: Comment doesn’t match behavior (filename pattern vs file content).The comment claims content-based detection (“files containing executor: bdd”), but the code only matches
*-bdd-*filenames. Either update the comment or actually grep file content..claude/agents/coding-standards-checker.md-213-217 (1)
213-217: Line 217 needs shell portability clarification; line 214 pattern is correct.Line 217's
**/*.{py,ts,js,go}glob requiresshopt -s globstar(bash) or zsh and won't work in basic sh. For portability, recommend usingfind . -name "*.py" -o -name "*.ts" -o -name "*.js" -o -name "*.go"or clarifying the shell requirement.Line 214's pattern
def.*=.*:works as intended—it correctly identifies default arguments without false positives from comments or docstrings on separate lines..claude/config.json-10-15 (1)
10-15: Consolidate hook paths to repository-relative format.Line 13 references
~/.claude/hooks/skill_discovery_hook.pyusing home directory expansion, while all other hooks in this config use repository-relative paths like.claude/hooks/.... This file does not exist in the repository, creating an external system dependency.Ensure the path uses the same repository-relative format as other hooks:
.claude/hooks/skill_discovery_hook.py..claude/commands/architect.md-37-41 (1)
37-41: Fix minor doc inconsistencies: filename + duplicate step number.
.feature_list.md.examplelooks like a typo (leading dot).- “### 6.” is used twice; makes referencing steps harder during execution.
Proposed fix
-1. Create .feature_list.md.example if feature_list.md doesn't exist +1. Create feature_list.md.example if feature_list.md doesn't exist @@ -### 6. **Completion & Continuation** +### 7. **Completion & Continuation**Also applies to: 148-166
.claude/hooks/post-coder-orchestrator-loop.py-39-44 (1)
39-44: Fail-open on file read errors (and prefer explicit UTF-8).
read_text()can raise (permissions, encoding). A hook crash here is worse than skipping the check. Considerencoding="utf-8"and catchingOSError/UnicodeDecodeErrorto return(0, [])(continue).Proposed fix
- content = feature_file.read_text() + try: + content = feature_file.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return 0, [] @@ - content = digest_file.read_text() + try: + content = digest_file.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return 0, []Also applies to: 60-65
.claude/commands/coder.md-113-146 (1)
113-146: Fix markdownlint MD040: add languages to fenced code blocks.
This is flagged by static analysis and is easy to address.Proposed fix
-``` +```text User: "Build a React todo app" @@ -``` +```text USER gives project @@ -``` +```text coder completes → SubagentStop eventAlso applies to: 150-188, 285-307
.claude/agents/gherkin-to-test.md-181-197 (1)
181-197: Example prompt filenames use.featureextension (but prompt format is.md).
This reads like a copy/paste slip and can confuse the orchestrator’s “report prompt numbers” step.Proposed doc fix
-006-bdd-user-registration.feature → 006 (foundation) -007-bdd-user-authentication.feature → 007 (depends on registration) -008-bdd-password-reset.feature → 008 (depends on auth) +006-bdd-user-registration.md → 006 (foundation) +007-bdd-user-authentication.md → 007 (depends on registration) +008-bdd-password-reset.md → 008 (depends on auth)
| import json | ||
| import re | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| def count_incomplete_features(cwd: str) -> tuple[int, list[str]]: | ||
| """ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the full file to see the annotations and context
cat -n ".claude/hooks/post-coder-orchestrator-loop.py"Repository: leonj1/3camels
Length of output: 5789
🏁 Script executed:
# Check for Python version requirements in the project
find . -maxdepth 2 -type f \( -name "*.toml" -o -name "*.cfg" -o -name "*.yaml" -o -name "*.yml" -o -name "setup.py" -o -name ".python-version" \) | head -20Repository: leonj1/3camels
Length of output: 69
🏁 Script executed:
# Look for any CI/workflow files that might specify Python version
find . -path "*/.github/workflows/*" -type f -o -path "*/\.circleci/*" -type f 2>/dev/null | head -10Repository: leonj1/3camels
Length of output: 40
🏁 Script executed:
# Check ruff.toml for Python version requirements
cat ruff.tomlRepository: leonj1/3camels
Length of output: 964
🏁 Script executed:
# Also check pyproject.toml or any other config files
ls -la | grep -E "(pyproject|setup|requirements|python)"
cat pyproject.toml 2>/dev/null || echo "No pyproject.toml"Repository: leonj1/3camels
Length of output: 144
🏁 Script executed:
# Check if __future__ annotations is used anywhere in the project
rg "from __future__ import annotations" --type py | head -20Repository: leonj1/3camels
Length of output: 206
🏁 Script executed:
# Search for Python version specification more broadly
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.toml" -o -name "*.yaml" \) -exec grep -l "python" {} \; | head -10Repository: leonj1/3camels
Length of output: 144
🏁 Script executed:
# Check README or similar for Python requirements
head -50 README.md 2>/dev/null || head -50 readme.md 2>/dev/null || echo "No README found"Repository: leonj1/3camels
Length of output: 67
🏁 Script executed:
# Verify the annotation syntax is actually on lines 32 and 53
grep -n "tuple\[" ".claude/hooks/post-coder-orchestrator-loop.py"Repository: leonj1/3camels
Length of output: 189
🏁 Script executed:
# Check if there's any Python version requirement documentation
find . -maxdepth 3 -type f -name "*.md" -exec grep -l "python.*3\.[0-9]" {} \; 2>/dev/null | head -5Repository: leonj1/3camels
Length of output: 93
🏁 Script executed:
# Let's check what Python version the project actually targets
# Look for any .python-version files or GitHub Actions workflows
find . -name ".python-version" -type fRepository: leonj1/3camels
Length of output: 40
🏁 Script executed:
# Check if there are any workflows that run tests
find . -type f \( -name "*.yml" -o -name "*.yaml" \) | head -10Repository: leonj1/3camels
Length of output: 184
🏁 Script executed:
# Verify the regex pattern behavior - does it match ANY unchecked item or only "Incomplete:"?
# The pattern is: r"^\s*-\s*\[ \]\s*(?:Incomplete:\s*)?(.+)$"
# Let's verify the docstring claim
grep -A 5 "Looks for" ".claude/hooks/post-coder-orchestrator-loop.py"Repository: leonj1/3camels
Length of output: 243
🏁 Script executed:
# Check error handling for Path.read_text() calls
grep -B 2 -A 2 "read_text()" ".claude/hooks/post-coder-orchestrator-loop.py"Repository: leonj1/3camels
Length of output: 292
🏁 Script executed:
# Let's check if there's any indication of Python 3.8 support in the project
# Search for any setup.py, pyproject.toml, or tox.ini
ls -la setup.py setup.cfg pyproject.toml tox.ini 2>/dev/null || echo "No standard config found"Repository: leonj1/3camels
Length of output: 82
🏁 Script executed:
# Check the actual docstring claim vs regex behavior
# The docstring says "Looks for '[ ] Incomplete' entries"
# But the regex makes "Incomplete:" optional
# Let's confirm what the comment says in the regex
sed -n '45,48p' ".claude/hooks/post-coder-orchestrator-loop.py"Repository: leonj1/3camels
Length of output: 330
🏁 Script executed:
# Double-check: does the regex actually match ALL [ ] items or only those with Incomplete:?
python3 << 'EOF'
import re
# The pattern from the file
pattern = r"^\s*-\s*\[ \]\s*(?:Incomplete:\s*)?(.+)$"
# Test cases
test_strings = [
"- [ ] Incomplete: Feature X",
"- [ ] Some random unchecked item",
"- [ ] Bug fix needed",
"- [x] Completed feature"
]
for test in test_strings:
match = re.search(pattern, test, re.MULTILINE)
print(f"Pattern matches '{test}': {bool(match)}")
EOFRepository: leonj1/3camels
Length of output: 260
Fix PEP 585 annotation incompatibility and regex over-matching for hook reliability.
The tuple[int, list[str]] annotations (lines 32, 53) require Python 3.9+ and will crash at import time on Python 3.8, breaking the entire hook. Additionally, the regex pattern at line 47 matches any unchecked item ([ ]), not only "Incomplete:" entries as documented, which can cause the hook to perpetually re-trigger on legitimate incomplete tasks. Finally, Path.read_text() calls lack exception handling for encoding or permission errors.
Add from __future__ import annotations at the top of imports to defer annotation evaluation, make the regex explicitly require "Incomplete:" prefix (or document the broader matching), and wrap read_text() calls in try-except to handle IO errors gracefully:
Proposed changes
#!/usr/bin/env python3
"""
Coder Orchestrator Loop Hook
@@ -23,6 +23,7 @@ Output format:
- If tasks remain: {"error": "..."} with exit code 1
- If complete: {"continue": true, "systemMessage": "..."} with exit code 0
"""
+from __future__ import annotations
import json
import re
import sys
@@ -40,7 +41,12 @@ def count_incomplete_features(cwd: str) -> tuple[int, list[str]]:
feature_file = Path(cwd) / "feature_list.md"
if not feature_file.exists():
return 0, []
- content = feature_file.read_text()
+ try:
+ content = feature_file.read_text()
+ except (UnicodeDecodeError, PermissionError, OSError):
+ # If file is unreadable, assume no incomplete features
+ return 0, []Also update regex to r"^\s*-\s*\[ \]\s*Incomplete:\s*(.+)$" (line 47) to match documented behavior, or update docstring to reflect that all unchecked items are flagged.
🤖 Prompt for AI Agents
In @.claude/hooks/post-coder-orchestrator-loop.py around lines 26 - 33, Add
"from __future__ import annotations" to the top of the imports to defer PEP 585
style annotations; update count_incomplete_features and any other functions
using "tuple[int, list[str]]" to rely on deferred evaluation rather than
changing types. In count_incomplete_features, tighten the regex to only match
documented "Incomplete:" checklist items (e.g., use r"^\s*-\s*\[
\]\s*Incomplete:\s*(.+)$" for the variable that builds the pattern) instead of
matching any unchecked box. Wrap all Path.read_text() calls in try-except and
handle OSError/UnicodeDecodeError by returning/propagating a clear error or an
empty result so the hook doesn't crash on encoding/permission issues (locate
uses in count_incomplete_features and any helper functions that call
Path.read_text()).
| # Validate SESSION_ID to prevent path traversal | ||
| if [[ "$SESSION_ID" =~ [^a-zA-Z0-9_-] ]]; then | ||
| echo "❌ Invalid session_id: contains disallowed characters" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate session_id format (alphanumeric, hyphen, underscore only) | ||
| if ! [[ "$SESSION_ID" =~ ^[a-zA-Z0-9_-]+$ ]]; then | ||
| echo "❌ Invalid session_id format" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate CWD is not empty | ||
| if [ -z "$CWD" ]; then | ||
| echo "❌ CWD is empty" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
CWD is trusted input; add traversal + realpath/ownership checks (avoid arbitrary file writes).
Right now any caller controlling .cwd can steer the state-file write. At minimum, ensure CWD is a directory, contains .claude/, and resolves inside the current working directory (or inside a configured workspace root).
Proposed patch (minimal safety checks)
-# Validate CWD is not empty
-if [ -z "$CWD" ]; then
- echo "❌ CWD is empty" >&2
- exit 1
-fi
+# Validate CWD is a safe directory
+if [[ "$CWD" =~ \.\. ]]; then
+ echo "❌ Invalid CWD: path traversal detected" >&2
+ exit 1
+fi
+if [ ! -d "$CWD" ]; then
+ echo "❌ Invalid CWD: not a directory" >&2
+ exit 1
+fi
+if [ ! -d "$CWD/.claude" ]; then
+ echo "❌ Invalid CWD: missing .claude directory" >&2
+ exit 1
+fi
+# Optional: pin writes to the *actual* current directory
+if [ "$(cd "$CWD" && pwd -P)" != "$(pwd -P)" ]; then
+ echo "❌ Invalid CWD: does not match current working directory" >&2
+ exit 1
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Validate SESSION_ID to prevent path traversal | |
| if [[ "$SESSION_ID" =~ [^a-zA-Z0-9_-] ]]; then | |
| echo "❌ Invalid session_id: contains disallowed characters" >&2 | |
| exit 1 | |
| fi | |
| # Validate session_id format (alphanumeric, hyphen, underscore only) | |
| if ! [[ "$SESSION_ID" =~ ^[a-zA-Z0-9_-]+$ ]]; then | |
| echo "❌ Invalid session_id format" >&2 | |
| exit 1 | |
| fi | |
| # Validate CWD is not empty | |
| if [ -z "$CWD" ]; then | |
| echo "❌ CWD is empty" >&2 | |
| exit 1 | |
| fi | |
| # Validate SESSION_ID to prevent path traversal | |
| if [[ "$SESSION_ID" =~ [^a-zA-Z0-9_-] ]]; then | |
| echo "❌ Invalid session_id: contains disallowed characters" >&2 | |
| exit 1 | |
| fi | |
| # Validate session_id format (alphanumeric, hyphen, underscore only) | |
| if ! [[ "$SESSION_ID" =~ ^[a-zA-Z0-9_-]+$ ]]; then | |
| echo "❌ Invalid session_id format" >&2 | |
| exit 1 | |
| fi | |
| # Validate CWD is a safe directory | |
| if [[ "$CWD" =~ \.\. ]]; then | |
| echo "❌ Invalid CWD: path traversal detected" >&2 | |
| exit 1 | |
| fi | |
| if [ ! -d "$CWD" ]; then | |
| echo "❌ Invalid CWD: not a directory" >&2 | |
| exit 1 | |
| fi | |
| if [ ! -d "$CWD/.claude" ]; then | |
| echo "❌ Invalid CWD: missing .claude directory" >&2 | |
| exit 1 | |
| fi | |
| # Optional: pin writes to the *actual* current directory | |
| if [ "$(cd "$CWD" && pwd -P)" != "$(pwd -P)" ]; then | |
| echo "❌ Invalid CWD: does not match current working directory" >&2 | |
| exit 1 | |
| fi | |
🤖 Prompt for AI Agents
In @.claude/hooks/post-coder-standards-check.sh around lines 17 - 34, Current
checks validate SESSION_ID but trust CWD; update the script to harden CWD before
any writes: resolve CWD to an absolute canonical path (realpath or readlink -f),
verify it is a directory (-d), confirm it contains a ".claude" subdirectory, and
ensure the canonical CWD is under the expected workspace root (for example $PWD
or a configured WORKSPACE_ROOT) to prevent path traversal or writes outside the
repo; additionally reject or fail if the resolved path differs from the original
due to symlinks (prevent symlink traversal) and optionally verify ownership
matches the current user (stat/uid) before proceeding.
| # Validate CWD is not empty | ||
| if [ -z "$CWD" ]; then | ||
| echo "CWD is empty" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate CWD does not contain path traversal sequences | ||
| if [[ "$CWD" =~ \.\. ]]; then | ||
| echo "Invalid CWD: path traversal detected" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
CWD validation still enables redirected writes; pin to workspace/current dir.
Same core issue: .cwd is trusted. Consider verifying CWD equals pwd -P (or is under an allowed root) and contains .claude/ before writing under $CWD/.claude/.state.
| # Validate CWD is not empty | ||
| if [ -z "$CWD" ]; then | ||
| echo "❌ CWD is empty" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate CWD does not contain path traversal sequences | ||
| if [[ "$CWD" =~ \.\. ]]; then | ||
| echo "❌ Invalid CWD: path traversal detected" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
CWD validation still allows writing outside the repo/workspace.
Rejecting .. helps, but an absolute path like /tmp/somewhere (or a symlinked dir) can still be used to redirect state writes. Consider verifying CWD matches pwd -P (or realpath is under a known workspace root) and that $CWD/.claude exists.
🤖 Prompt for AI Agents
In @.claude/hooks/post-test-consistency-validator.sh around lines 24 - 35, The
CWD validation currently only rejects '..' but still allows absolute or
symlinked paths outside the workspace; update the validator to resolve CWD to
its physical path (using pwd -P or realpath) and verify that the resolved CWD is
inside the expected workspace root (or equals `pwd -P`), and additionally check
that the directory contains the required marker `.claude` (i.e., ensure
`$CWD/.claude` exists and is a directory/file); if either check fails, print a
clear error and exit non‑zero.
| def load_state(): | ||
| """Load the current session state from disk.""" | ||
| if os.path.exists(STATE_FILE): | ||
| with open(STATE_FILE, 'r') as f: | ||
| return json.load(f) | ||
| return None |
There was a problem hiding this comment.
Add error handling for corrupt JSON state file.
If the state file exists but contains invalid JSON, json.load() will raise an unhandled JSONDecodeError, causing the script to crash. This could happen if the file is corrupted or manually edited.
🛡️ Proposed fix to handle JSON decode errors
def load_state():
"""Load the current session state from disk."""
if os.path.exists(STATE_FILE):
- with open(STATE_FILE, 'r') as f:
- return json.load(f)
+ try:
+ with open(STATE_FILE, 'r') as f:
+ return json.load(f)
+ except json.JSONDecodeError:
+ print(f"ERROR: State file {STATE_FILE} is corrupted.", file=sys.stderr)
+ print("Use 'crash.py cancel' to reset, or manually delete the file.", file=sys.stderr)
+ sys.exit(1)
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def load_state(): | |
| """Load the current session state from disk.""" | |
| if os.path.exists(STATE_FILE): | |
| with open(STATE_FILE, 'r') as f: | |
| return json.load(f) | |
| return None | |
| def load_state(): | |
| """Load the current session state from disk.""" | |
| if os.path.exists(STATE_FILE): | |
| try: | |
| with open(STATE_FILE, 'r') as f: | |
| return json.load(f) | |
| except json.JSONDecodeError: | |
| print(f"ERROR: State file {STATE_FILE} is corrupted.", file=sys.stderr) | |
| print("Use 'crash.py cancel' to reset, or manually delete the file.", file=sys.stderr) | |
| sys.exit(1) | |
| return None |
🤖 Prompt for AI Agents
In @.claude/scripts/crash.py around lines 31 - 36, The load_state function
should gracefully handle corrupt JSON in STATE_FILE: wrap json.load(f) in a
try/except that catches json.JSONDecodeError (and optionally ValueError), log an
error including the filename and exception (use existing logger or print if
none), optionally move/rename or back up the corrupted STATE_FILE for
inspection, and return None instead of letting the exception propagate; keep the
function name load_state and the STATE_FILE symbol so the change is easy to
locate.
1 participant
Summary
This PR syncs the
.claudefolder from leonj1/my-claude-code to keep Claude Code configurations consistent across repositories.Changes
.claude/folder contents to match the source repositoryThis PR was automatically generated by the sync-claude-annotations script.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.