fix: keep unmerged reviewer commits during reclaim + fix empty-stdin deadlock#63
Merged
Merged
Conversation
A reviewer is told to commit any fix it makes, but that commit lives ONLY on its review branch in an ephemeral checkout — never pushed or merged. The mid-objective reclaim (#62) tore those checkouts down the moment the reviewer went terminal, silently destroying the work. (This bit us live: a reviewer's compiled-run follow-up commit was lost when its checkout was reclaimed.) reclaimSpentCheckouts now keeps any non-publishing checkout whose branch has advanced past its base (checkoutHasUnmergedCommits): disk is the cheaper loss. Published-PR checkouts stay exempt — their commits are safely on the PR. The probe is conservative: it only reclaims when there is provably nothing to lose (dir gone / not a repo / HEAD == base) and keeps the checkout on any uncertain probe of a still-present dir.
…deadlock) An empty Stdin was indistinguishable from an interactive session, so the executor left the stdin pipe open — and over SSH allocated a -tt pty. A feed-and-wait write with empty content (tee'ing a blanked memory file) then hung forever waiting for an EOF that never came. A single 0-byte repo-memory row wedged seedRepoMemory on resume, which deadlocked EVERY manager's StartRun and took the whole fleet down after a redeploy. Add Command.CloseStdin: it forces the pipe written-and-closed even when Stdin is empty, and suppresses the SSH pty so the EOF reaches the remote reader. writeWorkspaceFile (the tee that seeds memory) sets it, so an empty memory file now writes cleanly instead of hanging.
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
1 participant
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.
Two fixes from today's a8641186 incident.
1.
fix(orch): keep reviewer checkouts with unmerged commits during reclaimReviewers are told to commit any fix they make, but that commit lives only on the review branch in an ephemeral checkout — never pushed or merged. The mid-objective reclaim from #62 tore those down the moment the reviewer went terminal, silently destroying the work. This actually happened: a reviewer's compiled-run follow-up commit (
cd91a5a62572d) was lost when I cleaned up disk.reclaimSpentCheckoutsnow keeps any non-publishing checkout whose branch advanced past its base (checkoutHasUnmergedCommits). Published-PR checkouts stay exempt (their commits are on the PR). The probe is conservative — only reclaims when there's provably nothing to lose (dir gone / not a repo / HEAD == base), keeps on any uncertain probe.2.
fix(exec): close stdin for empty feed-and-wait writes (manager-wedge deadlock)An empty
Stdinwas indistinguishable from an interactive session, so the executor left the stdin pipe open — and over SSH allocated a-ttpty. A feed-and-waitteewith empty content (seeding a blanked memory file) hung forever waiting for an EOF that never came. A single 0-byte repo-memory row deadlockedseedRepoMemoryon resume, which wedged every manager's StartRun and took the whole fleet down after a redeploy.New
Command.CloseStdinforces the pipe written-and-closed even whenStdinis empty, and suppresses the SSH pty so EOF reaches the remote reader.writeWorkspaceFilesets it.Tests
TestReclaimWorkspaces_KeepsReviewerCheckoutWithUnmergedCommit— real git repos: reviewer-with-commit kept, reviewer-without reclaimed.TestLocal_EmptyStdinClosedWithCloseStdin— the empty feed-and-wait no longer hangs.