fix(repo): repo run survives a crash mid-grade and a parallel run on one cache

dokimi/changelog.d/20261113_120000_vlad-perevezentsev_repo-run-aside-and-clone-race.md

@@ -0,0 +1,18 @@
+### Fixed
+
+- **`repo run` keeps the agent's git config across a crash, and two runs can
+  clone the same source at once** — a run killed after it created
+  `{run_dir}/agent-git/` and before it moved the workspace's `.git/config`
+  and hooks there made the next grade of that cell delete them unarchived,
+  as a regrade does. The moves now go to a temporary directory renamed to
+  `agent-git/` after the last one, and the next grade finishes them. A
+  regrade of a workspace whose `.git` is a file no longer overwrites the
+  archived `agent-git/dot-git` with whatever a grader left there. Two
+  processes cloning one remote source into the repo cache
+  (`$DOKIMI_REPO_CACHE`) shared one `.partial` directory, so the second
+  deleted the first's clone or failed on the rename; each now clones into
+  its own directory, and the one that loses the rename uses the other's
+  clone. The same holds for the full clone that task bases are checked out
+  from. A branch or tag source fetched by two runs at once failed one of
+  them with `cannot lock ref` whenever the origin had moved; that fetch is
+  now run again, and finds the refs current.

dokimi/dokimi/repos/grading.py

@@ -261,27 +261,33 @@ def _linked_in_task(hidden: Path, rel: str) -> bool:
 def _set_git_config_aside(run_dir: Path) -> None:
     """Move the workspace's ``.git/config``, hooks and other config sources to ``agent-git/`` and
     write a plain config: a filter, diff driver, alias or include the agent set does not run in the
-    grader. The first move is kept, so a regrade does not overwrite the agent's original."""
+    grader. The first move is kept, so a regrade does not overwrite the agent's original: the moves go to
+    a directory renamed to ``agent-git/`` after the last one, and a grade after a crash in between finishes them."""
     dot_git = run_dir / "workspace" / ".git"
     aside = run_dir / AGENT_GIT
-    first = not aside.exists()
-    aside.mkdir(exist_ok=True)
-    if dot_git.is_symlink() or dot_git.is_file():  # the agent's own repository elsewhere: the grader gets none
-        shutil.move(str(dot_git), str(aside / "dot-git"))
-        return
+    if not aside.exists():
+        partial = run_dir / f".{AGENT_GIT}.partial"  # a leading dot: no image.env scratch entry can have the name
+        if partial.is_symlink() or not partial.is_dir():
+            _clear(partial)
+            partial.mkdir()
+        for name, path in _git_config_sources(dot_git):
+            path.rename(partial / name)  # atomic: workspace/ is in the run dir, on its file system
+        partial.rename(aside)
+    for _, path in _git_config_sources(dot_git):  # what a grader wrote since the first move
+        _clear(path)
+    if dot_git.is_dir() and not dot_git.is_symlink():
+        (dot_git / "config").write_text(_PLAIN_GIT_CONFIG, encoding="utf-8")
+
+
+def _git_config_sources(dot_git: Path) -> list[tuple[str, Path]]:
+    """The workspace's git config sources and their names in ``agent-git/``: a ``.git`` file or link (the
+    agent's own repository elsewhere: the grader gets none), else the config, hooks and pointers in ``.git/``."""
+    if dot_git.is_symlink() or dot_git.is_file():
+        return [("dot-git", dot_git)]
     if not dot_git.is_dir():
-        return
-    for name in ("config", "config.worktree", "commondir", "hooks"):
-        path = dot_git / name
-        if not (path.exists() or path.is_symlink()):
-            continue
-        if first:
-            shutil.move(str(path), str(aside / name))
-        elif path.is_dir() and not path.is_symlink():
-            shutil.rmtree(path)
-        else:
-            path.unlink()
-    (dot_git / "config").write_text(_PLAIN_GIT_CONFIG, encoding="utf-8")
+        return []
+    sources = [(name, dot_git / name) for name in ("config", "config.worktree", "commondir", "hooks")]
+    return [(name, path) for name, path in sources if path.exists() or path.is_symlink()]
 
 
 def check_grade(doc: Any) -> list[str]:

dokimi/dokimi/repos/source.py

@@ -19,6 +19,7 @@
 import re
 import shutil
 import sys
+import time
 from dataclasses import dataclass
 from pathlib import Path, PurePosixPath
 from typing import Callable, Protocol
@@ -34,6 +35,8 @@
 _SHA_RE = re.compile(r"^[0-9a-f]{7,40}$")
 _REMOTE_PREFIXES = ("https://", "ssh://", "file://", "git@")
 _GIT_TIMEOUT_S = 1800
+_FETCH_TRIES = 3  # a fetch that lost a ref to another run's fetch into the cache is run again
+_FETCH_ENV = process.Env.inherit({"GIT_TERMINAL_PROMPT": "0", "LC_ALL": "C"})  # untranslated: its error is matched
 READ_MANY_LIMIT = 2 * 1024 * 1024  # bytes; larger files are not text guidance or code to search
 
 
@@ -461,17 +464,53 @@ def isolated_git_env() -> dict[str, str]:
 def ensure_clone(url: str, cache_dir: Path) -> Path:
     """Clone ``url`` bare and blob-less into the cache once; return the git dir."""
     git_dir = cache_path(url, cache_dir)
+    return clone_once(git_dir, lambda tmp: _git(None, "clone", "--bare", "--filter=blob:none", "--", url, str(tmp)),
+                      _GIT_TIMEOUT_S)
+
+
+def clone_once(git_dir: Path, clone: Callable[[Path], object], timeout_s: float) -> Path:
+    """``git_dir``, made by ``clone(<tmp>)`` and a rename unless it has a ``HEAD``. Another process may clone the
+    same URL meanwhile: each clones into its own directory, and the one whose rename fails uses the other's.
+    ``timeout_s``: the clone's timeout; a partial directory older than that is no live clone's."""
     if (git_dir / "HEAD").is_file():
         return git_dir
-    cache_dir.mkdir(parents=True, exist_ok=True)
-    tmp = git_dir.with_name(git_dir.name + ".partial")
-    if tmp.exists():
-        shutil.rmtree(tmp)
-    _git(None, "clone", "--bare", "--filter=blob:none", "--", url, str(tmp))
-    tmp.rename(git_dir)
+    git_dir.parent.mkdir(parents=True, exist_ok=True)
+    for old in git_dir.parent.glob(f"{git_dir.name}.*partial"):
+        try:
+            if time.time() - old.stat().st_mtime > timeout_s:
+                _remove_tree(old)
+        except FileNotFoundError:  # another process removed it first
+            pass
+    tmp = git_dir.with_name(f"{git_dir.name}.{os.getpid()}-{os.urandom(3).hex()}.partial")
+    try:
+        clone(tmp)
+        try:
+            tmp.rename(git_dir)
+        except OSError:
+            if not (git_dir / "HEAD").is_file():
+                raise
+    finally:
+        _remove_tree(tmp)
     return git_dir
 
 
+def _remove_tree(path: Path) -> None:
+    """Delete ``path``, also when it holds read-only entries (Git's object files on Windows); raise when it is
+    still there: a partial clone left in the cache would stay for good."""
+    if not path.exists():
+        return
+    try:
+        shutil.rmtree(path)
+    except OSError:
+        for root, dirs, files in os.walk(path):
+            for name in [root, *(os.path.join(root, n) for n in (*dirs, *files))]:
+                try:
+                    os.chmod(name, 0o700)
+                except OSError:
+                    pass
+        shutil.rmtree(path)
+
+
 def ensure_rev(git_dir: Path, rev: str) -> str:
     """Resolve ``rev`` in the cached clone; a branch or tag is re-fetched on every call."""
     base = re.split(r"[\^~]", rev, maxsplit=1)[0]
@@ -495,8 +534,17 @@ def ensure_rev(git_dir: Path, rev: str) -> str:
 
 
 def _fetch_refs(git_dir: Path) -> None:
-    _git(git_dir, "fetch", "--filter=blob:none", "origin",
-         "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*")
+    """Fetch every branch and tag. Another run fetching into the same cache at once can take a ref's lock or move
+    it first; the fetch is run again then, and finds the refs current."""
+    for attempt in range(_FETCH_TRIES):
+        try:
+            _git(git_dir, "fetch", "--filter=blob:none", "origin",
+                 "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*", env=_FETCH_ENV)
+            return
+        except SourceError as exc:
+            if "cannot lock ref" not in str(exc) or attempt == _FETCH_TRIES - 1:
+                raise
+        time.sleep(0.5)
 
 
 def _has_location_path(head: str) -> bool:

dokimi/dokimi/repos/workspace.py

@@ -61,7 +61,8 @@
 from .manifest import RepoManifest, _is_within, match_glob
 from .overlay_paths import _link_target, _overlay_targets, _path_hits, _supplies_a_file  # noqa: F401
 from .overlays import overlay_names, overlay_of
-from .source import GitTree, RepoSource, RepoTree, SourceError, cache_path, default_cache_dir, isolated_git_env, resolve_rev
+from .source import (GitTree, RepoSource, RepoTree, SourceError, cache_path, clone_once, default_cache_dir,
+                     isolated_git_env, resolve_rev)
 from .uptake import MCP_CONFIG, base_servers, mcp_servers
 from .useful_spec import Useful, parse_useful
 from dokimi.treatments.specs import COMBINE, split_parts
@@ -282,12 +283,7 @@ def source_git_dir(source: RepoSource, tree: RepoTree, cache_dir: Path | None =
         out = _run(["git", "-C", str(root), "rev-parse", "--path-format=absolute", "--git-common-dir"])
         return Path(out.strip())
     git_dir = cache_path(source.location, cache_dir or default_cache_dir()).with_suffix(".full.git")
-    if not (git_dir / "HEAD").is_file():
-        git_dir.parent.mkdir(parents=True, exist_ok=True)
-        tmp = git_dir.with_name(git_dir.name + ".partial")
-        shutil.rmtree(tmp, ignore_errors=True)
-        _run(["git", "clone", "--bare", "--", source.location, str(tmp)])
-        tmp.rename(git_dir)
+    clone_once(git_dir, lambda tmp: _run(["git", "clone", "--bare", "--", source.location, str(tmp)]), _GIT_TIMEOUT_S)
     sha = getattr(tree, "sha", None)
     if sha and not _has_commit(git_dir, sha):
         _run(["git", "-C", str(git_dir), "fetch", "origin", "--", sha])

dokimi/tests/test_repo_grading_aside.py

@@ -0,0 +1,93 @@
+# Copyright (C) 2026 Intel Corporation
+# SPDX-License-Identifier: Apache-2.0
+
+"""The workspace's git config is set aside before grading once, and a crash between the moves does not
+lose it (#687): the next grade finishes the moves instead of taking them for a regrade's."""
+
+from __future__ import annotations
+
+import os
+from pathlib import Path
+
+import pytest
+
+from dokimi.repos.grading import AGENT_GIT, _set_git_config_aside
+
+
+class Crash(BaseException):
+    """The process dies: no ``except Exception`` on the way catches it."""
+
+
+AGENT_CONFIG = "[core]\n\tfsmonitor = ./agent-hook\n"
+SOURCES = {"config": AGENT_CONFIG, "config.worktree": "[alias]\n\tx = !agent\n", "commondir": "../agent\n"}
+
+
+def _agent_git(run_dir: Path) -> Path:
+    dot_git = run_dir / "workspace" / ".git"
+    (dot_git / "hooks").mkdir(parents=True)
+    (dot_git / "hooks" / "pre-commit").write_text("#!/bin/sh\necho agent\n", encoding="utf-8")
+    for name, text in SOURCES.items():
+        (dot_git / name).write_text(text, encoding="utf-8")
+    return dot_git
+
+
+def _archived(run_dir: Path) -> dict[str, str]:
+    aside = run_dir / AGENT_GIT
+    out = {p.name: p.read_text(encoding="utf-8") for p in aside.iterdir() if p.is_file()}
+    out["hooks/pre-commit"] = (aside / "hooks" / "pre-commit").read_text(encoding="utf-8")
+    return out
+
+
+@pytest.mark.parametrize("moves_before_crash", [0, 1, 3])
+def test_a_crash_between_the_moves_loses_nothing_on_the_next_grade(tmp_path, monkeypatch, moves_before_crash):
+    dot_git = _agent_git(tmp_path)
+    real, done = os.rename, []
+
+    def crash_after_some(src, dst):
+        if Path(dst).parent.name == f".{AGENT_GIT}.partial":
+            if len(done) == moves_before_crash:
+                raise Crash
+            done.append(src)
+        return real(src, dst)
+
+    monkeypatch.setattr(os, "rename", crash_after_some)
+    with pytest.raises(Crash):
+        _set_git_config_aside(tmp_path)
+    monkeypatch.setattr(os, "rename", real)
+    _set_git_config_aside(tmp_path)
+    assert _archived(tmp_path) == {**SOURCES, "hooks/pre-commit": "#!/bin/sh\necho agent\n"}
+    assert sorted(p.name for p in dot_git.iterdir()) == ["config"]
+    assert "fsmonitor" not in (dot_git / "config").read_text(encoding="utf-8")
+    assert sorted(p.name for p in tmp_path.iterdir()) == [AGENT_GIT, "workspace"]  # no partial archive left
+
+
+def test_a_regrade_keeps_the_first_archive_and_drops_what_the_grader_wrote(tmp_path):
+    dot_git = _agent_git(tmp_path)
+    _set_git_config_aside(tmp_path)
+    (dot_git / "config").write_text("[core]\n\tfsmonitor = ./grader\n", encoding="utf-8")
+    (dot_git / "hooks").mkdir()
+    _set_git_config_aside(tmp_path)
+    assert _archived(tmp_path)["config"] == AGENT_CONFIG
+    assert sorted(p.name for p in dot_git.iterdir()) == ["config"]
+    assert "fsmonitor" not in (dot_git / "config").read_text(encoding="utf-8")
+
+
+def test_a_regrade_keeps_the_first_git_file_and_removes_a_later_one(tmp_path):
+    workspace = tmp_path / "workspace"
+    workspace.mkdir()
+    (workspace / ".git").write_text("gitdir: /agent/repo\n", encoding="utf-8")
+    _set_git_config_aside(tmp_path)
+    (workspace / ".git").write_text("gitdir: /grader/repo\n", encoding="utf-8")
+    _set_git_config_aside(tmp_path)
+    assert (tmp_path / AGENT_GIT / "dot-git").read_text(encoding="utf-8") == "gitdir: /agent/repo\n"
+    assert not (workspace / ".git").exists()
+
+
+def test_a_link_in_place_of_the_partial_archive_is_not_moved_into(tmp_path):
+    run_dir, outside = tmp_path / "rd", tmp_path / "outside"
+    outside.mkdir()
+    _agent_git(run_dir)
+    (run_dir / f".{AGENT_GIT}.partial").symlink_to(outside, target_is_directory=True)  # a grader's code left it
+    _set_git_config_aside(run_dir)
+    assert not list(outside.iterdir())
+    assert not (run_dir / AGENT_GIT).is_symlink() and _archived(run_dir)["config"] == AGENT_CONFIG