From 2450df9db146154bbc93ad3ac2dd37f84c39a665 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:27:31 +0800 Subject: [PATCH 01/18] fix(execenv): accept fast-forward worktree delivery --- .../internal/daemon/execenv/local_worktree.go | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 440bee6c4ef..2f54e1ab365 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -654,7 +654,7 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err // makes that recoverable — the worktree is still there to preserve, exactly // as for a commit that could not be made. if !dropped { - if verifyErr := w.verifyDeliveryPoint(tip); verifyErr != nil { + if verifyErr := w.verifyDeliveryPoint(tip, logger); verifyErr != nil { outcome.Branch = "" outcome.PreservedPath = w.Path if logger != nil { @@ -1419,18 +1419,17 @@ func quotedPaths(paths []string) string { // verifyDeliveryPoint checks that tip is a commit this conversation can put its // name on before it becomes the branch's recorded checkpoint. // -// Two things are asserted, and they are the two ways a delivery can be -// something other than what this task built. The tip has to BE the task's -// branch — a run that checked out something else, or a branch someone moved -// underneath it, delivers a commit this record has no business describing. And -// it has to still contain the commit this turn started from — this turn's own -// baseline when it made one, otherwise the branch tip it continued. A run that -// resets its worktree back to the user's own HEAD passes neither test but the -// second is the one that matters, twice over: recording a plain user commit as -// the checkpoint is what makes a branch they later recreate there look like -// ours, and a tip without this turn's starting point no longer carries the -// snapshot about to be recorded as delivered (MUL-6881 review). -func (w *LocalWorktree) verifyDeliveryPoint(tip string) error { +// The ordinary case ends on the conversation branch itself, so its ref already +// equals tip. A repository may instead require the agent to check out its own +// task branch inside the worktree. That is still a valid delivery when tip is a +// strict descendant of the conversation branch: advancing the conversation ref +// is then only a fast-forward, and keeps the next turn on the work just +// delivered. +// +// Refuse any divergence. The compare-and-swap form of update-ref also makes the +// advance atomic: if the conversation branch moves after we inspect it, this +// finalize fails rather than overwriting somebody else's work. +func (w *LocalWorktree) verifyDeliveryPoint(tip string, logger *slog.Logger) error { if !w.tracksState { // Nothing will be recorded for this branch, so there is nothing to prove. return nil @@ -1438,13 +1437,17 @@ func (w *LocalWorktree) verifyDeliveryPoint(tip string) error { if tip == "" { return errors.New("the task worktree has no resolvable HEAD") } - branchTip, err := runGitTrimmed(w.GitRoot, "rev-parse", "--verify", "refs/heads/"+w.Branch) + branchRef := "refs/heads/" + w.Branch + branchTip, err := runGitTrimmed(w.GitRoot, "rev-parse", "--verify", branchRef) if err != nil { return fmt.Errorf("resolve branch %s: %w", w.Branch, err) } - if branchTip != tip { - return fmt.Errorf("the worktree delivered %s while branch %s points at %s, so the run did not deliver onto its own branch", - shortID(tip), w.Branch, shortID(branchTip)) + advanceBranch := branchTip != tip + if advanceBranch { + if _, err := runGit(w.GitRoot, "merge-base", "--is-ancestor", branchTip, tip); err != nil { + return fmt.Errorf("the worktree delivered %s while branch %s points at %s, so the run did not deliver a fast-forward of its conversation branch", + shortID(tip), w.Branch, shortID(branchTip)) + } } if w.BaseCommit == "" { return fmt.Errorf("branch %s has no commit of this task's own to prove it by", w.Branch) @@ -1453,6 +1456,17 @@ func (w *LocalWorktree) verifyDeliveryPoint(tip string) error { return fmt.Errorf("the delivered commit %s no longer contains %s, the commit this turn started from", shortID(tip), shortID(w.BaseCommit)) } + if advanceBranch { + out, err := runGit(w.GitRoot, "update-ref", branchRef, tip, branchTip) + if err != nil { + return fmt.Errorf("fast-forward branch %s from %s to delivered commit %s: %s: %w", + w.Branch, shortID(branchTip), shortID(tip), strings.TrimSpace(out), err) + } + if logger != nil { + logger.Warn("execenv: worktree delivered on another branch; fast-forwarded the conversation branch", + "path", w.Path, "branch", w.Branch, "from", branchTip, "to", tip) + } + } return nil } From 4e10053181848a1b19eb290407ba604da5d7be1e Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:27:35 +0800 Subject: [PATCH 02/18] test(execenv): cover off-branch worktree delivery --- .../daemon/execenv/local_worktree_test.go | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 0fae1a72543..3c96db24c25 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1569,6 +1569,105 @@ func TestFinalizeRefusesToRecordADeliveryThatResetPastItsBaseline(t *testing.T) } } +// A repository may require the agent to work on its own task-named branch +// inside Multica's worktree. If that branch simply extends the conversation +// branch, Finalize can safely book the delivery by fast-forwarding the +// conversation ref instead of turning a successful run into a failure. +func TestFinalizeFastForwardsConversationBranchToOffBranchDelivery(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + conversationTip := gitRun(t, repo, "rev-parse", wt.Branch) + if conversationTip != wt.BaseCommit { + t.Fatalf("conversation tip = %s, want this turn's base %s", conversationTip, wt.BaseCommit) + } + + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "delivered on the repo branch\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + outcome := finalizeOK(t, wt) + if outcome.Branch != wt.Branch { + t.Fatalf("Branch = %q, want conversation branch %q", outcome.Branch, wt.Branch) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != delivered { + t.Fatalf("conversation branch = %s, want delivered tip %s", got, delivered) + } + ref, err := readUserStateRef(repo, wt.Branch) + if err != nil { + t.Fatalf("readUserStateRef: %v", err) + } + record, err := readBranchRecord(repo, ref) + if err != nil { + t.Fatalf("readBranchRecord: %v", err) + } + if record.checkpoint != delivered { + t.Errorf("checkpoint = %s, want delivered tip %s", record.checkpoint, delivered) + } + + // The next turn must continue from the recovered delivery rather than redo + // work from the old conversation tip. + next := prepareTurn(t, repo, "MUL-8541", turnTwoTask) + if !next.Continued { + t.Fatal("next turn did not continue the fast-forwarded conversation branch") + } + if got := readFile(t, filepath.Join(next.WorkDir, "agent.txt")); got != "delivered on the repo branch\n" { + t.Errorf("next turn lost the delivered file: %q", got) + } + finalizeOK(t, next) +} + +// A fast-forward recovery must never become a force-update. Once another +// worktree has advanced the conversation branch on a different line, the +// off-branch delivery and the conversation branch have diverged; Finalize must +// preserve the task worktree and leave the conversation branch untouched. +func TestFinalizeRefusesDivergedOffBranchDelivery(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "off-branch delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + other := filepath.Join(t.TempDir(), "conversation") + gitRun(t, repo, "worktree", "add", "--quiet", other, wt.Branch) + writeFile(t, filepath.Join(other, "other.txt"), "concurrent conversation work\n") + gitRun(t, other, "add", "-A") + gitRun(t, other, "commit", "-m", "concurrent conversation work") + conversationTip := gitRun(t, other, "rev-parse", "HEAD") + gitRun(t, repo, "worktree", "remove", "--force", other) + + if _, err := gitTry(t, repo, "merge-base", "--is-ancestor", conversationTip, delivered); err == nil { + t.Fatal("test setup did not create divergent histories") + } + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize accepted a delivery that diverged from the conversation branch") + } + if !strings.Contains(err.Error(), "did not deliver a fast-forward") { + t.Errorf("error does not explain the divergence: %v", err) + } + if outcome.Branch != "" { + t.Errorf("outcome named branch %q for a refused delivery", outcome.Branch) + } + if outcome.PreservedPath != wt.Path { + t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != conversationTip { + t.Errorf("conversation branch moved to %s, want concurrent tip %s", got, conversationTip) + } + if _, statErr := os.Stat(wt.Path); statErr != nil { + t.Errorf("worktree removed despite refusing the divergent delivery: %v", statErr) + } +} + // The same guard from the other side: whatever the worktree delivered has to BE // the task's branch. A run that ended somewhere else — a detached checkout, a // different branch — delivered a commit this record has no business describing, From c01f9666b1b9cf31a91823256681f9d3128e3d66 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:29:02 +0800 Subject: [PATCH 03/18] fix(execenv): protect checked-out conversation refs --- .../internal/daemon/execenv/local_worktree.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 2f54e1ab365..03fe35c2af4 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -1457,6 +1457,15 @@ func (w *LocalWorktree) verifyDeliveryPoint(tip string, logger *slog.Logger) err shortID(tip), shortID(w.BaseCommit)) } if advanceBranch { + // update-ref is atomic but deliberately lower-level than the porcelain + // branch commands: it will move a branch even when another worktree has + // it checked out. Do not change HEAD underneath a sibling/user checkout. + if checkedOutAt, checkedOut, err := branchWorktreePath(w.GitRoot, w.Branch); err != nil { + return fmt.Errorf("check whether branch %s is in use before fast-forwarding it: %w", w.Branch, err) + } else if checkedOut { + return fmt.Errorf("branch %s is checked out at %s, so it cannot be advanced to delivered commit %s without moving another worktree underneath it", + w.Branch, checkedOutAt, shortID(tip)) + } out, err := runGit(w.GitRoot, "update-ref", branchRef, tip, branchTip) if err != nil { return fmt.Errorf("fast-forward branch %s from %s to delivered commit %s: %s: %w", @@ -1470,6 +1479,27 @@ func (w *LocalWorktree) verifyDeliveryPoint(tip string, logger *slog.Logger) err return nil } +// branchWorktreePath reports where branch is currently checked out, if +// anywhere. A low-level update-ref does not enforce git's normal checked-out +// branch protection, so callers that move a branch ref must check this first. +func branchWorktreePath(gitRoot, branch string) (string, bool, error) { + out, err := runGitStdout(gitRoot, "worktree", "list", "--porcelain") + if err != nil { + return "", false, err + } + target := "branch refs/heads/" + branch + var worktreePath string + for _, line := range strings.Split(out, "\n") { + switch { + case strings.HasPrefix(line, "worktree "): + worktreePath = strings.TrimPrefix(line, "worktree ") + case line == target: + return worktreePath, true, nil + } + } + return "", false, nil +} + // unmergedPaths lists the files git considers unresolved in a worktree. func unmergedPaths(worktreePath string) ([]string, error) { out, err := runGitStdout(worktreePath, "diff", "--name-only", "--diff-filter=U", "-z") From d5ae97bec70837b2c1850b953d0e9f9c66200d05 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:29:06 +0800 Subject: [PATCH 04/18] test(execenv): protect live conversation checkout --- .../daemon/execenv/local_worktree_test.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 3c96db24c25..e1c50f17bc7 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1620,6 +1620,46 @@ func TestFinalizeFastForwardsConversationBranchToOffBranchDelivery(t *testing.T) finalizeOK(t, next) } +// Moving the ref is only safe when nobody else has the conversation branch +// checked out. git update-ref itself does not protect linked worktrees, so keep +// the successful delivery preserved rather than changing another checkout's +// HEAD underneath it. +func TestFinalizeRefusesFastForwardWhenConversationBranchIsCheckedOutElsewhere(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "off-branch delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + other := filepath.Join(t.TempDir(), "conversation") + gitRun(t, repo, "worktree", "add", "--quiet", other, wt.Branch) + conversationTip := gitRun(t, repo, "rev-parse", wt.Branch) + if _, err := gitTry(t, repo, "merge-base", "--is-ancestor", conversationTip, delivered); err != nil { + t.Fatal("test setup is not a fast-forward delivery") + } + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize moved a conversation branch that another worktree had checked out") + } + if !strings.Contains(err.Error(), "checked out at") { + t.Errorf("error does not explain that the branch is in use: %v", err) + } + if outcome.PreservedPath != wt.Path { + t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != conversationTip { + t.Errorf("conversation branch moved to %s, want %s", got, conversationTip) + } + if _, statErr := os.Stat(wt.Path); statErr != nil { + t.Errorf("delivery worktree was removed after the ref move was refused: %v", statErr) + } +} + // A fast-forward recovery must never become a force-update. Once another // worktree has advanced the conversation branch on a different line, the // off-branch delivery and the conversation branch have diverged; Finalize must From 9d4d002fade439fd16efeb43a6922758f4525cbe Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Thu, 8 Oct 2026 17:42:19 +0800 Subject: [PATCH 05/18] test(execenv): match worktree non-fast-forward delivery diagnostic --- server/internal/daemon/execenv/local_worktree_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index e1c50f17bc7..56c03f90573 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1729,7 +1729,7 @@ func TestFinalizeRefusesToRecordADeliveryFromOffTheBranch(t *testing.T) { if err == nil { t.Fatal("Finalize recorded a delivery that is not the branch's tip") } - if !strings.Contains(err.Error(), "did not deliver onto its own branch") { + if !strings.Contains(err.Error(), "did not deliver a fast-forward of its conversation branch") { t.Errorf("error does not explain the mismatch: %v", err) } if outcome.PreservedPath != wt.Path { From 39f30e3c9c4b7bf555ef393cb5e6bfd1b9740ec2 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:16:51 +0800 Subject: [PATCH 06/18] fix(execenv): atomically record fast-forward delivery and checkpoint --- .../internal/daemon/execenv/local_worktree.go | 108 +++++++++++++----- .../daemon/execenv/local_worktree_test.go | 95 +++++++++++++++ 2 files changed, 175 insertions(+), 28 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 03fe35c2af4..0d98b1408a1 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -654,7 +654,8 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err // makes that recoverable — the worktree is still there to preserve, exactly // as for a commit that could not be made. if !dropped { - if verifyErr := w.verifyDeliveryPoint(tip, logger); verifyErr != nil { + advanceFrom, verifyErr := w.verifyDeliveryPoint(tip) + if verifyErr != nil { outcome.Branch = "" outcome.PreservedPath = w.Path if logger != nil { @@ -666,15 +667,24 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err "recover the work from there, and let the run keep the commit the worktree started from instead of resetting past it", w.Branch, verifyErr, w.Path, w.GitRoot) } - if recErr := w.recordState(tip, logger); recErr != nil { + var recErr error + if advanceFrom != "" { + recErr = w.recordFastForwardState(advanceFrom, tip, logger) + } else { + recErr = w.recordState(tip, logger) + } + if recErr != nil { + if advanceFrom != "" { + outcome.Branch = "" // The conversation ref was not advanced. + } outcome.PreservedPath = w.Path if logger != nil { logger.Error("execenv: could not record the delivered task branch; keeping the worktree", "path", w.Path, "branch", w.Branch, "git_root", w.GitRoot, "error", recErr) } return outcome, fmt.Errorf( - "could not record branch %s as this conversation's: %w; the work is committed to that branch and the "+ - "task worktree is preserved at %s (listed by `git worktree list` in %s) — a follow-up run will start "+ + "could not record branch %s as this conversation's: %w; the task worktree is preserved "+ + "at %s (listed by `git worktree list` in %s) — a follow-up run will start "a new branch instead of continuing this one", w.Branch, recErr, w.Path, w.GitRoot) } @@ -1058,6 +1068,19 @@ type branchRecord struct { // means a branch that moved in that window simply fails the ancestor test next // time, which is the safe direction. func writeBranchRecord(gitRoot, branch, userState, checkpoint string, owner branchOwner) (string, error) { + record, err := createBranchRecord(gitRoot, branch, userState, checkpoint, owner) + if err != nil { + return "", err + } + if out, err := runGit(gitRoot, "update-ref", userStateRef(branch), record); err != nil { + return "", fmt.Errorf("git update-ref: %s: %w", strings.TrimSpace(out), err) + } + return record, nil +} + +// createBranchRecord writes the checkpoint object without publishing its ref. +// Finalize can then publish it with the conversation branch in one transaction. +func createBranchRecord(gitRoot, branch, userState, checkpoint string, owner branchOwner) (string, error) { if checkpoint == "" { return "", fmt.Errorf("no checkpoint to record for branch %s", branch) } @@ -1067,9 +1090,6 @@ func writeBranchRecord(gitRoot, branch, userState, checkpoint string, owner bran if err != nil { return "", fmt.Errorf("git commit-tree: %w", err) } - if out, err := runGit(gitRoot, "update-ref", userStateRef(branch), record); err != nil { - return "", fmt.Errorf("git update-ref: %s: %w", strings.TrimSpace(out), err) - } return record, nil } @@ -1426,34 +1446,34 @@ func quotedPaths(paths []string) string { // is then only a fast-forward, and keeps the next turn on the work just // delivered. // -// Refuse any divergence. The compare-and-swap form of update-ref also makes the -// advance atomic: if the conversation branch moves after we inspect it, this -// finalize fails rather than overwriting somebody else's work. -func (w *LocalWorktree) verifyDeliveryPoint(tip string, logger *slog.Logger) error { +// Refuse any divergence. Return the expected old tip rather than moving the +// branch here: Finalize atomically updates the branch AND its checkpoint, so a +// failed checkpoint write cannot strand an unrecorded branch advance. +func (w *LocalWorktree) verifyDeliveryPoint(tip string) (string, error) { if !w.tracksState { // Nothing will be recorded for this branch, so there is nothing to prove. - return nil + return "", nil } if tip == "" { - return errors.New("the task worktree has no resolvable HEAD") + return "", errors.New("the task worktree has no resolvable HEAD") } branchRef := "refs/heads/" + w.Branch branchTip, err := runGitTrimmed(w.GitRoot, "rev-parse", "--verify", branchRef) if err != nil { - return fmt.Errorf("resolve branch %s: %w", w.Branch, err) + return "", fmt.Errorf("resolve branch %s: %w", w.Branch, err) } advanceBranch := branchTip != tip if advanceBranch { if _, err := runGit(w.GitRoot, "merge-base", "--is-ancestor", branchTip, tip); err != nil { - return fmt.Errorf("the worktree delivered %s while branch %s points at %s, so the run did not deliver a fast-forward of its conversation branch", + return "", fmt.Errorf("the worktree delivered %s while branch %s points at %s, so the run did not deliver a fast-forward of its conversation branch", shortID(tip), w.Branch, shortID(branchTip)) } } if w.BaseCommit == "" { - return fmt.Errorf("branch %s has no commit of this task's own to prove it by", w.Branch) + return "", fmt.Errorf("branch %s has no commit of this task's own to prove it by", w.Branch) } if _, err := runGit(w.GitRoot, "merge-base", "--is-ancestor", w.BaseCommit, tip); err != nil { - return fmt.Errorf("the delivered commit %s no longer contains %s, the commit this turn started from", + return "", fmt.Errorf("the delivered commit %s no longer contains %s, the commit this turn started from", shortID(tip), shortID(w.BaseCommit)) } if advanceBranch { @@ -1461,20 +1481,38 @@ func (w *LocalWorktree) verifyDeliveryPoint(tip string, logger *slog.Logger) err // branch commands: it will move a branch even when another worktree has // it checked out. Do not change HEAD underneath a sibling/user checkout. if checkedOutAt, checkedOut, err := branchWorktreePath(w.GitRoot, w.Branch); err != nil { - return fmt.Errorf("check whether branch %s is in use before fast-forwarding it: %w", w.Branch, err) + return "", fmt.Errorf("check whether branch %s is in use before fast-forwarding it: %w", w.Branch, err) } else if checkedOut { - return fmt.Errorf("branch %s is checked out at %s, so it cannot be advanced to delivered commit %s without moving another worktree underneath it", + return "", fmt.Errorf("branch %s is checked out at %s, so it cannot be advanced to delivered commit %s without moving another worktree underneath it", w.Branch, checkedOutAt, shortID(tip)) } - out, err := runGit(w.GitRoot, "update-ref", branchRef, tip, branchTip) - if err != nil { - return fmt.Errorf("fast-forward branch %s from %s to delivered commit %s: %s: %w", - w.Branch, shortID(branchTip), shortID(tip), strings.TrimSpace(out), err) - } - if logger != nil { - logger.Warn("execenv: worktree delivered on another branch; fast-forwarded the conversation branch", - "path", w.Path, "branch", w.Branch, "from", branchTip, "to", tip) - } + return branchTip, nil + } + return "", nil +} + +// recordFastForwardState advances the conversation ref and its ownership +// checkpoint in one Git ref transaction. A failed update of either ref leaves +// both untouched; the worktree and its off-branch commit remain recoverable. +func (w *LocalWorktree) recordFastForwardState(from, tip string, logger *slog.Logger) error { + if w.userState == "" { + return fmt.Errorf("branch %s has no user snapshot to record", w.Branch) + } + record, err := createBranchRecord(w.GitRoot, w.Branch, w.userState, tip, w.owner) + if err != nil { + return err + } + branchRef := "refs/heads/" + w.Branch + input := fmt.Sprintf("start\nupdate %s %s %s\nupdate %s %s\nprepare\ncommit\n", + branchRef, tip, from, userStateRef(w.Branch), record) + out, err := runGitInput(w.GitRoot, input, "update-ref", "--stdin") + if err != nil { + return fmt.Errorf("atomically fast-forward branch %s from %s to %s and record its checkpoint: %s: %w", + w.Branch, shortID(from), shortID(tip), strings.TrimSpace(out), err) + } + if logger != nil { + logger.Warn("execenv: worktree delivered on another branch; fast-forwarded the conversation branch", + "path", w.Path, "branch", w.Branch, "from", from, "to", tip) } return nil } @@ -1726,6 +1764,20 @@ func runGitEnv(dir string, extraEnv []string, args ...string) (string, error) { return string(out), err } +// runGitInput runs a bounded local Git command with its protocol on stdin. +// Used for update-ref transactions so both delivery refs commit or neither does. +func runGitInput(dir, input string, args ...string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), gitTimeout) + defer cancel() + + full := append([]string{"-C", dir}, args...) + cmd := exec.CommandContext(ctx, "git", full...) + cmd.Stdin = strings.NewReader(input) + cmd.WaitDelay = 5 * time.Second + out, err := cmd.CombinedOutput() + return string(out), err +} + // runGitTrimmed runs git for its stdout value, discarding stderr so a // diagnostic line can't be mistaken for the value (`rev-parse` output, a // config value, a stash sha). diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 56c03f90573..c2bec47cf63 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1620,6 +1620,101 @@ func TestFinalizeFastForwardsConversationBranchToOffBranchDelivery(t *testing.T) finalizeOK(t, next) } +// A failed checkpoint update must not leave the conversation branch advanced +// without its matching state record. Git's multi-ref transaction is all-or-none +// even when the checkpoint ref is locked by another process. +func TestFinalizeFastForwardKeepsRefsWhenCheckpointIsLocked(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + originalTip := gitRun(t, repo, "rev-parse", wt.Branch) + originalState, err := readUserStateRef(repo, wt.Branch) + if err != nil { + t.Fatalf("read initial state: %v", err) + } + + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "delivered off branch\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + commonDir, err := gitCommonDirFor(repo) + if err != nil { + t.Fatalf("git common dir: %v", err) + } + lockPath := filepath.Join(commonDir, filepath.FromSlash(userStateRef(wt.Branch))+".lock") + if err := os.MkdirAll(filepath.Dir(lockPath), 0o700); err != nil { + t.Fatalf("create ref directory: %v", err) + } + if err := os.WriteFile(lockPath, []byte("busy"), 0o600); err != nil { + t.Fatalf("lock checkpoint ref: %v", err) + } + t.Cleanup(func() { _ = os.Remove(lockPath) }) + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize succeeded despite a locked checkpoint ref") + } + if outcome.Branch != "" { + t.Errorf("Branch = %q, want no claimed delivery", outcome.Branch) + } + if outcome.PreservedPath != wt.Path { + t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != originalTip { + t.Errorf("conversation branch advanced to %s, want %s", got, originalTip) + } + state, err := readUserStateRef(repo, wt.Branch) + if err != nil || state != originalState { + t.Errorf("state ref = %q, err = %v; want %s", state, err, originalState) + } + if got := gitRun(t, repo, "rev-parse", "repo/task-branch"); got != delivered { + t.Errorf("delivery ref = %s, want %s", got, delivered) + } + if _, statErr := os.Stat(wt.Path); statErr != nil { + t.Errorf("delivery worktree was not preserved: %v", statErr) + } +} + +// A concurrent branch move after verification must fail the expected-old-SHA +// check without publishing the new checkpoint either. +func TestFastForwardStateRejectsStaleConversationTip(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + originalTip := gitRun(t, repo, "rev-parse", wt.Branch) + originalState, err := readUserStateRef(repo, wt.Branch) + if err != nil { + t.Fatalf("read initial state: %v", err) + } + + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + other := filepath.Join(t.TempDir(), "conversation") + gitRun(t, repo, "worktree", "add", "--quiet", other, wt.Branch) + writeFile(t, filepath.Join(other, "concurrent.txt"), "concurrent work\n") + gitRun(t, other, "add", "-A") + gitRun(t, other, "commit", "-m", "concurrent work") + concurrentTip := gitRun(t, other, "rev-parse", "HEAD") + gitRun(t, repo, "worktree", "remove", "--force", other) + + if err := wt.recordFastForwardState(originalTip, delivered, worktreeTestLogger()); err == nil { + t.Fatal("stale compare-and-swap advanced the conversation branch") + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != concurrentTip { + t.Errorf("conversation branch = %s, want concurrent tip %s", got, concurrentTip) + } + state, err := readUserStateRef(repo, wt.Branch) + if err != nil || state != originalState { + t.Errorf("state ref = %q, err = %v; want %s", state, err, originalState) + } +} + // Moving the ref is only safe when nobody else has the conversation branch // checked out. git update-ref itself does not protect linked worktrees, so keep // the successful delivery preserved rather than changing another checkout's From e1fb12172cb39972991bbdf989fe105f7e647134 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:17:29 +0800 Subject: [PATCH 07/18] fix(execenv): correct checkpoint failure diagnostic concatenation --- server/internal/daemon/execenv/local_worktree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 0d98b1408a1..311bc8ba8e5 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -684,7 +684,7 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err } return outcome, fmt.Errorf( "could not record branch %s as this conversation's: %w; the task worktree is preserved "+ - "at %s (listed by `git worktree list` in %s) — a follow-up run will start + "at %s (listed by `git worktree list` in %s) — a follow-up run will start "+ "a new branch instead of continuing this one", w.Branch, recErr, w.Path, w.GitRoot) } From 9098f9df93623163c8691c500248f0afe0eb99dc Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 9 Oct 2026 00:15:01 +0800 Subject: [PATCH 08/18] fix(execenv): compare-and-swap prepared checkpoint during fast-forward --- .../internal/daemon/execenv/local_worktree.go | 18 +++- .../daemon/execenv/local_worktree_test.go | 87 +++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 311bc8ba8e5..978f8cc3e55 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -162,6 +162,10 @@ type LocalWorktree struct { // turn replayed and still be recorded as having delivered it, which is how // the user's edits went missing from the turn after (MUL-6881 review). BaseCommit string + // preparedStateRef is the exact checkpoint ref written before the agent ran. + // Finalize must not overwrite a concurrent checkpoint update, even when the + // conversation branch itself has not moved. + preparedStateRef string // DirtyBaseCaptured records that the user had uncommitted tracked edits // which were replayed into the worktree. DirtyBaseCaptured bool @@ -1498,13 +1502,19 @@ func (w *LocalWorktree) recordFastForwardState(from, tip string, logger *slog.Lo if w.userState == "" { return fmt.Errorf("branch %s has no user snapshot to record", w.Branch) } + if w.preparedStateRef == "" { + return fmt.Errorf("branch %s has no checkpoint recorded before this turn; refusing to overwrite an unknown state", w.Branch) + } record, err := createBranchRecord(w.GitRoot, w.Branch, w.userState, tip, w.owner) if err != nil { return err } branchRef := "refs/heads/" + w.Branch - input := fmt.Sprintf("start\nupdate %s %s %s\nupdate %s %s\nprepare\ncommit\n", - branchRef, tip, from, userStateRef(w.Branch), record) + // Compare-and-swap BOTH refs. The prepared checkpoint is the only state + // this turn is entitled to replace; a concurrent writer can update it + // without moving the branch, and must not be silently overwritten. + input := fmt.Sprintf("start\nupdate %s %s %s\nupdate %s %s %s\nprepare\ncommit\n", + branchRef, tip, from, userStateRef(w.Branch), record, w.preparedStateRef) out, err := runGitInput(w.GitRoot, input, "update-ref", "--stdin") if err != nil { return fmt.Errorf("atomically fast-forward branch %s from %s to %s and record its checkpoint: %s: %w", @@ -1594,9 +1604,11 @@ func (w *LocalWorktree) recordState(checkpoint string, logger *slog.Logger) erro if w == nil || !w.tracksState || w.Branch == "" || w.userState == "" { return nil } - if _, err := writeBranchRecord(w.GitRoot, w.Branch, w.userState, checkpoint, w.owner); err != nil { + record, err := writeBranchRecord(w.GitRoot, w.Branch, w.userState, checkpoint, w.owner) + if err != nil { return err } + w.preparedStateRef = record if logger != nil { logger.Debug("execenv: recorded the local-directory snapshot for the task branch", "branch", w.Branch, "checkpoint", checkpoint) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index c2bec47cf63..565be110a33 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1715,6 +1715,93 @@ func TestFastForwardStateRejectsStaleConversationTip(t *testing.T) { } } +// A concurrent writer may replace the checkpoint without moving the branch. +// Finalize must not erase that record: both refs have to be compare-and-swapped +// against the exact values this task owned before the agent ran. +func TestFinalizeFastForwardRejectsConcurrentCheckpointUpdate(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + originalTip := gitRun(t, repo, "rev-parse", wt.Branch) + preparedState := wt.preparedStateRef + if preparedState == "" { + t.Fatal("Prepare did not record its checkpoint") + } + + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + // A different snapshot makes a valid but distinct checkpoint record, + // even though the conversation branch still points at originalTip. + otherSnapshot := gitRun(t, repo, "commit-tree", wt.userState+"^{tree}", "-m", "concurrent snapshot") + concurrentState, err := writeBranchRecord(repo, wt.Branch, otherSnapshot, originalTip, wt.owner) + if err != nil { + t.Fatalf("write concurrent checkpoint: %v", err) + } + if concurrentState == preparedState { + t.Fatal("concurrent checkpoint did not change the state ref") + } + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize overwrote a concurrently updated checkpoint") + } + if outcome.Branch != "" { + t.Errorf("Branch = %q, want no claimed delivery", outcome.Branch) + } + if outcome.PreservedPath != wt.Path { + t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != originalTip { + t.Errorf("conversation branch = %s, want %s", got, originalTip) + } + state, err := readUserStateRef(repo, wt.Branch) + if err != nil || state != concurrentState { + t.Errorf("checkpoint ref = %q, err = %v; want %s", state, err, concurrentState) + } + if got := gitRun(t, repo, "rev-parse", "repo/task-branch"); got != delivered { + t.Errorf("delivery ref = %s, want %s", got, delivered) + } + if _, statErr := os.Stat(wt.Path); statErr != nil { + t.Errorf("delivery worktree was not preserved: %v", statErr) + } +} + +// If Prepare could not publish a checkpoint, Finalize must preserve the +// off-branch delivery rather than overwrite an unknown record. +func TestFinalizeFastForwardRejectsMissingPreparedCheckpoint(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + originalTip := gitRun(t, repo, "rev-parse", wt.Branch) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + + // Simulate another actor removing the state ref after Prepare. + gitRun(t, repo, "update-ref", "-d", userStateRef(wt.Branch)) + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize succeeded after its prepared checkpoint was deleted") + } + if outcome.Branch != "" || outcome.PreservedPath != wt.Path { + t.Errorf("outcome = %+v, want preserved worktree and no claimed branch", outcome) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != originalTip { + t.Errorf("conversation branch = %s, want %s", got, originalTip) + } + if _, err := readUserStateRef(repo, wt.Branch); err == nil { + t.Fatal("Finalize recreated a concurrently deleted checkpoint") + } + if _, statErr := os.Stat(wt.Path); statErr != nil { + t.Errorf("delivery worktree was not preserved: %v", statErr) + } +} + // Moving the ref is only safe when nobody else has the conversation branch // checked out. git update-ref itself does not protect linked worktrees, so keep // the successful delivery preserved rather than changing another checkout's From 89e2ce8f5d15bfceafc14159ed4414a7147a9060 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:18:08 +0800 Subject: [PATCH 09/18] fix(execenv): preserve prepared checkpoint across isolated prepare --- .../internal/daemon/execenv/local_worktree.go | 40 +++++++------ .../daemon/execenv/local_worktree_test.go | 59 +++++++++++++++++++ 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 978f8cc3e55..fed693e2d22 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -229,20 +229,22 @@ func (w *LocalWorktree) MarshalJSON() ([]byte, error) { type wire LocalWorktree return json.Marshal(struct { *wire - CreatedBranch bool `json:"created_branch"` - UserState string `json:"user_state"` - PriorState string `json:"prior_state"` - Owner branchOwner `json:"owner"` - TracksState bool `json:"tracks_state"` - SnapshotPending bool `json:"snapshot_pending"` + CreatedBranch bool `json:"created_branch"` + PreparedStateRef string `json:"prepared_state_ref"` + UserState string `json:"user_state"` + PriorState string `json:"prior_state"` + Owner branchOwner `json:"owner"` + TracksState bool `json:"tracks_state"` + SnapshotPending bool `json:"snapshot_pending"` }{ wire: (*wire)(w), - CreatedBranch: w.createdBranch, - UserState: w.userState, - PriorState: w.priorState, - Owner: w.owner, - TracksState: w.tracksState, - SnapshotPending: w.snapshotPending, + CreatedBranch: w.createdBranch, + PreparedStateRef: w.preparedStateRef, + UserState: w.userState, + PriorState: w.priorState, + Owner: w.owner, + TracksState: w.tracksState, + SnapshotPending: w.snapshotPending, }) } @@ -250,17 +252,19 @@ func (w *LocalWorktree) UnmarshalJSON(data []byte) error { type wire LocalWorktree aux := struct { *wire - CreatedBranch bool `json:"created_branch"` - UserState string `json:"user_state"` - PriorState string `json:"prior_state"` - Owner branchOwner `json:"owner"` - TracksState bool `json:"tracks_state"` - SnapshotPending bool `json:"snapshot_pending"` + CreatedBranch bool `json:"created_branch"` + PreparedStateRef string `json:"prepared_state_ref"` + UserState string `json:"user_state"` + PriorState string `json:"prior_state"` + Owner branchOwner `json:"owner"` + TracksState bool `json:"tracks_state"` + SnapshotPending bool `json:"snapshot_pending"` }{wire: (*wire)(w)} if err := json.Unmarshal(data, &aux); err != nil { return err } w.createdBranch = aux.CreatedBranch + w.preparedStateRef = aux.PreparedStateRef w.userState = aux.UserState w.priorState = aux.PriorState w.owner = aux.Owner diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 565be110a33..102fed598ad 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -2089,6 +2089,65 @@ func TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain(t *testing.T // issue claim, so the plumbing from the claim is covered too: the issue // identifier names the branch, and the workspace, agent and issue become the // identity the branch is recorded under. +// The daemon finalizes the JSON-decoded worktree returned by the preparation +// helper, not the in-process worktree. The expected checkpoint ref must survive +// that boundary or every off-branch fast-forward is rejected as unprepared. +func TestIsolatedPrepareFastForwardsOffBranchDelivery(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + + env, err := PrepareIsolated(ctx, preparationHelperTestCommand(), PrepareParams{ + WorkspacesRoot: t.TempDir(), + WorkspaceID: testBranchOwner.WorkspaceID, + TaskID: turnOneTask, + IssueIdentifier: "MUL-8541", + Provider: "claude", + AgentName: "J", + Task: TaskContextForEnv{ + IssueID: testBranchOwner.ConversationID, + AgentID: testBranchOwner.AgentID, + }, + LocalWorktree: &LocalWorktreeParams{LocalPath: repo}, + }, worktreeTestLogger()) + if err != nil { + t.Fatalf("PrepareIsolated: %v", err) + } + wt := env.LocalWorktree + if wt.preparedStateRef == "" { + t.Fatal("PrepareIsolated lost the prepared checkpoint ref across JSON") + } + original := gitRun(t, repo, "rev-parse", wt.Branch) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "isolated off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + if delivered == original { + t.Fatal("test did not create a new delivery commit") + } + + outcome := finalizeOK(t, wt) + if outcome.Branch != wt.Branch { + t.Fatalf("delivered branch = %q, want %q", outcome.Branch, wt.Branch) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != delivered { + t.Fatalf("conversation branch = %s, want %s", got, delivered) + } + ref, err := readUserStateRef(repo, wt.Branch) + if err != nil { + t.Fatalf("readUserStateRef: %v", err) + } + record, err := readBranchRecord(repo, ref) + if err != nil { + t.Fatalf("readBranchRecord: %v", err) + } + if record.checkpoint != delivered { + t.Errorf("checkpoint = %s, want delivered tip %s", record.checkpoint, delivered) + } +} + func TestIsolatedPrepareCarriesTheStateFinalizeNeeds(t *testing.T) { t.Parallel() repo := newTestRepo(t) From c75689843fd8a8373c05179e5f68c55ab9bc6fd5 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:18:48 +0800 Subject: [PATCH 10/18] style(execenv): align wire fields and isolate regression comment --- .../internal/daemon/execenv/local_worktree.go | 2 +- .../daemon/execenv/local_worktree_test.go | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index fed693e2d22..6de4e043fd3 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -237,7 +237,7 @@ func (w *LocalWorktree) MarshalJSON() ([]byte, error) { TracksState bool `json:"tracks_state"` SnapshotPending bool `json:"snapshot_pending"` }{ - wire: (*wire)(w), + wire: (*wire)(w), CreatedBranch: w.createdBranch, PreparedStateRef: w.preparedStateRef, UserState: w.userState, diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 102fed598ad..10e0742c06e 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -2078,17 +2078,6 @@ func TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain(t *testing.T } } -// Production never prepares in the daemon's own process: PrepareIsolated runs -// Prepare in a helper and the Environment comes back as JSON. Every guarantee -// Finalize makes depends on state this struct keeps unexported, and ordinary -// marshalling drops those fields without a word — the daemon then finalized a -// worktree it believed it owned nothing of. This test runs two turns across -// that boundary, which is where the in-process tests above cannot look. -// -// It also goes through Prepare, the entry point the daemon calls, with a full -// issue claim, so the plumbing from the claim is covered too: the issue -// identifier names the branch, and the workspace, agent and issue become the -// identity the branch is recorded under. // The daemon finalizes the JSON-decoded worktree returned by the preparation // helper, not the in-process worktree. The expected checkpoint ref must survive // that boundary or every off-branch fast-forward is rejected as unprepared. @@ -2148,6 +2137,17 @@ func TestIsolatedPrepareFastForwardsOffBranchDelivery(t *testing.T) { } } +// Production never prepares in the daemon's own process: PrepareIsolated runs +// Prepare in a helper and the Environment comes back as JSON. Every guarantee +// Finalize makes depends on state this struct keeps unexported, and ordinary +// marshalling drops those fields without a word — the daemon then finalized a +// worktree it believed it owned nothing of. This test runs two turns across +// that boundary, which is where the in-process tests above cannot look. +// +// It also goes through Prepare, the entry point the daemon calls, with a full +// issue claim, so the plumbing from the claim is covered too: the issue +// identifier names the branch, and the workspace, agent and issue become the +// identity the branch is recorded under. func TestIsolatedPrepareCarriesTheStateFinalizeNeeds(t *testing.T) { t.Parallel() repo := newTestRepo(t) From f6b6373ddff8be9ff1bd233a3b9435592d4e049b Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sat, 10 Oct 2026 10:21:40 +0800 Subject: [PATCH 11/18] fix(execenv): guard same-tip Finalize checkpoint retries with CAS --- .../internal/daemon/execenv/local_worktree.go | 35 +++++- .../daemon/execenv/local_worktree_test.go | 119 ++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 6de4e043fd3..6c34d60361c 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -679,12 +679,10 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err if advanceFrom != "" { recErr = w.recordFastForwardState(advanceFrom, tip, logger) } else { - recErr = w.recordState(tip, logger) + recErr = w.recordFinalizedState(tip) } if recErr != nil { - if advanceFrom != "" { - outcome.Branch = "" // The conversation ref was not advanced. - } + outcome.Branch = "" // Without a recorded checkpoint, no delivery can be claimed. outcome.PreservedPath = w.Path if logger != nil { logger.Error("execenv: could not record the delivered task branch; keeping the worktree", @@ -1524,6 +1522,7 @@ func (w *LocalWorktree) recordFastForwardState(from, tip string, logger *slog.Lo return fmt.Errorf("atomically fast-forward branch %s from %s to %s and record its checkpoint: %s: %w", w.Branch, shortID(from), shortID(tip), strings.TrimSpace(out), err) } + w.preparedStateRef = record if logger != nil { logger.Warn("execenv: worktree delivered on another branch; fast-forwarded the conversation branch", "path", w.Path, "branch", w.Branch, "from", from, "to", tip) @@ -1531,6 +1530,34 @@ func (w *LocalWorktree) recordFastForwardState(from, tip string, logger *slog.Lo return nil } +// recordFinalizedState handles a delivery whose HEAD already equals the +// conversation tip. This includes a retry after a successful fast-forward CAS +// when removing the worktree failed. Do not fall back to unconditional +// update-ref: that would erase a checkpoint written by another process. +func (w *LocalWorktree) recordFinalizedState(tip string) error { + if w == nil || !w.tracksState || w.Branch == "" || w.userState == "" { + return nil + } + if w.preparedStateRef == "" { + return fmt.Errorf("branch %s has no prepared checkpoint to finalize safely", w.Branch) + } + record, err := createBranchRecord(w.GitRoot, w.Branch, w.userState, tip, w.owner) + if err != nil { + return err + } + // Verify the branch tip and compare-and-swap the checkpoint atomically. + // This rejects both a concurrent branch move and a checkpoint rewrite. + input := fmt.Sprintf("start\nverify %s %s\nupdate %s %s %s\nprepare\ncommit\n", + "refs/heads/"+w.Branch, tip, userStateRef(w.Branch), record, w.preparedStateRef) + out, err := runGitInput(w.GitRoot, input, "update-ref", "--stdin") + if err != nil { + return fmt.Errorf("finalize branch %s checkpoint with CAS: %s: %w", + w.Branch, strings.TrimSpace(out), err) + } + w.preparedStateRef = record + return nil +} + // branchWorktreePath reports where branch is currently checked out, if // anywhere. A low-level update-ref does not enforce git's normal checked-out // branch protection, so callers that move a branch ref must check this first. diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 10e0742c06e..8097ca247d0 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -2262,3 +2262,122 @@ func TestIsolatedPrepareKeepsTheReadOnlyBranchDrop(t *testing.T) { t.Error("a turn that changed nothing left its branch behind") } } + +// A cleanup failure can leave the task worktree in place after the two-ref +// fast-forward CAS succeeded. Retrying Finalize must not replace a checkpoint +// that another actor wrote in the meantime. +func TestFinalizeSameTipRetryRejectsConcurrentCheckpoint(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + before := gitRun(t, repo, "rev-parse", wt.Branch) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + + if err := wt.recordFastForwardState(before, delivered, worktreeTestLogger()); err != nil { + t.Fatalf("first CAS: %v", err) + } + checkpointAfterCAS, err := readUserStateRef(repo, wt.Branch) + if err != nil || wt.preparedStateRef != checkpointAfterCAS { + t.Fatalf("prepared checkpoint = %s, read = %s, err = %v", wt.preparedStateRef, checkpointAfterCAS, err) + } + + otherSnapshot := gitRun(t, repo, "commit-tree", wt.userState+"^{tree}", "-m", "concurrent snapshot") + concurrentState, err := writeBranchRecord(repo, wt.Branch, otherSnapshot, delivered, wt.owner) + if err != nil { + t.Fatalf("concurrent checkpoint: %v", err) + } + if concurrentState == checkpointAfterCAS { + t.Fatal("concurrent checkpoint did not change") + } + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("same-tip retry overwrote a concurrently changed checkpoint") + } + if outcome.Branch != "" { + t.Errorf("Branch = %q, want no claimed delivery", outcome.Branch) + } + if outcome.PreservedPath != wt.Path { + t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != delivered { + t.Errorf("branch = %s, want delivered %s", got, delivered) + } + if got, err := readUserStateRef(repo, wt.Branch); err != nil || got != concurrentState { + t.Errorf("checkpoint = %s, err = %v; want %s", got, err, concurrentState) + } + if _, err := os.Stat(wt.Path); err != nil { + t.Errorf("worktree not preserved: %v", err) + } +} + +// Repeating Finalize with unchanged refs should complete the previously +// interrupted cleanup instead of rejecting the transaction as stale. +func TestFinalizeSameTipRetryAfterFastForwardSucceeds(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + before := gitRun(t, repo, "rev-parse", wt.Branch) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + if err := wt.recordFastForwardState(before, delivered, worktreeTestLogger()); err != nil { + t.Fatalf("first CAS: %v", err) + } + outcome, err := wt.Finalize(worktreeTestLogger()) + if err != nil { + t.Fatalf("same-tip retry: %v", err) + } + if outcome.PreservedPath != "" { + t.Errorf("unexpected preserved worktree: %s", outcome.PreservedPath) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != delivered { + t.Errorf("branch = %s, want %s", got, delivered) + } +} + +// A same-tip retry must also reject a branch that moved after the first CAS, +// even if its checkpoint was not modified by the other writer. +func TestFinalizeSameTipRetryRejectsConcurrentBranchMove(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) + before := gitRun(t, repo, "rev-parse", wt.Branch) + gitRun(t, wt.Path, "checkout", "-b", "repo/task-branch") + writeFile(t, filepath.Join(wt.WorkDir, "agent.txt"), "off-branch delivery\n") + gitRun(t, wt.Path, "add", "-A") + gitRun(t, wt.Path, "commit", "-m", "agent delivery") + delivered := gitRun(t, wt.Path, "rev-parse", "HEAD") + if err := wt.recordFastForwardState(before, delivered, worktreeTestLogger()); err != nil { + t.Fatalf("first CAS: %v", err) + } + checkpointAfterCAS, err := readUserStateRef(repo, wt.Branch) + if err != nil { + t.Fatalf("readUserStateRef: %v", err) + } + moved := gitRun(t, repo, "commit-tree", delivered+"^{tree}", "-p", delivered, "-m", "concurrent branch move") + gitRun(t, repo, "update-ref", "refs/heads/"+wt.Branch, moved, delivered) + + outcome, err := wt.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("same-tip retry accepted a concurrently moved conversation branch") + } + if outcome.Branch != "" || outcome.PreservedPath != wt.Path { + t.Errorf("failed delivery = %+v, want no branch and preserved worktree %q", outcome, wt.Path) + } + if got := gitRun(t, repo, "rev-parse", wt.Branch); got != moved { + t.Errorf("branch = %s, want concurrent tip %s", got, moved) + } + if got, err := readUserStateRef(repo, wt.Branch); err != nil || got != checkpointAfterCAS { + t.Errorf("checkpoint = %s, err = %v; want unchanged %s", got, err, checkpointAfterCAS) + } + if _, err := os.Stat(wt.Path); err != nil { + t.Errorf("worktree not preserved: %v", err) + } +} From cb0f27a9b329f712dd1a63bf4da05c6b483f332a Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:21:09 +0800 Subject: [PATCH 12/18] fix(execenv): retain conflict replay checkpoint for Finalize CAS --- .../internal/daemon/execenv/local_worktree.go | 7 +++ .../daemon/execenv/local_worktree_test.go | 61 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 6c34d60361c..54bb3502ee9 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -408,6 +408,13 @@ func PrepareLocalWorktree(params LocalWorktreeParams, logger *slog.Logger) (*Loc tracksState: plan.tracksState && actualBranch == plan.name, } + // A continued branch already has a verified checkpoint. A replay conflict + // skips recordState until the agent resolves it; retain that exact ref so + // Finalize can CAS against it instead of treating the turn as unprepared. + if wt.tracksState && plan.continues { + wt.preparedStateRef = plan.priorState + } + // Tear the worktree back down on every failure below. A half-replayed tree // is the worst outcome available: it looks like a working checkout, so // nothing downstream questions it, while the agent silently reads different diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 8097ca247d0..a88bc265af7 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -1009,6 +1009,11 @@ func TestConflictResolvedByTheAgentIsDeliveredAndNotReplayedAgain(t *testing.T) writeFile(t, filepath.Join(repo, "tracked.txt"), "C\n") second := prepareTurn(t, repo, "MUL-6881", turnTwoTask) + // A conflicted replay does not publish a new prepare checkpoint. It must + // retain the previously verified ref for Finalize's compare-and-swap. + if got, err := readUserStateRef(repo, second.Branch); err != nil || second.preparedStateRef != got { + t.Fatalf("conflict prepare checkpoint = %q, recorded = %q, err = %v", second.preparedStateRef, got, err) + } if len(second.ReplayConflicts) == 0 { t.Fatal("turn two saw no conflict between the agent's B and the user's C") } @@ -2047,6 +2052,11 @@ func TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain(t *testing.T writeFile(t, filepath.Join(repo, "tracked.txt"), "rewritten by the user instead\n") second := prepareTurn(t, repo, "MUL-6881", turnTwoTask) + // A conflicted replay does not publish a new prepare checkpoint. It must + // retain the previously verified ref for Finalize's compare-and-swap. + if got, err := readUserStateRef(repo, second.Branch); err != nil || second.preparedStateRef != got { + t.Fatalf("conflict prepare checkpoint = %q, recorded = %q, err = %v", second.preparedStateRef, got, err) + } if !second.Continued { t.Fatal("second turn did not continue the branch the user had committed on") } @@ -2078,6 +2088,57 @@ func TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain(t *testing.T } } + +func TestFinalizeConflictReplayRejectsConcurrentCheckpointUpdate(t *testing.T) { + t.Parallel() + repo := newTestRepo(t) + writeFile(t, filepath.Join(repo, "tracked.txt"), "original\n") + + first := prepareTurn(t, repo, "MUL-6881", turnOneTask) + writeFile(t, filepath.Join(first.WorkDir, "tracked.txt"), "agent\n") + finalizeOK(t, first) + + writeFile(t, filepath.Join(repo, "tracked.txt"), "user\n") + second := prepareTurn(t, repo, "MUL-6881", turnTwoTask) + if len(second.ReplayConflicts) == 0 { + t.Fatal("expected a conflicting user edit") + } + prior := gitRun(t, repo, "rev-parse", userStateRef(second.Branch)) + if second.preparedStateRef != prior { + t.Fatalf("prepared checkpoint = %q, want %q", second.preparedStateRef, prior) + } + tip := gitRun(t, repo, "rev-parse", second.Branch) + otherSnapshot := gitRun(t, repo, "commit-tree", second.userState+"^{tree}", "-m", "concurrent snapshot") + concurrentRef, err := writeBranchRecord(repo, second.Branch, otherSnapshot, tip, second.owner) + if err != nil { + t.Fatalf("concurrent checkpoint: %v", err) + } + if concurrentRef == prior { + t.Fatal("concurrent checkpoint did not change") + } + + // Resolve in favour of the existing branch content: there is no new + // commit, but Finalize must still refuse to overwrite the other writer. + writeFile(t, filepath.Join(second.WorkDir, "tracked.txt"), "agent\n") + gitRun(t, second.Path, "add", "tracked.txt") + outcome, err := second.Finalize(worktreeTestLogger()) + if err == nil { + t.Fatal("Finalize overwrote a concurrently changed conflict checkpoint") + } + if outcome.Branch != "" || outcome.PreservedPath != second.Path { + t.Errorf("failed delivery = %+v, want no branch and preserved worktree %q", outcome, second.Path) + } + if got := gitRun(t, repo, "rev-parse", userStateRef(second.Branch)); got != concurrentRef { + t.Errorf("checkpoint = %s, want concurrent ref %s", got, concurrentRef) + } + if got := gitRun(t, repo, "rev-parse", second.Branch); got != tip { + t.Errorf("branch = %s, want unchanged %s", got, tip) + } + if _, err := os.Stat(second.Path); err != nil { + t.Errorf("worktree not preserved: %v", err) + } +} + // The daemon finalizes the JSON-decoded worktree returned by the preparation // helper, not the in-process worktree. The expected checkpoint ref must survive // that boundary or every off-branch fast-forward is rejected as unprepared. From 5de7858e6fc768b70637a6b11779a598b4ee5682 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:13:46 +0800 Subject: [PATCH 13/18] fix(execenv): clear undelivered branch on commit failure and test Windows CAS paths Keep the preserved worktree authoritative if automatic commit fails; assert no branch delivery is reported. Run the existing 14 worktree delivery, checkpoint CAS, and conflict replay regressions on native Windows CI. AI-assisted implementation and review; project-level tests and native Windows results require CI. --- .github/workflows/ci.yml | 341 ++++++++++++++++++ .../internal/daemon/execenv/local_worktree.go | 1 + .../daemon/execenv/local_worktree_test.go | 3 + 3 files changed, 345 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 27e427835fd..08150df5147 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -506,6 +506,347 @@ jobs: check-latest: true cache-dependency-path: server/go.sum + - name: Test Windows worktree delivery, conflict replay and checkpoint CAS + working-directory: server + # Native Windows Git/worktree coverage for #8541, including isolated Prepare and conflict replay. + # Verbose output exposes a missing test selection or a skip. + run: go test ./internal/daemon/execenv -v -run '^(TestFinalizeFastForwardsConversationBranchToOffBranchDelivery|TestFinalizeFastForwardKeepsRefsWhenCheckpointIsLocked|TestFastForwardStateRejectsStaleConversationTip|TestFinalizeFastForwardRejectsConcurrentCheckpointUpdate|TestFinalizeFastForwardRejectsMissingPreparedCheckpoint|TestFinalizeRefusesFastForwardWhenConversationBranchIsCheckedOutElsewhere|TestFinalizeRefusesDivergedOffBranchDelivery|TestIsolatedPrepareFastForwardsOffBranchDelivery|TestFinalizeSameTipRetryRejectsConcurrentCheckpoint|TestFinalizeSameTipRetryAfterFastForwardSucceeds|TestFinalizeSameTipRetryRejectsConcurrentBranchMove|TestConflictResolvedByTheAgentIsDeliveredAndNotReplayedAgain|TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain|TestFinalizeConflictReplayRejectsConcurrentCheckpointUpdate) + working-directory: server + # Keep this job scoped to the runtime regression it exists to prove. + # The package's legacy OpenClaw HOME tests are not Windows-safe and are + # outside this PR; the normal backend job still runs the full package. + run: go test ./internal/daemon/execenv -run '^TestPrepareIsolated_WindowsKillsDescendantBeforeRetry$' -count=1 -timeout=5m + + - name: Test Windows daemon local-skill discovery + working-directory: server + # These fixtures must redirect both the Windows user profile and the + # platform-native Hermes home; otherwise discovery scans the runner's + # real skills instead of the test directories. + run: go test ./internal/daemon -v -run '^(TestListRuntimeLocalSkills_HermesFollowsTaskHome|TestLocalSkills_DiscoversACPProviderRoots)$' -count=1 -timeout=5m + + - name: Test Windows agent launcher argv/stdin handling + working-directory: server + # Agent prompts must never reach a Windows launcher through argv: the + # official cursor-agent.ps1, pi.ps1, and qwen.ps1 launch native children with + # `$args`, and PowerShell re-serialises them onto the child command + # line. Under + # Legacy native argument passing (powershell.exe 5.1, pwsh <= 7.2) a + # prompt holding embedded quotes is re-tokenised and fragments like + # `-X` become flags (#5649). Only a real PowerShell host proves this, + # so it cannot live in the ubuntu backend job. Scoped to the launcher + # tests, which are windows-tagged and therefore run nowhere else today; + # the backend job still runs the full package on Linux. + # -v so a silent skip (no PowerShell host resolved, or a -run pattern + # that stops matching) is visible in the log instead of passing as "ok". + run: go test ./pkg/agent -v -run '^(TestCursorExecutePromptSurvivesPowerShellShim|TestPiExecutePromptSurvivesPowerShellShim|TestQwenExecutePromptSurvivesPowerShellShim|TestPlatformCursorInvocation|TestPlatformCopilotInvocation|TestPlatformPiInvocation|TestPlatformQwenInvocation)' -count=1 -timeout=5m + + - name: Test Windows OpenCode oversized prompt reaches stdin + working-directory: server + # #6538: the daemon inlined the whole task prompt as an argv element, + # so every OpenCode task whose prompt cleared CreateProcess's 32,767 + # character lpCommandLine limit failed to start at all, with Go + # reporting ERROR_FILENAME_EXCED_RANGE as the misleading "The filename + # or extension is too long". Only a real CreateProcess enforces that + # ceiling, so this cannot run in the ubuntu backend job. The test + # spawns a native .exe directly (a Chocolatey-installed opencode.exe is + # a real PE binary, not a .cmd shim) with an oversized prompt and pins + # that the process starts, argv stays free of the prompt, and the full + # payload arrives on stdin. + # -v so a silent skip or a -run pattern that stops matching is visible + # in the log instead of passing as "ok". + run: go test ./pkg/agent -v -run '^TestOpencodeExecuteOversizedPromptStartsOnWindows$' -count=1 -timeout=5m + + - name: Test Windows agent process-tree ownership + working-directory: server + # A Job Object is the only way to prove whole-tree termination on + # Windows, and only a real Windows runner can exercise it: that a + # grandchild dies with the tree that owns it, that an unowned process + # still reports cleanup as unconfirmed, and that a descendant holding + # inherited stdout neither keeps Result blocked nor survives cleanup. + # -v makes RUN/PASS evidence explicit in CI logs. + run: go test ./pkg/agent -v -run '^(TestStartOwnedProcessTreeCapturesImmediateDescendants|TestStartOwnedProcessTreeLeavesNoSuspendedChild|TestWaitProcessGroupGoneWithoutOwnershipReportsUnconfirmed|TestCodexInitializeRetrySupportedWithOwnedProcessTree|TestCodexWindowsDescendantsDieWithTheOwnedProcessTree|TestCodeArtsWindowsCancellationTerminatesDescendants)$' -count=1 -timeout=5m + + - name: Test bounded CLI output collection on Windows + working-directory: server + # MUL-5467: RunCollect / RunCollectQuiet own the pipes as well as the + # process tree on the OpenClaw CLI paths, which outputOwned cannot. A real + # Windows host proves its Job Object owns and + # terminates descendants, a CLI that prints its answer and then refuses + # to exit still yields that answer, a response still streaming at the + # deadline is NOT reported as success, and no collector goroutine + # outlives the call. Unix uses process groups, so it cannot exercise this + # platform lifecycle. + # -v so a skip (a -run pattern that stops matching) is visible in the log + # instead of passing as "ok". + run: go test ./pkg/agent -v -run '^TestWindowsRunCollect' -count=1 -timeout=5m + + - name: Test Windows Pi session lock leaves the transcript readable + working-directory: server + # #7840: the session lock claimed the transcript itself, and Windows + # byte-range locks are mandatory. Pi's own read of the file we hand it + # through --session failed with EBUSY, so every Pi/omp task on Windows + # died ~0.5s after launch. Unix flock is advisory and never reproduces + # it, which is exactly how the regression shipped; the tests are + # windows-tagged and therefore run nowhere else today. + # -v so a skip (a -run pattern that stops matching) is visible in the + # log instead of passing as "ok". + run: go test ./pkg/agent -v -run '^TestPiSessionFileLock' -count=1 -timeout=5m + + - name: Test Windows OpenClaw npm shim interpreter resolution + working-directory: server + # #6061: every OpenClaw task failed execenv prep on a Windows host with + # a bare `exit status 1` and no stderr. A batch shim resolves and runs + # fine while the `node` it re-execs is unreachable, and npm's real + # template prefers a co-located node.exe over PATH — none of which can + # be proven without a real cmd.exe host. These tests pin: the positive + # control (node on PATH → success), that a missing node surfaces + # cmd.exe's own stderr (the first run of this job disproved #6061's + # premise that it does not), that a genuinely silent shim DOES reach the + # new diagnostic, that a co-located interpreter is credited, that a + # context timeout is not misdiagnosed as a missing interpreter, and that + # TEMP/TMP are NOT load-bearing (the originally reported root cause, + # since retracted upstream). + # Scoped to the windows-tagged shim tests — the package's legacy + # OpenClaw HOME tests are not Windows-safe; the backend job still runs + # the full package plus the cross-platform half on Linux. + # -v so a skip (no node on the runner) is visible instead of passing + # silently as "ok". + run: go test ./internal/daemon/execenv -v -run '^TestWindowsOpenclawShim' -count=1 -timeout=5m + + - name: Test Windows isolated repo checkout is committable + working-directory: server + # #6449: on Windows the daemon now hands Codex tasks a checkout whose + # .git lives inside the task workdir, because a linked worktree's + # external gitdir stays read-only under the native sandbox and breaks + # `git add` / `git commit` at the end of a task. Two of the guarantees + # are claims about Windows itself and cannot be made on ubuntu: that + # Git puts the gitdir inside the task directory, and that the clone's + # objects are private copies rather than NTFS hard links sharing one + # file and one security descriptor with the daemon-owned cache. The + # cross-volume test covers what a hard link cannot express at all. + # Scoped to the two windows-tagged tests by exact name. A prefix match + # also caught the package's cross-platform isolated-checkout test, + # which cannot run here: repocache derives a cache directory name from + # the full source repo path, so a t.TempDir() path that already embeds + # a long test name doubles and blows past MAX_PATH. That test belongs + # to the ubuntu backend job, which runs the whole package. + # -v so a skip (single-volume runner) is visible instead of passing + # silently as "ok". + run: go test ./internal/daemon/repocache -v -run '^(TestIsolatedCheckoutIsCommittableOnWindows|TestIsolatedCheckoutAcrossVolumesOnWindows)$' -count=1 -timeout=5m + + - name: Test Windows directory-junction link safety + working-directory: server + # MUL-6000: the per-task codex-home links the user's real skills into + # the task directory instead of copying them. On Windows that link is a + # directory junction whenever os.Symlink is denied (no Developer Mode), + # and a junction is the one link shape a ModeSymlink check misses: + # since Go 1.23 os.Lstat reports it as ModeDir|ModeIrregular with no + # ModeSymlink bit, while its DirEntry still answers IsDir() == true, so + # filepath.WalkDir descends into the target. Two claims about Windows + # itself cannot be made on ubuntu: that the per-task skills wipe + # (os.RemoveAll) drops the junction rather than the user's files, and + # that the GC's artifact sweep and size accounting refuse to walk + # through one. The tests call mklink /J directly so the junction shape + # is exercised even on a runner where symlinks are permitted. + # -v so a skip is visible instead of passing silently as "ok". + run: | + go test ./internal/daemon/execenv -v -run '^(TestSeedUserCodexSkills|TestHydrateCodexSkills)' -count=1 -timeout=5m + go test ./internal/daemon -v -run '^(TestCleanTaskArtifacts_DoesNotFollowDirectoryJunction|TestTaskSize_DoesNotCountDirectoryJunction)$' -count=1 -timeout=5m + + - name: Test Windows task temp dir sweep survives a sharing violation + working-directory: server + # The GC's task temp sweep has to cope with a file it cannot delete: a + # leftover child process holding one open is what left 174 directories + # behind in #7364. Only a real Windows filesystem produces that sharing + # violation — on unix an open file unlinks fine — so this is the only + # place we can prove the two things that matter: the failed cleanup + # keeps .task_lock (without it the directory reads as a pre-lock + # leftover and stops being reclaimable on liveness), and the next cycle + # removes the directory once the handle closes. + # -v so a skip is visible instead of passing silently as "ok". + run: go test ./internal/daemon/execenv -v -run '^TestPruneTaskTempDirsSurvivesRealSharingViolation$' -count=1 -timeout=5m + + - name: Test Windows agent executable junction resolution + working-directory: server + # The standalone Codex installer exposes bin as a directory junction. + # filepath.EvalSymlinks cannot traverse that reparse-point shape, so + # only a real Windows filesystem proves both PATH discovery and an + # explicitly configured path reach the release executable. The same job + # covers the npm shape, where the entry point is a `.cmd` shim that only + # the command interpreter can run. + run: go test ./internal/daemon -v -run '^(TestCanonicalExecutablePath|TestTrimExtendedLengthPrefix|TestResolveAgentExecutablePathKeeps|TestResolveAgentExecutablePath_ProfileOverride|TestResolveAgentEntry(FollowsRetargetedInstallerJunction|CanonicalizesRediscoveredJunction|ForLaunchKeepsCmdShimLaunchable|ForLaunchRejectsUnverifiedInitialJunctionTarget|ForLaunchRejectsRediscoveredJunctionWhenFinalPathResolutionFails|DoesNotSharePreRetargetSingleflightResult|ForLaunchFailsWhenJunctionKeepsRetargeting)|TestHandleTaskReportsWindowsCodexProcessStartFailure)' -count=1 -timeout=5m + + - name: Build Windows CLI helper entrypoint + working-directory: server + run: go build ./cmd/multica + + - name: Test Windows Cursor background ownership + working-directory: server + # Cursor's background shell is captured through a second Job Object, and + # TestCaptureCursorBackgroundProcessRejectsForeignJob is windows-tagged, + # so it builds nowhere else. This used to be its own windows-latest job + # on the same runtime scope as this one; same runner, same package, same + # concern, so it is a step here instead of a second runner per merge. + # Last on purpose: absorbing another job's steps means this job's own + # failure modes now truncate that job's coverage too, and Cursor + # ownership is the newest arrival, not the load-bearing one. The ubuntu + # backend job still covers the untagged and Unix arms. + run: go test -race ./pkg/agent -v -run 'TestCursorBackground|TestCaptureCursorBackground' -count=1 -timeout=5m + + # The only macOS runner here, and the only place cursor_background_process_darwin.go + # is ever built: it signals a process group through XNU's per-process unique + # identity so a recycled PID cannot redirect a kill, and the two tests pinning + # that are `//go:build darwin`. Windows keeps its arm as a step in + # windows-execenv, but macOS has no sibling job to join, and this covers one + # integration's background shell rather than the shared launch path every + # agent uses. So the daily full run carries it instead of charging every + # backend merge for a macOS runner; `full` also means the stress counts below + # are now the only finalization pass, replacing the single-count merge variant. + macos-runtime: + needs: changes + if: ${{ needs.changes.outputs.full == 'true' }} + runs-on: macos-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v5 + with: + go-version: "1.26.x" + cache-dependency-path: server/go.sum + - name: Verify Cursor background lifecycle and watchdog races + shell: bash + working-directory: server + # Explicit, verbose selection prevents the Unix regression from silently + # disappearing behind a build tag. + run: | + go test -race ./pkg/agent -v -run 'TestCursorBackground|TestCaptureCursorBackground' -count=1 + go test -race ./internal/daemon -v -run 'Test.*Background.*Watchdog|Test.*IdleWatchdog' -count=1 + + - name: Stress macOS finalization + shell: bash + working-directory: server + run: | + go test ./pkg/agent -run '^TestCursorBackgroundLifecycle/finish$' -count=5 + go test -race ./pkg/agent -run '^TestCursorBackgroundLifecycle/finish$' -count=5 + + - name: Verify cgo-free macOS ownership + shell: bash + working-directory: server + run: CGO_ENABLED=0 go test ./pkg/agent -run '^TestCaptureCursorBackground(KernelRejectsStaleIdentity|DetachedSession|LateDescendant)$' -count=1 + + image-budget: + # Soft gate against the 21.7MB of raw PNG/JPG that MUL-6352 cleared out: + # a bitmap added or grown past 300KB fails until the PR description says + # why. Pull requests only — a push to main has no description to read, + # and the branch it came from was already checked. + needs: changes + if: ${{ github.event_name == 'pull_request' && needs.changes.outputs.images == 'true' }} + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 1 + + - name: Fetch comparison base + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: git fetch --no-tags --depth=1 origin "$BASE_SHA" + + - name: Check image budget + env: + PR_BODY: ${{ github.event.pull_request.body }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: node scripts/check-image-budget.mjs --base "$BASE_SHA" + + installer: + # Stub-driven shell tests for scripts/install.sh and scripts/install.ps1. + # Kept off the heavy backend job so installer regressions surface + # independently, and exercised on macOS too because the installer targets + # macOS/Homebrew and `tar` / `sed` / `mktemp` differ between BSD and GNU + # userlands. Windows runs the PowerShell installer's own suite: the two + # installers share one port contract, and neither is covered by the + # frontend job. + needs: changes + if: ${{ needs.changes.outputs.installer == 'true' }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + runs-on: ${{ matrix.os }} + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Test shell installers + if: runner.os != 'Windows' + run: bash scripts/install.test.sh + + - name: Test PowerShell installer + if: runner.os == 'Windows' + shell: pwsh + run: ./scripts/install.ps1.test.ps1 + + script-checks: + needs: changes + if: ${{ needs.changes.outputs.scripts == 'true' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v6 + with: + node-version: 22 + + - name: Test self-host env derivation + run: bash scripts/selfhost-config.test.sh + + - name: Test worktree database cleanup + run: bash scripts/worktree-db.test.sh + + - name: Test development environment registry + run: bash scripts/dev-env.test.sh + + - name: Test UI performance comparison runner + run: bash scripts/perf-compare.test.sh + + - name: Verify reserved-slugs.ts is up to date + # Re-runs the generator and fails on any drift from the + # checked-in TypeScript output. The Go side embeds the JSON + # source directly, so a passing diff here proves both sides + # share one source of truth. + run: | + node scripts/generate-reserved-slugs.mjs + git diff --exit-code -- packages/core/paths/reserved-slugs.ts + + - name: Setup Helm + uses: azure/setup-helm@v4 + + - name: Test Helm chart + run: bash scripts/helm-config.test.sh + + - name: Test backend entrypoint signals + run: bash scripts/entrypoint.test.sh + + - name: Test build output naming + run: bash scripts/makefile-build.test.sh + + frontend-quality: + needs: changes + if: ${{ needs.changes.outputs.quality_only == 'true' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v6 + with: + node-version: 22 + cache: pnpm + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Check frontend quality + uses: ./.github/actions/frontend-quality + -count=1 -timeout=10m + - name: Test Windows execution-environment isolation working-directory: server # Keep this job scoped to the runtime regression it exists to prove. diff --git a/server/internal/daemon/execenv/local_worktree.go b/server/internal/daemon/execenv/local_worktree.go index 54bb3502ee9..9577f2f1185 100644 --- a/server/internal/daemon/execenv/local_worktree.go +++ b/server/internal/daemon/execenv/local_worktree.go @@ -612,6 +612,7 @@ func (w *LocalWorktree) Finalize(logger *slog.Logger) (LocalWorktreeOutcome, err if dirty { committed, err := w.commitAll(logger) if err != nil { + outcome.Branch = "" // No recorded delivery; the preserved worktree is authoritative. outcome.PreservedPath = w.Path if logger != nil { logger.Error("execenv: could not commit the agent's changes; keeping the worktree so the work is recoverable", diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index a88bc265af7..5d9cb4a97db 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -521,6 +521,9 @@ func TestFinalizeKeepsWorktreeWhenCommitFails(t *testing.T) { if err == nil { t.Fatal("Finalize returned nil error after the commit failed") } + if outcome.Branch != "" { + t.Errorf("Branch = %q, want empty: commit failure did not deliver the branch", outcome.Branch) + } if outcome.PreservedPath != wt.Path { t.Errorf("PreservedPath = %q, want %q", outcome.PreservedPath, wt.Path) } From 30757080df11bbda1af89e4728d7f4455e5335b3 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:14:38 +0800 Subject: [PATCH 14/18] fix(ci): repair Windows worktree regression test step YAML Restore the existing Windows test step after the new 14-case worktree suite, with a complete anchored -run expression and timeout. --- .github/workflows/ci.yml | 337 +-------------------------------------- 1 file changed, 1 insertion(+), 336 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08150df5147..b07b7ddb10d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -510,342 +510,7 @@ jobs: working-directory: server # Native Windows Git/worktree coverage for #8541, including isolated Prepare and conflict replay. # Verbose output exposes a missing test selection or a skip. - run: go test ./internal/daemon/execenv -v -run '^(TestFinalizeFastForwardsConversationBranchToOffBranchDelivery|TestFinalizeFastForwardKeepsRefsWhenCheckpointIsLocked|TestFastForwardStateRejectsStaleConversationTip|TestFinalizeFastForwardRejectsConcurrentCheckpointUpdate|TestFinalizeFastForwardRejectsMissingPreparedCheckpoint|TestFinalizeRefusesFastForwardWhenConversationBranchIsCheckedOutElsewhere|TestFinalizeRefusesDivergedOffBranchDelivery|TestIsolatedPrepareFastForwardsOffBranchDelivery|TestFinalizeSameTipRetryRejectsConcurrentCheckpoint|TestFinalizeSameTipRetryAfterFastForwardSucceeds|TestFinalizeSameTipRetryRejectsConcurrentBranchMove|TestConflictResolvedByTheAgentIsDeliveredAndNotReplayedAgain|TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain|TestFinalizeConflictReplayRejectsConcurrentCheckpointUpdate) - working-directory: server - # Keep this job scoped to the runtime regression it exists to prove. - # The package's legacy OpenClaw HOME tests are not Windows-safe and are - # outside this PR; the normal backend job still runs the full package. - run: go test ./internal/daemon/execenv -run '^TestPrepareIsolated_WindowsKillsDescendantBeforeRetry$' -count=1 -timeout=5m - - - name: Test Windows daemon local-skill discovery - working-directory: server - # These fixtures must redirect both the Windows user profile and the - # platform-native Hermes home; otherwise discovery scans the runner's - # real skills instead of the test directories. - run: go test ./internal/daemon -v -run '^(TestListRuntimeLocalSkills_HermesFollowsTaskHome|TestLocalSkills_DiscoversACPProviderRoots)$' -count=1 -timeout=5m - - - name: Test Windows agent launcher argv/stdin handling - working-directory: server - # Agent prompts must never reach a Windows launcher through argv: the - # official cursor-agent.ps1, pi.ps1, and qwen.ps1 launch native children with - # `$args`, and PowerShell re-serialises them onto the child command - # line. Under - # Legacy native argument passing (powershell.exe 5.1, pwsh <= 7.2) a - # prompt holding embedded quotes is re-tokenised and fragments like - # `-X` become flags (#5649). Only a real PowerShell host proves this, - # so it cannot live in the ubuntu backend job. Scoped to the launcher - # tests, which are windows-tagged and therefore run nowhere else today; - # the backend job still runs the full package on Linux. - # -v so a silent skip (no PowerShell host resolved, or a -run pattern - # that stops matching) is visible in the log instead of passing as "ok". - run: go test ./pkg/agent -v -run '^(TestCursorExecutePromptSurvivesPowerShellShim|TestPiExecutePromptSurvivesPowerShellShim|TestQwenExecutePromptSurvivesPowerShellShim|TestPlatformCursorInvocation|TestPlatformCopilotInvocation|TestPlatformPiInvocation|TestPlatformQwenInvocation)' -count=1 -timeout=5m - - - name: Test Windows OpenCode oversized prompt reaches stdin - working-directory: server - # #6538: the daemon inlined the whole task prompt as an argv element, - # so every OpenCode task whose prompt cleared CreateProcess's 32,767 - # character lpCommandLine limit failed to start at all, with Go - # reporting ERROR_FILENAME_EXCED_RANGE as the misleading "The filename - # or extension is too long". Only a real CreateProcess enforces that - # ceiling, so this cannot run in the ubuntu backend job. The test - # spawns a native .exe directly (a Chocolatey-installed opencode.exe is - # a real PE binary, not a .cmd shim) with an oversized prompt and pins - # that the process starts, argv stays free of the prompt, and the full - # payload arrives on stdin. - # -v so a silent skip or a -run pattern that stops matching is visible - # in the log instead of passing as "ok". - run: go test ./pkg/agent -v -run '^TestOpencodeExecuteOversizedPromptStartsOnWindows$' -count=1 -timeout=5m - - - name: Test Windows agent process-tree ownership - working-directory: server - # A Job Object is the only way to prove whole-tree termination on - # Windows, and only a real Windows runner can exercise it: that a - # grandchild dies with the tree that owns it, that an unowned process - # still reports cleanup as unconfirmed, and that a descendant holding - # inherited stdout neither keeps Result blocked nor survives cleanup. - # -v makes RUN/PASS evidence explicit in CI logs. - run: go test ./pkg/agent -v -run '^(TestStartOwnedProcessTreeCapturesImmediateDescendants|TestStartOwnedProcessTreeLeavesNoSuspendedChild|TestWaitProcessGroupGoneWithoutOwnershipReportsUnconfirmed|TestCodexInitializeRetrySupportedWithOwnedProcessTree|TestCodexWindowsDescendantsDieWithTheOwnedProcessTree|TestCodeArtsWindowsCancellationTerminatesDescendants)$' -count=1 -timeout=5m - - - name: Test bounded CLI output collection on Windows - working-directory: server - # MUL-5467: RunCollect / RunCollectQuiet own the pipes as well as the - # process tree on the OpenClaw CLI paths, which outputOwned cannot. A real - # Windows host proves its Job Object owns and - # terminates descendants, a CLI that prints its answer and then refuses - # to exit still yields that answer, a response still streaming at the - # deadline is NOT reported as success, and no collector goroutine - # outlives the call. Unix uses process groups, so it cannot exercise this - # platform lifecycle. - # -v so a skip (a -run pattern that stops matching) is visible in the log - # instead of passing as "ok". - run: go test ./pkg/agent -v -run '^TestWindowsRunCollect' -count=1 -timeout=5m - - - name: Test Windows Pi session lock leaves the transcript readable - working-directory: server - # #7840: the session lock claimed the transcript itself, and Windows - # byte-range locks are mandatory. Pi's own read of the file we hand it - # through --session failed with EBUSY, so every Pi/omp task on Windows - # died ~0.5s after launch. Unix flock is advisory and never reproduces - # it, which is exactly how the regression shipped; the tests are - # windows-tagged and therefore run nowhere else today. - # -v so a skip (a -run pattern that stops matching) is visible in the - # log instead of passing as "ok". - run: go test ./pkg/agent -v -run '^TestPiSessionFileLock' -count=1 -timeout=5m - - - name: Test Windows OpenClaw npm shim interpreter resolution - working-directory: server - # #6061: every OpenClaw task failed execenv prep on a Windows host with - # a bare `exit status 1` and no stderr. A batch shim resolves and runs - # fine while the `node` it re-execs is unreachable, and npm's real - # template prefers a co-located node.exe over PATH — none of which can - # be proven without a real cmd.exe host. These tests pin: the positive - # control (node on PATH → success), that a missing node surfaces - # cmd.exe's own stderr (the first run of this job disproved #6061's - # premise that it does not), that a genuinely silent shim DOES reach the - # new diagnostic, that a co-located interpreter is credited, that a - # context timeout is not misdiagnosed as a missing interpreter, and that - # TEMP/TMP are NOT load-bearing (the originally reported root cause, - # since retracted upstream). - # Scoped to the windows-tagged shim tests — the package's legacy - # OpenClaw HOME tests are not Windows-safe; the backend job still runs - # the full package plus the cross-platform half on Linux. - # -v so a skip (no node on the runner) is visible instead of passing - # silently as "ok". - run: go test ./internal/daemon/execenv -v -run '^TestWindowsOpenclawShim' -count=1 -timeout=5m - - - name: Test Windows isolated repo checkout is committable - working-directory: server - # #6449: on Windows the daemon now hands Codex tasks a checkout whose - # .git lives inside the task workdir, because a linked worktree's - # external gitdir stays read-only under the native sandbox and breaks - # `git add` / `git commit` at the end of a task. Two of the guarantees - # are claims about Windows itself and cannot be made on ubuntu: that - # Git puts the gitdir inside the task directory, and that the clone's - # objects are private copies rather than NTFS hard links sharing one - # file and one security descriptor with the daemon-owned cache. The - # cross-volume test covers what a hard link cannot express at all. - # Scoped to the two windows-tagged tests by exact name. A prefix match - # also caught the package's cross-platform isolated-checkout test, - # which cannot run here: repocache derives a cache directory name from - # the full source repo path, so a t.TempDir() path that already embeds - # a long test name doubles and blows past MAX_PATH. That test belongs - # to the ubuntu backend job, which runs the whole package. - # -v so a skip (single-volume runner) is visible instead of passing - # silently as "ok". - run: go test ./internal/daemon/repocache -v -run '^(TestIsolatedCheckoutIsCommittableOnWindows|TestIsolatedCheckoutAcrossVolumesOnWindows)$' -count=1 -timeout=5m - - - name: Test Windows directory-junction link safety - working-directory: server - # MUL-6000: the per-task codex-home links the user's real skills into - # the task directory instead of copying them. On Windows that link is a - # directory junction whenever os.Symlink is denied (no Developer Mode), - # and a junction is the one link shape a ModeSymlink check misses: - # since Go 1.23 os.Lstat reports it as ModeDir|ModeIrregular with no - # ModeSymlink bit, while its DirEntry still answers IsDir() == true, so - # filepath.WalkDir descends into the target. Two claims about Windows - # itself cannot be made on ubuntu: that the per-task skills wipe - # (os.RemoveAll) drops the junction rather than the user's files, and - # that the GC's artifact sweep and size accounting refuse to walk - # through one. The tests call mklink /J directly so the junction shape - # is exercised even on a runner where symlinks are permitted. - # -v so a skip is visible instead of passing silently as "ok". - run: | - go test ./internal/daemon/execenv -v -run '^(TestSeedUserCodexSkills|TestHydrateCodexSkills)' -count=1 -timeout=5m - go test ./internal/daemon -v -run '^(TestCleanTaskArtifacts_DoesNotFollowDirectoryJunction|TestTaskSize_DoesNotCountDirectoryJunction)$' -count=1 -timeout=5m - - - name: Test Windows task temp dir sweep survives a sharing violation - working-directory: server - # The GC's task temp sweep has to cope with a file it cannot delete: a - # leftover child process holding one open is what left 174 directories - # behind in #7364. Only a real Windows filesystem produces that sharing - # violation — on unix an open file unlinks fine — so this is the only - # place we can prove the two things that matter: the failed cleanup - # keeps .task_lock (without it the directory reads as a pre-lock - # leftover and stops being reclaimable on liveness), and the next cycle - # removes the directory once the handle closes. - # -v so a skip is visible instead of passing silently as "ok". - run: go test ./internal/daemon/execenv -v -run '^TestPruneTaskTempDirsSurvivesRealSharingViolation$' -count=1 -timeout=5m - - - name: Test Windows agent executable junction resolution - working-directory: server - # The standalone Codex installer exposes bin as a directory junction. - # filepath.EvalSymlinks cannot traverse that reparse-point shape, so - # only a real Windows filesystem proves both PATH discovery and an - # explicitly configured path reach the release executable. The same job - # covers the npm shape, where the entry point is a `.cmd` shim that only - # the command interpreter can run. - run: go test ./internal/daemon -v -run '^(TestCanonicalExecutablePath|TestTrimExtendedLengthPrefix|TestResolveAgentExecutablePathKeeps|TestResolveAgentExecutablePath_ProfileOverride|TestResolveAgentEntry(FollowsRetargetedInstallerJunction|CanonicalizesRediscoveredJunction|ForLaunchKeepsCmdShimLaunchable|ForLaunchRejectsUnverifiedInitialJunctionTarget|ForLaunchRejectsRediscoveredJunctionWhenFinalPathResolutionFails|DoesNotSharePreRetargetSingleflightResult|ForLaunchFailsWhenJunctionKeepsRetargeting)|TestHandleTaskReportsWindowsCodexProcessStartFailure)' -count=1 -timeout=5m - - - name: Build Windows CLI helper entrypoint - working-directory: server - run: go build ./cmd/multica - - - name: Test Windows Cursor background ownership - working-directory: server - # Cursor's background shell is captured through a second Job Object, and - # TestCaptureCursorBackgroundProcessRejectsForeignJob is windows-tagged, - # so it builds nowhere else. This used to be its own windows-latest job - # on the same runtime scope as this one; same runner, same package, same - # concern, so it is a step here instead of a second runner per merge. - # Last on purpose: absorbing another job's steps means this job's own - # failure modes now truncate that job's coverage too, and Cursor - # ownership is the newest arrival, not the load-bearing one. The ubuntu - # backend job still covers the untagged and Unix arms. - run: go test -race ./pkg/agent -v -run 'TestCursorBackground|TestCaptureCursorBackground' -count=1 -timeout=5m - - # The only macOS runner here, and the only place cursor_background_process_darwin.go - # is ever built: it signals a process group through XNU's per-process unique - # identity so a recycled PID cannot redirect a kill, and the two tests pinning - # that are `//go:build darwin`. Windows keeps its arm as a step in - # windows-execenv, but macOS has no sibling job to join, and this covers one - # integration's background shell rather than the shared launch path every - # agent uses. So the daily full run carries it instead of charging every - # backend merge for a macOS runner; `full` also means the stress counts below - # are now the only finalization pass, replacing the single-count merge variant. - macos-runtime: - needs: changes - if: ${{ needs.changes.outputs.full == 'true' }} - runs-on: macos-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v5 - with: - go-version: "1.26.x" - cache-dependency-path: server/go.sum - - name: Verify Cursor background lifecycle and watchdog races - shell: bash - working-directory: server - # Explicit, verbose selection prevents the Unix regression from silently - # disappearing behind a build tag. - run: | - go test -race ./pkg/agent -v -run 'TestCursorBackground|TestCaptureCursorBackground' -count=1 - go test -race ./internal/daemon -v -run 'Test.*Background.*Watchdog|Test.*IdleWatchdog' -count=1 - - - name: Stress macOS finalization - shell: bash - working-directory: server - run: | - go test ./pkg/agent -run '^TestCursorBackgroundLifecycle/finish$' -count=5 - go test -race ./pkg/agent -run '^TestCursorBackgroundLifecycle/finish$' -count=5 - - - name: Verify cgo-free macOS ownership - shell: bash - working-directory: server - run: CGO_ENABLED=0 go test ./pkg/agent -run '^TestCaptureCursorBackground(KernelRejectsStaleIdentity|DetachedSession|LateDescendant)$' -count=1 - - image-budget: - # Soft gate against the 21.7MB of raw PNG/JPG that MUL-6352 cleared out: - # a bitmap added or grown past 300KB fails until the PR description says - # why. Pull requests only — a push to main has no description to read, - # and the branch it came from was already checked. - needs: changes - if: ${{ github.event_name == 'pull_request' && needs.changes.outputs.images == 'true' }} - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v6 - with: - fetch-depth: 1 - - - name: Fetch comparison base - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: git fetch --no-tags --depth=1 origin "$BASE_SHA" - - - name: Check image budget - env: - PR_BODY: ${{ github.event.pull_request.body }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: node scripts/check-image-budget.mjs --base "$BASE_SHA" - - installer: - # Stub-driven shell tests for scripts/install.sh and scripts/install.ps1. - # Kept off the heavy backend job so installer regressions surface - # independently, and exercised on macOS too because the installer targets - # macOS/Homebrew and `tar` / `sed` / `mktemp` differ between BSD and GNU - # userlands. Windows runs the PowerShell installer's own suite: the two - # installers share one port contract, and neither is covered by the - # frontend job. - needs: changes - if: ${{ needs.changes.outputs.installer == 'true' }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, macos-latest, windows-latest] - runs-on: ${{ matrix.os }} - steps: - - name: Checkout - uses: actions/checkout@v6 - - - name: Test shell installers - if: runner.os != 'Windows' - run: bash scripts/install.test.sh - - - name: Test PowerShell installer - if: runner.os == 'Windows' - shell: pwsh - run: ./scripts/install.ps1.test.ps1 - - script-checks: - needs: changes - if: ${{ needs.changes.outputs.scripts == 'true' }} - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-node@v6 - with: - node-version: 22 - - - name: Test self-host env derivation - run: bash scripts/selfhost-config.test.sh - - - name: Test worktree database cleanup - run: bash scripts/worktree-db.test.sh - - - name: Test development environment registry - run: bash scripts/dev-env.test.sh - - - name: Test UI performance comparison runner - run: bash scripts/perf-compare.test.sh - - - name: Verify reserved-slugs.ts is up to date - # Re-runs the generator and fails on any drift from the - # checked-in TypeScript output. The Go side embeds the JSON - # source directly, so a passing diff here proves both sides - # share one source of truth. - run: | - node scripts/generate-reserved-slugs.mjs - git diff --exit-code -- packages/core/paths/reserved-slugs.ts - - - name: Setup Helm - uses: azure/setup-helm@v4 - - - name: Test Helm chart - run: bash scripts/helm-config.test.sh - - - name: Test backend entrypoint signals - run: bash scripts/entrypoint.test.sh - - - name: Test build output naming - run: bash scripts/makefile-build.test.sh - - frontend-quality: - needs: changes - if: ${{ needs.changes.outputs.quality_only == 'true' }} - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: pnpm/action-setup@v4 - - uses: actions/setup-node@v6 - with: - node-version: 22 - cache: pnpm - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Check frontend quality - uses: ./.github/actions/frontend-quality - -count=1 -timeout=10m + run: go test ./internal/daemon/execenv -v -run '^(TestFinalizeFastForwardsConversationBranchToOffBranchDelivery|TestFinalizeFastForwardKeepsRefsWhenCheckpointIsLocked|TestFastForwardStateRejectsStaleConversationTip|TestFinalizeFastForwardRejectsConcurrentCheckpointUpdate|TestFinalizeFastForwardRejectsMissingPreparedCheckpoint|TestFinalizeRefusesFastForwardWhenConversationBranchIsCheckedOutElsewhere|TestFinalizeRefusesDivergedOffBranchDelivery|TestIsolatedPrepareFastForwardsOffBranchDelivery|TestFinalizeSameTipRetryRejectsConcurrentCheckpoint|TestFinalizeSameTipRetryAfterFastForwardSucceeds|TestFinalizeSameTipRetryRejectsConcurrentBranchMove|TestConflictResolvedByTheAgentIsDeliveredAndNotReplayedAgain|TestConflictAfterAUserCommitOnTheBranchStillOffersTheEditAgain|TestFinalizeConflictReplayRejectsConcurrentCheckpointUpdate)$' -count=1 -timeout=10m - name: Test Windows execution-environment isolation working-directory: server From da5569e54d486a9a2d7604b0e6a0df9ca43845b4 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:17:14 +0800 Subject: [PATCH 15/18] test(execenv): stabilize worktree fixture line endings on Windows --- server/internal/daemon/execenv/local_worktree_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 5d9cb4a97db..9c771a7ca79 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -68,6 +68,9 @@ func buildTestRepoTemplate() (string, error) { {"init", "-b", "main"}, {"config", "user.name", "Test User"}, {"config", "user.email", "test@test.com"}, + // The fixtures assert exact bytes across worktree checkouts. Do not let + // a Windows runner's global core.autocrlf convert committed LF to CRLF. + {"config", "core.autocrlf", "false"}, {"add", "."}, {"commit", "-m", "initial"}, } { From 7c3d6839537cf1119a1279477a66b085663a955b Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:18:06 +0800 Subject: [PATCH 16/18] test(execenv): scope Windows LF fixture config to affected tests --- server/internal/daemon/execenv/local_worktree_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/internal/daemon/execenv/local_worktree_test.go b/server/internal/daemon/execenv/local_worktree_test.go index 9c771a7ca79..56b791894a7 100644 --- a/server/internal/daemon/execenv/local_worktree_test.go +++ b/server/internal/daemon/execenv/local_worktree_test.go @@ -68,9 +68,6 @@ func buildTestRepoTemplate() (string, error) { {"init", "-b", "main"}, {"config", "user.name", "Test User"}, {"config", "user.email", "test@test.com"}, - // The fixtures assert exact bytes across worktree checkouts. Do not let - // a Windows runner's global core.autocrlf convert committed LF to CRLF. - {"config", "core.autocrlf", "false"}, {"add", "."}, {"commit", "-m", "initial"}, } { @@ -1007,6 +1004,8 @@ func TestPrepareLocalWorktreeHandsConflictingUserEditsToTheAgent(t *testing.T) { func TestConflictResolvedByTheAgentIsDeliveredAndNotReplayedAgain(t *testing.T) { t.Parallel() repo := newTestRepo(t) + // This test asserts exact LF bytes after repeated Git worktree checkouts. + gitRun(t, repo, "config", "core.autocrlf", "false") writeFile(t, filepath.Join(repo, "tracked.txt"), "A\n") first := prepareTurn(t, repo, "MUL-6881", turnOneTask) @@ -1587,6 +1586,8 @@ func TestFinalizeRefusesToRecordADeliveryThatResetPastItsBaseline(t *testing.T) func TestFinalizeFastForwardsConversationBranchToOffBranchDelivery(t *testing.T) { t.Parallel() repo := newTestRepo(t) + // Git for Windows may default to CRLF checkout; keep the LF fixture exact. + gitRun(t, repo, "config", "core.autocrlf", "false") wt := prepareTurn(t, repo, "MUL-8541", turnOneTask) conversationTip := gitRun(t, repo, "rev-parse", wt.Branch) From 99fc396ad299bab88d9928ecb5887081f189bf04 Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:19:04 +0800 Subject: [PATCH 17/18] chore(ci): match Go version on main --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b07b7ddb10d..a9c6918cce0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -502,7 +502,7 @@ jobs: with: # Deliberately independent from the minimum patch in server/go.mod: # CI follows the newest 1.26 patch available to setup-go. - go-version: "1.26.x" + go-version: "~1.26.9" check-latest: true cache-dependency-path: server/go.sum From bd55fabf31e418f9e2866dc7ad6532cbf52913ed Mon Sep 17 00:00:00 2001 From: NanPan <111261006+poijygfdyy@users.noreply.github.com> Date: Sun, 11 Oct 2026 04:19:50 +0800 Subject: [PATCH 18/18] chore(ci): align Windows setup-go comment with main --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9c6918cce0..e646a7d7f2b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -500,8 +500,8 @@ jobs: - name: Setup Go uses: actions/setup-go@v5 with: - # Deliberately independent from the minimum patch in server/go.mod: - # CI follows the newest 1.26 patch available to setup-go. + # Keep the security floor even when the setup-go manifest lags; + # continue accepting newer patches within Go 1.26. go-version: "~1.26.9" check-latest: true cache-dependency-path: server/go.sum