Fix #372: false positive production conflicts in teach deploy #373
Conversation
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
…es (#372) _git_detect_production_conflicts() counted all commits (including --no-ff merge commits from direct deploy) as "production ahead", creating permanent false positives. Now uses --is-ancestor fast path and --no-merges filter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…currence After direct merge, ff-only merge production back into draft to keep branches in sync. Prevents #372 recurrence where merge commits accumulate. Reports outcome as step 6/6 — skip with guidance if draft has diverged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Escape hatch when auto back-merge skips (draft has new commits). Tries ff-only first, falls back to regular merge. Added to help text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ZSH always block guarantees _deploy_cleanup_globals runs on every exit path (return, error, signal). Removes two explicit cleanup calls that could be missed on early returns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three identical commit-failure blocks replaced with single helper function. Standardizes messaging across all commit paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Math blank-line check used two-dot diff which could surface production-only files as false positives. Three-dot shows only what changed on draft since divergence, matching the semantic intent of all other deploy diffs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7 test cases covering: same commit, draft ahead, real conflicts, --no-ff merge commits (core #372 scenario), is-ancestor fast path, back-merge sync, and accumulated merge commits. Also updates E2E step count 5→6. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…detection E2E (6 new tests): - Back-merge sync step appears in output [6/6] - Draft contains production HEAD after back-merge - Deploy succeeds when back-merge skips (non-ff) - No false positive after deploy + back-merge - Conflict detection ignores 3 cycles of --no-ff merges - _deploy_step renders skip status Dogfood (10 new tests using demo course): - Helper function loaded, help includes --sync - _deploy_step skip, commit failure guidance output - --sync accepted and works on synced branches - Direct deploy includes back-merge step [6/6] - Conflict detection returns 0 after demo deploy - Multiple deploys produce no false positives - Three-dot diff syntax verified in source Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Data-Wise
added a commit
that referenced
this pull request
Feb 11, 2026
* Fix #372: false positive production conflicts in teach deploy (#373) * fix: replace merge-base conflict detection with is-ancestor + no-merges (#372) _git_detect_production_conflicts() counted all commits (including --no-ff merge commits from direct deploy) as "production ahead", creating permanent false positives. Now uses --is-ancestor fast path and --no-merges filter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: auto back-merge after direct deploy to prevent false conflict recurrence After direct merge, ff-only merge production back into draft to keep branches in sync. Prevents #372 recurrence where merge commits accumulate. Reports outcome as step 6/6 — skip with guidance if draft has diverged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add teach deploy --sync for manual branch synchronization Escape hatch when auto back-merge skips (draft has new commits). Tries ff-only first, falls back to regular merge. Added to help text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: wrap deploy body in ZSH always block for global cleanup ZSH always block guarantees _deploy_cleanup_globals runs on every exit path (return, error, signal). Removes two explicit cleanup calls that could be missed on early returns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: extract _deploy_commit_failure_guidance helper (DRY) Three identical commit-failure blocks replaced with single helper function. Standardizes messaging across all commit paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: standardize three-dot diff syntax in deploy preflight Math blank-line check used two-dot diff which could surface production-only files as false positives. Three-dot shows only what changed on draft since divergence, matching the semantic intent of all other deploy diffs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add dedicated tests for production conflict detection 7 test cases covering: same commit, draft ahead, real conflicts, --no-ff merge commits (core #372 scenario), is-ancestor fast path, back-merge sync, and accumulated merge commits. Also updates E2E step count 5→6. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add E2E and dogfood tests for back-merge, --sync, and conflict detection E2E (6 new tests): - Back-merge sync step appears in output [6/6] - Draft contains production HEAD after back-merge - Deploy succeeds when back-merge skips (non-ff) - No false positive after deploy + back-merge - Conflict detection ignores 3 cycles of --no-ff merges - _deploy_step renders skip status Dogfood (10 new tests using demo course): - Helper function loaded, help includes --sync - _deploy_step skip, commit failure guidance output - --sync accepted and works on synced branches - Direct deploy includes back-merge step [6/6] - Conflict detection returns 0 after demo deploy - Multiple deploys produce no false positives - Three-dot diff syntax verified in source Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Test User <test@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * docs: post-merge sync for #372 — test counts, --sync flag, conflict detection docs - Register test-production-conflict-detection.zsh in run-all.sh (42→43 suites) - Update CLAUDE.md test counts (144 files, 43/43 suites) and deploy extras - Update API reference: _git_detect_production_conflicts new algorithm docs - Add --sync to refcard commands, flags, and output format (5→6 steps) - Update deploy guide: conflict detection ignores merges, --sync workflows Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: release v6.7.1 — version bump + changelog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Test User <test@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
_git_detect_production_conflicts()now uses--is-ancestorfast path +--no-mergesfilter instead of counting all commits (including--no-ffmerge commits created by direct deploy)teach deploy --directkeeps draft in sync with productionteach deploy --syncfor manual branch synchronization when auto back-merge skipsalwaysblock for cleanup, DRY helper extraction, three-dot diff fixCommit sequence
fix:replace merge-base conflict detection with is-ancestor + no-merges (teach deploy: false positive 'production conflicts' from merge commits #372)fix:auto back-merge after direct deploy to prevent recurrencefeat:addteach deploy --syncfor manual branch syncrefactor:wrap deploy body in ZSHalwaysblock for global cleanuprefactor:extract_deploy_commit_failure_guidancehelper (DRY)fix:standardize three-dot diff syntax in deploy preflighttest:7 dedicated tests for production conflict detectionTest plan
source flow.plugin.zshloads without errorsteach deploy --dry-runin a teaching project — no false conflictteach deploy -dthenteach deploy --dry-run— back-merge keeps syncteach deploy --sync— syncs branches🤖 Generated with Claude Code