diff --git a/AGENTS.md b/AGENTS.md index 399e946..3838db2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ CLI contract read `README.md` and `llms.txt`; for usage read `crq help`. ## Package layout Dependency rule (Go-enforced, no cycles): `dialect ← engine ← crq`, `state ← crq`, -`gh ← {state, crq}`. The engine does no I/O by construction. +`gh ← {state, crq}`, `workspace ← crq`. The engine does no I/O by construction. - `internal/dialect/` — ALL bot-text knowledge, zero deps. CodeRabbit/Codex completion, rate-limit, paused, in-progress, failed, summary-only-plan, @@ -26,6 +26,10 @@ Dependency rule (Go-enforced, no cycles): `dialect ← engine ← crq`, `state The only package (besides dialect) allowed to say "rate limit". `ListCheckRuns` fetches a ref's check runs (envelope-paged, ETag'd); matching them to a bot is dialect's `ClassifyCheckRun`, never gh's. +- `internal/workspace/` — reusable repository mirrors, detached PR worktrees, + credential-safe Git execution, stale-worktree pruning, and mirror migration. + Owns persistent filesystem and process I/O for checkouts; `crq` supplies only + configured roots and a current-token resolver. - `internal/state/` — persisted schema v3: one `Round` per PR, one global `FireSlot`, the CodeRabbit `AccountQuota`, an `Archive` ring. Round transition methods, the CAS store, and dashboard rendering. `Round.CoBots` holds per- diff --git a/internal/crq/config.go b/internal/crq/config.go index 98d6cdf..719d5a7 100644 --- a/internal/crq/config.go +++ b/internal/crq/config.go @@ -47,7 +47,16 @@ type Config struct { // Reviewers is the single description of who reviews and what they cost. // Bot / RequiredBots / FeedbackBots / CoBots above are DERIVED from it and // kept only so existing consumers keep compiling; new code should read this. - Reviewers []Reviewer + Reviewers []Reviewer + // WorkspaceRoot holds crq's own mirrors and worktrees. Read here rather than + // from the process environment, so a value in ~/.config/crq/env — the + // documented place for crq settings — is actually used. + WorkspaceRoot string + // WorkDir is the checkout the local-work probe inspects. Empty means the + // process's own directory, which is what an agent running crq from its + // working copy means. Set programmatically by a caller working in a + // worktree it made, since that caller is not standing in one. + WorkDir string RateLimitCommand string RateLimitMarker string CalibrationMarker string @@ -172,6 +181,7 @@ func LoadConfig() (Config, error) { AutoReviewMaxScan: intEnv(env, "CRQ_AUTOREVIEW_MAX_SCAN", 400), LeaderTTL: durationEnv(env, "CRQ_LEADER_TTL", 3*time.Minute), FiredMax: intEnv(env, "CRQ_FIRED_MAX", 500), + WorkspaceRoot: env["CRQ_WORKSPACE"], NoOpen: env["CRQ_NO_OPEN"] != "", DryRun: env["CRQ_DRY_RUN"] == "1", FeedbackWaitTimeout: durationEnv(env, "CRQ_FEEDBACK_WAIT_TIMEOUT", 20*time.Minute), diff --git a/internal/crq/next.go b/internal/crq/next.go index d72155a..1ee0d81 100644 --- a/internal/crq/next.go +++ b/internal/crq/next.go @@ -2,7 +2,6 @@ package crq import ( "context" - "os/exec" "strings" "time" @@ -293,7 +292,7 @@ func (s *Service) checkLocalWork(ctx context.Context, repos []string, head, head if s.localWorkFn != nil { return s.localWorkFn(ctx, head) } - return localWork(ctx, repos, head, headRef) + return localWork(ctx, s.cfg.WorkDir, repos, head, headRef) } // localWork reports whether the working copy holds changes the PR head does not @@ -313,13 +312,15 @@ func (s *Service) checkLocalWork(ctx context.Context, repos []string, head, head // `done` rather than `push`. That is the safe direction: `push` is only ever // emitted once the head is already released, so a missed one costs one extra // call, while a spurious `hold` would stall the loop. -func localWork(ctx context.Context, repos []string, head, headRef string) (bool, string) { +// dir is the checkout to inspect; "" means the process's own directory, +// which is what an agent running crq from its working copy means. +func localWork(ctx context.Context, dir string, repos []string, head, headRef string) (bool, string) { git := func(args ...string) (string, bool) { - out, err := exec.CommandContext(ctx, "git", args...).Output() + out, err := gitDir(ctx, dir, args...) if err != nil { return "", false } - return strings.TrimSpace(string(out)), true + return out, true } if _, ok := git("rev-parse", "--is-inside-work-tree"); !ok { return false, "not run inside a git checkout" diff --git a/internal/crq/target.go b/internal/crq/target.go index 36ed7d3..d577191 100644 --- a/internal/crq/target.go +++ b/internal/crq/target.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/url" - "os/exec" "strings" ) @@ -117,10 +116,8 @@ func remoteSlugs(remotes string) (repos, owners []string) { return repos, owners } +// gitIn runs git in the process's own working directory, which is what target +// inference is about: the checkout the caller is standing in. func gitIn(ctx context.Context, args ...string) (string, error) { - out, err := exec.CommandContext(ctx, "git", args...).Output() - if err != nil { - return "", err - } - return strings.TrimSpace(string(out)), nil + return gitDir(ctx, "", args...) } diff --git a/internal/crq/workspace_wiring.go b/internal/crq/workspace_wiring.go new file mode 100644 index 0000000..595be61 --- /dev/null +++ b/internal/crq/workspace_wiring.go @@ -0,0 +1,23 @@ +package crq + +import ( + "context" + + ghapi "github.com/kristofferR/coderabbit-queue/internal/gh" + "github.com/kristofferR/coderabbit-queue/internal/workspace" +) + +// workspace wires crq's configured cache root and GitHub credential resolution +// into the reusable workspace implementation. +func (s *Service) workspace(context.Context) workspace.Workspace { + return workspace.Workspace{ + Root: s.cfg.WorkspaceRoot, + TokenSource: ghapi.LookupToken, + } +} + +// gitDir keeps command-local Git probes in crq while the process and error +// handling live with the rest of the Git implementation. +func gitDir(ctx context.Context, dir string, args ...string) (string, error) { + return workspace.Git(ctx, dir, args...) +} diff --git a/internal/crq/workspace_wiring_test.go b/internal/crq/workspace_wiring_test.go new file mode 100644 index 0000000..3e13362 --- /dev/null +++ b/internal/crq/workspace_wiring_test.go @@ -0,0 +1,21 @@ +package crq + +import ( + "context" + "testing" +) + +// The daemon's workspace has to come from the config file, not the process +// environment, and credentials must be resolved when Git runs so token rotation +// does not leave a long-lived checkout with a stale value. +func TestServiceWorkspaceUsesTheConfiguredRootAndCurrentToken(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "ghp_from_the_environment") + s := &Service{cfg: Config{WorkspaceRoot: "/tmp/crq-configured-root"}} + ws := s.workspace(context.Background()) + if ws.Root != "/tmp/crq-configured-root" { + t.Errorf("workspace root = %q, want the configured one", ws.Root) + } + if got := ws.TokenSource(context.Background()); got != "ghp_from_the_environment" { + t.Errorf("workspace token = %q, want the one the API client resolves", got) + } +} diff --git a/internal/gh/github.go b/internal/gh/github.go index 0b81662..d4e8261 100644 --- a/internal/gh/github.go +++ b/internal/gh/github.go @@ -167,6 +167,11 @@ func (g *GitHub) cacheGET(url string, resp *http.Response) (*http.Response, erro return resp, nil } +// LookupToken resolves a GitHub token from the environment or the gh CLI, for +// callers outside this package that need the same credential — git, which does +// not read GITHUB_TOKEN or gh's store by itself. +func LookupToken(ctx context.Context) string { return lookupToken(ctx) } + // lookupToken resolves a GitHub token from the environment or the gh CLI. gh can // hand back a freshly-rotated OAuth token, which is why send re-runs this on a 401. func lookupToken(ctx context.Context) string { diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go new file mode 100644 index 0000000..12317ed --- /dev/null +++ b/internal/workspace/workspace.go @@ -0,0 +1,1057 @@ +package workspace + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "time" +) + +// Workspace is crq's own place on disk for a repository. +// +// Everything crq does with git today assumes it was RUN inside the checkout it +// cares about: localWork shells out with no directory, target inference reads +// the current branch. That is fine for a command an agent types from its own +// working copy, and useless for the daemon, which has no checkout of any of the +// repositories it reviews. +// +// A Workspace separates the two: a bare mirror per repository, fetched rather +// than re-cloned, and a throwaway worktree per head. Nothing here reads or +// writes the process's working directory. +// +// Credentials stay as close to the host's own as they can be. The remote is the +// ordinary https URL, so whatever git credential helper the host already uses — +// `gh auth setup-git` writes one — supplies the token. Only a daemon that has no +// such helper makes crq inject one of its own (see credentialHelper), and even +// then the secret lives in the environment rather than in argv or a config file. +type Workspace struct { + // Root holds the mirrors and worktrees. Empty means the default cache + // location (see DefaultWorkspaceRoot). + Root string + // Token authenticates clones and fetches. Empty falls back to whatever git + // credential helper the host already has, which is the right answer on a + // developer machine; a daemon configured with GITHUB_TOKEN alone has no + // helper, and git does not read that variable by itself. + Token string + // TokenSource resolves credentials immediately before each Git command. + // Long-lived checkouts use this instead of retaining the token that happened + // to be current when they were created. + TokenSource func(context.Context) string +} + +// DefaultWorkspaceRoot is $XDG_CACHE_HOME/crq (or ~/.cache/crq). +func DefaultWorkspaceRoot() (string, error) { + if dir := strings.TrimSpace(os.Getenv("CRQ_WORKSPACE")); dir != "" { + return dir, nil + } + cache, err := os.UserCacheDir() + if err != nil { + return "", err + } + return filepath.Join(cache, "crq"), nil +} + +// git runs a git command for this workspace, injecting credentials when a token +// is configured. +// +// The token travels in the ENVIRONMENT, never in argv: the helper is a shell +// snippet that reads it, so a process listing, a log line and this package's own +// error strings all carry the snippet and never the secret. +func (w Workspace) git(ctx context.Context, dir string, args ...string) (string, error) { + token := strings.TrimSpace(w.Token) + if w.TokenSource != nil { + token = strings.TrimSpace(w.TokenSource(ctx)) + } + if token == "" { + return gitDir(ctx, dir, args...) + } + full := append([]string{ + "-c", "credential.helper=", + "-c", "credential.helper=" + credentialHelper, + }, args...) + return gitEnv(ctx, dir, []string{TokenEnv + "=" + token}, full...) +} + +// TokenEnv is the variable credentialHelper reads the token from. A caller that +// runs git in a checkout — rather than through this package — has to set it, so +// the name is part of the contract rather than an internal detail. +const TokenEnv = "CRQ_GIT_TOKEN" + +// credentialHelper answers for https://github.com and nothing else. +// +// A git command can be led to a URL that repository CONTENT chose — a submodule, +// an LFS endpoint — and a helper that answers every request would hand the +// account's token to whatever host a pull request pointed it at. git writes the +// request to stdin as key=value lines and appends the operation as an argument, +// so read the protocol and host and stay quiet unless they are ours. The lines +// are drained whatever the operation is, so a store or an erase does not meet a +// closed pipe, and the snippet always exits 0. +// +// github.com literally: the token is a GitHub token, and CRQ_REMOTE_BASE (the +// only other remote crq ever uses) points at a local path in tests, which needs +// no credentials at all. +const credentialHelper = `!f() { p=; h=; while IFS= read -r l; do case "$l" in protocol=*) p=${l#protocol=} ;; host=*) h=${l#host=} ;; esac; done; if test "$1" = get && test "$p" = https && test "$h" = github.com; then printf 'username=x-access-token\npassword=%s\n' "$CRQ_GIT_TOKEN"; fi; }; f` + +func (w Workspace) root() (string, error) { + root := strings.TrimSpace(w.Root) + if root == "" { + var err error + if root, err = DefaultWorkspaceRoot(); err != nil { + return "", err + } + } + // Absolute, always: `git worktree add` runs inside the mirror, so a relative + // path would create the worktree under the mirror while the path handed back + // points at a directory that does not exist. + return filepath.Abs(root) +} + +// mirrorPath is where repo's bare mirror lives. The owner and name are path +// segments, so a repo string that is not exactly "owner/name" is refused rather +// than allowed to escape the root. +func (w Workspace) mirrorPath(repo string) (string, error) { + root, err := w.root() + if err != nil { + return "", err + } + owner, name, ok := splitRepo(repo) + if !ok { + return "", fmt.Errorf("repo must be owner/name, got %q", repo) + } + return filepath.Join(root, "mirrors", owner, name+".git"), nil +} + +// splitRepo splits owner/name, rejecting anything that could traverse a path. +func splitRepo(repo string) (owner, name string, ok bool) { + owner, name, ok = strings.Cut(normalizeRepo(repo), "/") + if !ok || owner == "" || name == "" { + return "", "", false + } + for _, part := range []string{owner, name} { + if part == "." || part == ".." || strings.ContainsAny(part, `/\`) { + return "", "", false + } + } + return owner, name, true +} + +// Mirror makes sure repo's bare mirror exists and is up to date, and returns its +// path. It clones on first use and fetches afterwards — a mirror is reused +// across PRs and across rounds, so the expensive clone happens once per +// repository rather than once per dispatch. +func (w Workspace) Mirror(ctx context.Context, repo string) (string, error) { + path, err := w.mirrorPath(repo) + if err != nil { + return "", err + } + // 0700, not the umask's 0755: a mirror of a private repository is private + // source, and on a shared host the default would let any local user read it. + if root, rerr := w.root(); rerr == nil { + if err := os.MkdirAll(root, 0o700); err != nil { + return "", err + } + if err := os.Chmod(root, 0o700); err != nil { + return "", err + } + } + if _, err := os.Stat(filepath.Join(path, "HEAD")); err == nil { + if err := w.refreshMirror(ctx, path, repo); err != nil { + return "", err + } + return path, nil + } else if !errors.Is(err, os.ErrNotExist) { + return "", err + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return "", err + } + // Clone somewhere else and move it into place, so two workers starting on the + // same repository at once cannot clone into one directory — the second would + // fail with "destination path already exists" and take the caller with it. + staging, err := os.MkdirTemp(filepath.Dir(path), ".clone-") + if err != nil { + return "", err + } + defer os.RemoveAll(staging) + pending := filepath.Join(staging, "mirror.git") + // Init and fetch, not `clone --bare`: a bare clone copies the remote's branch + // heads straight into refs/heads, and the refspec set afterwards only governs + // later fetches. refs/heads belongs to the sessions — a branch already sitting + // there is one a session cannot create, and a stale commit it could land on. + // Fetching into an empty repository leaves that namespace to them, and never + // sets the remote.origin.mirror that `clone --mirror` did. + if _, err := w.git(ctx, "", "init", "--bare", "--quiet", pending); err != nil { + return "", fmt.Errorf("creating mirror of %s: %w", repo, err) + } + if _, err := w.git(ctx, pending, "remote", "add", "origin", w.remoteURL(repo)); err != nil { + return "", fmt.Errorf("configuring %s: %w", repo, err) + } + if _, err := w.git(ctx, pending, "config", "remote.origin.fetch", originRefspec); err != nil { + return "", fmt.Errorf("configuring %s: %w", repo, err) + } + if err := w.enableWorktreeConfig(ctx, pending); err != nil { + return "", fmt.Errorf("configuring %s: %w", repo, err) + } + // Before the fetch, so the mirror is complete the moment it is renamed into + // place: another worker may pick it up as soon as it exists, and it would + // find one with no helper for its own git commands to use. + if err := w.persistCredentialHelper(ctx, pending); err != nil { + return "", fmt.Errorf("configuring %s: %w", repo, err) + } + if _, err := w.git(ctx, pending, "fetch", "--prune", "--no-tags", w.remoteURL(repo), originRefspec, tagRefspec); err != nil { + return "", fmt.Errorf("cloning %s: %w", repo, err) + } + // A new mirror never copied remote branches into refs/heads. Record that + // before exposing it: from this point on that namespace belongs entirely to + // sessions, so no later refresh may mistake one of their branches for a + // legacy clone's copy. + if _, err := w.git(ctx, pending, "config", fetchedHeadsMigratedKey, "true"); err != nil { + return "", fmt.Errorf("configuring %s: %w", repo, err) + } + if err := os.Rename(pending, path); err != nil { + if _, statErr := os.Stat(filepath.Join(path, "HEAD")); statErr == nil { + return path, nil // somebody else won the race; their mirror is as good + } + return "", err + } + return path, nil +} + +func (w Workspace) refreshMirror(ctx context.Context, path, repo string) error { + // A true marker proves refs/heads already belongs to sessions. The fast path + // avoids serializing ordinary refreshes after the one-time migration. + if w.fetchedHeadsMigrationComplete(ctx, path) && w.worktreeConfigComplete(ctx, path) { + return w.refreshMigratedMirror(ctx, path, repo) + } + + unlock, err := lockMirrorMigration(ctx, path) + if err != nil { + return fmt.Errorf("locking migration of %s: %w", repo, err) + } + defer unlock() + + // Another process may have completed the migration while this one waited. + if w.fetchedHeadsMigrationComplete(ctx, path) && w.worktreeConfigComplete(ctx, path) { + return w.refreshMigratedMirror(ctx, path, repo) + } + + dropFetched := w.hasLegacyFetchedHeads(ctx, path) + if dropFetched { + // Persist the pending state before migrateMirror clears the legacy + // configuration that identified these heads. A failed fetch or deletion + // must be retried by the next refresh. + if err := w.setConfig(ctx, path, fetchedHeadsMigratedKey, "false"); err != nil { + return fmt.Errorf("recording pending migration of %s: %w", repo, err) + } + } + if err := w.migrateMirror(ctx, path); err != nil { + return fmt.Errorf("configuring %s: %w", repo, err) + } + if err := w.fetchMirror(ctx, path, repo); err != nil { + return fmt.Errorf("fetching %s: %w", repo, err) + } + // The lock keeps a second refresh from acting on a stale migration decision + // or returning a checkout while legacy refs are still eligible for deletion. + if dropFetched { + if err := w.dropFetchedHeads(ctx, path); err != nil { + return fmt.Errorf("migrating %s: %w", repo, err) + } + } + if err := w.setConfig(ctx, path, fetchedHeadsMigratedKey, "true"); err != nil { + return fmt.Errorf("recording migration of %s: %w", repo, err) + } + return nil +} + +func (w Workspace) refreshMigratedMirror(ctx context.Context, path, repo string) error { + if err := w.migrateMirror(ctx, path); err != nil { + return fmt.Errorf("configuring %s: %w", repo, err) + } + if err := w.fetchMirror(ctx, path, repo); err != nil { + return fmt.Errorf("fetching %s: %w", repo, err) + } + return nil +} + +func (w Workspace) fetchedHeadsMigrationComplete(ctx context.Context, path string) bool { + migrated, err := w.git(ctx, path, "config", "--bool", "--get", fetchedHeadsMigratedKey) + return err == nil && migrated == "true" +} + +func (w Workspace) worktreeConfigComplete(ctx context.Context, path string) bool { + enabled, err := w.git(ctx, path, "config", "--bool", "--get", "extensions.worktreeConfig") + if err != nil || enabled != "true" { + return false + } + if _, err := w.git(ctx, path, "config", "--local", "--get", "core.bare"); err == nil { + return false + } + bare, err := w.git(ctx, path, "config", "--worktree", "--bool", "--get", "core.bare") + return err == nil && bare == "true" +} + +func lockMirrorMigration(ctx context.Context, path string) (func(), error) { + file, err := os.OpenFile(filepath.Join(path, "crq-migration.lock"), os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, err + } + for { + err = syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) + if err == nil { + return func() { + _ = syscall.Flock(int(file.Fd()), syscall.LOCK_UN) + _ = file.Close() + }, nil + } + if !errors.Is(err, syscall.EWOULDBLOCK) && !errors.Is(err, syscall.EAGAIN) { + _ = file.Close() + return nil, err + } + if err := sleepCtx(ctx, 100*time.Millisecond); err != nil { + _ = file.Close() + return nil, err + } + } +} + +// remoteURL is the URL crq clones from. CRQ_REMOTE_BASE exists so a test can +// point at a local path; in production it is github.com over https, which is +// what the host's credential helper is configured for. +func (w Workspace) remoteURL(repo string) string { + base := strings.TrimSpace(os.Getenv("CRQ_REMOTE_BASE")) + if base == "" { + return "https://github.com/" + normalizeRepo(repo) + ".git" + } + return strings.TrimRight(base, "/") + "/" + normalizeRepo(repo) +} + +// originRefspec keeps fetched refs out of refs/heads, which belongs to the +// worktrees: a session's branch there must survive a fetch, and a branch checked +// out in a worktree makes any fetch that would update it fail outright. +const originRefspec = "+refs/heads/*:refs/remotes/origin/*" + +// Remote tags live outside refs/tags, which belongs to sessions just like +// refs/heads. This lets refreshes prune and force-update the remote's view +// without deleting or moving tags a live checkout created. +const tagRefspec = "+refs/tags/*:refs/crq/remotes/origin/tags/*" + +// fetchedHeadsMigratedKey records whether refs/heads has been handed over from +// an old clone to sessions. False means cleanup is pending; true means later +// refreshes must not apply migration heuristics to session-created branches. +const fetchedHeadsMigratedKey = "crq.fetched-heads-migrated" + +// hasLegacyFetchedHeads identifies the one migration allowed to delete local +// branches. A false marker retries interrupted cleanup and a true marker proves +// it already ran; otherwise only old fetch/push mirror configuration proves +// refs/heads contains copies made by the clone. +// An unmarked mirror already using the current configuration may predate the +// marker, but its local branches belong to sessions and must be preserved. +func (w Workspace) hasLegacyFetchedHeads(ctx context.Context, path string) bool { + if migrated, err := w.git(ctx, path, "config", "--bool", "--get", fetchedHeadsMigratedKey); err == nil { + return migrated == "false" + } + if mirror, err := w.git(ctx, path, "config", "--bool", "--get", "remote.origin.mirror"); err == nil && mirror == "true" { + return true + } + fetch, err := w.git(ctx, path, "config", "--get-all", "remote.origin.fetch") + if err != nil { + return false + } + for _, refspec := range strings.Split(fetch, "\n") { + _, dst, ok := strings.Cut(strings.TrimPrefix(refspec, "+"), ":") + if ok && (dst == "refs/*" || dst == "refs/heads/*" || strings.HasPrefix(dst, "refs/heads/")) { + return true + } + } + return false +} + +// migrateMirror brings a mirror an older crq left behind up to the rules this +// one depends on, and is a no-op on a mirror that already follows them. +func (w Workspace) migrateMirror(ctx context.Context, path string) error { + if err := w.enableWorktreeConfig(ctx, path); err != nil { + return err + } + // Enforce the refspec on EVERY call, not only at clone time. A mirror + // created before this rule still fetches +refs/*:refs/*, and one branch + // created in a worktree then wedges every future fetch for the whole + // repository with "refusing to fetch into branch ... checked out at". + if err := w.setConfig(ctx, path, "remote.origin.fetch", originRefspec); err != nil { + return err + } + // A mirror cloned with --mirror also carries remote.origin.mirror=true, which + // the refspec does not clear: a plain `git push` from a session's worktree + // would then mirror the whole local ref namespace, publishing internal refs + // and deleting remote branches this repository has never heard of. + if err := w.unsetConfig(ctx, path, "remote.origin.mirror"); err != nil { + return err + } + return w.persistCredentialHelper(ctx, path) +} + +func (w Workspace) enableWorktreeConfig(ctx context.Context, path string) error { + // Push URLs are worktree-specific. Enabling that extension requires moving + // core.bare into the main worktree's config; otherwise linked worktrees also + // inherit bare=true and Git refuses commands that need a working tree. + if w.worktreeConfigComplete(ctx, path) { + return nil + } + if err := w.setConfig(ctx, path, "extensions.worktreeConfig", "true"); err != nil { + return err + } + if _, err := w.git(ctx, path, "config", "--worktree", "--replace-all", "core.bare", "true"); err != nil { + return err + } + bare, err := w.git(ctx, path, "config", "--worktree", "--bool", "--get", "core.bare") + if err != nil { + return fmt.Errorf("verifying worktree core.bare: %w", err) + } + if bare != "true" { + return fmt.Errorf("verifying worktree core.bare: got %q, want true", bare) + } + if _, err := w.git(ctx, path, "config", "--local", "--get", "core.bare"); err == nil { + if _, err := w.git(ctx, path, "config", "--local", "--unset-all", "core.bare"); err != nil { + return err + } + } + return nil +} + +// persistCredentialHelper writes the helper SNIPPET into the mirror's config, +// for the git commands this package does not run itself. +// +// A worktree is made for somebody else to work in, and that somebody runs a +// plain `git push`. Everything here injects the helper with -c, which lasts +// exactly as long as one command, and git reads no GITHUB_TOKEN of its own — so +// on a host holding only a token, a caller could do all of its work in a +// checkout and fail at the last step of every one of them. +// +// The secret still never lands on disk: what is persisted READS TokenEnv from +// the environment, so only a caller that sets it can use it, and a mirror +// somebody else finds on disk hands out nothing. +func (w Workspace) persistCredentialHelper(ctx context.Context, path string) error { + if strings.TrimSpace(w.Token) == "" && w.TokenSource == nil { + return nil // the host's own helper answers; leave its configuration alone + } + // gitDir, not w.git: w.git injects a credential.helper of its own, so this + // read would answer with that injected value and conclude the mirror is + // already configured when its config is in fact empty. + var err error + for attempt := 0; attempt < 3; attempt++ { + if cur, gerr := gitDir(ctx, path, "config", "--local", "--get-all", "credential.helper"); gerr == nil && cur == credentialHelper { + return nil + } + if _, err = gitDir(ctx, path, "config", "--local", "--replace-all", "credential.helper", credentialHelper); err == nil { + return nil + } + if !isConfigLockContention(err) { + return err + } + if serr := sleepCtx(ctx, time.Duration(attempt+1)*200*time.Millisecond); serr != nil { + return serr + } + } + // Another worker may have installed the same helper while this one waited + // for config.lock. Its value is the successful migration we wanted. + if cur, gerr := gitDir(ctx, path, "config", "--local", "--get-all", "credential.helper"); gerr == nil && cur == credentialHelper { + return nil + } + return err +} + +// fetchMirror brings the mirror at path up to date. +func (w Workspace) fetchMirror(ctx context.Context, path, repo string) error { + // Two workers fetching one mirror race on git's ref locks, and the loser + // reports "cannot lock ref" even though the winner has just made the mirror + // current. Retry briefly rather than failing a dispatch over that. + var err error + for attempt := 0; attempt < 3; attempt++ { + // Use the mirror's repository explicitly. Older checkouts may have + // repointed the shared origin before push URLs became worktree-local. + // Refreshing must neither trust nor rewrite that legacy state. + if _, err = w.git(ctx, path, "fetch", "--prune", "--no-tags", w.remoteURL(repo), originRefspec, tagRefspec); err == nil { + return nil + } + // Only contention is somebody else's success. Expired credentials, an + // unreachable remote or a corrupt mirror are this caller's problem, and + // swallowing them hands back stale refs — which surfaces later as an + // unreadable commit rather than the fetch error that explains it. + if !isRefLockContention(err) { + return err + } + if serr := sleepCtx(ctx, time.Duration(attempt+1)*200*time.Millisecond); serr != nil { + return serr + } + } + // Three losses in a row is no longer evidence of somebody else's success: a + // ref lock left behind by a killed git never clears, and reads exactly like a + // live race for as long as it sits there. Report the fetch that did not + // happen — the alternative is stale refs presented as current ones. + return err +} + +// dropFetchedHeads removes the branch copies an older `git clone --mirror` wrote +// into refs/heads. It runs once for an unmarked mirror; after it succeeds, +// Mirror records fetchedHeadsMigratedKey and this namespace belongs to sessions. +// +// Changing the refspec does not move refs a previous clone already made: they go +// on occupying the names refs/heads reserves for the sessions — `git checkout -b +// feature` fails with "a branch named 'feature' already exists" — and they stay +// frozen at whatever commit that clone saw. The fetch has just written the +// current value of each under refs/remotes/origin, so the copy is redundant. +// +// Three guards, because a session's own branch lives in refs/heads too, and +// deleting that one loses the commits it is the only ref for: never a name a +// worktree has checked out, only names origin actually has, and only when origin +// already has the commits. The last is what makes a deletion lossless rather +// than merely likely to be: checkout status alone would drop a session's branch +// the moment it detached HEAD to look at another commit. Deletion itself uses +// git branch rather than update-ref, so Git rechecks worktree occupancy after +// our snapshot and refuses a branch a session attached in the meantime. +func (w Workspace) dropFetchedHeads(ctx context.Context, path string) error { + heads, err := gitDir(ctx, path, "for-each-ref", "--format=%(refname:lstrip=2)", "refs/heads") + if err != nil { + return err + } + if heads == "" { + return nil // the usual case: a mirror crq made itself never writes refs/heads + } + live, err := checkedOutBranches(ctx, path) + if err != nil { + return err + } + for _, name := range strings.Split(heads, "\n") { + if live[name] { + continue + } + if _, err := gitDir(ctx, path, "show-ref", "--verify", "--quiet", "refs/remotes/origin/"+name); err != nil { + continue // origin has no such branch, so this one belongs to a session + } + // A clone's copy is at or behind what the fetch just wrote under + // refs/remotes/origin, so every commit it names survives its deletion. A + // branch holding commits origin has never seen is somebody's unpushed work + // — or a leftover of a force-push — and its ref is the only thing keeping + // those commits reachable. Keeping it costs one occupied name; dropping it + // costs the commits. + if _, err := gitDir(ctx, path, "merge-base", "--is-ancestor", "refs/heads/"+name, "refs/remotes/origin/"+name); err != nil { + continue + } + if err := deleteFetchedHead(ctx, path, name); err != nil { + return err + } + } + return nil +} + +func deleteFetchedHead(ctx context.Context, path, name string) error { + // Unlike update-ref, branch refuses to delete a branch that became checked + // out after dropFetchedHeads took its occupancy snapshot. + if _, err := gitDir(ctx, path, "branch", "-D", "--", name); err != nil { + live, lerr := checkedOutBranches(ctx, path) + if lerr == nil && live[name] { + return nil + } + // A concurrent migration may have deleted the same legacy branch after + // this caller selected it. Its absence is the desired end state. + if _, rerr := gitDir(ctx, path, "show-ref", "--verify", "--quiet", "refs/heads/"+name); rerr != nil { + return nil + } + return err + } + return nil +} + +// checkedOutBranches is the set of branch names some worktree of the mirror at +// path currently has checked out. +func checkedOutBranches(ctx context.Context, path string) (map[string]bool, error) { + out, err := gitDir(ctx, path, "worktree", "list", "--porcelain") + if err != nil { + return nil, err + } + live := map[string]bool{} + for _, line := range strings.Split(out, "\n") { + if name, ok := strings.CutPrefix(strings.TrimSpace(line), "branch refs/heads/"); ok { + live[name] = true + } + } + return live, nil +} + +// setConfig writes one config key of the mirror at path, unless it already holds +// exactly value and nothing else. +// +// git serializes configuration writes through config.lock, so two dispatches of +// the same repository arriving together make one of them fail with "could not +// lock config file" — before the fetch, which is the part written to tolerate +// concurrency. Reading first takes that write out of every call but the one that +// actually migrates something, and that one retries. +func (w Workspace) setConfig(ctx context.Context, path, key, value string) error { + var err error + for attempt := 0; attempt < 3; attempt++ { + // --get-all, not --get: a key holding several values answers --get with the + // last of them, so the one shape that most needs rewriting — a mirror with + // a second remote.origin.fetch — would read as already current. + if cur, gerr := w.git(ctx, path, "config", "--get-all", key); gerr == nil && cur == value { + return nil + } + // --replace-all for the same reason: a plain single-value write refuses a + // multi-valued key outright ("cannot overwrite multiple values with a + // single value"), which would fail every later call for that repository. + if _, err = w.git(ctx, path, "config", "--replace-all", key, value); err == nil { + return nil + } + if !isConfigLockContention(err) { + return err + } + if serr := sleepCtx(ctx, time.Duration(attempt+1)*200*time.Millisecond); serr != nil { + return serr + } + } + // Still contended: the other worker is writing this same migration, so its + // result is the one we wanted. Only its absence is worth an error. + if cur, gerr := w.git(ctx, path, "config", "--get-all", key); gerr == nil && cur == value { + return nil + } + return err +} + +// unsetConfig removes every value of one config key of the mirror at path. +// +// Read before writing and retry on contention, for setConfig's reasons. What +// differs is what a failure costs: a remote.origin.mirror that survives this +// turns a session's plain `git push` back into a mirror push, so the key's +// absence is verified rather than inferred from a command having run. +func (w Workspace) unsetConfig(ctx context.Context, path, key string) error { + var err error + for attempt := 0; attempt < 3; attempt++ { + if _, gerr := w.git(ctx, path, "config", "--get-all", key); gerr != nil { + return nil // not set, which is the normal case and worth no write at all + } + if _, err = w.git(ctx, path, "config", "--unset-all", key); err == nil { + return nil + } + if !isConfigLockContention(err) { + return err + } + if serr := sleepCtx(ctx, time.Duration(attempt+1)*200*time.Millisecond); serr != nil { + return serr + } + } + if _, gerr := w.git(ctx, path, "config", "--get-all", key); gerr != nil { + return nil + } + return err +} + +// isConfigLockContention reports whether a config write lost the race for +// config.lock — git's own serialization, and so somebody else's success. +func isConfigLockContention(err error) bool { + if err == nil { + return false + } + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "could not lock config file") || + strings.Contains(msg, "another git process") +} + +// isRefLockContention reports whether a fetch failed because another process +// held the locks — the one failure that is somebody else's success, and so the +// only one worth retrying and then ignoring. Everything else (a bad token, an +// unreachable remote, a broken mirror) has to reach the caller. +func isRefLockContention(err error) bool { + if err == nil { + return false + } + msg := strings.ToLower(err.Error()) + switch { + case strings.Contains(msg, "cannot lock ref"), + strings.Contains(msg, "another git process"), + strings.Contains(msg, ".lock") && strings.Contains(msg, "file exists"): + return true + } + return false +} + +// Checkout is a worktree at one commit, and the directory git commands for that +// PR run in. +type Checkout struct { + Dir string + Repo string + PR int + mirror string + // ws is the workspace that made this checkout, kept so the git commands run + // inside it authenticate the same way the clone did. A session's push is a + // network command like any other, and a daemon holding only GITHUB_TOKEN has + // no credential helper to fall back on. + ws Workspace + // token makes this handle's directory its own. Two checkouts of one PR + // otherwise share a path, and a deferred Remove on the older handle deletes + // the newer one's worktree out from under it. + token string + // stop ends the heartbeat that keeps this checkout from being pruned while + // the process holding it is alive. + stop context.CancelFunc +} + +// Checkout creates a worktree of repo at sha, on a detached HEAD. +// +// Detached on purpose: this is a place to inspect and build, not a branch to +// commit to by accident. A caller that means to write creates its own branch. +func (w Workspace) Checkout(ctx context.Context, repo string, pr int, sha string) (Checkout, error) { + if strings.TrimSpace(sha) == "" { + return Checkout{}, errors.New("checkout needs a commit") + } + mirror, err := w.Mirror(ctx, repo) + if err != nil { + return Checkout{}, err + } + prDir, err := w.workPath(repo, pr) + if err != nil { + return Checkout{}, err + } + // Old generations are pruned by AGE, not by being there: another worker may + // be building in one right now, and force-removing it would pull the ground + // out from under a live session. Each handle removes its own on the way out; + // this only collects what a killed process left behind. + if err := pruneStaleWork(ctx, mirror, filepath.Dir(prDir)); err != nil { + return Checkout{}, err + } + w.fetchPullRef(ctx, mirror, repo, pr, sha) + token := randomToken() + dir := filepath.Join(prDir, token) + if err := os.MkdirAll(prDir, 0o700); err != nil { + return Checkout{}, err + } + // `--` before the positional arguments: a commit-ish beginning with a dash + // would otherwise be read as an option, which is the shape behind a long line + // of clone-and-checkout CVEs. + // A linked checkout can change the mirror's shared configuration. Do not + // execute hooks or fsmonitor commands a previous session installed while + // materializing repository-controlled content for this one. + if _, err := w.git(ctx, mirror, + "-c", "core.hooksPath="+os.DevNull, + "-c", "core.fsmonitor=false", + "worktree", "add", "--detach", "--", dir, sha, + ); err != nil { + return Checkout{}, fmt.Errorf("checking out %s@%s: %w", repo, shortSHA(sha), err) + } + if _, err := w.git(ctx, dir, "config", "--worktree", "--replace-all", "remote.origin.pushurl", w.remoteURL(repo)); err != nil { + _ = removeWorktree(ctx, mirror, dir) + return Checkout{}, fmt.Errorf("configuring push target for %s: %w", repo, err) + } + // Held for as long as the caller's context lives, which is as long as this + // handle is worth anything. A caller whose context ends at once gets what it + // got before: a checkout that ages out of the workspace on its own. + alive, stop := context.WithCancel(ctx) + go keepAlive(alive, dir, heartbeatInterval) + return Checkout{Dir: dir, Repo: normalizeRepo(repo), PR: pr, mirror: mirror, ws: w, token: token, stop: stop}, nil +} + +// fetchPullRef fetches refs/pull//head when sha is not in the mirror yet. +// +// A PR opened from a fork has its head on no branch of the base repository, so +// the mirror's refspec never brings it down and the worktree would fail at an +// unknown commit. GitHub publishes it as refs/pull//head. Best effort on +// purpose: the checkout that follows is the real check, and its error names the +// commit that could not be found. +func (w Workspace) fetchPullRef(ctx context.Context, mirror, repo string, pr int, sha string) { + if pr <= 0 { + return + } + if _, err := gitDir(ctx, mirror, "cat-file", "-e", sha+"^{commit}"); err == nil { + return + } + ref := fmt.Sprintf("+refs/pull/%d/head:refs/remotes/origin/pull/%d", pr, pr) + _, _ = w.git(ctx, mirror, "fetch", "--no-tags", w.remoteURL(repo), ref) +} + +// workPath is the directory holding this PR's checkouts. Owner and name stay +// separate path components: joined with a dash, "a-b/c" and "a/b-c" would +// collide, and one repository's cleanup would delete the other's live worktree. +func (w Workspace) workPath(repo string, pr int) (string, error) { + root, err := w.root() + if err != nil { + return "", err + } + owner, name, ok := splitRepo(repo) + if !ok { + return "", fmt.Errorf("repo must be owner/name, got %q", repo) + } + return filepath.Join(root, "work", owner, name, strconv.Itoa(pr)), nil +} + +// staleWorkAge is how long an abandoned checkout survives. Longer than any fix +// session should take, short enough that a killed watcher does not leave the +// disk filling up. +const staleWorkAge = 12 * time.Hour + +// heartbeatInterval is how often a live checkout refreshes itself. Far inside +// staleWorkAge, so a checkout ages out only once nothing has refreshed it for +// several dozen missed beats. +const heartbeatInterval = 15 * time.Minute + +// keepAlive refreshes dir's own timestamp until ctx ends. +// +// Age alone does not mean abandoned: a session can legitimately touch no file +// for hours — holding uncommitted fixes while it waits on a reviewer or on the +// queue — and pruning would then delete a checkout somebody is still using. Age +// SINCE THE LAST BEAT does mean it, because the only thing that stops the beat +// is the process that owns the checkout going away. +// +// The directory's own timestamp, not a marker file: it is what newestModTime +// already reads, and a file inside would turn up in the session's own git +// status and in the diff it is about to commit. +func keepAlive(ctx context.Context, dir string, every time.Duration) { + for { + if err := sleepCtx(ctx, every); err != nil { + return + } + now := time.Now() + if err := os.Chtimes(dir, now, now); err != nil { + return // the checkout is gone; there is nothing left to keep alive + } + } +} + +// pruneStaleWork removes checkouts anywhere under repoDir that nothing has +// touched for staleWorkAge, leaving live ones alone. +// +// Every PR of the repository, not only the one being checked out: a process +// killed mid-dispatch leaves a generation behind, and if that PR is then merged, +// closed or simply never dispatched again, nothing would ever come back to +// collect it. Sweeping the repository means any dispatch of any PR does. +func pruneStaleWork(ctx context.Context, mirror, repoDir string) error { + prDirs, err := os.ReadDir(repoDir) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return err + } + for _, prDir := range prDirs { + entries, err := os.ReadDir(filepath.Join(repoDir, prDir.Name())) + if err != nil { + continue + } + for _, entry := range entries { + dir := filepath.Join(repoDir, prDir.Name(), entry.Name()) + // A fresh root is the checkout's heartbeat, so it proves the session + // is live without recursively statting a potentially huge generated + // tree. Only stale roots need the deeper scan for recent edits. + now := time.Now() + if rootHeartbeatFresh(dir, now) { + continue + } + if touched := newestModTimeAt(dir, now); now.Sub(touched) < staleWorkAge { + continue + } + if err := removeWorktree(ctx, mirror, dir); err != nil { + return err + } + } + } + return nil +} + +func rootHeartbeatFresh(dir string, now time.Time) bool { + info, err := os.Stat(dir) + return err == nil && (info.ModTime().After(now) || now.Sub(info.ModTime()) < staleWorkAge) +} + +// newestModTime is the most recent modification anywhere under dir that is not +// in the future. +// +// A file can carry a timestamp later than now — an extracted archive that kept +// the packer's clock, a build stamping ahead, a host whose clock was corrected +// backwards. `time.Since` of such a stamp is negative, which reads as "touched +// moments ago" for as long as the future lasts, so one bad timestamp would make +// an abandoned checkout immortal and every dispatch would rescan it. +// +// Ignored rather than clamped to now: clamping produces exactly the same +// "touched moments ago" answer on every later scan. A stamp in the future says +// nothing about when anybody last worked here, so the newest real modification +// is the honest answer. If every stamp is in the future, though, the clock may +// have moved behind a live session's heartbeat; return now because its age is +// indeterminate and deleting it would risk losing active work. +func newestModTime(dir string) time.Time { + return newestModTimeAt(dir, time.Now()) +} + +func newestModTimeAt(dir string, now time.Time) time.Time { + newest := time.Time{} + _ = filepath.WalkDir(dir, func(_ string, d os.DirEntry, err error) error { + if err != nil { + return nil + } + info, ierr := d.Info() + if ierr != nil || info.ModTime().After(now) { + return nil + } + if info.ModTime().After(newest) { + newest = info.ModTime() + } + return nil + }) + // A backward clock adjustment can put the entire live checkout in the + // future. With no usable timestamp its age is indeterminate, so preserve it + // rather than treating the zero time as ancient. + if newest.IsZero() { + return now + } + return newest +} + +// Remove deletes the worktree. Safe to call on an already-removed one. +func (c Checkout) Remove(ctx context.Context) error { + if c.stop != nil { + c.stop() // this handle is done either way, so stop claiming it is live + } + if c.mirror == "" || c.Dir == "" || filepath.Base(c.Dir) != c.token { + // Not this handle's directory: a later checkout replaced it, and removing + // it would delete a worktree somebody else is using. + return nil + } + return removeWorktree(ctx, c.mirror, c.Dir) +} + +func removeWorktree(ctx context.Context, mirror, dir string) error { + if _, err := os.Stat(dir); errors.Is(err, os.ErrNotExist) { + // Still prune: git may hold a record of a directory somebody deleted. + _, _ = gitDir(ctx, mirror, "worktree", "prune") + return nil + } + if _, err := gitDir(ctx, mirror, "worktree", "remove", "--force", dir); err != nil { + // Fall back to deleting it ourselves; a stale registration is pruned. + if rmErr := os.RemoveAll(dir); rmErr != nil { + return rmErr + } + } + _, _ = gitDir(ctx, mirror, "worktree", "prune") + return nil +} + +// Git runs a git command inside this checkout, with the same credentials the +// mirror was cloned with — a session's push is a network command too. +func (c Checkout) Git(ctx context.Context, args ...string) (string, error) { + return c.ws.git(ctx, c.Dir, args...) +} + +// SetPushURL gives this checkout its own origin push target. Fetch configuration +// remains shared with the mirror, while sibling checkouts keep their targets. +func (c Checkout) SetPushURL(ctx context.Context, url string) error { + if strings.TrimSpace(url) == "" { + return errors.New("push URL must not be empty") + } + _, err := c.ws.git(ctx, c.Dir, "config", "--worktree", "--replace-all", "remote.origin.pushurl", url) + return err +} + +// gitDir runs git in dir ("" means the process's own directory) and returns its +// trimmed stdout. Stderr is folded into the error, because "exit status 128" on +// its own has never told anybody what went wrong. +func gitDir(ctx context.Context, dir string, args ...string) (string, error) { + return gitEnv(ctx, dir, nil, args...) +} + +// Git runs git in dir ("" means the process's own directory). +func Git(ctx context.Context, dir string, args ...string) (string, error) { + return gitDir(ctx, dir, args...) +} + +var repositoryLocalGitEnv = map[string]bool{ + "GIT_ALTERNATE_OBJECT_DIRECTORIES": true, + "GIT_COMMON_DIR": true, + "GIT_CONFIG": true, + "GIT_CONFIG_COUNT": true, + "GIT_CONFIG_PARAMETERS": true, + "GIT_DIR": true, + "GIT_GRAFT_FILE": true, + "GIT_IMPLICIT_WORK_TREE": true, + "GIT_INDEX_FILE": true, + "GIT_NAMESPACE": true, + "GIT_NO_REPLACE_OBJECTS": true, + "GIT_OBJECT_DIRECTORY": true, + "GIT_PREFIX": true, + "GIT_REPLACE_REF_BASE": true, + "GIT_SHALLOW_FILE": true, + "GIT_WORK_TREE": true, +} + +// gitEnv is gitDir with extra environment entries. +func gitEnv(ctx context.Context, dir string, env []string, args ...string) (string, error) { + // A linked checkout can write executable settings into the shared mirror. + // Disable them even when no token is injected, and keep an invoking Git hook + // from redirecting commands away from dir through repository-local variables. + full := append([]string{ + "-c", "core.hooksPath=" + os.DevNull, + "-c", "core.fsmonitor=false", + }, args...) + cmd := exec.CommandContext(ctx, "git", full...) + cmd.Dir = dir + overrides := make(map[string]bool, len(env)) + for _, entry := range env { + name, _, _ := strings.Cut(entry, "=") + overrides[name] = true + } + parent := os.Environ() + cmd.Env = make([]string, 0, len(parent)+len(env)) + for _, entry := range parent { + name, _, _ := strings.Cut(entry, "=") + if !repositoryLocalGitEnv[name] && !overrides[name] { + cmd.Env = append(cmd.Env, entry) + } + } + cmd.Env = append(cmd.Env, env...) + var stderr strings.Builder + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + if detail := strings.TrimSpace(stderr.String()); detail != "" { + return "", fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, detail) + } + return "", fmt.Errorf("git %s: %w", strings.Join(args, " "), err) + } + return strings.TrimSpace(string(out)), nil +} + +// sleepCtx waits, or returns early when the context ends. +func sleepCtx(ctx context.Context, d time.Duration) error { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +func normalizeRepo(repo string) string { + repo = strings.TrimSpace(repo) + repo = strings.TrimSuffix(repo, ".git") + return strings.ToLower(repo) +} + +func randomToken() string { + var buf [16]byte + if _, err := io.ReadFull(rand.Reader, buf[:]); err != nil { + return strconv.FormatInt(time.Now().UnixNano(), 16) + } + return hex.EncodeToString(buf[:]) +} + +func shortSHA(sha string) string { + if len(sha) > 9 { + return sha[:9] + } + return sha +} diff --git a/internal/workspace/workspace_test.go b/internal/workspace/workspace_test.go new file mode 100644 index 0000000..e6b4668 --- /dev/null +++ b/internal/workspace/workspace_test.go @@ -0,0 +1,1541 @@ +package workspace + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +// originRepo builds a real repository on disk to clone from, so these tests +// exercise git itself rather than a mock of it — the whole point of this code is +// that the git invocations are right. +func originRepo(t *testing.T, dir string) (sha string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + run := func(args ...string) string { + t.Helper() + out, err := gitDir(context.Background(), dir, args...) + if err != nil { + t.Fatalf("git %s: %v", strings.Join(args, " "), err) + } + return out + } + run("init", "--initial-branch=main") + run("config", "user.email", "test@example.invalid") + run("config", "user.name", "crq test") + if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("one\n"), 0o644); err != nil { + t.Fatal(err) + } + run("add", "README.md") + run("commit", "-m", "first") + return run("rev-parse", "HEAD") +} + +// crq's daemon has no checkout of any repository it reviews, so it needs its own +// place on disk. This covers the whole shape: clone once, fetch after, worktree +// at a commit, and clean up. +func TestWorkspaceChecksOutAHeadWithoutACheckout(t *testing.T) { + // The remote base plus "owner/name" has to resolve to a real repository, so + // lay the origin out the way GitHub does. + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(mirror, "HEAD")); err != nil { + t.Fatalf("mirror is not a git directory: %v", err) + } + + // Second call fetches into the same mirror rather than cloning again: a + // mirror is reused across PRs and rounds, so the clone happens once. + again, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if again != mirror { + t.Errorf("second Mirror = %q, want the same path %q", again, mirror) + } + + co, err := ws.Checkout(ctx, repo, 7, sha) + if err != nil { + t.Fatal(err) + } + if body, err := os.ReadFile(filepath.Join(co.Dir, "README.md")); err != nil || string(body) != "one\n" { + t.Fatalf("worktree content = %q err=%v, want the committed file", body, err) + } + // Detached on purpose: this is a place to inspect, not a branch to commit to + // by accident. + if branch, err := co.Git(ctx, "rev-parse", "--abbrev-ref", "HEAD"); err != nil || branch != "HEAD" { + t.Errorf("HEAD = %q err=%v, want a detached checkout", branch, err) + } + if head, err := co.Git(ctx, "rev-parse", "HEAD"); err != nil || head != sha { + t.Errorf("checked out %q, want %q", head, sha) + } + + // A worktree left by a killed process must not wedge the next attempt. + if _, err := ws.Checkout(ctx, repo, 7, sha); err != nil { + t.Fatalf("re-checkout over an existing worktree: %v", err) + } + if err := co.Remove(ctx); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(co.Dir); !os.IsNotExist(err) { + t.Errorf("worktree still present after Remove: %v", err) + } + if err := co.Remove(ctx); err != nil { + t.Errorf("removing an already-removed worktree must be harmless, got %v", err) + } +} + +// A repo name is turned into path segments, so anything that could climb out of +// the workspace root has to be refused rather than joined. +func TestWorkspaceRefusesRepoNamesThatEscape(t *testing.T) { + ws := Workspace{Root: t.TempDir()} + for _, repo := range []string{"", "noslash", "../etc/passwd", "owner/../../etc", "owner/", "/name"} { + if _, err := ws.mirrorPath(repo); err == nil { + t.Errorf("mirrorPath(%q) was accepted; it must be refused", repo) + } + } +} + +// The error has to say what git said. "exit status 128" alone has never told +// anybody what went wrong. +func TestGitErrorsCarryStderr(t *testing.T) { + _, err := gitDir(context.Background(), t.TempDir(), "rev-parse", "HEAD") + if err == nil { + t.Fatal("expected an error outside a repository") + } + if !strings.Contains(err.Error(), "rev-parse") || !strings.Contains(strings.ToLower(err.Error()), "git") { + t.Errorf("error %q does not name the command that failed", err) + } +} + +// Four ways a workspace could destroy or expose something it should not. +func TestWorkspaceIsolatesCheckouts(t *testing.T) { + base := t.TempDir() + sha := originRepo(t, filepath.Join(base, "owner/thing")) + t.Setenv("CRQ_REMOTE_BASE", base) + root := t.TempDir() + ws := Workspace{Root: root} + ctx := context.Background() + + // A stale handle must not delete the checkout that replaced it. + first, err := ws.Checkout(ctx, "owner/thing", 9, sha) + if err != nil { + t.Fatal(err) + } + second, err := ws.Checkout(ctx, "owner/thing", 9, sha) + if err != nil { + t.Fatal(err) + } + if first.Dir == second.Dir { + t.Fatal("two checkouts of one PR share a directory, so either can delete the other") + } + if err := first.Remove(ctx); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(second.Dir); err != nil { + t.Errorf("the live checkout was removed by a stale handle: %v", err) + } + + // Repo names that differ only in where the slash falls must not collide: + // "a-b/c" and "a/b-c" joined with a dash are the same string. + sha2 := originRepo(t, filepath.Join(base, "a-b/c")) + sha3 := originRepo(t, filepath.Join(base, "a/b-c")) + one, err := ws.Checkout(ctx, "a-b/c", 1, sha2) + if err != nil { + t.Fatal(err) + } + two, err := ws.Checkout(ctx, "a/b-c", 1, sha3) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(one.Dir); err != nil { + t.Errorf("checking out a/b-c destroyed a-b/c's worktree: %v", err) + } + if strings.HasPrefix(two.Dir, one.Dir) || strings.HasPrefix(one.Dir, two.Dir) { + t.Errorf("checkout paths overlap: %q and %q", one.Dir, two.Dir) + } + + // A mirror of a private repository is private source: on a shared host the + // umask default would let any local user read it. + info, err := os.Stat(root) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o700 { + t.Errorf("workspace root mode = %o, want 0700", perm) + } +} + +// Two workers starting on the same repository at once must not clone into one +// directory — the second used to fail with "destination path already exists". +func TestMirrorSurvivesAConcurrentFirstClone(t *testing.T) { + base := t.TempDir() + originRepo(t, filepath.Join(base, "owner/thing")) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + + errs := make(chan error, 4) + for i := 0; i < 4; i++ { + go func() { + _, err := ws.Mirror(context.Background(), "owner/thing") + errs <- err + }() + } + for i := 0; i < 4; i++ { + if err := <-errs; err != nil { + t.Errorf("concurrent first clone: %v", err) + } + } +} + +// The token must never reach argv: a process listing, a log line and this +// package's own error strings would all carry it. +func TestGitTokenTravelsInTheEnvironment(t *testing.T) { + ws := Workspace{Root: t.TempDir(), Token: "ghp_secret_value"} + _, err := ws.git(context.Background(), t.TempDir(), "rev-parse", "HEAD") + if err == nil { + t.Fatal("expected an error outside a repository") + } + if strings.Contains(err.Error(), "ghp_secret_value") { + t.Errorf("the token leaked into an error message: %v", err) + } +} + +func TestGitIgnoresAnInvokingRepositoriesEnvironment(t *testing.T) { + intended := t.TempDir() + other := t.TempDir() + originRepo(t, intended) + originRepo(t, other) + t.Setenv("GIT_DIR", filepath.Join(other, ".git")) + t.Setenv("GIT_WORK_TREE", other) + t.Setenv("GIT_INDEX_FILE", filepath.Join(other, ".git", "index")) + + got, err := gitDir(context.Background(), intended, "rev-parse", "--show-toplevel") + if err != nil { + t.Fatal(err) + } + want, err := filepath.EvalSymlinks(intended) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("git selected %q from the invoking environment, want %q", got, want) + } +} + +// A relative root would make `git worktree add` — which runs inside the mirror — +// create the worktree under the mirror, while the path handed back points at a +// directory that does not exist. +func TestWorkspaceResolvesARelativeRoot(t *testing.T) { + base := t.TempDir() + sha := originRepo(t, filepath.Join(base, "owner/thing")) + t.Setenv("CRQ_REMOTE_BASE", base) + + // Run from a scratch directory so a relative root has somewhere to land. + scratch := t.TempDir() + prev, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(scratch); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(prev) }) + + co, err := Workspace{Root: "relative-cache"}.Checkout(context.Background(), "owner/thing", 5, sha) + if err != nil { + t.Fatal(err) + } + if !filepath.IsAbs(co.Dir) { + t.Errorf("checkout dir %q is relative", co.Dir) + } + if _, err := os.Stat(filepath.Join(co.Dir, "README.md")); err != nil { + t.Errorf("the checkout is not where it says it is: %v", err) + } +} + +// Another worker may be building in an earlier generation right now. Clearing +// the directory eagerly pulled the ground out from under a live session. +func TestCheckoutLeavesALiveSiblingAlone(t *testing.T) { + base := t.TempDir() + sha := originRepo(t, filepath.Join(base, "owner/thing")) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + first, err := ws.Checkout(ctx, "owner/thing", 6, sha) + if err != nil { + t.Fatal(err) + } + if _, err := ws.Checkout(ctx, "owner/thing", 6, sha); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(first.Dir, "README.md")); err != nil { + t.Errorf("a live checkout was removed by the next one: %v", err) + } +} + +// A fix session's documented way to make changes is a branch in its checkout. +// A --mirror clone's refspec is +refs/*:refs/*, so the next `fetch --prune` +// reached into refs/heads and deleted that branch — destroying the session's work +// between one dispatch and the next. +func TestFetchDoesNotDeleteABranchAWorktreeCreated(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + co, err := ws.Checkout(ctx, repo, 3, sha) + if err != nil { + t.Fatal(err) + } + if _, err := co.Git(ctx, "checkout", "-b", "crq/fix-3"); err != nil { + t.Fatal(err) + } + + // Another dispatch fetches the shared mirror. + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if branch, err := co.Git(ctx, "rev-parse", "--abbrev-ref", "HEAD"); err != nil || branch != "crq/fix-3" { + t.Fatalf("branch = %q err=%v, want the session's branch to survive a fetch", branch, err) + } +} + +// Pruning read the checkout directory's own mtime, which editing files inside +// does not update — so a busy session read as abandoned and was deleted. +func TestPruningMeasuresTheNewestFileNotTheDirectory(t *testing.T) { + dir := t.TempDir() + old := time.Now().Add(-48 * time.Hour) + nested := filepath.Join(dir, "sub") + if err := os.MkdirAll(nested, 0o755); err != nil { + t.Fatal(err) + } + file := filepath.Join(nested, "edited.go") + if err := os.WriteFile(file, []byte("package main"), 0o644); err != nil { + t.Fatal(err) + } + // The directories look ancient; the file inside was just written. + for _, d := range []string{dir, nested} { + if err := os.Chtimes(d, old, old); err != nil { + t.Fatal(err) + } + } + if since := time.Since(newestModTime(dir)); since > time.Minute { + t.Errorf("newest modification read as %s ago; a live session would be pruned", since) + } +} + +// One file stamped in the future — an extracted artifact, a corrected clock — +// made time.Since negative and so always under staleWorkAge, and the abandoned +// checkout holding it would never be collected. +func TestPruningIgnoresATimestampInTheFuture(t *testing.T) { + dir := t.TempDir() + old := time.Now().Add(-2 * staleWorkAge) + future := filepath.Join(dir, "artifact.tar") + if err := os.WriteFile(future, []byte("packed elsewhere"), 0o644); err != nil { + t.Fatal(err) + } + ahead := time.Now().Add(90 * 24 * time.Hour) + if err := os.Chtimes(future, ahead, ahead); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(dir, old, old); err != nil { + t.Fatal(err) + } + if since := time.Since(newestModTime(dir)); since < staleWorkAge { + t.Errorf("the checkout reads as %s old, want the future stamp ignored and the checkout collectable", since) + } +} + +// When the host clock moves backwards, every timestamp in a recently created +// checkout can be in the future. With no trustworthy age, pruning must preserve +// the checkout rather than treating the zero time as ancient. +func TestPruningPreservesACheckoutWithOnlyFutureTimestamps(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "edited.go") + if err := os.WriteFile(file, []byte("package main"), 0o644); err != nil { + t.Fatal(err) + } + now := time.Now() + future := now.Add(time.Hour) + for _, path := range []string{file, dir} { + if err := os.Chtimes(path, future, future); err != nil { + t.Fatal(err) + } + } + if got := newestModTimeAt(dir, now); got != now { + t.Errorf("newest modification = %s, want indeterminate checkout preserved at %s", got, now) + } +} + +// `clone --bare` copies the remote's branch heads straight into refs/heads, and +// a refspec set afterwards only governs later fetches. A session could then not +// create its branch — the name is taken — or would land on the commit the clone +// froze there. +func TestANewMirrorLeavesRefsHeadsToTheSessions(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + if _, err := gitDir(context.Background(), filepath.Join(base, repo), "branch", "feature"); err != nil { + t.Fatal(err) + } + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if heads, err := gitDir(ctx, mirror, "for-each-ref", "refs/heads"); err != nil || heads != "" { + t.Errorf("refs/heads = %q err=%v, want it empty and left to the sessions", heads, err) + } + if remotes, err := gitDir(ctx, mirror, "for-each-ref", "--format=%(refname)", "refs/remotes/origin"); err != nil || !strings.Contains(remotes, "refs/remotes/origin/feature") { + t.Errorf("remote refs = %q err=%v, want the branches fetched under origin", remotes, err) + } + + // The name a remote branch occupies is therefore free for a session. + co, err := ws.Checkout(ctx, repo, 8, sha) + if err != nil { + t.Fatal(err) + } + if _, err := co.Git(ctx, "checkout", "-b", "feature"); err != nil { + t.Errorf("a session could not create its branch: %v", err) + } +} + +// A mirror cloned by an older crq also has remote.origin.mirror=true, which the +// refspec does not clear: a plain push from a session's worktree would mirror +// every local ref, publishing internal refs and deleting remote branches. +func TestMirrorMigrationClearsThePushMirrorFlag(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "config", "remote.origin.mirror", "true"); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if got, err := gitDir(ctx, mirror, "config", "--get", "remote.origin.mirror"); err == nil { + t.Errorf("remote.origin.mirror = %q, want it unset", got) + } +} + +// Each checkout has its own push URL: a fork session must not repoint a sibling +// or the mirror, and a base-repository refresh must leave it alone. +func TestCheckoutPushURLsAreWorktreeLocal(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + other := filepath.Join(base, "fork/thing") + originRepo(t, other) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + first, err := ws.Checkout(ctx, repo, 1, sha) + if err != nil { + t.Fatal(err) + } + second, err := ws.Checkout(ctx, repo, 2, sha) + if err != nil { + t.Fatal(err) + } + if err := second.SetPushURL(ctx, other); err != nil { + t.Fatal(err) + } + if got, err := first.Git(ctx, "remote", "get-url", "--push", "origin"); err != nil || got != ws.remoteURL(repo) { + t.Errorf("first push URL = %q err=%v, want base %q", got, err, ws.remoteURL(repo)) + } + if got, err := second.Git(ctx, "remote", "get-url", "--push", "origin"); err != nil || got != other { + t.Errorf("second push URL = %q err=%v, want fork %q", got, err, other) + } + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if got, err := gitDir(ctx, mirror, "config", "--local", "--get", "remote.origin.pushurl"); err == nil { + t.Errorf("shared push URL = %q, want none", got) + } + + if err := os.WriteFile(filepath.Join(origin, "README.md"), []byte("base moved\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "README.md"}, {"commit", "-m", "base moved"}} { + if _, err := gitDir(ctx, origin, args...); err != nil { + t.Fatal(err) + } + } + want, err := gitDir(ctx, origin, "rev-parse", "HEAD") + if err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if got, err := second.Git(ctx, "remote", "get-url", "--push", "origin"); err != nil || got != other { + t.Errorf("fork push URL after refresh = %q err=%v, want %q", got, err, other) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/remotes/origin/main"); err != nil || got != want { + t.Errorf("fetched main = %q err=%v, want base repository head %q", got, err, want) + } +} + +// The mirror's HEAD exists because the mirror exists, so testing for it after a +// failed fetch declared every failure contention: expired credentials and an +// unreachable remote came back as success with stale refs, and the caller met +// them later as an unreadable commit instead. +func TestMirrorReportsAFetchFailureThatIsNotContention(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + // The remote goes away, the way an expired token or a network outage does. + if err := os.RemoveAll(filepath.Join(base, repo)); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err == nil { + t.Error("a failed fetch was reported as a current mirror") + } +} + +// A PR opened from a fork has its head on no branch of the base repository, so +// the mirror's refspec never brings it down; GitHub publishes it as +// refs/pull//head. +func TestCheckoutFetchesAForkHeadFromThePullRef(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + originRepo(t, origin) + ctx := context.Background() + run := func(args ...string) string { + t.Helper() + out, err := gitDir(ctx, origin, args...) + if err != nil { + t.Fatalf("git %s: %v", strings.Join(args, " "), err) + } + return out + } + // A commit reachable from the pull ref alone, exactly like a fork's head. + run("checkout", "-q", "-b", "from-a-fork") + if err := os.WriteFile(filepath.Join(origin, "fork.txt"), []byte("two\n"), 0o644); err != nil { + t.Fatal(err) + } + run("add", "fork.txt") + run("commit", "-m", "fork work") + forkSHA := run("rev-parse", "HEAD") + run("update-ref", "refs/pull/11/head", forkSHA) + run("checkout", "-q", "main") + run("branch", "-qD", "from-a-fork") + + t.Setenv("CRQ_REMOTE_BASE", base) + co, err := Workspace{Root: t.TempDir()}.Checkout(ctx, repo, 11, forkSHA) + if err != nil { + t.Fatalf("checking out a fork head: %v", err) + } + if head, err := co.Git(ctx, "rev-parse", "HEAD"); err != nil || head != forkSHA { + t.Errorf("checked out %q err=%v, want the fork head %q", head, err, forkSHA) + } +} + +// Pruning ran only under the PR being checked out, so a generation left by a +// killed process was collected only if that same PR was dispatched again — never, +// once it was merged or closed. +func TestPruningCollectsAnAbandonedCheckoutOfAnotherPR(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + abandoned, err := ws.workPath(repo, 21) + if err != nil { + t.Fatal(err) + } + abandoned = filepath.Join(abandoned, "leftover") + if err := os.MkdirAll(abandoned, 0o700); err != nil { + t.Fatal(err) + } + file := filepath.Join(abandoned, "stale.txt") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + old := time.Now().Add(-2 * staleWorkAge) + for _, p := range []string{file, abandoned} { + if err := os.Chtimes(p, old, old); err != nil { + t.Fatal(err) + } + } + + // A dispatch of a different PR is the only visitor this repository gets. + if _, err := ws.Checkout(ctx, repo, 22, sha); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(abandoned); !os.IsNotExist(err) { + t.Errorf("an abandoned checkout of another PR survived: %v", err) + } +} + +// A daemon holding only GITHUB_TOKEN has no credential helper, so a push from a +// session's checkout could not authenticate even though the clone did. +func TestCheckoutGitCarriesTheWorkspaceCredentials(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ctx := context.Background() + + co, err := Workspace{Root: t.TempDir(), Token: "ghp_secret_value"}.Checkout(ctx, repo, 12, sha) + if err != nil { + t.Fatal(err) + } + helper, err := co.Git(ctx, "config", "--get-all", "credential.helper") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(helper, "CRQ_GIT_TOKEN") { + t.Errorf("credential.helper = %q, want the workspace's helper", helper) + } + if strings.Contains(helper, "ghp_secret_value") { + t.Errorf("the token itself reached git's configuration: %q", helper) + } +} + +// A checkout can install executable Git configuration in the shared mirror. +// Later credential-bearing commands must not expose another session's token to +// that configuration. +func TestCheckoutGitProtectsCredentialsFromSharedHooks(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + co, err := Workspace{Root: t.TempDir(), Token: "ghp_secret_value"}.Checkout(ctx, repo, 12, sha) + if err != nil { + t.Fatal(err) + } + hooks := t.TempDir() + marker := filepath.Join(t.TempDir(), "token") + hook := filepath.Join(hooks, "pre-push") + script := "#!/bin/sh\nprintf '%s' \"$CRQ_GIT_TOKEN\" >\"" + marker + "\"\n" + if err := os.WriteFile(hook, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, co.Dir, "config", "core.hooksPath", hooks); err != nil { + t.Fatal(err) + } + + if _, err := co.Git(ctx, "push", "--dry-run", "origin", "HEAD:refs/heads/token-test"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Errorf("shared pre-push hook saw a later checkout's token: %v", err) + } +} + +// A mirror created before the refspec rule still fetches +refs/*:refs/*. One +// branch created in a worktree then wedges every future fetch for the whole +// repository — "refusing to fetch into branch ... checked out at" — which is how +// a single fix session stopped every dispatch for hours. +func TestMirrorMigratesAnOldRefspec(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + // Put it back the way an older crq left it. + if _, err := gitDir(ctx, mirror, "config", "remote.origin.fetch", "+refs/*:refs/*"); err != nil { + t.Fatal(err) + } + co, err := ws.Checkout(ctx, repo, 4, sha) + if err != nil { + t.Fatal(err) + } + if _, err := co.Git(ctx, "checkout", "-b", "session-branch"); err != nil { + t.Fatal(err) + } + + // The next dispatch of ANY pr in this repository fetches the same mirror. + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatalf("a branch in one worktree wedged the whole repository: %v", err) + } + got, err := gitDir(ctx, mirror, "config", "--get", "remote.origin.fetch") + if err != nil || got != originRefspec { + t.Errorf("refspec = %q err=%v, want it migrated to %q", got, err, originRefspec) + } +} + +// A mirror an older crq made with `git clone --mirror` had already copied the +// remote's branches into refs/heads, and changing the fetch refspec does not +// move them: the name a session needs stays taken by a ref frozen at the commit +// that clone saw. +func TestMirrorDropsHeadsAnOldCloneLeftInPlace(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + if _, err := gitDir(ctx, origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + + // Lay the mirror out the way the older crq left it. + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(mirror), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, "", "clone", "--mirror", "--quiet", ws.remoteURL(repo), mirror); err != nil { + t.Fatal(err) + } + // A branch of a session's own, which origin has never heard of. + if _, err := gitDir(ctx, mirror, "update-ref", "refs/heads/crq/fix-3", sha); err != nil { + t.Fatal(err) + } + + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + heads, err := gitDir(ctx, mirror, "for-each-ref", "--format=%(refname)", "refs/heads") + if err != nil { + t.Fatal(err) + } + if strings.Contains(heads, "refs/heads/feature") || strings.Contains(heads, "refs/heads/main") { + t.Errorf("refs/heads = %q, want the clone's copies of origin's branches gone", heads) + } + if !strings.Contains(heads, "refs/heads/crq/fix-3") { + t.Errorf("refs/heads = %q, want a session's own branch left alone", heads) + } + if remotes, err := gitDir(ctx, mirror, "for-each-ref", "--format=%(refname)", "refs/remotes/origin"); err != nil || !strings.Contains(remotes, "refs/remotes/origin/feature") { + t.Errorf("remote refs = %q err=%v, want origin's branches under origin", remotes, err) + } + + // The name is free for a session again — and stays the session's, even though + // origin has a branch of that name: `git update-ref -d` would delete a + // checked-out branch without complaint, work and all. + co, err := ws.Checkout(ctx, repo, 3, sha) + if err != nil { + t.Fatal(err) + } + if _, err := co.Git(ctx, "checkout", "-b", "feature"); err != nil { + t.Fatalf("a session could not create its branch: %v", err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if branch, err := co.Git(ctx, "rev-parse", "--abbrev-ref", "HEAD"); err != nil || branch != "feature" { + t.Errorf("branch = %q err=%v, want the session's checked-out branch to survive", branch, err) + } +} + +// Checked-out status alone is not what tells a clone's leftover from a session's +// branch: a session that detaches HEAD to look at another commit still owns the +// branch it committed to, and deleting that ref is the only thing keeping its +// commits reachable. +func TestMirrorKeepsASessionBranchThatIsNotCheckedOut(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + // Origin has the name too, which is what made the mirror read the session's + // branch as a copy of it. + if _, err := gitDir(ctx, origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + + co, err := ws.Checkout(ctx, repo, 7, sha) + if err != nil { + t.Fatal(err) + } + for _, args := range [][]string{ + {"config", "user.email", "test@example.invalid"}, + {"config", "user.name", "crq test"}, + {"checkout", "-b", "feature"}, + {"commit", "--allow-empty", "-m", "unpushed work"}, + {"checkout", "--detach"}, + } { + if _, err := co.Git(ctx, args...); err != nil { + t.Fatalf("git %s: %v", strings.Join(args, " "), err) + } + } + work, err := co.Git(ctx, "rev-parse", "refs/heads/feature") + if err != nil { + t.Fatal(err) + } + + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/heads/feature"); err != nil || got != work { + t.Errorf("refs/heads/feature = %q err=%v, want the session's unpushed commit %s", got, err, work) + } +} + +// Once an old mirror's fetched heads have been removed, refs/heads belongs to +// sessions. An equal-tip branch is still session state: deleting it on every +// refresh also deletes its tracking configuration and reflog. +func TestMirrorKeepsAnEqualTipSessionBranchAfterMigration(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + if _, err := gitDir(ctx, origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + + // Exercise the legacy-clone migration before the session takes ownership of + // the name. Later Mirror calls must not run that cleanup against its branch. + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(mirror), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, "", "clone", "--mirror", "--quiet", ws.remoteURL(repo), mirror); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + + co, err := ws.Checkout(ctx, repo, 7, sha) + if err != nil { + t.Fatal(err) + } + for _, args := range [][]string{ + {"checkout", "-b", "feature"}, + {"config", "branch.feature.remote", "origin"}, + {"config", "branch.feature.merge", "refs/heads/feature"}, + {"checkout", "--detach"}, + } { + if _, err := co.Git(ctx, args...); err != nil { + t.Fatalf("git %s: %v", strings.Join(args, " "), err) + } + } + if _, err := co.Git(ctx, "reflog", "exists", "refs/heads/feature"); err != nil { + t.Fatalf("session branch had no reflog before refresh: %v", err) + } + + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/heads/feature"); err != nil || got != sha { + t.Errorf("refs/heads/feature = %q err=%v, want equal-tip session branch %s", got, err, sha) + } + if got, err := gitDir(ctx, mirror, "config", "--get", "branch.feature.remote"); err != nil || got != "origin" { + t.Errorf("branch.feature.remote = %q err=%v, want tracking configuration preserved", got, err) + } + if got, err := gitDir(ctx, mirror, "config", "--get", "branch.feature.merge"); err != nil || got != "refs/heads/feature" { + t.Errorf("branch.feature.merge = %q err=%v, want tracking configuration preserved", got, err) + } + if _, err := gitDir(ctx, mirror, "reflog", "exists", "refs/heads/feature"); err != nil { + t.Errorf("session branch reflog was deleted: %v", err) + } +} + +func TestConcurrentMigrationRechecksOwnershipAfterLock(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + if _, err := gitDir(ctx, origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(mirror), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, "", "clone", "--mirror", "--quiet", ws.remoteURL(repo), mirror); err != nil { + t.Fatal(err) + } + + unlock, err := lockMirrorMigration(ctx, mirror) + if err != nil { + t.Fatal(err) + } + release := sync.OnceFunc(unlock) + defer release() + waiter := make(chan error, 1) + go func() { + _, err := ws.Mirror(ctx, repo) + waiter <- err + }() + select { + case err := <-waiter: + release() + t.Fatalf("concurrent refresh returned before the migration lock was released: %v", err) + case <-time.After(100 * time.Millisecond): + // The pre-lock checks are local config reads; still waiting proves this + // refresh reached the claim held above. + } + + // Complete the migration as the lock owner, then let a session reuse an + // equal-tip name before the waiting refresh acquires the claim. + if err := ws.migrateMirror(ctx, mirror); err != nil { + t.Fatal(err) + } + if err := ws.fetchMirror(ctx, mirror, repo); err != nil { + t.Fatal(err) + } + if err := ws.dropFetchedHeads(ctx, mirror); err != nil { + t.Fatal(err) + } + if err := ws.setConfig(ctx, mirror, fetchedHeadsMigratedKey, "true"); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "update-ref", "refs/heads/feature", sha); err != nil { + t.Fatal(err) + } + release() + + select { + case err := <-waiter: + if err != nil { + t.Fatal(err) + } + case <-time.After(10 * time.Second): + t.Fatal("concurrent refresh did not finish after the migration lock was released") + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/heads/feature"); err != nil || got != sha { + t.Errorf("session branch = %q err=%v, want preserved at %q", got, err, sha) + } +} + +// A mirror with a second remote.origin.fetch refused a single-value write — +// "cannot overwrite multiple values with a single value" — which would have +// failed every later Mirror call for that repository for good. +func TestMirrorMigratesAMultiValuedRefspec(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + // The refspec that reaches into refs/heads, added rather than replacing. + if _, err := gitDir(ctx, mirror, "config", "--add", "remote.origin.fetch", "+refs/*:refs/*"); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatalf("a mirror with two refspecs could not be migrated: %v", err) + } + if got, err := gitDir(ctx, mirror, "config", "--get-all", "remote.origin.fetch"); err != nil || got != originRefspec { + t.Errorf("refspec = %q err=%v, want only %q", got, err, originRefspec) + } +} + +// Normalizing a legacy mirror's configuration removes the evidence that its +// refs/heads came from clone --mirror. A pending marker must preserve that fact +// when deletion fails so the next refresh retries instead of abandoning cleanup. +func TestMirrorRetriesLegacyHeadCleanup(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + if _, err := gitDir(ctx, origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + + mirror, err := ws.mirrorPath(repo) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(mirror), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, "", "clone", "--mirror", "--quiet", ws.remoteURL(repo), mirror); err != nil { + t.Fatal(err) + } + // Keep a loose copy behind the lock. Some Git versions remove only the + // packed copy before reporting the lock error, which is successful cleanup + // and must not make this retry test fail. + feature, err := gitDir(ctx, mirror, "rev-parse", "refs/heads/feature") + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(mirror, "refs", "heads"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(mirror, "refs", "heads", "feature"), []byte(feature+"\n"), 0o600); err != nil { + t.Fatal(err) + } + lock := filepath.Join(mirror, "refs", "heads", "feature.lock") + if err := os.WriteFile(lock, nil, 0o600); err != nil { + t.Fatal(err) + } + + if _, err := ws.Mirror(ctx, repo); err == nil { + t.Fatal("legacy head cleanup unexpectedly succeeded while its ref was locked") + } + if pending, err := gitDir(ctx, mirror, "config", "--bool", "--get", fetchedHeadsMigratedKey); err != nil || pending != "false" { + t.Fatalf("cleanup marker = %q err=%v, want pending", pending, err) + } + if err := os.Remove(lock); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatalf("retrying legacy head cleanup: %v", err) + } + if heads, err := gitDir(ctx, mirror, "for-each-ref", "--format=%(refname)", "refs/heads"); err != nil || heads != "" { + t.Errorf("refs/heads = %q err=%v, want legacy heads removed by retry", heads, err) + } + if migrated, err := gitDir(ctx, mirror, "config", "--bool", "--get", fetchedHeadsMigratedKey); err != nil || migrated != "true" { + t.Errorf("cleanup marker = %q err=%v, want completed", migrated, err) + } +} + +// A ref lock left behind by a killed git never clears, and reads exactly like a +// live race for as long as it sits there. Retrying past it and handing back the +// mirror anyway presented refs known to be stale as current ones. +func TestMirrorReportsAFetchItCouldNotComplete(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + // Origin moves on, so the next fetch has a ref it must update... + if err := os.WriteFile(filepath.Join(origin, "README.md"), []byte("two\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "README.md"}, {"commit", "-m", "second"}} { + if _, err := gitDir(ctx, origin, args...); err != nil { + t.Fatal(err) + } + } + // ...and the lock makes that impossible for as long as it is there. + lock := filepath.Join(mirror, "refs", "remotes", "origin", "main.lock") + if err := os.MkdirAll(filepath.Dir(lock), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(lock, nil, 0o600); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err == nil { + t.Error("a mirror that could not be fetched was reported as current") + } +} + +// Discarding the failure to clear remote.origin.mirror handed back a mirror +// whose later plain push still had mirror semantics — publishing internal refs +// and deleting remote branches, the exact hazard the migration exists for. +func TestMirrorReportsAPushMirrorFlagItCouldNotClear(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "config", "remote.origin.mirror", "true"); err != nil { + t.Fatal(err) + } + // Somebody else holds the lock for good, the way a killed git does. + if err := os.WriteFile(filepath.Join(mirror, "config.lock"), nil, 0o600); err != nil { + t.Fatal(err) + } + defer os.Remove(filepath.Join(mirror, "config.lock")) + if _, err := ws.Mirror(ctx, repo); err == nil { + t.Error("a mirror still configured to push as a mirror was handed back as usable") + } +} + +// git serializes config writes through config.lock, so an unconditional write on +// every Mirror call made two dispatches of one repository collide with "could +// not lock config file" — before the fetch, which is the part written to survive +// concurrency. A mirror already holding the right value must not write at all. +func TestMirrorDoesNotWriteConfigThatIsAlreadyCurrent(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + // Somebody else is holding the lock, exactly as a concurrent dispatch would. + if err := os.WriteFile(filepath.Join(mirror, "config.lock"), nil, 0o600); err != nil { + t.Fatal(err) + } + defer os.Remove(filepath.Join(mirror, "config.lock")) + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Errorf("a held config.lock failed a dispatch that had nothing to write: %v", err) + } +} + +// A git command can be led to a URL that repository content chose — a submodule, +// an LFS endpoint — so a helper that answered every request would hand a pull +// request the account's token. +func TestCredentialHelperAnswersOnlyForGitHub(t *testing.T) { + ask := func(t *testing.T, request string) string { + t.Helper() + cmd := exec.Command("sh", "-c", strings.TrimPrefix(credentialHelper, "!")+" get") + cmd.Stdin = strings.NewReader(request) + cmd.Env = append(os.Environ(), "CRQ_GIT_TOKEN=ghp_secret_value") + out, err := cmd.Output() + if err != nil { + t.Fatalf("credential helper failed on %q: %v", request, err) + } + return string(out) + } + if got := ask(t, "protocol=https\nhost=github.com\n\n"); !strings.Contains(got, "password=ghp_secret_value") { + t.Errorf("helper answered github.com with %q, want the token", got) + } + for _, request := range []string{ + "protocol=https\nhost=evil.example\n\n", + "protocol=https\nhost=github.com.evil.example\n\n", + "protocol=http\nhost=github.com\n\n", + } { + if got := ask(t, request); strings.Contains(got, "ghp_secret_value") { + t.Errorf("helper leaked the token to %q: %q", request, got) + } + } +} + +// A session sitting idle — holding uncommitted fixes while it waits on a +// reviewer — touches no file for hours, and pruning by age alone would delete +// the checkout out from under it. A live handle keeps its own timestamp fresh. +func TestALiveCheckoutKeepsItselfFromBeingPruned(t *testing.T) { + dir := t.TempDir() + old := time.Now().Add(-2 * staleWorkAge) + if err := os.Chtimes(dir, old, old); err != nil { + t.Fatal(err) + } + if since := time.Since(newestModTime(dir)); since < staleWorkAge { + t.Fatalf("the checkout reads as %s old, wanted it stale to begin with", since) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go keepAlive(ctx, dir, time.Millisecond) + for deadline := time.Now().Add(2 * time.Second); time.Now().Before(deadline); { + if time.Since(newestModTime(dir)) < time.Minute { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Error("an idle checkout still read as abandoned while its process was alive") +} + +func TestCheckoutGitRefreshesRotatedCredentials(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + token := "ghp_first" + ws := Workspace{ + Root: t.TempDir(), + TokenSource: func(context.Context) string { + return token + }, + } + co, err := ws.Checkout(context.Background(), repo, 13, sha) + if err != nil { + t.Fatal(err) + } + + token = "ghp_rotated" + out, err := co.Git( + context.Background(), + "-c", `alias.current-token=!f() { printf '%s' "$CRQ_GIT_TOKEN"; }; f`, + "current-token", + ) + if err != nil { + t.Fatal(err) + } + if out != token { + t.Errorf("checkout used token %q after rotation, want %q", out, token) + } +} + +func TestMirrorTracksRemoteTagsWithoutChangingSessionTags(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + first := originRepo(t, origin) + ctx := context.Background() + if _, err := gitDir(ctx, origin, "tag", "deleted"); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, origin, "tag", "release"); err != nil { + t.Fatal(err) + } + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/crq/remotes/origin/tags/release"); err != nil || got != first { + t.Fatalf("tracked release tag = %q err=%v, want %q", got, err, first) + } + // These names now belong to sessions. A refresh must not infer ownership + // from whether an identically named tag exists on the remote. + for _, name := range []string{"deleted", "release"} { + if _, err := gitDir(ctx, mirror, "tag", name, first); err != nil { + t.Fatal(err) + } + } + + if _, err := gitDir(ctx, origin, "tag", "-d", "deleted"); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(origin, "README.md"), []byte("two\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "README.md"}, {"commit", "-m", "second"}} { + if _, err := gitDir(ctx, origin, args...); err != nil { + t.Fatal(err) + } + } + second, err := gitDir(ctx, origin, "rev-parse", "HEAD") + if err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, origin, "tag", "-f", "release", second); err != nil { + t.Fatal(err) + } + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/tags/deleted"); err != nil || got != first { + t.Errorf("session tag deleted = %q err=%v, want preserved at %q", got, err, first) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/tags/release"); err != nil || got != first { + t.Errorf("session release tag = %q err=%v, want preserved at %q", got, err, first) + } + if _, err := gitDir(ctx, mirror, "show-ref", "--verify", "--quiet", "refs/crq/remotes/origin/tags/deleted"); err == nil { + t.Error("deleted remote tag survived in the tracking namespace") + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/crq/remotes/origin/tags/release"); err != nil || got != second { + t.Errorf("tracked release tag = %q err=%v, want moved tag %q", got, err, second) + } +} + +func TestFetchedHeadDeletionRechecksWorktreeOccupancy(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + sha := originRepo(t, origin) + if _, err := gitDir(context.Background(), origin, "branch", "feature"); err != nil { + t.Fatal(err) + } + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "update-ref", "refs/heads/feature", sha); err != nil { + t.Fatal(err) + } + dir := filepath.Join(t.TempDir(), "work") + if _, err := gitDir(ctx, mirror, "worktree", "add", "--", dir, "feature"); err != nil { + t.Fatal(err) + } + + if err := deleteFetchedHead(ctx, mirror, "feature"); err != nil { + t.Fatalf("worktree-aware deletion rejected a branch a session attached: %v", err) + } + if got, err := gitDir(ctx, mirror, "rev-parse", "refs/heads/feature"); err != nil || got != sha { + t.Errorf("checked-out branch = %q err=%v, want it preserved at %q", got, err, sha) + } +} + +func TestFetchedHeadDeletionToleratesAConcurrentDeletion(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + if _, err := gitDir(ctx, "", "init", "--bare", "--quiet", repo); err != nil { + t.Fatal(err) + } + + if err := deleteFetchedHead(ctx, repo, "already-removed"); err != nil { + t.Fatalf("deleting an already-removed legacy branch: %v", err) + } +} + +// A checkout can install executable Git configuration in the shared mirror. +// Later worktree creation must not run that session's hook in the daemon. +func TestCheckoutDisablesSharedExecutableConfiguration(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + sha := originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + hooks := t.TempDir() + marker := filepath.Join(t.TempDir(), "post-checkout-ran") + hook := filepath.Join(hooks, "post-checkout") + if err := os.WriteFile(hook, []byte("#!/bin/sh\ntouch \""+marker+"\"\n"), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "config", "core.hooksPath", hooks); err != nil { + t.Fatal(err) + } + + if _, err := ws.Checkout(ctx, repo, 14, sha); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Errorf("shared post-checkout hook ran: %v", err) + } +} + +func TestTokenlessMirrorRefreshDisablesSharedHooks(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + origin := filepath.Join(base, repo) + originRepo(t, origin) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + hooks := t.TempDir() + marker := filepath.Join(t.TempDir(), "reference-transaction-ran") + hook := filepath.Join(hooks, "reference-transaction") + if err := os.WriteFile(hook, []byte("#!/bin/sh\ntouch \""+marker+"\"\n"), 0o700); err != nil { + t.Fatal(err) + } + if _, err := gitDir(ctx, mirror, "config", "core.hooksPath", hooks); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(origin, "README.md"), []byte("two\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "README.md"}, {"commit", "-m", "second"}} { + if _, err := gitDir(ctx, origin, args...); err != nil { + t.Fatal(err) + } + } + + if _, err := ws.Mirror(ctx, repo); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Errorf("shared reference-transaction hook ran during refresh: %v", err) + } +} + +func TestFreshHeartbeatAvoidsDeepPruningDecision(t *testing.T) { + dir := t.TempDir() + old := time.Now().Add(-2 * staleWorkAge) + file := filepath.Join(dir, "generated.bin") + if err := os.WriteFile(file, []byte("large tree stand-in"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(file, old, old); err != nil { + t.Fatal(err) + } + now := time.Now() + if err := os.Chtimes(dir, now, now); err != nil { + t.Fatal(err) + } + if !rootHeartbeatFresh(dir, now.Add(time.Second)) { + t.Fatal("fresh checkout root did not take the heartbeat fast path") + } +} + +// A backward clock correction can leave the live root heartbeat ahead of now +// while older files remain behind it. The future heartbeat is indeterminate, +// not evidence that the checkout is abandoned. +func TestFutureRootHeartbeatAvoidsDeepPruningDecision(t *testing.T) { + dir := t.TempDir() + old := time.Now().Add(-2 * staleWorkAge) + file := filepath.Join(dir, "generated.bin") + if err := os.WriteFile(file, []byte("old tree stand-in"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(file, old, old); err != nil { + t.Fatal(err) + } + now := time.Now() + future := now.Add(time.Hour) + if err := os.Chtimes(dir, future, future); err != nil { + t.Fatal(err) + } + if !rootHeartbeatFresh(dir, now) { + t.Fatal("future checkout heartbeat was discarded before the deep scan") + } +} + +// A worktree is made for somebody else to work in, and that somebody pushes with +// a plain `git push`. crq's own commands pass the credential helper with -c, +// which lasts one command, so the mirror has to carry it for the ones this +// package does not run. +func TestMirrorPersistsTheCredentialHelperForOtherCallers(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir(), Token: "ghp_secret_value"} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + got, err := gitDir(ctx, mirror, "config", "--local", "--get", "credential.helper") + if err != nil { + t.Fatalf("no credential helper persisted, so a session's own push has none: %v", err) + } + if got != credentialHelper { + t.Errorf("persisted credential.helper = %q, want the workspace helper", got) + } + // The snippet, never the secret: a mirror somebody else finds on disk must + // hand out nothing without TokenEnv set in the environment. + if strings.Contains(got, "ghp_secret_value") { + t.Error("the token itself was written into the mirror's config") + } +} + +// Two dispatches can both observe an old mirror without the helper, then race +// to install it. Losing config.lock is harmless when the winner wrote the same +// value, so the loser must retry and verify the resulting configuration. +func TestCredentialHelperMigrationSurvivesConfigLockContention(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + lock := filepath.Join(mirror, "config.lock") + if err := os.WriteFile(lock, nil, 0o600); err != nil { + t.Fatal(err) + } + winner := make(chan error, 1) + go func() { + time.Sleep(50 * time.Millisecond) + if err := os.Remove(lock); err != nil { + winner <- err + return + } + _, err := gitDir(ctx, mirror, "config", "--local", "--replace-all", "credential.helper", credentialHelper) + winner <- err + }() + + withToken := Workspace{Token: "ghp_secret_value"} + if err := withToken.persistCredentialHelper(ctx, mirror); err != nil { + t.Errorf("concurrent credential-helper migration failed: %v", err) + } + if err := <-winner; err != nil { + t.Fatalf("concurrent config writer: %v", err) + } +} + +// Without a token there is nothing to inject, and rewriting the host's own +// credential configuration would be crq overriding a choice that is not its own. +func TestMirrorLeavesTheHostsCredentialConfigurationAlone(t *testing.T) { + base := t.TempDir() + repo := "owner/thing" + originRepo(t, filepath.Join(base, repo)) + t.Setenv("CRQ_REMOTE_BASE", base) + ws := Workspace{Root: t.TempDir()} + ctx := context.Background() + + mirror, err := ws.Mirror(ctx, repo) + if err != nil { + t.Fatal(err) + } + if got, _ := gitDir(ctx, mirror, "config", "--local", "--get", "credential.helper"); got != "" { + t.Errorf("credential.helper = %q, want none written when crq has no token", got) + } +}