-
Notifications
You must be signed in to change notification settings - Fork 31
Description
Summary
This issue tracks the remaining 9 recommendations from the Claude Code review of PR #337 (Amber codebase agent introduction). These are deferred improvements - not blocking for merge, but valuable for long-term quality.
Completed in PR #337:
- ✅ Constitution validation grep patterns fixed
- ✅ Langfuse version tracking fixed
- ✅ Stale dates updated (2025-11-16 → 2025-11-17)
- ✅ Authority hierarchy examples added
- ✅ TodoWrite safety gates documented
This issue covers remaining items:
🟡 Major Issues (Deferred)
4. Validate Mermaid Diagram Complexity
Current State: 5 Mermaid diagrams added to docs, implementation plan sets 10-box limit
Issue: Need to validate all diagrams comply with the ≤10 box/step limit for readability
Action Items:
- Review each Mermaid diagram manually or with mermaid.live
- Count decision boxes/steps in each diagram
- Simplify any diagrams exceeding 10 steps
- Document in implementation plan:
grep 'participant\|rect\|alt' <file> | wc -l
Files to Check:
docs/user-guide/working-with-amber.md(4 diagrams)agents/amber.md(1 flowchart)
Estimated Effort: 30 minutes
7. Improve Dependency Sync Error Handling
Current State: scripts/sync-amber-dependencies.py has minimal error output
Issue: Silent failures make debugging difficult when sync breaks
Improvement Opportunities:
- Add structured logging with severity levels (INFO/WARN/ERROR)
- Print file paths being parsed (debug mode)
- Explicit error messages for missing files
- Validation warnings for unexpected dependency formats
- Exit codes: 0=success, 1=parse error, 2=file write error
Example:
import logging
logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)
try:
with open(pyproject_path) as f:
data = tomllib.loads(f.read())
except FileNotFoundError:
logger.error(f"pyproject.toml not found: {pyproject_path}")
sys.exit(1)Estimated Effort: 45 minutes
8. Strengthen Auto-Generated Section Markers
Current State: <!-- AUTO-GENERATED: Dependencies --> markers rely on exact match
Issue: Manual edits to markers break automation silently
Proposed Enhancement:
- Add validation step before sync: check markers exist and are unmodified
- Fail fast with clear error if markers are missing or edited
- Include marker format in script header comment
- Consider using UUID in markers for tamper detection
Example:
# Before updating content
if not re.search(r'<!-- AUTO-GENERATED: Dependencies.*-->', content):
logger.error("AUTO-GENERATED marker not found or modified")
logger.error("Expected: <!-- AUTO-GENERATED: Dependencies - Last updated: YYYY-MM-DD -->")
sys.exit(1)Estimated Effort: 30 minutes
🔵 Minor Issues (Low Priority)
9. Inconsistent Terminology ("Amber" vs "Amber agent")
Issue: Minor readability inconsistency in docs
Action: Find/replace "Amber agent" → "Amber" for consistency (or vice versa, establish convention)
Files: docs/user-guide/working-with-amber.md, agents/amber.md
Estimated Effort: 20 minutes
10. Workflow Permissions Over-Scoped
Current State: .github/workflows/amber-dependency-sync.yml has issues: write
Issue: Violates least-privilege principle (workflow only creates issues, doesn't update them)
Security Best Practice: Remove unused permissions
Action:
permissions:
contents: write # For committing dependency updates
pull-requests: write # For creating PRs (if needed)
issues: write # REVIEW: Only if filing violation issues, otherwise removeEstimated Effort: 30 minutes (requires testing workflow still works)
11. Missing Rollback Documentation
Issue: Dependency sync changes could introduce errors, no documented rollback
Proposed Addition:
Add comment to scripts/sync-amber-dependencies.py header:
"""
Rollback procedure if sync introduces errors:
1. git log agents/amber.md # Find last good commit
2. git checkout <commit> agents/amber.md
3. git commit -m "Revert dependency sync from <date>"
4. Investigate sync script error, fix, re-run
"""Estimated Effort: 10 minutes
12. Python Version Not in Shebang
Issue: Script uses tomllib (Python 3.11+) but shebang is generic
Current: #!/usr/bin/env python3
Better: #!/usr/bin/env python3.11 (or document in script header)
Alternative: Add version check at script start:
import sys
if sys.version_info < (3, 11):
print("Error: Python 3.11+ required (uses tomllib)", file=sys.stderr)
sys.exit(1)Estimated Effort: 5 minutes
13. No Markdownlint in CI
Issue: Documentation quality not auto-validated in CI pipeline
Proposed Addition:
Add step to .github/workflows/ (e.g., docs-lint.yml or existing workflow):
- name: Lint documentation
run: |
npm install -g markdownlint-cli
markdownlint 'docs/**/*.md' 'agents/*.md' --config .markdownlint.jsonNote: Implementation plan mentions this as future work
Estimated Effort: 45 minutes (CI setup + fixing existing lint errors)
14. Commit Message Line Length
Issue: PR commit messages exceed 72-char Git convention (minor)
Example: Git log readability suffers in terminal git log --oneline
Fix: Use multi-line commit messages:
fix: improve dependency sync error handling
- Add structured logging with INFO/WARN/ERROR levels
- Print file paths in debug mode
- Explicit errors for missing files
Not Blocking: Low priority, cosmetic Git convention
Estimated Effort: 5 minutes (awareness for future commits)
Total Estimated Effort
- Major (3): 1 hour 45 minutes
- Minor (6): 1 hour 55 minutes
- Grand Total: ~3.5 hours
Priority Recommendation
Phase 1 (Quality Sprint - 2 hours):
- Epic: AI Agent Development #4 (Mermaid diagrams)
- Test: Automation Workflow Validation #7 (Error handling)
- Test: Updated Workflow Validation #8 (Marker validation)
Phase 2 (Security Audit - 1 hour):
- Bump actions/checkout from 4 to 5 #10 (Permissions)
- Review context7 MCP server and patternfly team usage #13 (CI linting)
Phase 3 (Polish - 30 minutes):
- Bump actions/add-to-project from 0.5.0 to 1.0.2 #9, Bump actions/github-script from 6 to 7 #11, Add vTeam shared Claude Code configuration with hooks-based enforcement #12, Add OpenShift AI Virtual Team agents as source of truth #14
Labels
documentationenhancementgood-first-issue(for Bump actions/add-to-project from 0.5.0 to 1.0.2 #9, Bump actions/github-script from 6 to 7 #11, Add vTeam shared Claude Code configuration with hooks-based enforcement #12, Add OpenShift AI Virtual Team agents as source of truth #14)security(for Bump actions/checkout from 4 to 5 #10)
Related: PR #337