mirror of
https://github.com/0xWheatyz/handler.git
synced 2026-08-31 08:56:25 +00:00
Merge pull request #23 from 0xWheatyz/claude/agent-spawn-git-pull-cf0abm
Fetch origin and cut agent branches from origin/HEAD at spawn
This commit is contained in:
@@ -342,9 +342,14 @@ All routes require `Authorization: Bearer <AUTH_TOKEN>`. `GET /health` is unauth
|
||||
|
||||
Wired into each agent as `python -m handler.hooks <event>`:
|
||||
|
||||
- **`Stop` / `SessionEnd`** — checkpoint. On `Stop`, run `mise run test`; on failure,
|
||||
return `decision: "block"` with the output so the turn cannot end on red. Records
|
||||
`tests_status` / `tested_at`; `status = 'done'` only ever accompanies a pass.
|
||||
- **`Stop` / `SessionEnd`** — checkpoint + completion gate. On `Stop`, run
|
||||
`mise run test` and check the tree deterministically: failing tests, uncommitted
|
||||
changes, or commits that exist only locally each return `decision: "block"` with the
|
||||
full blocker list, so a turn cannot end on red or walk away from unshipped work.
|
||||
Records `tests_status` / `tested_at`; `status = 'done'` only ever accompanies a
|
||||
passing gate (tests green, tree clean, everything pushed). The agent's final message
|
||||
is captured from the session transcript onto the checkmark, so the dashboard always
|
||||
shows a real checkpoint whether or not the agent thought to leave one.
|
||||
- **`PreToolUse`** — two jobs. An `AskUserQuestion` is *deferred*: the question is
|
||||
persisted, the checkmark set to `paused_for_input`, and the tool call denied so control
|
||||
hands off to the async answer/resume flow. A `Bash` command running `git push` triggers
|
||||
|
||||
@@ -82,6 +82,29 @@ def ahead_count(cwd: str) -> int | None:
|
||||
return None
|
||||
|
||||
|
||||
def has_origin(cwd: str) -> bool:
|
||||
"""Whether the repo has an ``origin`` remote configured."""
|
||||
ok, out = _run(["remote"], cwd)
|
||||
return ok and "origin" in out.split()
|
||||
|
||||
|
||||
def unpushed_count(cwd: str) -> int | None:
|
||||
"""Commits reachable from HEAD that no ``origin/*`` ref contains.
|
||||
|
||||
0 means everything on the current line of history has been pushed *somewhere* on
|
||||
origin. Works with or without an upstream — worktree branches are created with
|
||||
``--no-track``, so an ``@{upstream}``-based count would come up empty for them.
|
||||
``None`` when the count cannot be computed.
|
||||
"""
|
||||
ok, out = _run(["rev-list", "--count", "HEAD", "--not", "--remotes=origin"], cwd)
|
||||
if not ok:
|
||||
return None
|
||||
try:
|
||||
return int(out.strip())
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def add(cwd: str, paths: list[str]) -> tuple[bool, str]:
|
||||
return _run(["add", *paths], cwd)
|
||||
|
||||
@@ -114,6 +137,34 @@ def clone(
|
||||
return _run(args, cwd=None, env=env, timeout=_NETWORK_TIMEOUT)
|
||||
|
||||
|
||||
def pull_ff(cwd: str, env: dict[str, str] | None = None) -> tuple[bool, str]:
|
||||
"""Fast-forward-only pull — never merges, so a diverged clone fails loudly."""
|
||||
return _run(["pull", "--ff-only"], cwd, env=env, timeout=_NETWORK_TIMEOUT)
|
||||
def fetch(cwd: str, env: dict[str, str] | None = None) -> tuple[bool, str]:
|
||||
"""``git fetch origin`` — refreshes every ``origin/*`` ref.
|
||||
|
||||
Unlike a plain pull, this works no matter which branch the working tree has
|
||||
checked out (or left parked), so the remote's state is always current locally.
|
||||
"""
|
||||
return _run(["fetch", "origin"], cwd, env=env, timeout=_NETWORK_TIMEOUT)
|
||||
|
||||
|
||||
def set_default_head(cwd: str, env: dict[str, str] | None = None) -> tuple[bool, str]:
|
||||
"""``git remote set-head origin --auto`` — (re)pin ``origin/HEAD``.
|
||||
|
||||
``origin/HEAD`` is how the remote's default branch is known locally; clones made
|
||||
by older tooling can lack it, and the remote's default can change.
|
||||
"""
|
||||
return _run(["remote", "set-head", "origin", "--auto"], cwd, env=env,
|
||||
timeout=_NETWORK_TIMEOUT)
|
||||
|
||||
|
||||
def default_branch_ref(cwd: str) -> str | None:
|
||||
"""The remote default branch as a local ref name (``origin/main``), or ``None``."""
|
||||
ok, out = _run(["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], cwd)
|
||||
if not ok or not out:
|
||||
return None
|
||||
return out.removeprefix("refs/remotes/") or None
|
||||
|
||||
|
||||
def merge_ff(cwd: str, ref: str) -> tuple[bool, str]:
|
||||
"""Fast-forward-only merge of ``ref`` — never merges, so a diverged checkout
|
||||
fails loudly instead of silently growing a merge commit."""
|
||||
return _run(["merge", "--ff-only", ref], cwd)
|
||||
|
||||
@@ -337,13 +337,15 @@ class RunSupervisor:
|
||||
)
|
||||
# Hooks are the authority on agent status — they ran inside the run and
|
||||
# may have set paused_for_input/blocked/done already. Only an agent still
|
||||
# marked ``working`` needs the process's verdict.
|
||||
# marked ``working`` needs the process's verdict — and ``done`` is a gate
|
||||
# outcome, not an exit code: an agent still ``working`` after a clean
|
||||
# exit means the Stop gate never recorded a verdict (hook misfired,
|
||||
# identity missing), so nothing has verified its tests/commits/pushes.
|
||||
# An operator cancel is the one completion that needs no gate.
|
||||
agent = repo.get_agent_by_id(conn, self.agent["id"])
|
||||
if finished and agent is not None and agent["status"] == "working":
|
||||
if self._canceled:
|
||||
repo.set_agent_status(conn, self.agent["id"], "done")
|
||||
elif clean:
|
||||
repo.set_agent_status(conn, self.agent["id"], "done")
|
||||
else:
|
||||
repo.set_agent_status(conn, self.agent["id"], "blocked")
|
||||
if not clean and not self._canceled:
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Clone-or-pull a project's repository — the stateless "always pull" step.
|
||||
"""Clone-or-fetch a project's repository — the stateless "always pull" step.
|
||||
|
||||
Where a clone lands on disk is Handler's concern, not the operator's: the API computes
|
||||
``root_dir`` under ``PROJECTS_ROOT`` at registration, and this module makes the checkout
|
||||
@@ -70,11 +70,16 @@ def ssh_env(git_remote: str | None, conn: Connection) -> dict[str, str]:
|
||||
|
||||
|
||||
def sync_project(project: dict, conn: Connection | None = None) -> dict:
|
||||
"""Clone the project's remote into ``root_dir``, or fast-forward an existing clone.
|
||||
"""Clone the project's remote into ``root_dir``, or refresh an existing clone.
|
||||
|
||||
Idempotent by design — scheduled/stateless runs call this before every spawn so the
|
||||
working tree always starts from the remote's latest state. Raises :class:`SyncError`
|
||||
when the project has no remote or git fails.
|
||||
working tree always starts from the remote's latest state. An existing clone is
|
||||
``git fetch``\\ ed rather than pulled: a pull only moves the branch the root happens
|
||||
to have checked out, and an agent parked on a feature branch used to leave
|
||||
``origin/*`` (and therefore every branch cut for a new agent) hours stale. After
|
||||
the fetch, ``origin/HEAD`` is re-pinned and the checkout is fast-forwarded only
|
||||
when it is sitting on the default branch. Raises :class:`SyncError` when the
|
||||
project has no remote or git fails.
|
||||
"""
|
||||
remote = project.get("git_remote")
|
||||
root = project["root_dir"]
|
||||
@@ -88,10 +93,22 @@ def sync_project(project: dict, conn: Connection | None = None) -> dict:
|
||||
env, config = _auth_context(project, conn)
|
||||
|
||||
if gitops.is_repo(root):
|
||||
ok, out = gitops.pull_ff(root, env=env)
|
||||
ok, out = gitops.fetch(root, env=env)
|
||||
if not ok:
|
||||
raise SyncError(f"pull failed in {root}: {out}")
|
||||
return {"action": "pulled", "root_dir": root, "detail": out}
|
||||
raise SyncError(f"fetch failed in {root}: {out}")
|
||||
# Best-effort: worktree spawns cut new branches from origin/HEAD, so keep it
|
||||
# pinned even for clones that predate it (or whose remote default changed).
|
||||
gitops.set_default_head(root, env=env)
|
||||
detail = out
|
||||
default_ref = gitops.default_branch_ref(root)
|
||||
if default_ref and gitops.current_branch(root) == default_ref.split("/", 1)[1]:
|
||||
ok, ff_out = gitops.merge_ff(root, default_ref)
|
||||
if not ok:
|
||||
raise SyncError(
|
||||
f"fast-forward of {default_ref} failed in {root}: {ff_out}"
|
||||
)
|
||||
detail = ff_out or detail
|
||||
return {"action": "pulled", "root_dir": root, "detail": detail}
|
||||
|
||||
os.makedirs(os.path.dirname(root) or ".", exist_ok=True)
|
||||
ok, out = gitops.clone(remote, root, env=env, config=config)
|
||||
|
||||
@@ -95,10 +95,12 @@ def spawn(
|
||||
raise SpawnError(f"agent '{name}' already exists in project '{project_id}'")
|
||||
|
||||
# Stateless workflows: start every run from the remote's latest state. An
|
||||
# existing clone is fast-forwarded (failure degrades to a note — a stale tree is
|
||||
# usable, an offline forge shouldn't brick spawning); a missing/empty root is
|
||||
# cloned, and that failing is fatal (there is nothing to run against). A
|
||||
# non-empty root that isn't a git repo is left alone — it's manually managed.
|
||||
# existing clone is fetched — refreshing origin/* even when the root checkout is
|
||||
# parked on an agent's branch — and fast-forwarded when it sits on the default
|
||||
# branch (failure degrades to a note — a stale tree is usable, an offline forge
|
||||
# shouldn't brick spawning); a missing/empty root is cloned, and that failing is
|
||||
# fatal (there is nothing to run against). A non-empty root that isn't a git
|
||||
# repo is left alone — it's manually managed.
|
||||
root = project["root_dir"]
|
||||
if project.get("git_remote"):
|
||||
if gitops.is_repo(root):
|
||||
|
||||
@@ -35,6 +35,25 @@ def _slug(name: str) -> str:
|
||||
return slug or "agent"
|
||||
|
||||
|
||||
def _default_branch_start(project_root: str) -> str | None:
|
||||
"""The remote default branch ref (``origin/main``) when the clone knows it.
|
||||
|
||||
New agent branches are cut from here rather than the root's ``HEAD``: the root
|
||||
checkout can legitimately be parked on some earlier agent's branch, and cutting
|
||||
from it handed fresh agents history that was hours behind the remote.
|
||||
"""
|
||||
result = subprocess.run(
|
||||
["git", "-C", project_root, "symbolic-ref", "--quiet",
|
||||
"refs/remotes/origin/HEAD"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
ref = result.stdout.strip()
|
||||
if result.returncode != 0 or not ref.startswith("refs/remotes/"):
|
||||
return None
|
||||
return ref.removeprefix("refs/remotes/")
|
||||
|
||||
|
||||
def _branch_exists(project_root: str, branch: str) -> bool:
|
||||
return (
|
||||
subprocess.run(
|
||||
@@ -69,13 +88,21 @@ def resolve_working_dir(
|
||||
raise IsolationError(f"{target} escapes project root {project_root}")
|
||||
# `git worktree add <path> <branch>` only checks out an *existing* ref; a fresh
|
||||
# feature branch won't exist yet (spawn starts from the remote's latest state), so
|
||||
# create it with -b. An existing local branch is checked out as-is; if it exists
|
||||
# only on a remote, git DWIMs a tracking branch from the bare `add`.
|
||||
# create it with -b — from origin/HEAD when the clone has a remote, so the new
|
||||
# branch starts at the remote default branch's tip regardless of where the root
|
||||
# checkout is parked (--no-track: the branch must not adopt the default branch as
|
||||
# its upstream, or the agent's plain `git push` would aim at it). An existing
|
||||
# local branch is checked out as-is; if it exists only on a remote, git DWIMs a
|
||||
# tracking branch from the bare `add`.
|
||||
cmd = ["git", "-C", project_root, "worktree", "add"]
|
||||
if _branch_exists(project_root, worktree_branch):
|
||||
cmd += [target, worktree_branch]
|
||||
else:
|
||||
cmd += ["-b", worktree_branch, target]
|
||||
start = _default_branch_start(project_root)
|
||||
if start:
|
||||
cmd += ["--no-track", "-b", worktree_branch, target, start]
|
||||
else:
|
||||
cmd += ["-b", worktree_branch, target]
|
||||
result = subprocess.run(cmd, capture_output=True, text=True)
|
||||
if result.returncode != 0:
|
||||
raise WorktreeError(
|
||||
|
||||
@@ -1,14 +1,19 @@
|
||||
"""Stop / SessionEnd — the checkpoint + verification gate (README 3.5).
|
||||
|
||||
On ``Stop`` the gate runs the project's own ``test`` task and blocks the turn on
|
||||
failure, so a turn cannot end on a broken suite. The result feeds straight into the
|
||||
schema: ``status = 'done'`` is only ever recorded alongside a passing test run — not a
|
||||
claim taken on faith. ``SessionEnd`` cannot be blocked, so it just records a final
|
||||
checkpoint with the end reason.
|
||||
On ``Stop`` the gate runs the project's own ``test`` task and checks the working tree
|
||||
deterministically: failing tests, uncommitted changes, or commits that exist only
|
||||
locally each block the turn, so an agent cannot end on a broken suite or walk away
|
||||
from work it never committed or pushed. ``status = 'done'`` is only ever recorded
|
||||
alongside a passing gate — not a claim taken on faith. The agent's final message is
|
||||
captured from the session transcript onto the checkmark, so the dashboard always has a
|
||||
real checkpoint to show regardless of whether the agent thought to leave one.
|
||||
``SessionEnd`` cannot be blocked, so it just records a final checkpoint with the end
|
||||
reason.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from sqlalchemy import Connection
|
||||
@@ -84,16 +89,89 @@ def handle_mise_init_stop(conn: Connection, ident: Identity, hook_input: HookInp
|
||||
return {}
|
||||
|
||||
|
||||
def _completion_blockers(working_dir: str) -> list[str]:
|
||||
"""The commit/push half of the completion gate — ``[]`` when the tree is settled.
|
||||
|
||||
Deterministic checks against git itself, not the agent's account of its work: a
|
||||
dirty tree means work was never committed; commits no ``origin/*`` ref contains
|
||||
mean work was never pushed. A working dir that isn't a git checkout (manually
|
||||
managed roots, empty repos) has nothing to gate.
|
||||
"""
|
||||
if gitops.head_sha(working_dir) is None:
|
||||
return []
|
||||
blockers = []
|
||||
if not gitops.is_clean(working_dir):
|
||||
blockers.append("there are uncommitted changes — commit your work")
|
||||
if gitops.has_origin(working_dir):
|
||||
unpushed = gitops.unpushed_count(working_dir)
|
||||
if unpushed:
|
||||
blockers.append(
|
||||
f"{unpushed} commit(s) exist only locally — push them "
|
||||
"(`git push -u origin <branch>`)"
|
||||
)
|
||||
return blockers
|
||||
|
||||
|
||||
def _final_assistant_text(transcript_path: str | None) -> str | None:
|
||||
"""The agent's last assistant message from the session transcript.
|
||||
|
||||
Captured deterministically so the dashboard's checkpoint never depends on the
|
||||
agent remembering to file one. Returns ``None`` when there is no transcript or no
|
||||
text in it; never raises.
|
||||
"""
|
||||
if not transcript_path:
|
||||
return None
|
||||
try:
|
||||
fh = open(transcript_path, encoding="utf-8")
|
||||
except OSError:
|
||||
return None
|
||||
last = None
|
||||
with fh:
|
||||
for line in fh:
|
||||
try:
|
||||
entry = json.loads(line)
|
||||
except ValueError:
|
||||
continue
|
||||
if entry.get("type") != "assistant":
|
||||
continue
|
||||
content = (entry.get("message") or {}).get("content")
|
||||
if isinstance(content, str):
|
||||
texts = [content]
|
||||
elif isinstance(content, list):
|
||||
texts = [
|
||||
block.get("text", "")
|
||||
for block in content
|
||||
if isinstance(block, dict) and block.get("type") == "text"
|
||||
]
|
||||
else:
|
||||
continue
|
||||
text = "\n".join(t for t in texts if t).strip()
|
||||
if text:
|
||||
last = text
|
||||
return last
|
||||
|
||||
|
||||
def handle_stop(conn: Connection, ident: Identity, hook_input: HookInput) -> dict:
|
||||
if ident.mise_init:
|
||||
return handle_mise_init_stop(conn, ident, hook_input)
|
||||
working_dir = ident.working_dir or hook_input.cwd or "."
|
||||
ok, output = verify.run_test(working_dir)
|
||||
tests_ok, output = verify.run_test(working_dir)
|
||||
blockers = [] if tests_ok else ["the test suite is failing (`mise run test`)"]
|
||||
blockers += _completion_blockers(working_dir)
|
||||
now = datetime.now(UTC)
|
||||
|
||||
status = "done" if ok else "blocked"
|
||||
tests_status = "pass" if ok else "fail"
|
||||
summary = "checkpoint: tests passed" if ok else "checkpoint blocked: tests failed"
|
||||
status = "done" if not blockers else "blocked"
|
||||
tests_status = "pass" if tests_ok else "fail"
|
||||
summary = (
|
||||
"checkpoint: tests passed, work committed and pushed"
|
||||
if not blockers
|
||||
else "checkpoint blocked: " + "; ".join(blockers)
|
||||
)
|
||||
# A done agent's checkmark carries its own closing message — the substance the
|
||||
# dashboard shows; a blocked one carries the blockers (the next turn will re-capture
|
||||
# the narrative once the gate clears).
|
||||
final_text = _final_assistant_text(hook_input.transcript_path)
|
||||
where_it_stopped = final_text[:4000] if not blockers and final_text else summary
|
||||
|
||||
log_id = repo.insert_log_entry(
|
||||
conn,
|
||||
@@ -108,25 +186,25 @@ def handle_stop(conn: Connection, ident: Identity, hook_input: HookInput) -> dic
|
||||
agent_id=ident.agent_id,
|
||||
checkpoint_at=now,
|
||||
status=status,
|
||||
where_it_stopped=summary,
|
||||
where_it_stopped=where_it_stopped,
|
||||
log_entry_id=log_id,
|
||||
tests_status=tests_status,
|
||||
tested_at=now,
|
||||
)
|
||||
repo.set_agent_status(conn, ident.agent_id, status)
|
||||
|
||||
if not ok:
|
||||
if blockers:
|
||||
# Guard against an infinite block loop: if we already re-invoked once, record
|
||||
# the failure but let the turn end rather than blocking forever.
|
||||
if hook_input.stop_hook_active:
|
||||
return {}
|
||||
return {
|
||||
"decision": "block",
|
||||
"reason": (
|
||||
"The test gate failed; the turn cannot end on a broken suite. "
|
||||
f"`mise run test` output:\n{output[-4000:]}"
|
||||
),
|
||||
}
|
||||
reason = (
|
||||
"The completion gate failed — an agent is only done when its tests pass "
|
||||
"and its work is committed and pushed. Blockers:\n- " + "\n- ".join(blockers)
|
||||
)
|
||||
if not tests_ok:
|
||||
reason += f"\n\n`mise run test` output:\n{output[-4000:]}"
|
||||
return {"decision": "block", "reason": reason}
|
||||
return {}
|
||||
|
||||
|
||||
|
||||
@@ -44,6 +44,10 @@ class HookInput:
|
||||
def message(self) -> str | None:
|
||||
return self.raw.get("message")
|
||||
|
||||
@property
|
||||
def transcript_path(self) -> str | None:
|
||||
return self.raw.get("transcript_path")
|
||||
|
||||
@property
|
||||
def stop_hook_active(self) -> bool:
|
||||
return bool(self.raw.get("stop_hook_active"))
|
||||
|
||||
@@ -81,7 +81,9 @@ def test_spawn_run_streams_events_and_completes(headless_env, tmp_path):
|
||||
assert [e["seq"] for e in events] == [1, 2, 3]
|
||||
# last_output is now derived from assistant text — the log the UI shows is real.
|
||||
assert updated["last_output"] == "working on: build the thing"
|
||||
assert updated["status"] == "done"
|
||||
# done is a gate outcome, not an exit code: the fake claude never ran the Stop
|
||||
# hook, so nothing verified tests/commits/pushes and the agent must not be done.
|
||||
assert updated["status"] == "blocked"
|
||||
assert updated["session_id"] == run["session_id"]
|
||||
assert updated["worker_id"] == "w1"
|
||||
# The session archive was uploaded at exit for cross-worker resume.
|
||||
|
||||
@@ -20,13 +20,22 @@ def _fake_mise_state(monkeypatch, *, has_test, clean, ahead):
|
||||
monkeypatch.setattr(gitops, "ahead_count", lambda cwd: ahead)
|
||||
|
||||
|
||||
def _fake_git_state(monkeypatch, *, is_repo=True, clean=True, unpushed=0, has_origin=True):
|
||||
monkeypatch.setattr(gitops, "head_sha", lambda cwd: "abc123" if is_repo else None)
|
||||
monkeypatch.setattr(gitops, "is_clean", lambda cwd: clean)
|
||||
monkeypatch.setattr(gitops, "has_origin", lambda cwd: has_origin)
|
||||
monkeypatch.setattr(gitops, "unpushed_count", lambda cwd: unpushed)
|
||||
|
||||
|
||||
def test_stop_blocks_on_failing_tests(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (False, "1 failed"))
|
||||
_fake_git_state(monkeypatch)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result["decision"] == "block"
|
||||
assert "test gate failed" in result["reason"]
|
||||
assert "test suite is failing" in result["reason"]
|
||||
assert "1 failed" in result["reason"]
|
||||
|
||||
cm = repo.get_checkmark(conn, ident.agent_id)
|
||||
assert cm["tests_status"] == "fail"
|
||||
@@ -35,9 +44,59 @@ def test_stop_blocks_on_failing_tests(conn, monkeypatch):
|
||||
assert repo.get_agent_by_name(conn, "p", "a")["status"] == "blocked"
|
||||
|
||||
|
||||
def test_stop_allows_done_on_passing_tests(conn, monkeypatch):
|
||||
def test_stop_blocks_on_uncommitted_changes_even_with_green_tests(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch, clean=False)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result["decision"] == "block"
|
||||
assert "uncommitted" in result["reason"]
|
||||
|
||||
cm = repo.get_checkmark(conn, ident.agent_id)
|
||||
assert cm["tests_status"] == "pass" # tests still recorded honestly
|
||||
assert cm["status"] == "blocked"
|
||||
assert repo.get_agent_by_name(conn, "p", "a")["status"] == "blocked"
|
||||
|
||||
|
||||
def test_stop_blocks_on_unpushed_commits(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch, unpushed=3)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result["decision"] == "block"
|
||||
assert "3 commit(s) exist only locally" in result["reason"]
|
||||
assert repo.get_agent_by_name(conn, "p", "a")["status"] == "blocked"
|
||||
|
||||
|
||||
def test_stop_reports_every_blocker_at_once(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (False, "boom"))
|
||||
_fake_git_state(monkeypatch, clean=False, unpushed=1)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert "test suite is failing" in result["reason"]
|
||||
assert "uncommitted" in result["reason"]
|
||||
assert "exist only locally" in result["reason"]
|
||||
|
||||
|
||||
def test_stop_skips_push_gate_without_an_origin_remote(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
# Local-only project: commits exist that no remote has, but there is no origin
|
||||
# to push them to — the push gate must not deadlock the agent.
|
||||
_fake_git_state(monkeypatch, unpushed=5, has_origin=False)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result == {}
|
||||
assert repo.get_checkmark(conn, ident.agent_id)["status"] == "done"
|
||||
|
||||
|
||||
def test_stop_allows_done_when_tests_pass_and_tree_is_settled(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result == {} # no block
|
||||
@@ -48,9 +107,60 @@ def test_stop_allows_done_on_passing_tests(conn, monkeypatch):
|
||||
assert cm["log_entry_id"] is not None
|
||||
|
||||
|
||||
def test_stop_allows_done_when_working_dir_is_not_a_repo(conn, monkeypatch):
|
||||
# A manually managed root has nothing to gate beyond its tests.
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch, is_repo=False, clean=False, unpushed=9)
|
||||
|
||||
result = checkpoint.handle_stop(conn, ident, HookInput({"session_id": "s1"}, "stop"))
|
||||
assert result == {}
|
||||
assert repo.get_checkmark(conn, ident.agent_id)["status"] == "done"
|
||||
|
||||
|
||||
def test_stop_captures_final_message_as_checkpoint(conn, monkeypatch, tmp_path):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
transcript.write_text(
|
||||
'{"type": "user", "message": {"content": "do the thing"}}\n'
|
||||
"this line is not json\n"
|
||||
'{"type": "assistant", "message": {"content": [{"type": "text", '
|
||||
'"text": "Working on it."}]}}\n'
|
||||
'{"type": "assistant", "message": {"content": [{"type": "tool_use", "id": "x"},'
|
||||
' {"type": "text", "text": "Shipped the fix in abc123; tests green."}]}}\n'
|
||||
)
|
||||
|
||||
hi = HookInput({"session_id": "s1", "transcript_path": str(transcript)}, "stop")
|
||||
result = checkpoint.handle_stop(conn, ident, hi)
|
||||
assert result == {}
|
||||
cm = repo.get_checkmark(conn, ident.agent_id)
|
||||
# The webui checkpoint carries the agent's own closing message, captured
|
||||
# deterministically from the transcript — not left to the agent's discretion.
|
||||
assert cm["where_it_stopped"] == "Shipped the fix in abc123; tests green."
|
||||
|
||||
|
||||
def test_blocked_stop_shows_blockers_not_narrative(conn, monkeypatch, tmp_path):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok"))
|
||||
_fake_git_state(monkeypatch, clean=False)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
transcript.write_text(
|
||||
'{"type": "assistant", "message": {"content": [{"type": "text", '
|
||||
'"text": "All done!"}]}}\n'
|
||||
)
|
||||
|
||||
hi = HookInput({"session_id": "s1", "transcript_path": str(transcript)}, "stop")
|
||||
checkpoint.handle_stop(conn, ident, hi)
|
||||
cm = repo.get_checkmark(conn, ident.agent_id)
|
||||
assert "uncommitted" in cm["where_it_stopped"]
|
||||
|
||||
|
||||
def test_stop_does_not_reblock_when_already_active(conn, monkeypatch):
|
||||
ident = _seed(conn)
|
||||
monkeypatch.setattr(verify, "run_test", lambda cwd: (False, "still failing"))
|
||||
_fake_git_state(monkeypatch)
|
||||
hi = HookInput({"session_id": "s1", "stop_hook_active": True}, "stop")
|
||||
result = checkpoint.handle_stop(conn, ident, hi)
|
||||
assert result == {} # recorded, but not an infinite block
|
||||
|
||||
@@ -71,13 +71,15 @@ def test_spawn_via_api_then_worker_runs_headless_claude(client, auth, headless_e
|
||||
assert got["result"]["name"] == "api"
|
||||
|
||||
# ...and the run's whole life shows up via the API: events stream in, the agent
|
||||
# reconciles to done, last_output is the assistant's text.
|
||||
# reconciles, last_output is the assistant's text. The fake claude never runs the
|
||||
# Stop gate, so the clean exit reconciles to blocked, not done — done is only ever
|
||||
# a gate verdict (tests pass, work committed and pushed).
|
||||
def finished():
|
||||
agents = client.get("/projects/proj/agents", headers=auth).json()
|
||||
return agents if agents and agents[0]["status"] == "done" else None
|
||||
return agents if agents and agents[0]["status"] == "blocked" else None
|
||||
|
||||
agents = _wait(finished)
|
||||
assert agents is not None, "run never reconciled to done"
|
||||
assert agents is not None, "run never reconciled"
|
||||
agent = agents[0]
|
||||
assert agent["name"] == "api"
|
||||
assert agent["session_id"]
|
||||
|
||||
@@ -150,7 +150,7 @@ def test_sync_endpoint_400_without_remote(client, auth, env, tmp_path):
|
||||
@pytest.fixture
|
||||
def fake_sync_gitops(monkeypatch):
|
||||
"""Fake the clone/pull side of the gitops seam."""
|
||||
state = {"clone": [], "pull": [], "config": [], "repos": set(), "ok": True, "out": ""}
|
||||
state = {"clone": [], "fetch": [], "config": [], "repos": set(), "ok": True, "out": ""}
|
||||
|
||||
def is_repo(path):
|
||||
return path in state["repos"]
|
||||
@@ -162,17 +162,25 @@ def fake_sync_gitops(monkeypatch):
|
||||
state["repos"].add(dest)
|
||||
return state["ok"], state["out"]
|
||||
|
||||
def pull_ff(cwd, env=None):
|
||||
state["pull"].append({"cwd": cwd, "env": env or {}})
|
||||
def fetch(cwd, env=None):
|
||||
state["fetch"].append({"cwd": cwd, "env": env or {}})
|
||||
return state["ok"], state["out"]
|
||||
|
||||
def set_default_head(cwd, env=None):
|
||||
return True, ""
|
||||
|
||||
def default_branch_ref(cwd):
|
||||
return None
|
||||
|
||||
def config_local(cwd, key, value):
|
||||
state["config"].append({"cwd": cwd, "key": key, "value": value})
|
||||
return True, ""
|
||||
|
||||
monkeypatch.setattr(gitops, "is_repo", is_repo)
|
||||
monkeypatch.setattr(gitops, "clone", clone)
|
||||
monkeypatch.setattr(gitops, "pull_ff", pull_ff)
|
||||
monkeypatch.setattr(gitops, "fetch", fetch)
|
||||
monkeypatch.setattr(gitops, "set_default_head", set_default_head)
|
||||
monkeypatch.setattr(gitops, "default_branch_ref", default_branch_ref)
|
||||
monkeypatch.setattr(gitops, "config_local", config_local)
|
||||
return state
|
||||
|
||||
@@ -192,7 +200,7 @@ def test_cmd_sync_clones_then_pulls(env, fake_sync_gitops):
|
||||
|
||||
result = worker.execute_command(command)
|
||||
assert result["action"] == "pulled"
|
||||
assert fake_sync_gitops["pull"][0]["cwd"] == "/tmp/r"
|
||||
assert fake_sync_gitops["fetch"][0]["cwd"] == "/tmp/r"
|
||||
|
||||
|
||||
def test_cmd_sync_failure_is_command_error(env, fake_sync_gitops):
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
"""``sync_project`` against real git repos — the fetch-not-pull regression.
|
||||
|
||||
A pull only moves whichever branch the root has checked out; with the root parked on
|
||||
an agent's feature branch, ``origin/*`` went stale and every branch cut for a new
|
||||
agent started several commits behind the remote's default branch. These tests use a
|
||||
filesystem remote (no host, no credentials) so the git behaviour itself is exercised.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
|
||||
import pytest
|
||||
|
||||
from handler.control import reposync
|
||||
|
||||
|
||||
def _git(cwd, *args):
|
||||
subprocess.run(["git", "-C", str(cwd), *args], check=True, capture_output=True)
|
||||
|
||||
|
||||
def _sha(cwd, ref="HEAD") -> str:
|
||||
return subprocess.run(
|
||||
["git", "-C", str(cwd), "rev-parse", ref],
|
||||
check=True, capture_output=True, text=True,
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def remote_and_root(tmp_path):
|
||||
"""An origin repo, a clone of it as the project root, then origin moves ahead."""
|
||||
origin = tmp_path / "origin"
|
||||
origin.mkdir()
|
||||
_git(origin, "init", "-q", "-b", "main")
|
||||
for repo_dir in (origin,):
|
||||
_git(repo_dir, "config", "user.email", "t@t.co")
|
||||
_git(repo_dir, "config", "user.name", "t")
|
||||
_git(repo_dir, "config", "commit.gpgsign", "false")
|
||||
_git(origin, "commit", "-q", "--allow-empty", "-m", "one")
|
||||
root = tmp_path / "proj"
|
||||
subprocess.run(
|
||||
["git", "clone", "-q", str(origin), str(root)], check=True, capture_output=True
|
||||
)
|
||||
_git(root, "config", "user.email", "t@t.co")
|
||||
_git(root, "config", "user.name", "t")
|
||||
_git(root, "config", "commit.gpgsign", "false")
|
||||
_git(origin, "commit", "-q", "--allow-empty", "-m", "two")
|
||||
return origin, root
|
||||
|
||||
|
||||
def _project(origin, root) -> dict:
|
||||
return {"id": "p", "root_dir": str(root), "git_remote": str(origin)}
|
||||
|
||||
|
||||
def test_sync_refreshes_origin_even_when_root_parked_on_agent_branch(
|
||||
env, remote_and_root
|
||||
):
|
||||
origin, root = remote_and_root
|
||||
_git(root, "checkout", "-q", "-b", "agent/old-work")
|
||||
# Simulate a clone that never had origin/HEAD — sync must re-pin it too.
|
||||
_git(root, "remote", "set-head", "origin", "-d")
|
||||
|
||||
result = reposync.sync_project(_project(origin, root))
|
||||
|
||||
assert result["action"] == "pulled"
|
||||
assert _sha(root, "origin/main") == _sha(origin, "main")
|
||||
assert _sha(root, "refs/remotes/origin/HEAD") == _sha(origin, "main")
|
||||
# The parked checkout itself is left alone — only origin/* moves.
|
||||
assert _sha(root, "HEAD") != _sha(origin, "main")
|
||||
|
||||
|
||||
def test_sync_fast_forwards_a_checkout_sitting_on_the_default_branch(
|
||||
env, remote_and_root
|
||||
):
|
||||
origin, root = remote_and_root
|
||||
result = reposync.sync_project(_project(origin, root))
|
||||
assert result["action"] == "pulled"
|
||||
assert _sha(root, "HEAD") == _sha(origin, "main")
|
||||
|
||||
|
||||
def test_sync_fails_loudly_when_the_default_branch_diverged(env, remote_and_root):
|
||||
origin, root = remote_and_root
|
||||
_git(root, "commit", "-q", "--allow-empty", "-m", "local divergence")
|
||||
with pytest.raises(reposync.SyncError, match="fast-forward"):
|
||||
reposync.sync_project(_project(origin, root))
|
||||
@@ -31,6 +31,36 @@ def repo(tmp_path):
|
||||
return root
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def cloned(tmp_path):
|
||||
"""A root cloned from an origin whose main has since moved on.
|
||||
|
||||
Mirrors the spawn-time state after ``reposync.sync_project``: origin/* is fresh
|
||||
(fetched), but the root's checkout still sits on the older commit.
|
||||
"""
|
||||
origin = tmp_path / "origin"
|
||||
origin.mkdir()
|
||||
_git(origin, "init", "-q", "-b", "main")
|
||||
_git(origin, "config", "user.email", "t@t.co")
|
||||
_git(origin, "config", "user.name", "t")
|
||||
_git(origin, "config", "commit.gpgsign", "false")
|
||||
_git(origin, "commit", "-q", "--allow-empty", "-m", "one")
|
||||
root = tmp_path / "proj"
|
||||
subprocess.run(
|
||||
["git", "clone", "-q", str(origin), str(root)], check=True, capture_output=True
|
||||
)
|
||||
_git(origin, "commit", "-q", "--allow-empty", "-m", "two")
|
||||
_git(root, "fetch", "-q", "origin")
|
||||
return root
|
||||
|
||||
|
||||
def _sha(cwd, ref="HEAD") -> str:
|
||||
return subprocess.run(
|
||||
["git", "-C", str(cwd), "rev-parse", ref],
|
||||
check=True, capture_output=True, text=True,
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
def _is_worktree(root, path) -> bool:
|
||||
out = subprocess.run(
|
||||
["git", "-C", str(root), "worktree", "list", "--porcelain"],
|
||||
@@ -54,6 +84,30 @@ def test_creates_branch_when_it_does_not_exist(repo):
|
||||
assert "feat/new-thing" in branches
|
||||
|
||||
|
||||
def test_new_branch_is_cut_from_remote_default_not_root_head(cloned):
|
||||
# The regression: new agent branches were cut from the root's HEAD, so a checkout
|
||||
# sitting behind (or parked on an earlier agent's branch) produced branches several
|
||||
# commits behind origin's default branch.
|
||||
target = worktree.resolve_working_dir(
|
||||
str(cloned), "agent", worktree_branch="feat/fresh"
|
||||
)
|
||||
assert _sha(target) == _sha(cloned, "origin/main")
|
||||
assert _sha(target) != _sha(cloned, "HEAD")
|
||||
|
||||
|
||||
def test_new_branch_from_remote_default_gets_no_upstream(cloned):
|
||||
# --no-track: without it the new branch adopts origin/main as upstream, and the
|
||||
# agent's plain `git push` would aim at the default branch.
|
||||
target = worktree.resolve_working_dir(
|
||||
str(cloned), "agent", worktree_branch="feat/fresh"
|
||||
)
|
||||
upstream = subprocess.run(
|
||||
["git", "-C", target, "rev-parse", "--abbrev-ref", "@{upstream}"],
|
||||
capture_output=True, text=True,
|
||||
)
|
||||
assert upstream.returncode != 0
|
||||
|
||||
|
||||
def test_checks_out_existing_branch(repo):
|
||||
_git(repo, "branch", "feat/existing")
|
||||
target = worktree.resolve_working_dir(
|
||||
|
||||
Reference in New Issue
Block a user