diff --git a/README.md b/README.md index fc5c9ff72f..4423afe8df 100644 --- a/README.md +++ b/README.md @@ -2208,6 +2208,12 @@ failures do not count as passes. Successful runs with all-skipped, all-expected-failure, empty or incomplete summaries remain `UNVERIFIED`; nonzero exits, failures and unexpected successes still take precedence. +For `tests_passed:node --test`, complete TAP or spec counter summaries must +contain an actual pass and no failed or cancelled tests. Individual TAP `ok` +points do not prove the run passed. Empty, all-skipped, all-todo, incomplete or +inconsistent summaries remain `UNVERIFIED`; execution and shell-provenance +checks still apply. + 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` diff --git a/backend/packages/harness/deerflow/subagents/AGENTS.md b/backend/packages/harness/deerflow/subagents/AGENTS.md index 3e89302ace..5800209dae 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -91,6 +91,12 @@ These summaries also participate in the command/output attribution guard. Cargo: complete positive siblings permit empty targets. All-empty/ignored or bare banners stay UNVERIFIED; failures win. Unittest needs complete counts with ordinary passes; uppercase OK is not Go output. Failures win. +Node TAP/spec acceptance uses complete, consistent tests/suites/pass/fail/cancelled/skipped/todo counters. +Normalize ANSI SGR for evidence matching and the output-attribution guard. +TAP `ok N` points are not Go package passes; skip/todo-only runs supply no proof. +Failures/cancellations win, and the normal command/output admission guards still apply. +Bounded bash evidence drops its first partial physical line when tail slicing +cuts a line; otherwise a long test label can lose its reporter prefix. **Events**: `task_started`, `task_running`, `task_completed`/`task_failed`/`task_timed_out` **Handled LLM failures**: `LLMErrorHandlingMiddleware` deliberately converts provider/model exceptions into an `AIMessage` so the graph can end cleanly, stamping `additional_kwargs.deerflow_error_fallback=true` plus error metadata. Clean graph termination does not imply subagent success: `SubagentExecutor` inspects the last assistant message at terminalization and maps a marked fallback to `SubagentStatus.FAILED`, which then emits `task_failed` and the existing structured `subagent_error`. Only the marker is authoritative — error-looking assistant prose without it remains a normal completed result, so neither the executor nor frontend parses display text as a status protocol. diff --git a/backend/packages/harness/deerflow/subagents/acceptance_checks.py b/backend/packages/harness/deerflow/subagents/acceptance_checks.py index 927e703780..ce66569117 100644 --- a/backend/packages/harness/deerflow/subagents/acceptance_checks.py +++ b/backend/packages/harness/deerflow/subagents/acceptance_checks.py @@ -146,6 +146,36 @@ def _unittest_test_summary_holds(text: str) -> bool: return False +# Node's TAP/spec reporters share the same ordered counters. Individual TAP +# points (including skipped/todo tests) are not a complete run summary and +# must not be mistaken for Go's package-level `ok` record above. +_NODE_TEST_POINT_RE = re.compile(r"^[ \t]*(?:\#[ \t]+Subtest:[^\r\n]*|(?:not )?ok[ \t]+[0-9]+(?:[ \t]+[-#][^\r\n]*|[ \t]*)|[✔✖﹣▶⚠][ \t]+[^\r\n]*)\r?$", re.MULTILINE) +_ANSI_SGR_RE = re.compile(r"\x1b\[[0-9;:]*m") +_NODE_TEST_SUMMARY_RE = re.compile( + r"^(?P\#|ℹ)[ \t]+tests (?P[0-9]{1,9})\r?\n" + r"(?P=prefix)[ \t]+suites [0-9]{1,9}\r?\n" + r"(?P=prefix)[ \t]+pass (?P[0-9]{1,9})\r?\n" + r"(?P=prefix)[ \t]+fail (?P[0-9]{1,9})\r?\n" + r"(?P=prefix)[ \t]+cancelled (?P[0-9]{1,9})\r?\n" + r"(?P=prefix)[ \t]+skipped (?P[0-9]{1,9})\r?\n" + r"(?P=prefix)[ \t]+todo (?P[0-9]{1,9})[ \t]*\r?$", + re.MULTILINE, +) + + +def _node_test_summary_holds(text: str) -> bool | None: + """Use complete, consistent counts; failures/cancellations win.""" + positive = False + for match in _NODE_TEST_SUMMARY_RE.finditer(text): + total, passed, failed, cancelled, skipped, todo = (int(match[key]) for key in ("total", "passed", "failed", "cancelled", "skipped", "todo")) + if total != passed + failed + cancelled + skipped + todo: + continue + if failed or cancelled: + return False + positive = positive or passed > 0 + return True if positive else None + + #: Zero-test evidence vetoes the pass shapes the count-bearing alternatives #: cannot see: "0 passed", go's no-test markers, unittest "Ran 0 tests". _TEST_ZERO_SHAPE_RE = re.compile(r"\b0\s+passed\b|\[no test files\]|\[no tests to run\]|\bRan 0 tests\b", re.IGNORECASE) @@ -238,8 +268,8 @@ def _build_test_summary_holds(text: str) -> bool | 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 + r"\b[1-9]\d*[ \t]+failed\b" # pytest / jest: same-line "1 failed", never a diagnostic location on the previous line + r"|\b[1-9]\d*[ \t]+errors?\b" # pytest: "1 error" — an errored collection means part of the selection never ran r"|^FAILED\b" # unittest summary line r"|^ERROR\s+\S" # pytest short summary: "ERROR tests/unit/test_auth.py" r"|test result: FAILED" # cargo test @@ -891,6 +921,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.""" + text = _ANSI_SGR_RE.sub("", text) return bool( _TEST_PASS_SHAPE_RE.search(text) or _GO_TEST_PASS_SHAPE_RE.search(text) @@ -900,6 +931,7 @@ def _carries_summary_shape(text: str) -> bool: or _MAVEN_TEST_SUMMARY_RE.search(text) or _GRADLE_TEST_SUMMARY_RE.search(text) or _BUILD_SUCCESS_RE.search(text) + or _NODE_TEST_SUMMARY_RE.search(text) ) @@ -2051,21 +2083,25 @@ def _check_tests_passed_leaf(command: str, bash_executions: list[dict[str, Any]] else: base["detail"] = f"latest matching run recorded status={status or 'unknown'}" return base - output_tail = str(latest.get("output_tail") or "") + output_tail = _ANSI_SGR_RE.sub("", str(latest.get("output_tail") or "")) unattributable = _output_attribution(str(latest.get("command") or ""), thread_data) if unattributable is not None: # Neither a pass nor a fail shape here can be trusted either way. base["detail"] = unattributable return base + # Test names are arbitrary data, not runner summaries. Exclude TAP/spec + # point labels from negative/zero evidence too (`test('1 failed', ...)`). + output_tail = _NODE_TEST_POINT_RE.sub("", output_tail) build_summary = _build_test_summary_holds(output_tail) - if _TEST_FAIL_SHAPE_RE.search(output_tail) or build_summary is False: + node_summary = _node_test_summary_holds(output_tail) + if _TEST_FAIL_SHAPE_RE.search(output_tail) or build_summary is False or node_summary is False: base["checked"] = True base["detail"] = "recorded output carries a failing test summary" return base # Empty packages/targets cannot veto complete positive sibling evidence. summary_tail = _without_empty_cargo_targets(_GO_ZERO_TEST_LINE_RE.sub("", output_tail)) go_ok = _GO_TEST_PASS_SHAPE_RE.search(summary_tail) and _go_package_summary_is_evidence(summary_tail) - passing = _TEST_PASS_SHAPE_RE.search(summary_tail) or go_ok or _unittest_test_summary_holds(summary_tail) or build_summary is True + passing = _TEST_PASS_SHAPE_RE.search(summary_tail) or go_ok or _unittest_test_summary_holds(summary_tail) or build_summary is True or node_summary is True if passing and not _TEST_ZERO_SHAPE_RE.search(summary_tail): base["checked"] = True base["holds"] = True diff --git a/backend/packages/harness/deerflow/subagents/executor.py b/backend/packages/harness/deerflow/subagents/executor.py index efd680bdfb..506f9a053c 100644 --- a/backend/packages/harness/deerflow/subagents/executor.py +++ b/backend/packages/harness/deerflow/subagents/executor.py @@ -590,13 +590,18 @@ def _harvest_bash_executions( meta_status = str(meta.get("status") or getattr(message, "status", "success") or "success") content = message.content if isinstance(message.content, str) else json.dumps(message.content, sort_keys=True, default=str) status, status_marker = _bash_evidence_status(content, meta_status) + output_tail = content[-_BASH_EVIDENCE_OUTPUT_TAIL_CHARS:] + if len(content) > _BASH_EVIDENCE_OUTPUT_TAIL_CHARS and content[-_BASH_EVIDENCE_OUTPUT_TAIL_CHARS - 1] != "\n": + # A clipped test name/log line can look like a summary after + # losing its reporter prefix. Retain only complete lines. + output_tail = output_tail.partition("\n")[2] executions.append( { "tool_call_id": tool_call_id, "tool_name": name, "command": command, "command_truncated": command_truncated, - "output_tail": content[-_BASH_EVIDENCE_OUTPUT_TAIL_CHARS:], + "output_tail": output_tail, "status": status, "status_marker": status_marker, } diff --git a/backend/tests/test_node_test_acceptance.py b/backend/tests/test_node_test_acceptance.py new file mode 100644 index 0000000000..8abce8daa7 --- /dev/null +++ b/backend/tests/test_node_test_acceptance.py @@ -0,0 +1,241 @@ +"""Node's native reporters must prove executed tests, not TAP point labels.""" + +import asyncio +import importlib.util +import os +import shlex +import shutil +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest +from langchain_core.messages import AIMessage, ToolMessage +from langchain_core.tools import StructuredTool +from langgraph.graph import END, START, MessagesState, StateGraph +from langgraph.prebuilt import ToolNode + +from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware +from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping +from deerflow.subagents.acceptance_checks import _carries_summary_shape, check_acceptance_criteria, render_acceptance_section + + +def _summary(prefix="#", *, passed=1, failed=0, cancelled=0, skipped=0, todo=0, total=None): + counters = {"tests": passed + failed + cancelled + skipped + todo if total is None else total, "suites": 0, "pass": passed, "fail": failed, "cancelled": cancelled, "skipped": skipped, "todo": todo} + return "\n".join(f"{prefix} {key} {value}" for key, value in counters.items()) + f"\n{prefix} duration_ms 12.5" + + +def _verdict(output, **overrides): + execution = {"command": "node --test", "status": "success", "shell_persistent": False, "output_tail": output, **overrides} + return check_acceptance_criteria(["tests_passed:node --test"], bash_executions=[execution]) + + +@pytest.mark.parametrize("prefix", ["#", "ℹ"]) +@pytest.mark.parametrize("newline", ["\n", "\r\n"]) +def test_native_reporters_accept_nonempty_passes(prefix, newline): + verdict = _verdict(_summary(prefix, skipped=1, todo=1).replace("\n", newline)) + assert verdict["all_hold"] is True + assert "[holds]" in render_acceptance_section(verdict) + + +@pytest.mark.parametrize("prefix", ["#", "ℹ"]) +@pytest.mark.parametrize("counts", [{"failed": 1}, {"cancelled": 1}]) +def test_native_failures_override_incidental_pass_text(prefix, counts): + verdict = _verdict("ok 1 - all tests passed\n" + _summary(prefix, **counts)) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (True, False) + + +@pytest.mark.parametrize("prefix", ["#", "ℹ"]) +@pytest.mark.parametrize("counts", [{}, {"skipped": 2}, {"todo": 2}, {"skipped": 1, "todo": 1}]) +def test_empty_skipped_and_todo_summaries_supply_no_proof(prefix, counts): + verdict = _verdict("ok 1 - all tests passed # SKIP\n" + _summary(prefix, passed=0, **counts)) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +@pytest.mark.parametrize( + "output", + [ + "ok 1 - sample", + "ok 12 # SKIP", + _summary(total=4), + _summary(passed="9" * 100, total=1), + _summary().replace("# fail 0\n", ""), + _summary().replace("# fail 0", "ℹ fail 0"), + _summary().replace("# pass 1", "# pass -1"), + _summary().split("# todo")[0], + ], +) +def test_tap_points_and_incomplete_or_inconsistent_counts_remain_unverified(output): + assert _verdict(output)["leaves"][0]["checked"] is False + + +@pytest.mark.parametrize("reverse", [False, True]) +def test_failing_summary_wins_over_successful_sibling(reverse): + summaries = [_summary(), _summary(failed=1)] + verdict = _verdict("\n".join(reversed(summaries) if reverse else summaries)) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (True, False) + + +@pytest.mark.parametrize( + "overrides", + [{"shell_persistent": True}, {"shell_persistent": None}, {"command_truncated": True}, {"command": "node --test sample.test.cjs"}, {"command": "echo other; node --test"}], +) +def test_node_counts_do_not_bypass_execution_admission(overrides): + assert _verdict(_summary(), **overrides)["leaves"][0]["checked"] is False + + +def test_go_package_and_pytest_controls_still_pass(): + assert _verdict("ok\texample.org/project\t0.005s")["all_hold"] is True + assert _verdict("ok\t123\t0.005s")["all_hold"] is True + assert _verdict("2 passed in 0.01s")["all_hold"] is True + + +@pytest.mark.parametrize( + "point", + [ + "ok 1 - all tests passed # SKIP", + "ok 12 # all tests passed", + "✔ all tests passed (1ms)", + "﹣ all tests passed (1ms)", + "▶ all tests passed", + "⚠ all tests passed", + " ok 1 - all tests passed # SKIP", + "# Subtest: all tests passed\nok 1 - all tests passed # SKIP", + ], +) +@pytest.mark.parametrize("tail", ["", _summary(passed=0, skipped=1).split("# todo")[0]]) +def test_point_labels_cannot_supply_pass_evidence(point, tail): + verdict = _verdict(point + "\n" + tail) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +def test_recorded_exit_error_overrides_node_positive_counts(): + verdict = _verdict(_summary(), status="error") + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (True, False) + + +def test_newest_matching_execution_is_decisive(): + first = {"command": "node --test", "status": "success", "shell_persistent": False, "output_tail": _summary(failed=1)} + second = {**first, "output_tail": _summary()} + assert check_acceptance_criteria(["tests_passed:node --test"], bash_executions=[first, second])["all_hold"] is True + assert check_acceptance_criteria(["tests_passed:node --test"], bash_executions=[second, first])["all_hold"] is False + + +def test_empty_node_sibling_does_not_veto_another_runner(): + output = "2 passed in 0.01s\n" + _summary(passed=0) + execution = {"command": "make test", "status": "success", "shell_persistent": False, "output_tail": output} + assert check_acceptance_criteria(["tests_passed:make test"], bash_executions=[execution])["all_hold"] is True + + +@pytest.mark.parametrize("label", ["0 passed", "1 failed", "1 error"]) +@pytest.mark.parametrize("point", ["# Subtest: {label}\n ok 1 - {label}", "⚠ {label} # TODO"]) +def test_test_names_cannot_supply_failure_or_zero_evidence(label, point): + verdict = _verdict(point.format(label=label) + "\n" + _summary()) + assert verdict["all_hold"] is True + + +@pytest.mark.parametrize("newline", ["\n", "\r\n"]) +def test_todo_diagnostic_location_cannot_supply_error_count(newline): + output = "⚠ 1 failed # TODO\n" + _summary("ℹ", todo=1) + "\ntest at sample.test.cjs:1:57\n\n Error: fixture" + assert _verdict(output.replace("\n", newline))["all_hold"] is True + + +def test_color_codes_are_normalized_for_counts_and_output_admission(): + output = "\x1b[90m﹣ all tests passed\x1b[39m\n" + "\n".join(f"\x1b[34m{line}\x1b[39m" for line in _summary("ℹ", passed=0, skipped=1).splitlines()) + assert _verdict(output)["leaves"][0]["checked"] is False + assert _carries_summary_shape(output) is True + + +@pytest.fixture +def executor_module(monkeypatch): + path = Path(__file__).parents[1] / "packages/harness/deerflow/subagents/executor.py" + spec = importlib.util.spec_from_file_location("_node_evidence_executor", path) + module = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, spec.name, module) + spec.loader.exec_module(module) + return module + + +class SandboxMessages(MessagesState): + sandbox: dict + + +@pytest.mark.parametrize("label", ["all tests passed", "1 failed", "0 passed"]) +def test_harvest_drops_a_partial_label_before_interpreting_counts(executor_module, monkeypatch, label): + output = "ok 1 - " + "x" * 1500 + " " + label + " # SKIP\n" + _summary(passed=0, skipped=1) + messages = [AIMessage(content="", tool_calls=[{"name": "bash", "id": "long-name", "args": {"command": "node --test"}}]), ToolMessage(content=output, tool_call_id="long-name")] + monkeypatch.setattr(executor_module, "_harvest_shell_persistence", lambda state: False) + executions = executor_module._harvest_bash_executions({"messages": messages}) + assert executions[0]["output_tail"] == _summary(passed=0, skipped=1) + assert messages[-1].content == output + verdict = check_acceptance_criteria(["tests_passed:node --test"], bash_executions=executions) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +@pytest.mark.skipif(os.name == "nt" or shutil.which("node") is None, reason="Requires POSIX LocalSandbox and Node.js") +@pytest.mark.asyncio +@pytest.mark.parametrize("reporter", ["tap", "spec", "color-spec"]) +@pytest.mark.parametrize( + "body,expected", + [ + ("test('sample', () => {});", (True, True)), + ("test('0 passed', () => {}); test('1 failed', () => {});", (True, True)), + ("test('sample', () => {}); test.skip('optional', () => {});", (True, True)), + ("test.skip('all tests passed', () => {});", (False, False)), + ("test.skip('x'.repeat(1500) + ' all tests passed', () => {});", (False, False)), + ("test('x'.repeat(1500) + ' 1 failed', () => {});", (True, True)), + ("test('x'.repeat(1500) + ' 0 passed', () => {});", (True, True)), + ("test.todo('pending');", (False, False)), + ("test('all tests passed', { todo: true }, () => { throw Error('fixture'); });", (False, False)), + ("test('sample', () => {}); test('1 failed', { todo: true }, () => { throw Error('fixture'); });", (True, True)), + ("describe('empty suite', () => {});", (False, False)), + ("describe('all tests passed', () => { test.skip('optional', () => {}); });", (False, False)), + ("describe('all tests passed', () => { test.todo('pending'); });", (False, False)), + ("describe('1 failed', () => { test('sample', () => {}); });", (True, True)), + ("test('sample', () => { throw Error('fixture'); });", (True, False)), + ("test('pending', { timeout: 20 }, () => new Promise(() => {}));", (True, False)), + ], + ids=[ + "pass", + "summary-like-names", + "pass-and-skip", + "all-skipped", + "long-skipped", + "long-failure-name", + "long-zero-name", + "all-todo", + "failing-todo-pass-name", + "failing-todo-failure-name", + "empty-suite", + "skipped-suite", + "todo-suite", + "summary-like-suite", + "failure", + "cancelled", + ], +) +async def test_real_node_results_reach_parent_acceptance(tmp_path, monkeypatch, executor_module, reporter, body, expected): + (tmp_path / "sample.test.cjs").write_text("const { test, describe } = require('node:test');\n" + body, encoding="utf-8") + sandbox = LocalSandbox("node-evidence", path_mappings=[PathMapping("/mnt/user-data/workspace", str(tmp_path))]) + monkeypatch.setattr("deerflow.sandbox.sandbox_provider.get_sandbox_provider", lambda: SimpleNamespace(get=lambda sandbox_id: sandbox if sandbox_id == sandbox.id else None)) + outputs = [] + + async def bash(command: str) -> str: + output = await asyncio.to_thread(sandbox.execute_command, command, timeout=30) + outputs.append(output) + return output + + tool = StructuredTool.from_function(coroutine=bash, name="bash", description="Run offline native Node tests.") + graph = StateGraph(SandboxMessages) + graph.add_node("tools", ToolNode([tool], awrap_tool_call=ToolErrorHandlingMiddleware().awrap_tool_call)) + graph.add_edge(START, "tools") + graph.add_edge("tools", END) + color = "FORCE_COLOR=1 " if reporter == "color-spec" else "" + reporter = reporter.removeprefix("color-") + command = f"{color}{shlex.quote(shutil.which('node'))} --test --test-reporter={reporter} /mnt/user-data/workspace/sample.test.cjs" + state = await graph.compile().ainvoke({"sandbox": {"sandbox_id": sandbox.id}, "messages": [AIMessage(content="", tool_calls=[{"name": "bash", "id": "node-call", "args": {"command": command}}])]}) + assert len(outputs) == 1 and "duration_ms" in outputs[0], outputs + executions = executor_module._harvest_bash_executions(state) + assert len(executions) == 1 and executions[0]["shell_persistent"] is False + verdict = check_acceptance_criteria([f"tests_passed:{command}"], bash_executions=executions) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == expected, (outputs, verdict) diff --git a/backend/tests/test_subagent_executor.py b/backend/tests/test_subagent_executor.py index a39667fcbe..38f369fd36 100644 --- a/backend/tests/test_subagent_executor.py +++ b/backend/tests/test_subagent_executor.py @@ -6509,15 +6509,28 @@ class TestBashExecutionHarvest: assert result.bash_executions is not None assert [e["command"] for e in result.bash_executions] == ["make test"] - def test_output_tail_is_bounded(self, classes, monkeypatch): + @pytest.mark.parametrize( + "output,expected,status", + [ + ("x" * 5000, "", "success"), + ("x" * 5000 + "\n12 passed in 1s", "12 passed in 1s", "success"), + ("x" * 5000 + "\n" + "a" * 1000, "a" * 1000, "success"), + ("x" * 5000 + "\r\n" + "a" * 1000, "a" * 1000, "success"), + ("x" * 5000 + "\n" + "a" * 1001, "", "success"), + ("x" * 5000 + "\nExit Code: 3", "Exit Code: 3", "error"), + ], + ) + def test_output_tail_is_bounded(self, classes, monkeypatch, output, expected, status): 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")) state = self._final_state(classes) - state["messages"][2].content = "x" * 5000 + state["messages"][2].content = output executions = executor_module._harvest_bash_executions(state) - assert len(executions[0]["output_tail"]) == 1000 + assert len(executions[0]["output_tail"]) <= 1000 + assert executions[0]["output_tail"] == expected + assert executions[0]["status"] == status def test_long_command_is_capped_and_flagged_truncated(self, classes, monkeypatch): """PR review: the matcher must know the command lost its suffix."""