Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
44 changes: 44 additions & 0 deletions .claude/plans/first-run-friction-fixes/002-testing-testcase.md
Original file line number Diff line number Diff line change
@@ -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.
42 changes: 42 additions & 0 deletions .claude/plans/first-run-friction-fixes/003-session-passive.md
Original file line number Diff line number Diff line change
@@ -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).
Original file line number Diff line number Diff line change
@@ -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.
Loading