Skip to content

fix(publish): serialize exact state mutations#722

Merged
brokemac79 merged 2 commits into
mainfrom
codex/csw-049-state-writer-collision
Jul 20, 2026
Merged

fix(publish): serialize exact state mutations#722
brokemac79 merged 2 commits into
mainfrom
codex/csw-049-state-writer-collision

Conversation

@brokemac79

@brokemac79 brokemac79 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • serialize the exact-result state mutation with a short remote lease
  • atomically renew/fence that lease in the same push that updates state
  • classify lease contention as a bounded transient publication failure

Problem and causal proof

Exact review generation and some GitHub publication calls still complete, but the queue is not draining because many exact publishers independently mutate the single state ref. A representative run on current main, 29750012187, spent 12.8 minutes in result publication, repeatedly lost state pushes, and performed repeated 13–41 second state fetches before eventually succeeding. Comparable observed runs scaled from 2.8 minutes with no rejection to 5.6/10.6/20.6 minutes with 5/11/19 rejections. These were non-fast-forward/CAS losses, not GitHub authentication failures; the affected logs contained no 401/403 signature.

At 2026-07-20T15:49:33Z the live publication lane still had 323 pending, 318 ready, 39 of 40 leases occupied, an oldest item age of 4h06m, and net drain of -88/hour over 15 minutes and -60/hour over 60 minutes.

The concurrency originated when #614 parallelized exact publication and #641 added adaptive capacity. #711 and #712 made losing state writers perform more recovery work, while #713 stopped those state-fetch timeouts from lowering global capacity. #714, #716, #720, and #721 repaired object/history recovery, but they do not prevent the 40 admitted exact publishers from colliding on the same ref. Thus #712 amplified/exposed this failure path; it did not create the underlying parallel-writer topology.

Implementation

  • Acquire refs/heads/clawsweeper-publish-lease/state before resetting/applying the exact record tuple.
  • Update the exact tuple and renew the lease in one atomic two-ref push. The lease CAS fences an expired or stolen owner at the same server transaction that updates state, so there is no check/push gap.
  • Release after the mutation; keep remote completion verification outside the critical section. Abandoned owners expire after two minutes, and acquisition is bounded to three minutes.
  • Return state_contention through the existing transient publication retry budget (12 attempts/24 hours) without reporting a GitHub failure kind or reducing global capacity.

The lock covers only the exact tuple mutation. It does not serialize review generation, GitHub comment/label calls, routing, or broad state publishers.

Validation

  • Crabbox Docker focused proof, local-container / node:24-bookworm, lease cbx_80097db415d4 (exit 0):
    • all 51 test/repair/git-publish.test.ts cases passed
    • eight simultaneously released exact writers each landed their tuple with one state push and no lost race
    • an owner held beyond its original TTL renewed and fenced the state update atomically
    • abandoned-owner recovery passed
    • exact workflow wiring and state_contention queue-budget tests passed
    • all three TypeScript builds, Oxlint, and Oxfmt passed
  • Crabbox Docker aggregate proof, local-container / node:24-bookworm, lease cbx_b793a71f332b (exit 0): pnpm run check passed, including all unit/repair suites, both coverage gates, builds, lint, formatting, active-surface, dashboard-boundary, and limits checks. The disposable proof environment normalized host-synced CRLF text and supplied checksum-verified jq; neither adjustment is in this patch.
  • Native focused proof: pnpm run build:all; the lease, queue, and workflow tests; Oxlint; and git diff --check passed. Native aggregate failures were unrelated Windows-host baseline constraints (Linux/Bash integrations, symlink privilege, and POSIX filesystem fixtures), so the authoritative aggregate proof ran in Crabbox Linux.
  • actionlint is not installed locally; workflow source assertions, YAML formatting, the aggregate Linux suite, and normal CI cover the one completion-reason allowlist edit.

CI timing follow-up

The first #722 pnpm check failed only two pre-existing action-ledger timing assertions: dead-owner reclamation took 3.64 seconds and Linux zombie-owner reclamation took 1.25 seconds against hard-coded one-second limits. The same tests passed on the immediately preceding main run in 361 ms and 66 ms. #722 does not change action-ledger runtime behavior; the limits originated in #511, while #709 had already widened comparable host-load-sensitive tests.

The test-only follow-up widens all four prompt-reclamation guards from one second to five seconds. This remains below the production lock-wait fallback of ten seconds, so the tests still distinguish prompt dead-owner reclamation from the fallback path.

  • pnpm run build:all: passed.
  • node --test --test-name-pattern 'producer locks reclaim fresh dead owners|Linux stale-lock checks bypass cached identities|Linux producer locks reclaim zombie owners' test/action-ledger-runtime.test.ts: one applicable Windows test passed in 1.475 seconds; two Linux-only tests skipped on Windows as designed.
  • pnpm exec oxfmt --check test/action-ledger-runtime.test.ts: passed.
  • git diff --check: passed.
  • A second Crabbox attempt for this test-only follow-up was explicitly waived after an unprivileged Corepack harness setup failure; the earlier exact-head focused and aggregate Crabbox proofs above remain valid for the production patch.

Review closeout

  • codex review --uncommitted first identified that a generic reason would use the poison-item budget. Accepted: added state_contention, its transient queue classification, and regression proof.
  • A repeated codex review --uncommitted identified expiry between ownership validation and a slow push. Accepted: changed renewal plus state update to an atomic fenced push and added post-expiry proof.
  • Final production-patch codex review --uncommitted: no actionable defects; lease, atomic publish, and retry-classification paths internally consistent.
  • Final production-patch codex review --base origin/main: no actionable correctness issues; focused build, lease, queue, and workflow tests passed.
  • Test-only CI follow-up codex review --uncommitted: clean, no accepted/actionable findings.
  • Test-only CI follow-up codex review --base origin/main: clean, no accepted/actionable findings; state-publish lease fencing and retry classification remain consistent across publisher, queue, and workflow contracts.

Risk and rollout

  • Pre-merge exact publishers do not know about the lease, so isolated collisions can continue until those executions drain. Post-merge executions serialize only the short state mutation.
  • A crashed publisher may leave the lease ref temporarily present; bounded TTL recovery is tested. Release uses compare-and-swap and cannot delete a newer owner's lease.
  • The atomic push deliberately fails closed if lease ownership changes. That item is retried as state_contention rather than dead-lettered early or fed into GitHub pressure control.
  • No queue capacity, gate, state reset, replay, direct Actions dispatch, token scope, or GitHub API behavior changes.
  • The CI follow-up changes test timing tolerance only; it does not change production lock behavior.

Credit

The remote-lease design builds on Vincent Koc's earlier state-publication serialization work in b72380fa711171e6d8cc0025ce8285472f25230e. It is narrowed here to exact result mutation and strengthened with atomic fencing. Peter Steinberger's recovery work in #711, #712, #714, #716, and #721 remains intact.

@brokemac79
brokemac79 requested a review from a team as a code owner July 20, 2026 15:50
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

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