Skip to content

Bound schedule audit snapshots and clean up purged schedules - #1149

Merged
rmcdaniel merged 9 commits into
mainfrom
fix/873-schedule-audit-retention
Nov 14, 2026
Merged

rmcdaniel merged 9 commits into
mainfrom
fix/873-schedule-audit-retention

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 14, 2026 •

Copy link
Copy Markdown
Member

Schedule updates can grow audit storage indefinitely, and physically purged schedules leave audit rows behind. This adds a default limit of 1,000 snapshots, preserving the first and newest events, plus namespace-scoped maintenance for legacy backlogs and orphaned rows.

Occurrence receipts preserve duplicate-firing protection when trigger snapshots are removed. Audit append, receipt retention and each bounded compaction pass share the schedule lock and transaction. Application query filters cannot hide correctness metadata or physical owners. Malformed legacy occurrence metadata refuses the transaction, and rollback cannot discard populated receipts. Upgrades must stop old schedulers and lifecycle writers before compaction, or explicitly disable compaction with null until those processes stop.

The maintenance command reports physical remaining rows under the same lock, including rows hidden by application scopes. The independent-writer regression uses separate PDO sessions: a committed sibling sequence survives the failed insert/savepoint, and MySQL repeatable-read retries use a current locking read without changing the 32-attempt bound. SQLite instead proves that writer admission blocks the competing insert.

Validation: 32 focused retention cases / 193 assertions pass on SQLite and MariaDB, with local PCOV exercising every previously uncovered statement in the three affected files. ECS and full Stan pass. The scoped-report counterfactual fails with 0 reported versus 106 physical rows; the MAX-only retry counterfactual fails the independent MySQL writer case. Both corrections pass. The earlier full 51048657 cycle passed runtime tests and the repository coverage ratchet but failed asynchronous Codecov patch/project checks; that result remains NONPASS. Current ordinary CI and full dispatcher 43612689518 validate combined head bdbb2d04, including accepted main f9bcaf06. Normal integration is complete at 2b6ba0eb, whose tree 546a30a9 exactly equals tested bdbb2d04. All four actual-main workflows pass, including build 43615384277: 29 successful checks, four intended skips, and both Codecov statuses successful. The remote topic branch is absent. No package release is selected; #873 remains open for published-package availability and the schedules guide.

Accepted source implementation for #873. Published-package availability and the public schedules-guide update remain separate acceptance requirements, so the issue stays open.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security whole-source review for exact 03b553bcd41b20a1caeb065367bf389691aa37c3: source boundary PASS; whole-head qualification HOLD on the current collision-test failure.

The retention design is fail-closed at the relevant ownership and deduplication seams:

  • every append, occurrence-receipt write and bounded compaction runs beneath the exact schedule row lock and one storage transaction; the refreshed owner must retain the same primary key, namespace and public schedule ID;
  • a scheduled trigger records the canonical UTC occurrence under the internal schedule ID before its audit row can be deleted. Legacy rows are converted only from a bounded, strict timestamp shape; malformed or noncanonical evidence aborts the transaction and preserves the original row;
  • first/newest sequence retention never renumbers history. Large legacy backlogs delete at most 100 rows per pass, while new appends remain bounded by the configured limit;
  • the backfill path consults compact receipts before retained history. A recreated public schedule ID receives a new internal owner, so it cannot inherit the purged owner's firing authority;
  • orphan cleanup constrains the child rows to an exact namespace and deletes only when the internal schedule owner no longer exists. Soft-deleted API schedules remain owners. The command bounds a pass to 1–1000 rows and makes legacy NULL namespaces an explicit, mutually exclusive selection;
  • receipt-table rollback refuses while any compact authority remains. The documented rolling-upgrade procedure keeps compaction disabled until old readers stop, instead of treating missing receipt support as safe.

I read the complete eleven-file delta, including the migration, command, configured model seam, trigger fallback and all focused tests. The new 15-case retention suite is passing on PostgreSQL in the current run, in addition to the recorded focused MySQL evidence.

The PR is not integration-qualified yet. Current feature-postgresql (5) fails V2ScheduleTest::testRecordRetriesWhenStaleMaxSequenceCollidesWithUniqueIndex: expected sequences [1,2,3], actual [1,2]. The new outer schedule transaction correctly rolls the fixture's same-connection sibling insert back with the failed inner savepoint, so that fixture no longer represents an independently committed old-writer race. Preserve coverage of the still-supported retry branch with a genuinely separate committed writer/connection (or otherwise reconcile the intended compatibility behavior), then require fresh exact-head backend/full gates. Do not relabel the present run as PASS.

