ci: drop the Windows Defender exclusion step, it bought nothing - #350
Merged
Conversation
Added on the theory that real-time scanning of build output and test temp files was behind the slow Windows leg. Measured warm-to-warm on master (run 30245176121 against a rerun of 30245841953), test execution went 74.7s to 72.9s -- inside the noise band -- for a step costing 3s. The ~90s figure that motivated it came from a PR run, not the master baseline it was compared against, so the premise was wrong before the step was written. The comment left behind records the measurement so the next person doesn't retry it blind. The same measurement confirms the change that did pay: dropping the duplicate cargo build took the Windows leg from 5m33s to 4m53s.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CI build workflow removes the Windows Defender exclusion attempt and its error handling, updates the related documentation, and removes an inline zizmor template-injection ignore comment. ChangesCI workflow updates
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
getappz
added a commit
that referenced
this pull request
Aug 11, 2026
…ge collisions (#456) * git-shim/worktree residual polish: teardown messaging, branch-create accuracy, stranded-checkout recovery - worktree deny message now distinguishes provisioning (claim) from teardown (check_merge/release/audit --prune) so an agent denied mid-teardown gets the exact cleanup action (vent #350) - is_branch_create() classifies checkout -b/-B and switch -c/-C; canonical-checkout deny says 'create a new branch' instead of the misleading 'would detach HEAD' (vent #395) - audit_orphans flags worktrees stranded on the default branch (intact gitdir) so worktree audit --prune clears the gh pr merge --delete- branch / post-merge-sync collision root cause (vents #351/#394/#423) - AGENTFLARE_GIT_ALLOW_CANONICAL_MUTATE now also lifts the default-branch checkout/switch deny in the canonical checkout, giving stranded checkouts a sanctioned way back (vent #386 residual) - allowlist classify.rs (already 1604L on master) in the LOC gate Agentflare-Agent: 1 Agentflare-Branch: task/441 Agentflare-Item: 441 * fix(git-shim): close branch-create detection gaps, protect dirty stranded worktrees CodeRabbit review on PR #456: - is_branch_create only matched exact -b/-B/-c/-C, missing --orphan (checkout+switch), switch's --create/--force-create long forms, and attached short-option spellings (-bname). Those forms slipped past both would_detach_head and deny_canonical_detach_reason undetected, letting an agent create a branch in the canonical checkout via e.g. 'git switch --orphan x' or 'git checkout -bx' with no deny at all. - audit_orphans' new on-default-branch detection could hand a dirty, uncommitted stranded worktree to gc_orphans for deletion; the only other gc_orphans caller (cleanup_item_worktree) already refuses on a dirty status check first. Apply the same guard here. Agentflare-Agent: claude-code_2-1-227_agent Agentflare-Branch: task/441 Agentflare-Item: 441
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.
Removes the Windows Defender exclusion step added in #349. It was justified by a hypothesis, and measurement refutes it.
The measurement
Warm-to-warm on master, Windows leg — run 30245176121 (before #349) against a rerun of 30245841953 (after, with its own cache restored):
cargo build --workspacecargo test --workspaceTest execution — the thing Defender scanning would have slowed — moved 74.7s to 72.9s. That is noise, for a step that costs 3s to run.
The premise was wrong before the step was written: the ~90s test-execution figure I cited came from a PR run, while the baseline it was implicitly compared against was a master run at 74.7s. There was never an 90s-vs-20s gap on the branch being measured.
The comment left in place of the step records this, so the next person to look at the slow Windows leg doesn't retry the same idea blind.
What is kept, and why
Dropping the duplicate
cargo build --workspace— kept. This is the entire measured win. Note it nets −40s, not the −101s the step's own duration suggests: the shared compilation moved intocargo test, which grew by 42s. Still a real 12%.CARGO_PROFILE_DEV_DEBUG=line-tables-only— kept, though also unsupported by this data. Cache restore did not shrink (30s → 37s). Keeping it anyway is a judgment call rather than a measured one: reverting costs another full cold rebuild across all three OSes to chase a difference inside the noise band, and smaller artifacts still have value against cache size limits. If it should go too, it is a one-line revert — but worth batching with something else that already invalidates the cache.Verification
Workflow YAML parsed and the resulting
buildstep list checked. No Rust code changes in this PR.Summary by CodeRabbit