Repository navigation
Conversation
…u.sh (#2537) ## Summary Settles #1094. `menu.ps1` serialized the shared hub cache with a named .NET `Mutex`, while `menu.sh` takes a `flock` on `$DIR/hub.lock`, so a menu of each kind sharing one cache directory never saw the other's lock. - **`menu.ps1` now locks `hub.lock` itself**, opened with `FileShare.None`, the shape `bootstrap.ps1`'s `Lock-Directory` already uses. .NET implements that as `flock(LOCK_EX|LOCK_NB)` on Linux, which conflicts with `menu.sh`'s shared reader lock and its exclusive writer lock in both directions. On Windows it refuses any other open handle on the file. Every caller still takes one exclusive lock, as with the mutex. - **The open does not block on either platform, so a held lock is waited out by retrying.** It retries only where the HResult says the file is held, measured on pwsh 7.4: `0x80070020` (low word 32, or 33 for a lock violation) on Windows, and raw errno 11 (`EWOULDBLOCK`) on Linux. Any other failure, the cache directory's own creation included, fails the task and returns the caller's own failure value. A `$false` there would read as exit code 0 to a caller comparing with `-eq`. - **The lock is re-entrant within a session.** `Invoke-FetchHub` is reached through `Confirm-HubRoot` from inside a span that already holds it, which the mutex allowed and a second open of the file would not. - **`-DryRun` runs unlocked**, matching `menu.sh`'s `hub_read_lock_acquire`, since a dry run never fetches or reads the cache and must not create the directory or the lock file. ## Verification - `TestPowerShellMenuHubLock`, 7 cases. On Linux (pwsh 7.4.2 in the offline `python:3.12-slim` image): all 7 pass, including the two cross-tool cases, a shared `flock` making the menu wait and the menu's lock refusing a shared `flock`. On Windows (pwsh 7, from Git Bash): 5 pass, and the 2 Linux-only cases skip. - Mutations, each failing the suite: `FileShare.Read` in place of `None` (3 failures on Linux), the re-entrancy guard removed (the nested case times out), and the `New-Item` catch removed. - `tests.test_bootstrap` passes on Windows. ruff, mypy, PSScriptAnalyzer, the prose gate, and the EOL gate are clean. - Three local strict-review passes. The first two raised five findings: dry run writing files, an unbounded wait on any plain `IOException`, an uncaught directory-creation failure ending the session, and two test gaps. Four were fixed in this branch. One was declined: a test for a non-contention `IOException`, which no portable fixture produces. The wait tests cover that classification from the other side. The third pass raised nothing. The `comments` label stands for the new lock's comments. Closes #1094 on promotion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2538 +/- ##
=======================================
Coverage 58.40% 58.40%
=======================================
Files 16 16
Lines 8090 8090
=======================================
Hits 4725 4725
Misses 3365 3365
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-platform locking and cleanup under concurrent sessions warrant final human review.
0 open findings
What changed in this PR
This promotion brings the shared hub-cache lock from develop to main, so the PowerShell and shell menus coordinate when they use the same cache directory.
Changes:
- Replace
menu.ps1's named mutex with a lock onhub.lock. - Add tests for contention, failure paths, dry runs, re-entry, and Linux interoperability.
| File | Description |
|---|---|
tests/test_bootstrap.py |
Adds behavioral tests for the PowerShell lock. |
host-setup/menu.ps1 |
Uses a shared lock file for hub-cache operations. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
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.
Summary
Promotes one change from
develop.host-setup/menu.ps1now locks the shared hub cache through$DIR/hub.lockwithFileShare.None, in place of a named .NETMutex, so it coordinates withmenu.sh'sflockon the same file. .NET implementsFileShare.Noneas an exclusiveflockon Linux, and on Windows it refuses any other open handle. The lock is re-entrant within a session and skipped under-DryRun, matchingmenu.sh. It waits only where the open's HResult says the file is held. Any other failure fails the task, using the value the task itself returns on failure.Closes #1094.
Verification
TestPowerShellMenuHubLockpassed all 7 cases on Linux, in pwsh 7.4.2. That includes the two cross-toolflockcases. On Windows 5 cases ran and passed, and the 2 Linux-only cases were skipped. Three mutations each failed the suite.WaitOne()andmenu.sh'sflockblock with no timeout too, and a hold as long as an OS upgrade is the documented case. The maintainer approved the merge over Recognize Copilot's Collapsible "N Open Findings" Overview Block in pr_review.py #2480's unparsed overview after the overview was read by hand.🤖 Generated with Claude Code