Commit Graph
9 Commits
Author SHA1 Message Date
kristianwindandClaude Opus 5 ae4112c773 Publish releases to kw/nolimit-discounts-dist and point the header there
The family shape is source private, releases in a separate public repo so a
shop can fetch an update without credentials and without the source being
readable. This plugin was the only one still serving its own update feed out of
the repository that holds ci/, docs/ and tests/.

kw/nolimit-discounts-dist created (public, zips only). RELEASE_REPO in
ci/plugin.conf now names it, and ci/publish-release.sh honours it over
GITHUB_REPOSITORY — falling back to the running repo, which is what a plugin
without a split still wants.

🔴 The "Update URI:" header moves in the SAME commit as the publishing target,
and ci/static.sh now fails when the two disagree. The header is what every
installed copy polls; if it names a repository nothing is published to any
more, the shop stops being offered updates and nothing announces it, because
latest_release() simply returns false. That is the same failure as a privacy
page standing still while the code moves, and the check is what makes it
impossible to forget.

Proven red before being believed: pointing RELEASE_REPO at another repository
fails with both sides named. RELEASE_REPO is required by ci_load_conf, because
an unset value would turn the comparison into a no-op and restore exactly the
bug it prevents.

Not done here, and it is the blocker for actually cutting a release: publishing
to another repository needs a token with write access to it. The job token only
covers the repo it runs in, so this needs a cross-repo PAT in the repository
secrets, and that is Kristian's to add.

Also not done, deliberately: the source repo stays public for now. Making it
private before the dist repo has releases would break the feed for the copy
installed on the live shop, whose header still points at the old address.
1.4.6 and earlier keep working through Gitea's rename redirect - measured, the
old name still answers 200 - and that redirect is the only thing holding them
up, so it must not be removed until a build carrying the new header has
propagated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 22:38:12 +02:00
kristianwindandClaude Opus 5 f3126b57e6 Stop the credential scan silently skipping binary files
Adding the name-anchored rule in the previous commit also put `-I` on the
value-pattern grep, which it did not have before. grep -I SILENTLY SKIPS a file
it decides is binary, so a PEM header inside one is waved through without a
word. Without -I the same file reports "binary file matches" and fails, which
is the right direction to be wrong in for a check whose whole subject cannot be
unpushed.

Nothing went red to announce this. The tree has no binary tracked files today,
so the narrowing was invisible from both sides — the clean run stayed green and
no probe covered it, because every probe so far planted a text file at the repo
root. It is the same class as the hole this scan already had: a check that
cannot match anything and a check that matches nothing print the same line.

The two rules now differ on -I deliberately, each with the reason inline. The
name rule keeps it: that one is heuristic, so a chance match inside a compiled
asset is noise rather than a finding.

Also proven, rather than asserted, after a sibling plugin found its own scan was
reading only the SHIPPED file list and therefore missing 36 of 73 tracked files
— every CI script, workflow and test fixture, which is exactly where a key gets
pasted to test it. This scan reads `git ls-files` and always has, but the claim
had never been measured: probes in ci/, tests/ and .github/workflows/ are all
caught while `dist-files.sh` lists none of them.

Battery now 19, both directions, through ci/static.sh:

  13 fire  — AKIA, PEM, sk-ant-, sk-, ghp_, gho_, github_pat_, array form,
             define() form, the same key in ci/ and tests/ and .github/, and a
             PEM header inside a binary file
  6 quiet  — a bare hash, an unfilled placeholder, a masked value, an empty or
             short value, a comma after 'token' in ordinary code, and a define
             whose value is still TODO

