Let the agent stop a merge for review after substantive conflict fixes
This commit is contained in:
@@ -8,8 +8,6 @@
|
||||
|
||||
# M4 — Merging and history
|
||||
|
||||
- Merge-resolution stage
|
||||
A stage triggered when merging the default branch into a task branch conflicts; its result goes to review.
|
||||
- Git notes on squash merges
|
||||
Attach a note under `refs/notes/byre` to each squash commit, holding the plan, the handoff bundle, review summaries, eval results and resolved inbox decisions, with full transcripts referenced by task ID. Notes don't push or fetch by default, so the merge pushes the notes ref explicitly and project setup configures fetching it.
|
||||
- Repeated merge cycles
|
||||
|
||||
+2
-2
@@ -30,7 +30,7 @@ Every task worktree is made by `createTask` (`internal/agents/tasks.go`): new ta
|
||||
|
||||
A task ends `merged` or `discarded`; either way its sessions are stopped, its snapshot refs, worktree and branch are removed, its `open` inbox items become `dismissed` (resolved by whoever closed it), and its record and events remain. `GET /api/tasks/{id}/unmerged` reports what would be lost or merged: commits on the branch that aren't on the default branch, and files, lines and uncommitted paths that differ from the merge base. The UI's Merge and Discard buttons each ask a confirmation (Merge's holds the editable commit title), opened just below the button like the task picker's switcher (centred when ⌘S opens Merge's from a tab without the button), and are disabled while the task's session is `running`. Merge is also disabled until `unmerged` reports a commit or a changed file; the transcript fetches it when the task is opened and again shortly after each tool result or turn end.
|
||||
|
||||
`POST /api/tasks/{id}/merge-in` (the transcript's **Sync from main** button, labelled with the default branch) brings the default branch into the task branch through the agent: it sends `mergeInPrompt` (`internal/agents/mergein.go`) as a `byre` message to the task's session, or starts a session with it when the task has none. The prompt has the agent run `git merge --no-ff --no-commit`, resolve conflicts, regenerate generated files rather than hand-merging them, check the build and tests, and leave the merge uncommitted; byre's end-of-turn commit concludes it as a merge commit. The prompt ends by showing the task's current merge title and asking the agent to set a better one with `set_merge_title` if it no longer describes all the work on the branch. It is refused (409) while a session is `running`.
|
||||
`POST /api/tasks/{id}/merge-in` (the transcript's **Sync from main** button, labelled with the default branch) brings the default branch into the task branch through the agent: it sends `mergeInPrompt` (`internal/agents/mergein.go`) as a `byre` message to the task's session, or starts a session with it when the task has none. The prompt has the agent run `git merge --no-ff --no-commit`, resolve conflicts, regenerate generated files rather than hand-merging them, check the build and tests, and leave the merge uncommitted; byre's end-of-turn commit concludes it as a merge commit. If resolving the conflicts took substantive changes rather than combining both sides as written, the prompt tells the agent to stop there and end its turn describing them; the turn's snapshot then gets its review like any other (see "Reviews"). byre doesn't inspect the merge itself: whether a change is substantive, and whether to stop, is the agent's call. On a Merge turn the prompt adds that byre squashes and pushes when the turn ends, and tells the agent to call the `stop_merge` MCP tool first if it stops (see below). The prompt ends by showing the task's current merge title and asking the agent to set a better one with `set_merge_title` if it no longer describes all the work on the branch. It is refused (409) while a session is `running`.
|
||||
|
||||
A task's `merge_title` is the title of the squash commit Merge makes. It is the task title until the agent calls the `set_merge_title` MCP tool (`internal/agents/mergein.go`), which it may do until a merge is confirmed: one line of at most 72 characters (`maxMergeTitle`, checked by `cleanMergeTitle`), stored in `tasks.merge_title` (empty there means "use the title"), recorded as a `task.merge_title` event that the transcript shows as a note. Malformed titles are rejected with a corrective message, and so is a call once the task is closed or while `merging_by` or `merge_queued_by` is set. The Merge confirm dialog shows the title in a text field the human can edit (Enter confirms), lists the co-authors, and sends the field's value as `merge_title` in the body of `POST /api/tasks/{id}/merge`. Merge validates it like the tool does (400 when malformed) and, when it differs from the stored title, stores it with the human as the event's actor before starting or holding the merge.
|
||||
|
||||
@@ -38,7 +38,7 @@ A task's participants are the users who put a message bubble with their name in
|
||||
|
||||
`POST /api/tasks/{id}/merge` starts the same turn, with the prompt's closing invitation to revise the title left out since the human just confirmed it, and records in `tasks.merging_by` that the task is to be merged by the requesting user, who must have a git identity (400 otherwise). Every turn end of the task's session reads and clears it. It survives a byre restart during the turn: startup resumes the session with the restart notice, and the merge happens when that resumed turn ends. When that turn ends normally (not interrupted, not an error), byre squashes the worktree, now containing the default branch, onto it with `git merge-tree --merge-base`, makes one commit titled with the task's `merge_title` with `git commit-tree`, authored and committed by the merging user with no agent attribution, its body one `Co-authored-by: Name <email>` trailer per co-author with a git identity (`squashMessage` in `internal/agents/mergein.go`; the co-authors are computed when the squash runs, relative to the user who clicked Merge), and pushes that commit to the default branch on the `origin` remote of the project's checkout (`git push --porcelain origin <commit>:refs/heads/<default branch>`, run in the checkout). Only once the push is accepted does it run `git merge --ff-only` in the project's checkout (which must have the default branch checked out) and close the task; the `task.merged` event carries the commit and `pushed` (`origin/<default branch>`), and the transcript shows both. When the merge changes nothing, no commit is made and nothing is pushed.
|
||||
|
||||
Before starting the turn, Merge runs `git fetch origin <default branch>` and `git merge --ff-only origin/<default branch>` in the checkout, so the agent merges in what the remote has; the request waits for the fetch, and a failure (network, or a checkout with commits the remote lacks) is returned as the request's error. If the push is still refused, for example because someone pushed during the turn, or fails for any other reason, byre files the error in the inbox and the task stays active; clicking Merge again fetches and goes round once more. A refused push leaves the checkout's default branch where it was, since the squash commit was never moved onto it. If the squash fails, for example because the default branch moved again during the turn, byre also files an error and the task stays active. An interrupted or failed turn, an agent process that exits mid-turn, or a session that can't be resumed after a restart clears `merging_by` and leaves the task active without squashing. `POST /api/tasks/{id}/discard` removes everything without merging.
|
||||
Before starting the turn, Merge runs `git fetch origin <default branch>` and `git merge --ff-only origin/<default branch>` in the checkout, so the agent merges in what the remote has; the request waits for the fetch, and a failure (network, or a checkout with commits the remote lacks) is returned as the request's error. If the push is still refused, for example because someone pushed during the turn, or fails for any other reason, byre files the error in the inbox and the task stays active; clicking Merge again fetches and goes round once more. A refused push leaves the checkout's default branch where it was, since the squash commit was never moved onto it. If the squash fails, for example because the default branch moved again during the turn, byre also files an error and the task stays active. The agent calls `stop_merge` (`internal/agents/mergein.go`) with a `reason` when its conflict resolution took substantive changes: it clears `merging_by` and releases the project's merge at once, and records a `task.merge_stopped` event carrying the reason, which the transcript shows as a note. The turn then ends without a squash, the task stays active, its snapshot is reviewed, and the human clicks Merge again when satisfied. Called with no merge pending (a Sync turn, say), it changes nothing and tells the agent so. An interrupted or failed turn, an agent process that exits mid-turn, or a session that can't be resumed after a restart clears `merging_by` and leaves the task active without squashing. `POST /api/tasks/{id}/discard` removes everything without merging.
|
||||
|
||||
One merge runs per project at a time. The Manager keeps the task holding each project's merge in memory (`mergeHolders` in `internal/agents/manager.go`), from just before the fetch until the merge closes the task, fails, or is abandoned as above, or the task is discarded; on startup it is rebuilt from the active tasks with `merging_by` set. Merge on another task of that project while one is held doesn't fetch or start a turn: it records the request in `tasks.merge_queued_by` and `merge_queued_at` (`merge_queued_at` on the Task), emits `task.merge_queued` (a transcript note), and returns the task with 202. A second Merge on a held or merging task is a 409. Whenever a project's merge is released, any task turn in it ends, and on startup, byre starts the oldest held merge in the project whose task session is not `running`: it clears the request (emitting `task.merge_unqueued`, not drawn) and goes through the same fetch and turn as a direct Merge by the user who asked, so it merges in what the earlier merge pushed. If that start fails (the fetch, or the user has no git identity), byre files the error in the task's inbox and tries the next held merge. The Merge button reads **Merge held**, disabled, while the task has `merge_queued_at`; discarding the task drops the request. `sqlite3 byre.db "SELECT id, merge_queued_by, merge_queued_at FROM tasks WHERE merge_queued_by != ''"` lists held merges.
|
||||
|
||||
|
||||
@@ -514,7 +514,7 @@ func (m *Manager) ensureLive(ctx context.Context, sess api.AgentSession, task ap
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
tools := []mcp.Tool{m.askTool(l), m.cancelQuestionTool(l), m.createTaskTool(l), m.setMergeTitleTool(l), m.exposePortTool(l)}
|
||||
tools := []mcp.Tool{m.askTool(l), m.cancelQuestionTool(l), m.createTaskTool(l), m.setMergeTitleTool(l), m.stopMergeTool(l), m.exposePortTool(l)}
|
||||
env := portEnv(task)
|
||||
stdio := []mcp.Stdio{}
|
||||
if sess.Role == api.Reviewer {
|
||||
|
||||
@@ -21,7 +21,9 @@ const maxMergeTitle = 72
|
||||
func mergeInPrompt(target string) string {
|
||||
return fmt.Sprintf("Merge %[1]s into this branch: run `git merge --no-ff --no-commit %[1]s`. "+
|
||||
"Resolve any conflicts keeping both sides' intent; regenerate generated files instead of hand-merging them. "+
|
||||
"Check that the build and tests pass. Leave the merge uncommitted: byre concludes it when the turn ends.", target)
|
||||
"Check that the build and tests pass. Leave the merge uncommitted: byre concludes it when the turn ends. "+
|
||||
"If resolving the conflicts took substantive changes rather than combining both sides as written, stop there: "+
|
||||
"end your turn with a description of those changes, so the review of the turn's snapshot covers them.", target)
|
||||
}
|
||||
|
||||
func (m *Manager) MergeIn(ctx context.Context, task api.Task, user api.User, target string) (api.AgentSession, error) {
|
||||
@@ -100,7 +102,8 @@ func (m *Manager) startMerge(ctx context.Context, task api.Task, project api.Pro
|
||||
}
|
||||
m.mergeHolders[project.Id] = task.Id
|
||||
|
||||
_, err = m.sendMergeIn(ctx, task, user, mergeInPrompt(project.DefaultBranch))
|
||||
prompt := mergeInPrompt(project.DefaultBranch) + fmt.Sprintf(" When this turn ends, byre squashes this branch onto %s and pushes it; if you stop, call stop_merge first so it doesn't.", project.DefaultBranch)
|
||||
_, err = m.sendMergeIn(ctx, task, user, prompt)
|
||||
if err != nil {
|
||||
m.takeMerge(task.Id)
|
||||
delete(m.mergeHolders, project.Id)
|
||||
@@ -395,6 +398,51 @@ func (m *Manager) setMergeTitle(ctx context.Context, l *live, raw json.RawMessag
|
||||
return fmt.Sprintf("Merge commit title set to %q.", title), nil
|
||||
}
|
||||
|
||||
type stopMergeArgs struct {
|
||||
Reason string `json:"reason"`
|
||||
}
|
||||
|
||||
func (m *Manager) stopMergeTool(l *live) mcp.Tool {
|
||||
return mcp.Tool{
|
||||
Name: "stop_merge",
|
||||
Description: "Stop the merge byre is to make when this turn ends, so the task stays active and its snapshot is reviewed before anything is pushed. " +
|
||||
"Call it during a Merge turn when resolving conflicts took substantive changes, then end your turn describing them. The human merges again after the review.",
|
||||
InputSchema: json.RawMessage(`{
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"reason": {"type": "string", "description": "The substantive changes the resolution made, in a sentence or two."}
|
||||
},
|
||||
"required": ["reason"]
|
||||
}`),
|
||||
Handler: func(ctx context.Context, raw json.RawMessage) (string, error) {
|
||||
return m.stopMerge(ctx, l, raw)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func (m *Manager) stopMerge(ctx context.Context, l *live, raw json.RawMessage) (string, error) {
|
||||
args := stopMergeArgs{}
|
||||
err := json.Unmarshal(raw, &args)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("malformed stop_merge call: %w", err)
|
||||
}
|
||||
reason := strings.TrimSpace(args.Reason)
|
||||
if reason == "" {
|
||||
return "", fmt.Errorf("%w: reason must say what the resolution changed", ErrInvalid)
|
||||
}
|
||||
|
||||
if m.takeMerge(l.taskID) == "" {
|
||||
return "No merge is pending for this task; just end your turn.", nil
|
||||
}
|
||||
go m.releaseMerge(l.projectID, l.taskID)
|
||||
|
||||
err = m.emit(ctx, l, agentActor, "task.merge_stopped", map[string]any{"id": l.taskID, "reason": reason})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return "Merge stopped: byre won't squash or push when this turn ends. End your turn now with a description of the changes.", nil
|
||||
}
|
||||
|
||||
func cleanMergeTitle(raw string) (string, error) {
|
||||
title := strings.TrimSpace(raw)
|
||||
if title == "" || strings.ContainsAny(title, "\r\n") {
|
||||
|
||||
Vendored
+2
@@ -911,6 +911,8 @@ function renderEvent(ev, ctx) {
|
||||
}
|
||||
case "task.merge_queued":
|
||||
return h("div", { class: "note" }, `${ev.actor} asked to merge; held until the merge in progress in this project is done `, when);
|
||||
case "task.merge_stopped":
|
||||
return h("div", { class: "note" }, `${ev.actor} stopped the merge for review: ${String(p["reason"])} `, when);
|
||||
case "preview.exposed": {
|
||||
const url = previewURL(Number(p["port"]));
|
||||
return h("div", { class: "note" }, `${ev.actor} exposed "${String(p["name"])}" at `, h("a", { href: url, target: "_blank", rel: "noopener" }, url), " ", when);
|
||||
|
||||
Vendored
+1
-1
File diff suppressed because one or more lines are too long
@@ -1078,6 +1078,9 @@ function renderEvent(ev: ByreEvent, ctx: Ctx): HTMLElement | null {
|
||||
case "task.merge_queued":
|
||||
return h("div", { class: "note" }, `${ev.actor} asked to merge; held until the merge in progress in this project is done `, when);
|
||||
|
||||
case "task.merge_stopped":
|
||||
return h("div", { class: "note" }, `${ev.actor} stopped the merge for review: ${String(p["reason"])} `, when);
|
||||
|
||||
case "preview.exposed": {
|
||||
const url = previewURL(Number(p["port"]));
|
||||
return h("div", { class: "note" }, `${ev.actor} exposed "${String(p["name"])}" at `, h("a", { href: url, target: "_blank", rel: "noopener" }, url), " ", when);
|
||||
|
||||
Reference in New Issue
Block a user