Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 74 additions & 1 deletion strands-command/scripts/python/agent_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
add_pr_comment,
create_issue,
create_pull_request,
get_invoked_write_tools,
get_issue,
get_issue_comments,
get_issue_label_names,
get_pull_request,
get_pr_files,
get_pr_review_and_comments,
Expand All @@ -55,6 +57,73 @@
DEFAULT_SYSTEM_PROMPT = "You are an autonomous GitHub agent powered by Strands Agents SDK."


# Write tools that touch the issue itself. A bug-verifier run must end up applying
# at least one of these (a triage label, or a comment) -- the SOP guarantees a
# label on every verdict, plus a comment in the derived-repro / cannot-reproduce
# cases.
_ISSUE_WRITE_TOOLS = {"add_issue_labels", "add_issue_comment"}

# Triage labels the bug-verifier applies. Used to recognise that a prior run's
# work already landed on the issue when the current (resumed) run wrote nothing.
_TRIAGE_LABELS = {"bug-validated", "bug-needs-info", "bug-cannot-reproduce"}


def _issue_number_from_session_id(session_id: str) -> int | None:
"""Extract the trailing issue number from an issue-scoped session ID.

Session IDs are formed as "<mode>-<issue>" (e.g. "bug-verifier-3216").
"""
tail = session_id.rsplit("-", 1)[-1]
return int(tail) if tail.isdigit() else None


def _enforce_required_writes(session_id: str | None) -> None:
"""Fail a bug-verifier run that neither labels nor comments on the issue.

The bug-verifier SOP guarantees at least a triage label on every verdict, and
that write is what records a deferred operation for the finalize step to
replay. An agent that only *describes* applying a label -- without ever
invoking the tool -- produces a green run that never touches the issue and
uploads no write-operations artifact. This turns that silent no-op into a hard
failure.

Sessions are resumed across triggers, so a re-run may legitimately write
nothing because the label already landed in a prior run. In that case the
issue already carries a triage label, which we accept.
"""
if not session_id or not session_id.startswith("bug-verifier"):
return

invoked = get_invoked_write_tools()
if _ISSUE_WRITE_TOOLS.intersection(invoked):
# This run applied (or, in read-only mode, deferred) a label or comment.
return

# No issue-facing write this run. Accept only if a prior run's triage label
# already landed on the issue (resumed, already-complete session).
issue_number = _issue_number_from_session_id(session_id)
if issue_number is not None:
try:
existing = get_issue_label_names(issue_number)
if _TRIAGE_LABELS.intersection(existing):
print(
f"ℹ️ No write this run, but issue #{issue_number} already carries a "
f"triage label {sorted(_TRIAGE_LABELS.intersection(existing))} from a "
"prior run -- accepting."
)
return
except Exception as e:
print(f"⚠️ Could not verify existing triage labels: {e}")

raise RuntimeError(
"bug-verifier finished without applying a label or comment to the issue, "
"and no triage label from a prior run is present. The SOP mandates at least "
f"a triage label on every verdict (write tools this run: {invoked or 'none'}). "
"Nothing was applied to the issue and nothing was recorded for deferred "
"execution -- failing so this is not a silent no-op."
)


def _send_eval_trigger(session_id: str, eval_type: str) -> None:
"""Send evaluation trigger to SQS queue after agent completion.

Expand Down Expand Up @@ -256,7 +325,11 @@ def run_agent(query: str):
result = agent(query)

print(f"\n\nAgent Result 🤖\nStop Reason: {result.stop_reason}\nMessage: {json.dumps(result.message, indent=2)}")


# Fail loudly if a mode with a mandatory write (e.g. bug-verifier) finished
# without actually invoking it, rather than ending as a green no-op.
_enforce_required_writes(session_id)

# Use the unique session ID from trace attributes (includes repo prefix)
unique_session_id = trace_attributes.get("session.id", session_id)
eval_type = session_id.split("-")[0] if "-" in session_id else session_id
Expand Down
28 changes: 28 additions & 0 deletions strands-command/scripts/python/github_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,38 @@ def _github_get_all_pages(endpoint: str, repo: str | None = None) -> list[dict[s
return f"Error {e!s}"


# Names of write tools invoked during this process. Every write tool records its
# invocation here -- whether it executed against the GitHub API (write mode) or
# was recorded for deferred execution (read-only mode). The runner uses this to
# verify that mandatory write operations (e.g. a bug-verifier's triage labels)
# were actually attempted, rather than only described in the agent's prose.
_invoked_write_tools: list[str] = []


def get_invoked_write_tools() -> list[str]:
"""Return the names of write tools invoked so far in this process."""
return list(_invoked_write_tools)


def get_issue_label_names(issue_number: int, repo: str | None = None) -> list[str]:
"""Return the label names currently on an issue.

