wip

.cursor/cli.json

@@ -0,0 +1,6 @@
+{
+    "permissions": {
+        "allow": [],
+        "deny": ["Shell(git push)", "Shell(gh pr create)", "Write(**)"]
+    }
+}

.cursor/commands/review-testing-rules.md

@@ -0,0 +1 @@
+Analyze the changes in this Git working tree and report back on tests that fall short of our testing rules.

.cursor/rules/01-architecture.mdc

@@ -4,12 +4,23 @@ globs: app/**/*.php,templates/**/*.yaml
 
 ### Architecture (MANDATORY)
 
-- **PHP:** PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly);
-- **Imports:** Always add `use` statements instead of fully qualified class names;
-- **Type Safety:** All methods must have explicit return types with proper generics (e.g., `Collection<int, User>`).
+- **PHP:** PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly)
+- **Imports:** Always add `use` statements instead of fully qualified class names
+- **Type Safety:** All methods must have explicit return types with proper generics (e.g., `Collection<int, User>`)
 
 **🚨 Architecture rules are IMMUTABLE - fix violating code, not the architecture rules**
 
+### Symfony Patterns (MANDATORY)
+
+- **Dependency Injection:** instead of manually resolving and instantiating classes
+- **Symfony Classes:** instead of native PHP functions for easier mocking during testing (eg. `Filesystem::`, `Process::`, etc.)
+
+### Dependency Injection (MANDATORY)
+
+- Dependencies are automatically injected by the `app/Container` class
+- ALL dependencies MUST be injected through constructors - NO manual instantiation
+- **Exceptions:** Only value objects, DTOs, and pure data structures can be manually instantiated
+
 ### Command Layer
 
 - Commands handle user interaction (input/output) and orchestrate Services
@@ -26,31 +37,17 @@ globs: app/**/*.php,templates/**/*.yaml
 - Services handle core business logic, external API calls, file operations
 - Complex orchestration shared by multiple Commands should be extracted to dedicated Services
 
-### Shared Utilities
-
-- Common path resolution, configuration loading goes in dedicated utility classes
-- Utility classes are static or singleton patterns for simple operations
-- No business logic or state in utilities - pure functions only
-
 ### Console I/O Rules
 
 - Only Commands perform console input/output operations
-- Services return exceptions or structured data for Commands to handle
 - Use SymfonyStyle consistently for all user-facing output
+- Services return exceptions or structured data for Commands to handle
 - Validation errors and business exceptions bubble up to Commands for display
 
-### Dependency Injection (Mandatory)
-
-- ALL dependencies MUST be injected through constructors - NO manual instantiation
-- Commands receive Services via constructor injection
-- Services receive other Services/utilities via constructor injection
-- Use ServiceContainer or dependency injection container for all object creation
-- NEVER use `new ClassName()` inside methods - always inject dependencies
-- Exceptions: Only value objects, DTOs, and pure data structures can be manually instantiated
-
 ### Dependency Rules
 
 - Commands depend on Services
 - Services depend on other Services or utilities
-- No circular dependencies allowed
 - All dependencies declared in constructor signatures
+
+**IMPORTANT:** No circular dependencies allowed.

.cursor/rules/03-tests.mdc

@@ -10,212 +10,93 @@ globs: app/**/*.php,tests/**/*.php
 
 ### Running Tests
 
-- `composer pest` run the entire test suite in parallel, with coverage
-- `vendor/bin/pest $TEST_FILE` run a specific test file
+- `composer pest` - run entire test suite in parallel, with coverage
+- `vendor/bin/pest $TEST_FILE` - run specific test file
 
-### Test Minimalism Rules
+### Test Minimalism
 
-**Write only essential tests that would break if business logic fails:**
+**Target:** Keep test files under 1.8x the size of source code they test.
 
-- **Core Business Logic Only:** Test critical paths, skip framework testing
-- **Minimal Test Data:** Use simplest possible setup, avoid complex scenarios
-- **Standard Assertions:** Prefer built-in `expect()` over custom assertions
-- **Essential Edge Cases:** Only test failure modes that actually matter
-- **No Performance Tests:** Unless performance is the primary concern
+**Rules:**
 
-### Static Analysis in Tests
+- Test core business logic only, skip framework testing
+- Use dataset-driven testing: `->with([])` for multiple scenarios
+- Eliminate test overlap: no two tests covering same functionality
+- Consolidate assertions: `expect($x)->toBe(1)->and($y)->toBe(2)`
+- Mock external dependencies only
+- No performance tests unless performance is the primary concern
+- Don't sacrifice readability for ratio targets
 
-- Ignore PHPStan issues in tests.
-- Avoid excessive phpdoc just to appease types; add only when necessary.
+**Don't consolidate when:**
 
-### AAA Pattern Requirements (MANDATORY)
+- Different public methods
+- Exception vs normal flow tests
+- Different setup requirements
+- Distinct business logic
 
