Skip to content

test(worktree-helper): wait on a deadline for the scheduled orphan removal - #2669

Merged
Chris0Jeky merged 2 commits into
mainfrom
issue-2664/helper-collision-wait
Sep 5, 2026
Merged

test(worktree-helper): wait on a deadline for the scheduled orphan removal#2669
Chris0Jeky merged 2 commits into
mainfrom
issue-2664/helper-collision-wait

Conversation

@Chris0Jeky

@Chris0Jeky Chris0Jeky commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

Docs Governance / Worktree Helper (Windows PowerShell) reddened PR #2662 at 5cd7f5e50 (run 33951635415, job 101267419144) in the late-branch-collision scenario of scripts/git/Test-New-CodexIssueWorktree.ps1: "Late branch collision must not leave an orphan helper-created worktree." The scenario asserted that the orphan worktree directory was gone after a fixed poll of 50 x 100 ms (5 seconds). The initializer hands that removal to a hidden, detached PowerShell host (Initialize-CodexIssueWorktree.ps1 lines 170 to 214) that re-verifies the worktree's top level, common directory and HEAD, re-inventories tracked, untracked, ignored and index-hidden content, and only then runs git worktree remove: one host start, a poll until the initializer process exits, and seven Git invocations against a fresh checkout, which the hosted runner did not finish in 5 seconds. The printed "removal ... was scheduled" assertion passed and the registration check never ran, so the failure says nothing about the cleanup itself.

This PR replaces that poll, and the separate-Git-dir sibling with the identical shape and a 10 second ceiling, with one Wait-ForScheduledWorktreeRemoval helper: a 90 second deadline (the same pattern the suite already uses at its 30 and 45 second waits), 100 ms polling, and a result carrying elapsed seconds. The assertions keep their meaning (a genuine orphan still fails), and the failure message now includes the elapsed wait and the current git worktree list --porcelain so a real orphan is diagnosable from the job log. The initializer, its timeout value and the refused-cleanup scenarios are unchanged.

scripts/git/** is not an SC-10 control path (.github/workflows/**, ci/**, scripts/ci/**, lockfiles, security config), so this is a normal T3 PR.

Verification

  • PowerShell parser on the changed file: 0 errors.
  • Full suite from a native PowerShell host in the worktree, the exact command the Docs Governance Windows job runs (powershell -NoLogo -NoProfile -NonInteractive -File scripts/git/Test-New-CodexIssueWorktree.ps1): PASS: 29/29 selected worktree helper regression checks, exit 0. Scenario 11 (the collision scenarios) and scenario 29 passed on the new helper.
  • git diff --check: clean.
  • Second commit eb9a67f corrects the helper comment only (the cleanup host runs seven Git invocations after a parent-exit poll, not six); parser 0 errors, no executable change, so the suite was not rerun for it.
  • One earlier local run launched from Git Bash stopped at scenario 19 (invalid-branch): its issue-471/COM¹ case is passed on the command line and the child host decoded the superscript differently from Git, so the helper's generic branch-name check fired before the Windows-compatibility diagnostic. The same file, same commit, native host: green. That is a launch-encoding artifact of the Bash-hosted run, not a suite defect, and it is unrelated to this diff.

Not verified

  • The hosted Windows job at this exact head has not run yet at PR-open time; ci-required is the gate.
  • The 90 second ceiling was chosen from the suite's existing 30 and 45 second waits and the observed hosted failure, not from a measured worst case of the detached cleanup host; the elapsed time is now in the failure message so a future red records the real number.

Closes #2664. Refs #2425, #2378, PR #2662.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Review disposition (beta-platform-integrity lane, T3 gate)

Fresh-context adversarial review at 153531b: SHIP. No CRITICAL or HIGH. Confirmed: the assertion predicate is unchanged (Removed = -not (Test-Path ...), so the check cannot pass with the orphan directory present); the wait is bounded by a 90 s deadline and the job declares no timeout-minutes, so worst case adds 180 s on the failure path only; Windows PowerShell 5.1 compatible (the same [DateTimeOffset]::UtcNow -lt $deadline idiom already ships in this file); -and binds looser than -lt; the helper emits exactly one object; the failure messages interpolate a double and the string Invoke-Git returns, so they cannot throw; both registration assertions still run unchanged.

Fixed (eb9a67f, comment only)

  • LOW: the new helper's comment undercounted the cleanup host's work (seven Git invocations after a poll until the initializer process exits, not six). Corrected in the comment and the PR body; parser 0 errors; no executable line changed, so the 29/29 native run at 153531b stands.

Tracked in #2670 (pre-existing, outside this diff)

  • MEDIUM: the late-collision registration assertion is vacuous because it compares a backslash path against Git's forward-slash worktree list output (the sibling normalizes, this one does not). Not tightened here on purpose: git worktree remove deletes the directory before its admin entry, so a strict check immediately after the directory assertion could race and reintroduce the timing red this PR fixes; the follow-up extends the wait helper to cover both.
  • MEDIUM: the refused-cleanup path's 500 ms settle cannot detect a wrongly scheduled cleanup host; it needs a positive no-host-started check, not a longer sleep.
  • MEDIUM: the askpass probe-server readiness deadline is a fixed 10 s, the same class as [CI][Windows] Worktree helper: the late-branch-collision scenario's fixed 5 s poll expires before the scheduled orphan removal finishes on the hosted runner #2664.

Declined

  • LOW: capturing git worktree list before the Removed assertion could replace the orphan diagnostic with a generic fixture-Git failure if worktree list ever exits non-zero. It cannot turn a failure into a pass, and worktree list exits 0 whether or not the directory exists; not worth restructuring.

Gate

Not measured by the reviewer (no shell): 90 s sufficiency on the hosted runner. The hosted Docs Governance / Worktree Helper (Windows PowerShell) job at the exact head is the proof that matters; merge once exact-head ci-required is green and the three-minute floor after eb9a67f has passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

1 participant