feat: self-refreshing mcp/lsp code index

.claude/plans/index-self-refresh/001-staleness-recheck-on-read.md

@@ -0,0 +1,63 @@
+# Task 001: IndexCache re-validates staleness on read (app/ + modules/; additions, edits, deletions)
+
+**Status**: complete
+**Depends on**: none
+**Retry count**: 0
+
+## Description
+Make `IndexCache` re-check staleness on reads that occur *after* the in-memory data is already populated, so the long-lived `mcp:serve` / `lsp:serve` singleton stops serving a frozen snapshot. Scope the re-check to the directories an agent edits at runtime — `app/*` and `modules/**` — and **skip `vendor/`** (vendor changes only via Composer; new vendor packages surface on the next `indexer:rebuild` / cold-warm, which is the documented path). Extend detection to catch **deletions** by persisting the tracked source-file path set in the cache and comparing it against the current walk. No TTL/clock is needed: the scoped walk is cheap (a few `stat()`s) and a rebuild fires at most once per actual change.
+
+## Context
+The bug: `ensureLoaded()` returns early whenever `$this->data !== null`, so `isStale()` only ever runs on the very first read in a process. Long-lived servers hold `IndexCache` as a singleton (`packages/codeindexer/module.php`), freezing the snapshot. `isStale()` already re-scans dirs and compares mtimes, but (a) it walks vendor too (~85 modules — unnecessary on the hot path) and (b) mtime cannot observe a removed file — hence the scoping + path-set comparison.
+
+Why no TTL: with vendor skipped, the re-check walks only `app/*` + `modules/**` (typically dozens of files, single-digit ms, pure `stat()`). Re-checking on every read is fine and is *more* responsive than a memo — there is no window where a fresh write is masked. A rebuild storm cannot happen: the first read after a change rebuilds and `save()` stamps the cache file mtime to "now", so subsequent reads see "not stale" until the next real change. At most one `build()` per change.
+
+Verified against source (do not re-derive):
+- `ensureLoaded()` short-circuits at line 146 on `$this->data !== null`. The re-check must be added here: when `$this->data` is populated, call the staleness check and, if stale, rebuild (set `$this->data = null` then `load()`/`build()`, or call `build()` directly). Do NOT early-return unconditionally.
+- **Ordering trap:** `load()` (line 102) calls `isStale()` at line 110 *before* `$this->data` is populated (line 132). So `isStale()` cannot read the stored path-set from `$this->data` — it isn't loaded yet. The deletion check must read the previously-persisted path-set **directly from the on-disk payload** (a cheap `unserialize` of just the saved set, or factor the comparison into a helper that `isStale()` calls with the on-disk set). The naive "compare `$this->data` path-set" reads null during `load()` and would never detect deletions or false-positive every load. Pick one approach and state it in the implementation notes.
+- The MCP/LSP tools (e.g. `packages/mcp/src/Tools/ListModulesTool.php`, `ListRoutesTool.php`) call getters (`getModules()`, `getRoutes()`) which all funnel through `ensureLoaded()`. There is no explicit `load()`/`build()` warm-up at server startup (`McpServer`/`LspServer` constructors do not touch the cache), so `ensureLoaded()` is the single correct seam — confirmed.
+- `build()` (line 48) still walks **everything** via `$this->moduleWalker->walk()` (vendor + modules + app) — vendor stays fully indexed; only the *staleness re-check* skips vendor. The tracked path-set is derived from the same `app`/`modules` subtrees the scoped `isStale()` scans. Store it as a plain `list<string>` (sorted) or a `string` hash under a new payload key inside the `compact(...)` array — a plain scalar/array needs **no** new entry in the `unserialize` allowlist (`load()` lines 114–126). Verify it survives the existing allowlist unchanged.
+
+- Related files:
+  - `packages/codeindexer/src/Cache/IndexCache.php` — `ensureLoaded()` (146), `isStale()` (157), `build()` (48), `load()` (102), `save()` (87)
+  - `packages/codeindexer/src/Module/ModuleWalker.php` — `walk()` (re-scans dirs live; inject a spy in tests to count walks). Note: vendor is `vendor/*/*`, the part the scoped staleness check must exclude.
+  - `packages/codeindexer/tests/Unit/Cache/IndexCacheTest.php` — existing suite to preserve (all 12 tests)
+- Patterns to follow: existing tmp-dir fixture style in `IndexCacheTest.php`; constructor property promotion; strict types; loud errors. No new constructor parameter is required (no clock).
+
+## Requirements (Test Descriptions)
+- [x] `it reflects a newly added app module on the next read after the data was already loaded` (singleton scenario: one `IndexCache` instance, populate via a getter, add a module under `app/` on disk, call the getter again, expect the new module — without constructing a fresh instance)
+- [x] `it reflects a newly added route via getRoutes after the data was already loaded`
+- [x] `it reflects an edit to a tracked app/modules source file on the next read after first load`
+- [x] `it reflects a deleted app module on the next read by comparing the tracked path set`
+- [x] `it does not rebuild when no tracked file under app or modules changed`
+- [x] `it does not trigger a rebuild when only a vendor file changes after first load` (vendor is excluded from the staleness re-check)
+- [x] `it rebuilds at most once for several successive reads following a single change` (no rebuild storm: count walker/build invocations)
+- [x] `it treats a cache payload missing the tracked path set as stale so old caches rebuild once` (write a legacy payload WITHOUT the path-set key, then read — expect a rebuild, not a crash)
+- [x] `it persists the tracked path set as a plain array or string that requires no new unserialize allowed class`
+- [x] `it preserves first-load semantics by loading a fresh on-disk cache without rescanning` (the existing "loads cache from disk without re-scanning when cache is fresh" test must still pass)
+
+## Acceptance Criteria
+- `ensureLoaded()` re-evaluates staleness even when `$this->data` is populated, and rebuilds when stale. The re-check fires on reads *after* the in-memory snapshot exists — this is the core singleton fix; an explicit single-instance test (not a fresh-object reload) is required.
+- The staleness re-check walks only `app/*` and `modules/**`; `vendor/` is excluded from the re-check (but remains fully indexed by `build()`).
+- A rebuild fires at most once per change across successive reads (no rebuild storm), verified by counting walker/`build()` calls.
+- `build()` persists the sorted tracked path-set (or hash) under a plain scalar/array payload key (no new `unserialize` allowed class). The deletion comparison reads the previously-persisted set from the **on-disk payload**, not `$this->data`.
+- A legacy cache payload missing the path-set key is treated as stale and rebuilt once, cleanly, without an exception.
+- All pre-existing `IndexCacheTest` tests still pass (12 tests). No change to runtime routing/autoloading.
+- Code follows code standards; `composer test` green for the package.
+
+## Implementation Notes
+
+### Ordering trap resolution
+Chose the **direct on-disk read** approach: `isStale()` calls a new private `loadTrackedPathsFromDisk(string $cachePath): ?array` method that reads and unserializes the cache file independently of `$this->data`. This avoids the null problem — during `load()`, `isStale()` is called before `$this->data` is set, so reading `$this->data['trackedPaths']` would always be null. The on-disk read uses the same allowlist as `load()` and is safe.
+
+### trackedPaths key
+`build()` calls a new private `collectTrackedPaths(): list<string>` that recursively walks `app/` and `modules/` directories, collects all file paths, sorts them, and returns a plain `list<string>`. This is stored as `trackedPaths` in the `compact()` payload. Since it is a plain PHP array of strings, no new entry in the `unserialize` allowlist is needed.
+
+### ensureLoaded() change
+Added a staleness re-check for the `$this->data !== null` branch: if `isStale()` returns true, call `build()` directly (which writes a fresh cache and updates `$this->data`). Subsequent reads after the rebuild see a fresh cache mtime so `isStale()` returns false — no rebuild storm.
+
+### Vendor exclusion
+`isStale()` skips modules whose `$module->path` starts with `{rootPath}/vendor/`. These are only re-indexed on a cold-warm / `indexer:rebuild`. The `collectTrackedPaths()` walk scans only `app/` and `modules/`, so vendor files never enter the tracked-path set either.
+
+### Build count tracking in tests
+Tests count `observers()` calls (fired only during `build()`) rather than `walk()` calls (which also fire during `isStale()`) to accurately measure rebuild frequency.

.claude/plans/index-self-refresh/002-create-module-skill-step6.md

@@ -0,0 +1,43 @@
+# Task 002: Rewrite create-module SKILL.md Step 6
+
+**Status**: complete
+**Depends on**: 001
+**Retry count**: 0
+
+## Description
+Step 6 of the create-module skill ("Verify the module is discovered") is the direct source of two wrong agent reflexes seen in real sessions: it instructs `composer dump-autoload` (app-module autoloading is automatic at runtime via `ModuleAutoloader`) and frames `list_modules` as a discovery gate (which trained the agent to distrust its own writes and reach for `indexer:rebuild`). Rewrite it to reflect reality: the module exists because you wrote it, the runtime discovers and autoloads it live on the next request, and the AI tooling self-refreshes on read (task 001) — so introspection is an optional cross-check, never a gate.
+
+## Context
+This is a documentation/skill-content task — no unit test. Verification is content review: the new text must not contain the removed instructions and must state the correct model.
+
+- Related files:
+  - `packages/claude-plugins/plugins/marko-skills/skills/create-module/SKILL.md` — Step 6 (lines 74–80) AND the "Verification" section (lines 82–84). Both contain gate-framing that must change.
+- Exact strings to remove/rewrite (verified in source):
+  - Line 76: `"After installing or registering the module, call the MCP tool list_modules. The new module should appear in the list. If not, check that:"` — the "If not, check that" framing trains distrust of the agent's own write. Reframe: the module exists because you wrote it; runtime discovers and autoloads it live next request.
+  - Line 79: `"Composer has run (composer dump-autoload or composer update)"` — REMOVE entirely; app-module autoloading is automatic via `ModuleAutoloader` at runtime, never needs `composer dump-autoload`.
+  - Line 84: `"Then call the list_modules MCP tool to confirm the module is discovered by the framework."` — soften from a discovery gate to an optional cross-check; keep the LSP-diagnostics-as-verification-gate sentence (that part is legitimate and stays).
+- Note line 78 (`extra.marko.module: true`) and line 80 (psr-4 resolves) are legitimate structural checks and may stay, reframed as "if you want to double-check structure" rather than "if the module didn't appear."
+- Patterns to follow: the skill's existing terse, imperative voice; consistency with `concepts/modularity.md` and `packages/codeindexer.md` (rewritten in task 003).
+
+## Requirements (Test Descriptions)
+- [x] `it removes the composer dump-autoload instruction from Step 6`
+- [x] `it removes the framing of list_modules as a required discovery gate`
+- [x] `it states module discovery and autoloading happen live at runtime with no build step`
+- [x] `it states the MCP/LSP index self-refreshes on read so indexer:rebuild is not needed to see newly created code`
+- [x] `it keeps validate_module as an optional structural cross-check rather than an existence check`
+
+## Acceptance Criteria
+- No occurrence of `composer dump-autoload` (or `composer update` as a discovery fix) remains in the skill.
+- No language tells the agent to run `indexer:rebuild` to make freshly-created code visible.
+- The verification section frames introspection tools as optional cross-checks.
+- Wording is consistent with the rewritten docs (task 003).
+
+## Implementation Notes
+- Step 6 (lines 74–80): Replaced the "If not, check that" framing with a positive assertion ("The module exists because you wrote it") plus runtime/self-refresh description. Removed the `composer dump-autoload` bullet entirely. Kept `extra.marko.module: true` and psr-4 structural checks, reframed under "If you want to double-check structure". Added an explicit sentence that `list_modules`/`validate_module` are optional cross-checks, not gates.
+- Verification section (lines 82–84): Kept LSP-diagnostics-as-gate sentence intact. Replaced "Then call the list_modules MCP tool to confirm the module is discovered by the framework" with a sentence framing `list_modules` as an optional cross-check that is not required for the module to be active.
+
+## Acceptance Criteria Status
+- No occurrence of `composer dump-autoload` remains in the skill. PASS
+- No language tells the agent to run `indexer:rebuild`. PASS
+- Verification section frames introspection tools as optional cross-checks. PASS
+- Wording consistent with self-refresh behavior. PASS

.claude/plans/index-self-refresh/003-docs-codeindexer-modularity.md

@@ -0,0 +1,40 @@
+# Task 003: Docs rewrite — codeindexer.md + concepts/modularity.md
+
+**Status**: complete
+**Depends on**: 001
+**Retry count**: 0
+
+## Description
+Two docs pages currently document the index staleness as an accepted limitation and tell readers to run `indexer:rebuild` to make the tooling see new code. The fix in task 001 reverses that, so rewrite (not append to) the affected passages to describe the new self-refreshing behavior: the running MCP/LSP server re-checks staleness **on every read** and auto-refreshes on additions, edits, and deletions under `app/` and `modules/`. `indexer:rebuild` is now only for forcing a clean rebuild, warming a cold cache, or picking up **vendor** changes (a `composer require`/`update`), which the on-read check intentionally skips.
+
+## Context
+Documentation task — no unit test; verify by reading the rendered passages and confirming no contradictory "must reindex to see new code" language remains.
+
+- Related files:
+  - `docs/src/content/docs/packages/codeindexer.md` — the "lazy-loads / rebuilds itself if stale" note (~22, ~52) and especially the staleness-limitation paragraph (~61, "won't re-check staleness afterward … until you run `marko indexer:rebuild`")
+  - `docs/src/content/docs/concepts/modularity.md` — the reindex note (~44, "If your AI tooling doesn't yet list the new module … run `marko indexer:rebuild` and reload")
+- Behavior to describe accurately (per task 001):
+  - The running server re-checks staleness on each read; **no TTL window** — a fresh write is visible on the very next tool read.
+  - Re-check scope is `app/` + `modules/` only. **Vendor is excluded** from the on-read check; new/removed vendor packages need an explicit `indexer:rebuild` (or the cold-warm during `devai:install`).
+  - Additions, edits, AND deletions under `app/`/`modules/` are picked up automatically.
+- Patterns to follow: existing docs voice; keep the distinction between the **app** (always live) and the **tooling index** (now self-refreshing for `app`/`modules`).
+
+## Requirements (Test Descriptions)
+- [x] `it rewrites the codeindexer staleness paragraph to state the running server re-checks staleness on every read`
+- [x] `it states additions, edits, and deletions under app and modules are picked up automatically without indexer:rebuild`
+- [x] `it documents that vendor changes are excluded from the on-read check and still need indexer:rebuild`
+- [x] `it reframes indexer:rebuild as a forced/clean-rebuild, cold-warm, and vendor-change tool, not a routine step`
+- [x] `it rewrites the modularity reindex note to reflect tooling self-refresh`
+- [x] `it removes any claim that newly created app/modules code stays invisible to tooling until a manual rebuild`
+
+## Acceptance Criteria
+- The `codeindexer.md` paragraph describing "won't re-check staleness afterward" is replaced with the on-read self-refresh behavior (no-TTL, app/modules scope, vendor excluded).
+- `modularity.md` no longer instructs a reindex to surface a freshly created module in tooling.
+- `indexer:rebuild` is still documented, but as forced/clean-rebuild + cold-warm + vendor-change.
+- No contradiction with the create-module skill (task 002) or troubleshooting/checklist (task 004).
+
+## Implementation Notes
+- `codeindexer.md` line 22: updated "lazy-loads on first read and rebuilds itself if stale" → "lazy-loads on first read and re-checks staleness on every subsequent read"
+- `codeindexer.md` line 52: rewrote the rebuilding-section intro to state no-TTL, on-every-read, app/modules scope; changed "force a rebuild" → "force a clean rebuild"
+- `codeindexer.md` lines 60-62: replaced `:::caution[Long-running servers hold the index in memory]` block (which said "won't re-check staleness afterward") with `:::note[On-read self-refresh covers app/ and modules/; vendor needs an explicit rebuild]` describing on-read self-refresh, vendor exclusion, and the three remaining `indexer:rebuild` scenarios
+- `modularity.md` line 44: rewrote reindex note — removed "run `marko indexer:rebuild` and reload" instruction, replaced with description of on-read self-refresh for app/modules and vendor as the sole remaining `indexer:rebuild` case