-**All tests MUST follow the AAA pattern with explicit section headers:**
+### AAA Pattern (MANDATORY)
 
 ```php
 it('does something specific', function () {
     // ARRANGE
-    $testData = ['key' => 'value'];
-    $mockService = mock(SomeService::class);
+    $service = new Service(mock(Dependency::class));
 
     // ACT
-    $result = $this->service->performAction($testData);
+    $result = $service->performAction();
 
     // ASSERT
-    expect($result)->toBe($expectedValue);
-    $mockService->shouldHaveReceived('method');
+    expect($result)->toBe('expected');
 
     // CLEANUP (when needed)
     $this->resetTimeState();
-    $this->cleanupTempFiles();
+    unlink($tempFile);
 });
 ```
 
-**Required Section Headers:**
+**Exception tests:** Use `// ACT & ASSERT` when act triggers assertion.
 
-- `// ARRANGE` - Setup test data, mocks, and dependencies
-- `// ACT` - Execute the code under test (single action)
-- `// ASSERT` - Verify expected outcomes and behaviors
-- `// CLEANUP` - Reset state when necessary (time state, temp files, etc.)
+**Organization:** Use `describe()` blocks, `beforeEach()` setup, extract helpers/traits for DRY tests.
 
-**Exception Pattern:**
+### Testing Patterns
 
-- `// ACT & ASSERT` - For exception tests where the act triggers the assertion
-
-### Organizing Tests & Helpers
-
-**DRY Principle:** Tests should be as DRY and streamlined as the code they're testing.
-
-**Best Practices:**
-
-- Logical grouping with `describe()` blocks
-- Extract repeated mocking into reusable helper methods
-- Create test traits for shared behavior across test classes
-- Use `beforeEach()` blocks for common setup within test groups
-- Use proper cleanup in tests (temp files, reset state) for test isolation
-- Build helper functions for creating test configurations and mock data
-
-#### Test Helpers
-
-- Evolution-friendly, loosely coupled
-- Support interface changes
-
-**Helper Examples:**
+**❌ FORBIDDEN:**
 
 ```php
-// Test trait for common mocking
-trait MocksExternalServices
-{
-    protected function mockSuccessfulProcess(): void
-    {
-        // Mock external process calls
-        $this->processRunner = mock(ProcessRunner::class);
-        $this->processRunner->shouldReceive('run')
-            ->andReturn(['output' => 'Success', 'exitCode' => 0]);
-    }
-}
-
-// Reusable test data builders
-function createCommandOptions(array $overrides = []): array
-{
-    return array_merge([
-        'composer' => true,
-        'npm' => false,
-        'force' => false,
-    ], $overrides);
-}
-
-// beforeEach for common setup
-describe('package installation', function () {
-    beforeEach(function () {
-        $this->mockSuccessfulProcess();
-        $this->baseOptions = createCommandOptions();
-    });
-});
-```
-
-**Benefits:**
-
-- Reduces test maintenance burden
-- Ensures consistent mocking patterns
-- Makes tests more readable and focused
-- Easier to update when dependencies change
-
-### FORBIDDEN Patterns (Auto-Reject)
-
-```php
-expect($x)->toBeInstanceOf(Class::class);     // Type-only
-expect($x)->toBeArray();                      // Generic
+expect($x)->toBeInstanceOf(Class::class);     // Type-only testing
+expect($x)->toBeArray();                      // Generic assertions
 expect($x)->not->toBeNull();                  // Meaningless
-expect($x)->toBeTrue();                       // No context
-expect(true)->toBeTrue();                     // This is literally meaningless
-expect($object->property())->toBeInstanceOf(); // Property type testing
-sleep(...);                                   // Use proper time mocking instead
+expect(true)->toBeTrue();                     // Literally meaningless
+sleep(...);                                   // Use time mocking
 ```
 
-### REQUIRED Patterns
+**✅ REQUIRED:**
 
 ```php
-// Test specific values and behavior
-expect($config->getValue('host'))->toBe('example.com')
-    ->and($config->getValue('port'))->toBe(22);
-
-// Test with datasets
-it('validates server hostnames', function (string $hostname, bool $valid) {
-    expect($this->validator->isValidHostname($hostname))->toBe($valid);
-})->with([
-    ['server.example.com', true],
-    ['invalid_hostname!', false],
-]);
-
-// Mock only external dependencies
-$processRunner = mock(ProcessRunner::class);
-$processRunner->shouldReceive('run')->with('ls *')->andReturn('file1.txt');
-
-$httpClient = mock(HttpClient::class);
-$httpClient->shouldReceive('get')->with('api/endpoint')->andReturn(['status' => 'ok']);
+expect($config->getValue('host'))->toBe('example.com');
+expect($this->validator->isValid($input))->toBe($expected);
+$mock->shouldReceive('method')->with('param')->andReturn('result');
 ```
 
