Skip to content

test(subagent): a git failure during worktree removal is not a test failure - #6956

Merged
Hmbown merged 2 commits into
codewhale-hq:mainfrom
Lstarsky0:test/subagent-cleanup-poll-tolerates-git-race
Oct 14, 2026
Merged

Hmbown merged 2 commits into
codewhale-hq:mainfrom
Lstarsky0:test/subagent-cleanup-poll-tolerates-git-race

Conversation

@Lstarsky0

Copy link
Copy Markdown
Contributor

What and why

failed_spawn_removes_its_worktree_despite_post_checkout_hook_output failed once on ubuntu-latest, in the CI run for #6952 (which doesn't touch subagent code): https://github.com/codewhale-hq/Codewhale/actions/runs/38057245676/job/114383431465

git ["branch", "--list", "codex/agent-hook-probe"] failed: fatal: failed to read .git/worktrees/codex-agent-hook-probe-path/commondir: No such file or directory

The test waits for the refused spawn's cleanup by polling git worktree list and git branch --list every 50 ms while the cleanup runs git worktree remove on the blocking pool, and each poll asserts that git exited 0. git checks that a linked worktree's commondir exists and then reads it (get_common_dir_noenv in setup.c). If the removal deletes the file between those two steps, the polling command dies with the message above. The cleanup itself goes on and finishes, so the product did the right thing and the test still failed.

The loop now reads a failed git command as "not settled yet" and polls again until the 30 s deadline. The assertions after the loop are unchanged and still use the strict helper. failed_spawn_removes_the_worktree_it_created shares the loop, so it gets the same fix.

Not in this change: remove_worktree_if_expired in the lane crate also starts with git worktree list --porcelain, and returns without removing anything when that command fails. Two cleanups overlapping in one repository could hit the same window and leave the second worktree behind. I haven't seen that happen, so I left it alone.

Issue

No-Issue: flaky test seen on CI, no issue filed for it

How I tested it

  • The two refused-spawn tests on their own (cargo nextest run -p codewhale-tui --lib with a filter): 2 passed.
  • To exercise the failure without waiting for the race, I put a wrapper named git first on PATH that makes the first two git branch --list calls exit 128 with git's message and passes everything else through. On main both tests fail at the first poll with the same panic as the CI run. With this change both pass.
  • I could not hit the real race on my Mac: 400 git worktree add / remove cycles against four concurrent git branch --list loops, about 7300 polls, no failure. A commondir that exists for lstat but not for open (a dangling symlink) gives the exact message from the CI log.
  • cargo fmt --all -- --check, cargo clippy -p codewhale-tui --all-targets --all-features with CI's flags, and cargo check -p codewhale-tui --all-targets: clean. Clippy also on 1.99, plus cargo +1.89.0 check -p codewhale-tui.
  • cargo nextest run -p codewhale-tui --lib: 15248 passed, 44 failed. The same 44 fail on main (0ea319a) on my machine; most need an extension-host Node version I don't have.

No new tests; this only changes how two existing ones wait.

Checklist

  • One focused change, rebased on current main
  • cargo fmt --all passes, and tests cover new or changed behavior
  • If this adds or changes a user-facing feature, I updated its docs and its row in docs/features.toml (cargo test -p codewhale-tui --test feature_registry checks it) — n/a

…ailure

The two refused-spawn tests wait for the cleanup by polling
`git worktree list` and `git branch --list` from the test thread while the
removal runs on the blocking pool. Each poll asserted that git exited 0.

git checks that `.git/worktrees/<id>/commondir` exists and then reads it
(`get_common_dir_noenv` in setup.c). When `git worktree remove` deletes the
file between those two steps, the polling command dies with

    fatal: failed to read .git/worktrees/<id>/commondir: No such file or directory

and the test panicked although the cleanup went on to finish. This failed
`failed_spawn_removes_its_worktree_despite_post_checkout_hook_output` once
on ubuntu-latest (run 38057245676).

The poll now reads a failed git command as "not settled yet" and tries
again until the deadline. The assertions after the loop are unchanged and
still go through the strict helper.

Signed-off-by: Lstarsky0 <59827030+Lstarsky0@users.noreply.github.com>
@Lstarsky0
Lstarsky0 requested a review from Hmbown as a code owner October 11, 2026 11:26

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T11:29:57.020465Z e929899 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm label Oct 11, 2026
@Hmbown

Hmbown commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Thank you @Lstarsky0 for narrowing the worktree-removal observation race: a transient Git read failure while commondir is being removed should wait for settlement, while the final worktree and branch checks must stay strict. Your fix has merged as its own PR in bbae5043, with your original authored commit and branch retained.

Exact own-head hosted results: macOS19,035passed/0failed/33skipped, Ubuntu19,032/0/34 and Windows18,301passed(oneleaky)/0/27; owner recovery passed all three. Both cleanup cases passed macOS/Ubuntu. The hook-output case is Unix-only; the portable cleanup case also passed Windows. The existing browser-launcher leak remains recorded. This carries the fix on main for the next0.10.2 integration; 0.10.2 has not shipped, and final assembled/package acceptance remains separate.

@Hmbown
Hmbown merged commit bbae504 into codewhale-hq:main Oct 14, 2026
27 checks passed
Hmbown added a commit that referenced this pull request Oct 14, 2026
…ixed-workspace

Pin session-picker workspace metadata so random temporary-directory names cannot satisfy the search fixture. Preserve the original contribution and all listing assertions.

Exact own-head CI: macOS 19035 passed; 0 failed; 33 skipped. Ubuntu 19032 passed; 0 failed; 34 skipped. Windows 18301 passed (1 leaky); 0 failed; 27 skipped; owner_crash_reentry PASS. CI-flags Clippy and all required checks passed. Current-main delta is the separately verified #6956 fixture in a disjoint file; combined main and final release artifacts still need their own gates.

Refs #5929
Hmbown pushed a commit that referenced this pull request Oct 15, 2026
Folds 26 main commits (incl. #6924 Runtime stores/workspace drivers, #6977,
#6943, #6959, #6958, #6956) into PR #6968's lane. The only conflicts were
web/data/agent-docs.generated.json and features.generated.json, regenerated
with web/scripts/derive-facts.mjs (check-facts OK, version 0.10.2).

Verified: scripts/dev-cargo.sh check --locked -p codewhale-tui --all-targets
finished in 9m00s with no errors. nextest -p codewhale-tui --lib on the overlap filter
(runtime_api|rlm::turn|runtime_threads|workspace_lease|snapshot):
902 passed, 0 failed, 14427 skipped (80.1s). Full suite left to hosted CI.

Branch exception: founder-authorized 2026-10-15 for this lane, recorded in
CW/coordination/BRANCH_EXCEPTIONS.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants