8.0

.claude/agent-memory/code-improvement-reviewer/MEMORY.md

@@ -0,0 +1,40 @@
+# Menu Bundle - Code Review Memory
+
+## Architecture
+- Namespace: `ChamberOrchestra\MenuBundle` (PSR-4 from package root, no src/)
+- PHP ^8.5, ext-ds required
+- Legacy alias bundle: `DevMenuBundle` (BC alias for `ChamberOrchestraMenuBundle`)
+- No test suite present as of Feb 2026
+
+## Key Patterns
+- Navigation: implement `NavigationInterface::build()`, auto-tagged `chamber_orchestra_menu.navigation`
+- Factory: `Factory` applies `ExtensionInterface` plugins sorted by priority (krsort = higher int = higher priority)
+- `CoreExtension` is priority -10 (runs last, sets defaults like uri/extras/current/attributes)
+- Caching: `AbstractCachedNavigation` uses Symfony Cache Contracts, 24h TTL, tag `navigation`
+- Matching: voter-based `SplObjectStorage` cache; `RouteVoter` treats route values as regex patterns
+- Access: `Ds\Map` caches `isGranted()` results per item object; role grants cached in plain array too
+
+## Known Issues Found (Feb 2026 review)
+See `patterns.md` for full details. Summary:
+- CRITICAL: `Matcher::isCurrent()` uses uninitialized variable `$current` — PHP TypeError if no voters
+- HIGH: `Accessor::hasAccessToChildren()` logic is wrong — returns false if ANY child is denied (should be: returns true only if AT LEAST ONE child is accessible for "show section" use-case)
+- HIGH: `RouteVoter::matchItem()` crashes with NPE if no current request (null check missing before `->attributes->get()`)
+- HIGH: `Item::getFirstChild()`/`getLastChild()` return `false` (not `null`) when collection is empty — violates `?ItemInterface` return type, causes TypeError
+- HIGH: `MenuBuilder::children()` calls `getLastChild()` which returns false — assigns false to `$this->current`, causing crash on next `add()` call
+- HIGH: `Item::serialize()`/`unserialize()` uses deprecated `Serializable` interface; `$section` not included in serialized data (lost after deserialization)
+- MEDIUM: `NavigationFactory::create()` takes untyped `$nav` parameter — should be `NavigationInterface|string`
+- MEDIUM: `MenuBuilder::end()` silently fails — if called with no parents, `array_pop` returns null and assigns null to `$this->current`
+- MEDIUM: `AbstractCachedNavigation::configureCacheItem()` does not set cache tags despite having `$this->cache['tags']`
+- MEDIUM: `Factory::addExtension()` has no return type declared
+- MEDIUM: `LabelExtension` uses `#[Required]` for optional translator injection — NPE if translator not set and translation_domain is used
+- MEDIUM: `TwigRenderer::render()` options merge order — user `$options` can be silently overridden by root/matcher/accessor keys
+
+## File Locations
+- `NavigationFactory.php` — orchestration, caching logic, $built instance cache
+- `Menu/MenuBuilder.php` — fluent tree builder (children/end state machine)
+- `Menu/Item.php` — tree node, uses ArrayCollection, implements deprecated Serializable
+- `Matcher/Matcher.php` — SplObjectStorage cache, voter chain (uninitialized $current bug)
+- `Matcher/Voter/RouteVoter.php` — regex route matching, fetches request twice
+- `Accessor/Accessor.php` — Ds\Map + array dual cache for role checks
+- `Factory/Factory.php` — priority-sorted extension chain (krsort)
+- `Resources/config/services.yml` — tagged_iterator wiring, instance tags

.claude/agent-memory/code-improvement-reviewer/patterns.md