-### Unit Test Isolation
-
-**Core Principle:** True unit tests must be isolated from external dependencies.
+### Test Types
 
-**Unit tests should:**
+**Unit Tests:**
 
-- Test single units of code in isolation
-- Use mocks/fakes for all external dependencies
-- Run without external dependencies
+- Mock all external dependencies (filesystem, HTTP, processes)
+- Test single units in isolation
 - Complete in milliseconds
 
-**Unit tests MUST NOT:**
+**Integration Tests:**
 
-- Use real file system, network calls, or shell commands
-- Test service integrations with external systems
-- Make actual HTTP requests or process executions
-- Depend on external services, APIs, or system processes
+- Real file operations and external processes
+- CLI commands and full workflows
 
-**Integration vs Unit:** Use integration tests for file system operations and external processes, unit tests (with mocks) for pure business logic.
+**Layer Strategy:**
 
-### Layer Testing Strategy
+- CLI Commands → Integration tests
+- Business Services → Unit tests (mocked dependencies)
+- Utilities/Helpers → Unit tests
 
-- **CLI Commands:** Integration tests, mock external processes and services
-- **Business Services:** Unit tests, mock all external calls and file operations
-- **Utilities/Helpers:** Unit tests with isolated scenarios
+### Static Analysis
 
-```php
-// ✅ Service unit test (mocked dependencies)
-$this->fileSystem->shouldReceive('exists')->andReturn(true);
-$this->processRunner->shouldReceive('execute')->andReturn(['output' => 'success']);
-
-// ✅ Command integration test (real file operations)
-$testFile = '/tmp/test-config.yml';
-file_put_contents($testFile, 'host: example.com');
-$result = $this->command->execute(['--config' => $testFile]);
-expect($result->exitCode)->toBe(0);
-unlink($testFile); // cleanup
-```
-
-### Performance Testing
-
-**When testing CLI operations:**
-
-- Monitor execution time for file operations and external processes
-- Test with realistic server counts and configuration sizes to expose performance issues
-- Verify efficient processing of multiple servers or long-running deployment operations
-
-```php
-// ✅ CLI performance monitoring pattern
-it('processes server provisioning efficiently', function () {
-    // ARRANGE
-    $serverConfigs = createTestServerConfigs(5);
-
-    // ACT
-    $startTime = microtime(true);
-    $result = $this->provisioningService->processServers($serverConfigs);
-    $executionTime = microtime(true) - $startTime;
-
-    // ASSERT
-    expect($result->getSuccessCount())->toBe(5)
-        ->and($executionTime)->toBeLessThan(2.0); // Should complete provisioning in under 2 seconds
-});
-```
+- Ignore PHPStan issues in tests - focus on test functionality over compliance
+- Avoid excessive phpdoc just to appease types

.github/workflows/testing-rules.yml

@@ -0,0 +1,58 @@
+name: Testing Rules
+
+permissions:
+  contents: read
+  pull-requests: write
+
+on:
+  pull_request:
+
+jobs:
+  testing-rules:
+    runs-on: ubuntu-latest
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.ref }}
+      cancel-in-progress: true
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install Cursor CLI
+        run: |
+          curl https://cursor.com/install -fsS | bash
+          echo "$HOME/.cursor/bin" >> $GITHUB_PATH
+
+      - name: Testing rules review
+        env:
+          CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }}
+          GH_TOKEN: ${{ github.token }}
+          MODEL: 'claude-4-sonnet'
+        run: |
+          cursor-agent --force --model "$MODEL" --output-format=text --print "You are operating in a GitHub Actions runner performing automated code review. The gh CLI is available and authenticated via GH_TOKEN. You may comment on pull requests.
+
+          Context:
+          - Repo: ${{ github.repository }}
+          - PR Number: ${{ github.event.pull_request.number }}
+          - PR Head SHA: ${{ github.event.pull_request.head.sha }}
+          - PR Base SHA: ${{ github.event.pull_request.base.sha }}
+
+          Review the current PR diff and...
+          1. report back on tests that fall short of our testing rules
+          2. identify gaps in our tests or missing tests
+          3. leave very short inline comments (1-2 sentences) on suggested changes and a brief summary at the end
+
+          Procedure:
+          - Get existing comments: gh pr view --json comments
+          - Get diff: gh pr diff
+          - If a previously reported issue appears fixed by nearby changes, reply: ✅ This issue appears to be resolved by the recent changes
+          - Avoid duplicates: skip if similar feedback already exists on or near the same lines
+
+          Commenting rules:
+          - Natural tone, specific and actionable; do not mention automated or high-confidence
+          - Use emojis: 🚨 Critical 🔒 Security ⚡ Performance ⚠️ Logic ✅ Resolved ✨ Improvement
+
+          Submission:
+          - Submit one review containing inline comments plus a concise summary
+          - Use only: gh pr review --comment
+          - Do not use: gh pr review --approve or --request-changes"