mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(subagents): require test evidence beyond build success (#6868)
* fix(subagents): require test evidence beyond build success * test(subagents): isolate test evidence from sandbox file readers
This commit is contained in:
parent
4e6c18ad9c
commit
8f0ca58653
@ -2132,6 +2132,13 @@ Cancelled or timed-out background sub-agent executions retain provider-reported
|
||||
|
||||
For `tests_passed:go test ./...`, packages marked `[no test files]` or `[no tests to run]` do not veto a passing summary from another package. Runs with only zero-test package summaries remain `UNVERIFIED`; failures still take precedence. This checks recorded execution evidence, not claim correctness.
|
||||
|
||||
For Maven and Gradle test acceptance, a `BUILD SUCCESS` / `BUILD SUCCESSFUL`
|
||||
banner alone remains `UNVERIFIED`: builds can succeed without running tests.
|
||||
Complete `Tests run: N, Failures: F, Errors: E, Skipped: S` or `N tests completed`
|
||||
summaries establish a hold only when at least one test ran without a reported
|
||||
failure. Zero-test or all-skipped modules do not supply positive evidence;
|
||||
failures in any module take precedence over passing counts.
|
||||
|
||||
For file acceptance criteria, an empty regular file in the shared workspace can satisfy `file:<path> exists` and `file_written:<path>`, including on remote sandboxes. It fails `file:<path> non-empty` with a deterministic empty-file result.
|
||||
|
||||
To require CSV header columns, set a `task` or `batch_task` item's criterion to
|
||||
|
||||
File diff suppressed because one or more lines are too long
@ -119,7 +119,6 @@ _TEST_PASS_SHAPE_RE = re.compile(
|
||||
r"|^OK$" # unittest: bare OK line
|
||||
r"|test result: ok" # cargo test
|
||||
r"|^ok\s+\S" # go test: "ok \tpkg/path"
|
||||
r"|\bBUILD SUCCESS(?:FUL)?\b" # maven / gradle
|
||||
r"|\ball tests passed\b",
|
||||
re.IGNORECASE | re.MULTILINE,
|
||||
)
|
||||
@ -130,6 +129,52 @@ _TEST_ZERO_SHAPE_RE = re.compile(r"\b0\s+passed\b|\[no test files\]|\[no tests t
|
||||
|
||||
_GO_ZERO_TEST_LINE_RE = re.compile(r"^(?:ok|\?)[ \t]+\S+[^\r\n]*\[(?:no test files|no tests to run)\][ \t]*\r?$", re.MULTILINE)
|
||||
|
||||
# A successful build can skip every test. Require runner counts instead of
|
||||
# accepting Maven/Gradle's build banner as positive test evidence.
|
||||
_MAVEN_TEST_SUMMARY_RE = re.compile(
|
||||
r"^[ \t]*(?:\[(?:INFO|WARNING|ERROR)\][ \t]*)?Tests run:[ \t]*(?P<total>[0-9]+),[ \t]*"
|
||||
r"Failures:[ \t]*(?P<failed>[0-9]+),[ \t]*Errors:[ \t]*(?P<errors>[0-9]+),[ \t]*"
|
||||
r"Skipped:[ \t]*(?P<skipped>[0-9]+)(?:,[ \t]*Time elapsed:[^\r\n]*)?[ \t]*\r?$",
|
||||
re.MULTILINE,
|
||||
)
|
||||
_GRADLE_TEST_SUMMARY_RE = re.compile(
|
||||
r"^[ \t]*(?P<total>[0-9]+) tests? completed(?:,[ \t]*(?P<failed>[0-9]+) failed)?"
|
||||
r"(?:,[ \t]*(?P<skipped>[0-9]+) skipped)?[ \t]*\r?$",
|
||||
re.MULTILINE,
|
||||
)
|
||||
# Preserve the command-attribution guard for build strings, even though these
|
||||
# strings no longer suffice as positive test evidence.
|
||||
_BUILD_SUCCESS_RE = re.compile(r"\bBUILD SUCCESS(?:FUL)?\b", re.IGNORECASE)
|
||||
|
||||
|
||||
def _build_test_summary_holds(text: str) -> bool | None:
|
||||
"""Return count-based JVM evidence; ``None`` means no executed-test proof.
|
||||
|
||||
Empty/all-skipped modules cannot supply positive evidence, but do not
|
||||
veto other tested modules. Failures in any complete summary take priority.
|
||||
Inconsistent counters are not evidence, and build banners never fill in
|
||||
fields missing from a truncated summary.
|
||||
"""
|
||||
positive = False
|
||||
for pattern in (_MAVEN_TEST_SUMMARY_RE, _GRADLE_TEST_SUMMARY_RE):
|
||||
for match in pattern.finditer(text):
|
||||
# Bounded tool tails may still contain enormous decimal strings;
|
||||
# leave these unverified instead of hitting Python's int limit.
|
||||
fields = match.groupdict()
|
||||
if any(len(value) > 9 for value in fields.values() if value is not None):
|
||||
continue
|
||||
total = int(fields["total"])
|
||||
failed = int(fields.get("failed") or 0)
|
||||
errors = int(fields.get("errors") or 0)
|
||||
skipped = int(fields.get("skipped") or 0)
|
||||
if failed + errors + skipped > total:
|
||||
continue
|
||||
if failed or errors:
|
||||
return False
|
||||
positive = positive or total > skipped
|
||||
return True if positive else None
|
||||
|
||||
|
||||
_TEST_FAIL_SHAPE_RE = re.compile(
|
||||
r"\b[1-9]\d*\s+failed\b" # pytest / jest: "1 failed"
|
||||
r"|\b[1-9]\d*\s+errors?\b" # pytest: "1 error" — an errored collection means part of the selection never ran
|
||||
@ -784,7 +829,7 @@ def _carries_summary_shape(text: str) -> bool:
|
||||
or zero-test veto). The pass shapes match as substrings, so
|
||||
subagent-chosen strings — a ``cd`` argument, a ``CDPATH`` value — must
|
||||
not be allowed to lend the recorded tail a summary shape."""
|
||||
return bool(_TEST_PASS_SHAPE_RE.search(text) or _TEST_FAIL_SHAPE_RE.search(text) or _TEST_ZERO_SHAPE_RE.search(text))
|
||||
return bool(_TEST_PASS_SHAPE_RE.search(text) or _TEST_FAIL_SHAPE_RE.search(text) or _TEST_ZERO_SHAPE_RE.search(text) or _MAVEN_TEST_SUMMARY_RE.search(text) or _GRADLE_TEST_SUMMARY_RE.search(text) or _BUILD_SUCCESS_RE.search(text))
|
||||
|
||||
|
||||
def _normalize_cd_scope_path(path: str) -> tuple[str, bool] | None:
|
||||
@ -1941,13 +1986,14 @@ def _check_tests_passed_leaf(command: str, bash_executions: list[dict[str, Any]]
|
||||
# Neither a pass nor a fail shape here can be trusted either way.
|
||||
base["detail"] = unattributable
|
||||
return base
|
||||
if _TEST_FAIL_SHAPE_RE.search(output_tail):
|
||||
build_summary = _build_test_summary_holds(output_tail)
|
||||
if _TEST_FAIL_SHAPE_RE.search(output_tail) or build_summary is False:
|
||||
base["checked"] = True
|
||||
base["detail"] = "recorded output carries a failing test summary"
|
||||
return base
|
||||
# Go zero-test markers describe one package; exclude the whole line from pass evidence.
|
||||
summary_tail = _GO_ZERO_TEST_LINE_RE.sub("", output_tail)
|
||||
if _TEST_PASS_SHAPE_RE.search(summary_tail) and not _TEST_ZERO_SHAPE_RE.search(summary_tail):
|
||||
if (_TEST_PASS_SHAPE_RE.search(summary_tail) or build_summary is True) and not _TEST_ZERO_SHAPE_RE.search(summary_tail):
|
||||
base["checked"] = True
|
||||
base["holds"] = True
|
||||
base["detail"] = "recorded output carries a passing test summary"
|
||||
|
||||
113
backend/tests/test_acceptance_build_summaries.py
Normal file
113
backend/tests/test_acceptance_build_summaries.py
Normal file
@ -0,0 +1,113 @@
|
||||
"""Build completion is not proof that delegated tests ran successfully."""
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.subagents.acceptance_checks import check_acceptance_criteria, render_acceptance_section
|
||||
|
||||
|
||||
def _check(output: str, command: str = "mvn test", **overrides):
|
||||
execution = {
|
||||
"tool_call_id": "test-build",
|
||||
"tool_name": "bash",
|
||||
"command": command,
|
||||
"output_tail": output,
|
||||
"status": "success",
|
||||
"shell_persistent": False,
|
||||
**overrides,
|
||||
}
|
||||
return check_acceptance_criteria([f"tests_passed:{command}"], bash_executions=[execution])
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"output,command",
|
||||
[
|
||||
("[INFO] BUILD SUCCESS", "mvn test"),
|
||||
("[INFO] Tests are skipped.\n[INFO] BUILD SUCCESS", "mvn test -DskipTests"),
|
||||
("[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0\n[INFO] BUILD SUCCESS", "mvn test"),
|
||||
("[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 3\n[INFO] BUILD SUCCESS", "mvn test"),
|
||||
("> Task :test NO-SOURCE\nBUILD SUCCESSFUL in 1s", "gradle test"),
|
||||
("> Task :test SKIPPED\nBUILD SUCCESSFUL in 1s", "gradle test"),
|
||||
("0 tests completed\nBUILD SUCCESSFUL", "gradle test"),
|
||||
("3 tests completed, 3 skipped\nBUILD SUCCESSFUL", "gradle test"),
|
||||
],
|
||||
)
|
||||
def test_build_without_executed_tests_remains_unverified(output, command):
|
||||
verdict = _check(output, command)
|
||||
leaf = verdict["leaves"][0]
|
||||
assert (leaf["checked"], leaf["holds"]) == (False, False)
|
||||
assert verdict["all_hold"] is False
|
||||
assert verdict["unchecked"] == [f"tests_passed:{command}"]
|
||||
assert "[UNVERIFIED]" in render_acceptance_section(verdict)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"output,command",
|
||||
[
|
||||
("[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0", "mvn test"),
|
||||
("[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.2 s -- in TestExample", "mvn test"),
|
||||
("Tests run: 1, Failures: 0, Errors: 0, Skipped: 0\r\n[INFO] BUILD SUCCESS", "mvn test"),
|
||||
("3 tests completed, 0 failed, 1 skipped\nBUILD SUCCESSFUL", "gradle test"),
|
||||
("3 tests completed\nBUILD SUCCESSFUL", "gradle test"),
|
||||
],
|
||||
)
|
||||
def test_nonzero_test_counts_are_positive_evidence(output, command):
|
||||
verdict = _check(output, command)
|
||||
assert verdict["all_hold"] is True
|
||||
assert "[holds]" in render_acceptance_section(verdict)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind", ["Failures", "Errors"])
|
||||
def test_maven_failure_counts_override_ignored_failure_build_success(kind):
|
||||
counts = {"Failures": 0, "Errors": 0, kind: 1}
|
||||
output = f"[INFO] Tests run: 3, Failures: {counts['Failures']}, Errors: {counts['Errors']}, Skipped: 0\n[INFO] BUILD SUCCESS"
|
||||
verdict = _check(output, "mvn test -Dmaven.test.failure.ignore=true")
|
||||
assert (verdict["leaves"][0]["checked"], verdict["leaves"][0]["holds"]) == (True, False)
|
||||
assert "[does not hold]" in render_acceptance_section(verdict)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("reverse", [False, True])
|
||||
def test_empty_maven_module_does_not_veto_a_tested_module(reverse):
|
||||
summaries = [
|
||||
"[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0",
|
||||
"[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 1",
|
||||
]
|
||||
if reverse:
|
||||
summaries.reverse()
|
||||
assert _check("\n".join(summaries))["all_hold"] is True
|
||||
|
||||
|
||||
def test_failing_module_overrides_a_passing_module():
|
||||
output = "[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0\n[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0"
|
||||
leaf = _check(output)["leaves"][0]
|
||||
assert (leaf["checked"], leaf["holds"]) == (True, False)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"summary",
|
||||
[
|
||||
"Tests run: 1, Failures: 0, Errors: 0, Skipped: 2",
|
||||
"Tests run: -1, Failures: 0, Errors: 0, Skipped: 0",
|
||||
"Tests run: 3, Failures: 0, Errors:",
|
||||
"Tests run: three, Failures: 0, Errors: 0, Skipped: 0",
|
||||
"3 tests completed, 4 skipped",
|
||||
],
|
||||
)
|
||||
def test_inconsistent_or_incomplete_count_summary_is_not_proof(summary):
|
||||
leaf = _check(summary + "\nBUILD SUCCESS")["leaves"][0]
|
||||
assert (leaf["checked"], leaf["holds"]) == (False, False)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("persistent", [None, True])
|
||||
def test_count_summary_does_not_bypass_shell_provenance(persistent):
|
||||
verdict = _check("Tests run: 3, Failures: 0, Errors: 0, Skipped: 0", shell_persistent=persistent)
|
||||
assert verdict["leaves"][0]["checked"] is False
|
||||
|
||||
|
||||
def test_count_summary_does_not_bypass_output_attribution():
|
||||
verdict = _check("Tests run: 3, Failures: 0, Errors: 0, Skipped: 0", "echo 'unrelated'; mvn test")
|
||||
assert verdict["leaves"][0]["checked"] is False
|
||||
|
||||
|
||||
def test_recorded_failure_status_still_overrides_positive_counts():
|
||||
leaf = _check("Tests run: 3, Failures: 0, Errors: 0, Skipped: 0", status="error")["leaves"][0]
|
||||
assert (leaf["checked"], leaf["holds"]) == (True, False)
|
||||
@ -2503,7 +2503,7 @@ class TestTestsPassedLeaf:
|
||||
".....\nOK\n",
|
||||
"test result: ok. 5 passed; 0 failed",
|
||||
"ok \tgithub.com/example/pkg\t0.5s",
|
||||
"BUILD SUCCESSFUL",
|
||||
"3 tests completed, 0 failed\nBUILD SUCCESSFUL",
|
||||
"All tests passed!",
|
||||
],
|
||||
)
|
||||
|
||||
@ -6293,6 +6293,41 @@ class TestBashExecutionHarvest:
|
||||
assert entry["status"] == "success"
|
||||
assert "12 passed" in entry["output_tail"]
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"output,checked,holds",
|
||||
[
|
||||
("[INFO] BUILD SUCCESS", False, False),
|
||||
("Tests run: 3, Failures: 0, Errors: 0, Skipped: 3\nBUILD SUCCESS", False, False),
|
||||
("Tests run: 3, Failures: 1, Errors: 0, Skipped: 0\nBUILD SUCCESS", True, False),
|
||||
("Tests run: 3, Failures: 0, Errors: 0, Skipped: 0\nBUILD SUCCESS", True, True),
|
||||
],
|
||||
)
|
||||
def test_build_summary_reaches_acceptance_through_stamped_tool_messages(self, classes, monkeypatch, output, checked, holds):
|
||||
from deerflow.subagents.acceptance_checks import check_acceptance_criteria, render_acceptance_section
|
||||
|
||||
executor_module = importlib.import_module("deerflow.subagents.executor")
|
||||
monkeypatch.setitem(sys.modules, "deerflow.agents.middlewares.tool_result_meta", _module("deerflow.agents.middlewares.tool_result_meta", TOOL_META_KEY="deerflow_tool_meta"))
|
||||
sandbox = SimpleNamespace(persistent_shell_sessions=False)
|
||||
monkeypatch.setattr("deerflow.sandbox.sandbox_provider.get_sandbox_provider", lambda: SimpleNamespace(get=lambda _id: sandbox))
|
||||
state = self._final_state(classes)
|
||||
state["sandbox"] = {"sandbox_id": "sb-jvm"}
|
||||
state["messages"][1].tool_calls[0]["args"]["command"] = "mvn test"
|
||||
state["messages"][2].content = output
|
||||
state["messages"][2].additional_kwargs["deerflow_tool_meta"] = {"status": "success"}
|
||||
|
||||
executions = executor_module._harvest_bash_executions(state)
|
||||
assert executions[0]["shell_persistent"] is False
|
||||
verdict = check_acceptance_criteria(
|
||||
["tests_passed:mvn test"],
|
||||
bash_executions=executions,
|
||||
content_reader=lambda *_args: pytest.fail("test-summary acceptance must not read files"),
|
||||
)
|
||||
leaf = verdict["leaves"][0]
|
||||
assert (leaf["checked"], leaf["holds"]) == (checked, holds)
|
||||
assert verdict["all_hold"] is holds
|
||||
if not checked:
|
||||
assert "[UNVERIFIED]" in render_acceptance_section(verdict)
|
||||
|
||||
def test_status_comes_from_tool_meta_when_present(self, classes, monkeypatch):
|
||||
executor_module = importlib.import_module("deerflow.subagents.executor")
|
||||
monkeypatch.setitem(sys.modules, "deerflow.agents.middlewares.tool_result_meta", _module("deerflow.agents.middlewares.tool_result_meta", TOOL_META_KEY="deerflow_tool_meta"))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user