This review does not authorize publication, a cleanup execution, or closure of #873; public guide follow-through and the user-visible release remain separate.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-successor review for c7792b84fb82f705eb93268dc49eec68edc9e154: HOLD — one remaining source-boundary finding.

The new internal retention queries correctly bypass application global scopes, but the --schedule command's completion signal still does not. V2ScheduleAuditPruneCommand.php:61-65 reports retained_events through $schedule->historyEvents()->count(). That relation resolves the configured history model and reapplies its global scopes. A custom filter like the new HiddenScheduleHistoryForAuditRetention can therefore report zero or a partial count while physical audit rows remain.

This contradicts the documented operator loop (“repeat … stop when the retained count reaches the configured limit”) and the new guarantee that application filters cannot hide maintenance state. It can make an operator stop an idle legacy compaction early even though the physical history remains above the configured bound.

Please compute the reported count through the same unscoped, storage-connection-bound internal history query keyed by the exact schedule owner, and add a causal command control using a globally filtered configured history model that proves the returned retained count is the physical count after pruning.

I reviewed the remainder of the 03b553b → c7792b8 delta. The nested-savepoint retry plus current locking read, scope-independent owner/receipt/history queries, and genuine independent-writer test close the previous collision/visibility findings; no additional source finding there. Current-head hosted integration remains a separate gate.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-successor review: source PASS for 510486577fee487fca1c51aa313091502d62b9ff / tree cc573c74afcac8484fe4bd427c8b853c2ac83f6b, closing the c7792b8 retained-count HOLD.

The direct fix is 33ee724dce2dd0d1d8032e58384f81f57126f2a7. Its four changed blobs are byte-identical in the current merge head. The accepted-main composition introduces no conflict at this boundary.

The command now delegates exact-schedule compaction and reporting to ScheduleAuditRetention::pruneWithReport(). Deletion and the reported retained count occur inside one locked() transaction against the refreshed exact schedule owner. Both use the same private history() builder, which binds the locked schedule storage connection, calls newQueryWithoutScopes(), and constrains workflow_schedule_id to the internal owner key. Application model filters therefore cannot hide either the rows being pruned or the remaining physical count. The configured limit is reported from the same operation.

The new command control configures the deliberately invisible HiddenScheduleHistoryForAuditRetention, drives a 206-row legacy backlog through bounded 100/100/3 deletion passes, and requires physical retained counts 106/6/3 plus the final exact sequences. Against the predecessor, the relation-based count would be zero, so the control is causal for the reported defect. The documentation now accurately names the result as a physical count.

I found no further source/security issue in this correction. Current-head hosted CI and final integration remain required; this source PASS does not borrow the predecessor run or convert the currently active checks into acceptance.

@rmcdaniel

rmcdaniel commented Nov 14, 2026 •

Copy link
Copy Markdown
Member Author

The remaining scoped-count finding is fixed at 510486577fee487fca1c51aa313091502d62b9ff (report fix 33ee724d, then a normal merge of accepted main 59fc408d).

pruneWithReport() performs pruning and the unscoped physical count under the same schedule lock and transaction. The command uses that report. The new command regression configures a history model whose global scope hides every row: three fixed 100-row passes report deleted/remaining pairs (100,106), (100,6), (3,3), verified independently against physical rows and the retained first/latest sequences.

The unchanged prior command fails this regression at its first report: 0 instead of 106, with one assertion failure and no setup errors. The correction passes the complete retention class: 19 tests / 136 assertions. ECS has reached a fixed point and full Stan passes. The complete combined source is an exact disjoint union of 12 topic paths and 18 accepted-main paths, with no content conflicts.

The complete current schedule cohort also passes against MariaDB 10.5.29 using the MariaDB driver: 119 tests / 685 assertions, including independent committed-writer contention and the physical-count regression. Reproduction: vendor/bin/phpunit tests/Feature/V2/V2ScheduleAuditRetentionTest.php tests/Feature/V2/V2ScheduleTest.php with the normal MariaDB test database. An initial local database readiness/authentication setup failure occurred before PHPUnit and is retained separately.

