You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: close failed focus checkpoints before verified cleanup - #285
This is the separately authorized runtime repair for the Windows defect exposed by #284; it is not added to the original tests-only PR. It changes only FocusService._checkpoint_file and adds a dedicated regression file (two-file delta against parent 5b6cbca2514d7c2f8ff059c83622ca479c3e12e2).
For early combined CI this branch temporarily includes the unchanged reviewed #281/#282 test stack. Merge sequencing: #281, then #282, then this PR. This is integration ordering, not a functional dependency of focus cleanup. Once the earlier PRs merge, the net main diff is these two files. All actual required checks and branch protections still apply.
Defect and repair
Windows CI evidence: a zero write failed before publication but left .focus.pt-snap-*.recovery, because unlink ran while the checkpoint descriptor was still open.
Attempt descriptor closure exactly once before failure cleanup; never retry a failed close against a possibly reused descriptor.
Recheck the existing safe-path and device/inode identity immediately before unlink; substituted paths are preserved.
Close on fstat failure but never delete a path whose identity could not be established.
Preserve the original error if close/cleanup raises OSError. Keep successful commit/rollback behavior. This does not claim atomic elimination of the check/unlink race.
Verification
Independent source review found no blocker. Exact reviewed source/test blobs are 054e95992aabe05216ed1acaca0c8f84a502ad07 / ed1a10666b80c408c507508242effbb5506ddaca. AST, Ruff, Black and diff checks pass; normal tests/build run only in exact-head CI.
16 portable cases cover zero/short-zero/write/permission/fsync/verify failure, short-write commit and rollback, secondary cleanup errors, close failure, missing identity, real replacement identity and symlink metadata. Real test-owned files/fds enforce close-before-unlink on every platform; no skips/xfails or warning suppression. The original #284 failure assertion remains unchanged and must pass on its final combined head.
No parser-v3 decision, schema/CLI behavior change, runner/security/network/credential change, tag/release/deployment or live-provider action. Refs #277. The tracking issue must remain open until #284 and its post-merge acceptance are complete.
First CI pass found one test-double scoping error, not a new platform failure: test_unknown_checkpoint_identity_closes_without_unverified_deletion replaced process-global os.fstat and intercepted an unrelated fd (Linux job 124878364575, Windows job 124878364245).
Commit 203950ad5a706284d05fd2ee1d9d480c5a00055e limits injection to the owned checkpoint descriptor and transaction context, delegating other descriptors to the original function. Runtime is unchanged. Original-error identity, exactly-once close, no deletion without verified identity, and unchanged focus bytes remain asserted. Independent review and static checks passed; exact-head CI is now pending. No checks or assertions were relaxed.
The reviewed repair head 203950ad5a706284d05fd2ee1d9d480c5a00055e passed all 12 applicable Linux/macOS/Windows checks (Test 41318220383 / Platform 41318220380), and #284's unchanged original Windows zero-checkpoint safety assertion also passed at its intermediate combined head.
After #282 merged, this branch now includes actual main e347be302632b328f29da212d65b062ec3f457ae through verified zero-file-delta normal merge 33c9f00b7767fb8f014a5f5d5b698f7e17be42fd. The source/test tree is unchanged, but new exact-head CI is still required. Merge waits for that and #282 post-merge CI. The focus-only scope uses the full ordinary matrix, actual installed-wheel CLI acceptance and fault/safety regressions; it does not claim nightly dataset evidence or waive such acceptance for unrelated runtime changes.
Final actual-main head 33c9f00b7767fb8f014a5f5d5b698f7e17be42fd passed all 12 applicable checks: Test 41322927437, Platform 41322927496. After independent review and final base/head/review/check rechecks, normal squash merge is e786bfc0395a035670cfa12d568022c758ec848a; its tree exactly matches the reviewed/tested repair. Post-merge CI is pending. This does not close #277: the final isolated tests-only #284 still needs its exact-head and post-merge gates.
Post-merge validation succeeded at main e786bfc0395a035670cfa12d568022c758ec848a: Test 41325988806 and Platform 41325988854, all 12 applicable checks including every Windows shard and macOS. The minimal focus cleanup repair is fully delivered. #277 remains open only for the final isolated coverage-test PR #284 and its post-merge validation; no nightly dataset result is implied by this focus-only delivery.
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
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.
Separate minimal runtime follow-up
This is the separately authorized runtime repair for the Windows defect exposed by #284; it is not added to the original tests-only PR. It changes only
FocusService._checkpoint_fileand adds a dedicated regression file (two-file delta against parent5b6cbca2514d7c2f8ff059c83622ca479c3e12e2).For early combined CI this branch temporarily includes the unchanged reviewed #281/#282 test stack. Merge sequencing: #281, then #282, then this PR. This is integration ordering, not a functional dependency of focus cleanup. Once the earlier PRs merge, the net main diff is these two files. All actual required checks and branch protections still apply.
Defect and repair
Windows CI evidence: a zero write failed before publication but left
.focus.pt-snap-*.recovery, because unlink ran while the checkpoint descriptor was still open.Verification
Independent source review found no blocker. Exact reviewed source/test blobs are
054e95992aabe05216ed1acaca0c8f84a502ad07/ed1a10666b80c408c507508242effbb5506ddaca. AST, Ruff, Black and diff checks pass; normal tests/build run only in exact-head CI.16 portable cases cover zero/short-zero/write/permission/fsync/verify failure, short-write commit and rollback, secondary cleanup errors, close failure, missing identity, real replacement identity and symlink metadata. Real test-owned files/fds enforce close-before-unlink on every platform; no skips/xfails or warning suppression. The original #284 failure assertion remains unchanged and must pass on its final combined head.
No parser-v3 decision, schema/CLI behavior change, runner/security/network/credential change, tag/release/deployment or live-provider action. Refs #277. The tracking issue must remain open until #284 and its post-merge acceptance are complete.