Skip to content

Keep committed payload references before summary projection - #835

Merged
rmcdaniel merged 1 commit into
mainfrom
fix/789-final-claim-visibility
Oct 28, 2026
Merged

rmcdaniel merged 1 commit into
mainfrom
fix/789-final-claim-visibility

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

A workflow start commits its run before dispatch publishes its summary. Payload cleanup used the summary to decide which runs still owned payloads, so it could delete an input object during that gap even though a committed run referenced it.

Use committed, unpruned primary runs as the reference authority. Strengthen the existing start-versus-retention test to hold dispatch after the outer commit, and verify that retained metadata from a pruned run does not prevent eventual deletion after the last live reference is pruned.

Refs #789. This repairs the source race seen in the PHP feature checks for #834; the broader installed ingress/recovery acceptance in #789 remains open.

Validation:

  • PHP 8.3.36 with the ordinary locked Core 2.15.4 package: both focused cases pass on MySQL, MariaDB, PostgreSQL and SQLite, 8 tests / 163 assertions, no skips or errors.
  • The same tests with the unchanged cleanup helper produce four missing-input failures, one on each engine; all four reclamation controls pass (8 tests / 135 assertions, zero errors or skips).
  • Removing only the pruned-run filter makes all four final-reference reclamation controls fail while the four start-wins cases pass (8 tests / 159 assertions, zero errors or skips).
  • Related feature coverage passes: 165 tests / 2,290 assertions, zero failures/errors/skips, six existing PHPUnit deprecations. This includes the full four-engine race class and retention, write claims, cleanup, storage, transport, backup holds and registration recovery. The SQL services use the feature workflow’s pinned images: MySQL 8.4.5, MariaDB 10.11.19 and PostgreSQL 16.15. The remaining cases use their normal SQLite configuration.
  • Pint passes for both changed files. Normal hosted head CI is complete: 15 successful checks and seven conditional skips, no pending or failed checks. Sonar reports zero open issues and an OK quality gate. Normal merge-group qualification remains required.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 28, 2026 07:02
@rmcdaniel

rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

Reviewed exact 9d29a6c87c65d07512b7edd748406205aa21fc8d. The cleanup authority now includes primary runs committed before summary projection. details_pruned_at IS NULL excludes metadata rows deliberately retained after detail pruning, and the existing deleted-run exclusion remains in place. The object-lock and read-committed protocol is unchanged.

Qualification with ordinary locked Core 2.15.4 (8af7a0c89be759dfd9eddeedc0f724bf4211a7cd), all installed package versions/source references matching the committed lock:

  • Both focused cases: 8 tests / 163 assertions across MySQL 8.4.5, MariaDB 10.11.19, PostgreSQL 16.15 and file-backed SQLite, no failures/errors/skips.
  • Original cleanup-helper counterfactual: all four forced start-wins cases lose the committed input object, while all four last-reference reclamation cases pass (8 / 135, four expected failures, zero errors/skips).
  • Removing only the pruned-run filter: all four start-wins cases pass and all four reclamation controls leave the obsolete object behind (8 / 159, four expected failures, zero errors/skips).
  • Full related race/retention/claims/cleanup/storage/transport/backup/registration coverage: 165 tests / 2,290 assertions, no failures/errors/skips, six existing PHPUnit deprecations. The race cases exercise the four real engines; other selected classes retain their normal SQLite configuration.
  • Pint passes both changed files; the diff is limited to those two files.

Task database services were healthy with no OOM at qualification completion. The local disposable SQL services, network and data are removed, with zero task resources remaining. Normal hosted head CI is complete: 15 successful checks and seven conditional skips, no pending or failed checks. Sonar API review reports zero open issues and an OK quality gate for this PR. #789 stays open for its broader installed SDK/Nexus ingress and recovery acceptance.

@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
@rmcdaniel
rmcdaniel removed this pull request from the merge queue due to a manual request Oct 28, 2026
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review at exact head 9d29a6c87c65d07512b7edd748406205aa21fc8d: no finding in the payload-retention authority scope.

The authoritative retained set now comes from committed primary workflow_runs, so a start is protected before its asynchronously published summary exists. details_pruned_at IS NULL and the existing current-deletion exclusion keep deliberately pruned/deleted runs from becoming permanent retention roots. The same subquery is used for primary payload columns and run-scoped child/reference rows, and the existing retained-reference check is repeated under each object lock, preserving the writer/cleanup race fence.

This change does not broaden payload read authority, namespace access, or storage credentials. Its failure mode is conservative retention rather than cross-tenant disclosure; the added final-reference case proves pruned metadata does not keep the object indefinitely. The four-engine commit-before-dispatch and final-reclamation evidence already recorded on this PR matches the changed boundary, and hosted head checks are terminal green.

This was an exact-source review; I did not duplicate the already completed PHP/database matrix. No security gate or follow-up is required from this review.

Merged via the queue into main with commit 536c41d Oct 28, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant