feat(adopt): batch operations — keep-all / use-all / auto-merge / per-file (#120)#131
Merged
Conversation
Adopt compares user-vault vs rendered-shard with no merge base, so a three-way merge (differ.ts) is impossible. twoWayUnionMerge does a best-effort union via diffLines: keep common lines, union each side's unique lines, and report a conflict when both sides replaced the same span (caller prompts on those). Documented limitations (inherent to having no base): keeps user bytes so it cannot apply shard deletions, and can duplicate non-adjacent related edits. content is only valid when hasConflict is false. Binary inputs always conflict. Tests: identical / pure-insertion each side / shard-deletion-kept / non-overlapping-both / overlapping-replacement-conflict / binary / empty / CRLF, plus determinism and common-line-survival properties (fast-check). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend AdoptResolution with a { kind:'merged', content, hash } variant so
the auto-merge mode can hand the executor pre-merged union bytes. The
executor writes them as `modified` ownership at the merged hash (it's
user-customized content), buckets them in a new summary.adoptedMerged,
adds the 'differs-merged' AdoptApplyKind, and snapshots merged paths for
rollback alongside use_shard (both overwrite the user's file). A future
`update` three-way-merges the result against the cached shard template.
labelForAction gains the merged glyph so its exhaustive switch typechecks.
Integration test: differs + merged resolution → union bytes on disk,
`modified` ownership at the merged hash, adoptedMerged bucket.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a top-level AdoptModePicker shown once (when files differ and no --mode/--yes) before the per-file loop, plus a --mode flag for non-interactive runs: - keep-all-mine / use-all-theirs resolve every divergent file one way. - auto-merge two-way-unions the non-conflicting files (writing merged bytes) and sends only the conflicting ones to the per-file prompt; non-interactive auto-merge falls those back to keep_mine. - decide-per-file is the existing per-file loop. Machine: new mode-select phase + applyMode; the diff-review phase now iterates a `queue` (the files needing a prompt) with pre-seeded resolutions, so per-file and auto-merge share one loop. --yes is shorthand for keep-all-mine; --mode overrides it. AdoptMode is component-owned (AdoptModePicker) and imported by the machine, mirroring AdoptDiffAction — components never import from commands. UI: AdoptSummary renders the new adoptedMerged bucket with a "review recommended" note. The auto-merge option is labelled best-effort. Tests: AdoptModePicker component test (modes + firedRef guard); Layer 1 scenarios 27-30 (keep-all / use-all / auto-merge mixed conflict / --mode non-interactive); scenarios 20-21 updated to pass through the picker; Layer 2 PTY scenario 20 picks decide-per-file; AdoptSummary merged-bucket test + fixture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…120) - ARCHITECTURE §10.5: new mode-select step (3a) + the four modes; flags table gains --mode; a plain-spoken note that auto-merge is a best-effort two-way union merge (no base) that keeps user bytes, does NOT apply shard deletions, and can duplicate non-adjacent edits. Apply step snapshots differs+merged too; implementation-modules line gains adopt-merge + AdoptModePicker. - AUTHORING: adopt description mentions the batch modes + --mode. - CHANGELOG: [Unreleased] Added entry. - CLAUDE.md: AdoptModePicker.tsx + adopt-merge.ts in the trees + module table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
/simplify findings (efficiency clean; altitude confirmed the union shape, queue refactor, AdoptMode ownership, and --mode/--yes modeling are right): - keep-all-mine/use-all-theirs build resolutions via Object.fromEntries instead of an explicit loop. - Extract a local enterDiffReview() to dedup the two diff-review setPhase calls (decide-per-file + auto-merge interactive). - Document the coupling between pickMode's arrow-count map and the picker's option order. Skipped (noted): formatSize duplication (CollisionReview/AdoptDiffView) is pre-existing and out of this diff's scope; the diff-review phase's `plan` field is consumed by executeAdopt, not dead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…120) /harden round 1 (adversarial + correctness + docs; correctness agent found the union shape, queue refactor, hash parity, type narrowing, and switch exhaustiveness all sound): - Docs drift the agents caught: SHARD-LAYOUT §Adopt still said "two choices, no third" and described per-file-only resolution — rewritten for the four modes + the three differs outcomes (keep_mine/use_shard/merged) + the best-effort auto-merge limits. IMPLEMENTATION §3.5 adopt mermaid gains the mode-select + auto-merge nodes (it omitted them). Both also fix a leftover #104 "Wizard" reference. - adopt-executor: the "missing resolution" error message now lists `merged`. - use-adopt-machine: applyMode deps tightened to [executeAdopt, finish]; clarifying comments on the two closure-stability eslint-disables. - Tests: document the conservative trailing-newline-only conflict (diffLines aligns it as a replacement; merge never normalizes newlines — pinned so the behaviour is explicit); flow scenario 31 covers the untested --yes --mode=auto-merge non-interactive fallback (conflict → keep-mine). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds batch resolution modes to shardmind adopt so users can resolve many divergent files with one top-level choice (keep all mine / use all theirs / auto-merge best-effort / decide per file), including a --mode flag and an auto-merge path that union-merges non-conflicting changes and prompts only on conflicts.
Changes:
- Introduces
AdoptModePicker+ newmode-selectphase and refactors diff-review to iterate aqueue(conflicts only for auto-merge). - Adds best-effort two-way union merge (
twoWayUnionMerge) and a new executor resolution kind{ kind: 'merged', content, hash }, plus summary bucketing for auto-merged files. - Expands docs/changelog and adds unit/integration/component/e2e coverage for the new modes and merged resolution.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/adopt-merge.test.ts | Unit + property tests for the two-way union merge behavior and edge cases. |
| tests/integration/adopt.test.ts | Integration test validating executor handling of merged resolution (writes bytes, ownership, summary). |
| tests/e2e/tui/adopt-diff.test.ts | PTY e2e flow updated to navigate the new mode picker before per-file prompts. |
| tests/component/flows/helpers.tsx | Adds mode option typing to adopt flow test harness. |
| tests/component/flows/adopt-flow.test.tsx | New Layer 1 flow scenarios covering all batch modes and --mode interactions. |
| tests/component/AdoptSummary.test.tsx | Verifies summary renders the new auto-merged bucket with review note. |
| tests/component/AdoptModePicker.test.tsx | Component tests for the new mode picker, including one-shot selection guard. |
| source/core/adopt-merge.ts | Implements best-effort two-way union merge for adopt auto-merge mode. |
| source/core/adopt-executor.ts | Adds merged resolution support, rollback snapshot inclusion, and summary bucket. |
| source/components/AdoptSummary.tsx | Displays auto-merged count with “review recommended” messaging. |
| source/components/AdoptModePicker.tsx | New UI component for selecting batch adopt mode across differing files. |
| source/commands/hooks/use-adopt-machine.ts | Adds mode-select phase + applyMode logic, queue-based diff-review, and auto-merge pre-resolution. |
| source/commands/adopt.tsx | Adds --mode CLI flag and renders AdoptModePicker during mode-select phase. |
| ROADMAP.md | Marks #120 adopt batch operations as completed. |
| docs/SHARD-LAYOUT.md | Updates adopt semantics to describe the four modes and merged outcome. |
| docs/IMPLEMENTATION.md | Updates adopt flow diagram to include mode-select and auto-merge paths. |
| docs/AUTHORING.md | Updates author-facing docs to mention adopt batch modes and --mode. |
| docs/ARCHITECTURE.md | Updates architecture description of adopt flow, modes, and flags. |
| CLAUDE.md | Updates repo map to include new adopt mode picker and merge module. |
| CHANGELOG.md | Adds unreleased entry documenting adopt batch operations and testing/docs updates. |
Comment on lines
+50
to
+52
| <Text bold color="yellow"> | ||
| {differsCount} file{differsCount === 1 ? '' : 's'} differ from the shard. | ||
| </Text> |
Owner
Author
There was a problem hiding this comment.
Fixed in 7f0da50. Header now renders "1 file differs from the shard" / "N files differ from the shard" (subject-verb agreement). Updated the component test and the flow-test pickMode regex (it keyed on "differ from", which the singular "differs" would have broken).
Comment on lines
+32
to
+37
| mode: zod | ||
| .enum(['keep-all-mine', 'use-all-theirs', 'auto-merge', 'decide-per-file']) | ||
| .optional() | ||
| .describe( | ||
| 'Resolve all divergent files non-interactively. auto-merge is best-effort (keeps your bytes, ignores shard deletions, may duplicate — review after)', | ||
| ), |
Owner
Author
There was a problem hiding this comment.
Fixed in 7f0da50. Reworded the --mode help: keep-all-mine/use-all-theirs/auto-merge are non-interactive (auto-merge still prompts on conflicts unless --yes), and decide-per-file is explicitly the per-file prompt loop.
Comment on lines
889
to
892
| Flags: | ||
| - `--yes` — skip the values gate + auto-pick `keep_mine` on every `differs`. Preserves the user's bytes on every divergence; safe default for retroactive adoption. | ||
| - `--yes` — skip the values gate + the mode picker; resolve every `differs` as `keep_mine` (shorthand for `--mode=keep-all-mine`). Preserves the user's bytes on every divergence; safe default for retroactive adoption. | ||
| - `--mode <keep-all-mine|use-all-theirs|auto-merge|decide-per-file>` (#120) — resolve the `differs` set non-interactively without the picker. Overrides `--yes`'s default. Combine with `--yes`/`--values` for a fully non-interactive run (the flag governs the diff set, not value collection). | ||
| - `--values <file>` — prefill value answers (same shape as `install --values`); shown on the gate's confirm page as `(from --values)`. |
Owner
Author
There was a problem hiding this comment.
Fixed in 7f0da50. Same reword applied to the ARCHITECTURE §10.5 flag description so the docs match the implementation (decide-per-file prompts; auto-merge prompts on conflicts unless --yes).
Comment on lines
+77
to
+80
| /** A resolution that overwrites the user's file → needs a rollback snapshot. */ | ||
| function overwritesUserFile(resolution: AdoptResolution | undefined): boolean { | ||
| return resolution === 'use_shard' || (typeof resolution === 'object' && resolution.kind === 'merged'); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in 7f0da50. Added a resolution != null guard before the typeof === 'object' branch (typeof null === "object") so a null map value can never reach .kind. The type excludes null, but the function runs on unvalidated resolutions[path] lookups, so the guard is cheap defense.
Comment on lines
+54
to
+60
| if (isBinary) { | ||
| // No meaningful line merge — force a prompt. `content` is unused. | ||
| return { content: userContent, hasConflict: true }; | ||
| } | ||
|
|
||
| const parts = diffLines(userContent.toString('utf8'), shardContent.toString('utf8')); | ||
|
|
Owner
Author
There was a problem hiding this comment.
Good catch — fixed in 7f0da50. twoWayUnionMerge now checks both inputs are round-trippable UTF-8 (Buffer.from(text,"utf8").equals(buf)) and forces a conflict (prompt) when they are not, so a non-binary-but-non-UTF-8 file (e.g. Latin-1, no NUL) is never lossy-merged through U+FFFD. Added a unit test with a 0xFF byte.
Comment on lines
+30
to
+36
| it('singularizes the count for one file', async () => { | ||
| const { lastFrame } = await mount( | ||
| <AdoptModePicker differsCount={1} onSelect={vi.fn()} />, | ||
| ); | ||
| await waitFor(lastFrame, (f) => f.includes('Keep all mine')); | ||
| expect(lastFrame() ?? '').toMatch(/1 file differ(?!s)/); | ||
| }); |
Owner
Author
There was a problem hiding this comment.
Fixed in 7f0da50 alongside the header fix — the test now asserts "1 file differs from the shard".
Six Copilot comments, all valid: 1. AdoptModePicker header was "1 file differ" — fix subject/verb agreement to "1 file differs" / "N files differ"; update the component test and the flow-test pickMode regex (which keyed on "differ from"). 2. twoWayUnionMerge round-trips through UTF-8 (decode → diff → re-encode). A non-binary file that isn't clean UTF-8 (Latin-1 bytes, no NUL) would have its bytes replaced by U+FFFD and silently corrupted. Detect non-round-trippable input and force a conflict (prompt) instead of merging. New unit test with a 0xFF byte. 3. overwritesUserFile: add a `!= null` guard before `typeof === 'object'` (typeof null === 'object') — defensive on the unvalidated resolutions map. 4. --mode help + ARCHITECTURE flag said "non-interactively", but decide-per-file still prompts and auto-merge prompts on conflicts. Reworded both to match actual behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #120.
Summary
shardmind adoptagainst a divergent vault asked for a decision on every differing file (the flagship obsidian-mind run had 35). A top-level mode picker now resolves the whole set at once, with per-file prompting kept as a mode, plus a--modeflag for scripted runs.Four modes: Keep all mine / Use all theirs / Auto-merge (best-effort) / Decide per file.
The auto-merge design decision (settled with the maintainer)
The issue framed "auto-merge non-conflicting" as a three-way merge, but adopt has no merge base — the vault was cloned without shardmind, so the original shard version that produced the user's bytes is unrecorded (
differscarries onlyuserContent+shardContent). A base-anchored three-way merge is genuinely impossible here.We ship a best-effort two-way union merge (
source/core/adopt-merge.ts) instead — the maintainer chose to include it rather than defer, with its limits surfaced, not hidden:Because of this it is labelled "best-effort" in the picker and docs, and the Summary lists every auto-merged file with a "review recommended" note. It's a convenience fast-path, not a correct merge.
How it works
AdoptModePickershows once before the per-file loop when files differ and neither--modenor--yesis set.--yesis now shorthand for keep-all-mine;--modeoverrides it.{ kind:'merged', content, hash }resolution: writes union bytes asmodifiedownership at the merged hash, snapshots merged paths for rollback (alongsideuse_shard), buckets them insummary.adoptedMerged. A laterupdatethree-way-merges the result against the cached shard template (the proper base).mode-selectphase +applyMode; thediff-reviewphase now iterates aqueue(only the files needing a prompt) with pre-seeded resolutions, so per-file and auto-merge share one loop.Quality gate
npm run typecheck— clean.npm test— 1116 passed | 30 skipped (Layer 2 PTY skipped on Windows per E2E: bridge SIGINT delivery reliably on GH Actions Windows runner #57). Was 1093 pre-PR.npm run build— clean.--yes/--valuespath unchanged).#120 acceptance criteria
AdoptModePicker, shown ahead of theAdoptDiffViewloop).shardmind adopt --mode=keep-all-mine|use-all-theirs|auto-merge|decide-per-fileflag for non-interactive use.Invariant 1 proxy
The mode picker/
--modeis interactive-flow only;--yes/--valuesadopt resolves in the machine and never reaches the picker, so the byte-equivalence suites (obsidian-mind-contract.test.ts,cli.test.tsadopt cases) exercise the unchanged classification/executor path and stay green.Harden Audit
Reference bar: #11 (merge engine — fixtures-first + property tests) for the merge piece, #101 for the UI.
Rounds
AdoptModeownership /--mode-vs---yesmodeling are right. Applied:Object.fromEntriesfor keep/use-all,enterDiffReviewdedup, pickMode coupling comment. Skipped (noted): pre-existingformatSizedup (out of diff scope);planfield is consumed byexecuteAdopt, not dead.Real issues found + fixed
ADOPT_WRITE_FAILEDmessage — now lists themergedresolution.applyModedeps — tightened to[executeAdopt, finish]+ clarifying comments on the closure-stability eslint-disables.Verified sound (no action)
Union shape (Buffer+hash in the resolution), hash parity (recorded hash == bytes written),
overwritesUserFilenarrowing + snapshot inclusion of merged,AdoptApplyKind/AdoptResolutionexhaustiveness,assertNeverswitch in adopt.tsx, firedRef (picker) + useOncePerKey (per-file) double-fire guards, module boundaries (core imports onlydiff;AdoptModecomponent-owned), zeroany/@ts-ignore.Tests added
adopt-merge): 11 — identical / pure-insertion each side / shard-deletion-kept / non-overlapping-both / overlapping-conflict / binary / empty / CRLF / trailing-newline, + determinism & common-line-survival properties (fast-check).modifiedownership +adoptedMergedbucket.AdoptModePicker(modes + firedRef) +AdoptSummarymerged bucket.--modenon-interactive /--yes --mode=auto-mergefallback); 20-21 re-driven through the picker; Layer 2 PTY picks decide-per-file.Deferrals
None. (A future base-aware adopt — recording a base so a true three-way merge becomes possible — would supersede the union merge, but that's a separate, larger change.)