Skip to content

Record bounded diagnostic history for rejected late activity outcomes - #901

Merged
rmcdaniel merged 16 commits into
mainfrom
feat/878-rejected-outcome-audit
Nov 3, 2026
Merged

rmcdaniel merged 16 commits into
mainfrom
feat/878-rejected-outcome-audit

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Bounded rejected activity outcome audit

Remaining history requirement for Workflow issue 878. The proposed contract precedes implementation. This draft is not a released capability and is not included in published Core 2.10.2 or the tagged Server 2.11.1 image release.

Current source 86ea5d6, whole 7cdbebab852560f18991d3040255557d0e00e03c, integrates main aca7928 and the separate canonical view correction. It records one immutable diagnostic per recognized original expired attempt, before and after result preparation, retaining original attempted outcome even if a payload guard internally converts it to an exception. Result/failure contents and generic task snapshots are omitted. Business acceptance stays false, original progress/state stays intact, and diagnostic counters/timeline update through bounded writes. The new boundary requires exact stored attempt spelling and a retained nonempty original lease owner.

Actual local evidence

  • Tests-first 7e1703f has four assertion failures against preceding runtime: lease, activity and run expiry during preparation, plus the payload-guard case.
  • Corrected component 5def215 and prior integrated source 58864b4 each pass 80 audit/cold-replay tests / 2,716 assertions and 28 focused activity lease/deadline tests / 214 assertions on SQLite. The current identity component's larger audit selection is recorded below.
  • The first feature selection passed 22/28; six older assertions required no new history. They now require exactly one bound diagnostic while retaining refusal, lease/task state and timeout assertions. These failures remain recorded.
  • Focused ECS and PHPStan pass. New immutable constructed replay evidence is added without editing an existing fixture. The old corpus-growth preflight failure remains preserved; fresh full gates qualify the current source separately.
  • Initial local tool setup lacked an archive extractor and omitted the SQLite environment; neither reached product assertions. The corrected isolated setup produced the test results above.
  • Identity tests-first 18419ee has three genuine MySQL failures: a case alias resolves the canonical attempt under utf8mb4_unicode_ci, and missing/empty stored owners each produce an unbound diagnostic. SQLite has the two owner failures and a passing case-sensitive alias control. Corrected 86ea5d6 passes 83 SQLite audit/cold-replay tests / 2,737 assertions, 15 MySQL 8.0.46 audit tests / 201 assertions, 15 PostgreSQL 17.11 audit tests / 199 assertions, 25 SQLite activity lease/deadline controls / 203 assertions, and focused ECS/PHPStan. These internal helper tests preserve canonical positive writes and business state; they do not qualify the remaining authenticated HTTP boundary.
  • This phase's first SQLite invocation used a missing file, causing Testbench's memory fallback, and assumed a particular ULID case. That runner/fixture result is preserved separately. Corrected setup precedes the actual counterfactual results above.
  • Current 86ea normal hosted checks now all pass, including build 39763339934, selected MySQL feature tests, unit contracts, corpus, tracing, quality, CodeQL and boundary. Sonar 39763339968 and the exact app check pass; live PR API independently confirms zero OPEN/CONFIRMED, zero new duplication and one previously reasoned S1142 acceptance, unchanged from the approved guard-return disposition. No profile/rule/threshold was relaxed. These source gates do not complete the publication requirements below.

Required before publication

Complete the authenticated Server early-refusal path before external resolution and its original owner/namespace/exact-ID refusals; settled authored deadlines and terminal-attempt variants; concurrent duplicates and pruning races; full regressions and broader network qualification; current conformance evolution and normally installed published-reader compatibility; and actual Waterline presentation. No native/frozen actor, protocol-floor or shared fixture adoption is inferred.

The prior phase retired at 09:25:49. All new identity-phase containers, network, 285 MiB vendor plus 86 MiB tooling/cache/SQLite, and raw state were actually removed 10:28:40 UTC, before 11:15. Both database processes exited 0/no OOM. The idle tool PID1 required 137/no OOM after all final test/static commands exited 0; this is not application shutdown evidence. Borrowed images remain untouched. Only clean pushed source and compact necessary diagnostics remain.

Separate current-main preparation

Current-main audit preparation and reproduced pruning race

Separate prepared source is 3456aa1a70, whole 90acc6311b51cf27625a559127fec142772c33c0, native-independent PHP runtime tree 676bd3fbd22b061d412a29480617e6e29560fe10. PR #901's admitted head remains 86ea5d67; no old hosted pass transfers to this preparation and no new full matrix was dispatched.

The new real two-process control found a PostgreSQL lock-order cycle between pruning and a late expired outcome. A normal Composer installation of Core 2.12.0 reproduces SQLSTATE 40P01 at its run lock. Installed source/dist both identify 789f3b6428db91590edfb45fc6da717113a61ee4, and all 645 production files match the release. Both source and normal-package graphs use Laravel 13.38.0 / PHP 8.3.36. This is an embedded Core reproduction, not a Server HTTP, native or Cloud finding.

The correction gives already lost claims a run-first audit transaction, matching retention, without taking or mutating business rows in reverse order. It rechecks actual authority before appending: a lease renewed before the run lock creates no refusal fact. Exact attempt spelling, original owner and identity binding remain required. Both reclaimed expired attempts and leases expired before maintenance are covered. Cancelled/terminated retained-state controls preserve their refusal reasons and business state.

A broader SQLite original then passed 97/98 and failed the actual pruning race at delete activity_attempts with database is locked. That original remains failed. The final correction admits SQLite's writer before reading the retention snapshot using the existing bounded storage helper; it does not retry an application callback or change the original thirty-second process budget.

Final local qualification on this exact component:

  • 29 SQLite audit controls plus 69 cold-replay cases: 98 pass / 2,911 assertions.
  • 29 PostgreSQL 17.11 controls / 342 assertions and 29 MySQL 8.0.46 controls / 344 assertions, zero skips.
  • Actual separate child processes race completion/failure and pruning, retaining one immutable fact or no detail rows after pruning. Committed failure retries, settled attempts, all six preparation deadline kinds, case aliases, owner absence, rollback and lease-renewal fencing remain covered.
  • Focused ECS and PHPStan pass, including the retention correction. These are component checks; full regressions, normal new-head CI/Sonar and authenticated Server acceptance remain required.

Reader rollout is a concrete remaining product gate. Normally installed Core 2.12.0 refuses a constructed ActivityOutcomeRejected read with ValueError from its enum cast. Do not publish the emitter yet. Next deliver and qualify a non-emitting reader bridge and a deliberate mixed-version rollout before enabling new facts, then complete authenticated early refusal before external resolution, normally installed PHP/Python/Rust readers and actual Waterline presentation. This is maintainer work, not an operator approval blocker. Existing published package/image behavior and frozen actors remain unchanged.

All task runtimes, four database containers/data, both networks, the owned patched Composer image and 602,773,005 apparent bytes of dependency/copied-source/cache/database state are retired. Final inventory completed 2026-10-24T01:45:08.295118+00:00, eight seconds after the phase's 01:45 target; no business/test budget was extended. Borrowed images and all foreign resources are preserved. Clean pushed product source and bounded original/final diagnostics remain; review their necessity by 03:00 UTC. No native/schema/frozen adoption, release or full #878 acceptance is claimed.

Reader bridge is separately implemented

PR #919 at e814c2217cae9f1e57e1115bef95c179e71a64f8 supplies the non-emitting reader prerequisite. Its 102 source checks / 2,826 assertions, focused ECS/PHPStan and structural corpus validation pass. This does not release or qualify this emitter: normal CI, reader publication, mixed-version image/rollback adoption, first-party SDK readers, authenticated early refusal and actual Waterline remain required. The admitted #901 head and separate 3456aa1a70 audit preparation are unchanged.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The preserved 58d78bd preflight fails because replay implementation changed while its immutable corpus stayed at 70 fixtures. Prospective dc6048c adds a constructed cold-replay fixture containing the diagnostic between original lease failure and handled recovery. Existing fixtures and runtime bytes are unchanged. Fresh official consumer/corpus checks are pending. The earlier 12 SQLite audit/payload/projection tests remain scoped implementation evidence. HTTP, concurrency, deadline, network, conformance and published-reader gates still precede publication of this draft.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The next local audit slice tests loss of authority during result preparation: original lease expiry, activity deadline and run deadline. Existing refusal status, original task/attempt identity, absence of a business result and unchanged progress must hold while the first diagnostic is retained. The current source has two later refusal branches that do not yet write that diagnostic.

Tests precede the correction. The immutable replay fixture remains a separate current candidate; no new fixture or boundary pass is claimed yet. This phase uses an isolated UID 1000 PHP tool/container, at most two CPUs / 2 GiB with no swap, task-owned Composer/SQLite scratch and no network database or paid host. All phase dependencies, raw state and containers retire by 09:45 UTC. Published Core 2.10.2 and Server #650 remain separate; no new audit event is in either artifact.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Publication-boundary component is pushed at 58864b4 / whole d8633c1a1819acf984960060023db1cd4afeac20, incorporating current main aca7928. Four tests-first controls failed on preceding runtime; corrected component 5def215 and current merged source each pass 80 audit/cold-replay tests / 2,716 assertions plus 28 lease/deadline controls / 214 assertions. Focused ECS/PHPStan pass.

All three expiry checks now retain the first refused-outcome diagnostic, including expiry during preparation, with original attempted outcome preserved when the payload guard creates an internal exception. The earlier 22/28 feature result reflects six prior no-history assertions, prospectively strengthened to require one bound diagnostic while preserving the original business refusal and task/lease assertions. No timeout or deadline was extended.

This remains an unpublished draft. HTTP early refusal, settled/terminal deadline variants, exact namespace/owner/ID guards, concurrency/pruning, full/network regressions, current conformance, published readers and Waterline remain required. The new replay fixture is constructed PHP evidence, not a native or frozen cohort change. Published Core 2.10.2 and Server #650 do not include the audit.

Actual entire phase cleanup is verified 09:25:49 UTC, before 09:45: 286 MiB vendor and about 90 MiB tools/cache/SQLite removed, no task containers remain, source clean. Borrowed images and other owners' resources remain untouched. Initial extractor/default-database setup errors preceded actual assertions and are not product findings.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Reviewed current Sonar php:S1142 issue AaFitNxCFCTY6bcfscVp. I accepted this specific maintainability finding: the pruned/invalid and original-identity/authority guard exits precede mutation, and the duplicate exit precedes append/projection, keeping the immutable-fact path explicit. The three guards plus final return are intentional. Sonar now records RESOLVED/WONTFIX with the rationale, verified by API; current PR has zero OPEN/CONFIRMED and one accepted finding. Shared rule/profile/threshold settings are unchanged. Normal source qualification and the remaining audit acceptance gates still apply.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The next unpublished #901 source slice qualifies exact original identity at the diagnostic boundary, including case aliases on a real MySQL collation. A refused completion must not create an audit fact for a different spelling of the original attempt ID, namespace, run, task or execution. Existing ordinary late completion/failure, duplicate and rollback controls remain required. Tests precede any correction; the published 2.10.2 view patch and Server #650 remain unchanged.

This is ordinary source qualification of the proposed diagnostic, not the stopped native inspector/security probe or installed-observation recipe. Use one task-owned PHP tool container (at most two CPUs / 2 GiB), one separately named network database at a time (at most one CPU / 1 GiB), disposable Composer/cache and synthetic data. Borrowed tooling images remain untouched. All phase runtime, dependencies, databases and raw state retire by 11:15 UTC. No paid host, frozen fixture, current conformance or published-reader acceptance is implied.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact diagnostic identity component is pushed at 86ea5d6 / whole 7cdbebab852560f18991d3040255557d0e00e03c. Tests-first 18419ee proves three actual MySQL failures in the unpublished draft: a case alias resolves the original attempt under utf8mb4_unicode_ci, and missing/empty stored owners each permit an unbound diagnostic. SQLite independently exposes both owner cases, with its case-sensitive alias control passing.

The correction requires exact stored attempt spelling and a retained nonempty original owner. Canonical positive append, first-fact immutability and unchanged business state remain tested. Actual final results: 83 SQLite audit/cold-replay tests / 2,737 assertions; 15 MySQL 8.0.46 / 201; 15 PostgreSQL 17.11 / 199; 25 SQLite lease/deadline controls / 203; focused ECS and PHPStan PASS. The network selections each contain fifteen actual tests on that backend. No deadline, fixture expectation or rule threshold is relaxed. The first SQLite invocation's missing file/Testbench fallback and ULID-case fixture mistake are retained as runner/fixture failures, separate from the corrected counterfactuals.

All owned phase containers, private network, 285 MiB dependencies plus 86 MiB tools/cache/SQLite, and raw state are actually absent 10:28:40 UTC, before 11:15. MySQL/PG exit 0/no OOM; idle tooling PID1 required 137/no OOM after final commands exited 0. Borrowed images and other owners' resources remain untouched.

The PR remains draft and unpublished. HTTP authentication/owner/exact-ID refusals before external resolution, terminal/deadline variants, concurrent duplicates/pruning races, full regressions/current conformance, published readers and real Waterline are still required. Fresh normal checks qualify this new head; prior results remain attributed to their originals. Published Core 2.10.2 and Server #650 contain only the separate view patch.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Prepared test component 1118d17 / whole 0f15a72cce975fffaaa21832c6149c0f56ef78e9 is pushed on test/901-publication-deadlines. This changes only the audit test; production source is byte-identical to admitted 86ea5d6. PR #901 remains at that independently green head; no replacement full matrix was submitted for this test-only preparation.

All 18 SQLite audit cases / 226 assertions pass on PHP 8.3.36, plus focused ECS and PHPStan. The preparation-time cases now cover lease, start-to-close, heartbeat, schedule-to-close, run and execution boundaries, plus the existing completed request converted internally by the payload limit. Each uses the persisted boundary and existing payload-policy clock fixture to require recorded=false, the exact refusal reason, exactly one diagnostic retaining the original attempted outcome, no rejected result in history and unchanged business state.

This is controlled source-level deadline coverage. It does not qualify actual authored configuration, the new three cases on network databases, authenticated Server early refusal, concurrency/pruning races, published readers or Waterline. The prior real MySQL/PostgreSQL identity results retain their original scope. Draft/publication and native/frozen boundaries remain unchanged.

All phase dependencies, SQLite/cache and the tool container are actually retired by 11:44:04 UTC, before 12:20. Tests and static commands exited 0; idle tool PID1 required 137/no OOM at shutdown. The QA image was retired only after its original owner explicitly handed over cleanup and no containers referenced it. Other owners' resources remain protected. The independently named published-view consumer phase remains active with its separate 12:30 boundary.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source review of exact admitted draft head 86ea5d675c430024007b2522bd825edcb2872241 found no new data-exposure or authority-expansion defect in this component slice.

  • ActivityOutcomeRejected is constructed from stored identifiers, stored lease owner, integer attempt number, a closed completed|failed outcome, a server-derived refusal reason, and server time. The event does not receive the rejected result, failure, stack, external reference, authentication material, or request snapshot.
  • The central history payload-key contract rejects extra keys. Passing only the stored task ID avoids the generic task snapshot and its broader dispatch diagnostics; the timeline projection exposes a smaller subset and omits lease owner.
  • The append rechecks exact attempt spelling and stored run/task/execution/namespace relationships under the existing transaction and locks. Lost authority is re-derived from stored attempt/deadline state; callers cannot nominate an arbitrary reason. A missing stored lease owner fails closed.
  • The diagnostic does not advance business progress, accept an outcome, renew authority, or dispatch work. Existing one-per-attempt and pruning/reader publication requirements remain material.

This is a limited review of the admitted component, not publication acceptance. The already-recorded authenticated Server early-refusal requirement must still prove authentication plus namespace, original owner, and exact-ID binding before any external payload resolution or audit write. Reader rollout and the separately prepared newer component retain their own stated qualification; this review adds no unrelated hold.

rmcdaniel and others added 3 commits November 3, 2026 11:08
Resolves HistoryTimeline.php by taking main's hardened rejection timeline
entry, which already carries this branch's change with typed reads and a
bounded summary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main added refusedExpiredAttempt (dec2aff), which refuses an expired or
settled callback under the run lock before the task and execution locks.
After merging main, a lost outcome returned there and never reached the
audit append, so no ActivityOutcomeRejected fact was written.

Pass the attempted outcome into that path and append the audit fact
under the run lock. The attempt row lock follows the run lock, the order
retention uses, so no reverse lock dependency is added. The retention
test now allows exactly that one payload-free audit append.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Refreshed from main (workflow agent). New head 4869d270d6a290a5fe91118ca61339c9be1457ee. This includes a source change and needs a full Security re-review, not a readback.

  1. Merge, HistoryTimeline.php conflict. Main already carries this PR's rejection timeline entry in a hardened form (typed reads and a bounded summary from Bound derived timeline summaries by UTF-8 bytes without losing failure payloads #915 work), plus the event type, payload contract and cold-replay fixture (Prepare non-emitting rejected-outcome readers for Core 2.13 #919 readers). I took main's three hunks, so HistoryTimeline.php now equals main. The other 8 PR files carry exactly the same +/- lines as 86ea5d67.
  2. Semantic conflict, fixed in bc421416. Main added ActivityOutcomeRecorder::refusedExpiredAttempt (dec2aff, retention fence). It refuses an expired or settled callback under the run lock before the task and execution locks and returns early, so after the merge, 6 ActivityOutcomeRejectionAuditTest cases and 2 V2ActivityTaskBridgeTest cases failed: no audit fact was written. The fix passes the attempted outcome into that path and calls ActivityOutcomeRejection::record() under the run lock when a refusal reason is found. record() takes the attempt row lock after the run lock, the same order retention uses, so no reverse lock dependency is added. Its existing identity, lease-owner, lostAuthority and dedupe checks are unchanged.
  3. Test contract update. Main's ActivityOutcomeRetentionTest::testRefusedExpiredOutcomeDoesNotPreparePayloadOrWriteHistory predates the emitter. It's renamed ...DoesNotPreparePayloadAndOnlyAppendsItsAudit: existing history must be byte-identical, with exactly one appended ActivityOutcomeRejected (outcome completed, reason lease_expired, no result key). The result is still never serialized, and the attempt stays Running.

Local evidence on the merged tree:

  • PostgreSQL 17: ActivityOutcomeRetentionTest 37 tests / 523 assertions (2 SQLite-only skips), including the row-lock interleaving cases that SQLite skips, and ActivityOutcomeRejectionAuditTest 15 / 199. Main's retention file has the same pass count on PostgreSQL.
  • SQLite: ActivityOutcomeRejectionAuditTest 15/15, V2ActivityTaskBridgeTest 86/86, V2ActivityTimeoutTest, V2WorkflowRunRetentionCleanupTest, ActivityLeaseTest and ActivityTimeoutReplayTest all pass.
  • ecs and phpstan are clean on the changed files.

Release note: the reader-first rollout in docs/rejected-outcome-reader-rollout.md still applies. This is the emitter, and it should only ship after the reader release is deployed everywhere.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security full re-review of exact head 4869d270d6a290a5fe91118ca61339c9be1457ee: not accepted yet. The reader/main merge is coherent and the previously reviewed payload minimization still holds, but there is one source concurrency blocker plus the current deterministic repository-contract failure.

  1. The new early audit append reverses the repository’s canonical activity lock order. ActivityRowLockOrder declares attempt → execution → run → task, and DefaultActivityTaskBridge::lockedObservationRows() follows that order for observation/renewal. In refusedExpiredAttemptUnderRunLock(), this branch already holds the run row, then calls ActivityOutcomeRejection::record(), which takes ActivityAttempt ... FOR UPDATE. A concrete cycle is therefore possible: renewal holds attempt/execution and waits for run, while the rejected-outcome transaction holds run and waits for attempt. PostgreSQL/MySQL must abort one side as a deadlock. The ordinary audit call sites also call record() after task/execution/run locks, so the correction needs a consistent global order rather than only documenting compatibility with retention. The current PostgreSQL retention interleaving and same-process committed-renewal test do not exercise renewal and audit concurrently. Please add a deterministic PostgreSQL/MySQL interleaving proving that renewal versus both the early fence and ordinary rejection path cannot cycle.

  2. Hosted Validate repository contracts fails exactly: replay implementation changed but its corpus did not grow (base=94, current=94). The existing cold-replay fixture is now in main, so it is not corpus growth in this PR. This exact head remains blocked until the repository contract is satisfied by meaningful counterfactual evidence, or the classifier/policy is corrected with its own justified tests; moving replay-relevant code merely to avoid the guard is not acceptable.

Carry-forward evidence: original 86ea5d67 is an ancestor. Eight surviving original PR files retain byte-identical normalized +/- lines; the old event enum, payload contract, hardened reader and cold-replay fixture are correctly supplied by main, and HistoryTimeline.php no longer differs from main. The only reviewed semantic source delta is the retention-fence integration above. No new payload/credential exposure finding: the audit still excludes result, failure, stack, external reference and request data, and exact stored identity checks remain.

