WP-CLI smoke test, and keep the root key across multisite conversion - #1
Merged
Merged
Conversation
docs/SPEC.md wraps tests/smoke/SPEC.md in the shape the Foundry planner reads, adding the repository's documentation, journal, and no-publish rules. docs/foundry.json is pre-seeded with the verify commands and auto permission mode for an unattended run.
Twenty tasks in seven phases, following docs/SPEC.md §8. The design is tests/smoke/SPEC.md; every task cites the section it implements. The pre-seeded docs/foundry.json keeps baseBranch, branchPrefix, permissionMode, and both verify commands unchanged and gains thirteen self-testing constraints. CLAUDE.md keeps the existing 'Working in this repository' section verbatim and adds the Foundry headings below it.
Goal: bin/smoke-install.sh provisions a fresh single-site WordPress in .smoke/wordpress/ with a SHA-256-verified wp-cli.phar, its own wordpress_smoke database, WP_SECRETS_KEY defined before any secret exists, and the plugin symlinked in and active. make smoke runs it. Tests: no PHPUnit surface; the script's own run is the test, executed inside this worktree's wp-env cli container. Ran the script (exit 0), core is-installed (exit 0), plugin is-active secrets-api (exit 0), config get WP_SECRETS_KEY (prints 44 chars decoding to 32 bytes), and a second run (exit 0, single-site, no drop-in). Checksum path: flipped a hex digit of WP_CLI_SHA256, deleted the cached phar, confirmed exit 1 with expected/actual digests printed, then restored the digit. Interpretation: WP_CLI_VERSION/WP_CLI_SHA256 resolved from https://api.github.com/repos/wp-cli/wp-cli/releases/latest on 2026-09-24 (v2.12.0) and verified against that release's own wp-cli-2.12.0.phar.sha256 before computing the pinned digest, per the Decisions section of docs/PLAN.md. `wp config set WP_SECRETS_KEY ... --type=constant` needed `--quiet`: WP-CLI's own "Success" line otherwise echoes the constant's value, which is exactly the plaintext this flight must never print. Manual check: this worktree's wp-env cli container ships PHP's memory_limit=128M, unlike a typical CLI SAPI (which defaults to -1). `wp core download`'s zip extraction needs more than that regardless of invocation (reproduced with the container's own pre-installed `wp` binary, not just this script), so the exact verification command in the task may need `WP_CLI_PHP_ARGS`/a higher container memory_limit on a from-scratch machine. Not a defect in this script or its fixed WP array; recorded here for whoever next runs `make smoke` cold.
Goal: push the branch with the install harness and record what only a human can verify. Tests: none new. Manual check: NOT VERIFIED (human) - make smoke on a host without Docker, with MySQL on 127.0.0.1 and DB_PASS set as needed, provisions the install. - The WP-CLI pin matches the release page by eye.
Goal: tests/smoke/smoke.sh exists with configuration, TAP helpers, the run/assert contract, a summary trap, and case A's registration matrix (11 subcommands under both secret and network-secret), and make smoke runs it after the install. Tests: ran tests/smoke/smoke.sh inside this worktree's wp-env cli container against a freshly provisioned install: 22 ok lines, `1..22`, `# passed 22, failed 0`, exit 0. Harness sanity check: temporarily added `not_ok "probe"` as the first statement in main, reran, got 23 assertions with `# passed 22, failed 1` and exit 1, then removed the probe (not committed). Interpretation: `wp cli has-command` sees every plugin-registered subcommand on this single-site install without the has-command/help contingency the task allows for, so no fallback was needed; all 22 assertions used has-command as written. Manual check: NOT VERIFIED (human) -- none required beyond the recorded run above; make ci wiring is out of scope for this task.
Goal: case A's second and third bullets: every subcommand's SYNOPSIS
flag set equals a table written at the top of the script, and
--version=previous is not a slot selector.
Tests: ran tests/smoke/smoke.sh against a fresh install inside this
worktree's wp-env cli container: 37 assertions, 0 failed, exit 0 (11
synopsis rows plus the four --version=previous pin assertions on top
of case A's 22 registration checks). Negative check: temporarily
dropped --reveal from the get row, reran, got `not ok 24 ... expected
[--field --format --slot] got [--field --format --reveal --slot]`,
then restored the row (not committed).
Interpretation: synopsis_flags()'s `grep -o -- '--[a-zA-Z-]*'` first
pass matched a bare "--" inside generate-key's prose ("wp-config.php
-- adding the constant..."), which sits inside the SYNOPSIS section
before the next header. Tightened the pattern to
`--[a-zA-Z][a-zA-Z-]*` (at least one letter after the dashes) so
free-text em-dashes never register as a flag.
Manual check: NOT VERIFIED (human) -- none required beyond the
recorded run above.
Goal: push the branch with case A and record what only a human can verify. Tests: none new. Manual check: NOT VERIFIED (human) - make smoke on a host without Docker runs case A green.
Goal: case B's first four bullets run end to end through the real binary with exit codes checked on every call. Tests: ran tests/smoke/smoke.sh against a fresh install inside this worktree's wp-env cli container: 58 assertions total, 0 failed, exit 0. 21 new assertions cover a positional set, set --stdin (via the new run_stdin() helper, since piping directly into run() would fight its own stdout capture), set --porcelain (single line, equals get --field=fingerprint), default masking vs --reveal (both --field=value and the table), the --slot=previous demotion round trip (bug 1, end to end), and --format=json (validity plus exactly one row named for the secret). Manual check: NOT VERIFIED (human) -- none required beyond the recorded run above.
…pin, import, migrate, and the single-site refusal
Goal: the remaining case B bullets plus the single-site network-secret
refusal the detailed spec places in this pass.
Tests: ran tests/smoke/smoke.sh against a fresh install inside this
worktree's wp-env cli container: 92 assertions total, 0 failed, exit
0. 34 new assertions cover list (JSON/CSV validity, fields header,
namespace filtering, never a value), retire, delete, absence, a
no-value set's non-zero exit, generate-key's length and decode,
health/dropin JSON and no-drop-in report, import-option's copy (source
option left in place), migrate-legacy --dry-run, and network-secret's
single-site refusal with its "multisite" explanation in stderr.
Interpretation: none beyond what the task specified; `WP_CLI::error()`
for the network refusal exits non-zero (confirmed 1) with "Network
secrets require a multisite installation." on stderr, matching the
assert_err_contains("multisite") check as written.
Manual check: NOT VERIFIED (human) -- none required beyond the
recorded run above.
Goal: push the branch with case B and record what only a human can verify. Tests: none new. Manual check: NOT VERIFIED (human) - Read the full TAP output once by eye for any line that shows a value or key. Every assertion in this phase's runs passed, so no diagnostic ever printed; the passing lines themselves are only descriptions, never OUT.
Goal: implement case C (rotation) in tests/smoke/smoke.sh: rotate
refuses without WP_SECRETS_KEY_PREVIOUS, and after moving the current
key to PREVIOUS and generating a new one, rotate --yes succeeds, the
value is still readable, and health reports nothing undecryptable.
Tests: case_c_rotation now runs 10 assertions (93-102): the refusal
and its message before WP_SECRETS_KEY_PREVIOUS exists, both config
writes, rotate --yes, get --reveal returning the original value, and
health --format=json showing "good" on the decrypt-check row. Full
suite verified inside a local wp-env-backed install: 102 assertions,
0 failed, exit 0.
Interpretation: the task's negative check ("skip step 2's config set
WP_SECRETS_KEY \"$new\" and confirm step 3 or 4 fails") does not
produce a failure as written. Skipping that line leaves
WP_SECRETS_KEY_PREVIOUS equal to WP_SECRETS_KEY (both the original
key), and per docs/spec/rotation.md, rotate unwraps the root key
under the old keyring and re-wraps it under the new one with no
requirement that they differ -- rotating to an unchanged key is a
legitimate no-op that succeeds. Verified directly: with that line
disabled the full suite still passed 102/102. This is consistent with
the spec, not a defect, so the negative check isn't encoded as a
committed assertion; the disabled-line experiment was run manually
and reverted, matching the spec's "As built" description of
site-key rotation.
Manual check: verified inside a real install (wp-env mariadb
container as the smoke DB) rather than only via the smoke-install
default; bin/ci-local.sh --keep and make reference-check both green.
Goal: implement case D in tests/smoke/smoke.sh: each of the four drop-in shapes plus the recorded uncatchable fatal, written to wp-content/secrets.php, asserted through the real loader, and removed, with an EXIT trap that removes any drop-in the script wrote. Tests: case_d_dropin adds 15 assertions (103-117): syntax error and throws-on-load both exit 2 with dropin reporting WP_Secrets_Broken_Provider; a wrong-type wp_secrets_provider global exits 2 (the 4 September fail-closed fix, through the real loader); a no-op drop-in leaves get and dropin behaving exactly as with no drop-in, still reporting WP_Secrets_Libsodium_Provider; the uncatchable fatal (a class implementing WP_Secrets_Keyring with no methods) exits non-zero with "Fatal error" on stderr; and a final check that no drop-in remains. write_dropin/remove_dropin helpers added, and the finish EXIT trap now calls remove_dropin first. Verified inside a real install (wp-env mariadb container as the smoke DB): full suite 117 assertions, 0 failed, exit 0. Confirmed `test ! -e .smoke/wordpress/wp-content/secrets.php` after the run. bin/ci-local.sh --keep and make reference-check both green. Manual check: trap check per the task -- inserted a temporary `exit 3` right after the first write_dropin, reran, confirmed the drop-in file was gone and the overall exit status was non-zero (1, via the finish trap), then reverted the exit statement (verified via diff against the pre-edit file, byte-identical).
Goal: push the branch with case C (rotation) and case D (drop-in loading) and record what only a human can verify. Tests: none new. Push: pushed to origin/build/cli-smoke (d6748e1..3578368). Manual check: NOT VERIFIED (human) - Run the uncatchable-fatal drop-in row (a class implementing WP_Secrets_Keyring with no methods) by hand on a PHP version newer than 8.3, and confirm it is still a fatal rather than a catchable TypeError/Error. This smoke run was verified on PHP 8.5, which is newer than 8.3, but that does not substitute for the specific cross-version check the task calls for by a human against whichever PHP versions the project targets going forward.
Goal: one dated dev-journal entry tells what was built, what it found, what was left out, and what it means for the Trac patch, and docs/index.md lists it. Tests: no code change; docs only. - docs/journal/2026-09-24-testing-the-cli-for-real.md (new): frontmatter title/description/date, voice matching the 4 September entry, sections in the required order, linking tests/smoke/smoke.sh and ADR 0008, naming the --version swallow as the concrete PHPUnit-can't-catch finding and the multisite root-key adoption fix as the one src/ change this work drove. - docs/index.md: added the entry's line under journal/, after the 0.1.0 line, same shape. - tests/smoke/SPEC.md: date P7-01 wrote already matches today (2026-09-24); no correction needed. Verification: head -5 on the new file shows the three frontmatter keys; grep -c 'smoke.sh' and grep -c '0008' are each 3; grep -n 'testing-the-cli-for-real' docs/index.md tests/smoke/SPEC.md shows both; git diff --stat -- docs/journal/_drafts is empty. bin/ci-local.sh --keep and make reference-check both clean. Interpretation: none needed. Manual check: none beyond the greps and verify run above.
Goal: push the finished branch and record the human-only checks the specs list. Tests: none new. git push origin HEAD succeeded (4886a53..ee4ad02, fast-forward); git status clean; git ls-files | grep -c wp-env.override prints 0. Manual check: NOT VERIFIED (human): (1) make smoke on a clean checkout with a local MySQL; (2) the CI smoke job green on 7.4 and 8.3; (3) npm run docs:build renders the new journal entry in the sidebar in date order; (4) a read of the journal entry for voice and for anything private.
… variable
Goal: the smoke-diagnostics-never-print-stdout constraint flags a
not_ok or diag that interpolates any variable holding plaintext or
key material, independent of the exact variable name, so newly
added locals (current_key, previous_key, VN, VS) can't silently
reopen the blind spot R1-03 was meant to close.
Tests: docs/foundry.json's smoke-diagnostics-never-print-stdout
pattern extended with two case-insensitive alternatives
([A-Za-z0-9_]*[Kk][Ee][Yy][A-Za-z0-9_]* and the same for "value")
plus the exact names VN and VS, alongside the existing alternation.
Added 5 shouldMatch fixtures (not_ok "rot" "$current_key", diag
"prev ${previous_key}", diag "$VN", not_ok "site" "$VS", diag "got
$some_value") and 2 shouldNotMatch fixtures copied verbatim from the
real file (the $desc/$STATUS/$expected diagnostic and the
$expected_sub synopsis-mismatch diagnostic). All prior shouldMatch
and shouldNotMatch entries kept and still pass. baseBranch,
branchPrefix, permissionMode, and both verify commands (with their
timeouts) untouched.
CLAUDE.md: updated only the one "## Constraints" bullet describing
this rule (adds VN, VS, and the key/value-in-any-case clause); the
"# Working in this repository" heading and every other line
untouched (git diff CLAUDE.md confirms a 2-line change confined to
that bullet).
Re-read tests/smoke/smoke.sh for any other new plaintext/key
variable since P5-01 landed: none found beyond current_key,
previous_key (already in R1-02), VN, and VS (P5-01), and none of
those are interpolated inside an existing not_ok/diag call, so the
constraint scan against the real tree still reports zero hits.
Verification: foundry_verify -- smoke-diagnostics-never-print-stdout
self-tests green (fixture null, hits []), every other constraint
unchanged and green, bin/ci-local.sh --keep green (139/139 smoke,
459/459 x2 PHPUnit), make reference-check clean.
Interpretation: none needed; followed the task's suggested pattern
shape directly.
Manual check: none beyond the foundry_verify run above.
Goal: every place this branch explains bug 1 says what the real wp
binary actually does -- WP-CLI passes --version after a command to
the subcommand; the historical symptom came from wp-env run's own
argument parsing dropping the flag first. Case A pins that against
the real binary; case E asserts directly that a secret written
before multisite-convert still decrypts after it.
Tests: tests/smoke/smoke.sh gains 5 assertions (139 -> 144):
ok 37 - get --version=previous exits 1: WP-CLI passes the flag to
get, which does not declare it
ok 38 - WP-CLI rejects --version as an undeclared get parameter
ok 128 - set smoke-NS/preconvert exits 0
ok 131 - get smoke-NS/preconvert --reveal --field=value exits 0
after multisite-convert
ok 132 - a secret written before multisite-convert still decrypts
after it
Full run: 1..144, # passed 144, failed 0. bin/ci-local.sh --keep and
make reference-check both green.
Interpretation: kept --slot as the flag name even though WP-CLI does
not actually consume --version itself, since a subcommand flag
sharing a name with a global WP-CLI flag (wp --version) is still
worth avoiding on its own; the docblock now states both reasons
(wrapper-swallowing and name confusion) without claiming WP-CLI
itself drops the value.
Manual check: none.
Goal: Remove the invented causal link between bug 1's rename and the root-key defect from the journal, and correct case A's comment in smoke.sh to describe what it actually pins. Tests: bin/ci-local.sh --keep, 144 of 144 smoke assertions still pass; no assertion added, removed, or changed (comment-only diff in smoke.sh). make reference-check clean. Interpretation: Replaced the "What it found" paragraph's account of reintroducing --version with what P6-01 actually recorded: on the current tree get() declares --slot and WP-CLI rejects --version outright (case A pins this); reintroducing --version by hand instead shows the previous value coming back, and the suite still fails on the synopsis table plus both --slot=previous rows, since it catches the rename rather than the historical symptom. Also rewrapped the "What was built" paragraph's over-length line to 100 columns, and reworded smoke.sh's case-A comment from "Pin bug 1's cause" to "Pin WP-CLI's side of bug 1", since the comment was only ever pinning WP-CLI's rejection behavior, not the root cause of the original bug. Manual check: NOT VERIFIED (human) — confirm the rewritten paragraph reads correctly in the rendered docs site.
The plan, progress log, reviews, summary, and pipeline state were working files for the build. The branch's substance is in its commits, docs, and journal entry. CLAUDE.md goes back to main's copy, and the lock-file ignore rule goes with the pipeline.
CLAUDE.md now says a tracking page's date: field is the date of its last substantive change.
Adds the smoke job to main's CI alongside the combined Moto and Vault examples job, and joins make smoke to make ci next to test-examples. The key manager merged cleanly: reads go through the multisite-aware lookup, then the request-scoped cache. Main's --from on wp secret rotate failed the smoke test's exact flag table, which is the drift it exists to catch. The table now expects --from, and the placeholder for it became a real case: --from=config refuses while the config keyring is still the active one. Also drops a duplicated examples row from the CI reference.
ericmann
changed the title
ericmann
marked this pull request as ready for review
September 25, 2026 01:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the WP-CLI smoke test described in
tests/smoke/SPEC.md: a dependency-free bash harness that drives a real, pinnedwpbinary against a throwaway install. It closes the last 🟡 coverage gap before the Trac patch (ADR 0008).Fix to
src/(part of the Trac patch)*_site_option(). Before conversion that writes towp_options; after it, reads go towp_sitemeta, andwp core multisite-convertdoesn't move the row. The first read after conversion found no root key and quietly generated a new one.WP_Secrets_Key_Managernow finds the main site's row from before the conversion, adopts it into sitemeta, and deletes the old row only once the copy has succeeded.docs/spec/network.md's claim that conversion doesn't strand secrets is now true.The smoke test (
make smoke, now part ofmake ci,bin/ci-local.sh, and asmokeCI job on PHP 7.4 and 8.3)wp secretandwp network-secret.--stdin, porcelain output,--slot=previous, and JSON/CSV output all work as documented.wp core multisite-convert.31961a1).A correction: the
--version=previousbug was caused by wp-env's argument parser dropping--versionbefore the command reached the container, not by WP-CLI. The pinned WP-CLI passes the flag through to the subcommand. The docblock, the generated reference, and the journal now say so. Renaming the flag to--slotstill stands.Docs: coverage gaps updated (the 🟡 CLI dispatch gap and the
--stdingap are closed), the README and CI reference, and the journal entry "Testing the CLI for real".Still to verify by hand:
make smokeon a host without Docker, and the uncatchable-fatal drop-in case on PHP newer than 7.4.