From f1d1d33d12f14f3f3c65b3cd9a31170e6477d637 Mon Sep 17 00:00:00 2001 From: Jack Yuan Date: Mon, 13 Jul 2026 17:04:05 -0400 Subject: [PATCH] fix(strands-command): fail bug-verifier runs that apply no label or comment A bug-verify run could finish green while applying nothing to the issue: the agent described a deferred triage label in its report without ever invoking add_issue_labels. Because the deferred-write record is only written when the tool is actually called, no write_operations artifact was produced, finalize had nothing to replay, and the issue got no label or comment -- with no error anywhere. Track write-tool invocations and, for bug-verifier sessions, fail the run unless it applied a label or comment this run, or (for a resumed session) a triage label from a prior run already exists on the issue. Session resumption is preserved. --- .../scripts/python/agent_runner.py | 75 ++++++++++++++++++- .../scripts/python/github_tools.py | 28 +++++++ .../scripts/python/tests/test_agent_runner.py | 48 ++++++++++++ .../scripts/python/tests/test_github_tools.py | 30 ++++++++ 4 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 strands-command/scripts/python/tests/test_agent_runner.py diff --git a/strands-command/scripts/python/agent_runner.py b/strands-command/scripts/python/agent_runner.py index 08af6a6..0d1d8ce 100644 --- a/strands-command/scripts/python/agent_runner.py +++ b/strands-command/scripts/python/agent_runner.py @@ -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, @@ -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 "-" (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. @@ -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 diff --git a/strands-command/scripts/python/github_tools.py b/strands-command/scripts/python/github_tools.py index 4a515a0..61ce05c 100644 --- a/strands-command/scripts/python/github_tools.py +++ b/strands-command/scripts/python/github_tools.py @@ -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 diff --git a/strands-command/scripts/python/tests/test_agent_runner.py b/strands-command/scripts/python/tests/test_agent_runner.py new file mode 100644 index 0000000..18110b1 --- /dev/null +++ b/strands-command/scripts/python/tests/test_agent_runner.py @@ -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 diff --git a/strands-command/scripts/python/tests/test_github_tools.py b/strands-command/scripts/python/tests/test_github_tools.py index f928a1e..99700c1 100644 --- a/strands-command/scripts/python/tests/test_github_tools.py +++ b/strands-command/scripts/python/tests/test_github_tools.py @@ -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()