Skip to content

[CI][Windows] Worktree helper suite follow-ups from PR #2669: vacuous late-collision registration check, 500 ms refused-cleanup settle, 10 s askpass readiness deadline #2670

Description

@Chris0Jeky

Three pre-existing findings from the fresh-context review of PR #2669 (closes #2664), all in scripts/git/Test-New-CodexIssueWorktree.ps1 and all outside that PR's diff. None blocks it. Line numbers are at PR #2669's head.

1. The late-collision registration assertion is vacuous (MEDIUM)

Line ~1417: Assert-True (-not $registrationsAfterCollision.Contains($initializerWorktree)) compares a backslash-separated Windows path against git worktree list --porcelain output, which Git prints with forward slashes on Windows. Contains is therefore always false and the assertion always passes; the separate-Git-dir sibling at line ~1498 normalizes with .Replace('\', '/') first and this one does not. A stale .git/worktrees/<name> entry left behind by the detached cleanup host would not be caught.

Caveat before making it strict: git worktree remove deletes the working directory before its admin entry, and the scenario asserts on the directory first, so a strict registration check immediately after could race the admin-entry deletion and produce exactly the kind of timing red #2664 fixed. The right shape is to extend Wait-ForScheduledWorktreeRemoval (PR #2669) to wait until both the directory and the normalized registration are gone, then assert both. Confirm first by printing both strings side by side in one local run.

2. The ignored-content collision path uses a 500 ms settle that cannot detect a wrongly scheduled cleanup (MEDIUM)

Line ~1443: Start-Sleep -Milliseconds 500 precedes the assertions that the refused-cleanup canary and worktree still exist. A regression that wrongly scheduled the detached cleanup host on this path would delete the canary one host start plus seven Git invocations later, well after 500 ms, so both Test-Path assertions would pass and the preservation invariant would be silently unguarded. A longer sleep does not fix this shape; it needs a positive check that no cleanup host was started (for example the initializer's output must not contain "was scheduled", or a test-only marker the cleanup script writes on start).

3. The askpass probe-server readiness deadline is a fixed 10 s (MEDIUM)

Line ~862: $serverReadyDeadline = [DateTimeOffset]::UtcNow.AddSeconds(10) guards a cold PowerShell host start plus an HttpListener bind on the hosted Windows runner, the same class of fixed short deadline against runner latency that #2664 hit. The suite's other readiness waits use 30 to 90 s. Raise it to the same order and include the elapsed time in the failure message.

Refs #2664, PR #2669, #2425, #2378.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingciCI/CD pipeline, workflow, and build infrastructure changes.testingTest coverage, harnesses, regression prevention, and verification pipeline work.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions