Repository navigation
Make the <Git.Push /> authentication cancellation test reliable (#950) - #959
Merged
Merged
Conversation
P21 spawned a halter and released the held credential delivery immediately afterwards, so nothing proved that cancellation had reached the pending authentication before the resolver was released. The acquisition could resume, open and be torn down in either order, which is the scheduling the preserved CI failure is consistent with. The injected authentication resource now registers its cleanup before it announces the pending acquisition. That cleanup acknowledges it started, waits for the same owed delivery and marks the delivered session released. The row observes `task.halt()`, waits for that acknowledgement, and asserts the session is unopened, unreleased and the halt still pending before this row — not the cancelled consumer — delivers it. The delivery sits in a yield-free `finally`, so an assertion that failed cannot strand the joined cleanup. Every real observation is unchanged: no counted push, an absent `review/credential` ref, both sessions for the admitted origin, one acknowledged request with no fabricated outcome, and a continuation publishing that request's exact source commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #959: ✅ Make the cancelled
|
taras
marked this pull request as ready for review
October 19, 2026 14:16
taras
changed the title
<Git.Push /> authentication test wait for cancellation (#950)<Git.Push /> authentication cancellation test reliable (#950)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
P21 now tests cancellation during pending
<Git.Push />authentication without depending on which task the scheduler runs first. Previously, it spawned a halter and immediately released the held credentials, without observing whether cancellation had reached the acquisition.The original CI failure observed a
pushon headcf0678bf23764ebc2dfb41701026bae620416d8e. The complete first failure is preserved. That CI schedule remains unknown; the demonstrated gap is in the test's synchronization. Related issue: #950.What changes
Only P21 in
packages/git/tests/retained-repository-push.test.tschanges.Before: the held authentication could resume before cancellation reached it.
After: the injected resource registers cleanup before announcing pending acquisition. The row waits for cleanup-start, asserts the session is unopened and unreleased with halt still pending, delivers the session late, then joins halt after cleanup releases it. Delivery uses a synchronous
finallyso a failed assertion cannot strand cleanup.All original real execution, bare-remote and history assertions remain:
pushand noreview/credentialremote ref before continuation.How to verify it
On signed commit
d3ebf99c7bbeb14641ee4ef74fb6b14cb318ba12, the focused file passed all 23 steps, including P21, in 41s. Scoped formatting andgit diff --checkpassed.CI run 39266219059 completed successfully on that exact head, including required
green. Deno 4/15 passed P21 on ubuntu in 689ms. No failed-check rerun was taken. This is validation of the repaired head, not an identical-head rerun of the historical failure.Reviews
Both original role reviews returned PASS on the identical signed commit
d3ebf99c7bbeb14641ee4ef74fb6b14cb318ba12:Complete first outputs and the exact-head review reports are retained with the central #950 delivery evidence.
Scope and limitations
This is a P21-only test repair (+57/-14 in one file). Production code, neighboring tests, dependencies, locks, runtime exclusions and runner commands are unchanged. No production defect is demonstrated. The existing failure-watchdog pattern only bounds a missing acknowledgment; no sleep or retry establishes ordering.
The repair removes the test's scheduling dependency; a green run alone is not a flake-closure claim. Recovery is an ordinary revert of the test change.