From bb9f628862eb98d1fa7bfafd5384a8c8ccd761a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 13:54:48 +0200 Subject: [PATCH 01/18] refactor(php): remove legacy format handling from version selection Simplify PHP version handling by removing support for legacy string/numeric format. Both SiteCreateCommand and ServerInfoCommand now only handle the current array format with version and extensions keys. --- app/Console/Server/ServerInfoCommand.php | 61 +++++++----------------- app/Console/Site/SiteCreateCommand.php | 40 ++++++---------- 2 files changed, 30 insertions(+), 71 deletions(-) diff --git a/app/Console/Server/ServerInfoCommand.php b/app/Console/Server/ServerInfoCommand.php index 993e8e40..c3d74c2f 100644 --- a/app/Console/Server/ServerInfoCommand.php +++ b/app/Console/Server/ServerInfoCommand.php @@ -190,57 +190,28 @@ private function displayServerInfo(array $info, string $host): void // Display PHP versions if available if (isset($info['php']) && is_array($info['php'])) { - $phpItems = []; + /** @var array{versions: array}>, default?: string} $phpInfo */ + $phpInfo = $info['php']; + $versions = $phpInfo['versions']; - if (isset($info['php']['versions']) && is_array($info['php']['versions']) && count($info['php']['versions']) > 0) { - $versions = $info['php']['versions']; - $defaultVersion = $info['php']['default'] ?? null; + if ([] !== $versions) { + $phpItems = []; + $defaultVersion = $phpInfo['default'] ?? ''; foreach ($versions as $versionData) { - // Handle both old format (string) and new format (array with version/extensions) - if (is_array($versionData) && isset($versionData['version'])) { - /** @var string|int|float */ - $version = $versionData['version']; - $versionStr = (string) $version; - $extensions = $versionData['extensions'] ?? []; - - // Build version label - $isDefault = false; - if ($defaultVersion !== null && (is_string($defaultVersion) || is_numeric($defaultVersion))) { - /** @var string|int|float $defaultVersion */ - $isDefault = $versionStr === (string) $defaultVersion; - } - - $versionLabel = "PHP {$versionStr}"; - if ($isDefault) { - $versionLabel .= ' (default)'; - } - - // Use version as key, extensions as value - if (is_array($extensions) && count($extensions) > 0) { - $phpItems[$versionLabel] = implode(', ', $extensions); - } else { - $phpItems[$versionLabel] = 'no extensions'; - } - } elseif (is_string($versionData) || is_numeric($versionData)) { - // Fallback for old format (simple string/numeric version) - $versionStr = (string) $versionData; - $isDefault = false; - if ($defaultVersion !== null && (is_string($defaultVersion) || is_numeric($defaultVersion))) { - /** @var string|int|float $defaultVersion */ - $isDefault = $versionStr === (string) $defaultVersion; - } - - $versionLabel = "PHP {$versionStr}"; - if ($isDefault) { - $versionLabel .= ' (default)'; - } - $phpItems[$versionLabel] = ''; + $version = $versionData['version']; + $extensions = $versionData['extensions']; + + $versionLabel = "PHP {$version}"; + if ($version === $defaultVersion) { + $versionLabel .= ' (default)'; } + + $phpItems[$versionLabel] = [] !== $extensions + ? implode(', ', $extensions) + : 'no extensions'; } - } - if (count($phpItems) > 0) { $this->displayDeets(['PHP' => $phpItems]); } } diff --git a/app/Console/Site/SiteCreateCommand.php b/app/Console/Site/SiteCreateCommand.php index 14851fa8..01ebbb62 100644 --- a/app/Console/Site/SiteCreateCommand.php +++ b/app/Console/Site/SiteCreateCommand.php @@ -169,12 +169,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } - // + // ---- // Validation // ---- /** - * Validate that server is ready to add site. + * Validate that server is ready to create a site. * * Checks for: * - Caddy web server installed @@ -215,39 +215,28 @@ private function validateServerReady(array $info): ?int */ private function selectPhpVersion(array $info): string|int { - // Extract installed PHP versions - $installedPhpVersions = []; - if (isset($info['php']) && is_array($info['php']) && isset($info['php']['versions']) && is_array($info['php']['versions'])) { - foreach ($info['php']['versions'] as $version) { - // Handle both new format (array with version/extensions) and old format (string) - if (is_array($version) && isset($version['version'])) { - /** @var string $versionStr */ - $versionStr = $version['version']; - $installedPhpVersions[] = $versionStr; - } elseif (is_string($version) || is_numeric($version)) { - $installedPhpVersions[] = (string) $version; - } - } - } + /** @var array{versions: array}>, default?: string} $phpInfo */ + $phpInfo = $info['php']; + $versions = $phpInfo['versions']; - if (empty($installedPhpVersions)) { + if ([] === $versions) { $this->nay('No PHP versions found on server'); return Command::FAILURE; } - // If only one version, use it automatically + $installedPhpVersions = array_map( + fn (array $v): string => $v['version'], + $versions + ); + if (1 === count($installedPhpVersions)) { return $installedPhpVersions[0]; } - // Multiple versions available - prompt user to select - rsort($installedPhpVersions, SORT_NATURAL); // Newest first + rsort($installedPhpVersions, SORT_NATURAL); - /** @var array{default?: string|int|float}|null $phpInfo */ - $phpInfo = $info['php'] ?? null; - $defaultVersion = is_array($phpInfo) ? ($phpInfo['default'] ?? null) : null; - $defaultVersionStr = null !== $defaultVersion ? (string) $defaultVersion : $installedPhpVersions[0]; + $defaultVersionStr = $phpInfo['default'] ?? $installedPhpVersions[0]; $phpVersion = (string) $this->io->getOptionOrPrompt( 'php-version', @@ -258,10 +247,9 @@ private function selectPhpVersion(array $info): string|int ) ); - // Validate CLI-provided version exists in available versions if (! in_array($phpVersion, $installedPhpVersions, true)) { $this->nay( - "PHP version {$phpVersion} is not installed on this server. Available: " . implode(', ', $installedPhpVersions) + "PHP version {$phpVersion} is not installed. Available: " . implode(', ', $installedPhpVersions) ); return Command::FAILURE; From 8e754c2405ba4b1b8a20ded9c12f545b1e016d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 15:32:16 +0200 Subject: [PATCH 02/18] fixup --- app/Console/Server/ServerInfoCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Console/Server/ServerInfoCommand.php b/app/Console/Server/ServerInfoCommand.php index c3d74c2f..7792cd75 100644 --- a/app/Console/Server/ServerInfoCommand.php +++ b/app/Console/Server/ServerInfoCommand.php @@ -189,7 +189,7 @@ private function displayServerInfo(array $info, string $host): void } // Display PHP versions if available - if (isset($info['php']) && is_array($info['php'])) { + if (isset($info['php']) && is_array($info['php']) && isset($info['php']['versions']) && is_array($info['php']['versions'])) { /** @var array{versions: array}>, default?: string} $phpInfo */ $phpInfo = $info['php']; $versions = $phpInfo['versions']; From 06783902e4513c4ec364d5b2f173853d85116111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 15:36:13 +0200 Subject: [PATCH 03/18] docs: add comprehensive development guides and reference documentation Add seven detailed markdown guides covering architecture, command design, exception handling, testing patterns, bash styling, playbook conventions, and documentation writing. Includes CLAUDE.md as a quick reference summarizing all project rules and linking to detailed docs. --- .claude/commands/create-branch.md | 18 +++ .claude/commands/create-commits.md | 19 +++ .claude/commands/deslop.md | 17 +++ .claude/commands/improve-tests.md | 13 ++ .claude/commands/review-branch.md | 16 +++ .claude/commands/review-diff.md | 15 ++ .claude/commands/review-pr-comment.md | 6 + .claude/settings.local.json | 14 ++ CLAUDE.md | 77 +++++++++++ docs/rules/architecture.md | 104 ++++++++++++++ docs/rules/bash-style.md | 113 ++++++++++++++++ docs/rules/commands.md | 188 ++++++++++++++++++++++++++ docs/rules/exceptions.md | 123 +++++++++++++++++ docs/rules/playbooks.md | 176 ++++++++++++++++++++++++ docs/rules/testing.md | 114 ++++++++++++++++ docs/rules/writing-docs.md | 100 ++++++++++++++ 16 files changed, 1113 insertions(+) create mode 100644 .claude/commands/create-branch.md create mode 100644 .claude/commands/create-commits.md create mode 100644 .claude/commands/deslop.md create mode 100644 .claude/commands/improve-tests.md create mode 100644 .claude/commands/review-branch.md create mode 100644 .claude/commands/review-diff.md create mode 100644 .claude/commands/review-pr-comment.md create mode 100644 .claude/settings.local.json create mode 100644 CLAUDE.md create mode 100644 docs/rules/architecture.md create mode 100644 docs/rules/bash-style.md create mode 100644 docs/rules/commands.md create mode 100644 docs/rules/exceptions.md create mode 100644 docs/rules/playbooks.md create mode 100644 docs/rules/testing.md create mode 100644 docs/rules/writing-docs.md diff --git a/.claude/commands/create-branch.md b/.claude/commands/create-branch.md new file mode 100644 index 00000000..febb23c4 --- /dev/null +++ b/.claude/commands/create-branch.md @@ -0,0 +1,18 @@ +--- +description: Create a branch based on working tree changes +allowed-tools: Bash(git:*) +model: haiku +--- + +Based on the changes made to this repository create a new branch with a suitable name. + +Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: +feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. + +Keep the branch name short (≤ 50 chars) yet informative. Do not push, pull, or rebase. + +Examples: + +- feat/parser-add-php-84-attributes +- fix/ci-matrix-php-versions +- chore/deps-bump-composer-installers-2-3 diff --git a/.claude/commands/create-commits.md b/.claude/commands/create-commits.md new file mode 100644 index 00000000..c616d535 --- /dev/null +++ b/.claude/commands/create-commits.md @@ -0,0 +1,19 @@ +--- +description: Create conventional commits from working tree changes +allowed-tools: Bash(git:*) +model: haiku +--- + +Based on the changes made to this repository create one or more commits with suitable titles. + +Use Conventional Commits to group related changes into cohesive commits (commits should be independently meaningful). + +Keep titles short (≤ 72 chars), imperative, no trailing period. Do not push, pull, or rebase. + +Examples: + +- feat(parser): add support for PHP 8.4 attributes +- fix(ci): correct matrix PHP versions in build workflow +- chore(deps): bump composer/installers to ^2.3 + +Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). diff --git a/.claude/commands/deslop.md b/.claude/commands/deslop.md new file mode 100644 index 00000000..fc978c9f --- /dev/null +++ b/.claude/commands/deslop.md @@ -0,0 +1,17 @@ +--- +description: Remove AI-generated code slop from the branch +argument-hint: [base-branch] +--- + +# Remove AI Code Slop + +Check the diff against $1 (or main if not specified), and remove all AI-generated slop introduced in this branch. + +This includes: + +- Extra comments that a human wouldn't add or are inconsistent with the rest of the file +- Extra defensive checks or try/catch blocks that are abnormal for that area of the codebase (especially if called by trusted / validated codepaths) +- Casts to any to get around type issues +- Any other violation inconsistent with the rest of the file or our rules + +Report at the end with only a 1-3 sentence summary of what you changed diff --git a/.claude/commands/improve-tests.md b/.claude/commands/improve-tests.md new file mode 100644 index 00000000..9e2a5501 --- /dev/null +++ b/.claude/commands/improve-tests.md @@ -0,0 +1,13 @@ +--- +description: Find and fix test overlap or testing theater +argument-hint: [test-file-or-directory] +--- + +Analyze the tests in $ARGUMENTS (or all tests if not specified) for: + +- Overlapping test coverage (multiple tests verifying the same behavior) +- Testing theater (tests that pass but don't actually verify behavior) +- Missing assertions or overly loose assertions +- Tests that mock too much and don't test real behavior + +Implement improvements if found. diff --git a/.claude/commands/review-branch.md b/.claude/commands/review-branch.md new file mode 100644 index 00000000..834b749d --- /dev/null +++ b/.claude/commands/review-branch.md @@ -0,0 +1,16 @@ +--- +description: Review all changes in current branch vs base +model: opus +--- + +Meticulously catalog and analyze all the changes in this branch, compared to the branch it's based on. + +Review against our documented rules in: +- docs/rules/architecture.md +- docs/rules/commands.md +- docs/rules/exceptions.md +- docs/rules/testing.md + +Focus on where changes fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. + +Provide a detailed report but don't make any changes yet. diff --git a/.claude/commands/review-diff.md b/.claude/commands/review-diff.md new file mode 100644 index 00000000..7ceec057 --- /dev/null +++ b/.claude/commands/review-diff.md @@ -0,0 +1,15 @@ +--- +description: Review staged/unstaged changes in working tree +--- + +Meticulously catalog and analyze all the changes in this Git working tree, staged or unstaged. + +Review against our documented rules in: +- docs/rules/architecture.md +- docs/rules/commands.md +- docs/rules/exceptions.md +- docs/rules/testing.md + +Focus on where changes fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. + +Provide a detailed report but don't make any changes yet. diff --git a/.claude/commands/review-pr-comment.md b/.claude/commands/review-pr-comment.md new file mode 100644 index 00000000..ba52f432 --- /dev/null +++ b/.claude/commands/review-pr-comment.md @@ -0,0 +1,6 @@ +--- +description: Assess PR comment concerns and propose solutions +argument-hint: +--- + +Please assess whether the concerns raised in the following PR comment are valid, and propose possible solutions to address them. diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 00000000..c5c73022 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,14 @@ +{ + "permissions": { + "allow": [ + "Bash(tree:*)", + "Bash(mkdir:*)", + "Bash(wc:*)", + "Bash(cat:*)", + "Bash(gh pr view:*)", + "WebFetch(domain:github.com)" + ], + "deny": [], + "ask": [] + } +} diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..9082b51c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,77 @@ +# Deployer PHP + +Server and site deployment tool for PHP. Composer package and CLI built on Symfony Console. + +## Commands + +```bash +# Quality gates (run before completing tasks) +vendor/bin/rector $CHANGED_PHP_FILES +vendor/bin/pint $CHANGED_PHP_FILES +vendor/bin/phpstan analyse $CHANGED_PHP_FILES # NEVER run on tests/ + +# All quality checks +composer pall + +# Testing +composer pest # Full suite with coverage +vendor/bin/pest $TEST_FILE # Specific file + +# Bash formatting +composer bash # Format playbooks/*.sh +composer bash:check # Check only +``` + +## Code Philosophy + +- **Minimalism:** Write minimum code necessary. Eliminate single-use methods. Cache computed values. +- **Organization:** Group related functions into comment-separated sections. Order alphabetically after grouping. +- **Consistency:** Same style throughout. Code should appear written by single person. + +## Architecture Summary + +| Concept | Rule | +|---------|------| +| DI | `$container->build(Class::class)` for all objects except DTOs | +| Layers | Commands (I/O) → Services (logic) → Repositories (data) | +| Exceptions | Services throw complete messages, Commands display directly | +| PHP | PSR-12, strict types, Yoda conditions (`null === $value`), always braces | +| Console | Use BaseCommand methods, never SymfonyStyle directly | +| Playbooks | Idempotent bash scripts, YAML output to `$DEPLOYER_OUTPUT_FILE` | + +## File Operations + +Use terminal commands for file management: + +```bash +mv old.php new.php # Rename/move +cp source.php dest.php # Copy +mkdir -p path/to/dir # Create directories +``` + +## Execution Protocol + +1. ULTRATHINK - analyze problem deeply +2. STEP BY STEP - break into logical steps +3. ACT - implement systematically + +## Test Policy + +Don't run, create, or update tests UNLESS explicitly instructed. + +## Detailed Rules by Domain + +| Working On | Reference | +|------------|-----------| +| PHP architecture, DI, layers | @docs/rules/architecture.md | +| Symfony Console commands | @docs/rules/commands.md | +| Exception handling | @docs/rules/exceptions.md | +| Pest testing | @docs/rules/testing.md | +| Bash playbooks | @docs/rules/playbooks.md | +| Shell script style | @docs/rules/bash-style.md | +| Writing documentation | @docs/rules/writing-docs.md | + +## References + +- Check `composer.json` and `package.json` for installed packages +- Plan with features from installed major versions diff --git a/docs/rules/architecture.md b/docs/rules/architecture.md new file mode 100644 index 00000000..efffa249 --- /dev/null +++ b/docs/rules/architecture.md @@ -0,0 +1,104 @@ +# Architecture Rules + +All rules MANDATORY. + +## PHP Standards + +- PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly) +- Explicit return types with generics: `Collection` +- Dependency injection via Symfony patterns +- Use Symfony classes over native PHP functions (Filesystem, Process) for testability +- **Yoda conditions:** Constants/literals on LEFT side of comparisons +- **Always use braces:** ALL control structures must use `{ }` + +```php +// Yoda conditions +if (null === $value) { ... } +if ('' === trim($name)) { ... } +if (0 !== $exitCode) { ... } + +// Two variables - Yoda doesn't apply +if ($typedName !== $server->name) { ... } +``` + +## PHPStan Type Hints + +Use `@var` annotations, not `assert()` in production: + +```php +/** @var string $apiToken */ +$apiToken = $this->env->get(['API_TOKEN']); +``` + +## Imports + +Always add `use` statements for vendor packages. Root namespace FQDNs acceptable (`\RuntimeException`). + +```php +use Symfony\Component\Filesystem\Filesystem; + +$fs = new Filesystem(); +throw new \InvalidArgumentException('Error'); +``` + +## Dependency Injection + +Use `$container->build(ClassName::class)` for all object creation except DTOs/value objects. + +```php +// Production +$service = $this->container->build(MyService::class); + +// Tests - container supports bind() for mocks +$container = new Container(); +$container->bind(SSHService::class, $mockSSH); +$command = $container->build(ServerAddCommand::class); +``` + +**Integration:** Entry point: `bin/deployer`. Command registration: `SymfonyApp.php`. Services: Auto-injected via constructor. + +## Layer Separation + +**Command Layer:** +- Handle user interaction (input/output), orchestrate Services +- NO business logic (delegate to Services) +- Responsible for console styling, error formatting, prompts +- Never invoke other commands + +**Service Layer:** +- Atomic, reusable functionality with NO console I/O +- Accept/return plain PHP data types +- Handle business logic, external APIs, file operations + +**Service State:** +- Stateless: Pure operations (validators, calculators, API clients) +- Stateful: Manage configuration/cached data, use lazy loading and explicit `load()`/`initialize()` methods + +**Dependencies:** +- Commands depend on Services +- Services depend on Services/utilities +- All dependencies in constructor signatures +- NO circular dependencies + +## Comments + +**DocBlock:** Minimalist descriptions, parameters, return types. + +**Comment structure:** + +``` +// ---- +// {h1} +// ---- + +// +// {h2} +// ---- + +// +// {h3} + +// {p} +``` + +Separate sections visually. No obvious comments. Remove comments when removing code. diff --git a/docs/rules/bash-style.md b/docs/rules/bash-style.md new file mode 100644 index 00000000..33c1250f --- /dev/null +++ b/docs/rules/bash-style.md @@ -0,0 +1,113 @@ +# Bash Style + +All rules MANDATORY. Based on https://style.ysap.sh/md + +## Core Syntax + +**Conditionals:** `[[ ... ]]` not `[ ... ]` + +```bash +[[ -d /etc ]] # CORRECT +[ -d /etc ] # WRONG +``` + +**Command Substitution:** `$(...)` not backticks + +```bash +foo=$(date) # CORRECT +foo=`date` # WRONG +``` + +**Math:** `((...))` and `$((...))`, never `let` + +```bash +if ((a > b)); then ... # CORRECT +if [[ $a -gt $b ]]; then # WRONG +``` + +**Functions:** No `function` keyword, always `local` + +```bash +foo() { local i=5; } # CORRECT +function foo { i=5; } # WRONG +``` + +**Block Statements:** `then`/`do` same line + +```bash +if true; then ... # CORRECT +while true; do ... # CORRECT +``` + +## Parameter Handling + +**Expansion:** Prefer over external commands + +```bash +prog=${0##*/} # CORRECT - basename +nonumbers=${name//[0-9]/} # CORRECT - remove numbers +prog=$(basename "$0") # WRONG - external command +``` + +**Quoting:** Double for expansions, single for literals + +```bash +echo "$foo" # expansion needs quotes +bar='literal' # no expansion +if [[ -n $foo ]]; then # [[ ]] doesn't word-split +``` + +**Arrays:** Use bash arrays, not strings + +```bash +modules=(a b c) # CORRECT +for m in "${modules[@]}" # CORRECT +modules='a b c' # WRONG +``` + +## Error Handling + +```bash +cd /path || exit # CORRECT - exit on failure +cd /path # WRONG - unchecked +``` + +- Use `set -o pipefail` in playbooks +- Don't use `set -e` - explicit checking preferred +- Never use `eval` + +## File Operations + +```bash +# Streaming read +while IFS=: read -r user _; do + echo "$user" +done < /etc/passwd + +# CORRECT +grep foo file + +# WRONG - useless cat +cat file | grep foo + +# CORRECT - globs +for f in *; do ... + +# WRONG - parsing ls +for f in $(ls); do ... +``` + +## Formatting + +- Tabs for indentation +- Max 80 columns +- Semicolons only in control statements +- Max 1 blank line between sections +- Shebang: `#!/usr/bin/env bash` + +## Quality Gates + +```bash +composer bash # Format playbooks/*.sh +composer bash:check # Check only +``` diff --git a/docs/rules/commands.md b/docs/rules/commands.md new file mode 100644 index 00000000..782aed20 --- /dev/null +++ b/docs/rules/commands.md @@ -0,0 +1,188 @@ +# Symfony Console Rules + +All rules MANDATORY. + +## Core Principle + +Every command MUST be fully runnable non-interactively via CLI options. Every prompt MUST have a corresponding CLI option. + +## Output Methods + +NEVER use Symfony IO methods directly - use BaseCommand methods: + +```php +$this->out(['Multiple', 'lines']); +$this->hr(); +$this->h1('Section Heading'); +$this->displayDeets(['Key' => 'value']); +$this->yay('Success'); // checkmark +$this->nay('Failed'); // red X +$this->warn('Warning'); // warning +$this->info('Info'); // info +$this->ul(['Item 1', 'Item 2']); // bullet list +$this->ol(['Step 1', 'Step 2']); // numbered list +``` + +**Trait Organization:** +- ConsoleOutputTrait: Output/formatting using `$this->io` +- ConsoleInputTrait: Input using `$this->input` +- BaseCommand: Shared initialization, configuration + +## User Input with Laravel Prompts + +```php +use function Laravel\Prompts\{text, password, confirm, select, multiselect, spin}; + +$name = text('Name?', required: true); +$env = select('Environment:', ['dev', 'staging', 'prod']); +$result = spin(fn() => $this->service->process(), 'Processing...'); +``` + +## Interactive + Options Pattern + +```php +protected function configure(): void { + parent::configure(); + $this->addOption('name', null, InputOption::VALUE_REQUIRED, 'Server name'); +} + +protected function execute(InputInterface $input, OutputInterface $output): int { + $name = $this->getOptionOrPrompt( + 'name', + fn () => $this->promptText(label: 'Server name:', required: true) + ); + return Command::SUCCESS; +} +``` + +## Input Validation + +**Validator Signature** - Returns `?string` (error message or null): + +```php +protected function validateNameInput(mixed $value): ?string +{ + if (!is_string($value)) { + return 'Name must be a string'; + } + if ('' === trim($value)) { + return 'Name cannot be empty'; + } + if (null !== $this->repo->findByName($value)) { + return "'{$value}' already exists"; + } + return null; +} +``` + +**Usage with getValidatedOptionOrPrompt:** + +```php +$name = $this->getValidatedOptionOrPrompt( + 'name', + fn ($validate) => $this->promptText(label: 'Name:', validate: $validate), + fn ($value) => $this->validateNameInput($value) +); + +if (null === $name) { + return Command::FAILURE; // Validation failed +} +``` + +**Naming Convention:** +- `validate*Input()` - Returns `?string` (for prompts/options) +- `validate*()` - Throws exceptions (for heavy I/O) + +## Boolean Flags + +```php +// VALUE_NONE - Simple flag +$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); + +// VALUE_NEGATABLE - Tri-state (--flag, --no-flag, or prompt) +$this->addOption('php-default', null, InputOption::VALUE_NEGATABLE, 'Set as default PHP'); +``` + +## Multiselect with CLI + +```php +$selected = $this->getOptionOrPrompt( + 'databases', + fn () => $this->promptMultiselect(label: 'Databases:', options: $options) +); + +// Handle both array (prompt) and string (CLI) +if (is_string($selected)) { + $selected = array_filter(array_map(trim(...), explode(',', $selected))); +} +``` + +## Multi-Path Prompts + +Create separate options for each path: + +```php +// Separate options for each path +$this->addOption('generate-deploy-key', null, InputOption::VALUE_NONE, 'Generate key'); +$this->addOption('custom-deploy-key', null, InputOption::VALUE_REQUIRED, 'Custom key path'); + +// Check for conflicts +if ($generateKey && null !== $customKeyPath) { + $this->nay('Cannot use both options'); + return Command::FAILURE; +} +``` + +## Confirmation Patterns + +**Simple (`--yes`):** + +```php +$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); +$confirmed = $this->getOptionOrPrompt('yes', fn () => $this->promptConfirm('Sure?')); +``` + +**Type-to-Confirm (`--force`) - Destructive ops:** + +```php +if (!$forceSkip) { + $typedName = $this->promptText(label: "Type '{$server->name}' to confirm:"); + if ($typedName !== $server->name) { + $this->nay('Name does not match'); + return Command::FAILURE; + } +} +``` + +## Command Options Naming + +| Option | Usage | Type | +|--------|-------|------| +| `--server` | Select existing server | VALUE_REQUIRED | +| `--domain` | Select existing site | VALUE_REQUIRED | +| `--name` | Define new resource name | VALUE_REQUIRED | +| `--yes` / `-y` | Skip confirmation | VALUE_NONE | +| `--force` / `-f` | Skip type-to-confirm | VALUE_NONE | + +**Golden Rule:** `--server`/`--domain` for SELECTING existing, `--name` for DEFINING new. + +## Command Completion + +Always call `commandReplay()` before SUCCESS: + +```php +$this->commandReplay('server:delete', [ + 'server' => $server->name, + 'yes' => true, +]); +return Command::SUCCESS; +``` + +## Checklist + +- [ ] Every prompt has corresponding `addOption()` +- [ ] Multi-path prompts have separate options +- [ ] Conflicting options detected and rejected +- [ ] CLI values validated against allowed options +- [ ] `commandReplay()` called before SUCCESS +- [ ] Type annotations on option retrievals diff --git a/docs/rules/exceptions.md b/docs/rules/exceptions.md new file mode 100644 index 00000000..c002c10b --- /dev/null +++ b/docs/rules/exceptions.md @@ -0,0 +1,123 @@ +# Exception Handling & Error Display + +All rules MANDATORY. + +## Core Principle + +Services throw complete, user-facing exceptions. Command layer displays them directly without adding prefixes. + +## Services & Repositories + +Throw `\RuntimeException` with complete, actionable messages: + +```php +// Complete message with context +throw new \RuntimeException("SSH key does not exist: {$privateKeyPath}"); +throw new \RuntimeException("Server '{$name}' already exists"); + +// Preserve exception chain when wrapping +} catch (\Throwable $e) { + throw new \RuntimeException( + "SSH authentication failed for {$username}@{$host}. Check username and key permissions", + previous: $e + ); +} +``` + +**Rules:** +- Messages must be user-facing and complete (not fragments) +- Include relevant context (paths, names, IDs, hosts) +- Use `previous: $e` to preserve exception chains +- Never catch and re-throw with generic prefixes like "Failed to..." + +## Validation Traits + +**Pattern A: Input Validation** - Returns `?string`: + +```php +protected function validateNameInput(mixed $name): ?string +{ + if (!is_string($name)) { + return 'Server name must be a string'; + } + if ('' === trim($name)) { + return 'Server name cannot be empty'; + } + return null; +} +``` + +**Pattern B: Heavy I/O Validation** - Throws: + +```php +protected function validateGitRepo(string $repo): void +{ + if (!$process->isSuccessful()) { + throw new \RuntimeException("Cannot access git repository '{$repo}'"); + } +} +``` + +## Commands & Orchestration Traits + +Display exceptions directly without redundant prefixes: + +```php +// CORRECT +try { + $this->servers->create($server); +} catch (\RuntimeException $e) { + $this->nay($e->getMessage()); // Already complete + return Command::FAILURE; +} + +// WRONG - redundant prefix +$this->nay('Failed to add server: ' . $e->getMessage()); +``` + +**When to add context:** +- Displaying raw output for debugging +- Adding actionable troubleshooting steps +- Exception message is too technical + +## Silent Failures + +Return `null`/`false` only for optional operations: + +```php +// CORRECT - Optional detection +public function detectRemoteUrl(): ?string +{ + try { + return $process->isSuccessful() ? trim($process->getOutput()) : null; + } catch (\Exception) { + return null; // Not in git repo, that's okay + } +} + +// WRONG - Required operation returning null +public function executeCommand(): ?array +{ + } catch (\Throwable) { + return null; // Caller doesn't know WHY + } +} +``` + +## Exception Message Quality + +Every message must be: +- Complete: "SSH key does not exist: /path/to/key" +- User-facing: "Cannot connect to database. Check host and port." +- Actionable with context +- Free of redundant prefixes + +## Layer Responsibility + +| Layer | Display Errors? | Pattern | +|-------|-----------------|---------| +| Services | No | Throw complete exceptions | +| Repositories | No | Throw complete exceptions | +| Validation Traits | No | Return `?string` or throw | +| Orchestration Traits | Yes | Catch & display without prefix | +| Commands | Yes | Catch & display without prefix | diff --git a/docs/rules/playbooks.md b/docs/rules/playbooks.md new file mode 100644 index 00000000..49f25458 --- /dev/null +++ b/docs/rules/playbooks.md @@ -0,0 +1,176 @@ +# Playbook Rules + +All rules MANDATORY. + +## Core Principles + +Playbooks are idempotent, non-interactive bash scripts that: +- Execute one or more related tasks +- MUST be idempotent (safe to run multiple times) +- Receive context via environment variables +- Never prompt for user input +- Return parsable YAML output + +## Structure + +```bash +#!/usr/bin/env bash +set -o pipefail +export DEBIAN_FRONTEND=noninteractive + +# Validation +[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 +[[ -z $DEPLOYER_DISTRO ]] && echo "Error: DEPLOYER_DISTRO required" && exit 1 +[[ -z $DEPLOYER_PERMS ]] && echo "Error: DEPLOYER_PERMS required" && exit 1 +export DEPLOYER_PERMS + +# +# Helper Functions +# ---- + +run_cmd() { + if [[ $DEPLOYER_PERMS == 'root' ]]; then + "$@" + else + sudo -n "$@" + fi +} + +# +# Main Execution +# ---- + +main() { + # Tasks go here + + if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 + exit 1 + fi +} + +main "$@" +``` + +**Requirements:** +- Shebang: `#!/usr/bin/env bash` +- Always `set -o pipefail` (NOT `set -e`) +- Export `DEBIAN_FRONTEND=noninteractive` +- Validate `$DEPLOYER_OUTPUT_FILE` before any work +- Use `main()` function with `main "$@"` at bottom + +## Environment Variables + +Use `DEPLOYER_` prefix: +- `DEPLOYER_OUTPUT_FILE` - YAML output path (automatic) +- `DEPLOYER_DISTRO` - Distribution: `ubuntu|debian` +- `DEPLOYER_PERMS` - Permissions: `root|sudo|none` + +## Distribution Support + +Ubuntu and Debian only. Use `case` only when they differ: + +```bash +# When distributions differ +case $DEPLOYER_DISTRO in + ubuntu) + distro_packages=(software-properties-common) + ;; + debian) + distro_packages=(apt-transport-https lsb-release) + ;; +esac + +# Universal (no branching) +run_cmd apt-get update -q +run_cmd apt-get install -y -q caddy +``` + +## Non-Interactive Operation + +- `export DEBIAN_FRONTEND=noninteractive` +- Package managers: `-y -q` flags +- GPG: `--batch --yes` +- Never use `read` or interactive prompts + +## Idempotency + +Check before acting: + +```bash +if ! command -v caddy >/dev/null 2>&1; then + echo "→ Installing Caddy..." + run_cmd apt-get install -y -q caddy +fi + +if [[ ! -d /var/www/app ]]; then + echo "→ Creating /var/www/app..." + run_cmd mkdir -p /var/www/app +fi + +# For config files, check for custom content markers +if ! grep -q "import conf.d/localhost.caddy" /etc/caddy/Caddyfile 2>/dev/null; then + echo "→ Creating Caddyfile..." + run_cmd tee /etc/caddy/Caddyfile > /dev/null <<- 'EOF' + # custom config + EOF +fi +``` + +## Error Handling + +Use `set -o pipefail` but NOT `set -e`. Check explicitly: + +```bash +# Validation → stdout +[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: required" && exit 1 + +# Runtime → stderr +if ! mkdir -p /var/www/app 2>&1; then + echo "Error: Failed to create directory" >&2 + exit 1 +fi + +# Check YAML writes +if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 + exit 1 +fi +``` + +## Action Messages + +Use `→` prefix. Be explicit with paths/names/versions: + +```bash +# CORRECT - Explicit +echo "→ Creating /var/www/app directory..." +echo "→ Installing PHP 8.5..." + +# WRONG - Generic +echo "→ Creating directory..." +echo "→ Installing package..." + +# Conditional ops - message INSIDE block +if ! command -v caddy >/dev/null 2>&1; then + echo "→ Installing Caddy..." + run_cmd apt-get install -y -q caddy +fi +``` + +## Shared Helpers + +Helpers from `helpers.sh` are automatically inlined during remote execution: + +```bash +# Shared helpers are automatically inlined when executing playbooks remotely +# source "$(dirname "$0")/helpers.sh" +``` + +Never manually inline helpers into playbook files. + +See: `playbooks/server-info.sh` diff --git a/docs/rules/testing.md b/docs/rules/testing.md new file mode 100644 index 00000000..4330a16a --- /dev/null +++ b/docs/rules/testing.md @@ -0,0 +1,114 @@ +# Testing Rules + +All rules MANDATORY. + +**Philosophy:** "A test that never fails is not a test, it's a lie." + +**Framework:** Pest exclusively, `it()` syntax, 70%+ coverage + +## Running Tests + +```bash +composer pest # Full suite with coverage (parallel) +vendor/bin/pest $TEST_FILE # Specific file +``` + +## Dependency Injection in Tests + +DI Container rule applies to PRODUCTION code, not tests. + +**Unit Tests - Manual Instantiation:** + +```php +$mockFs = mockFilesystem(true, 'content'); +$service = new EnvService(new FilesystemService($mockFs), new Dotenv()); +``` + +**Command Tests - Container with Bindings:** + +```php +$container = mockCommandContainer(); +$command = $container->build(ServerAddCommand::class); + +// Override services +$container = mockCommandContainer(ssh: $mockSSH); + +// Pre-populate data +$container = mockCommandContainer( + inventoryData: ['servers' => [['name' => 'web1', 'host' => '192.168.1.1']]] +); +``` + +**Maintenance:** When adding service to BaseCommand, update `mockCommandContainer()` in `tests/TestHelpers.php`. + +## Test Minimalism + +**Target:** Keep test files under 1.8x source code size. + +**Rules:** +- Test core business logic only +- Use datasets: `->with([])` for multiple scenarios +- Eliminate overlap: no two tests covering same functionality +- Consolidate assertions: `expect($x)->toBe(1)->and($y)->toBe(2)` +- Mock external dependencies only + +**Don't consolidate:** Different public methods, exception vs normal flow, distinct business logic. + +## AAA Pattern + +```php +it('does something', function () { + // ARRANGE + $service = new Service(mock(Dependency::class)); + + // ACT + $result = $service->action(); + + // ASSERT + expect($result)->toBe('expected'); +}); +``` + +Exception tests: `// ACT & ASSERT` when act triggers assertion. + +## Testing Patterns + +**Forbidden:** + +```php +expect($x)->toBeInstanceOf(Class::class); // Type-only +expect($x)->toBeArray(); // Generic +expect($x)->not->toBeNull(); // Meaningless alone +expect(true)->toBeTrue(); // Literally meaningless +sleep(...); // Test logic not time +``` + +**Required:** + +```php +expect($config->getValue('host'))->toBe('example.com'); +$mock->shouldReceive('method')->with('param')->andReturn('result'); + +// For polling/timeout - use zero intervals +$service->waitForReady('id', timeout: 10, pollInterval: 0); +``` + +## Test Types + +**Unit Tests:** +- Mock all external dependencies +- Test single units in isolation +- Complete in milliseconds + +**Integration Tests:** +- Real file operations and external processes +- CLI commands and full workflows + +**Layer Strategy:** +- CLI Commands → Integration tests +- Business Services → Unit tests +- Utilities/Helpers → Unit tests + +## Static Analysis + +PHPStan applies to PRODUCTION code, not tests. Focus on functionality over type compliance. diff --git a/docs/rules/writing-docs.md b/docs/rules/writing-docs.md new file mode 100644 index 00000000..6c00f720 --- /dev/null +++ b/docs/rules/writing-docs.md @@ -0,0 +1,100 @@ +# Rules for Writing Docs + +Guidelines for documentation optimized for AI agents with limited token windows. + +## Token Efficiency + +- Imperative mood, not conversational prose +- One example per pattern maximum +- Remove "Benefits", "Why This Matters" sections +- Inline comments over separate explanations +- Bullets over paragraphs +- Single "All rules MANDATORY" per file +- No repetitive CRITICAL/IMMUTABLE warnings +- No emoji in headers + +## Structure + +**File Header:** + +```markdown +# [Section Name] + +All rules MANDATORY. +``` + +**Organization:** +- Clear, scannable headers +- Related rules grouped +- Alphabetical when no logical grouping +- Max 3 heading levels + +## Examples + +Show correct first, wrong only when non-obvious: + +```php +// CORRECT +$result = $container->build(Service::class); + +// WRONG - breaks DI +$result = new Service(new Dependency()); +``` + +**Rules:** +- Under 10 lines per example +- Use `// CORRECT` and `// WRONG` markers +- Inline comments over prose +- Don't explain well-known patterns (AAA, SOLID) +- Don't explain framework features + +## Cross-File Coordination + +**Single source of truth:** +- One primary location per concept +- Cross-reference by filename: "See commands.md" +- No line number references + +**Valid references:** + +```markdown +See @docs/rules/commands.md +Covered in architecture.md +``` + +## Writing Style + +**Prefer:** + +```markdown +Commands handle I/O. Services contain logic. No circular dependencies. +``` + +**Over:** + +```markdown +Commands are responsible for handling all user interaction including input and output operations, while Services provide the core business logic functionality. +``` + +**Emphasis Hierarchy:** +1. Code examples (most efficient) +2. Imperative bullets +3. Short declarative sentences +4. Tables (reference data only) +5. Prose (last resort) + +## Token Budget + +- AI agents have 8K-32K context windows +- Rules should consume <20% of tokens +- Leave 80% for code, history, responses +- Target: <3000 tokens total (~600-800 lines) + +## Maintenance Checklist + +Before committing: +1. Remove outdated file references +2. Check for duplication +3. Verify no contradictions +4. Test code examples +5. Compare token count (target: 35-65% of verbose version) From bf8e44b17ee8cf1503245d77be6ce511636825c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:34:44 +0200 Subject: [PATCH 04/18] refactor(commands): consolidate branch and commit commands into /commit Merged separate /create-branch and /create-commits commands into unified /commit command that handles both branch creation and commits in one step. Updated /deslop description for clarity. --- .claude/commands/commit.md | 34 ++++++++++++++++++++++++++++++ .claude/commands/create-branch.md | 18 ---------------- .claude/commands/create-commits.md | 19 ----------------- .claude/commands/deslop.md | 2 +- 4 files changed, 35 insertions(+), 38 deletions(-) create mode 100644 .claude/commands/commit.md delete mode 100644 .claude/commands/create-branch.md delete mode 100644 .claude/commands/create-commits.md diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md new file mode 100644 index 00000000..f3477b9c --- /dev/null +++ b/.claude/commands/commit.md @@ -0,0 +1,34 @@ +--- +description: Create a branch based on working tree changes +allowed-tools: Bash(git:*) +model: sonnet +--- + +Based on the changes made to this repository: + +A. If we're on the main branch, create a new branch with a suitable name + +Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: +feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. + +Keep the branch name short (≤ 50 chars) yet informative. Do not push, pull, or rebase. + +Examples: + +- feat/parser-add-php-84-attributes +- fix/ci-matrix-php-versions +- chore/deps-bump-composer-installers-2-3 + +B. Create one or more commits with suitable titles + +Use Conventional Commits to group related changes into cohesive commits (commits should be independently meaningful). + +Keep titles short (≤ 72 chars), imperative, no trailing period. Do not push, pull, or rebase. + +Examples: + +- feat(parser): add support for PHP 8.4 attributes +- fix(ci): correct matrix PHP versions in build workflow +- chore(deps): bump composer/installers to ^2.3 + +Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). diff --git a/.claude/commands/create-branch.md b/.claude/commands/create-branch.md deleted file mode 100644 index febb23c4..00000000 --- a/.claude/commands/create-branch.md +++ /dev/null @@ -1,18 +0,0 @@ ---- -description: Create a branch based on working tree changes -allowed-tools: Bash(git:*) -model: haiku ---- - -Based on the changes made to this repository create a new branch with a suitable name. - -Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: -feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. - -Keep the branch name short (≤ 50 chars) yet informative. Do not push, pull, or rebase. - -Examples: - -- feat/parser-add-php-84-attributes -- fix/ci-matrix-php-versions -- chore/deps-bump-composer-installers-2-3 diff --git a/.claude/commands/create-commits.md b/.claude/commands/create-commits.md deleted file mode 100644 index c616d535..00000000 --- a/.claude/commands/create-commits.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -description: Create conventional commits from working tree changes -allowed-tools: Bash(git:*) -model: haiku ---- - -Based on the changes made to this repository create one or more commits with suitable titles. - -Use Conventional Commits to group related changes into cohesive commits (commits should be independently meaningful). - -Keep titles short (≤ 72 chars), imperative, no trailing period. Do not push, pull, or rebase. - -Examples: - -- feat(parser): add support for PHP 8.4 attributes -- fix(ci): correct matrix PHP versions in build workflow -- chore(deps): bump composer/installers to ^2.3 - -Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). diff --git a/.claude/commands/deslop.md b/.claude/commands/deslop.md index fc978c9f..ed0c0775 100644 --- a/.claude/commands/deslop.md +++ b/.claude/commands/deslop.md @@ -1,5 +1,5 @@ --- -description: Remove AI-generated code slop from the branch +description: Remove AI-generated code slop from the current branch argument-hint: [base-branch] --- From 7b13e768f7067f3ce151d4329711f3e9328070c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:34:48 +0200 Subject: [PATCH 05/18] feat(skills): add playbook skill for bash script development --- .claude/skills/playbook/SKILL.md | 376 +++++++++++++++++++++++++++++++ 1 file changed, 376 insertions(+) create mode 100644 .claude/skills/playbook/SKILL.md diff --git a/.claude/skills/playbook/SKILL.md b/.claude/skills/playbook/SKILL.md new file mode 100644 index 00000000..786dece7 --- /dev/null +++ b/.claude/skills/playbook/SKILL.md @@ -0,0 +1,376 @@ +--- +name: playbook +description: Use this skill when writing, creating, or modifying bash playbook scripts in the playbooks/ directory. Activates for tasks involving server provisioning, site deployment, package installation, or any idempotent bash automation scripts. +--- + +# Playbook Development + +Playbooks are idempotent, non-interactive bash scripts that execute server tasks remotely. They receive context via environment variables and return YAML output. + +All rules MANDATORY. Bash style based on https://style.ysap.sh/md + +## Required Structure + +Every playbook MUST follow this exact structure: + +```bash +#!/usr/bin/env bash + +# +# {Playbook Name} Playbook - Ubuntu/Debian Only +# +# {Brief description of what this playbook does} +# ---- +# +# {Detailed description including:} +# {- What the playbook installs/configures} +# {- Prerequisites or dependencies} +# {- Any important notes} +# +# Required Environment Variables: +# DEPLOYER_OUTPUT_FILE - Output file path +# DEPLOYER_DISTRO - Exact distribution: ubuntu|debian +# DEPLOYER_PERMS - Permissions: root|sudo|none +# {DEPLOYER_CUSTOM_VAR} - {Description} +# +# Returns YAML with: +# - status: success +# - {key}: {description} +# + +set -o pipefail +export DEBIAN_FRONTEND=noninteractive + +[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 +[[ -z $DEPLOYER_DISTRO ]] && echo "Error: DEPLOYER_DISTRO required" && exit 1 +[[ -z $DEPLOYER_PERMS ]] && echo "Error: DEPLOYER_PERMS required" && exit 1 +# Add validation for custom variables here +export DEPLOYER_PERMS + +# Shared helpers are automatically inlined when executing playbooks remotely +# source "$(dirname "$0")/helpers.sh" + +# ---- +# {Section Name} Functions +# ---- + +# +# {Function description} + +function_name() { + # Implementation +} + +# ---- +# Main Execution +# ---- + +main() { + # Execute tasks + function_name + + # Write output YAML + if ! cat > "$DEPLOYER_OUTPUT_FILE" <<- EOF; then + status: success + EOF + echo "Error: Failed to write output file" >&2 + exit 1 + fi +} + +main "$@" +``` + +## Critical Requirements + +### Header Block +- Shebang: `#!/usr/bin/env bash` (always first line) +- `set -o pipefail` (NEVER use `set -e`) +- `export DEBIAN_FRONTEND=noninteractive` +- Validate ALL required `DEPLOYER_*` variables before any work +- `export DEPLOYER_PERMS` after validation + +### Environment Variables +All variables use `DEPLOYER_` prefix: +- `DEPLOYER_OUTPUT_FILE` - YAML output path (always required) +- `DEPLOYER_DISTRO` - Distribution: `ubuntu|debian` +- `DEPLOYER_PERMS` - Permissions: `root|sudo|none` + +### Available Helper Functions +These are automatically inlined from `helpers.sh`. Never manually inline helpers into playbook files. + +| Function | Purpose | +|----------|---------| +| `run_cmd` | Execute with appropriate permissions (root or sudo) | +| `run_as_deployer` | Execute as deployer user with env preservation | +| `fail "message"` | Print error and exit | +| `detect_php_default` | Get default PHP version | +| `wait_for_dpkg_lock` | Wait for package manager lock | +| `apt_get_with_retry` | apt-get with automatic retry on lock | +| `link_shared_resources` | Link shared resources to release | + +See `playbooks/server-info.sh` for a simple example. + +## Non-Interactive Operation + +Playbooks must never prompt for user input: + +- `export DEBIAN_FRONTEND=noninteractive` - Prevents apt prompts +- Package managers: `-y -q` flags (`apt-get install -y -q`) +- GPG: `--batch --yes` flags (`gpg --batch --yes --dearmor`) +- Never use `read` or interactive prompts + +```bash +# Adding a repository key (non-interactive) +curl -fsSL https://example.com/key.gpg | gpg --batch --yes --dearmor -o /etc/apt/keyrings/example.gpg +``` + +## Idempotency Patterns + +ALWAYS check before acting: + +```bash +# Command existence +if ! command -v caddy >/dev/null 2>&1; then + echo "→ Installing Caddy..." + run_cmd apt-get install -y -q caddy +fi + +# Directory existence +if ! run_cmd test -d /var/www/app; then + echo "→ Creating /var/www/app..." + run_cmd mkdir -p /var/www/app +fi + +# File existence +if ! run_cmd test -f "$config_file"; then + echo "→ Creating configuration..." + run_cmd tee "$config_file" > /dev/null <<- 'EOF' + # config content + EOF +fi + +# Config content marker +if ! grep -q "DEPLOYER-MARKER" /etc/config 2>/dev/null; then + echo "→ Updating configuration..." + # modify config +fi + +# Service state +if ! systemctl is-enabled --quiet service 2>/dev/null; then + run_cmd systemctl enable --quiet service +fi +``` + +## Error Handling + +```bash +# Validation errors → stdout, then exit +[[ -z $DEPLOYER_VAR ]] && echo "Error: DEPLOYER_VAR required" && exit 1 + +# Runtime errors → stderr, then exit +if ! run_cmd mkdir -p /var/www/app 2>&1; then + echo "Error: Failed to create directory" >&2 + exit 1 +fi + +# Inline error check +cd /path || exit + +# Using fail helper +run_cmd chown deployer:deployer /path || fail "Failed to set ownership" + +# YAML output write check +if ! cat > "$DEPLOYER_OUTPUT_FILE" <<- EOF; then + status: success +EOF + echo "Error: Failed to write output file" >&2 + exit 1 +fi +``` + +Rules: +- Use `set -o pipefail` but NOT `set -e` - explicit checking preferred +- Never use `eval` + +## Action Messages + +Use `→` prefix. Be explicit with paths/names/versions: + +```bash +# CORRECT - Explicit +echo "→ Creating /var/www/app directory..." +echo "→ Installing PHP 8.4..." +echo "→ Configuring Caddy for example.com..." + +# WRONG - Generic +echo "→ Creating directory..." +echo "→ Installing package..." + +# Message INSIDE conditional block +if ! command -v caddy >/dev/null 2>&1; then + echo "→ Installing Caddy..." + run_cmd apt-get install -y -q caddy +fi +``` + +## Distribution Handling + +Only branch when Ubuntu and Debian differ: + +```bash +# When distributions differ +case $DEPLOYER_DISTRO in + ubuntu) + distro_packages=(software-properties-common) + ;; + debian) + distro_packages=(apt-transport-https lsb-release) + ;; +esac + +# Universal commands (no branching needed) +run_cmd apt-get update -q +apt_get_with_retry install -y -q "${packages[@]}" +``` + +## Bash Style Rules + +### Core Syntax + +**Conditionals:** `[[ ... ]]` not `[ ... ]` +```bash +[[ -d /etc ]] # CORRECT +[ -d /etc ] # WRONG +``` + +**Command Substitution:** `$(...)` not backticks +```bash +foo=$(date) # CORRECT +foo=`date` # WRONG +``` + +**Math:** `((...))` and `$((...))`, never `let` +```bash +if ((a > b)); then ... # CORRECT +if [[ $a -gt $b ]]; then # WRONG +``` + +**Functions:** No `function` keyword, always use `local` +```bash +foo() { local i=5; } # CORRECT +function foo { i=5; } # WRONG +``` + +**Block Statements:** `then`/`do` on same line +```bash +if true; then ... # CORRECT +while true; do ... # CORRECT +``` + +### Parameter Handling + +**Expansion:** Prefer over external commands +```bash +prog=${0##*/} # CORRECT - basename +nonumbers=${name//[0-9]/} # CORRECT - remove numbers +prog=$(basename "$0") # WRONG - external command +``` + +**Quoting:** Double for expansions, single for literals +```bash +echo "$foo" # expansion needs quotes +bar='literal' # no expansion +if [[ -n $foo ]]; then # [[ ]] doesn't word-split +``` + +**Arrays:** Use bash arrays, not strings +```bash +modules=(a b c) # CORRECT +for m in "${modules[@]}" # CORRECT +modules='a b c' # WRONG +``` + +### File Operations + +```bash +# Streaming read +while IFS=: read -r user _; do + echo "$user" +done < /etc/passwd + +# CORRECT +grep foo file + +# WRONG - useless cat +cat file | grep foo + +# CORRECT - globs +for f in *; do ... + +# WRONG - parsing ls +for f in $(ls); do ... +``` + +### Formatting + +- Tabs for indentation +- Max 80 columns +- Semicolons only in control statements +- Max 1 blank line between sections + +## Code Organization + +### Section Comments +```bash +# ---- +# Section Name +# ---- +``` + +### Function Comments +```bash +# +# Brief description of what function does + +function_name() { + # implementation +} +``` + +### Grouping +- Group related functions into comment-separated sections +- Order functions alphabetically within sections after grouping +- Place `main()` at the bottom + +## YAML Output + +Always write YAML output at end of `main()`: + +```bash +# Simple success +if ! cat > "$DEPLOYER_OUTPUT_FILE" <<- EOF; then + status: success +EOF + echo "Error: Failed to write output file" >&2 + exit 1 +fi + +# With additional data +if ! cat > "$DEPLOYER_OUTPUT_FILE" <<- EOF; then + status: success + site_path: ${site_path} + php_version: ${php_version} +EOF + echo "Error: Failed to write output file" >&2 + exit 1 +fi +``` + +## Quality Gates + +After writing playbook, run: +```bash +composer bash playbooks/new-playbook.sh # Format +composer bash:check # Verify +``` From 68981b85a14e8285fcb17349a4ce708042c4be29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:34:52 +0200 Subject: [PATCH 06/18] docs: remove separate bash style and playbook rules Consolidated into playbook skill. Removed docs/rules/bash-style.md and docs/rules/playbooks.md, updated CLAUDE.md references. --- CLAUDE.md | 2 - docs/rules/bash-style.md | 113 ------------------------- docs/rules/playbooks.md | 176 --------------------------------------- 3 files changed, 291 deletions(-) delete mode 100644 docs/rules/bash-style.md delete mode 100644 docs/rules/playbooks.md diff --git a/CLAUDE.md b/CLAUDE.md index 9082b51c..236bb36a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,8 +67,6 @@ Don't run, create, or update tests UNLESS explicitly instructed. | Symfony Console commands | @docs/rules/commands.md | | Exception handling | @docs/rules/exceptions.md | | Pest testing | @docs/rules/testing.md | -| Bash playbooks | @docs/rules/playbooks.md | -| Shell script style | @docs/rules/bash-style.md | | Writing documentation | @docs/rules/writing-docs.md | ## References diff --git a/docs/rules/bash-style.md b/docs/rules/bash-style.md deleted file mode 100644 index 33c1250f..00000000 --- a/docs/rules/bash-style.md +++ /dev/null @@ -1,113 +0,0 @@ -# Bash Style - -All rules MANDATORY. Based on https://style.ysap.sh/md - -## Core Syntax - -**Conditionals:** `[[ ... ]]` not `[ ... ]` - -```bash -[[ -d /etc ]] # CORRECT -[ -d /etc ] # WRONG -``` - -**Command Substitution:** `$(...)` not backticks - -```bash -foo=$(date) # CORRECT -foo=`date` # WRONG -``` - -**Math:** `((...))` and `$((...))`, never `let` - -```bash -if ((a > b)); then ... # CORRECT -if [[ $a -gt $b ]]; then # WRONG -``` - -**Functions:** No `function` keyword, always `local` - -```bash -foo() { local i=5; } # CORRECT -function foo { i=5; } # WRONG -``` - -**Block Statements:** `then`/`do` same line - -```bash -if true; then ... # CORRECT -while true; do ... # CORRECT -``` - -## Parameter Handling - -**Expansion:** Prefer over external commands - -```bash -prog=${0##*/} # CORRECT - basename -nonumbers=${name//[0-9]/} # CORRECT - remove numbers -prog=$(basename "$0") # WRONG - external command -``` - -**Quoting:** Double for expansions, single for literals - -```bash -echo "$foo" # expansion needs quotes -bar='literal' # no expansion -if [[ -n $foo ]]; then # [[ ]] doesn't word-split -``` - -**Arrays:** Use bash arrays, not strings - -```bash -modules=(a b c) # CORRECT -for m in "${modules[@]}" # CORRECT -modules='a b c' # WRONG -``` - -## Error Handling - -```bash -cd /path || exit # CORRECT - exit on failure -cd /path # WRONG - unchecked -``` - -- Use `set -o pipefail` in playbooks -- Don't use `set -e` - explicit checking preferred -- Never use `eval` - -## File Operations - -```bash -# Streaming read -while IFS=: read -r user _; do - echo "$user" -done < /etc/passwd - -# CORRECT -grep foo file - -# WRONG - useless cat -cat file | grep foo - -# CORRECT - globs -for f in *; do ... - -# WRONG - parsing ls -for f in $(ls); do ... -``` - -## Formatting - -- Tabs for indentation -- Max 80 columns -- Semicolons only in control statements -- Max 1 blank line between sections -- Shebang: `#!/usr/bin/env bash` - -## Quality Gates - -```bash -composer bash # Format playbooks/*.sh -composer bash:check # Check only -``` diff --git a/docs/rules/playbooks.md b/docs/rules/playbooks.md deleted file mode 100644 index 49f25458..00000000 --- a/docs/rules/playbooks.md +++ /dev/null @@ -1,176 +0,0 @@ -# Playbook Rules - -All rules MANDATORY. - -## Core Principles - -Playbooks are idempotent, non-interactive bash scripts that: -- Execute one or more related tasks -- MUST be idempotent (safe to run multiple times) -- Receive context via environment variables -- Never prompt for user input -- Return parsable YAML output - -## Structure - -```bash -#!/usr/bin/env bash -set -o pipefail -export DEBIAN_FRONTEND=noninteractive - -# Validation -[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 -[[ -z $DEPLOYER_DISTRO ]] && echo "Error: DEPLOYER_DISTRO required" && exit 1 -[[ -z $DEPLOYER_PERMS ]] && echo "Error: DEPLOYER_PERMS required" && exit 1 -export DEPLOYER_PERMS - -# -# Helper Functions -# ---- - -run_cmd() { - if [[ $DEPLOYER_PERMS == 'root' ]]; then - "$@" - else - sudo -n "$@" - fi -} - -# -# Main Execution -# ---- - -main() { - # Tasks go here - - if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 - fi -} - -main "$@" -``` - -**Requirements:** -- Shebang: `#!/usr/bin/env bash` -- Always `set -o pipefail` (NOT `set -e`) -- Export `DEBIAN_FRONTEND=noninteractive` -- Validate `$DEPLOYER_OUTPUT_FILE` before any work -- Use `main()` function with `main "$@"` at bottom - -## Environment Variables - -Use `DEPLOYER_` prefix: -- `DEPLOYER_OUTPUT_FILE` - YAML output path (automatic) -- `DEPLOYER_DISTRO` - Distribution: `ubuntu|debian` -- `DEPLOYER_PERMS` - Permissions: `root|sudo|none` - -## Distribution Support - -Ubuntu and Debian only. Use `case` only when they differ: - -```bash -# When distributions differ -case $DEPLOYER_DISTRO in - ubuntu) - distro_packages=(software-properties-common) - ;; - debian) - distro_packages=(apt-transport-https lsb-release) - ;; -esac - -# Universal (no branching) -run_cmd apt-get update -q -run_cmd apt-get install -y -q caddy -``` - -## Non-Interactive Operation - -- `export DEBIAN_FRONTEND=noninteractive` -- Package managers: `-y -q` flags -- GPG: `--batch --yes` -- Never use `read` or interactive prompts - -## Idempotency - -Check before acting: - -```bash -if ! command -v caddy >/dev/null 2>&1; then - echo "→ Installing Caddy..." - run_cmd apt-get install -y -q caddy -fi - -if [[ ! -d /var/www/app ]]; then - echo "→ Creating /var/www/app..." - run_cmd mkdir -p /var/www/app -fi - -# For config files, check for custom content markers -if ! grep -q "import conf.d/localhost.caddy" /etc/caddy/Caddyfile 2>/dev/null; then - echo "→ Creating Caddyfile..." - run_cmd tee /etc/caddy/Caddyfile > /dev/null <<- 'EOF' - # custom config - EOF -fi -``` - -## Error Handling - -Use `set -o pipefail` but NOT `set -e`. Check explicitly: - -```bash -# Validation → stdout -[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: required" && exit 1 - -# Runtime → stderr -if ! mkdir -p /var/www/app 2>&1; then - echo "Error: Failed to create directory" >&2 - exit 1 -fi - -# Check YAML writes -if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 -fi -``` - -## Action Messages - -Use `→` prefix. Be explicit with paths/names/versions: - -```bash -# CORRECT - Explicit -echo "→ Creating /var/www/app directory..." -echo "→ Installing PHP 8.5..." - -# WRONG - Generic -echo "→ Creating directory..." -echo "→ Installing package..." - -# Conditional ops - message INSIDE block -if ! command -v caddy >/dev/null 2>&1; then - echo "→ Installing Caddy..." - run_cmd apt-get install -y -q caddy -fi -``` - -## Shared Helpers - -Helpers from `helpers.sh` are automatically inlined during remote execution: - -```bash -# Shared helpers are automatically inlined when executing playbooks remotely -# source "$(dirname "$0")/helpers.sh" -``` - -Never manually inline helpers into playbook files. - -See: `playbooks/server-info.sh` From 97bf8b9d979e14fa61ef95e0f762529c12e90c11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:45:17 +0200 Subject: [PATCH 07/18] feat(skills): add command skill for Symfony Console development Replaces docs/rules/commands.md with interactive skill that provides: - Complete command structure template - Interactive + CLI options patterns - Input validation with Laravel Prompts - Confirmation and multi-path prompt patterns - Quality gates and checklist --- .claude/skills/command/SKILL.md | 342 ++++++++++++++++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100644 .claude/skills/command/SKILL.md diff --git a/.claude/skills/command/SKILL.md b/.claude/skills/command/SKILL.md new file mode 100644 index 00000000..c1fda3c7 --- /dev/null +++ b/.claude/skills/command/SKILL.md @@ -0,0 +1,342 @@ +--- +name: command +description: Use this skill when creating, modifying, or updating Symfony Console commands. Activates for tasks involving CLI commands, user prompts, input validation, or interactive/non-interactive command patterns. +--- + +# Symfony Console Command Development + +Commands are Symfony Console classes that handle user I/O. They use Laravel Prompts for interactive input and BaseCommand methods for output. + +All rules MANDATORY. + +## Core Principle + +Every command MUST be fully runnable non-interactively via CLI options. Every prompt MUST have a corresponding CLI option. + +## Required Structure + +Every command MUST follow this structure: + +```php +addOption('name', null, InputOption::VALUE_REQUIRED, 'Resource name'); + $this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $this->setupIo($input, $output); + + // Get input + $name = $this->getOptionOrPrompt( + 'name', + fn () => $this->promptText(label: 'Name:', required: true) + ); + + // Business logic via service + $this->service->doSomething($name); + + // Output and replay + $this->yay("Created {$name}"); + $this->commandReplay('namespace:action', [ + 'name' => $name, + 'yes' => true, + ]); + + return Command::SUCCESS; + } +} +``` + +## Output Methods + +NEVER use Symfony IO methods directly. Use BaseCommand methods: + +```php +// Text output +$this->out(['Multiple', 'lines']); // Plain text (array or string) +$this->hr(); // Horizontal rule + +// Headings +$this->h1('Section Heading'); // Large heading + +// Status messages +$this->yay('Success'); // Green checkmark +$this->nay('Failed'); // Red X +$this->warn('Warning'); // Yellow warning +$this->info('Info'); // Blue info + +// Structured output +$this->displayDeets(['Key' => 'value']); // Key-value pairs +$this->ul(['Item 1', 'Item 2']); // Bullet list +$this->ol(['Step 1', 'Step 2']); // Numbered list +``` + +**Trait Organization:** +- `ConsoleOutputTrait`: Output/formatting using `$this->io` +- `ConsoleInputTrait`: Input using `$this->input` +- `BaseCommand`: Shared initialization, configuration + +## User Input with Laravel Prompts + +```php +use function Laravel\Prompts\{text, password, confirm, select, multiselect, spin}; + +// Text input +$name = text('Name?', required: true); + +// Password (hidden input) +$secret = password('API Key?'); + +// Confirmation +$confirmed = confirm('Proceed?'); + +// Single selection +$env = select('Environment:', ['dev', 'staging', 'prod']); + +// Multiple selection +$features = multiselect('Features:', ['api', 'auth', 'cache']); + +// Long-running operation with spinner +$result = spin(fn() => $this->service->process(), 'Processing...'); +``` + +## Interactive + Options Pattern + +The pattern for combining interactive prompts with CLI options: + +```php +protected function configure(): void +{ + parent::configure(); + $this->addOption('name', null, InputOption::VALUE_REQUIRED, 'Server name'); +} + +protected function execute(InputInterface $input, OutputInterface $output): int +{ + $this->setupIo($input, $output); + + // If --name provided, use it; otherwise prompt + $name = $this->getOptionOrPrompt( + 'name', + fn () => $this->promptText(label: 'Server name:', required: true) + ); + + return Command::SUCCESS; +} +``` + +## Input Validation + +### Validator Signature + +Returns `?string` (error message or null for valid): + +```php +protected function validateNameInput(mixed $value): ?string +{ + if (!is_string($value)) { + return 'Name must be a string'; + } + if ('' === trim($value)) { + return 'Name cannot be empty'; + } + if (null !== $this->repo->findByName($value)) { + return "'{$value}' already exists"; + } + + return null; +} +``` + +### Usage with getValidatedOptionOrPrompt + +```php +$name = $this->getValidatedOptionOrPrompt( + 'name', + fn ($validate) => $this->promptText(label: 'Name:', validate: $validate), + fn ($value) => $this->validateNameInput($value) +); + +if (null === $name) { + return Command::FAILURE; // Validation failed (CLI mode) +} +``` + +### Naming Convention + +| Pattern | Returns | Use Case | +|---------|---------|----------| +| `validate*Input()` | `?string` | Prompts and CLI options | +| `validate*()` | throws | Heavy I/O validation | + +## Boolean Flags + +### Simple Flag (VALUE_NONE) + +```php +// Definition +$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); + +// Usage: --yes or -y +$skipConfirm = $this->input->getOption('yes'); +``` + +### Tri-State Flag (VALUE_NEGATABLE) + +```php +// Definition +$this->addOption('php-default', null, InputOption::VALUE_NEGATABLE, 'Set as default PHP'); + +// Usage: --php-default (true), --no-php-default (false), omitted (null/prompt) +$phpDefault = $this->input->getOption('php-default'); +if (null === $phpDefault) { + $phpDefault = $this->promptConfirm('Set as default PHP?'); +} +``` + +## Multiselect with CLI + +Handle both array (prompt) and comma-separated string (CLI): + +```php +$selected = $this->getOptionOrPrompt( + 'databases', + fn () => $this->promptMultiselect(label: 'Databases:', options: $options) +); + +// Normalize: CLI gives string, prompt gives array +if (is_string($selected)) { + $selected = array_filter(array_map(trim(...), explode(',', $selected))); +} +``` + +## Multi-Path Prompts + +When a prompt offers multiple paths (e.g., "generate new" vs "use existing"), create separate options: + +```php +// Separate options for each path +$this->addOption('generate-deploy-key', null, InputOption::VALUE_NONE, 'Generate new deploy key'); +$this->addOption('custom-deploy-key', null, InputOption::VALUE_REQUIRED, 'Path to existing key'); + +// Detect conflicts +$generateKey = $this->input->getOption('generate-deploy-key'); +$customKeyPath = $this->input->getOption('custom-deploy-key'); + +if ($generateKey && null !== $customKeyPath) { + $this->nay('Cannot use both --generate-deploy-key and --custom-deploy-key'); + return Command::FAILURE; +} +``` + +## Confirmation Patterns + +### Simple Confirmation (`--yes`) + +For standard confirmations: + +```php +// Definition +$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); + +// Usage +$confirmed = $this->getOptionOrPrompt('yes', fn () => $this->promptConfirm('Proceed?')); +if (!$confirmed) { + return Command::SUCCESS; +} +``` + +### Type-to-Confirm (`--force`) + +For destructive operations requiring explicit confirmation: + +```php +// Definition +$this->addOption('force', 'f', InputOption::VALUE_NONE, 'Skip type-to-confirm'); + +// Usage +$forceSkip = $this->input->getOption('force'); +if (!$forceSkip) { + $typedName = $this->promptText(label: "Type '{$server->name}' to confirm deletion:"); + if ($typedName !== $server->name) { + $this->nay('Name does not match. Aborting.'); + return Command::FAILURE; + } +} +``` + +## Command Options Naming + +| Option | Usage | Type | +|--------|-------|------| +| `--server` | Select existing server | VALUE_REQUIRED | +| `--domain` | Select existing site | VALUE_REQUIRED | +| `--name` | Define new resource name | VALUE_REQUIRED | +| `--yes` / `-y` | Skip confirmation | VALUE_NONE | +| `--force` / `-f` | Skip type-to-confirm | VALUE_NONE | + +**Golden Rule:** `--server`/`--domain` for SELECTING existing resources, `--name` for DEFINING new ones. + +## Command Completion + +Always call `commandReplay()` before returning SUCCESS: + +```php +$this->commandReplay('server:delete', [ + 'server' => $server->name, + 'yes' => true, +]); + +return Command::SUCCESS; +``` + +This outputs the equivalent non-interactive command for documentation/automation. + +## Quality Gates + +After writing command, run: + +```bash +vendor/bin/rector src/Command/NewCommand.php +vendor/bin/pint src/Command/NewCommand.php +vendor/bin/phpstan analyse src/Command/NewCommand.php +``` + +## Checklist + +Before completing a command: + +- [ ] Every prompt has corresponding `addOption()` +- [ ] Multi-path prompts have separate options +- [ ] Conflicting options detected and rejected +- [ ] CLI values validated against allowed options +- [ ] `commandReplay()` called before SUCCESS +- [ ] Type annotations on option retrievals +- [ ] Uses BaseCommand output methods (never SymfonyStyle directly) +- [ ] Command is fully runnable non-interactively From 3a0cbb6ce4396f164ef5c8fa4152afa8a0ea4a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:45:24 +0200 Subject: [PATCH 08/18] docs: remove commands.md in favor of command skill The command skill provides the same guidance in an interactive, context-aware format. Also updates /commit command description. --- .claude/commands/commit.md | 6 +- docs/rules/commands.md | 188 ------------------------------------- 2 files changed, 3 insertions(+), 191 deletions(-) delete mode 100644 docs/rules/commands.md diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md index f3477b9c..6b82dd18 100644 --- a/.claude/commands/commit.md +++ b/.claude/commands/commit.md @@ -1,12 +1,12 @@ --- -description: Create a branch based on working tree changes +description: Create a branch and commits based on working tree changes allowed-tools: Bash(git:*) -model: sonnet +model: haiku --- Based on the changes made to this repository: -A. If we're on the main branch, create a new branch with a suitable name +A. If we're on the main branch, create a new branch with a suitable name Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. diff --git a/docs/rules/commands.md b/docs/rules/commands.md deleted file mode 100644 index 782aed20..00000000 --- a/docs/rules/commands.md +++ /dev/null @@ -1,188 +0,0 @@ -# Symfony Console Rules - -All rules MANDATORY. - -## Core Principle - -Every command MUST be fully runnable non-interactively via CLI options. Every prompt MUST have a corresponding CLI option. - -## Output Methods - -NEVER use Symfony IO methods directly - use BaseCommand methods: - -```php -$this->out(['Multiple', 'lines']); -$this->hr(); -$this->h1('Section Heading'); -$this->displayDeets(['Key' => 'value']); -$this->yay('Success'); // checkmark -$this->nay('Failed'); // red X -$this->warn('Warning'); // warning -$this->info('Info'); // info -$this->ul(['Item 1', 'Item 2']); // bullet list -$this->ol(['Step 1', 'Step 2']); // numbered list -``` - -**Trait Organization:** -- ConsoleOutputTrait: Output/formatting using `$this->io` -- ConsoleInputTrait: Input using `$this->input` -- BaseCommand: Shared initialization, configuration - -## User Input with Laravel Prompts - -```php -use function Laravel\Prompts\{text, password, confirm, select, multiselect, spin}; - -$name = text('Name?', required: true); -$env = select('Environment:', ['dev', 'staging', 'prod']); -$result = spin(fn() => $this->service->process(), 'Processing...'); -``` - -## Interactive + Options Pattern - -```php -protected function configure(): void { - parent::configure(); - $this->addOption('name', null, InputOption::VALUE_REQUIRED, 'Server name'); -} - -protected function execute(InputInterface $input, OutputInterface $output): int { - $name = $this->getOptionOrPrompt( - 'name', - fn () => $this->promptText(label: 'Server name:', required: true) - ); - return Command::SUCCESS; -} -``` - -## Input Validation - -**Validator Signature** - Returns `?string` (error message or null): - -```php -protected function validateNameInput(mixed $value): ?string -{ - if (!is_string($value)) { - return 'Name must be a string'; - } - if ('' === trim($value)) { - return 'Name cannot be empty'; - } - if (null !== $this->repo->findByName($value)) { - return "'{$value}' already exists"; - } - return null; -} -``` - -**Usage with getValidatedOptionOrPrompt:** - -```php -$name = $this->getValidatedOptionOrPrompt( - 'name', - fn ($validate) => $this->promptText(label: 'Name:', validate: $validate), - fn ($value) => $this->validateNameInput($value) -); - -if (null === $name) { - return Command::FAILURE; // Validation failed -} -``` - -**Naming Convention:** -- `validate*Input()` - Returns `?string` (for prompts/options) -- `validate*()` - Throws exceptions (for heavy I/O) - -## Boolean Flags - -```php -// VALUE_NONE - Simple flag -$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); - -// VALUE_NEGATABLE - Tri-state (--flag, --no-flag, or prompt) -$this->addOption('php-default', null, InputOption::VALUE_NEGATABLE, 'Set as default PHP'); -``` - -## Multiselect with CLI - -```php -$selected = $this->getOptionOrPrompt( - 'databases', - fn () => $this->promptMultiselect(label: 'Databases:', options: $options) -); - -// Handle both array (prompt) and string (CLI) -if (is_string($selected)) { - $selected = array_filter(array_map(trim(...), explode(',', $selected))); -} -``` - -## Multi-Path Prompts - -Create separate options for each path: - -```php -// Separate options for each path -$this->addOption('generate-deploy-key', null, InputOption::VALUE_NONE, 'Generate key'); -$this->addOption('custom-deploy-key', null, InputOption::VALUE_REQUIRED, 'Custom key path'); - -// Check for conflicts -if ($generateKey && null !== $customKeyPath) { - $this->nay('Cannot use both options'); - return Command::FAILURE; -} -``` - -## Confirmation Patterns - -**Simple (`--yes`):** - -```php -$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); -$confirmed = $this->getOptionOrPrompt('yes', fn () => $this->promptConfirm('Sure?')); -``` - -**Type-to-Confirm (`--force`) - Destructive ops:** - -```php -if (!$forceSkip) { - $typedName = $this->promptText(label: "Type '{$server->name}' to confirm:"); - if ($typedName !== $server->name) { - $this->nay('Name does not match'); - return Command::FAILURE; - } -} -``` - -## Command Options Naming - -| Option | Usage | Type | -|--------|-------|------| -| `--server` | Select existing server | VALUE_REQUIRED | -| `--domain` | Select existing site | VALUE_REQUIRED | -| `--name` | Define new resource name | VALUE_REQUIRED | -| `--yes` / `-y` | Skip confirmation | VALUE_NONE | -| `--force` / `-f` | Skip type-to-confirm | VALUE_NONE | - -**Golden Rule:** `--server`/`--domain` for SELECTING existing, `--name` for DEFINING new. - -## Command Completion - -Always call `commandReplay()` before SUCCESS: - -```php -$this->commandReplay('server:delete', [ - 'server' => $server->name, - 'yes' => true, -]); -return Command::SUCCESS; -``` - -## Checklist - -- [ ] Every prompt has corresponding `addOption()` -- [ ] Multi-path prompts have separate options -- [ ] Conflicting options detected and rejected -- [ ] CLI values validated against allowed options -- [ ] `commandReplay()` called before SUCCESS -- [ ] Type annotations on option retrievals From 669b7c7ef6d5978f2f0a943b46a45a2690f175c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:57:35 +0200 Subject: [PATCH 09/18] feat(skills): add testing skill for Pest development Replaces docs/rules/testing.md with comprehensive testing skill that includes: - AAA pattern enforcement with examples - Test naming conventions - Datasets and assertion chaining patterns - Mockery patterns and examples - Architecture tests - 70%+ coverage requirement - Quality gates checklist --- .claude/skills/testing/SKILL.md | 345 ++++++++++++++++++++++++++++++++ 1 file changed, 345 insertions(+) create mode 100644 .claude/skills/testing/SKILL.md diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md new file mode 100644 index 00000000..d07f7e82 --- /dev/null +++ b/.claude/skills/testing/SKILL.md @@ -0,0 +1,345 @@ +--- +name: testing +description: Use this skill when writing, creating, or modifying Pest tests. Activates for tasks involving unit tests, integration tests, test assertions, mocking, or test coverage improvements. +--- + +# Pest Testing Development + +Tests verify behavior using Pest PHP with the `it()` syntax. Tests follow AAA pattern and prioritize testing business logic over type checking. + +All rules MANDATORY. + +**Philosophy:** "A test that never fails is not a test, it's a lie." + +**Coverage:** Maintain 70%+ code coverage. + +## Running Tests + +```bash +composer pest # Full suite with coverage (parallel) +vendor/bin/pest $TEST_FILE # Specific file +``` + +## Required Structure + +Every test file MUST follow this structure: + +```php +action(); + + // ASSERT + expect($result)->toBe('expected'); +}); + +it('throws when invalid input', function () { + $service = new Service(); + + // ACT & ASSERT + expect(fn () => $service->action('invalid')) + ->toThrow(InvalidArgumentException::class, 'Expected message'); +}); +``` + +## AAA Pattern + +Every test MUST follow Arrange-Act-Assert: + +```php +it('calculates total with tax', function () { + // ARRANGE + $calculator = new PriceCalculator(taxRate: 0.1); + $items = [['price' => 100], ['price' => 50]]; + + // ACT + $total = $calculator->calculateTotal($items); + + // ASSERT + expect($total)->toBe(165.0); +}); +``` + +**Exception tests:** Use `// ACT & ASSERT` when the act triggers the assertion: + +```php +it('throws on negative price', function () { + $calculator = new PriceCalculator(); + + // ACT & ASSERT + expect(fn () => $calculator->calculateTotal([['price' => -10]])) + ->toThrow(InvalidArgumentException::class); +}); +``` + +## Test Naming + +Use descriptive `it()` statements that read as sentences: + +```php +// CORRECT +it('returns empty array when no servers configured') +it('throws when SSH connection fails') +it('creates deploy key with correct permissions') + +// WRONG +it('test1') +it('works') +it('should return correct value') +``` + +## Dependency Injection in Tests + +DI Container rules apply to PRODUCTION code, not tests. + +**Unit Tests - Manual Instantiation:** + +```php +it('parses config correctly', function () { + $mockFs = mock(FilesystemInterface::class); + $mockFs->shouldReceive('read')->with('/path')->andReturn('content'); + + $service = new ConfigService($mockFs); + $result = $service->parse('/path'); + + expect($result)->toBe(['key' => 'value']); +}); +``` + +**Command Tests - Container with Bindings:** + +```php +it('adds server successfully', function () { + $mockSSH = mock(SSHService::class); + $mockSSH->shouldReceive('connect')->andReturn(true); + + $container = mockCommandContainer(ssh: $mockSSH); + $command = $container->build(ServerAddCommand::class); + + // Test command execution +}); +``` + +## Test Minimalism + +**Target:** Keep test files under 1.8x source code size. + +**Rules:** + +1. Test core business logic only +2. Use datasets for multiple scenarios: `->with([])` +3. Eliminate overlap: no two tests covering same functionality +4. Consolidate assertions: `expect($x)->toBe(1)->and($y)->toBe(2)` +5. Mock external dependencies only + +**Don't consolidate:** Different public methods, exception vs normal flow, distinct business logic. + +## Datasets for Multiple Scenarios + +```php +it('validates server names', function (string $name, bool $valid) { + $validator = new ServerValidator(); + + expect($validator->isValidName($name))->toBe($valid); +})->with([ + 'valid simple' => ['web-server', true], + 'valid with numbers' => ['web1', true], + 'invalid spaces' => ['web server', false], + 'invalid special chars' => ['web@server', false], + 'empty string' => ['', false], +]); +``` + +## Assertion Chaining + +```php +// CORRECT - Chained assertions +expect($server) + ->name->toBe('web1') + ->and($server) + ->host->toBe('192.168.1.1') + ->and($server) + ->port->toBe(22); + +// WRONG - Separate expect calls for related assertions +expect($server->name)->toBe('web1'); +expect($server->host)->toBe('192.168.1.1'); +expect($server->port)->toBe(22); +``` + +## Forbidden Patterns + +Never write these assertions: + +```php +// Type-only checks (prove nothing about behavior) +expect($x)->toBeInstanceOf(Class::class); +expect($x)->toBeArray(); +expect($x)->not->toBeNull(); + +// Literally meaningless +expect(true)->toBeTrue(); + +// Time-dependent (test logic, not time) +sleep(1); +usleep(1000); +``` + +## Required Patterns + +Always verify actual values and mock interactions: + +```php +// CORRECT - Verify actual value +expect($config->getValue('host'))->toBe('example.com'); + +// CORRECT - Verify mock interaction +$mock->shouldReceive('method')->with('param')->andReturn('result'); + +// For polling/timeout operations - use zero intervals +$service->waitForReady('id', timeout: 10, pollInterval: 0); +``` + +## Mocking with Mockery + +```php +use Mockery; + +it('calls external service', function () { + $mock = mock(ExternalService::class); + $mock->shouldReceive('fetch') + ->once() + ->with('param') + ->andReturn(['data']); + + $service = new MyService($mock); + $result = $service->process(); + + expect($result)->toBe('processed'); +}); +``` + +**Mock patterns:** + +```php +// Return value +$mock->shouldReceive('method')->andReturn('value'); + +// Multiple calls with different returns +$mock->shouldReceive('method') + ->andReturn('first', 'second', 'third'); + +// Throw exception +$mock->shouldReceive('method') + ->andThrow(new RuntimeException('error')); + +// Verify call count +$mock->shouldReceive('method')->once(); +$mock->shouldReceive('method')->twice(); +$mock->shouldReceive('method')->times(3); +$mock->shouldReceive('method')->never(); + +// Argument matching +$mock->shouldReceive('method')->with('exact'); +$mock->shouldReceive('method')->with(Mockery::any()); +$mock->shouldReceive('method')->with(Mockery::type('string')); +``` + +## Test Types + +**Unit Tests:** +- Mock all external dependencies +- Test single units in isolation +- Complete in milliseconds + +**Integration Tests:** +- Real file operations and external processes +- CLI commands and full workflows + +**Layer Strategy:** + +| Layer | Test Type | +|-------|-----------| +| CLI Commands | Integration tests | +| Business Services | Unit tests | +| Utilities/Helpers | Unit tests | + +## Architecture Tests + +Use Pest's arch testing for structural rules: + +```php +arch('commands extend BaseCommand', function () { + expect('Deployer\\Console\\') + ->classes() + ->toHaveSuffix('Command') + ->toExtend(BaseCommand::class); +}); + +arch('services are final', function () { + expect('Deployer\\Service\\') + ->classes() + ->toBeFinal(); +}); +``` + +## Static Analysis + +PHPStan applies to PRODUCTION code, not tests. Focus on functionality over type compliance. + +## Test Organization + +### Section Comments + +```php +// +// Server validation tests +// ---- + +it('validates server name format', ...); +it('validates server host', ...); + +// +// Server creation tests +// ---- + +it('creates server with defaults', ...); +``` + +### File Naming + +- Test files: `tests/Unit/ServiceNameTest.php` or `tests/Integration/FeatureTest.php` +- Mirror source structure where practical + +## Quality Gates + +After writing tests, run: + +```bash +vendor/bin/pest $TEST_FILE # Run the specific test +composer pest # Run full suite +``` + +## Checklist + +Before completing tests: + +- [ ] Every test follows AAA pattern with comments +- [ ] Test names are descriptive sentences +- [ ] No forbidden assertion patterns +- [ ] Datasets used for multiple similar scenarios +- [ ] Assertions verify actual values, not just types +- [ ] Mocks verify interactions where relevant +- [ ] No test overlap (each behavior tested once) +- [ ] Tests complete in milliseconds (unit) or seconds (integration) From a93c8e45c13b96b95f8aade495a3f7c9cc1a9653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 19:57:41 +0200 Subject: [PATCH 10/18] docs: remove testing.md in favor of testing skill --- CLAUDE.md | 2 - docs/rules/testing.md | 114 ------------------------------------------ 2 files changed, 116 deletions(-) delete mode 100644 docs/rules/testing.md diff --git a/CLAUDE.md b/CLAUDE.md index 236bb36a..5b069764 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,9 +64,7 @@ Don't run, create, or update tests UNLESS explicitly instructed. | Working On | Reference | |------------|-----------| | PHP architecture, DI, layers | @docs/rules/architecture.md | -| Symfony Console commands | @docs/rules/commands.md | | Exception handling | @docs/rules/exceptions.md | -| Pest testing | @docs/rules/testing.md | | Writing documentation | @docs/rules/writing-docs.md | ## References diff --git a/docs/rules/testing.md b/docs/rules/testing.md deleted file mode 100644 index 4330a16a..00000000 --- a/docs/rules/testing.md +++ /dev/null @@ -1,114 +0,0 @@ -# Testing Rules - -All rules MANDATORY. - -**Philosophy:** "A test that never fails is not a test, it's a lie." - -**Framework:** Pest exclusively, `it()` syntax, 70%+ coverage - -## Running Tests - -```bash -composer pest # Full suite with coverage (parallel) -vendor/bin/pest $TEST_FILE # Specific file -``` - -## Dependency Injection in Tests - -DI Container rule applies to PRODUCTION code, not tests. - -**Unit Tests - Manual Instantiation:** - -```php -$mockFs = mockFilesystem(true, 'content'); -$service = new EnvService(new FilesystemService($mockFs), new Dotenv()); -``` - -**Command Tests - Container with Bindings:** - -```php -$container = mockCommandContainer(); -$command = $container->build(ServerAddCommand::class); - -// Override services -$container = mockCommandContainer(ssh: $mockSSH); - -// Pre-populate data -$container = mockCommandContainer( - inventoryData: ['servers' => [['name' => 'web1', 'host' => '192.168.1.1']]] -); -``` - -**Maintenance:** When adding service to BaseCommand, update `mockCommandContainer()` in `tests/TestHelpers.php`. - -## Test Minimalism - -**Target:** Keep test files under 1.8x source code size. - -**Rules:** -- Test core business logic only -- Use datasets: `->with([])` for multiple scenarios -- Eliminate overlap: no two tests covering same functionality -- Consolidate assertions: `expect($x)->toBe(1)->and($y)->toBe(2)` -- Mock external dependencies only - -**Don't consolidate:** Different public methods, exception vs normal flow, distinct business logic. - -## AAA Pattern - -```php -it('does something', function () { - // ARRANGE - $service = new Service(mock(Dependency::class)); - - // ACT - $result = $service->action(); - - // ASSERT - expect($result)->toBe('expected'); -}); -``` - -Exception tests: `// ACT & ASSERT` when act triggers assertion. - -## Testing Patterns - -**Forbidden:** - -```php -expect($x)->toBeInstanceOf(Class::class); // Type-only -expect($x)->toBeArray(); // Generic -expect($x)->not->toBeNull(); // Meaningless alone -expect(true)->toBeTrue(); // Literally meaningless -sleep(...); // Test logic not time -``` - -**Required:** - -```php -expect($config->getValue('host'))->toBe('example.com'); -$mock->shouldReceive('method')->with('param')->andReturn('result'); - -// For polling/timeout - use zero intervals -$service->waitForReady('id', timeout: 10, pollInterval: 0); -``` - -## Test Types - -**Unit Tests:** -- Mock all external dependencies -- Test single units in isolation -- Complete in milliseconds - -**Integration Tests:** -- Real file operations and external processes -- CLI commands and full workflows - -**Layer Strategy:** -- CLI Commands → Integration tests -- Business Services → Unit tests -- Utilities/Helpers → Unit tests - -## Static Analysis - -PHPStan applies to PRODUCTION code, not tests. Focus on functionality over type compliance. From 043f0df3b7bfd08f4e6d364805f072a6fe4d2d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 23:41:00 +0200 Subject: [PATCH 11/18] docs: merge architecture and exception rules into CLAUDE.md Consolidate all project rules into the primary CLAUDE.md file for token efficiency. Removed separate architecture.md and exceptions.md rule files. All documentation now lives in a single ~200-line file optimized for AI agent context windows. --- .claude/agents/quality-gatekeeper.md | 121 ++++++++++++ .claude/settings.local.json | 3 +- .../skills/ai-docs/SKILL.md | 50 +++-- .claude/skills/command/SKILL.md | 10 - .claude/skills/playbook/SKILL.md | 8 - .claude/skills/testing/SKILL.md | 9 - CLAUDE.md | 187 ++++++++++++++---- docs/rules/architecture.md | 104 ---------- docs/rules/exceptions.md | 123 ------------ 9 files changed, 313 insertions(+), 302 deletions(-) create mode 100644 .claude/agents/quality-gatekeeper.md rename docs/rules/writing-docs.md => .claude/skills/ai-docs/SKILL.md (62%) delete mode 100644 docs/rules/architecture.md delete mode 100644 docs/rules/exceptions.md diff --git a/.claude/agents/quality-gatekeeper.md b/.claude/agents/quality-gatekeeper.md new file mode 100644 index 00000000..f9286ab0 --- /dev/null +++ b/.claude/agents/quality-gatekeeper.md @@ -0,0 +1,121 @@ +--- +name: quality-gatekeeper +description: Use this agent when you need to run quality gate checks on PHP files or bash playbooks. This includes running Rector, Pint, and PHPStan on changed PHP files, or formatting/checking playbook shell scripts. Call this agent after making changes to PHP files or playbook scripts to ensure code quality standards are met before committing or completing a task.\n\nExamples:\n\n\nContext: The user has just finished implementing a new feature in PHP files.\nuser: "Add a new method to the ServerService class that validates server connections"\nassistant: "I've added the validateConnection method to ServerService.php. Now let me use the php-quality-gate agent to run quality checks on the changed files."\n\n\n\n\nContext: The user has modified a playbook bash script.\nuser: "Update the deploy.sh playbook to include a backup step"\nassistant: "I've updated the deploy.sh playbook with the backup step. Let me run the php-quality-gate agent to format and validate the bash script."\n\n\n\n\nContext: The assistant proactively runs quality gates after completing PHP changes.\nassistant: "I've finished refactoring the Repository classes. Before we proceed, I'll use the php-quality-gate agent to ensure all quality checks pass."\n\n +model: haiku +color: cyan +--- + +You are an expert quality assurance engineer specializing in automated code quality enforcement. Your sole responsibility is to run quality gate commands on changed files and report results clearly and actionably. + +## Your Mission + +Execute quality gate commands on PHP files and playbook scripts, then report any issues, errors, or violations encountered. You are the final checkpoint before code is considered complete. + +## Commands You Execute + +### For PHP Files + +Run these commands in sequence on changed PHP files: + +1. **Rector** (automated refactoring): + + ```bash + vendor/bin/rector $CHANGED_PHP_FILES + ``` + +2. **Pint** (code style formatting): + + ```bash + vendor/bin/pint $CHANGED_PHP_FILES + ``` + +3. **PHPStan** (static analysis): + ```bash + vendor/bin/phpstan analyse $CHANGED_PHP_FILES + ``` + +### For Playbook Scripts + +When playbooks (\*.sh files in playbooks/) are involved: + +1. **Format playbooks**: + + ```bash + composer bash + ``` + +2. **Or check only** (if requested): + ```bash + composer bash:check + ``` + +## Critical Rules + +1. **NEVER run PHPStan on test files** - If a file path contains `tests/` or is a test file, exclude it from PHPStan analysis. Rector and Pint may still run on tests. + +2. **Identify changed files first** - Before running commands, determine which PHP files have been changed. Use git status, git diff, or context from the conversation to identify the relevant files. + +3. **Run commands sequentially** - Execute each command one at a time and capture all output. + +4. **Report everything** - Include both successes and failures in your report. + +## Workflow + +1. **Identify scope**: Determine which files need checking (PHP files, playbooks, or both) +2. **Filter appropriately**: Exclude test files from PHPStan, include them for Rector/Pint if changed +3. **Execute commands**: Run each applicable command +4. **Capture output**: Record all command output, exit codes, and any errors +5. **Report results**: Provide a clear summary + +## Reporting Format + +Structure your report as follows: + +``` +## Quality Gate Results + +### Files Checked +- [list of files] + +### Rector +✅ Passed (no changes needed) +— or — +⚠️ Applied fixes: + - [describe changes made] + +### Pint +✅ Passed (code style OK) +— or — +⚠️ Fixed formatting in: + - [list of files] + +### PHPStan +✅ Passed (0 errors) +— or — +❌ Found [N] errors: + - [file:line] [error message] + - ... + +### Playbooks (if applicable) +✅ Bash formatting OK +— or — +⚠️ Formatted playbook scripts + +### Summary +[Overall status: All checks passed / Issues found that need attention] +``` + +## Error Handling + +- If a command fails to execute (not found, permission denied), report the technical error +- If a command finds issues, report them as quality violations, not errors +- If you cannot determine which files changed, ask for clarification +- If no PHP files or playbooks were changed, report that no checks were needed + +## Behavior Guidelines + +- Be concise but complete in your reporting +- Highlight blocking issues (PHPStan errors) prominently +- Note when tools auto-fixed issues (Rector, Pint) vs. when manual intervention is needed +- If PHPStan errors exist, the quality gate has FAILED and this must be clearly communicated +- Do not attempt to fix issues yourself - only report them diff --git a/.claude/settings.local.json b/.claude/settings.local.json index c5c73022..0c7ff356 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -6,7 +6,8 @@ "Bash(wc:*)", "Bash(cat:*)", "Bash(gh pr view:*)", - "WebFetch(domain:github.com)" + "WebFetch(domain:github.com)", + "Skill(ai-docs)" ], "deny": [], "ask": [] diff --git a/docs/rules/writing-docs.md b/.claude/skills/ai-docs/SKILL.md similarity index 62% rename from docs/rules/writing-docs.md rename to .claude/skills/ai-docs/SKILL.md index 6c00f720..aa38e33b 100644 --- a/docs/rules/writing-docs.md +++ b/.claude/skills/ai-docs/SKILL.md @@ -1,6 +1,13 @@ -# Rules for Writing Docs +--- +name: docs +description: Use this skill when writing, creating, or modifying documentation or prompts for AI agents. Activates for tasks involving CLAUDE.md, the .claude folder, or any documentation optimized for token efficiency. +--- -Guidelines for documentation optimized for AI agents with limited token windows. +# AI-Optimized Documentation + +Guidelines for writing documentation optimized for AI agents with limited token windows. + +All rules MANDATORY. ## Token Efficiency @@ -11,7 +18,7 @@ Guidelines for documentation optimized for AI agents with limited token windows. - Bullets over paragraphs - Single "All rules MANDATORY" per file - No repetitive CRITICAL/IMMUTABLE warnings -- No emoji in headers +- No emoji ## Structure @@ -24,6 +31,7 @@ All rules MANDATORY. ``` **Organization:** + - Clear, scannable headers - Related rules grouped - Alphabetical when no logical grouping @@ -42,6 +50,7 @@ $result = new Service(new Dependency()); ``` **Rules:** + - Under 10 lines per example - Use `// CORRECT` and `// WRONG` markers - Inline comments over prose @@ -51,14 +60,15 @@ $result = new Service(new Dependency()); ## Cross-File Coordination **Single source of truth:** + - One primary location per concept -- Cross-reference by filename: "See commands.md" +- Cross-reference by filename: "See architecture.md" - No line number references **Valid references:** ```markdown -See @docs/rules/commands.md +See @docs/architecture.md Covered in architecture.md ``` @@ -77,6 +87,7 @@ Commands are responsible for handling all user interaction including input and o ``` **Emphasis Hierarchy:** + 1. Code examples (most efficient) 2. Imperative bullets 3. Short declarative sentences @@ -90,11 +101,26 @@ Commands are responsible for handling all user interaction including input and o - Leave 80% for code, history, responses - Target: <3000 tokens total (~600-800 lines) -## Maintenance Checklist +## Quality Gates + +After writing documentation: + +```bash +# Check file size (target: <800 lines) +wc -l docs/rules/new-doc.md + +# Estimate tokens (~4 chars per token) +wc -c docs/rules/new-doc.md | awk '{print int($1/4)}' +``` + +## Checklist + +Before committing documentation: -Before committing: -1. Remove outdated file references -2. Check for duplication -3. Verify no contradictions -4. Test code examples -5. Compare token count (target: 35-65% of verbose version) +- [ ] Remove outdated file references +- [ ] Check for duplication across files +- [ ] Verify no contradictions with other docs +- [ ] Make sure code examples compile/run +- [ ] Validate referenced files or URLs exist +- [ ] Token count under budget (<3000) +- [ ] Uses CORRECT/WRONG markers in examples diff --git a/.claude/skills/command/SKILL.md b/.claude/skills/command/SKILL.md index c1fda3c7..ef9f4f89 100644 --- a/.claude/skills/command/SKILL.md +++ b/.claude/skills/command/SKILL.md @@ -318,16 +318,6 @@ return Command::SUCCESS; This outputs the equivalent non-interactive command for documentation/automation. -## Quality Gates - -After writing command, run: - -```bash -vendor/bin/rector src/Command/NewCommand.php -vendor/bin/pint src/Command/NewCommand.php -vendor/bin/phpstan analyse src/Command/NewCommand.php -``` - ## Checklist Before completing a command: diff --git a/.claude/skills/playbook/SKILL.md b/.claude/skills/playbook/SKILL.md index 786dece7..3940015a 100644 --- a/.claude/skills/playbook/SKILL.md +++ b/.claude/skills/playbook/SKILL.md @@ -366,11 +366,3 @@ EOF exit 1 fi ``` - -## Quality Gates - -After writing playbook, run: -```bash -composer bash playbooks/new-playbook.sh # Format -composer bash:check # Verify -``` diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index d07f7e82..70dab8e5 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -322,15 +322,6 @@ it('creates server with defaults', ...); - Test files: `tests/Unit/ServiceNameTest.php` or `tests/Integration/FeatureTest.php` - Mirror source structure where practical -## Quality Gates - -After writing tests, run: - -```bash -vendor/bin/pest $TEST_FILE # Run the specific test -composer pest # Run full suite -``` - ## Checklist Before completing tests: diff --git a/CLAUDE.md b/CLAUDE.md index 5b069764..8fe4f2e0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,25 +2,9 @@ Server and site deployment tool for PHP. Composer package and CLI built on Symfony Console. -## Commands +## Quality Gates -```bash -# Quality gates (run before completing tasks) -vendor/bin/rector $CHANGED_PHP_FILES -vendor/bin/pint $CHANGED_PHP_FILES -vendor/bin/phpstan analyse $CHANGED_PHP_FILES # NEVER run on tests/ - -# All quality checks -composer pall - -# Testing -composer pest # Full suite with coverage -vendor/bin/pest $TEST_FILE # Specific file - -# Bash formatting -composer bash # Format playbooks/*.sh -composer bash:check # Check only -``` +Before completing a task or committing, use the `quality-gatekeeper` agent to run quality checks on changed files. ## Code Philosophy @@ -28,16 +12,157 @@ composer bash:check # Check only - **Organization:** Group related functions into comment-separated sections. Order alphabetically after grouping. - **Consistency:** Same style throughout. Code should appear written by single person. -## Architecture Summary +## PHP Standards + +- PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly) +- Explicit return types with generics: `Collection` +- Dependency injection via Symfony patterns +- Use Symfony classes over native PHP functions (Filesystem, Process) for testability +- **Yoda conditions:** Constants/literals on LEFT side of comparisons +- **Always use braces:** ALL control structures must use `{ }` + +```php +// Yoda conditions +if (null === $value) { ... } +if ('' === trim($name)) { ... } + +// Two variables - Yoda doesn't apply +if ($typedName !== $server->name) { ... } +``` + +**PHPStan:** Use `@var` annotations, not `assert()` in production: + +```php +/** @var string $apiToken */ +$apiToken = $this->env->get(['API_TOKEN']); +``` + +**Imports:** Always add `use` statements for vendor packages. Root namespace FQDNs acceptable (`\RuntimeException`). + +## Dependency Injection + +Use `$container->build(ClassName::class)` for all object creation except DTOs/value objects. + +```php +// Production +$service = $this->container->build(MyService::class); + +// Tests - container supports bind() for mocks +$container = new Container(); +$container->bind(SSHService::class, $mockSSH); +$command = $container->build(ServerAddCommand::class); +``` + +**Integration:** Entry point: `bin/deployer`. Command registration: `SymfonyApp.php`. Services: Auto-injected via constructor. + +## Layer Separation + +**Command Layer:** + +- Handle user interaction (input/output), orchestrate Services +- NO business logic (delegate to Services) +- Responsible for console styling, error formatting, prompts +- Never invoke other commands +- Use BaseCommand methods, never SymfonyStyle directly + +**Service Layer:** + +- Atomic, reusable functionality with NO console I/O +- Accept/return plain PHP data types +- Handle business logic, external APIs, file operations + +**Service State:** -| Concept | Rule | -|---------|------| -| DI | `$container->build(Class::class)` for all objects except DTOs | -| Layers | Commands (I/O) → Services (logic) → Repositories (data) | -| Exceptions | Services throw complete messages, Commands display directly | -| PHP | PSR-12, strict types, Yoda conditions (`null === $value`), always braces | -| Console | Use BaseCommand methods, never SymfonyStyle directly | -| Playbooks | Idempotent bash scripts, YAML output to `$DEPLOYER_OUTPUT_FILE` | +- Stateless: Pure operations (validators, calculators, API clients) +- Stateful: Manage configuration/cached data, use lazy loading and explicit `load()`/`initialize()` methods + +**Dependencies:** + +- Commands depend on Services +- Services depend on Services/utilities +- All dependencies in constructor signatures +- NO circular dependencies + +## Exception Handling + +Services throw complete, user-facing exceptions. Commands display directly without adding prefixes. + +```php +// Service - complete message with context +throw new \RuntimeException("SSH key does not exist: {$privateKeyPath}"); +throw new \RuntimeException("Server '{$name}' already exists"); + +// Preserve exception chain when wrapping +} catch (\Throwable $e) { + throw new \RuntimeException( + "SSH authentication failed for {$username}@{$host}. Check username and key permissions", + previous: $e + ); +} + +// Command - display directly, no prefix +try { + $this->servers->create($server); +} catch (\RuntimeException $e) { + $this->nay($e->getMessage()); // Already complete + return Command::FAILURE; +} + +// WRONG - redundant prefix +$this->nay('Failed to add server: ' . $e->getMessage()); +``` + +**Validation Patterns:** + +```php +// Pattern A: Input validation - returns ?string +protected function validateNameInput(mixed $name): ?string +{ + if ('' === trim($name)) { + return 'Server name cannot be empty'; + } + return null; +} + +// Pattern B: Heavy I/O validation - throws +protected function validateGitRepo(string $repo): void +{ + if (!$process->isSuccessful()) { + throw new \RuntimeException("Cannot access git repository '{$repo}'"); + } +} +``` + +**Silent Failures:** Return `null`/`false` only for optional operations (detection, optional lookups). + +| Layer | Display Errors? | Pattern | +| ----------------------------- | --------------- | ------------------------------ | +| Services/Repositories | No | Throw complete exceptions | +| Validation Traits | No | Return `?string` or throw | +| Commands/Orchestration Traits | Yes | Catch & display without prefix | + +## Comments + +**DocBlock:** Minimalist descriptions, parameters, return types. + +**Comment structure:** + +``` +// ---- +// {h1} +// ---- + +// +// {h2} +// ---- + +// +// {h3} + +// {p} +``` + +Separate sections visually. No obvious comments. Remove comments when removing code. ## File Operations @@ -59,14 +184,6 @@ mkdir -p path/to/dir # Create directories Don't run, create, or update tests UNLESS explicitly instructed. -## Detailed Rules by Domain - -| Working On | Reference | -|------------|-----------| -| PHP architecture, DI, layers | @docs/rules/architecture.md | -| Exception handling | @docs/rules/exceptions.md | -| Writing documentation | @docs/rules/writing-docs.md | - ## References - Check `composer.json` and `package.json` for installed packages diff --git a/docs/rules/architecture.md b/docs/rules/architecture.md deleted file mode 100644 index efffa249..00000000 --- a/docs/rules/architecture.md +++ /dev/null @@ -1,104 +0,0 @@ -# Architecture Rules - -All rules MANDATORY. - -## PHP Standards - -- PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly) -- Explicit return types with generics: `Collection` -- Dependency injection via Symfony patterns -- Use Symfony classes over native PHP functions (Filesystem, Process) for testability -- **Yoda conditions:** Constants/literals on LEFT side of comparisons -- **Always use braces:** ALL control structures must use `{ }` - -```php -// Yoda conditions -if (null === $value) { ... } -if ('' === trim($name)) { ... } -if (0 !== $exitCode) { ... } - -// Two variables - Yoda doesn't apply -if ($typedName !== $server->name) { ... } -``` - -## PHPStan Type Hints - -Use `@var` annotations, not `assert()` in production: - -```php -/** @var string $apiToken */ -$apiToken = $this->env->get(['API_TOKEN']); -``` - -## Imports - -Always add `use` statements for vendor packages. Root namespace FQDNs acceptable (`\RuntimeException`). - -```php -use Symfony\Component\Filesystem\Filesystem; - -$fs = new Filesystem(); -throw new \InvalidArgumentException('Error'); -``` - -## Dependency Injection - -Use `$container->build(ClassName::class)` for all object creation except DTOs/value objects. - -```php -// Production -$service = $this->container->build(MyService::class); - -// Tests - container supports bind() for mocks -$container = new Container(); -$container->bind(SSHService::class, $mockSSH); -$command = $container->build(ServerAddCommand::class); -``` - -**Integration:** Entry point: `bin/deployer`. Command registration: `SymfonyApp.php`. Services: Auto-injected via constructor. - -## Layer Separation - -**Command Layer:** -- Handle user interaction (input/output), orchestrate Services -- NO business logic (delegate to Services) -- Responsible for console styling, error formatting, prompts -- Never invoke other commands - -**Service Layer:** -- Atomic, reusable functionality with NO console I/O -- Accept/return plain PHP data types -- Handle business logic, external APIs, file operations - -**Service State:** -- Stateless: Pure operations (validators, calculators, API clients) -- Stateful: Manage configuration/cached data, use lazy loading and explicit `load()`/`initialize()` methods - -**Dependencies:** -- Commands depend on Services -- Services depend on Services/utilities -- All dependencies in constructor signatures -- NO circular dependencies - -## Comments - -**DocBlock:** Minimalist descriptions, parameters, return types. - -**Comment structure:** - -``` -// ---- -// {h1} -// ---- - -// -// {h2} -// ---- - -// -// {h3} - -// {p} -``` - -Separate sections visually. No obvious comments. Remove comments when removing code. diff --git a/docs/rules/exceptions.md b/docs/rules/exceptions.md deleted file mode 100644 index c002c10b..00000000 --- a/docs/rules/exceptions.md +++ /dev/null @@ -1,123 +0,0 @@ -# Exception Handling & Error Display - -All rules MANDATORY. - -## Core Principle - -Services throw complete, user-facing exceptions. Command layer displays them directly without adding prefixes. - -## Services & Repositories - -Throw `\RuntimeException` with complete, actionable messages: - -```php -// Complete message with context -throw new \RuntimeException("SSH key does not exist: {$privateKeyPath}"); -throw new \RuntimeException("Server '{$name}' already exists"); - -// Preserve exception chain when wrapping -} catch (\Throwable $e) { - throw new \RuntimeException( - "SSH authentication failed for {$username}@{$host}. Check username and key permissions", - previous: $e - ); -} -``` - -**Rules:** -- Messages must be user-facing and complete (not fragments) -- Include relevant context (paths, names, IDs, hosts) -- Use `previous: $e` to preserve exception chains -- Never catch and re-throw with generic prefixes like "Failed to..." - -## Validation Traits - -**Pattern A: Input Validation** - Returns `?string`: - -```php -protected function validateNameInput(mixed $name): ?string -{ - if (!is_string($name)) { - return 'Server name must be a string'; - } - if ('' === trim($name)) { - return 'Server name cannot be empty'; - } - return null; -} -``` - -**Pattern B: Heavy I/O Validation** - Throws: - -```php -protected function validateGitRepo(string $repo): void -{ - if (!$process->isSuccessful()) { - throw new \RuntimeException("Cannot access git repository '{$repo}'"); - } -} -``` - -## Commands & Orchestration Traits - -Display exceptions directly without redundant prefixes: - -```php -// CORRECT -try { - $this->servers->create($server); -} catch (\RuntimeException $e) { - $this->nay($e->getMessage()); // Already complete - return Command::FAILURE; -} - -// WRONG - redundant prefix -$this->nay('Failed to add server: ' . $e->getMessage()); -``` - -**When to add context:** -- Displaying raw output for debugging -- Adding actionable troubleshooting steps -- Exception message is too technical - -## Silent Failures - -Return `null`/`false` only for optional operations: - -```php -// CORRECT - Optional detection -public function detectRemoteUrl(): ?string -{ - try { - return $process->isSuccessful() ? trim($process->getOutput()) : null; - } catch (\Exception) { - return null; // Not in git repo, that's okay - } -} - -// WRONG - Required operation returning null -public function executeCommand(): ?array -{ - } catch (\Throwable) { - return null; // Caller doesn't know WHY - } -} -``` - -## Exception Message Quality - -Every message must be: -- Complete: "SSH key does not exist: /path/to/key" -- User-facing: "Cannot connect to database. Check host and port." -- Actionable with context -- Free of redundant prefixes - -## Layer Responsibility - -| Layer | Display Errors? | Pattern | -|-------|-----------------|---------| -| Services | No | Throw complete exceptions | -| Repositories | No | Throw complete exceptions | -| Validation Traits | No | Return `?string` or throw | -| Orchestration Traits | Yes | Catch & display without prefix | -| Commands | Yes | Catch & display without prefix | From dfe8db449579d0e65063147e9c672b3b293e2fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 23:42:46 +0200 Subject: [PATCH 12/18] chore: remove .cursor directory Cursor rules and commands are now superseded by Claude Code configuration in .claude/ directory. --- .claude/settings.local.json | 4 +- .cursor/commands/create-branch.md | 12 - .cursor/commands/create-commits.md | 13 - .cursor/commands/deslop.md | 12 - .cursor/commands/improve-tests.md | 3 - .cursor/commands/review-branch.md | 5 - .cursor/commands/review-diff.md | 5 - .cursor/commands/review-pr-comment.md | 1 - .cursor/rules/00-main.mdc | 64 ---- .cursor/rules/01-architecture.mdc | 188 --------- .cursor/rules/02-tests.mdc | 159 -------- .cursor/rules/03-commands.mdc | 525 -------------------------- .cursor/rules/04-exceptions.mdc | 220 ----------- .cursor/rules/05-bash.mdc | 128 ------- .cursor/rules/06-playbooks.mdc | 361 ------------------ .cursor/rules/rules.mdc | 154 -------- 16 files changed, 3 insertions(+), 1851 deletions(-) delete mode 100644 .cursor/commands/create-branch.md delete mode 100644 .cursor/commands/create-commits.md delete mode 100644 .cursor/commands/deslop.md delete mode 100644 .cursor/commands/improve-tests.md delete mode 100644 .cursor/commands/review-branch.md delete mode 100644 .cursor/commands/review-diff.md delete mode 100644 .cursor/commands/review-pr-comment.md delete mode 100644 .cursor/rules/00-main.mdc delete mode 100644 .cursor/rules/01-architecture.mdc delete mode 100644 .cursor/rules/02-tests.mdc delete mode 100644 .cursor/rules/03-commands.mdc delete mode 100644 .cursor/rules/04-exceptions.mdc delete mode 100644 .cursor/rules/05-bash.mdc delete mode 100644 .cursor/rules/06-playbooks.mdc delete mode 100644 .cursor/rules/rules.mdc diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 0c7ff356..33457c02 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -7,7 +7,9 @@ "Bash(cat:*)", "Bash(gh pr view:*)", "WebFetch(domain:github.com)", - "Skill(ai-docs)" + "Skill(ai-docs)", + "Bash(git add:*)", + "Bash(git commit:*)" ], "deny": [], "ask": [] diff --git a/.cursor/commands/create-branch.md b/.cursor/commands/create-branch.md deleted file mode 100644 index 813d3c49..00000000 --- a/.cursor/commands/create-branch.md +++ /dev/null @@ -1,12 +0,0 @@ -Based on the changes made to this repository create a new branch with a suitable name. - -Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: -feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. - -Keep the branch name short (≤ 50 chars) yet informative. Do not push, pull, or rebase. - -Examples: - -- feat/parser-add-php-84-attributes -- fix/ci-matrix-php-versions -- chore/deps-bump-composer-installers-2-3 diff --git a/.cursor/commands/create-commits.md b/.cursor/commands/create-commits.md deleted file mode 100644 index 68eb700c..00000000 --- a/.cursor/commands/create-commits.md +++ /dev/null @@ -1,13 +0,0 @@ -Based on the changes made to this repository create one or more commits with suitable titles. - -Use Conventional Commits to group related changes into cohesive commits (commits should be independently meaningful). - -Keep titles short (≤ 72 chars), imperative, no trailing period. Do not push, pull, or rebase. - -Examples: - -- feat(parser): add support for PHP 8.4 attributes -- fix(ci): correct matrix PHP versions in build workflow -- chore(deps): bump composer/installers to ^2.3 - -Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). diff --git a/.cursor/commands/deslop.md b/.cursor/commands/deslop.md deleted file mode 100644 index cd86aaa9..00000000 --- a/.cursor/commands/deslop.md +++ /dev/null @@ -1,12 +0,0 @@ -# Remove AI Code Slop - -Check the diff against main, and remove all AI-generated slop introduced in this branch. - -This includes: - -- Extra comments that a human wouldn't add or are inconsistent with the rest of the file -- Extra defensive checks or try/catch blocks that are abnormal for that area of the codebase (especially if called by trusted / validated codepaths) -- Casts to any to get around type issues -- Any other violation inconsistent with the rest of the file or our rules - -Report at the end with only a 1-3 sentence summary of what you changed diff --git a/.cursor/commands/improve-tests.md b/.cursor/commands/improve-tests.md deleted file mode 100644 index 6c15dbf4..00000000 --- a/.cursor/commands/improve-tests.md +++ /dev/null @@ -1,3 +0,0 @@ -Is there any overlap in these tests, or are any tests engaging in testing theater? - -Implement improvements if they are. diff --git a/.cursor/commands/review-branch.md b/.cursor/commands/review-branch.md deleted file mode 100644 index 7de8fc93..00000000 --- a/.cursor/commands/review-branch.md +++ /dev/null @@ -1,5 +0,0 @@ -Meticulously catalog and analyze all the changes in this branch, compared to the branch it's based on. - -Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules as well as finding potential bugs and regressions - -Provide a detailed report but don't make any changes yet. diff --git a/.cursor/commands/review-diff.md b/.cursor/commands/review-diff.md deleted file mode 100644 index d4cc786f..00000000 --- a/.cursor/commands/review-diff.md +++ /dev/null @@ -1,5 +0,0 @@ -Meticulously catalog and analyze all the changes in this Git working tree, staged or unstaged. - -Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. - -Provide a detailed report but don't make any changes yet. diff --git a/.cursor/commands/review-pr-comment.md b/.cursor/commands/review-pr-comment.md deleted file mode 100644 index 02630feb..00000000 --- a/.cursor/commands/review-pr-comment.md +++ /dev/null @@ -1 +0,0 @@ -Please assess whether the concerns raised in the following PR comment are valid, and propose possible solutions to address them. diff --git a/.cursor/rules/00-main.mdc b/.cursor/rules/00-main.mdc deleted file mode 100644 index dcc146d4..00000000 --- a/.cursor/rules/00-main.mdc +++ /dev/null @@ -1,64 +0,0 @@ ---- -alwaysApply: true ---- - -## Development Rules - -All rules MANDATORY. - -### Mission - -Build Deployer PHP: Composer package and CLI tool simplifying server provisioning and deployment across multiple Cloud providers. - -### References - -- Check `composer.json` and `package.json` for installed packages -- Plan with features from installed major versions -- Use Context7 MCP - -### Code Philosophy - -**Minimalism:** - -- Write minimum code necessary - no more, no less -- Always ask: can less code achieve same result? -- Refactor relentlessly for clarity and necessity -- Eliminate single-use methods: inline if called once -- Cache computed values: initialize expensive calculations in constructor -- Avoid method call overhead: direct property access when appropriate - -**Organization:** - -- Catalog like a librarian -- Group related functions into comment-separated sections -- Order alphabetically after grouping logically -- Code should be both functional and visually appealing - -**Consistency:** - -- Maintain rigorous consistency across codebase -- Same style, standards, aesthetic principles throughout -- Review surrounding code for reusable patterns -- Code should appear written by single person: naming, parameter precedence, logic flow, organization - -### File Operations - -Use terminal commands for file management—never read+write entire contents: - -```bash -mv old.php new.php # Rename/move -cp source.php dest.php # Copy -mkdir -p path/to/dir # Create directories -``` - -### Execution Protocol - -1. ULTRATHINK - analyze problem deeply -2. STEP BY STEP - break into logical steps -3. ACT - implement systematically - -### Test later - -Don't run or create or update tests UNLESS explicitly instructed to do so. - -Tests are something we will focus on separately from building features. diff --git a/.cursor/rules/01-architecture.mdc b/.cursor/rules/01-architecture.mdc deleted file mode 100644 index 0ac9ddb6..00000000 --- a/.cursor/rules/01-architecture.mdc +++ /dev/null @@ -1,188 +0,0 @@ ---- -alwaysApply: true ---- - -## Architecture Rules - -All rules MANDATORY. - -### PHP Standards - -- PSR-12, strict types, PHP 8.x features (unions, match, attributes, readonly) -- Explicit return types with generics: `Collection` -- Dependency injection via Symfony patterns -- Use Symfony classes over native PHP functions (Filesystem, Process) for testability -- **Yoda conditions:** Always place constants/literals on the LEFT side of comparisons to prevent accidental assignment -- **Always use braces:** ALL control structures (`if`, `else`, `elseif`, `for`, `foreach`, `while`, `do-while`) MUST use curly braces `{ }`, even for single-line bodies - -```php -// ✅ CORRECT - Yoda conditions (constant on left) -if (null === $value) { ... } -if ('' === trim($name)) { ... } -if (0 !== $exitCode) { ... } -if ('active' === $status) { ... } -if (null !== $this->repo->find($id)) { ... } - -// ❌ WRONG - Variable on left (risk of accidental assignment) -if ($value === null) { ... } -if (trim($name) === '') { ... } -if ($exitCode !== 0) { ... } -if ($status === 'active') { ... } - -// Note: Two variables - Yoda doesn't apply -if ($typedName !== $server->name) { ... } -if ($userInput === $expectedValue) { ... } -``` - -```php -// ✅ CORRECT - Always use braces -if (null === $value) { - return 'Value is required'; -} - -foreach ($items as $item) { - $this->process($item); -} - -// ❌ WRONG - Never omit braces -if (null === $value) return 'Value is required'; -foreach ($items as $item) $this->process($item); -``` - -### PHPStan Type Hints - -Use `@var` annotations to help PHPStan understand types it cannot infer, not `assert()` in production code. - -```php -// ✅ CORRECT - @var annotation (zero runtime impact) -/** @var string $apiToken */ -$apiToken = $this->env->get(['API_TOKEN']); - -// ❌ WRONG - assert() in production code (runtime cost, can be disabled) -$apiToken = $this->env->get(['API_TOKEN']); -assert(is_string($apiToken)); -``` - -### Imports - -Always add `use` statements for vendor packages and project classes. Root namespace FQDNs acceptable (`\InvalidArgumentException`, `\RuntimeException`). - -```php -// ✅ CORRECT -use Symfony\Component\Filesystem\Filesystem; -use PHPDeployer\Services\IOService; - -$fs = new Filesystem(); -throw new \InvalidArgumentException('Error'); - -// ❌ WRONG - inline FQDNs for non-root namespaces -$fs = new \Symfony\Component\Filesystem\Filesystem(); -``` - -### Dependency Injection System - -Use `$container->build(ClassName::class)` for all object creation. Container uses reflection to auto-wire dependencies. - -```php -// ✅ CORRECT -$service = $this->container->build(MyService::class); - -// ❌ WRONG -$service = new MyService(new Dependency()); -``` - -**Rule:** ALL object creation uses `$container->build()` except DTOs, value objects, pure data structures. - -**Container Access:** Constructor injection in production, direct instantiation in tests. - -```php -// Production -class SymfonyApp { - public function __construct(private readonly Container $container) {} - private function registerCommands(): void { - $command = $this->container->build(HelloCommand::class); - } -} - -// Tests -$container = new Container(); -$service = $container->build(TestService::class); -``` - -**Test Mocking:** Container supports `bind()` for mock instances: - -```php -$container = new Container(); -$container->bind(SSHService::class, $mockSSH); -$command = $container->build(ServerAddCommand::class); // Gets mock -``` - -**Integration:** Entry point: bin/deployer. Command registration: SymfonyApp.php. Services: Auto-injected via constructor. - -### Layer Separation - -**Command Layer:** - -- Handle user interaction (input/output), orchestrate Services -- NO business logic (delegate to Services) -- NO duplicate orchestration (extract to shared Services) -- Responsible for console styling, error formatting, prompts -- Never invoke other commands (NO proxy commands) - -**Service Layer:** - -- Atomic, reusable functionality with NO console I/O -- Accept/return plain PHP data types -- Dependency-injected via constructor -- Handle business logic, external APIs, file operations -- Complex orchestration shared by Commands extracted to dedicated Services - -**Service State:** - -- Stateless: Pure operations, no internal state (validators, calculators, API clients) -- Stateful: Manage configuration/cached data (config loaders, file managers, repositories) -- Stateful services use lazy loading, explicit initialization via public methods (`load()`, `initialize()`), document requirements - -**Console I/O:** Only Commands perform console I/O. SymfonyStyle for all output. Services return exceptions/data for Commands to display. See 03-commands.mdc. - -**Dependencies:** - -- Commands depend on Services -- Services depend on Services/utilities -- All dependencies in constructor signatures -- NO circular dependencies - -### Comments - -**DocBlock:** Minimalist descriptions, parameters, return types for classes and functions. - -**Comment structure:** - -``` -// ---- -// {h1} -// ---- - -// -// {h2} -// ---- - -// -// {h3} - -// {p} -``` - -Separate sections visually. One newline between headers/subheaders/paragraphs. No obvious comments. Remove comments when removing code. - -### Quality Gates - -ALWAYS run before completing task, fix all issues: - -```bash -vendor/bin/rector $CHANGED_PHP_FILES # Code improvements -vendor/bin/pint $CHANGED_PHP_FILES # Fix style -vendor/bin/phpstan analyze $CHANGED_PHP_FILES_EXCEPT_TESTS # NEVER run on tests/ -``` - -PHPStan excluded from tests - tests focus on testing functionality over type compliance. diff --git a/.cursor/rules/02-tests.mdc b/.cursor/rules/02-tests.mdc deleted file mode 100644 index bac3bd0b..00000000 --- a/.cursor/rules/02-tests.mdc +++ /dev/null @@ -1,159 +0,0 @@ ---- -alwaysApply: true ---- - -## Testing Rules - -All rules MANDATORY. - -**Philosophy:** "A test that never fails is not a test, it's a lie." - -**Framework:** Pest exclusively, `it()` syntax, 70%+ coverage - -### Running Tests - -```bash -composer pest # Full suite with coverage (parallel) -vendor/bin/pest $TEST_FILE # Specific file -``` - -### Dependency Injection in Tests - -**DI Container rule applies to PRODUCTION code, not tests.** - -**Unit Tests (Services/Utilities) - Manual Instantiation:** - -```php -$mockFs = mockFilesystem(true, 'content'); -$service = new EnvService(new FilesystemService($mockFs), new Dotenv()); -``` - -Clear dependency wiring, easy mocking, no container overhead. - -**Command/Integration Tests - Container with Bindings:** - -```php -// Basic usage -$container = mockCommandContainer(); -$command = $container->build(ServerAddCommand::class); - -// Override services -$customSSH = mockSSHServiceWithBehavior(canConnect: false); -$container = mockCommandContainer(ssh: $customSSH); - -// Pre-populate data -$container = mockCommandContainer( - inventoryData: ['servers' => [['name' => 'web1', 'host' => '192.168.1.1']]] -); -``` - -Sustainable pattern - no updates when BaseCommand grows. - -**Container Auto-wiring (Edge Cases Only):** - -```php -$container = new Container(); -$container->bind(Filesystem::class, $mockFs); -$service = $container->build(CustomService::class); -``` - -Use when verifying DI configuration or testing service integration. - -**Maintenance:** When adding service to BaseCommand, update `mockCommandContainer()` in `tests/TestHelpers.php`: - -```php -function mockCommandContainer( - ?NewService $newService = null, // 1. Add parameter - // ... existing params -) { - $newService = $newService ?? mockNewService(); // 2. Build or use provided - $container->bind(NewService::class, $newService); // 3. Bind -} -``` - -### Test Minimalism - -**Target:** Keep test files under 1.8x source code size. - -**Rules:** - -- Test core business logic only, skip framework testing -- Use datasets: `->with([])` for multiple scenarios -- Eliminate 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 primary concern -- Don't sacrifice readability for ratio targets - -**Don't consolidate:** Different public methods, exception vs normal flow, different setup, distinct business logic. - -### AAA Pattern - -```php -it('does something', function () { - // ARRANGE - $service = new Service(mock(Dependency::class)); - - // ACT - $result = $service->action(); - - // ASSERT - expect($result)->toBe('expected'); - - // CLEANUP (when needed) - unlink($tempFile); -}); -``` - -Exception tests: `// ACT & ASSERT` when act triggers assertion. - -Use `describe()` blocks, `beforeEach()` setup, extract helpers/traits for DRY tests. - -### Testing Patterns - -**Forbidden:** - -```php -expect($x)->toBeInstanceOf(Class::class); // Type-only testing -expect($x)->toBeArray(); // Generic assertion -expect($x)->not->toBeNull(); // Meaningless alone -expect($x)->not->toBeNull()->and($x)->toContain('text'); // Redundant -expect(true)->toBeTrue(); // Literally meaningless -sleep(...); // Test logic not time -``` - -**Required:** - -```php -expect($config->getValue('host'))->toBe('example.com'); -expect($this->validator->isValid($input))->toBe($expected); -$mock->shouldReceive('method')->with('param')->andReturn('result'); - -// For polling/timeout - use zero intervals -$service->waitForReady('id', timeout: 10, pollInterval: 0); -``` - -### Test Types - -**Unit Tests:** - -- Mock all external dependencies (filesystem, HTTP, processes) -- Test single units in isolation -- Complete in milliseconds - -**Integration Tests:** - -- Real file operations and external processes -- CLI commands and full workflows - -**Layer Strategy:** - -- CLI Commands → Integration tests -- Business Services → Unit tests (mocked dependencies) -- Utilities/Helpers → Unit tests - -### Static Analysis - -**PHPStan applies to PRODUCTION code, not tests.** - -Ignore PHPStan issues in tests. Focus on functionality over compliance. Avoid excessive phpdoc to appease types. diff --git a/.cursor/rules/03-commands.mdc b/.cursor/rules/03-commands.mdc deleted file mode 100644 index aadbb16a..00000000 --- a/.cursor/rules/03-commands.mdc +++ /dev/null @@ -1,525 +0,0 @@ ---- -alwaysApply: true ---- - -## Symfony Console Rules - -All rules MANDATORY. Fix violating code, not these rules. - -### Core Principle - -Every command MUST be fully runnable non-interactively via CLI options. Every prompt MUST have a corresponding CLI option. Test by running the command with only CLI options (no TTY). - -### Output Method Philosophy - -NEVER use Symfony IO methods directly - use BaseCommand methods exclusively. - -All console output flows through custom methods in BaseCommand for consistent TUI styling. - -**Custom IO Methods:** - -```php -// Output -$this->out(['Multiple', 'lines']); -$this->hr(); -$this->h1('Section Heading'); -$this->displayDeets(['Key' => 'value']); - -// Status messages -$this->yay('Operation completed'); // ✓ checkmark -$this->nay('Operation failed'); // ✗ red X -$this->warn('Skipping step'); // ! warning -$this->info('Configuration loaded'); // ℹ info -$this->ul(['Item 1', 'Item 2']); // • bullet list -$this->ol(['Step 1', 'Step 2']); // 1. numbered list -``` - -**Missing a method?** Add to BaseCommand with modern styling. - -**Integration Points:** - -- Base: BaseCommand.php -- Output: ConsoleOutputTrait.php -- Input: ConsoleInputTrait.php -- Methods: `out()`, `hr()`, `h1()`, `info()`, `yay()`, `nay()`, `warn()`, `ul()`, `ol()`, `displayDeets()`, `commandReplay()`, `getOptionOrPrompt()`, `getValidatedOptionOrPrompt()`, `promptText()`, `promptPassword()`, `promptConfirm()`, `promptSelect()`, `promptMultiselect()`, `promptSuggest()`, `promptSearch()`, `promptPause()`, `promptSpin()` - -**Trait Organization:** - -- ConsoleOutputTrait: Output/formatting methods using `$this->io` (SymfonyStyle) -- ConsoleInputTrait: Input methods using `$this->input` (InputInterface) -- BaseCommand: Shared initialization, configuration, orchestration (NOT individual I/O ops) - -### User Input with Laravel Prompts - -Use `laravel/prompts` for ALL user interactions: - -```php -use function Laravel\Prompts\{text, password, confirm, select, multiselect, suggest, search, spin}; - -$name = text('What is your name?', required: true); -$password = password('Enter password:', required: true); -$confirmed = confirm('Deploy?', default: false); -$env = select('Environment:', ['dev', 'staging', 'prod']); -$features = multiselect('Features:', ['cache', 'queue', 'logs']); -$result = spin(fn() => $this->service->process(), 'Processing...'); -``` - -Commands handle ALL user interaction. Services return plain data with NO console operations. - -### Interactive + Options Pattern - -Support both interactive prompts AND CLI options using `getOptionOrPrompt()`. - -**Basic Pattern:** - -```php -protected function configure(): void { - parent::configure(); - $this->addOption('name', null, InputOption::VALUE_REQUIRED, 'Server name'); - $this->addOption('environment', null, InputOption::VALUE_REQUIRED, 'Environment'); -} - -protected function execute(InputInterface $input, OutputInterface $output): int { - // Text input - checks CLI option first, prompts if not provided - $name = $this->io->getOptionOrPrompt( - 'name', - fn () => $this->io->promptText( - label: 'Server name:', - placeholder: 'web1', - required: true - ) - ); - - // Select input - $env = $this->io->getOptionOrPrompt( - 'environment', - fn () => $this->io->promptSelect( - label: 'Environment:', - options: ['dev', 'staging', 'prod'], - default: 'prod' - ) - ); - - return Command::SUCCESS; -} -``` - -**Signature:** `$this->io->getOptionOrPrompt(string $optionName, Closure $promptCallback): mixed` - -Prompt wrappers (`$this->io->promptText()`, `$this->io->promptSelect()`, etc.) automatically suppress extra spacing for clean output. - -### Input Validation - -**Core Principles:** - -- Validate everything: CLI options, prompts, and auto-resolved values -- Fail fast: Validate before expensive operations (API calls, SSH connections) -- Never silent fallback: Explicit user input must fail validation, not fall back to defaults -- Complete errors: Include invalid value and guidance - -**Validator Signature:** - -```php -// Returns ?string: error message if invalid, null if valid -protected function validateNameInput(mixed $value): ?string -{ - if (!is_string($value)) { - return 'Name must be a string'; - } - - if ('' === trim($value)) { - return 'Name cannot be empty'; - } - - if (null !== $this->repo->findByName($value)) { - return "'{$value}' already exists"; - } - - return null; -} -``` - -**Naming Convention:** - -- `validate*Input()` - Returns `?string` (for prompts/options) -- `validate*()` - Throws exceptions (for heavy I/O like git repo checks) - -**Usage with getValidatedOptionOrPrompt:** - -```php -$name = $this->io->getValidatedOptionOrPrompt( - 'name', - fn ($validate) => $this->io->promptText(label: 'Name:', validate: $validate), - fn ($value) => $this->validateNameInput($value) -); - -if (null === $name) { - return Command::FAILURE; // Validation failed, error already displayed -} -``` - -**Optional Input with Fallback Resolution:** - -```php -// Allow empty to trigger default resolution -protected function validateKeyPathInputAllowEmpty(mixed $path): ?string -{ - if (!is_string($path)) { - return 'Path must be a string'; - } - - if ('' === trim($path)) { - return null; // Allow empty - triggers default - } - - return $this->validateKeyPathInput($path); // Non-empty: validate strictly -} - -// After validation - only fallback for empty, expand explicit paths -$resolved = ('' === trim($pathRaw)) - ? $this->resolveDefaultPath() // Fallback resolution - : $this->fs->expandPath($pathRaw); // User's explicit path (validated) -``` - -**Selection from Dynamic Data:** - -```php -protected function validateRegion(mixed $region, array $validRegions): ?string -{ - if (!is_string($region)) { - return 'Region must be a string'; - } - - if (!isset($validRegions[$region])) { - return "Invalid region: '{$region}' not available"; - } - - return null; -} -``` - -**Error Message Guidelines:** - -- Include invalid value: `"Invalid region: 'xyz1' not available"` -- Provide guidance: `"Port must be 1-65535 (common: 22, 2222)"` -- Show format examples: `"UUID format: 12345678-1234-1234-1234-123456789abc"` - -**Common Mistakes:** - -```php -// ❌ Missing null check -$value = $this->io->getValidatedOptionOrPrompt(...); -$this->doSomething($value); // $value could be null! - -// ❌ Silent fallback on explicit input -$path = $this->getOptionOrPrompt('key-path', ...); -$resolved = $this->resolveKey($path); // Falls back even if user's path invalid! - -// ❌ CLI option bypasses validation -$env = $this->getOptionOrPrompt('env', fn() => promptSelect(..., options: $valid)); -// --env=invalid passes through unvalidated! -``` - -See: ServerValidationTrait.php, KeyValidationTrait.php - -### Boolean Flags - -**VALUE_NONE - Simple flags:** - -```php -// --yes or -y -$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); - -$confirmed = $this->io->getOptionOrPrompt( - 'yes', - fn () => $this->io->promptConfirm('Deploy now?', default: true) -); -``` - -**VALUE_NEGATABLE - Tri-state flags:** - -```php -// --php-default or --no-php-default -$this->addOption('php-default', null, InputOption::VALUE_NEGATABLE, 'Set as default PHP'); - -// Prompts if neither --flag nor --no-flag provided -$setDefault = $this->io->getOptionOrPrompt( - 'php-default', - fn () => $this->io->promptConfirm('Set as default?', default: false) -); -``` - -### Multiselect with Comma-Separated CLI Input - -Multiselect prompts return arrays, but CLI options are strings. Handle both: - -```php -$this->addOption('databases', null, InputOption::VALUE_REQUIRED, 'Comma-separated databases'); - -$selected = $this->io->getOptionOrPrompt( - 'databases', - fn () => $this->io->promptMultiselect( - label: 'Database servers (optional):', - options: ['postgresql' => 'PostgreSQL', 'mysql' => 'MySQL', 'mongodb' => 'MongoDB'], - default: [], - hint: 'Space to select, Enter to confirm' - ) -); - -// Handle both array (from prompt) and string (from CLI option) -if (is_string($selected)) { - $selected = array_filter( - array_map(trim(...), explode(',', $selected)), - static fn (string $item): bool => '' !== $item - ); -} - -// Validate CLI-provided values against allowed options -$unknown = array_diff($selected, array_keys($allowedOptions)); -if ([] !== $unknown) { - $this->nay('Unknown options: ' . implode(', ', $unknown)); - return Command::FAILURE; -} -``` - -### Multi-Path Prompts - -When a prompt offers choices leading to different execution paths, create separate options for each path: - -```php -// ❌ WRONG - Only one option, can't skip the choice prompt non-interactively -$this->addOption('deploy-key', null, InputOption::VALUE_REQUIRED, 'Path to deploy key'); - -// ✅ CORRECT - Separate options for each path -$this->addOption('generate-deploy-key', null, InputOption::VALUE_NONE, 'Use server-generated deploy key'); -$this->addOption('custom-deploy-key', null, InputOption::VALUE_REQUIRED, 'Path to custom deploy key'); -``` - -Handle the branching logic explicitly: - -```php -/** @var bool $generateKey */ -$generateKey = $input->getOption('generate-deploy-key'); -/** @var string|null $customKeyPath */ -$customKeyPath = $input->getOption('custom-deploy-key'); - -// Check for conflicting options -if ($generateKey && null !== $customKeyPath) { - $this->nay('Cannot use both --generate-deploy-key and --custom-deploy-key'); - return Command::FAILURE; -} - -if ($generateKey) { - $deployKeyPath = null; // Use server-generated -} elseif (null !== $customKeyPath) { - $deployKeyPath = $customKeyPath; // Use custom -} else { - // Interactive: prompt for choice, then conditionally prompt for path - $choice = $this->io->promptSelect( - label: 'Deploy key:', - options: [ - 'generate' => 'Use server-generated key pair', - 'custom' => 'Use your own key pair', - ], - default: 'generate' - ); - - if ('generate' === $choice) { - $deployKeyPath = null; - } else { - $deployKeyPath = $this->io->promptText( - label: 'Path to private key:', - placeholder: '~/.ssh/deploy_key', - required: true - ); - } -} -``` - -### Confirmation Patterns - -**Simple Confirmation (`--yes`):** - -```php -$this->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation'); - -$confirmed = $this->io->getOptionOrPrompt( - 'yes', - fn () => $this->io->promptConfirm('Are you sure?', default: false) -); - -if (!$confirmed) { - $this->warn('Operation cancelled'); - return Command::SUCCESS; -} -``` - -**Type-to-Confirm (`--force`) - For destructive operations:** - -```php -$this->addOption('force', 'f', InputOption::VALUE_NONE, 'Skip typing confirmation'); - -/** @var bool $forceSkip */ -$forceSkip = $input->getOption('force') ?? false; - -if (!$forceSkip) { - $typedName = $this->io->promptText( - label: "Type the server name '{$server->name}' to confirm deletion:", - required: true - ); - - if ($typedName !== $server->name) { - $this->nay('Name does not match. Operation cancelled.'); - return Command::FAILURE; - } -} -``` - -### Post-CLI Validation - -When using `getOptionOrPrompt()` (not validated version), validate CLI values explicitly: - -```php -$phpVersion = (string) $this->io->getOptionOrPrompt( - 'php-version', - fn () => $this->io->promptSelect( - label: 'PHP version:', - options: $availableVersions, - default: $defaultVersion - ) -); - -// Validate CLI-provided version exists -if (!in_array($phpVersion, $availableVersions, true)) { - $this->nay( - "PHP {$phpVersion} not available. Available: " . implode(', ', $availableVersions) - ); - return Command::FAILURE; -} -``` - -### Resource Selection Pattern - -For selecting from existing resources: - -```php -protected function selectServer(): ServerDTO|int -{ - $servers = $this->ensureServersAvailable(); - if (is_int($servers)) { - return Command::FAILURE; - } - - $serverNames = array_map(fn (ServerDTO $s) => $s->name, $servers); - - $name = (string) $this->io->getOptionOrPrompt( - 'server', - fn () => $this->io->promptSelect( - label: 'Select server:', - options: $serverNames, - ) - ); - - // Validate CLI-provided name exists - $server = $this->servers->findByName($name); - if (null === $server) { - $this->nay("Server '{$name}' not found in inventory"); - return Command::FAILURE; - } - - return $server; -} -``` - -### Command Options & Input - -**Naming Convention Rules:** - -| Option | Usage | Type | When | -| ---------------- | ------------------------ | ---------------- | -------------------- | -| `--server` | Select existing server | `VALUE_REQUIRED` | delete, info, deploy | -| `--domain` | Select existing site | `VALUE_REQUIRED` | site operations | -| `--name` | Define new resource name | `VALUE_REQUIRED` | add, create | -| `--host` | Host/IP address | `VALUE_REQUIRED` | server config | -| `--port` | Port number | `VALUE_REQUIRED` | server config | -| `--yes` / `-y` | Skip confirmation | `VALUE_NONE` | all confirmations | -| `--force` / `-f` | Skip type-to-confirm | `VALUE_NONE` | delete commands | -| `--skip` | Skip validation | `VALUE_NONE` | validation steps | - -**Golden Rule:** - -- `--server` / `--domain`: SELECTING existing (operation target) -- `--name`: DEFINING new resource property - -Prevents conflicts in commands working with multiple resource types. - -**Additional Rules:** - -- Use OPTIONS only, never ARGUMENTS (enables `getOptionOrPrompt()`) -- Pair all options with `getOptionOrPrompt()` for dual-mode support -- Boolean flags: `VALUE_NONE` or `VALUE_NEGATABLE` -- Data inputs: `VALUE_REQUIRED` -- Only `--yes` (`-y`) and `--force` (`-f`) get short flags -- Add type annotations on option retrievals: `/** @var bool $flag */` - -See: ServerDeleteCommand.php, ServerAddCommand.php - -### Command Completion - -Always call `commandReplay()` before returning SUCCESS to teach non-interactive usage: - -```php -$this->commandReplay('command:name', [ - 'option1' => $value1, - 'option2' => $value2, -]); - -return Command::SUCCESS; -``` - -**For conditional options, only include if applicable:** - -```php -$replayOptions = [ - 'server' => $server->name, - 'php-version' => $phpVersion, -]; - -if (null !== $deployKeyPath) { - $replayOptions['custom-deploy-key'] = $deployKeyPath; -} else { - $replayOptions['generate-deploy-key'] = true; -} - -if ($defaultPrompted) { - $replayOptions['php-default'] = $setDefault; -} - -$this->commandReplay('server:install', $replayOptions); -``` - -Teaches CLI syntax, improves DX, reduces support questions. - -**Output:** - -``` -◆ Run non-interactively: - - vendor/bin/deployer server:delete \ - --server='production-web-01' \ - --yes -``` - -See: ServerDeleteCommand.php - -### Summary Checklist - -When building commands, verify: - -- [ ] Every prompt has a corresponding `addOption()` -- [ ] Multi-path prompts have separate options for each path -- [ ] Conflicting options are detected and rejected -- [ ] CLI values are validated against allowed options -- [ ] Multiselect handles both array and comma-separated string -- [ ] `commandReplay()` called before SUCCESS return -- [ ] Type annotations on option retrievals (`/** @var bool $flag */`) diff --git a/.cursor/rules/04-exceptions.mdc b/.cursor/rules/04-exceptions.mdc deleted file mode 100644 index 3af74f26..00000000 --- a/.cursor/rules/04-exceptions.mdc +++ /dev/null @@ -1,220 +0,0 @@ ---- -alwaysApply: true ---- - -## Exception Handling & Error Display - -All rules MANDATORY. - -### Core Principle - -Services throw complete, user-facing exceptions. Command layer (Commands + Traits) displays them directly without adding prefixes. - -### Services & Repositories - -Throw `\RuntimeException` with complete, actionable messages: - -```php -// ✅ CORRECT - Complete message with context -throw new \RuntimeException("SSH key does not exist: {$privateKeyPath}"); -throw new \RuntimeException("Server '{$name}' already exists"); - -// ✅ CORRECT - Preserve exception chain when wrapping -} catch (\Throwable $e) { - throw new \RuntimeException( - "SSH authentication failed for {$username}@{$host}. Check username and key permissions", - previous: $e - ); -} - -// ❌ WRONG - Fragment requiring concatenation -throw new \RuntimeException("does not exist"); - -// ❌ WRONG - Generic prefix causing concatenation -} catch (\Throwable $e) { - throw new \RuntimeException("Failed to execute: " . $e->getMessage()); -} -``` - -**Rules:** - -- Messages must be user-facing and complete (not fragments) -- Include relevant context (paths, names, IDs, hosts) -- Use `previous: $e` to preserve exception chains for debugging -- Never catch and re-throw with generic prefixes like "Failed to..." -- Wrap when lower exception is technical; let bubble when already user-facing - -### Validation Traits - -Two distinct patterns: - -**Pattern A: Input Validation (for prompts/CLI options)** - -Returns `?string` - error message or null: - -```php -protected function validateNameInput(mixed $name): ?string -{ - if (!is_string($name)) { - return 'Server name must be a string'; - } - - if ('' === trim($name)) { - return 'Server name cannot be empty'; - } - - if (null !== $this->servers->findByName($name)) { - return "Server '{$name}' already exists"; - } - - return null; -} -``` - -**Pattern B: Heavy I/O Validation** - -Throws `\RuntimeException`: - -```php -protected function validateGitRepo(string $repo): void -{ - try { - $process = $this->proc->run(['git', 'ls-remote', '--exit-code', $repo], $cwd, 10.0); - - if (!$process->isSuccessful()) { - throw new \RuntimeException( - "Cannot access git repository '{$repo}'. Check the URL and your network connection." - ); - } - } catch (\Exception $e) { - throw new \RuntimeException( - "Failed to validate git repository '{$repo}': " . $e->getMessage(), - previous: $e - ); - } -} -``` - -**Naming Convention:** - -- `validate*Input()` - Returns `?string` (for prompts) -- `validate*()` - Throws exceptions (for I/O) - -### Orchestration Traits (Command Layer) - -Traits mixed into Commands ARE Command layer. Display errors directly without redundant prefixes: - -```php -// ✅ CORRECT - Display exception directly, no prefix -protected function serverInfo(ServerDTO $server): array|int -{ - try { - $result = $this->executePlaybook($server, 'server-info', 'Gathering...'); - } catch (\RuntimeException $e) { - $this->nay($e->getMessage()); // No "Failed to..." prefix - return Command::FAILURE; - } - - if (0 !== $result['exit_code']) { - $this->nay('Failed to gather server information'); - return Command::FAILURE; - } - - // Parse and return -} - -// ❌ WRONG - Adding redundant prefix -} catch (\RuntimeException $e) { - $this->nay('Failed to gather server information: ' . $e->getMessage()); - // Results in: "Failed to gather server information: SSH authentication failed..." -} -``` - -**When to add context:** - -- Displaying raw output for debugging -- Adding actionable troubleshooting steps -- Exception message is too technical/generic - -```php -// ✅ CORRECT - Adding helpful context, not redundant prefix -} catch (\RuntimeException $e) { - $this->nay($e->getMessage()); - $this->io->writeln([ - '', - 'Troubleshooting:', - ' • Ensure server is online', - ' • Check SSH credentials', - '', - ]); - return Command::FAILURE; -} -``` - -### Commands - -Catch exceptions, display directly, return status: - -```php -// ✅ CORRECT - Display exception message directly -try { - $this->servers->create($server); -} catch (\RuntimeException $e) { - $this->nay($e->getMessage()); // Already complete: "Server 'web1' already exists" - return Command::FAILURE; -} - -// ❌ WRONG - Adding redundant prefix -} catch (\RuntimeException $e) { - $this->nay('Failed to add server: ' . $e->getMessage()); - // Results in: "Failed to add server: Server 'web1' already exists" -} -``` - -### Silent Failures - -Return `null`/`false` only for optional operations: - -```php -// ✅ CORRECT - Optional detection -public function detectRemoteUrl(?string $workingDir = null): ?string -{ - try { - $process = $this->proc->run(['git', 'config', '--get', 'remote.origin.url'], $workingDir); - return $process->isSuccessful() ? trim($process->getOutput()) : null; - } catch (\Exception) { - return null; // Not in git repo, that's okay - } -} - -// ❌ WRONG - Required operation returning null -public function executeCommand(string $host, ...): ?array -{ - try { - return $ssh->exec($command); - } catch (\Throwable) { - return null; // Caller doesn't know WHY it failed - } -} -``` - -### Exception Message Quality - -Every exception message must be: - -- Complete (not: "does not exist", but: "SSH key does not exist: /path/to/key") -- User-facing (not: "PDO error 2002", but: "Cannot connect to database. Check host and port.") -- Actionable with context (paths, names, IDs, hosts) -- Free of redundant prefixes (not: "Failed to execute: Failed to connect: Connection refused") - -Exception chains preserved via `previous: $e` for debugging. - -### Layer Responsibility Summary - -| Layer | Display Errors? | Pattern | -| -------------------- | --------------- | ------------------------------ | -| Services | ❌ No | Throw complete exceptions | -| Repositories | ❌ No | Throw complete exceptions | -| Validation Traits | ❌ No | Return `?string` or throw | -| Orchestration Traits | ✅ Yes | Catch & display without prefix | -| Commands | ✅ Yes | Catch & display without prefix | diff --git a/.cursor/rules/05-bash.mdc b/.cursor/rules/05-bash.mdc deleted file mode 100644 index a6bdf1c6..00000000 --- a/.cursor/rules/05-bash.mdc +++ /dev/null @@ -1,128 +0,0 @@ ---- -alwaysApply: true ---- - -## Bash Style - -All rules MANDATORY. Based on https://style.ysap.sh/md - -### Core Syntax - -**Conditionals:** Use `[[ ... ]]` not `[ ... ]` or `test` - -```bash -[[ -d /etc ]] # ✅ CORRECT -[ -d /etc ] # ❌ WRONG -``` - -**Command Substitution:** Use `$(...)` not backticks - -```bash -foo=$(date) # ✅ CORRECT -foo=`date` # ❌ WRONG -``` - -**Math:** Use `((...))` and `$((...))`, never `let` - -```bash -if ((a > b)); then ... # ✅ CORRECT -if [[ $a -gt $b ]]; then # ❌ WRONG - use math syntax for comparisons -``` - -**Functions:** No `function` keyword, always use `local` for variables - -```bash -foo() { local i=5; } # ✅ CORRECT -function foo { i=5; } # ❌ WRONG - global variable, function keyword -``` - -**Block Statements:** `then` same line as `if`, `do` same line as `while` - -```bash -if true; then ... # ✅ CORRECT -while true; do ... # ✅ CORRECT -``` - -### Parameter Handling - -**Expansion:** Prefer parameter expansion over external commands - -```bash -prog=${0##*/} # ✅ CORRECT - basename -nonumbers=${name//[0-9]/} # ✅ CORRECT - remove numbers -prog=$(basename "$0") # ❌ WRONG - external command -``` - -**Quoting:** Double quotes for expansions, single for literals - -```bash -echo "$foo" # ✅ CORRECT - expansion needs quotes -bar='literal' # ✅ CORRECT - no expansion -if [[ -n $foo ]]; then # ✅ CORRECT - [[ ... ]] doesn't word-split -``` - -Exception: Variables controlled by script (not user input) may be unquoted in `[[ ... ]]` - -**Arrays:** Use bash arrays, not space-separated strings - -```bash -modules=(a b c) # ✅ CORRECT -for m in "${modules[@]}" # ✅ CORRECT - quoted array expansion -modules='a b c' # ❌ WRONG - string not array -``` - -### Error Handling - -**Check commands that can fail:** - -```bash -cd /path || exit # ✅ CORRECT - exit on failure -cd /path # ❌ WRONG - what if cd fails? -rm file -``` - -**Pipeline errors:** Use `set -o pipefail` in playbooks - -**Don't use `set -e`:** Explicit error checking preferred over errexit - -**Never use `eval`:** Security risk, static analysis impossible - -### File Operations - -**Reading files:** Use redirection or built-in read - -```bash -while IFS=: read -r user _; do - echo "$user" -done < /etc/passwd # ✅ CORRECT - streaming - -grep foo file # ✅ CORRECT -cat file | grep foo # ❌ WRONG - useless use of cat -``` - -**Listing files:** Never parse `ls`, use globs - -```bash -for f in *; do ... # ✅ CORRECT -for f in $(ls); do ... # ❌ WRONG - unsafe -``` - -### Formatting - -- Tabs for indentation -- Max 80 columns -- Semicolons only in control statements (`if true; then`) -- Max 1 blank line between sections -- Shebang: `#!/usr/bin/env bash` - -### Quality Gates - -ALWAYS run before completing task, fix all issues: - -```bash -# Format all bash scripts -composer bash - -# Check formatting without modifying -composer bash:check -``` diff --git a/.cursor/rules/06-playbooks.mdc b/.cursor/rules/06-playbooks.mdc deleted file mode 100644 index e70ea557..00000000 --- a/.cursor/rules/06-playbooks.mdc +++ /dev/null @@ -1,361 +0,0 @@ ---- -alwaysApply: true ---- - -## Playbook Rules - -All rules MANDATORY. - -### Core Principles - -Playbooks are idempotent, non-interactive bash scripts that: - -- Execute one or more related tasks -- MUST be idempotent (safe to run multiple times) -- Receive context via environment variables -- Never prompt for user input -- Run completely unattended -- Return parsable YAML output - -### Structure - -Standard playbook structure using `main()` function: - -```bash -#!/usr/bin/env bash -set -o pipefail -export DEBIAN_FRONTEND=noninteractive - -# Validation -[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 - -# -# Helper Functions -# ---- -# (see "Helper Functions" section below for run_cmd implementation) - -# -# Main Execution -# ---- - -main() { - echo "→ Starting..." - - # Tasks go here - - if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 - fi -} - -main "$@" -``` - -**Pattern Requirements:** - -- Shebang: `#!/usr/bin/env bash` -- Always set `set -o pipefail` (NOT `set -e`) -- Export `DEBIAN_FRONTEND=noninteractive` -- Validate `$DEPLOYER_OUTPUT_FILE` before any work -- Use `main()` function with `main "$@"` at bottom -- Group related functions with comment headers -- Check errors on YAML writes - -### Environment Variables - -Use `DEPLOYER_` prefix. Standard variables: - -- `DEPLOYER_OUTPUT_FILE` - YAML output path (provided automatically) -- `DEPLOYER_DISTRO` - Distribution: `ubuntu|debian` (if needed) -- `DEPLOYER_PERMS` - Permissions: `root|sudo|none` (if needed) - -**Validation:** - -Detection playbooks only validate `DEPLOYER_OUTPUT_FILE`. Provisioning playbooks additionally validate `DEPLOYER_DISTRO` and `DEPLOYER_PERMS`, then export `DEPLOYER_PERMS` for subshell availability. See Complete Example section for full pattern. - -### Distribution Support - -Support Ubuntu and Debian only (both use apt package manager). Use `case` statements when Ubuntu/Debian need different package names or configurations: - -```bash -# ✅ CORRECT - case statement when distributions differ -case $DEPLOYER_DISTRO in - ubuntu) - distro_packages=(software-properties-common) - run_cmd apt-get install -y "${distro_packages[@]}" - ;; - debian) - distro_packages=(apt-transport-https lsb-release ca-certificates) - run_cmd apt-get install -y "${distro_packages[@]}" - ;; -esac - -# ❌ WRONG - Unnecessary branching for identical operations -case $DEPLOYER_DISTRO in - ubuntu|debian) - run_cmd apt-get update -q # Same for both! - run_cmd apt-get install -y -q caddy # Same for both! - ;; -esac -``` - -**Universal operations (no branching needed):** - -```bash -run_cmd apt-get update -q -run_cmd apt-get install -y -q caddy -run_cmd systemctl start caddy -run_cmd systemctl enable caddy -run_cmd mkdir -p /var/www/app -``` - -### Non-Interactive Operation - -Never prompt for input. Use non-interactive flags: - -- `export DEBIAN_FRONTEND=noninteractive` (always set at top) -- Package managers: `-y -q` flags -- GPG operations: `--batch --yes` -- systemctl: `--quiet` (where appropriate) -- Never use `read`, confirm dialogs, or interactive prompts - -### Idempotency - -Check before acting. Don't fail if resource already exists: - -```bash -# ✅ CORRECT - Idempotent patterns -if ! command -v caddy >/dev/null 2>&1; then - run_cmd apt-get install -y -q caddy -fi - -if [[ ! -d /var/www/app ]]; then - run_cmd mkdir -p /var/www/app -fi - -if ! systemctl is-enabled --quiet caddy; then - run_cmd systemctl enable --quiet caddy -fi - -# For config files that may exist (from packages), check for custom content markers -if ! grep -q "import conf.d/localhost.caddy" /etc/caddy/Caddyfile 2> /dev/null; then - echo "→ Creating Caddyfile with custom configuration..." - run_cmd tee /etc/caddy/Caddyfile > /dev/null <<- 'EOF' - # ... custom config with marker ... - EOF -fi - -# ❌ WRONG - Not idempotent -run_cmd useradd deployer # Fails second time -echo "export PATH=\$PATH:/usr/local/bin" >> ~/.bashrc # Duplicates each run - -# ❌ WRONG - File existence check when package installs default config -if ! run_cmd test -f /etc/caddy/Caddyfile; then - # This will never run if package created a default file! -fi -``` - -### Error Handling - -Use `set -o pipefail` but NOT `set -e`. Check exit codes explicitly: - -```bash -# Validation errors (before any work) → stdout -[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 - -# Runtime errors (during execution) → stderr -if ! mkdir -p /var/www/app 2>&1; then - echo "Error: Failed to create directory" >&2 - exit 1 -fi - -# Silent checks (expected to sometimes fail) -if ! command -v nginx >/dev/null 2>&1; then - echo "→ Installing nginx..." - run_cmd apt-get install -y -q nginx -fi - -# Check YAML writes -if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 -fi -``` - -**Error Detection:** - -If playbook exits before creating `$DEPLOYER_OUTPUT_FILE`, framework treats all output as error message. - -### Helper Functions - -Standard helper for permission-aware command execution: - -```bash -run_cmd() { - if [[ $DEPLOYER_PERMS == 'root' ]]; then - "$@" - else - sudo -n "$@" - fi -} -``` - -The `-n` flag ensures sudo fails fast without prompting for a password, maintaining non-interactive operation. - -**Sourcing Shared Helpers:** - -Playbooks use shared helper functions from `helpers.sh`. These helpers are automatically inlined when executing playbooks remotely, so playbooks should include a commented source line: - -```bash -# Shared helpers are automatically inlined when executing playbooks remotely -# source "$(dirname "$0")/helpers.sh" -``` - -**Rules:** - -- NEVER manually inline helpers into playbook files -- Keep the commented source line for documentation -- Helpers are inlined automatically during remote execution -- The comment pattern allows local testing if needed while documenting the dependency - -### Output - -Write YAML to `$DEPLOYER_OUTPUT_FILE`. Progress messages to stdout/stderr. - -**Pattern:** - -```bash -# Action messages (stdout) - indicate what's about to happen -echo "→ Installing packages..." -echo "→ Configuring service..." - -# YAML output to file (check for errors) -if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 -fi - -# Error messages (stderr) -if ! some_command; then - echo "Error: Command failed" >&2 - exit 1 -fi -``` - -**Action Messages:** - -Use `→` (Unicode rightwards arrow U+2192) to indicate an operation is about to start. Messages describe what's about to happen, not what happened. Keep concise and action-oriented. Be explicit: include paths, package names, versions, or identifiers. - -```bash -# ✅ CORRECT - Explicit details (path, package name, repository name) -echo "→ Creating /var/www/app directory..." -echo "→ Installing PHP 8.5..." -echo "→ Adding Caddy GPG key..." - -# ❌ WRONG - Too generic -echo "→ Creating directory..." -echo "→ Installing package..." -echo "→ Adding key..." - -# ✅ CORRECT - Unconditional operations (always run) -echo "→ Updating package lists..." -if ! apt_get_with_retry update; then - echo "Error: Failed to update package lists" >&2 - exit 1 -fi - -# ✅ CORRECT - Conditional operations (message INSIDE block, only when needed) -if ! [[ -f /usr/share/keyrings/caddy-stable-archive-keyring.gpg ]]; then - echo "→ Adding Caddy GPG key..." - if ! curl -1sLf 'https://example.com/key.gpg' | run_cmd gpg --batch --yes --dearmor -o /usr/share/keyrings/caddy-stable-archive-keyring.gpg; then - echo "Error: Failed to add Caddy GPG key" >&2 - exit 1 - fi -fi - -# ❌ WRONG - Message outside conditional (shows even when nothing happens) -echo "→ Configuring Caddy repository..." -if ! [[ -f /usr/share/keyrings/caddy-stable-archive-keyring.gpg ]]; then - # GPG key logic... -fi -``` - -**Rules:** Be explicit with paths/names/versions. Place messages INSIDE conditional blocks for idempotent operations, OUTSIDE only for operations that always run. Never write progress to output file. See: `playbooks/package-manager.sh` - -### Complete Example - -```bash -#!/usr/bin/env bash -set -o pipefail -export DEBIAN_FRONTEND=noninteractive - -# Validation -[[ -z $DEPLOYER_OUTPUT_FILE ]] && echo "Error: DEPLOYER_OUTPUT_FILE required" && exit 1 -[[ -z $DEPLOYER_DISTRO ]] && echo "Error: DEPLOYER_DISTRO required" && exit 1 -[[ -z $DEPLOYER_PERMS ]] && echo "Error: DEPLOYER_PERMS required" && exit 1 -export DEPLOYER_PERMS - -# -# Helper Functions -# ---- - -run_cmd() { - if [[ $DEPLOYER_PERMS == 'root' ]]; then - "$@" - else - sudo -n "$@" - fi -} - -# -# Main Execution -# ---- - -main() { - local caddy_version - - if ! command -v caddy >/dev/null 2>&1; then - echo "→ Installing Caddy web server..." - run_cmd apt-get update -q - run_cmd apt-get install -y -q caddy - fi - - if [[ ! -d /var/www/app ]]; then - echo "→ Creating /var/www/app directory..." - run_cmd mkdir -p /var/www/app - fi - - if ! systemctl is-enabled --quiet caddy; then - echo "→ Enabling Caddy service..." - run_cmd systemctl enable --quiet caddy - fi - - caddy_version=$(caddy version 2>&1 | cut -d' ' -f1) - - if ! cat > "$DEPLOYER_OUTPUT_FILE" <&2 - exit 1 - fi -} - -main "$@" -``` - -See: `playbooks/server-info.sh` diff --git a/.cursor/rules/rules.mdc b/.cursor/rules/rules.mdc deleted file mode 100644 index 278e1bf0..00000000 --- a/.cursor/rules/rules.mdc +++ /dev/null @@ -1,154 +0,0 @@ ---- -alwaysApply: false ---- - -# Rules for Writing Rules - -Guidelines for creating and maintaining rule files optimized for AI agents with limited token windows. - -## Token Efficiency Principles - -- Use imperative mood, not conversational prose -- One example per pattern maximum -- Remove "Benefits", "Why This Matters", "Key Benefits" sections -- Use inline comments over separate explanations -- Use bullets over paragraphs -- Single "All rules MANDATORY" statement per file -- No repetitive CRITICAL/IMMUTABLE warnings -- No emoji in headers (wastes tokens, no AI value) - -## Structure Standards - -**File Header:** - -```yaml ---- -alwaysApply: true ---- -## [Section Name] - -All rules MANDATORY unless marked optional. -``` - -**Section Organization:** - -- Clear, scannable headers -- Related rules grouped together -- Alphabetical ordering when no logical grouping exists -- No more than 3 heading levels - -## Example Guidelines - -**Pattern: Show correct first, wrong only when non-obvious** - -```php -// ✅ CORRECT -$result = $container->build(Service::class); - -// ❌ WRONG - manual instantiation breaks DI -$result = new Service(new Dependency()); -``` - -**Rules:** - -- Keep examples under 10 lines -- Use `// ✅ CORRECT` and `// ❌ WRONG` markers consistently -- Prefer inline comments to prose explanations -- Remove examples for well-known patterns (AAA, SOLID, etc.) -- Don't explain framework features (Laravel Prompts, Pest, Symfony) - -## Cross-File Coordination - -**Avoid Duplication:** - -- Single source of truth per concept -- If rule appears in multiple contexts, pick primary location -- Cross-reference by filename only: "See 03-commands.mdc" -- No line number references (brittle) - -**Valid Cross-References:** - -```markdown -See [ServerValidationTrait.php](mdc:app/Traits/ServerValidationTrait.php) -Covered in 03-commands.mdc -``` - -**Verify Links:** - -- All `mdc://` references must point to existing files -- Remove references to deleted files immediately - -## Maintenance Checklist - -Before committing rule changes: - -1. Remove outdated file references -2. Check for duplication with other rule files -3. Verify no contradictions introduced -4. Test that code examples compile/run -5. Run token count comparison (target: 35-65% of original verbosity) -6. Confirm critical rules still emphasized (but not repetitively) - -## Anti-Patterns - -**Avoid:** - -- Copying third-party documentation verbatim (summarize key points only) -- Multiple examples showing identical patterns -- Teaching language/framework fundamentals -- Explaining obvious concepts -- Tables where bullet lists suffice -- Conversational explanations of patterns shown in code - -**Example - Too Verbose:** - -```markdown -**How It Works:** - -1. Container analyzes constructor via reflection -2. Recursively builds dependencies -3. Caches reflection data -4. Handles errors gracefully - -**Key Benefits:** - -- Zero configuration required -- Type-safe with generics -- Easy testing with mocks -``` - -**Example - Optimized:** - -```markdown -Use `$container->build(Class::class)` for all object creation. -Exceptions: DTOs, value objects, pure data structures. -``` - -## Token Budget Awareness - -- AI agents may have 8K-32K context windows -- Rules should consume <20% of available tokens -- Leave 80% for code, history, and responses -- Total rule corpus target: <3000 tokens (~600-800 lines) - -## Writing Style - -**Prefer:** - -```markdown -Commands handle user I/O. Services contain business logic. No circular dependencies. -``` - -**Over:** - -```markdown -Commands are responsible for handling all user interaction including input and output operations, while Services provide the core business logic functionality. It's important to note that circular dependencies between these layers are not allowed and should be avoided at all costs. -``` - -**Emphasis Hierarchy:** - -1. Code examples (most efficient) -2. Imperative bullets -3. Short declarative sentences -4. Tables (only for reference data) -5. Prose explanations (last resort) From 93f5e26e3bd69a8c0ed9d95ca2e6a0e265699cab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 23:45:07 +0200 Subject: [PATCH 13/18] docs: update commit command to exclude AI attribution lines --- .claude/commands/commit.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md index 6b82dd18..f9bec29f 100644 --- a/.claude/commands/commit.md +++ b/.claude/commands/commit.md @@ -32,3 +32,5 @@ Examples: - chore(deps): bump composer/installers to ^2.3 Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). + +Do NOT include any AI attribution, "Generated with", or "Co-Authored-By" lines in commit messages. From e337c9f675b5bb1542d8e21f228eeab91fea3a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Thu, 4 Dec 2025 23:51:17 +0200 Subject: [PATCH 14/18] feat(commands): add /push command for GitHub PR workflow --- .claude/commands/push.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 .claude/commands/push.md diff --git a/.claude/commands/push.md b/.claude/commands/push.md new file mode 100644 index 00000000..354d085c --- /dev/null +++ b/.claude/commands/push.md @@ -0,0 +1,35 @@ +--- +description: Push branch and open a draft PR on GitHub +allowed-tools: Bash(git:*), Bash(gh:*) +model: haiku +--- + +Based on the current branch and its commits: + +A. Push the branch to GitHub + +Push the current branch to origin with tracking (-u flag). Do not force push. + +B. Open a draft pull request + +Create a draft PR using `gh pr create --draft` with: + +**Title:** Use Conventional Commits format matching the branch prefix: +- feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/ + +Keep titles short (≤ 72 chars), imperative, no trailing period. + +Examples: +- feat(parser): add support for PHP 8.4 attributes +- fix(ci): correct matrix PHP versions in build workflow +- chore(deps): bump composer/installers to ^2.3 + +**Body:** Generate a concise summary of changes from the commits on this branch. Include: +- Brief description of what changed +- Key implementation details (if relevant) + +Do NOT include any AI attribution, "Generated with", or "Co-Authored-By" lines. + +**Base branch:** Target `main` unless the branch name or commits suggest otherwise. + +After creating the PR, output the PR URL. From d9d6a23b7cb65d62d268eb139e6f69af8cead7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 5 Dec 2025 00:46:41 +0200 Subject: [PATCH 15/18] docs: clarify quality-gatekeeper commands and SymfonyApp.php path --- .claude/agents/quality-gatekeeper.md | 6 ++--- .claude/commands/improve-tests.md | 13 ----------- .claude/commands/review-branch.md | 16 -------------- .claude/commands/review-diff.md | 15 ------------- .claude/commands/review-pr-comment.md | 6 ----- .claude/settings.local.json | 10 ++++++++- .claude/skills/command/SKILL.md | 32 ++++++++++++++++++++++----- CLAUDE.md | 2 +- 8 files changed, 40 insertions(+), 60 deletions(-) delete mode 100644 .claude/commands/improve-tests.md delete mode 100644 .claude/commands/review-branch.md delete mode 100644 .claude/commands/review-diff.md delete mode 100644 .claude/commands/review-pr-comment.md diff --git a/.claude/agents/quality-gatekeeper.md b/.claude/agents/quality-gatekeeper.md index f9286ab0..a1437865 100644 --- a/.claude/agents/quality-gatekeeper.md +++ b/.claude/agents/quality-gatekeeper.md @@ -1,6 +1,6 @@ --- name: quality-gatekeeper -description: Use this agent when you need to run quality gate checks on PHP files or bash playbooks. This includes running Rector, Pint, and PHPStan on changed PHP files, or formatting/checking playbook shell scripts. Call this agent after making changes to PHP files or playbook scripts to ensure code quality standards are met before committing or completing a task.\n\nExamples:\n\n\nContext: The user has just finished implementing a new feature in PHP files.\nuser: "Add a new method to the ServerService class that validates server connections"\nassistant: "I've added the validateConnection method to ServerService.php. Now let me use the php-quality-gate agent to run quality checks on the changed files."\n\n\n\n\nContext: The user has modified a playbook bash script.\nuser: "Update the deploy.sh playbook to include a backup step"\nassistant: "I've updated the deploy.sh playbook with the backup step. Let me run the php-quality-gate agent to format and validate the bash script."\n\n\n\n\nContext: The assistant proactively runs quality gates after completing PHP changes.\nassistant: "I've finished refactoring the Repository classes. Before we proceed, I'll use the php-quality-gate agent to ensure all quality checks pass."\n\n +description: Use this agent when you need to run quality gate checks on PHP files or bash playbooks. This includes running Rector, Pint, and PHPStan on changed PHP files, or formatting/checking playbook shell scripts. Call this agent after making changes to PHP files or playbook scripts to ensure code quality standards are met before committing or completing a task.\n\nExamples:\n\n\nContext: The user has just finished implementing a new feature in PHP files.\nuser: "Add a new method to the ServerService class that validates server connections"\nassistant: "I've added the validateConnection method to ServerService.php. Now let me use the quality-gatekeeper agent to run quality checks on the changed files."\n\n\n\n\nContext: The user has modified a playbook bash script.\nuser: "Update the deploy.sh playbook to include a backup step"\nassistant: "I've updated the deploy.sh playbook with the backup step. Let me run the quality-gatekeeper agent to format and validate the bash script."\n\n\n\n\nContext: The assistant proactively runs quality gates after completing PHP changes.\nassistant: "I've finished refactoring the Repository classes. Before we proceed, I'll use the quality-gatekeeper agent to ensure all quality checks pass."\n\n model: haiku color: cyan --- @@ -20,7 +20,7 @@ Run these commands in sequence on changed PHP files: 1. **Rector** (automated refactoring): ```bash - vendor/bin/rector $CHANGED_PHP_FILES + vendor/bin/rector process $CHANGED_PHP_FILES ``` 2. **Pint** (code style formatting): @@ -31,7 +31,7 @@ Run these commands in sequence on changed PHP files: 3. **PHPStan** (static analysis): ```bash - vendor/bin/phpstan analyse $CHANGED_PHP_FILES + vendor/bin/phpstan analyse --memory-limit=2G $CHANGED_PHP_FILES ``` ### For Playbook Scripts diff --git a/.claude/commands/improve-tests.md b/.claude/commands/improve-tests.md deleted file mode 100644 index 9e2a5501..00000000 --- a/.claude/commands/improve-tests.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -description: Find and fix test overlap or testing theater -argument-hint: [test-file-or-directory] ---- - -Analyze the tests in $ARGUMENTS (or all tests if not specified) for: - -- Overlapping test coverage (multiple tests verifying the same behavior) -- Testing theater (tests that pass but don't actually verify behavior) -- Missing assertions or overly loose assertions -- Tests that mock too much and don't test real behavior - -Implement improvements if found. diff --git a/.claude/commands/review-branch.md b/.claude/commands/review-branch.md deleted file mode 100644 index 834b749d..00000000 --- a/.claude/commands/review-branch.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -description: Review all changes in current branch vs base -model: opus ---- - -Meticulously catalog and analyze all the changes in this branch, compared to the branch it's based on. - -Review against our documented rules in: -- docs/rules/architecture.md -- docs/rules/commands.md -- docs/rules/exceptions.md -- docs/rules/testing.md - -Focus on where changes fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. - -Provide a detailed report but don't make any changes yet. diff --git a/.claude/commands/review-diff.md b/.claude/commands/review-diff.md deleted file mode 100644 index 7ceec057..00000000 --- a/.claude/commands/review-diff.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -description: Review staged/unstaged changes in working tree ---- - -Meticulously catalog and analyze all the changes in this Git working tree, staged or unstaged. - -Review against our documented rules in: -- docs/rules/architecture.md -- docs/rules/commands.md -- docs/rules/exceptions.md -- docs/rules/testing.md - -Focus on where changes fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. - -Provide a detailed report but don't make any changes yet. diff --git a/.claude/commands/review-pr-comment.md b/.claude/commands/review-pr-comment.md deleted file mode 100644 index ba52f432..00000000 --- a/.claude/commands/review-pr-comment.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -description: Assess PR comment concerns and propose solutions -argument-hint: ---- - -Please assess whether the concerns raised in the following PR comment are valid, and propose possible solutions to address them. diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 33457c02..f9f63e40 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -9,7 +9,15 @@ "WebFetch(domain:github.com)", "Skill(ai-docs)", "Bash(git add:*)", - "Bash(git commit:*)" + "Bash(git commit:*)", + "Bash(git log:*)", + "Bash(git filter-branch:*)", + "Bash(FILTER_BRANCH_SQUELCH_WARNING=1 git filter-branch:*)", + "Bash(git stash:*)", + "Bash(git push:*)", + "Bash(git update-ref:*)", + "Bash(git merge-base:*)", + "Bash(git ls-tree:*)" ], "deny": [], "ask": [] diff --git a/.claude/skills/command/SKILL.md b/.claude/skills/command/SKILL.md index ef9f4f89..8a6f258c 100644 --- a/.claude/skills/command/SKILL.md +++ b/.claude/skills/command/SKILL.md @@ -9,9 +9,11 @@ Commands are Symfony Console classes that handle user I/O. They use Laravel Prom All rules MANDATORY. -## Core Principle +## Core Principles -Every command MUST be fully runnable non-interactively via CLI options. Every prompt MUST have a corresponding CLI option. +- Every command MUST be fully runnable non-interactively via CLI options +- Every prompt MUST have a corresponding CLI option +- Never invoke other commands (NO proxy commands) ## Required Structure @@ -50,9 +52,7 @@ final class ActionCommand extends BaseCommand protected function execute(InputInterface $input, OutputInterface $output): int { - $this->setupIo($input, $output); - - // Get input + // Get input (IOService is auto-initialized via BaseCommand::initialize()) $name = $this->getOptionOrPrompt( 'name', fn () => $this->promptText(label: 'Name:', required: true) @@ -318,6 +318,28 @@ return Command::SUCCESS; This outputs the equivalent non-interactive command for documentation/automation. +## Common Mistakes + +```php +// WRONG - Missing null check after validated prompt +$value = $this->getValidatedOptionOrPrompt(...); +$this->doSomething($value); // $value could be null! + +// CORRECT - Check for validation failure +$value = $this->getValidatedOptionOrPrompt(...); +if (null === $value) { + return Command::FAILURE; +} + +// WRONG - Silent fallback on explicit input +$path = $this->getOptionOrPrompt('key-path', ...); +$resolved = $this->resolveKey($path); // Falls back even if user's path invalid! + +// WRONG - CLI option bypasses validation +$env = $this->getOptionOrPrompt('env', fn() => $this->promptSelect(..., options: $valid)); +// --env=invalid passes through unvalidated! Use getValidatedOptionOrPrompt instead. +``` + ## Checklist Before completing a command: diff --git a/CLAUDE.md b/CLAUDE.md index 8fe4f2e0..f7b5f6e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ $container->bind(SSHService::class, $mockSSH); $command = $container->build(ServerAddCommand::class); ``` -**Integration:** Entry point: `bin/deployer`. Command registration: `SymfonyApp.php`. Services: Auto-injected via constructor. +**Integration:** Entry point: `bin/deployer`. Command registration: `app/SymfonyApp.php`. Services: Auto-injected via constructor. ## Layer Separation From ba2929bd1ef439fa31d10ab0503dc850bdd2786e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 5 Dec 2025 00:49:00 +0200 Subject: [PATCH 16/18] fix(commands): check for existing PR before creating new one --- .claude/commands/push.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/commands/push.md b/.claude/commands/push.md index 354d085c..33fd5255 100644 --- a/.claude/commands/push.md +++ b/.claude/commands/push.md @@ -10,7 +10,15 @@ A. Push the branch to GitHub Push the current branch to origin with tracking (-u flag). Do not force push. -B. Open a draft pull request +B. Check for existing pull request + +Use `gh pr list --head --json number,url` to check if a PR already exists for this branch. + +C. If PR exists: Output the existing PR URL + +If a PR already exists, simply output the PR URL and confirm that the pushed changes have been added to the existing PR. + +D. If no PR exists: Create a draft pull request Create a draft PR using `gh pr create --draft` with: From 06fde97ee16b8d427bf47868b54f82329a11fee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 5 Dec 2025 00:49:57 +0200 Subject: [PATCH 17/18] feat(commands): add /rereview command with peer-rereviewer agent Adds automated PR comment assessment workflow: - /rereview command fetches PR comments and delegates to peer-rereviewer agents - peer-rereviewer agent evaluates comment validity against project standards - Supports parallel agent spawning for efficient review - Filters out bot comments and auto-generated content - Updates settings.local.json with required gh and git permissions --- .claude/agents/peer-rereviewer.md | 98 +++++++++++++++++++++++++++++++ .claude/commands/rereview.md | 69 ++++++++++++++++++++++ .claude/settings.local.json | 5 +- 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 .claude/agents/peer-rereviewer.md create mode 100644 .claude/commands/rereview.md diff --git a/.claude/agents/peer-rereviewer.md b/.claude/agents/peer-rereviewer.md new file mode 100644 index 00000000..5af04881 --- /dev/null +++ b/.claude/agents/peer-rereviewer.md @@ -0,0 +1,98 @@ +--- +name: peer-rereviewer +description: Use this agent when you need to evaluate whether a PR comment or suggestion is valid, accurate, and worth implementing. This includes assessing technical correctness, relevance to the codebase, alignment with project standards, and proposing concrete solutions when comments are valid.\n\nExamples:\n\n\nContext: A reviewer has left a comment suggesting a code change.\nuser: "The reviewer says I should use dependency injection instead of creating objects directly in my ServerAddCommand class. Is this valid?"\nassistant: "I'll use the peer-rereviewer agent to assess this PR comment and determine if it's valid for your codebase."\n\nSince the user is asking about the validity of a PR comment regarding dependency injection, use the peer-rereviewer agent to analyze the suggestion against project standards and propose a solution if valid.\n\n\n\n\nContext: User received feedback on a pull request.\nuser: "Someone commented that my exception message is wrong because I'm prefixing it with 'Failed to:'. Should I change it?"\nassistant: "Let me use the peer-rereviewer agent to evaluate this feedback against the project's exception handling patterns."\n\nThe user needs validation of a PR comment about exception handling patterns. Use the peer-rereviewer agent to check if this aligns with CLAUDE.md guidelines about exception messages being complete and not prefixed.\n\n\n\n\nContext: User is reviewing feedback on their PHP code.\nuser: "A colleague suggested I remove the braces from my single-line if statement. Is that a good idea?"\nassistant: "I'll launch the peer-rereviewer agent to assess whether this suggestion aligns with the project's coding standards."\n\nThis is a code style PR comment that needs validation against project standards. The peer-rereviewer agent should check CLAUDE.md which explicitly requires braces on ALL control structures.\n\n +model: opus +color: blue +--- + +You are an expert code review analyst specializing in evaluating the validity and merit of PR comments. You possess deep knowledge of software engineering best practices, design patterns, and the ability to assess feedback objectively against project-specific standards and industry conventions. + +## Your Role + +You assess PR comments to determine: +1. Whether the comment is technically correct +2. Whether it aligns with project-specific standards (from CLAUDE.md or similar) +3. Whether implementing the suggestion would improve the code +4. What concrete solution should be implemented if the comment is valid + +## Assessment Framework + +For each PR comment, you will: + +### 1. Understand the Context +- Identify the specific code being reviewed +- Understand the reviewer's concern or suggestion +- Note any project-specific standards that apply + +### 2. Evaluate Technical Validity +- Is the reviewer's technical assessment correct? +- Are there edge cases the reviewer missed? +- Does the suggestion introduce new problems? + +### 3. Check Project Alignment +- Does the suggestion align with CLAUDE.md guidelines? +- Does it follow established patterns in the codebase? +- Would it maintain consistency with existing code? + +### 4. Assess Improvement Value +- Would the change improve readability? +- Would it improve maintainability? +- Would it improve performance (if relevant)? +- Is the effort proportional to the benefit? + +### 5. Deliver Your Verdict + +Provide a clear assessment with one of these verdicts: +- **VALID - Implement**: The comment is correct and should be addressed +- **VALID - Consider**: The comment has merit but implementation is optional +- **PARTIALLY VALID**: Some aspects are correct, others need adjustment +- **INVALID - Reject**: The comment is incorrect or conflicts with project standards +- **INVALID - Subjective**: The comment is a matter of preference with no clear benefit + +## Response Format + +Structure your response as: + +``` +## Assessment + +**Verdict:** [Your verdict] + +**Reasoning:** +[Explain why the comment is or isn't valid, referencing specific standards or best practices] + +**Project Standards Check:** +[Note any relevant CLAUDE.md or project-specific guidelines that apply] + +## Proposed Solution + +[If VALID: Provide the specific code changes needed] +[If INVALID: Explain why no change is needed and optionally suggest what the reviewer might have meant] +``` + +## Key Principles + +1. **Be Objective**: Evaluate comments on technical merit, not personal preference +2. **Cite Standards**: Reference specific guidelines from CLAUDE.md when applicable +3. **Provide Context**: Explain the reasoning behind your assessment +4. **Be Constructive**: Even when rejecting a comment, explain respectfully why +5. **Propose Solutions**: Always provide actionable next steps + +## Common Patterns to Check (PHP/Symfony Context) + +- Yoda conditions (literals on left side of comparisons) +- Braces on all control structures +- Dependency injection via container->build() +- Exception messages being complete and user-facing +- Service layer having no console I/O +- Command layer delegating business logic to services +- PSR-12 compliance and strict types + +## Quality Assurance + +Before finalizing your assessment: +- Have you read the actual code in question? +- Have you checked relevant project standards? +- Is your proposed solution syntactically correct? +- Does your solution follow all applicable guidelines? +- Have you considered edge cases in your solution? diff --git a/.claude/commands/rereview.md b/.claude/commands/rereview.md new file mode 100644 index 00000000..436821b3 --- /dev/null +++ b/.claude/commands/rereview.md @@ -0,0 +1,69 @@ +--- +description: Fetch PR comments and assess concerns +allowed-tools: Bash(gh:*), Bash(jq:*), Task +model: haiku +--- + +Fetch PR comments and delegate each to a peer-rereviewer agent for assessment. + +## Steps + +1. Get PR info using `gh pr view --json number,headRepository` +2. Get repository owner/name using `gh repo view --json nameWithOwner` +3. Fetch PR-level comments: `gh api /repos/{owner}/{repo}/issues/{number}/comments` +4. Fetch review comments: `gh api /repos/{owner}/{repo}/pulls/{number}/comments` +5. Filter out bot comments and auto-generated content (look for bot usernames, "[bot]" suffix, auto-generated summaries) +6. For each substantive comment, spawn a `peer-rereviewer` agent using the Task tool + +## Agent Delegation + +For each substantive comment, use the Task tool with `subagent_type: "peer-rereviewer"` and include: + +- The comment text +- The file path and line number (if available) +- The diff hunk (if available) +- The author's username + +**IMPORTANT:** Spawn ALL agents in parallel using a single message with multiple Task tool calls. + +Example prompt for each agent: +``` +Assess this PR comment: + +**Author:** @username +**File:** path/to/file.php:123 + +```diff +[diff_hunk] +``` + +**Comment:** +> [comment text] + +Read the relevant code file and CLAUDE.md, then provide your assessment. +``` + +## Output Format + +After all agents complete, summarize the results: + +### Summary + +- **Total comments:** X +- **Valid (implement):** X +- **Valid (consider):** X +- **Invalid:** X + +### Details + +For each agent result, include: +- File and line reference +- Verdict +- Brief recommendation + +## Guidelines + +- Skip bot comments (usernames ending in `[bot]`, containing "bot", or common CI bots) +- Skip auto-generated content (dependency updates, changelog entries) +- If no substantive comments found, report "No actionable comments found." +- Each agent runs independently - they will read code and CLAUDE.md themselves diff --git a/.claude/settings.local.json b/.claude/settings.local.json index f9f63e40..ce4fbc93 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -17,7 +17,10 @@ "Bash(git push:*)", "Bash(git update-ref:*)", "Bash(git merge-base:*)", - "Bash(git ls-tree:*)" + "Bash(git ls-tree:*)", + "Bash(gh repo view:*)", + "Bash(gh api:*)", + "Bash(git checkout:*)" ], "deny": [], "ask": [] From 54dc2dfc16eded74ea0b5921891c5c404d5a0a7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 5 Dec 2025 00:51:05 +0200 Subject: [PATCH 18/18] fix(commands): ensure /commit doesn't miss uncommitted files --- .claude/commands/commit.md | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md index f9bec29f..0cd0f1fd 100644 --- a/.claude/commands/commit.md +++ b/.claude/commands/commit.md @@ -4,9 +4,18 @@ allowed-tools: Bash(git:*) model: haiku --- -Based on the changes made to this repository: +## Workflow -A. If we're on the main branch, create a new branch with a suitable name +### Step 1: Identify ALL changes + +Run `git status` to see: +- Modified files (staged and unstaged) +- Untracked files +- Deleted files + +Read relevant files to understand what changed and group them logically. + +### Step 2: If on main branch, create a feature branch Create the branch only (no commits yet). Use Conventional Commit types as branch prefixes: feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert/. @@ -14,23 +23,33 @@ feat/, fix/, docs/, style/, refactor/, perf/, test/, build/, ci/, chore/, revert Keep the branch name short (≤ 50 chars) yet informative. Do not push, pull, or rebase. Examples: - - feat/parser-add-php-84-attributes - fix/ci-matrix-php-versions - chore/deps-bump-composer-installers-2-3 -B. Create one or more commits with suitable titles +### Step 3: Create commits for ALL changes -Use Conventional Commits to group related changes into cohesive commits (commits should be independently meaningful). +**IMPORTANT:** Create commits for ALL modified, untracked, and deleted files. Nothing should be left uncommitted. -Keep titles short (≤ 72 chars), imperative, no trailing period. Do not push, pull, or rebase. +Group related changes into cohesive commits (commits should be independently meaningful). -Examples: +Use Conventional Commits format: +- Keep titles short (≤ 72 chars), imperative, no trailing period +- Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:) +- Do NOT include any AI attribution, "Generated with", or "Co-Authored-By" lines +Examples: - feat(parser): add support for PHP 8.4 attributes - fix(ci): correct matrix PHP versions in build workflow - chore(deps): bump composer/installers to ^2.3 -Body (optional): explain motivation, context, and breaking changes (use BREAKING CHANGE:). +### Step 4: Verify everything is committed + +Run `git status` again to confirm: +- Working tree is clean +- No untracked files remain +- No modified files remain + +If anything is left uncommitted, create additional commits until working tree is clean. -Do NOT include any AI attribution, "Generated with", or "Co-Authored-By" lines in commit messages. +Do not push, pull, or rebase.