recopy-check-docs
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kristianwindandClaude Opus 5
|
dccb87d15e |
Document the plugin, and check that the documentation still describes it
Five files under docs/, the names the family agreed: README, SETUP,
ARCHITECTURE, CONTRACT, RELEASE. Plus ci/check-docs.php, ported from
woo-security-manager 52dd99d, wired into the static job.
CONTRACT.md is generated from the source rather than written from memory, and
that was the right instruction: my first sweep read only single-quoted hook
names and missed two rows, one of them update_plugins_{$hostname} — the hook
behind the live-shop collision, written with interpolation and therefore
invisible to a pattern built from the concept of the line. Nine hooks
documented: six wc_edm_* filters this plugin offers, three wpmgr_* it both
offers and answers through its copy of the frozen shared-menu.php.
Two hooks the checker structurally cannot see are listed by hand with a note
saying so: wc_edm_prune_audit_log (add_action on a class constant) and
update_plugins_{$hostname} (interpolated). A generated list beats a remembered
one and is still a list of what the generator can see.
The checker needed two fixes before it was safe to run here.
🔴 The published version reads HOOK_PREFIX from ci/plugin.conf for the
documented side, and refuses without it — then tests the source side against a
hardcoded 'nlsec_'. Ported verbatim that is not a green over nothing, which
would be easy to spot. Measured in this tree with CONTRACT.md documenting no
hook at all:
3 hook(s) in the shipped source and not in docs/CONTRACT.md:
wpmgr_dashboard_cards, wpmgr_menu_glyph, wpmgr_menu_title
Red, specific, and blind to all six wc_edm_* hooks — because 'wpmgr_' is the
same string in every tree and 'nlsec_' is the one that is not. Fix those three
and it goes green over six undocumented contract hooks, with a red run behind
you as evidence that it works. A partially blind check that fails is more
convincing than one that passes.
Note which step disarmed the guard. The floor at 20 shipped PHP files refused
to run in this 14-file tree, and that refusal was the only thing standing in
the way; the porting instructions were to add HOOK_PREFIX and adjust the floor.
Doing as told is what removed the protection — two wrongs cancelling into a
pass, in the order CLAUDE.md predicts, because the floor is the error you can
see and the prefix is not.
Both numbers now come from the conf (HOOK_PREFIX, MIN_SHIPPED_PHP_FILES) and
both refuse rather than defaulting. The inherited literal 20 is gone: a second
hardcoded number per plugin is a second thing to re-measure on every port.
Ten selftest assertions, 58 total. Four failure modes, the two refusals, the
counts on the pass, and the row that matters most:
🔴 A hook name in a COMMENT must not read as a hook call. CONTRACT.md documents
a filter by showing the apply_filters() a consumer makes, and the same example
sits in a docblock — so a checker that reads comments reports this plugin
consuming a filter it answers, in the file explaining why it must not. That
assertion is a control asserting a PASS, which is the kind that can quietly
become unable to fail, so it names the specific success line rather than
reading an exit code, and it is proven both ways: with token_get_all's
T_COMMENT branch made inert, exactly one of 58 assertions fails and it is that
one.
docs/TESTING-AND-RELEASES.md is folded into RELEASE.md and deleted rather than
left beside it — two copies of a rule is how the copies start to disagree.
Folding surfaced that it still documented the pre-rename Update URI
(kw/woocommerce-email-discount-manager) and still recommended, as future work,
the split into a releases-only repository that has already happened. Both
corrected in place, with the correction dated.
docs/ is not in ci/dist-files.sh — measured, 0 docs files in the zip.
static=0 selftest=0 site=0; site still 69 assertions on WordPress 7.1.2 with
WooCommerce 11.1.2. ci/ and docs/ ship nothing, so no version bump.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
9b0b0ab475 |
Make the CI checks say how many files they actually read
Four checks in ci/ printed a reassuring line having examined nothing. Found independently by the woo-ai-manager and woo-cache-manager sessions in their own copies of this kit; all four reproduce here. A check that examined nothing prints the same sentence as one that examined everything, and only the passing side can tell them apart. That is why these survived being verified: a pattern that is too broad fails loudly the first time it meets real code, while one that finds nothing passes forever. So every success line now carries a count — the old text domain line could not tell 142 from 0. - static.sh, direct-access guard: counts what the loop examined and fails below MIN_SHIPPED_PHP_FILES. shipped_files() swallows a dist-files.sh failure with `|| true`, so an empty list is one broken .distignore away, and this is the check standing between a PHP file and being requested straight from a browser outside every capability check the code assumes has happened. The floor is above 1 because a pattern that wrongly excludes includes/ collapses the list without emptying it. - static.sh, credential scan: counts what it read, fatal on zero. A pushed key is compromised even after the commit is deleted, so a silent no-op here is not a weak check but the absence of one wearing a green tick. The floor is 1 and is not configurable: this list has no filter that could go partially wrong, so the only way it misreports is by coming out empty. - check-textdomain.php: three zeros, three verdicts. No PHP files at all is always fatal and deliberately not caller-configurable — it says the scan root is wrong, not that the plugin is small. Below --min is fatal. Zero calls with no --min is a warning, since a small plugin may legitimately have no translatable strings, but it is never silent. - check-textdomain.php: fix the annotated path. str_replace( $root, '', $path ) with the default root of "." removed every dot anywhere in the path, so a real error in includes/class-db.php was reported on "includes/class-dbphp" — not a file, not clickable, and not greppable. Every error this script printed with the default root was mangled that way. The floors live in ci/plugin.conf, the only file under ci/ that knows anything about this plugin, rather than hardcoded in static.sh: static.sh is copied verbatim between plugins too, so a number in it would make this plugin's shape everybody's constraint. ci_load_conf now requires them — an unset floor turns its comparison into a no-op and silently restores the bug. Each floor was broken once and watched to fail before being believed: empty shipped list, wrong scan root, --min above the real count, zero calls without --min, empty credential list, and a plugin.conf missing both floors. Static checks green, and now report 13 shipped PHP files, 142 translation calls across 17 files, 44 tracked files read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kristianwindandClaude Opus 5
|
f14c51dcf2 |
Rename the repository to woo-discount-manager, keep the installed slug
Kristian named the five plugins as a family, and three repos disagreed with what
he calls them. His decision: rename the repo, never the installed slug.
MEASURED BEFORE DOING IT, because four published releases carry the old URL in
their Update URI header. A throwaway repo was created, renamed and deleted:
old API path 301 -> new
old web path 301 -> new
old /releases/download/... path 301 -> new
and WordPress follows up to 5 redirects by default (class-wp-http.php:191), on
GET. Confirmed afterwards against the real thing: the old feed URL resolves to
v1.4.5 and the old download path still serves 48185 bytes.
So a shop on 1.4.2 keeps updating through the redirect, and 1.4.6 moves it onto
the new address directly.
PLUGIN_SLUG and PLUGIN_MAIN are deliberately untouched and now disagree with the
repository name. That is permanent: the installed directory has been
woocommerce-email-discount-manager since the first release and is on a live
shop, and changing it would not move that installation — it would put a SECOND
copy beside it and leave the old one active. ci/plugin.conf says so in the file,
because otherwise somebody will helpfully align them.
Also in, both for the family rather than for this plugin alone:
- docs/REUSE.md gains the hostname-keyed hook as a first-class section. It is
the only bug we have found that is CAUSED by there being more than one plugin,
so it is invisible to anyone reading a single repo — along with the same shape
in a shared-*.php file copied into five plugins, which fatals with "Cannot
redeclare" on the second activation without function_exists() guards.
- ci/static.sh fails on anything shaped like a credential in a tracked file.
Suggested by the AI-manager port: a pushed key cannot be unpushed, only
rotated. Deliberately narrow — sk-ant-, sk-, ghp_, gho_, github_pat_, AKIA,
PRIVATE KEY blocks — because a broad entropy heuristic fires on minified
assets and a check that cries wolf gets commented out.
Verified failing before being believed: a file containing sk-ant-api03-... makes
the step report the filename and the suite exit 1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
416c74ac5b |
Answer the details modal, and make two dead checks check
wp-admin builds "View version details" as
plugin-install.php?tab=plugin-information&plugin=<slug>, which calls
plugins_api() — which asks wordpress.org. This plugin is not there, so the link
opened a modal reading "Plugin not found." on the same screen that was telling
the shop owner an update was ready. Seen on the live shop, not inferred.
The filter is answered here now: name, version from the feed, author, links,
compatibility, and the release notes as the changelog — escaped rather than
rendered, because a release body is not a template language. Another plugin's
slug falls through untouched, with an assertion on that: answering for somebody
else would replace their modal with ours.
Two checks in ci/update-smoke.sh could not fail, both found by the subscriptions
port, both sitting under a comment explaining exactly what they were for:
- the plugin directories were counted into a variable that was never read. The
comment above it describes a zip installing a SECOND copy beside the first;
nothing tested it. Now: directories starting with the slug, exactly one.
- the last step ran `wp eval 'echo WC_EDM_VERSION;'`, hard-coding this plugin's
constant into a script that is copied between plugins. Elsewhere it printed
an empty value and carried on. The name is read out of the plugin header now
and the value is compared against the version that was installed, because
files can be replaced while the running code still reports the old version.
Both verified failing before being believed: copies=2 and copies=0 fail,
running != expected fails, and a plugin with no *_VERSION constant takes the
branch that says so instead of comparing against an empty string.
CORRECTION to the note added with the release-id fix. It said those four
releases published correctly "because this instance's author id is -1". That is
wrong in a way that invites relaxing: gitea.nolimit.dk is one instance and both
plugins publish to it. The difference is WHO publishes. A release created by the
Actions job token has the ghost user as its author ("id":-1) and the minus sign
hides it from [0-9]+; a release created by a real user — a cross-repo PAT, or
one published by hand — has "id":1 and the greedy match lands on it. Verified:
this repo's latest release reports {'id': -1, 'login': 'Ghost'}. Corrected in
CLAUDE.md and docs/REUSE.md.
docs/REUSE.md gains "Make a check fail once before you believe it", listing all
three dead checks found today. A green build is exactly what a broken check
looks like.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
43816fdecc |
Read the release id out of JSON, not out of a greedy regex
ci/publish-release.sh found the release id with
sed -n -E 's/.*"id":[[:space:]]*([0-9]+).*/\1/p' | head -1
`.*` is greedy, the response is one line, and a Gitea release payload carries
an embedded "author" object and — once anything is attached — an "assets"
array, each with an id of its own. So it matches the LAST id, not the release's.
Measured against the four published releases here: their real ids are
416/417/419/421, and that pattern returns 1/2/3/4, which are the asset ids.
All four were nonetheless published correctly, and only by luck. The author id
on this instance is -1, the minus sign stops `[0-9]+` matching, and a create
response has no assets yet — so the last numeric id happened to be the
release's. A real author id, a Gitea that orders its fields differently, or a
re-run against a release that already has its zip, and it picks a number that
belongs to something else. Release 4 does not exist in this repository, so a
re-run of v1.4.3 today would 404 loudly rather than attach the zip to a
stranger's release, but that is the blast radius by accident, not by design.
Found by the session porting this into subscriptions, where the same line
picked the author id and the upload 404'd on its first real release.
json_field() reads one top-level field with python3, which release.yml already
requires. Verified against the live responses: 416 and 421 where the sed gives
1 and 4, and empty rather than a traceback on empty input, on "Not found." and
on an error object.
Two guards added with it, both verified against the live v1.4.3 release:
- the id's tag_name must equal the tag before anything is uploaded to it. The
failure worth guarding is not a 404 — that is loud — but an id that exists
and belongs to something else, which would attach this plugin's zip to an
unrelated release with every check still green.
- a re-run skips the upload when the zip is already attached, instead of
failing on the half that previously succeeded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
a1f6e54f9d |
The smoke test could not fail, and other people's eyes
Four fixes, two of them serious, three of them found by the session porting
this into subscriptions.
1. THE SMOKE TEST COULD NOT FAIL. `wp eval-file` runs the file through eval()
INSIDE A METHOD, so the top level is function scope. `$wc_edm_failures = 0`
at the top and `global $wc_edm_failures` in the helper were two different
variables: the helpers incremented the real global, the verdict read the
local, and the local was always zero.
Measured rather than reasoned. A deliberately false assertion added to the
unmodified file:
FAIL DELIBERATE FAILURE — this run must go red
All assertions passed on WordPress 7.1 with WooCommerce 11.1.0.
EXIT=0
and with the fix, the same forced failure:
FAIL DELIBERATE FAILURE — this run must go red
1 assertion(s) failed on WordPress 7.1.
EXIT=1
The FAIL lines always printed, so failures found by reading the output were
real finds. What was broken is the CI gate: a regression on either forge
would have gone green. Four lines at the top now prove the counter reaches
the verdict before anything relies on it — this is the one file whose whole
job is to be able to go red, so it checks that it can.
2. `git ls-files` QUOTES NON-ASCII PATHS. "TESTMILJØ.md" comes back as the
literal string "TESTMILJ\303\230.md", which matches no pattern in
.distignore, so the file is kept — an internal document walking into the
release zip with nothing said. Fixed in ci/dist-files.sh and in
ci/static.sh's trailing-?> check, which had the same bug and would have
silently skipped the file it exists to check. Verified by adding such a
filename and watching it be excluded. `done < <(...)` rather than a pipe, or
the loop runs in a subshell and the failure counter stops incrementing.
3. THE FEED CACHE WAS SIX HOURS, and Kristian hit it: a release published and
his shop shown nothing. The reasoning ("WordPress checks twice a day") was
wrong. WordPress gates its own check by screen in wp_update_plugins() — one
minute on Dashboard → Updates, one hour on Plugins, 2–12 hours in the
background — so WordPress is already the rate limiter and a longer cache
only hides releases underneath it. Fifteen minutes now, pinned by an
assertion.
4. The wp_update_plugins() assertion made a real HTTP request, because the
force-check tests had just deleted the transient. It passed only because
this feed repo is public, which made it measure the environment rather than
the code. Re-primed first.
ci_in_container() replaces the inline docker plumbing in ci_php(), taken from
the subscriptions port: the script goes in as a file rather than interpolated
into `sh -c`, and --entrypoint sh matters for images whose entrypoint swallows
its arguments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
200f791f40 |
An icon, a Check again that checks, and the auto-update rule written down
Three gaps found by being asked plain questions about it.
**The auto-update toggle was a promise nobody had checked.** offer_update()
answers even when the installed version is current, on the reasoning that the
answer has to land in WordPress's no_update list for the "Enable auto-updates"
link to exist at all. That was a comment and an intention; it is now an
assertion against the real WP_Plugins_List_Table, which is where the flag is
actually read:
$plugin_data = array_merge( (array) $plugin_info->no_update[ $plugin_file ],
array( 'update-supported' => true ), $plugin_data );
A plugin in neither response nor no_update gets "Auto-updates are not available
for this plugin" while looking entirely healthy.
**An icon.** Without one, Dashboard → Updates shows a generic grey plug.
assets/icon.svg, handed over in the offer's icons key, served from the shop's
own copy rather than the release host — one fewer thing that must be reachable
for wp-admin to render, and it cannot go mixed content on an HTTPS shop. Drawn
as paths rather than <text>: an SVG that leans on a font renders differently on
every machine, and this one is shown at about 40px, which is the size where
that is the difference between a mark and a smudge.
**"Check again" now checks again.** The six-hour cache is right for the check
WordPress runs twice a day and wrong for a link whose whole promise is that it
looks now. Publishing a release and then clicking it showed nothing for up to
six hours, which reads exactly like a broken updater.
And docs/REUSE.md was missing all of this: it covered Update URI, where to
instantiate the updater and making the repo public, but not the one rule that
is genuinely easy to get wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kristianwindandClaude Opus 5
|
646d5e5cc1 |
Document the update smoke test and why the download address is rebuilt
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kristianwindandClaude Opus 5
|
eca2851d35 |
Test against the newest WordPress and WooCommerce, and ship updates
Three things this repository could not do: prove the plugin still works on a
WordPress or WooCommerce released after it, get a fix onto a shop without
someone uploading a zip, and do either of those for the next plugin without
starting over.
ci/ is the test harness. ci/site-test.sh builds a throwaway WordPress +
WooCommerce in containers at any versions you name — including `nightly` and
`beta` — installs exactly the files the release zip will contain, and runs
tests/smoke.php inside it with wp eval-file. Real cart, real coupons, real
WC_Shipping_Rate objects: a mocked cart keeps passing after WooCommerce has
changed underneath it, which is the one failure worth catching here. WP_DEBUG
is on and any notice logged from this plugin's directory fails the build,
because that is the shape an upgrade actually breaks in — nothing fails, a
notice appears, and two releases later it is a fatal.
compat.yml runs that weekly against today's releases, the next WordPress and
the next WooCommerce, the oldest PHP the header promises and the newest PHP.
When it is green it opens a PR bumping "Tested up to" and "WC tested up to" —
only when green, because WooCommerce shows that second value to the shop owner
as an untested-plugin banner, and that banner is what they read before deciding
whether to press update.
includes/class-updater.php is the distribution half. It hooks
update_plugins_{hostname} for the host in the new "Update URI" header and reads
the Gitea release API, so WordPress handles the notice, the download, the
"Enable auto-updates" link and wp plugin update with no further code. Update
URI earns its place twice: since 5.8 a plugin that declares one is never asked
about on wordpress.org, so a .org plugin sharing this slug cannot be pushed
onto a shop running this one. The check sends nothing about the site and
overrides WordPress's default user agent, which carries the site's own URL.
release.yml tags, tests, builds, publishes — then reads the feed back with no
credentials. A private repository and a release with no zip attached both leave
a page that looks perfect to anyone logged in and an update no shop is ever
offered.
Measured before committing, all green: WP 7.1 + WC 11.1 on PHP 8.3 and 7.4,
WP 7.2-alpha on 8.3, and the beta cell falling back to stable on 8.4. No
deprecations from this plugin on any of them.
Three guards are there because the bug they catch is invisible without them:
the annotated tag is re-fetched before its message is read (checkout turns it
into a lightweight tag pointing at the merge commit), the two workflow roots
are kept in sync (.gitea/workflows hides .github/workflows entirely), and every
file the code loads must be in the release zip — which caught class-updater.php
being referenced and uncommitted while every local test passed.
docs/REUSE.md is the port to the next plugin: copy ci/, rewrite five lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|