From ec2ff714c0c070fcb0eae504ffe0118be11f09b1 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 17 Jul 2026 02:00:05 +0530 Subject: [PATCH 1/3] fix(init): harden --list/--json into a real machine contract --list errored in setup mode; manage mode never exposed capability sets. Both pushed AI agents to grep engine source instead of using one deterministic, JSON-safe command. - Works in both modes; all errors now answer as JSON on stderr, not prose. - Reconciles recorded capability intent against disk state, surfacing drift. - Guards detect() probes; a crash degrades to a warning, not a failure. - Fixes stale scaffold-skill TDD wording and two pre-existing lint errors. --- .../claude-skills/templates/scaffold-SKILL.md | 10 +- .../wp-tooling/skills/scaffold/SKILL.md | 10 +- .../wp-tooling/src/init/capabilities.js | 138 +++++ node-packages/wp-tooling/src/init/features.js | 25 + node-packages/wp-tooling/src/init/index.js | 277 ++++++++- node-packages/wp-tooling/src/init/manage.js | 31 +- node-packages/wp-tooling/src/init/persist.js | 31 +- node-packages/wp-tooling/src/scaffolds/add.js | 37 +- .../wp-tooling/src/scaffolds/errors.js | 49 +- .../wp-tooling/tests/init/_helpers.js | 62 ++ .../tests/init/capabilities.test.js | 555 ++++++++++++++++++ .../wp-tooling/tests/init/persist.test.js | 72 +++ .../wp-tooling/tests/ui/selects.test.js | 10 +- 13 files changed, 1226 insertions(+), 81 deletions(-) create mode 100644 node-packages/wp-tooling/src/init/capabilities.js create mode 100644 node-packages/wp-tooling/tests/init/_helpers.js create mode 100644 node-packages/wp-tooling/tests/init/capabilities.test.js create mode 100644 node-packages/wp-tooling/tests/init/persist.test.js diff --git a/node-packages/wp-tooling/scaffolds/setup/claude-skills/templates/scaffold-SKILL.md b/node-packages/wp-tooling/scaffolds/setup/claude-skills/templates/scaffold-SKILL.md index e0c1784..09cfa93 100644 --- a/node-packages/wp-tooling/scaffolds/setup/claude-skills/templates/scaffold-SKILL.md +++ b/node-packages/wp-tooling/scaffolds/setup/claude-skills/templates/scaffold-SKILL.md @@ -96,6 +96,8 @@ Write a test-case checklist covering: - `wp/cron`: `wp_next_scheduled()`, callback fires, unschedule works. - `wp/cli`: `WP_CLI::add_command` registered, `__invoke` behaviour, dry-run flag. +The engine's shipped test file already covers **Integration** for a plain instance of the kind (e.g. `post_type_exists()` for `wp/cpt` ships written and passing, not as a stub) — list it to confirm coverage, not to write it. Your effort in §7 goes to **Happy path / Edge cases / Error paths**: brief-specific behaviour the engine can't know. + Show the checklist to the developer. Ask: confirm, add, remove? Resolve before scaffolding. This is the cheapest place to catch a misread requirement. ### 5. Apply conventions, invoke the engine @@ -127,7 +129,7 @@ Result shape: `{ scaffold, engine, developer, ai, warnings }`. | `developer.install.composer` / `developer.install.npm` | Print as copy-paste command. **Never run `composer require` / `npm install`.** | | `developer.secrets` | Print as `gh secret set` checklist. **Never read/write/log/transmit values.** | | `ai.wiring` | Adaptive wiring with consent (see 6a). | -| `ai.tests` | Mandatory expansion under TDD loop (see 7). | +| `ai.tests` | Shipped complete + passing for the generic pattern; add brief-specific methods under the TDD loop (see 7). | | `warnings` | Print to developer. | #### 6a. Adaptive wiring @@ -147,8 +149,8 @@ For block scaffolds, surface a developer action before testing: "run `npm run bu | Step | Action | |---|---| -| A | Expand the engine's stub into the full suite from §4's checklist. Strip every `markTestIncomplete`. | -| B | Run: `composer test` / `composer test:unit` (PHP), `npm run test:js` / `npx jest` (JS). Expect red. If the runner errors before running, invoke the relevant `setup/*` scaffold and retry. | +| A | Confirm the engine's shipped tests pass as-is (they cover §4's Integration row already, complete and green — not a step you perform). Write one new test method per remaining §4 row: the brief-specific behaviour the engine couldn't know. | +| B | Run: `composer test` / `composer test:unit` (PHP), `npm run test:js` / `npx jest` (JS). Expect red only for the methods just added — the shipped tests stay green throughout; one going red means you broke the generic pattern, so stop and investigate. Confirm each red is an assertion failure, not a bootstrap/fatal error (env or wiring trouble isn't a valid TDD red). Runner errors before running → invoke the relevant `setup/*` scaffold and retry. | | C | Implement just enough production code to flip **one** failing test green. | | D | Re-run. Confirm that one test passes. | | E | Loop B-D one test at a time. | @@ -186,7 +188,7 @@ Escalation report format: **what you tried, what you observed, what's blocking, ## Hard rules - never violate -- Never write production code before its test exists on disk. +- Never hand-write behaviour code before its test exists on disk. (The engine's scaffolded class + test ship together, already passing, for the generic pattern — you didn't author it, so it's not a violation. The rule governs the brief-specific behaviour you add: test first, confirm red, then extend. Consented §6a wiring is likewise sanctioned, not authored behaviour.) - Never hand-write an artifact the engine can scaffold. - Never group multiple kinds under a per-feature folder (`Modules//...`). - Never declare an artifact done without its test file passing. diff --git a/node-packages/wp-tooling/skills/scaffold/SKILL.md b/node-packages/wp-tooling/skills/scaffold/SKILL.md index e0c1784..09cfa93 100644 --- a/node-packages/wp-tooling/skills/scaffold/SKILL.md +++ b/node-packages/wp-tooling/skills/scaffold/SKILL.md @@ -96,6 +96,8 @@ Write a test-case checklist covering: - `wp/cron`: `wp_next_scheduled()`, callback fires, unschedule works. - `wp/cli`: `WP_CLI::add_command` registered, `__invoke` behaviour, dry-run flag. +The engine's shipped test file already covers **Integration** for a plain instance of the kind (e.g. `post_type_exists()` for `wp/cpt` ships written and passing, not as a stub) — list it to confirm coverage, not to write it. Your effort in §7 goes to **Happy path / Edge cases / Error paths**: brief-specific behaviour the engine can't know. + Show the checklist to the developer. Ask: confirm, add, remove? Resolve before scaffolding. This is the cheapest place to catch a misread requirement. ### 5. Apply conventions, invoke the engine @@ -127,7 +129,7 @@ Result shape: `{ scaffold, engine, developer, ai, warnings }`. | `developer.install.composer` / `developer.install.npm` | Print as copy-paste command. **Never run `composer require` / `npm install`.** | | `developer.secrets` | Print as `gh secret set` checklist. **Never read/write/log/transmit values.** | | `ai.wiring` | Adaptive wiring with consent (see 6a). | -| `ai.tests` | Mandatory expansion under TDD loop (see 7). | +| `ai.tests` | Shipped complete + passing for the generic pattern; add brief-specific methods under the TDD loop (see 7). | | `warnings` | Print to developer. | #### 6a. Adaptive wiring @@ -147,8 +149,8 @@ For block scaffolds, surface a developer action before testing: "run `npm run bu | Step | Action | |---|---| -| A | Expand the engine's stub into the full suite from §4's checklist. Strip every `markTestIncomplete`. | -| B | Run: `composer test` / `composer test:unit` (PHP), `npm run test:js` / `npx jest` (JS). Expect red. If the runner errors before running, invoke the relevant `setup/*` scaffold and retry. | +| A | Confirm the engine's shipped tests pass as-is (they cover §4's Integration row already, complete and green — not a step you perform). Write one new test method per remaining §4 row: the brief-specific behaviour the engine couldn't know. | +| B | Run: `composer test` / `composer test:unit` (PHP), `npm run test:js` / `npx jest` (JS). Expect red only for the methods just added — the shipped tests stay green throughout; one going red means you broke the generic pattern, so stop and investigate. Confirm each red is an assertion failure, not a bootstrap/fatal error (env or wiring trouble isn't a valid TDD red). Runner errors before running → invoke the relevant `setup/*` scaffold and retry. | | C | Implement just enough production code to flip **one** failing test green. | | D | Re-run. Confirm that one test passes. | | E | Loop B-D one test at a time. | @@ -186,7 +188,7 @@ Escalation report format: **what you tried, what you observed, what's blocking, ## Hard rules - never violate -- Never write production code before its test exists on disk. +- Never hand-write behaviour code before its test exists on disk. (The engine's scaffolded class + test ship together, already passing, for the generic pattern — you didn't author it, so it's not a violation. The rule governs the brief-specific behaviour you add: test first, confirm red, then extend. Consented §6a wiring is likewise sanctioned, not authored behaviour.) - Never hand-write an artifact the engine can scaffold. - Never group multiple kinds under a per-feature folder (`Modules//...`). - Never declare an artifact done without its test file passing. diff --git a/node-packages/wp-tooling/src/init/capabilities.js b/node-packages/wp-tooling/src/init/capabilities.js new file mode 100644 index 0000000..0e6c0ba --- /dev/null +++ b/node-packages/wp-tooling/src/init/capabilities.js @@ -0,0 +1,138 @@ +/** + * Capability enumeration for `--list`. + * + * "Capabilities" are the keep/remove example sets a project declares in its + * scaffold config under `examples.groups` (post types, taxonomies, blocks, CI + * workflows, ...). Unlike features (tailwind / hmr), they are a one-shot choice + * at setup, so their state is RECONCILED from two sources (mirroring the + * features reconcile pattern -- neither side silently overrides the other): + * + * - `detected`: does the group's primary on-disk artifact (first concrete, + * non-glob `remove` path) still exist? Null when the group has no concrete + * artifact (glob-only / file-less groups) -- unknown, never guessed. + * - `intent`: the selection recorded at setup in `.wp-scaffold.json` under + * `examples.removed`. Null for identities written before that field + * existed (legacy). + * + * `present` prefers detection, falls back to intent, then to true (a fresh + * starter ships everything). `drift` flags a disagreement when BOTH sides are + * known -- e.g. the sanctioned manual removal of a module after setup. + * + * This module is engine-generic: `module` comes from the group's own config + * declaration when present; the `inc/Modules/.php` artifact fallback is + * a convenience for configs that predate the declared field. + */ + +'use strict'; + +const fs = require('fs'); +const { resolveWithin } = require('./transform'); + +/** + * Existence check confined to the project root; never throws. + * + * @param {string} root - Project root. + * @param {string} rel - Project-relative path. + * @return {boolean} True when the path exists inside root. + */ +const safeExists = (root, rel) => { + try { + return fs.existsSync(resolveWithin(root, rel)); + } catch { + return false; + } +}; + +/** + * The removed-at-setup record from `.wp-scaffold.json`, or null when the + * identity predates the `examples.removed` field (or there is no identity). + * + * @param {Object|null} identity - Parsed identity file. + * @return {Set|null} Removed group keys, or null when unrecorded. + */ +const recordedRemovals = (identity) => + identity && identity.examples && Array.isArray(identity.examples.removed) + ? new Set(identity.examples.removed) + : null; + +/** + * Enumerate the project's keep/remove capabilities, reconciling recorded + * intent against detected reality. Reads the filesystem but changes nothing. + * + * @param {Object} config - Per-project scaffold config. + * @param {string} root - Project root. + * @param {Object|null} [identity] - Parsed .wp-scaffold.json (manage mode). + * @return {Array} One row per capability: + * `{ key, label, category, module, present, detected, intent, drift }`. + */ +const listCapabilities = (config, root, identity = null) => { + const groups = (config.examples && config.examples.groups) || []; + const removed = recordedRemovals(identity); + + return groups.map((g) => { + const remove = g.remove || []; + // Detection uses the first concrete (non-glob) `remove` entry, not + // `strip` -- strip targets shared files (inc/Core/*) that survive removal. + const artifact = remove.find((r) => !/[*?]/.test(r)) || null; + const detected = artifact ? safeExists(root, artifact) : null; + const intent = removed ? !removed.has(g.key) : null; + + let module = null; + if (undefined !== g.module) { + module = g.module || null; + } else if (artifact) { + const match = artifact.match( + /(?:^|\/)inc\/Modules\/([\w-]+)\.php$/ + ); + module = (match && match[1]) || null; + } + + let present = true; + if (null !== detected) { + present = detected; + } else if (null !== intent) { + present = intent; + } + + return { + key: g.key, + label: g.label, + category: g.category || 'Other', + module, + present, + detected, + intent, + drift: null !== detected && null !== intent && detected !== intent, + }; + }); +}; + +/** + * Render capabilities as a human-readable table (mirrors manage's showStatus). + * + * @param {Array} rows - Rows from `listCapabilities`. + * @param {Object} ui - UI kit. + * @param {Object} [opts] - Options. + * @param {string} [opts.mode] - 'setup' | 'manage' (drives the hint line). + * @return {void} + */ +const showCapabilities = (rows, ui, opts = {}) => { + if (!rows.length) { + ui.info('No keep/remove capabilities are declared for this project.'); + return; + } + ui.table( + rows.map((r) => [ + r.label, + `${r.present ? 'present' : 'removed'}${r.drift ? ' (drift)' : ''}`, + ]), + { title: 'Capabilities' } + ); + if ('manage' === opts.mode && rows.some((r) => !r.present)) { + ui.info( + 'Removed capabilities were example sets dropped at setup; add real ones with `npx wp-tooling add`.' + ); + } +}; + +module.exports = { listCapabilities, showCapabilities }; diff --git a/node-packages/wp-tooling/src/init/features.js b/node-packages/wp-tooling/src/init/features.js index 2acdd16..32e325e 100644 --- a/node-packages/wp-tooling/src/init/features.js +++ b/node-packages/wp-tooling/src/init/features.js @@ -518,6 +518,30 @@ const detectMap = (config, api) => { return map; }; +/** + * Guarded variant of `detectMap` for read-only reporting (`--list`): a + * throwing config-supplied `detect` probe must degrade to "unknown" instead of + * crashing the machine contract. Toggle flows keep the loud `detectMap`. + * + * @param {Object} config - Per-project scaffold config. + * @param {Object} api - FeatureApi. + * @return {{map: Object, errors: Array<{key: string, message: string}>}} + * `map[key]` is boolean, or null when that feature's probe threw. + */ +const safeDetectMap = (config, api) => { + const map = {}; + const errors = []; + (config.features || []).forEach((feature) => { + try { + map[feature.key] = detectFeature(feature, api); + } catch (err) { + map[feature.key] = null; + errors.push({ key: feature.key, message: err.message }); + } + }); + return { map, errors }; +}; + /** * Whether a feature touches package.json (so we know to suggest npm install). * @@ -702,5 +726,6 @@ module.exports = { reconcile, computeDiff, detectMap, + safeDetectMap, toggleFeatures, }; diff --git a/node-packages/wp-tooling/src/init/index.js b/node-packages/wp-tooling/src/init/index.js index b952384..885fbce 100644 --- a/node-packages/wp-tooling/src/init/index.js +++ b/node-packages/wp-tooling/src/init/index.js @@ -35,17 +35,24 @@ const { renameFiles, applyVersion, } = require('./transform'); -const { writeIdentityFile, readIdentityFile } = require('./persist'); +const { + writeIdentityFile, + readIdentityFile, + IdentityFileError, +} = require('./persist'); const { initRepo, commitAll, installGitHooks } = require('./git'); const { runCleanup } = require('./cleanup'); const { validateFeatures, makeFeatureApi, detectMap, + safeDetectMap, toggleFeatures, } = require('./features'); -const { manageFlow } = require('./manage'); +const { manageFlow, showStatus } = require('./manage'); const { applyExamples } = require('./examples'); +const { listCapabilities, showCapabilities } = require('./capabilities'); +const { formatErrorPayload } = require('../scaffolds/errors'); const DEFAULT_VERSION = '1.0.0'; const GENERATED_BY = '@rtcamp/wp-tooling init'; @@ -90,12 +97,17 @@ Scaffold options (first run): prompt (by category); unchecking a capability removes it entirely. Manage options (after set up): - --list Print feature status and exit. --features=a,b Set the exact enabled feature set (empty = none). --enable=a,b Enable features (delta). --disable=a,b Disable features (delta). -y, --yes Apply the flag selection without confirming. +Query options (any time, before or after set up): + --list List capabilities and optional features, then exit. + --json With --list: emit one JSON line + ({ mode, capabilities, features, warnings }). + +General: -c, --clean Run cleanup only (remove scaffolding files). -h, --help Show this help. `); @@ -229,7 +241,15 @@ const composerDump = (root) => { const setupSteps = (config, root, flags) => { const kind = config.kind || 'project'; const steps = config.steps || {}; - const existing = readIdentityFile(root); + // Corrupt identity reads as absent here: run() already refused to enter + // setup on corruption without --reinit, so reaching this point means the + // file may be discarded. + let existing = null; + try { + existing = readIdentityFile(root); + } catch { + existing = null; + } return [ { @@ -347,7 +367,9 @@ const setupSteps = (config, root, flags) => { skip: (c) => c.cancelled || (!(config.features || []).length && - !(config.examples && (config.examples.groups || []).length)), + !( + config.examples && (config.examples.groups || []).length + )), async run(c) { const features = config.features || []; const groups = @@ -387,18 +409,18 @@ const setupSteps = (config, root, flags) => { ]; const order = []; const byCat = new Map(); - for (const cap of caps) { - if (!byCat.has(cap.category)) { - byCat.set(cap.category, []); - order.push(cap.category); + for (const entry of caps) { + if (!byCat.has(entry.category)) { + byCat.set(entry.category, []); + order.push(entry.category); } - byCat.get(cap.category).push(cap); + byCat.get(entry.category).push(entry); } const treeGroups = order.map((category) => ({ label: category, - items: byCat.get(category).map((cap) => ({ - label: cap.label, - checked: cap.checked, + items: byCat.get(category).map((entry) => ({ + label: entry.label, + checked: entry.checked, })), })); const checked = new Set( @@ -448,6 +470,11 @@ const setupSteps = (config, root, flags) => { if (groups.length) { applyExamples(config, root, ui, removeKeys); } + // Record the selection: `--list` reconciles this intent against + // detected reality instead of re-deriving it from disk alone. + c.persistPayload.examples = { + removed: Array.from(removeKeys).sort(), + }; if (features.length) { const result = await toggleFeatures(config, root, { mode: 'scaffold', @@ -596,6 +623,143 @@ const cleanFlow = async (config, root) => { ui.success(`Cleanup complete (${removed} removed).`); }; +/** + * Emit one machine-readable error line on stderr -- the `--json` failure + * contract, shared with `wp-tooling add` via `formatErrorPayload`. + * + * @param {Error} err - The error to report. + * @return {void} + */ +const emitJsonError = (err) => { + process.stderr.write(`${JSON.stringify(formatErrorPayload(err))}\n`); +}; + +/** + * Build a usage error carrying the stable `EUSAGE` machine code. + * + * @param {string} message - Human-readable message. + * @return {Error} Coded error. + */ +const usageError = (message) => + Object.assign(new Error(message), { code: 'EUSAGE' }); + +/** + * `--list`: report the project's capabilities and optional features, in either + * mode, without mutating anything. Human tables by default; a single JSON line + * ({ mode, capabilities, features, warnings }) with `--json` -- the AI-facing + * contract, so orchestrators never read the scaffold config to enumerate them. + * + * Feature `on` means the EFFECTIVE state in both modes: detected reality in + * manage mode, `defaultOn || detected` (what a non-interactive setup would + * enable) in setup mode. Detect probes are guarded: a throwing probe degrades + * that feature to `on: null` plus a warning instead of breaking the contract. + * + * @param {Object} config - Per-project scaffold config. + * @param {string} root - Project root. + * @param {Object} opts - Options. + * @param {string} opts.mode - 'setup' | 'manage'. + * @param {boolean} opts.json - Emit machine-readable JSON. + * @param {Object|null} opts.identity - Parsed .wp-scaffold.json (manage only). + * @param {string[]} [opts.seedWarnings] - Warnings collected by the caller. + * @return {void} + */ +const listFlow = (config, root, { mode, json, identity, seedWarnings }) => { + const kind = config.kind || 'project'; + const warnings = [...(seedWarnings || [])]; + const manage = 'manage' === mode; + + const capabilityRows = listCapabilities( + config, + root, + manage ? identity : null + ); + if ( + manage && + capabilityRows.length && + capabilityRows.every((r) => null === r.intent) + ) { + warnings.push( + 'capability selection was not recorded by this setup (older init); state is disk-detected only.' + ); + } + const capabilities = capabilityRows.map((r) => + manage + ? { + key: r.key, + label: r.label, + category: r.category, + module: r.module, + present: r.present, + intent: r.intent, + drift: r.drift, + } + : { + key: r.key, + label: r.label, + category: r.category, + module: r.module, + present: r.present, + } + ); + + // Detection needs an identity on the api (probes read api.identity.*). + // Pre-setup, the starter's own placeholder identity IS the on-disk reality. + let apiIdentity = identity; + if (!manage) { + apiIdentity = config.source ? placeholderIdentity(config) : {}; + } + const api = makeFeatureApi(root, apiIdentity, ui); + const { map, errors } = safeDetectMap(config, api); + errors.forEach(({ key, message }) => + warnings.push(`${key}: feature detect failed: ${message}`) + ); + + const persisted = (manage && identity && identity.features) || {}; + const features = (config.features || []).map((f) => { + const detected = map[f.key]; + // Setup mode reports the non-interactive default: defaultOn || detected. + let on = detected; + if (!manage && f.defaultOn) { + on = true; + } + const row = { + key: f.key, + label: f.label, + description: f.description || '', + on, + }; + if (manage) { + row.intent = Boolean(persisted[f.key]); + row.drift = null === detected ? false : detected !== row.intent; + } + return row; + }); + if (manage) { + const known = new Set((config.features || []).map((f) => f.key)); + Object.keys(persisted) + .filter((key) => !known.has(key)) + .forEach((key) => + warnings.push( + `${key}: recorded in .wp-scaffold.json but no longer declared.` + ) + ); + } + + if (json) { + process.stdout.write( + `${JSON.stringify({ mode, capabilities, features, warnings })}\n` + ); + return; + } + + ui.heading( + `${cap(kind)} — ${manage ? 'status' : 'available capabilities'}` + ); + showCapabilities(capabilityRows, ui, { mode }); + showStatus(features, [], ui); + warnings.forEach((w) => ui.warn(w)); +}; + /** * Entry point. Called by each starter's `bin/init.js`. * @@ -619,6 +783,69 @@ const run = async (config, options = {}) => { return; } + // --list / --json: the read-only machine-query contract. Intercepted ahead + // of every other flow so config and identity failures also answer in JSON + // when asked to: one stdout line on success, one stderr line on failure. + if (argv.includes('--list') || argv.includes('--json')) { + const wantJson = argv.includes('--json'); + try { + if (!argv.includes('--list')) { + throw usageError('--json is only supported with --list'); + } + // --manage is excluded: mode derives from identity/--reinit alone, + // so accepting it here would silently ignore it. --yes is accepted + // (and ignored) so scripted `--list --yes` calls keep working. + const allowed = new Set([ + '--list', + '--json', + '--reinit', + '--yes', + '-y', + ]); + const extra = argv.filter((arg) => !allowed.has(arg)); + if (extra.length) { + throw usageError( + `--list cannot be combined with: ${extra.join(' ')}` + ); + } + try { + validateFeatures(config); + } catch (err) { + err.code = 'ECONFIG'; + throw err; + } + const seedWarnings = []; + let identity = null; + try { + identity = readIdentityFile(root); + } catch (err) { + if (!argv.includes('--reinit')) { + throw err; + } + // --reinit means "discard what's there": report setup mode. + seedWarnings.push( + '.wp-scaffold.json is corrupt; --reinit will overwrite it.' + ); + } + const mode = + identity && !argv.includes('--reinit') ? 'manage' : 'setup'; + listFlow(config, root, { + mode, + json: wantJson, + identity: 'manage' === mode ? identity : null, + seedWarnings, + }); + } catch (err) { + if (wantJson) { + emitJsonError(err); + } else { + ui.error(err.message); + } + process.exitCode = 1; + } + return; + } + try { // Cleanup works in either mode. if (argv.includes('--clean') || argv.includes('-c')) { @@ -643,8 +870,24 @@ const run = async (config, options = {}) => { return; } + // A corrupt identity file must not silently re-enter setup mode (that + // would re-run destructive scaffold steps on an initialized project). + // Only an explicit --reinit may discard it. + let identity = null; + try { + identity = readIdentityFile(root); + } catch (err) { + if (!argv.includes('--reinit')) { + ui.error(err.message); + process.exitCode = 1; + return; + } + ui.warn( + '.wp-scaffold.json is corrupt; --reinit will overwrite it.' + ); + } + // Manage mode: already scaffolded (unless forced to re-scaffold with --reinit). - const identity = readIdentityFile(root); if (identity && !argv.includes('--reinit')) { await manageFlow(config, root, argv, identity, ui, () => setupFlow(config, root, { yes: false }) @@ -673,6 +916,12 @@ const run = async (config, options = {}) => { process.exitCode = 130; return; } + if (err instanceof IdentityFileError) { + // Mid-flow corruption (e.g. a manage re-read): report, don't crash. + ui.error(err.message); + process.exitCode = 1; + return; + } throw err; } }; diff --git a/node-packages/wp-tooling/src/init/manage.js b/node-packages/wp-tooling/src/init/manage.js index 99ac41f..ac616cb 100644 --- a/node-packages/wp-tooling/src/init/manage.js +++ b/node-packages/wp-tooling/src/init/manage.js @@ -42,14 +42,12 @@ const splitList = (value) => * @return {{flags: Object, unknown: string[]}} Parsed. */ const parseManageFlags = (argv) => { - const flags = { yes: false, list: false }; + const flags = { yes: false }; const unknown = []; argv.forEach((arg) => { if ('--yes' === arg || '-y' === arg) { flags.yes = true; - } else if ('--list' === arg) { - flags.list = true; } else if ('--manage' === arg) { // Explicit manage marker; already in manage mode, no-op. } else if (arg.startsWith('--features=')) { @@ -66,6 +64,19 @@ const parseManageFlags = (argv) => { return { flags, unknown }; }; +/** + * Human label for a feature's effective state (null = probe failed). + * + * @param {boolean|null} on - Detected state. + * @return {string} 'enabled' | 'disabled' | 'unknown'. + */ +const stateLabel = (on) => { + if (null === on) { + return 'unknown'; + } + return on ? 'enabled' : 'disabled'; +}; + /** * Render a read-only feature status table. * @@ -82,7 +93,7 @@ const showStatus = (rows, unknown, ui) => { ui.table( rows.map((r) => [ r.label, - `${r.on ? 'enabled' : 'disabled'}${r.drift ? ' (drift)' : ''}`, + `${stateLabel(r.on)}${r.drift ? ' (drift)' : ''}`, ]), { title: 'Feature status' } ); @@ -127,13 +138,6 @@ const manageFlow = async (config, root, argv, identity, ui, reinit) => { process.exitCode = 1; return; } - if (flags.list && (flags.features || flags.enable || flags.disable)) { - ui.error( - '--list cannot be combined with --features/--enable/--disable.' - ); - process.exitCode = 1; - return; - } const features = config.features || []; const api = makeFeatureApi(root, identity, ui); @@ -143,11 +147,6 @@ const manageFlow = async (config, root, argv, identity, ui, reinit) => { api ); - if (flags.list) { - showStatus(rows, retired, ui); - return; - } - const validKeys = new Set(features.map((f) => f.key)); const requested = [ ...(flags.features || []), diff --git a/node-packages/wp-tooling/src/init/persist.js b/node-packages/wp-tooling/src/init/persist.js index d746186..820582a 100644 --- a/node-packages/wp-tooling/src/init/persist.js +++ b/node-packages/wp-tooling/src/init/persist.js @@ -13,6 +13,21 @@ const path = require('path'); /** Name of the persisted identity file at the project root. */ const IDENTITY_FILE = '.wp-scaffold.json'; +/** + * Thrown when `.wp-scaffold.json` exists but cannot be parsed. A corrupt + * identity file must never be silently treated as absent: that would drop an + * initialized project back into setup mode (and re-run destructive scaffold + * steps). Callers branch on `code === 'EIDENTITYCORRUPT'`. + */ +class IdentityFileError extends Error { + constructor(message, details = {}) { + super(message); + this.name = 'IdentityFileError'; + this.code = 'EIDENTITYCORRUPT'; + Object.assign(this, details); + } +} + /** * Write the identity payload to `/.wp-scaffold.json` (tab-indented). * @@ -35,10 +50,13 @@ const writeIdentityFile = (root, payload, ui) => { }; /** - * Read the persisted identity, or null when absent / unparseable. + * Read the persisted identity. Absent file -> null; unparseable file -> throw. * * @param {string} root - Project root. - * @return {Object|null} The parsed identity, or null. + * @return {Object|null} The parsed identity, or null when the file is absent. + * @throws {IdentityFileError} EIDENTITYCORRUPT when the file exists but cannot + * be parsed (callers decide whether --reinit may + * discard it). */ const readIdentityFile = (root) => { const filePath = path.join(root, IDENTITY_FILE); @@ -47,8 +65,12 @@ const readIdentityFile = (root) => { } try { return JSON.parse(fs.readFileSync(filePath, 'utf8')); - } catch { - return null; + } catch (err) { + throw new IdentityFileError( + `${IDENTITY_FILE} exists but is not valid JSON (${err.message}). ` + + 'Fix or delete the file, or pass --reinit to discard it.', + { path: filePath } + ); } }; @@ -83,5 +105,6 @@ module.exports = { readIdentityFile, readFeatures, writeFeatures, + IdentityFileError, IDENTITY_FILE, }; diff --git a/node-packages/wp-tooling/src/scaffolds/add.js b/node-packages/wp-tooling/src/scaffolds/add.js index 91606e9..23d2307 100644 --- a/node-packages/wp-tooling/src/scaffolds/add.js +++ b/node-packages/wp-tooling/src/scaffolds/add.js @@ -18,6 +18,7 @@ 'use strict'; const { ScaffoldError } = require('./registry'); +const { formatErrorPayload } = require('./errors'); const { buildRegistry, fetchOptsFrom, @@ -105,42 +106,6 @@ function printHelp() { ); } -function formatErrorPayload(err) { - if (err instanceof ScaffoldError) { - const payload = { code: err.code, message: err.message }; - for (const k of [ - 'scaffold', - 'requested', - 'available', - 'missing', - 'missingDetails', - 'path', - 'errno', - 'placeholder', - 'template', - 'url', - 'statusCode', - 'rateLimited', - 'timeout', - 'cause', - 'file', - 'errors', - 'id', - 'source', - 'repository', - ]) { - if (err[k] !== undefined) { - payload[k] = err[k]; - } - } - return payload; - } - return { - code: 'EUNKNOWN', - message: err && err.message ? err.message : String(err), - }; -} - function printHumanReport(result) { const { scaffold, engine, developer, ai, warnings } = result; const lines = []; diff --git a/node-packages/wp-tooling/src/scaffolds/errors.js b/node-packages/wp-tooling/src/scaffolds/errors.js index 71b2662..95c6b84 100644 --- a/node-packages/wp-tooling/src/scaffolds/errors.js +++ b/node-packages/wp-tooling/src/scaffolds/errors.js @@ -20,4 +20,51 @@ class ScaffoldError extends Error { } } -module.exports = { ScaffoldError }; +/** + * Shape an error as the one-line JSON payload emitted on stderr in `--json` + * mode. Shared by `wp-tooling add` and the init engine's `--list` so every + * machine-facing command speaks the same error contract. + * + * Known errors keep their `code` and a whitelist of context fields; anything + * else degrades to `{ code: 'EUNKNOWN', message }`. + * + * @param {Error} err - The error to shape. + * @return {Object} `{ code, message, ...context }`. + */ +function formatErrorPayload(err) { + if (err && typeof err.code === 'string' && err.code) { + const payload = { code: err.code, message: err.message }; + for (const k of [ + 'scaffold', + 'requested', + 'available', + 'missing', + 'missingDetails', + 'path', + 'errno', + 'placeholder', + 'template', + 'url', + 'statusCode', + 'rateLimited', + 'timeout', + 'cause', + 'file', + 'errors', + 'id', + 'source', + 'repository', + ]) { + if (err[k] !== undefined) { + payload[k] = err[k]; + } + } + return payload; + } + return { + code: 'EUNKNOWN', + message: err && err.message ? err.message : String(err), + }; +} + +module.exports = { ScaffoldError, formatErrorPayload }; diff --git a/node-packages/wp-tooling/tests/init/_helpers.js b/node-packages/wp-tooling/tests/init/_helpers.js new file mode 100644 index 0000000..f9a048c --- /dev/null +++ b/node-packages/wp-tooling/tests/init/_helpers.js @@ -0,0 +1,62 @@ +/** + * Shared helpers for the init test suites (mirrors tests/release/_helpers.js). + */ + +'use strict'; + +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +/** + * Create a throwaway project root under the OS temp dir. + * + * @param {string} [prefix] - mkdtemp prefix. + * @return {string} Absolute root path. + */ +const makeRoot = (prefix = 'init-test-') => + fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + +/** + * Create an empty (or content-filled) file, making parent dirs. + * + * @param {string} root - Project root. + * @param {string} rel - Relative path to create. + * @param {string} [content] - File body. + * @return {void} + */ +const touch = (root, rel, content = '') => { + const abs = path.join(root, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); +}; + +/** + * Run `fn` with process.stdout/stderr captured; always restores the streams. + * + * @param {Function} fn - Sync or async workload. + * @return {Promise<{stdout: string, stderr: string}>} Captured output. + */ +const capture = async (fn) => { + const outW = process.stdout.write.bind(process.stdout); + const errW = process.stderr.write.bind(process.stderr); + let stdout = ''; + let stderr = ''; + process.stdout.write = (chunk) => { + stdout += chunk; + return true; + }; + process.stderr.write = (chunk) => { + stderr += chunk; + return true; + }; + try { + await fn(); + } finally { + process.stdout.write = outW; + process.stderr.write = errW; + } + return { stdout, stderr }; +}; + +module.exports = { makeRoot, touch, capture }; diff --git a/node-packages/wp-tooling/tests/init/capabilities.test.js b/node-packages/wp-tooling/tests/init/capabilities.test.js new file mode 100644 index 0000000..b0e768e --- /dev/null +++ b/node-packages/wp-tooling/tests/init/capabilities.test.js @@ -0,0 +1,555 @@ +/** + * Tests for `--list`: the capability reconciler (src/init/capabilities.js) and + * the run() list flow (setup / manage, the JSON contract, guarded detection, + * corrupt-identity handling, and usage errors). + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const { + listCapabilities, + showCapabilities, +} = require('../../src/init/capabilities'); +const { run } = require('../../src/init/index'); +const { IDENTITY_FILE } = require('../../src/init/persist'); +const { makeRoot, touch, capture } = require('./_helpers'); + +const CONFIG = { + kind: 'plugin', + features: [ + { + key: 'hmr', + label: 'HMR', + category: 'Dev', + defaultOn: true, + detect: () => true, + }, + { + key: 'tailwind', + label: 'Tailwind', + category: 'Editor', + detect: () => false, + }, + { + key: 'detected-only', + label: 'Detected Only', + category: 'Dev', + detect: (api) => api.exists('detected-only.flag'), + }, + ], + examples: { + groups: [ + { + key: 'post-types', + label: 'Post Types', + category: 'Content', + marker: 'wp:example:post-types', + strip: ['inc/Main.php'], + remove: [ + 'inc/Modules/PostTypes.php', + 'inc/Modules/PostTypes', + 'tests/php/PostTypesTest.php', + ], + }, + { + key: 'cron', + label: 'Cron', + category: 'APIs', + marker: 'wp:example:cron', + strip: ['inc/Main.php', 'inc/Core/PluginSetup.php'], + remove: ['inc/Modules/Cron.php', 'inc/Modules/Cron'], + }, + { + key: 'ci-lint-php', + label: 'CI: PHPCS', + category: 'Developer Tooling', + marker: 'wp:ci:ci-lint-php', + module: null, + strip: [], + remove: ['.github/workflows/ci-lint-php.yml'], + }, + { + key: 'declared', + label: 'Declared Module', + category: 'Other', + marker: 'wp:example:declared', + module: 'Custom', + strip: [], + remove: ['lib/whatever.php'], + }, + { + key: 'dashed', + label: 'Dashed Module', + category: 'Other', + marker: 'wp:example:dashed', + strip: [], + remove: ['inc/Modules/my-mod.php'], + }, + { + key: 'globonly', + label: 'Glob Only', + category: 'Other', + marker: 'wp:example:globonly', + strip: [], + remove: ['src/blocks/example-*'], + }, + ], + }, +}; + +const byKey = (rows) => Object.fromEntries(rows.map((r) => [r.key, r])); + +const writeIdentity = (root, payload) => + fs.writeFileSync(path.join(root, IDENTITY_FILE), JSON.stringify(payload)); + +let root; + +beforeEach(() => { + root = makeRoot('init-list-'); + // Present artifacts; Cron is intentionally left absent (i.e. removed). + touch(root, 'inc/Modules/PostTypes.php'); + touch(root, '.github/workflows/ci-lint-php.yml'); + touch(root, 'lib/whatever.php'); + touch(root, 'inc/Modules/my-mod.php'); +}); + +afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); + // run() sets process.exitCode on usage errors; clear it so a failing-flag + // test does not leak a non-zero exit code into the Jest process. + process.exitCode = 0; +}); + +describe('listCapabilities (no identity: detection only)', () => { + it('detects present/removed from the first concrete remove path', () => { + const caps = byKey(listCapabilities(CONFIG, root)); + expect(caps['post-types']).toMatchObject({ + present: true, + detected: true, + intent: null, + drift: false, + }); + expect(caps.cron).toMatchObject({ present: false, detected: false }); + }); + + it('reports unknown detection (glob-only group) as present', () => { + const caps = byKey(listCapabilities(CONFIG, root)); + expect(caps.globonly).toMatchObject({ + present: true, + detected: null, + intent: null, + drift: false, + }); + }); + + it('prefers a config-declared module and falls back to the artifact', () => { + const caps = byKey(listCapabilities(CONFIG, root)); + expect(caps.declared.module).toBe('Custom'); + expect(caps['ci-lint-php'].module).toBeNull(); // declared null + expect(caps['post-types'].module).toBe('PostTypes'); // fallback + expect(caps.dashed.module).toBe('my-mod'); // dashed fallback + }); + + it('returns [] when no examples are declared', () => { + expect(listCapabilities({ kind: 'plugin' }, root)).toEqual([]); + }); +}); + +describe('listCapabilities (identity record: reconcile)', () => { + const identity = (removed) => ({ name: 'X', examples: { removed } }); + + it('agreement: recorded removal + missing artifact -> removed, no drift', () => { + const caps = byKey(listCapabilities(CONFIG, root, identity(['cron']))); + expect(caps.cron).toMatchObject({ + present: false, + detected: false, + intent: false, + drift: false, + }); + expect(caps['post-types']).toMatchObject({ + present: true, + intent: true, + drift: false, + }); + }); + + it('manual removal after setup -> present false + drift', () => { + fs.rmSync(path.join(root, 'inc/Modules/PostTypes.php')); + const caps = byKey(listCapabilities(CONFIG, root, identity([]))); + expect(caps['post-types']).toMatchObject({ + present: false, + detected: false, + intent: true, + drift: true, + }); + }); + + it('glob-only group follows recorded intent (no detection to lie)', () => { + const caps = byKey( + listCapabilities(CONFIG, root, identity(['globonly'])) + ); + expect(caps.globonly).toMatchObject({ + present: false, + detected: null, + intent: false, + drift: false, + }); + }); + + it('legacy identity (no examples key) -> intent null, detection only', () => { + const caps = byKey(listCapabilities(CONFIG, root, { name: 'X' })); + expect(caps['post-types']).toMatchObject({ + present: true, + intent: null, + drift: false, + }); + }); +}); + +describe('showCapabilities', () => { + const fakeUi = () => { + const calls = { table: [], info: [] }; + return { + ui: { + table: (rows, opts) => calls.table.push({ rows, opts }), + info: (msg) => calls.info.push(msg), + }, + calls, + }; + }; + + it('renders a Capabilities table', () => { + const { ui, calls } = fakeUi(); + showCapabilities(listCapabilities(CONFIG, root), ui, { mode: 'setup' }); + expect(calls.table).toHaveLength(1); + expect(calls.table[0].opts).toEqual({ title: 'Capabilities' }); + }); + + it('marks drifted rows in the table', () => { + fs.rmSync(path.join(root, 'inc/Modules/PostTypes.php')); + const { ui, calls } = fakeUi(); + showCapabilities( + listCapabilities(CONFIG, root, { examples: { removed: [] } }), + ui, + { mode: 'manage' } + ); + const row = calls.table[0].rows.find( + ([label]) => 'Post Types' === label + ); + expect(row[1]).toMatch(/removed\s+\(drift\)/); + }); + + it('shows the empty message when there are no capabilities', () => { + const { ui, calls } = fakeUi(); + showCapabilities([], ui, {}); + expect(calls.table).toHaveLength(0); + expect(calls.info).toHaveLength(1); + }); + + it('adds an npx re-add hint in manage mode when a capability is removed', () => { + const { ui, calls } = fakeUi(); + showCapabilities(listCapabilities(CONFIG, root), ui, { + mode: 'manage', + }); + expect(calls.info.some((m) => /npx wp-tooling add/.test(m))).toBe(true); + }); +}); + +describe('run --list (JSON contract)', () => { + it('setup mode: one stdout line, empty stderr, exit 0', async () => { + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + expect(process.exitCode || 0).toBe(0); + expect(stderr).toBe(''); + const lines = stdout.trim().split('\n'); + expect(lines).toHaveLength(1); + const payload = JSON.parse(lines[0]); + expect(payload.mode).toBe('setup'); + expect(payload.warnings).toEqual([]); + }); + + it('setup mode: capabilities carry only the documented keys', async () => { + const { stdout } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + const payload = JSON.parse(stdout.trim()); + for (const cap of payload.capabilities) { + expect(Object.keys(cap).sort()).toEqual([ + 'category', + 'key', + 'label', + 'module', + 'present', + ]); + } + }); + + it('setup mode: `on` is the non-interactive default (defaultOn || detected)', async () => { + touch(root, 'detected-only.flag'); + const { stdout } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + const features = byKey(JSON.parse(stdout.trim()).features); + expect(features.hmr.on).toBe(true); // defaultOn + expect(features.tailwind.on).toBe(false); // detected false + expect(features['detected-only'].on).toBe(true); // detected true + expect(features.hmr.intent).toBeUndefined(); + expect(features.hmr.drift).toBeUndefined(); + }); + + it('manage mode: reconciled capabilities + feature intent/drift', async () => { + writeIdentity(root, { + name: 'X', + examples: { removed: ['cron', 'globonly'] }, + features: { hmr: true, tailwind: true, retired: true }, + }); + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + expect(stderr).toBe(''); + const payload = JSON.parse(stdout.trim()); + expect(payload.mode).toBe('manage'); + + const caps = byKey(payload.capabilities); + expect(caps.cron).toMatchObject({ + present: false, + intent: false, + drift: false, + }); + expect(caps.globonly).toMatchObject({ present: false, intent: false }); + expect(caps['post-types']).toMatchObject({ + present: true, + intent: true, + }); + + const features = byKey(payload.features); + expect(features.hmr).toMatchObject({ + on: true, + intent: true, + drift: false, + }); + // Persisted true but detect() says false -> drift. + expect(features.tailwind).toMatchObject({ + on: false, + intent: true, + drift: true, + }); + expect(payload.warnings.some((w) => /retired/.test(w))).toBe(true); + }); + + it('manage mode: legacy identity gets an unrecorded-selection warning', async () => { + writeIdentity(root, { name: 'X', features: {} }); + const { stdout } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + const payload = JSON.parse(stdout.trim()); + expect(payload.warnings.some((w) => /was not recorded/.test(w))).toBe( + true + ); + expect(byKey(payload.capabilities)['post-types'].intent).toBeNull(); + }); + + it('a throwing detect probe degrades to on:null + warning, exit 0', async () => { + const throwConfig = { + ...CONFIG, + features: [ + ...CONFIG.features, + { + key: 'exploding', + label: 'Exploding', + detect: () => { + throw new Error('boom'); + }, + }, + ], + }; + const { stdout, stderr } = await capture(() => + run(throwConfig, { root, argv: ['--list', '--json'] }) + ); + expect(process.exitCode || 0).toBe(0); + expect(stderr).toBe(''); + const payload = JSON.parse(stdout.trim()); + expect(byKey(payload.features).exploding.on).toBeNull(); + expect( + payload.warnings.some((w) => + /exploding: feature detect failed: boom/.test(w) + ) + ).toBe(true); + }); +}); + +describe('run --list (usage + failure contract)', () => { + it('--json without --list is a usage error on stderr, exit 1', async () => { + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--json'] }) + ); + expect(stdout).toBe(''); + expect(JSON.parse(stderr.trim()).code).toBe('EUSAGE'); + expect(process.exitCode).toBe(1); + }); + + it('rejects --list combined with a mutating flag, exit 1', async () => { + const { stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json', '--features=x'] }) + ); + expect(JSON.parse(stderr.trim()).message).toMatch(/--features=x/); + expect(process.exitCode).toBe(1); + }); + + it('rejects --list --manage rather than silently choosing setup', async () => { + const { stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--manage', '--json'] }) + ); + expect(JSON.parse(stderr.trim()).message).toMatch(/--manage/); + expect(process.exitCode).toBe(1); + }); + + it('rejects --list --clean', async () => { + const { stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json', '--clean'] }) + ); + expect(JSON.parse(stderr.trim()).code).toBe('EUSAGE'); + expect(process.exitCode).toBe(1); + }); + + it('accepts (and ignores) --yes alongside --list', async () => { + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json', '--yes'] }) + ); + expect(stderr).toBe(''); + expect(JSON.parse(stdout.trim()).mode).toBe('setup'); + expect(process.exitCode || 0).toBe(0); + }); + + it('--list --reinit on an initialized project reports setup mode', async () => { + writeIdentity(root, { name: 'X', features: {} }); + const { stdout } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json', '--reinit'] }) + ); + expect(JSON.parse(stdout.trim()).mode).toBe('setup'); + }); + + it('a broken feature manifest answers ECONFIG in JSON', async () => { + const broken = { + ...CONFIG, + features: [ + { key: 'dup', label: 'A', detect: () => true }, + { key: 'dup', label: 'B', detect: () => true }, + ], + }; + const { stdout, stderr } = await capture(() => + run(broken, { root, argv: ['--list', '--json'] }) + ); + expect(stdout).toBe(''); + const err = JSON.parse(stderr.trim()); + expect(err.code).toBe('ECONFIG'); + expect(err.message).toMatch(/Duplicate feature key/); + expect(process.exitCode).toBe(1); + }); + + it('corrupt identity answers EIDENTITYCORRUPT in JSON', async () => { + fs.writeFileSync(path.join(root, IDENTITY_FILE), '{broken'); + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json'] }) + ); + expect(stdout).toBe(''); + const err = JSON.parse(stderr.trim()); + expect(err.code).toBe('EIDENTITYCORRUPT'); + expect(err.path).toBe(path.join(root, IDENTITY_FILE)); + expect(process.exitCode).toBe(1); + }); + + it('corrupt identity + human --list errors without JSON noise', async () => { + fs.writeFileSync(path.join(root, IDENTITY_FILE), '{broken'); + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list'] }) + ); + expect(stderr).toBe(''); + expect(stdout).toMatch(/not valid JSON/); + expect(process.exitCode).toBe(1); + }); + + it('corrupt identity + --reinit lists setup mode with a warning', async () => { + fs.writeFileSync(path.join(root, IDENTITY_FILE), '{broken'); + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list', '--json', '--reinit'] }) + ); + expect(stderr).toBe(''); + const payload = JSON.parse(stdout.trim()); + expect(payload.mode).toBe('setup'); + expect( + payload.warnings.some((w) => /--reinit will overwrite/.test(w)) + ).toBe(true); + expect(process.exitCode || 0).toBe(0); + }); +}); + +describe('run --list (human output)', () => { + it('renders heading + both tables on stdout, stderr stays empty', async () => { + const { stdout, stderr } = await capture(() => + run(CONFIG, { root, argv: ['--list'] }) + ); + expect(stderr).toBe(''); + expect(stdout).toMatch(/available capabilities/); + expect(stdout).toMatch(/Capabilities/); + expect(stdout).toMatch(/Feature status/); + expect(stdout).toMatch(/Post Types/); + expect(process.exitCode || 0).toBe(0); + }); +}); + +describe('run (non-list) corrupt identity', () => { + it('refuses to fall through to setup mode', async () => { + fs.writeFileSync(path.join(root, IDENTITY_FILE), '{broken'); + const { stdout } = await capture(() => run(CONFIG, { root, argv: [] })); + expect(stdout).toMatch(/not valid JSON/); + expect(process.exitCode).toBe(1); + // Nothing rewrote the file: setup never ran. + expect(fs.readFileSync(path.join(root, IDENTITY_FILE), 'utf8')).toBe( + '{broken' + ); + }); +}); + +describe('setup records the capability selection', () => { + const FULL_CONFIG = { + ...CONFIG, + source: { name: 'Project Name' }, + }; + + it('persists examples.removed, which --list then reconciles', async () => { + await capture(() => + run(FULL_CONFIG, { + root, + argv: [ + '--yes', + '--name=Acme Blog', + '--remove-examples=cron,globonly', + ], + }) + ); + expect(process.exitCode || 0).toBe(0); + + const identity = JSON.parse( + fs.readFileSync(path.join(root, IDENTITY_FILE), 'utf8') + ); + expect(identity.examples).toEqual({ removed: ['cron', 'globonly'] }); + + const { stdout } = await capture(() => + run(FULL_CONFIG, { root, argv: ['--list', '--json'] }) + ); + const payload = JSON.parse(stdout.trim()); + expect(payload.mode).toBe('manage'); + const caps = byKey(payload.capabilities); + expect(caps.globonly).toMatchObject({ present: false, intent: false }); + expect(caps['post-types']).toMatchObject({ + present: true, + intent: true, + drift: false, + }); + }); +}); diff --git a/node-packages/wp-tooling/tests/init/persist.test.js b/node-packages/wp-tooling/tests/init/persist.test.js new file mode 100644 index 0000000..45265de --- /dev/null +++ b/node-packages/wp-tooling/tests/init/persist.test.js @@ -0,0 +1,72 @@ +/** + * Tests for src/init/persist.js -- the .wp-scaffold.json read/write layer, + * including the corrupt-vs-missing distinction the --list contract relies on. + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const { + readIdentityFile, + writeIdentityFile, + writeFeatures, + IdentityFileError, + IDENTITY_FILE, +} = require('../../src/init/persist'); +const { makeRoot } = require('./_helpers'); + +let root; + +beforeEach(() => { + root = makeRoot('persist-'); +}); + +afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('readIdentityFile', () => { + it('returns null when the file is absent', () => { + expect(readIdentityFile(root)).toBeNull(); + }); + + it('returns the parsed object for a valid file', () => { + writeIdentityFile(root, { name: 'X', features: { hmr: true } }); + expect(readIdentityFile(root)).toEqual({ + name: 'X', + features: { hmr: true }, + }); + }); + + it('throws EIDENTITYCORRUPT (not null) for an unparseable file', () => { + fs.writeFileSync(path.join(root, IDENTITY_FILE), '{broken'); + let caught; + try { + readIdentityFile(root); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(IdentityFileError); + expect(caught.code).toBe('EIDENTITYCORRUPT'); + expect(caught.path).toBe(path.join(root, IDENTITY_FILE)); + expect(caught.message).toMatch(/--reinit/); + }); +}); + +describe('writeFeatures', () => { + it('updates features while preserving the examples record', () => { + writeIdentityFile(root, { + name: 'X', + examples: { removed: ['cron'] }, + features: { hmr: false }, + }); + writeFeatures(root, { hmr: true }); + expect(readIdentityFile(root)).toEqual({ + name: 'X', + examples: { removed: ['cron'] }, + features: { hmr: true }, + }); + }); +}); diff --git a/node-packages/wp-tooling/tests/ui/selects.test.js b/node-packages/wp-tooling/tests/ui/selects.test.js index 54edfb5..7078216 100644 --- a/node-packages/wp-tooling/tests/ui/selects.test.js +++ b/node-packages/wp-tooling/tests/ui/selects.test.js @@ -205,7 +205,10 @@ describe('checkboxTree (non-TTY)', () => { { label: 'Taxonomies', checked: true }, ], }, - { label: 'Dev', items: [{ label: 'Tailwind', checked: false }] }, + { + label: 'Dev', + items: [{ label: 'Tailwind', checked: false }], + }, ], }); @@ -227,7 +230,10 @@ describe('checkboxTree (non-TTY)', () => { { label: 'Taxonomies', checked: true }, ], }, - { label: 'Dev', items: [{ label: 'Tailwind', checked: false }] }, + { + label: 'Dev', + items: [{ label: 'Tailwind', checked: false }], + }, ], }); From 131cbae79f59fe6014c6737907ed736bf92f6260 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 17 Jul 2026 02:02:33 +0530 Subject: [PATCH 2/3] feat(scaffolds): add ci/test-measure consolidated CI workflow One caller workflow delegating lint/test/build/a11y to the wp-ci.yml orchestrator in rtCamp/wp-shared-workflows, replacing per-check callers. - Supports a PHP x WP version matrix and private-repo tokens, which the per-check callers could not express. - actionlint validates the rendered template. --- .../scaffolds/ci/test-measure/scaffold.json | 71 +++++++++++++++++++ .../templates/test-measure.yml.mustache | 39 ++++++++++ 2 files changed, 110 insertions(+) create mode 100644 node-packages/wp-tooling/scaffolds/ci/test-measure/scaffold.json create mode 100644 node-packages/wp-tooling/scaffolds/ci/test-measure/templates/test-measure.yml.mustache diff --git a/node-packages/wp-tooling/scaffolds/ci/test-measure/scaffold.json b/node-packages/wp-tooling/scaffolds/ci/test-measure/scaffold.json new file mode 100644 index 0000000..b946eac --- /dev/null +++ b/node-packages/wp-tooling/scaffolds/ci/test-measure/scaffold.json @@ -0,0 +1,71 @@ +{ + "slug": "test-measure", + "category": "ci", + "name": "CI: Test and Measure (consolidated)", + "description": "One caller workflow delegating lint (CSS/JS/PHP), JS tests, a PHP test matrix, build, the build-artifact gate, and optional a11y to the wp-ci.yml orchestrator in rtCamp/wp-shared-workflows. Jobs are gated on detected changes. Use this instead of the per-check ci/* callers; unlike them it supports a PHP x WP matrix and forwards private-repo tokens.", + "source": "template", + "inputs": [ + { + "key": "wsw_ref", + "description": "Git ref of rtCamp/wp-shared-workflows to pin (tag, branch, or SHA).", + "default": "v1" + }, + { + "key": "project_type", + "description": "Project shape the orchestrator presets on: plugin | theme | package.", + "default": "plugin" + }, + { + "key": "default_branch", + "description": "Branch the push trigger watches (pull requests always trigger).", + "default": "main" + }, + { + "key": "php_versions", + "description": "JSON array of PHP versions for the test-php matrix, crossed with wp_versions.", + "default": "[\"8.2\", \"8.3\", \"8.4\"]" + }, + { + "key": "wp_versions", + "description": "JSON array of WordPress core versions for the test-php matrix, crossed with php_versions.", + "default": "[\"6.7\", \"6.8\", \"6.9\", \"7.0\"]" + }, + { + "key": "test_php_exclude", + "description": "JSON array of {php, wp} cells to drop from the matrix, e.g. [{\"php\": \"8.4\", \"wp\": \"6.5\"}].", + "default": "[]" + }, + { + "key": "run_a11y", + "description": "Set to true to run the (slow) pa11y accessibility job on every run.", + "default": "false" + } + ], + "files": [ + { + "src": "templates/test-measure.yml.mustache", + "dest": ".github/workflows/test-measure.yml" + } + ], + "secrets": [ + { + "key": "WP_TOOLING_TOKEN", + "scope": "github-actions", + "description": "Token with read access to the private rtCamp/wp-tooling repo (change detection installs the interim CLI). Needed only while that repo is private; one token may serve both secrets.", + "required": false + }, + { + "key": "PACKAGES_TOKEN", + "scope": "github-actions", + "description": "Token with read access to private Composer source repos (e.g. rtCamp/wp-framework) for `composer install` in lint-php/test-php. Omit for projects with only public deps.", + "required": false + } + ], + "tests": [ + { + "src": "templates/test-measure.yml.mustache", + "dest": ".github/workflows/test-measure.yml", + "framework": "actionlint" + } + ] +} diff --git a/node-packages/wp-tooling/scaffolds/ci/test-measure/templates/test-measure.yml.mustache b/node-packages/wp-tooling/scaffolds/ci/test-measure/templates/test-measure.yml.mustache new file mode 100644 index 0000000..7278a37 --- /dev/null +++ b/node-packages/wp-tooling/scaffolds/ci/test-measure/templates/test-measure.yml.mustache @@ -0,0 +1,39 @@ +name: Test and Measure + +# Scaffolded by @rtcamp/wp-tooling. +# Single caller that delegates lint, JS/PHP tests (PHP x WP matrix), build, +# the build-artifact gate, and optional a11y to the wp-ci.yml orchestrator; +# every job there is gated on detected changes. +# Source: https://github.com/rtCamp/wp-shared-workflows/blob/{{wsw_ref}}/.github/workflows/wp-ci.yml + +on: + pull_request: + types: + - opened + - synchronize + - reopened + - ready_for_review + push: + branches: + - {{default_branch}} + +permissions: + contents: read + +# Cancel previous in-flight runs of this workflow for the same ref. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.ref }} + cancel-in-progress: true + +jobs: + ci: + uses: rtCamp/wp-shared-workflows/.github/workflows/wp-ci.yml@{{wsw_ref}} + with: + project-type: {{project_type}} + php-versions: '{{php_versions}}' + wp-versions: '{{wp_versions}}' + test-php-exclude: '{{test_php_exclude}}'{{#run_a11y}} + run-a11y: true{{/run_a11y}} + secrets: + wp-tooling-token: ${{ secrets.WP_TOOLING_TOKEN }} + packages-token: ${{ secrets.PACKAGES_TOKEN }} From fcea40d3fe0faab71366c66a6d8fdf422ec49339 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Tue, 21 Jul 2026 13:05:47 +0530 Subject: [PATCH 3/3] feat(debug): add opt-in diagnostic logger; harden init/scaffold AI contracts --- .../templates/admin-page.php.mustache | 19 +- .../templates/block-class.php.mustache | 8 +- .../scaffolds/wp/rest/scaffold.json | 2 +- .../wp/rest/templates/controller.php.mustache | 22 +- node-packages/wp-tooling/src/debug.js | 475 ++++++++++++++++++ node-packages/wp-tooling/src/init/index.js | 237 ++++----- node-packages/wp-tooling/src/init/manage.js | 15 + .../wp-tooling/src/init/transform.js | 9 +- node-packages/wp-tooling/src/scaffolds/add.js | 33 +- .../wp-tooling/src/scaffolds/list.js | 16 + .../wp-tooling/src/scaffolds/registry.js | 15 +- .../wp-tooling/src/ui/wizard/index.js | 4 +- 12 files changed, 716 insertions(+), 139 deletions(-) create mode 100644 node-packages/wp-tooling/src/debug.js diff --git a/node-packages/wp-tooling/scaffolds/wp/admin-page/templates/admin-page.php.mustache b/node-packages/wp-tooling/scaffolds/wp/admin-page/templates/admin-page.php.mustache index 6311cf8..952231a 100644 --- a/node-packages/wp-tooling/scaffolds/wp/admin-page/templates/admin-page.php.mustache +++ b/node-packages/wp-tooling/scaffolds/wp/admin-page/templates/admin-page.php.mustache @@ -46,17 +46,22 @@ class {{class}} extends AbstractAdminPage { public function get_menu_title(): string { return __( '{{menu_title}}', '{{text_domain}}' ); } - +{{#parent_slug}} /** - * Parent menu slug. Return null for a top-level menu. + * Parent menu slug (this page is a submenu under an existing menu). + * + * Emitted only when a parent slug is set. For a top-level menu the method is + * omitted so the framework's nullable default (get_parent_slug(): ?string + * returning null) applies: returning a concrete value from a ?string + * override would make the return type too wide for PHPStan level 5's + * checkTooWideReturnTypesInProtectedAndPublicMethods rule. * - * @return string|null + * @return string */ - public function get_parent_slug(): ?string { - $parent = '{{parent_slug}}'; - return '' === $parent ? null : $parent; + public function get_parent_slug(): string { + return '{{parent_slug}}'; } - +{{/parent_slug}} /** * Capability required to view the page. * diff --git a/node-packages/wp-tooling/scaffolds/wp/block-dynamic/templates/block-class.php.mustache b/node-packages/wp-tooling/scaffolds/wp/block-dynamic/templates/block-class.php.mustache index a4b9c0c..468edb5 100644 --- a/node-packages/wp-tooling/scaffolds/wp/block-dynamic/templates/block-class.php.mustache +++ b/node-packages/wp-tooling/scaffolds/wp/block-dynamic/templates/block-class.php.mustache @@ -36,9 +36,13 @@ class {{class}} extends AbstractBlock { * so the editor receives the bundled assets and not raw ES modules. * Run `npm run build` to populate this directory. * - * @return string|null + * Narrowed to a non-null `string` (the parent declares `?string`): this + * override always returns a concrete path, and an always-non-null `?string` + * trips PHPStan level 5's checkTooWideReturnTypesInProtectedAndPublicMethods. + * + * @return string */ - public function get_block_dir(): ?string { + public function get_block_dir(): string { return dirname( __DIR__, 2 ) . '/{{build_dir}}/{{slug}}'; } diff --git a/node-packages/wp-tooling/scaffolds/wp/rest/scaffold.json b/node-packages/wp-tooling/scaffolds/wp/rest/scaffold.json index 8d99c18..1caf392 100644 --- a/node-packages/wp-tooling/scaffolds/wp/rest/scaffold.json +++ b/node-packages/wp-tooling/scaffolds/wp/rest/scaffold.json @@ -41,7 +41,7 @@ }, { "key": "rest_namespace", - "description": "REST namespace prefix used in the URL path, e.g. 'myplugin'. Final route becomes /wp-json/{rest_namespace}/v{version}/{name}.", + "description": "REST namespace prefix used in the URL path, base only and WITHOUT the version segment, e.g. 'myplugin' (NOT 'myplugin/v1'). The version is added separately from rest_version, so the final route is /wp-json/{rest_namespace}/v{rest_version}/{name}. Passing a version here produces a doubled '/v1/v1'.", "required": true }, { diff --git a/node-packages/wp-tooling/scaffolds/wp/rest/templates/controller.php.mustache b/node-packages/wp-tooling/scaffolds/wp/rest/templates/controller.php.mustache index 66a3e65..f6198a6 100644 --- a/node-packages/wp-tooling/scaffolds/wp/rest/templates/controller.php.mustache +++ b/node-packages/wp-tooling/scaffolds/wp/rest/templates/controller.php.mustache @@ -14,8 +14,6 @@ declare(strict_types=1); namespace {{namespace}}; use rtCamp\WPFramework\Contracts\Abstracts\AbstractRESTController; -use WP_REST_Request; -use WP_REST_Response; /** * Registers REST routes for the {{name}} resource. @@ -65,23 +63,31 @@ class {{class}}Controller extends AbstractRESTController { /** * Return the collection of items. * - * @param WP_REST_Request $request The REST request. + * Overrides WP_REST_Controller::get_items(), whose signature is untyped, so + * the parameter and return stay untyped here (a typed override is a fatal + * LSP violation that aborts wp-env provisioning). Types live in PHPDoc, and + * WP-global classes are fully qualified with no `use` (so `composer format` + * does not strip a docblock-only import and break PHPStan). * - * @return WP_REST_Response + * @param \WP_REST_Request $request The REST request. + * + * @return \WP_REST_Response */ - public function get_items( WP_REST_Request $request ): WP_REST_Response { + public function get_items( $request ) { // Replace with your collection response. - return new WP_REST_Response( [], 200 ); + return new \WP_REST_Response( [], 200 ); } /** * Permission check for the collection endpoint. * - * @param WP_REST_Request $request The REST request. + * Untyped for the same override-compatibility reason as get_items(). + * + * @param \WP_REST_Request $request The REST request. * * @return bool */ - public function get_items_permissions_check( WP_REST_Request $request ): bool { + public function get_items_permissions_check( $request ) { return current_user_can( 'read' ); } } diff --git a/node-packages/wp-tooling/src/debug.js b/node-packages/wp-tooling/src/debug.js new file mode 100644 index 0000000..992d4db --- /dev/null +++ b/node-packages/wp-tooling/src/debug.js @@ -0,0 +1,475 @@ +/* eslint no-console: 0 */ + +/** + * Opt-in diagnostic logger for the init and scaffold flows. + * + * INERT unless the environment variable `WP_TOOLING_DEBUG` is truthy (anything + * other than unset / "0" / "false" / "off"). When it is off every function here + * is a cheap no-op, so there is zero runtime cost and zero extra output in a + * normal run. + * + * When on, it records, per phase: + * - wall time (a bottleneck signal), + * - the volume of stdout / stderr the phase produced (a direct proxy for the + * tokens an AI agent ingests when it reads command output), and + * - any errors (the reliability signal; errors are what send an agent digging + * through node_modules). + * + * It then APPENDS one self-contained, self-describing block per run to a log + * file (default `/.wp-tooling-debug.log`, override with + * `WP_TOOLING_DEBUG_LOG`). Nothing is ever written to stdout, so the log never + * pollutes the very output it measures. + * + * The report is written FOR an AI reader: alongside a machine-parseable phase + * table it emits an "observations" section that pre-flags the slowest phase, + * the noisiest phase, and every error, so the reader gets ready-made leads for + * cutting time, tokens, and failures without recomputing them. + * + * Usage (all no-ops when disabled): + * const debug = require('../debug'); + * debug.start('npm run init -- ...', { kind: 'plugin', mode: 'scaffold', cwd: root }); + * await debug.phase('setupFlow', async () => { ... }); + * debug.event('capabilities', { selected: 4 }); + * debug.finish({ result: 'ok' }); + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +/** + * Whether a value read from the environment should be treated as "on". + * + * @param {string|undefined} value - Raw env value. + * @return {boolean} True when enabled. + */ +const truthy = (value) => + !!value && '0' !== value && 'false' !== value && 'off' !== value; + +const ENABLED = truthy(process.env.WP_TOOLING_DEBUG); + +// All mutable run state. Untouched while disabled. +const state = { + active: false, + logPath: null, + startNs: 0n, + command: '', + context: {}, + phaseStack: [], // Names of currently-open phases (innermost last). + phases: new Map(), // name -> { calls, ms, stdoutBytes, stdoutLines, stderrBytes } + events: [], // { atMs, phase, label, detail } + errors: [], // { phase, message } + totals: { stdoutBytes: 0, stdoutLines: 0, stderrBytes: 0 }, + orig: { stdout: null, stderr: null }, + exitHooked: false, +}; + +/** + * Whether diagnostics are enabled for this process. + * + * @return {boolean} True when WP_TOOLING_DEBUG is truthy. + */ +const enabled = () => ENABLED; + +/** + * Current high-resolution time in nanoseconds. + * + * @return {bigint} Nanoseconds. + */ +const nowNs = () => process.hrtime.bigint(); + +/** + * Milliseconds elapsed since the run started, rounded. + * + * @return {number} Elapsed ms. + */ +const sinceStartMs = () => Math.round(Number(nowNs() - state.startNs) / 1e6); + +/** + * The phase currently receiving output (innermost open phase), or "(root)". + * + * @return {string} Phase name. + */ +const currentPhase = () => + state.phaseStack.length + ? state.phaseStack[state.phaseStack.length - 1] + : '(root)'; + +/** + * Lazily create the per-phase accumulator. + * + * @param {string} name - Phase name. + * @return {Object} The accumulator. + */ +const phaseRow = (name) => { + let row = state.phases.get(name); + if (!row) { + row = { + calls: 0, + ms: 0, + stdoutBytes: 0, + stdoutLines: 0, + stderrBytes: 0, + }; + state.phases.set(name, row); + } + return row; +}; + +/** + * Count bytes + newlines of a stream chunk and attribute them to the current + * phase and the run totals. Never throws (real output must never break). + * + * @param {'stdout'|'stderr'} stream - Which stream. + * @param {*} chunk - The chunk passed to write(). + * @param {*} enc - The encoding argument (may be a callback). + * @return {void} + */ +const recordOutput = (stream, chunk, enc) => { + try { + let bytes; + let text; + if (Buffer.isBuffer(chunk)) { + bytes = chunk.length; + text = chunk.toString('utf8'); + } else { + text = String(chunk); + bytes = Buffer.byteLength( + text, + 'string' === typeof enc ? enc : 'utf8' + ); + } + const lines = text ? text.split('\n').length - 1 : 0; + if ('stdout' === stream) { + state.totals.stdoutBytes += bytes; + state.totals.stdoutLines += lines; + const row = phaseRow(currentPhase()); + row.stdoutBytes += bytes; + row.stdoutLines += lines; + } else { + state.totals.stderrBytes += bytes; + phaseRow(currentPhase()).stderrBytes += bytes; + } + } catch { + // Diagnostics must never interfere with real output. + } +}; + +/** + * Wrap process.stdout/stderr `write` so output volume is measured, then passed + * straight through unchanged. + * + * @return {void} + */ +const patchStreams = () => { + ['stdout', 'stderr'].forEach((name) => { + const stream = process[name]; + const orig = stream.write.bind(stream); + state.orig[name] = orig; + stream.write = (chunk, enc, cb) => { + recordOutput(name, chunk, enc); + return orig(chunk, enc, cb); + }; + }); +}; + +/** + * Restore the original stream writers. + * + * @return {void} + */ +const unpatchStreams = () => { + ['stdout', 'stderr'].forEach((name) => { + if (state.orig[name]) { + process[name].write = state.orig[name]; + state.orig[name] = null; + } + }); +}; + +/** + * Begin a diagnostic run. No-op when disabled. + * + * @param {string} command - The command being run (for the report header). + * @param {Object} [context] - Free-form context: { kind, mode, cwd, ... }. + * @return {void} + */ +const start = (command, context = {}) => { + if (!ENABLED || state.active) { + return; + } + state.active = true; + state.command = command || ''; + state.context = context || {}; + state.phaseStack = []; + state.phases = new Map(); + state.events = []; + state.errors = []; + state.totals = { stdoutBytes: 0, stdoutLines: 0, stderrBytes: 0 }; + const base = context.cwd || process.cwd(); + state.logPath = + process.env.WP_TOOLING_DEBUG_LOG || + path.join(base, '.wp-tooling-debug.log'); + state.startNs = nowNs(); + patchStreams(); + // Flush even if the process exits before finish() is called. + if (!state.exitHooked) { + state.exitHooked = true; + process.on('exit', () => { + if (state.active) { + finish({ result: 'incomplete (process exit)' }); + } + }); + } + event('run', { event: 'start', command: state.command }); +}; + +/** + * Record a timeline event. No-op when inactive. + * + * @param {string} label - Short label. + * @param {Object} [detail] - Optional structured detail. + * @return {void} + */ +const event = (label, detail) => { + if (!state.active) { + return; + } + state.events.push({ + atMs: sinceStartMs(), + phase: currentPhase(), + label, + detail: detail || null, + }); +}; + +/** + * Record an error against the current (or named) phase. No-op when inactive. + * + * @param {Error|string} err - The error. + * @param {string} [phase] - Optional phase override. + * @return {void} + */ +const recordError = (err, phase) => { + if (!state.active) { + return; + } + const message = err && err.message ? err.message : String(err); + state.errors.push({ phase: phase || currentPhase(), message }); + event('error', { phase: phase || currentPhase(), message }); +}; + +/** + * Time a phase. Runs `fn`, recording its wall time and output volume against + * `name`. When disabled it simply awaits `fn`. Errors are recorded then + * rethrown so control flow is unchanged. + * + * @param {string} name - Phase name. + * @param {Function} fn - Async (or sync) function to run. + * @return {Promise<*>} Whatever `fn` returns. + */ +const phase = async (name, fn) => { + if (!state.active) { + return fn(); + } + state.phaseStack.push(name); + const row = phaseRow(name); + row.calls += 1; + // Captured before fn() runs (required for elapsed timing) and consumed in the + // finally below, which executes on every path including the try's return. + // eslint-disable-next-line @wordpress/no-unused-vars-before-return + const startedNs = nowNs(); + event('phase', { event: 'start', name }); + try { + return await fn(); + } catch (err) { + recordError(err, name); + throw err; + } finally { + row.ms += Math.round(Number(nowNs() - startedNs) / 1e6); + event('phase', { event: 'end', name }); + state.phaseStack.pop(); + } +}; + +/** + * Note a step the flow chose to skip (shows up in the timeline). + * + * @param {string} name - Step name. + * @return {void} + */ +const skipped = (name) => { + if (!state.active) { + return; + } + event('skip', { name }); +}; + +/** + * Right-pad a string to a column width. + * + * @param {string} value - Value. + * @param {number} width - Column width. + * @return {string} Padded value. + */ +const pad = (value, width) => String(value).padEnd(width); + +/** + * Percentage of a total, as an integer string with a "%" suffix. + * + * @param {number} part - Part. + * @param {number} total - Total. + * @return {string} e.g. "79%". + */ +const pct = (part, total) => + total > 0 ? `${Math.round((part / total) * 100)}%` : '0%'; + +/** + * Build the auto-flagged "observations" lines for an AI reader. + * + * @param {Array} rows - Phase rows sorted slowest-first. + * @param {number} wallMs - Total wall time. + * @return {string[]} Observation lines. + */ +const observations = (rows, wallMs) => { + const out = []; + const slowest = rows[0]; + if (slowest && wallMs > 0 && slowest.ms / wallMs >= 0.4) { + out.push( + `BOTTLENECK: phase "${slowest.name}" took ${slowest.ms}ms (${pct( + slowest.ms, + wallMs + )} of wall time).` + ); + } + const noisiest = rows + .slice() + .sort((a, b) => b.stdoutBytes - a.stdoutBytes)[0]; + if ( + noisiest && + state.totals.stdoutBytes > 400 && + noisiest.stdoutBytes / state.totals.stdoutBytes >= 0.3 + ) { + out.push( + `NOISY: phase "${noisiest.name}" wrote ${ + noisiest.stdoutBytes + } bytes / ${noisiest.stdoutLines} lines to stdout (${pct( + noisiest.stdoutBytes, + state.totals.stdoutBytes + )} of total). An AI reads all of it; consider summarising this phase.` + ); + } + if (state.errors.length) { + state.errors.forEach((e) => + out.push(`ERROR in "${e.phase}": ${e.message}`) + ); + } else { + out.push('OK: no errors recorded.'); + } + return out; +}; + +/** + * Compose the report block for the run. + * + * @param {Object} extra - Extra fields, e.g. { result }. + * @return {string} The report text. + */ +const buildReport = (extra = {}) => { + const wallMs = sinceStartMs(); + const rows = Array.from(state.phases.entries()) + .map(([name, r]) => ({ name, ...r })) + .sort((a, b) => b.ms - a.ms); + + const ctx = Object.entries(state.context) + .map(([k, v]) => `${k}=${v}`) + .join(' '); + + const lines = []; + const rule = '='.repeat(80); + lines.push(rule); + lines.push('wp-tooling debug run'); + lines.push(`run: ${new Date().toISOString()} pid ${process.pid}`); + lines.push(`command: ${state.command}`); + if (ctx) { + lines.push(`context: ${ctx}`); + } + lines.push(`result: ${extra.result || 'ok'}`); + lines.push(`wall_ms: ${wallMs}`); + lines.push( + `stdout: ${state.totals.stdoutBytes} bytes / ${state.totals.stdoutLines} lines ` + + `stderr: ${state.totals.stderrBytes} bytes errors: ${state.errors.length}` + ); + lines.push(''); + + lines.push( + 'phases (slowest first) [name | calls | wall_ms | %wall | stdout_bytes | stdout_lines]' + ); + if (rows.length) { + rows.forEach((r) => { + lines.push( + ` ${pad(r.name, 22)} | ${pad(r.calls, 3)} | ${pad( + r.ms, + 7 + )} | ${pad(pct(r.ms, wallMs), 5)} | ${pad( + r.stdoutBytes, + 7 + )} | ${r.stdoutLines}` + ); + }); + } else { + lines.push(' (no phases recorded)'); + } + lines.push(''); + + lines.push('observations:'); + observations(rows, wallMs).forEach((o) => lines.push(` - ${o}`)); + lines.push(''); + + lines.push('timeline (ms | phase | label | detail):'); + state.events.forEach((e) => { + const detail = e.detail ? ` ${JSON.stringify(e.detail)}` : ''; + lines.push( + ` ${pad(e.atMs, 6)} | ${pad(e.phase, 22)} | ${pad( + e.label, + 8 + )} |${detail}` + ); + }); + lines.push(rule); + lines.push(''); + return lines.join('\n'); +}; + +/** + * Finish the run: restore streams and append the report. No-op when inactive. + * Safe to call more than once (subsequent calls are ignored). + * + * @param {Object} [extra] - Extra fields, e.g. { result }. + * @return {void} + */ +const finish = (extra = {}) => { + if (!state.active) { + return; + } + event('run', { event: 'end', result: extra.result || 'ok' }); + unpatchStreams(); + state.active = false; + try { + const report = buildReport(extra); + fs.appendFileSync(state.logPath, report, 'utf8'); + } catch { + // Never let diagnostics break the command. + } +}; + +module.exports = { + enabled, + start, + phase, + event, + recordError, + skipped, + finish, +}; diff --git a/node-packages/wp-tooling/src/init/index.js b/node-packages/wp-tooling/src/init/index.js index 885fbce..02dc712 100644 --- a/node-packages/wp-tooling/src/init/index.js +++ b/node-packages/wp-tooling/src/init/index.js @@ -22,6 +22,7 @@ const { execFileSync } = require('child_process'); // Every UI primitive -- Wizard, prompts, spinner, styled status lines, table -- // comes from the wp-tooling kit. const ui = require('../ui'); +const debug = require('../debug'); const { identityFromName, @@ -783,146 +784,158 @@ const run = async (config, options = {}) => { return; } - // --list / --json: the read-only machine-query contract. Intercepted ahead - // of every other flow so config and identity failures also answer in JSON - // when asked to: one stdout line on success, one stderr line on failure. - if (argv.includes('--list') || argv.includes('--json')) { - const wantJson = argv.includes('--json'); - try { - if (!argv.includes('--list')) { - throw usageError('--json is only supported with --list'); + // Diagnostic timing + output capture (opt-in via WP_TOOLING_DEBUG); inert + // when off. --help returns above, so it is intentionally not timed. + debug.start(`init ${argv.join(' ')}`.trim(), { kind, cwd: root }); + let result = 'ok'; + try { + // --list / --json: the read-only machine-query contract. Intercepted ahead + // of every other flow so config and identity failures also answer in JSON + // when asked to: one stdout line on success, one stderr line on failure. + if (argv.includes('--list') || argv.includes('--json')) { + const wantJson = argv.includes('--json'); + try { + if (!argv.includes('--list')) { + throw usageError('--json is only supported with --list'); + } + // --manage is excluded: mode derives from identity/--reinit alone, + // so accepting it here would silently ignore it. --yes is accepted + // (and ignored) so scripted `--list --yes` calls keep working. + const allowed = new Set([ + '--list', + '--json', + '--reinit', + '--yes', + '-y', + ]); + const extra = argv.filter((arg) => !allowed.has(arg)); + if (extra.length) { + throw usageError( + `--list cannot be combined with: ${extra.join(' ')}` + ); + } + try { + validateFeatures(config); + } catch (err) { + err.code = 'ECONFIG'; + throw err; + } + const seedWarnings = []; + let identity = null; + try { + identity = readIdentityFile(root); + } catch (err) { + if (!argv.includes('--reinit')) { + throw err; + } + // --reinit means "discard what's there": report setup mode. + seedWarnings.push( + '.wp-scaffold.json is corrupt; --reinit will overwrite it.' + ); + } + const mode = + identity && !argv.includes('--reinit') ? 'manage' : 'setup'; + listFlow(config, root, { + mode, + json: wantJson, + identity: 'manage' === mode ? identity : null, + seedWarnings, + }); + } catch (err) { + result = 'error'; + if (wantJson) { + emitJsonError(err); + } else { + ui.error(err.message); + } + process.exitCode = 1; } - // --manage is excluded: mode derives from identity/--reinit alone, - // so accepting it here would silently ignore it. --yes is accepted - // (and ignored) so scripted `--list --yes` calls keep working. - const allowed = new Set([ - '--list', - '--json', - '--reinit', - '--yes', - '-y', - ]); - const extra = argv.filter((arg) => !allowed.has(arg)); - if (extra.length) { - throw usageError( - `--list cannot be combined with: ${extra.join(' ')}` + return; + } + + try { + // Cleanup works in either mode. + if (argv.includes('--clean') || argv.includes('-c')) { + const others = argv.filter( + (arg) => '--clean' !== arg && '-c' !== arg ); + if (others.length) { + ui.error('Invalid arguments.'); + process.exitCode = 1; + return; + } + await cleanFlow(config, root); + return; } + + // Validate the feature manifest once, before touching disk, for both modes. try { validateFeatures(config); } catch (err) { - err.code = 'ECONFIG'; - throw err; + ui.error(err.message); + process.exitCode = 1; + return; } - const seedWarnings = []; + + // A corrupt identity file must not silently re-enter setup mode (that + // would re-run destructive scaffold steps on an initialized project). + // Only an explicit --reinit may discard it. let identity = null; try { identity = readIdentityFile(root); } catch (err) { if (!argv.includes('--reinit')) { - throw err; + ui.error(err.message); + process.exitCode = 1; + return; } - // --reinit means "discard what's there": report setup mode. - seedWarnings.push( + ui.warn( '.wp-scaffold.json is corrupt; --reinit will overwrite it.' ); } - const mode = - identity && !argv.includes('--reinit') ? 'manage' : 'setup'; - listFlow(config, root, { - mode, - json: wantJson, - identity: 'manage' === mode ? identity : null, - seedWarnings, - }); - } catch (err) { - if (wantJson) { - emitJsonError(err); - } else { - ui.error(err.message); + + // Manage mode: already scaffolded (unless forced to re-scaffold with --reinit). + if (identity && !argv.includes('--reinit')) { + await manageFlow(config, root, argv, identity, ui, () => + setupFlow(config, root, { yes: false }) + ); + return; } - process.exitCode = 1; - } - return; - } - try { - // Cleanup works in either mode. - if (argv.includes('--clean') || argv.includes('-c')) { - const others = argv.filter( - (arg) => '--clean' !== arg && '-c' !== arg - ); - if (others.length) { - ui.error('Invalid arguments.'); + // Scaffold mode (no identity, or --reinit). + const setupArgv = argv.filter((arg) => '--reinit' !== arg); + const { flags, unknown } = parseFlags(setupArgv); + if (unknown.length) { + ui.error(`Unknown argument(s): ${unknown.join(' ')}`); + process.exitCode = 1; + return; + } + if (flags.yes && !flags.name) { + ui.error('--yes requires --name=.'); process.exitCode = 1; return; } - await cleanFlow(config, root); - return; - } - - // Validate the feature manifest once, before touching disk, for both modes. - try { - validateFeatures(config); - } catch (err) { - ui.error(err.message); - process.exitCode = 1; - return; - } - // A corrupt identity file must not silently re-enter setup mode (that - // would re-run destructive scaffold steps on an initialized project). - // Only an explicit --reinit may discard it. - let identity = null; - try { - identity = readIdentityFile(root); + await setupFlow(config, root, flags); } catch (err) { - if (!argv.includes('--reinit')) { + if (err instanceof ui.CancelledError) { + result = 'cancelled'; + ui.warn('\nCancelled.'); + process.exitCode = 130; + return; + } + if (err instanceof IdentityFileError) { + // Mid-flow corruption (e.g. a manage re-read): report, don't crash. + result = 'error'; ui.error(err.message); process.exitCode = 1; return; } - ui.warn( - '.wp-scaffold.json is corrupt; --reinit will overwrite it.' - ); - } - - // Manage mode: already scaffolded (unless forced to re-scaffold with --reinit). - if (identity && !argv.includes('--reinit')) { - await manageFlow(config, root, argv, identity, ui, () => - setupFlow(config, root, { yes: false }) - ); - return; - } - - // Scaffold mode (no identity, or --reinit). - const setupArgv = argv.filter((arg) => '--reinit' !== arg); - const { flags, unknown } = parseFlags(setupArgv); - if (unknown.length) { - ui.error(`Unknown argument(s): ${unknown.join(' ')}`); - process.exitCode = 1; - return; - } - if (flags.yes && !flags.name) { - ui.error('--yes requires --name=.'); - process.exitCode = 1; - return; - } - - await setupFlow(config, root, flags); - } catch (err) { - if (err instanceof ui.CancelledError) { - ui.warn('\nCancelled.'); - process.exitCode = 130; - return; - } - if (err instanceof IdentityFileError) { - // Mid-flow corruption (e.g. a manage re-read): report, don't crash. - ui.error(err.message); - process.exitCode = 1; - return; + result = 'error'; + throw err; } - throw err; + } finally { + debug.finish({ result }); } }; diff --git a/node-packages/wp-tooling/src/init/manage.js b/node-packages/wp-tooling/src/init/manage.js index ac616cb..473ec59 100644 --- a/node-packages/wp-tooling/src/init/manage.js +++ b/node-packages/wp-tooling/src/init/manage.js @@ -129,6 +129,21 @@ const reqReadFeatures = (root) => readFeatures(root); const manageFlow = async (config, root, argv, identity, ui, reinit) => { const { flags, unknown } = parseManageFlags(argv); if (unknown.length) { + // Scaffold-only flags (advertised in the pre-setup help) do not apply once + // the project is set up. Explain the mode instead of a bare error, so a + // caller (including an AI agent) does not treat it as a failure and investigate. + const scaffoldOnly = unknown.filter((arg) => + /^--(name|version|remove-examples|keep-examples)(=|$)/.test(arg) + ); + if (scaffoldOnly.length) { + ui.info( + `${scaffoldOnly.join( + ', ' + )} apply only during first-time setup and are ignored now: this project is already set up (.wp-scaffold.json exists), so 'npm run init' is in MANAGE mode. To edit the name or version, run 'npm run init' with no flags and use the interactive editor. Run 'npm run init -- --help' to see the manage options, or 'npm run init -- --reinit' to re-run first-time setup.` + ); + process.exitCode = 1; + return; + } ui.error(`Unknown argument(s): ${unknown.join(' ')}`); process.exitCode = 1; return; diff --git a/node-packages/wp-tooling/src/init/transform.js b/node-packages/wp-tooling/src/init/transform.js index d851036..3185807 100644 --- a/node-packages/wp-tooling/src/init/transform.js +++ b/node-packages/wp-tooling/src/init/transform.js @@ -63,6 +63,10 @@ const BINARY_EXTENSIONS = [ * * `bin` is skipped so the per-project scaffold config (which embeds the search * tokens verbatim) is never corrupted; `build`/lock files avoid generated noise. + * `.claude` is skipped because it holds generic, reusable AI skills whose prose + * uses the placeholder name ("Project Name", "project name") as example text - + * renaming those to the real project name corrupts the instructions (e.g. + * "Never assume a project name" would become "Never assume a "). */ const DEFAULT_IGNORE = [ '.git', @@ -70,6 +74,7 @@ const DEFAULT_IGNORE = [ 'vendor', 'bin', 'build', + '.claude', 'package-lock.json', 'composer.lock', ]; @@ -281,7 +286,9 @@ const renameFiles = (files, replacements, ui) => { } try { fs.renameSync(filePath, path.join(path.dirname(filePath), newBase)); - ui.info(`${base} -> ${newBase}`); + // No per-file line here: the caller prints a "renamed N file(s)" + // summary. One line per renamed file is pure noise for a human and + // wasted tokens for an AI reading the run. Failures still warn below. renamed++; } catch (err) { ui.warn(`Could not rename ${base}: ${err.message}`); diff --git a/node-packages/wp-tooling/src/scaffolds/add.js b/node-packages/wp-tooling/src/scaffolds/add.js index 23d2307..592afcb 100644 --- a/node-packages/wp-tooling/src/scaffolds/add.js +++ b/node-packages/wp-tooling/src/scaffolds/add.js @@ -19,6 +19,7 @@ const { ScaffoldError } = require('./registry'); const { formatErrorPayload } = require('./errors'); +const debug = require('../debug'); const { buildRegistry, fetchOptsFrom, @@ -339,12 +340,16 @@ async function runInteractive(opts) { } async function runNonInteractive(opts) { - const registry = await buildRegistry(opts.cwd, fetchOptsFrom(opts)); - const result = await registry.execute(opts.id, opts.inputs, { - dryRun: opts.dryRun, - cwd: opts.cwd, - fetchOpts: fetchOptsFrom(opts), - }); + const registry = await debug.phase('scan', () => + buildRegistry(opts.cwd, fetchOptsFrom(opts)) + ); + const result = await debug.phase('execute', () => + registry.execute(opts.id, opts.inputs, { + dryRun: opts.dryRun, + cwd: opts.cwd, + fetchOpts: fetchOptsFrom(opts), + }) + ); if (opts.json) { process.stdout.write(JSON.stringify(result) + '\n'); } else { @@ -373,14 +378,28 @@ async function runCli(argv) { printHelp(); return 1; } + let mode = 'interactive'; + if (opts.json) { + mode = 'json'; + } else if (opts.nonInteractive) { + mode = 'non-interactive'; + } + debug.start(`add ${argv.join(' ')}`.trim(), { + id: opts.id, + mode: mode + (opts.dryRun ? '+dry-run' : ''), + cwd: opts.cwd, + }); + let result = 'ok'; try { return opts.nonInteractive ? await runNonInteractive(opts) : await runInteractive(opts); } catch (err) { if (err && err.name === 'CancelledError') { + result = 'cancelled'; throw err; // dispatcher handles } + result = 'error'; if (opts.json) { process.stderr.write( JSON.stringify(formatErrorPayload(err)) + '\n' @@ -401,6 +420,8 @@ async function runCli(argv) { } } return 1; + } finally { + debug.finish({ result }); } } diff --git a/node-packages/wp-tooling/src/scaffolds/list.js b/node-packages/wp-tooling/src/scaffolds/list.js index 9457671..7144e11 100644 --- a/node-packages/wp-tooling/src/scaffolds/list.js +++ b/node-packages/wp-tooling/src/scaffolds/list.js @@ -122,6 +122,9 @@ function summarise(scaffold) { kind: 'template', origin: 'remote', counts: null, + // Remote manifests are not fetched during `list`, so the full input + // schema is unknown until `add` hydrates them (mirrors `counts: null`). + inputs: null, }; } return { @@ -146,6 +149,19 @@ function summarise(scaffold) { ) : 0, }, + // Full input schema, so a caller (including an AI agent) has every input's + // key/required/default/discover_from/description without running a + // `--dry-run` add and parsing the EMISSINGINPUT error to discover them. + inputs: Array.isArray(scaffold.inputs) + ? scaffold.inputs.map((d) => ({ + key: d.key, + required: !!d.required, + default: d.default ?? null, + discover_from: d.discover_from ?? null, + transform: d.transform ?? null, + description: d.description, + })) + : [], }; } diff --git a/node-packages/wp-tooling/src/scaffolds/registry.js b/node-packages/wp-tooling/src/scaffolds/registry.js index 6102469..0188321 100644 --- a/node-packages/wp-tooling/src/scaffolds/registry.js +++ b/node-packages/wp-tooling/src/scaffolds/registry.js @@ -393,8 +393,21 @@ class ScaffoldRegistry { // `target_file` templates often build on a path input (e.g. // `{{base_path}}/../Modules/Cli.php`); normalise so the caller gets // `includes/Modules/Cli.php`, not a `..` segment to clean up. + // + // The `../Modules/.php` shape assumes `base_path` is a SIBLING of + // the Modules directory (the flat default, e.g. `includes/Cli`). When a + // consumer nests artifact dirs INSIDE Modules (e.g. base_path + // `inc/Modules/Cli`), the `..` climbs to `inc/Modules` and the literal + // `Modules/` re-descends, producing a doubled `inc/Modules/Modules/...`. + // Collapse that accidental double so the emitted wiring target is the + // real module file in either layout. This only fires when the double + // actually occurs, so the flat layout is unaffected. + const collapseModules = (p) => + p.replace('/Modules/Modules/', '/Modules/'); const aiWiring = (scaffold.wiring || []).map((w) => ({ - targetFile: path.posix.normalize(render(w.target_file, resolved)), + targetFile: collapseModules( + path.posix.normalize(render(w.target_file, resolved)) + ), anchor: w.anchor, snippet: render(w.snippet_template, resolved), description: w.description || '', diff --git a/node-packages/wp-tooling/src/ui/wizard/index.js b/node-packages/wp-tooling/src/ui/wizard/index.js index d71764c..43a0e13 100644 --- a/node-packages/wp-tooling/src/ui/wizard/index.js +++ b/node-packages/wp-tooling/src/ui/wizard/index.js @@ -6,6 +6,7 @@ 'use strict'; const { writeLine, ANSI, isTTY } = require('../core/terminal'); +const debug = require('../../debug'); /** * Wizard class -- orchestrates an array of steps in order. @@ -55,6 +56,7 @@ class Wizard { ? `${ANSI.dim}>> ${label} (skipped)${ANSI.reset}` : `>> ${label} (skipped)` ); + debug.skipped(step.name); continue; } @@ -63,7 +65,7 @@ class Wizard { ? `\n${ANSI.cyan}>${ANSI.reset} ${ANSI.bold}${label}${ANSI.reset}` : `\n> ${label}` ); - await step.run(this.context); + await debug.phase(step.name, () => step.run(this.context)); } return this.context; }