diff --git a/.claude/pr-review-process.md b/.claude/pr-review-process.md index 3edab105a..7c1902ad1 100644 --- a/.claude/pr-review-process.md +++ b/.claude/pr-review-process.md @@ -68,6 +68,7 @@ Check each changed file against project standards: - **No unnecessary imports** (self-namespace, unused) - **No hardcoded paths or environment-specific values** — use `PHP_BINARY`, env vars, etc. - **Tests exist** for new classes, bindings, and behavior +- **No tests read documentation** — reject any test that opens a docs page, README, `CLAUDE.md` or `.claude/` file (see `.claude/testing.md`) - **Module bindings** have corresponding tests in `ModuleBindingsTest.php` - **No `final` classes**, including exceptions (blocks Preferences extensibility — see CLAUDE.md) - **Type declarations** on all parameters, returns, properties @@ -251,6 +252,7 @@ Before merging any package PR: - [ ] `composer phpstan` reports zero errors - [ ] `gh pr checks ` green — `Tests`, `Lint`, `Static analysis` all passing, and `Integration` too (not yet a required check, but still blocking) - [ ] New code has corresponding tests +- [ ] No test reads documentation (docs pages, READMEs, `CLAUDE.md`, `.claude/`) - [ ] No hardcoded paths or environment-specific values - [ ] No `final` classes (including exceptions) - [ ] No defensive `instanceof` checks on container resolutions diff --git a/.claude/testing.md b/.claude/testing.md index 74397a90b..28a38a2d0 100644 --- a/.claude/testing.md +++ b/.claude/testing.md @@ -242,6 +242,17 @@ packages/ - **Feature tests** (`tests/Feature/`): Test integrated functionality, may touch multiple classes - **Browser tests** (`tests/Browser/`): End-to-end tests using Playwright (Pest 4) +## Tests Never Read Documentation (Hard Rule) + +No test may open, assert on, or depend on documentation: docs pages under `packages/docs-markdown/docs/`, package `README.md` files, `CLAUDE.md`, anything in `.claude/`, or `docs/DOCS-STANDARDS.md`. Docs explain how things work for humans and AI; they are not a spec for the implementation, and a test that greps them turns every wording change into a red build. + +- Do not write `ReadmeTest`, `DocsTest`, "documents X" or "README has section Y" tests. +- Do not check that a docs page or README exists, or that it links somewhere. +- Code that reads Markdown (e.g. `MarkdownRepository`) is tested against fixtures under `tests/Fixtures/`, never the real docs tree. +- Files the code itself ships or emits (templates, generated config, installed skill files, exception messages that contain a docs URL) are product, not docs, and may be tested. + +Docs accuracy is checked outside the test suite: `composer docs:lint` (run by the CI `Lint` job on every PR) fails when a PHP example in a README or docs page references a `Marko\...` class that does not exist; the `doc-updater` agent and PR review cover the rest. + ## Coverage Requirements - Minimum: 80% - All new code must have tests diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e19b364c1..21c0a8725 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: # Redis integration suites (packages/cache-redis, packages/ratelimiter) # skip without a server; this keeps them running on every build. redis: - image: redis:7-alpine + image: public.ecr.aws/docker/library/redis:7-alpine ports: - 6379:6379 options: >- @@ -72,6 +72,10 @@ jobs: - name: PHP CS Fixer run: vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.php --dry-run --diff + - name: Docs class references + if: ${{ !cancelled() }} + run: composer docs:lint + static: name: Static analysis runs-on: ubuntu-latest @@ -105,7 +109,7 @@ jobs: runs-on: ubuntu-latest services: postgres: - image: postgres:17 + image: public.ecr.aws/docker/library/postgres:17 env: POSTGRES_USER: marko POSTGRES_PASSWORD: marko @@ -118,7 +122,7 @@ jobs: --health-timeout 5s --health-retries 10 mysql: - image: mysql:8.4 + image: public.ecr.aws/docker/library/mysql:8.4 env: MYSQL_ROOT_PASSWORD: marko MYSQL_DATABASE: marko_test @@ -133,7 +137,7 @@ jobs: # stored as LONGTEXT) run only against MariaDB. 11.8 is the first LTS # with innodb_snapshot_isolation=ON by default. mariadb: - image: mariadb:11.8 + image: public.ecr.aws/docker/library/mariadb:11.8 env: MARIADB_ROOT_PASSWORD: marko MARIADB_DATABASE: marko_test @@ -147,7 +151,7 @@ jobs: # The oldest MariaDB LTS still supported. The MySQL driver suite runs # against it too, so the documented floor is tested. mariadb-10-11: - image: mariadb:10.11 + image: public.ecr.aws/docker/library/mariadb:10.11 env: MARIADB_ROOT_PASSWORD: marko MARIADB_DATABASE: marko_test @@ -159,7 +163,7 @@ jobs: --health-timeout 5s --health-retries 20 redis: - image: redis:7 + image: public.ecr.aws/docker/library/redis:7 ports: - 6379:6379 options: >- diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e8b3e3b46..f3bb18de6 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -17,7 +17,7 @@ jobs: # Redis integration suites (packages/cache-redis, packages/ratelimiter) # skip without a server; this keeps them running on every build. redis: - image: redis:7-alpine + image: public.ecr.aws/docker/library/redis:7-alpine ports: - 6379:6379 options: >- diff --git a/CLAUDE.md b/CLAUDE.md index 9e4ea7300..f77871534 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,10 @@ composer test:all # Static analysis — NOT part of composer test; must be zero errors composer phpstan -# Everything the CI gate runs (tests + lint + static analysis) +# Docs lint — fails on any Marko class in a README/docs PHP example that does not exist +composer docs:lint + +# Everything the CI gate runs (tests + lint + static analysis + docs lint) composer ci # Real-service suite: fixture app + pgsql/mysql drivers + live Redis (needs the @@ -51,6 +54,7 @@ Every PR is gated by the `CI` workflow on `Tests`, `Lint`, and `Static analysis` - **No final classes** - blocks Preferences (extensibility) - **readonly** - use when appropriate for immutability, not as blanket rule - **Type declarations** - required on all parameters, returns, properties +- **Tests never read documentation** - no test may open, assert on, or depend on a docs page, README, `CLAUDE.md`, `.claude/` file, or any other prose written for readers. Docs explain how things work; they are not a spec for the implementation. Test the code's behavior instead. Files the code itself ships or emits (templates, generated config, skill files a package installs) are product, not docs, and may be tested. ## Feature Development diff --git a/bin/docs-lint.php b/bin/docs-lint.php new file mode 100644 index 000000000..acea6e4dd --- /dev/null +++ b/bin/docs-lint.php @@ -0,0 +1,69 @@ +] + * + * --docs-root scans another directory laid out like packages/ (used by the + * scanner's own tests); classes still resolve against the real packages. + * + * Exits 1 on any unresolved class, or when no Markdown files are found. + */ + +use Marko\Tests\Support\DocsClassReference\DocsClassReferenceScanner; + +require_once dirname(__DIR__) . '/tests/Support/DocsClassReference/DocsClassReferenceScanner.php'; + +const ALLOWED_FICTIONAL_PREFIXES = []; + +$repositoryRoot = dirname(__DIR__); +$packagesRoot = $repositoryRoot . '/packages'; +$options = getopt('', ['docs-root:']); +$docsRoot = is_string($options['docs-root'] ?? null) ? rtrim($options['docs-root'], '/') : $packagesRoot; + +$files = DocsClassReferenceScanner::discoverMarkdownFiles($docsRoot); + +// The repository README sits one level above the packages directory. +if (is_file(dirname($docsRoot) . '/README.md')) { + $files[] = dirname($docsRoot) . '/README.md'; +} + +if ($files === []) { + fwrite(STDERR, "docs:lint found no Markdown files under $docsRoot. Refusing to pass an empty scan.\n"); + exit(1); +} + +$scanner = new DocsClassReferenceScanner($packagesRoot, ALLOWED_FICTIONAL_PREFIXES); +$unresolved = $scanner->findUnresolved($files); + +if ($unresolved !== []) { + foreach ($unresolved as $reference) { + fwrite(STDERR, sprintf( + "%s:%d references missing class %s\n", + str_replace($repositoryRoot . '/', '', $reference['file']), + $reference['line'], + $reference['class'], + )); + } + + fwrite(STDERR, sprintf( + "\ndocs:lint failed: %d missing class reference(s). Fix the docs, or the class name.\n", + count($unresolved), + )); + exit(1); +} + +echo sprintf("docs:lint passed: %d Markdown files, every Marko class reference resolves.\n", count($files)); diff --git a/composer.json b/composer.json index c055db95d..6f4d712f7 100644 --- a/composer.json +++ b/composer.json @@ -529,11 +529,13 @@ "phpcbf --standard=phpcs.xml || true" ], "phpstan": "php -d memory_limit=2G vendor/bin/phpstan analyse", + "docs:lint": "php bin/docs-lint.php", "ci": [ "@test", "@cs:check", "php-cs-fixer fix --config=.php-cs-fixer.php --dry-run --diff", - "@phpstan" + "@phpstan", + "@docs:lint" ], "rector": "rector process" }, diff --git a/packages/amphp/tests/PackageScaffoldingTest.php b/packages/amphp/tests/PackageScaffoldingTest.php index 3f0330295..f63d1001f 100644 --- a/packages/amphp/tests/PackageScaffoldingTest.php +++ b/packages/amphp/tests/PackageScaffoldingTest.php @@ -2,16 +2,6 @@ declare(strict_types=1); -it('creates README.md for marko/amphp with all required sections', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme) - ->toContain('## Overview') - ->and($readme)->toContain('## Installation') - ->and($readme)->toContain('## Usage') - ->and($readme)->toContain('## API Reference'); -}); - describe('Package Scaffolding', function (): void { it('has valid module.php for marko/amphp with empty bindings', function (): void { $modulePath = dirname(__DIR__) . '/module.php'; diff --git a/packages/authentication/tests/ReadmeTest.php b/packages/authentication/tests/ReadmeTest.php deleted file mode 100644 index 865ece584..000000000 --- a/packages/authentication/tests/ReadmeTest.php +++ /dev/null @@ -1,31 +0,0 @@ -toBeTrue(); -}); - -it('README starts with the package title', function () { - expect(authenticationReadme())->toStartWith('# marko/authentication'); -}); - -it('README includes installation instructions', function () { - expect(authenticationReadme())->toContain('## Installation') - ->toContain('composer require marko/authentication'); -}); - -it('README includes a quick example', function () { - expect(authenticationReadme())->toContain('## Quick Example') - ->toContain('AuthManager') - ->toContain('attempt('); -}); - -it('README links to the full documentation', function () { - expect(authenticationReadme())->toContain('https://marko.build/docs/packages/authentication/'); -}); diff --git a/packages/broadcasting-amphp/tests/ModuleTest.php b/packages/broadcasting-amphp/tests/ModuleTest.php index cf57316b3..6a78d8f2b 100644 --- a/packages/broadcasting-amphp/tests/ModuleTest.php +++ b/packages/broadcasting-amphp/tests/ModuleTest.php @@ -128,14 +128,4 @@ ->and($composer['autoload']['psr-4'])->toBe(['Marko\\Broadcasting\\Amphp\\' => 'src/']) ->and($composer['extra']['marko']['module'])->toBeTrue(); }); - - it('has a README with installation, quick example and docs link', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toStartWith("# marko/broadcasting-amphp\n") - ->toContain("## Installation\n") - ->toContain('composer require marko/broadcasting-amphp') - ->toContain("## Quick Example\n") - ->toContain('https://marko.build/docs/packages/broadcasting-amphp/'); - }); }); diff --git a/packages/claude-plugins/tests/Unit/MarketplaceTest.php b/packages/claude-plugins/tests/Unit/MarketplaceTest.php index c2d414b69..86f4128c8 100644 --- a/packages/claude-plugins/tests/Unit/MarketplaceTest.php +++ b/packages/claude-plugins/tests/Unit/MarketplaceTest.php @@ -72,14 +72,6 @@ function (): void { ); }); - it('architecture.md AI Development Tooling table includes a row for marko/claude-plugins', function (): void { - $archPath = dirname(__DIR__, 4) . '/.claude/architecture.md'; - $contents = file_get_contents($archPath); - - expect(str_contains($contents, 'marko/claude-plugins'))->toBeTrue() - ->and(str_contains($contents, 'AI Development Tooling'))->toBeTrue(); - }); - it( 'marketplace.json each plugin entry includes name, description, author, category fields per Task 001\'s required-fields finding', function (): void { diff --git a/packages/claude-plugins/tests/Unit/MarkoLspPluginTest.php b/packages/claude-plugins/tests/Unit/MarkoLspPluginTest.php index 827f5b9b9..6bbf3f2b0 100644 --- a/packages/claude-plugins/tests/Unit/MarkoLspPluginTest.php +++ b/packages/claude-plugins/tests/Unit/MarkoLspPluginTest.php @@ -8,7 +8,6 @@ $this->pluginJsonPath = $this->pluginRoot . '/.claude-plugin/plugin.json'; $this->lspJsonPath = $this->pluginRoot . '/.lsp.json'; $this->shimPath = $this->pluginRoot . '/bin/marko-lsp'; - $this->readmePath = $this->pluginRoot . '/README.md'; }); it( @@ -110,17 +109,4 @@ function (): void { expect(str_contains($contents, '>&2'))->toBeTrue() ->and(str_contains($contents, 'exit 1'))->toBeTrue(); }); - - it( - 'README.md explains marko-lsp coexists with php-lsp (intelephense), what marko-lsp adds beyond it, the recommendation to uninstall php-lsp@claude-plugins-official to avoid duplication, and how to verify with claude plugin list', - function (): void { - $contents = file_exists($this->readmePath) ? file_get_contents($this->readmePath) : ''; - - expect(file_exists($this->readmePath))->toBeTrue() - ->and(str_contains($contents, 'intelephense'))->toBeTrue() - ->and(str_contains($contents, 'php-lsp@claude-plugins-official'))->toBeTrue() - ->and(str_contains($contents, 'uninstall'))->toBeTrue() - ->and(str_contains($contents, 'claude plugin list'))->toBeTrue(); - }, - ); }); diff --git a/packages/claude-plugins/tests/Unit/MarkoMcpPluginTest.php b/packages/claude-plugins/tests/Unit/MarkoMcpPluginTest.php index 06c3f72c7..8c8878946 100644 --- a/packages/claude-plugins/tests/Unit/MarkoMcpPluginTest.php +++ b/packages/claude-plugins/tests/Unit/MarkoMcpPluginTest.php @@ -129,20 +129,4 @@ function (): void { ->and(str_contains($contents, 'exit 1'))->toBeTrue(); }, ); - - it( - 'README.md explains what the plugin registers, how to install via the marko marketplace, and how to verify with claude mcp list', - function (): void { - $readmePath = $this->pluginRoot . '/README.md'; - $contents = file_exists($readmePath) ? file_get_contents($readmePath) : ''; - - expect(file_exists($readmePath))->toBeTrue() - // What the plugin registers - ->and(str_contains($contents, 'mcp:serve') || str_contains($contents, 'MCP server'))->toBeTrue() - // How to install via marketplace - ->and(str_contains($contents, '/plugin install marko-mcp@marko'))->toBeTrue() - // How to verify - ->and(str_contains($contents, 'claude mcp list'))->toBeTrue(); - }, - ); }); diff --git a/packages/claude-plugins/tests/Unit/MarkoSkillsPluginTest.php b/packages/claude-plugins/tests/Unit/MarkoSkillsPluginTest.php index 82d2edacc..af2240dbf 100644 --- a/packages/claude-plugins/tests/Unit/MarkoSkillsPluginTest.php +++ b/packages/claude-plugins/tests/Unit/MarkoSkillsPluginTest.php @@ -45,19 +45,4 @@ function (): void { expect(is_dir($this->pluginRoot . '/skills'))->toBeTrue() ->and(is_dir($this->pluginRoot . '/.claude-plugin/skills'))->toBeFalse(); }); - - it( - 'README.md lists the included skills (create-module, create-plugin), how they\'re invoked with the plugin namespace, and how to install via the marko marketplace', - function (): void { - $readmePath = $this->pluginRoot . '/README.md'; - $content = file_exists($readmePath) ? file_get_contents($readmePath) : ''; - - expect(file_exists($readmePath))->toBeTrue() - ->and($content)->toContain('create-module') - ->and($content)->toContain('create-plugin') - ->and($content)->toContain('/marko-skills:create-module') - ->and($content)->toContain('/marko-skills:create-plugin') - ->and($content)->toContain('/plugin install marko-skills@marko'); - }, - ); }); diff --git a/packages/claude-plugins/tests/Unit/ReadmeTest.php b/packages/claude-plugins/tests/Unit/ReadmeTest.php deleted file mode 100644 index 9798241eb..000000000 --- a/packages/claude-plugins/tests/Unit/ReadmeTest.php +++ /dev/null @@ -1,56 +0,0 @@ -readmePath = dirname(__DIR__, 2) . '/README.md'; - $this->contents = file_exists($this->readmePath) - ? file_get_contents($this->readmePath) - : null; - }); - - it('README.md starts with the package title (# marko/claude-plugins or similar)', function (): void { - expect($this->contents)->not->toBeNull() - ->and($this->contents)->toStartWith('# marko/claude-plugins'); - }); - - it('README.md includes a one-paragraph description of what the package provides', function (): void { - expect($this->contents)->not->toBeNull() - ->and($this->contents)->toContain('Claude Code') - ->and($this->contents)->toContain('marketplace'); - }); - - it( - 'README.md lists all three plugins (marko-skills, marko-lsp, marko-mcp) with one-line descriptions', - function (): void { - expect($this->contents)->not->toBeNull() - ->and($this->contents)->toContain('marko-skills') - ->and($this->contents)->toContain('marko-lsp') - ->and($this->contents)->toContain('marko-mcp'); - }, - ); - - it('README.md includes the install command via marko devai:install', function (): void { - expect($this->contents)->not->toBeNull() - ->and($this->contents)->toContain('devai:install'); - }); - - it('README.md links to the full docs page in the docs site', function (): void { - expect($this->contents)->not->toBeNull() - ->and($this->contents)->toContain('https://marko.build/docs/ai-assisted-development/'); - }); - - it( - 'README.md follows the slim-pointer convention from docs/DOCS-STANDARDS.md (no exhaustive feature lists, no inline code samples beyond a quick install example)', - function (): void { - expect($this->contents)->not->toBeNull(); - - // Slim-pointer: file should be short (under 80 lines) - $lines = substr_count($this->contents, "\n") + 1; - expect($lines)->toBeLessThan(80) - // Must have a Documentation section - ->and($this->contents)->toContain('## Documentation'); - }, - ); -}); diff --git a/packages/clock/tests/PackageStructureTest.php b/packages/clock/tests/PackageStructureTest.php index 52314b6be..6fad7f525 100644 --- a/packages/clock/tests/PackageStructureTest.php +++ b/packages/clock/tests/PackageStructureTest.php @@ -30,10 +30,3 @@ it('is marked as a marko module', function () use ($composer): void { expect($composer['extra']['marko']['module'])->toBeTrue(); }); - -it('ships a README pointing to the docs page', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('composer require marko/clock') - ->and($readme)->toContain('https://marko.build/docs/packages/clock/'); -}); diff --git a/packages/database/tests/PackageScaffoldingTest.php b/packages/database/tests/PackageScaffoldingTest.php index 5ddb6e0a1..0b35447fc 100644 --- a/packages/database/tests/PackageScaffoldingTest.php +++ b/packages/database/tests/PackageScaffoldingTest.php @@ -98,59 +98,4 @@ expect($pgsqlComposer['autoload']['psr-4'])->toHaveKey('Marko\\Database\\PgSql\\') ->and($pgsqlComposer['autoload']['psr-4']['Marko\\Database\\PgSql\\'])->toBe('src/'); }); - - it( - 'creates slim README.md for marko/database per docs standards', - function (): void { - $readmePath = dirname(__DIR__) . '/README.md'; - - expect(file_exists($readmePath))->toBeTrue(); - - $content = file_get_contents($readmePath); - - expect($content)->toContain('marko/database') - ->and($content)->toContain('composer require marko/database') - ->and($content)->toContain('#[Column') - ->and($content)->toContain('Repository'); - }, - ); - - it('creates README.md for marko/database-mysql with installation and configuration', function (): void { - $readmePath = dirname(__DIR__, 2) . '/database-mysql/README.md'; - - expect(file_exists($readmePath))->toBeTrue(); - - $content = file_get_contents($readmePath); - - // Installation instructions - expect($content)->toContain('composer require marko/database-mysql') - // Configuration - ->and($content)->toContain('config/database.php') - ->and($content)->toContain('DB_HOST') - ->and($content)->toContain('DB_DATABASE') - ->and($content)->toContain('DB_USERNAME') - ->and($content)->toContain('DB_PASSWORD') - // Driver-specific notes - ->and($content)->toContain('MySQL') - ->and($content)->toContain('MariaDB'); - }); - - it('creates README.md for marko/database-pgsql with installation and configuration', function (): void { - $readmePath = dirname(__DIR__, 2) . '/database-pgsql/README.md'; - - expect(file_exists($readmePath))->toBeTrue(); - - $content = file_get_contents($readmePath); - - // Installation instructions - expect($content)->toContain('composer require marko/database-pgsql') - // Configuration - ->and($content)->toContain('config/database.php') - ->and($content)->toContain('DB_HOST') - ->and($content)->toContain('DB_DATABASE') - ->and($content)->toContain('DB_USERNAME') - ->and($content)->toContain('DB_PASSWORD') - // Driver-specific notes - ->and($content)->toContain('PostgreSQL'); - }); }); diff --git a/packages/devai/tests/Unit/DocsPageTest.php b/packages/devai/tests/Unit/DocsPageTest.php deleted file mode 100644 index 7897a431d..000000000 --- a/packages/devai/tests/Unit/DocsPageTest.php +++ /dev/null @@ -1,54 +0,0 @@ -toContain('') - ->and($content)->toContain(''); -}); - -it('states that content outside markers is never modified', function () use ($docsPage): void { - $content = (string) file_get_contents($docsPage); - - expect($content)->toContain('outside') - ->and( - str_contains($content, 'never modified') - || str_contains($content, 'never touched') - || str_contains($content, 'never change'), - )->toBeTrue(); -}); - -it('states that removing markers makes devai stop managing the file', function () use ($docsPage): void { - $content = (string) file_get_contents($docsPage); - - expect( - str_contains($content, 'removing the markers') - || str_contains($content, 'remove the markers') - || str_contains($content, 'Removing the markers') - || str_contains($content, 'Remove the markers'), - )->toBeTrue() - ->and( - str_contains($content, 'stop managing') - || str_contains($content, 'stops managing') - || str_contains($content, 'back off') - || str_contains($content, 'backs off') - || str_contains($content, 'full ownership'), - )->toBeTrue(); -}); - -it('keeps the README a slim pointer per docs standards', function () use ($readmePath): void { - $content = (string) file_get_contents($readmePath); - - // Must have the required slim-pointer sections - expect($content)->toContain('## Installation') - ->and($content)->toContain('## Documentation') - ->and($content)->toContain('marko.build/docs/packages/devai'); - - // Must NOT be bloated with override-model detail (that belongs in the docs page) - expect(str_contains($content, ''))->toBeFalse(); -}); diff --git a/packages/devai/tests/Unit/ReadmeStandardsTest.php b/packages/devai/tests/Unit/ReadmeStandardsTest.php deleted file mode 100644 index 6fa919cf1..000000000 --- a/packages/devai/tests/Unit/ReadmeStandardsTest.php +++ /dev/null @@ -1,22 +0,0 @@ -toBeTrue("README.md missing for $pkg"); - $content = (string) file_get_contents($readme); - foreach ($required as $section) { - expect(str_contains($content, $section))->toBeTrue("README.md for $pkg missing $section"); - } - } -}); diff --git a/packages/devserver/tests/PackageStructureTest.php b/packages/devserver/tests/PackageStructureTest.php index 6da07f4e7..28439aed5 100644 --- a/packages/devserver/tests/PackageStructureTest.php +++ b/packages/devserver/tests/PackageStructureTest.php @@ -74,40 +74,3 @@ ->and($config['docker'])->toBeTrue() ->and($config['frontend'])->toBeTrue(); }); - -it('has README.md with required sections', function (): void { - $readmePath = __DIR__ . '/../README.md'; - - expect(file_exists($readmePath))->toBeTrue(); - - $content = file_get_contents($readmePath); - - expect($content) - ->toContain('# marko/devserver') - ->toContain('## Installation') - ->toContain('## Documentation'); -}); - -it('documents installation via Composer', function (): void { - $content = file_get_contents(__DIR__ . '/../README.md'); - - expect($content)->toContain('composer require marko/devserver'); -}); - -it('documents up, down, and marko status commands', function (): void { - $content = file_get_contents(__DIR__ . '/../README.md'); - - expect($content) - ->toContain('marko up') - ->toContain('marko down') - ->toContain('marko status'); -}); - -it('documents key commands in quick example', function (): void { - $content = file_get_contents(__DIR__ . '/../README.md'); - - expect($content) - ->toContain('marko up') - ->toContain('marko down') - ->toContain('marko status'); -}); diff --git a/packages/devserver/tests/Unit/RenameReferencesTest.php b/packages/devserver/tests/Unit/RenameReferencesTest.php index c4204cfcc..c97a0891b 100644 --- a/packages/devserver/tests/Unit/RenameReferencesTest.php +++ b/packages/devserver/tests/Unit/RenameReferencesTest.php @@ -23,18 +23,6 @@ expect($hits)->toBeEmpty('These files still reference marko/dev-server: ' . implode(', ', $hits)); }); -it('updates .claude/architecture.md package inventory to reference marko/devserver', function (): void { - $monorepoRoot = dirname(__DIR__, 4); - $architectureFile = $monorepoRoot . '/.claude/architecture.md'; - - expect(file_exists($architectureFile))->toBeTrue(); - - $content = file_get_contents($architectureFile); - - expect($content)->not->toContain('marko/dev-server') - ->and($content)->not->toContain('packages/dev-server'); -}); - it('runs the full test suite clean after the rename propagates', function (): void { $monorepoRoot = dirname(__DIR__, 4); diff --git a/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md b/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md deleted file mode 100644 index dc1d86405..000000000 --- a/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md +++ /dev/null @@ -1,237 +0,0 @@ ---- -title: RoadRunner state-leak audit -description: Mechanical audit of every source of cross-request state in the monorepo, and the verdict on whether it leaks under a long-running RoadRunner worker. ---- - -Task 005's discovery spike for `marko/roadrunner`. A long-running worker keeps -one PHP process alive across many requests, so anything that survives a -request without being explicitly cleared is a candidate for leaking one -user's state into the next user's response. This document is the mechanical -audit trail: every container singleton, boot-time `instance()` binding, -mutable class static, superglobal reader and process-global PHP setting in -the monorepo, each given an explicit **Leaks: Yes** or **Leaks: No** verdict. - -**Method.** `Marko\Roadrunner\Tests\Support\InProcessRequestHarness` (task -004a) boots one `Application` against a fixture project wiring -`marko/config`, `marko/session`, `marko/session-file` and -`marko/authentication`, then drives sequential `Request` objects through -`$app->router->handle()` with interleaved identities — authenticated, -anonymous, a different session — and several hundred requests for the memory -curve. See `packages/roadrunner/tests/StateLeakSpikeTest.php`. The -harness's opt-in `reset()` delegates to `Marko\Core\RequestStateResetter`, -the same resetter the worker calls before each request, so it resets the -same instances in the same (sorted by binding id) order. Services from -packages the fixture does not wire (debugbar, inertia, authorization, ...) -are audited by reading the source directly; that is called out per item -below, since it could not be exercised through a live request cycle in this -spike. - -**Scope note.** `Session` and `SessionGuard` were already made -request-scoped and given `ResettableInterface` implementations in #150 task -009. They are listed below for completeness, not because this spike -discovered them. - -Task 006 reads this document as the sole source of truth for what to wire -into the worker's per-request reset. It should wire every `Leaks: Yes` item -below and nothing else. - -## 1. Container singletons (`singletons` in `module.php`) - -Seventeen `module.php` files across the monorepo declare a `singletons` key. -For each, every mutable instance property on the declared class(es) is -listed with a verdict on whether it is request-derived. - -| Package | Service | Leaks: | Mechanism / verdict | -| --- | --- | --- | --- | -| `marko/session-file` | `SessionInterface` → `Session` | **Leaks: No** (already fixed) | `Session` implements `ResettableInterface`; `reset()` clears `$id`, `$data`, `$flashBag`. Confirmed via harness: interleaved authenticated → anonymous → different-session requests never see a prior session's `visits` count once `reset()` runs between requests. Fixed in #150 task 009, not by this spike. | -| `marko/session-database` | `SessionInterface` → `Session` | **Leaks: No** (already fixed) | Same class as `session-file`; same verdict. Source read, not re-exercised (fixture only wires the file driver). | -| `marko/admin` | `AdminSectionRegistryInterface` → `AdminSectionRegistry` | **Leaks: No** | `$sections` maps section ids to an `AdminSectionDefinition` until the section is first used, then to the built `AdminSectionInterface` instance. `register()` and `registerDefinition()` add entries, and marko/admin's boot callback calls `registerDefinition()` once per worker for every `#[AdminSection]` class without building any. A duplicate id throws `AdminException`, so a second boot cannot add sections to the same registry. The only request-time write is `get()` (also reached through `all()`) replacing a definition with the instance it builds through the container; the entry, its id and its class never change, and a failed build stores nothing. Once built, a section instance lives as long as the worker, so a section must not keep request or user data in its properties (documented on the marko/admin page). The shipped packages declare no sections. Source read only. | -| `marko/admin` | `DiscoveredAdminSections` | **Leaks: No** | `$definitions` is a lazily filled list of `AdminSectionDefinition` value objects (class name, id, label, icon, sort order, permission id/label pairs). It is built from the discovery cache or a scan of the module `src/` directories, which is class metadata, never request data. Boot fills it once and it never changes after that. Source read only. | -| `marko/admin-auth` | `PermissionRegistryInterface` → `PermissionRegistry` | **Leaks: No** | `$permissions` maps permission keys to `RegisteredPermission` value objects. Only `register()` writes to it, and it throws `AdminAuthException` on a duplicate key. marko/admin-auth's boot callback registers every `#[AdminPermission]` on the discovered `#[AdminSection]` classes, and an app may register more from its own `boot` callback. Both run once per worker and describe the application's code, not a request or user. No request writes to it: `matches()` and the other readers only read. Persisting permissions to the database is the explicit `admin-auth:permissions:sync` command, never boot. Source read only. | -| `marko/authentication` | `AuthManager` | **Leaks: No** (resettable) | `$guards` is a `array` cache of already-constructed guard instances keyed by guard *name* ("web", "api", ...), not by request or user. The guard instances it caches carry per-request identity, but they are built with `new`, so only the default guard, and only once something resolves `GuardInterface::class` from the container, is reachable through `Container::resolvedInstances(ResettableInterface::class)`. A guard used only through `$authManager->guard()` (the default one included) kept the previous request's user. The integration suite caught this (#226); `AuthManager` now implements `ResettableInterface` and its `reset()` resets every cached guard that is resettable, keeping the guard instances themselves. | -| `marko/authentication` | `GuardInterface` → `SessionGuard` | **Leaks: No** (already fixed) | `SessionGuard` implements `ResettableInterface`. Confirmed via harness: interleaved authenticated → anonymous → different-session requests never see the previous request's `user=` id once `reset()` runs between requests — proven directly against the currently logged-in fixture user. Fixed in #150 task 009. | -| `marko/authentication` | `RequestCookieJar` (also bound as `CookieJarInterface`) | **Leaks: No** (resettable) | Holds the current `Request` plus the cookies queued for its response. `QueuedCookiesMiddleware` replaces the request at the start of every request and pulls (empties) the queue before returning the response, and `RequestCookieJar` implements `ResettableInterface`, so `reset()` drops the request and any queue left behind by a request that threw before the middleware could flush it. | -| `marko/authentication` | `RememberTokenManager` | **Leaks: No** | Only the configured remember lifetime (an `int` set at construction) and the shared `ClockInterface` singleton, which holds no request data. Token generation, hashing and validation are pure. | -| `marko/authentication` | `GuardDriverRegistry` | **Leaks: No** | `$factories` maps guard driver names to factory closures, written only by `extend()` calls from module `boot` callbacks. No request writes to it, and it holds no request or user data. | -| `marko/authentication` | `CurrentRequest` | **Leaks: No** (resettable) | Holds the current `Request` for the login throttle. `QueuedCookiesMiddleware` replaces it at the start of every request, and `reset()` drops it. | -| `marko/authentication` | `LoginThrottleInterface` (`LoginThrottle` or `NullLoginThrottle`) | **Leaks: No** | Readonly: configured limits plus the shared cache, clock and `CurrentRequest` singletons. Failure counts and lockouts live in the cache by design, keyed by login identifier and client, so they persist across requests on purpose. | -| `marko/authentication-token` | `CurrentRequest` | **Leaks: No** (resettable) | Holds the `Request` being handled so the token guard can read its `Authorization` header. `TokenRequestMiddleware` sets it at the start of every request and clears it in a `finally` block, and `CurrentRequest` implements `ResettableInterface`. The token guard that `AuthManager` caches remembers which `Request` instance it resolved a token for and re-resolves as soon as `CurrentRequest` holds a different one, so a cached token guard never serves an earlier request's user. The token guard also implements `ResettableInterface`, and `AuthManager::reset()` resets it between requests, so it does not keep the previous request or its token alive. | -| `marko/authorization` | `PolicyRegistry` | **Leaks: No** | `$policies` is `array`, populated only by `register()` calls at module-boot time (throws `PolicyException` on duplicate registration — a boot-time invariant, not a per-request write path). No request ever calls `register()`. Source read only; `marko/authorization` is not wired into the fixture app. | -| `marko/authorization` | `GateInterface` → `Gate` | **Leaks: No** | `Gate` is constructed fresh from `AuthManager`/`PolicyRegistry` inside the binding closure and holds no declared mutable properties beyond its constructor-injected collaborators (`guard`, `policyRegistry`), both already covered. Source read only. | -| `marko/authorization` | `AuthorizationMiddleware` | **Leaks: No** | Its only mutable property, `$resolved`, caches the `#[Can]` attribute per `controller::action` key. It holds immutable attribute data derived from class code, never request or user data, so every request sees the same values. `$resolvedGate` and `$resolvedGuard` memoize the lazily built `Gate` and guard on the first `#[Can]` route. They hold the same instances `AuthManager` caches, not request or user data, and the guards themselves are covered above (the token guard re-resolves per `CurrentRequest`). Source read only. | -| `marko/codeindexer` | `IndexCache` | **Leaks: No** | `$data` is a lazily-built, file-backed index of the *codebase* (modules, routes, config keys, ...), invalidated by comparing file mtimes against a persisted `trackedPaths` set — nothing here is derived from an HTTP request. Used by MCP/LSP tooling, not the request/response cycle. Source read only. | -| `marko/codeindexer` | `ModuleWalker` | **Leaks: No** | Holds only `readonly string $rootPath`. No mutable state. | -| `marko/codeindexer` test fixture | `FooBarService` (`vendor/foo/bar/module.php`) | **Leaks: No** | A fictional class name used only to exercise `codeindexer`'s own singleton-parsing tests; never instantiated, never installed in a real app. Out of scope by construction. | -| `marko/database` | `EntityMetadataFactory` | **Leaks: No** | `$cache` is `array`, keyed and populated from static class reflection (`linkExtendersFrom()` at module `boot`), not from request data. Source read only; `marko/database` is not wired into the fixture app. | -| `marko/database` | `EntityHydrator` | **Leaks: No** | Shared since #159 so the dirty-check snapshot taken when one repository loads an entity is still there when another repository saves it. `$originalValues` and `$originalDatabaseValues` (the cast/encrypted column snapshot) are `WeakMap`s keyed by entity objects, so a snapshot is dropped as soon as the request that loaded the entity releases it; nothing request-derived outlives the entity itself. Its `CastResolver` caches one instance per cast class for the worker's lifetime, so a custom `CastInterface` implementation must be stateless (the built-in casts are). | -| `marko/database-mysql` | `ConnectionInterface` → `MySqlConnection` | **Leaks: No** (fixed in #159) | Shared since #159 so every repository, query builder and `TransactionInterface` consumer uses one PDO handle. Keeping that handle open across requests is the intended connection reuse. `MySqlConnection` implements `ResettableInterface`: `reset()` rolls back a transaction a thrown request left open, without opening a connection when none exists. | -| `marko/database-mysql` | `MySqlServer` | **Leaks: No** | Owned by the shared `MySqlConnection` (`server()`) and shared through the container. `$version` caches the `SELECT VERSION()` answer (MySQL or MariaDB, and the version number) the first time SQL that differs between the servers is compiled or `supportsReturning()` is asked; the connection keeps it across requests, `disconnect()` and `reset()`. It describes the database server, never a request or user, so every request sees the same value. A worker keeps it until it restarts, so restart workers after moving the database to a different server type. Source read only. | -| `marko/database-pgsql` | `ConnectionInterface` → `PgSqlConnection` | **Leaks: No** (fixed in #159) | Same shape and verdict as `MySqlConnection` above: one shared PDO handle per worker, and `reset()` rolls back an abandoned transaction. | -| `marko/debugbar` | `Debugbar` | **Leaks: Yes** | `$messages`, `$openMeasures`, `$measures`, `$queries`, `$logs`, `$viewRenders` all accumulate via `record*()` calls made during request handling and are **never cleared**. Since `Debugbar` is a container singleton, every one of these arrays grows without bound for the life of the worker process across every request that touches a collector. Also: `boot()` is guarded by `$booted` so `ob_start()` (see §5) only runs once — but that single, never-closed output buffer then captures output from *every subsequent request* in the worker, not just the one active when `boot()` ran. Already flagged as worker-incompatible by `Marko\Roadrunner\GuardRails\UnsafePackageChecker::warnDebugbar()` (task 004): "a dev-only tool ... does not fit a long-running worker cleanly." Source read only — `marko/debugbar` is not wired into the fixture app and the guard rail already refuses to make it safe, only to warn. **Reset requirement**: not resettable in the `ResettableInterface` sense (the ob_start() buffer problem is architectural, not a per-request state problem) — do not enable `marko/debugbar` in a worker-served production environment, per the existing guard rail. | -| `marko/debugbar` | `DebugbarStorage` | **Leaks: No** | Only `readonly` constructor-injected collaborators (`ConfigRepositoryInterface`, `ProjectPaths`); all actual state lives in files on disk (`put()`/`get()`/`all()`/`clear()` are pure I/O), not in instance properties. | -| `marko/debugbar` | `DatabaseConnectionPlugin` | **Leaks: Yes** | `$started` is `array>` keyed by `type . ':' . md5($sql)`, pushed in `beforeQuery()`/`beforeExecute()` and popped in `afterQuery()`/`afterExecute()`. If a request throws (or the query itself throws) between the `#[Before]` and `#[After]` hooks, the pushed timestamp is never popped and the entry stays in `$started` forever — both an unbounded memory leak (one stale array entry per aborted query, keyed by SQL content so it recurs for any repeated query text) and a correctness bug: a *later*, unrelated request running the same SQL text will `array_pop()` the stale timestamp left by the aborted request and record a nonsensical (too-long or negative-looking) duration for its own query. Source read only. **Reset requirement**: clear `$started` between requests, or key entries by a per-request correlation id instead of SQL content so a leftover entry cannot corrupt a different request's timing. | -| `marko/debugbar` | `LoggerPlugin` | **Leaks: No** | Only `readonly Debugbar $debugbar` — no mutable state beyond the `Debugbar` collaborator already listed above. | -| `marko/debugbar` | `ViewPlugin` | **Leaks: Yes** | Same `$started` push/pop-by-content-hash pattern as `DatabaseConnectionPlugin`, same failure mode on a thrown render, same reset requirement. | -| `marko/docs-fts` | `DocsSearchInterface` → `FtsSearch` | **Leaks: No** | `$pdo` is a lazily-opened, read-only connection to a static on-disk SQLite index (`resources/docs.sqlite`), not request-derived — holding it open across requests is the intended behaviour (a connection pool of one), not a leak of request data. | -| `marko/docs-markdown` | `MarkdownRepository` | **Leaks: No** | Only `readonly string $docsPath`. Docs content is read from disk per call, no cached request-derived state. | -| `marko/docs` | *(no singleton classes declared — `singletons => []`)* | **Leaks: No** | Nothing to audit. | -| `marko/inertia` | `Inertia` | **Leaks: Yes** | `$shared` is `array`, written by the public `share()` API and merged into every subsequent `render()` call's props — but `Inertia` is a container singleton and nothing ever clears `$shared` between requests. A typical usage pattern (share the current authenticated user or flash data once per request, e.g. from middleware) would leave that data visible to every following request's Inertia response until the same key is overwritten, and if a later request shares a *different* key, both keys accumulate indefinitely. This is a genuine cross-request/cross-user data leak, not just a memory leak. Source read only — `marko/inertia` is not wired into the fixture app. **Reset requirement**: implement `ResettableInterface`, clearing `$shared` — the same shape as the `Session`/`SessionGuard` fix in #150 task 009. | -| `marko/inertia` | `SsrClient` | **Leaks: No** | `readonly class`; `render()` takes the page payload as a parameter and returns a result, storing nothing between calls. | -| `marko/layout` | `HandleResolver` | **Leaks: No** | No declared properties at all in the class body. | -| `marko/layout` | `LayoutResolver` | **Leaks: No** | No declared properties at all in the class body. | -| `marko/lsp` | `LspServer` | **Leaks: No** *(out of scope)* | `$initialized`/`$shuttingDown` track a stdio LSP server's own lifecycle for editor integrations — this singleton lives in a separate long-running process from any RoadRunner HTTP worker and never participates in the request/response cycle this package serves. Not applicable. | -| `marko/mcp` | *(no singleton classes declared — `singletons => []`)* | **Leaks: No** | Nothing to audit. `McpServer` itself is bound via a plain (non-singleton) closure. | -| `marko/devai` | *(no singleton classes declared — `singletons => []`)* | **Leaks: No** *(out of scope)* | CLI-only tooling (guideline generation), never boots inside an HTTP request cycle. | -| `marko/vite` | `Vite` | **Leaks: No** | Only non-promoted-readonly constructor collaborators (`ConfigRepositoryInterface`, `ProjectPaths`); asset-manifest reads are pure and re-read from disk per call, nothing cached in an instance property. | - -## 2. Boot-time `Container::instance()` bindings - -Registered once during `Application::initialize()` (or a module's `boot` -callback) rather than via the `singletons` array, but singletons in -practice — nothing ever re-registers them. - -| Binding | Registered by | Leaks: | Verdict | -| --- | --- | --- | --- | -| `ContainerInterface` | `Application::initialize()` | **Leaks: No** | The container itself; already audited above (§3 in the source, `$resolving` is a call-scoped guard cleared in a `finally`, `$instances`/`$bindings`/`$shared` are boot-time-populated and by-design persistent — see "Container resolution" below). | -| `PluginInterceptor` | `Application::initialize()` | **Leaks: No** | Wraps class generation for plugin interception; holds no per-request state (verified: constructed once from `PluginRegistry`, which is itself populated only at boot). | -| `PluginRegistry` | `Application::initialize()` | **Leaks: No** | Populated once during module registration at boot; never mutated during request handling. | -| `ProjectPaths` | `Application::initialize()` | **Leaks: No** | `readonly`, derived from the boot-time base path. | -| `EventDispatcherInterface` | `Application::initialize()` | **Leaks: No** *(not exercised — no listener state observed in this spike; the dispatcher only holds boot-time-registered observer maps, not request data)* | Registered once; observers are registered at boot from `ObserverDefinition`s, not per request. | -| `ModuleRepositoryInterface` | `Application::initialize()` | **Leaks: No** | Backed by the boot-time module list; read-only after boot. | -| `CommandRegistry` | `Application::initialize()` (registered twice under the same key, once per discovery fork) | **Leaks: No** | CLI command definitions, populated at boot; not touched by HTTP request handling. | -| `RouteCollection` | `RoutingBootstrapper::boot()` (line ~58) | **Leaks: No — confirmed via harness** | `$routes` is populated once by `discoverRoutes()` during `boot()` and never written to again; `add()` is only ever called from that one boot-time pass. The harness drove hundreds of requests across many routes with no route-table mutation. | -| `RouteMatcherInterface` → `RouteMatcher` | `RoutingBootstrapper::boot()` (line ~63) | **Leaks: No — confirmed via harness** | Wraps the immutable `RouteCollection` above. `match()` memoizes dynamic-route hits only (never misses) and clears the memo when it reaches `RouteMatcher::MAX_MEMO_ENTRIES` (1,000), so unique 404 paths from scanners or random IDs cannot grow it for the life of the worker. The memo holds only route definitions and URL parameters, never request data from another user. | -| `Router` | `RoutingBootstrapper::boot()` (line ~67) | **Leaks: No — confirmed via harness** | `readonly class`; `handle()` resolves a fresh controller instance via `$this->container->get($matched->route->controller)` on every call (confirmed directly: two calls to `container()->get(DemoController::class)` in the same booted app return `!==` instances, since `DemoController` is not declared a singleton). Nothing about handling one request's controller is retained on `Router` itself. | -| `ConnectionInterface` → `ReadWriteConnection` | `database-readwrite/module.php:44` (boot callback, only when `database.driver === 'readwrite'`) | **Leaks: Yes (uncommitted transactions), No (sticky-write flag, already fixed)** | See §5 "Open database transactions" below — the `$stickyWrite` flag is already covered by `reset()` (#150 task 009), but `reset()` does **not** roll back a transaction left open by `beginTransaction()` when a request throws before `commit()`/`rollback()`. Source read only — `marko/database-readwrite` is not wired into the fixture app. | -| `TransactionInterface` → `ReadWriteConnection` | `database-readwrite/module.php:45` | *(same instance as above)* | Same verdict as `ConnectionInterface` above — one object registered under two interface keys. | - -## 3. Mutable class statics - -Four verified across `packages/*/src`. Only two are runtime concerns for a -RoadRunner worker; the other two never execute inside a served HTTP request. - -| Static | Leaks: | Verdict | -| --- | --- | --- | -| `Marko\Debugbar\Debugbar::$current` | **Leaks: Yes** | Set in the constructor (`self::$current = $this`), read via `Debugbar::current()`/cleared via `Debugbar::forgetCurrent()`. Since `Debugbar` is only ever constructed once (a container singleton), this static simply mirrors that one instance's already-confirmed leak (§1) — nothing additional to reset here beyond resetting `Debugbar` itself, but `forgetCurrent()` exists and should be considered if `Debugbar` is ever rebuilt mid-process. | -| `Marko\DevAi\Writing\GuidelinesWriter::$notices` | **Leaks: No** *(out of scope)* | CLI-only (`devai:update` guideline generation); never runs inside a served HTTP request. | -| `Marko\Testing\TestCase::$registeredRoots` | **Leaks: No** *(out of scope)* | Test-suite bookkeeping only; irrelevant to a production worker process. | -| `Marko\Database\Entity\EntityCompanionStorage::$instance` | **Leaks: No** | Holds a single `WeakMap>`. `WeakMap` entries are automatically removed by the garbage collector once the keyed `Entity` object itself is no longer referenced — since an `Entity` built for one request goes out of scope at the end of that request (nothing else retains it), its companion-bag entry is collected too. Self-cleaning by construction; no explicit reset needed. | - -## 4. Superglobal readers outside `Request::fromGlobals()` - -Verified complete. **Under a RoadRunner worker, `$_SERVER`, `$_GET`, -`$_POST` and `$_COOKIE` are frozen at whatever the worker process's PHP CLI -entrypoint started with** — confirmed by reading -`packages/roadrunner/src/Http/Psr7RequestBridge.php`, which builds `Request` -entirely from the incoming PSR-7 message and never touches a superglobal. -This changes the nature of every finding below: it is **not** a -cross-request identity leak (the values never change between requests, so -one user's data can't bleed into another's), but a **staleness/correctness** -bug — these code paths would show the same (effectively empty, CLI-derived) -snapshot for every request forever under a worker. - -| Location | Leaks (cross-request identity): | Verdict | -| --- | --- | --- | -| `errors-advanced/src/RequestDataCollector.php:48-51` | **No** (staleness only) | Constructor accepts injectable `$server`/`$get`/`$post`/`$cookie` overrides, falling back to the superglobals only when the caller passes nothing. `PrettyHtmlFormatter` (`errors-advanced/src/PrettyHtmlFormatter.php:20`) does call `new RequestDataCollector()` with no arguments, so it *does* hit the frozen-superglobal path under a worker — the error page's "Request" section would show stale/empty data on every error, not another user's data. Not exercised in this spike (`marko/errors-advanced` not wired into the fixture app); recorded from source. | -| `debugbar/src/Debugbar.php:517` (`serverString()`) | **No** (staleness only) | Reads `$_SERVER[$key]` directly with no injectable override. Same frozen-value effect as above. Moot in practice: `marko/debugbar` is already flagged unsafe for worker mode (§1). | -| `debugbar/src/Controller/ProfilerController.php:92` (`serverHeader()`) | **No** (staleness only) | Same pattern, same package, same existing guard-rail mitigation. | -| `debugbar/src/Collectors/RequestCollector.php:18-39` | **No** (staleness only) | Reads `$_SERVER['REQUEST_METHOD']`, `$_SERVER['REQUEST_URI']`, `$_GET`, `$_POST`, and iterates `$_SERVER` for `HTTP_*` headers — all frozen under a worker. Same package, same existing mitigation. | -| `debugbar/src/Collectors/InertiaCollector.php:127` | **No** (staleness only) | Same pattern. | - -## 5. Process-global PHP state (no singleton audit finds these) - -| Item | Leaks: | Verdict | -| --- | --- | --- | -| `register_shutdown_function()` accumulation — `Session::configure()` | **Leaks: No — already fixed, confirmed via harness** | `Session.php` guards the `session_set_save_handler($this->handler, true)` call (which registers a shutdown callback as a side effect) behind a private `$handlerRegistered` flag, set once and never cleared by `reset()`. Confirmed by driving ten `/session/write` requests through the harness and asserting via reflection that `handlerRegistered` is `true` and the code path that flips it only ever runs once (the flag would prevent a second `session_set_save_handler()` call for the life of the process). | -| `register_shutdown_function()` — `SimpleErrorHandler::register()` / `AdvancedErrorHandler::register()` | **Leaks: No** | Both guard `register()` behind a `$registered` flag, and both are invoked exactly once, from a module `boot` callback (not from request handling) — confirmed by reading `errors-simple/module.php` and `errors-advanced/module.php`, whose `boot` closures call `$handler->register()` unconditionally but only run once per process (during `Application::initialize()`). | -| `set_exception_handler()` / `set_error_handler()` stack depth | **Leaks: No** | Same two handler classes as above; `register()`'s guard prevents re-registration, so the handler stack depth is fixed once at boot and never grows per request. `unregister()` exists and correctly calls `restore_exception_handler()`/`restore_error_handler()`, but nothing in the request path calls it. | -| `ob_get_level()` drift — `Debugbar::boot()` | **Leaks: Yes (architectural, already flagged)** | `boot()` calls `ob_start()` exactly once (guarded by `$booted`), but since it is never matched by a per-request `ob_end_*()`/`ob_get_clean()`, the buffer opened for the *first* request captures output from every request that follows in the same worker process. Already surfaced by `UnsafePackageChecker::warnDebugbar()` (task 004) as incompatible with a long-running worker. Confirmed clean by contrast: driving 20 requests through the harness (which does **not** wire `marko/debugbar`) shows `ob_get_level()` unchanged before and after — the drift is specific to Debugbar's `ob_start()`, not a property of request handling in general. | -| `ob_get_level()` drift — `SimpleErrorHandler::clearOutputBuffers()` | **Leaks: No** | Drains buffers down to level 0 (`while (ob_get_level() > 0) { ob_end_clean(); }`) only when rendering a fatal error page — a safety net that *reduces* drift, not a source of it. | -| `ini_set()` drift — `Session::configure()` (seven calls, lines 268-278) | **Leaks: No** | All seven calls re-apply the same config-derived values (`session.gc_maxlifetime`, `gc_probability`, `gc_divisor`, `use_strict_mode`, `lazy_write`, `use_cookies`, `use_only_cookies`) on every `start()`, computed fresh from `SessionConfig` each time — idempotent re-application of the same values, not drift from a previous request's mutated state. No other `ini_set()` call exists anywhere under `packages/*/src`. | -| `session_status()` left `PHP_SESSION_ACTIVE` when a request throws | **Leaks: No — already safe, confirmed via harness** | `SessionMiddleware::handle()` wraps `$next($request)` in `try { ... } finally { ... }`, and the `finally` block either saves the session (resumed or modified) or discards it (neither). `Session::save()` calls `session_write_close()` and `Session::discard()` calls `session_abort()` whenever `$this->started` is true, so both paths close the native session. A session that was only armed (a cookieless request that never used it) has no native session open; both calls just clear the armed flag, as does `reset()`. Confirmed directly: a new fixture route (`GET /session/throw`) writes to the session and then throws; driving it through the harness and catching the propagated exception, `session_status()` is `PHP_SESSION_NONE` immediately after — not left active. | -| Timezone / locale (`date_default_timezone_set`, `setlocale`) | **Leaks: No** | Neither function is called anywhere under `packages/*/src` in the monorepo (verified by grep). Nothing to reset. | -| Open database transactions left uncommitted by a thrown request | **Leaks: Yes** | `ReadWriteConnection::reset()` only calls `resetStickyState()` (clears `$stickyWrite`); it does **not** roll back an in-progress transaction. If a request calls `beginTransaction()` directly (not via the `transaction()` helper, which already wraps its work in `try/finally { $this->stickyWrite = false; }`) and then throws before `commit()`/`rollback()`, the underlying write connection is left with `inTransaction() === true` on a connection that is pooled across requests — the next request's writes on that connection are silently appended to the previous request's abandoned transaction. Source read only; `marko/database-readwrite` is not wired into the fixture app. **Reset requirement**: task 006 should call `rollback()` (guarded by `inTransaction()`) as part of resetting `ConnectionInterface`/`TransactionInterface` instances, in addition to the existing `resetStickyState()`. | -| `mt_srand()` / `srand()` seeding | **Leaks: No** | Neither function is called anywhere under `packages/*/src` in the monorepo (verified by grep). PHP's Mersenne Twister is auto-seeded per-process by default and nothing in this codebase re-seeds it, so there is no request-derived seed state to leak. | - -## 6. Memory growth over several hundred requests - -Driven directly (not through Pest, to get raw numbers for this document): -600 sequential `/session/write` requests through the harness, each with a -distinct session cookie, sampling `memory_get_usage(true)` every 100 -requests. - -**With `reset()` called between every request** (the shape task 006 wires): - -| Requests | Memory (RSS-equivalent) | -| --- | --- | -| 0 | 4.00 MB | -| 100 | 4.00 MB | -| 200 | 4.00 MB | -| 300 | 4.00 MB | -| 400 | 4.00 MB | -| 500 | 4.00 MB | - -**Without ever calling `reset()`** (today's unfixed-worker shape, for -comparison): - -| Requests | Memory (RSS-equivalent) | -| --- | --- | -| 0 | 4.00 MB | -| 100 | 4.00 MB | -| 200 | 4.00 MB | -| 300 | 4.00 MB | -| 400 | 4.00 MB | -| 500 | 4.00 MB | - -**Verdict: Leaks: No, flat curve — for the services this fixture wires** -(`Session`, `SessionGuard`, `AuthManager`). `AuthManager::$guards` only ever -holds one entry (the default "web" guard, cached by *name* not by request), -`Session::$data` holds a single overwritten `visits` key, and -`SessionGuard`'s cached user is a single reference reused across logins as -the same fixture user — none of these grow with request count, with or -without `reset()`. This does **not** contradict the confirmed leaks in §1 -(`Debugbar`, `Inertia`, `DatabaseConnectionPlugin`/`ViewPlugin`) — those -packages are not installed in this fixture app, so their unbounded-growth -arrays (`Debugbar::$queries`, `$messages`, ...; `Inertia::$shared`; -`DatabaseConnectionPlugin`/`ViewPlugin::$started`) are not exercised here. -Those are confirmed by source-level analysis (§1) and the general shape of -the mechanism — every push with no matching pop, on a singleton, across -however many requests reach it — makes them unbounded regardless of this -fixture's flat curve. A production app that installs those packages would -need to reproduce this same memory-curve method against them directly. - -The Pest test for this requirement (`it does not grow memory unboundedly -across several hundred requests`) drives 400 requests and asserts total -growth stays under a generous 25 MB bound — looser than the ~0 MB observed -above, so the test remains robust to environment-specific allocator -behaviour while still catching a real unbounded leak (which would blow far -past 25 MB over 400 requests). - -## 7. Anything caching a `Request` or `Response` for the process lifetime - -**Leaks: No.** Verified by grep across every `packages/*/src` directory: -no class declares an instance or static property typed `Request` or -`Response` outside a method parameter or local variable scope. The only -`static function fromGlobals(): self` on `Request` builds a fresh instance -per call and stores nothing statically. - -## Summary for task 006 - -Confirmed `Leaks: Yes` items needing a per-request reset, beyond the -already-fixed `Session`/`SessionGuard`: - -1. **`ReadWriteConnection`** — roll back an open transaction (`inTransaction()` - guarded `rollback()`) in addition to the existing `resetStickyState()`. -2. **`Inertia::$shared`** — needs a new `reset()` (implement - `ResettableInterface`) clearing `$shared`. Only relevant if - `marko/inertia` is installed. -3. **`Debugbar`**, **`DatabaseConnectionPlugin`**, **`ViewPlugin`** — not - resettable in a way that makes the package worker-safe (the `ob_start()` - buffer problem on `Debugbar` is architectural). Covered by the existing - `UnsafePackageChecker` guard rail (task 004): do not enable - `marko/debugbar` in a worker-served production environment. No - `ResettableInterface` wiring recommended for these three — the fix is - "don't run this package in a worker," already enforced. - -Everything else in this document is `Leaks: No` and should not receive -speculative reset wiring. diff --git a/packages/docs-markdown/docs/packages/roadrunner.md b/packages/docs-markdown/docs/packages/roadrunner.md index dd3d6952e..6747e7542 100644 --- a/packages/docs-markdown/docs/packages/roadrunner.md +++ b/packages/docs-markdown/docs/packages/roadrunner.md @@ -90,7 +90,12 @@ The reset runs **before** each request, not after: a request that throws, or a w This driver is the only place in the monorepo that runs `packages/*/src` code under PHPStan level 6 outside `marko/core` — deliberately, since a type error here is a plausible path to a cross-user security bug rather than an ordinary bug. -For the full mechanical audit behind this section — every container singleton, boot-time binding, class static, superglobal reader, and process-global PHP setting in the monorepo, each given an explicit leak verdict — see [RoadRunner state-leak audit](/docs/packages/roadrunner-state-leaks/). +Some state lives for the whole worker on purpose. Keep these in mind when extending the framework: + +- **Admin sections** (`marko/admin`) are built once and live as long as the worker. A section must not keep request or user data in its properties. +- **Custom casts** — `CastResolver` caches one instance per `CastInterface` class for the worker's lifetime, so a custom cast must be stateless (the built-in casts are). +- **Login throttle counts** (`marko/authentication`) live in the cache, keyed by login identifier and client, so they persist across requests by design. +- **Database server detection** — `MySqlServer` caches whether it is talking to MySQL or MariaDB (and the version) for the worker's lifetime. Restart workers after moving the database to a different server type. ## The Session Cookie Caveat diff --git a/packages/docs-markdown/tests/Fixtures/docs/getting-started/installation.md b/packages/docs-markdown/tests/Fixtures/docs/getting-started/installation.md new file mode 100644 index 000000000..2a5391179 --- /dev/null +++ b/packages/docs-markdown/tests/Fixtures/docs/getting-started/installation.md @@ -0,0 +1,5 @@ +--- +title: Installation +--- + +Fixture page. diff --git a/packages/docs-markdown/tests/Unit/AiAssistedDocsTest.php b/packages/docs-markdown/tests/Unit/AiAssistedDocsTest.php deleted file mode 100644 index 2452532f2..000000000 --- a/packages/docs-markdown/tests/Unit/AiAssistedDocsTest.php +++ /dev/null @@ -1,24 +0,0 @@ -toBeTrue("Missing: $rel"); - $content = (string) file_get_contents($path); - expect(str_starts_with($content, '---'))->toBeTrue("Missing frontmatter in $rel"); - expect(str_contains($content, 'title:'))->toBeTrue("Missing title frontmatter in $rel"); - } -}); diff --git a/packages/docs-markdown/tests/Unit/AstroPipelineTest.php b/packages/docs-markdown/tests/Unit/AstroPipelineTest.php deleted file mode 100644 index 57f5bd64e..000000000 --- a/packages/docs-markdown/tests/Unit/AstroPipelineTest.php +++ /dev/null @@ -1,63 +0,0 @@ -toBeTrue(); -}); - -it( - 'produces a clean build of marko.build/docs after the path change', - function () use ($symlinkPath, $packageDocs): void { - $target = readlink($symlinkPath); - $resolvedTarget = realpath(dirname($symlinkPath) . '/' . $target); - - expect($resolvedTarget)->toBe(realpath($packageDocs)); - }, -); - -it('renders the same pages count as before the migration', function () use ($symlinkPath): void { - // Count .md and .mdx files reachable through the symlinked source. - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($symlinkPath, FilesystemIterator::SKIP_DOTS), - ); - - $count = 0; - - foreach ($files as $file) { - if (in_array($file->getExtension(), ['md', 'mdx'], true)) { - $count++; - } - } - - // Migration-integrity guard: the symlinked source must expose at least - // the migrated baseline of 119 pages (.md + .mdx). Asserted as a floor, - // not an exact count, so adding new doc pages in later phases doesn't - // churn this test — the invariant we care about is "no pages were lost - // in the move," not a frozen total. - expect($count)->toBeGreaterThanOrEqual(119); -}); - -it('preserves image and asset paths through the rename', function () use ($packageDocs): void { - expect(is_dir($packageDocs))->toBeTrue(); - - $repo = new MarkdownRepository($packageDocs); - $pages = $repo->listAllPages(); - - expect($pages)->not->toBeEmpty(); -}); - -it('updates any navigation config to reflect the new source path', function () use ($symlinkPath, $packageDocs): void { - // The symlink IS the navigation config update: Astro reads docs via the - // symlink which now points to packages/docs-markdown/docs - expect(is_link($symlinkPath))->toBeTrue() - ->and(realpath($symlinkPath))->toBe(realpath($packageDocs)); -}); diff --git a/packages/docs-markdown/tests/Unit/MarkdownRepositoryTest.php b/packages/docs-markdown/tests/Unit/MarkdownRepositoryTest.php index 3ef81022f..b2732c870 100644 --- a/packages/docs-markdown/tests/Unit/MarkdownRepositoryTest.php +++ b/packages/docs-markdown/tests/Unit/MarkdownRepositoryTest.php @@ -11,37 +11,23 @@ }); it('exposes absolute path to docs root via a dedicated accessor', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); expect($repo->getDocsPath())->toBe($docsPath); }); -it('returns file content matching the docs files reachable via the Astro symlink', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; - $repo = new MarkdownRepository($docsPath); - - $content = $repo->getRawMarkdown('getting-started/installation'); - - // The Astro site reads the same files through the symlink at - // docs/src/content/docs → packages/docs-markdown/docs. Derive the path - // relative to the repo root (portable; no hardcoded absolute path). - $symlinkedPath = dirname(__DIR__, 4) . '/docs/src/content/docs/getting-started/installation.md'; - expect($content)->toBe((string) file_get_contents($symlinkedPath)); -}); - it('reads markdown for a legitimate page id', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); $content = $repo->getRawMarkdown('getting-started/installation'); - expect($content)->toBeString() - ->and(strlen($content))->toBeGreaterThan(0); + expect($content)->toBe((string) file_get_contents($docsPath . '/getting-started/installation.md')); }); it('throws DocsMarkdownException for an id containing path-traversal segments', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); // Create a real .md file outside the docs root so traversal resolves to a real path @@ -60,7 +46,7 @@ }); it('does not read a .md file outside the docs directory via a traversal id', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); // Create a real .md file outside the docs root to confirm traversal is blocked @@ -85,7 +71,7 @@ }); it('still throws pageNotFound for a clean id that does not exist', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); expect(fn () => $repo->getRawMarkdown('nonexistent-page')) @@ -93,7 +79,7 @@ }); it('populates message, context, and suggestion on the traversal exception', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; + $docsPath = dirname(__DIR__) . '/Fixtures/docs'; $repo = new MarkdownRepository($docsPath); // Create a real .md file outside the docs root so traversal resolves to a real path diff --git a/packages/docs-markdown/tests/Unit/PackageStructureTest.php b/packages/docs-markdown/tests/Unit/PackageStructureTest.php index 9548cc18b..43cf89d34 100644 --- a/packages/docs-markdown/tests/Unit/PackageStructureTest.php +++ b/packages/docs-markdown/tests/Unit/PackageStructureTest.php @@ -11,19 +11,3 @@ ->and($composer['autoload']['psr-4'])->toHaveKey('Marko\\DocsMarkdown\\') ->and($composer['autoload']['psr-4']['Marko\\DocsMarkdown\\'])->toBe('src/'); }); - -it('preserves the original navigation metadata file or equivalent index', function (): void { - $indexPath = dirname(__DIR__, 2) . '/docs/index.mdx'; - - expect(file_exists($indexPath))->toBeTrue(); -}); - -it('ships docs content under docs/ inside the package', function (): void { - $docsPath = dirname(__DIR__, 2) . '/docs'; - - expect(is_dir($docsPath))->toBeTrue(); - - $files = glob($docsPath . '/**/*.md'); - - expect($files)->not->toBeEmpty(); -}); diff --git a/packages/framework/tests/ArchitectureDocTest.php b/packages/framework/tests/ArchitectureDocTest.php deleted file mode 100644 index 0873b8c35..000000000 --- a/packages/framework/tests/ArchitectureDocTest.php +++ /dev/null @@ -1,59 +0,0 @@ -toContain('## Engine-Specific Template Siblings'); -}); - -it('the section is placed between Package Architecture and Dependency Injection', function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - $packageArchPos = strpos($content, '## Package Architecture'); - $siblingPos = strpos($content, '## Engine-Specific Template Siblings'); - $diPos = strpos($content, '## Dependency Injection'); - - expect($packageArchPos)->toBeLessThan($siblingPos) - ->and($siblingPos)->toBeLessThan($diPos); -}); - -it('the section explains the marko/{module}-{engine} naming pattern', function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - expect($content)->toContain('marko/{module}-{engine}'); -}); - -it('the section documents the templates_for composer-extra key', function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - expect($content)->toContain('templates_for'); -}); - -it('the section explains when to use the pattern (reusable UI packages)', function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - expect($content)->toContain('reusable') - ->and($content)->toContain('UI'); -}); - -it( - 'the section explains when NOT to use the pattern (application-specific modules)', - function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - expect($content)->toContain('application-specific'); - }, -); - -it( - 'the section mentions the CrossEngineTemplateParityTest as the enforcement mechanism', - function () use ($architecturePath) { - $content = file_get_contents($architecturePath); - - expect($content)->toContain('CrossEngineTemplateParityTest'); - }, -); diff --git a/packages/framework/tests/PackageTest.php b/packages/framework/tests/PackageTest.php index 30a118c63..4367fa31d 100644 --- a/packages/framework/tests/PackageTest.php +++ b/packages/framework/tests/PackageTest.php @@ -96,36 +96,3 @@ expect($composer['suggest'])->toHaveKey($package); } }); - -it('README exists in package root', function () { - $readmePath = dirname(__DIR__) . '/README.md'; - - expect(file_exists($readmePath))->toBeTrue(); -}); - -it('README documents included packages', function () { - $readmePath = dirname(__DIR__) . '/README.md'; - $content = file_get_contents($readmePath); - - $includedPackages = [ - 'marko/core', - 'marko/routing', - 'marko/cli', - 'marko/errors', - 'marko/errors-simple', - 'marko/config', - 'marko/hashing', - 'marko/validation', - ]; - - foreach ($includedPackages as $package) { - expect($content)->toContain($package); - } -}); - -it('README shows installation examples', function () { - $readmePath = dirname(__DIR__) . '/README.md'; - $content = file_get_contents($readmePath); - - expect($content)->toContain('composer require marko/framework'); -}); diff --git a/packages/page-cache-entity/tests/PackageStructureTest.php b/packages/page-cache-entity/tests/PackageStructureTest.php index 170295a28..0062df230 100644 --- a/packages/page-cache-entity/tests/PackageStructureTest.php +++ b/packages/page-cache-entity/tests/PackageStructureTest.php @@ -112,31 +112,3 @@ function (): void { expect(str_contains($bugReportContent, '- page-cache-entity'))->toBeTrue() ->and(str_contains($featureRequestContent, '- page-cache-entity'))->toBeTrue(); }); - -it('has a README.md at packages/page-cache-entity/README.md', function (): void { - $readmePath = dirname(__DIR__) . '/README.md'; - - expect(file_exists($readmePath))->toBeTrue(); -}); - -it('documents installation via composer require marko/page-cache-entity', function (): void { - $readmePath = dirname(__DIR__) . '/README.md'; - $content = file_get_contents($readmePath); - - expect(str_contains($content, 'composer require marko/page-cache-entity'))->toBeTrue(); -}); - -it('documents implementing IdentityInterface on an entity', function (): void { - $readmePath = dirname(__DIR__) . '/README.md'; - $content = file_get_contents($readmePath); - - expect(str_contains($content, 'IdentityInterface'))->toBeTrue() - ->and(str_contains($content, 'getIdentities'))->toBeTrue(); -}); - -it('links to the docs page for full documentation', function (): void { - $readmePath = dirname(__DIR__) . '/README.md'; - $content = file_get_contents($readmePath); - - expect($content)->toContain('https://marko.build/docs/packages/page-cache-entity/'); -}); diff --git a/packages/pubsub-pgsql/tests/PackageScaffoldingTest.php b/packages/pubsub-pgsql/tests/PackageScaffoldingTest.php index 464eecdb1..164bfba9b 100644 --- a/packages/pubsub-pgsql/tests/PackageScaffoldingTest.php +++ b/packages/pubsub-pgsql/tests/PackageScaffoldingTest.php @@ -2,16 +2,6 @@ declare(strict_types=1); -it('creates README.md for marko/pubsub-pgsql with all required sections', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme) - ->toContain('## Overview') - ->and($readme)->toContain('## Installation') - ->and($readme)->toContain('## Usage') - ->and($readme)->toContain('## API Reference'); -}); - it( 'has valid module.php for marko/pubsub-pgsql binding PublisherInterface and SubscriberInterface', function (): void { diff --git a/packages/pubsub-redis/tests/PackageScaffoldingTest.php b/packages/pubsub-redis/tests/PackageScaffoldingTest.php index cd6d401ea..67ef7dda9 100644 --- a/packages/pubsub-redis/tests/PackageScaffoldingTest.php +++ b/packages/pubsub-redis/tests/PackageScaffoldingTest.php @@ -2,16 +2,6 @@ declare(strict_types=1); -it('has a README with installation, quick example and docs link', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toStartWith("# marko/pubsub-redis\n") - ->toContain("## Installation\n") - ->toContain('composer require marko/pubsub-redis') - ->toContain("## Quick Example\n") - ->toContain('https://marko.build/docs/packages/pubsub-redis/'); -}); - it( 'has valid module.php for marko/pubsub-redis binding PublisherInterface and SubscriberInterface', function (): void { diff --git a/packages/pubsub/tests/PackageScaffoldingTest.php b/packages/pubsub/tests/PackageScaffoldingTest.php index afe057c41..6d8300866 100644 --- a/packages/pubsub/tests/PackageScaffoldingTest.php +++ b/packages/pubsub/tests/PackageScaffoldingTest.php @@ -33,16 +33,6 @@ ->and($module['bindings'])->toBeEmpty(); }); -it('creates README.md for marko/pubsub with all required sections', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme) - ->toContain('## Overview') - ->and($readme)->toContain('## Installation') - ->and($readme)->toContain('## Usage') - ->and($readme)->toContain('## API Reference'); -}); - it('has valid composer.json with name marko/pubsub and required fields', function (): void { $composerPath = dirname(__DIR__) . '/composer.json'; diff --git a/packages/ratelimiter/tests/Unit/RenameReferencesTest.php b/packages/ratelimiter/tests/Unit/RenameReferencesTest.php index 9a91a11c1..db107ba49 100644 --- a/packages/ratelimiter/tests/Unit/RenameReferencesTest.php +++ b/packages/ratelimiter/tests/Unit/RenameReferencesTest.php @@ -66,18 +66,6 @@ function (): void { expect($hits)->toBe([], 'Found Marko\\RateLimiting references in: ' . implode(', ', $hits)); }); -it('updates .claude/architecture.md package inventory to reference marko/ratelimiter', function (): void { - $monorepoRoot = dirname(__DIR__, 4); - $architectureFile = $monorepoRoot . '/.claude/architecture.md'; - - expect(file_exists($architectureFile))->toBeTrue(); - - $contents = file_get_contents($architectureFile); - - // Should not contain the old package name in the inventory - expect($contents)->not->toContain('marko/rate-limiting'); -}); - it('runs composer dump-autoload without errors', function (): void { $monorepoRoot = dirname(__DIR__, 4); diff --git a/packages/roadrunner/tests/DocsTest.php b/packages/roadrunner/tests/DocsTest.php deleted file mode 100644 index aeed2c93e..000000000 --- a/packages/roadrunner/tests/DocsTest.php +++ /dev/null @@ -1,98 +0,0 @@ -toBeTrue(); - - $readme = file_get_contents($readmePath); - - expect($readme)->toContain('# marko/roadrunner') - ->and($readme)->toContain('## Installation') - ->and($readme)->toContain('composer require marko/roadrunner') - ->and($readme)->toContain('## Quick Example') - ->and($readme)->toContain('## Documentation') - ->and($readme)->toContain('https://marko.build/docs/packages/roadrunner/'); - }); - - it('ships a docs page for the roadrunner package', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - - expect(file_exists($docsPath))->toBeTrue(); - - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('title: marko/roadrunner') - ->and($docs)->toContain('## Installation') - ->and($docs)->toContain('rr:serve') - ->and($docs)->toContain('vendor/marko/roadrunner/worker.php') - ->and($docs)->toContain('.rr.yaml') - ->and($docs)->toContain('http.static.dir') - ->and($docs)->toContain('pool.max_jobs') - ->and($docs)->toContain('pool.supervisor.max_worker_memory') - ->and($docs)->toContain('server.env.MARKO_BASE_PATH'); - }); - - it('documents the unsupported packages and the reason for each', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('marko/sse') - ->and($docs)->toContain('acknowledged_unsafe_packages') - ->and($docs)->toContain('StreamingResponseException') - ->and($docs)->toContain('marko/debugbar') - ->and($docs)->toContain('ob_start()'); - }); - - it('documents that file uploads are mapped to UploadedFile', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('File Uploads') - ->and($docs)->toContain('Marko\Routing\Http\UploadedFile') - ->and($docs)->toContain('removeTemporaryFiles()') - ->and($docs)->not->toContain('UploadedFilesNotSupportedException'); - }); - - it('documents the session cookie caveat', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('Session Cookie Caveat') - ->and($docs)->toContain('SessionMiddleware') - ->and($docs)->toContain('Set-Cookie'); - }); - - it('documents the stdout restriction', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('Do Not Write to STDOUT') - ->and($docs)->toContain('var_dump()') - ->and($docs)->toContain('STDERR'); - }); - - it('documents the reset lifecycle and the stateful singleton rule', function (): void { - $docsPath = monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner.md'; - $docs = file_get_contents($docsPath); - - expect($docs)->toContain('ResettableInterface') - ->and($docs)->toContain('cross-user leak') - ->and($docs)->toContain('SessionGuard') - ->and($docs)->toContain('Inertia::$shared') - ->and($docs)->toContain('roadrunner-state-leaks'); - }); - - it('lists the package in the architecture package inventory', function (): void { - $architecturePath = monorepoRootPath() . '/.claude/architecture.md'; - $architecture = file_get_contents($architecturePath); - - expect($architecture)->toContain('## Package Inventory') - ->and($architecture)->toContain('`marko/roadrunner`'); - }); -}); diff --git a/packages/roadrunner/tests/Helpers.php b/packages/roadrunner/tests/Helpers.php index c8e33ef5b..2150acffe 100644 --- a/packages/roadrunner/tests/Helpers.php +++ b/packages/roadrunner/tests/Helpers.php @@ -199,39 +199,3 @@ function sharedRoadRunnerServer(): RoadRunnerServerProcess return SharedRoadRunnerServer::get($binary, inProcessHarnessFixturePath()); } - -/** - * Parse a module.php file's `singletons` declaration into the short - * (unqualified) identifiers it registers, used to mechanically cross-check - * the state-leak findings document against every singleton actually - * declared in the monorepo, rather than trusting a hand-maintained list. - * - * Handles both list form (`[Foo::class]`) and keyed form - * (`[Interface::class => Concrete::class]` or `[Interface::class => Closure]`) - * — the identifier is the string key when present, the string value - * otherwise. - * - * @return list - */ -function moduleSingletonIdentifiers( - string $moduleFile, -): array { - /** @var array{singletons?: array} $config */ - $config = require $moduleFile; - $singletons = $config['singletons'] ?? []; - $identifiers = []; - - foreach ($singletons as $key => $value) { - $identifier = is_string($key) ? $key : $value; - - if (!is_string($identifier)) { - continue; - } - - $identifiers[] = str_contains($identifier, '\\') - ? substr($identifier, strrpos($identifier, '\\') + 1) - : $identifier; - } - - return array_values(array_unique($identifiers)); -} diff --git a/packages/roadrunner/tests/StateLeakSpikeTest.php b/packages/roadrunner/tests/StateLeakSpikeTest.php index 19fce143f..4f3073c2d 100644 --- a/packages/roadrunner/tests/StateLeakSpikeTest.php +++ b/packages/roadrunner/tests/StateLeakSpikeTest.php @@ -12,9 +12,7 @@ /** * Task 005's discovery spike: drive many requests with interleaved * identities through one booted application and empirically confirm what - * leaks between them. The findings document this suite verifies against is - * task 006's sole source of truth for what to wire — see - * packages/docs-markdown/docs/packages/roadrunner-state-leaks.md. + * leaks between them. */ describe('State-leak discovery spike', function (): void { it('does not carry session data from one request into the next', function (): void { @@ -174,93 +172,4 @@ // warmup and PHP's own allocator behaviour is expected and fine. expect($growth)->toBeLessThan(25 * 1024 * 1024); }); - - it('records every confirmed leak in the findings document', function (): void { - $doc = file_get_contents( - monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md', - ); - - expect($doc) - ->toContain('Session') - ->toContain('SessionGuard') - ->toContain('ReadWriteConnection') - ->toContain('Inertia') - ->toContain('Debugbar') - ->toContain('DatabaseConnectionPlugin') - ->toContain('ViewPlugin'); - }); - - it('records investigated leads that turned out not to leak', function (): void { - $doc = file_get_contents( - monorepoRootPath() . '/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md', - ); - - expect($doc) - ->toContain('EntityCompanionStorage') - ->toContain('RouteCollection') - ->toContain('PolicyRegistry') - ->toContain('IndexCache') - ->toContain('mt_srand') - ->toContain('date_default_timezone_set'); - }); - - it('records a verdict for every singleton declared across the monorepo', function (): void { - $root = monorepoRootPath(); - $doc = file_get_contents( - $root . '/packages/docs-markdown/docs/packages/roadrunner-state-leaks.md', - ); - - $moduleFiles = [ - 'admin' => $root . '/packages/admin/module.php', - 'admin-auth' => $root . '/packages/admin-auth/module.php', - 'authentication' => $root . '/packages/authentication/module.php', - 'authorization' => $root . '/packages/authorization/module.php', - 'codeindexer' => $root . '/packages/codeindexer/module.php', - 'database' => $root . '/packages/database/module.php', - 'database-mysql' => $root . '/packages/database-mysql/module.php', - 'database-pgsql' => $root . '/packages/database-pgsql/module.php', - 'debugbar' => $root . '/packages/debugbar/module.php', - 'devai' => $root . '/packages/devai/module.php', - 'docs-fts' => $root . '/packages/docs-fts/module.php', - 'docs-markdown' => $root . '/packages/docs-markdown/module.php', - 'docs' => $root . '/packages/docs/module.php', - 'inertia' => $root . '/packages/inertia/module.php', - 'layout' => $root . '/packages/layout/module.php', - 'lsp' => $root . '/packages/lsp/module.php', - 'mcp' => $root . '/packages/mcp/module.php', - 'session-database' => $root . '/packages/session-database/module.php', - 'session-file' => $root . '/packages/session-file/module.php', - 'vite' => $root . '/packages/vite/module.php', - 'codeindexer fixture' => $root - . '/packages/codeindexer/tests/Fixtures/MiniMonorepo/vendor/foo/bar/module.php', - ]; - - $lines = explode("\n", $doc); - $missingVerdicts = []; - - foreach ($moduleFiles as $package => $moduleFile) { - $identifiers = moduleSingletonIdentifiers($moduleFile); - - if ($identifiers === []) { - if (!str_contains($doc, $package)) { - $missingVerdicts[] = $package . ' (no singletons declared)'; - } - - continue; - } - - foreach ($identifiers as $identifier) { - $hasVerdictLine = array_any( - $lines, - fn (string $line): bool => str_contains($line, $identifier) && str_contains($line, 'Leaks:'), - ); - - if (!$hasVerdictLine) { - $missingVerdicts[] = "$package: $identifier"; - } - } - } - - expect($missingVerdicts)->toBeEmpty(); - }); }); diff --git a/packages/skeleton/tests/PackageStructureTest.php b/packages/skeleton/tests/PackageStructureTest.php index 038b4c380..8ac2267e0 100644 --- a/packages/skeleton/tests/PackageStructureTest.php +++ b/packages/skeleton/tests/PackageStructureTest.php @@ -97,12 +97,6 @@ expect(file_exists($gitkeepPath))->toBeTrue(); }); -it('has a README.md file', function (): void { - $packagePath = __DIR__ . '/..'; - - expect(file_exists($packagePath . '/README.md'))->toBeTrue(); -}); - it('ships a .gitignore to generated projects', function (): void { $gitignorePath = __DIR__ . '/../.gitignore'; diff --git a/packages/testing/tests/ReadmeTest.php b/packages/testing/tests/ReadmeTest.php deleted file mode 100644 index 4cb8c4c58..000000000 --- a/packages/testing/tests/ReadmeTest.php +++ /dev/null @@ -1,102 +0,0 @@ -toBeTrue() - ->and(file_get_contents($readmePath))->toContain('# marko/testing') - ->and(file_get_contents($readmePath))->toMatch('/^# marko\/testing\n\n.+/m'); -}); - -it('has an overview section explaining the benefit', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('## Overview'); -}); - -it('has an installation section with composer command', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('## Installation') - ->and($readme)->toContain('composer require marko/testing'); -}); - -it('has a usage section with code examples for each fake', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('## Usage') - ->and($readme)->toContain('FakeEventDispatcher') - ->and($readme)->toContain('FakeMailer') - ->and($readme)->toContain('FakeQueue') - ->and($readme)->toContain('FakeSession') - ->and($readme)->toContain('FakeCookieJar') - ->and($readme)->toContain('FakeLogger') - ->and($readme)->toContain('FakeConfigRepository') - ->and($readme)->toContain('FakeAuthenticatable') - ->and($readme)->toContain('FakeUserProvider') - ->and($readme)->toContain('```php'); -}); - -it('has an API reference section listing all public methods', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('## API Reference') - ->and($readme)->toContain('dispatch(') - ->and($readme)->toContain('assertDispatched(') - ->and($readme)->toContain('assertNotDispatched(') - ->and($readme)->toContain('assertDispatchedCount(') - ->and($readme)->toContain('assertSent(') - ->and($readme)->toContain('assertNothingSent(') - ->and($readme)->toContain('assertSentCount(') - ->and($readme)->toContain('assertPushed(') - ->and($readme)->toContain('assertNotPushed(') - ->and($readme)->toContain('assertPushedCount(') - ->and($readme)->toContain('assertNothingPushed(') - ->and($readme)->toContain('assertLogged(') - ->and($readme)->toContain('assertNothingLogged('); -}); - -it('documents FakeGuard in the available fakes table', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('FakeGuard'); -}); - -it('includes FakeGuard usage example with guard configuration', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('### FakeGuard') - ->and($readme)->toContain('new FakeGuard(') - ->and($readme)->toContain('setUser(') - ->and($readme)->toContain('attempt('); -}); - -it('documents FakeGuard assertion methods', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('assertAuthenticated(') - ->and($readme)->toContain('assertGuest(') - ->and($readme)->toContain('assertAttempted(') - ->and($readme)->toContain('assertNotAttempted(') - ->and($readme)->toContain('assertLoggedOut('); -}); - -it('documents toHaveAttempted and toBeAuthenticated Pest expectations', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('toHaveAttempted') - ->and($readme)->toContain('toBeAuthenticated'); -}); - -it('follows the Package README Standards from code-standards.md', function (): void { - $readme = file_get_contents(dirname(__DIR__) . '/README.md'); - - expect($readme)->toContain('# marko/testing') - ->and($readme)->toContain('## Overview') - ->and($readme)->toContain('## Installation') - ->and($readme)->toContain('## Usage') - ->and($readme)->toContain('## API Reference') - ->and($readme)->toContain('```php'); -}); diff --git a/packages/validation/tests/DocsTest.php b/packages/validation/tests/DocsTest.php deleted file mode 100644 index 0bc12a151..000000000 --- a/packages/validation/tests/DocsTest.php +++ /dev/null @@ -1,21 +0,0 @@ -toContain('### Validating Arrays with Wildcards') - ->and(validationDocsPage())->toContain("'items.*.name'") - ->and(validationDocsPage())->toContain("'matrix.*.*'") - ->and(validationDocsPage())->toContain('### Multi-File Uploads') - ->and(validationDocsPage())->toContain("'photos.*' => 'image|max_size:2048'") - ->and(validationDocsPage())->toContain('same:items.*.sku'); -}); - -it('no longer says per-file rules are unsupported', function (): void { - expect(validationDocsPage())->not->toContain('not supported yet'); -}); diff --git a/tests/CiWorkflowTest.php b/tests/CiWorkflowTest.php index eaf4797ca..bda3a3c77 100644 --- a/tests/CiWorkflowTest.php +++ b/tests/CiWorkflowTest.php @@ -38,6 +38,20 @@ ->toContain('composer phpstan'); }); +it('runs docs:lint in the required Lint job on every pull request', function () use ($ci): void { + $start = (int) strpos($ci, 'name: Lint'); + $lintJob = substr($ci, $start, (int) strpos($ci, 'name: Static analysis') - $start); + + expect($lintJob)->toContain('run: composer docs:lint'); +}); + +it('includes docs:lint in composer ci', function (): void { + $composer = json_decode((string) file_get_contents(dirname(__DIR__) . '/composer.json'), true); + + expect($composer['scripts']['docs:lint'])->toBe('php bin/docs-lint.php') + ->and($composer['scripts']['ci'])->toContain('@docs:lint'); +}); + it( 'runs the integration-services group in its own job against postgres and redis services', function () use ($ci): void { @@ -46,8 +60,8 @@ function () use ($ci): void { expect($ci)->toContain('name: Integration') ->and($job) ->toContain('services:') - ->toContain('image: postgres:17') - ->toContain('image: redis:7') + ->toContain('image: public.ecr.aws/docker/library/postgres:17') + ->toContain('image: public.ecr.aws/docker/library/redis:7') ->toContain('- 5432:5432') ->toContain('- 6379:6379') ->toContain('pdo_pgsql') @@ -76,7 +90,7 @@ function () use ($ci): void { $job = substr($ci, (int) strpos($ci, 'name: Integration')); expect($job) - ->toContain('image: mysql:8.4') + ->toContain('image: public.ecr.aws/docker/library/mysql:8.4') ->toContain('- 3306:3306') ->toContain('MYSQL_DATABASE: marko_test') ->toContain('--health-cmd "mysqladmin ping'); @@ -198,13 +212,27 @@ function () use ($ci): void { function () use ($ci, $nightly): void { $redisService = " services:\n # Redis integration suites (packages/cache-redis, packages/ratelimiter)\n" . " # skip without a server; this keeps them running on every build.\n" - . " redis:\n image: redis:7-alpine\n ports:\n - 6379:6379\n"; + . " redis:\n image: public.ecr.aws/docker/library/redis:7-alpine\n ports:\n - 6379:6379\n"; expect($ci)->toContain(" name: Tests\n runs-on: ubuntu-latest\n" . $redisService) ->and($nightly)->toContain($redisService); }, ); +it('pulls service images from the ECR mirror, never rate-limited Docker Hub', function () use ($ci, $nightly): void { + // Anonymous Docker Hub pulls on shared runners hit `toomanyrequests` and fail + // jobs before a single test runs. public.ecr.aws mirrors the official images. + foreach (['ci.yml' => $ci, 'nightly.yml' => $nightly] as $workflow => $content) { + preg_match_all('/^\s+image: (\S+)$/m', $content, $matches); + + expect($matches[1])->not->toBeEmpty("$workflow declares no service images"); + + foreach ($matches[1] as $image) { + expect($image)->toStartWith('public.ecr.aws/docker/library/', "$workflow pulls $image from Docker Hub"); + } + } +}); + it('installs the roadrunner binary in the nightly workflow', function () use ($nightly): void { // Without this, packages/roadrunner's end-to-end suite finds no `rr` // binary on the nightly runner and every one of its security-critical @@ -254,12 +282,3 @@ function () use ($ci, $nightly): void { expect($phpcs)->toContain('src/Broken/'); }); - -it('adds phpstan to the PR review checklist that previously omitted it', function (): void { - $process = file_get_contents(dirname(__DIR__) . '/.claude/pr-review-process.md'); - - expect($process) - ->toContain('composer phpstan') - ->toContain('PHPStan is not optional') - ->not->toContain('No PR CI workflow exists in this repo'); -}); diff --git a/tests/DocsClassReferenceTest.php b/tests/DocsClassReferenceTest.php index 1ff4fb95d..9eccde5fa 100644 --- a/tests/DocsClassReferenceTest.php +++ b/tests/DocsClassReferenceTest.php @@ -7,50 +7,54 @@ require_once __DIR__ . '/Support/DocsClassReference/DocsClassReferenceScanner.php'; /* - * Guards every `Marko\...` class reference in fenced PHP code blocks of the - * package READMEs and the docs site against classes that do not exist. - * - * Fictional example modules should use a non-Marko namespace (`App\`, `Acme\`). - * If one genuinely has to live under `Marko\`, add its namespace prefix to - * $allowedFictionalPrefixes below rather than weakening the scan. + * Tests the scanner behind `composer docs:lint` against fixtures. The real + * docs are checked by `composer docs:lint` in the CI Lint job, never here: + * tests do not read documentation. */ $repositoryRoot = dirname(__DIR__); $packagesRoot = $repositoryRoot . '/packages'; $fixturePackagesRoot = __DIR__ . '/Fixtures/DocsClassReference/packages'; -/** @var list $allowedFictionalPrefixes */ -$allowedFictionalPrefixes = []; - /** - * @param list $references - * @return list + * @return array{exitCode: int, output: string} */ -function formatDocsClassReferences(array $references, string $root): array +function runDocsLint(string ...$arguments): array { - return array_map( - fn (array $reference): string => sprintf( - '%s:%d references missing class %s', - str_replace($root . '/', '', $reference['file']), - $reference['line'], - $reference['class'], - ), - $references, - ); + $command = array_merge([PHP_BINARY, dirname(__DIR__) . '/bin/docs-lint.php'], $arguments); + $process = proc_open($command, [1 => ['pipe', 'w'], 2 => ['redirect', 1]], $pipes); + $output = (string) stream_get_contents($pipes[1]); + fclose($pipes[1]); + + return ['exitCode' => proc_close($process), 'output' => $output]; } -it('resolves every Marko class referenced in README and docs PHP snippets', function () use ( - $packagesRoot, - $repositoryRoot, - $allowedFictionalPrefixes, +it('fails docs:lint with file and line for every unresolved Marko class', function () use ( + $fixturePackagesRoot, ): void { - $files = DocsClassReferenceScanner::discoverMarkdownFiles($packagesRoot); - $scanner = new DocsClassReferenceScanner($packagesRoot, $allowedFictionalPrefixes); + $result = runDocsLint('--docs-root=' . $fixturePackagesRoot); + + expect($result['exitCode'])->toBe(1) + ->and($result['output']) + ->toContain('demo/README.md:7 references missing class Marko\\Nope\\Thing') + ->toContain('nested/section/page.md:13 references missing class Marko\\Missing\\Fqcn') + ->toContain('DocsClassReference/README.md:4 references missing class Marko\\Root\\Missing') + ->not->toContain('Marko\\Routing\\Http\\Response'); +}); + +it('fails docs:lint loudly when it finds no Markdown to scan', function (): void { + $base = sys_get_temp_dir() . '/marko-docs-lint-empty-' . uniqid(); + mkdir($base . '/packages', recursive: true); - $missing = formatDocsClassReferences($scanner->findUnresolved($files), $repositoryRoot); + try { + $result = runDocsLint('--docs-root=' . $base . '/packages'); + } finally { + rmdir($base . '/packages'); + rmdir($base); + } - expect($files)->not->toBeEmpty() - ->and($missing)->toBe([]); + expect($result['exitCode'])->toBe(1) + ->and($result['output'])->toContain('found no Markdown files'); }); it('discovers package READMEs and nested docs pages', function () use ($fixturePackagesRoot): void { diff --git a/tests/Fixtures/DocsClassReference/README.md b/tests/Fixtures/DocsClassReference/README.md new file mode 100644 index 000000000..662374e59 --- /dev/null +++ b/tests/Fixtures/DocsClassReference/README.md @@ -0,0 +1,5 @@ +# Fixture root README + +```php +use Marko\Root\Missing; +``` diff --git a/tests/Integration/App/HarnessTest.php b/tests/Integration/App/HarnessTest.php index 34bcfae0a..a21526298 100644 --- a/tests/Integration/App/HarnessTest.php +++ b/tests/Integration/App/HarnessTest.php @@ -96,20 +96,6 @@ ->and(file_exists(__DIR__ . '/KnownGapsTest.php'))->toBeFalse(); }); -it('documents how to run and extend the integration suite', function (): void { - $docs = (string) file_get_contents(dirname(__DIR__, 3) . '/.claude/testing.md'); - - expect($docs) - ->toContain('## Integration Tests (`integration-services` Group)') - ->toContain('docker compose -f tests/Integration/compose.yml up -d') - ->toContain('composer test:integration') - ->toContain('MARKO_INTEGRATION_REQUIRED') - ->toContain('setUpIntegrationTest($this)') - ->toContain('MARKO_TEST_PGSQL_HOST') - ->toContain('MARKO_TEST_MYSQL_HOST') - ->toContain('### Known gaps'); -}); - it('passes the script name and command ahead of the arguments to the command runner', function (): void { $input = integrationCommandInput('queue:retry', ['5', '--queue=default', '--once']); diff --git a/tests/ReleaseDocumentationTest.php b/tests/ReleaseDocumentationTest.php deleted file mode 100644 index edf788faf..000000000 --- a/tests/ReleaseDocumentationTest.php +++ /dev/null @@ -1,78 +0,0 @@ -toBeTrue('.claude/release-process.md must exist'); - - $content = file_get_contents($docPath); - - expect($content) - ->toContain('# Release Process') - ->toContain('## How It Works') - ->toContain('## Cutting a Release') - ->toContain('bin/release.sh') - ->toContain('marko-php/marko'); -}); - -it('documents the initial one-time setup process (org creation, repo creation, Packagist registration)', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('## Initial Setup') - ->toContain('SPLIT_TOKEN') - ->toContain('bin/create-split-repos.sh') - ->toContain('bin/register-packagist.sh') - ->toContain('marko-php'); -}); - -it('documents the regular release process (merge to main, run release script)', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('git checkout main') - ->toContain('git merge develop') - ->toContain('./bin/release.sh') - ->toContain('github.com/marko-php/marko/actions'); -}); - -it('documents how to add a new package to the ecosystem', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('## Adding a New Package') - ->toContain('bin/add-package.sh') - ->toContain('PACKAGIST_USERNAME') - ->toContain('PACKAGIST_TOKEN'); -}); - -it('documents the branch strategy (develop, main, tags)', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('## Branch Strategy') - ->toContain('develop') - ->toContain('main') - ->toContain('Tags'); -}); - -it('documents versioning rules (unified semver, self.version, 0.x means unstable)', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('## Versioning') - ->toContain('self.version') - ->toContain('0.x') - ->toContain('semver'); -}); - -it('documents troubleshooting for common issues', function () use ($docPath): void { - $content = file_get_contents($docPath); - - expect($content) - ->toContain('## Troubleshooting') - ->toContain('SPLIT_TOKEN') - ->toContain('Packagist'); -}); diff --git a/tests/ReleaseSkillTest.php b/tests/ReleaseSkillTest.php deleted file mode 100644 index 38093e1cb..000000000 --- a/tests/ReleaseSkillTest.php +++ /dev/null @@ -1,113 +0,0 @@ -toBeTrue('.claude/skills/release/SKILL.md must exist'); - - $content = file_get_contents($skillPath); - - expect($content) - ->toStartWith("---\n") - ->toContain('name: release') - ->toContain('description:'); -}); - -it('takes no arguments and decides the version in conversation', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('## Arguments') - ->toContain('None.') - ->toContain('/release'); -}); - -it('stops at a single approval gate before tagging', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('one approval gate') - ->toContain('Present the recommendation, then stop') - ->toContain('Never tag without explicit approval'); -}); - -it('documents 0.x version rules including Composer caret reachability', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('## Step 4 — Decide the version') - ->toContain('**Patch**') - ->toContain('**Minor**') - ->toContain('^0.8.4') - ->toContain('composer update'); -}); - -it('judges each enhancement on its merits instead of auto-escalating the bump', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('### Labels do not decide the version') - ->toContain('Judge every `enhancement` on its own merits') - ->toContain('Never leaves the monorepo') - ->toContain('adds no callable surface') - ->toContain('Only tier 3 forces the minor'); -}); - -it('audits labels against every category in .github/release.yml', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - $releaseConfig = file_get_contents(dirname(__DIR__) . '/.github/release.yml'); - $categories = substr($releaseConfig, (int) strpos($releaseConfig, 'categories:')); - - preg_match_all('/- title: (.+)/', $categories, $titles); - preg_match_all('/^\s+- (?!title: )(\S+)$/m', $categories, $labels); - - expect($titles[1])->not->toBeEmpty() - ->and($labels[1])->not->toBeEmpty(); - - foreach ($titles[1] as $title) { - expect($content)->toContain(trim($title)); - } - - foreach ($labels[1] as $label) { - expect($content)->toContain("`$label`"); - } -}); - -it('delegates mechanical work to bin/release.sh, never the changelog', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('./bin/release.sh ') - ->toContain('Never hand-edit `CHANGELOG.md`') - ->toContain('integration-destructive'); -}); - -it('describes the mid-run failure state accurately', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - $script = file_get_contents(dirname(__DIR__) . '/bin/release.sh'); - - // The skill's claim only holds while release.sh merges into main before running tests. - expect(strpos($script, 'git merge develop'))->toBeLessThan(strpos($script, 'vendor/bin/pest')); - - expect($content) - ->toContain('do not promise a clean abort') - ->toContain('checked out on `main` with the merge already made') - ->toContain('git checkout develop'); -}); - -it('verifies preconditions before proposing a release', function () use ($skillPath): void { - $content = file_get_contents($skillPath); - - expect($content) - ->toContain('## Step 1 — Verify preconditions') - ->toContain('PHP_BIN') - ->toContain('develop'); -}); - -it('points at the skill from .claude/release-process.md', function (): void { - $content = file_get_contents(dirname(__DIR__) . '/.claude/release-process.md'); - - expect($content)->toContain('/release'); -}); diff --git a/tests/RemoveDemoDirectoryTest.php b/tests/RemoveDemoDirectoryTest.php index 1aba60f3f..524f0537d 100644 --- a/tests/RemoveDemoDirectoryTest.php +++ b/tests/RemoveDemoDirectoryTest.php @@ -7,28 +7,3 @@ it('removes the entire demo/ directory', function () use ($root): void { expect(is_dir($root . '/demo'))->toBeFalse('demo/ directory should not exist'); }); - -it('updates CLAUDE.md to remove demo/ references and the demo application section', function () use ($root): void { - $content = file_get_contents($root . '/CLAUDE.md'); - - expect($content) - ->not->toContain('## Demo Application') - ->not->toContain('demo/') - ->not->toContain('demo/app/') - ->not->toContain('CRITICAL: Plans must NEVER include demo/app/'); -}); - -it('removes any other references to demo/ in .claude/ configuration files', function () use ($root): void { - $files = [ - $root . '/.claude/architecture.md', - $root . '/.claude/project-overview.md', - $root . '/.claude/testing.md', - ]; - - foreach ($files as $file) { - if (file_exists($file)) { - $content = file_get_contents($file); - expect($content)->not->toContain('demo/', "File {$file} still references demo/"); - } - } -});