A lightweight read helper (not an agent tool) used to verify issue state --
e.g. whether a triage label already landed from a prior run. Returns an empty
list on any error rather than raising, so callers can treat "unknown" as
"no label present".
"""
result = _github_request("GET", f"issues/{issue_number}", repo)
if isinstance(result, dict):
return [label.get("name", "") for label in result.get("labels", [])]
return []


def check_should_call_write_api_or_record(func):
"""Decorator that checks if a write api should be called, or if the tool should record to JSONL."""
@wraps(func)
def wrapper(*args, **kwargs):
_invoked_write_tools.append(func.__name__)
try:
if not _should_call_write_api():
# Record the tool request to JSONL file
Expand Down
48 changes: 48 additions & 0 deletions strands-command/scripts/python/tests/test_agent_runner.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
"""Tests for the bug-verifier write guard in agent_runner."""
from unittest.mock import patch

import pytest

from .. import agent_runner


def test_guard_passes_when_label_tool_invoked_this_run():
"""A run that invoked add_issue_labels satisfies the guard."""
with patch.object(agent_runner, "get_invoked_write_tools", lambda: ["add_issue_labels"]):
agent_runner._enforce_required_writes("bug-verifier-3216") # must not raise


def test_guard_passes_when_comment_tool_invoked_this_run():
"""A comment on the issue also satisfies the guard."""
with patch.object(agent_runner, "get_invoked_write_tools", lambda: ["add_issue_comment"]):
agent_runner._enforce_required_writes("bug-verifier-3216") # must not raise


def test_guard_passes_on_resume_when_issue_already_has_triage_label():
"""A resumed run that writes nothing is accepted if a prior triage label landed."""
with patch.object(agent_runner, "get_invoked_write_tools", lambda: []), \
patch.object(agent_runner, "get_issue_label_names",
lambda *a, **k: ["bug", "bug-needs-info"]):
agent_runner._enforce_required_writes("bug-verifier-3216") # must not raise


def test_guard_fails_when_no_write_and_no_existing_triage_label():
"""The silent no-op: no issue-facing write and no prior triage label -> failure."""
with patch.object(agent_runner, "get_invoked_write_tools", lambda: []), \
patch.object(agent_runner, "get_issue_label_names",
lambda *a, **k: ["bug", "area-otel"]):
with pytest.raises(RuntimeError):
agent_runner._enforce_required_writes("bug-verifier-3216")


def test_guard_ignores_non_bug_verifier_modes():
"""Other modes have no mandatory-write guarantee and are never failed here."""
with patch.object(agent_runner, "get_invoked_write_tools", lambda: []):
agent_runner._enforce_required_writes("implementer-my-branch")
agent_runner._enforce_required_writes("reviewer-42")
agent_runner._enforce_required_writes(None)


def test_issue_number_parsed_from_session_id():
assert agent_runner._issue_number_from_session_id("bug-verifier-3216") == 3216
assert agent_runner._issue_number_from_session_id("bug-verifier-abc") is None
30 changes: 30 additions & 0 deletions strands-command/scripts/python/tests/test_github_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,33 @@ def fake_get(url, **kwargs):
# Should terminate (at the page cap) rather than hang.
out = github_tools.get_pr_files(1, repo="o/r")
assert "f.py" in out


def test_write_tool_invocation_is_tracked_in_write_mode():
"""Every write tool records its invocation so the runner can verify mandatory writes happened."""
github_tools._invoked_write_tools.clear()
with patch.object(github_tools, "_should_call_write_api", lambda: True), \
patch.object(github_tools, "_github_request",
lambda *a, **k: [{"name": "bug-needs-info"}]):
github_tools.add_issue_labels(3216, ["bug-needs-info"], repo="o/r")

assert "add_issue_labels" in github_tools.get_invoked_write_tools()


def test_write_tool_invocation_tracked_and_recorded_when_deferred(tmp_path, monkeypatch):
"""In read-only mode the write is recorded to JSONL *and* its invocation is tracked.

Guards the silent no-op: a bug-verifier that only describes applying a label
without calling the tool produces neither a tracked invocation nor a JSONL
record, which the runner now treats as a hard failure.
"""
monkeypatch.chdir(tmp_path)
github_tools._invoked_write_tools.clear()
with patch.object(github_tools, "_should_call_write_api", lambda: False):
result = github_tools.add_issue_labels(3216, ["bug-needs-info"], repo="o/r")

assert "add_issue_labels" in github_tools.get_invoked_write_tools()
jsonl = tmp_path / ".artifact" / "write_operations.jsonl"
assert jsonl.exists()
assert "add_issue_labels" in jsonl.read_text()
assert "deferred" in result.lower()