Skip to content

fix(publish): coordinate every state branch writer#724

Merged
brokemac79 merged 1 commit into
mainfrom
codex/csw-049-global-state-lease
Jul 20, 2026
Merged

fix(publish): coordinate every state branch writer#724
brokemac79 merged 1 commit into
mainfrom
codex/csw-049-global-state-lease

Conversation

@brokemac79

Copy link
Copy Markdown
Contributor

Summary

  • coordinate every mutation of the shared state branch with the existing state-publish lease
  • serialize ordinary status/comment/action-ledger writers with exact-result publishers
  • clean an acquired lease when a publisher is externally terminated, without shortening the existing lease TTL or acquisition budget

Problem

Exact-result publishers and ordinary state writers advance the same state branch. The exact publisher lease introduced in the recent publication repairs only fenced exact publishers; comment-router, action-ledger, sweep-status, and apply-status writes could still advance state outside that lease.

In production after #723, exact publishers successfully acquired and renewed the lease, then repeatedly lost their expected state SHA while those ordinary writers advanced state. New publishers queued behind the lease holder, timed out, and filled publication capacity. The sampled failures were not GitHub authentication failures: no 401/403 signature was present.

Causal proof

  • post-fix(publish): recover exact state races without full reset #723 publication run 29769864078 acquired and renewed the state-publish lease around 2026-07-20 19:00:29Z
  • during the held lease, comment/router/action-ledger/status commits advanced the same state ref
  • the exact publisher's atomic push then lost its expected state SHA, while later publishers waited behind that holder
  • a deterministic multi-process reproduction on pristine origin/main showed an ordinary state writer did not wait for an exact publisher lease
  • the same reproduction with this patch showed the ordinary writer waiting and both state updates surviving
  • a terminated synchronous lease owner left its lease behind on pristine origin/main; with this patch the watchdog removed that exact owner OID in about 220 ms

PR #712's synchronous recovery/history-fetch work amplified the later capacity stall, but it did not introduce the shared-writer race. PR #711's broad no-common-base replacement path remains a separate correctness risk and is intentionally not changed here.

Implementation

  • wrap shared state mutations in withStatePublishMutationLease
  • reuse a matching active lease when the exact publisher already owns it; otherwise acquire the lease for ordinary writers
  • route direct immutable action-ledger commits through the same atomic commit-push helper
  • hold the lease across GitHub's immutable-ledger server merge path
  • start a detached cleanup watchdog for acquired leases; it tracks the owner's possible lease OIDs and CAS-deletes only a matching OID when the owner process disappears
  • keep preparation and temporary immutable-branch work outside the shared-state critical section
  • preserve the existing 120-second lease TTL, renewal behavior, and 180-second acquisition budget

Validation

Crabbox Docker proof

Provider: local-container using Docker-backed Crabbox and Node 24.

  • A/B causal proof, lease cbx_8326f7aa23ea (amber-crayfish): pristine origin/main failed all three causal cases; the patch passed ordinary-writer serialization, terminated-owner cleanup, and immutable server-merge coordination
  • focused watchdog proof, lease cbx_da0a05d5fee7 (amber-prawn): main retained the terminated owner's lease after about five seconds; the patch removed it in about 220 ms
  • complete publication suite, lease cbx_af0df260d8b5 (jade-hermit): pnpm run build:repair, pnpm run lint:repair, and all 54 test/repair/git-publish.test.ts tests passed (79.5 seconds)

Local focused proof

  • pnpm run build:repair
  • pnpm run lint:repair
  • pnpm run lint:scripts
  • focused ordinary-writer serialization test
  • pnpm exec oxfmt --check src/repair/git-publish.ts test/repair/git-publish.test.ts
  • git diff --check

Baseline noise

An aggregate Crabbox pnpm run check completed all builds and all four lint groups, then hit unrelated CRLF fixtures copied from the Windows worktree into Linux (pipefail\r / $'\r') and downstream sweep-workflow assertions. This patch does not touch those scripts or line-ending policy. The authoritative changed publication suite is clean: 54/54 tests on Linux Crabbox.

The Linux-only immutable server-merge test also has a pre-existing native-Windows includeIf gitdir path failure on pristine main; the Docker-backed Linux proof passes.

Risks and rollout

  • shared state-ref mutations are serialized; repository preparation remains concurrent
  • the watchdog cannot delete a successor's lease because cleanup is conditional on a tracked owner OID
  • no capacity, retry, gate, queue, dashboard, or workflow-dispatch behavior changes
  • production recovery should be judged only from post-merge/default-branch publishers and at least two consecutive samples with declining pending depth and positive net drain

Attribution and related work

This builds on Peter's state correctness and reconciliation work in #714 and #716, and on the publication lease work in #722 and #723. It preserves those repairs while closing the remaining unfenced-writer gap. Related incident work: #711, #712, #713, #715, #720, #721, #722, and #723.

Codex review closeout

  • dirty-patch review: codex review --uncommitted
  • accepted findings: make the multi-process barrier observe real child output, remove an unused test variable, and clean leases after externally terminated synchronous Git work
  • proof was rerun after each accepted fix
  • final dirty-patch result: No actionable change-induced defects were found. Targeted state publish lease tests passed.
  • committed branch review: codex review --base origin/main
  • committed branch result: The diff consistently fences state-branch mutations with the lease and adds targeted coverage for ordinary writers and terminated owners. No actionable correctness issues were identified.

@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