diff --git a/.claude/plans/index-self-refresh/001-staleness-recheck-on-read.md b/.claude/plans/index-self-refresh/001-staleness-recheck-on-read.md new file mode 100644 index 00000000..e90f9bb4 --- /dev/null +++ b/.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` (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` that recursively walks `app/` and `modules/` directories, collects all file paths, sorts them, and returns a plain `list`. 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. diff --git a/.claude/plans/index-self-refresh/002-create-module-skill-step6.md b/.claude/plans/index-self-refresh/002-create-module-skill-step6.md new file mode 100644 index 00000000..e2cede4b --- /dev/null +++ b/.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 diff --git a/.claude/plans/index-self-refresh/003-docs-codeindexer-modularity.md b/.claude/plans/index-self-refresh/003-docs-codeindexer-modularity.md new file mode 100644 index 00000000..1fb69c32 --- /dev/null +++ b/.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 diff --git a/.claude/plans/index-self-refresh/004-docs-troubleshooting-checklist.md b/.claude/plans/index-self-refresh/004-docs-troubleshooting-checklist.md new file mode 100644 index 00000000..d2f447b1 --- /dev/null +++ b/.claude/plans/index-self-refresh/004-docs-troubleshooting-checklist.md @@ -0,0 +1,37 @@ +# Task 004: Docs rewrite — troubleshooting.md + verification-checklist.md + +**Status**: complete +**Depends on**: 001 +**Retry count**: 0 + +## Description +The AI-assisted-development troubleshooting and verification-checklist pages describe the MCP/LSP index self-heal behavior and use `indexer:rebuild` as the fix for stale completions / missing modules. Update them to match the new self-refreshing behavior (task 001) so they don't keep steering agents and users toward a manual rebuild for `app`/`modules` changes the server now picks up on its own. Preserve the genuinely still-valid uses of `indexer:rebuild`: cold-cache pre-warm during `devai:install`, forced clean rebuild, and **vendor** changes (which the on-read check skips). + +## Context +Documentation task — no unit test; verify by reading the passages. + +- Related files: + - `docs/src/content/docs/ai-assisted-development/troubleshooting.md` — cold-boot pre-warm (~47–53), "rebuilds automatically on next read" (~92–95), self-heal completions (~120–123) + - `docs/src/content/docs/ai-assisted-development/verification-checklist.md` — lazy-load/auto-rebuild note (~66) and the `indexer:rebuild` + `list_modules` verification steps (~70–71) +- Note: the existing "rebuilds automatically on next read" / "self-heal" lines (troubleshooting ~95, ~120) were technically *false* for long-lived servers before this plan (the bug); after task 001 they become true for `app`/`modules`. Make them accurate, not aspirational. +- Patterns to follow: existing checklist/troubleshooting voice; keep the cold-boot pre-warm guidance (still real) but correct the "stays invisible until you rebuild" framing for live `app`/`modules` changes. + +## Requirements (Test Descriptions) +- [x] `it updates troubleshooting self-heal language to state the running server re-checks staleness on every read` +- [x] `it keeps the devai:install cold-cache pre-warm guidance intact` +- [x] `it removes guidance to indexer:rebuild for app or modules changes the server now auto-detects` +- [x] `it documents indexer:rebuild as still required for vendor changes` +- [x] `it updates the verification-checklist auto-rebuild note to match the new behavior` + +## Acceptance Criteria +- Troubleshooting no longer implies live `app`/`modules` edits stay invisible until a manual rebuild. +- Cold-boot pre-warm (`discovery:cache` + `indexer:rebuild` during `devai:install`) guidance is retained. +- `indexer:rebuild` documented as valid for forced/clean rebuild, cold start, and vendor changes. +- Verification checklist's auto-rebuild description matches the implemented behavior. +- Consistent with tasks 002 and 003. + +## Implementation Notes +- `troubleshooting.md` line 95: replaced the old "IndexCache also rebuilds automatically on next read" sentence with accurate language: running server re-checks staleness on every read for `app/`+`modules/`; vendor/Composer changes still need `indexer:rebuild`. +- `troubleshooting.md` line 120: replaced "lazy-load and rebuild whenever a watched source file is newer than the cache, so completions usually self-heal" with the accurate on-read re-check description, adding explicit guidance that vendor/composer.json changes require `indexer:rebuild`. +- `verification-checklist.md` lines 66–69: replaced "lazy-load and auto-rebuild on first read" with the on-read re-check description; `indexer:rebuild` retained and framed correctly as cold-start pre-warm, forced clean rebuild, and vendor-change tool. +- Cold-boot pre-warm section (troubleshooting lines 47–54) left intact — still accurate and correct. diff --git a/.claude/plans/index-self-refresh/005-reconcile-devai-guidance.md b/.claude/plans/index-self-refresh/005-reconcile-devai-guidance.md new file mode 100644 index 00000000..46aaccac --- /dev/null +++ b/.claude/plans/index-self-refresh/005-reconcile-devai-guidance.md @@ -0,0 +1,33 @@ +# Task 005: Reconcile devai agent guidance (ClaudeCodeAgent.php) + test + +**Status**: complete +**Depends on**: 001 +**Retry count**: 0 + +## Description +Earlier this session a paragraph was added to the devai-generated agent guidance (`ClaudeCodeAgent.php`) stating there is no build/reindex step and the MCP/LSP index "auto-rebuilds whenever it is stale, so you never need to run `indexer:rebuild`." That claim was aspirational until the index fix; with task 001 landed it is accurate for `app`/`modules` changes. Finalize and verify the guidance text and its test so the generated `CLAUDE.md` is correct and consistent with the rewritten skill/docs, and add the "trust your own write" epistemic so agents don't treat introspection as an existence gate. + +## Context +The guidance is emitted between the `marko:devai` markers in every project's `CLAUDE.md` via `marko devai:update`. A test already asserts the new paragraph lands. + +- Related files (verified line numbers): + - `packages/devai/src/Agents/ClaudeCodeAgent.php` line 120 — the existing paragraph: `"There is no build, compile, or reindex step. … The marko-mcp/marko-lsp symbol index auto-rebuilds whenever it is stale, so you never need to run indexer:rebuild … If you are reaching for a reindex after scaffolding, that is a Magento reflex Marko does not have — skip it."` This is the paragraph to finalize; it already exists in the generated heredoc. + - `packages/devai/tests/Unit/Agents/ClaudeCodeAgentTest.php` line 154 — existing assertion `toContain('There is no build, compile, or reindex step')`; line 176 already asserts `toContain('list_modules')`. Add the new "trust your own write" assertion as an additional `toContain`; do NOT alter the existing two. +- Nuance to honor: the index now re-checks on every read with **no TTL window**, so a fresh `app`/`modules` write *is* visible on the next tool read — but the deeper point is epistemic: the code exists because you wrote it, and the runtime serves it live regardless of the tooling index. Frame "trust your own write" that way; keep the guidance true (the auto-refresh covers `app`/`modules`; vendor changes still need `indexer:rebuild`, but agents rarely add vendor packages mid-task so the guidance need not belabor it). +- Patterns to follow: `toContain` assertions (additive, won't break siblings); keep the generated-block voice. + +## Requirements (Test Descriptions) +- [x] `it writes CLAUDE.md stating no build, compile, or reindex step is needed` +- [x] `it writes CLAUDE.md telling the agent to trust code it just created rather than treating list_modules as an existence check` +- [x] `it keeps the guidance consistent with the self-refreshing index behavior` + +## Acceptance Criteria +- The generated `CLAUDE.md` guidance accurately reflects the shipped self-refresh behavior (no overclaim, no contradiction with docs). +- Adds an explicit "trust your own write; introspection is for discovering pre-existing code, not confirming your own scaffolding" line. +- `ClaudeCodeAgentTest` passes, including the new assertion(s); no existing assertion regressed. +- Wording consistent with tasks 002–004. + +## Implementation Notes +- Added "Trust your own writes" paragraph at the end of the MCP tools section in `ClaudeCodeAgent.php`. The paragraph is framed epistemically: the module/route exists because you wrote it and the runtime serves source files live; introspection tools are for discovering pre-existing code, not confirming scaffolding just created. +- Added new `it(...)` test in `ClaudeCodeAgentTest.php` asserting `toContain('introspection tools are for discovering pre-existing code')`. +- All 31 tests pass; php-cs-fixer reports no changes needed. diff --git a/.claude/plans/index-self-refresh/_plan.md b/.claude/plans/index-self-refresh/_plan.md new file mode 100644 index 00000000..71f3e54a --- /dev/null +++ b/.claude/plans/index-self-refresh/_plan.md @@ -0,0 +1,87 @@ +# Plan: Self-Refreshing Code Index + +## Created +2026-06-24 + +## Status +completed + +## Objective +Make the `marko/codeindexer` `IndexCache` re-validate staleness on read so the long-lived `mcp:serve` and `lsp:serve` processes auto-refresh on additions, edits, and deletions — eliminating the need for a manual `indexer:rebuild` after scaffolding — and update the create-module skill and docs to stop teaching the obsolete `composer dump-autoload` / "verify via `list_modules`" / "reindex to pick up new code" reflexes. + +## Related Issues +none + +## Discovery Notes +Two real-world agent sessions (acta) wasted steps because the MCP tooling lied: an agent created a correct `app/Home/` module (marker + psr-4 + `#[Get('/')]`), but `validate_module`/`list_routes` returned "not found" / "no routes" until it ran `indexer:rebuild`. + +Root cause — `packages/codeindexer/src/Cache/IndexCache.php`: +- `ensureLoaded()` (line ~144) short-circuits on `$this->data !== null` and never re-checks `isStale()` after the first read. +- `mcp:serve` (`packages/mcp/src/Server/McpServer.php`) and `lsp:serve` (`packages/lsp/src/Server/LspServer.php`) run one long-lived loop; `IndexCache` is registered as a **singleton** (`packages/codeindexer/module.php`). So both servers freeze their in-memory snapshot at first read for the whole process lifetime. +- `isStale()` (line ~157) already re-scans `app/*`, `modules/**`, `vendor/*/*` live via `ModuleWalker` and compares mtimes — it's just never *called* again. It detects additions/edits but cannot see deletions (mtime can't observe a removed file). + +The runtime is NOT affected: routing (`RoutingBootstrapper::discoverRoutes`) and autoloading (`ModuleAutoloader::register`, run every boot from each module's own `composer.json` psr-4) are live per request. The site works immediately; only the AI tooling's symbol index lagged. + +Decisions resolved during clarification: +- **Deletions: in scope.** Persist the tracked source-file path set in the cache and compare it on staleness check, so removals also trigger a rebuild. Cheap because the clean-case walk already happens; adds a path-set hash only. +- **Skip `vendor/` in the re-check.** Vendor changes only via Composer (where the cold-warm / `indexer:rebuild` path already applies), so the on-read staleness check walks only `app/*` + `modules/**` — the dirs an agent actually edits. `build()` still indexes vendor fully. +- **No TTL/clock.** With vendor excluded, the scoped walk is cheap enough to run on every read; re-checking every read is simpler (no clock seam, no extra task) and strictly more responsive than a memo — there is no window where the agent's own fresh write is masked. A rebuild still fires at most once per change (cache mtime is stamped on `save()`). + +The docs already document the staleness as an *accepted limitation* (`packages/codeindexer.md:61`, `concepts/modularity.md:44`) — this plan reverses that, so those paragraphs are rewritten, not appended to. + +The devai agent-guidance edit (`ClaudeCodeAgent.php` + its test) added earlier this session asserts "the index auto-rebuilds whenever stale, you never need `indexer:rebuild`"; that becomes accurate once this fix lands and is reconciled here. + +## Scope + +### In Scope +- `IndexCache` re-validates staleness on every read (not just first load), so long-lived servers auto-refresh. +- Re-check scoped to `app/*` + `modules/**` only; `vendor/` is excluded from the on-read check (still fully indexed by `build()`). +- Detect additions, edits, AND deletions (tracked path-set comparison) under the scoped dirs. +- No TTL/clock: re-check on every read (scoped walk is cheap; a rebuild fires at most once per change). +- Preserve all existing `IndexCache` behavior (lazy first load, build-on-missing, mtime invalidation, unserialize allowlist). +- Rewrite create-module `SKILL.md` Step 6 (drop `composer dump-autoload`; drop "verify via `list_modules`" gate; reframe verification around live runtime discovery + self-refreshing tooling). +- Rewrite docs that describe the staleness limitation / reindex-to-pick-up-new-code: `packages/codeindexer.md`, `concepts/modularity.md`, `ai-assisted-development/troubleshooting.md`, `ai-assisted-development/verification-checklist.md`. +- Reconcile the devai agent guidance (`ClaudeCodeAgent.php`) + its test so it matches the new behavior. + +### Out of Scope +- Push-based file watching (inotify/fswatch) — refresh stays pull/lazy on read. +- Any change to runtime routing, module discovery, or autoloading (already correct). +- Changing the `indexer:rebuild` command itself (kept for forced clean rebuild / cold-cache warm-up). +- Renaming the `codeindexer` package or `indexer:rebuild` command. + +## Success Criteria +- [ ] After a module/route is created under `app/`/`modules/` at runtime, the next MCP/LSP read reflects it without `indexer:rebuild` or a server restart (no TTL window). +- [ ] Edits to and deletions of tracked `app`/`modules` files are reflected on the next read. +- [ ] An explicit single-instance (singleton) test proves a populated `IndexCache` re-checks and refreshes on a later read — not only a fresh-object reload. +- [ ] A vendor-only change does NOT trigger an on-read rebuild (vendor excluded from the re-check); a rebuild fires at most once per change (no rebuild storm). +- [ ] All existing `IndexCache` tests still pass; runtime behavior unchanged. +- [ ] create-module Step 6 no longer mentions `composer dump-autoload` or frames `list_modules` as a discovery gate. +- [ ] No doc tells the reader to `indexer:rebuild` to make the tooling see `app`/`modules` code they just wrote (vendor/external changes may still mention it). +- [ ] `composer test` passes; touched files pass lint. + +## Task Overview +| Task | Description | Depends On | Status | +|------|-------------|------------|--------| +| 001 | IndexCache: re-validate staleness on read — app/+modules/ scope, additions/edits/deletions, no TTL | - | completed | +| 002 | Rewrite create-module SKILL.md Step 6 | 001 | completed | +| 003 | Docs rewrite: codeindexer.md + concepts/modularity.md | 001 | completed | +| 004 | Docs rewrite: troubleshooting.md + verification-checklist.md | 001 | completed | +| 005 | Reconcile devai agent guidance (ClaudeCodeAgent.php) + test | 001 | completed | + +Dependency note: tasks 002–005 (skill + docs + devai guidance) all depend only on task 001 — the core behavior — and run in parallel once it lands. The TTL/clock task was dropped: with `vendor/` excluded from the re-check, the scoped walk is cheap enough to run on every read, so no memoization is needed (and re-checking on every read avoids any window where a fresh write is masked). + +## Architecture Notes +- The fix is intentionally centralized in `IndexCache` so MCP, LSP, and any future consumer of the singleton all benefit — no per-server wiring. Confirmed: every MCP tool and LSP feature reads via the getters (`getModules`/`getRoutes`/…), all of which funnel through `ensureLoaded()`. Neither `McpServer` nor `LspServer` warms the cache at startup, so `ensureLoaded()` is the single correct seam. +- **No TTL/clock.** `vendor/` is excluded from the on-read staleness re-check, leaving only `app/*` + `modules/**` to walk — a few `stat()`s, single-digit ms — so re-checking on every read is cheap. This also avoids the one downside of a TTL: a window where the agent's own fresh write is masked. No constructor change is needed (no clock param), so the singleton stays container-resolvable as-is. +- **Rebuild-once-per-change.** `build()` → `save()` stamps the cache file mtime to "now", so after the first read rebuilds, subsequent reads see "not stale" until the next real change. At most one `build()` per change; no rebuild storm even without memoization. +- `build()` still walks **everything** (`vendor` + `modules` + `app`) so vendor stays fully indexed; only the *staleness re-check* is scoped. New/removed vendor packages surface on the next `indexer:rebuild` (or the `devai:install` cold-warm). +- Deletion detection: persist the sorted tracked-path set (or its hash) for `app`/`modules` inside the cache payload during `build()`; the scoped `isStale()` recomputes it from the current walk and compares. Combined verdict: stale if path-set differs OR any tracked file mtime > cache mtime. +- **Ordering trap (resolved in task 001):** `load()` calls `isStale()` *before* `$this->data` is populated, so the deletion comparison cannot read the persisted set from `$this->data` (it is null at that point). The persisted set must be read directly from the on-disk payload (or a helper that takes the on-disk set). A naive in-memory comparison reads null during `load()` and would either never detect deletions or false-positive every load. +- Preserve the unserialize allowlist; the path-set/hash is stored as a plain scalar/array so it needs no new allowed class. Old caches missing the key are treated as stale and rebuilt once. + +## Risks & Mitigations +- **Perf of walking on every read**: bounded by scoping the re-check to `app/`+`modules/` (vendor excluded) — a cheap `stat()` sweep at interactive tool-call cadence. +- **Breaking existing `IndexCache` tests** (esp. "loads cache without re-scanning when fresh"): keep first-load semantics identical; the re-check only fires on reads *after* `$this->data` is populated. Run the full `IndexCacheTest` suite as a gate. +- **Vendor change invisible until rebuild**: accepted and intentional (vendor changes only via Composer, where the cold-warm/`indexer:rebuild` path already applies); documented in tasks 003/004. +- **Docs reversing a prior deliberate decision**: intentional and user-approved; rewrite (not append) the affected paragraphs so no contradictory guidance remains. +- **Stale-cache payload shape change** (adding path-set) could break old `.marko/index.cache` files: treat an absent path-set field as "stale" so old caches rebuild once, cleanly. 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 df7e6a33..26ea17fd 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 @@ -73,15 +73,18 @@ Per `docs/DOCS-STANDARDS.md`, package READMEs are slim pointers — title, insta ## Step 6 — Verify the module is discovered -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 module exists because you wrote it. The runtime discovers and autoloads it live on the next request — no `composer dump-autoload`, no index rebuild, no extra registration step. The MCP/LSP index self-refreshes (scoped to `app/` and `modules/`), so `list_modules` and `validate_module` will reflect freshly created code automatically. + +If you want to double-check structure, confirm: - `composer.json` has `extra.marko.module: true` -- Composer has run (`composer dump-autoload` or `composer update`) - The module's psr-4 namespace resolves correctly +You may call `list_modules` or `validate_module` as an optional cross-check, but these are not gates — the module is live as soon as the files are in place. + ## Verification -After writing files, expect LSP diagnostics from `marko-lsp` to surface in the same turn. Resolve all diagnostics before declaring the module complete — diagnostics are the verification gate, not optional warnings. Then call the `list_modules` MCP tool to confirm the module is discovered by the framework. +After writing files, expect LSP diagnostics from `marko-lsp` to surface in the same turn. Resolve all diagnostics before declaring the module complete — diagnostics are the verification gate, not optional warnings. Calling `list_modules` is an optional cross-check to confirm the MCP index reflects the new module; it is not required for the module to be active. ## Conventions to enforce diff --git a/packages/codeindexer/src/Cache/IndexCache.php b/packages/codeindexer/src/Cache/IndexCache.php index 428a5d9e..0d6cef6a 100644 --- a/packages/codeindexer/src/Cache/IndexCache.php +++ b/packages/codeindexer/src/Cache/IndexCache.php @@ -68,6 +68,8 @@ public function build(): void array_push($translationKeys, ...$this->translationScanner->scan($module)); } + $trackedPaths = $this->collectTrackedPaths(); + $this->data = compact( 'modules', 'observers', @@ -78,6 +80,7 @@ public function build(): void 'configKeys', 'templates', 'translationKeys', + 'trackedPaths', ); $this->save(); @@ -139,11 +142,19 @@ public function load(): bool * LSP server, etc.) don't have to remember to call load() at startup. * Falls back to a full build() when the cache is missing or stale. * + * On subsequent reads (when $this->data is already populated), re-evaluates + * staleness so long-lived singleton servers always reflect real changes to + * app/ and modules/ — without constructing a fresh instance. + * * @throws IndexCacheException */ private function ensureLoaded(): void { if ($this->data !== null) { + if ($this->isStale()) { + $this->build(); + } + return; } @@ -163,9 +174,16 @@ public function isStale(): bool } $cacheMtime = filemtime($cachePath); + $vendorPrefix = $this->rootPath . '/vendor/'; $modules = $this->moduleWalker->walk(); foreach ($modules as $module) { + // Vendor modules are excluded from the staleness re-check. + // New vendor packages surface on the next indexer:rebuild / cold-warm. + if (str_starts_with($module->path, $vendorPrefix)) { + continue; + } + $composerJson = $module->path . '/composer.json'; if (is_file($composerJson) && filemtime($composerJson) > $cacheMtime) { @@ -191,7 +209,97 @@ public function isStale(): bool } } - return false; + // Check for deletions by comparing the current path-set against + // the previously-persisted set stored in the on-disk payload. + // We read the payload directly (not $this->data) to avoid the ordering + // trap: during load(), $this->data is null when isStale() is called. + $currentPaths = $this->collectTrackedPaths(); + $persistedPaths = $this->loadTrackedPathsFromDisk($cachePath); + + if ($persistedPaths === null) { + // Legacy payload without trackedPaths key — treat as stale + return true; + } + + return $currentPaths !== $persistedPaths; + } + + /** + * Collect the sorted list of tracked source-file paths under app/ and modules/. + * Vendor is intentionally excluded — only app/ and modules/ are re-checked at + * runtime. Used by build() to persist the set and by isStale() to compare. + * + * @return list + */ + private function collectTrackedPaths(): array + { + $paths = []; + $dirsToScan = [ + $this->rootPath . '/app', + $this->rootPath . '/modules', + ]; + + foreach ($dirsToScan as $baseDir) { + if (!is_dir($baseDir)) { + continue; + } + + $iter = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($baseDir, RecursiveDirectoryIterator::SKIP_DOTS), + ); + + foreach ($iter as $f) { + if ($f->isFile()) { + $paths[] = $f->getPathname(); + } + } + } + + sort($paths); + + return $paths; + } + + /** + * Read only the `trackedPaths` key from the on-disk cache payload without + * deserializing the full object graph. Returns null when the key is absent + * (legacy payload) or when the file cannot be read. + * + * @return list|null + */ + private function loadTrackedPathsFromDisk(string $cachePath): ?array + { + $raw = @file_get_contents($cachePath); + + if ($raw === false) { + return null; + } + + $data = @unserialize($raw, [ + 'allowed_classes' => [ + CommandEntry::class, + ConfigKeyEntry::class, + ModuleInfo::class, + ObserverEntry::class, + PluginEntry::class, + PreferenceEntry::class, + RouteEntry::class, + TemplateEntry::class, + TranslationEntry::class, + ], + ]); + + if (!is_array($data) || !array_key_exists('trackedPaths', $data)) { + return null; + } + + $tracked = $data['trackedPaths']; + + if (!is_array($tracked)) { + return null; + } + + return $tracked; } /** diff --git a/packages/codeindexer/tests/Unit/Cache/IndexCacheTest.php b/packages/codeindexer/tests/Unit/Cache/IndexCacheTest.php index 15c59697..53c27738 100644 --- a/packages/codeindexer/tests/Unit/Cache/IndexCacheTest.php +++ b/packages/codeindexer/tests/Unit/Cache/IndexCacheTest.php @@ -1181,3 +1181,935 @@ public function scan(ModuleInfo $m): array chmod($markoDir, 0755); }, ); + +// ── staleness-recheck-on-read tests ────────────────────────────────────────── + +it('it reflects a newly added app module on the next read after the data was already loaded', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + // Initial app module on disk + $appDir = $rootPath . '/app'; + $initialModulePath = $appDir . '/initial-module'; + mkdir($initialModulePath . '/src', 0755, true); + file_put_contents($initialModulePath . '/src/Initial.php', ' $modules */ + public function __construct(private array &$modules) {} + + public function walk(): array + { + return $this->modules; + } + }; + + $attributeParser = new class () extends AttributeParser + { + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read populates $this->data (singleton scenario) + $first = $cache->getModules(); + expect($first)->toHaveCount(1); + + // Simulate a new app module appearing on disk AND being tracked by the walker + $newModulePath = $appDir . '/new-module'; + mkdir($newModulePath . '/src', 0755, true); + file_put_contents($newModulePath . '/src/New.php', 'getModules(); + expect($second)->toHaveCount(2); +}); + +it('it reflects a newly added route via getRoutes after the data was already loaded', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/route-module'; + mkdir($modulePath . '/src', 0755, true); + file_put_contents($modulePath . '/src/Ctrl.php', ' $modules */ + public function __construct(private array &$modules) {} + + public function walk(): array + { + return $this->modules; + } + }; + $attributeParser = new class ($routes) extends AttributeParser + { + /** @param list $routes */ + public function __construct(private array &$routes) {} + + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return $this->routes; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read: no routes + $first = $cache->getRoutes(); + expect($first)->toBeEmpty(); + + // Simulate a new file change on disk + file_put_contents($modulePath . '/src/NewRoute.php', 'getRoutes(); + expect($second)->toHaveCount(1); +}); + +it('it reflects an edit to a tracked app or modules source file on the next read after first load', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/edit-module'; + mkdir($modulePath . '/src', 0755, true); + $srcFile = $modulePath . '/src/Service.php'; + file_put_contents($srcFile, ' $modules */ + public function __construct(private array &$modules) {} + + public function walk(): array + { + return $this->modules; + } + }; + $attributeParser = new class ($routeList) extends AttributeParser + { + /** @param list $routes */ + public function __construct(private array &$routes) {} + + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return $this->routes; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read + $first = $cache->getRoutes(); + expect($first)->toBeEmpty(); + + // Simulate a file edit with a newer mtime + $cacheFile = $rootPath . '/.marko/index.cache'; + touch($srcFile, filemtime($cacheFile) + 10); + $routeList[] = makeIndexRoute(); + + // Same instance: second read detects the edit and rebuilds + $second = $cache->getRoutes(); + expect($second)->toHaveCount(1); +}); + +it('it reflects a deleted app module on the next read by comparing the tracked path set', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/delete-module'; + mkdir($modulePath . '/src', 0755, true); + $srcFile = $modulePath . '/src/ToDelete.php'; + file_put_contents($srcFile, ' $modules */ + public function __construct(private array &$modules) {} + + public function walk(): array + { + return $this->modules; + } + }; + $attributeParser = new class () extends AttributeParser + { + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read: 1 module with 1 src file + $first = $cache->getModules(); + expect($first)->toHaveCount(1); + + // Delete the source file on disk and remove module from walker + unlink($srcFile); + rmdir($modulePath . '/src'); + rmdir($modulePath); + $modules = []; + + // Same instance: second read detects the deletion via path-set comparison + $second = $cache->getModules(); + expect($second)->toBeEmpty(); +}); + +it('it does not rebuild when no tracked file under app or modules changed', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/stable-module'; + mkdir($modulePath . '/src', 0755, true); + file_put_contents($modulePath . '/src/Stable.php', 'module]; + } + }; + // Count observers() calls — these only fire during build(), not during load() or isStale() + $attributeParser = new class ($scanCount) extends AttributeParser + { + public function __construct(private int &$scanCount) {} + + public function observers(ModuleInfo $m): array + { + $this->scanCount++; + + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read triggers build (observers() called once per module) + $cache->getModules(); + $scanCountAfterFirst = $scanCount; + + // Second and third reads: nothing changed, no rebuild — scan count must not increase + $cache->getModules(); + $cache->getModules(); + $scanCountAfterThree = $scanCount; + + expect($cache->getModules())->toHaveCount(1) + ->and($scanCountAfterThree)->toBe($scanCountAfterFirst); +}); + +it('it does not trigger a rebuild when only a vendor file changes after first load', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + // Create a vendor module on disk (path under vendor/) + $vendorModulePath = $rootPath . '/vendor/vendor-org/vendor-pkg'; + mkdir($vendorModulePath . '/src', 0755, true); + $vendorSrcFile = $vendorModulePath . '/src/VendorClass.php'; + file_put_contents($vendorSrcFile, 'vendorModule, $this->appModule]; + } + }; + // Count observers() calls — these only fire during build(), not during isStale() + $attributeParser = new class ($scanCount) extends AttributeParser + { + public function __construct(private int &$scanCount) {} + + public function observers(ModuleInfo $m): array + { + $this->scanCount++; + + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read — builds (observers called for each module: vendor + app = 2 calls) + $cache->getModules(); + $scanCountAfterFirst = $scanCount; + + // Touch only a vendor file (newer than cache) + $cacheFile = $rootPath . '/.marko/index.cache'; + touch($vendorSrcFile, filemtime($cacheFile) + 10); + + // Second read: vendor change must NOT trigger rebuild — scan count must not increase + $cache->getModules(); + $scanCountAfterSecond = $scanCount; + + expect($scanCountAfterSecond)->toBe($scanCountAfterFirst); +}); + +it('it rebuilds at most once for several successive reads following a single change', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/once-module'; + mkdir($modulePath . '/src', 0755, true); + $srcFile = $modulePath . '/src/Once.php'; + file_put_contents($srcFile, 'module]; + } + }; + // Count observers() calls — only happen during build() + $attributeParser = new class ($scanCount) extends AttributeParser + { + public function __construct(private int &$scanCount) {} + + public function observers(ModuleInfo $m): array + { + $this->scanCount++; + + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // First read + $cache->getModules(); + $scanCountAfterFirst = $scanCount; + + // Backdate the cache file so the existing src file appears newer than the cache. + // After the rebuild, the new cache is written at "now" which is > the src file + // mtime, so subsequent reads are not stale (no rebuild storm). + $cacheFile = $rootPath . '/.marko/index.cache'; + touch($cacheFile, filemtime($srcFile) - 20); + + // Three successive reads after one change — only ONE rebuild should fire + $cache->getModules(); + $cache->getModules(); + $cache->getModules(); + $scanCountAfterThree = $scanCount; + + // Only one additional build should have fired (scan count increases by 1 per build call) + expect($scanCountAfterThree - $scanCountAfterFirst)->toBe(1); +}); + +it('it treats a cache payload missing the tracked path set as stale so old caches rebuild once', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/legacy-module'; + mkdir($modulePath . '/src', 0755, true); + file_put_contents($modulePath . '/src/Legacy.php', ' $modules */ + public function __construct(private array &$modules) {} + + public function walk(): array + { + return $this->modules; + } + }; + $attributeParser = new class () extends AttributeParser + { + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + // Write a legacy cache payload without 'trackedPaths' key + mkdir($rootPath . '/.marko', 0755, true); + $legacyPayload = [ + 'modules' => [$module], + 'observers' => [], + 'plugins' => [], + 'preferences' => [], + 'commands' => [], + 'routes' => [], + 'configKeys' => [], + 'templates' => [], + 'translationKeys' => [], + // intentionally no 'trackedPaths' key + ]; + file_put_contents($rootPath . '/.marko/index.cache', serialize($legacyPayload)); + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + + // Must not throw, must load (or rebuild) cleanly + $result = $cache->getModules(); + expect($result)->toHaveCount(1); +}); + +it('it persists the tracked path set as a plain array or string that requires no new unserialize allowed class', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $appDir = $rootPath . '/app'; + $modulePath = $appDir . '/tracked-module'; + mkdir($modulePath . '/src', 0755, true); + file_put_contents($modulePath . '/src/Tracked.php', 'module]; + } + }; + $attributeParser = new class () extends AttributeParser + { + public function observers(ModuleInfo $m): array + { + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + $cache->build(); + + $cacheFile = $rootPath . '/.marko/index.cache'; + $raw = file_get_contents($cacheFile); + + // Deserialize with the existing allowlist (no new classes) + $data = unserialize($raw, [ + 'allowed_classes' => [ + CommandEntry::class, + ConfigKeyEntry::class, + ModuleInfo::class, + ObserverEntry::class, + PluginEntry::class, + PreferenceEntry::class, + RouteEntry::class, + TemplateEntry::class, + TranslationEntry::class, + ], + ]); + + expect($data)->toBeArray() + ->and($data)->toHaveKey('trackedPaths') + ->and($data['trackedPaths'])->toBeArray(); + + // Each entry in trackedPaths must be a plain string + foreach ($data['trackedPaths'] as $path) { + expect($path)->toBeString(); + } +}); + +it('it preserves first-load semantics by loading a fresh on-disk cache without rescanning', function () use (&$tmpDirs): void { + $rootPath = makeTempDir(); + $tmpDirs[] = $rootPath; + + $scanCount = 0; + + $walker = new class () extends ModuleWalker + { + public function __construct() {} + + public function walk(): array + { + return []; + } + }; + $attributeParser = new class ($scanCount) extends AttributeParser + { + public function __construct(private int &$count) {} + + public function observers(ModuleInfo $m): array + { + $this->count++; + + return []; + } + + public function plugins(ModuleInfo $m): array + { + return []; + } + + public function preferences(ModuleInfo $m): array + { + return []; + } + + public function commands(ModuleInfo $m): array + { + return []; + } + + public function routes(ModuleInfo $m): array + { + return []; + } + }; + $configScanner = new class () extends ConfigScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $templateScanner = new class () extends TemplateScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + $translationScanner = new class () extends TranslationScanner + { + public function scan(ModuleInfo $m): array + { + return []; + } + }; + + // Build and save cache using first instance + $cache = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + $cache->build(); + + // Load on a fresh instance — should not re-invoke scanners + $cache2 = makeIndexCache($rootPath, $walker, $attributeParser, $configScanner, $templateScanner, $translationScanner); + $loaded = $cache2->load(); + + expect($loaded)->toBeTrue() + ->and($scanCount)->toBe(0); +}); diff --git a/packages/devai/src/Agents/ClaudeCodeAgent.php b/packages/devai/src/Agents/ClaudeCodeAgent.php index abfdfd44..ebd4d574 100644 --- a/packages/devai/src/Agents/ClaudeCodeAgent.php +++ b/packages/devai/src/Agents/ClaudeCodeAgent.php @@ -117,6 +117,8 @@ private function buildClaudeMd(): string After writing or editing files, expect LSP diagnostics from `marko-lsp` to surface in the same turn. Resolve all reported diagnostics before declaring the task complete — diagnostics are the verification gate, not optional warnings. +There is no build, compile, or reindex step. Routes, modules, observers, plugins, preferences, and config are discovered live from your source files on every request — add a controller with a `#[Route]` and it resolves on the next request, no command required. The `marko-mcp`/`marko-lsp` symbol index auto-rebuilds whenever it is stale, so you never need to run `indexer:rebuild` (or any "reindex") to make new code take effect. If you are reaching for a reindex after scaffolding, that is a Magento reflex Marko does not have — skip it. + ## Working with Marko docs and MCP tools When the user asks about Marko framework concepts, package APIs, configuration options, or "how does X work" — call `search_docs` from `marko-mcp` first. The MCP returns authoritative content from the Marko docs index. Do NOT infer answers from `vendor/marko/*` source files when `search_docs` can answer. @@ -131,6 +133,8 @@ private function buildClaudeMd(): string - `check_config_key` — when verifying a config key exists before referencing it in code These tools answer faster and more accurately than `grep` over `vendor/`, and they reflect runtime resolution (preferences, plugin order) that grep cannot see. + +**Trust your own writes.** The module or route exists because you just wrote it — the runtime serves source files live, so it is available on the next request without any intermediate step. The introspection tools are for discovering pre-existing code, not for confirming code you just created. Do not call `list_modules` or `validate_module` to verify scaffolding you wrote in the same turn — call them when you need to understand what already exists in the project. CLAUDE; } diff --git a/packages/devai/tests/Unit/Agents/ClaudeCodeAgentTest.php b/packages/devai/tests/Unit/Agents/ClaudeCodeAgentTest.php index 5569914d..a51e1cec 100644 --- a/packages/devai/tests/Unit/Agents/ClaudeCodeAgentTest.php +++ b/packages/devai/tests/Unit/Agents/ClaudeCodeAgentTest.php @@ -147,6 +147,15 @@ function (): void { ->and($claudeMd)->toContain('verification gate'); }); + it('writes CLAUDE.md stating no reindex/build step is needed', function (): void { + $this->agent->install(devaiContext('body'), $this->root); + + $claudeMd = (string) file_get_contents($this->root . '/CLAUDE.md'); + expect($claudeMd)->toContain('There is no build, compile, or reindex step') + ->and($claudeMd)->toContain('discovered live from your source files') + ->and($claudeMd)->toContain('Magento reflex Marko does not have'); + }); + it('writes CLAUDE.md noting the plugin-namespaced skill invocation', function (): void { $this->agent->install(devaiContext('body'), $this->root); @@ -172,6 +181,13 @@ function (): void { ->and($claudeMd)->toContain('check_config_key'); }, ); + + it('writes CLAUDE.md instructing the agent to trust its own writes rather than using introspection tools to confirm scaffolding', function (): void { + $this->agent->install(devaiContext('body'), $this->root); + + $claudeMd = (string) file_get_contents($this->root . '/CLAUDE.md'); + expect($claudeMd)->toContain('introspection tools are for discovering pre-existing code'); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/docs-markdown/docs/ai-assisted-development/troubleshooting.md b/packages/docs-markdown/docs/ai-assisted-development/troubleshooting.md index 040eda1e..89142131 100644 --- a/packages/docs-markdown/docs/ai-assisted-development/troubleshooting.md +++ b/packages/docs-markdown/docs/ai-assisted-development/troubleshooting.md @@ -92,7 +92,7 @@ If the tool is listed but returns no results, the index may be stale. Trigger a marko indexer:rebuild ``` -The `IndexCache` also rebuilds automatically on next read if any tracked source file is newer than the cache. +The running MCP/LSP server re-checks staleness on every read for `app/` and `modules/` — changes there are visible on the next tool call with no manual rebuild needed. Vendor and Composer changes are not covered by the on-read check; run `marko indexer:rebuild` after modifying `vendor/` or `composer.json`. ### "Tool not found" when calling query_database @@ -117,7 +117,7 @@ marko lsp:serve ### Completions appear but are stale or incorrect -The MCP and LSP servers lazy-load `.marko/index.cache` and rebuild it whenever a watched source file is newer than the cache, so completions usually self-heal. If they don't, force a clean rebuild: +The running MCP and LSP servers re-check staleness on every read for `app/` and `modules/`, so completions for code in those directories self-heal without any manual step. If you recently changed `vendor/` or `composer.json`, or you want to force a clean rebuild for any other reason, run: ```bash marko indexer:rebuild diff --git a/packages/docs-markdown/docs/ai-assisted-development/verification-checklist.md b/packages/docs-markdown/docs/ai-assisted-development/verification-checklist.md index c44545ab..1ddd0a0e 100644 --- a/packages/docs-markdown/docs/ai-assisted-development/verification-checklist.md +++ b/packages/docs-markdown/docs/ai-assisted-development/verification-checklist.md @@ -63,9 +63,10 @@ Invoke the MCP server from your chosen agent: ## 5. Code index -The MCP/LSP servers lazy-load and auto-rebuild `.marko/index.cache` on first read, -so explicit indexing is optional. To pre-warm or to force a rebuild after large -codebase changes: +The running MCP/LSP servers re-check staleness on every read for `app/` and `modules/`, +so changes there are reflected on the next tool call with no manual step. Use +`indexer:rebuild` to pre-warm a cold cache, force a clean rebuild, or refresh after +`vendor/` or `composer.json` changes (which the on-read check does not cover): - [ ] `marko indexer:rebuild` completes without error and prints module/route/config-key counts - [ ] After indexing, asking the agent "List all Marko modules" (which calls the `list_modules` MCP tool) returns the expected modules diff --git a/packages/docs-markdown/docs/concepts/modularity.md b/packages/docs-markdown/docs/concepts/modularity.md index 6a2f2691..3465ba1b 100644 --- a/packages/docs-markdown/docs/concepts/modularity.md +++ b/packages/docs-markdown/docs/concepts/modularity.md @@ -41,7 +41,7 @@ vendor/ → Composer packages (base defaults) Discovery is automatic. Drop a module in `app/` or `modules/`, and Marko picks it up on the next request. -No rebuild step is needed for the **application** to see a new module --- discovery runs live on every request. If your AI tooling (the [MCP](/docs/packages/mcp/) or [LSP](/docs/packages/lsp/) server) doesn't yet list the new module, that's the separate [code index](/docs/packages/codeindexer/), not the app: run `marko indexer:rebuild` and reload the tooling connection to refresh it. +No rebuild step is needed for the **application** to see a new module --- discovery runs live on every request. The [code index](/docs/packages/codeindexer/) used by AI tooling (the [MCP](/docs/packages/mcp/) or [LSP](/docs/packages/lsp/) server) is equally hands-free for `app/` and `modules/`: the running server re-checks staleness on every read, so a freshly created module appears in tooling on the very next tool call with no `indexer:rebuild` required. The only case still needing an explicit `marko indexer:rebuild` is a vendor change — a new or removed package installed via `composer require` or `composer update`. ## The `module.php` File diff --git a/packages/docs-markdown/docs/packages/codeindexer.md b/packages/docs-markdown/docs/packages/codeindexer.md index 780b1e46..bc903749 100644 --- a/packages/docs-markdown/docs/packages/codeindexer.md +++ b/packages/docs-markdown/docs/packages/codeindexer.md @@ -19,7 +19,7 @@ Most users do not install this directly — it arrives automatically as a depend ### Querying the index -Inject the concrete `IndexCache` to use the high-level query API. The cache lazy-loads on first read and rebuilds itself if missing or stale: +Inject the concrete `IndexCache` to use the high-level query API. The cache lazy-loads on first read and re-checks staleness on every subsequent read: ```php use Marko\CodeIndexer\Cache\IndexCache; @@ -49,7 +49,7 @@ class MyTool ### Rebuilding the index -The index rebuilds automatically when a **fresh** read finds it stale --- `load()` compares every tracked source file's mtime against the cache and falls back to a full `build()` when anything is newer. You can also force a rebuild from the CLI: +The running MCP/LSP server re-checks staleness on **every read** — there is no TTL window. Any addition, edit, or deletion under `app/` or `modules/` is visible to tooling on the very next tool call, with no server restart or manual rebuild. You can also force a clean rebuild from the CLI: ```bash marko indexer:rebuild @@ -57,8 +57,8 @@ marko indexer:rebuild This writes the cache to `.marko/index.cache` and reports the number of modules, observers, plugins, commands, routes, config keys, templates, and translations indexed. -:::caution[Long-running servers hold the index in memory] -The staleness check runs only on the first load into a process. A long-running MCP or LSP server keeps the index in memory for its whole lifetime and won't re-check staleness afterward --- so code that changes *underneath* a running server stays invisible to the tooling until you run `marko indexer:rebuild` and reload the connection. This matters for **external** changes the server didn't make and can't know about: a `git pull`, a branch switch, a dependency install, or edits from another process or editor. It does **not** apply to code the current agent just wrote --- the agent already has that in context, and the running application discovers it live on the next request regardless. +:::note[On-read self-refresh covers `app/` and `modules/`; vendor needs an explicit rebuild] +The running MCP/LSP server re-checks staleness on every read — there is no TTL window. Additions, edits, and deletions under `app/` and `modules/` are picked up automatically; the very next tool call sees the updated index. **Vendor is excluded** from the on-read check: new or removed packages installed via `composer require` / `composer update` are not detected until you run `marko indexer:rebuild`. Use `marko indexer:rebuild` for three scenarios: a forced clean rebuild, warming a cold cache (e.g. after a fresh clone or a `devai:install`), or picking up vendor changes. It is not needed for routine development under `app/` or `modules/`. ::: ## API Reference