The comma control comes from the sibling plugin: it put `,` into the shared
alternation and had to prove `wp_die( 'token', 'a long sentence' )` stays
quiet. This tree anchors the comma case on `define(` instead, so the same
string was never at risk — but it is worth a control either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 00:25:52 +02:00
kristianwindandClaude Opus 5 82d6cc7d7d Close the inert-pattern hole in the credential scan
A count proves the scan READ a file. It says nothing about whether the pattern
can match anything — and "now it reports a number" and "now it only reports a
number" print the identical line. The woo-ai-manager session proved this the
hard way: it planted an AWS key, watched its file count rise from 37 to 38, and
the scan passed green. Its prefix list had no AKIA entry at all.

This tree's list did have AKIA, but the proof behind that claim was a grep of
the regex in isolation, not a run of ci/static.sh — a stub harness that proves
the pattern compiles, not that the check fires. Re-done properly: every shape
the pattern claims to catch is now planted in a tracked file and run through
the whole script.

That exercise found a real hole. A prefix list matches the shape of a VALUE, so
it is blind to any credential with no vendor marker: an api_key assigned twenty
opaque characters walked straight past it. Added a second, name-anchored rule
that fires only where the code itself calls the value a secret, covering both
the array/assignment form and define(). It is not an entropy heuristic — the
file's existing refusal of those still stands, because they fire on minified
assets and hashes and a check that cries wolf gets commented out.

Placeholders are excluded, or the rule would go red on the one thing a settings
screen is guaranteed to contain.

Proven from both sides, through ci/static.sh rather than in isolation: 9 shapes
planted and all 9 flagged (AKIA, PEM, sk-ant-, sk-, ghp_, gho_, github_pat_,
array form, define form), and 4 lookalikes correctly ignored (a bare hash, an
unfilled placeholder, a masked value, an empty/short value). 13/13.

The one false positive measured was this change's own explanation: the first
version of the comment spelled the example out in full and the rule flagged it.
Fixed by writing it in prose. Deliberately no bypass marker — an unexercised
escape hatch is its own hollow check; add one when a real documentation case
needs it.

Note for the sibling plugins: an untracked fixture is invisible here, since the
scan reads git ls-files. A probe planted without `git add -N` passes clean and
looks like the scan working.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 00:20:44 +02:00
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>
2026-09-17 00:14:40 +02:00
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>
2026-09-16 18:18:50 +02:00
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>
2026-09-16 13:18:22 +02:00
kristianwindandClaude Opus 5 056be0e917 Credit the plugin to its actual author, and 1.4.3
The header went out as "Author: WEDSMP" with a Plugin URI of example.com. That
is what wp-admin shows to whoever installs it, which is why it is worth a
release of its own rather than waiting for the next change.

Author is now Kristian Wind, with an Author URI pointing at where the source
lives, and the readme credits kristianwind rather than wedsmp.

ci/static.sh now fails on a placeholder in the header. It catches the generic
markers — example.com, TODO, yourname, an unfilled <...> — and it would have
caught the Plugin URI. It would NOT have caught "WEDSMP": a made-up vendor name
is indistinguishable from a real one to any check, so an empty field is warned
about and a wrong-but-plausible one cannot be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 12:59:44 +02:00
kristianwindandClaude Opus 5 4a5e3d8879 Run the text-domain check in a container when the runner has no PHP
The first Gitea run failed on `php: command not found`. GitHub's hosted
runners ship PHP; the Gitea runner image does not, and the whole static job
went red on one line after everything else in it had passed.

ci_php() now uses the local php when there is one — a laptop, GitHub's
runners — and a php:8.4-cli container otherwise. The repository goes in over a
tarball and `docker cp` rather than a bind mount, because a Gitea Actions job
runs inside a container talking to the HOST's docker daemon, so a host path
there lands on an empty directory nobody created.

Also: the include scan now says when it found nothing to check instead of
reporting success. Running the suite with a deliberately crippled PATH showed
it printing "0 plugin-relative include(s) checked" and then "green" — a check
passing by checking nothing, which is the same failure the PHP lint step
already guards against.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 10:40:19 +02:00
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>
2026-09-16 10:34:38 +02:00