From 7bee2be22b16dc4331a64dfa6d80fe1fb22dd564 Mon Sep 17 00:00:00 2001 From: Guo-Yixin Date: Wed, 14 Oct 2026 18:48:00 -0500 Subject: [PATCH] fix(acceptance): keep empty Cargo targets from vetoing executed tests (#6891) --- README.md | 6 + .../harness/deerflow/subagents/AGENTS.md | 27 +-- .../deerflow/subagents/acceptance_checks.py | 23 ++- backend/tests/test_cargo_acceptance.py | 178 ++++++++++++++++++ 4 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 backend/tests/test_cargo_acceptance.py diff --git a/README.md b/README.md index 6a0a59f307..50db98b04b 100644 --- a/README.md +++ b/README.md @@ -2146,6 +2146,12 @@ 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 `tests_passed:cargo test`, a complete passing target summary can coexist with +empty unit, integration or documentation targets. Complete zero-pass successful +summaries do not veto that sibling evidence. All-empty or all-ignored runs and +bare `test result: ok` banners remain `UNVERIFIED`; incomplete zero-test summaries +are kept as vetoes, and failures still take precedence. + 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 267f118095..4df3ff98dd 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -1,19 +1,18 @@ ### Subagent System (`packages/harness/deerflow/subagents/`) -Validated ToolMessage knowledge evidence accumulates across values frames in -`SubagentResult.knowledge_sources` (100 sources, deduplicated by evidence ID). -Publication precedes terminal state and survives compaction. Ordinary `task` -forwards reported handles with a bounded evidence appendix and source artifact. -Mixed task results keep native RAGFlow snapshots in `knowledge_sources` and -shared evidence in `retrieval_sources`; pruning must preserve the native snapshot. -Shared evidence scans start at the changed message suffix; compaction or prefix -replacement resets that boundary. Null frames must not crash the accumulator. +Accumulate validated ToolMessage knowledge across values frames in +`SubagentResult.knowledge_sources`: at most 100 sources, deduplicated by evidence ID. +Publish before terminal state; retain through compaction. Ordinary `task` forwards +reported handles via a bounded appendix/artifact. Mixed artifacts keep native +RAGFlow snapshots in `knowledge_sources` and shared `retrieval_sources`; pruning +preserves native snapshots. Scan changed message suffixes; reset on compaction or +prefix replacement, and tolerate null frames. -Result-reader acceptance runs on SQLite and, with TEST_POSTGRES_URI, an isolated -PostgreSQL schema; both exercise submission, completion, reopen and ToolNode -continuation without paid model work. Always clean up the temporary schema. +Result-reader tests cover SQLite and isolated TEST_POSTGRES_URI schemas: +submission, completion, reopen and ToolNode continuation, without paid models. +Clean up temporary schemas. -**CSV headers**: `file: csv-columns:["id","name"]` uses shared file admission and authorized bounded reads. Require UTF-8-encodable decoded column names before file access. Parse only the first logical record; definite syntax errors within the header budget remain checked negatives regardless of later rows, while truncated quoting and exhausted budgets stay UNVERIFIED. Local reads and parsing share a cooperative 5-second deadline; remote transport keeps its own timeouts and parsing starts a separate 5-second deadline after the read returns. See README for exact matching and dialect. +**CSV headers**: `file: csv-columns:["id","name"]` shares authorized file admission/bounded reads. Require UTF-8-encodable decoded names before access. Parse the first logical record: definite header syntax errors are checked negatives even with later rows; truncated quoting/exhausted budgets stay UNVERIFIED. Local read/parse share a cooperative 5s deadline; remote keeps transport timeouts and starts a separate 5s parse deadline after reading. Exact matching/dialect: README. **JSON**: See README. Authorize before local metadata and reads; inaccessible paths stay UNVERIFIED. @@ -34,7 +33,7 @@ deduplicate at each forwarding boundary before publishing the durable count. **Remote empty files**: GNU stat's `regular file`/`regular empty file` are regular files. Empty files pass exists/file_written, fail non-empty; reject symlinks, directories, FIFOs. -**Context**: Capture after validation, before setup. Keep genuine replies, even hidden clarifications; exclude framework state and unpaired calls. Mark unserializable media as omitted. Content blocks are classified by equality probes on `type` (never set membership): a provider-emitted block whose type is not a string — including unhashable JSON such as `type: []` — is skipped with its surrounding history intact, never a TypeError out of the snapshot the child is built from. A skipped block with a non-string type logs one warning so the loss is observable; the deliberate reasoning/signature/tool-use exclusions stay quiet. +**Context**: Capture after validation, before setup. Keep genuine replies/hidden clarifications; exclude framework state/unpaired calls; mark unserializable media omitted. Probe block `type` by equality, never set membership. Skip non-string types, including unhashable JSON (`type: []`), with surrounding history intact and no snapshot TypeError. Log one warning per non-string block; intentional reasoning/signature/tool-use exclusions stay quiet. Native `document` snapshots neutralize reserved tags and input markers in textual `title`, `context`, text-source `data`, content-source text, and citation prose at @@ -81,6 +80,8 @@ counters and a non-skipped test. Failure/error counters take precedence; empty/all-skipped modules contribute no proof but do not veto tested modules. 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. + **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. **Guardrail caps & `stop_reason` (#3875 Phase 2)**: three independent axes can end a subagent run early, and all now surface *why* through one additive field rather than a new status enum. **Turn axis**: `max_turns` counts turns, `recursion_limit` counts super-steps — one per graph node, and `create_agent` compiles a node per middleware hook — so `turn_budget.py::resolve_recursion_limit` scales the turn budget by the assembled chain's depth (`test_subagent_turn_budget.py` pins it against the real compiled graph); a verbatim hand-off bought `general-purpose` ~18 of its 150 turns. Exhausting it raises `GraphRecursionError` from `agent.astream`; `executor.py::_aexecute` catches it specifically (before the generic `except Exception`). **Token axis**: `TokenBudgetMiddleware` is attached per-agent via `build_subagent_runtime_middlewares` from `subagents.token_budget` (default `max_tokens` **coupled to `summarization.enabled`** — 1,000,000 when subagent summarization is on, 2,000,000 when off, warn at 0.7, hard-stop at 1.0; a user-set budget always wins regardless of the switch — #3875 Phase 3; a backstop against a subagent that burns tokens on trivial work). It does *not* raise: at the hard-stop threshold it strips the in-flight turn's tool calls, forces `finish_reason="stop"`, and lets the run complete naturally with a final answer. **Loop axis**: `LoopDetectionMiddleware` (attached at the same point) catches repeated identical tool-call sets — or one tool *type* called many times with varying args — and its hard-stop likewise strips `tool_calls` and forces a final answer without raising, recording `loop_capped`. Each guard exposes its cap on a per-`run_id` `consume_stop_reason(run_id)` accessor; `_aexecute` collects **every** middleware with that method (duck-typed via `hasattr`, so the executor has no import coupling to the guard classes) and surfaces the first non-`None` reason — adding a future guard needs no executor change. **Surfacing**: whichever axis fired, `_aexecute` stamps a normal status plus an additive reason — `completed` + `stop_reason=token_capped|turn_capped|loop_capped` when a usable final answer (or usable partial work recovered at the turn cap) was produced; `failed` + `stop_reason=turn_capped` when nothing usable survived. `SubagentResult.stop_reason` flows through `task_tool.py::_task_result_command` → `format_subagent_result_message` (renders `Task Succeeded (capped: ...)` / `Task failed (capped: ...)`) and `make_subagent_additional_kwargs`, which stamps the additive `subagent_stop_reason` key alongside the normal `subagent_status`. **Why additive, not an enum**: a new status value would break v1 consumers; an optional field is ignored by older frontends and ledger readers, so the cross-language contract (`contracts/subagent_status_contract.json` v2 + `subagents/status_contract.py` + `frontend/.../subtask-result.ts`, pinned by `test_status_values_match_contract` / `test_stop_reason_values_match_contract`) stays backward-compatible. The durable delegation ledger captures `stop_reason` onto the entry and renders model-facing guidance ("hit a guardrail cap with a partial result; reuse it, retry tighter, or raise the per-agent budget (`max_turns` / `token_budget`)") so the lead reuses a capped completion knowingly instead of mistaking it for a clean one. (Phase 1 shipped this surfacing as a `MAX_TURNS_REACHED` status enum in #3949; Phase 2 replaced that enum with the additive `stop_reason` field per the agreed design — the `max_turns_reached` status value and `SubagentStatus.MAX_TURNS_REACHED` are gone.) diff --git a/backend/packages/harness/deerflow/subagents/acceptance_checks.py b/backend/packages/harness/deerflow/subagents/acceptance_checks.py index 1638825ef2..ed10b16633 100644 --- a/backend/packages/harness/deerflow/subagents/acceptance_checks.py +++ b/backend/packages/harness/deerflow/subagents/acceptance_checks.py @@ -117,7 +117,6 @@ _FILE_CONTENT_READ_CAP_BYTES = 50_000 _TEST_PASS_SHAPE_RE = re.compile( r"\b[1-9]\d*\s+passed\b" # pytest / jest: "5 passed" (zero is not a pass) r"|^OK$" # unittest: bare OK line - r"|test result: ok" # cargo test r"|^ok\s+\S" # go test: "ok \tpkg/path" r"|\ball tests passed\b", re.IGNORECASE | re.MULTILINE, @@ -129,6 +128,24 @@ _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) +# Cargo emits separate summaries for unit, integration and documentation +# targets. Only complete successful records may qualify an empty sibling; +# a bare success banner or clipped counters cannot erase zero-test evidence. +_CARGO_SUCCESS_SUMMARY_RE = re.compile( + r"^[ \t]*test result: ok\.[ \t]+(?P0|[1-9][0-9]{0,8}) passed;[ \t]+0 failed;" + r"[ \t]+[0-9]{1,9} ignored;[ \t]+[0-9]{1,9} measured;[ \t]+[0-9]{1,9} filtered out;" + r"[ \t]+finished in [0-9]+(?:\.[0-9]+)?s[ \t]*\r?$", + re.IGNORECASE | re.MULTILINE, +) + + +def _without_empty_cargo_targets(text: str) -> str: + """Keep zero records unless a complete sibling proves tests executed.""" + if not any(match["passed"] != "0" for match in _CARGO_SUCCESS_SUMMARY_RE.finditer(text)): + return text + return _CARGO_SUCCESS_SUMMARY_RE.sub(lambda match: "" if match["passed"] == "0" else match[0], text) + + # 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( @@ -1991,8 +2008,8 @@ def _check_tests_passed_leaf(command: str, bash_executions: list[dict[str, Any]] 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) + # Empty packages/targets cannot veto complete positive sibling evidence. + summary_tail = _without_empty_cargo_targets(_GO_ZERO_TEST_LINE_RE.sub("", output_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 diff --git a/backend/tests/test_cargo_acceptance.py b/backend/tests/test_cargo_acceptance.py new file mode 100644 index 0000000000..79b10ca877 --- /dev/null +++ b/backend/tests/test_cargo_acceptance.py @@ -0,0 +1,178 @@ +"""Cargo's empty test targets must not invalidate completed sibling tests.""" + +import asyncio +import importlib.util +import os +import shutil +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest +from langchain_core.messages import AIMessage +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 check_acceptance_criteria, render_acceptance_section, render_acceptance_segment + + +def _summary(passed=0, failed=0, ignored=0): + return f"test result: {'FAILED' if failed else 'ok'}. {passed} passed; {failed} failed; {ignored} ignored; 0 measured; 0 filtered out; finished in 0.00s" + + +def _verdict(output, **overrides): + execution = {"command": "cargo test", "output_tail": output, "status": "success", "shell_persistent": False, **overrides} + return check_acceptance_criteria(["tests_passed:cargo test"], bash_executions=[execution]) + + +@pytest.mark.parametrize("reverse", [False, True]) +@pytest.mark.parametrize("ignored", [0, 2]) +@pytest.mark.parametrize("newline", ["\n", "\r\n"]) +def test_successful_tests_survive_complete_empty_target_summaries(reverse, ignored, newline): + records = [_summary(passed=2), _summary(ignored=ignored)] + if reverse: + records.reverse() + verdict = _verdict(newline.join(records)) + assert verdict["all_hold"] is True + assert verdict["unchecked"] == [] + assert "[holds]" in render_acceptance_section(verdict) + assert "execution evidence only" in render_acceptance_section(verdict) + assert "acceptance:" in render_acceptance_segment(verdict) + + +@pytest.mark.parametrize("output", [_summary(), _summary(ignored=3), _summary() + "\n" + _summary(ignored=1)]) +def test_only_empty_or_ignored_targets_remain_unverified(output): + verdict = _verdict(output) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + assert "[UNVERIFIED]" in render_acceptance_section(verdict) + + +@pytest.mark.parametrize("output", ["test result: ok", "test result: ok. 0", _summary() + "\ntest result: ok"]) +def test_bare_cargo_success_banner_cannot_supply_test_count_evidence(output): + verdict = _verdict(output) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +@pytest.mark.parametrize("banner", ["OK", "all tests passed", "test result: ok. 2 passed; 0 failed"]) +def test_zero_target_records_need_a_complete_positive_cargo_sibling(banner): + verdict = _verdict(_summary() + "\n" + banner) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +def test_positive_counts_do_not_bypass_output_attribution(): + verdict = _verdict(_summary(passed=2) + "\n" + _summary(), command="echo 'unrelated'; cargo test") + assert verdict["leaves"][0]["checked"] is False + + +@pytest.mark.parametrize( + "zero", + [ + _summary().split("; 0 filtered out")[0], + "warning: " + _summary(), + _summary().replace("finished in 0.00s", "finished in"), + _summary().replace("0 ignored", "9" * 100 + " ignored"), + "0 passed in 0.1s", + "Ran 0 tests\nOK", + ], +) +def test_incomplete_or_other_runner_zero_evidence_is_not_silenced(zero): + verdict = _verdict(_summary(passed=2) + "\n" + zero) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (False, False) + + +@pytest.mark.parametrize("failure", [_summary(failed=1), _summary(failed=1).replace("FAILED", "ok"), "error: doctest failed"]) +@pytest.mark.parametrize("reverse", [False, True]) +def test_failure_in_a_sibling_target_still_wins(failure, reverse): + # Cargo normally exits nonzero; the terminal FAILED/count summary must + # also prevent a positive verdict if a surrounding runner swallows it. + records = [_summary(passed=2), _summary(), failure] + output = "\n".join(reversed(records) if reverse else records) + if failure.startswith("error:"): + verdict = _verdict(output, status="error") + else: + verdict = _verdict(output) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == (True, False) + assert "[does not hold]" in render_acceptance_section(verdict) + + +@pytest.mark.parametrize("overrides", [{"shell_persistent": True}, {"shell_persistent": None}, {"command_truncated": True}]) +def test_empty_target_handling_does_not_bypass_execution_provenance(overrides): + verdict = _verdict(_summary(passed=2) + "\n" + _summary(), **overrides) + assert verdict["leaves"][0]["checked"] is False + + +@pytest.fixture +def executor_module(monkeypatch): + # conftest preloads an executor stub; harvest from the production module. + path = Path(__file__).parents[1] / "packages/harness/deerflow/subagents/executor.py" + spec = importlib.util.spec_from_file_location("_cargo_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.skipif(os.name == "nt" or shutil.which("cargo") is None, reason="POSIX Cargo integration requires an installed Rust toolchain") +@pytest.mark.asyncio +@pytest.mark.parametrize( + "source,selection,expected", + [ + ("#[test] fn unit() { assert_eq!(2 + 2, 4); }", "", (True, True)), + ("/// ```\n/// assert_eq!(2 + 2, 4);\n/// ```\npub fn documented() {}", "", (True, True)), + ("#[test] #[ignore] fn ignored() {}", "", (False, False)), + ('#[test] fn broken() { panic!("broken"); }', "", (True, False)), + ("/// ```\n/// assert_eq!(1, 2);\n/// ```\npub fn documented() {}\n#[test] fn unit() {}", "", (True, False)), + ("#[test] fn unit() {}", " no_test_matches", (False, False)), + ], + ids=["unit-with-empty-doc", "doc-with-empty-unit", "ignored", "failed-unit", "failed-doc", "filtered"], +) +async def test_native_cargo_output_reaches_parent_acceptance_through_tool_node(tmp_path, monkeypatch, executor_module, source, selection, expected): + (tmp_path / "src").mkdir() + (tmp_path / "Cargo.toml").write_text('[package]\nname = "acceptance_fixture"\nversion = "0.1.0"\nedition = "2021"\n', encoding="utf-8") + (tmp_path / "src/lib.rs").write_text(source, encoding="utf-8") + sandbox = LocalSandbox("cargo-evidence", path_mappings=[PathMapping("/mnt/user-data/workspace", str(tmp_path))], environment={"CARGO_TERM_COLOR": "never"}) + 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=60) + outputs.append(output) + return output + + tool = StructuredTool.from_function(coroutine=bash, name="bash", description="Execute the offline Rust fixture.") + 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) + command = "cargo test --offline --manifest-path /mnt/user-data/workspace/Cargo.toml" + selection + state = await graph.compile().ainvoke( + { + "sandbox": {"sandbox_id": sandbox.id}, + "messages": [AIMessage(content="", tool_calls=[{"name": "bash", "id": "cargo-call", "type": "tool_call", "args": {"command": command}}])], + } + ) + assert len(outputs) == 1 + assert "test result:" in outputs[0], outputs[0] + message = state["messages"][-1] + assert (message.name, message.tool_call_id) == ("bash", "cargo-call") + executions = executor_module._harvest_bash_executions(state) + assert len(executions) == 1 + assert executions[0]["shell_persistent"] is False + assert executions[0]["tool_call_id"] == "cargo-call" + verdict = check_acceptance_criteria([f"tests_passed:{command}"], bash_executions=executions) + assert (verdict["leaves"][0]["checked"], verdict["all_hold"]) == expected, verdict + if expected == (True, True): + assert "1 passed" in outputs[0] and "0 passed" in outputs[0] + assert "[holds]" in render_acceptance_section(verdict) + elif expected == (True, False): + assert "Exit Code: 101" in outputs[0] + assert executions[0]["status"] == "error" + else: + assert "[UNVERIFIED]" in render_acceptance_section(verdict)