@@ -0,0 +1,124 @@
+# Menu Bundle - Detailed Issue Notes
+
+## Critical Bugs
+
+### Matcher::isCurrent() uninitialized variable
+File: `Matcher/Matcher.php:43`
+If the `$voters` iterable is empty OR all voters return null, the loop never assigns `$current`.
+Line 43 `$current = (bool) $current;` then reads an uninitialized variable.
+In PHP 8 strict mode this raises a TypeError / notice that evaluates $current as null→false.
+The result is silently cached as `false`, which is usually correct but masks the bug.
+
+### Item::getFirstChild() / getLastChild() return false, not null
+File: `Menu/Item.php:74-82`
+`ArrayCollection::first()` and `::last()` return `false` when the collection is empty, but the
+return type annotation is `?ItemInterface`. PHP doesn't enforce this at runtime for object returns
+but callers expecting null will receive false. In MenuBuilder::children() line 31, this false is
+assigned to `$this->current`, causing the next `add()` call to crash on `$this->current->add()`.
+
+## High Priority Bugs
+
+### RouteVoter null-request crash
+File: `Matcher/Voter/RouteVoter.php:18`
+`$this->stack->getCurrentRequest()` can return null (CLI, sub-requests, test contexts).
+Line 18 calls `->attributes->get('_route')` directly on the result without a null check.
+This throws an Error in non-HTTP contexts.
+
+### Item serialization loses $section field
+File: `Menu/Item.php:99-117`
+`$this->section` is never included in the serialized array, so after deserialization `isSection()`
+always returns false. Also, the `Serializable` interface is deprecated in PHP 8.1+ in favor of
+`__serialize()`/`__unserialize()`.
+
+### MenuBuilder::end() silent null assignment
+File: `Menu/MenuBuilder.php:38-41`
+If `end()` is called more times than `children()`, `array_pop($this->parents)` returns null
+and `$this->current` is set to null. Subsequent `add()` calls will throw a null dereference.
+
+### Accessor::hasAccessToChildren() semantics wrong
+File: `Accessor/Accessor.php:25-34`
+The method returns false as soon as ANY child is inaccessible. The likely intended use in templates
+is "does this section have any accessible children (should we render the section heading)?".
+The current logic is AND — returns true only if ALL children are accessible.
+This can completely hide menu sections that have a mix of accessible and restricted items.
+
+## Medium Priority Issues
+
+### NavigationFactory::create() untyped $nav parameter
+File: `NavigationFactory.php:34`
+Signature: `public function create($nav, array $options): Menu\ItemInterface`
+The $nav parameter should be typed as `NavigationInterface|string`.
+
+### AbstractCachedNavigation cache tags not applied
+File: `Navigation/AbstractCachedNavigation.php:26-29`
+`configureCacheItem()` calls `$item->expiresAfter()` but never calls `$item->tag($this->cache['tags'])`.
+Cache invalidation via `$cachePool->invalidateTags(['navigation'])` therefore has no effect.
+
+### Factory::addExtension() missing return type
+File: `Factory/Factory.php:26`
+`public function addExtension(ExtensionInterface $extension, int $priority = 0)` — no return type.
+Should be `: void`.
+
+### LabelExtension #[Required] with nullable property
+File: `Factory/Extension/LabelExtension.php:12-17`
+Uses `#[Required]` attribute meaning Symfony will inject the translator via setter, but the property
+is declared `protected ?TranslatorInterface $translator = null`. If the service container somehow
+skips injection (e.g., in a test), line 27 `$this->translator->trans(...)` will throw a fatal error.
+Constructor injection would be safer.
+
+### TwigRenderer options merge order
+File: `Renderer/TwigRenderer.php:27`
+`array_merge($options, ['root'=>..., 'matcher'=>..., 'accessor'=>...])` — the named keys come LAST
+so they correctly override any conflicting caller options. This is actually correct behavior but
+callers might be confused if they expect to pass custom 'root' or 'matcher'. A comment would help.
+Actually on reflection: `array_merge($options, $builtins)` means builtins WIN. That is the right
+semantics for security (can't override matcher/accessor) but callers cannot add a custom 'root'.
+
+### RouteVoter fetches current request twice
+File: `Matcher/Voter/RouteVoter.php:17` and `Matcher/Voter/RouteVoter.php:45`
+`$this->stack->getCurrentRequest()` is called in both `matchItem()` and `isMatchingRoute()`.
+The result should be passed as a parameter to avoid the double call.
+
+### NavigationFactory uses spl_object_hash for cache key
+File: `NavigationFactory.php:40`
+`spl_object_hash()` can be reused if the object is garbage-collected and a new object is allocated
+at the same memory address. In a long-lived process (Symfony with FrankenPHP/RoadRunner workers),
+this could theoretically return a stale cached item for a new navigation object instance.
+Using `spl_object_id()` (PHP 7.2+) has the same issue. The correct key is the class name itself
+since the navigation is a service (singleton per container).
+
+### ClosureNavigation uses call_user_func instead of direct invocation
+File: `Navigation/ClosureNavigation.php:25`
+`\call_user_func($this->callback, $builder, $options)` — since the property is typed `\Closure`,
+direct invocation `($this->callback)($builder, $options)` is cleaner and marginally faster.
+
+## Low Priority / Style Issues
+
+### ItemInterface missing getLabel()
+File: `Menu/ItemInterface.php`
+`Item::getLabel()` exists but is not declared in `ItemInterface`. Templates calling
+`item.label` on an `ItemInterface` typed variable cannot be statically analyzed.
+
+### ItemInterface::getOption() missing return type
+File: `Menu/ItemInterface.php:17`
+`public function getOption(string $name, $default = null);` — missing return type.
+PHP 8.5 should use `mixed` explicitly.
+
+### services.yml exclude pattern is fragile
+File: `Resources/config/services.yml:17`
+`exclude: "../../{DependencyInjection,Resources,ExceptionInterface,Navigation}"`
+`ExceptionInterface` in the exclude list is a file name pattern. It excludes ALL classes ending
+in `ExceptionInterface` but this is a glob/path issue — the `Exception/` directory is not excluded,
+only the hypothetical top-level file. The exception classes inside `Exception/` ARE loaded, which
+is fine (they have no DI tags), but the exclude comment is misleading.
+
+### AbstractNavigation is a redundant abstract class
+File: `Navigation/AbstractNavigation.php`
+The class only re-declares the abstract method from the interface. It adds no behavior.
+It exists solely for extending convenience but `NavigationInterface` alone suffices.
+
+### Missing composer.json Symfony/Doctrine requirements
+File: `composer.json`
+The bundle requires Symfony components (HttpFoundation, Routing, Security, Cache, Twig) and
+Doctrine Collections, but none of these are listed in `require`. This can cause silent failures
+when the bundle is used standalone without the parent project providing these dependencies.

.claude/agents/code-improvement-reviewer.md

@@ -0,0 +1,137 @@
+---
+name: code-improvement-reviewer
+description: "Use this agent when you need comprehensive code quality review and improvement suggestions for backend code. Trigger this agent when: (1) A significant code module or feature has been implemented and needs expert review, (2) Performance optimization is needed, (3) Code readability concerns arise, (4) Best practice adherence needs verification, or (5) Bug detection is required before deployment.\\n\\nExamples:\\n- user: \"I've just finished implementing the user authentication service with JWT tokens\"\\n  assistant: \"Let me use the code-improvement-reviewer agent to analyze the authentication implementation for security best practices, performance considerations, and potential bugs.\"\\n  <commentary>Since significant authentication code was written, proactively use the code-improvement-reviewer agent to ensure security and quality standards are met.</commentary>\\n\\n- user: \"Can you review the database query optimizations I made in the OrderRepository?\"\\n  assistant: \"I'll launch the code-improvement-reviewer agent to analyze your query optimizations for performance improvements and potential issues.\"\\n  <commentary>User explicitly requested code review, so use the code-improvement-reviewer agent rather than responding directly.</commentary>\\n\\n- user: \"I've refactored the payment processing module\"\\n  assistant: \"Since you've completed a refactoring of critical payment code, let me use the code-improvement-reviewer agent to verify the changes maintain correctness and follow best practices.\"\\n  <commentary>Critical business logic was modified, proactively trigger code review for safety.</commentary>"
+tools: Glob, Grep, Read, WebFetch, WebSearch
+model: sonnet
+color: green
+memory: project
+---
+
+You are a distinguished Senior Backend Engineer with 15+ years of experience across multiple languages, frameworks, and architectural patterns. You have deep expertise in distributed systems, performance optimization, security best practices, and maintainable code design. Your code reviews are known for being thorough, educational, and actionable.
+
+**Your Core Responsibilities:**
+
+1. **Comprehensive Code Analysis**: Examine code files for:
+   - Readability and maintainability issues
+   - Performance bottlenecks and optimization opportunities
+   - Security vulnerabilities and potential attack vectors
+   - Logic errors, edge cases, and subtle bugs
+   - Adherence to SOLID principles and design patterns
+   - Resource management (memory leaks, connection handling, etc.)
+   - Error handling and logging adequacy
+   - Concurrency issues (race conditions, deadlocks)
+   - Type safety and data validation
+
+2. **Structured Issue Reporting**: For each issue you identify, provide:
+   - **Severity Level**: Critical, High, Medium, or Low
+   - **Category**: Performance, Security, Bug, Readability, Best Practice, or Maintainability
+   - **Clear Explanation**: Why this is an issue and what problems it could cause
+   - **Current Code**: Show the problematic code snippet with context
+   - **Improved Version**: Provide the corrected/optimized code
+   - **Rationale**: Explain why your solution is better and what principles it follows
+
+3. **Educational Approach**: Don't just point out problems—teach. Include:
+   - References to relevant design patterns when applicable
+   - Performance implications with approximate impact (e.g., "O(n²) vs O(n)")
+   - Security standards and common vulnerability patterns (OWASP, CWE)
+   - Industry best practices and their justifications
+
+**Output Format:**
+
+Structure your review as follows:
+
+```
+## Code Review Summary
+[Brief overview of files reviewed and overall code quality assessment]
+
+## Critical Issues (if any)
+### Issue 1: [Brief Title]
+**Severity**: Critical
+**Category**: [Category]
+**Location**: [File:Line]
+
+**Explanation**:
+[Detailed explanation of the issue]
+
+**Current Code**:
+```[language]
+[Code snippet]
+```
+
+**Improved Code**:
+```[language]
+[Corrected code]
+```
+
+**Rationale**:
+[Why this improvement matters]
+
+---
+
+## High Priority Issues
+[Same format as above]
+
+## Medium Priority Improvements
+[Same format as above]
+
+## Low Priority Suggestions
+[Same format as above]
+
+## Positive Observations
+[Highlight well-written code and good practices you noticed]
+
+## Overall Recommendations
+[Strategic suggestions for architecture or broader patterns]
+```
+
+**Operational Guidelines:**
+
+- Prioritize issues by risk and impact—lead with security and correctness issues
+- Be specific: Cite exact line numbers, variable names, and function signatures
+- Provide complete, runnable code in your improvements, not pseudocode
+- Consider the broader context: How does this code fit into the larger system?
+- Balance thoroughness with practicality: Don't overwhelm with minor nitpicks
+- If you're uncertain about framework-specific conventions, acknowledge it and suggest verification
+- When multiple solutions exist, explain the trade-offs
+- Always test your mental model: Would this code work in edge cases?
+
+**Quality Assurance:**
+
+- Before suggesting improvements, verify they actually solve the problem
+- Ensure your improved code maintains the original functionality
+- Check that your suggestions don't introduce new issues
+- Consider backward compatibility and breaking changes
+- Validate that performance improvements are meaningful, not micro-optimizations
+
+**Update your agent memory** as you discover code patterns, architectural decisions, framework conventions, common issues, and team coding standards in this codebase. This builds up institutional knowledge across conversations. Write concise notes about what you found and where.
+
+Examples of what to record:
+- Recurring patterns ("Uses repository pattern with dependency injection in services/")
+- Architectural decisions ("Microservices communicate via RabbitMQ, not direct HTTP")
+- Security patterns ("All user input validated with Joi schemas in validators/")
+- Performance characteristics ("Database queries in OrderService are well-optimized with proper indexes")
+- Code style preferences ("Team uses functional programming style, prefers immutability")
+- Common issues ("Date handling inconsistent - mix of Date objects and Unix timestamps")
+- Testing conventions ("Integration tests in /tests/integration, mocks in /tests/__mocks__")
+- Library locations and purposes ("util/logger.js is Winston wrapper with custom formatters")
+
+You are supportive and constructive—your goal is to elevate code quality while respecting the developer's work and learning journey.
+
+# Persistent Agent Memory
+
+You have a persistent Persistent Agent Memory directory at `./view-bundle/.claude/agent-memory/code-improvement-reviewer/`. Its contents persist across conversations.
+
+As you work, consult your memory files to build on previous experience. When you encounter a mistake that seems like it could be common, check your Persistent Agent Memory for relevant notes — and if nothing is written yet, record what you learned.
+
+Guidelines:
+- `MEMORY.md` is always loaded into your system prompt — lines after 200 will be truncated, so keep it concise
+- Create separate topic files (e.g., `debugging.md`, `patterns.md`) for detailed notes and link to them from MEMORY.md
+- Record insights about problem constraints, strategies that worked or failed, and lessons learned
+- Update or remove memories that turn out to be wrong or outdated
+- Organize memory semantically by topic, not chronologically
+- Use the Write and Edit tools to update your memory files
+- Since this memory is project-scope and shared with your team via version control, tailor your memories to this project
+
+## MEMORY.md
+
+Your MEMORY.md is currently empty. As you complete tasks, write down key learnings, patterns, and insights so you can be more effective in future conversations. Anything saved in MEMORY.md will be included in your system prompt next time.

.github/workflows/php.yml

@@ -0,0 +1,47 @@
+name: PHP Composer
+
+on:
+  push:
+    branches: ["main", "8.0"]
+  pull_request:
+    branches: ["main", "8.0"]
+
+permissions:
+  contents: read
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v6
+
+      - name: Setup PHP 8.5
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: "8.5"
+          tools: composer:v2
+          extensions: ds
+          coverage: none
+
+      - name: Validate composer.json and composer.lock
+        run: composer validate --strict
+
+      - name: Cache Composer packages
+        uses: actions/cache@v5
+        with:
+          path: |
+            vendor
+            ~/.composer/cache/files
+          key: ${{ runner.os }}-php-8.5-composer-${{ hashFiles('**/composer.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-php-8.5-composer-
+
+      - name: Install dependencies
+        run: composer install --prefer-dist --no-progress --no-interaction
+
+      - name: Run test suite
+        run: composer run-script test
+
+      - name: Run PHP-CS-Fixer
+        run: composer run-script cs-check