Files
Bohan Jiangandmultica-agent 3622db21cf MUL-6491 fix(daemon): give each task an exclusively claimed env root (#7347)
* fix(daemon): derive task env roots and branches from the full task id

Task ids are UUIDv7, whose leading 8 hex chars are the high 32 bits of a
48-bit millisecond timestamp: they only advance once every 2^16 ms
(~65.5s). Deriving the env-root segment from that prefix meant every task
created inside one such window landed on the same directory.

Prepare removes an existing env root before recreating it, guarded only by
the comment "task IDs are unique". That premise held for the id but not for
the truncation, so the second task's Prepare ran os.RemoveAll over a live
sibling's env root — taking its workdir (including a worktree with
uncommitted work), output/, logs/ and the 0700 multica-config/ with it.
Overwritten CLAUDE.md / AGENTS.md identity files were the visible symptom;
the deletion was the actual damage.

Use the whole id for the env root and for the agent/<name>/<task> branch
names, which truncated the same way and collided for concurrent tasks of
one agent. shortID stays for logs and display only. Prepare now derives its
root through PredictRootDir so the two can no longer drift apart.

Existing short-prefix directories need no migration: the GC enumerates task
dirs with ReadDir and reads .gc_meta.json, so it still reclaims them.

Fixes #7326

Co-authored-by: multica-agent <[email protected]>

* fix(daemon): take the task path segment from the id's random tail

The first attempt used the whole 32-char task id, which overflowed Windows
MAX_PATH: a branch name becomes a path under .git/refs/heads/ inside an
already-deep task checkout, and CI failed with

    cannot lock ref 'refs/heads/agent/windows-codex/<32 chars>':
    Filename too long

Length was never the fix. The bug was which end the segment came from: a
UUIDv7 leads with a 48-bit millisecond timestamp, so its first 8 hex chars
are the high 32 bits and only advance once every ~65.5s. Read from the tail
instead, where the id is random, and 12 chars are enough — 4 more than the
original segment rather than 24.

That trades a certainty for a probability, so make the destructive step
prove itself instead of assuming: Prepare stamps a .task_owner marker into
the env root and refuses to run os.RemoveAll over a root another task owns.
A rerun of the same task still resets its own directory. A task that fails
to start is recoverable; one that deletes a running sibling's uncommitted
work is not.

Two existing fixtures used synthetic ids differing only in the head and so
shared a tail. The guard caught them; they now differ where a real UUIDv7
does.

Fixes #7326

Co-authored-by: multica-agent <[email protected]>

* fix(daemon): make the env-root claim atomic instead of check-then-delete

Review caught that the ownership guard did not cover the case it was added
for. It read .task_owner, then deleted, then wrote the marker — three steps,
so two tasks sharing a taskKey could both observe "no owner", and the slower
one would still os.RemoveAll the faster one's live directory. The guard only
held when a foreign marker was already durably in place, i.e. serially.

Replace it with a claim protocol built on two atomic filesystem primitives:
os.Mkdir fails if the directory exists, and O_EXCL fails if the marker
exists. Exactly one racing caller wins each, and a caller that wins neither
never reaches the destructive path.

The claim is then held for the rest of Prepare. A rerun clears the env
root's CONTENTS and leaves the marker in place rather than removing and
recreating the directory, so there is no window in which the root looks
unowned to a racing task.

A directory that holds files but names no owner now fails closed instead of
being deleted as abandoned. The marker is written before any content, so the
only unowned directory that is safe to adopt is an empty one — the residue
of a crash between Mkdir and the marker write, which would otherwise wedge
the task permanently.

Tests: a barrier-synchronised concurrent Prepare asserting exactly one task
starts and the winner's tree survives (fails against the previous sequence
with "directory not empty" / "invalid argument" — the deletion racing the
create), plus fail-closed and empty-adoption cases. repocache carried a
third copy of the task-segment rule with no test; it now has the same
contract test as the handler.

Fixes #7326

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: multica-agent <[email protected]>
2026-08-21 00:36:59 +08:00

244 lines
7.9 KiB
Go

package execenv
import (
"fmt"
"log/slog"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"time"
)
// detectGitRepo checks if dir is inside a git repository (regular or bare).
// Returns the git root path and true if found.
func detectGitRepo(dir string) (string, bool) {
// Try regular repo first.
cmd := exec.Command("git", "-C", dir, "rev-parse", "--show-toplevel")
if out, err := cmd.Output(); err == nil {
return strings.TrimSpace(string(out)), true
}
// Try bare repo: git-dir is "." for bare repos when -C points at the repo.
cmd = exec.Command("git", "-C", dir, "rev-parse", "--is-bare-repository")
if out, err := cmd.Output(); err == nil && strings.TrimSpace(string(out)) == "true" {
return dir, true
}
return "", false
}
// fetchOrigin runs `git fetch origin` to ensure the local repo has the latest remote refs.
func fetchOrigin(gitRoot string) error {
cmd := exec.Command("git", "-C", gitRoot, "fetch", "origin")
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("git fetch origin: %s: %w", strings.TrimSpace(string(out)), err)
}
return nil
}
// getRemoteDefaultBranch returns "origin/<branch>" for the remote's default branch.
// Falls back to "origin/main", then "HEAD".
func getRemoteDefaultBranch(gitRoot string) string {
// Try symbolic-ref of origin/HEAD (set by `git clone` or `git remote set-head`).
cmd := exec.Command("git", "-C", gitRoot, "symbolic-ref", "refs/remotes/origin/HEAD")
if out, err := cmd.Output(); err == nil {
ref := strings.TrimSpace(string(out))
// ref looks like "refs/remotes/origin/main" — return "origin/main".
if strings.HasPrefix(ref, "refs/remotes/") {
return strings.TrimPrefix(ref, "refs/remotes/")
}
return ref
}
// Fallback: check if origin/main exists.
cmd = exec.Command("git", "-C", gitRoot, "rev-parse", "--verify", "origin/main")
if err := cmd.Run(); err == nil {
return "origin/main"
}
// Fallback: check if origin/master exists.
cmd = exec.Command("git", "-C", gitRoot, "rev-parse", "--verify", "origin/master")
if err := cmd.Run(); err == nil {
return "origin/master"
}
return "HEAD"
}
// setupGitWorktree creates a git worktree at worktreePath with a new branch.
func setupGitWorktree(gitRoot, worktreePath, branchName, baseRef string) error {
// Remove the workdir created by caller — git worktree add needs to create it.
if err := os.Remove(worktreePath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("remove placeholder workdir: %w", err)
}
err := runGitWorktreeAdd(gitRoot, worktreePath, branchName, baseRef)
if err != nil && strings.Contains(err.Error(), "already exists") {
// Branch name collision: append timestamp and retry once.
branchName = fmt.Sprintf("%s-%d", branchName, time.Now().Unix())
err = runGitWorktreeAdd(gitRoot, worktreePath, branchName, baseRef)
}
return err
}
func runGitWorktreeAdd(gitRoot, worktreePath, branchName, baseRef string) error {
cmd := exec.Command("git", "-C", gitRoot, "worktree", "add", "-b", branchName, worktreePath, baseRef)
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("git worktree add: %s: %w", strings.TrimSpace(string(out)), err)
}
return nil
}
// removeGitWorktree removes a worktree and its branch. Best-effort: logs errors.
func removeGitWorktree(gitRoot, worktreePath, branchName string, logger *slog.Logger) {
// Remove the worktree.
cmd := exec.Command("git", "-C", gitRoot, "worktree", "remove", "--force", worktreePath)
if out, err := cmd.CombinedOutput(); err != nil {
logger.Warn("execenv: git worktree remove failed", "output", strings.TrimSpace(string(out)), "error", err)
}
// Delete the branch (best-effort).
if branchName != "" {
cmd = exec.Command("git", "-C", gitRoot, "branch", "-D", branchName)
if out, err := cmd.CombinedOutput(); err != nil {
logger.Warn("execenv: git branch delete failed", "branch", branchName, "output", strings.TrimSpace(string(out)), "error", err)
}
}
}
// excludeFromGit adds a pattern to the worktree's .git/info/exclude file.
func excludeFromGit(worktreePath, pattern string) error {
// Resolve the actual git dir for this worktree.
cmd := exec.Command("git", "-C", worktreePath, "rev-parse", "--git-dir")
out, err := cmd.Output()
if err != nil {
return fmt.Errorf("resolve git dir: %w", err)
}
gitDir := strings.TrimSpace(string(out))
if !filepath.IsAbs(gitDir) {
gitDir = filepath.Join(worktreePath, gitDir)
}
excludePath := filepath.Join(gitDir, "info", "exclude")
// Ensure the info directory exists.
if err := os.MkdirAll(filepath.Dir(excludePath), 0o755); err != nil {
return fmt.Errorf("create info dir: %w", err)
}
// Check if pattern is already present.
existing, _ := os.ReadFile(excludePath)
if strings.Contains(string(existing), pattern) {
return nil
}
f, err := os.OpenFile(excludePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return fmt.Errorf("open exclude file: %w", err)
}
defer f.Close()
if _, err := fmt.Fprintf(f, "\n%s\n", pattern); err != nil {
return fmt.Errorf("write exclude pattern: %w", err)
}
return nil
}
// repoNameFromURL extracts a short directory name from a git remote URL.
// e.g. "https://github.com/org/my-repo.git" → "my-repo"
func repoNameFromURL(url string) string {
// Strip trailing slashes and .git suffix.
url = strings.TrimRight(url, "/")
url = strings.TrimSuffix(url, ".git")
// Take the last path segment.
if i := strings.LastIndex(url, "/"); i >= 0 {
url = url[i+1:]
}
// Also handle SSH-style "host:org/repo".
if i := strings.LastIndex(url, ":"); i >= 0 {
url = url[i+1:]
if j := strings.LastIndex(url, "/"); j >= 0 {
url = url[j+1:]
}
}
name := strings.TrimSpace(url)
if name == "" {
return "repo"
}
return name
}
// taskKeyLen is how many hex chars of the task id identify a task in a path or
// a branch name. Every char here is spent twice — the env root prefixes the
// agent's whole workdir, and the branch name becomes a path under
// .git/refs/heads/ inside that workdir — and Windows still enforces MAX_PATH
// (260). The full 32-char id overflows it on a deep checkout, so the segment
// stays short and buys its uniqueness from entropy instead of length.
const taskKeyLen = 12
// taskKey returns the segment identifying a task in a path or branch name: the
// LAST taskKeyLen hex chars of the id.
//
// Which end matters more than how many chars. Task ids are UUIDv7 — 48 bits of
// millisecond timestamp, then randomness. The leading 8 hex chars are the high
// 32 bits of that timestamp, so they only advance once every 2^16 ms (~65.5s):
// taking them from the front gave every task started inside one such window an
// identical segment, and therefore one shared env root. That is not a rare hash
// collision, it is the common case, and it made Prepare's "remove existing env"
// step delete a concurrently running task's directory (#7326).
//
// The tail is drawn from the id's random field, so 12 chars carry 48 random
// bits. Prepare additionally refuses to delete an env root another task owns,
// so even an improbable clash fails closed instead of destroying work.
//
// Use shortID for logs, never for identity.
func taskKey(uuid string) string {
s := strings.ReplaceAll(uuid, "-", "")
if len(s) > taskKeyLen {
return s[len(s)-taskKeyLen:]
}
return s
}
// shortID returns the first 8 characters of a UUID string (dashes stripped).
// Display and logging only — see taskKey for anything that must be unique.
func shortID(uuid string) string {
s := strings.ReplaceAll(uuid, "-", "")
if len(s) > 8 {
return s[:8]
}
return s
}
var nonAlphanumeric = regexp.MustCompile(`[^a-z0-9]+`)
// sanitizeName produces a git-branch-safe name from a human-readable string.
func sanitizeName(name string) string {
s := strings.ToLower(strings.TrimSpace(name))
s = nonAlphanumeric.ReplaceAllString(s, "-")
s = strings.Trim(s, "-")
if len(s) > 30 {
s = s[:30]
s = strings.TrimRight(s, "-")
}
if s == "" {
s = "agent"
}
return s
}