Independent source/security rebind is PASS. Current ordinary checks and full dispatcher 43609700776 remain pending. The earlier PostgreSQL fixture failure, MySQL snapshot counterfactual and scoped-report failure retain their original scopes.

Owner review of the 17 open Sonar findings: all are maintainability CODE_SMELL advisories. The return-count/cognitive-complexity findings concern explicit argument refusals, the ordered prune/receipt transaction and staged-upgrade lookup paths; those branches keep refusal messages and authority boundaries visible. The regex findings concern the explicit ASCII decimal classes and complete timestamp grammar used for durable occurrence keys. I retain those forms to preserve the reviewed validation and transaction flow. No analyzer suppression, ignored path, threshold change or quality-gate override was added. Actual current checks and final integration are still required.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final exact-head integration disposition: PASS for 510486577fee487fca1c51aa313091502d62b9ff / tree cc573c74afcac8484fe4bd427c8b853c2ac83f6b.

The reviewed physical-retention fix remains unchanged: pruneWithReport() deletes and counts through the same selected-connection, unscoped, exact-owner history query while holding the existing schedule lock/transaction, and the causal hidden-model command covers the bounded 100/100/3 deletion sequence and the retained 106/6/3 counts.

The complete current-head check set is terminal with 24 SUCCESS / 6 intended SKIPPED / 0 failed / 0 pending. That includes repository contracts, central policy preflight, quality/static analysis, regression corpus, CodeQL, both public-boundary scans, Sonar analyses, unit/contract/tracing/MySQL selection, and all six PostgreSQL shards. Build run 43609680045 completed SUCCESS on this exact head.

This closes the bounded source/security and current-head integration review for normal repository handling. It does not select a release or assert any broader runtime/product acceptance beyond PR #1149.

@codecov

codecov Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.14%. Comparing base (f9bcaf0) to head (bdbb2d0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1149      +/-   ##
==========================================
+ Coverage   94.13%   94.14%   +0.01%     
==========================================
  Files         691      694       +3     
  Lines       90596    90835     +239     
==========================================
+ Hits        85283    85520     +237     
- Misses       5313     5315       +2     
Flag Coverage Δ
v2 94.14% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel

rmcdaniel commented Nov 14, 2026 •

Copy link
Copy Markdown
Member Author

Current integration HOLD: Codecov completed after the original 30-context snapshot and failed patch/project: 91.89% patch against 94.13%, with 21 uncovered changed lines. The full dispatcher and retained 93.91% repository ratchet remain PASS at their scopes. Focused refusal/maintenance tests and a fresh successor cycle are required; no threshold or gate waiver is selected.

Final source validation is complete at 510486577fee487fca1c51aa313091502d62b9ff, tree cc573c74afcac8484fe4bd427c8b853c2ac83f6b, based on accepted main 59fc408d.

  • Ordinary build 43609680045 and full dispatcher 43609700776 are terminal SUCCESS. The full dispatcher has 23 successful jobs and 5 intended skips. The current PR has 24 successful contexts and 6 intended skips, with no active or failed contexts.
  • Full covered unit: 6,365 tests / 84,866 assertions, 49 declared skips and 15 inherited PHPUnit mock notices. Unit coverage is 70,078 / 87,382 statements, 80.20%, above the 60% requirement. The ordinary unit run also retains the two previously classified PHPUnit deprecations in unchanged fixtures.
  • Complete six-shard feature inventories: MySQL 3,221 tests / 32,619 assertions; PostgreSQL 3,221 / 31,910. Both logs include the actual independent-session sequence race and filtered-model physical-count control. Current quality, corpus, MariaDB controls and both Laravel source-upgrade journeys pass.
  • Current local MariaDB 10.5.29 schedule cohort: 119 tests / 685 assertions, zero failures/errors/skips. Local ECS and full Stan pass. The scoped-count and MAX-snapshot causal failures remain retained; the superseded c779 full dispatcher was cancelled after the corrected combined-head run started and is not promoted to a full-cycle PASS.
  • Downloaded and verified the three artifact ZIP digests. The retained combined summary binds this exact head and reproduces 82,135 / 87,459 covered lines, 93.91% > 93.08%. Its 794-file source inventory and SHA256 match this source tree exactly.
Artifact ID ZIP SHA256
unit-coverage 14120053971 405f14686f23be9ecce9a96c74cef16b57e43950d1a33686ac2566450fe9c071
sonar-coverage 14120263711 9690f6501cb57500ca7d9406e9424a06b9100bac49438d625a9c6f4c2b6f6792
v2-coverage-summary 14119929322 732d26b4a21d277bcc11c95111fd22fa39fd0b76a83ea308f3f1ef91399d40d0

Exact source/security PASS, ordinary current-head integration PASS, and the causal controls / maintainability disposition remain at their stated scopes. Normal non-bypass integration and accepted-main verification follow. #873 remains open for published-package availability and the public schedules guide; this is source delivery only.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Correction to my earlier final integration disposition: integration is HOLD, not PASS, on exact 510486577fee487fca1c51aa313091502d62b9ff / tree cc573c74afcac8484fe4bd427c8b853c2ac83f6b.

The earlier comment relied on the then-current 30-context snapshot. Codecov completed asynchronously afterward, and the authoritative current check rollup now has two failures: codecov/patch and codecov/project. The reported patch result is 91.89% against the 94.13% target with 21 uncovered changed lines. That later required evidence supersedes the green-snapshot integration conclusion.

The separate full-dispatcher evidence remains valid at its narrower scope. Run 43609700776 is terminal SUCCESS on this exact head with 23 successful jobs and 5 intended skips; all six MySQL and PostgreSQL shards, MariaDB, both Laravel upgrade cells, quality, corpus, coverage and central-policy jobs passed. I independently matched the three GitHub artifact IDs and API SHA-256 digests, including unit-coverage 405f1468…, sonar-coverage 9690f650…, and v2-coverage-summary 732d26b4…. Those facts do not override the failed patch/project gates.

Do not use comment 6879821837 as merge/integration clearance. Require the narrow tests for the actual uncovered refusal/maintenance paths, a fresh exact-head source rebind if the commit changes, and all required current-head checks—including both Codecov contexts—terminal PASS. No threshold reduction, production substitution, merge, or published #873 acceptance is selected.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Coverage successor: bdbb2d04ed16781ecfda58f914f1ee146b1e69a4, tree 546a30a958b38e57c1923bb0b808a64725a04458.

The topic change from 51048657 is test-only: 13 additional cases for canonical environment limits and overflow, manual triggers, malformed legacy clock types/syntax/calendar dates, conflicting receipt ownership, cleanup bounds, invalid namespaces, missing or differently cased schedule IDs, and idempotent migration up/down behavior. Assertions require refused operations to preserve the original rows and receipts. No production code, predicate, retry bound, threshold or existing assertion changed.

The complete retention class passes: 32 tests / 193 assertions. A local PCOV run exercises every statement previously uncovered in the three affected files (the Clover input reports 23 statements; Codecov classified 21 changed lines). That closes the concrete local coverage gaps, rather than assuming another unchanged full upload will improve them. The prior failed Codecov checks remain NONPASS.

The successor also normally merges accepted main f9bcaf06 after #1148. Independent tree comparison proves the exact disjoint union of 12 topic paths and 9 accepted-main paths; all schedule production blobs remain identical to reviewed 51048657. Current ordinary CI, full dispatcher 43612689518, exact source rebind and both Codecov contexts remain required before integration. Publication and the public schedules guide remain outside this source-stage result.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security bounded source/test-composition review: PASS for exact bdbb2d04ed16781ecfda58f914f1ee146b1e69a4 / tree 546a30a958b38e57c1923bb0b808a64725a04458.

I independently verified:

  • direct topic successor 291258fcd9c7cb1dd2ab762af48368df88d94bb0 changes only tests/Feature/V2/V2ScheduleAuditRetentionTest.php (+182/-0); git diff --check is clean;
  • the added cases cover the reported uncovered refusal and maintenance seams: canonical/overflow limits, manual-trigger compaction without an invented occurrence receipt, unusable legacy clocks with row preservation, conflicting receipt ownership rollback, cleanup/namespace/schedule bounds, and occurrence-migration idempotence;
  • schedule-retention production, command, and migration blobs are byte-identical to reviewed 51048657;
  • git merge-tree --write-tree 291258fcd9c7cb1dd2ab762af48368df88d94bb0 f9bcaf060952f15cdc0b4c437dfe002e4c713ba6 reproduces 546a30a958b38e57c1923bb0b808a64725a04458, so the current head is the exact disjoint composition with accepted PR #1148.

This closes only the bounded source/test-composition review. Integration remains HOLD until fresh full dispatcher 43612689518 and both asynchronous Codecov patch/project checks are terminal PASS on this exact head. No threshold, production, release, or prior failed-check waiver is inferred.

@rmcdaniel

Copy link
Copy Markdown
Member Author

All current integration gates are clear at bdbb2d04ed16781ecfda58f914f1ee146b1e69a4, tree 546a30a958b38e57c1923bb0b808a64725a04458, based on accepted main f9bcaf06.

  • Ordinary CI and full dispatcher 43612689518 are terminal SUCCESS. Current PR checks are 26 SUCCESS / 6 intended SKIP / 0 failed or pending, including Codecov patch 100.00% against 94.13%, and Codecov project 94.14%, up 0.01% from the base.
  • Complete covered unit: 6,376 tests / 84,894 assertions, 49 declared skips and the 15 inherited mock notices. Unit coverage remains 80.20%, above 60%. All six MySQL shards pass 3,240 / 32,726; all six PostgreSQL shards pass 3,240 / 32,017. Both actual SQL logs include the independent-writer, physical-count and new legacy-refusal controls. Extra dashboard unit controls are excluded from those feature totals.
  • Local changed retention class passes on SQLite with PCOV and on MariaDB 10.5.29: 32 / 193. ECS and the exact current full Stan path set pass. No schedule production, retry bound, threshold or original assertion changed in the coverage successor.
  • Downloaded all three retained ZIPs and reproduced their API digests. The combined summary binds this exact head, matches the current 794-file source inventory and reports 82,158 / 87,457 = 93.94%, above 93.08%.
Artifact ID ZIP SHA256
unit-coverage 14121169610 d64891f0b18c699b6a67fbacf3b0864a2bc3c7250fb0ba6ffe30d656e6581aa9
sonar-coverage 14121880744 5c9d9b3904de7eca8cde320ba2a96afc70f53fa56842eb1c548ee77418d40e0c
v2-coverage-summary 14122090218 7700085841c13b660b1c9ef6859c48734f90495f3cf2f90338cc0830002ee8f1

The exact source/test-composition review passes, and its remaining full-dispatcher/Codecov conditions are now met. The preceding 51048657 Codecov failure remains NONPASS. Normal integration and accepted-main readback follow; no release is selected. #873 remains open for publication and the public schedules guide.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 14, 2026 16:23
@rmcdaniel
rmcdaniel merged commit 2b6ba0e into main Nov 14, 2026
60 checks passed

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security final exact-head integration disposition: PASS for bdbb2d04ed16781ecfda58f914f1ee146b1e69a4 / tree 546a30a958b38e57c1923bb0b808a64725a04458.

Independent current-head reconciliation:

  • ordinary PR run 43612674263: terminal SUCCESS;
  • required full dispatcher 43612689518: terminal SUCCESS on exact head, 23 SUCCESS / 5 intended SKIP / 0 failed or active;
  • codecov/patch check 132508204901: SUCCESS, 100.00% of diff hit versus 94.13% target;
  • codecov/project check 132508199519: SUCCESS, 94.14% (+0.01%) versus accepted base f9bcaf0;
  • coverage summary artifact 14122090218: API/archive SHA-256 7700085841c13b660b1c9ef6859c48734f90495f3cf2f90338cc0830002ee8f1, internally binds commit bdbb2d04..., 794 PHP source files, 82,158/87,457 covered lines = 93.94%, ratchet PASS over 93.08%;
  • unit coverage 14121169610: API/archive SHA-256 d64891f0b18c699b6a67fbacf3b0864a2bc3c7250fb0ba6ffe30d656e6581aa9;
  • Sonar coverage 14121880744: API/archive SHA-256 5c9d9b3904de7eca8cde320ba2a96afc70f53fa56842eb1c548ee77418d40e0c; both retained coverage XML payloads are byte-identical (420a833952b8a82e10826fae7a9a0f9312a91d6316e637c02508ee2855c438f6).

The earlier 51048657 Codecov failure remains correctly NONPASS and is not reused. Together with the bounded source/test-composition PASS in review 5905622118, this clears normal integration for this exact head. It does not select a release, published #873 acceptance, or any broader runtime effect.

@rmcdaniel
rmcdaniel deleted the fix/873-schedule-audit-retention branch November 14, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant