From 8f0ca586538c9499290cc1a174746d6ac295eab0 Mon Sep 17 00:00:00 2001 From: Guo-Yixin Date: Wed, 14 Oct 2026 04:15:59 -0500 Subject: [PATCH] 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 --- README.md | 7 ++ .../harness/deerflow/subagents/AGENTS.md | 6 + .../deerflow/subagents/acceptance_checks.py | 54 ++++++++- .../tests/test_acceptance_build_summaries.py | 113 ++++++++++++++++++ backend/tests/test_acceptance_checks.py | 2 +- backend/tests/test_subagent_executor.py | 35 ++++++ 6 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 backend/tests/test_acceptance_build_summaries.py diff --git a/README.md b/README.md index a4e95fb4d6..56a82bf8dd 100644 --- a/README.md +++ b/README.md @@ -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: exists` and `file_written:`, including on remote sandboxes. It fails `file: non-empty` with a deterministic empty-file result. To require CSV header columns, set a `task` or `batch_task` item's criterion to diff --git a/backend/packages/harness/deerflow/subagents/AGENTS.md b/backend/packages/harness/deerflow/subagents/AGENTS.md index 3befa99dea..267f118095 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -75,6 +75,12 @@ does not change retry policy. **Acceptance checklist (RFC #4651 PR4)**: `acceptance_checks.py` checks lead-supplied `acceptance_criteria` in code on the `task` tool's completed branch (offloaded via `asyncio.to_thread`, failure-isolated). Decidable leaves: `file: exists|non-empty` and `file_written:` read through `read_current_file_content` scoped to the shared thread workspace (`workspace_path`/`outputs_path`; virtual `/mnt/user-data/...` prefixes and workspace-relative spellings normalized first) — the read itself uses the sandbox-native virtual form because the local read validator and provider mount tables resolve virtual paths, not host paths; a remote provider's `"Error: ..."` return string (remote providers return instead of raise for missing files) is normalized to a failed check, never evaluated as content — provider-typed via `is_local_sandbox`, so a genuine `Error:`-prefixed file on the local sandbox stays valid content; a UnicodeDecodeError on a text read marks a binary deliverable (PDF/image) as existing and non-empty instead of dropping the verdict; reads are byte-bounded — the size is established first (`os.stat` on the validated host path locally, so the host-bash-disabled configuration needs no shell; a metadata-only `stat`/`realpath` probe in a fresh `env -i` shell on remote providers — absolute-path utilities and a marker env that routes AIO off its persistent shell, so a completed subagent's poisoned session state cannot steer it; `stat` never opens content, so a FIFO cannot block the parent for the provider's idle timeout; the file must stat as a regular non-symlink file, and containment is canonicalized — the file's realpath must stay under the mount root's realpath, which is exactly what the provider's own read path resolves (e2b and Tenki realize `/mnt/user-data` as a symlink to the home dir by default), so a final-component symlink is rejected outright and an intermediate dir-link escape under a sane root still lands outside the canonical root), leaves above `_FILE_CONTENT_READ_CAP_BYTES` answer from the size alone — `file_written` only with an added bounded one-byte open probe (a mode-000 file stats fine while any open raises EACCES), and smaller files run the full read, and an unestablishable size degrades to UNVERIFIED rather than an unbounded fallback read; out-of-scope paths degrade to UNVERIFIED, never misjudge — on the local sandbox the scope decision canonicalizes with realpath, so a workspace symlink into uploads cannot satisfy a scoped leaf with upload content. `tests_passed:` anchors to a matching bash execution (newest match wins) with `status=success` and a test-summary shape in its bounded output tail — each harvested execution carries a `shell_persistent` provenance stamp — the producing sandbox's `persistent_shell_sessions` flag (AIO's legacy exec path) resolved from the state that carried the evidence, never the parent runtime, which has no `sandbox` key when the parent delegated before touching one — and a persistent stamp (or an unidentifiable or undeclared one — a custom provider that never declared its session semantics is unknown, not fresh-shell; unknown provenance fails closed) degrades the leaf to UNVERIFIED instead, because any earlier call in the shared session could have mutated the state the clean-looking run executed in and a fresh controlled session (RFC §6 verifier) would be needed to prove otherwise, harvested by the executor (`_harvest_bash_executions`, only when criteria were delegated) from the same stamped `ToolMessage`s the receipt layer reads, accumulated per streamed chunk (merged by `tool_call_id`, newest-capped) so subagent summarization compacting earlier messages cannot erase a recorded execution, with over-cap commands carrying `command_truncated` so the matcher degrades to UNVERIFIED instead of proving a match on a suffix-less prefix; the recorded status is the actual shell exit status parsed from the output's `Exit Code: N` / `Command exited with code N` marker (a nonzero bash exit returns ordinary text that `deerflow_tool_meta` still reports as success; every provider appends a marker on nonzero exit — local/e2b/opensandbox/tenki/boxlite textually, aio via the SDK's structured exit_code — and timeouts append `Exit Code: 124` (local process-group kill, AIO `hard_timeout`, e2b/tenki SDK deadlines, boxlite bridge deadline) while signal kills parse as signed markers (`Exit Code: -9` records error); `_truncate_bash_output` preserves a trailing exit marker inside its budget, with a 32-char floor for smaller configured limits and the remote `Command exited with code N` form accepted only as the whole trimmed output; the matched marker text travels on the entry as `status_marker`), falling back to the meta status only when no marker exists; matching is shell-structure aware (operator-separated segments — a physical newline separates with `;` semantics — or with the continuation operator the next line opens with (`cmd1\n&& cmd2` is `&&`; `cmd1\n|| cmd2` is `||`, which after a successful first command skips the rest while exiting 0, so flattening it to `;` would record a run that never happened) —, so a multi-line script's trailing `echo`/`seq` lines are never merged into the matched segment's arguments, status, or output attribution —, executable identity — directional: a bare criterion executable accepts any path spelling of the name, while an explicitly path-spelled criterion requires a path-spelled execution of the same normalized executable path (spelling judged on the raw token: `./pytest` names the project-local file and normpath collapsing `./` must not demote it to a PATH lookup; a `..` component on either side is unprovable outright), so `/tmp/fake/pytest` cannot certify `/opt/project/.venv/bin/pytest` —, ordered argument subsequence whose env-assignment prefix must equal the criterion's exactly as an effective name → final-value mapping — extra, missing, or differently-valued assignments degrade the match, as does any reordering of a repeated name (`CI=0 CI=1` vs `CI=1 CI=0` are last-wins opposite environments), since no variable is provably inert across repositories, and any assignment or argumented `export`/`unset` in a preceding segment is state pollution, and any span token carrying a runtime expansion (`$VAR`/`$( )`/backticks) or an extra token carrying glob metacharacters (crafted option-looking filenames narrow invisibly) is likewise unprovable), so a command that merely mentions the criterion string (`echo '12 passed'; # pytest x.py`) cannot anchor the leaf, and control flow is preserved — the matching span must end at the command's last segment with provable execution (`&&` needs recorded success, `||` needs recorded failure, pipelines inside the span and backgrounding are never provable), and the criterion's own connectors are preserved — an expected `&&` executed as `;` (`cd missing; pytest x` for `cd missing && pytest x`) lets a failed preceding step be bypassed, so it degrades; only the stricter direction (criterion `;` executed as `&&` with recorded success) survives), so a short-circuited segment (`false && pytest x; echo '3 passed'`) degrades to UNVERIFIED instead of a false hold; the summary shape is evaluated only when the output is attributable to the matched segment (every preceding segment provably silent by invocation form — only shape-free `cd dir` and pure assignments qualify; `pushd`/`umask`/`ulimit`, any `export`/`unset` (including an invalid identifier, whose bash error text is subagent-chosen), and any `source`/`.`, whose `*/bin/activate` path shape says nothing about what a crafted script prints, are not; the `cd` print channel is closed too — CDPATH makes `cd` print the subagent-chosen resolved path and the pass shapes match as substrings, so a `cd` argument carrying a summary shape or runtime expansion is non-silent, and any `CDPATH=` assignment degrades the match as state pollution), Go `[no test files]` / `[no tests to run]` lines are package-local: other packages' passes survive; all-zero stays UNVERIFIED; `0 passed` / `Ran 0 tests` veto, argument matching treats tokens consumed by negating options (`--ignore`/`--deselect`/`--exclude`/`--skip` families) as ineligible evidence (`pytest --ignore tests/security tests` cannot affirm the excluded tests) and a target negated anywhere in the command degrades the match (`pytest tests/security tests/unit --ignore tests/security` cannot certify the excluded target), and a bare criterion with no positional selection target (bare `pytest`, `make test`) treats ANY negating option as a different selection, and accepts extra executed flags only when provably selection-preserving (verbosity/output/xdist/coverage forms; `-k`/`-m`/`--lf`/`--collect-only`/`-c`/`-p` and friends degrade to UNVERIFIED — `pytest -k smoke tests/security` cannot certify the full selection; extra positionals widen only after a path-scoped criterion — scoping reads positional targets by option arity, so a path embedded in an option (`--basetemp=/tmp/p`) never counts as a target, and an option of unknown arity (absent from the value-taking table, no glued `=`) followed by a path-like token (`--rootdir /tmp/project`) fails closed, so `python -m unittest pkg.OneTest` cannot certify bare `python -m unittest`), and criterion text is neutralized with `neutralize_untrusted_tags` before storage/rendering so a blocked tag in a criterion can never reach the lead-visible checklist section. Other criteria stay UNVERIFIED. The verdict (`subagent_acceptance_verdict` in `additional_kwargs`, validated on read) feeds the delegation ledger's `acceptance:` segment and the model-visible checklist section appended to the result text; the gateway strips caller-forged verdicts from both ledger entries and message metadata like the citation verdict. Leaf read errors catch only `OSError`/`SandboxError` — unexpected errors propagate to the task-tool-level isolation so the blocking-IO gate keeps teeth (`tests/blocking_io/test_task_tool_acceptance_checklist.py`). Known accepted boundaries (pinned by `TestKnownBoundaries` — conscious, do not re-raise): a bare criterion executable trusts PATH/filesystem spelling (a subagent-crafted same-name script is textually indistinguishable; spelling an absolute criterion path restricts evidence to that normalized path); relative criterion targets resolve in the wrapper's cwd, so `cd` prefixes must stay in scope (relative without `..`, or absolute under the thread data roots / virtual prefix) while a symlink inside an allowed root pointing out is a filesystem-layer concern; runner semantics are trusted (a Makefile swallowing failures, a runner exiting 0 on failure) — Layer 2 is execution evidence only, claim correctness belongs to the PR5 judge / RFC §6 re-execution; evidence is bounded (500-char command, 1000-char tail) and truncation degrades to UNVERIFIED rather than proving. **Acceptance checklist path portability**: Paths are host-independent and raw `..` fails closed. Drive/UNC absolutes retain their class across `ntpath` normalization and cannot cross the root; drive-relative, shell-dependent provider/PSDrive, and POSIX-rooted `cd` on Windows are unprovable. Selection overlap separates pytest node IDs, normalizes safe `.`/duplicate separators, rejects `..`, and applies Windows casing to drive, UNC, PSDrive, or Windows-context paths; provider-qualified drive/UNC roots remain paths until a later node-ID split. POSIX paths and all node IDs keep their required case sensitivity. Ambiguous trailing-dot/space or 8.3 components, different volume IDs, cross-family pairs, and absolute/relative PSDrive pairs fail closed without execution/filesystem provenance; only same-volume, same-root-form paths compare lexically. Without shell provenance, raw commands reject backslashes; cmd `%VAR%`/`!VAR!`, `^`, `#`; Bash braces/tilde; and PowerShell splatting, typographic quotes, or unquoted parentheses. A lone `%` remains eligible. POSIX-only markers must short-circuit on Windows because pytest evaluates them at import. +**JVM test evidence**: Build-success banners do not establish `tests_passed`. +Require complete Maven or Gradle count summaries with consistent nonnegative +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. + **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 8abc4107ff..1638825ef2 100644 --- a/backend/packages/harness/deerflow/subagents/acceptance_checks.py +++ b/backend/packages/harness/deerflow/subagents/acceptance_checks.py @@ -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[0-9]+),[ \t]*" + r"Failures:[ \t]*(?P[0-9]+),[ \t]*Errors:[ \t]*(?P[0-9]+),[ \t]*" + r"Skipped:[ \t]*(?P[0-9]+)(?:,[ \t]*Time elapsed:[^\r\n]*)?[ \t]*\r?$", + re.MULTILINE, +) +_GRADLE_TEST_SUMMARY_RE = re.compile( + r"^[ \t]*(?P[0-9]+) tests? completed(?:,[ \t]*(?P[0-9]+) failed)?" + r"(?:,[ \t]*(?P[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" diff --git a/backend/tests/test_acceptance_build_summaries.py b/backend/tests/test_acceptance_build_summaries.py new file mode 100644 index 0000000000..b986621716 --- /dev/null +++ b/backend/tests/test_acceptance_build_summaries.py @@ -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) diff --git a/backend/tests/test_acceptance_checks.py b/backend/tests/test_acceptance_checks.py index 3c359a9dfb..d45d50dc39 100644 --- a/backend/tests/test_acceptance_checks.py +++ b/backend/tests/test_acceptance_checks.py @@ -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!", ], ) diff --git a/backend/tests/test_subagent_executor.py b/backend/tests/test_subagent_executor.py index 70e13120ea..2100ed8644 100644 --- a/backend/tests/test_subagent_executor.py +++ b/backend/tests/test_subagent_executor.py @@ -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"))