diff --git a/CHANGELOG.md b/CHANGELOG.md index b73e270b..8e2982b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.23] - 2026-08-05 + +### Changed + +- **`mini_swe_agent_v2` submits a pull request instead of writing `patch.txt`.** Submission was a unified diff the agent wrote to a local file, which nothing else could see — so the artifact that got graded was decoupled from anything a colleague could read, and an agent could submit work it had never shared. The agent now commits what it wants to submit, pushes its branch, and opens a PR; the PR is what is graded. A `gh` shim (`connectors/gh_shim.sh`) implements `gh pr create/list/view/diff/checkout` over the shared remote using plain git, so agents use the spelling they already know rather than a command invented for this benchmark. A PR **tracks its branch**, as on a forge: agents are told to open one early so a colleague has context, and later commits are included once pushed. + + Consequences worth knowing: an agent that never opens a PR submits nothing and scores zero — that is an agent failure, and it is logged (`NO PR OPENED by `) so it stays attributable rather than looking like failed tests. Solo runs use the same path against a bare repo inside their own sandbox, so there is one submission mechanism rather than two that can drift. Both prompts got *smaller*: `patch.txt` needed ~30 lines teaching a diff incantation that exists nowhere in real engineering, while `gh pr create` needs none. + +- **The shared remote is now `origin`, and the clone's upstream is removed.** It was `team`, while `origin` pointed at github.com and was unreachable — so the name agents reach for by reflex was the one that could not work. + +### Fixed + +- **Coop agents could read the entire upstream history, including the commits that came after the task commit.** A task image runs `git clone && git checkout `, which leaves every later commit reachable through `refs/remotes/origin/*`, tags, and the local branch the clone left at the tip — `git checkout ` only detaches HEAD, it does not move that branch. For any task derived from a real pull request, `git log --all -p` could therefore show the upstream implementation of the feature the agent was being asked to write. Setup now removes the remote, deletes remote refs, tags and leftover local branches, expires the reflog and prunes, so those objects are gone from the object database rather than merely unreferenced (verified with `git cat-file -e`). History *before* the task commit is kept — that is ordinary context, and `git log` still works. + +- **A submission could be silently re-baselined by anyone pushing to `main`.** Submissions were diffed against `origin/main`, a movable ref on a daemon that runs `--enable=receive-pack` with no access control. One `git push origin HEAD:main`, from either agent, would change what both submissions contained. The base commit is now pinned at setup. + +- **A failed `gh` shim install was a log warning.** Submission goes through `gh pr create`, so a sandbox without the shim cannot submit at all — and it would have surfaced hours later as an empty patch, indistinguishable from an agent that simply failed the task. Setup now raises. + +### Fixed + +- **Coop agents were told the shared git remote was read-only, so they never shared code.** The prompt titled the section "Shared Git Remote (read-only)", listed only `fetch`/`log`/`diff` under "Allowed (read-only)", never mentioned `push`, and instructed *"Do not merge, pull, or rebase their branch into yours"*. Both claims are wrong: `GitConnector` runs `git daemon --enable=receive-pack`, so the remote accepts writes, and grading never reads those branches — the evaluator applies each agent's submitted `patch.txt` to branches it creates itself (`eval/sandbox.py:473,480`). Measured across **117 trajectories in three `flash_10` runs: 292 `git fetch team` and 0 `git push team`.** Every one of those fetches returned the untouched baseline, because 0.0.22 only publishes an agent's patch at exit — by which point the peer can no longer act on it. Agents were following their instructions exactly; the instructions disabled the channel. + + This prompt was the only surface that said so. `GitConnector`'s own docstring already advertised `git push team ` and `git merge team/`, and the shared coop prompt used by the `claude_code` / `codex` adapters (`agents/_coop/prompt.py`) already instructs agents to commit and "push so peers can fetch you". Only `mini_swe_agent_v2`'s coop config disagreed with the infrastructure it runs on. + + The section now describes a real read/write remote, tells agents to commit and `git push team HEAD:` as they work so a colleague can read their actual diff, and replaces the blanket prohibition with the one constraint that genuinely matters: **the submitted `patch.txt` must contain only that agent's own changes**, since the two submitted patches are merged and duplicated edits break the merge. Local `fetch`/`merge`/`cherry-pick` are now explicitly allowed — the rule is about what you submit, not what you do in your worktree — with `git diff -- ` given as the way to scope a patch when both agents had to touch the same file. + +### Changed + +- Exit-time publication of `patch.txt` to `team/` (added in 0.0.22) is unchanged and still runs; it is now described as the *final* state of a branch agents are expected to have been pushing to all along, rather than the only thing that branch ever holds. + ## [0.0.22] - 2026-08-04 ### Fixed diff --git a/src/cooperbench/__about__.py b/src/cooperbench/__about__.py index bdb1092b..7c0e47d2 100644 --- a/src/cooperbench/__about__.py +++ b/src/cooperbench/__about__.py @@ -1,3 +1,3 @@ """Version information for CooperBench.""" -__version__ = "0.0.22" +__version__ = "0.0.23" diff --git a/src/cooperbench/agents/mini_swe_agent_v2/adapter.py b/src/cooperbench/agents/mini_swe_agent_v2/adapter.py index f3434803..3b8dc1f1 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/adapter.py +++ b/src/cooperbench/agents/mini_swe_agent_v2/adapter.py @@ -205,14 +205,15 @@ def run( model_cfg = {**model_cfg, "model_kwargs": model_kwargs} model = LitellmModel(model_name=model_name, **model_cfg) - # Setup git connector if enabled - if git_enabled and git_server_url and agents: - git_connector = GitConnector( - agent_id=agent_id, - agents=agents, - server_url=git_server_url, - ) - git_connector.setup(env) + # Always set up a git connector. Coop points it at the shared server; solo gets a + # bare repo in its own sandbox. Both then submit by opening a PR, so there is one + # submission path rather than two that can drift apart. + git_connector = GitConnector( + agent_id=agent_id, + agents=agents or [agent_id], + server_url=git_server_url if (git_enabled and git_server_url) else "", + ) + git_connector.setup(env) # Setup team CLI in the container if either of its consumers # (the task_list or the typed protocol verbs) is active. Both @@ -251,15 +252,16 @@ def run( status = "Error" error_msg = str(e) + # The submitted artifact is the agent's PR, not a local file. Reading a local + # patch.txt let an agent submit work it had never shared, so its colleague could not + # see what was coming and the shared remote had nothing to show. The PR is pushed, so + # what gets graded is exactly what the other agent could read. patch = "" try: - r = env.execute({"command": "cat patch.txt 2>/dev/null"}) - if r.get("returncode") == 0: - # git apply rejects diffs without a terminal newline; normalize - # to one trailing newline (matches claude_code / codex adapters). - from cooperbench.agents._coop.runtime import normalize_patch + from cooperbench.agents._coop.runtime import normalize_patch - patch = normalize_patch(r.get("output") or "") + raw = git_connector.submitted_patch(env) + patch = normalize_patch(raw) except Exception: pass diff --git a/src/cooperbench/agents/mini_swe_agent_v2/agents/default.py b/src/cooperbench/agents/mini_swe_agent_v2/agents/default.py index b5c7060f..4be1a718 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/agents/default.py +++ b/src/cooperbench/agents/mini_swe_agent_v2/agents/default.py @@ -18,7 +18,7 @@ # Name of the shared git remote created by GitConnector. Referenced in the messages the # agent sees, so it must match GitConnector.REMOTE_NAME. -GIT_REMOTE = "team" +GIT_REMOTE = "origin" class AgentConfig(BaseModel): @@ -201,73 +201,25 @@ def run(self, task: str = "", **kwargs) -> dict: finally: self.save(self.config.output_path) if self.messages[-1].get("role") == "exit": - published = self._publish_final_work() if self.comm: - self.comm.mark_exited(published=published) + # `published` means "the peer can see my work on the remote". That is now + # true exactly when the agent opened a PR, which it does itself -- there + # is no separate publish step to perform on its behalf. + self.comm.mark_exited(published=self._opened_pr()) break return self.messages[-1].get("extra", {}) - def _publish_final_work(self) -> bool: - """Publish this agent's *submitted patch* to the shared remote before exiting. + def _opened_pr(self) -> bool: + """Whether this agent's PR exists on the shared remote. - The remote is seeded with the base commit at setup and never updated again, so a - peer inspecting ``{GIT_REMOTE}/`` sees the untouched baseline no matter how - much work was done — and reasonably concludes their colleague has not started. - The prompt presents that remote as the sanctioned way to see a colleague's code, - so today it is a documented capability that silently returns nothing. - - We publish ``patch.txt``, not the working tree: patch.txt is the artifact that - gets evaluated and merged, and agents are free to submit a subset of their edits. - Showing a peer the tree would show them something other than what will be merged. - - Built in a detached worktree so the agent's own branch, index and working tree - are untouched — the adapter still reads patch.txt from the container afterwards. - - Returns True only when the patch actually reached the remote. Peers are told to - read that branch, so a failed publication must not be reported as a success. - Best-effort otherwise: publication must never fail a run. + Replaces `_publish_final_work`, which pushed `patch.txt` to the agent's branch at + exit. Submission is now a PR the agent opens itself, so there is nothing left to + publish -- that method only logged `no patch.txt to publish` on every run. """ - if not getattr(self, "comm", None): - return False # solo run: nobody to publish to - agent_id = self.comm.agent_id - script = ( - "set -e; " - # the repo is normally /workspace/repo, but fall back rather than guess - 'repo=/workspace/repo; [ -d "$repo/.git" ] || repo=/workspace; cd "$repo"; ' - # No patch means nothing to publish. Exit non-zero so this is NOT reported as - # a successful publication -- claiming the branch holds their submission when - # it holds the baseline is the exact failure this change removes. - 'test -s patch.txt || { echo "no patch.txt to publish"; exit 3; }; ' - # Branch from the pristine base, not from HEAD: the evaluator applies patch.txt - # to the base, so mirroring that is what makes the branch equal the submission. - # If the agent committed its work, HEAD already contains it and applying the - # patch on top would double-apply. setup() seeds the remote's main with base. - f"git fetch -q {GIT_REMOTE} 2>/dev/null || true; " - f"base=$(git rev-parse {GIT_REMOTE}/main 2>/dev/null " - "|| git rev-parse main 2>/dev/null || git rev-parse HEAD); " - 'tmp=$(mktemp -d); git worktree add -q --detach "$tmp" "$base"; ' - 'cp patch.txt "$tmp/.__submitted.patch"; cd "$tmp"; ' - "git apply --ignore-whitespace .__submitted.patch || git apply --3way .__submitted.patch; " - "rm -f .__submitted.patch; git add -A; " - f"git -c user.email=agent@cooperbench -c user.name={agent_id} " - f'commit -q --allow-empty -m "submitted work by {agent_id}"; ' - f"git push -q -f {GIT_REMOTE} HEAD:refs/heads/{agent_id}; " - # `worktree remove` matches on git's own resolved path, which differs from - # $tmp wherever the temp dir sits behind a symlink; deleting the directory and - # pruning is equivalent and does not depend on that matching. - 'cd /; rm -rf "$tmp"; git -C "$repo" worktree prune 2>/dev/null || true' - ) - try: - result = self.env.execute({"command": script}) - if (result or {}).get("returncode") == 0: - self.log(f"PUBLISHED submitted patch to {GIT_REMOTE}/{agent_id}") - return True - self.logger.warning( - f"could not publish to {GIT_REMOTE}/{agent_id}: {(result or {}).get('output', '')[:300]}" - ) - except Exception as e: # noqa: BLE001 - never fail a run over publication - self.logger.warning(f"could not publish to {GIT_REMOTE}/{agent_id}: {e}") - return False + if not self.comm: + return False + r = self.env.execute({"command": f"git ls-remote --tags {GIT_REMOTE} refs/tags/pr/{self.comm.agent_id}"}) + return bool((r.get("output") or "").strip()) def step(self) -> list[dict]: """Query the LM, execute actions. Polls for inter-agent messages diff --git a/src/cooperbench/agents/mini_swe_agent_v2/config/coop.yaml b/src/cooperbench/agents/mini_swe_agent_v2/config/coop.yaml index 1503007d..429fe06d 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/config/coop.yaml +++ b/src/cooperbench/agents/mini_swe_agent_v2/config/coop.yaml @@ -29,6 +29,10 @@ agent: 5. Test edge cases to ensure your fix is robust 6. Submit your changes — see the **Submission** section below for the exact procedure. + Working as a team, open your pull request as soon as you know which files you are + taking, not at the end — it is how your colleague finds out what you are doing. + Keep committing and pushing to it as you go. + But you are not working solo but rather as a team, so follow the workflow above for your individual tasks but you have complete freedom to communicate with your colleague in whatever way, whenever, and however often you see fit. Think about how experienced software engineers coordinate when working on the same codebase — and do that. {% if messaging_enabled %} @@ -61,28 +65,31 @@ agent: {% endif %} {% if git_enabled %} - ## Shared Git Remote (read-only) - - A shared remote called `team` lets you see your colleague's actual code changes. Use communication as your primary way to coordinate. Use git only to peek at your colleague's code in complex cases or to verify merging towards the end. Think about how experienced software engineers would make the most of direct communication and git at various stages — and do that. + ## Shared Git Remote - **What that branch holds, and when.** `team/{{ agents | reject('equalto', agent_id) | first }}` stays at the repository's starting state until your colleague submits; their patch is published there automatically at that moment. So an empty diff early on means "they have not submitted yet" — it does NOT mean they have made no changes. Ask them directly rather than inferring from git. You are told explicitly when they finish. + `origin` is a real git remote you can push to and fetch from. Your colleague + cannot see your working tree — only what you push. - Your patches are merged automatically after you both submit. Do not merge, pull, or rebase their branch into yours, as the automatic merge will see duplicate changes and fail. + Share progress as you work: + ```bash + git add + git commit -m "wip: " + git push + ``` - Allowed (read-only): + See what your colleague is doing — their PR if they opened one, otherwise + whatever they have pushed: ```bash - git fetch team - git log team/{{ agents | reject('equalto', agent_id) | first }} --oneline - git diff HEAD...team/{{ agents | reject('equalto', agent_id) | first }} -- + gh pr list + gh pr diff {{ agents | reject('equalto', agent_id) | first }} + git fetch origin && git diff origin/main origin/{{ agents | reject('equalto', agent_id) | first }} ``` - Check for conflicts (dry-run merge): + Check whether you would collide, and settle it over messaging if so: ```bash - git merge --no-commit --no-ff team/{{ agents | reject('equalto', agent_id) | first }} && git merge --abort + git merge --no-commit --no-ff origin/{{ agents | reject('equalto', agent_id) | first }} && git merge --abort ``` - If this reports conflicts, you may coordinate with your colleague via messaging to adjust your approaches. - Do NOT run: `git merge` (without --abort), `git pull`, `git rebase`, or `git reset --hard` against your colleague's branch or `origin/main`. These will corrupt your patch. {% endif %} ## Command Execution Rules @@ -167,37 +174,24 @@ agent: ## Submission - `patch.txt` is the artifact we evaluate — write whatever unified diff - you want to submit to that file, however it makes sense given how you - worked: - - Write the patch (one common way — `git diff` of your in-place edits): + Your pull request is your submission. Open it early so your colleague knows + what you are taking. ```bash - git diff -- path/to/file1 path/to/file2 > patch.txt + git push + gh pr create --title "..." --body "..." ``` - Verify it contains what you intend: + Both PRs are merged and both feature test suites run against the result, so + your PR must contain only your own work — do not commit your colleague's + changes into your branch. - ```bash - cat patch.txt - ``` - - Submit (EXACT command required) - You MUST use this EXACT command to submit: + End the task (EXACT command required): ```bash echo COMPLETE_TASK_AND_SUBMIT_FINAL_OUTPUT ``` - The patch must be a unified diff and contain only source files you - intentionally modified. Exclude: - - - reproduction or scratch test scripts you wrote - - helper scripts or tools you created - - installation, build, packaging, or configuration files - - binaries or compiled files - Do NOT run `rm -rf .git`, `git init`, `git rm -rf .`, or `git reset --hard` inside `/workspace/repo` — these corrupt `.git/` and your patch will be diff --git a/src/cooperbench/agents/mini_swe_agent_v2/config/solo.yaml b/src/cooperbench/agents/mini_swe_agent_v2/config/solo.yaml index ec054376..99adba0d 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/config/solo.yaml +++ b/src/cooperbench/agents/mini_swe_agent_v2/config/solo.yaml @@ -99,37 +99,21 @@ agent: ## Submission - `patch.txt` is the artifact we evaluate — write whatever unified diff - you want to submit to that file, however it makes sense given how you - worked: - - Write the patch (one common way — `git diff` of your in-place edits): + Your pull request is your submission. ```bash - git diff -- path/to/file1 path/to/file2 > patch.txt + git add + git commit -m "" + git push + gh pr create --title "..." --body "..." ``` - Verify it contains what you intend: - - ```bash - cat patch.txt - ``` - - Submit (EXACT command required) - You MUST use this EXACT command to submit: + End the task (EXACT command required): ```bash echo COMPLETE_TASK_AND_SUBMIT_FINAL_OUTPUT ``` - The patch must be a unified diff and contain only source files you - intentionally modified. Exclude: - - - reproduction or scratch test scripts you wrote - - helper scripts or tools you created - - installation, build, packaging, or configuration files - - binaries or compiled files - Do NOT run `rm -rf .git`, `git init`, `git rm -rf .`, or `git reset --hard` inside `/workspace/repo` — these corrupt `.git/` and your patch will be diff --git a/src/cooperbench/agents/mini_swe_agent_v2/connectors/gh_shim.sh b/src/cooperbench/agents/mini_swe_agent_v2/connectors/gh_shim.sh new file mode 100644 index 00000000..5aead73b --- /dev/null +++ b/src/cooperbench/agents/mini_swe_agent_v2/connectors/gh_shim.sh @@ -0,0 +1,129 @@ +#!/bin/sh +# Minimal `gh` for CooperBench: pull requests against the shared `origin` remote. +# +# Agents already know `gh pr create` / `gh pr diff` — those commands are everywhere in +# training data. A bespoke command with the same behaviour would not be, so this shim keeps +# the real spelling and implements it with plain git against the team remote. +# +# A PR tracks a BRANCH, the way GitHub does: you open it early so your colleague has context +# for what you are doing, then keep committing, and the new commits are part of it +# automatically. Opening one is recorded as an annotated tag holding the title and body; the +# content is always the current tip of your branch, never a commit frozen at open time. +# +# refs/tags/pr/ -> marker, message = "\n\n<body>" +# refs/heads/<agent_id> -> the content, updated by every push +# +# There is no GitHub and no network here. Anything not implemented says so plainly rather +# than failing in a way that reads like a transient error. +set -eu + +AGENT="${COOPERBENCH_AGENT_ID:-}" +REMOTE=origin +BASE="${REMOTE}/main" + +die() { echo "$*" >&2; exit 1; } + +[ -n "$AGENT" ] || die "gh: COOPERBENCH_AGENT_ID is not set; this shim needs to know who you are." + +unsupported() { + cat >&2 <<EOF +gh: '$*' is not available in this environment. + +There is no GitHub here — PRs live on the shared 'origin' git remote. Available: + gh pr create --title T --body B open your PR (proposes your current commit) + gh pr list PRs opened so far + gh pr view <agent> title, body and summary of their PR + gh pr diff <agent> the code they are proposing + gh pr checkout <agent> check out their PR locally + +Review happens over send_message, not PR comments. +EOF + exit 2 +} + +pr_ref() { echo "refs/tags/pr/$1"; } + +fetch_prs() { git fetch -q --tags "$REMOTE" 2>/dev/null || true; git fetch -q "$REMOTE" 2>/dev/null || true; } + +cmd_create() { + title=""; body="" + while [ $# -gt 0 ]; do + case "$1" in + -t|--title) title="${2:-}"; shift 2 ;; + -b|--body) body="${2:-}"; shift 2 ;; + -T|--body-file) body="$(cat "${2:-}")"; shift 2 ;; + --draft|-d|--fill) shift ;; + *) shift ;; + esac + done + [ -n "$title" ] || die "gh pr create: --title is required." + + # Deliberately no "you have no commits yet" check: opening a PR early, before there is + # much to show, is the point -- it tells your colleague what you are taking. + # + # Refuse to publish from someone else's branch. `gh pr checkout <peer>` leaves you on + # pr-<peer>; opening a PR from there would submit your colleague's code as your own, and + # nothing downstream could tell the difference. + current="$(git rev-parse --abbrev-ref HEAD)" + if [ "$current" != "$AGENT" ]; then + die "gh pr create: you are on branch '$current', not your own branch '$AGENT'. +Switch back with: git checkout $AGENT" + fi + git push -q -f "$REMOTE" "HEAD:$AGENT" + git tag -f -a "pr/$AGENT" -m "$title + +$body" HEAD >/dev/null + git push -f -q "$REMOTE" "refs/tags/pr/$AGENT" + echo "opened PR for $AGENT: $title" + echo "further commits are included automatically — push them with: git push team HEAD:$AGENT" + git --no-pager diff --stat "$BASE" HEAD +} + +cmd_list() { + fetch_prs + found=0 + for ref in $(git for-each-ref --format='%(refname)' 'refs/tags/pr/*'); do + who="${ref##*/}" + printf '%s\t%s\n' "$who" "$(git tag -l --format='%(contents:subject)' "pr/$who")" + found=1 + done + [ "$found" = 1 ] || echo "no PRs opened yet" +} + +cmd_view() { + who="${1:?gh pr view: which agent?}" + fetch_prs + git rev-parse -q --verify "$(pr_ref "$who")" >/dev/null \ + || die "gh pr view: $who has not opened a PR yet." + git tag -l --format='%(contents)' "pr/$who" + git --no-pager diff --stat "$BASE" "$REMOTE/$who" +} + +cmd_diff() { + who="${1:?gh pr diff: which agent?}" + fetch_prs + git rev-parse -q --verify "$(pr_ref "$who")" >/dev/null \ + || die "gh pr diff: $who has not opened a PR yet." + git --no-pager diff "$BASE" "$REMOTE/$who" +} + +cmd_checkout() { + who="${1:?gh pr checkout: which agent?}" + fetch_prs + git checkout -q -B "pr-$who" "$REMOTE/$who" + echo "checked out $who's PR as branch pr-$who" +} + +[ $# -ge 1 ] || unsupported "" +[ "$1" = "pr" ] || unsupported "$@" +shift +[ $# -ge 1 ] || unsupported "pr" +sub="$1"; shift +case "$sub" in + create) cmd_create "$@" ;; + list|ls) cmd_list ;; + view) cmd_view "$@" ;; + diff) cmd_diff "$@" ;; + checkout|co) cmd_checkout "$@" ;; + *) unsupported "pr $sub" ;; +esac diff --git a/src/cooperbench/agents/mini_swe_agent_v2/connectors/git.py b/src/cooperbench/agents/mini_swe_agent_v2/connectors/git.py index b0781be6..2ea24ae8 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/connectors/git.py +++ b/src/cooperbench/agents/mini_swe_agent_v2/connectors/git.py @@ -44,7 +44,9 @@ from __future__ import annotations +import base64 import logging +from pathlib import Path from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -62,7 +64,7 @@ class GitConnector: """ # Remote name used in agent's git config - REMOTE_NAME = "team" + REMOTE_NAME = "origin" def __init__( self, @@ -82,6 +84,7 @@ def __init__( self.server_url = server_url self._logger = logging.getLogger("cooperbench.agents.mini_swe_agent_v2.git_connector") self._initialized = False + self._base_sha = "" def _exec(self, env: DockerEnvironment, command: str) -> dict: """Execute a command in the environment (v2 uses dict-based actions).""" @@ -105,11 +108,22 @@ def setup(self, env: DockerEnvironment) -> None: self._exec(env, 'git config user.email "agent@cooperbench.local"') self._exec(env, f'git config user.name "{self.agent_id}"') + # Solo runs have no shared server, but the submission path should not fork on that: + # a second mechanism for solo is exactly what let solo break silently while coop was + # being changed. A bare repo inside the agent's own sandbox gives solo the identical + # flow -- push, open a PR, graded from the PR -- with no extra infrastructure. + self._detach_upstream(env) + + server = self.server_url + if not server: + server = "/tmp/team.git" + self._exec(env, f"git init -q --bare {server} 2>/dev/null || true") + # Add shared remote - result = self._exec(env, f"git remote add {self.REMOTE_NAME} {self.server_url}") + result = self._exec(env, f"git remote add {self.REMOTE_NAME} {server}") if result.get("returncode", 0) != 0: # Remote might already exist - self._exec(env, f"git remote set-url {self.REMOTE_NAME} {self.server_url}") + self._exec(env, f"git remote set-url {self.REMOTE_NAME} {server}") # Create agent's branch self._exec(env, f"git checkout -b {self.agent_id}") @@ -123,6 +137,13 @@ def setup(self, env: DockerEnvironment) -> None: # Also push main/master as base reference self._exec(env, f"git push {self.REMOTE_NAME} HEAD:refs/heads/main --force 2>/dev/null || true") + self._install_gh_shim(env) + + # Pin the base commit. Submissions are diffed against it, and `team/main` is a + # movable ref on a daemon with no access control -- one `git push team HEAD:main`, + # from either agent, would silently re-baseline both submissions. + self._base_sha = self._exec(env, "git rev-parse HEAD").get("output", "").strip() + self._initialized = True self._logger.debug(f"Git setup complete for {self.agent_id}") @@ -130,3 +151,100 @@ def setup(self, env: DockerEnvironment) -> None: def is_initialized(self) -> bool: """Whether setup() has been called.""" return self._initialized + + def _install_gh_shim(self, env: DockerEnvironment) -> None: + """Install a minimal `gh` implementing PRs over the team remote. + + Agents know `gh pr create` and `gh pr diff`; they do not know any command we invent, + so the shim keeps the real spelling. The agent id is substituted in at install time + rather than read from the environment, because each bash call the agent makes is a + fresh shell and an exported variable would not survive between them. + """ + shim = (Path(__file__).parent / "gh_shim.sh").read_text() + shim = shim.replace( + 'AGENT="${COOPERBENCH_AGENT_ID:-}"', + f'AGENT="${{COOPERBENCH_AGENT_ID:-{self.agent_id}}}"', + ) + encoded = base64.b64encode(shim.encode()).decode() + result = self._exec( + env, + f"echo {encoded} | base64 -d > /usr/local/bin/gh && chmod +x /usr/local/bin/gh", + ) + if result.get("returncode", 0) != 0 or not self._exec(env, "command -v gh >/dev/null && echo ok").get( + "output", "" + ).strip().endswith("ok"): + # Submission goes through `gh pr create`. Without the shim the agent cannot + # submit anything at all, and it would only surface as an empty patch hours + # later, indistinguishable from an agent that simply failed the task. + raise RuntimeError(f"gh shim not installed for {self.agent_id}: {result.get('output', '')}") + + def submitted_patch(self, env: DockerEnvironment) -> str: + """The diff the agent proposed in its PR, or "" if it never opened one. + + This is the graded artifact. An agent that opened no PR submits nothing, which is the + correct outcome rather than something to paper over -- the previous mechanism let an + agent submit from a local file it never shared, so its colleague could not see what + was coming. + """ + self._exec(env, f"git fetch -q --tags {self.REMOTE_NAME} 2>/dev/null || true") + self._exec(env, f"git fetch -q {self.REMOTE_NAME} 2>/dev/null || true") + # The tag records that a PR was opened; the CONTENT is the branch tip, so commits + # pushed after opening are included -- the same way a PR on a forge updates when you + # push. Grading the tag instead would silently freeze the submission at whatever the + # agent had written the moment it opened the PR. + # Checked against the REMOTE: the agent creates the tag locally first, so a local + # check would call a PR "opened" even when the push that publishes it failed. + opened = ( + self._exec(env, f"git ls-remote --tags {self.REMOTE_NAME} refs/tags/pr/{self.agent_id}") + .get("output", "") + .strip() + ) + if not opened: + # Never opening a PR is an agent failure, not something to paper over -- it scores + # zero, correctly. Log it so that outcome is attributable afterwards, instead of + # being indistinguishable from an agent whose code simply failed the tests. + self._logger.info(f"NO PR OPENED by {self.agent_id}: submitting nothing") + return "" + result = self._exec( + env, + f"git --no-pager diff {self._base_sha} {self.REMOTE_NAME}/{self.agent_id}", + ) + return result.get("output", "") or "" + + def _detach_upstream(self, env: DockerEnvironment) -> None: + """Cut the sandbox off from the repository it was cloned from. + + The task image runs `git clone <upstream> && git checkout <task-sha>`, so the sandbox + holds the project's **entire history, including every commit after the task commit**. + Those are reachable through `refs/remotes/origin/*` and tags, which means an agent can + run `git log --all` or `git show` and, for a task derived from a real pull request, + read the upstream implementation of the very feature it is being asked to write. + + Removing the remote alone does not help: it stops the network (which is already + unreachable) and leaves every object in place. The refs have to go, and the objects + they pinned have to be pruned. + + History *before* the task commit is left intact -- that is ordinary context an engineer + would have, and `git log` should still work. + """ + self._exec(env, "git remote remove origin 2>/dev/null || true") + self._exec( + env, + "git for-each-ref --format='%(refname)' refs/remotes | xargs -r -n1 git update-ref -d 2>/dev/null || true", + ) + self._exec(env, "git tag -l | xargs -r git tag -d >/dev/null 2>&1 || true") + # `git clone` leaves a LOCAL branch (usually `main`) at the tip of the default branch, + # and `git checkout <task-sha>` only detaches HEAD -- it does not move that branch. So + # the future stays reachable through a local ref even after every remote ref and tag + # is gone. Drop every branch except the one HEAD is on. + # -n1 so a branch that cannot be deleted (the checked-out one, if HEAD is attached) + # does not abort deletion of the rest. In the task image HEAD is detached at the task + # commit, so every branch here is one the clone left behind. + self._exec( + env, + "git for-each-ref --format='%(refname:short)' refs/heads | xargs -r -n1 git branch -D >/dev/null 2>&1 || true", + ) + self._exec( + env, + "git reflog expire --expire=now --all >/dev/null 2>&1; git gc --prune=now --quiet >/dev/null 2>&1 || true", + ) diff --git a/src/cooperbench/agents/mini_swe_agent_v2/environments/modal.py b/src/cooperbench/agents/mini_swe_agent_v2/environments/modal.py index 80b0df1d..f3c7f91a 100644 --- a/src/cooperbench/agents/mini_swe_agent_v2/environments/modal.py +++ b/src/cooperbench/agents/mini_swe_agent_v2/environments/modal.py @@ -215,9 +215,14 @@ def execute(self, action: dict, cwd: str = "", *, timeout: int | None = None) -> try: if self.sb is None: raise RuntimeError("Sandbox not initialized") - proc = self.sb.exec("bash", "-lc", f"cd {cwd} && {command}") - stdout = proc.stdout.read() - stderr = proc.stderr.read() + # text=False so WE decode, leniently. Modal's text mode decodes as strict + # UTF-8 and raises on the first non-UTF-8 byte -- and that exception kills the + # agent's run. An agent only has to `cat` a binary file once (observed: + # `tail -5 .git/index`, byte 0xb3) to end its own episode, which is then + # recorded as an ordinary agent error rather than a harness limitation. + proc = self.sb.exec("bash", "-lc", f"cd {cwd} && {command}", text=False) + stdout = proc.stdout.read().decode("utf-8", errors="replace") + stderr = proc.stderr.read().decode("utf-8", errors="replace") proc.wait() output = stdout + stderr if stderr else stdout result = {"output": output, "returncode": proc.returncode, "exception_info": ""} diff --git a/tests/agents/mini_swe_agent_v2/test_gh_shim.py b/tests/agents/mini_swe_agent_v2/test_gh_shim.py new file mode 100644 index 00000000..2b33ee51 --- /dev/null +++ b/tests/agents/mini_swe_agent_v2/test_gh_shim.py @@ -0,0 +1,367 @@ +"""Tests for the `gh` shim that gives coop agents pull requests over the team remote. + +These run the real script against real git repositories — no mocks — because every property +worth asserting here is a property of git refs, and a mocked `git` would assert only that the +script calls the commands the test already assumed it would. + +The behaviour that matters most is that a PR **tracks the branch**: agents are told to open one +early so a colleague has context, then keep committing. A PR pinned to the commit that existed +at open time would silently submit a fraction of the work, and nothing downstream could tell. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +SHIM = Path(__file__).resolve().parents[3] / "src/cooperbench/agents/mini_swe_agent_v2/connectors/gh_shim.sh" + + +def run(cmd: str, cwd: Path, agent: str | None = None, check: bool = True): + env = {**os.environ, "PATH": os.environ["PATH"]} + if agent: + env["COOPERBENCH_AGENT_ID"] = agent + r = subprocess.run(cmd, shell=True, cwd=cwd, env=env, capture_output=True, text=True) + if check and r.returncode != 0: + raise AssertionError(f"{cmd}\nrc={r.returncode}\n{r.stdout}\n{r.stderr}") + return r + + +def gh(args: str, cwd: Path, agent: str, check: bool = True): + return run(f"sh {SHIM} {args}", cwd, agent=agent, check=check) + + +@pytest.fixture +def team(tmp_path): + """A bare 'origin' remote plus two agent clones, mirroring the real sandbox topology.""" + server = tmp_path / "server.git" + subprocess.run(["git", "init", "-q", "--bare", str(server)], check=True) + + seed = tmp_path / "seed" + seed.mkdir() + run("git init -q -b main .", seed) + run("git config user.email a@b.c && git config user.name t", seed) + (seed / "shared.py").write_text("def quantize(im):\n return im\n") + run("git add -A && git commit -qm base", seed) + run(f"git remote add origin {server} && git push -q origin HEAD:refs/heads/main", seed) + + clones = {} + for agent in ("agent1", "agent2"): + d = tmp_path / agent + run(f"git clone -q {server} {d}", tmp_path) + run("git config user.email a@b.c && git config user.name t", d) + run(f"git fetch -q origin && git checkout -q -B {agent} origin/main", d) + clones[agent] = d + return clones + + +def test_pr_tracks_the_branch_not_the_opening_commit(team): + """The property agents depend on: open early, keep committing, PR follows. + + If the PR froze at open time, an agent that opened a PR after its first commit would + submit only that commit — and the omission would be invisible to it, to its colleague, + and to the grader. + """ + a1, a2 = team["agent1"], team["agent2"] + + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam 'first'", a1) + gh("pr create --title 'add error_threshold' --body 'wip'", a1, "agent1") + + # More work lands after the PR was opened. + (a1 / "extra.py").write_text("HELPER = 1\n") + run("git add -A && git commit -qm 'second'", a1) + run("git push -q origin HEAD:agent1", a1) + + diff = gh("pr diff agent1", a2, "agent2").stdout + assert "error_threshold" in diff, "first commit missing from PR" + assert "extra.py" in diff, "commit made AFTER opening the PR is missing — PR froze" + + +def test_colleague_sees_the_pr_and_its_description(team): + a1, a2 = team["agent1"], team["agent2"] + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + gh("pr create --title 'error threshold' --body 'touching quantize()'", a1, "agent1") + + listed = gh("pr list", a2, "agent2").stdout + assert "agent1" in listed and "error threshold" in listed + + view = gh("pr view agent1", a2, "agent2").stdout + assert "error threshold" in view + assert "touching quantize()" in view + + +def test_pr_can_be_opened_before_any_work_exists(team): + """Opening early is the intended flow, so an empty PR must not be rejected.""" + a1, a2 = team["agent1"], team["agent2"] + gh("pr create --title 'taking quantize()' --body 'starting now'", a1, "agent1") + assert "taking quantize()" in gh("pr view agent1", a2, "agent2").stdout + + +def test_reading_a_pr_that_was_never_opened_fails_clearly(team): + r = gh("pr diff agent1", team["agent2"], "agent2", check=False) + assert r.returncode != 0 + assert "has not opened a PR" in r.stderr + + +def test_unsupported_subcommands_explain_themselves(team): + """`gh pr merge` and friends must not look like a transient failure.""" + for args in ("pr merge agent1", "pr comment agent1", "issue list"): + r = gh(args, team["agent1"], "agent1", check=False) + assert r.returncode == 2, args + assert "not available in this environment" in r.stderr + assert "gh pr create" in r.stderr, "error should name what IS available" + + +def test_create_requires_a_title(team): + r = gh("pr create --body x", team["agent1"], "agent1", check=False) + assert r.returncode != 0 + assert "--title is required" in r.stderr + + +def test_uncommitted_work_is_not_in_the_pr(team): + """The agent's control over its submission: commit what you mean, leave the rest.""" + a1, a2 = team["agent1"], team["agent2"] + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + (a1 / "scratch.py").write_text("print('DEBUG')\n") # never committed + gh("pr create --title t --body b", a1, "agent1") + + diff = gh("pr diff agent1", a2, "agent2").stdout + assert "error_threshold" in diff + assert "scratch.py" not in diff and "DEBUG" not in diff + + +def test_checkout_gives_the_peers_code(team): + a1, a2 = team["agent1"], team["agent2"] + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + gh("pr create --title t --body b", a1, "agent1") + + gh("pr checkout agent1", a2, "agent2") + assert "error_threshold" in (a2 / "shared.py").read_text() + + +def test_two_agents_open_prs_independently(team): + """One ref per agent, so simultaneous opens cannot race or clobber each other.""" + a1, a2 = team["agent1"], team["agent2"] + for d, agent, fn in ((a1, "agent1", "one.py"), (a2, "agent2", "two.py")): + (d / fn).write_text("x = 1\n") + run(f"git add -A && git commit -qm {agent}", d) + gh(f"pr create --title {agent}-work --body b", d, agent) + + listed = gh("pr list", a1, "agent1").stdout + assert "agent1" in listed and "agent2" in listed + assert "two.py" in gh("pr diff agent2", a1, "agent1").stdout + assert "one.py" in gh("pr diff agent1", a2, "agent2").stdout + + +def test_plain_git_push_reaches_the_agents_own_branch(team, tmp_path): + """The prompt tells agents to run a bare `git push`, which only works because + `GitConnector.setup` leaves them on a branch named after their agent id with an + upstream already set (`git checkout -b <id>` + `git push -u origin <id>`). + + That coupling is invisible: change setup to a detached HEAD or a differently-named + branch and the prompt's instruction silently stops publishing anything, which is + precisely the failure mode that produced 0 pushes across 117 trajectories. + """ + a1 = team["agent1"] + run("git push -u -q origin agent1", a1) # what setup() does + + assert run("git rev-parse --abbrev-ref HEAD", a1).stdout.strip() == "agent1" + assert run("git rev-parse --abbrev-ref @{u}", a1).stdout.strip() == "origin/agent1" + + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + run("git push", a1) # exactly what the prompt says + + assert ( + "error_threshold" + in run("git fetch -q origin && git --no-pager diff origin/main origin/agent1", team["agent2"]).stdout + ) + + +def test_cannot_open_a_pr_from_a_colleagues_branch(team): + """`gh pr checkout <peer>` leaves you on their code. Opening a PR from there would + submit their work as yours, and the merge would see the same edit twice — a wrong + submission that nothing downstream could detect.""" + a1, a2 = team["agent1"], team["agent2"] + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + gh("pr create --title t --body b", a1, "agent1") + + gh("pr checkout agent1", a2, "agent2") # agent2 now sits on pr-agent1 + r = gh("pr create --title stolen --body b", a2, "agent2", check=False) + assert r.returncode != 0 + assert "not your own branch" in r.stderr + assert "git checkout agent2" in r.stderr + + +def test_solo_can_open_a_pr_against_a_local_bare_repo(tmp_path): + """Solo has no shared server, but must not get a second submission mechanism. + + Two paths are how solo broke silently while coop was being changed: extraction moved to + the PR, solo kept being told to write patch.txt, and nothing read it. A bare repo inside + solo's own sandbox gives it the identical flow. + """ + work = tmp_path / "repo" + work.mkdir() + run("git init -q -b main .", work) + run("git config user.email a@b.c && git config user.name t", work) + (work / "shared.py").write_text("def quantize(im):\n return im\n") + run("git add -A && git commit -qm base", work) + + # what GitConnector.setup does when there is no shared server + bare = tmp_path / "solo_team.git" + run(f"git init -q --bare {bare}", work) + run(f"git remote add origin {bare}", work) + run("git push -q origin HEAD:refs/heads/main", work) + run("git checkout -q -b agent1 && git push -u -q origin agent1", work) + + (work / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", work) + run("git push -q", work) + gh("pr create --title solo --body b", work, "agent1") + + diff = run("git --no-pager diff origin/main origin/agent1", work).stdout + assert "error_threshold" in diff + assert "solo" in gh("pr view agent1", work, "agent1").stdout + + +def test_submission_survives_someone_moving_main(team): + """The git daemon has no access control, so either agent can push to `main`. + + Diffing a submission against the movable `origin/main` ref meant one such push — accidental + or not — silently re-baselined BOTH agents' patches. The base is pinned at setup instead. + """ + a1, a2 = team["agent1"], team["agent2"] + base = run("git rev-parse HEAD", a1).stdout.strip() + + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + run("git commit -qam work", a1) + run("git push -q origin HEAD:agent1", a1) + + # agent2 moves main to its own work — nothing stops it + (a2 / "other.py").write_text("x = 1\n") + run("git add -A && git commit -qm hijack", a2) + run("git push -q -f origin HEAD:main", a2) + run("git fetch -q origin", a1) + + against_pinned = run(f"git --no-pager diff {base} origin/agent1", a1).stdout + against_main = run("git --no-pager diff origin/main origin/agent1", a1).stdout + + assert "error_threshold" in against_pinned, "pinned base must still see agent1's work" + assert "other.py" in against_main, "diffing against main leaks the mover's changes in" + assert against_pinned != against_main + + +def test_detaching_upstream_removes_commits_after_the_task_commit(tmp_path): + """The clone carries the project's future, and that future may contain the answer. + + A task image runs `git clone <upstream> && git checkout <task-sha>`. Every commit after + the task commit is still present, reachable via refs/remotes and tags — so for a task + derived from a real PR, `git log --all` can show the upstream implementation of the + feature the agent is being asked to write. Dropping the remote alone leaves all of it. + """ + upstream = tmp_path / "upstream" + upstream.mkdir() + run("git init -q -b main .", upstream) + run("git config user.email a@b.c && git config user.name t", upstream) + (upstream / "shared.py").write_text("def quantize(im):\n return im\n") + run("git add -A && git commit -qm 'base'", upstream) + task_sha = run("git rev-parse HEAD", upstream).stdout.strip() + # the future: the real implementation, plus a release tag + (upstream / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im # THE ANSWER\n") + run("git commit -qam 'feat: add error_threshold'", upstream) + run("git tag v2.0", upstream) + + sandbox = tmp_path / "repo" + run(f"git clone -q {upstream} {sandbox}", tmp_path) + run(f"git checkout -q {task_sha}", sandbox) + + assert "THE ANSWER" in run("git log --all -p", sandbox).stdout, "fixture should leak" + + # what _detach_upstream does + run("git remote remove origin", sandbox) + run("git for-each-ref --format='%(refname)' refs/remotes | xargs -r -n1 git update-ref -d", sandbox) + run("git tag -l | xargs -r git tag -d", sandbox) + run( + "git for-each-ref --format='%(refname:short)' refs/heads | xargs -r -n1 git branch -D 2>/dev/null || true", + sandbox, + ) + run("git reflog expire --expire=now --all && git gc --prune=now --quiet", sandbox) + + assert "THE ANSWER" not in run("git log --all -p", sandbox).stdout + assert run("git tag -l", sandbox).stdout.strip() == "" + assert run("git remote", sandbox).stdout.strip() == "" + # history before the task commit survives — that is legitimate context + assert "base" in run("git log --oneline", sandbox).stdout + + +def test_extracted_patch_applies_to_the_base_and_carries_the_work(team, tmp_path): + """End-to-end on the artifact grading actually consumes. + + `submitted_patch` runs `git diff <pinned-base> origin/<agent>` and hands the result to the + evaluator, which applies it to a fresh checkout of the base. Everything upstream can be + correct — PR opened, branch pushed — and still produce a diff that does not apply, at + which point the pair is scored `missing_input` and reads as an agent failure. + """ + a1 = team["agent1"] + base = run("git rev-parse HEAD", a1).stdout.strip() + + (a1 / "shared.py").write_text("def quantize(im, error_threshold=0.0):\n return im\n") + (a1 / "helper.py").write_text("LIMIT = 5\n") + run("git add -A && git commit -qm work", a1) + (a1 / "scratch.py").write_text("print('DEBUG')\n") # never committed + run("git push -q origin HEAD:agent1", a1) + gh("pr create --title t --body b", a1, "agent1") + + # exactly what GitConnector.submitted_patch does + patch = run(f"git --no-pager diff {base} origin/agent1", a1).stdout + assert patch.strip(), "submission is empty" + + # what the evaluator does: apply it to a clean checkout of the base + fresh = tmp_path / "fresh" + run(f"git clone -q {a1} {fresh}", tmp_path) + run(f"git checkout -q {base}", fresh) + (fresh / "p.patch").write_text(patch) + run("git apply p.patch", fresh) # must not raise + + assert "error_threshold" in (fresh / "shared.py").read_text() + assert (fresh / "helper.py").exists(), "second changed file missing from submission" + assert not (fresh / "scratch.py").exists(), "uncommitted scratch leaked into submission" + + +def test_two_submissions_merge_the_way_the_evaluator_merges_them(team, tmp_path): + """The evaluator applies each patch to its own branch and merges. Disjoint work must + merge clean and both features must survive — that is the only path that scores a pass.""" + a1, a2 = team["agent1"], team["agent2"] + base = run("git rev-parse HEAD", a1).stdout.strip() + + for d, agent, fn, body in ((a1, "agent1", "feat_a.py", "A = 1\n"), (a2, "agent2", "feat_b.py", "B = 2\n")): + (d / fn).write_text(body) + run(f"git add -A && git commit -qm {agent}", d) + run(f"git push -q origin HEAD:{agent}", d) + gh(f"pr create --title {agent} --body b", d, agent) + + p1 = run(f"git --no-pager diff {base} origin/agent1", a1).stdout + run("git fetch -q origin", a2) + p2 = run(f"git --no-pager diff {base} origin/agent2", a2).stdout + + ev = tmp_path / "ev" + run(f"git clone -q {a1} {ev}", tmp_path) + run("git config user.email a@b.c && git config user.name t", ev) + # patches live OUTSIDE the repo: `git add -A` would otherwise commit them as part of the + # work, and they would disappear on the next checkout + p1f, p2f = tmp_path / "p1.patch", tmp_path / "p2.patch" + p1f.write_text(p1) + p2f.write_text(p2) + run(f"git checkout -q -B agent1 {base} && git apply {p1f} && git add -A && git commit -qm a1", ev) + run(f"git checkout -q -B agent2 {base} && git apply {p2f} && git add -A && git commit -qm a2", ev) + run("git merge --no-commit --no-ff agent1", ev) # must not conflict + + assert (ev / "feat_a.py").exists() and (ev / "feat_b.py").exists() diff --git a/tests/agents/mini_swe_agent_v2/test_peer_exit_agent.py b/tests/agents/mini_swe_agent_v2/test_peer_exit_agent.py index e33a7376..bec8f2bf 100644 --- a/tests/agents/mini_swe_agent_v2/test_peer_exit_agent.py +++ b/tests/agents/mini_swe_agent_v2/test_peer_exit_agent.py @@ -135,35 +135,44 @@ def test_solo_run_announces_nothing(self): assert agent.messages == [] -class TestPublishFinalWork: - def test_solo_run_does_not_publish(self): +class TestOpenedPR: + """`published` now means "the peer can see my work", which is true exactly when the agent + opened a PR. It replaces `_publish_final_work`, which pushed `patch.txt` to the agent's + branch at exit -- dead once submission became a PR the agent opens itself, and it logged + `no patch.txt to publish` on every run. + """ + + def test_solo_run_reports_no_pr(self): agent = DefaultAgent( _StubModel(), _StubEnv(), comm=None, agent_id="agent1", system_template="s", instance_template="i" ) - assert agent._publish_final_work() is False + assert agent._opened_pr() is False - def test_publish_reports_failure_when_the_container_command_fails(self, pair): - """ - Expected: False, so mark_exited(published=False) and peers are not misdirected - Catches: treating a failed publish as success -- e.g. the `test -s patch.txt` - guard exiting 0 when there is no patch, which would tell the peer the - branch holds a submission that was never pushed. - """ + def test_no_pr_on_the_remote_reports_false(self, pair): + """Expected: False, so mark_exited(published=False) and the peer is not pointed at a + branch holding nothing. Catches treating an empty ls-remote as success.""" alice_comm, _ = pair agent = _agent("agent1", alice_comm) - class _FailingEnv(_StubEnv): + class _NoPR(_StubEnv): def execute(self, action): - return {"output": "no patch.txt to publish", "returncode": 3} + return {"output": "", "returncode": 0} - agent.env = _FailingEnv() - assert agent._publish_final_work() is False + agent.env = _NoPR() + assert agent._opened_pr() is False - def test_publish_reports_success_and_targets_the_agent_branch(self, pair): + def test_pr_on_the_remote_reports_true_and_is_checked_remotely(self, pair): alice_comm, _ = pair agent = _agent("agent1", alice_comm) - assert agent._publish_final_work() is True + + class _HasPR(_StubEnv): + def execute(self, action): + self.commands.append(action) + return {"output": "abc123\trefs/tags/pr/agent1", "returncode": 0} + + agent.env = _HasPR() + assert agent._opened_pr() is True cmd = agent.env.commands[-1]["command"] - assert "HEAD:refs/heads/agent1" in cmd - assert "worktree add" in cmd and "patch.txt" in cmd - assert f"{GIT_REMOTE}/main" in cmd, "must branch from the pristine base, not HEAD" + assert "ls-remote" in cmd, "must ask the remote, not the local repo" + assert "refs/tags/pr/agent1" in cmd + assert GIT_REMOTE in cmd