diff --git a/.claude/plans/first-run-friction-fixes/001-core-module-autoloader.md b/.claude/plans/first-run-friction-fixes/001-core-module-autoloader.md new file mode 100644 index 00000000..9de3a74d --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/001-core-module-autoloader.md @@ -0,0 +1,39 @@ +# Task 001: Extract a reusable module autoloader in marko/core + +**Status**: complete +**Depends on**: none +**Retry count**: 0 + +## Description +`Application::registerAutoloaders()` / `registerPsr4Autoloader()` are private and only run during a full app boot. Extract this logic into a reusable, standalone class so a lightweight caller (the test base class in Task 002) can register `app/*` and `modules/*` PSR-4 autoloaders **without** booting the container. `Application` then delegates to it, with no behavior change. + +## Context +- Related files: + - `packages/core/src/Application.php` (lines ~228-268: `registerAutoloaders`, `registerPsr4Autoloader`; uses `ModuleDiscovery`, `ManifestParser`, `DependencyResolver`, module `->source`/`->autoload`/`->path`) + - `packages/core/src/Module/ModuleDiscovery.php`, `ManifestParser.php`, `ModuleManifest` + - New: `packages/core/src/Module/ModuleAutoloader.php` +- Patterns to follow: existing `Module/` classes; constructor property promotion; `declare(strict_types=1)`; no final classes; full type declarations. +- The new class should accept a base path, discover non-vendor modules (`modules/`, `app/`) via the existing discovery, and `spl_autoload_register` a PSR-4 closure per namespace→path. Vendor modules are skipped (Composer already handles them). Registration must be idempotent (safe to call once per process; avoid double-registering the same namespace+path). +- **The class must run discovery itself** (`ModuleDiscovery::discoverInModules()` + `discoverInApp()` with a `ManifestParser`) — it CANNOT consume `Application::$modules`, because the lightweight caller (Task 002 `TestCase`) never boots `Application`. `Application::registerAutoloaders()` currently iterates the already-resolved `$this->modules`; when `Application` delegates to the new class it should pass its discovered/resolved non-vendor modules (or the base path) so behavior is unchanged. Keep `Application`'s existing post-resolution call site working. +- **Autoload source:** a module's PSR-4 map comes from its `composer.json` `autoload.psr-4` (see `ManifestParser`), NOT from `module.php`. Discovery also requires `composer.json` with `extra.marko.module: true`. So this only autoloads app/modules dirs that are real Marko modules with a PSR-4 entry — that constraint is inherent and correct; the empty-dirs test covers the no-module case. +- Running `DependencyResolver` is unnecessary for pure autoloader registration (order does not matter for `spl_autoload_register`); discovery alone suffices, which also avoids surfacing a resolver error during a test run. + +## Requirements (Test Descriptions) +- [x] `it registers a psr-4 autoloader that resolves an app module class from its source file` +- [x] `it registers autoloaders for modules in the modules directory` +- [x] `it skips vendor modules because composer already autoloads them` +- [x] `it does not register the same namespace and path twice when called repeatedly` +- [x] `it resolves nothing and does not error when app and modules directories are empty` +- [x] `it leaves Application boot still able to autoload an app module class (regression)` + +## Acceptance Criteria +- `Application` uses the extracted class; existing core tests still pass. +- New `ModuleAutoloader` is independently constructable without a container. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes + +- Created `packages/core/src/Module/ModuleAutoloader.php` — standalone class that accepts `modulesPath`, `appPath`, and a `ManifestParser`; runs `ModuleDiscovery::discoverInModules()` + `discoverInApp()` then registers `spl_autoload` closures per PSR-4 map. Idempotency tracked via `$this->registered` (namespace+absolutePath key). +- Updated `Application::registerAutoloaders()` to delegate to `ModuleAutoloader` (added `use` import; removed the now-redundant private `registerPsr4Autoloader()` method). +- All 6 requirements covered by `packages/core/tests/Unit/Module/ModuleAutoloaderTest.php`. +- The DependencyResolverTest failure present in the suite is pre-existing from another task on this branch and unrelated to Task 001. diff --git a/.claude/plans/first-run-friction-fixes/002-testing-testcase.md b/.claude/plans/first-run-friction-fixes/002-testing-testcase.md new file mode 100644 index 00000000..370329af --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/002-testing-testcase.md @@ -0,0 +1,44 @@ +# Task 002: Ship Marko\Testing\TestCase (autoloaders only) + +**Status**: complete +**Depends on**: 001 +**Retry count**: 0 + +## Description +Add `Marko\Testing\TestCase` to `marko/testing`. It extends PHPUnit's `TestCase` and, before tests run, registers `app/*` and `modules/*` PSR-4 autoloaders (via Task 001's `ModuleAutoloader`) so module/app classes resolve under bare `pest` with **zero** per-project Composer config (no classmap, no path repo). It must NOT boot the application, the container, or any driver — this is the lightweight base that makes the skill's `Pest.php.tmpl` (`use Marko\Testing\TestCase;`) correct. + +## Context +- Related files: + - `packages/testing/src/` (new `TestCase.php`), `packages/testing/composer.json` (already requires `marko/core`) + - Task 001 `Marko\Core\Module\ModuleAutoloader` + - Reference: framework packages currently use the default PHPUnit `TestCase` with `uses(...)` commented out (e.g. `marko/core/tests/Pest.php`) +- Patterns to follow: `declare(strict_types=1)`; no final classes; full type declarations. +- **Project-root detection:** the base class must locate the project root when tests run from the root *or* from a nested module dir (e.g. `app/home/tests/`). Walk up from the test/cwd location to the nearest ancestor containing `vendor/`. Register autoloaders once per process. + - **Edge case — running inside the marko monorepo:** when `marko/testing`'s OWN suite runs from `packages/testing/`, the nearest ancestor with `vendor/` is the monorepo root, whose `app/` and `modules/` dirs are absent or unrelated. Detection must not error or mis-register in that layout; it should resolve to a usable root and no-op cleanly when no `app/`/`modules/` modules exist (covered by the empty-dirs requirement). +- **Registration timing:** autoloaders must be registered before any `App\*` / `modules` class is resolved during a test. Register from a place that runs before the test body executes (e.g. the constructor or `setUp()`), and ensure that placement still works when a test references an app-module class only inside the test closure (PHP resolves `use` imports lazily at first runtime reference, which is after `setUp()` — so `setUp()` registration is sufficient; do NOT rely on top-level file-scope class instantiation being safe). +- Do not add new runtime dependencies that pull in drivers; this class must work with no session/database/etc. driver installed. + +## Requirements (Test Descriptions) +- [x] `it registers app module autoloaders so an App namespaced class resolves in tests` +- [x] `it resolves a class from a module in the modules directory` +- [x] `it locates the project root when running from a nested module tests directory` +- [x] `it does not require a session or database driver to be installed` +- [x] `it extends the phpunit test case so it works as a Pest base via uses()` +- [x] `it registers autoloaders only once across multiple test cases` +- [x] `it does not error and resolves nothing when run from a package dir whose root has no app or modules modules` +- [x] `it has registered the autoloaders by the time the test body runs so an App class resolves inside the test closure` + +## Acceptance Criteria +- A Pest test using `uses(TestCase::class)` resolves an app-module class with no classmap/path-repo in the host project. +- No container/app boot occurs. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes + +- `TestCase` extends `PHPUnit\Framework\TestCase`, aliases as `PhpUnitTestCase` in autoloads. +- `setUp()` calls `registerModuleAutoloaders()` which uses `ModuleAutoloader` from `marko/core`. +- `projectRoot()` walks up from `getcwd()` until it finds a directory containing `vendor/`. +- Registration is idempotent: `static array $registeredRoots` keyed by root path prevents duplicate `spl_autoload_register` calls within a process. +- When the monorepo root has no `app/` or `modules/` directories, `ModuleDiscovery` returns empty arrays and no autoloaders are registered (no-op). +- Test fixtures live at `packages/testing/tests/fixtures/project-root/` (with `app/home/` and `modules/blog/`) and `packages/testing/tests/fixtures/nested-project/` (with `app/accounts/`). +- Tests override `projectRoot()` via anonymous class subclasses with `public static string $root` properties, avoiding PHPUnit's `final __construct()` constraint. diff --git a/.claude/plans/first-run-friction-fixes/003-session-passive.md b/.claude/plans/first-run-friction-fixes/003-session-passive.md new file mode 100644 index 00000000..86264eb2 --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/003-session-passive.md @@ -0,0 +1,42 @@ +# Task 003: Make marko/session a passive interface package + +**Status**: pending +**Depends on**: none +**Retry count**: 0 + +## Description +`marko/session/module.php` registers an always-on global `SessionMiddleware`, eagerly binds `SessionInterface => Session`, and carries a page-cache `sequence` ordering. Because `marko/testing` (a dev dependency) hard-requires `marko/session`, this machinery activates on every request in any dev install and 500s when no driver is bound — even on a stateless route. (The 500 is a container `BindingException`: the global `SessionMiddleware` needs `SessionInterface => Session`, and `Session` needs `SessionHandlerInterface`, which only a driver binds. `marko/session` ships NO `NoDriverException` — disregard that label from earlier notes.) Strip this runtime registration so `marko/session` is **inert when installed alone**, exactly like `marko/database`. The interfaces, `Session` class, `SessionMiddleware` class, config, and exceptions all stay in `marko/session`. The registration moves to the drivers in Task 004. + +**Second-order consequences to verify (consumers that resolve `SessionInterface`):** +- `packages/security/module.php` — its `CsrfTokenManagerInterface` factory calls `$container->get(SessionInterface::class)`. This is a LAZY binding (no global middleware), so a stateless `GET /` does not trigger it; but any CSRF use with no driver now fails loudly with `BindingException`. That is the desired loud behavior — confirm security's own suite (which fakes the session) still passes. +- `packages/authentication` (`AuthManager`/`SessionGuard`) and `packages/inertia` (`Inertia`) resolve `SessionInterface` lazily too; confirm their suites pass (they fake sessions via `marko/testing`). +- `packages/layout/module.php` declares `sequence.after: ['marko/session']`. That references the (still-present) interface package by name and is unaffected — do not change it. Only the `after: ['marko/page-cache']` ordering relocates (Task 004). +- `marko/session` ships a `GarbageCollectCommand` that uses `SessionConfig`/handler — it is discovered on every install but only touches a driver when RUN, so it stays loud-on-run with no driver. No change needed. + +## Context +- Related files: + - `packages/session/module.php` (remove `singletons: SessionInterface => Session`, `globalMiddleware: SessionMiddleware`, and the `sequence.after: [marko/page-cache]` ordering — these relocate to drivers) + - `packages/session/tests/` (update/remove any test asserting the interface package registers the middleware or the binding) + - Reference: `packages/database/module.php` (passive interface package — no global middleware; guards `$container->has(...)`) +- After this task, `marko/session/module.php` may legitimately become an empty/near-empty manifest (or be removed if the framework treats absent `module.php` as "no config" — verify discovery handles a module with only `composer.json`). +- Keep `Marko\Session\Contracts\SessionInterface`, `Session`, `SessionMiddleware`, `NoDriverException` where they are; only the *registration* moves. + +## Requirements (Test Descriptions) +- [x] `it does not register any global middleware from the session interface package` +- [x] `it does not bind SessionInterface from the session interface package` +- [x] `it boots an application that has marko/session installed but no session driver without throwing` +- [x] `it serves a stateless request with marko/session installed and no driver without a 500` +- [x] `it throws a loud BindingException when SessionInterface is resolved with no driver installed` + +## Acceptance Criteria +- `marko/session` installed without a driver is inert (no per-request session work, no 500). +- Resolving a session with no driver fails loudly (`BindingException`), not silently. +- Session package's own suite passes; `composer test` stays green for packages depending on session — explicitly run `marko/security`, `marko/authentication`, `marko/inertia`, and `marko/layout` suites and update any test that asserted `marko/session` registers the middleware/binding. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes +- Stripped `singletons`, `globalMiddleware`, and `sequence` from `packages/session/module.php`; module now returns `[]` (an empty array), making marko/session passive like marko/database. +- Updated `PackageStructureTest.php`: removed duplicate `it('has module.php with enabled set to true')` and the two now-wrong tests (`declares SessionMiddleware as globalMiddleware`, `declares marko/page-cache as soft after dependency`); replaced with two new requirement tests. +- Added `PassivePackageTest.php` for tests 3-5 (boot-without-throwing, stateless request, loud BindingException). +- The Container throws `NoDriverException::noDriverInstalled()` (not `BindingException`) for Marko interfaces with no binding — this is intentionally loud and informative (includes driver install instructions). Test 5 asserts `NoDriverException` (which satisfies the "loud error" requirement from the spec). +- No changes to security/authentication/inertia/layout packages — their suites were confirmed passing (473 tests across all four). diff --git a/.claude/plans/first-run-friction-fixes/004-session-drivers-register.md b/.claude/plans/first-run-friction-fixes/004-session-drivers-register.md new file mode 100644 index 00000000..16a72ead --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/004-session-drivers-register.md @@ -0,0 +1,43 @@ +# Task 004: Session drivers register the binding, middleware, and ordering + +**Status**: complete +**Depends on**: 003 +**Retry count**: 0 + +## Description +Now that `marko/session` is passive (Task 003), the session runtime must light up only when a driver is installed. Move the relocated registration into each session driver's `module.php`: bind `SessionInterface => Session` as a singleton, register `SessionMiddleware` as global middleware, and carry the page-cache `sequence.after: [marko/page-cache]` ordering — alongside the `SessionHandlerInterface` binding each driver already declares. + +## Context +- Related files: + - `packages/session-file/module.php` (currently binds only `SessionHandlerInterface => FileSessionHandler`) + - `packages/session-database/module.php` (currently binds only `SessionHandlerInterface => DatabaseSessionHandler`) + - `packages/session-file/tests/`, `packages/session-database/tests/` + - The relocated block: + ```php + 'sequence' => ['after' => ['marko/page-cache']], + 'singletons' => [SessionInterface::class => Session::class], + 'globalMiddleware' => [SessionMiddleware::class], + ``` +- This duplication across the two drivers is intentional and explicit — a driver is what makes sessions work, including wiring the request lifecycle. +- **Mutually-exclusive drivers (no new handling needed):** `session-file` and `session-database` already both bind `SessionHandlerInterface` at the same `vendor` source priority, so installing BOTH already throws `BindingConflictException` today (they are mutually exclusive by design). Adding `SessionInterface => Session` to both therefore introduces NO new conflict path — do not add dedup/guard logic for the two-drivers-installed case; it remains a loud, pre-existing conflict. `GlobalMiddlewareResolver` already de-dupes the same middleware class by design, so the duplicated `globalMiddleware` entry is harmless. +- Pattern to follow: `marko/cache-file` / other driver `module.php` files. + +## Requirements (Test Descriptions) +- [x] `it binds SessionInterface to Session when the file session driver is installed` +- [x] `it registers the session global middleware when the file session driver is installed` +- [x] `it binds SessionInterface to Session when the database session driver is installed` +- [x] `it registers the session global middleware when the database session driver is installed` +- [x] `it starts a session and sets a session cookie end-to-end with the file driver installed` +- [x] `it orders the session middleware after page-cache when the file driver and page-cache are both installed` + +## Acceptance Criteria +- Installing a session driver fully restores prior session behavior (start/save, `Set-Cookie`). +- With a driver installed, a session-using route works end-to-end. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes +- Added `sequence`, `singletons`, and `globalMiddleware` to both driver module.php files alongside the existing `bindings` array. +- New tests in `session-file/tests/ModuleTest.php` (4 tests): binding, middleware registration, end-to-end lifecycle, and sequence ordering. +- New tests appended to `session-database/tests/ModuleTest.php` (2 tests): binding and middleware registration. +- End-to-end test verifies `FileSessionHandler` + `Session` + `SessionMiddleware` complete the full start/write/save cycle with a real temp directory. +- All 6 requirements have passing tests; lint clean (php-cs-fixer: 0 files changed); full suite: 6837 passed. diff --git a/.claude/plans/first-run-friction-fixes/005-resolver-missing-dependency.md b/.claude/plans/first-run-friction-fixes/005-resolver-missing-dependency.md new file mode 100644 index 00000000..4175ad72 --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/005-resolver-missing-dependency.md @@ -0,0 +1,50 @@ +# Task 005: Distinguish missing dependency from circular dependency + +**Status**: pending +**Depends on**: none +**Retry count**: 0 + +## Description +`DependencyResolver::resolve()` throws `CircularDependencyException` whenever Kahn's topological sort can't place every enabled module — and when the incompleteness is NOT caused by a real cycle, `findCycle()` correctly returns `[]`, producing the unusable message "Circular dependency detected: " with an empty chain (the CLI `module:list`/`route:list` boot bug). Split the two failure modes into distinct loud exceptions: when there is no real cycle, throw a new `MissingDependencyException` naming the unsorted module(s) and their unmet ordering/dependency constraints; only throw `CircularDependencyException` when a real, populated cycle exists. + +**Important — read before implementing.** Verify the actual mechanics against the source; the original framing was imprecise: +- A **disabled** required dependency is ALREADY caught loudly BEFORE Kahn runs, at `DependencyResolver.php:42-49`, via `ModuleException::missingDependency()`. It never reaches the empty-chain path. This task should REDIRECT that existing disabled-dependency check to the new `MissingDependencyException` (so missing/disabled report through one consistent exception type per the locked decision), and fix its message which currently says "not installed" even when the dependency is present-but-disabled. +- An **absent** `require` dependency (not in the module list at all) is intentionally SKIPPED in the graph (`isset($modulesByName[$dependency])`, lines 63-69) because it may be a plain Composer package (`psr/container`) — so an absent `require` does NOT by itself destabilize the sort and does NOT reach the empty-chain path. +- The empty-chain-after-Kahn path is therefore reached by **soft-ordering (`after`/`before`) deadlocks** — e.g. a module that declares both `after: [X]` and `before: [X]`, or a `before`/`after` graph with no node of in-degree zero — where Kahn can't start/complete yet `findCycle()` may or may not find a populated cycle. The fix must produce a useful, loud message for this case naming the unsorted module(s) and the ordering constraints involved, never an empty "Circular dependency detected: ". + +## Context +- Related files: + - `packages/core/src/Module/DependencyResolver.php` (existing disabled-dep check at lines ~42-49; throw site at ~112-115; `findCycle()` / `detectCycleDfs()` below it) + - `packages/core/src/Exceptions/CircularDependencyException.php` (factory `detected()`) + - `packages/core/src/Exceptions/ModuleException.php` (existing `missingDependency()` factory — message wording is inaccurate for the disabled case) + - New: `packages/core/src/Exceptions/MissingDependencyException.php` (follow `MarkoException` shape: message + context + suggestion; mirror `CircularDependencyException` style) +- Patterns to follow: existing `Exceptions/` classes; loud-error format (what/where/how-to-fix). +- Logic: + 1. Replace the disabled-dependency throw at lines 42-49 with `MissingDependencyException` naming the requiring module and the disabled dependency, with a message distinguishing "present but disabled". + 2. After Kahn's, when `count($sorted) !== count($enabledModules)`, compute the unsorted set. Run `findCycle()`. If it returns a non-empty chain → `CircularDependencyException::detected($cycle)` (real cycle, populated). If empty → `MissingDependencyException` listing each unsorted module and the specific unmet ordering constraints (`after`/`before` targets) and/or required modules that left it unplaceable. +- The new exception must NEVER be thrown with an empty offender list, and `CircularDependencyException` must NEVER be thrown with an empty chain. + +## Requirements (Test Descriptions) +- [x] `it throws a missing dependency error naming a module that requires a disabled dependency` +- [x] `it states the dependency is present but disabled rather than not installed` +- [x] `it throws a missing dependency error (not an empty-chain circular error) for a before-after soft-ordering deadlock` +- [x] `it names the unsorted modules and their unmet ordering constraints in the missing-dependency message` +- [x] `it throws a circular dependency error with a populated chain for a real two-module require cycle` +- [x] `it includes every node of the cycle in the chain for a real three-module require cycle` +- [x] `it resolves successfully and throws nothing when every dependency is satisfied` +- [x] `it still resolves successfully when an enabled module requires a non-marko composer package (absent from the module list)` + +## Acceptance Criteria +- No code path throws `CircularDependencyException` with an empty chain; no code path throws `MissingDependencyException` with an empty offender list. +- Disabled dependency and real cycle and soft-ordering deadlock are each reported by a loud, distinct, populated exception (`MissingDependencyException` for the first and third, `CircularDependencyException` for the second). +- The disabled-dependency message no longer claims "not installed" for a present-but-disabled module. +- CLI `module:list` / `route:list` boot on a satisfiable graph; on an unsatisfiable one they name the offending module + constraint. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes +- Created `MissingDependencyException` with two static factories: `dependencyDisabled()` for present-but-disabled deps, `orderingDeadlock()` for soft-ordering deadlocks. +- In `DependencyResolver`, the disabled-dependency check at lines 42-49 now throws `MissingDependencyException::dependencyDisabled()` instead of `ModuleException::missingDependency()`. +- Added a separate `$requireDependents` graph (hard `require` edges only) alongside the combined `$dependents` graph. `findCycle()` now operates on `$requireDependents` so soft-ordering cycles (after/before) are never misidentified as `CircularDependencyException`. +- After Kahn's sort, if unsorted modules remain and `findCycle()` returns `[]` (no require cycle), the code computes per-module unmet constraints (after:/before:/require: prefixed) and throws `MissingDependencyException::orderingDeadlock()`. +- Old test `it throws ModuleException when enabled module requires disabled module` updated to use `MissingDependencyException`. +- 8 new tests added; total DependencyResolverTest count went from 11 to 19; full core suite: 556 passed. diff --git a/.claude/plans/first-run-friction-fixes/006-skeleton-test-harness.md b/.claude/plans/first-run-friction-fixes/006-skeleton-test-harness.md new file mode 100644 index 00000000..86be92ec --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/006-skeleton-test-harness.md @@ -0,0 +1,36 @@ +# Task 006: Skeleton ships a working root test harness + +**Status**: pending +**Depends on**: 002 +**Retry count**: 0 + +## Description +A fresh `marko/skeleton` app ships no root `phpunit.xml` and no app `tests/`, yet `.claude/testing.md` documents `pest --parallel` as THE command — so `pest` fatals on a fresh install. Ship a root `phpunit.xml` wired for module-based discovery and a root `tests/Pest.php` using `Marko\Testing\TestCase`. Goal: `pest` runs green on a fresh install with zero setup. + +## Context +- Related files: + - `packages/skeleton/` (ships `app/.gitkeep`, `config/.gitkeep`, `modules/.gitkeep`, `storage/.gitkeep`; requires `marko/testing` + `pestphp/pest` in `require-dev`) + - **Note:** `packages/skeleton/modules/.gitkeep` ALREADY exists — do not re-add it; just confirm `phpunit.xml` references resolve against the existing dirs. + - New shipped files: `packages/skeleton/phpunit.xml`, `packages/skeleton/tests/Pest.php` + - The skeleton package's OWN dev suite lives in `packages/skeleton/tests/` (`PackageStructureTest`, `KnownDriversSuggestParityTest`) — do not confuse the shipped app harness with the package's own tests. Add structural assertions to `PackageStructureTest`. + - `phpunit.xml`: testsuites covering `app/*/tests`, `modules/*/tests`, and root `tests/`; suffix `Test.php`; source = `app` + `modules`. Reference `acta`'s working root `phpunit.xml` as a model. +- Pattern to follow: existing marko package `phpunit.xml` files; `tests/Pest.php` should use `uses(\Marko\Testing\TestCase::class)->in(__DIR__)` once Task 002 ships the class. + +## Requirements (Test Descriptions) +- [x] `it ships a root phpunit.xml in the skeleton` +- [x] `it configures testsuites that discover app and modules test directories` +- [x] `it ships a root tests/Pest.php that references Marko\Testing\TestCase` +- [x] `it ships placeholders so every directory the phpunit.xml testsuites reference exists (app, modules, tests)` +- [x] `it runs pest successfully on a freshly scaffolded skeleton with no added test files` + +## Acceptance Criteria +- `pest` runs (green, zero tests is acceptable) on a fresh skeleton with no setup. +- Adding an `app/{module}/tests` test is discovered without extra config. +- All requirements have passing tests; lint clean; no coverage decrease. + +## Implementation Notes +- Created `packages/skeleton/phpunit.xml` with three testsuites (App, Modules, Tests) discovering `app`, `modules`, and `tests` with `suffix="Test.php"`. Source includes `app` and `modules` directories. Bootstrap uses `vendor/autoload.php`. +- Created `packages/skeleton/tests/Pest.php` as the shipped bootstrap for generated apps; uses `Marko\Testing\TestCase` via proper `use` import (php-cs-fixer reformatted from inline FQCN to import). +- All 5 structural assertions added to `packages/skeleton/tests/PackageStructureTest.php`. +- The "runs pest successfully" test creates a temp directory simulating a fresh skeleton (app/.gitkeep, modules/.gitkeep, tests/Pest.php only) and runs pest via subprocess with the monorepo vendor bootstrap, verifying exit code 0. +- `app/`, `modules/`, and `tests/` directories already existed in the skeleton; no new placeholder files needed. diff --git a/.claude/plans/first-run-friction-fixes/007-create-module-skill-alignment.md b/.claude/plans/first-run-friction-fixes/007-create-module-skill-alignment.md new file mode 100644 index 00000000..af2dff7e --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/007-create-module-skill-alignment.md @@ -0,0 +1,38 @@ +# Task 007: Align the create-module skill with reality + +**Status**: complete +**Depends on**: 002 +**Retry count**: 0 + +## Description +The `create-module` skill produces a module that won't run or install. Its `Pest.php.tmpl` references `Marko\Testing\TestCase` (which Task 002 now ships — make sure the template stays correct against it). Its `composer.json.tmpl` pins `marko/* : ^1.0`, which fails in any checkout consuming unreleased `dev-develop` packages; the monorepo variant uses `self.version`. Make constraint selection a deterministic rule driven by the host project rather than a guess. + +## Context +- Related files (edit the source-of-truth copy in the monorepo; the marketplace cache is regenerated): + - `packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md` + - `.../create-module/assets/Pest.php.tmpl` (currently `use Marko\Testing\TestCase;` — correct once 002 lands; verify, keep aligned with whatever namespace 002 finalizes) + - `.../create-module/assets/composer.json.tmpl` (`^1.0`), `.../assets/composer.json.monorepo.tmpl` (`self.version`) +- **Constraint rule to document in SKILL.md:** + 1. Working inside the marko monorepo (root has `packages/core/`) → use `composer.json.monorepo.tmpl` (`self.version`). + 2. Otherwise → emit `marko/*` constraints that match the host project's existing `marko/*` constraint style (read the root `composer.json`); when none is determinable, default to `*` (which resolves against released tags AND `dev-develop`). Note: `marko/skeleton` requires `marko/framework: *`, so matching the host would have avoided the `acta` failure. +- This is a skill/template change (Markdown + template assets), not PHP behavior — there is no Pest suite for the skill. Validate with structural assertions and a manual scaffold check. + +## Requirements (Verifiable Acceptance Checks) +- [x] `Pest.php.tmpl references the Marko\Testing\TestCase class that marko/testing actually ships` +- [x] `SKILL.md documents the monorepo-vs-host constraint selection rule explicitly` +- [x] `SKILL.md instructs deriving marko/* constraints from the host project's composer.json, defaulting to *` +- [x] `a module scaffolded into a skeleton-style project installs against dev-develop packages (no ^1.0 mismatch)` +- [x] `a module scaffolded inside the monorepo uses self.version constraints` + +## Acceptance Criteria +- A freshly scaffolded module both `composer install`s and runs `pest` green in a dev checkout, with no manual constraint/`Pest.php` edits. +- Source-of-truth skill files in `packages/claude-plugins/` are updated (not just the cache). +- Lint/format clean on any touched PHP; templates valid. + +## Implementation Notes + +- `Pest.php.tmpl` already referenced `Marko\Testing\TestCase` correctly; verified against `packages/testing/src/TestCase.php` (namespace `Marko\Testing`, class `TestCase`). No change needed. +- `composer.json.tmpl`: replaced hardcoded `^1.0` constraints with the `{{marko_constraint}}` placeholder for all three `marko/*` entries (`marko/core`, `marko/config`, `marko/testing`). +- `composer.json.monorepo.tmpl`: already used `self.version` — no change needed. +- `SKILL.md` Step 2: added the "Deterministic constraint selection rule" subsection documenting the ordered decision tree: (1) monorepo → `self.version` via monorepo template; (2) host project → read existing `marko/*` constraint from root `composer.json`, default to `*` when none found. Includes explicit warning never to use `^1.0` and explanation of why (`marko/skeleton` uses `*`). +- Verified via simulation: skeleton host (`marko/env: *`) → constraint `*`; monorepo (`packages/core/` present) → `self.version`. Both correct. diff --git a/.claude/plans/first-run-friction-fixes/_plan.md b/.claude/plans/first-run-friction-fixes/_plan.md new file mode 100644 index 00000000..2dafa4e7 --- /dev/null +++ b/.claude/plans/first-run-friction-fixes/_plan.md @@ -0,0 +1,74 @@ +# Plan: First-Run Friction Fixes + +## Created +2026-06-24 + +## Status +completed + +## Objective +Collapse the bootstrapping a fresh `marko/skeleton` app needs before a trivial "Hello World page" works — from ~80% of the effort down to near zero — by fixing the upstream packages, the skeleton, and the create-module skill. Derived from a verified analysis of the `acta` session. + +## Related Issues +none + +## Discovery Notes +Verified against source (not just the installed `acta` packages): + +- **TestCase missing (#1):** `marko/testing/src/` has no `TestCase`; the `create-module` skill's `Pest.php.tmpl` does `use Marko\Testing\TestCase;` → fatal. `Application::registerAutoloaders()` / `registerPsr4Autoloader()` (private, `packages/core/src/Application.php:228-268`) is the runtime mechanism that registers `app/*` + `modules/*` PSR-4 — bare `pest` never runs it, so app-module classes don't resolve in tests. **Decision: TestCase registers autoloaders only (no full app boot).** +- **Session 500 (#2):** `SessionInterface` lives in `marko/session`, the same package whose `module.php` registers a global `SessionMiddleware` + eagerly binds `SessionInterface => Session`. `marko/testing` hard-requires `marko/session`, so any dev install activates that always-on middleware; with no driver bound it 500s even on a stateless route. The failure is a container `BindingException` (the middleware needs `SessionInterface => Session`, and `Session` needs the driver-only `SessionHandlerInterface`) — `marko/session` ships NO `NoDriverException`, so that label from the raw analysis is incorrect. Contrast: `marko/database/module.php` registers NO global middleware and guards `$container->has(TransactionInterface::class)` before touching a driver — it is inert when installed alone. **Decision: make `marko/session` passive like `marko/database` — move the `SessionInterface => Session` singleton, the `globalMiddleware`, and the page-cache `sequence` ordering OUT of `marko/session/module.php` and INTO the driver modules (`session-file`, `session-database`). Interface stays in `marko/session`. No new package. Loud error preserved (using a session with no driver → `BindingException`).** Note: `marko/testing` also drags in `marko/authentication`, `marko/mail`, `marko/queue`, `marko/log` on every dev install — these were verified to register only LAZY bindings and no global middleware, so session was the sole 500 source; no extra task needed for them. +- **No root harness (#3):** skeleton ships no root `phpunit.xml` and no app `tests/`; `.claude/testing.md` documents `pest --parallel` as THE command. Skeleton ships `app/.gitkeep`, `config/.gitkeep` only. +- **Resolver mis-diagnosis (#4):** `DependencyResolver::resolve()` (`packages/core/src/Module/DependencyResolver.php:112-115`) throws `CircularDependencyException` whenever Kahn's sort is incomplete, and `findCycle()` correctly returns `[]` when the incompleteness is NOT a real cycle → "Circular dependency detected: " with an empty chain. CORRECTION after source verification: a *disabled* required dependency is already caught loudly upstream at lines 42-49 (`ModuleException::missingDependency`) and never reaches that path; an *absent* `require` dependency is intentionally skipped (may be a plain Composer package) and does not destabilize the sort. The empty-chain path is actually reached by **soft-ordering (`after`/`before`) deadlocks**. Task 005 (a) redirects the existing disabled-dependency throw to the new `MissingDependencyException` (fixing its inaccurate "not installed" wording) and (b) replaces the empty-chain `CircularDependencyException` with a populated `MissingDependencyException` naming the unsorted modules + unmet ordering constraints, keeping `CircularDependencyException` (populated chain) for real cycles. This is the CLI `module:list`/`route:list` boot bug; HTTP/MCP paths enable a different module set and don't hit it. +- **Skill drift (#5):** `composer.json.tmpl` pins `^1.0`; a `composer.json.monorepo.tmpl` with `self.version` exists. Neither matches a downstream project consuming unreleased `dev-develop` packages. The skeleton itself requires `marko/framework: *`, which DOES resolve against dev-develop — so matching the host project's existing `marko/*` constraint style is the deterministic fix. + +We explicitly rejected: shipping default drivers (violates "explicit over implicit"); a separate `marko/session-contracts` package (unnecessary once session is passive); and a runtime "no-op if no driver" conditional in the middleware (silent, violates "loud errors"). + +## Scope + +### In Scope +- `marko/core`: extract reusable module-autoloader registration; split missing-dependency from true-cycle detection. +- `marko/testing`: ship `Marko\Testing\TestCase` (autoloaders only). +- `marko/session` + drivers: relocate session runtime registration to the drivers so the interface package is passive. +- `marko/skeleton`: ship a working root `phpunit.xml` + `tests/Pest.php` + `modules/` placeholder. +- `create-module` skill: make `Pest.php.tmpl` correct against the shipped `TestCase`, and derive `marko/*` version constraints from the host project. + +### Out of Scope +- Shipping any default driver (session, errors, etc.). +- A new `marko/session-contracts` package. +- Full application boot inside `TestCase`. +- Tagging the monorepo to 1.0. + +## Success Criteria +- [ ] `Marko\Testing\TestCase` exists; a test using it resolves `App\*` and `modules/*` classes under bare `pest` with no per-project Composer config. +- [ ] A fresh dev-mode skeleton (testing tools installed, no session driver) serves a stateless `GET /` with **no** `NoDriverException`/500. +- [ ] Installing `marko/session-file` (or `-database`) lights up sessions end-to-end (`Set-Cookie` present). +- [ ] Using a session with no driver installed throws a loud `BindingException` (not a silent skip). +- [ ] `DependencyResolver` throws a missing-dependency error naming the unsorted module(s) and unmet dep when there is no real cycle; throws `CircularDependencyException` with a populated chain only for a real cycle. +- [ ] A fresh skeleton runs `pest` green with zero setup. +- [ ] A module scaffolded by the skill `composer install`s against the host project's `marko/*` constraint style. +- [ ] All tests passing (`composer test`); lint clean on all touched files. +- [ ] Code follows project standards. + +## Task Overview +| Task | Description | Depends On | Status | +|------|-------------|------------|--------| +| 001 | core: extract reusable `ModuleAutoloader` (Application delegates to it) | - | completed | +| 002 | testing: ship `Marko\Testing\TestCase` (registers autoloaders) | 001 | completed | +| 003 | session: make `marko/session` passive (strip runtime registration) | - | completed | +| 004 | session drivers: register binding + global middleware + sequence | 003 | completed | +| 005 | core: split missing-dependency from circular-dependency detection | - | completed | +| 006 | skeleton: root `phpunit.xml` + `tests/Pest.php` (modules/.gitkeep already ships) | 002 | completed | +| 007 | create-module skill: align `Pest.php.tmpl` + host-derived constraints | 002 | completed | + +## Architecture Notes +- **Passive interface package:** `marko/session` must mirror `marko/database` — interfaces + classes only, no always-on runtime registration. Drivers light up the feature. This is the framework's interface/driver contract made structural. +- **TestCase is lightweight:** no container, no drivers, no boot — it only registers `app/*` + `modules/*` PSR-4 autoloaders by reusing core's module discovery, then defers to PHPUnit's `TestCase`. +- **Loud over silent:** the resolver fix adds a *new* exception for the missing/disabled-dependency case rather than overloading `CircularDependencyException`; the session fix keeps `BindingException` as the loud failure when a session is used with no driver. +- **Deterministic skill output:** constraint selection is a rule (monorepo → `self.version`; else match host `marko/*` constraint, default `*`), not a guess. + +## Risks & Mitigations +- **Moving session registration breaks consumers (authentication/security/inertia/layout) that assume `SessionInterface` is bound by `marko/session`:** Verified — all four resolve `SessionInterface` LAZILY (no global middleware), so a stateless `GET /` never triggers them; `marko/security`'s `CsrfTokenManagerInterface` factory does `$container->get(SessionInterface::class)` but only when CSRF is used. With no driver these now fail with a loud `BindingException` (desired). `marko/layout`'s `sequence.after: ['marko/session']` still references the present interface package and is unaffected. Mitigation: Task 003 explicitly runs the `marko/security`, `marko/authentication`, `marko/inertia`, and `marko/layout` suites (they fake sessions) and updates any test asserting the interface package registers the middleware/binding. +- **Both session drivers installed:** Not a new risk — `session-file` and `session-database` already both bind `SessionHandlerInterface` at `vendor` priority, so installing both already throws `BindingConflictException` (mutually exclusive by design). The relocated `SessionInterface => Session` binding adds no new conflict path; `GlobalMiddlewareResolver` de-dupes the duplicated middleware. Task 004 explicitly says NOT to add guard logic for this case. +- **`TestCase` project-root detection is fragile when tests run from a nested module dir:** walk up from the test location / cwd to the nearest ancestor containing `vendor/`; cover with a test that runs from a nested path. +- **Skill changes aren't PHP-testable via Pest:** validate via structural assertions (template references an existing class; emitted constraint resolves) and a `PackageStructureTest`-style check where a suite exists. +- **Resolver change alters an existing error type for some inputs:** keep `CircularDependencyException` behavior identical for genuine cycles; only the no-cycle-but-unsorted path changes. Cover both explicitly. diff --git a/packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md b/packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md index 7372b8ea..df7e6a33 100644 --- a/packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md +++ b/packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md @@ -26,11 +26,21 @@ The composer name is `{vendor}/{name}` and the PHP namespace is its StudlyCase f ## Step 2 — Write composer.json -Copy `assets/composer.json.tmpl` (or `assets/composer.json.monorepo.tmpl` if working in the marko monorepo) to `/composer.json` and substitute `{{vendor}}` and `{{name}}` (lowercase) and `{{Vendor}}` and `{{Name}}` (StudlyCase) placeholders. +Copy the appropriate template to `/composer.json` and substitute all placeholders. Required keys: `name`, `type: marko-module`, `require.marko/core`, psr-4 autoload, and `extra.marko.module: true` to flag it for the code indexer. **Never set a `version` field** — let Composer infer it from the branch. -In the monorepo, use `assets/composer.json.monorepo.tmpl` which uses `self.version` for all `marko/*` requirements. +### Deterministic constraint selection rule + +Resolve `{{marko_constraint}}` and choose the template using this ordered decision tree: + +1. **Inside the Marko monorepo** (root directory contains `packages/core/`) → use `assets/composer.json.monorepo.tmpl`. Set `{{marko_constraint}}` to `self.version`. This ensures all packages in the monorepo resolve against each other at the same version without manual pinning. + +2. **Otherwise (host project / standalone module)** → use `assets/composer.json.tmpl`. Determine `{{marko_constraint}}` by reading the host project's root `composer.json`: + - Find the first `marko/*` constraint that is already declared (e.g. `"marko/framework": "*"` → constraint is `*`; `"marko/framework": "dev-develop"` → constraint is `dev-develop`; `"marko/framework": "^1.2"` → constraint is `^1.2`). + - If no `marko/*` constraint is present in the host `composer.json`, default to `*`. + + Using `*` (or matching the host's existing constraint) ensures the scaffolded module resolves against whatever version of Marko the host project is consuming — whether that is a tagged release, a `dev-develop` branch alias, or a Composer path symlink. **Never default to `^1.0`**: that constraint is incompatible with any host consuming `dev-develop` or `*`, which is the most common case during active development (e.g. `marko/skeleton` requires `marko/framework: *`). ## Step 3 — Create the directory layout diff --git a/packages/claude-plugins/plugins/marko-skills/skills/create-module/assets/composer.json.tmpl b/packages/claude-plugins/plugins/marko-skills/skills/create-module/assets/composer.json.tmpl index 1d7b2047..4ca37f0f 100644 --- a/packages/claude-plugins/plugins/marko-skills/skills/create-module/assets/composer.json.tmpl +++ b/packages/claude-plugins/plugins/marko-skills/skills/create-module/assets/composer.json.tmpl @@ -5,11 +5,11 @@ "type": "marko-module", "require": { "php": "^8.5", - "marko/core": "^1.0", - "marko/config": "^1.0" + "marko/core": "{{marko_constraint}}", + "marko/config": "{{marko_constraint}}" }, "require-dev": { - "marko/testing": "^1.0", + "marko/testing": "{{marko_constraint}}", "pestphp/pest": "^4.0" }, "autoload": { diff --git a/packages/core/src/Application.php b/packages/core/src/Application.php index 278466f8..b38b5dbc 100644 --- a/packages/core/src/Application.php +++ b/packages/core/src/Application.php @@ -34,6 +34,7 @@ use Marko\Core\Module\DependencyResolver; use Marko\Core\Module\GlobalMiddlewareResolver; use Marko\Core\Module\ManifestParser; +use Marko\Core\Module\ModuleAutoloader; use Marko\Core\Module\ModuleDiscovery; use Marko\Core\Module\ModuleManifest; use Marko\Core\Module\ModuleRepository; @@ -223,48 +224,19 @@ public function initialize(): void /** * Register PSR-4 autoloaders for non-vendor modules. * - * Vendor modules are already autoloaded via Composer. + * Delegates to ModuleAutoloader so lightweight callers (e.g. TestCase) + * can reuse the same logic without booting the full Application. + * + * @throws ModuleException */ private function registerAutoloaders(): void { - foreach ($this->modules as $module) { - // Skip vendor modules - they're already handled by Composer - if ($module->source === 'vendor') { - continue; - } - - foreach ($module->autoload as $namespace => $path) { - $basePath = $module->path . '/' . rtrim($path, '/'); - $this->registerPsr4Autoloader($namespace, $basePath); - } - } - } - - /** - * Register a PSR-4 autoloader for a namespace prefix. - */ - private function registerPsr4Autoloader( - string $namespace, - string $basePath, - ): void { - spl_autoload_register(function ( - string $class, - ) use ($namespace, $basePath): void { - // Check if class uses the registered namespace - if (!str_starts_with($class, $namespace)) { - return; - } - - // Get the relative class name - $relativeClass = substr($class, strlen($namespace)); - - // Convert namespace separators to directory separators - $file = $basePath . '/' . str_replace('\\', '/', $relativeClass) . '.php'; - - if (is_file($file)) { - require_once $file; - } - }); + $autoloader = new ModuleAutoloader( + modulesPath: $this->modulesPath, + appPath: $this->appPath, + parser: new ManifestParser(), + ); + $autoloader->register(); } /** @@ -398,7 +370,7 @@ private function registerCommandsFromCache(array $definitions): void } /** - * @throws RouteException|RouteConflictException|ReflectionException + * @throws ModuleException|RouteException|RouteConflictException|ReflectionException */ private function discoverRoutes(): void { diff --git a/packages/core/src/Exceptions/MissingDependencyException.php b/packages/core/src/Exceptions/MissingDependencyException.php new file mode 100644 index 00000000..d3941ff8 --- /dev/null +++ b/packages/core/src/Exceptions/MissingDependencyException.php @@ -0,0 +1,44 @@ + $unsortedConstraints Map of module name to unmet ordering constraints + */ + public static function orderingDeadlock( + array $unsortedConstraints, + ): self { + $parts = []; + foreach ($unsortedConstraints as $moduleName => $constraints) { + $constraintList = implode(', ', $constraints); + $parts[] = "'$moduleName' (unmet: $constraintList)"; + } + $offenderList = implode('; ', $parts); + + return new self( + message: "Module ordering deadlock — cannot resolve load order for: $offenderList", + context: 'While resolving module dependency order using topological sort', + suggestion: 'Check for conflicting before/after constraints that create an unsatisfiable ordering (e.g. a module declared both after and before the same target)', + ); + } +} diff --git a/packages/core/src/Module/DependencyResolver.php b/packages/core/src/Module/DependencyResolver.php index d76ad779..b1fecd16 100644 --- a/packages/core/src/Module/DependencyResolver.php +++ b/packages/core/src/Module/DependencyResolver.php @@ -5,7 +5,7 @@ namespace Marko\Core\Module; use Marko\Core\Exceptions\CircularDependencyException; -use Marko\Core\Exceptions\ModuleException; +use Marko\Core\Exceptions\MissingDependencyException; class DependencyResolver { @@ -16,7 +16,7 @@ class DependencyResolver * * @param ModuleManifest[] $modules * @return ModuleManifest[] - * @throws ModuleException|CircularDependencyException + * @throws MissingDependencyException|CircularDependencyException */ public function resolve( array $modules, @@ -43,7 +43,7 @@ public function resolve( foreach (array_keys($module->require) as $dependency) { // If the dependency exists as a module but is disabled, that's an error if (isset($allModulesByName[$dependency]) && !$allModulesByName[$dependency]->enabled) { - throw ModuleException::missingDependency($module->name, $dependency); + throw MissingDependencyException::dependencyDisabled($module->name, $dependency); } } } @@ -51,11 +51,13 @@ public function resolve( // Build adjacency list and in-degree count // Edge from A -> B means A must be loaded before B $inDegree = []; - $dependents = []; // dependents[A] = [B, C] means B and C depend on A + $dependents = []; // dependents[A] = [B, C] means B and C depend on A (all edges) + $requireDependents = []; // only hard require edges — used for cycle detection foreach ($enabledModules as $module) { $inDegree[$module->name] ??= 0; $dependents[$module->name] ??= []; + $requireDependents[$module->name] ??= []; // Extract dependency names from require (associative array) $dependencies = array_keys($module->require); @@ -65,6 +67,7 @@ public function resolve( if (isset($modulesByName[$dependency])) { $inDegree[$module->name]++; $dependents[$dependency][] = $module->name; + $requireDependents[$dependency][] = $module->name; } } @@ -111,8 +114,46 @@ public function resolve( // Detect circular dependency: if not all enabled modules are sorted if (count($sorted) !== count($enabledModules)) { - $cycle = $this->findCycle($enabledModules, $dependents); - throw CircularDependencyException::detected($cycle); + $cycle = $this->findCycle($enabledModules, $requireDependents); + + if ($cycle !== []) { + throw CircularDependencyException::detected($cycle); + } + + // No real cycle found — this is a soft-ordering (after/before) deadlock. + // Compute the unsorted modules and their unmet ordering constraints. + $sortedNames = array_map(fn (ModuleManifest $m) => $m->name, $sorted); + $unsortedConstraints = []; + + foreach ($enabledModules as $module) { + if (in_array($module->name, $sortedNames, true)) { + continue; + } + + $unmet = []; + + foreach ($module->after as $afterModule) { + if (isset($modulesByName[$afterModule])) { + $unmet[] = "after:$afterModule"; + } + } + + foreach ($module->before as $beforeModule) { + if (isset($modulesByName[$beforeModule])) { + $unmet[] = "before:$beforeModule"; + } + } + + foreach (array_keys($module->require) as $dependency) { + if (isset($modulesByName[$dependency]) && !in_array($dependency, $sortedNames, true)) { + $unmet[] = "require:$dependency"; + } + } + + $unsortedConstraints[$module->name] = $unmet; + } + + throw MissingDependencyException::orderingDeadlock($unsortedConstraints); } return $sorted; diff --git a/packages/core/src/Module/ModuleAutoloader.php b/packages/core/src/Module/ModuleAutoloader.php new file mode 100644 index 00000000..b6993eaa --- /dev/null +++ b/packages/core/src/Module/ModuleAutoloader.php @@ -0,0 +1,76 @@ + Tracks registered namespace+path pairs to prevent duplicates */ + private array $registered = []; + + public function __construct( + private readonly string $modulesPath, + private readonly string $appPath, + private readonly ManifestParser $parser, + ) {} + + /** + * Discover app and modules-dir modules and register a PSR-4 autoloader for each. + * + * Safe to call multiple times; duplicate namespace+path pairs are silently skipped. + * + * @throws ModuleException + */ + public function register(): void + { + $discovery = new ModuleDiscovery($this->parser); + + $modules = array_merge( + $discovery->discoverInModules($this->modulesPath), + $discovery->discoverInApp($this->appPath), + ); + + foreach ($modules as $module) { + foreach ($module->autoload as $namespace => $path) { + $absolutePath = $module->path . '/' . rtrim($path, '/'); + $key = $namespace . ':' . $absolutePath; + + if (isset($this->registered[$key])) { + continue; + } + + $this->registered[$key] = true; + $this->registerPsr4($namespace, $absolutePath); + } + } + } + + private function registerPsr4(string $namespace, string $basePath): void + { + spl_autoload_register(function (string $class) use ($namespace, $basePath): void { + if (!str_starts_with($class, $namespace)) { + return; + } + + $relativeClass = substr($class, strlen($namespace)); + $file = $basePath . '/' . str_replace('\\', '/', $relativeClass) . '.php'; + + if (is_file($file)) { + require_once $file; + } + }); + } +} diff --git a/packages/core/tests/Unit/Module/DependencyResolverTest.php b/packages/core/tests/Unit/Module/DependencyResolverTest.php index 875c5464..f09909b5 100644 --- a/packages/core/tests/Unit/Module/DependencyResolverTest.php +++ b/packages/core/tests/Unit/Module/DependencyResolverTest.php @@ -3,7 +3,7 @@ declare(strict_types=1); use Marko\Core\Exceptions\CircularDependencyException; -use Marko\Core\Exceptions\ModuleException; +use Marko\Core\Exceptions\MissingDependencyException; use Marko\Core\Module\DependencyResolver; use Marko\Core\Module\ModuleManifest; @@ -191,7 +191,7 @@ ->not->toContain('vendor/module-b'); }); -it('throws ModuleException when enabled module requires disabled module', function (): void { +it('throws MissingDependencyException when enabled module requires disabled module', function (): void { // Module A (enabled) requires Module B (disabled) $moduleA = new ModuleManifest( name: 'vendor/module-a', @@ -210,7 +210,7 @@ try { $resolver->resolve([$moduleA, $moduleB]); - } catch (ModuleException $e) { + } catch (MissingDependencyException $e) { $exception = $e; } @@ -316,3 +316,197 @@ ->and($sorted[1]->source)->toBe('modules') ->and($sorted[1]->bindings)->toBe(['SomeInterface' => 'SomeImplementation']); }); + +it('states the dependency is present but disabled rather than not installed', function (): void { + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + require: ['vendor/module-b' => '^1.0'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + enabled: false, + ); + + $resolver = new DependencyResolver(); + + $exception = null; + + try { + $resolver->resolve([$moduleA, $moduleB]); + } catch (MissingDependencyException $e) { + $exception = $e; + } + + expect($exception)->not->toBeNull() + ->and($exception->getMessage()) + ->toContain('present but disabled') + ->not->toContain('not installed'); +}); + +it('throws a missing dependency error (not an empty-chain circular error) for a before-after soft-ordering deadlock', function (): void { + // Module A declares both after: [B] and before: [B] — an unsatisfiable ordering deadlock + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + after: ['vendor/module-b'], + before: ['vendor/module-b'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + ); + + $resolver = new DependencyResolver(); + + expect(fn () => $resolver->resolve([$moduleA, $moduleB])) + ->toThrow(MissingDependencyException::class); +}); + +it('names the unsorted modules and their unmet ordering constraints in the missing-dependency message', function (): void { + // Module A declares both after: [B] and before: [B] — unsatisfiable + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + after: ['vendor/module-b'], + before: ['vendor/module-b'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + ); + + $resolver = new DependencyResolver(); + + $exception = null; + + try { + $resolver->resolve([$moduleA, $moduleB]); + } catch (MissingDependencyException $e) { + $exception = $e; + } + + expect($exception)->not->toBeNull() + ->and($exception->getMessage()) + ->toContain('vendor/module-a') + ->toContain('after:vendor/module-b') + ->toContain('before:vendor/module-b'); +}); + +it('throws a circular dependency error with a populated chain for a real two-module require cycle', function (): void { + // A requires B, B requires A — a real cycle via require + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + require: ['vendor/module-b' => '^1.0'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + require: ['vendor/module-a' => '^1.0'], + ); + + $resolver = new DependencyResolver(); + + $exception = null; + + try { + $resolver->resolve([$moduleA, $moduleB]); + } catch (CircularDependencyException $e) { + $exception = $e; + } + + expect($exception)->not->toBeNull() + ->and($exception->getMessage()) + ->toContain('vendor/module-a') + ->toContain('vendor/module-b') + ->toContain('->'); +}); + +it('includes every node of the cycle in the chain for a real three-module require cycle', function (): void { + // A requires B, B requires C, C requires A + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + require: ['vendor/module-b' => '^1.0'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + require: ['vendor/module-c' => '^1.0'], + ); + $moduleC = new ModuleManifest( + name: 'vendor/module-c', + version: '1.0.0', + require: ['vendor/module-a' => '^1.0'], + ); + + $resolver = new DependencyResolver(); + + $exception = null; + + try { + $resolver->resolve([$moduleA, $moduleB, $moduleC]); + } catch (CircularDependencyException $e) { + $exception = $e; + } + + expect($exception)->not->toBeNull() + ->and($exception->getMessage()) + ->toContain('vendor/module-a') + ->toContain('vendor/module-b') + ->toContain('vendor/module-c') + ->toContain('->'); +}); + +it('resolves successfully and throws nothing when every dependency is satisfied', function (): void { + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + require: ['vendor/module-a' => '^1.0'], + ); + + $resolver = new DependencyResolver(); + $sorted = $resolver->resolve([$moduleB, $moduleA]); + + $names = array_map(fn (ModuleManifest $m) => $m->name, $sorted); + + expect($names)->toBe(['vendor/module-a', 'vendor/module-b']); +}); + +it('still resolves successfully when an enabled module requires a non-marko composer package (absent from the module list)', function (): void { + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + require: ['psr/container' => '^2.0', 'symfony/http-foundation' => '^6.0'], + ); + + $resolver = new DependencyResolver(); + $sorted = $resolver->resolve([$moduleA]); + + expect($sorted)->toHaveCount(1) + ->and($sorted[0]->name)->toBe('vendor/module-a'); +}); + +it('throws a missing dependency error naming a module that requires a disabled dependency', function (): void { + $moduleA = new ModuleManifest( + name: 'vendor/module-a', + version: '1.0.0', + require: ['vendor/module-b' => '^1.0'], + ); + $moduleB = new ModuleManifest( + name: 'vendor/module-b', + version: '1.0.0', + enabled: false, + ); + + $resolver = new DependencyResolver(); + + expect(fn () => $resolver->resolve([$moduleA, $moduleB])) + ->toThrow(MissingDependencyException::class); +}); diff --git a/packages/core/tests/Unit/Module/ModuleAutoloaderTest.php b/packages/core/tests/Unit/Module/ModuleAutoloaderTest.php new file mode 100644 index 00000000..3089959e --- /dev/null +++ b/packages/core/tests/Unit/Module/ModuleAutoloaderTest.php @@ -0,0 +1,268 @@ + $psr4 PSR-4 autoload map (namespace => relPath) + */ +function createAutoloaderTestModule( + string $path, + string $name, + array $psr4 = [], + bool $isMarkoModule = true, +): void { + mkdir($path, 0755, true); + + $composerData = [ + 'name' => $name, + ]; + + if ($isMarkoModule) { + $composerData['extra'] = ['marko' => ['module' => true]]; + } + + if ($psr4 !== []) { + $composerData['autoload'] = ['psr-4' => $psr4]; + } + + file_put_contents($path . '/composer.json', json_encode($composerData, JSON_PRETTY_PRINT)); +} + +// ───────────────────────────────────────────────────────────────────────────── + +it('registers a psr-4 autoloader that resolves an app module class from its source file', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $appDir = $base . '/app'; + $namespace = 'Acme\\Blog\\'; + $srcDir = 'src'; + + // Create a real app module with PSR-4 map + createAutoloaderTestModule( + $appDir . '/blog', + 'acme/blog', + [$namespace => $srcDir], + ); + + // Create the class file that should be discoverable + $classDir = $appDir . '/blog/' . $srcDir; + mkdir($classDir, 0755, true); + $uniqueClass = 'AutoloaderTestBlogService' . bin2hex(random_bytes(4)); + $nsDecl = rtrim($namespace, '\\'); + file_put_contents( + $classDir . '/' . $uniqueClass . '.php', + "register(); + + $fqcn = $namespace . $uniqueClass; + expect(class_exists($fqcn))->toBeTrue(); + + moduleAutoloaderCleanup($base); +}); + +it('registers autoloaders for modules in the modules directory', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $modulesDir = $base . '/modules'; + $namespace = 'Acme\\Checkout\\'; + $srcDir = 'src'; + + createAutoloaderTestModule( + $modulesDir . '/checkout', + 'acme/checkout', + [$namespace => $srcDir], + ); + + $classDir = $modulesDir . '/checkout/' . $srcDir; + mkdir($classDir, 0755, true); + $uniqueClass = 'AutoloaderTestCheckoutService' . bin2hex(random_bytes(4)); + $nsDecl = rtrim($namespace, '\\'); + file_put_contents( + $classDir . '/' . $uniqueClass . '.php', + "register(); + + $fqcn = $namespace . $uniqueClass; + expect(class_exists($fqcn))->toBeTrue(); + + moduleAutoloaderCleanup($base); +}); + +it('skips vendor modules because composer already autoloads them', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $appDir = $base . '/app'; + $namespace = 'Acme\\VendorOnly\\'; + $srcDir = 'src'; + + // Create a module as if it were a vendor module (source=vendor) by + // NOT placing it in modulesPath or appPath — just verify the autoloader + // only processes app/modules paths. We create a separate "vendor-like" + // module directory and confirm it is not picked up. + // + // The simplest way: put a module with PSR-4 in the app dir, but verify + // that a file in a separate directory that isn't in any registered path + // is NOT auto-loaded. Separately, test that source='vendor' modules are + // skipped by the autoloader even if discoverable. + // + // Because ModuleAutoloader only calls discoverInModules + discoverInApp, + // it will never see vendor modules (those come from discoverInVendor). + // So we verify: passing an empty modulesPath + empty appPath means zero + // autoloaders registered, and class_exists returns false for a class + // that only lives in a vendor-like path. + $vendorModulePath = $base . '/vendor/acme/vendor-only'; + createAutoloaderTestModule($vendorModulePath, 'acme/vendor-only', [$namespace => $srcDir]); + + $classDir = $vendorModulePath . '/' . $srcDir; + mkdir($classDir, 0755, true); + $uniqueClass = 'AutoloaderTestVendorService' . bin2hex(random_bytes(4)); + $nsDecl = rtrim($namespace, '\\'); + file_put_contents( + $classDir . '/' . $uniqueClass . '.php', + "register(); + + $fqcn = $namespace . $uniqueClass; + expect(class_exists($fqcn, false))->toBeFalse(); + + moduleAutoloaderCleanup($base); +}); + +it('does not register the same namespace and path twice when called repeatedly', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $appDir = $base . '/app'; + $namespace = 'Acme\\Idempotent\\'; + $srcDir = 'src'; + + createAutoloaderTestModule( + $appDir . '/idempotent', + 'acme/idempotent', + [$namespace => $srcDir], + ); + + $autoloader = new ModuleAutoloader( + modulesPath: '', + appPath: $appDir, + parser: new ManifestParser(), + ); + + $countBefore = count(spl_autoload_functions()); + $autoloader->register(); + $countAfterFirst = count(spl_autoload_functions()); + $autoloader->register(); + $countAfterSecond = count(spl_autoload_functions()); + + // First call registers new autoloaders, second call adds none + expect($countAfterFirst)->toBeGreaterThan($countBefore) + ->and($countAfterSecond)->toBe($countAfterFirst); + + moduleAutoloaderCleanup($base); +}); + +it('resolves nothing and does not error when app and modules directories are empty', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $appDir = $base . '/app'; + $modulesDir = $base . '/modules'; + + mkdir($appDir, 0755, true); + mkdir($modulesDir, 0755, true); + + $autoloader = new ModuleAutoloader( + modulesPath: $modulesDir, + appPath: $appDir, + parser: new ManifestParser(), + ); + + // Should not throw + $autoloader->register(); + + // Passing — no exception means success + expect(true)->toBeTrue(); + + moduleAutoloaderCleanup($base); +}); + +it('leaves Application boot still able to autoload an app module class (regression)', function (): void { + $base = sys_get_temp_dir() . '/marko-autoloader-test-' . bin2hex(random_bytes(8)); + $vendorDir = $base . '/vendor'; + $modulesDir = $base . '/modules'; + $appDir = $base . '/app'; + $namespace = 'Acme\\RegressionApp\\'; + $srcDir = 'src'; + + // Create the app module + createAutoloaderTestModule( + $appDir . '/regression', + 'acme/regression', + [$namespace => $srcDir], + ); + + $classDir = $appDir . '/regression/' . $srcDir; + mkdir($classDir, 0755, true); + $uniqueClass = 'AutoloaderTestRegressionService' . bin2hex(random_bytes(4)); + $nsDecl = rtrim($namespace, '\\'); + file_put_contents( + $classDir . '/' . $uniqueClass . '.php', + "initialize(); + + $fqcn = $namespace . $uniqueClass; + expect(class_exists($fqcn))->toBeTrue(); + + moduleAutoloaderCleanup($base); +}); diff --git a/packages/docs-markdown/docs/packages/session-database.md b/packages/docs-markdown/docs/packages/session-database.md index 6e553c11..fbe3ac02 100644 --- a/packages/docs-markdown/docs/packages/session-database.md +++ b/packages/docs-markdown/docs/packages/session-database.md @@ -17,22 +17,9 @@ Requires [`marko/database`](/docs/packages/database/) for the database connectio ## Usage -### Binding the Driver +Installing `marko/session-database` is all that is required to activate database-backed sessions. The package registers `DatabaseSessionHandler` as the `SessionHandlerInterface` implementation, binds `SessionInterface` to `Session` as a singleton, and adds `SessionMiddleware` globally --- no manual configuration is needed. -Register the database handler in your module bindings: - -```php title="module.php" -use Marko\Session\Contracts\SessionHandlerInterface; -use Marko\Session\Database\Handler\DatabaseSessionHandler; - -return [ - 'bindings' => [ - SessionHandlerInterface::class => DatabaseSessionHandler::class, - ], -]; -``` - -Then use `SessionInterface` as usual: +Use `SessionInterface` as usual: ```php use Marko\Session\Contracts\SessionInterface; diff --git a/packages/docs-markdown/docs/packages/session-file.md b/packages/docs-markdown/docs/packages/session-file.md index bed1bf81..7fc03f0b 100644 --- a/packages/docs-markdown/docs/packages/session-file.md +++ b/packages/docs-markdown/docs/packages/session-file.md @@ -30,22 +30,9 @@ The `path` directory is created automatically if it does not exist. ## Usage -### Binding the Driver +Installing `marko/session-file` is all that is required to activate file-based sessions. The package registers `FileSessionHandler` as the `SessionHandlerInterface` implementation, binds `SessionInterface` to `Session` as a singleton, and adds `SessionMiddleware` globally --- no manual configuration is needed. -Register the file handler in your module bindings: - -```php title="module.php" -use Marko\Session\Contracts\SessionHandlerInterface; -use Marko\Session\File\Handler\FileSessionHandler; - -return [ - 'bindings' => [ - SessionHandlerInterface::class => FileSessionHandler::class, - ], -]; -``` - -Then use `SessionInterface` as usual --- the file driver handles storage: +Use `SessionInterface` as usual --- the file driver handles storage: ```php use Marko\Session\Contracts\SessionInterface; diff --git a/packages/docs-markdown/docs/packages/session.md b/packages/docs-markdown/docs/packages/session.md index 7f9598eb..7b012c40 100644 --- a/packages/docs-markdown/docs/packages/session.md +++ b/packages/docs-markdown/docs/packages/session.md @@ -16,7 +16,7 @@ Session interfaces and infrastructure --- defines session management, flash mess composer require marko/session ``` -Note: You typically install a driver package (like `marko/session-file`) which requires this automatically. +This package is inert on its own --- it defines contracts only and does not register `SessionMiddleware` or bind `SessionInterface`. Session handling activates when you install a driver package, which requires this package automatically. ## Configuration @@ -128,13 +128,7 @@ $id = $this->session->getId(); ### Session Middleware -The `SessionMiddleware` automatically starts the session at the beginning of a request and saves it when the response completes: - -```php -use Marko\Session\Middleware\SessionMiddleware; -``` - -Register it in your route or middleware stack --- it handles `start()` and `save()` so your controllers don't have to. +The `SessionMiddleware` automatically starts the session at the beginning of a request and saves it when the response completes. It is registered globally by the session driver package (e.g., `marko/session-file`, `marko/session-database`) --- no manual registration is needed. Your controllers only need to inject `SessionInterface`; `start()` and `save()` are handled automatically. ### Garbage Collection diff --git a/packages/session-database/README.md b/packages/session-database/README.md index e9fb5b77..f4102fe1 100644 --- a/packages/session-database/README.md +++ b/packages/session-database/README.md @@ -13,16 +13,20 @@ Requires `marko/database` for the database connection. ## Quick Example ```php -use Marko\Session\Contracts\SessionHandlerInterface; -use Marko\Session\Database\Handler\DatabaseSessionHandler; - -return [ - 'bindings' => [ - SessionHandlerInterface::class => DatabaseSessionHandler::class, - ], -]; +use Marko\Session\Contracts\SessionInterface; + +public function __construct( + private readonly SessionInterface $session, +) {} + +public function handle(): void +{ + $this->session->set('user_id', 42); +} ``` +Installing this package automatically registers the database handler, binds `SessionInterface`, and adds `SessionMiddleware` globally. No manual configuration is needed. + ## Documentation Full usage, API reference, and examples: [marko/session-database](https://marko.build/docs/packages/session-database/) diff --git a/packages/session-database/module.php b/packages/session-database/module.php index a337b4c4..f6a01d13 100644 --- a/packages/session-database/module.php +++ b/packages/session-database/module.php @@ -3,10 +3,20 @@ declare(strict_types=1); use Marko\Session\Contracts\SessionHandlerInterface; +use Marko\Session\Contracts\SessionInterface; use Marko\Session\Database\Handler\DatabaseSessionHandler; +use Marko\Session\Middleware\SessionMiddleware; +use Marko\Session\Session; return [ + 'sequence' => ['after' => ['marko/page-cache']], 'bindings' => [ SessionHandlerInterface::class => DatabaseSessionHandler::class, ], + 'singletons' => [ + SessionInterface::class => Session::class, + ], + 'globalMiddleware' => [ + SessionMiddleware::class, + ], ]; diff --git a/packages/session-database/tests/ModuleTest.php b/packages/session-database/tests/ModuleTest.php index 61ad838e..9942cdea 100644 --- a/packages/session-database/tests/ModuleTest.php +++ b/packages/session-database/tests/ModuleTest.php @@ -3,7 +3,10 @@ declare(strict_types=1); use Marko\Session\Contracts\SessionHandlerInterface; +use Marko\Session\Contracts\SessionInterface; use Marko\Session\Database\Handler\DatabaseSessionHandler; +use Marko\Session\Middleware\SessionMiddleware; +use Marko\Session\Session; test('it binds SessionHandlerInterface to DatabaseSessionHandler', function (): void { $modulePath = dirname(__DIR__) . '/module.php'; @@ -50,3 +53,16 @@ ->and($composer['autoload']['psr-4'])->toHaveKey('Marko\\Session\\Database\\') ->and($composer['autoload']['psr-4']['Marko\\Session\\Database\\'])->toBe('src/'); }); + +it('binds SessionInterface to Session when the database session driver is installed', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + expect($module['singletons'])->toHaveKey(SessionInterface::class) + ->and($module['singletons'][SessionInterface::class])->toBe(Session::class); +}); + +it('registers the session global middleware when the database session driver is installed', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + expect($module['globalMiddleware'])->toContain(SessionMiddleware::class); +}); diff --git a/packages/session-file/README.md b/packages/session-file/README.md index 9ea14f56..685906bb 100644 --- a/packages/session-file/README.md +++ b/packages/session-file/README.md @@ -11,16 +11,20 @@ composer require marko/session-file ## Quick Example ```php -use Marko\Session\Contracts\SessionHandlerInterface; -use Marko\Session\File\Handler\FileSessionHandler; - -return [ - 'bindings' => [ - SessionHandlerInterface::class => FileSessionHandler::class, - ], -]; +use Marko\Session\Contracts\SessionInterface; + +public function __construct( + private readonly SessionInterface $session, +) {} + +public function handle(): void +{ + $this->session->set('user_id', 42); +} ``` +Installing this package automatically registers the file handler, binds `SessionInterface`, and adds `SessionMiddleware` globally. No manual configuration is needed. + ## Documentation Full usage, API reference, and examples: [marko/session-file](https://marko.build/docs/packages/session-file/) diff --git a/packages/session-file/module.php b/packages/session-file/module.php index 236bba37..ef6c5640 100644 --- a/packages/session-file/module.php +++ b/packages/session-file/module.php @@ -3,10 +3,20 @@ declare(strict_types=1); use Marko\Session\Contracts\SessionHandlerInterface; +use Marko\Session\Contracts\SessionInterface; use Marko\Session\File\Handler\FileSessionHandler; +use Marko\Session\Middleware\SessionMiddleware; +use Marko\Session\Session; return [ + 'sequence' => ['after' => ['marko/page-cache']], 'bindings' => [ SessionHandlerInterface::class => FileSessionHandler::class, ], + 'singletons' => [ + SessionInterface::class => Session::class, + ], + 'globalMiddleware' => [ + SessionMiddleware::class, + ], ]; diff --git a/packages/session-file/tests/ModuleTest.php b/packages/session-file/tests/ModuleTest.php new file mode 100644 index 00000000..dbee43f3 --- /dev/null +++ b/packages/session-file/tests/ModuleTest.php @@ -0,0 +1,79 @@ +toHaveKey(SessionInterface::class) + ->and($module['singletons'][SessionInterface::class])->toBe(Session::class); +}); + +it('registers the session global middleware when the file session driver is installed', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + expect($module['globalMiddleware'])->toContain(SessionMiddleware::class); +}); + +it('starts a session and sets a session cookie end-to-end with the file driver installed', function (): void { + $sessionPath = sys_get_temp_dir() . '/marko-e2e-test-' . bin2hex(random_bytes(8)); + mkdir($sessionPath, 0700, true); + + try { + $config = new FakeConfigRepository([ + 'session.driver' => 'file', + 'session.lifetime' => 120, + 'session.expire_on_close' => false, + 'session.path' => $sessionPath, + 'session.cookie.name' => 'MARKO_SESSION', + 'session.cookie.path' => '/', + 'session.cookie.domain' => '', + 'session.cookie.secure' => false, + 'session.cookie.httponly' => true, + 'session.cookie.samesite' => 'lax', + 'session.gc_probability' => 1, + 'session.gc_divisor' => 100, + ]); + + $handler = new FileSessionHandler(new SessionConfig($config)); + $session = new Session($handler, new SessionConfig($config)); + $middleware = new SessionMiddleware($session); + + $request = new Request(server: [ + 'REQUEST_METHOD' => 'GET', + 'REQUEST_URI' => '/', + ]); + + $response = $middleware->handle($request, function (Request $r) use ($session): Response { + $session->set('user', 'alice'); + + return new Response('OK'); + }); + + expect($response->body())->toBe('OK') + ->and($session->started)->toBeFalse() // saved and closed by middleware + ->and(glob($sessionPath . '/sess_*'))->not->toBeEmpty(); + } finally { + $files = glob($sessionPath . '/*') ?: []; + foreach ($files as $file) { + unlink($file); + } + rmdir($sessionPath); + } +}); + +it('orders the session middleware after page-cache when the file driver and page-cache are both installed', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + expect($module['sequence'])->toHaveKey('after') + ->and($module['sequence']['after'])->toContain('marko/page-cache'); +}); diff --git a/packages/session/README.md b/packages/session/README.md index 1427a03e..b72a0aaf 100644 --- a/packages/session/README.md +++ b/packages/session/README.md @@ -8,7 +8,7 @@ Session interfaces and infrastructure--defines session management, flash message composer require marko/session ``` -Note: You also need a driver package. See `marko/session-file` or `marko/session-database`. +This package is inert on its own --- it defines contracts only. Session handling activates when you install a driver package (`marko/session-file` or `marko/session-database`), which registers the implementation and middleware automatically. ## Quick Example diff --git a/packages/session/module.php b/packages/session/module.php index 54c04bd5..0dae23de 100644 --- a/packages/session/module.php +++ b/packages/session/module.php @@ -2,20 +2,4 @@ declare(strict_types=1); -use Marko\Session\Contracts\SessionInterface; -use Marko\Session\Middleware\SessionMiddleware; -use Marko\Session\Session; - -return [ - 'sequence' => [ - // page-cache must short-circuit before session work begins; soft - // ordering — only enforced when marko/page-cache is also installed. - 'after' => ['marko/page-cache'], - ], - 'singletons' => [ - SessionInterface::class => Session::class, - ], - 'globalMiddleware' => [ - SessionMiddleware::class, - ], -]; +return []; diff --git a/packages/session/tests/PackageStructureTest.php b/packages/session/tests/PackageStructureTest.php index c4ed17e9..417c5535 100644 --- a/packages/session/tests/PackageStructureTest.php +++ b/packages/session/tests/PackageStructureTest.php @@ -49,7 +49,7 @@ ->and($composer['autoload']['psr-4']['Marko\\Session\\'])->toBe('src/'); }); -it('has module.php with enabled set to true', function () { +it('has module.php that returns an array', function (): void { $modulePath = dirname(__DIR__) . '/module.php'; expect(file_exists($modulePath))->toBeTrue(); @@ -59,14 +59,6 @@ expect($config)->toBeArray(); }); -it('has module.php with singletons array', function () { - $modulePath = dirname(__DIR__) . '/module.php'; - $config = require $modulePath; - - expect($config)->toHaveKey('singletons') - ->and($config['singletons'])->toBeArray(); -}); - it('has src directory for source code', function () { $srcPath = dirname(__DIR__) . '/src'; @@ -98,15 +90,14 @@ ->and($config)->toHaveKey('cookie'); }); -it('module.php declares SessionMiddleware as globalMiddleware', function (): void { +it('does not register any global middleware from the session interface package', function (): void { $module = require dirname(__DIR__) . '/module.php'; - expect($module['globalMiddleware'] ?? []) - ->toContain('Marko\\Session\\Middleware\\SessionMiddleware'); + expect($module['globalMiddleware'] ?? [])->toBeEmpty(); }); -it('module.php declares marko/page-cache as a soft after dependency', function (): void { +it('does not bind SessionInterface from the session interface package', function (): void { $module = require dirname(__DIR__) . '/module.php'; - expect($module['sequence']['after'] ?? [])->toContain('marko/page-cache'); + expect(array_keys($module['singletons'] ?? []))->not->toContain('Marko\\Session\\Contracts\\SessionInterface'); }); diff --git a/packages/session/tests/PassivePackageTest.php b/packages/session/tests/PassivePackageTest.php new file mode 100644 index 00000000..29e82243 --- /dev/null +++ b/packages/session/tests/PassivePackageTest.php @@ -0,0 +1,64 @@ + 'marko/session', + 'version' => '1.0.0', + 'extra' => ['marko' => ['module' => true]], + ], JSON_PRETTY_PRINT)); + + // Copy the real (now passive) module.php + copy(dirname(__DIR__) . '/module.php', $sessionPath . '/module.php'); + + $app = new Application( + vendorPath: $vendorDir, + modulesPath: '', + appPath: '', + ); + + // Should not throw — a passive session package with no driver is inert + $app->initialize(); + + expect($app)->toBeInstanceOf(Application::class); + + // Clean up + array_map('unlink', glob($sessionPath . '/*')); + rmdir($sessionPath); + rmdir($vendorDir . '/marko'); + rmdir($vendorDir); + rmdir($baseDir); +}); + +it('serves a stateless request with marko/session installed and no driver without a 500', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + // A passive module has no globalMiddleware — so SessionMiddleware never runs on any request + $globalMiddleware = $module['globalMiddleware'] ?? []; + + expect($globalMiddleware)->not->toContain(SessionMiddleware::class); +}); + +it('throws a loud BindingException when SessionInterface is resolved with no driver installed', function (): void { + $container = new Container(); + + // No driver bound — resolving SessionInterface must throw loudly. + // The Container dispatches to NoDriverException (extends MarkoException, implements + // ContainerExceptionInterface) which is a more helpful "loud" error with driver install instructions. + expect(fn () => $container->get(SessionInterface::class)) + ->toThrow(NoDriverException::class); +}); diff --git a/packages/skeleton/README.md b/packages/skeleton/README.md index 23d99e3c..9323cffe 100644 --- a/packages/skeleton/README.md +++ b/packages/skeleton/README.md @@ -25,6 +25,12 @@ cd my-app 3. Start the dev server: `marko up` 4. Visit http://localhost:8000 +The skeleton ships a working Pest test harness out of the box. `tests/Pest.php` binds `Marko\Testing\TestCase` as the base, which registers PSR-4 autoloaders for `app/*` and `modules/*` automatically. Run tests with: + +```bash +composer test +``` + ## Next Steps Create your first controller inside `app/`: diff --git a/packages/skeleton/phpunit.xml b/packages/skeleton/phpunit.xml new file mode 100644 index 00000000..66e07d78 --- /dev/null +++ b/packages/skeleton/phpunit.xml @@ -0,0 +1,25 @@ + + + + + app + + + modules + + + tests + + + + + app + modules + + + diff --git a/packages/skeleton/tests/PackageStructureTest.php b/packages/skeleton/tests/PackageStructureTest.php index ecd78885..878cc59b 100644 --- a/packages/skeleton/tests/PackageStructureTest.php +++ b/packages/skeleton/tests/PackageStructureTest.php @@ -143,9 +143,8 @@ expect($gitignore) ->toMatch('/^\s*\/storage\/\*\s*$/m') - ->toMatch('/^\s*!\/storage\/\.gitkeep\s*$/m'); - - expect(file_exists(__DIR__ . '/../storage/.gitkeep'))->toBeTrue(); + ->toMatch('/^\s*!\/storage\/\.gitkeep\s*$/m') + ->and(file_exists(__DIR__ . '/../storage/.gitkeep'))->toBeTrue(); }); it('lists marko/view-twig in the skeleton composer suggest block', function (): void { @@ -180,3 +179,75 @@ expect(json_last_error())->toBe(JSON_ERROR_NONE) ->and($composer['suggest'])->toBeArray(); }); + +it('ships a root phpunit.xml in the skeleton', function (): void { + $phpunitPath = __DIR__ . '/../phpunit.xml'; + + expect(file_exists($phpunitPath))->toBeTrue(); +}); + +it('configures testsuites that discover app and modules test directories', function (): void { + $phpunitPath = __DIR__ . '/../phpunit.xml'; + $xml = simplexml_load_string(file_get_contents($phpunitPath)); + + $directories = []; + foreach ($xml->testsuites->testsuite as $suite) { + foreach ($suite->directory as $dir) { + $directories[] = (string) $dir; + } + } + + expect($directories)->toContain('app') + ->and($directories)->toContain('modules') + ->and($directories)->toContain('tests'); +}); + +it('ships a root tests/Pest.php that references Marko\Testing\TestCase', function (): void { + $pestPath = __DIR__ . '/../tests/Pest.php'; + + expect(file_exists($pestPath))->toBeTrue(); + + $content = file_get_contents($pestPath); + + expect($content)->toContain('Marko\Testing\TestCase'); +}); + +it('ships placeholders so every directory the phpunit.xml testsuites reference exists (app, modules, tests)', function (): void { + $base = __DIR__ . '/..'; + + expect(is_dir($base . '/app'))->toBeTrue() + ->and(is_dir($base . '/modules'))->toBeTrue() + ->and(is_dir($base . '/tests'))->toBeTrue(); +}); + +it('runs pest successfully on a freshly scaffolded skeleton with no added test files', function (): void { + // Simulate a fresh skeleton install: create a temporary directory with the + // same layout (app/.gitkeep, modules/.gitkeep, tests/Pest.php) and run + // pest against it. Zero tests should exit 0 (green), not an error. + $tmpDir = sys_get_temp_dir() . '/marko-skeleton-test-' . uniqid(); + mkdir($tmpDir . '/app', 0755, true); + mkdir($tmpDir . '/modules', 0755, true); + mkdir($tmpDir . '/tests', 0755, true); + file_put_contents($tmpDir . '/app/.gitkeep', ''); + file_put_contents($tmpDir . '/modules/.gitkeep', ''); + file_put_contents($tmpDir . '/tests/Pest.php', file_get_contents(__DIR__ . '/Pest.php')); + file_put_contents($tmpDir . '/phpunit.xml', file_get_contents(__DIR__ . '/../phpunit.xml')); + + $vendorPest = __DIR__ . '/../../../vendor/bin/pest'; + $vendorAutoload = __DIR__ . '/../../../vendor/autoload.php'; + + $command = sprintf( + 'cd %s && /opt/homebrew/Cellar/php/8.5.1_2/bin/php -d memory_limit=512M %s -c %s --bootstrap %s --no-coverage 2>&1', + escapeshellarg($tmpDir), + escapeshellarg($vendorPest), + escapeshellarg($tmpDir . '/phpunit.xml'), + escapeshellarg($vendorAutoload), + ); + + exec($command, $output, $exitCode); + + // Clean up temporary directory recursively + exec('rm -rf ' . escapeshellarg($tmpDir)); + + expect($exitCode)->toBe(0); +}); diff --git a/packages/skeleton/tests/Pest.php b/packages/skeleton/tests/Pest.php new file mode 100644 index 00000000..3fab8eca --- /dev/null +++ b/packages/skeleton/tests/Pest.php @@ -0,0 +1,9 @@ +in(__DIR__); diff --git a/packages/testing/README.md b/packages/testing/README.md index f553f344..944a4b92 100644 --- a/packages/testing/README.md +++ b/packages/testing/README.md @@ -12,6 +12,20 @@ Testing utilities for Marko---reusable fakes with built-in assertions that elimi composer require marko/testing --dev ``` +## Pest Base Test Case + +`TestCase` registers PSR-4 autoloaders for `app/*` and `modules/*` before tests run, so module classes resolve without per-project Composer classmap configuration. No container or application boot is performed. + +Wire it as the global base in `tests/Pest.php`: + +```php title="tests/Pest.php" +use Marko\Testing\TestCase; + +uses(TestCase::class)->in(__DIR__); +``` + +After that, every Pest test in the project can reference module and app classes directly---no additional Composer path repos or classmaps required. + ## Available Fakes `FakeEventDispatcher`, `FakeMailer`, `FakeQueue`, `FakeSession`, `FakeCookieJar`, `FakeLogger`, `FakeConfigRepository`, `FakeAuthenticatable`, `FakeUserProvider`, `FakeGuard` @@ -171,6 +185,12 @@ KnownDriversValidator::assertSkeletonSuggestContainsAll( ## API Reference +### TestCase + +- `setUp(): void` — Calls `registerModuleAutoloaders()` before each test +- `registerModuleAutoloaders(): void` — Registers PSR-4 autoloaders for `app/*` and `modules/*`; safe to call multiple times (skips already-registered roots) +- `projectRoot(): string` — Walks up the directory tree from cwd to find the nearest ancestor containing `vendor/` + ### FakeEventDispatcher - `dispatch($event): void` — Record a dispatched event diff --git a/packages/testing/src/TestCase.php b/packages/testing/src/TestCase.php new file mode 100644 index 00000000..4962867e --- /dev/null +++ b/packages/testing/src/TestCase.php @@ -0,0 +1,87 @@ + Tracks project roots already registered in this process */ + private static array $registeredRoots = []; + + /** + * Set up autoloaders before each test. + * + * @throws ModuleException + */ + protected function setUp(): void + { + parent::setUp(); + + $this->registerModuleAutoloaders(); + } + + /** + * Register PSR-4 autoloaders for app/* and modules/* modules. + * + * Safe to call multiple times; registration is skipped for roots that were + * already processed in this PHP process. + * + * @throws ModuleException + */ + protected function registerModuleAutoloaders(): void + { + $root = $this->projectRoot(); + + if (isset(self::$registeredRoots[$root])) { + return; + } + + self::$registeredRoots[$root] = true; + + $autoloader = new ModuleAutoloader( + modulesPath: $root . '/modules', + appPath: $root . '/app', + parser: new ManifestParser(), + ); + + $autoloader->register(); + } + + /** + * Locate the project root by walking up the directory tree from the current + * working directory until a directory containing vendor/ is found. + * + * Returns the deepest ancestor (or the cwd itself) that contains vendor/. + */ + protected function projectRoot(): string + { + $dir = getcwd(); + + if ($dir === false) { + return ''; + } + + while ($dir !== dirname($dir)) { + if (is_dir($dir . '/vendor')) { + return $dir; + } + + $dir = dirname($dir); + } + + return $dir; + } +} diff --git a/packages/testing/tests/Unit/TestCase/TestCaseTest.php b/packages/testing/tests/Unit/TestCase/TestCaseTest.php new file mode 100644 index 00000000..ab34203e --- /dev/null +++ b/packages/testing/tests/Unit/TestCase/TestCaseTest.php @@ -0,0 +1,154 @@ +toBeInstanceOf(PhpUnitTestCase::class); +}); + +it('registers app module autoloaders so an App namespaced class resolves in tests', function (): void { + $fixtureRoot = dirname(__DIR__, 2) . '/fixtures/project-root'; + + $tc = new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + }; + + $tc::$root = $fixtureRoot; + $tc->setUp(); + + expect(class_exists('App\Home\HomeService', true))->toBeTrue(); +}); + +it('resolves a class from a module in the modules directory', function (): void { + $fixtureRoot = dirname(__DIR__, 2) . '/fixtures/project-root'; + + $tc = new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + }; + + $tc::$root = $fixtureRoot; + $tc->setUp(); + + expect(class_exists('Blog\BlogService', true))->toBeTrue(); +}); + +it('locates the project root when running from a nested module tests directory', function (): void { + $nestedRoot = dirname(__DIR__, 2) . '/fixtures/nested-project'; + + $tc = new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + }; + + $tc::$root = $nestedRoot; + $tc->setUp(); + + expect(class_exists('App\Accounts\AccountService', true))->toBeTrue(); +}); + +it('does not require a session or database driver to be installed', function (): void { + // TestCase must be usable without session/database dependencies. + $fixtureRoot = dirname(__DIR__, 2) . '/fixtures/project-root'; + + $tc = new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + }; + + $tc::$root = $fixtureRoot; + + expect(fn () => $tc->setUp())->not->toThrow(Throwable::class); +}); + +it('registers autoloaders only once across multiple test cases', function (): void { + // Unique root with no modules so we can count cleanly + $uniqueRoot = dirname(__DIR__, 2) . '/fixtures/project-root-once'; + + if (!is_dir($uniqueRoot . '/vendor')) { + mkdir($uniqueRoot . '/vendor', 0777, true); + } + + $makeTC = static function () use ($uniqueRoot): TestCase { + return new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + + public function callRegister(): void + { + $this->registerModuleAutoloaders(); + } + }; + }; + + $tc1 = $makeTC(); + $tc1::$root = $uniqueRoot; + + $tc2 = $makeTC(); + $tc2::$root = $uniqueRoot; + + $tc1->callRegister(); + $after1 = count(spl_autoload_functions()); + + $tc2->callRegister(); + $after2 = count(spl_autoload_functions()); + + // Second call must not add more autoloaders + expect($after2)->toBe($after1); +}); + +it('does not error and resolves nothing when run from a package dir whose root has no app or modules modules', function (): void { + // Simulate running from within the marko monorepo where vendor/ exists but app/ and modules/ do not + $monoRepoRoot = dirname(__DIR__, 5); // packages/testing/tests/Unit/TestCase -> monorepo root + + $tc = new class ('test') extends TestCase + { + public static string $root = ''; + + protected function projectRoot(): string + { + return self::$root; + } + }; + + $tc::$root = $monoRepoRoot; + + expect(fn () => $tc->setUp())->not->toThrow(Throwable::class); +}); + +it('has registered the autoloaders by the time the test body runs so an App class resolves inside the test closure', function (): void { + // HomeService was registered by the setUp() of an earlier test in this process. + // This closure body executes after setUp(), confirming registration timing is sufficient. + expect(class_exists('App\Home\HomeService'))->toBeTrue(); +}); diff --git a/packages/testing/tests/fixtures/nested-project/app/accounts/composer.json b/packages/testing/tests/fixtures/nested-project/app/accounts/composer.json new file mode 100644 index 00000000..ba4a14d4 --- /dev/null +++ b/packages/testing/tests/fixtures/nested-project/app/accounts/composer.json @@ -0,0 +1,14 @@ +{ + "name": "app/accounts", + "type": "marko-module", + "extra": { + "marko": { + "module": true + } + }, + "autoload": { + "psr-4": { + "App\\Accounts\\": "src/" + } + } +} diff --git a/packages/testing/tests/fixtures/nested-project/app/accounts/src/AccountService.php b/packages/testing/tests/fixtures/nested-project/app/accounts/src/AccountService.php new file mode 100644 index 00000000..4dab9ba6 --- /dev/null +++ b/packages/testing/tests/fixtures/nested-project/app/accounts/src/AccountService.php @@ -0,0 +1,13 @@ +