[CompleteDotTech]

Refs CompleteTech-LLC/complete.tech#17 (gap 6, retention mechanics). Targets main (#10, #11 and #12, which it was stacked on, are merged; main merged in, schema 7). Data-deletion code: please review slowly. Scanning stays disabled by default.

What changes (owner decisions of 2026-10-22)

• Explicit closure: the first time a job is seen terminal (completed, failed, failed_unknown, cancelled, expired, blocked) it gets closed_at; accepted-but-unverified requests close when they expire. A job that can still run is never closed or deleted; a reopened job's stamp is cleared.
• Findings cleared 12 months (366 days) after closure; the signed evidence stays.
• Signed authorization evidence (job row + linked private request in any post status) deleted at retain_until, fixed at closure, may only be extended (never shortened): 6 years after closure; r2 and earlier (YYYY-MM-DD-rN <= 2026-10-21-r2) keep their promised 3 years from acceptance; never < 3 years whatever the config; unparseable versions get the longer period.
• Doubtful data is kept: deletion needs real post-2020 dates, a stored date that has arrived and is no earlier than the date required today (derived under today's config, and >= 3 years after creation), a terminal state, no hold, and sane clocks. A stored date short of the required one is extended to it (never shortened), and the job is deleted when that later date arrives. At most 50 deletion attempts per run, shared by all deleting steps.
• Narrow holds (admin queue): one job, stated purpose (20-190 chars), mandatory expiry (<= 730 days), setter/releaser recorded; honoured by retention and tombstone cleanup; 3-day grace after expiry or release (release ends the hold now and keeps its purpose).
• Tombstones: identifier-only (job id, request id, scope hash, creation time, opaque identity, reason; no personal data) written before deletion; a restored job row is deleted again; a restored request post only by opaque identity and only if no live job uses that request/job id. Kept 120 days (> 90-day backup rotation).
• retain_until is readable per accepted version for the contract archive (issue 11).

Fail-closed behavior

Anything doubtful (bad/zero dates, missing base, stored date earlier than derived, hold present/damaged, deletion step failing) keeps the record and retries; contact fields are scrubbed before the link is dropped; a failed post deletion leaves the job row.

Tests

retention.php (new, ~70 assertions: closure, deadlines on both sides of each period, version rules, floors, zero/garbled dates, tampered dates, reopen, holds incl. grace/release/non-admin, tombstones incl. identity, recycled ids, trash status, bounded batches) plus the adapted integration.php retention test. Mutation-checked; independent review found a blocker (zero date => immediate deletion), 2 highs (tombstone vs recycled ids; trashed post survives), and mediums (hold vs tombstone, legacy versions, lock cost, hold lapse/audit); all fixed with tests. CI on the head: see checks.

Reviewer: please attack

Any input that deletes before the promised period or deletes a live job's records; hold bypasses; tombstone matching; what survives deletion (post meta, _ct_terms_snapshot, control evidence, accepted-copy status, nonces).

Not done / needs the owner

• Backup rotation, the contract archive's historical Git objects/clones (issue 11), mail systems, and unsigned caller leads (this plugin has no lead record; the current form saves nothing without a signature, and "lead intake distinct from signature" needs its own design).
• Jobs created before this change have no identity: tombstone recognition of a restored post needs it (their job rows are still recognised).
• Schema 6 is unreleased; its columns were amended during review.

Second independent review (after the first fixes)

Verified the blocker/high fixes and found further problems, all fixed in the last commit with tests: the three-year creation floor would have made every real r2 job undeletable (and 50 of them could stall all deletion); jobs that predate the identity were never recognised on restore (identities are now backfilled, adopting what the request carries; the duplicate-event path sets the meta); a hold with a zero expiry protected evidence but not findings; two loops could starve (doubtful rows are re-examined at most daily; the restored-post loop is one SQL join excluding live ids); years above 2200 are rejected; a re-hold keeps no stale release audit.

Fail-closed review round 2 (coordinator findings)

Fixed in this stack, each with tests that fail without the fix (mutation-checked):

• Per-request admission guard for the scan worker #10: activation requires egress_enforced: true (loader refuses otherwise); docs/README/example config state it; unverified nftables egress sketch added (not a claim of enforcement). In-flight bound is exactly 10 s from the grant (no grace). Authorization read from the DB, not the object cache. A completed result needs >= 1 admitted request. Failed schema upgrade: one error_log, 5-minute back-off, plugin 0.2.0. Special-purpose IP ranges listed explicitly (CGNAT, documentation, benchmarking, 3fff::/20, 6to4, NAT64, ...), tested independent of the interpreter's ipaddress.
• Verify bound acceptance evidence; accepted copy for the verified customer #11: bound evidence is required for every version (terms_policy, strict by default; legacy only for r2-or-earlier); the loader refuses to enable without an explicit entry for the served version. ct_scan_target_summary() is the one canonical summary and now includes limits and period.
• Exact-target control gate (DNS TXT / HTTPS file / recorded authority review) #12: control is required by the same policy (replaces the opt-in list).
• Retention mechanics: closure, per-version deadlines, holds, deletion tombstones #13: non-legacy retention floored at 6 years (words/null/0/1 mean 6); tombstone scope is a keyed MAC; deletion needs closed_at >= created_at and a sane clock; retention runs outside the queue lock (own lock, throttled, one short queue lock per deletion) so Stop never queues behind a batch.

Third review fixes (head after 36aba06)

• No "kept forever": raising evidence_retention_years/legacy_retention_years after closure used to make every closed job undeletable (stored date < derived). Now retain_until is extended to the required date (compare-and-set, under the queue lock) and the job is deleted when that date arrives; lowering the config never shortens it. A legacy acceptance dated > 1 day before its record's creation gets 3 full years from creation (later than the promise, and finite).
• Grace after release: releasing a hold sets hold_until to now (purpose and releaser kept), so the documented 3-day grace applies; releasing an already expired hold never lengthens it.
• Clocks: deletions stop if PHP and database UTC_TIMESTAMP() disagree by > 5 minutes (in addition to the backward-clock checks); the problem is logged once per occurrence.
• Budget: one budget of 50 deletion attempts per run shared by the evidence step and both tombstone loops; a refused restored-post deletion is counted and skipped.
• Recycled post ids: a restored job row's post is deleted only if it carries the tombstone's identity; a due job whose post carries another identity is kept whole.
• Retention exceptions are caught on the admin path; on /claim retention runs only when no job is handed out, so a lease is never delayed.
• Tests for each, plus a 29-February base date. Still owner decisions (unchanged): the legacy-version cut-off for unsuffixed/2026-10-21 versions and calendar validation of version strings.

Fourth review fixes (f2a33e6, 97f834d)

• Blocker: a second "place hold" could shorten/replace a standing hold. ct_scan_admin_set_hold now refuses (hold_active, explained in the admin notice) while an unreleased hold still protects the job (unexpired or in grace), and a released hold in grace can only be replaced by one that ends later. The write is a compare-and-set on the hold values read under the queue lock (hold_changed otherwise). To change a hold: release it (grace still protects), then place the new one. Refusal was chosen over extend-in-place because it needs no schema change and never mixes two purposes in one record.
• A job with no opaque identity (NULL or empty) is kept until the backfill gives it one, so no tombstone is written without an identity; the backfill also covers empty identities.
• Retention deletion requires the linked post to carry exactly the job's non-empty identity; an empty or missing post identity keeps everything.
• Tombstone loop: held and leased restored rows are excluded in SQL, and candidates are paged from a stored cursor (wrapping around), so skipped or repeatedly failing rows cannot starve later restored rows.
• Docs: hold replacement rule, identity requirement, wp_salt('secure_auth') rotation vs tombstone matching, and the far-future-timestamp halt (logged once).
• Tests: second hold (1/729/730 days, other admin) never changes expiry/purpose/setter; refused in grace; allowed after release with its own purpose/setter; damaged released-but-future hold not shortened; identity NULL/''/missing kept; empty post identity (missing/empty) kept whole; 56 held/leased restored rows do not starve a later one.

Fifth review fixes (2a62ccf)

• Blocker: a stale "Release hold" form could end a newer hold. The form now carries the hold's identity (hash of purpose, expiry, setter, set time); ct_scan_admin_release_hold is a compare-and-set on hold_reason, hold_until, hold_set_at, hold_set_by, hold_released_at IS NULL; a mismatch or 0 rows returns hold_changed ("reload the queue"). Test: release H1, place a 730-day H2, replay H1's release: H2 unchanged and the due job kept.
• Identity backfill adopts/writes a post's identity only if the post's _ct_scan_job_id names the job; otherwise both stay unchanged (job kept). Paged from a stored cursor.
• Restored-post loop: job-id match moved into SQL, paged by tombstone id from a stored cursor with wrap-around; the job-row loop leaves 1/5 of the shared budget for it. Test: 55 refused + 55 foreign newer posts do not starve an older one.
• Clock sanity check now runs before closure stamping (a wrong "now" is never stamped). Test.
• Retention years > 100 clamped (logged once) so dates stay below 2200; floors unchanged. Test.
• Test fixture ids moved to 887xxxxxx with insert assertions (CI no longer logs Duplicate entry).
• Owner-policy items unchanged: legacy cut-off for unsuffixed/2026-10-21 versions, calendar validation of version strings, backup rotation / archive / mail / unsigned leads, 6-year/3-year periods, 730-day hold cap and 3-day grace, 120-day tombstone retention.

Sixth review fixes (5e0e2af)

• Blocker: collation mismatch silently disabled restored-post deletion. The restored-post query now compares _ct_scan_identity / _ct_scan_job_id meta with the tombstone as CAST(... AS BINARY), so a tombstones table in another collation than wp_postmeta cannot raise error 1267. Audit: this was the only cross-table text comparison in the PR (the job-row loop joins on numeric ids and datetime; the scope MAC is compared in PHP).
• Every retention/tombstone query (clock check, reopen, identity backfill, retain_until, evidence, both tombstone loops including the live-row lookup, tombstone expiry) now checks $wpdb->last_error: a failure stops that step and keeps the records (never "no candidates" / "no live record"), logged once until a clean run.
• The tombstone loop deletes a recognised job row only once the deadline retention would apply today has passed (later of stored retain_until and the date required under current config). A live row whose deletion wrote its tombstone and then failed part-way is therefore protected by a period raised since. A row with no derivable deadline cannot be such a row (retention only deletes rows with valid dates, unchanged by a failed deletion), so the tombstone decides there. Documented in docs/WORDPRESS.md.
• Tests: identity column switched to a second collation of the post-meta charset (the plain comparison is asserted to fail first), restored post still deleted, column restored; broken candidate query and broken live lookup keep the post and log, recovery deletes it; tombstone written + deletion failed + period raised to 8 years: row kept, then completed under the original period. Restored-row fixtures now use real-looking acceptance dates and retention_checked_at (instead of a future retain_until) to reach only the tombstone loop.
• CI for 5e0e2af: all checks pass; the retention step logs only the two intended tombstone_posts warnings, no SQL errors.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T21:03:43.242006Z	5e0e2af	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.