mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 01:16:45 +08:00
* 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]>
244 lines
7.9 KiB
Go
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
|
|
}
|