e4ee584e865d878408bf35bcdcc73a8210381cdb
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 twooey
|
e4ee584e86 |
Fix six low-severity issues from the full-session code review
- PricingEngine::round_to_99(): ceil($price) - 0.01 undershoots whenever $price's cents are already .99 or higher — an exact integer (ceil() equals floor(), landing a full cent below $price) or, more subtly, any fractional price above X.99 itself (a division result, not something pre-rounded to 2 decimals, e.g. 20.995 -> old formula gave 20.99, below the input). Rewritten as floor()+0.99, bumped by 1 if still under $price. Verified against 8 cases including both boundary classes: every result now >= its input. - CoverSync::attach_cover(): wp_generate_attachment_metadata()'s return value was never checked. Assumed it'd return empty on failure — verified directly it does NOT: fed it 2000 bytes of garbage and got back ['filesize' => 2000], no width/height, since GD/Imagick couldn't decode it. Old code would report "attached" for a degraded image with no dimensions/srcset. Now checks for width+height specifically, and cleans up the orphaned attachment on failure so a re-run retries the product. Verified both the corrupt-image rejection (no orphan left, no thumbnail set) and that a real image still attaches normally. - Makefile: the per-invocation .env.$(ENV).compose file (holds every API key and both DB passwords, stripped of DB_PASSWORD/DB_ROOT_PASSWORD only) was never cleaned up, left at default 644 in the repo root after every `make` command. Now chmod 600 on creation and removed at the end of every target, preserving the underlying command's exit code through the cleanup. Verified both the happy path (file gone after, exit 0) and the failure path (bad ENV: file still cleaned up, real exit code still propagates through make). - docker-compose.yml: added a healthcheck to the wordpress service (bash's /dev/tcp against php-fpm's port 9000 — no HTTP endpoint to hit directly, and no `nc` in this image; verified it correctly succeeds once php-fpm is listening and fails against a closed port) and switched cron's and caddy's depends_on (across all three env overlays) from bare container-started to condition: service_healthy. Previously both could start against a wordpress container that had started but wasn't actually ready yet. Verified via a full down/up cycle: db+redis healthy, then wordpress starts and becomes healthy, only then do cron and caddy start. - .env.dev/.env.staging/.env.production chmod'd 600 (were 644) — same plaintext-credential content as secrets/<env>/, which is already 700/644 at the directory/file level respectively for a different reason (container UID readability); these have no such constraint, only the host CLI reads them. Also removed a stray .env.staging.compose left over from before the Makefile fix above existed. Noted the convention in .env.example so newly created env files follow it too. |
||
|
twooey
|
1020a496ab |
Fix seven medium-severity bugs from the full-session code review
- SupplierOffer::insert(): now checks $wpdb->insert()'s return value and
throws, matching Work/Edition/Isbn (the one Catalog class that hadn't
been hardened this way). Work::set_product_id() gets the same treatment
(was flagged low-severity but same fix, bundled here) — a real update
failure now counts as a per-work sync failure instead of silently
leaving a stale wc_product_id pointer.
- SupplierOffer: fetched_at was written via current_time('mysql') (site-
local) while expires_at (SyntheticOfferGenerator) is written in UTC, and
best_offer_for_isbns() compared against site-local time too — a mismatch
masked today only because dev's gmt_offset is 0. Switched both writer and
reader to current_time('mysql', true) (UTC). Verified the read path
still finds all active offers correctly under a simulated -5 (US
Eastern) offset, not just at offset 0.
- HardcoverAdapter: rate-limit throttling moved from "once per work" (in
Commands.php) to "once per actual HTTP request" (inside query() itself).
find_book()'s ISBN-then-title/author fallback can fire two real requests
per work — under the old scheme both shared one throttle sleep, roughly
doubling the real request rate against a beta API. query() also now
retries network errors/5xx/429 up to 3x (mirroring
OpenLibraryAdapter::get_with_retry()), while a GraphQL-level `errors` field
or other 4xx throws immediately (retrying a rejected query can't fix it).
Commands.php adds a 5-consecutive-failure circuit breaker so a bad token
or a wrong field in the still-unverified schema can't silently burn
through the whole catalog with zero progress. Verified all of this
directly against Hardcover's real API with a deliberately invalid token:
5 fast (non-retried) 401s, correct abort message, and confirmed the
failed works were NOT marked synced (so a real token can retry them).
- restore.sh: now drops and recreates the target database before restoring
the dump, and clears the uploads directory before extracting the
archive — previously both restored on top of existing state, so a stray
table or file NOT in the backup would silently survive a restore drill.
Verified end-to-end against a real local staging stack: planted a stray
table and a stray upload file after taking a backup, ran restore.sh, and
confirmed both were gone afterward while the actual backed-up data (20
works, a known upload file) came back correctly. Also fixed a real
permission gap hit during that same test: a fresh volume's uploads dir
is root-owned until something chowns it, which broke the new www-data
clear step — now clears as root and chowns to www-data afterward, which
also means restore self-heals the exact root-owned-uploads class of bug
fixed earlier this session for the cron sidecar.
- poll-deploy.sh: added a non-blocking flock so a deploy that runs longer
than the cron interval can't have a second poll fire mid-deploy and race
its git checkout/reset against the same live working tree. Verified: a
concurrent run correctly skips instantly while the lock is held, and
proceeds normally once it's released. (Full atomicity of the live PHP
file swap under real traffic is a bigger architectural question —
blue-green or symlinked releases — flagged to the user rather than
attempted here.)
- backup.sh: now also archives .env.<environment> itself (chmod 600) and
includes it in the off-host rclone sync alongside the DB dump and
uploads archive. Every API key and both DB passwords previously lived
only on the host in this one gitignored file — losing the host lost all
of it even with DB/uploads backups intact.
|
||
|
twooey
|
df34fe8e54 |
Fix five high-severity bugs from the full-session code review
- ProductSync: bsc_work.wc_product_id could go stale if a product was ever
deleted out-of-band (wp-admin, a cleanup script). wc_get_product() then
correctly detects "no product" and creates a new one, but the repoint was
gated behind `if (!$existing_id)` — which was already true, so the stale
ID never got corrected. Every future sync repeated this, one duplicate
product per run. Fixed by making the repoint unconditional (cheap,
idempotent UPDATE either way). Reproduced the exact scenario against dev
(deleted a product out-of-band, ran sync-products twice) and confirmed:
one repoint, zero duplicates, product count and per-work product count
both correct across repeated runs.
- Commands.php (sync-hardcover-tags): wp_set_object_terms() with an empty
array clears the taxonomy rather than leaving it alone — verified
directly. Hardcover legitimately returns no moods/content-warnings for
plenty of books, so a --force re-sync could silently wipe existing tags,
including anything hand-tagged. Fixed by skipping the call per-category
when that category's array is empty. Verified via a direct eval test:
pre-existing genre tag survives a sync where genre comes back empty,
mood still gets set normally.
- docker-compose.yml: two stray root-owned secrets/db_password and
secrets/db_root_password directories were already sitting on disk —
Docker auto-creating a bind-mount source as a directory from an earlier
manual `docker compose` call that ran without ENVIRONMENT exported
(reproducing the exact bug already fixed once this session). Removed
the stray dirs and changed every `${ENVIRONMENT}` in a volume mount to
`${ENVIRONMENT:?ENVIRONMENT must be set}` so Compose now hard-fails
instead of silently defaulting to empty. Verified: unset ENVIRONMENT now
fails config validation with a clear error; set, it still works.
- HARDCOVER_API_TOKEN moved to the same _FILE secrets pattern already used
for DB_PASSWORD/DB_ROOT_PASSWORD (docker-compose.yml, deploy.sh,
HardcoverAdapter.php) — it was a live, consumed secret still going
through Compose's ${VAR} interpolation, exposed to the same
mangling bug already fixed for the DB passwords, plus visible via
`docker inspect`. deploy.sh now writes secrets/<env>/hardcover_api_token
(optionally empty). Verified via deploy.sh dev + wp eval: empty file ->
is_configured() false, a real token value -> true.
- backup.sh: mariadb-dump had no --single-transaction, so a dump against a
live site would either table-lock for its duration or produce a
non-atomic/inconsistent dump. Added; verified a real dump still runs
clean and produces a valid, restorable-looking .sql.gz.
|
||
|
twooey
|
05e97b53f6 |
Harden sync_all() for future live-site use: per-row savepoints, scoped cache invalidation
Ahead of running this against a live site (not just staging bulk-imports), addressed the two risks that only matter under concurrent traffic: - One bad product no longer wastes a whole batch: each product save is wrapped in its own SAVEPOINT within the batch transaction, so a failure rolls back just that row (counted in the new failed[] result, surfaced via WP_CLI::warning like import-gutenberg already does) instead of discarding up to 200 already-good rows. Verified SAVEPOINT/ROLLBACK TO SAVEPOINT actually works on this MariaDB instance via a direct $wpdb test before relying on it. - Cache invalidation suspension is now scoped per-batch instead of the whole run, with clean_post_cache() called explicitly per product once each batch commits, replacing the single end-of-run wp_cache_flush(). This site runs a shared Redis object cache across PHP-FPM workers, so a whole-run suspension + full flush would let a concurrent visitor see stale cached product data for the run's duration, and the flush itself would evict sessions and everything else site-wide. Scoping bounds staleness to one batch and targets only the products actually touched. Also moved term resolution (term_exists()/wp_insert_term()) out of the per-product savepoint entirely: warm_term_cache() now resolves every distinct subject for a batch before its transaction opens, so a term created for one product can never get silently rolled back by a *different* product's savepoint failure while a stale term_id sits cached in memory and gets attached to a later product (an orphaned term_relationships row, checked for directly post-fix: 0 found). Verified after: 2000-item sync still ~90s (savepoints add no measurable overhead), 0 lookup-table mismatches, 0 term-count mismatches, 0 orphaned term_relationships, idempotent re-run with 0 duplicates. |
||
|
twooey
|
9bca215a7b |
Speed up bulk product sync: defer term counting, suspend cache invalidation, batch transactions
Measured 2000-product sync_all() at ~2.5 products/sec baseline (2-core/8GB staging box). Confirmed via source inspection that WooCommerce's wc_product_meta_lookup sync is synchronous inside WC_Product::save(), so every product save was its own implicit autocommit transaction -> its own fsync. Batching each 200-item chunk into one explicit transaction, deferring product_cat term counting, suspending post cache invalidation for the duration, and caching subject->term_id lookups in memory brings the same 2000-item sync to ~21 products/sec (~95s), with the update-path re-run at ~82/sec. Verified correctness after, not just speed: wc_product_meta_lookup price/ stock matched postmeta exactly (0 mismatches across all 2000), product_cat term counts matched actual term_relationships (0 mismatches), no orphaned category assignments, no duplicate products on re-run. |
||
 twooeyandClaude Sonnet 5
|
dcad1b7512 |
Product cover images via Open Library; Gutenberg literature filter; cron root fix
Cover sync (OpenLibraryAdapter + CoverSync + sync-covers command): fetches a real cover from Open Library's free, keyless, explicitly-licensed-for- this-use Covers API and attaches it as a genuine Media Library attachment (not a hotlinked <img> — WooCommerce's shop loop/gallery/structured data all need a real _thumbnail_id). ISBN-first, title/author-search fallback via the confirmed cover_i field, matching the pattern already established in HardcoverAdapter. Two real bugs found and fixed while verifying this against the actual catalog, not assumed: - A Range-header HEAD-equivalent probe (added to avoid double-fetching) caused Open Library's server to redirect with a misleading content-type, producing a false positive on a known-fake ISBN. Removed — fetch once, verify the real bytes. - Their search endpoint has genuine transient failures under repeated querying (same request, same input, failed then succeeded seconds later) — added retry-with-backoff on network errors/5xx, matching how the rest of this codebase already treats transient failures as retriable rather than fatal. Also found chasing what looked like a third cover-sync bug, but wasn't one: uploads/2026/08 was owned by root, silently blocking www-data-run wp-cli from writing new files. Root cause was a gap in the earlier root-hardening pass (docker-compose.yml, Commands.php) — it fixed our own deploy.sh/backup.sh/Makefile invocations but missed the cron sidecar's own internal process, which was still running its wp-cli loop as root via a leftover --allow-root. Fixed at the container level (`user: www-data` on the cron service) since it's a plain shell loop with none of php-fpm's master-process-needs-root-to-drop-privileges concern. Gutenberg importer: filters to actual literature via LoCC (Library of Congress Classification) — verified directly against the real catalog that novels consistently get a P* code while government documents/ speeches/law get E/JK/KF/DA and never a P code. Removes the Declaration of Independence, Bill of Rights, etc. from what was importing as "books." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
71b8065f20 |
AO3-style tag browsing: taxonomies, display, Hardcover sync (schema unverified)
Four flat taxonomies attached to product (bsc_genre, bsc_mood, bsc_content_warning, bsc_tag) reuse WordPress's native taxonomy archive system for the "click a tag, see everything with it" browsing AO3 is known for — no custom archive templates or query logic needed. Verified: all four register correctly, render as clickable chips on the product page (content warnings get a distinct notice instead of just another chip), and the archive page + term count both work end-to-end with real test data. HardcoverAdapter + `wp bookstore sync-hardcover-tags` pull genre/mood/ content-warning/freeform tags from Hardcover's API to populate these. This part is explicitly NOT verified against a live response — Hardcover's API is in beta with informal docs, and there was no API token available to confirm the exact query/response shape. Flagged clearly in the adapter itself; needs a real token + introspection query before trusting the field-parsing logic in production. ISBN-first-then-title/author matching handles both real future ISBNs and the current Gutenberg-synthetic catalog's fake ones. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
6299391f23 |
Run wp-cli as www-data instead of root; drop --allow-root
--allow-root was routing around wp-cli's own safety check rather than addressing why root was there in the first place: docker exec defaults to the container's root user because the image never sets a non-root user for exec sessions — it was never about the actual web-facing attack surface, which already runs as www-data (verified: php-fpm's worker processes, the ones executing plugin/theme code for real requests, run as uid 33, not root; only our own deliberate admin commands were root). wp-config.php and the rest of wp-core are already owned by www-data (the official entrypoint sets this up), so there's no actual reason for our own commands to run as anything else. Verified: a full clean deploy and a bookstore-core wp-cli command both work identically running as www-data, no functional change, just removed an unnecessary privilege. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
a782f880bc |
Week 2: catalog schema, Gutenberg importer, pricing engine, membership shipping
Builds bookstore-core's first real business logic (design doc §02-§05,§07), replacing the plugin stub with: - Schema: bsc_work/edition/isbn/cover_variant/supplier_offer/member tables via dbDelta, with a versioned upgrade path. - GutenbergImporter: imports Project Gutenberg's live bulk catalog as synthetic staging data. Deterministic checksum-valid ISBN-13s (there are no real ISBNs in PG data) keep re-imports idempotent. - SyntheticOfferGenerator: the GutenbergTestAdapter of §04 — realistic condition/stock/price distribution with deliberate out-of-stock/stale/ discontinued cases for later exception-path testing. - PricingEngine: the §05 margin formula, verified against the design doc's own worked example ($6.00 + $3.99 -> $14.99 exactly). - ProductSync: one WooCommerce product per Work, never per ISBN (§03), idempotent (verified: re-sync of 2000 works produces 0 duplicates). - BSC_Free_Shipping: a real WC_Shipping_Method (WooCommerce's built-in Free Shipping only supports one static threshold, not membership- conditional), verified through actual browser-session cart flows in both directions, not just unit calls. Found and fixed one real data-integrity bug while scale-testing: PG's Authors field lists every contributor for anthology works, overflowing primary_author's column — Work::insert() was failing silently (Catalog insert methods never checked $wpdb->insert()'s return value), while the following Edition/Isbn inserts for that same row went ahead anyway and attached to whichever work_id was last successful. Fixed at the root: inserts now throw on failure, the importer takes just the first author (matching the column's actual semantic intent), and the import loop catches per-row failures so one bad title can't abort a bulk run. Verified end-to-end against a real 2000-title import: exact work/edition/ isbn/product count parity, no duplicates on re-run, real HTTP cart tests for both shipping thresholds, live storefront search. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
ea144d0298 |
Fix staging HTTPS: force tls internal, require a hostname not a bare IP
Two real, verified findings from actually testing the "let Caddy issue a self-signed cert automatically" approach against a literal IP: 1. TLS SNI is not sent for literal IP connections (out of spec — SNI's HostName type explicitly excludes IPs). Confirmed via openssl s_client: the handshake fails with a TLS-layer internal_error, even though Caddy logs "certificate obtained successfully" — Caddy has no way to select a cert without SNI. No Caddyfile config fixes this; the address has to be a hostname. 2. Caddy's automatic-HTTPS heuristic only treats bare IPs and "localhost" as obviously-private. Any other name — including a made-up LAN hostname like "bookstore-staging.test" — it assumes might be real and tries Let's Encrypt, which fails the same way staging.example.com did. Fixed by forcing `tls internal` explicitly in Caddyfile.staging, removing the guesswork entirely. Verified end-to-end with curl --resolve (proper SNI, no real DNS/hosts change needed to test): direct HTTPS 200, HTTP->HTTPS redirect chain 200. README now documents the /etc/hosts requirement and how to trust Caddy's root cert to skip the one-time browser warning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
f65581bc50 |
Permanently eliminate the "$X variable not set" warning, and fix Caddy auto-HTTPS on private IPs
Two separate leaks were causing the same cosmetic-but-annoying warning to
survive every previous fix attempt:
1. Compose's own `secrets:` block reads the referenced file's *content*
as part of its config model, and applies the same interpolation
warning to it — even with DB_PASSWORD fully removed from every ${VAR}
and --env-file path. Switched from Compose's native `secrets:` to plain
bind mounts at the same /run/secrets/* paths: a bind mount only ever
touches the file's path, never its content, so it's immune. (Verified
this precisely with an isolated repro before rolling it out — the two
mechanisms behave differently even though they look equivalent.)
2. caddy's `env_file: .env.staging` (a leftover from the since-removed
basic-auth setup) loaded the *raw*, unfiltered env file directly,
bypassing deploy.sh/backup.sh/restore.sh's filtered-copy mechanism
entirely. Caddy only ever needed SITE_DOMAIN; switched to passing that
one value directly instead of the whole file.
Also fixed Caddyfile.staging: the site address had no explicit scheme, so
Caddy's automatic-HTTPS logic still applied to a private IP (registering
its own internal CA and redirecting HTTP->HTTPS) — not the "plain HTTP
only" behavior I'd assumed and told the user earlier. Prefixed with
`http://` to genuinely disable automatic HTTPS for this LAN-only box.
Verified end-to-end: fresh deploy with dollar-sign DB passwords produces
zero warnings, serves HTTP 200 on plain http:// with no redirect, and the
DB connection genuinely authenticates.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
twooeyandClaude Sonnet 5
|
0c41d9f70b |
Fix wordpress-readiness race in deploy.sh
The bring-up wait only checked that php-cli runs, not that the official image had finished copying WordPress's core files into the (possibly freshly-emptied) wp_core volume — a step that takes a few real seconds on first boot. On a fast pass through that window, wp-cli would run against an empty /var/www/html and fail with "This does not seem to be a WordPress installation," even though the container was otherwise healthy. Now waits on wp-settings.php actually existing instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
3876a6ba30 |
Fix DB password corruption via Docker secrets file mechanism
The reported bug (a generated password containing "$BRjTx3tlmSpz" got
silently blanked, breaking the DB connection) is the same Compose
interpolation issue as the earlier bcrypt hash, but this time in fields
that are genuinely user-chosen and can't just be avoided by convention.
Switched DB_PASSWORD/DB_ROOT_PASSWORD to Docker's official `_FILE`
secrets convention (MARIADB_PASSWORD_FILE / WORDPRESS_DB_PASSWORD_FILE),
backed by Compose's native `secrets:` mechanism: deploy.sh writes the raw
value to secrets/<env>/db_password, and the container reads that file
directly — the value never passes through Compose's ${VAR} interpolation
at all. Verified end-to-end with an actual `$`-containing password,
including a full deploy → backup → restore → still-serving round trip.
Also fixed along the way (found while actually testing, not assumed):
- Makefile never exported ENVIRONMENT, so `make up` alone (bypassing
deploy.sh) would have left the new secrets path unresolved.
- deploy.sh chmod'd the secret files 600, unreadable by the container's
own UID (www-data) — fixed to 644, relying on the containing directory
(700) to keep other host users out instead.
- backup.sh/restore.sh still called `mysqldump`/`mysql`, which don't
exist in the mariadb:11 image under those names — renamed to
mariadb-dump/mariadb. (This means neither script had actually
succeeded before now; both are verified working end-to-end here.)
The supplier/payment API keys remain passed the old way — nothing reads
them yet (bookstore-core is still a stub), so there's no live bug to fix
there; noted in .env.example that the same _FILE pattern should be used
once that code exists.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
twooeyandClaude Sonnet 5
|
cd48f85a47 |
Remove staging basic-auth wall — box is LAN-only
Staging runs on a private-network VM, not reachable from outside, so the HTTP basic-auth layer was unnecessary defense-in-depth. Removing it also lets the Caddyfile drop the render-around-Compose workaround entirely (that workaround existed specifically because Compose's interpolation mangles a bcrypt hash) — the noindex header and blog_public=0 stay, since those guard against search-engine indexing, a separate concern from network-level access. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
twooeyandClaude Sonnet 5
|
b43852e733 |
Switch deploy pipeline to Gitea polling; fix env-parsing and volume-isolation bugs
- Replace the bare-repo post-receive hook with deploy/poll-deploy.sh: Gitea
and the Docker hosts are separate machines, so each box polls its branch
via host crontab instead of needing an exposed webhook receiver.
- Add Blocksy theme + Blocksy Companion auto-install to deploy.sh (free
tier; the paid Book Store starter site still needs a manual license step).
- Fix deploy.sh/backup.sh/restore.sh sourcing .env files as bash: a bcrypt
hash's `$2a$14$...` shape breaks under `set -u`. Replaced with
deploy/lib/env.sh, a literal (non-executing) KEY=VALUE reader.
- Fix docker compose itself mangling the same kind of value: both
`environment: ${VAR}` and `env_file:` run values through Compose's
interpolation, which silently blanks `$identifier`-shaped substrings.
The staging basic-auth hash is now rendered directly into the Caddyfile
by deploy.sh, bypassing Compose's variable system entirely.
- Fix dev/staging/production silently sharing one Compose project (and
therefore one db_data volume) by pinning an explicit -p per environment.
- cron and wordpress now share one environment anchor so they can't drift
apart again (cron was silently missing WORDPRESS_CONFIG_EXTRA before).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
twooeyandClaude Sonnet 5
|
c9d637c907 |
Week 1 infrastructure: Docker environments, deploy pipeline, bookstore-core scaffold
Docker Compose environments for dev/staging/production (MariaDB, Redis, Caddy, Action Scheduler cron sidecar), an idempotent deploy script, git-hook-based deploy pipeline, backup/restore scripts, and the bookstore-core plugin stub with WooCommerce HPOS compatibility declared. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |