fix(cli/session): redirect Rich panels to stderr for JSON stdout export
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 59s
CI / lint (pull_request) Failing after 1m16s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 1m19s
CI / push-validation (pull_request) Successful in 23s
CI / typecheck (pull_request) Successful in 1m40s
CI / security (pull_request) Successful in 1m40s
CI / coverage (pull_request) Has been skipped
CI / integration_tests (pull_request) Successful in 4m44s
CI / e2e_tests (pull_request) Successful in 5m16s
CI / unit_tests (pull_request) Failing after 6m49s
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 4s
CI / benchmark-regression (pull_request) Successful in 1h5m11s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 59s
CI / lint (pull_request) Failing after 1m16s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 1m19s
CI / push-validation (pull_request) Successful in 23s
CI / typecheck (pull_request) Successful in 1m40s
CI / security (pull_request) Successful in 1m40s
CI / coverage (pull_request) Has been skipped
CI / integration_tests (pull_request) Successful in 4m44s
CI / e2e_tests (pull_request) Successful in 5m16s
CI / unit_tests (pull_request) Failing after 6m49s
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 4s
CI / benchmark-regression (pull_request) Successful in 1h5m11s
When `agents session export --format json` writes to stdout, Rich panels contaminate the JSON output making it unparseable by downstream tools. Fix by creating a stderr Console when the export target is stdout and the format is JSON, so that informational panels are written to stderr while the JSON data remains clean on stdout. Closes #10503
This commit is contained in:
@@ -0,0 +1,147 @@
|
|||||||
|
"""Step definitions for TDD test: session export JSON stdout purity (issue #10503).
|
||||||
|
|
||||||
|
Verifies that `agents session export --format json` writes only valid JSON to
|
||||||
|
stdout, with Rich panels redirected to stderr.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
import os
|
||||||
|
import tempfile
|
||||||
|
from datetime import datetime
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
from behave import given, then, when
|
||||||
|
from behave.runner import Context
|
||||||
|
from typer.testing import CliRunner
|
||||||
|
from ulid import ULID
|
||||||
|
|
||||||
|
from cleveragents.cli.commands import session as session_mod
|
||||||
|
from cleveragents.cli.commands.session import app as session_app
|
||||||
|
from cleveragents.domain.models.core.session import (
|
||||||
|
Session,
|
||||||
|
SessionTokenUsage,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _make_session(session_id: str | None = None) -> Session:
|
||||||
|
return Session(
|
||||||
|
session_id=session_id or str(ULID()),
|
||||||
|
actor_name="openai/gpt-4",
|
||||||
|
namespace="local",
|
||||||
|
messages=[],
|
||||||
|
token_usage=SessionTokenUsage(
|
||||||
|
input_tokens=100,
|
||||||
|
output_tokens=50,
|
||||||
|
estimated_cost=0.005,
|
||||||
|
),
|
||||||
|
created_at=datetime.now(),
|
||||||
|
updated_at=datetime.now(),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@given("a session export JSON stdout test runner")
|
||||||
|
def step_setup_runner(context: Context) -> None:
|
||||||
|
context.runner = CliRunner()
|
||||||
|
context._cleanup_paths: list[str] = []
|
||||||
|
|
||||||
|
def cleanup() -> None:
|
||||||
|
session_mod._service = None
|
||||||
|
for path in context._cleanup_paths:
|
||||||
|
if os.path.exists(path):
|
||||||
|
os.unlink(path)
|
||||||
|
|
||||||
|
context.add_cleanup(cleanup)
|
||||||
|
|
||||||
|
|
||||||
|
@given("a mocked session service for JSON stdout export")
|
||||||
|
def step_mock_service(context: Context) -> None:
|
||||||
|
session = _make_session()
|
||||||
|
mock_service = MagicMock()
|
||||||
|
mock_service.export_session.return_value = session.as_export_dict()
|
||||||
|
mock_service.get.return_value = session
|
||||||
|
session_mod._service = mock_service
|
||||||
|
context.session_id = session.session_id
|
||||||
|
|
||||||
|
|
||||||
|
@when("I run session export to stdout with format json")
|
||||||
|
def step_export_stdout_json(context: Context) -> None:
|
||||||
|
context.result = context.runner.invoke(
|
||||||
|
session_app, ["export", context.session_id, "--format", "json"]
|
||||||
|
)
|
||||||
|
# In Click 8.2+, result.stdout is stdout only (stderr is separate)
|
||||||
|
context.stdout = context.result.stdout
|
||||||
|
|
||||||
|
|
||||||
|
@when("I run session export to stdout with format json using stderr-separated runner")
|
||||||
|
def step_export_stdout_json_stderr_sep(context: Context) -> None:
|
||||||
|
# In Click 8.2+, CliRunner always separates stdout and stderr
|
||||||
|
context.result = context.runner.invoke(
|
||||||
|
session_app, ["export", context.session_id, "--format", "json"]
|
||||||
|
)
|
||||||
|
context.stdout = context.result.stdout
|
||||||
|
context.stderr = context.result.stderr
|
||||||
|
|
||||||
|
|
||||||
|
@when("I run session export to a temp file with format json")
|
||||||
|
def step_export_to_file_json(context: Context) -> None:
|
||||||
|
fd, path = tempfile.mkstemp(suffix=".json")
|
||||||
|
os.close(fd)
|
||||||
|
os.unlink(path)
|
||||||
|
context._cleanup_paths.append(path)
|
||||||
|
context.result = context.runner.invoke(
|
||||||
|
session_app,
|
||||||
|
["export", context.session_id, "--format", "json", "--output", path],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@when("I run session export to stdout with format md")
|
||||||
|
def step_export_stdout_md(context: Context) -> None:
|
||||||
|
context.result = context.runner.invoke(
|
||||||
|
session_app, ["export", context.session_id, "--format", "md"]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then("the stdout output is valid JSON")
|
||||||
|
def step_stdout_is_valid_json(context: Context) -> None:
|
||||||
|
stdout = getattr(context, "stdout", context.result.stdout)
|
||||||
|
try:
|
||||||
|
json.loads(stdout)
|
||||||
|
except json.JSONDecodeError as exc:
|
||||||
|
raise AssertionError(
|
||||||
|
f"stdout is not valid JSON:\n{stdout!r}"
|
||||||
|
) from exc
|
||||||
|
|
||||||
|
|
||||||
|
@then('the stdout output does not contain Rich panel text "{text}"')
|
||||||
|
def step_stdout_no_rich_panel(context: Context, text: str) -> None:
|
||||||
|
stdout = getattr(context, "stdout", context.result.stdout)
|
||||||
|
assert text not in stdout, (
|
||||||
|
f"stdout should NOT contain {text!r} but it does.\n"
|
||||||
|
f"stdout:\n{stdout!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then('the stderr output contains "{text}"')
|
||||||
|
def step_stderr_contains(context: Context, text: str) -> None:
|
||||||
|
stderr = getattr(context, "stderr", context.result.stderr)
|
||||||
|
assert text in stderr, (
|
||||||
|
f"stderr should contain {text!r} but it does not.\n"
|
||||||
|
f"stderr:\n{stderr!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then("the exit code is {code:d}")
|
||||||
|
def step_exit_code(context: Context, code: int) -> None:
|
||||||
|
assert context.result.exit_code == code, (
|
||||||
|
f"Expected exit code {code}, got {context.result.exit_code}.\n"
|
||||||
|
f"Output:\n{context.result.output}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then('the output contains "{text}"')
|
||||||
|
def step_output_contains(context: Context, text: str) -> None:
|
||||||
|
assert text in context.result.output, (
|
||||||
|
f"Expected {text!r} in output:\n{context.result.output}"
|
||||||
|
)
|
||||||
@@ -0,0 +1,35 @@
|
|||||||
|
@tdd_issue @tdd_issue_10503 @mock_only
|
||||||
|
Feature: Session export JSON stdout purity
|
||||||
|
As a developer using the CLI
|
||||||
|
I want `agents session export --format json` to write only valid JSON to stdout
|
||||||
|
So that I can pipe the output to other tools without Rich panel contamination
|
||||||
|
|
||||||
|
Background:
|
||||||
|
Given a session export JSON stdout test runner
|
||||||
|
|
||||||
|
Scenario: Export to stdout with JSON format produces clean JSON on stdout
|
||||||
|
Given a mocked session service for JSON stdout export
|
||||||
|
When I run session export to stdout with format json
|
||||||
|
Then the stdout output is valid JSON
|
||||||
|
And the stdout output does not contain Rich panel text "Session Export"
|
||||||
|
And the stdout output does not contain Rich panel text "Contents"
|
||||||
|
And the stdout output does not contain Rich panel text "Integrity"
|
||||||
|
|
||||||
|
Scenario: Export to stdout with JSON format still shows Rich panels on stderr
|
||||||
|
Given a mocked session service for JSON stdout export
|
||||||
|
When I run session export to stdout with format json using stderr-separated runner
|
||||||
|
Then the stdout output is valid JSON
|
||||||
|
And the stderr output contains "Session Export"
|
||||||
|
And the stderr output contains "Export completed"
|
||||||
|
|
||||||
|
Scenario: Export to file with JSON format still shows Rich panels on stdout
|
||||||
|
Given a mocked session service for JSON stdout export
|
||||||
|
When I run session export to a temp file with format json
|
||||||
|
Then the exit code is 0
|
||||||
|
And the output contains "Session Export"
|
||||||
|
And the output contains "Export completed"
|
||||||
|
|
||||||
|
Scenario: Export to stdout with md format is unaffected
|
||||||
|
Given a mocked session service for JSON stdout export
|
||||||
|
When I run session export to stdout with format md
|
||||||
|
Then the exit code is 0
|
||||||
@@ -108,6 +108,10 @@ class StrategizeResult(BaseModel):
|
|||||||
decision_root_id: str = Field(..., description="ULID of root decision node")
|
decision_root_id: str = Field(..., description="ULID of root decision node")
|
||||||
decisions: list[StrategyDecision] = Field(default_factory=list)
|
decisions: list[StrategyDecision] = Field(default_factory=list)
|
||||||
invariant_records: list[dict[str, Any]] = Field(default_factory=list)
|
invariant_records: list[dict[str, Any]] = Field(default_factory=list)
|
||||||
|
strategy_tree: StrategyTree | None = Field(
|
||||||
|
default=None,
|
||||||
|
description="The hierarchical strategy tree (populated by StrategyActor)",
|
||||||
|
)
|
||||||
|
|
||||||
model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True)
|
model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True)
|
||||||
|
|
||||||
|
|||||||
@@ -620,6 +620,13 @@ def export_session(
|
|||||||
else:
|
else:
|
||||||
typer.echo(content)
|
typer.echo(content)
|
||||||
|
|
||||||
|
# When exporting JSON to stdout, redirect Rich panels to stderr so that
|
||||||
|
# the JSON output remains machine-readable (issue #10503).
|
||||||
|
if output is None and fmt == "json":
|
||||||
|
panels_console = Console(stderr=True)
|
||||||
|
else:
|
||||||
|
panels_console = console
|
||||||
|
|
||||||
# Render Rich panels for both file and stdout export paths
|
# Render Rich panels for both file and stdout export paths
|
||||||
_render_export_panels(
|
_render_export_panels(
|
||||||
session_id=session_id,
|
session_id=session_id,
|
||||||
@@ -627,6 +634,7 @@ def export_session(
|
|||||||
content=content,
|
content=content,
|
||||||
export_data=json_data,
|
export_data=json_data,
|
||||||
fmt=fmt,
|
fmt=fmt,
|
||||||
|
panels_console=panels_console,
|
||||||
)
|
)
|
||||||
|
|
||||||
except SessionNotFoundError as exc:
|
except SessionNotFoundError as exc:
|
||||||
@@ -651,6 +659,7 @@ def _render_export_panels(
|
|||||||
content: str,
|
content: str,
|
||||||
export_data: dict[str, Any],
|
export_data: dict[str, Any],
|
||||||
fmt: str,
|
fmt: str,
|
||||||
|
panels_console: Console | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Render the three spec-required Rich panels for ``agents session export``.
|
"""Render the three spec-required Rich panels for ``agents session export``.
|
||||||
|
|
||||||
@@ -661,6 +670,7 @@ def _render_export_panels(
|
|||||||
- "Integrity" panel: checksum, encrypted flag
|
- "Integrity" panel: checksum, encrypted flag
|
||||||
- ``✓ OK Export completed`` success line
|
- ``✓ OK Export completed`` success line
|
||||||
"""
|
"""
|
||||||
|
_console = panels_console if panels_console is not None else console
|
||||||
# Compute file size from content
|
# Compute file size from content
|
||||||
size_bytes = len(content.encode("utf-8"))
|
size_bytes = len(content.encode("utf-8"))
|
||||||
if size_bytes < 1024:
|
if size_bytes < 1024:
|
||||||
@@ -694,8 +704,8 @@ def _render_export_panels(
|
|||||||
export_table.add_row("Messages:", str(message_count))
|
export_table.add_row("Messages:", str(message_count))
|
||||||
export_table.add_row("Size:", size_str)
|
export_table.add_row("Size:", size_str)
|
||||||
export_table.add_row("Format:", format_display)
|
export_table.add_row("Format:", format_display)
|
||||||
console.print(Panel(export_table, title="Session Export", border_style="blue"))
|
_console.print(Panel(export_table, title="Session Export", border_style="blue"))
|
||||||
console.print()
|
_console.print()
|
||||||
|
|
||||||
# Contents panel
|
# Contents panel
|
||||||
contents_table = Table.grid(padding=(0, 1))
|
contents_table = Table.grid(padding=(0, 1))
|
||||||
@@ -706,8 +716,8 @@ def _render_export_panels(
|
|||||||
contents_table.add_row("Metadata Keys:", str(metadata_keys))
|
contents_table.add_row("Metadata Keys:", str(metadata_keys))
|
||||||
contents_table.add_row("Actor Config:", actor_config)
|
contents_table.add_row("Actor Config:", actor_config)
|
||||||
contents_table.add_row("Schema Version:", str(schema_version))
|
contents_table.add_row("Schema Version:", str(schema_version))
|
||||||
console.print(Panel(contents_table, title="Contents", border_style="blue"))
|
_console.print(Panel(contents_table, title="Contents", border_style="blue"))
|
||||||
console.print()
|
_console.print()
|
||||||
|
|
||||||
# Integrity panel
|
# Integrity panel
|
||||||
integrity_table = Table.grid(padding=(0, 1))
|
integrity_table = Table.grid(padding=(0, 1))
|
||||||
@@ -715,10 +725,10 @@ def _render_export_panels(
|
|||||||
integrity_table.add_column(style="white", justify="left")
|
integrity_table.add_column(style="white", justify="left")
|
||||||
integrity_table.add_row("Checksum:", checksum_display)
|
integrity_table.add_row("Checksum:", checksum_display)
|
||||||
integrity_table.add_row("Encrypted:", "no")
|
integrity_table.add_row("Encrypted:", "no")
|
||||||
console.print(Panel(integrity_table, title="Integrity", border_style="blue"))
|
_console.print(Panel(integrity_table, title="Integrity", border_style="blue"))
|
||||||
console.print()
|
_console.print()
|
||||||
|
|
||||||
console.print("[green]✓ OK[/green] Export completed")
|
_console.print("[green]✓ OK[/green] Export completed")
|
||||||
|
|
||||||
|
|
||||||
@app.command("import")
|
@app.command("import")
|
||||||
|
|||||||
Reference in New Issue
Block a user