diff --git a/.changes/unreleased/Added-20260521-102440.yaml b/.changes/unreleased/Added-20260521-102440.yaml new file mode 100644 index 00000000..f6a34ba9 --- /dev/null +++ b/.changes/unreleased/Added-20260521-102440.yaml @@ -0,0 +1,3 @@ +kind: Added +body: 'integration: Add AI-driven conflict resolution via JSON protocol; configured by spice.integration.resolver and spice.integration.autoResolve' +time: 2026-05-21T10:24:40.113598-04:00 diff --git a/branch_delete.go b/branch_delete.go index bdff27a4..6ac71b7a 100644 --- a/branch_delete.go +++ b/branch_delete.go @@ -6,6 +6,7 @@ import ( "go.abhg.dev/gs/internal/git" "go.abhg.dev/gs/internal/handler/delete" + "go.abhg.dev/gs/internal/silog" "go.abhg.dev/gs/internal/spice" "go.abhg.dev/gs/internal/spice/state" "go.abhg.dev/gs/internal/text" @@ -84,11 +85,22 @@ type DeleteHandler interface { func (cmd *branchDeleteCmd) Run( ctx context.Context, + log *silog.Logger, handler DeleteHandler, + integrationHandler IntegrationHandler, ) error { - return handler.DeleteBranches(ctx, &delete.Request{ + if err := handler.DeleteBranches(ctx, &delete.Request{ Branches: cmd.Branches, Force: cmd.Force, Restack: cmd.Restack, - }) + }); err != nil { + return err + } + + for _, b := range cmd.Branches { + if err := integrationHandler.OnBranchRemoved(ctx, b); err != nil { + log.Warnf("prune integration resolution file: %v", err) + } + } + return nil } diff --git a/branch_untrack.go b/branch_untrack.go index 27b01ab4..d33aaace 100644 --- a/branch_untrack.go +++ b/branch_untrack.go @@ -6,6 +6,7 @@ import ( "fmt" "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" "go.abhg.dev/gs/internal/spice" "go.abhg.dev/gs/internal/spice/state" "go.abhg.dev/gs/internal/text" @@ -29,8 +30,10 @@ func (*branchUntrackCmd) Help() string { func (cmd *branchUntrackCmd) Run( ctx context.Context, + log *silog.Logger, wt *git.Worktree, svc *spice.Service, + integrationHandler IntegrationHandler, ) error { if cmd.Branch == "" { var err error @@ -48,5 +51,9 @@ func (cmd *branchUntrackCmd) Run( return fmt.Errorf("forget branch: %w", err) } + if err := integrationHandler.OnBranchRemoved(ctx, cmd.Branch); err != nil { + log.Warnf("prune integration resolution file: %v", err) + } + return nil } diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index 898fc454..6a7b34cc 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -310,9 +310,11 @@ Before merging, the stack is checked for branches whose base PR was already merged on the forge. Use --no-branch-check to skip this validation. -Before each merge, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait -before failing if checks are not ready. +Before each merge, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait +before failing if merge readiness is not reached. By default, a branch failure skips that branch's upstack descendants, but independent sibling branches continue. @@ -321,12 +323,12 @@ Use --fail-fast to stop the queue after the first branch failure. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--no-branch-check`: Skip stale base validation before merging. * `--fail-fast`: Stop the merge queue after the first branch failure. * `--branch=NAME`: Branch whose stack to merge -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice stack restack {#gs-stack-restack} @@ -624,7 +626,7 @@ This command acts as a local merge queue: it merges one Change Request, waits for that merge to finish, restacks and updates the next Change Request, -waits for its CI checks to pass, +waits for merge readiness on the updated Change Request, and then repeats the process. For a stack like this: @@ -642,13 +644,15 @@ Before merging, the downstack is checked for branches whose base PR was already merged on the forge. Use --no-branch-check to skip this validation. -Before each merge, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait +Before each merge, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait (default: 30m, 0 means fail immediately if not ready). Between merges, the command waits for each merge to complete, restacks and updates the next PR, -waits for CI checks on the updated PR, +waits for merge readiness on the updated PR, and syncs merged branch cleanup. Use --no-wait for single branch merging @@ -658,12 +662,12 @@ when you don't want to wait for the merge to propagate. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--no-wait`: Skip polling for a single branch merge to propagate. * `--no-branch-check`: Skip stale base validation before merging. * `--branch=NAME`: Branch to start merging from -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice downstack edit {#gs-downstack-edit} @@ -1133,16 +1137,18 @@ Use --branch to merge a different branch. The branch must be based directly on trunk. To merge a stacked branch, use 'gs downstack merge'. -Before merging, waits for CI checks to pass. -Use --build-timeout to configure the maximum wait. +Before merging, waits for merge readiness: +the forge must observe the pushed head +and report that the CR is ready to merge. +Use --ready-timeout to configure the maximum wait. **Flags** * `--method=METHOD` ([:material-wrench:{ .middle title="spice.merge.method" }](/cli/config.md#spicemergemethod)): Preferred merge method. One of 'merge', 'squash', and 'rebase'. -* `--build-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.buildTimeout" }](/cli/config.md#spicemergebuildtimeout)): Max time to wait for CI checks before each merge. 0 means check once. +* `--ready-timeout=30m` ([:material-wrench:{ .middle title="spice.merge.readyTimeout" }](/cli/config.md#spicemergereadytimeout)): Max time to wait for merge readiness before each merge. 0 means check once. * `--branch=NAME`: Branch to merge -**Configuration**: [spice.merge.buildTimeout](/cli/config.md#spicemergebuildtimeout), [spice.merge.method](/cli/config.md#spicemergemethod) +**Configuration**: [spice.merge.method](/cli/config.md#spicemergemethod), [spice.merge.readyTimeout](/cli/config.md#spicemergereadytimeout) ### git-spice branch submit {#gs-branch-submit} @@ -1442,9 +1448,17 @@ On conflict, the merge is left in the worktree. Resolve the conflicting files, commit with 'git merge --continue', then re-run 'gs integration rebuild' (or 'gs intrb') to resume. +With --auto-resolve (or spice.integration.autoResolve=true), a +configured resolver script is invoked to attempt automatic +resolution before surfacing conflicts. See the recipe for +details on the JSON protocol the script must implement. + **Flags** * `--push`: Also push the integration branch after rebuilding +* `--[no-]auto-resolve` ([:material-wrench:{ .middle title="spice.integration.autoResolve" }](/cli/config.md#spiceintegrationautoresolve)): Auto-resolve merge conflicts using the configured resolver script + +**Configuration**: [spice.integration.autoResolve](/cli/config.md#spiceintegrationautoresolve) ### git-spice integration submit {#gs-integration-submit} diff --git a/doc/src/guide/integration.md b/doc/src/guide/integration.md index 3bb90a49..c5cf7df8 100644 --- a/doc/src/guide/integration.md +++ b/doc/src/guide/integration.md @@ -127,6 +127,155 @@ subsequent `gs stack submit`, `gs upstack submit`, and `gs downstack submit` invocations will keep the published branch in sync by pushing the latest rebuild. +## Auto-resolving conflicts + + + +Even with rerere and the `regenerate` merge driver handling the +mechanical conflicts, rebuilds can still surface hand-resolution +conflicts when two tips touch the same file. For local-only integration +branches, you can configure a script that resolves these conflicts +automatically. + +The protocol the script speaks (JSON output schema, environment +contract, persistent resolution file, iteration cap, fall-through +rules) is the same across all of git-spice's script-driven features. +See **[Script integrations](scripts.md)** for the complete contract. +This section only documents what is integration-specific. + +### Configuration + +Two git-config keys control the feature: + +- $$spice.integration.resolver$$ — the resolver script body. The value + is the script itself (not a path); git-spice runs it via `sh -c`, + or executes it directly if it starts with `#!`. Typically set at + `--global` scope since the resolver is a personal preference. +- $$spice.integration.autoResolve$$ — when `true`, the resolver runs + automatically on every $$gs integration rebuild$$. + +The shared $$spice.scriptResolve.maxIterations$$ key bounds the Q&A +loop. See [Script integrations](scripts.md#iteration-cap). + +Per-invocation overrides are available on $$gs integration rebuild$$: + +- `--auto-resolve` enables auto-resolve for this invocation. +- `--no-auto-resolve` disables it, even when the config has it on. + +### Integration-specific environment + +In addition to the shared +[`GS_OPERATION` / `GS_BRANCH` / `GS_BASE`](scripts.md#environment-contract) +values, the resolver runs from the repository root with an in-progress +`git merge`. `GS_OPERATION` is always `integration-rebuild`. `GS_BRANCH` +is the tip being merged in; `GS_BASE` is the integration branch (also +recorded in the resolution file's `current_merge.theirs` and +`current_merge.ours` fields, respectively). + +### Resolution file + +Integration's persistent Q&A lives at +`/.spice/resolutions/integration.json`. The schema is +described in +[Script integrations: persistent resolution files](scripts.md#persistent-resolution-files). +Each entry is keyed by the `(ours, theirs)` branch pair, so an answer +recorded on one rebuild is reused on subsequent rebuilds. + +Entries are pruned automatically when their branches are untracked +($$gs branch untrack$$), deleted ($$gs branch delete$$), or removed +by $$gs repo sync$$ after the underlying CR merges. + +### Example: Claude Code resolver + +The resolver is configured as a user-level preference — the config +value is the script body itself, run by git-spice via `sh -c`, +the same shape as $$spice.messageGenerator$$. Use `--global` so the +script applies to every repo you run integration rebuilds in. + +Paste the block below into a terminal to install a resolver that +delegates to [Claude Code](https://docs.claude.com/en/docs/claude-code) +and turn auto-resolve on by default: + +```bash +git config --global spice.integration.resolver "$(cat <<'GITCONFIG' +#!/bin/sh +exec claude --print --max-turns 30 <<'PROMPT' +You are resolving merge conflicts on a throwaway integration branch. + +CONTEXT +- 'git ls-files --unmerged' lists the conflicted paths. +- 'git log -p ours' and 'git log -p theirs' show the commits on each + side; 'git diff ours theirs -- ' compares them on one file. +- GS_OPERATION, GS_BRANCH, and GS_BASE are set in the environment; + see https://abhinav.github.io/git-spice/guide/scripts/ for the + full env contract. +- .spice/resolutions/integration.json names the merge in progress + (current_merge: ours = GS_BASE, theirs = GS_BRANCH) and carries + any prior Q&A you have recorded under resolutions. Honor that + prior guidance when it applies. + +WHAT TO DO +- Edit each conflicted file in place. Remove the <<<<<<<, =======, + and >>>>>>> markers and produce a syntactically valid merged file. +- Do NOT run 'git add' or 'git commit'. After you exit, git-spice + stages every originally-conflicted path and commits the merge. + +OUTPUT — emit exactly one JSON document on stdout, then exit: + + {"assumptions": [...], "questions": [...], "unresolved_files": [...]} + +- All three keys are optional. Empty (or assumptions-only) means + "everything resolved cleanly"; git-spice will stage and commit. +- "assumptions" — short notes on judgement calls you made. They are + surfaced in the rebuild log so a human can spot-check them. +See https://abhinav.github.io/git-spice/guide/scripts/ for what each +field means and when to use it. +PROMPT +GITCONFIG +)" + +git config --global spice.integration.autoResolve true +``` + +For Claude Code to run unattended, pre-approve the tools it needs in +`~/.claude/settings.json`. The schema is a `permissions` object with +an `allow` array of tool patterns +([reference](https://docs.claude.com/en/docs/claude-code/settings#permissions)): + +```json +{ + "permissions": { + "allow": [ + "Read", + "Edit", + "Bash(git log:*)", + "Bash(git show:*)", + "Bash(git diff:*)", + "Bash(git ls-files:*)" + ] + } +} +``` + +Bare names like `Read` and `Edit` allow the tool on any path; scope +them with `Read(/repo/path/**)` if you'd rather only let the resolver +read inside the integration repo. + +If you already run Claude Code with a permissive default — e.g., +`permissions.defaultMode` set to `auto` (skip routine prompts) or +`bypassPermissions` (skip all prompts) — you can omit the `allow` +list entirely. The integration branch is throwaway, so this is a +reasonable trade-off for unattended rebuilds. See the upstream +[settings reference](https://docs.claude.com/en/docs/claude-code/settings#permissions) +for what each mode covers. + +If a resolution turns out to be wrong, the integration branch is +throwaway: investigate the diff, update your prompt or hand-edit +`resolution_instructions` in `.spice/resolutions/integration.json`, +then run $$gs integration rebuild$$ again. If `rerere` has recorded +an incorrect resolution from an earlier run, `git rerere clear` wipes +the cache. + ## Removing the configuration Use $$gs integration delete$$ to remove the configuration. The diff --git a/integration.go b/integration.go index 84adfd70..738c2fc4 100644 --- a/integration.go +++ b/integration.go @@ -26,11 +26,12 @@ type IntegrationHandler interface { RemoveTip(ctx context.Context, branch string) error Show(ctx context.Context) (*integration.Status, error) Checkout(ctx context.Context) error - Rebuild(ctx context.Context) (*integration.RebuildResult, error) + Rebuild(ctx context.Context, opts *integration.RebuildOptions) (*integration.RebuildResult, error) Submit(ctx context.Context) error MarkPushed(ctx context.Context, hash git.Hash) error MaybeRebuild(ctx context.Context) error MaybeRebuildAndSubmit(ctx context.Context) error + OnBranchRemoved(ctx context.Context, branch string) error } var _ IntegrationHandler = (*integration.Handler)(nil) diff --git a/integration_rebuild.go b/integration_rebuild.go index bec1b09c..4fc215db 100644 --- a/integration_rebuild.go +++ b/integration_rebuild.go @@ -12,7 +12,8 @@ import ( ) type integrationRebuildCmd struct { - Push bool `name:"push" help:"Also push the integration branch after rebuilding"` + Push bool `name:"push" help:"Also push the integration branch after rebuilding"` + AutoResolve bool `name:"auto-resolve" negatable:"" config:"integration.autoResolve" help:"Auto-resolve merge conflicts using the configured resolver script"` } func (*integrationRebuildCmd) Help() string { @@ -25,6 +26,11 @@ func (*integrationRebuildCmd) Help() string { On conflict, the merge is left in the worktree. Resolve the conflicting files, commit with 'git merge --continue', then re-run 'gs integration rebuild' (or 'gs intrb') to resume. + + With --auto-resolve (or spice.integration.autoResolve=true), a + configured resolver script is invoked to attempt automatic + resolution before surfacing conflicts. See the recipe for + details on the JSON protocol the script must implement. `) } @@ -33,7 +39,9 @@ func (cmd *integrationRebuildCmd) Run( log *silog.Logger, handler IntegrationHandler, ) error { - res, err := handler.Rebuild(ctx) + res, err := handler.Rebuild(ctx, &integration.RebuildOptions{ + AutoResolve: &cmd.AutoResolve, + }) if err != nil { conflict := new(integration.ConflictError) if errors.As(err, &conflict) { diff --git a/internal/git/merge_wt.go b/internal/git/merge_wt.go index 3a224345..2f5e06ee 100644 --- a/internal/git/merge_wt.go +++ b/internal/git/merge_wt.go @@ -136,3 +136,37 @@ func (w *Worktree) MergeAbort(ctx context.Context) error { } return nil } + +// MergeContinue stages the listed paths and commits an in-progress +// merge. Used after an external resolver has modified the conflicting +// files. Returns an error if any unmerged paths remain after staging. +// +// message is used as the merge commit message. +func (w *Worktree) MergeContinue( + ctx context.Context, paths []string, message string, +) error { + if len(paths) > 0 { + addArgs := append([]string{"--"}, paths...) + if err := w.gitCmd(ctx, "add", addArgs...).Run(); err != nil { + return fmt.Errorf("git add: %w", err) + } + } + + unmerged, err := sliceutil.CollectErr( + w.ListFilesPaths(ctx, &ListFilesOptions{Unmerged: true})) + if err != nil { + return fmt.Errorf("list unmerged files: %w", err) + } + if len(unmerged) > 0 { + return fmt.Errorf("unmerged paths remain after resolution: %s", + strings.Join(unmerged, ", ")) + } + + if message == "" { + message = "Merge" + } + if err := w.gitCmd(ctx, "commit", "--no-edit", "-m", message).Run(); err != nil { + return fmt.Errorf("git commit: %w", err) + } + return nil +} diff --git a/internal/git/merge_wt_test.go b/internal/git/merge_wt_test.go index 5e926963..0eb9b39c 100644 --- a/internal/git/merge_wt_test.go +++ b/internal/git/merge_wt_test.go @@ -173,6 +173,137 @@ func TestWorktree_Merge_conflictLeaveInWorktree(t *testing.T) { require.NoError(t, wt.MergeAbort(t.Context())) } +func TestWorktree_MergeContinue(t *testing.T) { + t.Parallel() + + fixture, err := gittest.LoadFixtureScript([]byte(text.Dedent(` + as 'Test ' + at '2025-01-01T00:00:00Z' + + git init + git add shared.txt + git commit -m 'Initial commit' + + git checkout -b feature + cp feature.txt shared.txt + git add shared.txt + git commit -m 'Feature changes shared.txt' + + git checkout main + cp main.txt shared.txt + git add shared.txt + git commit -m 'Main changes shared.txt' + + -- shared.txt -- + base content + -- feature.txt -- + feature content + -- main.txt -- + main content + `))) + require.NoError(t, err) + t.Cleanup(fixture.Cleanup) + + wt, err := git.OpenWorktree(t.Context(), fixture.Dir(), git.OpenOptions{ + Log: silogtest.New(t), + }) + require.NoError(t, err) + + mergeErr := wt.Merge(t.Context(), git.MergeOptions{ + Refs: []string{"feature"}, + NoFF: true, + Message: "Merge feature", + LeaveConflict: true, + }) + require.Error(t, mergeErr) + require.True(t, errors.As(mergeErr, new(*git.MergeConflictError))) + + // Simulate an external resolver writing a resolution. + require.NoError(t, + os.WriteFile(filepath.Join(fixture.Dir(), "shared.txt"), + []byte("resolved content\n"), 0o600)) + + require.NoError(t, wt.MergeContinue(t.Context(), + []string{"shared.txt"}, "Merge feature")) + + clean, err := wt.IsClean(t.Context()) + require.NoError(t, err) + assert.True(t, clean, "worktree should be clean after MergeContinue") + + content, err := os.ReadFile(filepath.Join(fixture.Dir(), "shared.txt")) + require.NoError(t, err) + assert.Equal(t, "resolved content", strings.TrimSpace(string(content))) +} + +func TestWorktree_MergeContinue_unmergedRemain(t *testing.T) { + t.Parallel() + + // Set up a merge that conflicts on TWO files; pass only one to + // MergeContinue. The other remains unmerged and MergeContinue + // must refuse to commit. + fixture, err := gittest.LoadFixtureScript([]byte(text.Dedent(` + as 'Test ' + at '2025-01-01T00:00:00Z' + + git init + git add a.txt b.txt + git commit -m 'Initial commit' + + git checkout -b feature + cp a-feature.txt a.txt + cp b-feature.txt b.txt + git add a.txt b.txt + git commit -m 'feature edits a and b' + + git checkout main + cp a-main.txt a.txt + cp b-main.txt b.txt + git add a.txt b.txt + git commit -m 'main edits a and b' + + -- a.txt -- + base a + -- b.txt -- + base b + -- a-feature.txt -- + feature a + -- b-feature.txt -- + feature b + -- a-main.txt -- + main a + -- b-main.txt -- + main b + `))) + require.NoError(t, err) + t.Cleanup(fixture.Cleanup) + + wt, err := git.OpenWorktree(t.Context(), fixture.Dir(), git.OpenOptions{ + Log: silogtest.New(t), + }) + require.NoError(t, err) + + mergeErr := wt.Merge(t.Context(), git.MergeOptions{ + Refs: []string{"feature"}, + NoFF: true, + Message: "Merge feature", + LeaveConflict: true, + }) + require.Error(t, mergeErr) + require.True(t, errors.As(mergeErr, new(*git.MergeConflictError))) + + // Resolve only a.txt; leave b.txt unresolved. + require.NoError(t, + os.WriteFile(filepath.Join(fixture.Dir(), "a.txt"), + []byte("resolved a\n"), 0o600)) + + err = wt.MergeContinue(t.Context(), + []string{"a.txt"}, "Merge feature") + require.Error(t, err) + assert.Contains(t, err.Error(), "unmerged paths remain") + + require.NoError(t, wt.MergeAbort(t.Context())) +} + func TestWorktree_Merge_noRefs(t *testing.T) { t.Parallel() diff --git a/internal/handler/integration/handler.go b/internal/handler/integration/handler.go index 1a458422..f18abb16 100644 --- a/internal/handler/integration/handler.go +++ b/internal/handler/integration/handler.go @@ -12,14 +12,17 @@ import ( "fmt" "iter" "slices" + "strings" "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/scriptrun" "go.abhg.dev/gs/internal/silog" "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/spicedir" "go.abhg.dev/gs/internal/spice/state" ) -//go:generate mockgen -typed -destination mocks_test.go -package integration -write_package_comment=false . GitRepository,GitWorktree,Store,Service +//go:generate mockgen -typed -destination mocks_test.go -package integration -write_package_comment=false . GitRepository,GitWorktree,Store,Service,Resolver,QuestionPrompter // GitRepository is the subset of [git.Repository] used by the handler. type GitRepository interface { @@ -37,6 +40,7 @@ type GitWorktree interface { CheckoutBranch(ctx context.Context, branch string) error CheckoutNewBranch(ctx context.Context, req git.CheckoutNewBranchRequest) error Merge(ctx context.Context, opts git.MergeOptions) error + MergeContinue(ctx context.Context, paths []string, message string) error IsClean(ctx context.Context) (bool, error) Push(ctx context.Context, opts git.PushOptions) error } @@ -70,6 +74,44 @@ type Handler struct { Worktree GitWorktree // required Store Store // required Service Service // required + + // Resolver is invoked when a merge conflicts and auto-resolve is + // enabled. nil means no resolver is configured; conflicts surface + // normally. + Resolver Resolver + + // Prompter collects user answers when the resolver returns + // questions. nil disables the question-iteration loop (questions + // become an immediate error). + Prompter QuestionPrompter + + // DefaultAutoResolve sets the default behavior when + // [RebuildOptions.AutoResolve] is nil. Typically populated from + // spice.integration.autoResolve. + DefaultAutoResolve bool + + // RepoRoot is the directory containing the resolution file. + RepoRoot string + + // MaxResolveIterations bounds how many times the resolver may be + // invoked for a single tip merge. Typically populated from + // spice.scriptResolve.maxIterations via + // Config.ScriptResolveMaxIterations. A non-positive value falls + // back to the package default. + MaxResolveIterations int +} + +// defaultMaxResolveIterations is the fallback when +// Handler.MaxResolveIterations is non-positive. Matches +// DefaultScriptResolveMaxIterations in internal/spice. +const defaultMaxResolveIterations = 10 + +// resolveIterationCap returns the effective per-tip iteration cap. +func (h *Handler) resolveIterationCap() int { + if h.MaxResolveIterations > 0 { + return h.MaxResolveIterations + } + return defaultMaxResolveIterations } // ErrNotConfigured indicates that no integration branch is configured. @@ -300,6 +342,14 @@ func (e *ConflictError) Error() string { return fmt.Sprintf("merge of tip %q conflicted in %d file(s)", e.Tip, len(e.Paths)) } +// RebuildOptions allows callers to override per-invocation behavior. +type RebuildOptions struct { + // AutoResolve, if non-nil, overrides [Handler.DefaultAutoResolve] + // for this invocation. A true value enables the configured + // resolver; a false value disables it even when configured. + AutoResolve *bool +} + // Rebuild regenerates the integration branch by sequentially merging // each configured tip onto trunk. // @@ -310,7 +360,15 @@ func (e *ConflictError) Error() string { // On conflict, the merge is left in the worktree for the user to // resolve, and a [*ConflictError] is returned. After resolving and // committing the merge, the user re-runs Rebuild to continue. -func (h *Handler) Rebuild(ctx context.Context) (*RebuildResult, error) { +// +// If a resolver is configured and auto-resolve is enabled (via opts +// or [Handler.DefaultAutoResolve]), Rebuild attempts to resolve +// conflicts automatically before surfacing them. +// +// opts may be nil to accept defaults. +func (h *Handler) Rebuild( + ctx context.Context, opts *RebuildOptions, +) (*RebuildResult, error) { info, err := h.loadConfigured(ctx) if err != nil { return nil, err @@ -337,9 +395,9 @@ func (h *Handler) Rebuild(ctx context.Context) (*RebuildResult, error) { } if pending != nil { - return h.resumeRebuild(ctx, info, pending) + return h.resumeRebuild(ctx, info, pending, opts) } - return h.freshRebuild(ctx, info) + return h.freshRebuild(ctx, info, opts) } // ensureWorktreeSafe refuses to run an integration rebuild in a worktree @@ -411,7 +469,19 @@ func (h *Handler) ensureWorktreeSafe( return nil } -func (h *Handler) freshRebuild(ctx context.Context, info *state.IntegrationInfo) (*RebuildResult, error) { +// shouldAutoResolve resolves opts against DefaultAutoResolve. +func (h *Handler) shouldAutoResolve(opts *RebuildOptions) bool { + if opts != nil && opts.AutoResolve != nil { + return *opts.AutoResolve + } + return h.DefaultAutoResolve +} + +func (h *Handler) freshRebuild( + ctx context.Context, + info *state.IntegrationInfo, + opts *RebuildOptions, +) (*RebuildResult, error) { currentBranch, err := h.Worktree.CurrentBranch(ctx) if err != nil { return nil, fmt.Errorf("current branch: %w", err) @@ -451,13 +521,14 @@ func (h *Handler) freshRebuild(ctx context.Context, info *state.IntegrationInfo) return nil, fmt.Errorf("create integration branch: %w", err) } - return h.runMerges(ctx, info, tips, 0, currentBranch) + return h.runMerges(ctx, info, tips, 0, currentBranch, opts) } func (h *Handler) resumeRebuild( ctx context.Context, info *state.IntegrationInfo, pending *state.IntegrationRebuild, + opts *RebuildOptions, ) (*RebuildResult, error) { clean, err := h.Worktree.IsClean(ctx) if err != nil { @@ -479,25 +550,32 @@ func (h *Handler) resumeRebuild( h.Log.Infof("Resuming integration rebuild at tip %d of %d", pending.NextTipIndex+1, len(pending.Tips)) - return h.runMerges(ctx, info, pending.Tips, pending.NextTipIndex, currentBranch) + return h.runMerges(ctx, info, pending.Tips, pending.NextTipIndex, currentBranch, opts) } // runMerges merges tips[start:] onto HEAD, finalizes the rebuild on // success, and saves pending state + returns a [*ConflictError] on // conflict (without aborting the merge). +// +// If auto-resolve is enabled for this invocation and a resolver is +// configured, conflicts are passed to the resolver before being +// surfaced. A successful resolve continues to the next tip; a failed +// one falls through to the original conflict-surfacing path. func (h *Handler) runMerges( ctx context.Context, info *state.IntegrationInfo, tips []state.IntegrationTip, start int, originalBranch string, + opts *RebuildOptions, ) (*RebuildResult, error) { for i := start; i < len(tips); i++ { tip := tips[i] + mergeMsg := fmt.Sprintf("Merge %s into %s", tip.Name, info.Name) err := h.Worktree.Merge(ctx, git.MergeOptions{ Refs: []string{tip.Hash.String()}, NoFF: true, - Message: fmt.Sprintf("Merge %s into %s", tip.Name, info.Name), + Message: mergeMsg, EnableRerere: true, LeaveConflict: true, }) @@ -506,17 +584,32 @@ func (h *Handler) runMerges( } conflict := new(git.MergeConflictError) - if errors.As(err, &conflict) { - if saveErr := h.Store.SetPendingIntegrationRebuild(ctx, &state.IntegrationRebuild{ - Integration: info.Name, - Tips: tips, - NextTipIndex: i + 1, - }); saveErr != nil { - h.Log.Warnf("save pending rebuild: %v", saveErr) + if !errors.As(err, &conflict) { + return nil, fmt.Errorf("merge tip %q: %w", tip.Name, err) + } + + // Try the auto-resolver if enabled and configured. + if h.shouldAutoResolve(opts) && h.Resolver != nil { + resolved, resolveErr := h.autoResolveLoop( + ctx, info.Name, tip.Name, conflict.ConflictPaths, mergeMsg) + switch { + case resolveErr != nil: + h.Log.Warnf("Auto-resolve failed for tip %q: %v", + tip.Name, resolveErr) + // fall through to conflict-surfacing + case resolved: + continue } - return nil, &ConflictError{Tip: tip.Name, Paths: conflict.ConflictPaths} } - return nil, fmt.Errorf("merge tip %q: %w", tip.Name, err) + + if saveErr := h.Store.SetPendingIntegrationRebuild(ctx, &state.IntegrationRebuild{ + Integration: info.Name, + Tips: tips, + NextTipIndex: i + 1, + }); saveErr != nil { + h.Log.Warnf("save pending rebuild: %v", saveErr) + } + return nil, &ConflictError{Tip: tip.Name, Paths: conflict.ConflictPaths} } info.Tips = tips @@ -747,7 +840,7 @@ func (h *Handler) MaybeRebuild(ctx context.Context) error { } h.Log.Infof("Rebuilding integration branch %q", info.Name) - if _, err := h.Rebuild(ctx); err != nil { + if _, err := h.Rebuild(ctx, nil); err != nil { conflict := new(git.MergeConflictError) if errors.As(err, &conflict) { h.Log.Warnf("Integration rebuild failed: %v", err) @@ -758,6 +851,162 @@ func (h *Handler) MaybeRebuild(ctx context.Context) error { return nil } +// autoResolveLoop drives the resolver iteration for a single tip merge. +// Returns resolved=true if the merge was completed automatically. +// +// On resolver error, partial resolution, or iteration-cap hit, returns +// resolved=false along with an error describing the failure. +func (h *Handler) autoResolveLoop( + ctx context.Context, + integrationName, tipName string, + conflictPaths []string, + mergeMsg string, +) (resolved bool, err error) { + req := &ResolveRequest{ + IntegrationName: integrationName, + TipName: tipName, + } + + maxIters := h.resolveIterationCap() + for iter := range maxIters { + resp, resErr := h.Resolver.Resolve(ctx, req) + if resErr != nil { + return false, fmt.Errorf("resolver: %w", resErr) + } + + for _, a := range resp.Assumptions { + h.Log.Infof("Auto-resolve: %s", a) + } + + if len(resp.Questions) > 0 { + if h.Prompter == nil { + return false, fmt.Errorf( + "resolver returned %d question(s) but no prompter is configured", + len(resp.Questions)) + } + answers, askErr := h.Prompter.AskAnswers(ctx, resp.Questions) + if askErr != nil { + return false, fmt.Errorf("collect answers: %w", askErr) + } + if err := h.appendQAToFile(integrationName, tipName, + resp.Questions, answers); err != nil { + return false, fmt.Errorf("append Q&A: %w", err) + } + _ = iter + continue + } + + if len(resp.UnresolvedFiles) > 0 { + return false, fmt.Errorf( + "resolver reported unresolved files with no questions: %s", + strings.Join(resp.UnresolvedFiles, ", ")) + } + + // All resolved. Commit the merge. + if err := h.Worktree.MergeContinue(ctx, conflictPaths, mergeMsg); err != nil { + return false, fmt.Errorf("commit merge: %w", err) + } + return true, nil + } + + return false, fmt.Errorf( + "resolver exceeded iteration cap (%d); investigate manually", + maxIters) +} + +// appendQAToFile appends the given question/answer pairs to the +// resolution file's entry for (ours, theirs). +func (h *Handler) appendQAToFile( + ours, theirs string, questions, answers []string, +) error { + if err := spicedir.EnsureResolutionsDir(h.RepoRoot); err != nil { + return err + } + path := spicedir.ResolutionPath(h.RepoRoot, ResolutionFeatureName) + file, err := LoadResolutionFile(path) + if err != nil { + return err + } + + pair := MergePair{Ours: ours, Theirs: theirs} + qa := make([]scriptrun.QAPair, 0, len(questions)) + for i, q := range questions { + a := "" + if i < len(answers) { + a = answers[i] + } + qa = append(qa, scriptrun.QAPair{Question: q, Answer: a}) + } + file.AppendInstructions(pair, qa...) + return file.Save(path) +} + +// OnBranchRemoved prunes references to the removed branch: +// - resolution-file entries that name it +// - the integration's configured tip list, if it appears there +// +// Used as a hook from branch_untrack, branch_delete, and +// repo_sync's cleanup of merged branches. Skipping the tip-list +// prune would leave a dangling tip name in state, causing the +// next 'gs integration rebuild' to fail with "resolve head: does +// not exist" on a branch the user already chose to delete. +// +// Errors are returned; callers may choose to log them as warnings. +func (h *Handler) OnBranchRemoved(ctx context.Context, branch string) error { + if err := h.pruneTipFromIntegration(ctx, branch); err != nil { + return err + } + + if h.RepoRoot == "" { + // Cannot prune resolution file without a known root; treat as no-op. + return nil + } + path := spicedir.ResolutionPath(h.RepoRoot, ResolutionFeatureName) + + file, err := LoadResolutionFile(path) + if err != nil { + return err + } + if file.PruneBranch(branch) == 0 { + return nil + } + return file.Save(path) +} + +// pruneTipFromIntegration removes branch from the integration's +// configured tip list if it appears there. No-op when no +// integration is configured, when the branch is not a tip, or +// when state is otherwise unreadable. +func (h *Handler) pruneTipFromIntegration( + ctx context.Context, branch string, +) error { + info, err := h.Store.Integration(ctx) + if errors.Is(err, state.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("get integration: %w", err) + } + + idx := slices.IndexFunc(info.Tips, func(t state.IntegrationTip) bool { + return t.Name == branch + }) + if idx < 0 { + return nil + } + + info.Tips = slices.Delete(info.Tips, idx, idx+1) + if err := h.Store.SetIntegration(ctx, info); err != nil { + return fmt.Errorf("save integration: %w", err) + } + + h.Log.Infof( + "Removed %q from integration tips (branch was removed).", + branch, + ) + return nil +} + // MaybeRebuildAndSubmit invokes [MaybeRebuild] and, if the integration // has previously been pushed (non-empty LastPushedHash), also submits. // diff --git a/internal/handler/integration/handler_test.go b/internal/handler/integration/handler_test.go index 8a7afe8b..aaff20fa 100644 --- a/internal/handler/integration/handler_test.go +++ b/internal/handler/integration/handler_test.go @@ -8,8 +8,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/scriptrun" "go.abhg.dev/gs/internal/silog/silogtest" "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/spicedir" "go.abhg.dev/gs/internal/spice/state" gomock "go.uber.org/mock/gomock" ) @@ -422,7 +424,7 @@ func TestHandler_Rebuild(t *testing.T) { CheckoutBranch(gomock.Any(), "main"). Return(nil) - res, err := h.Rebuild(t.Context()) + res, err := h.Rebuild(t.Context(), nil) require.NoError(t, err) assert.Equal(t, "preview", res.Name) assert.Equal(t, []git.Hash{"hash-a", "hash-b"}, res.TipHashes) @@ -475,7 +477,7 @@ func TestHandler_Rebuild(t *testing.T) { // No final CheckoutBranch call expected since we started on // the integration branch already. - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.NoError(t, err) }) @@ -494,7 +496,7 @@ func TestHandler_Rebuild(t *testing.T) { IsClean(gomock.Any()). Return(false, nil) - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.Error(t, err) assert.Contains(t, err.Error(), "uncommitted") }) @@ -558,7 +560,7 @@ func TestHandler_Rebuild(t *testing.T) { Return(nil) // No CheckoutBranch: the conflict is left in the worktree. - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.Error(t, err) var conflict *ConflictError assert.True(t, errors.As(err, &conflict)) @@ -611,7 +613,7 @@ func TestHandler_Rebuild(t *testing.T) { ClearPendingIntegrationRebuild(gomock.Any()). Return(nil) - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.NoError(t, err) }) @@ -628,7 +630,7 @@ func TestHandler_Rebuild(t *testing.T) { &git.WorktreeListItem{Path: "/wt/preview", Branch: "preview"}, )) - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.Error(t, err) assert.Contains(t, err.Error(), "another worktree") }) @@ -650,12 +652,368 @@ func TestHandler_Rebuild(t *testing.T) { LookupBranch(gomock.Any(), "feat-x"). Return(&spice.LookupBranchResponse{}, nil) - _, err := h.Rebuild(t.Context()) + _, err := h.Rebuild(t.Context(), nil) require.Error(t, err) assert.Contains(t, err.Error(), "would be reverted") }) } +// boolPtr returns a *bool with the given value, for RebuildOptions. +// +//nolint:unparam // helper kept for symmetry across true/false call sites +func boolPtr(b bool) *bool { + p := new(bool) + *p = b + return p +} + +// newHandlerWithResolver returns a handler set up for auto-resolve tests: +// resolver + prompter mocks + repo root in a temp dir. +func newHandlerWithResolver(t *testing.T) (*Handler, *handlerMocks, *MockResolver, *MockQuestionPrompter) { + t.Helper() + mockCtrl := gomock.NewController(t) + mocks := &handlerMocks{ + Repository: NewMockGitRepository(mockCtrl), + Worktree: NewMockGitWorktree(mockCtrl), + Store: NewMockStore(mockCtrl), + Service: NewMockService(mockCtrl), + } + resolver := NewMockResolver(mockCtrl) + prompter := NewMockQuestionPrompter(mockCtrl) + h := &Handler{ + Log: silogtest.New(t), + Repository: mocks.Repository, + Worktree: mocks.Worktree, + Store: mocks.Store, + Service: mocks.Service, + Resolver: resolver, + Prompter: prompter, + DefaultAutoResolve: false, + RepoRoot: t.TempDir(), + } + expectBorrowableWorktree(mocks) + return h, mocks, resolver, prompter +} + +// setupConflictMerge primes the mocks for a fresh rebuild with a single +// tip that conflicts. Returns the merge message gs will pass to the +// resolver and MergeContinue. +func setupConflictMerge(t *testing.T, mocks *handlerMocks) string { + t.Helper() + info := &state.IntegrationInfo{ + Name: "preview", + Tips: []state.IntegrationTip{{Name: "feat-a"}}, + } + mocks.Store.EXPECT().Integration(gomock.Any()).Return(info, nil) + mocks.Store.EXPECT(). + PendingIntegrationRebuild(gomock.Any()). + Return(nil, state.ErrNotExist) + mocks.Worktree.EXPECT().CurrentBranch(gomock.Any()).Return("main", nil) + mocks.Worktree.EXPECT().IsClean(gomock.Any()).Return(true, nil) + mocks.Store.EXPECT().Trunk().Return("main").AnyTimes() + mocks.Repository.EXPECT(). + PeelToCommit(gomock.Any(), "main"). + Return(git.Hash("trunk-hash"), nil) + mocks.Service.EXPECT(). + LookupBranch(gomock.Any(), "feat-a"). + Return(&spice.LookupBranchResponse{}, nil) + mocks.Repository.EXPECT(). + PeelToCommit(gomock.Any(), "feat-a"). + Return(git.Hash("hash-a"), nil) + mocks.Worktree.EXPECT(). + CheckoutNewBranch(gomock.Any(), gomock.Any()). + Return(nil) + mocks.Worktree.EXPECT(). + Merge(gomock.Any(), gomock.Any()). + Return(&git.MergeConflictError{ + Refs: []string{"hash-a"}, + ConflictPaths: []string{"shared.txt"}, + }) + return "Merge feat-a into preview" +} + +func TestHandler_Rebuild_autoResolveSuccess(t *testing.T) { + h, mocks, resolver, _ := newHandlerWithResolver(t) + mergeMsg := setupConflictMerge(t, mocks) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Cond(func(req *ResolveRequest) bool { + return req.IntegrationName == "preview" && + req.TipName == "feat-a" + })). + Return(&scriptrun.ResolveResponse{}, nil) + mocks.Worktree.EXPECT(). + MergeContinue(gomock.Any(), []string{"shared.txt"}, mergeMsg). + Return(nil) + + mocks.Store.EXPECT(). + SetIntegration(gomock.Any(), gomock.Any()). + Return(nil) + mocks.Store.EXPECT(). + ClearPendingIntegrationRebuild(gomock.Any()). + Return(nil) + mocks.Worktree.EXPECT(). + CheckoutBranch(gomock.Any(), "main"). + Return(nil) + + res, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.NoError(t, err) + assert.Equal(t, "preview", res.Name) +} + +func TestHandler_Rebuild_autoResolveQuestions(t *testing.T) { + h, mocks, resolver, prompter := newHandlerWithResolver(t) + mergeMsg := setupConflictMerge(t, mocks) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(&scriptrun.ResolveResponse{ + Questions: []string{"Should feat-a win?"}, + }, nil) + prompter.EXPECT(). + AskAnswers(gomock.Any(), []string{"Should feat-a win?"}). + Return([]string{"yes"}, nil) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(&scriptrun.ResolveResponse{}, nil) + mocks.Worktree.EXPECT(). + MergeContinue(gomock.Any(), []string{"shared.txt"}, mergeMsg). + Return(nil) + + mocks.Store.EXPECT(). + SetIntegration(gomock.Any(), gomock.Any()). + Return(nil) + mocks.Store.EXPECT(). + ClearPendingIntegrationRebuild(gomock.Any()). + Return(nil) + mocks.Worktree.EXPECT(). + CheckoutBranch(gomock.Any(), "main"). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.NoError(t, err) + + file, err := LoadResolutionFile(spicedir.ResolutionPath(h.RepoRoot, ResolutionFeatureName)) + require.NoError(t, err) + require.Len(t, file.Resolutions, 1) + require.Len(t, file.Resolutions[0].ResolutionInstructions, 1) + assert.Equal(t, "Should feat-a win?", + file.Resolutions[0].ResolutionInstructions[0].Question) + assert.Equal(t, "yes", + file.Resolutions[0].ResolutionInstructions[0].Answer) +} + +func TestHandler_Rebuild_autoResolveUnresolvedNoQuestions(t *testing.T) { + h, mocks, resolver, _ := newHandlerWithResolver(t) + setupConflictMerge(t, mocks) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(&scriptrun.ResolveResponse{ + UnresolvedFiles: []string{"shared.txt"}, + }, nil) + + mocks.Store.EXPECT(). + SetPendingIntegrationRebuild(gomock.Any(), gomock.Any()). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.Error(t, err) + var conflictErr *ConflictError + require.True(t, errors.As(err, &conflictErr)) + assert.Equal(t, "feat-a", conflictErr.Tip) +} + +func TestHandler_Rebuild_autoResolveResolverError(t *testing.T) { + h, mocks, resolver, _ := newHandlerWithResolver(t) + setupConflictMerge(t, mocks) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(nil, errors.New("resolver crashed")) + + mocks.Store.EXPECT(). + SetPendingIntegrationRebuild(gomock.Any(), gomock.Any()). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.Error(t, err) + var conflictErr *ConflictError + require.True(t, errors.As(err, &conflictErr)) +} + +func TestHandler_Rebuild_autoResolveDisabledByOpts(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + h.DefaultAutoResolve = true + setupConflictMerge(t, mocks) + + mocks.Store.EXPECT(). + SetPendingIntegrationRebuild(gomock.Any(), gomock.Any()). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(false)}) + require.Error(t, err) + var conflictErr *ConflictError + require.True(t, errors.As(err, &conflictErr)) +} + +func TestHandler_Rebuild_autoResolveDisabledByDefault(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + setupConflictMerge(t, mocks) + + mocks.Store.EXPECT(). + SetPendingIntegrationRebuild(gomock.Any(), gomock.Any()). + Return(nil) + + _, err := h.Rebuild(t.Context(), nil) + require.Error(t, err) + var conflictErr *ConflictError + require.True(t, errors.As(err, &conflictErr)) +} + +func TestHandler_Rebuild_autoResolveIterationCap(t *testing.T) { + h, mocks, resolver, prompter := newHandlerWithResolver(t) + setupConflictMerge(t, mocks) + + for range defaultMaxResolveIterations { + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(&scriptrun.ResolveResponse{ + Questions: []string{"stuck question"}, + }, nil) + prompter.EXPECT(). + AskAnswers(gomock.Any(), gomock.Any()). + Return([]string{"some answer"}, nil) + } + + mocks.Store.EXPECT(). + SetPendingIntegrationRebuild(gomock.Any(), gomock.Any()). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.Error(t, err) + var conflictErr *ConflictError + require.True(t, errors.As(err, &conflictErr)) +} + +func TestHandler_Rebuild_autoResolveAssumptions(t *testing.T) { + h, mocks, resolver, _ := newHandlerWithResolver(t) + mergeMsg := setupConflictMerge(t, mocks) + + resolver.EXPECT(). + Resolve(gomock.Any(), gomock.Any()). + Return(&scriptrun.ResolveResponse{ + Assumptions: []string{"picked feat-a per commit timestamp"}, + }, nil) + mocks.Worktree.EXPECT(). + MergeContinue(gomock.Any(), gomock.Any(), mergeMsg). + Return(nil) + + mocks.Store.EXPECT(). + SetIntegration(gomock.Any(), gomock.Any()). + Return(nil) + mocks.Store.EXPECT(). + ClearPendingIntegrationRebuild(gomock.Any()). + Return(nil) + mocks.Worktree.EXPECT(). + CheckoutBranch(gomock.Any(), "main"). + Return(nil) + + _, err := h.Rebuild(t.Context(), &RebuildOptions{AutoResolve: boolPtr(true)}) + require.NoError(t, err) +} + +func TestHandler_OnBranchRemoved(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + // No integration configured -> tip pruning is a no-op. + mocks.Store.EXPECT(). + Integration(gomock.Any()). + Return(nil, state.ErrNotExist) + + path := spicedir.ResolutionPath(h.RepoRoot, ResolutionFeatureName) + seed := &ResolutionFile{ + Resolutions: []ResolutionEntry{ + {MergingBranches: MergePair{Ours: "preview", Theirs: "feat-a"}}, + {MergingBranches: MergePair{Ours: "preview", Theirs: "feat-b"}}, + }, + } + require.NoError(t, seed.Save(path)) + + require.NoError(t, h.OnBranchRemoved(t.Context(), "feat-a")) + + file, err := LoadResolutionFile(path) + require.NoError(t, err) + require.Len(t, file.Resolutions, 1) + assert.Equal(t, "feat-b", file.Resolutions[0].MergingBranches.Theirs) +} + +func TestHandler_OnBranchRemoved_noFile(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + mocks.Store.EXPECT(). + Integration(gomock.Any()). + Return(nil, state.ErrNotExist) + require.NoError(t, h.OnBranchRemoved(t.Context(), "feat-a")) +} + +func TestHandler_OnBranchRemoved_noMatchingEntries(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + mocks.Store.EXPECT(). + Integration(gomock.Any()). + Return(nil, state.ErrNotExist) + + path := spicedir.ResolutionPath(h.RepoRoot, ResolutionFeatureName) + seed := &ResolutionFile{ + Resolutions: []ResolutionEntry{ + {MergingBranches: MergePair{Ours: "preview", Theirs: "feat-a"}}, + }, + } + require.NoError(t, seed.Save(path)) + + require.NoError(t, h.OnBranchRemoved(t.Context(), "ghost")) + + file, err := LoadResolutionFile(path) + require.NoError(t, err) + assert.Len(t, file.Resolutions, 1) +} + +func TestHandler_OnBranchRemoved_prunesTip(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + + info := &state.IntegrationInfo{ + Name: "preview", + Tips: []state.IntegrationTip{ + {Name: "feat-a"}, + {Name: "feat-b"}, + }, + } + mocks.Store.EXPECT(). + Integration(gomock.Any()). + Return(info, nil) + mocks.Store.EXPECT(). + SetIntegration(gomock.Any(), &state.IntegrationInfo{ + Name: "preview", + Tips: []state.IntegrationTip{{Name: "feat-b"}}, + }). + Return(nil) + + require.NoError(t, h.OnBranchRemoved(t.Context(), "feat-a")) +} + +func TestHandler_OnBranchRemoved_branchNotATip(t *testing.T) { + h, mocks, _, _ := newHandlerWithResolver(t) + + mocks.Store.EXPECT(). + Integration(gomock.Any()). + Return(&state.IntegrationInfo{ + Name: "preview", + Tips: []state.IntegrationTip{{Name: "feat-a"}}, + }, nil) + // SetIntegration should NOT be called. + + require.NoError(t, h.OnBranchRemoved(t.Context(), "ghost")) +} + func TestHandler_Submit(t *testing.T) { t.Run("ForceWithLease", func(t *testing.T) { h, mocks := newHandler(t) diff --git a/internal/handler/integration/mocks_test.go b/internal/handler/integration/mocks_test.go index 928ef7a5..a63c33d1 100644 --- a/internal/handler/integration/mocks_test.go +++ b/internal/handler/integration/mocks_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: go.abhg.dev/gs/internal/handler/integration (interfaces: GitRepository,GitWorktree,Store,Service) +// Source: go.abhg.dev/gs/internal/handler/integration (interfaces: GitRepository,GitWorktree,Store,Service,Resolver,QuestionPrompter) // // Generated by this command: // -// mockgen -typed -destination mocks_test.go -package integration -write_package_comment=false . GitRepository,GitWorktree,Store,Service +// mockgen -typed -destination mocks_test.go -package integration -write_package_comment=false . GitRepository,GitWorktree,Store,Service,Resolver,QuestionPrompter // package integration @@ -14,6 +14,7 @@ import ( reflect "reflect" git "go.abhg.dev/gs/internal/git" + scriptrun "go.abhg.dev/gs/internal/scriptrun" spice "go.abhg.dev/gs/internal/spice" state "go.abhg.dev/gs/internal/spice/state" gomock "go.uber.org/mock/gomock" @@ -374,6 +375,44 @@ func (c *MockGitWorktreeMergeCall) DoAndReturn(f func(context.Context, git.Merge return c } +// MergeContinue mocks base method. +func (m *MockGitWorktree) MergeContinue(ctx context.Context, paths []string, message string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MergeContinue", ctx, paths, message) + ret0, _ := ret[0].(error) + return ret0 +} + +// MergeContinue indicates an expected call of MergeContinue. +func (mr *MockGitWorktreeMockRecorder) MergeContinue(ctx, paths, message any) *MockGitWorktreeMergeContinueCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MergeContinue", reflect.TypeOf((*MockGitWorktree)(nil).MergeContinue), ctx, paths, message) + return &MockGitWorktreeMergeContinueCall{Call: call} +} + +// MockGitWorktreeMergeContinueCall wrap *gomock.Call +type MockGitWorktreeMergeContinueCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockGitWorktreeMergeContinueCall) Return(arg0 error) *MockGitWorktreeMergeContinueCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockGitWorktreeMergeContinueCall) Do(f func(context.Context, []string, string) error) *MockGitWorktreeMergeContinueCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockGitWorktreeMergeContinueCall) DoAndReturn(f func(context.Context, []string, string) error) *MockGitWorktreeMergeContinueCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // Push mocks base method. func (m *MockGitWorktree) Push(ctx context.Context, opts git.PushOptions) error { m.ctrl.T.Helper() @@ -805,3 +844,129 @@ func (c *MockServiceLookupBranchCall) DoAndReturn(f func(context.Context, string c.Call = c.Call.DoAndReturn(f) return c } + +// MockResolver is a mock of Resolver interface. +type MockResolver struct { + ctrl *gomock.Controller + recorder *MockResolverMockRecorder + isgomock struct{} +} + +// MockResolverMockRecorder is the mock recorder for MockResolver. +type MockResolverMockRecorder struct { + mock *MockResolver +} + +// NewMockResolver creates a new mock instance. +func NewMockResolver(ctrl *gomock.Controller) *MockResolver { + mock := &MockResolver{ctrl: ctrl} + mock.recorder = &MockResolverMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockResolver) EXPECT() *MockResolverMockRecorder { + return m.recorder +} + +// Resolve mocks base method. +func (m *MockResolver) Resolve(ctx context.Context, req *ResolveRequest) (*scriptrun.ResolveResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Resolve", ctx, req) + ret0, _ := ret[0].(*scriptrun.ResolveResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Resolve indicates an expected call of Resolve. +func (mr *MockResolverMockRecorder) Resolve(ctx, req any) *MockResolverResolveCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Resolve", reflect.TypeOf((*MockResolver)(nil).Resolve), ctx, req) + return &MockResolverResolveCall{Call: call} +} + +// MockResolverResolveCall wrap *gomock.Call +type MockResolverResolveCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockResolverResolveCall) Return(arg0 *scriptrun.ResolveResponse, arg1 error) *MockResolverResolveCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockResolverResolveCall) Do(f func(context.Context, *ResolveRequest) (*scriptrun.ResolveResponse, error)) *MockResolverResolveCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockResolverResolveCall) DoAndReturn(f func(context.Context, *ResolveRequest) (*scriptrun.ResolveResponse, error)) *MockResolverResolveCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockQuestionPrompter is a mock of QuestionPrompter interface. +type MockQuestionPrompter struct { + ctrl *gomock.Controller + recorder *MockQuestionPrompterMockRecorder + isgomock struct{} +} + +// MockQuestionPrompterMockRecorder is the mock recorder for MockQuestionPrompter. +type MockQuestionPrompterMockRecorder struct { + mock *MockQuestionPrompter +} + +// NewMockQuestionPrompter creates a new mock instance. +func NewMockQuestionPrompter(ctrl *gomock.Controller) *MockQuestionPrompter { + mock := &MockQuestionPrompter{ctrl: ctrl} + mock.recorder = &MockQuestionPrompterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockQuestionPrompter) EXPECT() *MockQuestionPrompterMockRecorder { + return m.recorder +} + +// AskAnswers mocks base method. +func (m *MockQuestionPrompter) AskAnswers(ctx context.Context, questions []string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AskAnswers", ctx, questions) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AskAnswers indicates an expected call of AskAnswers. +func (mr *MockQuestionPrompterMockRecorder) AskAnswers(ctx, questions any) *MockQuestionPrompterAskAnswersCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AskAnswers", reflect.TypeOf((*MockQuestionPrompter)(nil).AskAnswers), ctx, questions) + return &MockQuestionPrompterAskAnswersCall{Call: call} +} + +// MockQuestionPrompterAskAnswersCall wrap *gomock.Call +type MockQuestionPrompterAskAnswersCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockQuestionPrompterAskAnswersCall) Return(arg0 []string, arg1 error) *MockQuestionPrompterAskAnswersCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockQuestionPrompterAskAnswersCall) Do(f func(context.Context, []string) ([]string, error)) *MockQuestionPrompterAskAnswersCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockQuestionPrompterAskAnswersCall) DoAndReturn(f func(context.Context, []string) ([]string, error)) *MockQuestionPrompterAskAnswersCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/internal/handler/integration/qa.go b/internal/handler/integration/qa.go new file mode 100644 index 00000000..5cb12efc --- /dev/null +++ b/internal/handler/integration/qa.go @@ -0,0 +1,63 @@ +package integration + +import ( + "context" + "errors" + "fmt" + + "go.abhg.dev/gs/internal/ui" +) + +// QuestionPrompter collects answers from the user for a list of +// questions raised by the resolver. +// +// Implementations must return one answer per input question, in the +// same order. If the user aborts, return a non-nil error. +type QuestionPrompter interface { + AskAnswers(ctx context.Context, questions []string) ([]string, error) +} + +// ViewPrompter is a QuestionPrompter backed by a [ui.View]. +// +// If the view is not interactive (e.g., gs is being piped), +// AskAnswers returns an error. +type ViewPrompter struct { + View ui.View +} + +var _ QuestionPrompter = (*ViewPrompter)(nil) + +// NewViewPrompter wraps view as a QuestionPrompter. +func NewViewPrompter(view ui.View) *ViewPrompter { + return &ViewPrompter{View: view} +} + +// AskAnswers prompts for one answer per question. +// +// Questions are presented one at a time so each answer can inform +// the user's reading of the next prompt (questions may build on +// each other). +func (p *ViewPrompter) AskAnswers( + _ context.Context, questions []string, +) ([]string, error) { + if len(questions) == 0 { + return nil, nil + } + if !ui.Interactive(p.View) { + return nil, errors.New( + "cannot prompt for answers: not running in interactive mode") + } + + answers := make([]string, len(questions)) + for i, q := range questions { + input := ui.NewInput(). + WithTitle(fmt.Sprintf("Question %d of %d", i+1, len(questions))). + WithDescription(q). + WithValue(&answers[i]) + + if err := ui.Run(p.View, input); err != nil { + return nil, fmt.Errorf("prompt: %w", err) + } + } + return answers, nil +} diff --git a/internal/handler/integration/resolution_file.go b/internal/handler/integration/resolution_file.go new file mode 100644 index 00000000..84cd5809 --- /dev/null +++ b/internal/handler/integration/resolution_file.go @@ -0,0 +1,154 @@ +package integration + +import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "slices" + + "go.abhg.dev/gs/internal/scriptrun" +) + +// ResolutionFeatureName is the feature key used in the resolution +// file path: .spice/resolutions/integration.json. +const ResolutionFeatureName = "integration" + +// MergePair identifies a single in-progress merge by branch names. +type MergePair struct { + Ours string `json:"ours"` + Theirs string `json:"theirs"` +} + +// ResolutionEntry holds the accumulated resolution instructions for a +// specific (ours, theirs) pair. +type ResolutionEntry struct { + MergingBranches MergePair `json:"merging_branches"` + ResolutionInstructions []scriptrun.QAPair `json:"resolution_instructions"` +} + +// ResolutionFile is the on-disk format of the resolution state file. +// +// CurrentMerge is a transient pointer set by gs before each resolver +// invocation. Resolutions is the persistent history. +type ResolutionFile struct { + CurrentMerge *MergePair `json:"current_merge,omitempty"` + Resolutions []ResolutionEntry `json:"resolutions"` +} + +// LoadResolutionFile reads the resolution file at path. If the file +// does not exist, returns an empty ResolutionFile (not an error). +// +// Returns an error if the file exists but cannot be read or parsed. +func LoadResolutionFile(path string) (*ResolutionFile, error) { + data, err := os.ReadFile(path) + switch { + case errors.Is(err, fs.ErrNotExist): + return &ResolutionFile{Resolutions: []ResolutionEntry{}}, nil + case err != nil: + return nil, fmt.Errorf("read %s: %w", path, err) + } + + var f ResolutionFile + if err := json.Unmarshal(data, &f); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + if f.Resolutions == nil { + f.Resolutions = []ResolutionEntry{} + } + return &f, nil +} + +// Save writes the resolution file to path atomically: contents are +// written to a temporary file in the same directory and then renamed +// into place. The parent directory is created if missing. +func (f *ResolutionFile) Save(path string) error { + data, err := json.MarshalIndent(f, "", " ") + if err != nil { + return fmt.Errorf("marshal: %w", err) + } + data = append(data, '\n') + + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("create parent dir: %w", err) + } + tmp, err := os.CreateTemp(dir, "integration-resolution.*.json.tmp") + if err != nil { + return fmt.Errorf("create temp: %w", err) + } + tmpPath := tmp.Name() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + _ = os.Remove(tmpPath) + return fmt.Errorf("write temp: %w", err) + } + if err := tmp.Close(); err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("close temp: %w", err) + } + + if err := os.Rename(tmpPath, path); err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("rename: %w", err) + } + return nil +} + +// EntryFor returns the existing entry matching p, or nil if absent. +func (f *ResolutionFile) EntryFor(p MergePair) *ResolutionEntry { + for i := range f.Resolutions { + if f.Resolutions[i].MergingBranches == p { + return &f.Resolutions[i] + } + } + return nil +} + +// EnsureEntry returns the entry matching p, creating one with an empty +// instruction list if it does not already exist. +func (f *ResolutionFile) EnsureEntry(p MergePair) *ResolutionEntry { + if e := f.EntryFor(p); e != nil { + return e + } + f.Resolutions = append(f.Resolutions, ResolutionEntry{ + MergingBranches: p, + ResolutionInstructions: []scriptrun.QAPair{}, + }) + return &f.Resolutions[len(f.Resolutions)-1] +} + +// AppendInstructions appends Q&A pairs to the entry matching p. The +// entry is created if it does not exist. +func (f *ResolutionFile) AppendInstructions(p MergePair, qa ...scriptrun.QAPair) { + e := f.EnsureEntry(p) + e.ResolutionInstructions = append(e.ResolutionInstructions, qa...) +} + +// PruneBranch removes every entry where either side of merging_branches +// equals name. Returns the number of entries removed. +func (f *ResolutionFile) PruneBranch(name string) int { + before := len(f.Resolutions) + f.Resolutions = slices.DeleteFunc(f.Resolutions, + func(e ResolutionEntry) bool { + return e.MergingBranches.Ours == name || + e.MergingBranches.Theirs == name + }) + return before - len(f.Resolutions) +} + +// PruneStale removes every entry that references a branch not present +// in the provided set. Returns the number of entries removed. +func (f *ResolutionFile) PruneStale(tracked map[string]struct{}) int { + before := len(f.Resolutions) + f.Resolutions = slices.DeleteFunc(f.Resolutions, + func(e ResolutionEntry) bool { + _, oursOK := tracked[e.MergingBranches.Ours] + _, theirsOK := tracked[e.MergingBranches.Theirs] + return !oursOK || !theirsOK + }) + return before - len(f.Resolutions) +} diff --git a/internal/handler/integration/resolution_file_test.go b/internal/handler/integration/resolution_file_test.go new file mode 100644 index 00000000..b9217e21 --- /dev/null +++ b/internal/handler/integration/resolution_file_test.go @@ -0,0 +1,200 @@ +package integration_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/handler/integration" + "go.abhg.dev/gs/internal/scriptrun" + "go.abhg.dev/gs/internal/spice/spicedir" +) + +func TestLoadResolutionFile_absent(t *testing.T) { + dir := t.TempDir() + + f, err := integration.LoadResolutionFile( + filepath.Join(dir, "nonexistent.json")) + require.NoError(t, err) + assert.Equal(t, []integration.ResolutionEntry{}, f.Resolutions) + assert.Nil(t, f.CurrentMerge) +} + +func TestLoadResolutionFile_malformed(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "bad.json") + require.NoError(t, os.WriteFile(path, []byte("not json"), 0o600)) + + _, err := integration.LoadResolutionFile(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse") +} + +func TestLoadResolutionFile_roundTrip(t *testing.T) { + dir := t.TempDir() + path := spicedir.ResolutionPath(dir, integration.ResolutionFeatureName) + + want := &integration.ResolutionFile{ + CurrentMerge: &integration.MergePair{ + Ours: "preview", + Theirs: "feat-a", + }, + Resolutions: []integration.ResolutionEntry{ + { + MergingBranches: integration.MergePair{ + Ours: "preview", + Theirs: "feat-a", + }, + ResolutionInstructions: []scriptrun.QAPair{ + {Question: "q1", Answer: "a1"}, + {Question: "q2", Answer: "a2"}, + }, + }, + }, + } + require.NoError(t, want.Save(path)) + + got, err := integration.LoadResolutionFile(path) + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func TestResolutionFile_Save_atomic(t *testing.T) { + dir := t.TempDir() + path := spicedir.ResolutionPath(dir, integration.ResolutionFeatureName) + + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + { + MergingBranches: integration.MergePair{ + Ours: "a", + Theirs: "b", + }, + ResolutionInstructions: []scriptrun.QAPair{}, + }, + }, + } + require.NoError(t, f.Save(path)) + + // File exists, no leftover .tmp. + entries, err := os.ReadDir(dir) + require.NoError(t, err) + for _, e := range entries { + assert.NotContains(t, e.Name(), ".tmp", + "unexpected leftover temp file: %s", e.Name()) + } + + // File is parseable JSON. + data, err := os.ReadFile(path) + require.NoError(t, err) + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) +} + +func TestResolutionFile_EnsureEntry_idempotent(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{}, + } + + p := integration.MergePair{Ours: "x", Theirs: "y"} + e1 := f.EnsureEntry(p) + e2 := f.EnsureEntry(p) + assert.Same(t, e1, e2, + "second call should return the same entry instance") + assert.Len(t, f.Resolutions, 1) +} + +func TestResolutionFile_EntryFor(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + { + MergingBranches: integration.MergePair{ + Ours: "preview", + Theirs: "feat-a", + }, + ResolutionInstructions: []scriptrun.QAPair{ + {Question: "q", Answer: "a"}, + }, + }, + }, + } + + got := f.EntryFor(integration.MergePair{Ours: "preview", Theirs: "feat-a"}) + require.NotNil(t, got) + assert.Equal(t, "q", got.ResolutionInstructions[0].Question) + + missing := f.EntryFor(integration.MergePair{ + Ours: "preview", + Theirs: "feat-z", + }) + assert.Nil(t, missing) +} + +func TestResolutionFile_AppendInstructions(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{}, + } + p := integration.MergePair{Ours: "preview", Theirs: "feat-a"} + + f.AppendInstructions(p, + scriptrun.QAPair{Question: "q1", Answer: "a1"}) + f.AppendInstructions(p, + scriptrun.QAPair{Question: "q2", Answer: "a2"}) + + e := f.EntryFor(p) + require.NotNil(t, e) + assert.Len(t, e.ResolutionInstructions, 2) + assert.Equal(t, "q2", e.ResolutionInstructions[1].Question) +} + +func TestResolutionFile_PruneBranch(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + {MergingBranches: integration.MergePair{Ours: "preview", Theirs: "feat-a"}}, + {MergingBranches: integration.MergePair{Ours: "preview", Theirs: "feat-b"}}, + {MergingBranches: integration.MergePair{Ours: "other", Theirs: "feat-a"}}, + }, + } + + // feat-a appears as theirs in two entries. + removed := f.PruneBranch("feat-a") + assert.Equal(t, 2, removed) + assert.Len(t, f.Resolutions, 1) + assert.Equal(t, "feat-b", f.Resolutions[0].MergingBranches.Theirs) +} + +func TestResolutionFile_PruneBranch_onOursSide(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + {MergingBranches: integration.MergePair{Ours: "preview", Theirs: "feat-a"}}, + {MergingBranches: integration.MergePair{Ours: "other", Theirs: "feat-b"}}, + }, + } + + removed := f.PruneBranch("preview") + assert.Equal(t, 1, removed) + assert.Len(t, f.Resolutions, 1) + assert.Equal(t, "other", f.Resolutions[0].MergingBranches.Ours) +} + +func TestResolutionFile_PruneStale(t *testing.T) { + f := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + {MergingBranches: integration.MergePair{Ours: "preview", Theirs: "feat-a"}}, + {MergingBranches: integration.MergePair{Ours: "preview", Theirs: "ghost"}}, + {MergingBranches: integration.MergePair{Ours: "gone", Theirs: "feat-a"}}, + }, + } + + tracked := map[string]struct{}{ + "preview": {}, + "feat-a": {}, + } + removed := f.PruneStale(tracked) + assert.Equal(t, 2, removed) + require.Len(t, f.Resolutions, 1) + assert.Equal(t, "feat-a", f.Resolutions[0].MergingBranches.Theirs) +} diff --git a/internal/handler/integration/resolver.go b/internal/handler/integration/resolver.go new file mode 100644 index 00000000..fefc19e5 --- /dev/null +++ b/internal/handler/integration/resolver.go @@ -0,0 +1,160 @@ +package integration + +import ( + "context" + "errors" + "fmt" + + "go.abhg.dev/gs/internal/scriptrun" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice/spicedir" +) + +// Resolver attempts to resolve the in-progress merge conflict. +// +// Successful return indicates the resolver ran to completion with a +// well-formed response. The shape of the response indicates whether +// the conflict was actually resolved. +type Resolver interface { + Resolve(ctx context.Context, req *ResolveRequest) (*scriptrun.ResolveResponse, error) +} + +// ResolveRequest describes the conflict that the resolver should +// attempt to resolve. +type ResolveRequest struct { + // IntegrationName is the local branch name of the integration + // branch (also known as "ours" during the in-progress merge). + IntegrationName string + + // TipName is the branch name being merged into the integration + // branch (also known as "theirs"). + TipName string +} + +// ScriptRunner is the subset of [scriptrun.Runner] used by the +// resolver. It is named so tests can supply a fake. +type ScriptRunner interface { + Run(ctx context.Context, req *scriptrun.RunRequest) (*scriptrun.RunResult, error) +} + +var _ ScriptRunner = (*scriptrun.Runner)(nil) + +// ScriptResolver invokes a user-configured shell script and parses +// the JSON document it produces on stdout. +type ScriptResolver struct { + // Log receives diagnostic messages. + Log *silog.Logger + + // Script is the resolver script body. + Script string + + // Runner is the [ScriptRunner] used to execute Script. + Runner ScriptRunner + + // RepoRoot is the directory containing the resolution file. + RepoRoot string +} + +// ScriptResolveError indicates that the script ran but its output did +// not conform to the expected JSON protocol. The captured stdout and +// stderr are preserved so callers can surface them to the user. +type ScriptResolveError struct { + // Stage describes what went wrong (e.g., "exit", "parse"). + Stage string + + // ExitCode is the script's exit code. + ExitCode int + + // Stdout is the captured standard output of the script. + Stdout []byte + + // Stderr is the captured standard error of the script. + Stderr []byte + + // Err is the underlying error, if any. + Err error +} + +func (e *ScriptResolveError) Error() string { + switch e.Stage { + case "exit": + return fmt.Sprintf( + "resolver exited with code %d", e.ExitCode) + case "parse": + return fmt.Sprintf( + "resolver output is not valid JSON: %v", e.Err) + default: + return fmt.Sprintf("resolver failed: %v", e.Err) + } +} + +func (e *ScriptResolveError) Unwrap() error { return e.Err } + +// Resolve writes the current_merge pointer to the resolution file, +// invokes the script, and parses its stdout as JSON. +func (r *ScriptResolver) Resolve( + ctx context.Context, req *ResolveRequest, +) (*scriptrun.ResolveResponse, error) { + if req == nil { + return nil, errors.New("nil resolve request") + } + if r.Script == "" { + return nil, errors.New("no resolver script configured") + } + + pair := MergePair{Ours: req.IntegrationName, Theirs: req.TipName} + if err := r.writeCurrentMerge(pair); err != nil { + return nil, fmt.Errorf("update resolution file: %w", err) + } + + res, err := r.Runner.Run(ctx, &scriptrun.RunRequest{ + Script: r.Script, + Dir: r.RepoRoot, + Env: scriptrun.EnvFor( + scriptrun.OpIntegrationRebuild, + req.TipName, // GS_BRANCH = the tip being merged in + req.IntegrationName, // GS_BASE = the integration branch + ), + }) + if err != nil { + return nil, fmt.Errorf("run resolver: %w", err) + } + + if res.ExitCode != 0 { + return nil, &ScriptResolveError{ + Stage: "exit", + ExitCode: res.ExitCode, + Stdout: res.Stdout, + Stderr: res.Stderr, + } + } + + resp, err := scriptrun.ParseResponse(res.Stdout) + if err != nil { + return nil, &ScriptResolveError{ + Stage: "parse", + ExitCode: res.ExitCode, + Stdout: res.Stdout, + Stderr: res.Stderr, + Err: err, + } + } + return resp, nil +} + +// writeCurrentMerge updates the resolution file's current_merge +// pointer to pair, preserving existing resolutions. Creates the file +// (and the parent .spice/resolutions directory) if absent. +func (r *ScriptResolver) writeCurrentMerge(pair MergePair) error { + if err := spicedir.EnsureResolutionsDir(r.RepoRoot); err != nil { + return err + } + path := spicedir.ResolutionPath(r.RepoRoot, ResolutionFeatureName) + file, err := LoadResolutionFile(path) + if err != nil { + return err + } + file.CurrentMerge = &pair + file.EnsureEntry(pair) + return file.Save(path) +} diff --git a/internal/handler/integration/resolver_test.go b/internal/handler/integration/resolver_test.go new file mode 100644 index 00000000..0f6f3abd --- /dev/null +++ b/internal/handler/integration/resolver_test.go @@ -0,0 +1,307 @@ +package integration_test + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/handler/integration" + "go.abhg.dev/gs/internal/scriptrun" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice/spicedir" +) + +// fakeRunner is a minimal in-memory ScriptRunner used by resolver tests. +type fakeRunner struct { + lastReq *scriptrun.RunRequest + result *scriptrun.RunResult + err error +} + +func (f *fakeRunner) Run(_ context.Context, req *scriptrun.RunRequest) (*scriptrun.RunResult, error) { + f.lastReq = req + if f.err != nil { + return nil, f.err + } + return f.result, nil +} + +func TestScriptResolver_Resolve_emptyResponse(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`{}`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `echo '{}'`, + Runner: runner, + RepoRoot: dir, + } + + resp, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.NoError(t, err) + assert.Empty(t, resp.Assumptions) + assert.Empty(t, resp.Questions) + assert.Empty(t, resp.UnresolvedFiles) +} + +func TestScriptResolver_Resolve_populatedResponse(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`{ + "assumptions": ["chose feat-a per commit timestamp"], + "questions": ["should feat-a win in shared.txt?"], + "unresolved_files": ["shared.txt"] + }`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `# unused with fake runner`, + Runner: runner, + RepoRoot: dir, + } + + resp, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.NoError(t, err) + assert.Equal(t, + []string{"chose feat-a per commit timestamp"}, resp.Assumptions) + assert.Equal(t, + []string{"should feat-a win in shared.txt?"}, resp.Questions) + assert.Equal(t, []string{"shared.txt"}, resp.UnresolvedFiles) +} + +func TestScriptResolver_Resolve_currentMergeWritten(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`{}`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `echo '{}'`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.NoError(t, err) + + file, err := integration.LoadResolutionFile( + spicedir.ResolutionPath(dir, integration.ResolutionFeatureName)) + require.NoError(t, err) + require.NotNil(t, file.CurrentMerge) + assert.Equal(t, "preview", file.CurrentMerge.Ours) + assert.Equal(t, "feat-a", file.CurrentMerge.Theirs) + // EnsureEntry should have created an entry for this pair. + require.Len(t, file.Resolutions, 1) + assert.Equal(t, "preview", file.Resolutions[0].MergingBranches.Ours) + assert.Equal(t, "feat-a", file.Resolutions[0].MergingBranches.Theirs) + + // Verify the script saw a Dir set to the repo root. + assert.Equal(t, dir, runner.lastReq.Dir) +} + +func TestScriptResolver_Resolve_existingEntryPreserved(t *testing.T) { + dir := t.TempDir() + path := spicedir.ResolutionPath(dir, integration.ResolutionFeatureName) + + // Pre-seed file with a Q&A entry for (preview, feat-a). + seed := &integration.ResolutionFile{ + Resolutions: []integration.ResolutionEntry{ + { + MergingBranches: integration.MergePair{ + Ours: "preview", Theirs: "feat-a", + }, + ResolutionInstructions: []scriptrun.QAPair{ + {Question: "prior Q", Answer: "prior A"}, + }, + }, + }, + } + require.NoError(t, seed.Save(path)) + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`{}`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `echo '{}'`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.NoError(t, err) + + file, err := integration.LoadResolutionFile(path) + require.NoError(t, err) + require.Len(t, file.Resolutions, 1) + require.Len(t, file.Resolutions[0].ResolutionInstructions, 1) + assert.Equal(t, "prior Q", + file.Resolutions[0].ResolutionInstructions[0].Question) +} + +func TestScriptResolver_Resolve_invalidJSON(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`not-json`), + Stderr: []byte(`some debugging info`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `echo not-json`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.Error(t, err) + + var sre *integration.ScriptResolveError + require.True(t, errors.As(err, &sre)) + assert.Equal(t, "parse", sre.Stage) + assert.Equal(t, []byte(`not-json`), sre.Stdout) + assert.Equal(t, []byte(`some debugging info`), sre.Stderr) +} + +func TestScriptResolver_Resolve_emptyOutput(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(``), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `:`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.Error(t, err) + var sre *integration.ScriptResolveError + require.True(t, errors.As(err, &sre)) + assert.Equal(t, "parse", sre.Stage) +} + +func TestScriptResolver_Resolve_nonZeroExit(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 7, + Stdout: []byte(`partial stdout`), + Stderr: []byte(`error trace`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `exit 7`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.Error(t, err) + + var sre *integration.ScriptResolveError + require.True(t, errors.As(err, &sre)) + assert.Equal(t, "exit", sre.Stage) + assert.Equal(t, 7, sre.ExitCode) + assert.Equal(t, []byte(`partial stdout`), sre.Stdout) + assert.Equal(t, []byte(`error trace`), sre.Stderr) +} + +func TestScriptResolver_Resolve_runnerError(t *testing.T) { + dir := t.TempDir() + + runner := &fakeRunner{ + err: errors.New("could not spawn"), + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `:`, + Runner: runner, + RepoRoot: dir, + } + + _, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "could not spawn") +} + +func TestScriptResolver_Resolve_unknownFieldIgnored(t *testing.T) { + dir := t.TempDir() + + // Per the shared script protocol (doc/src/guide/scripts.md), + // extra fields are ignored. A document with only an unknown + // field parses as an empty response. + runner := &fakeRunner{ + result: &scriptrun.RunResult{ + ExitCode: 0, + Stdout: []byte(`{"made_up_field": 1}`), + }, + } + r := &integration.ScriptResolver{ + Log: silog.Nop(), + Script: `echo '{"made_up_field":1}'`, + Runner: runner, + RepoRoot: dir, + } + + resp, err := r.Resolve(t.Context(), &integration.ResolveRequest{ + IntegrationName: "preview", + TipName: "feat-a", + }) + require.NoError(t, err) + assert.Empty(t, resp.Assumptions) + assert.Empty(t, resp.Questions) + assert.Empty(t, resp.UnresolvedFiles) +} diff --git a/internal/handler/sync/handler.go b/internal/handler/sync/handler.go index af116412..b25e4bb3 100644 --- a/internal/handler/sync/handler.go +++ b/internal/handler/sync/handler.go @@ -89,6 +89,13 @@ type AutostashHandler interface { var _ AutostashHandler = (*autostash.Handler)(nil) +// BranchRemovedHook is invoked once per branch that sync removes from +// tracking (typically because the change request was merged upstream). +// +// Implementations are expected to clean up auxiliary state. Errors are +// logged and do not abort the sync. +type BranchRemovedHook func(ctx context.Context, branch string) error + // Handler implements syncing commands. type Handler struct { Log *silog.Logger // required @@ -101,6 +108,10 @@ type Handler struct { Restack RestackHandler // required Autostash AutostashHandler // required + // OnBranchRemoved is called after each branch is removed during + // merged-branch cleanup. Optional; nil disables the callback. + OnBranchRemoved BranchRemovedHook + Remote string // required // RemoteRepository is set only if remote refers to a supported forge. RemoteRepository forge.Repository // optional @@ -1032,6 +1043,14 @@ func (h *Handler) deleteBranches(ctx context.Context, branchesToDelete []branchD return nil, fmt.Errorf("delete merged branches: %w", err) } + if h.OnBranchRemoved != nil { + for _, branchName := range deleteBranchNames { + if err := h.OnBranchRemoved(ctx, branchName); err != nil { + h.Log.Warnf("branch-removed hook: %v", err) + } + } + } + // Also delete the remote tracking branch for this branch // if it still exists. for _, branchName := range deleteBranchNames { diff --git a/internal/spice/config.go b/internal/spice/config.go index b6d8cc56..71950b80 100644 --- a/internal/spice/config.go +++ b/internal/spice/config.go @@ -264,6 +264,39 @@ func (c *Config) ScriptResolveMaxIterations() int { return n } +// IntegrationResolver returns the configured script for auto-resolving +// integration branch merge conflicts, or empty if none is set. +// +// Read from spice.integration.resolver. +func (c *Config) IntegrationResolver() string { + values := c.items[git.ConfigKey( + _spiceSection+".integration.resolver", + ).Canonical()] + if len(values) == 0 { + return "" + } + return values[len(values)-1] +} + +// IntegrationAutoResolve reports whether the configured resolver +// should run automatically during integration rebuilds. +// +// Read from spice.integration.autoResolve. Defaults to false. +// Unparseable values are treated as false. +func (c *Config) IntegrationAutoResolve() bool { + values := c.items[git.ConfigKey( + _spiceSection+".integration.autoResolve", + ).Canonical()] + if len(values) == 0 { + return false + } + v, err := strconv.ParseBool(values[len(values)-1]) + if err != nil { + return false + } + return v +} + // Validate checks if the configuration is valid for the given application. // This is a no-op, as we allow unknown configuration keys. func (*Config) Validate(*kong.Application) error { return nil } diff --git a/internal/spice/config_test.go b/internal/spice/config_test.go index 70896020..ca4b4464 100644 --- a/internal/spice/config_test.go +++ b/internal/spice/config_test.go @@ -641,3 +641,132 @@ func TestIntegrationConfig_gitConfigReferences(t *testing.T) { }) } } + +func TestConfig_IntegrationResolver(t *testing.T) { + tests := []struct { + name string + config string + want string + }{ + { + name: "Unset", + config: "", + want: "", + }, + { + name: "Simple", + config: text.Dedent(` + [spice "integration"] + resolver = "/path/to/resolver.sh" + `), + want: "/path/to/resolver.sh", + }, + { + name: "LastValueWins", + config: text.Dedent(` + [spice "integration"] + resolver = "/first.sh" + resolver = "/second.sh" + `), + want: "/second.sh", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + home := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(home, ".gitconfig"), + []byte(tt.config), + 0o600, + )) + + ctx := t.Context() + gitCfg := git.NewConfig(git.ConfigOptions{ + Log: silogtest.New(t), + Dir: home, + Env: []string{ + "HOME=" + home, + "USER=testuser", + "GIT_CONFIG_NOSYSTEM=1", + }, + }) + spicecfg, err := spice.LoadConfig(ctx, gitCfg, spice.ConfigOptions{ + Log: silogtest.New(t), + }) + require.NoError(t, err) + + assert.Equal(t, tt.want, spicecfg.IntegrationResolver()) + }) + } +} + +func TestConfig_IntegrationAutoResolve(t *testing.T) { + tests := []struct { + name string + config string + want bool + }{ + { + name: "Unset", + config: "", + want: false, + }, + { + name: "True", + config: text.Dedent(` + [spice "integration"] + autoResolve = true + `), + want: true, + }, + { + name: "False", + config: text.Dedent(` + [spice "integration"] + autoResolve = false + `), + want: false, + }, + { + name: "Garbage", + config: text.Dedent(` + [spice "integration"] + autoResolve = not-a-bool + `), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + home := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(home, ".gitconfig"), + []byte(tt.config), + 0o600, + )) + + ctx := t.Context() + gitCfg := git.NewConfig(git.ConfigOptions{ + Log: silogtest.New(t), + Dir: home, + Env: []string{ + "HOME=" + home, + "USER=testuser", + "GIT_CONFIG_NOSYSTEM=1", + }, + }) + spicecfg, err := spice.LoadConfig(ctx, gitCfg, spice.ConfigOptions{ + Log: silogtest.New(t), + }) + require.NoError(t, err) + + assert.Equal(t, tt.want, spicecfg.IntegrationAutoResolve()) + }) + } +} diff --git a/main.go b/main.go index f3c5c662..00f0e0b4 100644 --- a/main.go +++ b/main.go @@ -37,6 +37,7 @@ import ( "go.abhg.dev/gs/internal/handler/submit" "go.abhg.dev/gs/internal/handler/sync" "go.abhg.dev/gs/internal/handler/track" + "go.abhg.dev/gs/internal/scriptrun" "go.abhg.dev/gs/internal/secret" "go.abhg.dev/gs/internal/sigstack" "go.abhg.dev/gs/internal/silog" @@ -172,7 +173,7 @@ func main() { kong.Name(cmdName), kong.Description("git-spice is a command line tool for stacking Git branches."), kong.Resolvers(spiceConfig), - kong.Bind(logger, &forges, &sigStack, &cmd), + kong.Bind(logger, &forges, &sigStack, &cmd, spiceConfig), kong.BindTo(&cmd.Forge, (*forgeOptions)(nil)), kong.BindTo(ctx, (*context.Context)(nil)), kong.BindTo(spiceConfig, (*experiment.Enabler)(nil)), @@ -601,6 +602,7 @@ func (cmd *mainCmd) AfterApply( deleteHandler DeleteHandler, restackHandler RestackHandler, autostashHandler AutostashHandler, + integrationHandler IntegrationHandler, ) (SyncHandler, error) { remote, err := ensureRemote(ctx, repo, store, log, view) // TODO: move ensure remote to Service @@ -635,6 +637,7 @@ func (cmd *mainCmd) AfterApply( Delete: deleteHandler, Restack: restackHandler, Autostash: autostashHandler, + OnBranchRemoved: integrationHandler.OnBranchRemoved, Remote: remote.Upstream, RemoteRepository: remoteRepo, PushRepository: pushRepository, @@ -699,17 +702,37 @@ func (cmd *mainCmd) AfterApply( }), kctx.BindSingletonProvider(func( log *silog.Logger, + view ui.View, repo *git.Repository, wt *git.Worktree, store *state.Store, svc *spice.Service, + cfg *spice.Config, ) (IntegrationHandler, error) { + repoRoot := wt.RootDir() + var resolver integration.Resolver + if script := cfg.IntegrationResolver(); script != "" { + resolver = &integration.ScriptResolver{ + Log: log, + Script: script, + Runner: &scriptrun.Runner{ + Log: log, + Args: os.Args, + }, + RepoRoot: repoRoot, + } + } return &integration.Handler{ - Log: log, - Repository: repo, - Worktree: wt, - Store: store, - Service: svc, + Log: log, + Repository: repo, + Worktree: wt, + Store: store, + Service: svc, + Resolver: resolver, + Prompter: integration.NewViewPrompter(view), + DefaultAutoResolve: cfg.IntegrationAutoResolve(), + RepoRoot: repoRoot, + MaxResolveIterations: cfg.ScriptResolveMaxIterations(), }, nil }), ) diff --git a/testdata/help/integration_rebuild.txt b/testdata/help/integration_rebuild.txt index d7cf469c..906fe7ea 100644 --- a/testdata/help/integration_rebuild.txt +++ b/testdata/help/integration_rebuild.txt @@ -10,8 +10,15 @@ On conflict, the merge is left in the worktree. Resolve the conflicting files, commit with 'git merge --continue', then re-run 'gs integration rebuild' (or 'gs intrb') to resume. +With --auto-resolve (or spice.integration.autoResolve=true), a configured +resolver script is invoked to attempt automatic resolution before surfacing +conflicts. See the recipe for details on the JSON protocol the script must +implement. + Flags: - --push Also push the integration branch after rebuilding + --push Also push the integration branch after rebuilding + --[no-]auto-resolve Auto-resolve merge conflicts using the configured + resolver script (🔧 spice.integration.autoResolve) Global Flags: -h, --help Show help for the command diff --git a/testdata/script/integration_auto_resolve_assumptions.txt b/testdata/script/integration_auto_resolve_assumptions.txt new file mode 100644 index 00000000..0eac5f8a --- /dev/null +++ b/testdata/script/integration_auto_resolve_assumptions.txt @@ -0,0 +1,46 @@ +# Auto-resolve script returns assumptions; they are logged at info level +# and the merge completes. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +gs integration rebuild +stderr 'Auto-resolve: picked feat-b because it has the newer commit' +stderr 'rebuilt with 2 tip\(s\)' + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- repo/resolved.txt -- +resolved content +-- resolver.sh -- +#!/bin/sh +cp resolved.txt shared.txt +cat <<'EOF' +{"assumptions": ["picked feat-b because it has the newer commit"]} +EOF diff --git a/testdata/script/integration_auto_resolve_disabled.txt b/testdata/script/integration_auto_resolve_disabled.txt new file mode 100644 index 00000000..80a63058 --- /dev/null +++ b/testdata/script/integration_auto_resolve_disabled.txt @@ -0,0 +1,44 @@ +# With --no-auto-resolve, the resolver is NOT invoked even if +# configured and autoResolve is true. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +# --no-auto-resolve must short-circuit before the resolver runs; +# the resolver never gets to write its sentinel file. +! gs integration rebuild --no-auto-resolve +stderr 'Merge conflict in tip feat-b' +! exists $WORK/resolver-ran + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- resolver.sh -- +#!/bin/sh +touch "$WORK/resolver-ran" +echo '{}' diff --git a/testdata/script/integration_auto_resolve_env.txt b/testdata/script/integration_auto_resolve_env.txt new file mode 100644 index 00000000..825a3f0c --- /dev/null +++ b/testdata/script/integration_auto_resolve_env.txt @@ -0,0 +1,57 @@ +# The shared env contract (GS_OPERATION, GS_BRANCH, GS_BASE) is set +# for the resolver during an integration rebuild. Verified by having +# the resolver echo the values back as assumptions. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +gs integration rebuild +stderr 'GS_OPERATION=integration-rebuild' +stderr 'GS_BRANCH=feat-b' +stderr 'GS_BASE=preview' + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- repo/resolved.txt -- +resolved content +-- resolver.sh -- +#!/bin/sh +cp resolved.txt shared.txt +op="${GS_OPERATION:-}" +br="${GS_BRANCH:-}" +ba="${GS_BASE:-}" +cat <' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +! gs integration rebuild +stderr 'Merge conflict in tip feat-b' + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- resolver.sh -- +#!/bin/sh +echo not-json diff --git a/testdata/script/integration_auto_resolve_iteration_cap.txt b/testdata/script/integration_auto_resolve_iteration_cap.txt new file mode 100644 index 00000000..9d20a9e3 --- /dev/null +++ b/testdata/script/integration_auto_resolve_iteration_cap.txt @@ -0,0 +1,56 @@ +# spice.scriptResolve.maxIterations bounds the integration rebuild +# Q&A loop. A resolver that always returns a question hits the +# configured cap and the rebuild fails with a clear message. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true +git config spice.scriptResolve.maxIterations 3 + +env ROBOT_INPUT=$WORK/robot.golden ROBOT_OUTPUT=$WORK/robot.actual + +! gs integration rebuild +stderr 'exceeded iteration cap \(3\)' + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- robot.golden -- +=== +> Auto-resolve question 1: Which side wins? +"left" +=== +> Auto-resolve question 1: Which side wins? +"left" +=== +> Auto-resolve question 1: Which side wins? +"left" +-- resolver.sh -- +#!/bin/sh +cat <<'EOF' +{"questions": ["Which side wins?"]} +EOF diff --git a/testdata/script/integration_auto_resolve_persists_across_rebuilds.txt b/testdata/script/integration_auto_resolve_persists_across_rebuilds.txt new file mode 100644 index 00000000..7880f827 --- /dev/null +++ b/testdata/script/integration_auto_resolve_persists_across_rebuilds.txt @@ -0,0 +1,64 @@ +# The resolution file persists across rebuilds. If an entry for the +# current (ours, theirs) already exists, gs preserves it so the user +# doesn't re-answer. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +# Pre-seed the resolution file with a Q&A for (preview, feat-b). +mkdir .spice/resolutions +cp $WORK/golden/resolution.json .spice/resolutions/integration.json + +gs integration rebuild +stderr 'rebuilt with 1 tip\(s\)' + +# Resolution file should still carry the pre-seeded entry (with +# instruction count unchanged). +exists $WORK/repo/.spice/resolutions/integration.json + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- golden/resolution.json -- +{ + "resolutions": [ + { + "merging_branches": {"ours": "preview", "theirs": "feat-b"}, + "resolution_instructions": [ + {"question": "which version wins?", "answer": "feat-b"} + ] + } + ] +} +-- resolver.sh -- +#!/bin/sh +# Verify the pre-seeded Q&A reached us through the file. +grep -q 'which version wins' .spice/resolutions/integration.json || exit 1 +grep -q 'feat-b' .spice/resolutions/integration.json || exit 1 +cp feat-b-version.txt shared.txt +echo '{}' diff --git a/testdata/script/integration_auto_resolve_prune_on_delete.txt b/testdata/script/integration_auto_resolve_prune_on_delete.txt new file mode 100644 index 00000000..c7586fc0 --- /dev/null +++ b/testdata/script/integration_auto_resolve_prune_on_delete.txt @@ -0,0 +1,53 @@ +# gs branch delete prunes resolution-file entries that reference the +# deleted branch. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +gs bc -m 'feat-a' feat-a +git checkout main +gs bc -m 'feat-b' feat-b +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +mkdir .spice/resolutions +cp $WORK/golden/resolution-before.json .spice/resolutions/integration.json + +gs branch delete --force feat-a + +cmpenvJSON .spice/resolutions/integration.json $WORK/golden/resolution-after.json + +-- repo/.placeholder -- +-- golden/resolution-before.json -- +{ + "resolutions": [ + { + "merging_branches": {"ours": "preview", "theirs": "feat-a"}, + "resolution_instructions": [ + {"question": "q-a", "answer": "a-a"} + ] + }, + { + "merging_branches": {"ours": "preview", "theirs": "feat-b"}, + "resolution_instructions": [ + {"question": "q-b", "answer": "a-b"} + ] + } + ] +} +-- golden/resolution-after.json -- +{ + "resolutions": [ + { + "merging_branches": {"ours": "preview", "theirs": "feat-b"}, + "resolution_instructions": [ + {"question": "q-b", "answer": "a-b"} + ] + } + ] +} diff --git a/testdata/script/integration_auto_resolve_prune_on_untrack.txt b/testdata/script/integration_auto_resolve_prune_on_untrack.txt new file mode 100644 index 00000000..2e4948b5 --- /dev/null +++ b/testdata/script/integration_auto_resolve_prune_on_untrack.txt @@ -0,0 +1,54 @@ +# gs branch untrack prunes resolution-file entries that reference the +# untracked branch. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +gs bc -m 'feat-a' feat-a +git checkout main +gs bc -m 'feat-b' feat-b +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +mkdir .spice/resolutions +cp $WORK/golden/resolution-before.json .spice/resolutions/integration.json + +gs branch untrack feat-a + +# After untrack, the file should no longer reference feat-a. +cmpenvJSON .spice/resolutions/integration.json $WORK/golden/resolution-after.json + +-- repo/.placeholder -- +-- golden/resolution-before.json -- +{ + "resolutions": [ + { + "merging_branches": {"ours": "preview", "theirs": "feat-a"}, + "resolution_instructions": [ + {"question": "q-a", "answer": "a-a"} + ] + }, + { + "merging_branches": {"ours": "preview", "theirs": "feat-b"}, + "resolution_instructions": [ + {"question": "q-b", "answer": "a-b"} + ] + } + ] +} +-- golden/resolution-after.json -- +{ + "resolutions": [ + { + "merging_branches": {"ours": "preview", "theirs": "feat-b"}, + "resolution_instructions": [ + {"question": "q-b", "answer": "a-b"} + ] + } + ] +} diff --git a/testdata/script/integration_auto_resolve_success.txt b/testdata/script/integration_auto_resolve_success.txt new file mode 100644 index 00000000..a251ec5a --- /dev/null +++ b/testdata/script/integration_auto_resolve_success.txt @@ -0,0 +1,55 @@ +# Auto-resolve completes a single-tip conflict in one iteration when +# the resolver script writes the resolved file and emits an empty JSON +# response. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +# Configure the resolver: it's a shebang script in the WORK dir that +# copies the canonical resolved content and emits '{}' on stdout. +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +gs integration rebuild +stderr 'rebuilt with 2 tip\(s\)' + +# preview branch should have the resolved content. +git show preview:shared.txt +cmp stdout $WORK/golden/resolved.txt + +# Resolution file should have been written with current_merge cleared +# after the last invocation… actually current_merge persists per spec. +# Just verify the file exists. +exists $WORK/repo/.spice/resolutions/integration.json + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- golden/resolved.txt -- +resolved content +-- resolver.sh -- +#!/bin/sh +cp "$WORK/golden/resolved.txt" shared.txt +echo '{}' diff --git a/testdata/script/integration_auto_resolve_unresolved_no_questions.txt b/testdata/script/integration_auto_resolve_unresolved_no_questions.txt new file mode 100644 index 00000000..686652e6 --- /dev/null +++ b/testdata/script/integration_auto_resolve_unresolved_no_questions.txt @@ -0,0 +1,45 @@ +# Resolver reports unresolved_files with no questions: auto-resolve +# fails, falling through to the existing conflict-surfacing path. + +as 'Test ' +at '2025-01-01T00:00:00Z' + +cd repo +git init +git add shared.txt +git commit -m 'Initial commit' +gs repo init + +cp feat-a-version.txt shared.txt +git add shared.txt +gs bc feat-a -m 'feat-a changes shared.txt' + +git checkout main +cp feat-b-version.txt shared.txt +git add shared.txt +gs bc feat-b -m 'feat-b changes shared.txt' + +git checkout main +gs integration create preview --tip feat-a --tip feat-b + +chmod 700 $WORK/resolver.sh +git config spice.integration.resolver $WORK/resolver.sh +git config spice.integration.autoResolve true + +! gs integration rebuild +stderr 'unresolved files with no questions: shared.txt' +stderr 'Merge conflict in tip feat-b' + +# Worktree should be left in conflict state (LeaveConflict semantics). +git status +stdout 'You have unmerged paths' + +-- repo/shared.txt -- +base content +-- repo/feat-a-version.txt -- +feat-a content +-- repo/feat-b-version.txt -- +feat-b content +-- resolver.sh -- +#!/bin/sh +echo '{"unresolved_files": ["shared.txt"]}' diff --git a/testdata/script/integration_tip_auto_remove.txt b/testdata/script/integration_tip_auto_remove.txt new file mode 100644 index 00000000..8c121e6e --- /dev/null +++ b/testdata/script/integration_tip_auto_remove.txt @@ -0,0 +1,59 @@ +# Verifies that removing a branch via 'gs branch untrack', +# 'gs branch delete', or 'gs repo sync' automatically prunes that +# branch from the integration's configured tip list. Without this, +# a tip's underlying branch can be deleted (e.g. by sync after +# upstream merge) while the tip name lingers in state, causing the +# next 'gs integration rebuild' to fail with "resolve head: does +# not exist" on a branch the user already chose to delete. + +as 'Test ' +at '2025-06-01T00:00:00Z' + +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +git add feat-a.txt +gs branch create feat-a -m 'Add feat-a' + +gs trunk +git add feat-b.txt +gs branch create feat-b -m 'Add feat-b' + +gs trunk +git add feat-c.txt +gs branch create feat-c -m 'Add feat-c' + +gs trunk +gs integration create preview --tip feat-a --tip feat-b --tip feat-c + +# Untracking a configured tip removes it from the tip list. +gs branch untrack feat-a +stderr 'Removed "feat-a" from integration tips' +gs integration tip list +cmp stdout $WORK/golden/tips-after-untrack.txt + +# Deleting a configured tip removes it from the tip list too. +gs branch delete feat-b --force +stderr 'Removed "feat-b" from integration tips' +gs integration tip list +cmp stdout $WORK/golden/tips-after-delete.txt + +# After both cleanups the rebuild succeeds against the remaining +# tip; before this fix it would have failed when trying to resolve +# the missing branches. +gs integration rebuild +stderr 'rebuilt with 1 tip\(s\)' + +-- repo/feat-a.txt -- +feature a +-- repo/feat-b.txt -- +feature b +-- repo/feat-c.txt -- +feature c +-- golden/tips-after-untrack.txt -- +feat-b +feat-c +-- golden/tips-after-delete.txt -- +feat-c