Przeglądaj źródła

fix: map benchmark output failures

Problem: benchmark summary and final error stream failures escaped run_cli after reports were written, conflating system output failures with sample failures and bypassing stable exit codes.\n\nRisk: explicit stream flushing can surface previously deferred pipe errors; these are now mapped to exit code 2, while KeyboardInterrupt remains 130 and error text remains redacted.
zhenyu.hu 2 tygodni temu
rodzic
commit
4583dbe088

+ 45 - 14
src/agent_lab/presentation/benchmark_cli.py

@@ -112,8 +112,7 @@ def run_cli(
     except KeyboardInterrupt:
     except KeyboardInterrupt:
         return 130
         return 130
     except Exception as exc:
     except Exception as exc:
-        _print_error(exc, deps.stderr, secrets=())
-        return 2
+        return _system_error_exit_code(exc, deps.stderr, secrets=())
 
 
     results: list[BenchmarkRunResult] = []
     results: list[BenchmarkRunResult] = []
     runner_error: Exception | None = None
     runner_error: Exception | None = None
@@ -163,19 +162,34 @@ def run_cli(
     except KeyboardInterrupt:
     except KeyboardInterrupt:
         return 130
         return 130
     except Exception as exc:
     except Exception as exc:
-        _print_error(exc, deps.stderr, secrets=(api_key,))
-        return 2
-
-    _print_summary(
-        report,
-        json_path,
-        markdown_path,
-        deps.stdout,
-        secrets=(api_key,),
-    )
+        return _system_error_exit_code(
+            exc,
+            deps.stderr,
+            secrets=(api_key,),
+        )
+
+    try:
+        _print_summary(
+            report,
+            json_path,
+            markdown_path,
+            deps.stdout,
+            secrets=(api_key,),
+        )
+    except KeyboardInterrupt:
+        return 130
+    except Exception as exc:
+        return _system_error_exit_code(
+            exc,
+            deps.stderr,
+            secrets=(api_key,),
+        )
     if execution_error is not None:
     if execution_error is not None:
-        _print_error(execution_error, deps.stderr, secrets=(api_key,))
-        return 2
+        return _system_error_exit_code(
+            execution_error,
+            deps.stderr,
+            secrets=(api_key,),
+        )
     return 1 if report.overall.failed else 0
     return 1 if report.overall.failed else 0
 
 
 
 
@@ -218,6 +232,7 @@ def _print_summary(
     )
     )
     for line in lines:
     for line in lines:
         print(redact_text(line, secrets=secrets), file=stream)
         print(redact_text(line, secrets=secrets), file=stream)
+    stream.flush()
 
 
 
 
 def _print_error(
 def _print_error(
@@ -228,6 +243,22 @@ def _print_error(
 ) -> None:
 ) -> None:
     message = sanitize_error(str(exc), secrets=secrets) or exc.__class__.__name__
     message = sanitize_error(str(exc), secrets=secrets) or exc.__class__.__name__
     print(f"benchmark error: {message}", file=stream)
     print(f"benchmark error: {message}", file=stream)
+    stream.flush()
+
+
+def _system_error_exit_code(
+    exc: Exception,
+    stream: TextIO,
+    *,
+    secrets: Sequence[str],
+) -> int:
+    try:
+        _print_error(exc, stream, secrets=secrets)
+    except KeyboardInterrupt:
+        return 130
+    except Exception:
+        pass
+    return 2
 
 
 
 
 def main() -> NoReturn:
 def main() -> NoReturn:

+ 117 - 0
tests/test_benchmark_cli.py

@@ -79,6 +79,28 @@ class FakeRunner:
         return self.results
         return self.results
 
 
 
 
+class FailingTextStream(StringIO):
+    def __init__(
+        self,
+        *,
+        write_error: BaseException | None = None,
+        flush_error: BaseException | None = None,
+    ) -> None:
+        super().__init__()
+        self.write_error = write_error
+        self.flush_error = flush_error
+
+    def write(self, value: str) -> int:
+        if self.write_error is not None:
+            raise self.write_error
+        return super().write(value)
+
+    def flush(self) -> None:
+        if self.flush_error is not None:
+            raise self.flush_error
+        super().flush()
+
+
 def _dependencies(
 def _dependencies(
     cli,
     cli,
     *,
     *,
@@ -431,6 +453,101 @@ def test_unexpected_runner_failure_still_attempts_report_write(tmp_path: Path):
     assert "Bearer [REDACTED]" in stderr.getvalue()
     assert "Bearer [REDACTED]" in stderr.getvalue()
 
 
 
 
+@pytest.mark.parametrize("failure_point", ["write", "flush"])
+def test_summary_output_failure_after_report_write_exits_two_and_redacts_stderr(
+    tmp_path: Path,
+    failure_point: str,
+):
+    cli = _cli_module()
+    secret = "sk-summary-output-secret"
+    config_path = _write_config(tmp_path / "config.json")
+    output_dir = tmp_path / "reports"
+    error = BrokenPipeError(f"Bearer {secret}")
+    stdout = FailingTextStream(
+        write_error=error if failure_point == "write" else None,
+        flush_error=error if failure_point == "flush" else None,
+    )
+    stderr = StringIO()
+    dependencies = _dependencies(
+        cli,
+        results=[_run_result()],
+        api_key=secret,
+        stdout=stdout,
+        stderr=stderr,
+    )
+
+    exit_code = cli.run_cli(
+        [
+            "--config",
+            str(config_path),
+            "--output-dir",
+            str(output_dir),
+        ],
+        dependencies=dependencies,
+    )
+
+    assert exit_code == 2
+    assert any(output_dir.rglob("*.json"))
+    assert any(output_dir.rglob("*.md"))
+    assert secret not in stderr.getvalue()
+    assert "Bearer [REDACTED]" in stderr.getvalue()
+
+
+def test_execution_error_stderr_failure_still_exits_two(tmp_path: Path):
+    cli = _cli_module()
+    secret = "sk-execution-error-secret"
+    config_path = _write_config(tmp_path / "config.json")
+    output_dir = tmp_path / "reports"
+    stderr = FailingTextStream(
+        write_error=BrokenPipeError(f"Bearer {secret}")
+    )
+    dependencies = _dependencies(
+        cli,
+        results=[],
+        api_key=secret,
+        stderr=stderr,
+    )
+
+    exit_code = cli.run_cli(
+        [
+            "--config",
+            str(config_path),
+            "--output-dir",
+            str(output_dir),
+        ],
+        dependencies=dependencies,
+    )
+
+    assert exit_code == 2
+    assert any(output_dir.rglob("*.json"))
+    assert any(output_dir.rglob("*.md"))
+    assert secret not in stderr.getvalue()
+
+
+def test_summary_output_keyboard_interrupt_still_exits_130(tmp_path: Path):
+    cli = _cli_module()
+    config_path = _write_config(tmp_path / "config.json")
+    output_dir = tmp_path / "reports"
+    dependencies = _dependencies(
+        cli,
+        results=[_run_result()],
+        stdout=FailingTextStream(write_error=KeyboardInterrupt()),
+    )
+
+    exit_code = cli.run_cli(
+        [
+            "--config",
+            str(config_path),
+            "--mock",
+            "--output-dir",
+            str(output_dir),
+        ],
+        dependencies=dependencies,
+    )
+
+    assert exit_code == 130
+
+
 @pytest.mark.parametrize(
 @pytest.mark.parametrize(
     ("results", "attempted", "passed", "failed", "actual_count"),
     ("results", "attempted", "passed", "failed", "actual_count"),
     [
     [