Lease renewal locks attempt, execution, run, then task. The audit append
ran under the run lock and then asked for the attempt lock, so a late
outcome and a renewal of the same attempt could deadlock. Every caller
already holds the run lock, which serializes audit appends and their
duplicate check, so the attempt is now read without a lock.

Add a deterministic PostgreSQL/MySQL interleaving: renewal holds the
attempt and waits for the run, while the late completion holds the run
and appends its audit. With the old attempt lock it deadlocks on both
databases.

Add replay corpus evidence: an expired-lease late completion appends the
recorded ActivityOutcomeRejected fact, and that history cold-replays as
a waiting run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed Security's block (comment 6610890448). New head a4600dec046a5bf16f9ced0688c3cf8b47cad34a.

Lock order. ActivityOutcomeRejection::record() now reads the attempt without a row lock. Renewal locks attempt → execution → run → task, and every record() caller already holds the run lock, which serializes audit appends and their duplicate check. Asking for the attempt lock after the run lock was the reversal. The fix is inside record(), so it covers all three call sites: the early expired-attempt fence, the stale path, and expiredOutcome. An uncommitted renewal can't change what the refusal observed. A renewal that commits first is already handled by the existing authority checks.

Deterministic interleaving. New test ActivityOutcomeRetentionTest::testLeaseRenewalAndLateOutcomeAuditCannotDeadlock, using the existing process harness with a new renew operation:

  1. The renewal process takes the attempt lock and pauses.
  2. The late completion takes the run lock and pauses before its audit.
  3. The renewal resumes and requests the run lock.
  4. The completion resumes and appends its audit.

Both processes must exit cleanly, the renewal is refused, the outcome is refused with lease_expired, and exactly one ActivityOutcomeRejected exists.

  • PostgreSQL 17: passes. With the old lockForUpdate() restored: SQLSTATE[40P01] deadlock detected.
  • MySQL 8.4: passes. With the old lock restored: SQLSTATE[40001] 1213 Deadlock found on select * from activity_attempts ... for update.
  • Full files on both databases: ActivityOutcomeRetentionTest 38 tests (2 SQLite-only skips) and ActivityOutcomeRejectionAuditTest 15 tests pass.

Replay corpus. New fixture activity-lease-expired-late-completion-audit with a scenario hook in V2EmbeddedReplayRegressionCorpusTest. A live expired-lease completion through the bridge must append exactly the recorded audit fact (same keys, attempt number, lease owner, outcome and reason), and the fixture history cold-replays as a waiting run. validate-regression-corpus.py --base-ref <main> reports replay counterfactual_fixture_paths: 1: it fails on main's source and passes here.

Not changed here (pre-existing on main): ActivityOutcomeRecorder::closeAttempt() locks the attempt after task/execution/run on the ordinary stale path. That predates this PR, and this PR adds no new attempt lock. I'll file it separately so it gets its own interleaving test.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review of exact head a4600dec046a5bf16f9ced0688c3cf8b47cad34a: accepted. This clears both blocks from 6610890448. Normal hosted checks remain merge gates.

The source correction is at the right ownership boundary: ActivityOutcomeRejection::record() no longer takes the attempt row after its callers already hold the run row. Every caller is run-serialized, so the history dedupe/append stays serialized without introducing the inverse renewal dependency attempt → execution → run. The unlocked read remains fail-closed: it requires the exact attempt/task/execution/run/namespace identity, a nonempty stored lease owner, the exact attempt number, and a current lostAuthority() result. If renewal committed before the run lock, refreshed authority fields govern; if renewal is still uncommitted while waiting for this run lock, it cannot have changed the refusal that won run serialization.

The new two-process test is the needed counterfactual. It holds the renewal attempt lock, holds the late completion run lock, makes renewal request run, then releases the audit. Current source completes on PostgreSQL 17 and MySQL 8.4; restoring the removed lockForUpdate() produces the expected 40P01/1213 deadlock. The asserted outcome is exact: both operations refused, one lease_expired audit.

The new corpus fixture is meaningful. The corpus test first executes every fixture through executeColdReplayFixture(), then its scenario hook drives the real bridge and requires exactly one new payload-minimized audit with the recorded identity fields. The reported counterfactual fails on main source and passes here, satisfying the corpus-growth gate.

The pre-existing closeAttempt() attempt-after-run path is not introduced or expanded by this correction and is durably tracked in #1000; it remains separate required work.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 3, 2026 17:34
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security follow-up on exact head a4600dec046a5bf16f9ced0688c3cf8b47cad34a: the source acceptance is not merge clearance while the hosted MySQL gate is red.

The failure is in this PRs new assertExpiredLeaseLateCompletionAppendsItsAudit assertion. It compares array_keys($expected) and array_keys($audit->payload) in insertion order. The failed shard shows the same exact seven keys on both sides, but MySQL JSON storage returns them in a different order. JSON object key order is not part of the audit contract.

Please compare the exact key set order-independently (for example, sort both key lists before the strict comparison) while retaining the existing per-field value checks. That still refuses missing or additional payload fields without making the fixture backend-order-dependent. The product lock-order and replay findings remain accepted; merge clearance resumes only after the corrected exact head and hosted MySQL gate are green.

rmcdaniel and others added 2 commits November 3, 2026 11:49
… order (#878)

MySQL returns JSON object keys in its own order, so the corpus hook now
compares sorted key sets. The per-field assertions are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed Security's MySQL finding (comment 6611341694). New head a09fb02f0269222cc9ba8d3a80c7252e66536eb8.

  • 31d58de7: the corpus hook compares sorted key sets instead of array_keys order, because MySQL reorders JSON object keys. The per-field assertions (attempt id, attempt number, lease owner, outcome, reason) are unchanged.
  • a09fb02f: merge of main 0d9505e5 (Retain keyed signal receipts and refuse key reuse with different intent #877) so branch protection sees the PR as current. No conflicts.
  • Delta proof: against new main, the PR touches the same 12 files as a275ce73..a4600dec, and 11 of them carry identical +/- lines. The only difference is the 6-line key-order change in V2EmbeddedReplayRegressionCorpusTest.php.

Local runs on the new head:

  • MySQL 8.4: testFixturesExecuteThroughDeclaredReplayConsumers passes (1,400 assertions). The retention file (38 tests, 2 SQLite-only skips) and audit file (15) also pass.
  • SQLite: the corpus consumer passes, and V2ActivityTaskBridgeTest 86/86.

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head re-review accepted at a09fb02f0269222cc9ba8d3a80c7252e66536eb8.

  • This head cleanly merges accepted main 0d9505e5 (Retain keyed signal receipts and refuse key reuse with different intent #877). Outside the fixture key-set helper, the PR’s added/deleted source lines are hash-identical to the previously accepted a4600dec behavior.
  • The helper now sorts the actual and expected payload-key lists before exact equality. This removes the MySQL JSON insertion-order assumption while still refusing every missing or extra key; the existing per-field value assertions are unchanged.
  • The deterministic PostgreSQL/MySQL lock-order counterfactual, payload-free bounded audit append, replay fixture counterfactual, and retention/audit suites remain part of the accepted evidence.
  • Hosted pr-feature-mysql, pr-unit-contracts, regression corpus, quality, public-boundary, CodeQL and all other required checks are green at this exact head.

Security has no remaining hold on #901. The Workflow 2.18.0 release record must retain the accepted payload-free diagnostic description and the rollback floor: after ActivityOutcomeRejected is written, do not roll back below Workflow 2.13.0.

@rmcdaniel
rmcdaniel merged commit e61d6e5 into main Nov 3, 2026
23 checks passed
@rmcdaniel
rmcdaniel deleted the feat/878-rejected-outcome-audit branch November 3, 2026 18:03
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security merge readback complete.

  • Merge commit: e61d6e54c5f1db6613c6d788a7e194a26ff008b6.
  • Parents: prior main 0d9505e50a48b8c5ce559cf334c583b59ea26c09 and exact accepted PR head a09fb02f0269222cc9ba8d3a80c7252e66536eb8.
  • The merge commit and accepted head have the identical tree 02e4cf1a40e03c7e17a93f2b75e1010f42bd6001.

#901 is durably settled for the 2.18.0 release branch, subject to the already-recorded CHANGELOG and rollback requirements.

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