Conversation
reset --soft/--mixed silently stages the full content diff between HEAD and target when they've diverged (not just fast-forwarded) -- including any unrelated drift the current branch never had. Adds a warning (not a block) when neither ref is an ancestor of the other, using merge-base --is-ancestor in both directions plus rev-list --left-right --count for the divergence size. --hard is unaffected -- that's is_destructive's existing snapshot-based protection, a different concern (working-tree loss vs. staged-diff surprise). Agentflare-Branch: task/98 Agentflare-Item: 98
📝 WalkthroughWalkthroughThe change adds divergence detection for ChangesReset divergence warning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant flare-git-shim
participant flare-git-core
participant Git
User->>flare-git-shim: Run reset command
flare-git-shim->>flare-git-core: Check reset divergence
flare-git-core->>Git: Resolve HEAD and target histories
Git-->>flare-git-core: Return ancestry data
flare-git-core-->>flare-git-shim: Return warning or no warning
flare-git-shim->>User: Print warning when applicable
flare-git-shim->>Git: Execute reset command
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-git-core/src/classify.rs`:
- Around line 313-320: Update the reset classification logic around the
subcommand mode check to parse only arguments before the `--` boundary, treat an
absent mode as the default `--mixed`, and reject pathspec and `--patch` forms.
Ensure target selection and divergence handling use the pre-boundary arguments,
and extend the related tests covering implicit mixed resets and arguments after
`--`.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38041212-1ea1-41e9-acaa-168a125192b4
📒 Files selected for processing (2)
crates/flare-git-core/src/classify.rscrates/flare-git-shim/src/main.rs
| if subcommand != "reset" || !args.iter().any(|a| a == "--soft" || a == "--mixed") { | ||
| return None; | ||
| } | ||
| let target = args | ||
| .iter() | ||
| .take_while(|a| a.as_str() != "--") | ||
| .find(|a| !a.starts_with('-'))?; | ||
| let commits = reset_soft_divergence(repo_root, target)?; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file outline ---'
ast-grep outline crates/flare-git-core/src/classify.rs 2>/dev/null | head -200 || true
printf '%s\n' '--- relevant implementation ---'
sed -n '260,345p' crates/flare-git-core/src/classify.rs
printf '%s\n' '--- referenced tests ---'
sed -n '1370,1485p' crates/flare-git-core/src/classify.rs
printf '%s\n' '--- related symbols and call sites ---'
rg -n -C 3 'reset_soft_divergence|--soft|--mixed|subcommand.*reset|reset.*subcommand' crates/flare-git-coreRepository: getappz/agentflare
Length of output: 25163
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository files related to classify tests ---'
git ls-files | rg '(^|/)(classify|git-core|Cargo\.toml|Cargo\.lock)' | head -200
printf '%s\n' '--- exact test names and reset test bodies ---'
rg -n -C 8 'reset|divergen|soft|mixed' crates/flare-git-core/src/classify.rs crates/flare-git-core/tests 2>/dev/null | head -500Repository: getappz/agentflare
Length of output: 20883
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Git reset synopsis ---'
git reset -h 2>&1 | sed -n '1,120p'
printf '%s\n' '--- warning callers ---'
rg -n -C 8 'reset_soft_divergence_warning' crates src
printf '%s\n' '--- classify orchestration around warning handling ---'
sed -n '680,805p' crates/flare-git-core/src/classify.rs
printf '%s\n' '--- current argument behavior for representative inputs ---'
python3 - <<'PY'
def current(args):
if not any(a in ('--soft', '--mixed') for a in args):
return None
before = []
for a in args:
if a == '--':
break
before.append(a)
return next((a for a in before if not a.startswith('-')), None)
cases = [
['origin/master'],
['origin/master', '--', 'path'],
['origin/master', '--', '--soft'],
['--soft', 'origin/master', '--', 'path'],
['--', '--soft'],
['--patch', 'origin/master'],
]
for args in cases:
print(args, '=>', current(args))
PYRepository: getappz/agentflare
Length of output: 17311
🌐 Web query:
git reset documentation default mixed mode pathspec -- boundary patch form
💡 Result:
The git reset command documentation defines multiple invocation forms, each handling different scopes such as entire branches, specific paths, or interactive changes [1][2]. Default Behavior and Mixed Mode When git reset is used without path specifications—specifically in the form git reset [] []—it acts on the current branch head [1][3]. In this form, if the argument is omitted, it defaults to --mixed [1][4]. The --mixed mode updates the index to match the specified commit (defaulting to HEAD) while leaving the working directory unchanged [1][5]. Pathspec Usage When you provide path specifications (pathspec) to git reset, the command operates differently; it does not move the branch head [2][6]. Instead, it updates the staged version of the specified files or directories to match the version from the given commit or tree (which also defaults to HEAD) [7][2]. This essentially unstages changes for the specified paths and is considered the opposite of git add [7][6]. Patch Mode The git reset --patch (or -p) option allows for interactive selection of changes [7][2]. In this mode, Git presents differences between the index and the specified commit or tree (defaulting to HEAD) [6][3]. You can interactively choose hunks to "reset," which reverses the chosen changes in the index [2][3]. Like the pathspec form, this is the opposite of git add --patch and does not affect the working tree or the current branch head [7][2]. Summary of Forms 1. Branch-level reset: git reset [--soft | --mixed | --hard |...] [] (defaults to --mixed HEAD) [1][2]. 2. Path-level reset: git reset [] [--] ... (updates index for specific paths) [2][6]. 3. Interactive reset: git reset --patch [] [--] [...] (selectively unstage changes) [2][3].
Citations:
- 1: https://git-scm.com/docs/git-reset
- 2: https://github.com/git/git/blob/e082631e51ebe2c7ee6756a3b45d10732a6480df/Documentation/git-reset.txt
- 3: https://github.com/git/git/blob/f0850875fd44a58cd1eae349244327b2eff29fa7/Documentation/git-reset.txt
- 4: https://www.atlassian.com/git/tutorials/undoing-changes/git-reset
- 5: https://code.googlesource.com/git/+/HEAD/Documentation/git-reset.adoc
- 6: https://man7.org/linux/man-pages/man1/git-reset.1.html
- 7: https://git-scm.com/docs/git-reset.html
Handle default mixed resets before the -- boundary.
git reset <target> defaults to --mixed and moves HEAD, but lines 313-320 require an explicit mode. The mode check also scans arguments after --, so git reset <target> -- --soft can produce a warning even though this is a pathspec form. Parse only options before --, treat no explicit mode as --mixed, and exclude pathspec and --patch forms. Update tests at lines 1420-1451 for the implicit mixed case and the -- boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flare-git-core/src/classify.rs` around lines 313 - 320, Update the
reset classification logic around the subcommand mode check to parse only
arguments before the `--` boundary, treat an absent mode as the default
`--mixed`, and reject pathspec and `--patch` forms. Ensure target selection and
divergence handling use the pre-boundary arguments, and extend the related tests
covering implicit mixed resets and arguments after `--`.
1 participant
Summary
git reset --soft/--mixedonto a target that's diverged from HEAD (not just fast-forwarded) silently stages the full content diff between them — including any unrelated drift the current branch never had, not just the intended commit(s).git reset --soft origin/masterstaged ~20 files including a phantom crate deletion that was actually a refactor on master — caught manually before committing, but nothing warned.reset --soft/--mixedtargets a diverged ref, usingmerge-base --is-ancestorin both directions to detect true divergence vs. a clean fast-forward, plusrev-list --left-right --countfor the commit count shown in the message.--hardis unaffected — that'sis_destructive's existing snapshot-based protection (a working-tree-loss concern), orthogonal to this (a staged-diff-surprise concern).Test plan
cargo test -p flare-git-core reset_soft_divergence— 4/4 pass (soft/mixed-only gating, no-target no-op, fires on real divergence, silent on both fast-forward directions)cargo clippyclean on both touched filescargo fmt --checkcleancargo test -p flare-git-shim— 3 pre-existing failures unrelated to this diff, confirmed identical on unmodifiedmaster(agent/human detection quirk in this environment, not caused by this change)Agentflare-Item: 98
Summary by CodeRabbit
New Features
git reset --softandgit reset --mixedoperations when histories have diverged.Bug Fixes
Tests