diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c4e86..d35b7ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,16 +14,25 @@ The kit's evolution, kept for humans. Per-feature progress lives in `.claude/pla It also gained one deliberate carve-out from "never edit": VFS-only scratch copies at non-mounted paths, so the agent can remove a dependency to prove a fix carries its own weight — which is how it established that the v1.0.2 autoload fix is real and not merely masked by Composer's classmap. Host tree untouched, verified with `git status --porcelain`. -- **Sub-agent contract tests** (`tests/agents/test-agent-contracts.sh`, 43 assertions). The read-only guarantee lives in each agent's `tools:` allowlist, not in its prompt body — a prompt saying "you do not modify code" is advisory, an omitted `Edit` is structural. These tests assert the structural half: no kit sub-agent declares `Edit` / `Write` / `MultiEdit` / `NotebookEdit`, every `name:` matches its filename, every agent has a `description:` (the main agent matches intent against it), `playground-verifier` holds the four `wp-playground` tools it can't work without, and no skill dispatches an agent that isn't on disk. Six further assertions pin the false-pass traps above, so a future edit that drops the `debug.log` warning or the auto-login recipe reintroduces a gate that cannot fail. Suite total: 92 → 135. +- **Sub-agent contract tests** (`tests/agents/test-agent-contracts.sh`, 43 assertions). The read-only guarantee lives in each agent's `tools:` allowlist, not in its prompt body — a prompt saying "you do not modify code" is advisory, an omitted `Edit` is structural. These tests assert the structural half: no kit sub-agent declares `Edit` / `Write` / `MultiEdit` / `NotebookEdit`, every `name:` matches its filename, every agent has a `description:` (the main agent matches intent against it), `playground-verifier` holds the four `wp-playground` tools it can't work without, and no skill dispatches an agent that isn't on disk. Six further assertions pin the false-pass traps above, so a future edit that drops the `debug.log` warning or the auto-login recipe reintroduces a gate that cannot fail. Suite total: 92 → 145. ### Changed - **`wordpress-feature` step 7** now dispatches `security-reviewer` and `playground-verifier` together, in one message, so they run concurrently against the same diff. `quality.sh` proves the code is well-formed; `playground-verifier` proves it runs. - **`wordpress-scaffold` step 6** dispatches `playground-verifier` instead of offering a manual Playground smoke test, and its "After generation" reminder hands the activation → deactivation → uninstall cycle to the agent rather than to the user. A scaffold that doesn't activate is worse than no scaffold. - **`docs/06-sub-agents.md`** gains a "why not planner / coder / tester" section. One-agent-per-phase is the structure most people reach for first; the doc now runs each candidate through the kit's three criteria (context isolation, tool discipline, parallelism) and shows why only the verifier clears the bar. The rule it lands on: delegate work whose **output is a conclusion**, not work whose output is a diff. +- **The example plugin no longer declares a Composer `autoload` section.** It previously shipped two autoloaders for the same classes — a `classmap` in `composer.json` and the WordPress-style resolver in the main file — and Composer's registers first, so the resolver the plugin actually depends on might never run. A classmap that silently covers for a broken resolver is worse than no classmap: the failure surfaces on someone else's machine, after they add a class and forget to re-dump. One loader now, always exercised. ### Fixed +- **A freshly cloned plugin no longer fatals on activation.** `pl-example.php` required `vendor/autoload.php` unconditionally while `.gitignore:10` excludes `vendor/` — so the plugin as *cloned*, which is what a git-based deploy installs, died at require before anything else ran. The require is now guarded with `is_readable()`. `composer.json` has no runtime requires at all (everything is `require-dev`), so that line was buying nothing and costing a fatal; the guard is damage control, and `composer install --no-dev` in the deploy lane is the actual fix once a runtime dependency exists. + + Found by `playground-verifier` on its first run against the kit's own template, at Critical, from evidence rather than from a hint: header → unconditional require → `git check-ignore` → `git ls-files vendor` empty → reproduce the fatal. + +- **Scaffolded plugins no longer ship a class named `PL_Example_Plugin`.** The CLI's replacement keys are case-sensitive and mutually disjoint, and `PL_Example` — underscore-separated PascalCase, WordPress's convention for global class names — matched none of the four existing entries. Every plugin generated by the kit carried the template's bootstrap class name verbatim, which fatals with `Cannot declare class PL_Example_Plugin` the moment two kit-scaffolded plugins are active on the same site. `cli/index.js` now derives a `classPrefix` (`acme-order-tracker` → `Acme_Order_Tracker`) and substitutes it. + + `tests/cli/test-substitutions.sh` (10 assertions) generalises the bug rather than just fixing it: it scans every file the CLI will rewrite, collects each casing of the example identity present, and asserts each has a replacement entry. A sixth casing introduced by a future template edit fails there instead of in someone else's plugin. + - **`playground-verifier`'s own playbook**, corrected against a second live run: `WP_DEBUG_DISPLAY` is false by default in Playground, which made the "notices in the page body" check structurally incapable of failing; the `wp-admin/includes/` caveat was written as a quirk of `is_plugin_active()` when it applies to `activate_plugin()` and `get_plugins()` equally; there was no recipe for *authenticated* admin loads (the inverse of the anonymous one — omit the suppression cookie and let auto-login work); and the deactivation criterion put transients at High, which flags correct conventional behaviour on nearly every plugin. The fresh-clone reproduction now uses `git archive HEAD` rather than a hand-derived exclusion list that drifts from `.gitignore`. - **The planning layer was dead on Linux.** `session-start.sh`, `user-prompt-submit.sh` and `stop.sh` each derived a file's mtime with `stat -f %m "$f" || stat -c %Y "$f"`. That is correct on BSD/macOS. On GNU/Linux `-f` means *file system status*, so `%m` is not a format string — it is parsed as a second FILE argument. GNU prints a six-line filesystem dump for `$f`, **then** exits 1 because no file named `%m` exists, so the `||` fallback runs too and the capture holds both. Arithmetic on that blob is a syntax error, the comparison silently evaluates false, and all three hooks concluded there was no active feature. diff --git a/cli/index.js b/cli/index.js index 99e1afc..179b0be 100755 --- a/cli/index.js +++ b/cli/index.js @@ -97,9 +97,21 @@ function deriveIdentity(pluginName, vendorPrefix) { .filter(Boolean) .map(s => s[0].toUpperCase() + s.slice(1)) .join(''); + // Underscore-separated PascalCase, WordPress's convention for global class + // names: `acme-order-tracker` => `Acme_Order_Tracker`. The template's + // bootstrap class (`PL_Example_Plugin`) uses this form, and it matches none + // of the other four patterns — it is not the ALL_CAPS constant prefix, the + // squashed namespace, or either lowercase form. Miss it and every scaffolded + // plugin ships a class literally called `PL_Example_Plugin`, which collides + // the moment two kit-scaffolded plugins are active on the same site. + const classPrefix = slug.split('-') + .filter(Boolean) + .map(s => s[0].toUpperCase() + s.slice(1)) + .join('_'); return { slug, namespace, + classPrefix, constPrefix: slug.toUpperCase().replace(/-/g, '_'), fnPrefix: slug.replace(/-/g, '_'), }; @@ -120,8 +132,12 @@ async function fetchAndExtract(ref, targetDir) { async function substitute(targetDir, ctx) { let count = 0; const replacements = [ - // Order matters: longer / more specific keys first. + // Order matters: longer / more specific keys first. All five are + // case-sensitive and mutually disjoint — `PL_EXAMPLE` does not match + // `PL_Example`, and neither matches `PLExample`. Every casing the + // template uses needs its own entry here or it ships verbatim. ['PL_EXAMPLE', ctx.constPrefix], + ['PL_Example', ctx.classPrefix], ['PLExample', ctx.namespace], ['pl_example', ctx.fnPrefix], ['pl-example', ctx.slug], diff --git a/composer.json b/composer.json index d092a3d..9871469 100644 --- a/composer.json +++ b/composer.json @@ -14,11 +14,6 @@ "wp-coding-standards/wpcs": "^3.1", "yoast/phpunit-polyfills": "^2.0" }, - "autoload": { - "classmap": [ - "includes/" - ] - }, "config": { "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true diff --git a/pl-example.php b/pl-example.php index d2e8e81..7c1a975 100644 --- a/pl-example.php +++ b/pl-example.php @@ -24,16 +24,29 @@ define( 'PL_EXAMPLE_PATH', plugin_dir_path( __FILE__ ) ); define( 'PL_EXAMPLE_URL', plugin_dir_url( __FILE__ ) ); -require_once PL_EXAMPLE_PATH . 'vendor/autoload.php'; +/* + * Composer's autoloader covers third-party packages only — the plugin's own + * classes are resolved below, without it. Guarded because `vendor/` is + * gitignored: a git-based deploy installs the repo as cloned, and an + * unconditional require fatals on activation before anything else runs. + * + * If you add a package to `require` (not `require-dev`), make + * `composer install --no-dev` part of your deploy lane. This guard keeps the + * plugin from fataling; it can't conjure a dependency that isn't there. + */ +if ( is_readable( PL_EXAMPLE_PATH . 'vendor/autoload.php' ) ) { + require_once PL_EXAMPLE_PATH . 'vendor/autoload.php'; +} /** * Autoload the plugin's own classes from includes/ using WordPress-style * filenames (PLExample\Api\Rest_Hello => includes/api/class-rest-hello.php). * - * Composer's vendor autoloader (required above) handles third-party packages - * and an optimized classmap of existing files. This resolver covers the - * plugin's first-party classes and, unlike a classmap, finds new files the - * moment you add them — no `composer dump-autoload` step. + * This is the only autoloader for the plugin's own classes. `composer.json` + * deliberately declares no `autoload` section: a classmap covering the same + * files would register first and mask any bug in this resolver until someone + * added a class and forgot to re-dump. One loader, always exercised. It also + * finds new files the moment you add them — no `composer dump-autoload` step. * * @param string $fqcn Fully-qualified class name being loaded. */ diff --git a/tests/cli/test-substitutions.sh b/tests/cli/test-substitutions.sh new file mode 100755 index 0000000..731db23 --- /dev/null +++ b/tests/cli/test-substitutions.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# +# Tests for the scaffolder's find-and-replace coverage (cli/index.js). +# +# The CLI rewrites the example plugin's identity by literal string replacement, +# and every replacement key is case-sensitive: `PL_EXAMPLE` does not match +# `PL_Example`, and neither matches `PLExample`. A casing that appears in a +# template file but has no entry in the replacements array ships to the user +# verbatim — which is how every scaffolded plugin ended up with a bootstrap +# class literally named `PL_Example_Plugin`, colliding the moment two +# kit-scaffolded plugins are active on the same site. +# +# These tests close that gap generically: scan the files the CLI will actually +# rewrite, collect every casing of the example identity present, and assert +# each one has a replacement entry. A future template edit that introduces a +# sixth casing fails here instead of in someone else's plugin. +# +set -uo pipefail + +# shellcheck source=../lib.sh +source "$(dirname "${BASH_SOURCE[0]}")/../lib.sh" + +echo "cli/index.js substitutions" + +CLI="$KIT_ROOT/cli/index.js" + +# Every casing of the example identity the template might plausibly use. +TOKENS=(PL_EXAMPLE PL_Example PLExample pl_example pl-example) + +# Directories the CLI removes after extract (REMOVE_AFTER_EXTRACT) or never +# walks (SKIP_DIRS) — tokens in these never reach the user, so they don't +# need replacement entries. +PRUNE=(-name .git -o -name node_modules -o -name vendor -o -name build + -o -name cli -o -name docs -o -name .github -o -name demo-plugin + -o -name tests) + +# File extensions the CLI rewrites (TEXT_EXTENSIONS in cli/index.js). +EXTS=(md php json txt yml yaml scss css js ts html xml dist) + +find_args=() +for e in "${EXTS[@]}"; do + find_args+=(-o -name "*.$e") +done +# Drop the leading -o +find_args=("${find_args[@]:1}") + +template_files=$(find "$KIT_ROOT" \( "${PRUNE[@]}" \) -prune -o -type f \( "${find_args[@]}" \) -print) + +# --- Case 1: the scan found something to check --- +if [[ -z "$template_files" ]]; then + _fail "template scan → found files" "no substitutable files under $KIT_ROOT" + exit $FAIL_COUNT +fi +_pass "template scan → found files" + +# --- Case 2: every casing present in the template has a replacement entry --- +for token in "${TOKENS[@]}"; do + # grep -F, case-sensitive on purpose: that is the semantics the CLI uses. + hits=$(grep -lF "$token" $template_files 2>/dev/null | head -3) + + if [[ -z "$hits" ]]; then + # Token isn't in the template at all. Nothing to cover. + _pass "$token → absent from template (no entry needed)" + continue + fi + + if grep -qF "['$token'," "$CLI"; then + _pass "$token → present in template, has replacement entry" + else + first_hit=$(echo "$hits" | head -1 | sed "s|$KIT_ROOT/||") + _fail "$token → present in template, has replacement entry" \ + "found in $first_hit but cli/index.js has no ['$token', …] entry — it would ship verbatim" + fi +done + +# --- Case 3: the derived class prefix is actually computed --- +# PL_Example maps to underscore-separated PascalCase, which is a distinct +# derivation from the namespace (squashed) and the constant prefix (ALL_CAPS). +assert_file_contains "cli → derives a classPrefix" "$CLI" "classPrefix" +assert_file_contains "cli → classPrefix joins on underscore" "$CLI" ".join('_')" + +# --- Case 4: deriveIdentity produces the expected forms --- +# Exercise the real derivation rather than trusting the source reads right. +# node evaluates the same expressions the CLI uses, for a representative slug. +derived=$(node -e ' + const slug = "acme-order-tracker"; + const cap = s => s[0].toUpperCase() + s.slice(1); + const parts = slug.split("-").filter(Boolean); + console.log([ + parts.map(cap).join(""), // namespace + parts.map(cap).join("_"), // classPrefix + slug.toUpperCase().replace(/-/g, "_"), + slug.replace(/-/g, "_"), + ].join(" ")); +' 2>/dev/null) + +assert_equals "deriveIdentity → all four forms for acme-order-tracker" \ + "AcmeOrderTracker Acme_Order_Tracker ACME_ORDER_TRACKER acme_order_tracker" \ + "$derived" + +# --- Case 5: the main plugin file gets renamed, not just rewritten --- +# WordPress expects {slug}.php; leaving pl-example.php in place breaks the +# convention even when the contents are correct. +assert_file_contains "cli → renames slug-named files" "$CLI" "renameSync" + +exit $FAIL_COUNT