[pull] main from jdx:main - #662
Merged
Merged
Conversation
…icts (#13232) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ry's preferred one (#13238)
…13204) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | ghcr.io/jdx/mise | container | digest | `1a4fe93` → `a826df7` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Configuration 📅 **Schedule**: (in timezone America/Chicago) - Branch creation - Only on Tuesday (`* * * * 2`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/jdx/mise). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC43OS4xIiwidXBkYXRlZEluVmVyIjoiNDQuOTMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ctory (#13239) Follow-up to #13233, which added the blanket conflict flags. A review comment landed a few minutes after that PR merged; this is the fix. ## The problem `mise dot pull --take-remote-all` is advertised as the one command that decides every paused conflict at once. It swept *every* conflict into the blanket choice, including a path whose live side is not a regular file or symlink — a tracked dotfile that has since been replaced by a directory, for example. Recording a choice for such a path fails with `~/.one is not a regular file or symlink`, and that failure propagated out of the resolution loop before a single decision was written. One unusable path meant the command decided nothing, with an error that named the path but not the reason it surfaced or what to do about it. That contradicts what mise already does elsewhere. `mise bootstrap` tells users "a path held for another reason, such as a directory where the repository has a file, says so in `mise dot status` and needs that fix instead", and the sync preflight notes that "a changed type or unreadable live file must hold incoming application, not abort fetching". Such a path is supposed to be *held*, not to take the rest of the pass down with it. ## What changes A path the blanket choice cannot describe is held, the pass decides and records every other conflict, and the command then fails naming what it could not do: ```console $ mise dot pull --take-remote-all --yes mise ERROR sync paused: resolve all 1 conflict(s) before sharing resumes; a blanket choice cannot decide them all: ~/.one is not a regular file or symlink. Fix each of those paths, then pull again ``` The failure is the point: a blanket flag promises to decide *every* conflict, so it has not succeeded while one is still held. But the decisions it did make are written before it fails, so fixing the held path and deciding just that one finishes the setup: ```console $ rm -rf ~/.one && cp /backup/.one ~/.one $ mise dot pull --take-remote ~/.one --yes ~/.one write home/.one ~/.two write home/.two mise history: applied 2 incoming change(s) ``` `~/.two` applies here because the earlier blanket command had already recorded a decision for it — no need to re-answer conflicts that were never in question. Two behaviors are deliberately unchanged: a path named explicitly (`--take-remote ~/.one`) still reports the failure, because the user asked for that path specifically; and `--keep-local-all` still refuses a conflict with unsaved local edits, since keeping the local side would publish a stale saved version. ## Implementation The check sits on the same `live_object` call that records the choice, rather than in a pass that classifies paths beforehand — so a path that changes underneath the command is held too, instead of aborting it. Only a failure that is the live path's own fault is held: an unreadable file, or one that is neither a file nor a symlink. A repository failure on a sound file (Git failing to hash it, say) is nobody's to fix by hand, so it still stops the pass with its own error. Also included, found while reviewing that function: `live_object` hashed a symlink's target with `to_string_lossy`, while capture uses `shadow::path_bytes`. A target that is not valid UTF-8 therefore hashed differently than the saved object, which read as a live edit and rejected `--keep-local` for a file nobody had touched. It now hashes the same bytes capture does. ## Validation `mise run test:e2e e2e/cli/test_dotfiles_resolve_all` passes. It gained a case that puts a directory where one of two conflicting files belongs, and covers the failure and its reason, that nothing is half-applied, and recovery afterwards. The recovery step names only the previously stuck path, so the test fails if the earlier blanket command did not persist its decision for the other conflict. The test goes from 22s to 30s. Fifteen neighbouring `test_dotfiles_*` e2e tests pass, including those covering symlink tracking, rollback and unapply. `cargo clippy --workspace --all-features --all-targets -- -D warnings` is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) *AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: 2.1.270.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes `mise dot pull` conflict recording and error handling in sync apply; behavior is narrower but affects dotfiles sync, with new e2e coverage. > > **Overview** > **Blanket dot pull conflict resolution** (`--take-remote-all` / `--keep-local-all`) no longer fails the whole pass when one conflicting path’s live side is not a regular file or symlink (e.g. a directory). Those paths are **held**, resolutions for every other conflict are still recorded, and the command exits with a message listing what could not be decided and asking you to fix those paths before pulling again. **Dry-run** and automatic passes **warn** instead of bailing on the same condition. > > `live_object` now classifies unreadable or non-file live paths via an **`UnusableLive`** error (explicit paths still fail immediately). Symlink targets are hashed with **`shadow::path_bytes`** so they match capture, fixing false “unsaved” mismatches on non-UTF-8 targets. > > E2E coverage adds the directory-at-conflict scenario, including partial apply safety and recovery after fixing only the stuck path. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 74fce04. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved conflict resolution when a live path is an unusable directory. - Blanket resolution now continues processing other conflicts while clearly reporting paths that require manual attention. - Explicitly selected paths still report errors immediately when they cannot be resolved. - Interactive resolution identifies each blocked path and explains that it must be fixed before pulling again. - Dry-run and automatic modes now warn about unresolved paths without discarding the paused setup. - Remote resolution succeeds after the conflicting directory is removed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…l-all (#13242) `mise dot pull --keep-local-all` promises to decide every conflict at once. Until now a single path with unsaved local changes made the whole flag useless: the pass aborted, and every other conflict it had already decided was discarded. The refusal on that one path is correct and stays. `--keep-local` publishes this machine's *saved* version of a file, so a path whose live file has drifted from its saved version — or which has no saved version yet — cannot be kept without publishing something stale. What was unintended is that the refusal took the rest of the pass down with it. ## Before Two conflicts, `~/.one` edited since its last `mise dot save`: ``` $ mise dot pull --keep-local-all --yes Error: run `mise dot save ~/.one` first: --keep-local publishes this machine's saved version of ~/.one, and it has unsaved changes ``` `~/.two` was decidable, but nothing was recorded for it. Saving `~/.one` and pulling again was the only way forward, and any path in the same state restarted the cycle. ## After The decision for `~/.two` is recorded, `~/.one` is held, and the failure names every held path and what to do about it: ``` $ mise dot pull --keep-local-all --yes Error: sync paused: resolve all 1 conflict(s) before sharing resumes; a blanket choice cannot decide them all: ~/.one has unsaved changes, so run `mise dot save ~/.one` first. Fix each of those paths, then pull again ``` Deciding just that one path now finishes the setup, because the blanket choice for `~/.two` is still on file: ``` $ mise dot pull --take-remote ~/.one --yes ~/.one write home/.one mise history: applied 1 incoming change(s) ``` `~/.two` keeps this machine's version, as `--keep-local-all` chose. Naming a path explicitly is a different request, so `mise dot pull --keep-local <path>` still fails with its original message — the user asked for that path in particular. A `--dry-run` preview and the background watcher warn rather than fail, as they already do for a path whose live side is unusable. ## Implementation This reuses the `blanket_chosen` / `blanket_held` mechanism #13239 added in `apply_locked_with_scope` for a path whose live side is a directory rather than a file, extending it to the second bail with the same shape. Both held reasons flow through the same failure and the same `dry_run` / `automatic` warning. ## Validation `mise run test:e2e e2e/cli/test_dotfiles_resolve_all` passes. It gains two sections, one for each reason a local version cannot be kept, both with a second conflict that *is* decidable: one where the held path has unsaved edits, one where it is a newly tracked path the machine has never saved. Each asserts that `--keep-local-all` fails naming the held path, that neither file is touched, and that deciding the held path alone then applies both decisions — including the `--keep-local-all` choice recorded for the other conflict. The unsaved-edits section also checks that a `--dry-run` preview says the reason instead of failing, and the existing single-conflict block now asserts that the explicit `--keep-local <path>` form still fails on its own terms. `mise run test:e2e e2e/cli/test_dotfiles_bootstrap_from_git` and the surrounding dotfiles sync tests pass, as do `cargo clippy --workspace --all-features --all-targets -- -D warnings` and `cargo test --bin mise system::history`. The test now runs 36s and trips the harness's "not marked as slow" warning (it was already over the 20s line before this change); leaving it unmarked keeps it running by default. Stacked on #13239, which introduces the mechanism this extends. Retarget to `main` once that merges. 🤖 Generated with [Claude Code](https://claude.com/claude-code) *AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes conflict-resolution behavior in dotfiles sync apply logic; incorrect handling could apply partial sync state or wrong versions across machines. > > **Overview** > **`mise dot pull --keep-local-all`** no longer aborts the entire pass when one conflict cannot be kept locally. Paths with **unsaved changes** or **no saved version on this machine** are **held** (like directory live sides), while decisions for other conflicts are still recorded. The error aggregates held paths and tells you to `mise dot save` before retrying; resolving only the held path can finish sync using the blanket choice already stored for the rest. > > **Explicit `mise dot pull --keep-local <path>`** still fails immediately on that path—the user asked for that file in particular. > > Error copy is tightened (`has unsaved changes` / `has no saved version…`, plus grammar in the immediate bail). **E2E** in `test_dotfiles_resolve_all` adds multi-conflict cases for unsaved drift, missing saved baseline, dry-run preview, and per-path follow-up pulls. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 661be0e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Clarified `--keep-local` errors for paths with no saved version or unsaved changes. - Error messages now provide the exact `mise dot save {path}` command needed before retrying. - Improved conflict-resolution feedback for blanket choices, including clearer reasons when a path cannot be applied. - Dry-run previews now report unsaved-change conflicts without failing. - Preserved previously selected resolutions when resolving remaining conflicts individually. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )