fix(sync): never lose a PostgreSQL forward between the ref CAS and its push run by ThomasK33 · Pull Request #679 · coder/openstorage

ThomasK33 · 2026-11-05T17:54:15Z

Why

Every accepted write to a forward_on_push repository must reach the upstream, and no crash window may silently lose a forward. On PostgreSQL, accepted git pushes and REST writes could miss the upstream in four ways. Nothing polls the upstream, so a lost forward stayed lost until the next authorized write or push-upstream call. This PR fixes all four. SQLite and S3-WAL behave as before.

On PostgreSQL, a push whose COMMIT reply is lost reports failure and drops its push event, although the ref moved. Main has this bug too. The forward-intent hook needs the real outcome of its COMMIT, so this PR fixes it as well (item 5).

What was broken

Intent was recorded after the ref CAS. The update hook and REST writes committed the ref, then the sync poke (enqueueSyncOrForward) ran RequestPush and EnqueuePush as two more statements. A hook or serve process that died in between, or a failed RequestPush or enqueue, left the write with no push_pending and no River job. Nothing on PostgreSQL scans push_pending: River's rescuer and the push job's completion gate act only on a job that exists.
A River push run could snapshot a stale cache. The hook inserts the push job right after the CAS, before receive-pack publishes the cache ref. A push run on the same node could claim the job, hydrate the cache while the push's hold kept the old value, forward that, and end done. The claim consumed the intent. Only local-schedule runs waited for publication.
The completion gate completed a job over a held claim. A run whose FinishPush (or, with item 2, RestorePush) failed after its retries kept its claim and the request it had consumed, and the push job completed. Nothing re-ran it.
The poke could start a second forward run (found in review of the first fix). Once the CAS inserts the job, a run can claim it before the poke runs. The poke then marked the level again, and the job's completion scheduled another run with its own repo.push_upstream.* events.
A lost COMMIT reply read as a failure. On PostgreSQL, a push whose COMMIT reply is lost reports failure and drops its push event, although the ref moved. The cause is in pgx v5.11 (vendored): MultiResultReader.NextResult drops the read error of peekMessage, which has already closed the connection, so the next receiveMessage returns connLockError "conn closed", and pgconn's SafeToRetry is true for it. ref.IdempotentCAS took that as a request never sent and did not recheck the ref, although the casRef comment said that a lost COMMIT reply stays ambiguous. When the loss did read as ambiguous (for example, when the request context ends during COMMIT, pgx returns a context error), the recovered win never ran Committed, so the poke marked the level again and could start a second forward run (Codex review, round 2).

Fix

The CAS transaction records the intent. ref.WithCASTxHook runs InTx after a won swap statement and before COMMIT, and Committed after COMMIT succeeds. An InTx error rolls the swap back and is marked not executed, so ref.IdempotentCAS does not recover it. On PostgreSQL, serve and the hook build their events decorator with newPGEventsStore (cmd/openstorage/wire_eventing_pg.go). It sets the hook for every write that the poke forwards: refs outside the ephemeral namespace, by a caller that may forward, and not a sync run's own loopback push. The hook calls jobs.River.RecordForwardTx, which reads forward_on_push, inserts the same unique-by-repo upstream_push job as EnqueuePush, and then raises push_pending, all in the CAS's transaction. Metadata and refs share one PostgreSQL database, so the ref, the job and the pending level commit together or not at all.
The poke skips a forward that the committed CAS recorded. Committed marks a per-CAS record in the context, and the poke returns early for it. A win that IdempotentCAS recovered (another caller's identical write, or a COMMIT outcome that PostgreSQL could not confirm) still gets the poke.
River push runs wait for publication. buildServeEventing wires worker.WaitPublication on PostgreSQL as buildLocalSyncQueue does, and RunPush calls it after the claim on both lanes. Holds are per node, and the run's loopback fetch reads its own node's cache, so its own node's holds are the ones that matter. A failed wait hands the request back with RestorePush and fails the run, so the job snoozes. Under shutdown the run restores within the shutdown budget and returns ErrShutdown, so River keeps the job.
The push gate never completes a job while a push claim is held. Push jobs are unique by repo, so a held claim belongs to one of the job's own runs (or an older binary's) and holds a consumed request. The job snoozes until the lease ends, and its re-run steals the claim with its force level. A run for a deleted repository returns ErrRepoGone, and its job completes.
The CAS asks PostgreSQL for the outcome of a failed COMMIT. A PostgreSQL swap that changed a row reads txid_current() before COMMIT. When COMMIT fails, casRef returns its connection to the pool, so the status query also works in a pool of one, then asks txid_status with a detached context, and asks again while the transaction is in progress, for at most commitStatusTimeout (5 s). committed is a win, and Committed runs, so the poke skips the recorded forward. aborted is a definitive failure. When the status query fails, or the transaction is still in progress at the timeout, the error is marked ambiguous (commitUnknown, never SafeToRetry), and IdempotentCAS rechecks the ref as before. The vendored pgx is unchanged. Tests inject the faults with internal/testutil/pgfault, a TCP proxy that loses, drops or holds the next COMMIT.
River interplay. The vendored River v0.50.0 unique insert is ON CONFLICT (unique_key) ... DO UPDATE, so it row-locks a live job. The push worker's completion transaction updates that row first (JobCompleteTx). Either the CAS locks the job first, and the completion waits for the CAS commit, reads the new push_pending and inserts a successor. Or the completion commits first, and the CAS's insert joins its successor or adds a new job. The job is inserted before the upstream row is marked. The completion reads that row through the metadata pool, so a CAS must not hold the row while it waits for the job.

Rolling upgrade. No schema change, no new column, the same job kind and args. Old replicas keep their after-CAS poke, so their writes keep the old window until the fleet runs this build. Old and new workers run each other's jobs. Rollback is safe.

Cost per PostgreSQL write. Every PostgreSQL swap that changes a row runs one more statement, SELECT txid_current(). A hooked swap also reads forward_on_push by primary key. A failed COMMIT of such a swap adds txid_status queries for at most 5 s. For forward_on_push repositories, the job insert and the push_pending update move into the CAS transaction, and the poke's three statements no longer run. A CAS that meets a live job waits for that job's completion transaction. Push runs list the node's push-hold directory once.

Not covered here

Finding 2 of the verify-lane-schedule skill (a hold reserved by a stale-advertisement push after a run's wait passed) also applies to PostgreSQL. River runs now call the same wait and snapshot code as local runs. fix(sync): record git forward intent before the CAS and refuse stale held snapshots #678's strict held snapshot is not enough for them alone: fix(sync): record git forward intent before the CAS and refuse stale held snapshots #678 turns StrictSyncFetch on only for the local schedule and restores only when w.Local is set. A follow-up PR in this run turns it on for PostgreSQL and restores the run through RestorePush.
River's rescuer discards a push job after three crashed attempts (syncRescueAttempts), which can leave push_pending or a held claim with no job. A follow-up PR adds a periodic scan that gives such a lane a new job.
The pull lane's gate has the same gap as item 3 for a failed FinishSync, which includes the mirror-back pull after a forward. A follow-up PR applies item 3's rule to the sync gate.
A double network fault can still start one extra forward run. It needs a lost COMMIT reply, then a txid_status query that fails or still reads "in progress" after 5 s, then an IdempotentCAS recheck that finds the new value. Committed does not run then, so the poke marks push_pending again, and a run that claimed the job meanwhile is followed by a second run. That run pushes nothing new, but it emits duplicate repo.push_upstream.* events. Skipping the poke in this case could lose a forward instead, so the poke runs.

Tests

Each reproduction fails on the commit before its fix:

Test	Package	Fails on
`TestPGHookForwardBeforeCachePublication`	`internal/githttp`, PostgreSQL lane, real serve binary	`7242220`: the run finished while the push was parked (`PushRunCount:1 PushSuccessCount:1`), and the upstream main stayed empty
`TestPGHookForwardAfterLostCASOutcome`	`internal/githttp`, PostgreSQL lane	`7242220` and `8258fc8`: the hook's CAS committed, then `EnvCASDropHard` lost the outcome. `PushPending:0 PushRunCount:0`, upstream main empty
`TestServeRecordsForwardIntentWithTheCAS/may_forward`	`cmd/openstorage`	`1f4aa15`: a REST CAS whose process dies after the commit left 0 push jobs
`TestServeRecordsForwardIntentWithTheCAS/claimed_before_poke`	`cmd/openstorage`	`bbfaac5`: `PushPending:1` after a run claimed the new job
`TestPushRunShutdownDuringFailedWaitRestoreKeepsJob`	`internal/sync`	`19007fe`: plain error, no `ErrShutdown`
`TestRiverPushFailedFinishKeepsJobUntilClaimExpires`	`internal/jobs`	`19007fe` (`plain`), `ff29ff5` (`pre_run_read_fails`, `completion_fails_once`): no re-run stole the held claim
`TestPGCASResolvesFailedCommit`	`internal/ref`	`7845ac9`: `lost-reply` returned `commit: conn closed` while the swap committed (generation 2); `in-progress` and `status-query-fails` returned a definitive error
`TestServeSkipsPokeAfterLostCommitReply`	`cmd/openstorage`	`7845ac9`: `CASRef` returned `commit: conn closed` after the server committed the swap and a run claimed its job

Other new tests: TestPGCASTxHookCommitsWithSwap (internal/ref, including a COMMIT that fails after the hook ran), TestRiverRecordForwardTxWaitsForJobBeforeUpstreamRow, TestRiverRecordForwardTxSkipsReposThatDoNotForward, TestRiverPushRestoredRunSnoozesForRetry and TestRiverPushDeletedRepoCompletesWithClaimHeld (internal/jobs), TestPushRunWaitsForPublicationOnRiverLane, TestPushRunFailedPublicationWaitRestoresOnRiverLane, TestPushRunShutdownDuringPublicationWaitRestoreIsBounded and TestPushRunDeletedRepoOnRiverLane (internal/sync), TestHookRecordsNoForwardForSyncLoopbackPush (cmd/openstorage), and the write_only, ephemeral_ref, ephemeral_repo_key and recovered_twin subtests. Twenty-one source mutations each turn at least one of these tests red: seventeen from the first pass, and four guards that a later verifier found no test caught (Committed before COMMIT, the ephemeral exclusion, RecordForwardTx's forward_on_push check, and the hook's sync-claim guard). Twelve mutations of the COMMIT resolution in item 5 each turn TestPGCASResolvesFailedCommit or TestServeSkipsPokeAfterLostCommitReply red: no resolution, aborted read as a win, an unknown outcome read as definitive, the ambiguity marker made SafeToRetry, a failed status query read as aborted, a wait with no timeout, Committed skipped after a confirmed commit, the CAS connection kept during the status query (lost-reply-one-conn), no second status query while in progress (delayed-commit), a status context that inherits the caller's cancellation (lost-reply-canceled), no pass-through for a swap that changed no row (no-row-changed), and a 50 ms budget (delayed-commit). pgfault marks each fault fired before it closes any connection. With the old order, a 30 ms pause between the close and the mark turns dropped-commit and status-query-fails red. With the new order the same pause stays green, and 40 fresh processes of TestPGCASResolvesFailedCommit, each stopped for 20 ms of every 30 ms (SIGSTOP and SIGCONT) while its subtests ran, all passed.

The verify-lane-schedule model covers only the local schedule. Five covered declarations changed (buildServeEventing, wireHookEventing, Worker.waitPublication, Worker.RunPush and ErrRepoGone), each only on its PostgreSQL or River branch, so the model still describes the local code. check.sh passes, and check.sh --record moved the sync commit. The COMMIT resolution in item 5 changes no covered file. After the merge of main (#677 hashes imports), check.sh --record passed and recorded that merge commit as the sync commit. It is an ancestor of the PR head, so refs/pull/679/head keeps it reachable after a squash merge.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh

…h-lane schedule Model-checks the ADR-50 push lane (SQLite and S3-WAL) against git pushes, push-upstream calls, runs, steals, retries, restores, crashes and shutdown. Nine code facts tie the model to both backends and the worker; fingerprints in the lane-schedule-fingerprints CI job flag covered-code changes. The NoCompletionAssumption probe records two findings: a push that dies after its authority CAS can lose its forward. Change-Id: I07baac11eba4d51333064aa0fb5e90dd84e48653 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…e model variables Change-Id: Iafc903e2e9a284d33bd0cb076b93f5c621837eb2 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…print the push-hold path Codex round 1 on #675: fingerprint the push-hold lifecycle, both refspec vars and LocalScheduler.Start; model pushes that record no intent; state that ref deletions are not forwarded by design. The model now counts a write as owed at its CAS, adds REST writes (OtherWriters.cfg) and a HeldRefsFresh probe that fails with no fault. Change-Id: I863914ac2567ec75ba1b8f92a9b68663b3816c6d Signed-off-by: Thomas Kosiewski <tk@coder.com>

On PostgreSQL a River push run on the node that took the push can snapshot the cache before receive-pack publishes the pushed ref, and a hook that loses its CAS outcome after the commit records no forward intent. Both tests fail on this commit: the upstream never gets the accepted main. Change-Id: Ia41ff2be8d3b15ff159e7531f8955a667355b237 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…pshots A git push's River job exists from its authority CAS on, before receive-pack publishes the cache ref. A push run on the same node could claim it, snapshot the cache while the push's hold kept the old value, forward that, and end done: the accepted push never reached the upstream. River push runs now call the publication wait after the claim, like local runs. A failed wait hands the request back with RestorePush and fails the run, so the job's completion gate snoozes it. Fixes TestPGHookForwardBeforeCachePublication. Change-Id: I5c7fc02af849c5fe27d90e121156e1b29d6d4c4d Signed-off-by: Thomas Kosiewski <tk@coder.com>

…EST CAS On PostgreSQL a REST write records forward intent only after its ref CAS commits. A serve process that dies between the two leaves the accepted write with no push job. The test fails on this commit. Change-Id: I4f1721fdef341cf8834e00c5a79a87f484afa930 Signed-off-by: Thomas Kosiewski <tk@coder.com>

On PostgreSQL a write to a forward_on_push repository recorded its forward intent (push_pending and the River push job) only after the ref CAS committed. A hook or serve process that died in between, or a failed RequestPush or EnqueuePush, left the accepted write with no intent, and nothing on PostgreSQL scans for it. Metadata and refs share one PostgreSQL database, so the CAS now writes the intent in its own transaction: ref.WithCASTxHook runs a hook after a won swap statement and before COMMIT, and the serve and hook wiring set it for every write the sync poke forwards. The pending level, the push job and the ref commit together or not at all. The after-CAS poke stays, for old replicas during a rolling upgrade and for wins recovered from another caller's identical write. Fixes TestPGHookForwardAfterLostCASOutcome and TestServeRecordsForwardIntentWithTheCAS. Change-Id: I18b0962ee614728adffac868277ab8164c8ce97a Signed-off-by: Thomas Kosiewski <tk@coder.com>

…saction Change-Id: Ib7dd1c7257f42c1f6a65197cfb066cec9616b9a9 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…efore the poke The CAS transaction now inserts the push job, so a River run can claim it before the decorator's after-CAS sync poke runs. The poke then marks push_pending again, and the job's completion schedules a second forward run with its own repo.push_upstream events. The new subtest fails on this commit (PushPending:1 after the claim). Change-Id: Id72e26deed6ab966bbc467120b2295901d8efae2 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…e job first The sync poke after a PostgreSQL CAS re-marked a forward that the CAS transaction had already recorded. When a run claimed the new job before the poke, that scheduled a second forward run. The CAS hook now reports through ref.CASTxHook.Committed that its committed transaction recorded the intent, and the poke skips the forward then. A win that ref.IdempotentCAS recovered still gets the poke. RecordForwardTx now inserts the push job before it marks push_pending. It used to hold the upstream row while it waited for a job row held by the job's completion transaction, which reads that row through another pool. With that pool full of statements waiting on the row, neither side could move, and PostgreSQL cannot see that cycle. Fixes TestServeRecordsForwardIntentWithTheCAS/claimed_before_poke; adds TestRiverRecordForwardTxWaitsForJobBeforeUpstreamRow. Change-Id: Id5c9241b5b84457fbe41988cc62e57a2a30bd9ef Signed-off-by: Thomas Kosiewski <tk@coder.com>

… River publication wait A River push run whose publication wait the shutdown drain cancelled restored its request through the 30-second detached finish instead of the short shutdown restore that a cancelled transfer uses. It now uses restoreForShutdown and returns ErrShutdown, so River keeps the job. TestRiverPushRestoredRunSnoozesForRetry pins that the job of a run that restored its request after a failed wait snoozes and runs again. Change-Id: I4c97e34e77aa6812b2949a34cee0d96358cc6867 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I008f14e5ef848e5505344bebcf6288b8e3bec719 Signed-off-by: Thomas Kosiewski <tk@coder.com>

… caller's write Change-Id: Id162f732f5606bf1f0157e29aa2a6425c5039415 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…inish Two River push runs end with their claim held and the request they consumed: a run whose publication-wait restore the shutdown drain cancels returns an error without ErrShutdown, and a run whose finish or restore write fails returns a plain error. The push job's completion gate then completes the job, and nothing forwards the request. Both new tests fail on this commit. Change-Id: Id0fdef5b68c69bfebd9e5177657e4523f9035505 Signed-off-by: Thomas Kosiewski <tk@coder.com>

… held A River push run whose finish or restore write failed returned a plain error with its claim still held and the request it had consumed, and the job's completion gate completed the job. The gate now snoozes such a job until the claim's lease runs out; the re-run steals the claim and its force level, and forwards. A run that the shutdown drain cancels during its publication-wait restore now also returns ErrShutdown, so River keeps that job too. Fixes TestPushRunShutdownDuringFailedWaitRestoreKeepsJob and TestRiverPushFailedFinishKeepsJobUntilClaimExpires. Change-Id: I7eb270392d85832ea7b510eb3dd331bca0ee46d0 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…ead or completion The completion gate keeps a push job whose failed run left its claim held only when that same Work call saw the run fail and read the row before the run. When the pre-run read fails, or when the first completion fails and the re-run meets the unexpired claim, the gate completes the job and the consumed request is lost. Both new subtests fail on this commit. Change-Id: I7e7b8b8a6012adf537cd91c54e2d8fdffdd4c70e Signed-off-by: Thomas Kosiewski <tk@coder.com>

Only the push job runs a repository's push lane, so a claim still held at completion belongs to one of the job's own runs whose finish or restore failed, or that died, and it holds the request it consumed. The completion gate now snoozes the job until the lease can be stolen whatever this Work call saw, instead of only when it saw the run fail and knew the run count. Fixes TestRiverPushFailedFinishKeepsJobUntilClaimExpires subtests pre_run_read_fails and completion_fails_once. Change-Id: I0b69741189f40561b874cb313fec0436a11e55e3 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Also correct the shutdownSnooze and RecordForwardTx docs. Change-Id: Ida509a3c433a7cc4b746c6621ef89e1868414704 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…im held The gate snoozes a push job while its push claim is held, so that the re-run can steal the claim. A run for a repository deleted while queued returns before it claims, so nothing stole the claim, and the job snoozed every 5 seconds forever. On the River lane RunPush now returns ErrRepoGone for a deleted repository, and the push worker completes the job on it. Also correct the pushWorker and RecordForwardTx docs. Change-Id: I7345fd33816f75cf1d1725592507c742f2d77af3 Signed-off-by: Thomas Kosiewski <tk@coder.com>

… log no warning for a deleted repo's run Change-Id: I892bc4f7a4d8cba5b77ed9d4ad4e76543473342d Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I5b2a152ddfdc740e0956b9e08d1d309eb31c5d31 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: Ia945d392c7b59bf3485df519eff43e8b319d86e6 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…SQL forward fix The covered functions that changed (buildServeEventing, wireHookEventing, Worker.waitPublication, Worker.RunPush, ErrRepoGone) changed only on their PostgreSQL and River branches. The local schedule the model describes is unchanged, and check.sh passes. Change-Id: Ib7ff1062c8c9b67db8b7917d3d24debd56fd27ab Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I97768bca03a2121d6725a7a5849329047193d7a9 Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 · 2026-11-05T17:54:24Z

@codex review

ThomasK33 · 2026-11-05T17:54:31Z

@codex security review

chatgpt-codex-connector · 2026-11-05T17:54:43Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T23:53:19.645627Z	`17ac879`	Manual request
🔒 Security Review	✅ Completed 2026-11-05T23:54:21.788435Z	`17ac879`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-11-05T17:58:11Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: d66e8c6ae4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-05T18:00:48Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: d66e8c6ae4

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

A verifier mutated four guards of the forward-intent fix and every test stayed green. Each now has a test that fails under its mutation: - TestPGCASTxHookCommitsWithSwap: a COMMIT that fails on a deferred constraint after the hook ran must not run Committed. - TestServeRecordsForwardIntentWithTheCAS: an ephemeral ref, and a write under the repo's +ephemeral key, record no forward intent. - TestRiverRecordForwardTxSkipsReposThatDoNotForward: a repository whose link does not forward, or that has no link, gets no push job and no pending level. - TestHookRecordsNoForwardForSyncLoopbackPush: a sync run's own loopback push records no forward intent in the hook's CAS. Change-Id: I279809bcb8aef983024950c2d4a51f235fff445c Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 · 2026-11-05T20:07:48Z

@codex review

ThomasK33 · 2026-11-05T20:07:55Z

@codex security review

chatgpt-codex-connector · 2026-11-05T20:13:27Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 7845ac9526

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7845ac9526

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

On PostgreSQL, a push whose COMMIT reply was lost reported failure and dropped its push event, although the ref moved. pgx v5.11 reports that loss as a request never sent: MultiResultReader.NextResult drops the read error of peekMessage, which has already closed the connection, so the next receiveMessage returns connLockError "conn closed", and SafeToRetry is true for it. ref.IdempotentCAS then took the failure as definitive and never rechecked the ref. A swap that changed a row now reads txid_current() before COMMIT. When COMMIT fails, the CAS asks txid_status on another pooled connection, waiting at most 5 s while the transaction is in progress. Committed is a win and runs CASTxHook.Committed, so the forward-intent poke sees the recorded forward and does not schedule a second forward run. Aborted is definitive. An outcome PostgreSQL cannot confirm is marked ambiguous, so IdempotentCAS rechecks the ref. internal/testutil/pgfault is a TCP proxy that loses, drops, or holds the next COMMIT for these tests. Change-Id: I99076d2bb367457c946500bf1fe877ba06322f60 Signed-off-by: Thomas Kosiewski <tk@coder.com>

…closes A CAS that fails at once could see the closed connection before Fired was set, so CutAll and HoldCommit read as "never fired" on code that does not wait for the outcome. Change-Id: I4191e8286e52df79b2f47ec758e3a247722d2dc0 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I86e9db5e82af432f565ddd5016a0f9b6d32bea1e Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I87e8b2995d4f768c0d8a4b4b67a62615aa57504f Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 · 2026-11-05T21:33:54Z

@codex review

ThomasK33 · 2026-11-05T21:34:01Z

@codex security review

chatgpt-codex-connector · 2026-11-05T21:38:10Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 04d4893b07

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-05T21:39:53Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 04d4893b07

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

…come casRef kept its pool connection while commitStatus asked the pool for another one. With pool_max_conns=1, or a full pool, a lost COMMIT reply waited the whole 5 s budget and came back ambiguous, so Committed never ran and the forward poke ran after the recovered win. The connection is closed after a failed COMMIT, so casRef now returns it to the pool first. pgfault now marks every fault fired before it closes any connection: DropCommit and CutAll closed the server side first, and the relay could close the client side before Fired was set. A new DelayCommit fault commits 300 ms late. TestPGCASResolvesFailedCommit adds a one-connection pool, a late COMMIT (the in-progress loop and the 5 s budget), a context that ends with the fault (the detached status context), and a swap that changed no row (the pass-through). Change-Id: I62aa6a71be0d2547d5cf3fa65aae76010b8edbcb Signed-off-by: Thomas Kosiewski <tk@coder.com>

Change-Id: I4f2e40ee912d755c05273f34e4dcb8e247bb06da Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 · 2026-11-05T23:49:29Z

@codex review

ThomasK33 · 2026-11-05T23:49:36Z

@codex security review

chatgpt-codex-connector · 2026-11-05T23:53:18Z

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 17ac879552

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-05T23:54:20Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 17ac879552

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Change-Id: Ib57fdfb972853f6f778efdd69495112629eb8181 Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 added 24 commits November 5, 2026 08:33

docs(skills): explain probe UNEXPECTED lines, FAIL over STALE, and th…

90dea59

…e model variables Change-Id: Iafc903e2e9a284d33bd0cb076b93f5c621837eb2 Signed-off-by: Thomas Kosiewski <tk@coder.com>

docs(adr): amend ADR-44 for PostgreSQL forward intent in the CAS tran…

5ef37ea

…saction Change-Id: Ib7dd1c7257f42c1f6a65197cfb066cec9616b9a9 Signed-off-by: Thomas Kosiewski <tk@coder.com>

docs(adr): say that the sync poke skips a forward the CAS recorded

1e74dab

Change-Id: I008f14e5ef848e5505344bebcf6288b8e3bec719 Signed-off-by: Thomas Kosiewski <tk@coder.com>

test(openstorage): pin the sync poke for a win recovered from another…

52c32dc

… caller's write Change-Id: Id162f732f5606bf1f0157e29aa2a6425c5039415 Signed-off-by: Thomas Kosiewski <tk@coder.com>

test(jobs): check that a held push claim's job waits for the lease

20f35b7

Also correct the shutdownSnooze and RecordForwardTx docs. Change-Id: Ida509a3c433a7cc4b746c6621ef89e1868414704 Signed-off-by: Thomas Kosiewski <tk@coder.com>

docs(jobs): name every holder of a claim the push gate waits for, and…

4b4e54c

… log no warning for a deleted repo's run Change-Id: I892bc4f7a4d8cba5b77ed9d4ad4e76543473342d Signed-off-by: Thomas Kosiewski <tk@coder.com>

docs(adr): say that the push gate keeps a job while its claim is held

c0f3093

Change-Id: I5b2a152ddfdc740e0956b9e08d1d309eb31c5d31 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Merge remote-tracking branch 'origin/main' into fix-pg-forward-intent

61a018a

Change-Id: Ia945d392c7b59bf3485df519eff43e8b319d86e6 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Merge remote-tracking branch 'origin/main' into fix-pg-forward-intent

d66e8c6

Change-Id: I97768bca03a2121d6725a7a5849329047193d7a9 Signed-off-by: Thomas Kosiewski <tk@coder.com>

chatgpt-codex-connector Bot reviewed Nov 5, 2026

View reviewed changes

Comment thread cmd/openstorage/wire_eventing_pg.go Resolved

Comment thread .agents/skills/verify-lane-schedule/SKILL.md Resolved

ThomasK33 added 4 commits November 5, 2026 20:45

Merge remote-tracking branch 'origin/main' into fix-pg-forward-intent

3964ce7

Change-Id: I86e9db5e82af432f565ddd5016a0f9b6d32bea1e Signed-off-by: Thomas Kosiewski <tk@coder.com>

docs(skills): record the lane-schedule sync commit after merging main

04d4893

Change-Id: I87e8b2995d4f768c0d8a4b4b67a62615aa57504f Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 added 2 commits November 5, 2026 23:02

Merge remote-tracking branch 'origin/main' into fix-pg-forward-intent

17ac879

Change-Id: I4f2e40ee912d755c05273f34e4dcb8e247bb06da Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 added this pull request to the merge queue Nov 6, 2026

Merged via the queue into main with commit faea6e1 Nov 6, 2026
10 of 11 checks passed

ThomasK33 deleted the fix-pg-forward-intent branch November 6, 2026 02:02

github-actions Bot mentioned this pull request Nov 6, 2026

chore(main): release 0.20.0 #653

Merged

This was referenced Nov 6, 2026

fix(jobs): never complete a sync job while its sync claim is held #716

Merged

test(ref): tighten three PostgreSQL COMMIT-status tests #717

Merged

ThomasK33 added a commit that referenced this pull request Nov 6, 2026

docs(skills): record verify-lane-schedule after merging #679 and #689

745c7fb

Change-Id: Ib57fdfb972853f6f778efdd69495112629eb8181 Signed-off-by: Thomas Kosiewski <tk@coder.com>

Conversation

ThomasK33 commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What was broken

Fix

Not covered here

Tests

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

🛡️ Codex Security Review

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

🛡️ Codex Security Review

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

🛡️ Codex Security Review

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

ThomasK33 commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026

🛡️ Codex Security Review

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ThomasK33 commented Nov 5, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 5, 2026 •

edited

Loading