Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9340c243
ℹ️ 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".
| update = writer.update_ref( | ||
| repository, | ||
| branch=branch, | ||
| new_sha=new_head_sha, | ||
| force=False, |
There was a problem hiding this comment.
Setting force=False only requires the update to be fast-forward; it does not atomically require the branch to still equal head_sha. If another actor force-resets the PR branch to an ancestor after the final get_ref check but before this PATCH, GitHub accepts new_head_sha as a fast-forward and this stale authorized write silently reverses that concurrent reset. Use an API operation with an atomic expected-head predicate, such as createCommitOnBranch(expectedHeadOid: ...), rather than treating a non-force ref update as compare-and-swap.
Useful? React with 👍 / 👎.
| except Exception as exc: | ||
| raise GitHubBranchWriteError( | ||
| "GitHub branch compare-and-swap update failed; expected HEAD may be stale" | ||
| ) from exc |
There was a problem hiding this comment.
If the PATCH reaches GitHub and updates the ref but the response is lost, times out, or contains invalid JSON, update_ref raises even though the mutation may already have occurred. Wrapping every such exception as an ordinary GitHubBranchWriteError incorrectly tells callers this was a failed compare-and-swap instead of an indeterminate write, bypassing the reconciliation behavior represented by GitHubBranchWriteAmbiguousError. Transport or response-processing failures after issuing the mutation must be reported as ambiguous.
Useful? React with 👍 / 👎.
2 participants
Change rationale
Affected scope
Evidence
Risk
Rollback
Governance approvals