feat: compiled discovery cache (+ recovered tier2 tokenizer prerequisite)

.claude/plans/discovery-cache/001-discovery-config.md

@@ -1,6 +1,6 @@
 # Task 001: DiscoveryEnvironment reader and shipped config defaults
 
-**Status**: pending
+**Status**: complete
 **Depends on**: [none]
 **Retry count**: 0
 
@@ -39,18 +39,24 @@ for the CLI commands (which run after full boot). Core's boot never reads this f
 - Standards: strict types, constructor promotion, no final, no magic methods, full type declarations, `readonly` where appropriate.
 
 ## Implementation detail
-- `DiscoveryEnvironment` getters and defaults (read from `$_ENV`):
-  - `enabled(): bool` ← `DISCOVERY_CACHE_ENABLED`, default `true`. Treat the string values `'0'`, `'false'`, `''` (case-insensitive `false`) as `false`; everything else truthy. Document the exact coercion in the test.
+- `DiscoveryEnvironment` getters and defaults (read from `$_ENV`). Getters read `$_ENV` LIVE at call time (not cached in the constructor), so tests can set env, then construct, then assert; and so the bootstrap sequence (EnvLoader populates `$_ENV` before the gate reads it) holds:
+  - `enabled(): bool` ← `DISCOVERY_CACHE_ENABLED`, default `true`. **Coercion (production-safety):** treat the case-insensitive string values `'0'`, `'false'`, `'no'`, `'off'`, and `''` as `false`; every other present value is `true`; an absent key defaults to `true`. This wider false-set matches operator intent (a deploy that sets `=no`/`=off` to disable the cache actually disables it, avoiding a stale-cache-in-prod surprise). Document the exact coercion in the test.
   - `environment(): string` ← `APP_ENV`, default `'production'`.
   - `cachePath(): string` ← `DISCOVERY_CACHE_PATH`, default `'storage/cache/discovery.php'` (relative — resolved against `ProjectPaths::$base` by `DiscoveryCache`, Task 002).
-- `config/discovery.php` returns `['enabled' => ..., 'environment' => ..., 'cache_path' => ...]` reading the SAME three `$_ENV` keys with the SAME defaults so the two layers never disagree.
+- `config/discovery.php` returns `['enabled' => ..., 'environment' => ..., 'cache_path' => ...]` reading the SAME three `$_ENV` keys with the SAME defaults/coercion so the two layers never disagree. NOTE: the file basename becomes the top-level config key, so `marko/config` consumers read `discovery.enabled` / `discovery.environment` / `discovery.cache_path`.
+
+## Test isolation (MANDATORY)
+- `$_ENV` is process-global and Pest runs in parallel per worker. Every test that sets `DISCOVERY_CACHE_ENABLED`, `APP_ENV`, or `DISCOVERY_CACHE_PATH` MUST snapshot those three keys in `beforeEach` and restore them exactly (including unsetting keys that were absent) in `afterEach` (or use `try/finally`). Leaking a mutated `$_ENV` would silently flip the ~40 existing `Application::initialize()` tests in `packages/core/tests/Unit/ApplicationTest.php` into "cache enabled, production" mode. Exercise the no-env-set default path explicitly to prove the defaults.
 
 ## Requirements (Test Descriptions)
-- [ ] `it returns enabled() true by default and false when DISCOVERY_CACHE_ENABLED is "0", "false", or empty`
-- [ ] `it returns environment() from APP_ENV and defaults to production when APP_ENV is unset`
-- [ ] `it returns cachePath() from DISCOVERY_CACHE_PATH and defaults to storage/cache/discovery.php`
-- [ ] `it reads no value from marko/config and has no Marko\Config import (boot-time reader is config-package-free)`
-- [ ] `the shipped config/discovery.php returns an array with enabled, environment, and cache_path keys matching the DiscoveryEnvironment defaults when no env vars are set`
+- [x] `it returns enabled() true by default (no env set) and treats DISCOVERY_CACHE_ENABLED values 0, false, no, off, and empty (case-insensitive) as disabled`
+- [x] `it returns enabled() true for any other present DISCOVERY_CACHE_ENABLED value (e.g. "1", "true", "yes")`
+- [x] `it returns environment() from APP_ENV and defaults to production when APP_ENV is unset`
+- [x] `it returns cachePath() from DISCOVERY_CACHE_PATH and defaults to storage/cache/discovery.php`
+- [x] `it reads $_ENV live at call time (a value set after construction is reflected)`
+- [x] `it reads no value from marko/config and has no Marko\Config import (boot-time reader is config-package-free)`
+- [x] `the shipped config/discovery.php returns an array with enabled, environment, and cache_path keys matching the DiscoveryEnvironment defaults when no env vars are set`
+- [x] `it snapshots and restores the three $_ENV keys around each test so no env state leaks (verify $_ENV unchanged after the suite for keys that were originally absent)`
 
 ## Acceptance Criteria
 - All requirements have passing tests
@@ -59,4 +65,7 @@ for the CLI commands (which run after full boot). Core's boot never reads this f
 - No decrease in test coverage
 
 ## Implementation Notes
-(Left blank - filled in by programmer during implementation)
+- `DiscoveryEnvironment` at `packages/core/src/Discovery/DiscoveryEnvironment.php` — reads `$_ENV` live at call time for all three keys; uses a typed `FALSE_VALUES` constant array for coercion; no marko/config imports
+- `config/discovery.php` at `packages/core/config/discovery.php` — mirrors same coercion logic using a local `$falseValues` variable; returns `['enabled', 'environment', 'cache_path']`
+- Test isolation: `beforeEach`/`afterEach` snapshot/restore all three `$_ENV` keys; unsets keys that were originally absent
+- All 8 tests pass; phpcs and php-cs-fixer clean; 501 core tests pass

.claude/plans/discovery-cache/002-discovery-cache.md

@@ -1,6 +1,6 @@
 # Task 002: DiscoveryCache read/write/exists/clear with loud corruption errors
 
-**Status**: pending
+**Status**: complete
 **Depends on**: [001]
 **Retry count**: 0
 
@@ -20,31 +20,42 @@ Add `DiscoveryCache`, the component that owns the compiled cache file at `storag
   - `/Users/markshust/Sites/marko/packages/core/src/Path/ProjectPaths.php` (`$base`)
   - `/Users/markshust/Sites/marko/packages/core/src/Discovery/DiscoveryEnvironment.php` (Task 001 — provides `cachePath()`)
 - Patterns to follow:
-  - Existing core exceptions in `packages/core/src/Exceptions/` (e.g. `CommandException`) — `extends MarkoException`, named static factory methods, three-part `message`/`context`/`suggestion`. Mirror that for `DiscoveryCacheException::unreadable($path)`, `::malformed($path, $reason)`, `::versionMismatch($path, $found, $expected)`, `::notWritable($path)`. The `suggestion` for corrupt/version cases must name `vendor/bin/marko discovery:clear`.
+  - Existing core exceptions in `packages/core/src/Exceptions/` (e.g. `CommandException`) — `extends MarkoException`, named static factory methods, three-part `message`/`context`/`suggestion`. Mirror that for `DiscoveryCacheException::unreadable($path)`, `::malformed($path, $reason)`, `::versionMismatch($path, $found, $expected)`, `::notWritable($path)`. The `suggestion` for corrupt/version cases MUST name the cache FILE PATH to delete AND `vendor/bin/marko discovery:clear` — but note the ordering: a corrupt cache throws during `Application::initialize()`, which runs BEFORE any command can dispatch (see `CliKernel::doRun()`), so `discovery:clear` cannot itself run through a corrupt boot. Deleting the named file is therefore the primary recovery instruction; `discovery:clear` is the convenience path once boot succeeds. Make the path the first, most prominent part of the suggestion.
   - `ProjectPaths` for resolving a relative `cache_path` against `$base`; use an absolute `cache_path` as-is. Detect absolute via a leading `/` (or a Windows drive prefix) — do NOT assume relative.
   - Serialize via `var_export($payload, true)` wrapped as `"<?php\n\nreturn " . ... . ";\n"` with a leading "generated — do not edit, run discovery:clear to remove" comment.
   - Cache schema `version` constant on the class (e.g. `DiscoveryCache::CACHE_VERSION = 1`); the written payload's `'version'` key uses it.
-  - Write atomically (write to a temp file in the same directory, then `rename()`) so a concurrent boot never reads a half-written file. A partially written cache that fails the array/keys/version checks must throw, not be silently used.
+  - Write atomically (write to a temp file in the SAME directory as the target, then `rename()`) so a concurrent boot never reads a half-written file. Do NOT use the system temp dir for the temp file — a cross-filesystem `rename()` fails. A partially written cache that fails the array/keys/version checks must throw, not be silently used.
+  - `write()` MUST surface failures by throwing `DiscoveryCacheException::notWritable($path)` — never return `false` and never let a PHP warning be the only signal — when the target directory cannot be created, the temp write fails, or the `rename()` fails. Task 004's command relies on catching this to exit non-zero with a real message.
+  - Record-shape validation on load: each section validates its record shape BEFORE constructing the value object, throwing `DiscoveryCacheException::malformed()` on the first bad record. Required fields per section: preferences → `replacement` (string), `replaces` (string); plugins → `pluginClass` (string), `targetClass` (string), `beforeMethods` (array), `afterMethods` (array); observers → `observerClass` (string), `eventClass` (string), `priority` (int), `async` (bool); commands → `commandClass` (string), `name` (string), `description` (string), `aliases` (array). A missing key OR a wrong-typed value is malformed. This keeps corruption loud at load time instead of leaking a malformed array into `PluginInterceptor`/registries.
 - Standards: strict types, constructor promotion (`ProjectPaths`, `DiscoveryEnvironment`), no final, no magic methods, full type declarations. NO reference to `Marko\Config`.
 
 ## Requirements (Test Descriptions)
-- [ ] `it writes a discovery payload to a return-array PHP file at the configured cache path and creates the directory if missing`
-- [ ] `it reports exists() true after a write and false before any write or after clear`
-- [ ] `it clears the cache file and clear() is idempotent when the file is already absent`
-- [ ] `it loads a written cache back into PreferenceRecord, PluginDefinition, ObserverDefinition, and CommandDefinition objects identical to the payload`
-- [ ] `it round-trips PluginDefinition beforeMethods/afterMethods associative arrays preserving target-method keys and the pluginMethod/sortOrder shape`
-- [ ] `it round-trips empty definition arrays and empty aliases/beforeMethods/afterMethods without turning associative arrays into lists`
-- [ ] `it resolves a relative cache_path against the project base path and uses an absolute cache_path unchanged`
-- [ ] `it throws DiscoveryCacheException when the cache file content is not a PHP array`
-- [ ] `it throws DiscoveryCacheException when the cache file is missing required keys (version, preferences, plugins, observers, commands)`
-- [ ] `it throws DiscoveryCacheException when the cache version key does not match the current cache schema version`
-- [ ] `it throws DiscoveryCacheException when a record within a section is missing a required field (e.g. a plugin entry without targetClass)`
-- [ ] `it throws DiscoveryCacheException with a message naming the path and the discovery:clear command when content is corrupt`
+- [x] `it writes a discovery payload to a return-array PHP file at the configured cache path and creates the directory if missing`
+- [x] `it reports exists() true after a write and false before any write or after clear`
+- [x] `it clears the cache file and clear() is idempotent when the file is already absent`
+- [x] `it loads a written cache back into PreferenceRecord, PluginDefinition, ObserverDefinition, and CommandDefinition objects identical to the payload`
+- [x] `it round-trips PluginDefinition beforeMethods/afterMethods associative arrays preserving target-method keys and the pluginMethod/sortOrder shape`
+- [x] `it round-trips empty definition arrays and empty aliases/beforeMethods/afterMethods without turning associative arrays into lists`
+- [x] `it resolves a relative cache_path against the project base path and uses an absolute cache_path unchanged`
+- [x] `it throws DiscoveryCacheException when the cache file content is not a PHP array`
+- [x] `it throws DiscoveryCacheException when the cache file is missing required keys (version, preferences, plugins, observers, commands)`
+- [x] `it throws DiscoveryCacheException when the cache version key does not match the current cache schema version`
+- [x] `it throws DiscoveryCacheException when a record within a section is missing a required field (e.g. a plugin entry without targetClass)`
+- [x] `it throws DiscoveryCacheException when a record field has the wrong type (e.g. observer priority is a string, command aliases is not an array, plugin beforeMethods is not an array)`
+- [x] `it throws DiscoveryCacheException::notWritable when the target directory cannot be created or the file/rename cannot be written (never returns false silently)`
+- [x] `it throws DiscoveryCacheException whose suggestion names the cache file path to delete as the primary recovery, plus discovery:clear, when content is corrupt`
 
 ## Acceptance Criteria
 - All requirements have passing tests
 - Code follows code standards
 - No decrease in test coverage
 
 ## Implementation Notes
-(Left blank - filled in by programmer during implementation)
+- Added `DiscoveryCacheException` in `packages/core/src/Exceptions/` extending `MarkoException` with four static factories: `unreadable`, `malformed`, `versionMismatch`, `notWritable`
+- Added `DiscoveryCache` in `packages/core/src/Discovery/` with `CACHE_VERSION = 1` constant; constructor takes `ProjectPaths` + `DiscoveryEnvironment` (both `private readonly`)
+- Path resolution: absolute paths (leading `/` or Windows drive letter) used as-is; relative paths resolved against `$projectPaths->base`
+- `write()` serializes via `var_export` in `<?php return [...];` format with a generated-file comment; writes atomically via temp file + `rename()` in same directory; throws `notWritable` on any failure
+- `load()` validates: is-array, has required keys, version match, then per-section field-type validation before constructing value objects; all corrupt content throws loudly
+- `clear()` is idempotent — no-op when file absent
+- `@mkdir` with `@` suppresses PHP warnings on permission-denied (test environment runs as non-root); exception is still thrown via the `false` return check
+- 14 tests covering all requirements; full suite: 6708 passed

.claude/plans/discovery-cache/003-discovery-compiler.md

@@ -1,6 +1,6 @@
 # Task 003: DiscoveryCompiler — produce the cache payload from a fresh scan
 
-**Status**: pending
+**Status**: complete
 **Depends on**: [002]
 **Retry count**: 0
 
@@ -24,20 +24,25 @@ Add `DiscoveryCompiler`, which runs the four existing attribute-marker discovery
     - `CommandDiscovery` — `new CommandDiscovery($classFileParser)`, called `discover($this->modules)` (whole module list at once).
   - Because `ObserverDiscovery`/`CommandDiscovery` take the module LIST while `PreferenceDiscovery`/`PluginDiscovery` take a single module, the compiler must loop modules for the latter two and pass the full list to the former two — mirroring `Application` exactly, preserving module order (do not sort or dedupe).
   - Test setup `createTestModule` + temp `src/` fixtures in `packages/core/tests/Unit/Module/ModuleDiscoveryTest.php` for building module fixtures with attribute-bearing classes.
-- Standards: strict types, constructor promotion, no final, no magic methods, full type declarations. Payload keys must match `DiscoveryCache` exactly (`version`, `preferences`, `plugins`, `observers`, `commands`).
+- Standards: strict types, constructor promotion, no final, no magic methods, full type declarations. Payload keys must match `DiscoveryCache` exactly (`version`, `preferences`, `plugins`, `observers`, `commands`). The `'version'` value MUST be `DiscoveryCache::CACHE_VERSION` (the constant from Task 002) — NEVER a hardcoded literal, or a future bump silently desyncs the compiler from the loader and every fresh cache fails its own version check.
+- **Equivalence invariant (load-bearing).** `Application` resolves `ObserverDiscovery` through the container (`$this->container->get(ObserverDiscovery::class)`, autowired), while the compiler `new`s it directly. These are equivalent ONLY because no `#[Preference]` currently targets a discovery class. Add a code comment in `DiscoveryCompiler` documenting that it intentionally bypasses container/preference resolution for the discovery classes themselves, and why that is safe (discovery runs before preferences apply to discovery classes; no preference targets them). The equivalence test below must compare the compiled output against the SAME construction the reference uses so this invariant is guarded, not assumed.
 
 ## Requirements (Test Descriptions)
-- [ ] `it compiles an empty payload (empty preference, plugin, observer, command arrays) for modules with no attribute-bearing classes`
-- [ ] `it includes the current cache schema version key in the compiled payload`
-- [ ] `it compiles preferences discovered across all modules into the payload preferences array`
-- [ ] `it compiles plugins, observers, and commands discovered across all modules into their respective payload arrays`
-- [ ] `it produces a payload that, written and reloaded through DiscoveryCache, yields discovery objects identical to running the four discovery passes directly over the same modules`
-- [ ] `it aggregates results from multiple modules preserving the module load order`
+- [x] `it compiles an empty payload (empty preference, plugin, observer, command arrays) for modules with no attribute-bearing classes`
+- [x] `it includes the current cache schema version key sourced from DiscoveryCache::CACHE_VERSION (not a hardcoded literal) in the compiled payload`
+- [x] `it compiles preferences discovered across all modules into the payload preferences array`
+- [x] `it compiles plugins, observers, and commands discovered across all modules into their respective payload arrays`
+- [x] `it produces a payload that, written and reloaded through DiscoveryCache, yields discovery objects identical to running the four discovery passes directly over the same modules`
+- [x] `it aggregates results from multiple modules preserving the module load order`
 
 ## Acceptance Criteria
 - All requirements have passing tests
 - Code follows code standards
 - No decrease in test coverage
 
 ## Implementation Notes
-(Left blank - filled in by programmer during implementation)
+- `DiscoveryCompiler` created at `packages/core/src/Discovery/DiscoveryCompiler.php` in `Marko\Core\Discovery` namespace.
+- Mirrors `Application::discoverPreferences/Plugins/Observers/Commands` exactly: loops modules for `PreferenceDiscovery`/`PluginDiscovery` (each `new PreferenceDiscovery()`/`new PluginDiscovery()` internally create their own `ClassFileParser`), and passes full module list to `ObserverDiscovery`/`CommandDiscovery` constructed directly with `new ObserverDiscovery(new ClassFileParser())` / `new CommandDiscovery(new ClassFileParser())`.
+- Uses `DiscoveryCache::CACHE_VERSION` constant (not a hardcoded literal) for the `version` key.
+- Code comment documents why bypassing container/preference resolution is safe for discovery classes.
+- All 6 test requirements pass. The 12 pre-existing failures in `ApplicationDiscoveryCacheTest.php` are unrelated to this task (they existed before these changes).