From c21791a0f65944dd2ec7468fb1e61ccb7ae030dc Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 18:09:43 +0000 Subject: [PATCH 1/8] feat(repo): record public_at and guidance:rev history for leak checks Settles the recording half of #290 ("repo compare: flag replay tasks a guidance revision may have seen"), as decided on the issue: committer date, not author date; the field is public_at; the per-file latest commit is uncapped and the newest-first list is capped. - repo tasks replay writes public_at into each task.json: the earliest createdAt of a gold thread's first comment, compared as times, as the threads file gives it. Threads the task leaves out do not count; with no timed gold thread the key is absent. - A guidance:rev: arm's meta.json arm_rev adds guidance_latest (the commit reachable from that last touched each guidance file at ), guidance_commits (the commits touching a path the guidance globs match, newest first by committer date, at most 200) and guidance_commits_truncated, each commit with %aI and %cI dates. One git log per (git dir, revision, globs) in a process, in the new repos/guidance_history.py. The keys are records only: meta.json feeds neither the cell name, the execution id nor the batch's reuse and replace checks, so an existing batch resumes as before. repo compare is unchanged; its flags come in a later change. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 11 ++ dokimi/docs/repo-evaluation-contract.md | 21 ++- dokimi/dokimi/repos/guidance_history.py | 87 +++++++++++ dokimi/dokimi/repos/replay/build.py | 23 ++- dokimi/dokimi/repos/workspace.py | 4 +- dokimi/tests/test_repo_arms.py | 3 +- dokimi/tests/test_repo_guidance_history.py | 137 ++++++++++++++++++ dokimi/tests/test_repo_replay.py | 26 ++++ 8 files changed, 305 insertions(+), 7 deletions(-) create mode 100644 dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md create mode 100644 dokimi/dokimi/repos/guidance_history.py create mode 100644 dokimi/tests/test_repo_guidance_history.py diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md new file mode 100644 index 000000000..3a8ddee58 --- /dev/null +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -0,0 +1,11 @@ +### Added + +- **Replay tasks and `guidance:rev:` arms record what a leak check needs** — + `repo tasks replay` writes `public_at` into each `task.json`: the earliest + `createdAt` of a gold thread's first comment. A `guidance:rev:` cell's + `meta.json` `arm_rev` adds `guidance_latest` (the last commit to touch each + guidance file at ``), `guidance_commits` (the commits that touch a + guidance path, newest first by committer date, at most 200) and + `guidance_commits_truncated`, each commit with its author and committer + date, so a reader can see whether a guidance commit came after a task's + review was public. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index c2b873922..dd83d5965 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -363,7 +363,18 @@ commit and is not in the workspace, because step 3 starts a new history; a grader diffs the agent's changes from `start_sha`. With the arm come `arm_files`, the files it deleted, wrote or changed; for `guidance:rev:` an `arm_rev` object (`rev` as given, its `sha`, and `removed`: the base's guidance -files with no counterpart at ``); for `patch:` an `arm_patch` object (the +files with no counterpart at ``), which also records the guidance history +of `` (`dokimi/repos/guidance_history.py`): `guidance_latest`, for +each guidance file the arm wrote, the commit reachable from `` that last +touched it by committer date; `guidance_commits`, the commits reachable from +`` that touch a path the guidance globs match, newest first by committer +date, at most 200, each with its `subject`; and `guidance_commits_truncated`, +`true` when that cap cut the list. Each commit has its `sha`, `authored` and +`committed` (strict ISO 8601, `%aI` and `%cI`); a merge commit lists no files +and is not counted. A reader can compare `committed` with a replay task's +`public_at` (§4.6) to see whether a guidance commit came after the task's +answer was public; the author date alone cannot show it, as an amend or a +rebase keeps it. For `patch:` an `arm_patch` object (the file's absolute `path` and full `sha256`, `added`: the files the patch added that neither the guidance globs nor `.mcp.json` match, and `mcp_servers`: the servers the patch added to the `.mcp.json` at the root, read before the agent runs). `args_if_present` lists the profile's @@ -731,8 +742,12 @@ by a reviewer: it has no line and is never gold. `score: {metric: recall, max: 1.0}`, `review_patch`, `gold`, `window` (`--window`, default 5) and the prompt (`--prompt-file` replaces the default, which asks for the findings JSON of §4.5). For the reader it adds - `source_pr`, `source_url`, `reviewed_commit`, `merge_commit` and - `left_out_files`. + `source_pr`, `source_url`, `reviewed_commit`, `merge_commit`, + `left_out_files` and `public_at`: the earliest `createdAt` of a gold + thread's first comment, as the threads file gives it, the time the task's + answer was public (absent when no gold thread has a time). Against the + `committed` dates a `guidance:rev:` arm records (§3), it shows whether a + guidance commit came after it. - `gold.json` holds `{id, file, line, kind, comment, addressed, reply, url}` per thread. `id` is `pr-t`, `n` being the thread's position among all the PR's threads, so ids stay the same when a thread is dropped. `kind` diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py new file mode 100644 index 000000000..0070275b0 --- /dev/null +++ b/dokimi/dokimi/repos/guidance_history.py @@ -0,0 +1,87 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""The guidance history of a ``guidance:rev:`` arm, for its ``meta.json`` ``arm_rev`` (contract §3). + +A replay task's answer is public from its ``public_at`` (§4.6); a guidance commit made after it may +encode that review. :func:`arm_rev_history` records what a reader needs to check that: the commit +that last touched each guidance file at the revision, and the newest commits that touch a guidance +path, each with its author and committer date. The flag reads the committer date: ``git commit +--amend`` and a rebase keep the author date (https://github.com/intel/dokimi/issues/290). + +The history is one ``git log`` over the revision, filtered with :func:`~.manifest.match_glob`, and +read once per git dir, revision and globs in a process: every cell of the arm asks the same. +``git log`` lists no files for a merge commit, so a merge is never listed: the commits it merged are. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from datetime import datetime +from functools import lru_cache +from pathlib import Path +from typing import Any + +from dokimi import process + +from .errors import PrepError +from .manifest import match_glob + +#: At most this many commits in ``guidance_commits``; ``guidance_latest`` is not capped +GUIDANCE_COMMITS_CAP = 200 +_GIT_TIMEOUT_S = 1800.0 +_FORMAT = "--format=%x1e%H%x1f%aI%x1f%cI%x1f%s" # subject last: a \x1f in it stays in it + + +@dataclass(frozen=True) +class _Commit: + sha: str + authored: str + committed: str + subject: str + paths: tuple[str, ...] # the guidance paths it touched + + def dates(self) -> dict[str, str]: + return {"sha": self.sha, "authored": self.authored, "committed": self.committed} + + +def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list[str]) -> dict[str, Any]: + """``guidance_latest`` for each of ``files`` (the guidance at ``sha``), ``guidance_commits`` (newest + first by committer date, at most :data:`GUIDANCE_COMMITS_CAP`) and ``guidance_commits_truncated``.""" + commits = _guidance_commits(git_dir, sha, globs) + latest: dict[str, dict[str, str]] = {} + for c in commits: # newest first: the first commit naming a file is its latest + for path in c.paths: + latest.setdefault(path, c.dates()) + return {"guidance_latest": {p: latest[p] for p in sorted(files) if p in latest}, + "guidance_commits": [{**c.dates(), "subject": c.subject} for c in commits[:GUIDANCE_COMMITS_CAP]], + "guidance_commits_truncated": len(commits) > GUIDANCE_COMMITS_CAP} + + +@lru_cache(maxsize=64) +def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...]) -> tuple[_Commit, ...]: + """The commits reachable from ``sha`` that touch a path ``globs`` match, newest first by committer date.""" + found = [] + for record in _git_log(git_dir, sha).split("\x1e")[1:]: + header, _, names = record.partition("\0") + commit, authored, committed, subject = header.split("\x1f", 3) + paths = tuple(p for p in names.lstrip("\n").split("\0") if p and any(match_glob(g, p) for g in globs)) + if paths: + found.append(_Commit(commit, authored, committed, subject, paths)) + # stable: commits with one committer date keep git's order + return tuple(sorted(found, key=lambda c: datetime.fromisoformat(c.committed), reverse=True)) + + +def _git_log(git_dir: Path, sha: str) -> str: + """Every commit reachable from ``sha`` with its dates, subject and the paths it touched.""" + keep = ("PATH", "LANG", "TMPDIR", "SYSTEMROOT") + env = process.Env.only(keep, {"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "HOME": os.devnull, + "GIT_TERMINAL_PROMPT": "0"}) + done = process.git("-C", str(git_dir), "log", "-z", "--no-renames", "--name-only", _FORMAT, sha, "--", + timeout=_GIT_TIMEOUT_S, env=env) + if done.timed_out: + raise PrepError(f"git log {sha[:12]}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") + if done.returncode: + raise PrepError(f"git log {sha[:12]} exited {done.returncode}: {process.git_failure(done)}") + return done.stdout diff --git a/dokimi/dokimi/repos/replay/build.py b/dokimi/dokimi/repos/replay/build.py index 69477ccb1..532c45c09 100644 --- a/dokimi/dokimi/repos/replay/build.py +++ b/dokimi/dokimi/repos/replay/build.py @@ -24,6 +24,7 @@ import json from collections import Counter from dataclasses import dataclass, field +from datetime import datetime from pathlib import Path from typing import Any @@ -118,7 +119,8 @@ def _make(h: History, manifest: RepoManifest, pr: PullRequest, opts: Options, gold = _gold(h, manifest, pr, threads, rev, opts.window, notes) if not gold: raise _Skip("no gold thread is left") - return _task(pr, base, rev, leave_out, opts), gold, _mbox(pr, rev, h.patch(base, rev, leave_out)) + task = _task(pr, base, rev, leave_out, opts, _public_at(threads, {g["id"] for g in gold})) + return task, gold, _mbox(pr, rev, h.patch(base, rev, leave_out)) def _needed(pr: PullRequest) -> list[str]: @@ -204,7 +206,22 @@ def _where(t: Thread) -> str: return f"{t.path}:{t.line}" if t.line else (t.path or "no path") -def _task(pr: PullRequest, base: str, rev: str, leave_out: list[str], opts: Options) -> dict[str, Any]: +def _public_at(threads: list[Thread], gold: set[str]) -> str | None: + """The earliest first-comment ``createdAt`` of the threads whose id is in ``gold``, as the input gives it; + a time that is not ISO 8601 with an offset (GitHub writes ``Z``) counts as none. ``None`` when none has one.""" + times = [] + for t in (t for t in threads if t.id in gold): + try: + at = datetime.fromisoformat(t.created) + except ValueError: + continue + if at.tzinfo is not None: # a time with no offset is not comparable with one that has it + times.append((at, t.created)) + return min(times)[1] if times else None + + +def _task(pr: PullRequest, base: str, rev: str, leave_out: list[str], opts: Options, + public: str | None) -> dict[str, Any]: task: dict[str, Any] = { "id": task_id(pr), "family": "review", "base": base, "grader": "dokimi:review_replay", "score": {"metric": "recall", "max": 1.0}, "review_patch": PATCH, "gold": GOLD, "window": opts.window, @@ -214,6 +231,8 @@ def _task(pr: PullRequest, base: str, rev: str, leave_out: list[str], opts: Opti task["source_url"] = pr.url if leave_out: task["left_out_files"] = leave_out + if public: + task["public_at"] = public # when the gold's answer was public (contract §4.6) return task diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index b5e0dba74..82ba9008f 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -51,6 +51,7 @@ from .arms import ArmError, Skill, resolve_skills, tree_sha256 from .errors import PrepError +from .guidance_history import arm_rev_history from .harness_profile import HarnessProfile from .manifest import RepoManifest, _is_within, match_glob from .source import GitTree, RepoSource, RepoTree, SourceError, cache_path, default_cache_dir, resolve_rev @@ -504,8 +505,9 @@ def _apply_rev(ws: Path, git_dir: Path, manifest: RepoManifest, arm: Arm, if mode == "100755": dest_path.chmod(dest_path.stat().st_mode | 0o111) # arm_files: the files it wrote and the base's guidance it deleted (contract §3 meta.json) + history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written) return sorted(set(written) | set(removed)), {"arm_rev": {"rev": arm.rev, "sha": arm.rev_sha, - "removed": sorted(set(removed) - set(written))}} + "removed": sorted(set(removed) - set(written)), **history}} def _apply_bridge(ws: Path, matched: list[str], bridge: dict[str, str]) -> tuple[list[str], dict[str, Any]]: diff --git a/dokimi/tests/test_repo_arms.py b/dokimi/tests/test_repo_arms.py index ef18e3314..a3ba4f662 100644 --- a/dokimi/tests/test_repo_arms.py +++ b/dokimi/tests/test_repo_arms.py @@ -116,7 +116,8 @@ def _commit_objects(root: Path, sha: str) -> set[str]: def test_rev_arm_puts_another_revisions_guidance_on_the_base(toy): meta, ws = _prep(toy, "guidance:rev:pr") assert meta["arm"] == f"guidance:rev:{toy['pr']}" - assert meta["arm_rev"] == {"rev": "pr", "sha": toy["pr"], "removed": ["sub/AGENTS.md"]} + assert {k: meta["arm_rev"][k] for k in ("rev", "sha", "removed")} == { + "rev": "pr", "sha": toy["pr"], "removed": ["sub/AGENTS.md"]} # written and deleted (contract §3): the deleted base guidance is in arm_files too, not the eval dir's assert meta["arm_files"] == ["AGENTS.md", "src/AGENTS.md", "sub/AGENTS.md"] assert (ws / "AGENTS.md").read_text() == "# pr guidance\n" diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py new file mode 100644 index 000000000..91d0f02f8 --- /dev/null +++ b/dokimi/tests/test_repo_guidance_history.py @@ -0,0 +1,137 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""The guidance history a ``guidance:rev:`` arm records in ``meta.json`` ``arm_rev`` (contract §3). + +Each commit of the toy history gets its own author and committer date, so the order by committer +date is not the order git walks in. No docker, no LLM. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +import dokimi.repos.guidance_history as guidance_history +from dokimi.repos import RepoSource, load_manifest, open_source +from dokimi.repos.workspace import export_eval_dir, load_tasks, prepare_workspace, source_git_dir +from repo_eval_fixture import git, make_repo, write + +TOOLING_PATCH = """diff --git a/Makefile b/Makefile +new file mode 100644 +--- /dev/null ++++ b/Makefile +@@ -0,0 +1 @@ ++check: +""" + + +def _commit_at(root: Path, files: dict[str, str], subject: str, authored: str, committed: str) -> str: + for rel, text in files.items(): + write(root, rel, text) + git(root, "add", "-A") + env = {**os.environ, "GIT_AUTHOR_DATE": authored, "GIT_COMMITTER_DATE": committed} + subprocess.run(["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", subject], + check=True, capture_output=True, env=env) + return git(root, "rev-parse", "HEAD") + + +@pytest.fixture +def hist(tmp_path): + """The toy repository's two commits, then: ``amended`` edits AGENTS.md (authored long before it was + committed), ``code`` only src/calc.py, ``skewed`` sub/AGENTS.md with a committer date before its + parent's, and ``added`` adds src/AGENTS.md. The arm's revision is ``added``.""" + root, _ = make_repo(tmp_path) + shas = { + "amended": _commit_at(root, {"AGENTS.md": "# guidance, from review\n"}, "Guidance for the fix in #12", + "2030-01-01T00:00:00+00:00", "2030-03-10T00:00:00+00:00"), + "code": _commit_at(root, {"src/calc.py": "def add(a, b):\n return a + b # code\n"}, "code only", + "2030-03-11T00:00:00+00:00", "2030-03-11T00:00:00+00:00"), + "skewed": _commit_at(root, {"sub/AGENTS.md": "# sub guidance 2\n"}, "sub guidance", + "2030-03-01T00:00:00+00:00", "2030-03-01T00:00:00+00:00"), + "added": _commit_at(root, {"src/AGENTS.md": "# src guidance\n"}, "src guidance", + "2030-04-01T00:00:00+02:00", "2030-04-01T00:00:00+02:00"), + } + first = git(root, "rev-list", "--max-parents=0", "HEAD") # make_repo's code commit: AGENTS.md, sub/AGENTS.md + source = RepoSource.parse(f"{root}@HEAD") + tree = open_source(source) + manifest = load_manifest(tree, ".agent-eval/repo-eval.yaml") + tasks = {t.id: t for t in load_tasks(export_eval_dir(tree, manifest, tmp_path / "content"), manifest)} + return {**shas, "first": first, "root": root, "manifest": manifest, "tasks": tasks, + "git_dir": source_git_dir(source, tree), "tmp": tmp_path} + + +def _prep(h, arm, name="rd"): + return prepare_workspace(h["git_dir"], h["manifest"], h["tasks"]["fix_add"], arm, h["tmp"] / name) + + +def _dates(h, name, authored, committed): + return {"sha": h[name], "authored": authored, "committed": committed} + + +def test_a_rev_arm_records_the_latest_commit_of_each_guidance_file_with_both_dates(hist): + rev = _prep(hist, f"guidance:rev:{hist['added']}")["arm_rev"] + assert rev["guidance_latest"] == { + "AGENTS.md": _dates(hist, "amended", "2030-01-01T00:00:00Z", "2030-03-10T00:00:00Z"), # git's %aI, %cI + "src/AGENTS.md": _dates(hist, "added", "2030-04-01T00:00:00+02:00", "2030-04-01T00:00:00+02:00"), + "sub/AGENTS.md": _dates(hist, "skewed", "2030-03-01T00:00:00Z", "2030-03-01T00:00:00Z"), + } + assert rev["rev"] == rev["sha"] == hist["added"] and rev["removed"] == [] + + +def test_a_files_latest_commit_is_chosen_by_committer_date_not_author_date(hist): + backdated = _commit_at(hist["root"], {"sub/AGENTS.md": "# sub guidance 3\n"}, "newer author date", + "2031-01-01T00:00:00+00:00", "2030-02-01T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{backdated}")["arm_rev"] + assert rev["guidance_latest"]["sub/AGENTS.md"]["sha"] == hist["skewed"] # committed 2030-03-01 + assert [c["sha"] for c in rev["guidance_commits"]][2:4] == [hist["skewed"], backdated] + + +def test_guidance_commits_are_newest_first_by_committer_date_without_code_only_commits(hist): + rev = _prep(hist, f"guidance:rev:{hist['added']}")["arm_rev"] + # skewed descends from amended, so git lists it first; its committer date is older + assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]] == [ + (hist["added"], "src guidance"), (hist["amended"], "Guidance for the fix in #12"), + (hist["skewed"], "sub guidance"), (hist["first"], "code")] + assert rev["guidance_commits"][1]["authored"] == "2030-01-01T00:00:00Z" + assert rev["guidance_commits_truncated"] is False + + +def test_the_guidance_commits_cap_cuts_the_oldest_and_says_so(hist, monkeypatch): + monkeypatch.setattr(guidance_history, "GUIDANCE_COMMITS_CAP", 2) + rev = _prep(hist, f"guidance:rev:{hist['added']}")["arm_rev"] + assert [c["sha"] for c in rev["guidance_commits"]] == [hist["added"], hist["amended"]] + assert rev["guidance_commits_truncated"] is True + assert set(rev["guidance_latest"]) == {"AGENTS.md", "src/AGENTS.md", "sub/AGENTS.md"} # not capped + monkeypatch.setattr(guidance_history, "GUIDANCE_COMMITS_CAP", 4) # all four fit: nothing was cut + rev = _prep(hist, f"guidance:rev:{hist['added']}", "at-cap")["arm_rev"] + assert len(rev["guidance_commits"]) == 4 and rev["guidance_commits_truncated"] is False + + +def test_an_older_revision_records_only_its_own_history(hist): + rev = _prep(hist, f"guidance:rev:{hist['amended']}")["arm_rev"] + assert [c["sha"] for c in rev["guidance_commits"]] == [hist["amended"], hist["first"]] + assert sorted(rev["guidance_latest"]) == ["AGENTS.md", "sub/AGENTS.md"] + assert rev["guidance_latest"]["sub/AGENTS.md"]["sha"] == hist["first"] + + +def test_raw_and_patch_arms_record_no_guidance_history(hist): + patch = hist["tmp"] / "tooling.diff" + patch.write_text(TOOLING_PATCH) + for arm, name in (("guidance:raw", "raw"), (f"patch:{patch}", "patch")): + meta = _prep(hist, arm, name) + assert "arm_rev" not in meta and not any(k.startswith("guidance_") for k in meta), arm + assert _prep(hist, "guidance:rev:HEAD", "rev")["arm_rev"]["guidance_commits"] # the rev arm does + + +def test_the_history_is_read_once_for_every_cell_of_the_arm(hist, monkeypatch): + calls = [] + real = guidance_history._git_log + monkeypatch.setattr(guidance_history, "_git_log", lambda *a: calls.append(a) or real(*a)) + first = _prep(hist, f"guidance:rev:{hist['added']}", "c1")["arm_rev"] + second = _prep(hist, f"guidance:rev:{hist['added']}", "c2")["arm_rev"] + assert calls == [(hist["git_dir"], hist["added"])] + assert first == second and len(first["guidance_commits"]) == 4 diff --git a/dokimi/tests/test_repo_replay.py b/dokimi/tests/test_repo_replay.py index c21ddc9d5..247dd12fb 100644 --- a/dokimi/tests/test_repo_replay.py +++ b/dokimi/tests/test_repo_replay.py @@ -254,6 +254,32 @@ def test_a_merged_pr_becomes_a_task_with_gold_from_the_reviewers_line_threads(hi assert "1 task(s) written, 0 kept, 0 pull request(s) skipped" in printed +def _timed_pr7(h, t1, t2): + """PR 7 with two gold threads opened at ``t1`` and ``t2``, and two left out with earlier times.""" + rev = h["rev"] + return _pr(7, h["r2"], h["merge7"], [ + _t("src/long.py", 10, rev, created=t1), _t("src/long.py", 22, rev, created=t2), + _t("AGENTS.md", 2, rev, created="2026-08-01T00:00:00Z"), # left out by _gold: a guidance file + _t("src/long.py", 3, rev, "copilot-pull-request-reviewer", typename="Bot", created="2026-07-01T00:00:00Z"), + ]) + + +def test_public_at_is_the_earliest_gold_threads_first_comment_compared_as_times(hist, tmp_path): + out = tmp_path / "tasks" + # 20:00+09:00 is 11:00Z, before 12:00Z, though it sorts after it as a string + _replay(hist["root"], _pages(_timed_pr7(hist, "2026-10-02T12:00:00Z", "2026-10-02T20:00:00+09:00")), out) + task, gold, _ = _load(out, 7) + assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] + assert task["public_at"] == "2026-10-02T20:00:00+09:00" + + +def test_a_task_whose_gold_threads_have_no_time_has_no_public_at(hist, tmp_path): + out = tmp_path / "tasks" + _replay(hist["root"], _pages(_timed_pr7(hist, None, "yesterday")), out) + task, gold, _ = _load(out, 7) + assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] and "public_at" not in task + + def test_addressed_counts_only_the_prs_own_commits_not_main_merged_in(hist, tmp_path): """t2: main changed line 21 and the PR merged main in. The diff from the reviewed revision to the merge commit has that change next to the thread; the PR's own commits do not.""" From d35b58179c0f19e4b5a148bee8b8dc8d29362f9b Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 19:52:36 +0000 Subject: [PATCH 2/8] fix(repo): NUL-framed guidance history; count merges and link targets Settles the review of #359 on the guidance history a guidance:rev: arm records in meta.json arm_rev (intel/dokimi#290, first half). - The log was split on \x1e and \x1f, which a subject or a path may hold: one such commit anywhere in the history raised ValueError and aborted the arm's workspace preparation. It is now read with -z --raw, where every field ends with a NUL and each file is a ':'-led status record followed by its one path, so a path is never read as a header nor a header as a path. Git cuts a subject at a NUL; a header that does not start with a commit id is a PrepError. - lru_cache did not coalesce concurrent misses, so cells of one arm that repo run's thread pool prepared together each ran the full-history log. A lock per key now lets one thread read while the others wait for its answer; different keys still read in parallel. - A merge listed no files, so a guidance edit made while resolving a merge was in neither output. With --diff-merges=dense-combined a merge counts for a path its result changes against every parent (a conflict resolution, an edit made in the merge, or a clean merge of edits to different parts of one file); a merge that takes a file from one parent still does not. - The arm installs a guidance link as its target's bytes, but only paths the globs match were counted. The links now reach arm_rev_history and are in its cache key: a commit touching a link's target counts for the link's path. The contract (section 3) and the changelog fragment say what is counted. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 11 +- dokimi/docs/repo-evaluation-contract.md | 12 +- dokimi/dokimi/repos/guidance_history.py | 97 +++++++++++--- dokimi/dokimi/repos/workspace.py | 2 +- dokimi/tests/test_repo_guidance_history.py | 124 ++++++++++++++++++ 5 files changed, 219 insertions(+), 27 deletions(-) diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md index 3a8ddee58..65440d4db 100644 --- a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -4,8 +4,9 @@ `repo tasks replay` writes `public_at` into each `task.json`: the earliest `createdAt` of a gold thread's first comment. A `guidance:rev:` cell's `meta.json` `arm_rev` adds `guidance_latest` (the last commit to touch each - guidance file at ``), `guidance_commits` (the commits that touch a - guidance path, newest first by committer date, at most 200) and - `guidance_commits_truncated`, each commit with its author and committer - date, so a reader can see whether a guidance commit came after a task's - review was public. + guidance file at ``, or the file a guidance link resolves to), + `guidance_commits` (the commits that touch a guidance path or a link's + target, a merge when its own result changes one, newest first by committer + date, at most 200) and `guidance_commits_truncated`, each commit with its + author and committer date, so a reader can see whether a guidance commit + came after a task's review was public. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index dd83d5965..14ecf05fd 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -366,12 +366,16 @@ grader diffs the agent's changes from `start_sha`. With the arm come files with no counterpart at ``), which also records the guidance history of `` (`dokimi/repos/guidance_history.py`): `guidance_latest`, for each guidance file the arm wrote, the commit reachable from `` that last -touched it by committer date; `guidance_commits`, the commits reachable from -`` that touch a path the guidance globs match, newest first by committer +touched it, or for a link the file it resolves to, by committer date; +`guidance_commits`, the commits reachable from `` that touch a path the +guidance globs match or a guidance link's target, newest first by committer date, at most 200, each with its `subject`; and `guidance_commits_truncated`, `true` when that cap cut the list. Each commit has its `sha`, `authored` and -`committed` (strict ISO 8601, `%aI` and `%cI`); a merge commit lists no files -and is not counted. A reader can compare `committed` with a replay task's +`committed` (strict ISO 8601, `%aI` and `%cI`). A merge commit counts for a +path when its result differs there from every parent: a conflict resolution, +an edit made in the merge, or a clean merge of its parents' edits to +different parts of one file; a merge that takes the path as one parent has +it does not, and the commits it merged count. A reader can compare `committed` with a replay task's `public_at` (§4.6) to see whether a guidance commit came after the task's answer was public; the author date alone cannot show it, as an amend or a rebase keeps it. For `patch:` an `arm_patch` object (the diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index 0070275b0..709164f09 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -9,14 +9,28 @@ path, each with its author and committer date. The flag reads the committer date: ``git commit --amend`` and a rebase keep the author date (https://github.com/intel/dokimi/issues/290). -The history is one ``git log`` over the revision, filtered with :func:`~.manifest.match_glob`, and -read once per git dir, revision and globs in a process: every cell of the arm asks the same. -``git log`` lists no files for a merge commit, so a merge is never listed: the commits it merged are. +The history is one ``git log -z --raw`` over the revision, filtered with :func:`~.manifest.match_glob`. +``-z`` ends every field with a NUL, and each file the commit touched is a ``:``-led status record +followed by its one path, so a subject or a path, whatever it holds, cannot pass for the other. +Git cuts a subject at a NUL (``git commit`` refuses one), so a subject is one field. The history is +read once per git dir, revision, globs and links in a process, also when cells of the arm prepare at +once: every cell asks the same. + +The arm installs a guidance link (``AGENTS.md -> docs/GUIDE.md``) as its target's bytes, so a +commit that touches the target counts for the link's path, whether or not a glob matches the target. + +A merge counts when its own result changes a guidance path against every parent +(``--diff-merges=dense-combined``): a conflict resolution, an edit made in the merge, or a clean +merge that joins its parents' edits to different parts of one file. A merge that takes a file as +one parent has it does not: the commits it merged count. """ from __future__ import annotations import os +import re +import threading +from collections.abc import Iterator from dataclasses import dataclass from datetime import datetime from functools import lru_cache @@ -31,7 +45,10 @@ #: At most this many commits in ``guidance_commits``; ``guidance_latest`` is not capped GUIDANCE_COMMITS_CAP = 200 _GIT_TIMEOUT_S = 1800.0 -_FORMAT = "--format=%x1e%H%x1f%aI%x1f%cI%x1f%s" # subject last: a \x1f in it stays in it +_FORMAT = "--format=%H%x00%aI%x00%cI%x00%s" # with -z, a NUL ends the subject too +_COMMIT_ID = re.compile(r"[0-9a-f]{40}(?:[0-9a-f]{24})?") # SHA-1 or SHA-256 + +_Links = tuple[tuple[str, str], ...] # (link path, the file it resolves to), sorted @dataclass(frozen=True) @@ -46,10 +63,12 @@ def dates(self) -> dict[str, str]: return {"sha": self.sha, "authored": self.authored, "committed": self.committed} -def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list[str]) -> dict[str, Any]: - """``guidance_latest`` for each of ``files`` (the guidance at ``sha``), ``guidance_commits`` (newest - first by committer date, at most :data:`GUIDANCE_COMMITS_CAP`) and ``guidance_commits_truncated``.""" - commits = _guidance_commits(git_dir, sha, globs) +def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list[str], + links: dict[str, str]) -> dict[str, Any]: + """``guidance_latest`` for each of ``files`` (the guidance at ``sha``; ``links`` maps those that + are links to their targets), ``guidance_commits`` (newest first by committer date, at most + :data:`GUIDANCE_COMMITS_CAP`) and ``guidance_commits_truncated``.""" + commits = _guidance_commits(git_dir, sha, globs, tuple(sorted(links.items()))) latest: dict[str, dict[str, str]] = {} for c in commits: # newest first: the first commit naming a file is its latest for path in c.paths: @@ -59,27 +78,71 @@ def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list "guidance_commits_truncated": len(commits) > GUIDANCE_COMMITS_CAP} +_key_locks: dict[tuple[Any, ...], threading.Lock] = {} +_key_locks_guard = threading.Lock() + + +def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _Links) -> tuple[_Commit, ...]: + """:func:`_read_guidance_commits`, one thread per key at a time: a cell that asks while another + cell reads the same history waits for that read and takes its answer from the cache.""" + key = (git_dir, sha, globs, links) + with _key_locks_guard: + lock = _key_locks.setdefault(key, threading.Lock()) + with lock: + return _read_guidance_commits(git_dir, sha, globs, links) + + @lru_cache(maxsize=64) -def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...]) -> tuple[_Commit, ...]: - """The commits reachable from ``sha`` that touch a path ``globs`` match, newest first by committer date.""" +def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _Links) -> tuple[_Commit, ...]: + """The commits reachable from ``sha`` that touch a path ``globs`` match or a target of ``links``, + newest first by committer date.""" + linked: dict[str, list[str]] = {} + for link, target in links: + linked.setdefault(target, []).append(link) found = [] - for record in _git_log(git_dir, sha).split("\x1e")[1:]: - header, _, names = record.partition("\0") - commit, authored, committed, subject = header.split("\x1f", 3) - paths = tuple(p for p in names.lstrip("\n").split("\0") if p and any(match_glob(g, p) for g in globs)) + for (commit, authored, committed, subject), touched in _parse_log(_git_log(git_dir, sha)): + paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, linked))) if paths: found.append(_Commit(commit, authored, committed, subject, paths)) # stable: commits with one committer date keep git's order return tuple(sorted(found, key=lambda c: datetime.fromisoformat(c.committed), reverse=True)) +def _counts_for(path: str, globs: tuple[str, ...], linked: dict[str, list[str]]) -> list[str]: + """The guidance paths a change to ``path`` counts for: itself when a glob matches it, and each link to it.""" + return ([path] if any(match_glob(g, path) for g in globs) else []) + linked.get(path, []) + + +def _parse_log(out: str) -> list[tuple[tuple[str, str, str, str], list[str]]]: + """Each commit of :func:`_git_log`'s output as ``((sha, authored, committed, subject), paths)``. + A field that starts with ``:`` (after the newline that ends a header) is a status record, and the + field after it is its path; a path is never read as a header. Any other non-empty field starts a + header of four fields; an empty one ends a merge's header, and the output.""" + fields = iter(out.split("\0")) + commits: list[tuple[tuple[str, str, str, str], list[str]]] = [] + for field in fields: + if field.lstrip("\n").startswith(":") and commits: + commits[-1][1].append(next(fields, "")) + elif field: + commits.append((_header(field, fields), [])) + return commits + + +def _header(sha: str, fields: Iterator[str]) -> tuple[str, str, str, str]: + authored, committed, subject = next(fields, ""), next(fields, ""), next(fields, "") + if not _COMMIT_ID.fullmatch(sha): + raise PrepError(f"git log: a commit starts with {sha[:60]!r}, not a commit id") + return sha, authored, committed, subject + + def _git_log(git_dir: Path, sha: str) -> str: - """Every commit reachable from ``sha`` with its dates, subject and the paths it touched.""" + """Every commit reachable from ``sha`` with its dates, subject and the paths it touched; a merge + with the paths its result changes against every parent.""" keep = ("PATH", "LANG", "TMPDIR", "SYSTEMROOT") env = process.Env.only(keep, {"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "HOME": os.devnull, "GIT_TERMINAL_PROMPT": "0"}) - done = process.git("-C", str(git_dir), "log", "-z", "--no-renames", "--name-only", _FORMAT, sha, "--", - timeout=_GIT_TIMEOUT_S, env=env) + done = process.git("-C", str(git_dir), "log", "-z", "--raw", "--no-abbrev", "--no-renames", + "--diff-merges=dense-combined", _FORMAT, sha, "--", timeout=_GIT_TIMEOUT_S, env=env) if done.timed_out: raise PrepError(f"git log {sha[:12]}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") if done.returncode: diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 82ba9008f..1e3a44737 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -505,7 +505,7 @@ def _apply_rev(ws: Path, git_dir: Path, manifest: RepoManifest, arm: Arm, if mode == "100755": dest_path.chmod(dest_path.stat().st_mode | 0o111) # arm_files: the files it wrote and the base's guidance it deleted (contract §3 meta.json) - history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written) + history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written, links) return sorted(set(written) | set(removed)), {"arm_rev": {"rev": arm.rev, "sha": arm.rev_sha, "removed": sorted(set(removed) - set(written)), **history}} diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py index 91d0f02f8..affd5024b 100644 --- a/dokimi/tests/test_repo_guidance_history.py +++ b/dokimi/tests/test_repo_guidance_history.py @@ -11,12 +11,15 @@ import os import subprocess +import threading +from concurrent.futures import ThreadPoolExecutor from pathlib import Path import pytest import dokimi.repos.guidance_history as guidance_history from dokimi.repos import RepoSource, load_manifest, open_source +from dokimi.repos.errors import PrepError from dokimi.repos.workspace import export_eval_dir, load_tasks, prepare_workspace, source_git_dir from repo_eval_fixture import git, make_repo, write @@ -135,3 +138,124 @@ def test_the_history_is_read_once_for_every_cell_of_the_arm(hist, monkeypatch): second = _prep(hist, f"guidance:rev:{hist['added']}", "c2")["arm_rev"] assert calls == [(hist["git_dir"], hist["added"])] assert first == second and len(first["guidance_commits"]) == 4 + + +def test_cells_that_ask_for_one_history_at_once_read_it_once(hist, monkeypatch): + """repo run prepares cells in a thread pool: the cells of an arm that start together share one read.""" + n, asked, all_asked, calls = 4, [], threading.Event(), [] + real_commits, real_log = guidance_history._guidance_commits, guidance_history._git_log + + def asking(*key): + asked.append(key) + if len(asked) == n: + all_asked.set() + return real_commits(*key) + + def log(*args): # holds the first read until every cell has asked; without a lock, each cell reads + calls.append(args) + all_asked.wait(timeout=10) + return real_log(*args) + + monkeypatch.setattr(guidance_history, "_guidance_commits", asking) + monkeypatch.setattr(guidance_history, "_git_log", log) + args = (hist["git_dir"], hist["manifest"].guidance_globs, hist["added"], ["AGENTS.md"], {}) + with ThreadPoolExecutor(n) as pool: + results = list(pool.map(lambda _: guidance_history.arm_rev_history(*args), range(n))) + assert all_asked.is_set() and calls == [(hist["git_dir"], hist["added"])] + assert all(r == results[0] for r in results) and len(results[0]["guidance_commits"]) == 4 + + +def test_a_subject_or_a_path_that_holds_a_separator_or_looks_like_a_header_stays_in_its_field(hist): + """``-z`` framing: subjects and paths may hold \\x1e and \\x1f, a path may look like a commit id or a + status record, and none of them starts a commit or ends one.""" + hexname = "0123456789abcdef0123456789abcdef01234567" + code = _commit_at(hist["root"], {"src/calc.py": "# code\n", hexname: "x\n", ":M": "x\n"}, "code\x1eonly\x1fhere", + "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00") + odd = _commit_at(hist["root"], {hexname: "y\n", "docs/a\x1eb/AGENTS.md": "# odd\n", f"sub/{hexname}/AGENTS.md": "#\n"}, + "guidance\x1eedit\x1fnote", "2030-05-02T00:00:00+00:00", "2030-05-02T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{odd}")["arm_rev"] + assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]] == [ + (odd, "guidance\x1eedit\x1fnote"), (hist["added"], "src guidance"), + (hist["amended"], "Guidance for the fix in #12"), (hist["skewed"], "sub guidance"), (hist["first"], "code")] + assert code not in {c["sha"] for c in rev["guidance_commits"]} + assert rev["guidance_latest"]["docs/a\x1eb/AGENTS.md"] == rev["guidance_latest"][f"sub/{hexname}/AGENTS.md"] == { + "sha": odd, "authored": "2030-05-02T00:00:00Z", "committed": "2030-05-02T00:00:00Z"} + assert len(rev["guidance_latest"]) == 5 + + +def test_a_log_field_that_should_start_a_commit_but_is_no_commit_id_is_a_prep_error(): + with pytest.raises(PrepError, match="'edit', not a commit id"): # not a ValueError out of prepare_workspace + guidance_history._parse_log("edit\x002030-01-01T00:00:00Z\x002030-01-01T00:00:00Z\x00guidance\x00") + + +def _branch_commit(root: Path, branch: str, files: dict[str, str], subject: str, date: str) -> str: + """A commit on a new ``branch`` from HEAD; HEAD stays where it was.""" + git(root, "checkout", "-qb", branch) + sha = _commit_at(root, files, subject, date, date) + git(root, "checkout", "-q", "-") + return sha + + +def _merge_at(root: Path, branch: str, files: dict[str, str], subject: str, authored: str, committed: str) -> str: + """A merge of ``branch`` into HEAD, its result edited (or its conflict resolved) to hold ``files``.""" + subprocess.run(["git", "-C", str(root), "merge", "-q", "--no-ff", "--no-commit", branch], + capture_output=True) # a conflict exits 1: ``files`` resolves it + return _commit_at(root, files, subject, authored, committed) + + +def test_a_merge_whose_resolution_edits_guidance_is_listed_with_its_committer_date(hist): + theirs = _branch_commit(hist["root"], "theirs", {"AGENTS.md": "# theirs\n"}, "theirs", "2030-05-01T00:00:00+00:00") + ours = _commit_at(hist["root"], {"AGENTS.md": "# ours\n"}, "ours", "2030-05-02T00:00:00+00:00", + "2030-05-02T00:00:00+00:00") + merge = _merge_at(hist["root"], "theirs", {"AGENTS.md": "# resolved from review\n"}, "Merge theirs", + "2030-05-03T00:00:00+00:00", "2030-06-01T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{merge}")["arm_rev"] + assert rev["guidance_latest"]["AGENTS.md"] == { + "sha": merge, "authored": "2030-05-03T00:00:00Z", "committed": "2030-06-01T00:00:00Z"} + assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]][:3] == [ + (merge, "Merge theirs"), (ours, "ours"), (theirs, "theirs")] + assert rev["guidance_commits"][0]["committed"] == "2030-06-01T00:00:00Z" + + +def test_a_merge_that_takes_a_guidance_file_from_one_parent_is_not_listed(hist): + feat = _branch_commit(hist["root"], "feat", {"sub/AGENTS.md": "# feat\n"}, "feat guidance", + "2030-05-01T00:00:00+00:00") + code = _commit_at(hist["root"], {"src/calc.py": "# main\n"}, "main code", "2030-05-02T00:00:00+00:00", + "2030-05-02T00:00:00+00:00") + merge = _merge_at(hist["root"], "feat", {}, "Merge feat", "2030-06-01T00:00:00+00:00", "2030-06-01T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{merge}")["arm_rev"] + assert rev["guidance_latest"]["sub/AGENTS.md"]["sha"] == feat + assert [c["sha"] for c in rev["guidance_commits"]][:2] == [feat, hist["added"]] + assert not {merge, code} & {c["sha"] for c in rev["guidance_commits"]} + + +def test_a_clean_merge_of_edits_to_different_parts_of_one_guidance_file_is_listed(hist): + """Its result differs from each parent, so git's dense combined listing names the file (contract §3).""" + lines = [f"line {i}\n" for i in range(9)] + base = _commit_at(hist["root"], {"AGENTS.md": "".join(lines)}, "lines", "2030-05-01T00:00:00+00:00", + "2030-05-01T00:00:00+00:00") + top = _branch_commit(hist["root"], "top", {"AGENTS.md": "".join(["top\n", *lines[1:]])}, "top", + "2030-05-02T00:00:00+00:00") + end = _commit_at(hist["root"], {"AGENTS.md": "".join([*lines, "end\n"])}, "end", "2030-05-03T00:00:00+00:00", + "2030-05-03T00:00:00+00:00") + merge = _merge_at(hist["root"], "top", {}, "Merge top", "2030-06-01T00:00:00+00:00", "2030-06-01T00:00:00+00:00") + assert (hist["root"] / "AGENTS.md").read_text() == "".join(["top\n", *lines[1:], "end\n"]) # clean, no edit + rev = _prep(hist, f"guidance:rev:{merge}")["arm_rev"] + assert [c["sha"] for c in rev["guidance_commits"]][:4] == [merge, end, top, base] + + +@pytest.mark.skipif(os.name == "nt", reason="symbolic links") +def test_an_edit_to_a_guidance_links_target_counts_for_the_link(hist): + """The arm installs ``AGENTS.md -> docs/GUIDE.md`` as the target's bytes; no glob matches docs/GUIDE.md.""" + (hist["root"] / "AGENTS.md").unlink() + os.symlink("docs/GUIDE.md", hist["root"] / "AGENTS.md") + link = _commit_at(hist["root"], {"docs/GUIDE.md": "# guide\n"}, "link the guide", "2030-05-01T00:00:00+00:00", + "2030-05-01T00:00:00+00:00") + edit = _commit_at(hist["root"], {"docs/GUIDE.md": "# guide, from review\n"}, "Guide for #12", + "2030-06-01T00:00:00+00:00", "2030-06-01T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{edit}")["arm_rev"] + assert rev["guidance_latest"]["AGENTS.md"] == { + "sha": edit, "authored": "2030-06-01T00:00:00Z", "committed": "2030-06-01T00:00:00Z"} + assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]][:2] == [(edit, "Guide for #12"), + (link, "link the guide")] + assert "docs/GUIDE.md" not in rev["guidance_latest"] From 1b75473866d2ea1b61cf2b4ffef5fe495228bc4f Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 20:25:18 +0000 Subject: [PATCH 3/8] fix(repo): pin git log config; count every link of a guidance chain Settles two review points on the guidance history of a guidance:rev: arm (intel/dokimi#290, recording half). The repository's own config could change what the history's git log prints. log.showSignature=true put verification text outside the NUL-framed fields, so one signed commit anywhere in the history stopped the arm's workspace from being prepared. Checked against git 2.51, four local keys change the output: log.showSignature, log.showRoot=false (the root commit with no files), i18n.logOutputEncoding (a UTF-16 subject holds NULs) and diff.ignoreSubmodules=all (a commit that adds a gitlink is dropped). The log now passes --no-show-signature, --root, --encoding=UTF-8 and --ignore-submodules=none. A guidance link resolved through intermediate links (AGENTS.md -> docs/current -> old.md) recorded only its final file, so a commit that only retargets docs/current changed what the arm installs but was in neither history output. _guidance_at now also returns every path each link resolves through (split into _link_chain, which lowers its complexity below the baseline), and a commit that touches any of them counts for the link's path and is part of the cache key. The arm still installs the final file's bytes. Tests with real repositories: a signed commit under repository-local log.showSignature=true, each other pinned key leaving arm_rev as it is without it, and a commit that only retargets an intermediate link. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 6 +- dokimi/docs/repo-evaluation-contract.md | 5 +- dokimi/dokimi/repos/guidance_history.py | 39 ++++++---- dokimi/dokimi/repos/workspace.py | 62 ++++++++-------- dokimi/scripts/growth_baseline.json | 1 - dokimi/tests/test_repo_guidance_history.py | 73 ++++++++++++++++++- 6 files changed, 133 insertions(+), 53 deletions(-) diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md index 65440d4db..8eac4549b 100644 --- a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -4,9 +4,9 @@ `repo tasks replay` writes `public_at` into each `task.json`: the earliest `createdAt` of a gold thread's first comment. A `guidance:rev:` cell's `meta.json` `arm_rev` adds `guidance_latest` (the last commit to touch each - guidance file at ``, or the file a guidance link resolves to), - `guidance_commits` (the commits that touch a guidance path or a link's - target, a merge when its own result changes one, newest first by committer + guidance file at ``, or a path a guidance link resolves through), + `guidance_commits` (the commits that touch a guidance path or a path a link + resolves through, a merge when its own result changes one, newest first by committer date, at most 200) and `guidance_commits_truncated`, each commit with its author and committer date, so a reader can see whether a guidance commit came after a task's review was public. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 14ecf05fd..2ad5a314f 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -366,9 +366,10 @@ grader diffs the agent's changes from `start_sha`. With the arm come files with no counterpart at ``), which also records the guidance history of `` (`dokimi/repos/guidance_history.py`): `guidance_latest`, for each guidance file the arm wrote, the commit reachable from `` that last -touched it, or for a link the file it resolves to, by committer date; +touched it, or for a link it or any path its resolution goes through (each +intermediate link and the file it resolves to), by committer date; `guidance_commits`, the commits reachable from `` that touch a path the -guidance globs match or a guidance link's target, newest first by committer +guidance globs match or a path a guidance link resolves through, newest first by committer date, at most 200, each with its `subject`; and `guidance_commits_truncated`, `true` when that cap cut the list. Each commit has its `sha`, `authored` and `committed` (strict ISO 8601, `%aI` and `%cI`). A merge commit counts for a diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index 709164f09..d600eaf2f 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -14,10 +14,12 @@ followed by its one path, so a subject or a path, whatever it holds, cannot pass for the other. Git cuts a subject at a NUL (``git commit`` refuses one), so a subject is one field. The history is read once per git dir, revision, globs and links in a process, also when cells of the arm prepare at -once: every cell asks the same. +once: every cell asks the same. A key of the repository's own config that changes what the log +prints (as checked with git 2.51) is overridden by a flag (:data:`_PINNED`). -The arm installs a guidance link (``AGENTS.md -> docs/GUIDE.md``) as its target's bytes, so a -commit that touches the target counts for the link's path, whether or not a glob matches the target. +The arm installs a guidance link (``AGENTS.md -> docs/current -> GUIDE.md``) as the bytes of the file +it resolves to, so a commit that touches any path of its chain, an intermediate link or that file, +counts for the link's path, whether or not a glob matches that path. A merge counts when its own result changes a guidance path against every parent (``--diff-merges=dense-combined``): a conflict resolution, an edit made in the merge, or a clean @@ -47,8 +49,15 @@ _GIT_TIMEOUT_S = 1800.0 _FORMAT = "--format=%H%x00%aI%x00%cI%x00%s" # with -z, a NUL ends the subject too _COMMIT_ID = re.compile(r"[0-9a-f]{40}(?:[0-9a-f]{24})?") # SHA-1 or SHA-256 +#: Each overrides a key of the repository's config that changes the output (git 2.51) +_PINNED = ( + "--no-show-signature", # log.showSignature: verification text outside the fields + "--root", # log.showRoot=false: the root commit with no files + "--encoding=UTF-8", # i18n.logOutputEncoding: a subject re-encoded, in UTF-16 with NULs + "--ignore-submodules=none", # diff.ignoreSubmodules=all: no commit that adds or moves a gitlink +) -_Links = tuple[tuple[str, str], ...] # (link path, the file it resolves to), sorted +_Links = tuple[tuple[str, tuple[str, ...]], ...] # (link path, the paths it resolves through), sorted @dataclass(frozen=True) @@ -64,11 +73,12 @@ def dates(self) -> dict[str, str]: def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list[str], - links: dict[str, str]) -> dict[str, Any]: + links: dict[str, list[str]]) -> dict[str, Any]: """``guidance_latest`` for each of ``files`` (the guidance at ``sha``; ``links`` maps those that - are links to their targets), ``guidance_commits`` (newest first by committer date, at most - :data:`GUIDANCE_COMMITS_CAP`) and ``guidance_commits_truncated``.""" - commits = _guidance_commits(git_dir, sha, globs, tuple(sorted(links.items()))) + are links to every path they resolve through, the file they resolve to last), ``guidance_commits`` + (newest first by committer date, at most :data:`GUIDANCE_COMMITS_CAP`) and + ``guidance_commits_truncated``.""" + commits = _guidance_commits(git_dir, sha, globs, tuple(sorted((k, tuple(v)) for k, v in links.items()))) latest: dict[str, dict[str, str]] = {} for c in commits: # newest first: the first commit naming a file is its latest for path in c.paths: @@ -94,11 +104,12 @@ def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _L @lru_cache(maxsize=64) def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _Links) -> tuple[_Commit, ...]: - """The commits reachable from ``sha`` that touch a path ``globs`` match or a target of ``links``, - newest first by committer date.""" + """The commits reachable from ``sha`` that touch a path ``globs`` match or a path a link of ``links`` + resolves through, newest first by committer date.""" linked: dict[str, list[str]] = {} - for link, target in links: - linked.setdefault(target, []).append(link) + for link, chain in links: + for path in chain: + linked.setdefault(path, []).append(link) found = [] for (commit, authored, committed, subject), touched in _parse_log(_git_log(git_dir, sha)): paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, linked))) @@ -109,7 +120,7 @@ def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], link def _counts_for(path: str, globs: tuple[str, ...], linked: dict[str, list[str]]) -> list[str]: - """The guidance paths a change to ``path`` counts for: itself when a glob matches it, and each link to it.""" + """The guidance paths a change to ``path`` counts for: itself when a glob matches it, and each link through it.""" return ([path] if any(match_glob(g, path) for g in globs) else []) + linked.get(path, []) @@ -142,7 +153,7 @@ def _git_log(git_dir: Path, sha: str) -> str: env = process.Env.only(keep, {"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "HOME": os.devnull, "GIT_TERMINAL_PROMPT": "0"}) done = process.git("-C", str(git_dir), "log", "-z", "--raw", "--no-abbrev", "--no-renames", - "--diff-merges=dense-combined", _FORMAT, sha, "--", timeout=_GIT_TIMEOUT_S, env=env) + "--diff-merges=dense-combined", *_PINNED, _FORMAT, sha, "--", timeout=_GIT_TIMEOUT_S, env=env) if done.timed_out: raise PrepError(f"git log {sha[:12]}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") if done.returncode: diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 1e3a44737..8837e17cb 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -470,7 +470,7 @@ def _apply_rev(ws: Path, git_dir: Path, manifest: RepoManifest, arm: Arm, removed: list[str]) -> tuple[list[str], dict[str, Any]]: """The guidance files of ``arm.rev_sha`` into ``ws``, whose own were ``removed``.""" assert arm.rev_sha is not None - written, links = _guidance_at(git_dir, manifest, arm.rev_sha) + written, links, chains = _guidance_at(git_dir, manifest, arm.rev_sha) files = [p for p in written if p not in links] if files: # checked out as files, as in step 1: no object of the source is copied _checkout_tree(git_dir, arm.rev_sha, ws, paths=files) @@ -505,7 +505,7 @@ def _apply_rev(ws: Path, git_dir: Path, manifest: RepoManifest, arm: Arm, if mode == "100755": dest_path.chmod(dest_path.stat().st_mode | 0o111) # arm_files: the files it wrote and the base's guidance it deleted (contract §3 meta.json) - history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written, links) + history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written, chains) return sorted(set(written) | set(removed)), {"arm_rev": {"rev": arm.rev, "sha": arm.rev_sha, "removed": sorted(set(removed) - set(written)), **history}} @@ -524,10 +524,11 @@ def _apply_bridge(ws: Path, matched: list[str], bridge: dict[str, str]) -> tuple return written, ({"bridge_skipped": skipped} if skipped else {}) -def _guidance_at(git_dir: Path, manifest: RepoManifest, sha: str) -> tuple[list[str], dict[str, str]]: +def _guidance_at(git_dir: Path, manifest: RepoManifest, + sha: str) -> tuple[list[str], dict[str, str], dict[str, list[str]]]: """The files of commit ``sha`` the guidance globs match, outside ``eval_dir`` (step 2 deletes it), and for each - one that is a symbolic link the file it resolves to in that commit. A link out of the tree, into ``eval_dir`` - or to nothing is refused.""" + one that is a symbolic link the file it resolves to in that commit and every path its resolution goes through + (:func:`_link_chain`). A link out of the tree, into ``eval_dir`` or to nothing is refused.""" eval_dir = manifest.run_paths()[0] listing = _run(["git", "-C", str(git_dir), "ls-tree", "-r", "-z", "--full-tree", sha], env=_git_env(SNAPSHOT_AUTHOR)) entries = {} @@ -538,32 +539,33 @@ def _guidance_at(git_dir: Path, manifest: RepoManifest, sha: str) -> tuple[list[ entries[path] = (mode, oid) found = sorted(p for p in entries if not _is_within(p, eval_dir) and any(match_glob(g, p) for g in manifest.guidance_globs)) - links = {} + links, chains = {}, {} for path in found: - target = path - for _ in range(8): # a chain of links; a loop runs out - mode, oid = entries.get(target, ("", "")) - if mode != "120000": - break - dest = _run(["git", "-C", str(git_dir), "cat-file", "blob", oid], env=_git_env(SNAPSHOT_AUTHOR)) - target = posixpath.normpath(posixpath.join(posixpath.dirname(target), dest)) - if dest.startswith("/") or target == ".." or target.startswith("../"): - raise PrepError(f"guidance {path} at {sha[:12]} links out of the tree ({dest})") - if _is_within(target, eval_dir): - raise PrepError(f"guidance {path} at {sha[:12]} links into eval_dir {eval_dir}") - # Always validate the final entry's mode, even if target == path (cyclic link) - final_mode = entries.get(target, ("",))[0] - if final_mode not in ("", "120000"): - # Normal case: target is a file (mode is a file blob mode) - if target != path: - links[path] = target - elif final_mode == "120000": - # Cyclic link: target is still a symlink after loop - raise PrepError(f"guidance {path} at {sha[:12]} links to {target}, which is not a file there") - else: - # Target doesn't exist - raise PrepError(f"guidance {path} at {sha[:12]} links to {target}, which is not a file there") - return found, links + if chain := _link_chain(git_dir, entries, path, sha, eval_dir): + links[path], chains[path] = chain[-1], chain + return found, links, chains + + +def _link_chain(git_dir: Path, entries: dict[str, tuple[str, str]], path: str, sha: str, eval_dir: str) -> list[str]: + """The paths guidance ``path`` resolves through in commit ``sha`` (``entries``: its blobs' mode and id): each + link after ``path`` and, last, the file it resolves to; empty when ``path`` is a file.""" + chain: list[str] = [] + target = path + for _ in range(8): # a chain of links; a loop runs out + mode, oid = entries.get(target, ("", "")) + if mode != "120000": + break + dest = _run(["git", "-C", str(git_dir), "cat-file", "blob", oid], env=_git_env(SNAPSHOT_AUTHOR)) + target = posixpath.normpath(posixpath.join(posixpath.dirname(target), dest)) + if dest.startswith("/") or target == ".." or target.startswith("../"): + raise PrepError(f"guidance {path} at {sha[:12]} links out of the tree ({dest})") + if _is_within(target, eval_dir): + raise PrepError(f"guidance {path} at {sha[:12]} links into eval_dir {eval_dir}") + chain.append(target) + # the final entry is a file: still a link after the loop is a cycle, no entry a dangling link + if entries.get(target, ("",))[0] in ("", "120000"): + raise PrepError(f"guidance {path} at {sha[:12]} links to {target}, which is not a file there") + return chain def _apply_patch(ws: Path, env: dict[str, str], manifest: RepoManifest, arm: Arm) -> tuple[list[str], dict[str, Any]]: diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index d751c4643..67e0ab606 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -85,7 +85,6 @@ "dokimi/repos/manifest.py::parse_manifest": {"complexity": 15}, "dokimi/repos/source.py::GitTree.read_many": {"complexity": 14}, "dokimi/repos/workspace.py::_apply_rev": {"complexity": 11, "rationale": "pre-existing when the gate was introduced"}, - "dokimi/repos/workspace.py::_guidance_at": {"complexity": 11, "rationale": "pre-existing when the gate was introduced"}, "dokimi/schema_compat.py::_compare": {"complexity": 41}, "dokimi/subjects/loader.py::_parse_subject": {"complexity": 13}, "dokimi/tool_preference.py::summarize": {"complexity": 12}, diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py index affd5024b..9c7a202c5 100644 --- a/dokimi/tests/test_repo_guidance_history.py +++ b/dokimi/tests/test_repo_guidance_history.py @@ -10,6 +10,7 @@ from __future__ import annotations import os +import shutil import subprocess import threading from concurrent.futures import ThreadPoolExecutor @@ -32,12 +33,15 @@ """ -def _commit_at(root: Path, files: dict[str, str], subject: str, authored: str, committed: str) -> str: +def _commit_at(root: Path, files: dict[str, str], subject: str, authored: str, committed: str, + *opts: str, stage: bool = True) -> str: + """A commit of ``files`` (``stage=False``: of the index as it is), ``opts`` before ``commit``.""" for rel, text in files.items(): write(root, rel, text) - git(root, "add", "-A") + if stage: + git(root, "add", "-A") env = {**os.environ, "GIT_AUTHOR_DATE": authored, "GIT_COMMITTER_DATE": committed} - subprocess.run(["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", subject], + subprocess.run(["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t", *opts, "commit", "-qm", subject], check=True, capture_output=True, env=env) return git(root, "rev-parse", "HEAD") @@ -259,3 +263,66 @@ def test_an_edit_to_a_guidance_links_target_counts_for_the_link(hist): assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]][:2] == [(edit, "Guide for #12"), (link, "link the guide")] assert "docs/GUIDE.md" not in rev["guidance_latest"] + + +@pytest.mark.skipif(os.name == "nt", reason="symbolic links") +def test_a_commit_that_only_retargets_an_intermediate_link_counts_for_the_guidance_link(hist): + """``AGENTS.md -> docs/current -> old.md``; no glob matches docs/current or a file it names.""" + (hist["root"] / "AGENTS.md").unlink() + os.symlink("docs/current", hist["root"] / "AGENTS.md") + write(hist["root"], "docs/old.md", "# old guide\n") + os.symlink("old.md", hist["root"] / "docs" / "current") + chain = _commit_at(hist["root"], {}, "chain the guide", "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00") + added = _commit_at(hist["root"], {"docs/review-derived.md": "# from review\n"}, "add a guide", + "2030-05-02T00:00:00+00:00", "2030-05-02T00:00:00+00:00") + (hist["root"] / "docs" / "current").unlink() + os.symlink("review-derived.md", hist["root"] / "docs" / "current") + retarget = _commit_at(hist["root"], {}, "Point the guide at #12's rule", "2030-06-01T00:00:00+00:00", + "2030-06-01T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{retarget}")["arm_rev"] + assert rev["guidance_latest"]["AGENTS.md"] == { + "sha": retarget, "authored": "2030-06-01T00:00:00Z", "committed": "2030-06-01T00:00:00Z"} + assert [c["sha"] for c in rev["guidance_commits"]][:3] == [retarget, added, chain] + assert (hist["tmp"] / "rd" / "workspace" / "AGENTS.md").read_text() == "# from review\n" # the final file's bytes + + +def _signing_key(tmp: Path) -> Path: + key = tmp / "signing-key" + subprocess.run(["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", "t", "-f", str(key)], check=True, + capture_output=True) + return key + + +@pytest.mark.skipif(shutil.which("ssh-keygen") is None, reason="needs ssh-keygen to sign a commit") +def test_a_signed_commit_under_the_repositorys_show_signature_is_read_and_listed(hist): + """Repository-local ``log.showSignature=true`` would print verification text outside the fields.""" + key = _signing_key(hist["tmp"]) + signed = _commit_at(hist["root"], {"AGENTS.md": "# signed guidance\n"}, "signed guidance", + "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00", + "-c", "gpg.format=ssh", "-c", f"user.signingkey={key}", "-c", "commit.gpgSign=true") + assert "gpgsig" in git(hist["root"], "cat-file", "commit", signed) + git(hist["root"], "config", "log.showSignature", "true") + rev = _prep(hist, f"guidance:rev:{signed}")["arm_rev"] + assert [(c["sha"], c["subject"]) for c in rev["guidance_commits"]][:2] == [ + (signed, "signed guidance"), (hist["added"], "src guidance")] + assert rev["guidance_latest"]["AGENTS.md"]["sha"] == signed + + +@pytest.mark.parametrize(("key", "value"), [ + ("log.showRoot", "false"), # the root commit, which adds AGENTS.md, with no files + ("i18n.logOutputEncoding", "UTF-16"), # every subject re-encoded, NULs in it + ("diff.ignoreSubmodules", "all"), # no commit that adds a gitlink +]) +def test_the_repositorys_own_config_does_not_change_the_history(hist, key, value): + guidance = _commit_at(hist["root"], {"AGENTS.md": "# Leitfaden\n"}, "Leitfaden für #12", + "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00") + git(hist["root"], "update-index", "--add", "--cacheinfo", f"160000,{guidance},vendor/AGENTS.md") + gitlink = _commit_at(hist["root"], {}, "vendor a gitlink", "2030-05-02T00:00:00+00:00", + "2030-05-02T00:00:00+00:00", stage=False) + plain = _prep(hist, f"guidance:rev:{gitlink}", "plain")["arm_rev"] + assert [(c["sha"], c["subject"]) for c in plain["guidance_commits"]][:2] == [ + (gitlink, "vendor a gitlink"), (guidance, "Leitfaden für #12")] + assert plain["guidance_commits"][-1]["sha"] == hist["first"] + guidance_history._read_guidance_commits.cache_clear() + git(hist["root"], "config", key, value) + assert _prep(hist, f"guidance:rev:{gitlink}", "configured")["arm_rev"] == plain From f6a9cb91306e0baa3c7092ae857ddb4f607a6158 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 10:47:53 +0000 Subject: [PATCH 4/8] fix(repo): refuse a shallow guidance history; keep CR and UTC dates Settles four review points on the guidance history a guidance:rev: arm records in meta.json arm_rev (intel/dokimi#290, recording half). - A local source is used as it is, and in a shallow checkout git log takes the boundary commit for a root: with guidance v1 -> v2 -> a code commit cloned at --depth 1, the code commit was recorded as the last to touch AGENTS.md and the only guidance commit. The history now asks git rev-parse --is-shallow-repository first and raises PrepError for a shallow git dir, naming it and git fetch --unshallow. dokimi's own clones of a remote are bare and never shallow; a blob-less clone keeps working, as log --raw compares object ids only. - process.git's text output turns each \r into \n, so a guidance path or subject holding a \r lost it, and the commit matched no glob and was dropped without a report. The log is now split on NUL as bytes and each field decoded as UTF-8 on its own (with replacement, as dokimi.process decodes). - git 2.51 prints a UTC %aI / %cI as Z, git 2.43 as +00:00. A UTC date is now recorded as Z whichever git read it; other offsets stay as printed. - The _public_at docstring said a Z time counts as none. A time without an offset counts as none; GitHub's Z is UTC and counts, now pinned by a test. The contract (section 3) and the changelog fragment name the refusal and the date form. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 6 +- dokimi/docs/repo-evaluation-contract.md | 5 +- dokimi/dokimi/repos/guidance_history.py | 51 +++++++++---- dokimi/dokimi/repos/replay/build.py | 3 +- dokimi/tests/test_repo_guidance_history.py | 74 ++++++++++++++++++- dokimi/tests/test_repo_replay.py | 8 ++ 6 files changed, 128 insertions(+), 19 deletions(-) diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md index 8eac4549b..89d006c83 100644 --- a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -8,5 +8,7 @@ `guidance_commits` (the commits that touch a guidance path or a path a link resolves through, a merge when its own result changes one, newest first by committer date, at most 200) and `guidance_commits_truncated`, each commit with its - author and committer date, so a reader can see whether a guidance commit - came after a task's review was public. + author and committer date (a UTC date as `Z`), so a reader can see whether + a guidance commit came after a task's review was public. A `guidance:rev:` + cell whose source is a shallow clone fails its preparation: the history + needs the full one (`git fetch --unshallow`). diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index ac7e2e403..5630e455f 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -400,7 +400,10 @@ intermediate link and the file it resolves to), by committer date; guidance globs match or a path a guidance link resolves through, newest first by committer date, at most 200, each with its `subject`; and `guidance_commits_truncated`, `true` when that cap cut the list. Each commit has its `sha`, `authored` and -`committed` (strict ISO 8601, `%aI` and `%cI`). A merge commit counts for a +`committed` (strict ISO 8601, `%aI` and `%cI`; a UTC date is written `Z`, +whichever form the git that read the history prints). The history needs the whole of it: a shallow +git dir (a local checkout cloned with `--depth`) fails the cell's preparation +(`git fetch --unshallow`), as its history stops at the boundary commit. A merge commit counts for a path when its result differs there from every parent: a conflict resolution, an edit made in the merge, or a clean merge of its parents' edits to different parts of one file; a merge that takes the path as one parent has diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index d600eaf2f..934bb832f 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -12,10 +12,15 @@ The history is one ``git log -z --raw`` over the revision, filtered with :func:`~.manifest.match_glob`. ``-z`` ends every field with a NUL, and each file the commit touched is a ``:``-led status record followed by its one path, so a subject or a path, whatever it holds, cannot pass for the other. -Git cuts a subject at a NUL (``git commit`` refuses one), so a subject is one field. The history is -read once per git dir, revision, globs and links in a process, also when cells of the arm prepare at -once: every cell asks the same. A key of the repository's own config that changes what the log -prints (as checked with git 2.51) is overridden by a flag (:data:`_PINNED`). +Git cuts a subject at a NUL (``git commit`` refuses one), so a subject is one field. The output is +split on NUL as bytes and each field then decoded as UTF-8 (``--encoding=UTF-8``; a byte that is not +UTF-8 becomes U+FFFD, as :mod:`dokimi.process` decodes), so a ``\\r`` in a subject or a path stays one. +A date is git's ``%aI`` / ``%cI`` with a UTC offset written ``Z``, which git 2.51 prints and older git +(2.43) prints as ``+00:00``. The history is read once per git dir, revision, globs and links in a +process, also when cells of the arm prepare at once: every cell asks the same. A key of the +repository's own config that changes what the log prints (as checked with git 2.51) is overridden by +a flag (:data:`_PINNED`). A shallow git dir is refused: its history stops at the boundary commit, so +it would name a later commit as the last to touch a file. The arm installs a guidance link (``AGENTS.md -> docs/current -> GUIDE.md``) as the bytes of the file it resolves to, so a commit that touches any path of its chain, an intermediate link or that file, @@ -110,6 +115,7 @@ def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], link for link, chain in links: for path in chain: linked.setdefault(path, []).append(link) + _refuse_shallow(git_dir) found = [] for (commit, authored, committed, subject), touched in _parse_log(_git_log(git_dir, sha)): paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, linked))) @@ -124,12 +130,13 @@ def _counts_for(path: str, globs: tuple[str, ...], linked: dict[str, list[str]]) return ([path] if any(match_glob(g, path) for g in globs) else []) + linked.get(path, []) -def _parse_log(out: str) -> list[tuple[tuple[str, str, str, str], list[str]]]: +def _parse_log(out: bytes) -> list[tuple[tuple[str, str, str, str], list[str]]]: """Each commit of :func:`_git_log`'s output as ``((sha, authored, committed, subject), paths)``. A field that starts with ``:`` (after the newline that ends a header) is a status record, and the field after it is its path; a path is never read as a header. Any other non-empty field starts a - header of four fields; an empty one ends a merge's header, and the output.""" - fields = iter(out.split("\0")) + header of four fields; an empty one ends a merge's header, and the output. Each field is decoded + on its own, after the split: no ``\\r`` is turned into a newline.""" + fields = (field.decode("utf-8", "replace") for field in out.split(b"\0")) commits: list[tuple[tuple[str, str, str, str], list[str]]] = [] for field in fields: if field.lstrip("\n").startswith(":") and commits: @@ -143,19 +150,35 @@ def _header(sha: str, fields: Iterator[str]) -> tuple[str, str, str, str]: authored, committed, subject = next(fields, ""), next(fields, ""), next(fields, "") if not _COMMIT_ID.fullmatch(sha): raise PrepError(f"git log: a commit starts with {sha[:60]!r}, not a commit id") - return sha, authored, committed, subject + return sha, _utc_as_z(authored), _utc_as_z(committed), subject -def _git_log(git_dir: Path, sha: str) -> str: +def _utc_as_z(date: str) -> str: + """A strict ISO 8601 date of git's with a UTC offset as ``Z``, whichever form the git that printed it uses.""" + return date.removesuffix("+00:00") + "Z" if date.endswith("+00:00") else date + + +def _refuse_shallow(git_dir: Path) -> None: + if _git(git_dir, "rev-parse", "rev-parse", "--is-shallow-repository").strip() == b"true": + raise PrepError(f"{git_dir} is a shallow clone: the guidance history of a guidance:rev: arm needs its full " + "history (git fetch --unshallow)") + + +def _git_log(git_dir: Path, sha: str) -> bytes: """Every commit reachable from ``sha`` with its dates, subject and the paths it touched; a merge with the paths its result changes against every parent.""" + return _git(git_dir, f"log {sha[:12]}", "log", "-z", "--raw", "--no-abbrev", "--no-renames", + "--diff-merges=dense-combined", *_PINNED, _FORMAT, sha, "--") + + +def _git(git_dir: Path, what: str, *args: str) -> bytes: + """The stdout of ``git -C git_dir *args`` as bytes, with no system or global config; ``what`` names it in an error.""" keep = ("PATH", "LANG", "TMPDIR", "SYSTEMROOT") env = process.Env.only(keep, {"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "HOME": os.devnull, "GIT_TERMINAL_PROMPT": "0"}) - done = process.git("-C", str(git_dir), "log", "-z", "--raw", "--no-abbrev", "--no-renames", - "--diff-merges=dense-combined", *_PINNED, _FORMAT, sha, "--", timeout=_GIT_TIMEOUT_S, env=env) + done = process.git("-C", str(git_dir), *args, timeout=_GIT_TIMEOUT_S, env=env) if done.timed_out: - raise PrepError(f"git log {sha[:12]}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") + raise PrepError(f"git {what}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") if done.returncode: - raise PrepError(f"git log {sha[:12]} exited {done.returncode}: {process.git_failure(done)}") - return done.stdout + raise PrepError(f"git {what} exited {done.returncode}: {process.git_failure(done)}") + return done.stdout_bytes diff --git a/dokimi/dokimi/repos/replay/build.py b/dokimi/dokimi/repos/replay/build.py index 532c45c09..a1450f819 100644 --- a/dokimi/dokimi/repos/replay/build.py +++ b/dokimi/dokimi/repos/replay/build.py @@ -208,7 +208,8 @@ def _where(t: Thread) -> str: def _public_at(threads: list[Thread], gold: set[str]) -> str | None: """The earliest first-comment ``createdAt`` of the threads whose id is in ``gold``, as the input gives it; - a time that is not ISO 8601 with an offset (GitHub writes ``Z``) counts as none. ``None`` when none has one.""" + a time that is not ISO 8601 or has no offset counts as none. GitHub's ``Z`` is an offset, UTC, and counts. + ``None`` when none has one.""" times = [] for t in (t for t in threads if t.id in gold): try: diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py index 9c7a202c5..225b86042 100644 --- a/dokimi/tests/test_repo_guidance_history.py +++ b/dokimi/tests/test_repo_guidance_history.py @@ -189,7 +189,79 @@ def test_a_subject_or_a_path_that_holds_a_separator_or_looks_like_a_header_stays def test_a_log_field_that_should_start_a_commit_but_is_no_commit_id_is_a_prep_error(): with pytest.raises(PrepError, match="'edit', not a commit id"): # not a ValueError out of prepare_workspace - guidance_history._parse_log("edit\x002030-01-01T00:00:00Z\x002030-01-01T00:00:00Z\x00guidance\x00") + guidance_history._parse_log(b"edit\x002030-01-01T00:00:00Z\x002030-01-01T00:00:00Z\x00guidance\x00") + + +def test_a_utc_date_is_recorded_with_z_whichever_form_git_prints(): + """git 2.51 prints ``Z`` for a UTC ``%aI`` / ``%cI``, older git (2.43) ``+00:00``: the record is one form.""" + to_z = guidance_history._utc_as_z + assert to_z("2030-01-01T00:00:00+00:00") == to_z("2030-01-01T00:00:00Z") == "2030-01-01T00:00:00Z" + assert to_z("2030-01-01T00:00:00+05:30") == "2030-01-01T00:00:00+05:30" + assert to_z("2030-01-01T00:00:00-04:00") == "2030-01-01T00:00:00-04:00" + sha = "a" * 40 + log = f"{sha}\0002030-01-01T00:00:00+00:00\0002030-02-01T00:00:00+00:00\0g\0\n:000000 100644 A\0AGENTS.md\0" + assert guidance_history._parse_log(log.encode()) == [ + ((sha, "2030-01-01T00:00:00Z", "2030-02-01T00:00:00Z", "g"), ["AGENTS.md"])] + + +@pytest.mark.skipif(os.name == "nt", reason="a carriage return in a file name") +def test_a_carriage_return_in_a_guidance_path_or_a_subject_is_kept(hist): + """``process.git``'s text turns ``\\r`` into a newline: the log is split as bytes, then decoded.""" + cr = _commit_at(hist["root"], {"a\rb/AGENTS.md": "# cr\n"}, "guidance\redit", "2030-05-01T00:00:00+00:00", + "2030-05-01T00:00:00+00:00") + rev = guidance_history.arm_rev_history(hist["git_dir"], hist["manifest"].guidance_globs, cr, + ["AGENTS.md", "a\rb/AGENTS.md"], {}) + assert rev["guidance_latest"]["a\rb/AGENTS.md"] == { + "sha": cr, "authored": "2030-05-01T00:00:00Z", "committed": "2030-05-01T00:00:00Z"} + assert rev["guidance_commits"][0] == {"sha": cr, "authored": "2030-05-01T00:00:00Z", + "committed": "2030-05-01T00:00:00Z", "subject": "guidance\redit"} + + +def _three_commits(tmp: Path) -> tuple[Path, dict[str, str]]: + """guidance v1, guidance v2, then a code-only commit, in a repository of its own.""" + root = tmp / "three" + root.mkdir() + git(root, "init", "-q") + return root, { + "v1": _commit_at(root, {"AGENTS.md": "# v1\n"}, "guidance v1", "2030-01-01T00:00:00+00:00", + "2030-01-01T00:00:00+00:00"), + "v2": _commit_at(root, {"AGENTS.md": "# v2\n"}, "guidance v2", "2030-02-01T00:00:00+00:00", + "2030-02-01T00:00:00+00:00"), + "code": _commit_at(root, {"src/calc.py": "# code\n"}, "code only", "2030-03-01T00:00:00+00:00", + "2030-03-01T00:00:00+00:00"), + } + + +def _clone(tmp: Path, src: Path, name: str, *opts: str) -> Path: + subprocess.run(["git", "clone", "-q", *opts, f"file://{src}", str(tmp / name)], check=True, capture_output=True) + return tmp / name + + +def test_a_shallow_git_dir_is_refused_and_a_full_clone_gives_the_whole_history(tmp_path): + """A shallow clone's history stops at its boundary commit, which then looks like the last to touch AGENTS.md.""" + root, shas = _three_commits(tmp_path) + shallow = _clone(tmp_path, root, "shallow", "--depth", "1") + with pytest.raises(PrepError, match=r"shallow/\.git is a shallow clone:.*full history \(git fetch --unshallow\)"): + guidance_history.arm_rev_history(shallow / ".git", ("AGENTS.md",), shas["code"], ["AGENTS.md"], {}) + full = _clone(tmp_path, root, "full") + rev = guidance_history.arm_rev_history(full / ".git", ("AGENTS.md",), shas["code"], ["AGENTS.md"], {}) + assert rev["guidance_latest"]["AGENTS.md"]["sha"] == shas["v2"] + assert [c["subject"] for c in rev["guidance_commits"]] == ["guidance v2", "guidance v1"] + + +def test_a_blobless_clone_gives_the_history_without_fetching_a_blob(tmp_path): + """``log --raw`` compares object ids only, also for a merge: the clone's origin is gone when it is read.""" + root, shas = _three_commits(tmp_path) + side = _branch_commit(root, "side", {"AGENTS.md": "# side\n"}, "side guidance", "2030-04-01T00:00:00+00:00") + merge = _merge_at(root, "side", {"AGENTS.md": "# resolved\n"}, "Merge side", "2030-05-01T00:00:00+00:00", + "2030-05-01T00:00:00+00:00") + git(root, "config", "uploadpack.allowFilter", "true") + blobless = _clone(tmp_path, root, "blobless.git", "--bare", "--filter=blob:none") + assert git(blobless, "config", "remote.origin.promisor") == "true" + git(blobless, "remote", "set-url", "origin", str(tmp_path / "gone")) # a blob it needed could not be fetched + rev = guidance_history.arm_rev_history(blobless, ("AGENTS.md",), merge, ["AGENTS.md"], {}) + assert [c["sha"] for c in rev["guidance_commits"]] == [merge, side, shas["v2"], shas["v1"]] + assert rev["guidance_latest"]["AGENTS.md"]["sha"] == merge def _branch_commit(root: Path, branch: str, files: dict[str, str], subject: str, date: str) -> str: diff --git a/dokimi/tests/test_repo_replay.py b/dokimi/tests/test_repo_replay.py index 247dd12fb..c2e5133f6 100644 --- a/dokimi/tests/test_repo_replay.py +++ b/dokimi/tests/test_repo_replay.py @@ -273,6 +273,14 @@ def test_public_at_is_the_earliest_gold_threads_first_comment_compared_as_times( assert task["public_at"] == "2026-10-02T20:00:00+09:00" +def test_a_z_time_counts_and_a_time_with_no_offset_does_not(hist, tmp_path): + out = tmp_path / "tasks" + _replay(hist["root"], _pages(_timed_pr7(hist, "2026-10-02T12:00:00Z", "2026-10-01T00:00:00")), out) + task, gold, _ = _load(out, 7) + assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] + assert task["public_at"] == "2026-10-02T12:00:00Z" # GitHub's form; the earlier time has no offset + + def test_a_task_whose_gold_threads_have_no_time_has_no_public_at(hist, tmp_path): out = tmp_path / "tasks" _replay(hist["root"], _pages(_timed_pr7(hist, None, "yesterday")), out) From 3c1f93dc45937371e562f932e6ea4210f4932d81 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 14:28:06 +0000 Subject: [PATCH 5/8] fix(replay): leave public_at out when a gold time is unusable Settles the review point on intel/dokimi#359 that public_at was the minimum over only the gold threads with a usable time: a skipped thread posted first moved it later, so a guidance commit between the two times read as made before the answer was public. _public_at now returns None when any gold thread's first-comment createdAt is missing, not ISO 8601 or has no offset, and the build notes name those threads ("no public_at: ..."). The no-offset test now expects no public_at; a test with a missing createdAt is added. Contract 4.6 and the changelog fragment say when the key is absent. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 3 ++- dokimi/docs/repo-evaluation-contract.md | 4 +++- dokimi/dokimi/repos/replay/build.py | 21 ++++++++++++------- dokimi/tests/test_repo_replay.py | 18 ++++++++++++---- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md index 89d006c83..bc7d5f62c 100644 --- a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -2,7 +2,8 @@ - **Replay tasks and `guidance:rev:` arms record what a leak check needs** — `repo tasks replay` writes `public_at` into each `task.json`: the earliest - `createdAt` of a gold thread's first comment. A `guidance:rev:` cell's + `createdAt` of a gold thread's first comment, left out (with a note) when + a gold thread has no time with an offset. A `guidance:rev:` cell's `meta.json` `arm_rev` adds `guidance_latest` (the last commit to touch each guidance file at ``, or a path a guidance link resolves through), `guidance_commits` (the commits that touch a guidance path or a path a link diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 5630e455f..14dafbe80 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -781,7 +781,9 @@ by a reviewer: it has no line and is never gold. `source_pr`, `source_url`, `reviewed_commit`, `merge_commit`, `left_out_files` and `public_at`: the earliest `createdAt` of a gold thread's first comment, as the threads file gives it, the time the task's - answer was public (absent when no gold thread has a time). Against the + answer was public. It is absent when any gold thread's `createdAt` is + missing, not ISO 8601 or has no offset (`Z` is one), as the earliest time + is then unknown; the build's notes name those threads. Against the `committed` dates a `guidance:rev:` arm records (§3), it shows whether a guidance commit came after it. - `gold.json` holds `{id, file, line, kind, comment, addressed, reply, url}` diff --git a/dokimi/dokimi/repos/replay/build.py b/dokimi/dokimi/repos/replay/build.py index a1450f819..35fb5174f 100644 --- a/dokimi/dokimi/repos/replay/build.py +++ b/dokimi/dokimi/repos/replay/build.py @@ -119,7 +119,7 @@ def _make(h: History, manifest: RepoManifest, pr: PullRequest, opts: Options, gold = _gold(h, manifest, pr, threads, rev, opts.window, notes) if not gold: raise _Skip("no gold thread is left") - task = _task(pr, base, rev, leave_out, opts, _public_at(threads, {g["id"] for g in gold})) + task = _task(pr, base, rev, leave_out, opts, _public_at(threads, {g["id"] for g in gold}, notes)) return task, gold, _mbox(pr, rev, h.patch(base, rev, leave_out)) @@ -206,18 +206,23 @@ def _where(t: Thread) -> str: return f"{t.path}:{t.line}" if t.line else (t.path or "no path") -def _public_at(threads: list[Thread], gold: set[str]) -> str | None: - """The earliest first-comment ``createdAt`` of the threads whose id is in ``gold``, as the input gives it; - a time that is not ISO 8601 or has no offset counts as none. GitHub's ``Z`` is an offset, UTC, and counts. - ``None`` when none has one.""" - times = [] +def _public_at(threads: list[Thread], gold: set[str], notes: list[str]) -> str | None: + """The earliest first-comment ``createdAt`` of the threads whose id is in ``gold``, as the input gives it. + ``None``, with a note naming them, when any of them has no usable time: missing, not ISO 8601 or with no + offset (GitHub's ``Z`` is one, UTC). The minimum of the others could be later than the real one.""" + times, unusable = [], [] for t in (t for t in threads if t.id in gold): try: at = datetime.fromisoformat(t.created) except ValueError: - continue - if at.tzinfo is not None: # a time with no offset is not comparable with one that has it + at = None + if at is None or at.tzinfo is None: # a time with no offset is not comparable with one that has it + unusable.append(t.id) + else: times.append((at, t.created)) + if unusable: + notes.append(f"no public_at: no first-comment createdAt with an offset on {', '.join(unusable)}") + return None return min(times)[1] if times else None diff --git a/dokimi/tests/test_repo_replay.py b/dokimi/tests/test_repo_replay.py index c2e5133f6..8d18ff3cf 100644 --- a/dokimi/tests/test_repo_replay.py +++ b/dokimi/tests/test_repo_replay.py @@ -273,19 +273,29 @@ def test_public_at_is_the_earliest_gold_threads_first_comment_compared_as_times( assert task["public_at"] == "2026-10-02T20:00:00+09:00" -def test_a_z_time_counts_and_a_time_with_no_offset_does_not(hist, tmp_path): +def test_a_gold_thread_whose_time_has_no_offset_leaves_public_at_out(hist, tmp_path, capsys): + """The thread with no offset may be the first: the other's ``Z`` time could be later than the real one.""" out = tmp_path / "tasks" _replay(hist["root"], _pages(_timed_pr7(hist, "2026-10-02T12:00:00Z", "2026-10-01T00:00:00")), out) task, gold, _ = _load(out, 7) - assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] - assert task["public_at"] == "2026-10-02T12:00:00Z" # GitHub's form; the earlier time has no offset + assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] and "public_at" not in task + assert "no public_at: no first-comment createdAt with an offset on pr7-t2" in capsys.readouterr().out + + +def test_a_gold_thread_with_no_created_at_leaves_public_at_out(hist, tmp_path, capsys): + out = tmp_path / "tasks" + _replay(hist["root"], _pages(_timed_pr7(hist, None, "2026-10-02T12:00:00Z")), out) + task, gold, _ = _load(out, 7) + assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] and "public_at" not in task + assert "no public_at: no first-comment createdAt with an offset on pr7-t1" in capsys.readouterr().out -def test_a_task_whose_gold_threads_have_no_time_has_no_public_at(hist, tmp_path): +def test_a_task_whose_gold_threads_have_no_time_has_no_public_at(hist, tmp_path, capsys): out = tmp_path / "tasks" _replay(hist["root"], _pages(_timed_pr7(hist, None, "yesterday")), out) task, gold, _ = _load(out, 7) assert [g["id"] for g in gold] == ["pr7-t1", "pr7-t2"] and "public_at" not in task + assert "no public_at: no first-comment createdAt with an offset on pr7-t1, pr7-t2" in capsys.readouterr().out def test_addressed_counts_only_the_prs_own_commits_not_main_merged_in(hist, tmp_path): From 06f64a6945192a12be61121c0fb2e8181ab8a95e Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 14:30:36 +0000 Subject: [PATCH 6/8] fix(repo): leave eval_dir paths out of the guidance history Settles the review point on intel/dokimi#359 that the history counted paths under eval_dir, which the arm never installs: with **/AGENTS.md, a commit that only edited a task fixture's AGENTS.md was listed in guidance_commits and took a place under the cap. arm_rev_history takes eval_dir (manifest.run_paths()[0], as _guidance_at uses), it is part of the cache and lock key, and _counts_for skips a path within it before the glob match. A link chain needs no filter: _link_chain refuses one that reaches into eval_dir. The contract (section 3) and the changelog fragment say so. Signed-off-by: Anton Volkov --- ...01_120000_antonwolfy_replay-leak-record.md | 3 +- dokimi/docs/repo-evaluation-contract.md | 3 +- dokimi/dokimi/repos/guidance_history.py | 36 +++++++++++-------- dokimi/dokimi/repos/workspace.py | 2 +- dokimi/tests/test_repo_guidance_history.py | 25 ++++++++++--- 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md index bc7d5f62c..59af25de6 100644 --- a/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md +++ b/dokimi/changelog.d/20261101_120000_antonwolfy_replay-leak-record.md @@ -6,7 +6,8 @@ a gold thread has no time with an offset. A `guidance:rev:` cell's `meta.json` `arm_rev` adds `guidance_latest` (the last commit to touch each guidance file at ``, or a path a guidance link resolves through), - `guidance_commits` (the commits that touch a guidance path or a path a link + `guidance_commits` (the commits that touch a guidance path outside + `eval_dir` or a path a link resolves through, a merge when its own result changes one, newest first by committer date, at most 200) and `guidance_commits_truncated`, each commit with its author and committer date (a UTC date as `Z`), so a reader can see whether diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 14dafbe80..92fa4e987 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -397,7 +397,8 @@ each guidance file the arm wrote, the commit reachable from `` that last touched it, or for a link it or any path its resolution goes through (each intermediate link and the file it resolves to), by committer date; `guidance_commits`, the commits reachable from `` that touch a path the -guidance globs match or a path a guidance link resolves through, newest first by committer +guidance globs match outside `eval_dir` (which the arm leaves out, as it does +the files) or a path a guidance link resolves through, newest first by committer date, at most 200, each with its `subject`; and `guidance_commits_truncated`, `true` when that cap cut the list. Each commit has its `sha`, `authored` and `committed` (strict ISO 8601, `%aI` and `%cI`; a UTC date is written `Z`, diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index 934bb832f..16ba1cf9a 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -7,7 +7,8 @@ encode that review. :func:`arm_rev_history` records what a reader needs to check that: the commit that last touched each guidance file at the revision, and the newest commits that touch a guidance path, each with its author and committer date. The flag reads the committer date: ``git commit ---amend`` and a rebase keep the author date (https://github.com/intel/dokimi/issues/290). +--amend`` and a rebase keep the author date (https://github.com/intel/dokimi/issues/290). A path +under the manifest's ``eval_dir`` is not guidance, as the arm installs none of them. The history is one ``git log -z --raw`` over the revision, filtered with :func:`~.manifest.match_glob`. ``-z`` ends every field with a NUL, and each file the commit touched is a ``:``-led status record @@ -47,7 +48,7 @@ from dokimi import process from .errors import PrepError -from .manifest import match_glob +from .manifest import _is_within, match_glob #: At most this many commits in ``guidance_commits``; ``guidance_latest`` is not capped GUIDANCE_COMMITS_CAP = 200 @@ -77,13 +78,13 @@ def dates(self) -> dict[str, str]: return {"sha": self.sha, "authored": self.authored, "committed": self.committed} -def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list[str], +def arm_rev_history(git_dir: Path, globs: tuple[str, ...], eval_dir: str, sha: str, files: list[str], links: dict[str, list[str]]) -> dict[str, Any]: """``guidance_latest`` for each of ``files`` (the guidance at ``sha``; ``links`` maps those that are links to every path they resolve through, the file they resolve to last), ``guidance_commits`` (newest first by committer date, at most :data:`GUIDANCE_COMMITS_CAP`) and - ``guidance_commits_truncated``.""" - commits = _guidance_commits(git_dir, sha, globs, tuple(sorted((k, tuple(v)) for k, v in links.items()))) + ``guidance_commits_truncated``. A path under ``eval_dir`` is not guidance: the arm installs none.""" + commits = _guidance_commits(git_dir, sha, globs, eval_dir, tuple(sorted((k, tuple(v)) for k, v in links.items()))) latest: dict[str, dict[str, str]] = {} for c in commits: # newest first: the first commit naming a file is its latest for path in c.paths: @@ -97,20 +98,22 @@ def arm_rev_history(git_dir: Path, globs: tuple[str, ...], sha: str, files: list _key_locks_guard = threading.Lock() -def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _Links) -> tuple[_Commit, ...]: +def _guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], eval_dir: str, + links: _Links) -> tuple[_Commit, ...]: """:func:`_read_guidance_commits`, one thread per key at a time: a cell that asks while another cell reads the same history waits for that read and takes its answer from the cache.""" - key = (git_dir, sha, globs, links) + key = (git_dir, sha, globs, eval_dir, links) with _key_locks_guard: lock = _key_locks.setdefault(key, threading.Lock()) with lock: - return _read_guidance_commits(git_dir, sha, globs, links) + return _read_guidance_commits(*key) @lru_cache(maxsize=64) -def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], links: _Links) -> tuple[_Commit, ...]: - """The commits reachable from ``sha`` that touch a path ``globs`` match or a path a link of ``links`` - resolves through, newest first by committer date.""" +def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], eval_dir: str, + links: _Links) -> tuple[_Commit, ...]: + """The commits reachable from ``sha`` that touch a path ``globs`` match outside ``eval_dir`` or a + path a link of ``links`` resolves through, newest first by committer date.""" linked: dict[str, list[str]] = {} for link, chain in links: for path in chain: @@ -118,16 +121,19 @@ def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], link _refuse_shallow(git_dir) found = [] for (commit, authored, committed, subject), touched in _parse_log(_git_log(git_dir, sha)): - paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, linked))) + paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, eval_dir, linked))) if paths: found.append(_Commit(commit, authored, committed, subject, paths)) # stable: commits with one committer date keep git's order return tuple(sorted(found, key=lambda c: datetime.fromisoformat(c.committed), reverse=True)) -def _counts_for(path: str, globs: tuple[str, ...], linked: dict[str, list[str]]) -> list[str]: - """The guidance paths a change to ``path`` counts for: itself when a glob matches it, and each link through it.""" - return ([path] if any(match_glob(g, path) for g in globs) else []) + linked.get(path, []) +def _counts_for(path: str, globs: tuple[str, ...], eval_dir: str, linked: dict[str, list[str]]) -> list[str]: + """The guidance paths a change to ``path`` counts for: itself when a glob matches it outside ``eval_dir`` + (as :func:`~.workspace._guidance_at` lists them), and each link through it (no chain reaches into + ``eval_dir``: :func:`~.workspace._link_chain` refuses one).""" + own = not _is_within(path, eval_dir) and any(match_glob(g, path) for g in globs) + return ([path] if own else []) + linked.get(path, []) def _parse_log(out: bytes) -> list[tuple[tuple[str, str, str, str], list[str]]]: diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 8837e17cb..0cbfe37f6 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -505,7 +505,7 @@ def _apply_rev(ws: Path, git_dir: Path, manifest: RepoManifest, arm: Arm, if mode == "100755": dest_path.chmod(dest_path.stat().st_mode | 0o111) # arm_files: the files it wrote and the base's guidance it deleted (contract §3 meta.json) - history = arm_rev_history(git_dir, manifest.guidance_globs, arm.rev_sha, written, chains) + history = arm_rev_history(git_dir, manifest.guidance_globs, manifest.run_paths()[0], arm.rev_sha, written, chains) return sorted(set(written) | set(removed)), {"arm_rev": {"rev": arm.rev, "sha": arm.rev_sha, "removed": sorted(set(removed) - set(written)), **history}} diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py index 225b86042..09481233d 100644 --- a/dokimi/tests/test_repo_guidance_history.py +++ b/dokimi/tests/test_repo_guidance_history.py @@ -125,6 +125,21 @@ def test_an_older_revision_records_only_its_own_history(hist): assert rev["guidance_latest"]["sub/AGENTS.md"]["sha"] == hist["first"] +def test_a_commit_that_only_edits_a_guidance_named_file_under_eval_dir_is_not_listed(hist): + """``**/AGENTS.md`` matches a task fixture's AGENTS.md too; the arm installs nothing under eval_dir.""" + eval_dir = hist["manifest"].run_paths()[0] + assert eval_dir == ".agent-eval" + fixture = _commit_at(hist["root"], {f"{eval_dir}/tasks/x/AGENTS.md": "# a task's fixture\n"}, "task fixture", + "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00") + root = _commit_at(hist["root"], {"AGENTS.md": "# root guidance\n"}, "root guidance", "2030-05-02T00:00:00+00:00", + "2030-05-02T00:00:00+00:00") + rev = _prep(hist, f"guidance:rev:{root}")["arm_rev"] + assert [c["sha"] for c in rev["guidance_commits"]][:2] == [root, hist["added"]] + assert fixture not in {c["sha"] for c in rev["guidance_commits"]} + assert rev["guidance_latest"]["AGENTS.md"]["sha"] == root and not any(p.startswith(eval_dir) + for p in rev["guidance_latest"]) + + def test_raw_and_patch_arms_record_no_guidance_history(hist): patch = hist["tmp"] / "tooling.diff" patch.write_text(TOOLING_PATCH) @@ -162,7 +177,7 @@ def log(*args): # holds the first read until every cell has asked; without a lo monkeypatch.setattr(guidance_history, "_guidance_commits", asking) monkeypatch.setattr(guidance_history, "_git_log", log) - args = (hist["git_dir"], hist["manifest"].guidance_globs, hist["added"], ["AGENTS.md"], {}) + args = (hist["git_dir"], hist["manifest"].guidance_globs, ".agent-eval", hist["added"], ["AGENTS.md"], {}) with ThreadPoolExecutor(n) as pool: results = list(pool.map(lambda _: guidance_history.arm_rev_history(*args), range(n))) assert all_asked.is_set() and calls == [(hist["git_dir"], hist["added"])] @@ -209,7 +224,7 @@ def test_a_carriage_return_in_a_guidance_path_or_a_subject_is_kept(hist): """``process.git``'s text turns ``\\r`` into a newline: the log is split as bytes, then decoded.""" cr = _commit_at(hist["root"], {"a\rb/AGENTS.md": "# cr\n"}, "guidance\redit", "2030-05-01T00:00:00+00:00", "2030-05-01T00:00:00+00:00") - rev = guidance_history.arm_rev_history(hist["git_dir"], hist["manifest"].guidance_globs, cr, + rev = guidance_history.arm_rev_history(hist["git_dir"], hist["manifest"].guidance_globs, ".agent-eval", cr, ["AGENTS.md", "a\rb/AGENTS.md"], {}) assert rev["guidance_latest"]["a\rb/AGENTS.md"] == { "sha": cr, "authored": "2030-05-01T00:00:00Z", "committed": "2030-05-01T00:00:00Z"} @@ -242,9 +257,9 @@ def test_a_shallow_git_dir_is_refused_and_a_full_clone_gives_the_whole_history(t root, shas = _three_commits(tmp_path) shallow = _clone(tmp_path, root, "shallow", "--depth", "1") with pytest.raises(PrepError, match=r"shallow/\.git is a shallow clone:.*full history \(git fetch --unshallow\)"): - guidance_history.arm_rev_history(shallow / ".git", ("AGENTS.md",), shas["code"], ["AGENTS.md"], {}) + guidance_history.arm_rev_history(shallow / ".git", ("AGENTS.md",), ".agent-eval", shas["code"], ["AGENTS.md"], {}) full = _clone(tmp_path, root, "full") - rev = guidance_history.arm_rev_history(full / ".git", ("AGENTS.md",), shas["code"], ["AGENTS.md"], {}) + rev = guidance_history.arm_rev_history(full / ".git", ("AGENTS.md",), ".agent-eval", shas["code"], ["AGENTS.md"], {}) assert rev["guidance_latest"]["AGENTS.md"]["sha"] == shas["v2"] assert [c["subject"] for c in rev["guidance_commits"]] == ["guidance v2", "guidance v1"] @@ -259,7 +274,7 @@ def test_a_blobless_clone_gives_the_history_without_fetching_a_blob(tmp_path): blobless = _clone(tmp_path, root, "blobless.git", "--bare", "--filter=blob:none") assert git(blobless, "config", "remote.origin.promisor") == "true" git(blobless, "remote", "set-url", "origin", str(tmp_path / "gone")) # a blob it needed could not be fetched - rev = guidance_history.arm_rev_history(blobless, ("AGENTS.md",), merge, ["AGENTS.md"], {}) + rev = guidance_history.arm_rev_history(blobless, ("AGENTS.md",), ".agent-eval", merge, ["AGENTS.md"], {}) assert [c["sha"] for c in rev["guidance_commits"]] == [merge, side, shas["v2"], shas["v1"]] assert rev["guidance_latest"]["AGENTS.md"]["sha"] == merge From bcb96c12cdf4753b74461f9711bcb18adaf8d369 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 14:31:31 +0000 Subject: [PATCH 7/8] perf(repo): match each history path once; note the read's cost Settles the review point on intel/dokimi#359 that the first guidance:rev: cell reads and parses the whole history of the repository. The read is not narrowed with pathspecs here: git log -- turns on history simplification, and git's :(glob) matching is not match_glob's, so a pre-filter needs a proof that it keeps every commit the matcher would. The Python side was mostly match_glob, which builds its pattern on every call; it now runs once per distinct path of a read. On a generated history of 42,000 commits (5-6 paths each), git log took about 1.0 s (28.8 MB) and the parse and match 0.92 s, now 0.12 s, with the same commits. The module docstring states the cost. Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/guidance_history.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index 16ba1cf9a..e2aa65c69 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -17,8 +17,11 @@ split on NUL as bytes and each field then decoded as UTF-8 (``--encoding=UTF-8``; a byte that is not UTF-8 becomes U+FFFD, as :mod:`dokimi.process` decodes), so a ``\\r`` in a subject or a path stays one. A date is git's ``%aI`` / ``%cI`` with a UTC offset written ``Z``, which git 2.51 prints and older git -(2.43) prints as ``+00:00``. The history is read once per git dir, revision, globs and links in a -process, also when cells of the arm prepare at once: every cell asks the same. A key of the +(2.43) prints as ``+00:00``. The history is read once per git dir, revision, globs, ``eval_dir`` and +links in a process, also when cells of the arm prepare at once: every cell asks the same. The read +covers every commit reachable from the revision and every path each one touched, so its cost grows +with the history: for 42,000 commits, about a second of ``git log`` (some 20-30 MB) and a fraction of +one to parse it, the globs matched once per distinct path; the other cells of the arm wait. A key of the repository's own config that changes what the log prints (as checked with git 2.51) is overridden by a flag (:data:`_PINNED`). A shallow git dir is refused: its history stops at the boundary commit, so it would name a later commit as the last to touch a file. @@ -41,7 +44,7 @@ from collections.abc import Iterator from dataclasses import dataclass from datetime import datetime -from functools import lru_cache +from functools import cache, lru_cache from pathlib import Path from typing import Any @@ -120,8 +123,10 @@ def _read_guidance_commits(git_dir: Path, sha: str, globs: tuple[str, ...], eval linked.setdefault(path, []).append(link) _refuse_shallow(git_dir) found = [] + # once per distinct path: a path recurs in many commits, and match_glob builds its pattern on every call + counts_for = cache(lambda p: _counts_for(p, globs, eval_dir, linked)) for (commit, authored, committed, subject), touched in _parse_log(_git_log(git_dir, sha)): - paths = tuple(dict.fromkeys(g for p in touched for g in _counts_for(p, globs, eval_dir, linked))) + paths = tuple(dict.fromkeys(g for p in touched for g in counts_for(p))) if paths: found.append(_Commit(commit, authored, committed, subject, paths)) # stable: commits with one committer date keep git's order From 3f2895a6c72095a94ca18ebd500a4029f035673c Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 14:34:27 +0000 Subject: [PATCH 8/8] refactor(repo): one isolated git env and one shallow check Settles the review nit on intel/dokimi#359 that guidance_history built again the isolated git environment workspace._git_env defines, and repeated base_opener's shallow check. source.isolated_git_env() is the one policy (PATH, LANG, TMPDIR and SYSTEMROOT kept; no system or global config, HOME at devnull, no prompt): workspace._git_env adds the fixed author and date to it, and the history's reads use it. source.is_shallow(git_dir, env) is the one check, called by base_opener and by the history's refusal. The history passes its own environment: source's default inherits the caller's, and a GIT_DIR there wins over -C, so the check would answer for another repository than the log after it reads. No behaviour change: the existing tests are unchanged, and a new one pins that a git dir git cannot read is still a PrepError, not source's SourceError. Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/guidance_history.py | 18 ++++++++++------ dokimi/dokimi/repos/source.py | 25 +++++++++++++++++----- dokimi/dokimi/repos/workspace.py | 8 +++---- dokimi/tests/test_repo_guidance_history.py | 7 ++++++ 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/dokimi/dokimi/repos/guidance_history.py b/dokimi/dokimi/repos/guidance_history.py index e2aa65c69..75b8d3868 100644 --- a/dokimi/dokimi/repos/guidance_history.py +++ b/dokimi/dokimi/repos/guidance_history.py @@ -38,7 +38,6 @@ from __future__ import annotations -import os import re import threading from collections.abc import Iterator @@ -52,6 +51,7 @@ from .errors import PrepError from .manifest import _is_within, match_glob +from .source import SourceError, is_shallow, isolated_git_env #: At most this many commits in ``guidance_commits``; ``guidance_latest`` is not capped GUIDANCE_COMMITS_CAP = 200 @@ -170,7 +170,11 @@ def _utc_as_z(date: str) -> str: def _refuse_shallow(git_dir: Path) -> None: - if _git(git_dir, "rev-parse", "rev-parse", "--is-shallow-repository").strip() == b"true": + try: # in the log's own environment: a caller's GIT_DIR would answer for another repository + shallow = is_shallow(git_dir, _env()) + except SourceError as exc: + raise PrepError(str(exc)) from exc + if shallow: raise PrepError(f"{git_dir} is a shallow clone: the guidance history of a guidance:rev: arm needs its full " "history (git fetch --unshallow)") @@ -184,12 +188,14 @@ def _git_log(git_dir: Path, sha: str) -> bytes: def _git(git_dir: Path, what: str, *args: str) -> bytes: """The stdout of ``git -C git_dir *args`` as bytes, with no system or global config; ``what`` names it in an error.""" - keep = ("PATH", "LANG", "TMPDIR", "SYSTEMROOT") - env = process.Env.only(keep, {"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "HOME": os.devnull, - "GIT_TERMINAL_PROMPT": "0"}) - done = process.git("-C", str(git_dir), *args, timeout=_GIT_TIMEOUT_S, env=env) + done = process.git("-C", str(git_dir), *args, timeout=_GIT_TIMEOUT_S, env=_env()) if done.timed_out: raise PrepError(f"git {what}: timed out after {_GIT_TIMEOUT_S:.0f} seconds") if done.returncode: raise PrepError(f"git {what} exited {done.returncode}: {process.git_failure(done)}") return done.stdout_bytes + + +def _env() -> process.Env: + """git's environment for the history: :func:`~.source.isolated_git_env`, as the arm's own reads have it.""" + return process.Env.only((), isolated_git_env()) diff --git a/dokimi/dokimi/repos/source.py b/dokimi/dokimi/repos/source.py index 84d9e0c50..1083fa8aa 100644 --- a/dokimi/dokimi/repos/source.py +++ b/dokimi/dokimi/repos/source.py @@ -385,7 +385,7 @@ def base_opener(source: RepoSource, tree: RepoTree) -> Callable[[str], RepoTree] git_dir = tree.root else: return None - shallow = _git(git_dir, "rev-parse", "--is-shallow-repository").strip() == "true" + shallow = is_shallow(git_dir) def open_base(rev: str) -> RepoTree: try: @@ -400,6 +400,20 @@ def open_base(rev: str) -> RepoTree: return open_base +def is_shallow(git_dir: Path, env: process.Env = process.GIT_ENV) -> bool: + """Whether ``git_dir`` is a shallow clone, whose history stops at its boundary commits. ``env``: + git's environment, the caller's by default; a ``GIT_DIR`` in it wins over ``git_dir``.""" + return _git(git_dir, "rev-parse", "--is-shallow-repository", env=env).strip() == "true" + + +def isolated_git_env() -> dict[str, str]: + """git with no user or system configuration (hooks, signing, templates, credential helpers) and no + prompt: of the caller's environment only ``PATH``, ``LANG``, ``TMPDIR`` and ``SYSTEMROOT``.""" + keep = {k: v for k, v in os.environ.items() if k in ("PATH", "LANG", "TMPDIR", "SYSTEMROOT")} + return {**keep, "GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "GIT_TERMINAL_PROMPT": "0", + "HOME": os.devnull} + + 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) @@ -480,8 +494,8 @@ def _norm(path: str) -> str: return PurePosixPath(path).as_posix().removeprefix("./").rstrip("/") or "." -def _git(git_dir: Path | None, *args: str, check: bool = True) -> str: - raw = _run_git(git_dir, list(args), None, check).stdout_bytes +def _git(git_dir: Path | None, *args: str, check: bool = True, env: process.Env = process.GIT_ENV) -> str: + raw = _run_git(git_dir, list(args), None, check, env).stdout_bytes return raw.decode("utf-8").replace("\r\n", "\n").replace("\r", "\n") # strict, as a blob read always was @@ -503,11 +517,12 @@ def _git_bytes(git_dir: Path, args: list[str], stdin: bytes) -> bytes: return _run_git(git_dir, args, stdin, True).stdout_bytes -def _run_git(git_dir: Path | None, args: list[str], stdin: bytes | None, check: bool) -> process.Completed: +def _run_git(git_dir: Path | None, args: list[str], stdin: bytes | None, check: bool, + env: process.Env = process.GIT_ENV) -> process.Completed: # core.fsmonitor=false: a repository's own config must not run a command when it is read. where = ["-c", "core.fsmonitor=false"] + (["-C", str(git_dir)] if git_dir is not None else []) try: - proc = process.git(*where, *args, input=stdin, timeout=_GIT_TIMEOUT_S) + proc = process.git(*where, *args, input=stdin, timeout=_GIT_TIMEOUT_S, env=env) except OSError as exc: raise SourceError(f"git {args[0]} failed: {exc}") from exc if proc.timed_out: diff --git a/dokimi/dokimi/repos/workspace.py b/dokimi/dokimi/repos/workspace.py index 0cbfe37f6..33df2be15 100644 --- a/dokimi/dokimi/repos/workspace.py +++ b/dokimi/dokimi/repos/workspace.py @@ -54,7 +54,7 @@ from .guidance_history import arm_rev_history from .harness_profile import HarnessProfile from .manifest import RepoManifest, _is_within, match_glob -from .source import GitTree, RepoSource, RepoTree, SourceError, cache_path, default_cache_dir, resolve_rev +from .source import GitTree, RepoSource, RepoTree, SourceError, cache_path, default_cache_dir, isolated_git_env, resolve_rev from .uptake import MCP_CONFIG, base_servers, mcp_servers ARMS = ("guidance:none", "guidance:raw", "guidance:bridge") # the guidance parts @@ -647,11 +647,9 @@ def _has_object(git_dir: Path, rev: str) -> bool: def _git_env(author: tuple[str, str]) -> dict[str, str]: - """git with no user or system configuration (hooks, signing, templates) and a fixed identity and date.""" + """git with no user or system configuration (:func:`~.source.isolated_git_env`) and a fixed identity and date.""" name, email = author - keep = {k: v for k, v in os.environ.items() if k in ("PATH", "LANG", "TMPDIR", "SYSTEMROOT")} - return {**keep, "GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, "GIT_TERMINAL_PROMPT": "0", - "HOME": os.devnull, "GIT_AUTHOR_NAME": name, "GIT_AUTHOR_EMAIL": email, "GIT_COMMITTER_NAME": name, + return {**isolated_git_env(), "GIT_AUTHOR_NAME": name, "GIT_AUTHOR_EMAIL": email, "GIT_COMMITTER_NAME": name, "GIT_COMMITTER_EMAIL": email, "GIT_AUTHOR_DATE": FIXED_DATE, "GIT_COMMITTER_DATE": FIXED_DATE} diff --git a/dokimi/tests/test_repo_guidance_history.py b/dokimi/tests/test_repo_guidance_history.py index 09481233d..51c2e5179 100644 --- a/dokimi/tests/test_repo_guidance_history.py +++ b/dokimi/tests/test_repo_guidance_history.py @@ -264,6 +264,13 @@ def test_a_shallow_git_dir_is_refused_and_a_full_clone_gives_the_whole_history(t assert [c["subject"] for c in rev["guidance_commits"]] == ["guidance v2", "guidance v1"] +def test_a_git_dir_git_cannot_read_is_a_prep_error(tmp_path): + """The shallow check runs first; its failure fails the cell's preparation, not the run.""" + (tmp_path / "not-a-repo").mkdir() + with pytest.raises(PrepError, match="git rev-parse failed"): + guidance_history.arm_rev_history(tmp_path / "not-a-repo", ("AGENTS.md",), ".agent-eval", "a" * 40, [], {}) + + def test_a_blobless_clone_gives_the_history_without_fetching_a_blob(tmp_path): """``log --raw`` compares object ids only, also for a merge: the clone's origin is gone when it is read.""" root, shas = _three_commits(tmp_path)