From 156872eba48e6e3da08e26bac05f1739988c66d8 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:56:10 +0100 Subject: [PATCH 1/9] fix(ai): litellm prints nothing to stdout, so `ai test --json` is JSON On a failed call litellm printed its "Give Feedback / Get Help" banner to stdout, ahead of `fluid ai test --json`'s report. quiet_litellm() sets litellm's own switch, suppress_debug_info (the one its Router and proxy set, BerriAI/litellm#5942), wherever forge loads litellm: _get_litellm, ai_setup._validate_api_key and the agent loop's tool call. The failure still reaches forge as the exception, and forge reports it as before. The tests use the installed litellm: mock_response=Exception() fails inside litellm's error mapping, where the banner is printed, with no network call. --- CHANGELOG.md | 4 + fluid_build/cli/ai_setup.py | 3 + fluid_build/cli/forge_copilot_agent_loop.py | 3 + fluid_build/llm/litellm_backend.py | 21 ++++ tests/test_litellm_banner.py | 117 ++++++++++++++++++++ 5 files changed, 148 insertions(+) create mode 100644 tests/test_litellm_banner.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c1dbc29..bc2a7039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 for a model the endpoint has. All three now ask the stated endpoint. A `--tiered` stage now calls a stated endpoint too, as the routing model already did; it called the provider's default address instead. +- **`fluid ai test --json` prints only its report when the call fails.** litellm printed its + "Give Feedback / Get Help" banner to stdout ahead of the JSON. Forge now sets litellm's own + switch, `suppress_debug_info`, wherever it loads litellm (AI calls, `fluid ai setup`'s key + check, the agent loop), so no command prints the banner. The error is reported as before. ### Security diff --git a/fluid_build/cli/ai_setup.py b/fluid_build/cli/ai_setup.py index 0e57f80d..e8ec2c1f 100644 --- a/fluid_build/cli/ai_setup.py +++ b/fluid_build/cli/ai_setup.py @@ -247,8 +247,11 @@ def _validate_api_key(provider: Any, api_key: str) -> Optional[str]: from fluid_build.cli.forge_copilot_llm_litellm import ( _LITELLM_PREFIX_BY_PROVIDER, + quiet_litellm, ) + quiet_litellm(litellm) + prefix = _LITELLM_PREFIX_BY_PROVIDER.get(provider.name.lower(), provider.name.lower()) model = ( provider.default_model diff --git a/fluid_build/cli/forge_copilot_agent_loop.py b/fluid_build/cli/forge_copilot_agent_loop.py index ee0e5f78..798c67ad 100644 --- a/fluid_build/cli/forge_copilot_agent_loop.py +++ b/fluid_build/cli/forge_copilot_agent_loop.py @@ -643,6 +643,9 @@ def _call_litellm_with_tools(payload: Dict[str, Any]) -> Dict[str, Any]: """ import litellm + from fluid_build.llm.litellm_backend import quiet_litellm + + quiet_litellm(litellm) payload = dict(payload) payload["tools"] = _to_openai_tool_shape(list(payload.get("tools") or [])) response = litellm.completion(_skip_mcp_handler=True, **payload) diff --git a/fluid_build/llm/litellm_backend.py b/fluid_build/llm/litellm_backend.py index e04bac6d..0894753d 100644 --- a/fluid_build/llm/litellm_backend.py +++ b/fluid_build/llm/litellm_backend.py @@ -100,6 +100,27 @@ def _get_litellm() -> Any: "(installs the litellm version this Python supports)" ], ) from exc + return quiet_litellm(litellm) + + +def quiet_litellm(litellm: Any) -> Any: + """Turn off what litellm prints to stdout by itself, and return the module. + + On a failed call litellm prints a "Give Feedback / Get Help" banner (and, + for a model it cannot route, a "Provider List" link) with ``print``, which + puts text ahead of ``fluid ai test --json``'s report and in ``fluid forge + --agent``'s event stream. ``suppress_debug_info`` is litellm's own switch for + it, the one its Router and proxy set for the same reason (BerriAI/litellm + issue 5942). No error detail is lost: the failure still arrives as the + exception, which forge reports, and the provider-list link is in its text. + What goes is litellm's request for bug reports and its debug-mode hint. + + Forge loads litellm here, in ``ai_setup._validate_api_key`` and in the + agent loop's tool call; each calls this, so whichever runs first sets it. + The fallback router (``fluid_build.llm.router``) builds a ``litellm.Router``, + which sets it too. + """ + litellm.suppress_debug_info = True return litellm diff --git a/tests/test_litellm_banner.py b/tests/test_litellm_banner.py new file mode 100644 index 00000000..515dda16 --- /dev/null +++ b/tests/test_litellm_banner.py @@ -0,0 +1,117 @@ +# Copyright 2025 Agentics Transformation Limited +# SPDX-License-Identifier: Apache-2.0 + +"""litellm prints nothing of its own to stdout, wherever forge loads it. + +On a failed call litellm prints a "Give Feedback / Get Help" banner to stdout +unless ``litellm.suppress_debug_info`` is set, which put text ahead of +``fluid ai test --json``'s report. These tests use the installed litellm, not a +fake: the point is that the switch forge sets is the one this litellm reads. +``mock_response=Exception(...)`` makes litellm fail inside its own error mapping, +where the banner is printed, with no network call. +""" + +from __future__ import annotations + +import argparse +import json +from unittest import mock + +import pytest + +_MESSAGES = [{"role": "user", "content": "Say ok"}] +_DUMMY_KEY = "not-a-real-key" # pragma: allowlist secret + + +@pytest.fixture +def litellm(monkeypatch): + """The installed litellm, its banner switch back at litellm's default (on).""" + monkeypatch.setenv("LITELLM_LOCAL_MODEL_COST_MAP", "True") # no price-map fetch + module = pytest.importorskip("litellm") + monkeypatch.setattr(module, "suppress_debug_info", False) + return module + + +@pytest.fixture +def failing_completion(litellm, monkeypatch, capsys): + """Make every ``litellm.completion`` fail inside litellm, as a provider error does. + + It first checks that this litellm prints its banner with the switch off: one + that never prints has nothing to switch off, and the test skips. + """ + real = litellm.completion + + def completion(*args, **kwargs): + return real(*args, **{**kwargs, "mock_response": Exception("provider down")}) + + monkeypatch.setattr(litellm, "completion", completion) + with pytest.raises(Exception): + litellm.completion(model="openai/gpt-4o-mini", messages=_MESSAGES, api_key=_DUMMY_KEY) + if "Give Feedback / Get Help" not in capsys.readouterr().out: + pytest.skip("this litellm prints no banner on a failed call: nothing to switch off") + + +def test_get_litellm_switches_the_banner_off(litellm, failing_completion, capsys): + from fluid_build.llm.litellm_backend import _get_litellm + + loaded = _get_litellm() + with pytest.raises(Exception): + loaded.completion(model="openai/gpt-4o-mini", messages=_MESSAGES, api_key=_DUMMY_KEY) + + assert capsys.readouterr().out == "" + + +def test_ai_test_json_stdout_is_the_report_alone_when_the_call_fails( + litellm, failing_completion, monkeypatch, capsys, tmp_path +): + """``fluid ai test --json`` on a failed call: one JSON document, nothing before it.""" + from fluid_build.cli.ai_setup import run_ai_test + + monkeypatch.chdir(tmp_path) + for name in ("ANTHROPIC_API_KEY", "GEMINI_API_KEY", "GOOGLE_API_KEY", "FLUID_LLM_API_KEY"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("OPENAI_API_KEY", _DUMMY_KEY) + args = argparse.Namespace( + llm_provider="openai", + llm_model=None, + llm_endpoint=None, + llm_timeout_seconds=None, + json=True, + ) + with ( + mock.patch("fluid_build.cli.ai_setup._load_ai_config", return_value=None), + mock.patch( + "fluid_build.cli.ai_setup._check_ai_test_model_availability", + return_value=("unavailable", None), + ), + ): + code, report = run_ai_test(None, args) + + assert code != 0 + assert report["error"]["code"] == "copilot_litellm_request_failed" + assert json.loads(capsys.readouterr().out) == json.loads(json.dumps(report)) + + +def test_ai_setup_key_check_prints_no_banner( + litellm, failing_completion, monkeypatch, capsys, tmp_path +): + """``fluid ai setup`` loads litellm itself to check a key.""" + import fluid_build.cli.ai_setup as ai_setup + from fluid_build.cli.forge_copilot_llm_litellm import get_litellm_provider + + monkeypatch.chdir(tmp_path) # no project dotenv to hold the check back + error = ai_setup._validate_api_key(get_litellm_provider("openai"), _DUMMY_KEY) + + assert error # the check failed, and said so + assert capsys.readouterr().out == "" + + +def test_agent_loop_tool_call_prints_no_banner(litellm, failing_completion, capsys): + """The agent loop's tool-use call imports litellm directly.""" + from fluid_build.cli.forge_copilot_agent_loop import _call_litellm_with_tools + + payload = {"model": "openai/gpt-4o-mini", "messages": _MESSAGES, "api_key": _DUMMY_KEY} + with pytest.raises(Exception): + _call_litellm_with_tools(payload) + + assert capsys.readouterr().out == "" From ae1f7955059afabfd392c62d15ac401e68b91c28 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:56:20 +0100 Subject: [PATCH 2/9] fix(forge): `--agent` reports a failure as a forge.error event, not a traceback A typed AI error such as copilot_no_llm_provider printed a full traceback on stderr: _create_project_minimal logged it with logger.exception, which the CLI's console handler prints at the default INFO level, in every mode. The --agent stream said only forge.done with exit_code 1. log_forge_failure (cli/_forge_mode_errors.py) logs a FLUID error once at ERROR without the traceback, kept off the console when the caller prints the message itself, and the traceback at DEBUG; a crash keeps logger.exception. It notes the failure in a ContextVar, and forge.run sends it as one forge.error event before forge.done: code, message, suggestions and docs_url, redacted; an unexpected exception by its class only, since MCP forge_run hands the events to the IDE's model. Template mode, the agent loop, the minimal path, the mode boundary and _run_main use it; _run_main now prints a FluidUserError's fix. A failed --seed-from notes the console's words, not its exception's text. MCP forge_run writes each forge.error to the server's log, since it discards forge's console output. --- CHANGELOG.md | 14 ++ fluid_build/cli/_forge_mode_errors.py | 144 +++++++++++++++- fluid_build/cli/_template_mode.py | 32 +--- fluid_build/cli/forge.py | 63 ++++++- fluid_build/cli/forge_modes.py | 13 +- fluid_build/cli/mcp/dispatch.py | 15 ++ tests/cli/test_forge_agent_mode.py | 213 +++++++++++++++++++++++- tests/cli/test_forge_mode_errors.py | 228 ++++++++++++++++++++++++++ 8 files changed, 679 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc2a7039..f5eb1de7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 for a model the endpoint has. All three now ask the stated endpoint. A `--tiered` stage now calls a stated endpoint too, as the routing model already did; it called the provider's default address instead. +- **`fluid forge --agent` reports a failure as a `forge.error` event, not a traceback.** An AI + error such as `copilot_no_llm_provider` printed a full Python traceback on stderr, and the + stream said only `forge.done` with `exit_code` 1. A run that fails on an error now sends one + `forge.error` event before `forge.done`, with the error's `code`, `message`, `suggestions` + and, when the error catalog has one, `docs_url` (the shape of `fluid ai test --json`'s + `error`), redacted as the console redacts. An unexpected exception is named by its class only + (`code: forge_unexpected_error`, `exception`), and a failed `--seed-from` by its path and class + (`forge_seed_failed`), because the MCP `forge_run` hands these events to the IDE's model; it + also writes the failure to the server's log, since it discards forge's console output. A usage + error (exit 2) or a stop without an error, such as blank mode finding a contract in place, + still sends only `forge.done`. In every forge mode, interactive runs included, a FLUID error's + traceback now shows only at DEBUG (`--debug` or `FLUID_LOG_LEVEL=DEBUG`); stderr keeps its + message and next steps, and one that reaches forge's last handler prints them instead of its + event code. A crash keeps its traceback. - **`fluid ai test --json` prints only its report when the call fails.** litellm printed its "Give Feedback / Get Help" banner to stdout ahead of the JSON. Forge now sets litellm's own switch, `suppress_debug_info`, wherever it loads litellm (AI calls, `fluid ai setup`'s key diff --git a/fluid_build/cli/_forge_mode_errors.py b/fluid_build/cli/_forge_mode_errors.py index d4173833..00d00382 100644 --- a/fluid_build/cli/_forge_mode_errors.py +++ b/fluid_build/cli/_forge_mode_errors.py @@ -41,8 +41,9 @@ from __future__ import annotations import logging +from contextvars import ContextVar from types import TracebackType -from typing import Any, Callable, Optional +from typing import Any, Callable, Dict, List, Optional, Tuple # Render callback: receives ``(console, exc)`` and prints the # handler-specific failure message. ``console`` may be ``None`` on the @@ -73,8 +74,10 @@ class ForgeModeErrorBoundary: set: ``logger.info(cancel_label)`` then exit code ``130``. When ``cancel_label`` is ``None`` the interrupt propagates unchanged (the domain-agent handler never caught Ctrl-C). - * Any ``Exception`` logs ``logger.exception(fail_label)``, invokes - ``render_error(console, exc)``, and yields exit code ``1``. + * Any ``Exception`` is logged through :func:`log_forge_failure` (a crash + as ``logger.exception(fail_label)``, a FLUID error with its traceback + at DEBUG only), invokes ``render_error(console, exc)``, and yields exit + code ``1``. * ``SystemExit`` / ``GeneratorExit`` and other non-``Exception`` ``BaseException``\\ s propagate untouched — matching a bare ``except Exception`` clause, which never caught them either. @@ -130,7 +133,9 @@ def __exit__( # ``return False`` below — exactly as a bare ``except Exception`` # would leave them uncaught. if isinstance(exc, Exception): - self._logger.exception(self._fail_label) + # Without a console a renderer may print nothing, so the log line + # is then the only place the failure shows. + log_forge_failure(self._logger, self._fail_label, exc, shown=self._console is not None) self._render_error(self._console, exc) self.exit_code = 1 return True @@ -154,3 +159,134 @@ def forge_mode_error_boundary( render_error=render_error, cancel_label=cancel_label, ) + + +# --------------------------------------------------------------------------- +# Why a forge run failed: ``log_forge_failure`` and the ``--agent`` event +# --------------------------------------------------------------------------- +# +# A mode handler catches its own failure, prints it and returns an exit code, +# so by the time ``forge.run`` writes ``forge.done`` the exception is gone. +# Each handler that swallows one hands it to :func:`log_forge_failure`, which +# notes it here; ``forge.run`` reads the note back and, under ``--agent``, +# writes it as one ``forge.error`` event. A ``ContextVar``, not a module global: +# the MCP ``forge_run`` runs forge in a worker thread, and a run reads only its +# own note. + +_RUN_FAILURE: ContextVar[Optional[BaseException]] = ContextVar( + "fluid_forge_run_failure", default=None +) + +#: ``forge.error``'s ``code`` for an exception that is not a FLUID error. +UNEXPECTED_FAILURE_CODE = "forge_unexpected_error" + + +def note_forge_failure(exc: BaseException) -> None: + """Remember *exc* as why this forge run is failing; a later failure replaces it.""" + _RUN_FAILURE.set(exc) + + +def take_forge_failure() -> Optional[BaseException]: + """Return the failure noted for this run, and forget it.""" + exc = _RUN_FAILURE.get() + _RUN_FAILURE.set(None) + return exc + + +def is_typed_forge_error(exc: BaseException) -> bool: + """Whether *exc* is a FLUID error, written for the user, rather than a crash. + + ``CLIError`` (``CopilotGenerationError``, ``ForgeError`` and the rest), + ``FluidError``, ``FluidUserError`` and a missing optional extra each say what + went wrong and, mostly, what to do next. Anything else is a bug. + """ + from fluid_build._contract_loader import CLIError + from fluid_build._errors import FluidUserError + from fluid_build._optional import MissingOptionalDependency + from fluid_build.errors import FluidError + + return isinstance(exc, (CLIError, FluidError, FluidUserError, MissingOptionalDependency)) + + +def _failure_parts(exc: BaseException) -> Tuple[str, str, List[str], Optional[str]]: + """``(code, message, suggestions, docs_url)`` for *exc*, not yet redacted.""" + import re + + from fluid_build._contract_loader import CLIError + from fluid_build._errors import FluidUserError + from fluid_build._optional import MissingOptionalDependency + from fluid_build.errors import FluidError + + if isinstance(exc, MissingOptionalDependency): + # ``str(exc)`` runs the message, the install line and any alternatives + # together as paragraphs; the event keeps them apart. + hints = [f"pip install 'data-product-forge[{exc.extra}]'"] + if exc.alternatives: + hints.append(exc.alternatives) + message = f"{exc.feature} needs {exc.package}, which is not installed." + return exc.event, message, hints, None + if isinstance(exc, CLIError): + return exc.event, str(exc.message), list(exc.suggestions or []), exc.docs_url + if isinstance(exc, FluidUserError): + return exc.code, exc.what, [exc.fix] if exc.fix else [], exc.doc or None + if isinstance(exc, FluidError): + # No event key on this family: the class name, snake_cased like one. + code = re.sub(r"(? Dict[str, Any]: + """``forge.error``'s fields for *exc*: ``code``, ``message``, ``suggestions``. + + The shape of ``fluid ai test --json``'s ``error`` object, plus ``docs_url`` + when the error catalog has one. The text is redacted as the console redacts + it, since the events go to stdout directly, past the logging redactor. An + exception that is not a FLUID error is named by its class (``exception``), + never by its text: that can carry paths or provider detail, and the MCP + ``forge_run`` returns these events to the IDE's model, where the copilot + tools' typed-error rule (SECURITY_REVIEW I7) applies. stderr has the detail. + """ + from fluid_build._console import _redact_error_str + + code, message, suggestions, docs_url = _failure_parts(exc) + fields: Dict[str, Any] = { + "code": code, + "message": _redact_error_str(message), + "suggestions": [_redact_error_str(s) for s in suggestions], + } + if docs_url: + fields["docs_url"] = docs_url + if not is_typed_forge_error(exc): + fields["exception"] = type(exc).__name__ + return fields + + +def log_forge_failure( + logger: logging.Logger, label: str, exc: BaseException, *, shown: bool = True +) -> None: + """Log a failure that its caller prints for the user, and note it for ``--agent``. + + A FLUID error (:func:`is_typed_forge_error`) is an outcome with its own + message and next steps, not a crash. It gets one ERROR record (*label* and + its code) with no traceback, and the traceback at DEBUG. The ERROR record + reaches ``--log-file`` but skips the console when the caller prints the + message there itself (*shown*): the CLI's console handler drops a record + marked ``fluid_console_shown``. Anything else keeps ``logger.exception``, + whose traceback is the bug report. + + Call it while *exc* is being handled: in its ``except`` block, or in the + ``__exit__`` of a context manager that it is unwinding. + """ + note_forge_failure(exc) + if not is_typed_forge_error(exc): + logger.exception(label) + return + code = _failure_parts(exc)[0] + logger.error("%s: %s", label, code, extra={"fluid_console_shown": shown}) + logger.debug("%s: %s", label, code, exc_info=exc) diff --git a/fluid_build/cli/_template_mode.py b/fluid_build/cli/_template_mode.py index ee095fed..9c8afd95 100644 --- a/fluid_build/cli/_template_mode.py +++ b/fluid_build/cli/_template_mode.py @@ -40,6 +40,7 @@ # that this module references via ``_fm.`` so test patches that # target ``fluid_build.cli.forge_modes.`` resolve at call time. from fluid_build.cli import forge_modes as _fm # noqa: E402 +from fluid_build.cli._forge_mode_errors import log_forge_failure from fluid_build.cli.console import cprint, success from fluid_build.cli.console import error as console_error from fluid_build.cli.forge_copilot_llm_providers import ( @@ -151,7 +152,7 @@ def run_template_mode( logger.info("3. fluid validate contract.fluid.yaml") return 0 except Exception as exc: # noqa: BLE001 - logger.exception("Template mode failed") + log_forge_failure(logger, "Template mode failed", exc) if console: console.print(literal_error_text(f"❌ Template mode failed: {exc}", "red")) else: @@ -273,7 +274,7 @@ def _create_project_agent_loop( return True except CopilotGenerationError as exc: - logger.exception("Agent loop failed") + log_forge_failure(logger, "Agent loop failed", exc, shown=console is not None) if console: try: console.print(literal_error_text(exc.message, "red")) @@ -283,7 +284,7 @@ def _create_project_agent_loop( pass return False except Exception as exc: # noqa: BLE001 - logger.exception("Agent loop crashed") + log_forge_failure(logger, "Agent loop crashed", exc, shown=console is not None) if console: try: console.print(literal_error_text(f"Agent loop failed: {exc}", "red")) @@ -292,20 +293,6 @@ def _create_project_agent_loop( return False -def _is_missing_optional(exc: BaseException) -> bool: - """True when *exc* is, or was caused by, a missing optional dependency.""" - from fluid_build._optional import MissingOptionalDependency - - cur: Optional[BaseException] = exc - for _ in range(8): - if cur is None: - return False - if isinstance(cur, MissingOptionalDependency): - return True - cur = cur.__cause__ or cur.__context__ - return False - - def _emit_incomplete_forge_span(logger: logging.Logger) -> None: """Emit a ``run_completed=False`` ``forge.invocation`` span on failure. @@ -1023,12 +1010,9 @@ def _serialize_logical_model(value: Any) -> Optional[str]: return True except CopilotGenerationError as exc: - if _is_missing_optional(exc): - # An absent optional package is an install problem, not a crash: - # the message and hint are the whole story, so no traceback. - logger.error("AI Copilot minimal flow unavailable: %s", exc.message) - else: - logger.exception("AI Copilot minimal flow failed") + # The message and its suggestions, printed below, are the whole story + # (a missing extra's install line included): the traceback is DEBUG. + log_forge_failure(logger, "AI Copilot minimal flow failed", exc) _emit_incomplete_forge_span(logger) if console: try: @@ -1041,7 +1025,7 @@ def _serialize_logical_model(value: Any) -> Optional[str]: console_error(exc.message) return False except (OSError, ValueError, KeyError, TypeError, RuntimeError) as exc: - logger.exception("AI Copilot minimal flow crashed") + log_forge_failure(logger, "AI Copilot minimal flow crashed", exc) _emit_incomplete_forge_span(logger) if console: try: diff --git a/fluid_build/cli/forge.py b/fluid_build/cli/forge.py index 90804c98..df6b5fd3 100644 --- a/fluid_build/cli/forge.py +++ b/fluid_build/cli/forge.py @@ -1484,7 +1484,12 @@ def run(args, logger: logging.Logger) -> int: is_agent = bool(getattr(args, "agent", False)) if not is_agent: - return _run_main(args, logger) + from fluid_build.cli._forge_mode_errors import take_forge_failure + + try: + return _run_main(args, logger) + finally: + take_forge_failure() # read by --agent only: drop it and its frames _setup_agent_mode(args) run_id = _agent_run_id() @@ -1519,7 +1524,7 @@ def run(args, logger: logging.Logger) -> int: # point says the remedy on stderr under --agent # (``cli._render_missing_optional``), and an in-process caller # (the MCP ``forge_run``) gets the exception itself. - rc = _run_main(args, logger) + rc = _run_main_noting_failure(args, logger) # On success, optionally emit the structured plan checklist BEFORE # forge.done so consumers parsing the stream see the plan in order. if rc == 0 and getattr(args, "emit_plan", False): @@ -1528,10 +1533,46 @@ def run(args, logger: logging.Logger) -> int: _emit_forge_plan(args, contract_path) return rc finally: + _emit_forge_error(run_id, rc) _emit_agent_jsonl("forge.done", run_id=run_id, exit_code=int(rc or 0)) _AGENT_EVENTS = None +def _run_main_noting_failure(args: Any, logger: logging.Logger) -> int: + """``_run_main`` for an ``--agent`` run, noting an exception that propagates. + + A handler that catches its failure notes it itself (``log_forge_failure``); + one that escapes ``_run_main`` (a missing extra) is noted here, then re-raised. + """ + from fluid_build.cli._forge_mode_errors import note_forge_failure, take_forge_failure + + take_forge_failure() # an earlier in-process run's failure is not this run's + try: + return _run_main(args, logger) + except Exception as exc: + note_forge_failure(exc) + raise + + +def _emit_forge_error(run_id: str, rc: int) -> None: + """Send ``forge.error`` for a failed run: why it failed, as data. + + The failure is the one a handler noted. A run that failed without an + exception (blank mode finding a contract in place) has none to send. + """ + from fluid_build.cli._forge_mode_errors import describe_forge_failure, take_forge_failure + + failure = take_forge_failure() + if rc == 0 or failure is None: + return + try: + fields = describe_forge_failure(failure) + except Exception: # noqa: BLE001 — never at the cost of forge.done + LOG.debug("forge_error_event_failed", exc_info=True) + return + _emit_agent_jsonl("forge.error", run_id=run_id, **fields) + + #: Per-process flag for the personal-memory sanitiser. See #: ``_sanitize_personal_memory_once`` for the rationale — issue #51 fix. _PERSONAL_MEMORY_SANITISED: bool = False @@ -1947,18 +1988,23 @@ def _run_main(args, logger: logging.Logger) -> int: # operator-facing output is just the message + actionable # next steps. from fluid_build._optional import MissingOptionalDependency - from fluid_build.errors import FluidError + from fluid_build.cli._forge_mode_errors import ( + describe_forge_failure, + is_typed_forge_error, + log_forge_failure, + ) if isinstance(exc, MissingOptionalDependency): # "Install the extra" is the whole remedy: let the CLI entry point # print the install line and exit 1, with no traceback. raise - logger.debug("Forge command failed", exc_info=True) - suggestions = getattr(exc, "suggestions", None) or [] - message = getattr(exc, "message", None) or str(exc) - if isinstance(exc, FluidError): - # Typed Fluid error — clean message + suggestions. + log_forge_failure(logger, "Forge command failed", exc) + if is_typed_forge_error(exc): + # Typed Fluid error — clean message + suggestions, the ones the + # ``--agent`` event carries (a FluidUserError's ``fix`` included). + failure = describe_forge_failure(exc) + message, suggestions = failure["message"], failure["suggestions"] if console: console.print( literal_text("Forge failed:", "red") + literal_error_text(f" {message}") @@ -1975,7 +2021,6 @@ def _run_main(args, logger: logging.Logger) -> int: cprint_literal(f" - {s}") else: # Unexpected — full message; suggest doctor. - logger.exception("Forge command failed") if console: console.print(literal_error_text(f"Forge failed: {exc}", "red")) console.print(f"[dim]Run 'fluid doctor' to diagnose, or see {_DOCS_URL}[/dim]") diff --git a/fluid_build/cli/forge_modes.py b/fluid_build/cli/forge_modes.py index ee867798..5f91f540 100644 --- a/fluid_build/cli/forge_modes.py +++ b/fluid_build/cli/forge_modes.py @@ -22,7 +22,7 @@ from typing import Any, Callable, Dict, List, Mapping, Optional, Sequence from fluid_build._console import literal_error_text -from fluid_build.cli._forge_mode_errors import forge_mode_error_boundary +from fluid_build.cli._forge_mode_errors import forge_mode_error_boundary, note_forge_failure from fluid_build.cli.console import cprint, cprint_literal, success from fluid_build.cli.console import error as console_error from fluid_build.cli.forge_copilot_interview import ( @@ -1268,9 +1268,14 @@ def run_ai_copilot_mode( "exc_type": type(_seed_exc).__name__, }, ) - console_error( - f"--seed-from failed for {_seed_from!s}: " - f"{type(_seed_exc).__name__} (run with --verbose for details)" + failed = f"--seed-from failed for {_seed_from!s}: {type(_seed_exc).__name__}" + console_error(f"{failed} (run with --verbose for details)") + # ``--agent``'s forge.error gets the same words, not the exception: + # a typed one (ProviderError) would carry its text into the event. + note_forge_failure( + CopilotGenerationError( + "forge_seed_failed", failed, suggestions=["Run with --verbose for details"] + ) ) return 1 diff --git a/fluid_build/cli/mcp/dispatch.py b/fluid_build/cli/mcp/dispatch.py index dcba5be4..3d251b86 100644 --- a/fluid_build/cli/mcp/dispatch.py +++ b/fluid_build/cli/mcp/dispatch.py @@ -1200,6 +1200,19 @@ def _scope_from_args(arguments: Dict[str, Any]) -> Any: return CatalogScope.model_validate(flat) +def _log_forge_errors(events: List[Dict[str, Any]], logger: logging.Logger) -> None: + """Write each ``forge.error`` event of an in-process run to the server's log. + + ``_run_forge_inproc`` discards forge's console output, and forge marks its own + record of a typed failure as shown on that console, so the server's console + handler drops it. Without this line a failed ``forge_run`` leaves the server + log empty, though the tool errors' "see server logs" points there. + """ + for event in events: + if event["event"] == "forge.error": + logger.warning("forge_run failed: %s: %s", event.get("code"), event.get("message")) + + def _run_forge_inproc( mode: str, target_dir: str, @@ -1284,6 +1297,8 @@ def _run_forge_inproc( if isinstance(obj, dict) and isinstance(obj.get("event"), str): events.append(obj) + _log_forge_errors(events, forge_logger) + contract_path = Path(target_dir) / "contract.fluid.yaml" return { "mode": mode, diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index 2e3df781..bcc149eb 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -22,6 +22,7 @@ import json import logging import os +import shutil import subprocess import sys from pathlib import Path @@ -632,7 +633,10 @@ def _raise_missing_litellm(args, logger): def test_agent_missing_extra_still_propagates_in_process(tmp_path, capsys, monkeypatch): - """An in-process caller (the MCP ``forge_run``) gets the exception, after forge.done.""" + """An in-process caller (the MCP ``forge_run``) gets the exception, after forge.done. + + The stream says which extra is missing, in forge.error, before forge.done. + """ from fluid_build._optional import MissingOptionalDependency from fluid_build.cli import forge as forge_mod @@ -642,8 +646,10 @@ def test_agent_missing_extra_still_propagates_in_process(tmp_path, capsys, monke forge_mod.run(args, logging.getLogger("test")) events = _json_lines_only(capsys.readouterr().out) - assert events[-1]["event"] == "forge.done" + assert [e["event"] for e in events][-2:] == ["forge.error", "forge.done"] assert events[-1]["exit_code"] == 1 + assert events[-2]["code"] == "copilot_litellm_unavailable" + assert events[-2]["suggestions"] == ["pip install 'data-product-forge[litellm]'"] def test_agent_missing_extra_remedy_goes_to_stderr(tmp_path, capsys, monkeypatch): @@ -659,3 +665,206 @@ def test_agent_missing_extra_remedy_goes_to_stderr(tmp_path, capsys, monkeypatch events = _json_lines_only(captured.out) assert events[-1]["event"] == "forge.done" assert "pip install 'data-product-forge[litellm]'" in captured.err + + +# --------------------------------------------------------------------------- +# 8. A failure is one forge.error event; its traceback is a DEBUG detail +# --------------------------------------------------------------------------- + + +def _no_provider_env(tmp_path: Path) -> dict: + """The hermetic env with nothing an LLM provider could be found from. + + No key or Ollama variable, no coding-agent CLI on PATH, and every proxy + variable on a dead port, so a local Ollama on :11434 goes unseen too. + """ + from fluid_build.llm.coding_agent import _AGENT_SPECS + + env = { + k: v + for k, v in _hermetic_env(tmp_path).items() + if not k.startswith("OLLAMA_") + and not k.endswith(("_BASE_URL", "_API_BASE", "_AUTH_TOKEN")) + and k not in ("FLUID_HOME", "FLUID_CONFIG") # the saved choice lives under HOME + } + agent_clis = [spec.binary for spec in _AGENT_SPECS.values()] + env["PATH"] = os.pathsep.join( + entry + for entry in env.get("PATH", "").split(os.pathsep) + if entry and not any(shutil.which(cli, path=entry) for cli in agent_clis) + ) + dead = "http://127.0.0.1:9" + for name in ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"): + env[name] = env[name.lower()] = dead + env["NO_PROXY"] = env["no_proxy"] = "" + env["LITELLM_LOCAL_MODEL_COST_MAP"] = "True" + return env + + +@pytest.mark.parametrize("log_level", ["INFO", "DEBUG"]) +def test_agent_ai_error_is_a_forge_error_event_not_a_traceback(log_level, tmp_path: Path): + """No provider configured, through the CLI entry point: the stream says why. + + Every stdout line is JSON at either log level, and stderr shows the + traceback only at DEBUG. + """ + proc = subprocess.run( + [*_fluid_invocation(), "--agent", "--context", "x", "-d", str(tmp_path / "product")], + cwd=tmp_path, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=60, + env={**_no_provider_env(tmp_path), "FLUID_LOG_LEVEL": log_level}, + ) + + assert proc.returncode == 1, proc.stderr + events = _json_lines_only(proc.stdout) + assert [e["event"] for e in events] == ["forge.start", "forge.error", "forge.done"] + start, error, done = events + assert error["run_id"] == start["run_id"] == done["run_id"] + assert error["code"] == "copilot_no_llm_provider" + assert error["message"].startswith("No LLM provider configured") + assert any("fluid ai setup" in s for s in error["suggestions"]) + assert error["docs_url"].startswith("https://") + assert done["exit_code"] == 1 + assert ("Traceback (most recent call last)" in proc.stderr) is (log_level == "DEBUG") + assert "No LLM provider configured" in proc.stderr # the human still reads why + + +@pytest.fixture +def _agent_env(monkeypatch): + """The variables ``--agent`` sets, owned by monkeypatch so they do not leak.""" + for var in ( + "FLUID_FORGE_NO_PICKER", + "FLUID_FORGE_NO_PREVIEW", + "FLUID_FORGE_NO_WELCOME", + "FLUID_FORGE_NO_STREAMING_PREVIEW", + ): + monkeypatch.setenv(var, "1") + + +def test_agent_crash_is_named_by_class_not_text(tmp_path, capsys, monkeypatch, _agent_env): + """An unexpected exception: its class and a pointer to stderr, never its text. + + The MCP ``forge_run`` hands these events to the IDE's model, and an + exception's text can carry paths or provider detail. + """ + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + events, _ = _run_agent(args, capsys, monkeypatch, ai_result=RuntimeError("/secret/path")) + + error = next(e for e in events if e["event"] == "forge.error") + assert error["code"] == "forge_unexpected_error" + assert error["exception"] == "RuntimeError" + assert "/secret/path" not in json.dumps(error) + + +def test_agent_failure_without_an_exception_sends_no_forge_error( + tmp_path, capsys, monkeypatch, _agent_env +): + """Exit 1 with nothing raised: forge.done says it failed; nothing is made up. + + A failure noted before the run (an earlier in-process run, a handler outside + ``--agent``) is not this run's: the run starts by forgetting it. + """ + from fluid_build.cli._forge_mode_errors import note_forge_failure + + note_forge_failure(KeyError("an earlier run")) + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + events, _ = _run_agent(args, capsys, monkeypatch, ai_result=1) + + assert [e["event"] for e in events] == ["forge.start", "forge.done"] + assert events[-1]["exit_code"] == 1 + + +def test_agent_seed_failure_is_a_forge_error_without_the_exception_text( + tmp_path, capsys, monkeypatch, _agent_env +): + """``--seed-from`` swallows its error; the event says what the console says. + + The exception is a typed one whose text can carry a remote seed's body, so + the event gets the console's words (path and class), never that text. + """ + from fluid_build.cli import forge as forge_mod + + monkeypatch.chdir(tmp_path) + missing = tmp_path / "missing-seed.yaml" + args = _agent_args( + ["--llm-provider", "openai", "--context", '{"project_goal": "x"}', "--seed-from"] + + [str(missing)], + tmp_path, + ) + assert forge_mod.run(args, logging.getLogger("test")) == 1 + + events = _json_lines_only(capsys.readouterr().out) + assert [e["event"] for e in events] == ["forge.start", "forge.error", "forge.done"] + error = events[1] + assert error["code"] == "forge_seed_failed" + assert error["message"].startswith(f"--seed-from failed for {missing}: ") + assert "exception" not in error + + +def test_mcp_forge_run_logs_the_failure_for_the_server(tmp_path, monkeypatch, caplog, _agent_env): + """MCP ``forge_run`` discards forge's console output, so it logs the failure itself. + + Forge marks its own record of a typed failure as shown on the console, and + the server's console handler drops it: without this line the server log has + nothing on a failed run. + """ + from fluid_build.cli import forge as forge_mod + from fluid_build.cli import mcp as mcp_mod + from fluid_build.cli.mcp import McpPolicy, _run_forge_inproc + from fluid_build.llm.providers import CopilotGenerationError + + def _fail(args, logger): + raise CopilotGenerationError("mcp_sampling_failed", "The IDE refused the request.") + + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(forge_mod, "run_ai_copilot_mode", _fail) + monkeypatch.setattr( + mcp_mod, + "_policy", + lambda: McpPolicy( + readable_paths=(tmp_path,), + writable_paths=(tmp_path,), + writable_namespaces=("history", "audit"), + ), + ) + with caplog.at_level(logging.INFO, logger="fluid.mcp.forge_run"): + result = _run_forge_inproc("ai", str(tmp_path / "product"), "SDP", None) + + assert result["exit_code"] == 1 + assert [e["event"] for e in result["events"]][-2:] == ["forge.error", "forge.done"] + (logged,) = [ + r + for r in caplog.records + if r.name == "fluid.mcp.forge_run" and r.levelno == logging.WARNING + ] + assert logged.getMessage() == ( + "forge_run failed: mcp_sampling_failed: The IDE refused the request." + ) + assert not getattr(logged, "fluid_console_shown", False) + + +def test_agent_error_text_is_redacted(tmp_path, capsys, monkeypatch, _agent_env): + """The events go to stdout directly, past the logging redactor: they redact too.""" + from fluid_build.cli import forge as forge_mod + from fluid_build.llm.providers import CopilotGenerationError + + leaked = "sk-proj-" + "A" * 40 # pragma: allowlist secret + + def _fail(args, logger): + raise CopilotGenerationError( + "copilot_litellm_request_failed", + f"litellm request failed: Incorrect API key provided: {leaked}", + suggestions=[f"Rotate {leaked}"], + ) + + monkeypatch.setattr(forge_mod, "run_ai_copilot_mode", _fail) + args = _agent_args(["--llm-provider", "openai"], tmp_path) + forge_mod.run(args, logging.getLogger("test")) + + out = capsys.readouterr().out + assert leaked not in out + event = next(e for e in _json_lines_only(out) if e["event"] == "forge.error") + assert event["code"] == "copilot_litellm_request_failed" diff --git a/tests/cli/test_forge_mode_errors.py b/tests/cli/test_forge_mode_errors.py index 9f543120..277f2a93 100644 --- a/tests/cli/test_forge_mode_errors.py +++ b/tests/cli/test_forge_mode_errors.py @@ -350,3 +350,231 @@ def test_keyboard_interrupt_returns_130(self): result = run_ai_copilot_mode(self._args(), logger, **self._kwargs(copilot_class, console)) assert result == 130 logger.info.assert_any_call("AI Copilot cancelled by user") + + +# ── a failure is logged once, noted for --agent, described as data ──── + + +def _raised(exc: BaseException) -> BaseException: + """*exc* with a traceback, as a handler meets it.""" + try: + raise exc + except BaseException as caught: # noqa: BLE001 + return caught + + +def _handled(logger: logging.Logger, label: str, exc: BaseException, **kwargs) -> None: + """Call ``log_forge_failure`` from the ``except`` block, as handlers do.""" + from fluid_build.cli._forge_mode_errors import log_forge_failure + + try: + raise exc + except BaseException as caught: # noqa: BLE001 + log_forge_failure(logger, label, caught, **kwargs) + + +class TestLogForgeFailure: + def test_a_fluid_error_has_no_traceback_above_debug(self, caplog): + logger = logging.getLogger("test.forge.failure") + exc = CopilotGenerationError("copilot_no_llm_provider", "No LLM provider configured.") + with caplog.at_level(logging.DEBUG, logger=logger.name): + _handled(logger, "AI Copilot minimal flow failed", exc) + + error, debug = caplog.records + assert (error.levelno, error.getMessage()) == ( + logging.ERROR, + "AI Copilot minimal flow failed: copilot_no_llm_provider", + ) + assert error.exc_info is None + # The caller prints the message; the record is for --log-file only. + assert error.fluid_console_shown is True + assert debug.levelno == logging.DEBUG and debug.exc_info[1] is exc + + def test_a_fluid_error_nobody_printed_stays_on_the_console(self, caplog): + logger = logging.getLogger("test.forge.failure") + exc = CopilotGenerationError("copilot_x", "x") + with caplog.at_level(logging.DEBUG, logger=logger.name): + _handled(logger, "Agent loop failed", exc, shown=False) + assert caplog.records[0].fluid_console_shown is False + + def test_a_crash_keeps_its_traceback(self, caplog): + logger = logging.getLogger("test.forge.failure") + with caplog.at_level(logging.DEBUG, logger=logger.name): + _handled(logger, "AI Copilot minimal flow crashed", KeyError("boom")) + + (record,) = caplog.records + assert record.levelno == logging.ERROR + assert isinstance(record.exc_info[1], KeyError) + + def test_the_failure_is_noted_for_the_run(self): + from fluid_build.cli._forge_mode_errors import take_forge_failure + + exc = ValueError("boom") + _handled(MagicMock(), "X failed", exc) + assert take_forge_failure() is exc + assert take_forge_failure() is None # read once + + def test_the_boundary_logs_a_fluid_error_without_a_traceback(self): + from fluid_build.cli._forge_mode_errors import take_forge_failure + + logger = MagicMock() + exc = CopilotGenerationError("internal_error_code", "friendly message") + + def handler() -> int: + with forge_mode_error_boundary( + logger, MagicMock(), fail_label="X failed", render_error=lambda c, e: None + ) as boundary: + raise exc + return boundary.exit_code + + assert handler() == 1 + logger.exception.assert_not_called() + logger.error.assert_called_once_with( + "%s: %s", "X failed", "internal_error_code", extra={"fluid_console_shown": True} + ) + assert take_forge_failure() is exc + + +class TestDescribeForgeFailure: + def _describe(self, exc): + from fluid_build.cli._forge_mode_errors import describe_forge_failure + + return describe_forge_failure(_raised(exc)) + + def test_a_copilot_error_carries_its_code_message_and_next_steps(self): + fields = self._describe( + CopilotGenerationError("copilot_no_llm_provider", "No provider.", suggestions=["a"]) + ) + assert fields["code"] == "copilot_no_llm_provider" + assert fields["message"] == "No provider." + assert fields["suggestions"] == ["a"] + assert fields["docs_url"].startswith("https://") + assert "exception" not in fields + + def test_a_missing_extra_names_its_install_line(self): + from fluid_build._optional import MissingOptionalDependency + + fields = self._describe(MissingOptionalDependency("mcp", "mcp", "fluid mcp serve")) + assert fields == { + "code": "mcp_sdk_unavailable", + "message": "fluid mcp serve needs mcp, which is not installed.", + "suggestions": ["pip install 'data-product-forge[mcp]'"], + } + + def test_a_fluid_error_gets_a_snake_case_code(self): + from fluid_build.errors import ConfigurationError + + fields = self._describe(ConfigurationError("bad config", suggestions=["fix it"])) + assert (fields["code"], fields["message"], fields["suggestions"]) == ( + "configuration_error", + "bad config", + ["fix it"], + ) + + def test_a_typed_user_error_maps_what_fix_and_doc(self): + from fluid_build._errors import FluidUserError + + fields = self._describe( + FluidUserError(what="w", why="y", fix="f", doc="https://d", code="ERR_X") + ) + assert fields == { + "code": "ERR_X", + "message": "w", + "suggestions": ["f"], + "docs_url": "https://d", + } + + def test_a_crash_is_named_by_its_class_never_its_text(self): + fields = self._describe(OSError("/home/alice/.ssh/id_rsa: permission denied")) + assert fields["code"] == "forge_unexpected_error" + assert fields["exception"] == "OSError" + assert "alice" not in repr(fields) + + def test_secrets_in_the_text_are_redacted(self): + leaked = "ghp_" + "a" * 36 # pragma: allowlist secret + fields = self._describe( + CopilotGenerationError("c", f"token {leaked} refused", suggestions=[f"drop {leaked}"]) + ) + assert leaked not in repr(fields) + + +# ── the handlers that swallow a failure note it ─────────────────────── + + +class TestHandlersNoteTheirFailure: + def test_the_agent_loop_notes_a_typed_failure_without_a_traceback( + self, tmp_path, caplog, monkeypatch + ): + from fluid_build.cli._forge_mode_errors import take_forge_failure + from fluid_build.cli._template_mode import _create_project_agent_loop + + error = CopilotGenerationError("copilot_no_llm_provider", "No LLM provider configured.") + + def _resolve(*args, **kwargs): + raise error + + monkeypatch.setattr( + "fluid_build.cli.forge_copilot_llm_providers.resolve_llm_config", _resolve + ) + logger = logging.getLogger("test.forge.agent_loop") + with caplog.at_level(logging.DEBUG, logger=logger.name): + ok = _create_project_agent_loop( + target_dir=tmp_path, + context={}, + copilot_options={}, + copilot=None, + dry_run=False, + logger=logger, + console=None, + ) + + assert ok is False + assert take_forge_failure() is error + assert not [r for r in caplog.records if r.exc_info and r.levelno > logging.DEBUG] + # Nothing printed it (no console), so its log line stays on the console. + assert caplog.records[0].fluid_console_shown is False + + @pytest.mark.parametrize( + "exc", + [CopilotGenerationError("copilot_x", "typed"), OSError("disk full")], + ids=["typed", "crash"], + ) + def test_template_mode_notes_its_failure(self, exc): + from fluid_build.cli._forge_mode_errors import take_forge_failure + from fluid_build.cli._template_mode import run_template_mode + + def _target(args, default_name): + raise exc + + args = argparse.Namespace(non_interactive=True, template="starter", provider=None) + rc = run_template_mode( + args, MagicMock(), get_target_directory_fn=_target, console_factory=None + ) + + assert rc == 1 + assert take_forge_failure() is exc + + +def test_forge_prints_a_typed_user_errors_fix(tmp_path, capsys, monkeypatch): + """``_run_main`` prints what the ``--agent`` event carries, a FluidUserError's fix too.""" + from fluid_build._errors import FluidUserError + from fluid_build.cli import forge as forge_mod + + def _fail(args, logger): + raise FluidUserError(what="The target is locked.", why="w", fix="Unlock it first.", doc="") + + monkeypatch.chdir(tmp_path) + for var in ("FLUID_FORGE_NO_PICKER", "FLUID_FORGE_NO_WELCOME", "FLUID_FORGE_NO_PREVIEW"): + monkeypatch.setenv(var, "1") + monkeypatch.setattr(forge_mod, "run_ai_copilot_mode", _fail) + parser = argparse.ArgumentParser() + forge_mod.register(parser.add_subparsers()) + args = parser.parse_args( + ["forge", "--non-interactive", "--llm-provider", "openai", "-d", str(tmp_path / "p")] + ) + + assert forge_mod.run(args, logging.getLogger("test")) == 1 + + printed = capsys.readouterr() + assert "Forge failed: The target is locked." in printed.out + assert "Unlock it first." in printed.out From f2d88375eeef84ac56ced3ebb55d1dbb0a18ae88 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:56:27 +0100 Subject: [PATCH 3/9] fix(forge): `--agent --blank` reports its contract once Blank mode emitted its own forge.contract_written and the receipt emitted another for the same file, first by its absolute path and then by its path from the current directory. Since #852 the receipt scans every --target-dir, so blank mode's own event goes, as in every other mode. --- CHANGELOG.md | 4 ++++ fluid_build/cli/forge.py | 17 +++-------------- tests/cli/test_forge_agent_mode.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5eb1de7..5d5fb29a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,6 +163,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 "Give Feedback / Get Help" banner to stdout ahead of the JSON. Forge now sets litellm's own switch, `suppress_debug_info`, wherever it loads litellm (AI calls, `fluid ai setup`'s key check, the agent loop), so no command prints the banner. The error is reported as before. +- **`fluid forge --agent --blank` reports its contract once.** Blank mode sent its own + `forge.contract_written` and the receipt sent another, so the stream named one file twice, + first by its absolute path and then by its path from the current directory. Only the receipt's + event is left, as in every other mode. ### Security diff --git a/fluid_build/cli/forge.py b/fluid_build/cli/forge.py index df6b5fd3..7c217d2a 100644 --- a/fluid_build/cli/forge.py +++ b/fluid_build/cli/forge.py @@ -1148,20 +1148,9 @@ def _run_blank_mode(args: Any, logger: logging.Logger) -> int: if not result_path: return 1 - # Agent-mode JSONL: blank-mode bypasses ``_write_forge_receipt`` (no LLM - # cost to record), so we emit ``forge.contract_written`` directly here - # to keep the event stream contract consistent. - if bool(get_cli_arg(args, "agent", False)): - try: - size = result_path.stat().st_size - except OSError: - size = -1 - _emit_agent_jsonl( - "forge.contract_written", - path=str(result_path), - action="created", - size=size, - ) + # No ``forge.contract_written`` here: ``_run_main`` writes the receipt after + # this returns, and the receipt emits one event per file written, for every + # mode (``_write_forge_receipt``). # In --agent mode we skip the interactive CI-pipeline prompt entirely. # The agent can shell-run ``fluid scaffold-ci --system `` separately diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index bcc149eb..9cc8181e 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -868,3 +868,32 @@ def _fail(args, logger): assert leaked not in out event = next(e for e in _json_lines_only(out) if e["event"] == "forge.error") assert event["code"] == "copilot_litellm_request_failed" + + +# --------------------------------------------------------------------------- +# 9. One contract written, one forge.contract_written event +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("inside_cwd", [True, False], ids=["under-cwd", "outside-cwd"]) +def test_agent_blank_reports_its_contract_once( + inside_cwd, tmp_path, capsys, monkeypatch, _agent_env +): + """Blank mode used to send its own event as well as the receipt's: two for one file.""" + from fluid_build.cli import forge as forge_mod + + work = tmp_path / "work" + work.mkdir() + monkeypatch.chdir(work) + product = (work if inside_cwd else tmp_path) / "product" + args = _agent_args(["--blank", "--data-product-type", "SDP"], tmp_path) + args.target_dir = str(product) + + assert forge_mod.run(args, logging.getLogger("test")) == 0 + + events = _json_lines_only(capsys.readouterr().out) + written = [(e["path"], e["action"]) for e in events if e["event"] == "forge.contract_written"] + contract = product / "contract.fluid.yaml" + path = "product/contract.fluid.yaml" if inside_cwd else str(contract.resolve()) + assert written == [(path, "created")] + assert "forge.error" not in [e["event"] for e in events] From f97573454c201966a65eacb5b3c9ccad788b0575 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:42:08 +0100 Subject: [PATCH 4/9] fix(forge): a failed `--seed-from` names what it accepts, not a missing --verbose The console line ended "(run with --verbose for details)", and the new forge_seed_failed event repeated it as a suggestion, but forge has no --verbose flag (exit 2). Both now say what load_seed accepts: a *.odps.yaml or *.odcs.yaml file, or a directory of them. --- fluid_build/cli/forge_modes.py | 10 ++++++---- tests/cli/test_forge_agent_mode.py | 4 ++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/fluid_build/cli/forge_modes.py b/fluid_build/cli/forge_modes.py index 5f91f540..00224606 100644 --- a/fluid_build/cli/forge_modes.py +++ b/fluid_build/cli/forge_modes.py @@ -1269,13 +1269,15 @@ def run_ai_copilot_mode( }, ) failed = f"--seed-from failed for {_seed_from!s}: {type(_seed_exc).__name__}" - console_error(f"{failed} (run with --verbose for details)") + # What load_seed accepts; there is no --verbose to point at. + accepts = ( + "--seed-from takes a *.odps.yaml or *.odcs.yaml file, or a directory of them" + ) + console_error(f"{failed} ({accepts})") # ``--agent``'s forge.error gets the same words, not the exception: # a typed one (ProviderError) would carry its text into the event. note_forge_failure( - CopilotGenerationError( - "forge_seed_failed", failed, suggestions=["Run with --verbose for details"] - ) + CopilotGenerationError("forge_seed_failed", failed, suggestions=[accepts]) ) return 1 diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index 9cc8181e..bef738d8 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -801,6 +801,10 @@ def test_agent_seed_failure_is_a_forge_error_without_the_exception_text( error = events[1] assert error["code"] == "forge_seed_failed" assert error["message"].startswith(f"--seed-from failed for {missing}: ") + # A next step that exists: forge has no --verbose flag. + assert error["suggestions"] == [ + "--seed-from takes a *.odps.yaml or *.odcs.yaml file, or a directory of them" + ] assert "exception" not in error From 4929054ddd386675c9f8759cac7d3320397593ec Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:43:31 +0100 Subject: [PATCH 5/9] fix(mcp): log a forge_run failure on one line _log_forge_errors wrote the forge.error message with %s, so a newline in it could add a made-up line to the server log. %r keeps it on one line (security re-check, LOW). --- fluid_build/cli/mcp/dispatch.py | 3 ++- tests/cli/test_forge_agent_mode.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fluid_build/cli/mcp/dispatch.py b/fluid_build/cli/mcp/dispatch.py index 3d251b86..eaf6fdfb 100644 --- a/fluid_build/cli/mcp/dispatch.py +++ b/fluid_build/cli/mcp/dispatch.py @@ -1210,7 +1210,8 @@ def _log_forge_errors(events: List[Dict[str, Any]], logger: logging.Logger) -> N """ for event in events: if event["event"] == "forge.error": - logger.warning("forge_run failed: %s: %s", event.get("code"), event.get("message")) + # ``%r``: a newline in the message stays one log line, not a forged second. + logger.warning("forge_run failed: %s: %r", event.get("code"), event.get("message")) def _run_forge_inproc( diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index bef738d8..99aa9dca 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -845,7 +845,7 @@ def _fail(args, logger): if r.name == "fluid.mcp.forge_run" and r.levelno == logging.WARNING ] assert logged.getMessage() == ( - "forge_run failed: mcp_sampling_failed: The IDE refused the request." + "forge_run failed: mcp_sampling_failed: 'The IDE refused the request.'" ) assert not getattr(logged, "fluid_console_shown", False) From a6ac2f4ed440ad171c913d07a6b0ccb551647c67 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:44:53 +0100 Subject: [PATCH 6/9] fix(forge): `--agent` help lists the events the stream sends The help named forge.start, forge.done, forge.contract_written and forge.cost. forge.cost is never sent, and forge.error (new), forge.file_written and forge.plan were missing (delivery review, P2). --- fluid_build/cli/forge.py | 5 +++-- tests/cli/test_forge_agent_mode.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/fluid_build/cli/forge.py b/fluid_build/cli/forge.py index 7c217d2a..14de12c6 100644 --- a/fluid_build/cli/forge.py +++ b/fluid_build/cli/forge.py @@ -567,8 +567,9 @@ def register(subparsers: argparse._SubParsersAction): help=( "Headless agent-drivable preset. Bundles --yes + all " "FLUID_FORGE_NO_{PICKER,PREVIEW,WELCOME,STREAMING_PREVIEW}=1 + " - "emits JSON-Lines progress events (forge.start, forge.done, " - "forge.contract_written, forge.cost) to stdout so the IDE's " + "emits JSON-Lines events (forge.start, forge.contract_written, " + "forge.file_written, forge.plan with --emit-plan, forge.error on " + "a failure, forge.done) to stdout so the IDE's " "shell tool can parse them. Any of --llm-provider, --llm-model, " "--llm-endpoint or --context selects AI mode (strictly " "non-interactive; a failed generation exits non-zero). With no " diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index 99aa9dca..88901c6a 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -337,6 +337,25 @@ def test_agent_flag_registered_in_argparse(): assert getattr(args, "blank", False) is True +def test_agent_flag_help_names_the_events_the_stream_sends(): + """``--agent``'s help lists the stream's events: forge.error too, never a phantom one.""" + import argparse + + from fluid_build.cli import forge as forge_mod + + parser = argparse.ArgumentParser() + sp = parser.add_subparsers() + forge_mod.register(sp) + forge_parser = sp.choices["forge"] + (agent,) = [a for a in forge_parser._actions if "--agent" in a.option_strings] + + for event in ("forge.start", "forge.contract_written", "forge.file_written"): + assert event in agent.help + for event in ("forge.plan", "forge.error", "forge.done"): + assert event in agent.help + assert "forge.cost" not in agent.help # listed once, never sent + + # --------------------------------------------------------------------------- # 6. Explicit AI flags under --agent select AI mode (never a silent blank) # --------------------------------------------------------------------------- From 51847c35e29630b2c2dcba356d011a21e90853f5 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:49:38 +0100 Subject: [PATCH 7/9] fix(ai): the agent loop's summarizer quiets litellm too The summarizer imports litellm itself and calls litellm.completion, so it gets quiet_litellm like the other three callers (defect scan, Low). TestLogForgeFailure also drops the note it leaves, with the traceback frames it holds, after each test. --- fluid_build/cli/forge_copilot_default_summarizer.py | 3 +++ fluid_build/llm/litellm_backend.py | 10 ++++++---- tests/cli/test_forge_mode_errors.py | 8 ++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/fluid_build/cli/forge_copilot_default_summarizer.py b/fluid_build/cli/forge_copilot_default_summarizer.py index 64388368..a87d4c37 100644 --- a/fluid_build/cli/forge_copilot_default_summarizer.py +++ b/fluid_build/cli/forge_copilot_default_summarizer.py @@ -121,8 +121,11 @@ def summarize(blob: str) -> str: from fluid_build.cli.forge_copilot_llm_litellm import ( _LITELLM_PREFIX_BY_PROVIDER, + quiet_litellm, ) + quiet_litellm(litellm) + prefix = _LITELLM_PREFIX_BY_PROVIDER.get( routing_config.provider.lower(), routing_config.provider.lower() ) diff --git a/fluid_build/llm/litellm_backend.py b/fluid_build/llm/litellm_backend.py index 0894753d..2aa2af62 100644 --- a/fluid_build/llm/litellm_backend.py +++ b/fluid_build/llm/litellm_backend.py @@ -115,10 +115,12 @@ def quiet_litellm(litellm: Any) -> Any: exception, which forge reports, and the provider-list link is in its text. What goes is litellm's request for bug reports and its debug-mode hint. - Forge loads litellm here, in ``ai_setup._validate_api_key`` and in the - agent loop's tool call; each calls this, so whichever runs first sets it. - The fallback router (``fluid_build.llm.router``) builds a ``litellm.Router``, - which sets it too. + Every forge call into ``litellm.completion`` loads litellm here, in + ``ai_setup._validate_api_key``, in the agent loop's tool call or in the + agent loop's summarizer, and each calls this, so whichever runs first sets + it. The fallback router (``fluid_build.llm.router``) builds a + ``litellm.Router``, which sets it too. The cost and token-budget helpers + only read litellm's tables, after an AI call has loaded it here. """ litellm.suppress_debug_info = True return litellm diff --git a/tests/cli/test_forge_mode_errors.py b/tests/cli/test_forge_mode_errors.py index 277f2a93..59135bad 100644 --- a/tests/cli/test_forge_mode_errors.py +++ b/tests/cli/test_forge_mode_errors.py @@ -374,6 +374,14 @@ def _handled(logger: logging.Logger, label: str, exc: BaseException, **kwargs) - class TestLogForgeFailure: + @pytest.fixture(autouse=True) + def _forget_the_note(self): + """Drop what a test noted, and the traceback frames it holds, after the test.""" + from fluid_build.cli._forge_mode_errors import take_forge_failure + + yield + take_forge_failure() + def test_a_fluid_error_has_no_traceback_above_debug(self, caplog): logger = logging.getLogger("test.forge.failure") exc = CopilotGenerationError("copilot_no_llm_provider", "No LLM provider configured.") From 926d98203b180bc5057f575e16fc934f9067a8de Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:57:38 +0100 Subject: [PATCH 8/9] fix(forge): the `--seed-from` hint names every input load_seed takes It named only *.odps.yaml / *.odcs.yaml files and directories; load_seed also takes .yml / .json names and any file with kind: DataProduct or DataContract (fix re-check, P3). --- fluid_build/cli/forge_modes.py | 3 ++- tests/cli/test_forge_agent_mode.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fluid_build/cli/forge_modes.py b/fluid_build/cli/forge_modes.py index 00224606..a5b4fc25 100644 --- a/fluid_build/cli/forge_modes.py +++ b/fluid_build/cli/forge_modes.py @@ -1271,7 +1271,8 @@ def run_ai_copilot_mode( failed = f"--seed-from failed for {_seed_from!s}: {type(_seed_exc).__name__}" # What load_seed accepts; there is no --verbose to point at. accepts = ( - "--seed-from takes a *.odps.yaml or *.odcs.yaml file, or a directory of them" + "--seed-from takes an ODPS or ODCS file (*.odps.yaml, *.odcs.yaml, or " + "one with kind: DataProduct or DataContract), or a directory of them" ) console_error(f"{failed} ({accepts})") # ``--agent``'s forge.error gets the same words, not the exception: diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index 88901c6a..1a5a93ba 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -822,7 +822,8 @@ def test_agent_seed_failure_is_a_forge_error_without_the_exception_text( assert error["message"].startswith(f"--seed-from failed for {missing}: ") # A next step that exists: forge has no --verbose flag. assert error["suggestions"] == [ - "--seed-from takes a *.odps.yaml or *.odcs.yaml file, or a directory of them" + "--seed-from takes an ODPS or ODCS file (*.odps.yaml, *.odcs.yaml, or one with " + "kind: DataProduct or DataContract), or a directory of them" ] assert "exception" not in error From b9009805e9b345d39da7a5917548f45a38061bc9 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 21:26:50 +0100 Subject: [PATCH 9/9] test: address the code-quality review on #858 The failure helpers catch Exception, not BaseException; the boundary test raises through a helper call inside the with, so the line after it is reachable; tests/test_litellm_banner.py imports fluid_build.cli.ai_setup one way. --- tests/cli/test_forge_mode_errors.py | 25 ++++++++++++++----------- tests/test_litellm_banner.py | 4 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/cli/test_forge_mode_errors.py b/tests/cli/test_forge_mode_errors.py index 59135bad..c52099a2 100644 --- a/tests/cli/test_forge_mode_errors.py +++ b/tests/cli/test_forge_mode_errors.py @@ -355,21 +355,26 @@ def test_keyboard_interrupt_returns_130(self): # ── a failure is logged once, noted for --agent, described as data ──── -def _raised(exc: BaseException) -> BaseException: +def _raised(exc: Exception) -> Exception: """*exc* with a traceback, as a handler meets it.""" try: raise exc - except BaseException as caught: # noqa: BLE001 + except Exception as caught: # noqa: BLE001 return caught -def _handled(logger: logging.Logger, label: str, exc: BaseException, **kwargs) -> None: +def _fail(exc: Exception) -> None: + """Raise *exc*; called inside a ``with`` so the code after the block stays reachable.""" + raise exc + + +def _handled(logger: logging.Logger, label: str, exc: Exception, **kwargs) -> None: """Call ``log_forge_failure`` from the ``except`` block, as handlers do.""" from fluid_build.cli._forge_mode_errors import log_forge_failure try: raise exc - except BaseException as caught: # noqa: BLE001 + except Exception as caught: # noqa: BLE001 log_forge_failure(logger, label, caught, **kwargs) @@ -428,14 +433,12 @@ def test_the_boundary_logs_a_fluid_error_without_a_traceback(self): logger = MagicMock() exc = CopilotGenerationError("internal_error_code", "friendly message") - def handler() -> int: - with forge_mode_error_boundary( - logger, MagicMock(), fail_label="X failed", render_error=lambda c, e: None - ) as boundary: - raise exc - return boundary.exit_code + with forge_mode_error_boundary( + logger, MagicMock(), fail_label="X failed", render_error=lambda c, e: None + ) as boundary: + _fail(exc) - assert handler() == 1 + assert boundary.exit_code == 1 logger.exception.assert_not_called() logger.error.assert_called_once_with( "%s: %s", "X failed", "internal_error_code", extra={"fluid_console_shown": True} diff --git a/tests/test_litellm_banner.py b/tests/test_litellm_banner.py index 515dda16..e9b009c5 100644 --- a/tests/test_litellm_banner.py +++ b/tests/test_litellm_banner.py @@ -65,7 +65,7 @@ def test_ai_test_json_stdout_is_the_report_alone_when_the_call_fails( litellm, failing_completion, monkeypatch, capsys, tmp_path ): """``fluid ai test --json`` on a failed call: one JSON document, nothing before it.""" - from fluid_build.cli.ai_setup import run_ai_test + import fluid_build.cli.ai_setup as ai_setup monkeypatch.chdir(tmp_path) for name in ("ANTHROPIC_API_KEY", "GEMINI_API_KEY", "GOOGLE_API_KEY", "FLUID_LLM_API_KEY"): @@ -85,7 +85,7 @@ def test_ai_test_json_stdout_is_the_report_alone_when_the_call_fails( return_value=("unavailable", None), ), ): - code, report = run_ai_test(None, args) + code, report = ai_setup.run_ai_test(None, args) assert code != 0 assert report["error"]["code"] == "copilot_litellm_request_failed"