mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-09 19:27:45 +02:00
[eric] agents: an agent is told how far behind its checkout is, so it stops diagnosing from a stale tree (ENG-280)
This commit is contained in:
@@ -11,6 +11,7 @@ from typeguard import typechecked
|
||||
|
||||
from backend.apps.agents.core.models import AgentSession
|
||||
from backend.apps.agents.manager.prompt.tool_catalog import get_all_tool_names
|
||||
from backend.apps.agents.manager.prompt.repo_staleness_note import repo_staleness_note
|
||||
from backend.apps.agents.manager.prompt.prompt_context import (
|
||||
build_app_runtime_contract,
|
||||
build_browser_context,
|
||||
@@ -45,6 +46,12 @@ def compose_turn_system_prompt(
|
||||
skills_catalog_ctx,
|
||||
)
|
||||
|
||||
# Pin the agent's notion of the checkout's vintage the same way we pin "now": an agent that
|
||||
# diagnoses from a tree 545 commits behind, with nothing saying so, is confidently wrong (ENG-280).
|
||||
staleness = repo_staleness_note(getattr(session, "cwd", None))
|
||||
if staleness:
|
||||
composed_prompt = f"{composed_prompt}\n\n{staleness}" if composed_prompt else staleness
|
||||
|
||||
# Pin the agent's notion of "now" to the host wall clock + zone so it can answer day-of-week questions without hallucinating.
|
||||
try:
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
"""Tell an agent how old the checkout it is standing in actually is (ENG-280).
|
||||
|
||||
An agent was asked to fix the Workflows UI, read a checkout **545 commits behind** origin/main,
|
||||
diagnosed against code that had not existed for weeks, and started editing. Nothing surfaced it.
|
||||
It only stopped after the user pushed back twice with "I 100% see this feature, you're hella out
|
||||
of date."
|
||||
|
||||
Reading a stale tree is legitimate; diagnosing from a tree of UNKNOWN vintage is the bug. This
|
||||
puts the vintage in context every time, so that state stops being reachable.
|
||||
|
||||
Deliberately offline: it compares against the refs already on disk and never fetches. A prompt
|
||||
builder is not a place to block on the network, and "behind by whatever we last saw" is enough to
|
||||
make an agent check before it theorises.
|
||||
"""
|
||||
import subprocess
|
||||
from typing import List, Optional
|
||||
|
||||
from typeguard import typechecked
|
||||
|
||||
P_TIMEOUT_S = 2.0
|
||||
# Below this, a checkout is normal working drift and a note would just be noise.
|
||||
MIN_BEHIND_TO_WARN = 20
|
||||
|
||||
|
||||
@typechecked
|
||||
def p_git(cwd: str, args: List[str]) -> Optional[str]:
|
||||
"""Run a read-only git command, or None if anything at all goes wrong."""
|
||||
try:
|
||||
out = subprocess.run(
|
||||
["git", *args], cwd=cwd, capture_output=True, text=True, timeout=P_TIMEOUT_S,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return None
|
||||
if out.returncode != 0:
|
||||
return None
|
||||
return out.stdout.strip()
|
||||
|
||||
|
||||
@typechecked
|
||||
def repo_staleness_note(cwd: Optional[str]) -> Optional[str]:
|
||||
"""One line naming how far behind this checkout is, or None when there is nothing to say."""
|
||||
if not cwd:
|
||||
return None
|
||||
if p_git(cwd, ["rev-parse", "--is-inside-work-tree"]) != "true":
|
||||
return None
|
||||
upstream = None
|
||||
for candidate in ("@{upstream}", "origin/main", "origin/master"):
|
||||
if p_git(cwd, ["rev-parse", "--verify", "--quiet", candidate]):
|
||||
upstream = candidate
|
||||
break
|
||||
if upstream is None:
|
||||
return None
|
||||
counts = p_git(cwd, ["rev-list", "--left-right", "--count", f"HEAD...{upstream}"])
|
||||
if not counts:
|
||||
return None
|
||||
parts = counts.split()
|
||||
if len(parts) != 2 or not parts[1].isdigit():
|
||||
return None
|
||||
behind = int(parts[1])
|
||||
if behind < MIN_BEHIND_TO_WARN:
|
||||
return None
|
||||
head = p_git(cwd, ["rev-parse", "--short", "HEAD"]) or "unknown"
|
||||
when = p_git(cwd, ["log", "-1", "--format=%cs", "HEAD"]) or "unknown date"
|
||||
return (
|
||||
f"This checkout is {behind} commits behind {upstream} (HEAD {head}, {when}). "
|
||||
"It may not contain the code the user is running, so check before diagnosing from it."
|
||||
)
|
||||
@@ -0,0 +1,122 @@
|
||||
"""An agent must know how old its checkout is before it diagnoses from it (ENG-280).
|
||||
|
||||
The incident: an agent read a tree 545 commits behind origin/main, formed a confident
|
||||
diagnosis about code that had been replaced weeks earlier, and started editing. Nothing in
|
||||
its context said the tree was stale, so the user had to notice and say so twice.
|
||||
|
||||
These build REAL git repositories rather than mocking subprocess, because the thing under
|
||||
test is exactly whether the git plumbing is invoked correctly. A mocked git proves the mock.
|
||||
|
||||
Run:
|
||||
backend/.venv/bin/python -m pytest backend/tests/test_repo_staleness_note.py -v
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
from typing import Any, List
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.apps.agents.manager.prompt.repo_staleness_note import (
|
||||
MIN_BEHIND_TO_WARN,
|
||||
repo_staleness_note,
|
||||
)
|
||||
|
||||
|
||||
def p_run(cwd: str, *args: str) -> None:
|
||||
subprocess.run(list(args), cwd=cwd, check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def p_commit(cwd: str, msg: str) -> None:
|
||||
with open(f"{cwd}/f.txt", "a") as fh:
|
||||
fh.write(msg + "\n")
|
||||
p_run(cwd, "git", "add", "f.txt")
|
||||
p_run(cwd, "git", "commit", "-m", msg, "--no-gpg-sign")
|
||||
|
||||
|
||||
P_PAIR_SEQ = [0]
|
||||
|
||||
|
||||
def p_repo_pair(tmp_path: Any, behind: int) -> str:
|
||||
"""An origin with `behind` extra commits, and a clone parked before them.
|
||||
|
||||
Each call gets its own subdirectory: two pairs in one test previously collided on the same
|
||||
path and raised FileExistsError, which read as a failure of the code under test.
|
||||
"""
|
||||
P_PAIR_SEQ[0] += 1
|
||||
tmp_path = tmp_path / f"pair{P_PAIR_SEQ[0]}"
|
||||
tmp_path.mkdir()
|
||||
origin = tmp_path / "origin"
|
||||
origin.mkdir()
|
||||
p_run(str(origin), "git", "init", "-q", "-b", "main")
|
||||
p_run(str(origin), "git", "config", "user.email", "t@t.t")
|
||||
p_run(str(origin), "git", "config", "user.name", "t")
|
||||
p_commit(str(origin), "base")
|
||||
|
||||
clone = tmp_path / "clone"
|
||||
p_run(str(tmp_path), "git", "clone", "-q", str(origin), str(clone))
|
||||
p_run(str(clone), "git", "config", "user.email", "t@t.t")
|
||||
p_run(str(clone), "git", "config", "user.name", "t")
|
||||
|
||||
for i in range(behind):
|
||||
p_commit(str(origin), f"newer-{i}")
|
||||
if behind:
|
||||
p_run(str(clone), "git", "fetch", "-q", "origin")
|
||||
return str(clone)
|
||||
|
||||
|
||||
def test_a_badly_stale_checkout_is_named_with_its_number(tmp_path: Any) -> None:
|
||||
note = repo_staleness_note(p_repo_pair(tmp_path, behind=30))
|
||||
assert note, "a 30-commit-behind checkout said nothing; this is the ENG-280 silence"
|
||||
assert "30 commits behind" in note, f"note did not carry the count: {note}"
|
||||
assert "check before diagnosing" in note
|
||||
|
||||
|
||||
def test_an_up_to_date_checkout_says_nothing(tmp_path: Any) -> None:
|
||||
assert repo_staleness_note(p_repo_pair(tmp_path, behind=0)) is None
|
||||
|
||||
|
||||
def test_ordinary_drift_stays_quiet(tmp_path: Any) -> None:
|
||||
"""A note on every 2-commit gap is noise, and noise gets ignored when it matters."""
|
||||
assert repo_staleness_note(p_repo_pair(tmp_path, behind=MIN_BEHIND_TO_WARN - 1)) is None
|
||||
|
||||
|
||||
def test_the_threshold_is_actually_exercised(tmp_path: Any) -> None:
|
||||
"""Both sides of the boundary, so 'quiet' is never quiet because the check is broken."""
|
||||
assert repo_staleness_note(p_repo_pair(tmp_path, behind=MIN_BEHIND_TO_WARN)) is not None
|
||||
assert repo_staleness_note(p_repo_pair(tmp_path, behind=MIN_BEHIND_TO_WARN - 1)) is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cwd", [None, "", "/nonexistent-path-zzq", "/tmp"])
|
||||
def test_a_non_repo_never_produces_a_note_or_an_error(cwd: Any) -> None:
|
||||
"""Most agent sessions are not in a git repo at all; this must be silent and cheap there."""
|
||||
assert repo_staleness_note(cwd) is None
|
||||
|
||||
|
||||
def test_it_never_reaches_the_network(tmp_path: Any, monkeypatch: Any) -> None:
|
||||
"""A prompt builder that fetches would block every turn on someone's slow VPN."""
|
||||
repo = p_repo_pair(tmp_path, behind=25) # built BEFORE patching; its own fetch is not ours
|
||||
seen: List[List[str]] = []
|
||||
real = subprocess.run
|
||||
|
||||
def p_watch(args: Any, *a: Any, **k: Any) -> Any:
|
||||
if isinstance(args, list):
|
||||
seen.append(args)
|
||||
return real(args, *a, **k)
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", p_watch)
|
||||
repo_staleness_note(repo)
|
||||
assert seen, "the probe recorded no git calls at all, so it proves nothing"
|
||||
networked = [c for c in seen if any(x in ("fetch", "pull", "ls-remote") for x in c)]
|
||||
assert not networked, f"the note builder hit the network: {networked}"
|
||||
|
||||
|
||||
def test_the_turn_prompt_actually_carries_the_note() -> None:
|
||||
"""The note is only a fix if it reaches the model. A module nobody calls is the ENG-280 shape."""
|
||||
import inspect
|
||||
from backend.apps.agents.manager.prompt import compose_turn_system_prompt as mod
|
||||
|
||||
src = inspect.getsource(mod)
|
||||
assert "repo_staleness_note(" in src, "nothing composes the note into the turn prompt"
|
||||
assert "composed_prompt" in src.split("repo_staleness_note(")[1][:400], (
|
||||
"the note is computed but never appended to the prompt the model sees"
|
||||
)
|
||||
Reference in New Issue
Block a user