fix(dbt_cli, codegen): use returncode for success signal so stderr noise can't mask result (#772)

## Summary

**Alternative to #750** — addresses the same
urllib3-warning-bleed-through bug, but preserves stderr information on
failures rather than discarding it entirely.

### The bug

When dbt is invoked with `--quiet` (as the MCP does for
build/run/test/etc.), stdout is empty on success. Previously
`stderr=subprocess.STDOUT` merged stderr into stdout, so unrelated
stderr noise — e.g. urllib3 deprecation warnings emitted under
externalbrowser auth — would fill `output` and be returned verbatim to
the LLM in place of `"OK"`, confusing the model into thinking the
command had failed.

### The fix

Capture stdout and stderr separately, and use `process.returncode` as
the success signal:

- **On success (`returncode == 0`)**: return stdout (often empty under
`--quiet`) or `"OK"`. Stderr is dropped — it's typically just noise when
the command actually succeeded.
- **On failure (`returncode != 0`)**: surface both stdout and stderr in
the returned message. Some dbt errors (auth, connection issues,
tracebacks) only appear on stderr, so the LLM still gets full diagnostic
context.

Applied identically to `_run_dbt_command`
(`src/dbt_mcp/dbt_cli/tools.py`) and `_run_codegen_operation`
(`src/dbt_mcp/dbt_codegen/tools.py`).

### Why an alternative to #750?

[#750](#750) takes the simpler
approach of switching `stderr` from `STDOUT` to `PIPE` and discarding
it. @DevonFulcher [pointed
out](#750 (review))
that some stderr content is useful — e.g. error messages that only
appear there — and silently dropping it could hide real problems. This
PR addresses that concern by keeping stderr available exactly where it's
diagnostic (the failure path), while still fixing the original bug.

| | #750 | This PR |
|---|---|---|
| Fixes urllib3 warnings on success | ✅ | ✅ |
| Preserves stderr diagnostics on failure | ❌ (always discarded) | ✅ |
| Decision signal | "is stdout non-empty" | `returncode == 0` |

## Test plan
- [x] `uv run pytest tests/unit/dbt_cli/test_tools.py
tests/unit/dbt_codegen/test_tools.py -v` — 63 pass, including 3 new
tests for dbt_cli (success with stderr noise, failure with stderr-only
error, failure with both streams) and 2 new for codegen (success drops
stderr noise, failure surfaces stderr)
- [x] `task check` — ruff, mypy, doc generation
- [x] `uv run pytest tests/ --ignore=tests/integration` — full unit
suite green
- [ ] Manual verification: run a real `build`/`run` against a project
under externalbrowser auth and confirm `"OK"` is returned instead of
urllib3 warning text

Author: b-per

.changes/unreleased/Bug Fix-20260512-162150.yaml

@@ -0,0 +1,3 @@
+kind: Bug Fix
+body: 'list/run/test/build/codegen tools no longer return stderr noise (e.g. urllib3 deprecation warnings under externalbrowser auth) in place of ''OK'' on success. The fix uses process.returncode as the success signal: on success only stdout is returned, on failure stderr is surfaced alongside stdout so error diagnostics aren''t lost.'
+time: 2026-05-12T16:21:50.978253+02:00

src/dbt_mcp/dbt_cli/tools.py

@@ -135,12 +135,25 @@ def _run_dbt_command(
                 args=args,
                 cwd=cwd_path,
                 stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT,
+                stderr=subprocess.PIPE,
                 stdin=subprocess.DEVNULL,
                 text=True,
             )
-            output, _ = process.communicate(timeout=config.dbt_cli_timeout)
-            return output or "OK"
+            stdout, stderr = process.communicate(timeout=config.dbt_cli_timeout)
+            # Use returncode as the success signal so noise on stderr (e.g.
+            # urllib3 deprecation warnings under externalbrowser auth) can't
+            # masquerade as the result of a successful command. On failure
+            # we surface stderr too, since some dbt errors only appear there.
+            if process.returncode == 0:
+                return stdout or "OK"
+            parts = []
+            if stdout:
+                parts.append(f"--- stdout ---\n{stdout.rstrip()}")
+            if stderr:
+                parts.append(f"--- stderr ---\n{stderr.rstrip()}")
+            if parts:
+                return "\n".join(parts)
+            return f"Command failed with exit code {process.returncode} (no output)"
         except subprocess.TimeoutExpired:
             return "Timeout: dbt command took too long to complete." + (
                 " Try using a specific selector to narrow down the results."

src/dbt_mcp/dbt_codegen/tools.py

@@ -53,19 +53,32 @@ def _run_codegen_operation(
                 args=args_list,
                 cwd=cwd_path,
                 stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT,
+                stderr=subprocess.PIPE,
                 stdin=subprocess.DEVNULL,
                 text=True,
             )
-            output, _ = process.communicate(timeout=config.dbt_cli_timeout)
+            stdout, stderr = process.communicate(timeout=config.dbt_cli_timeout)
 
-            # Return the output directly or handle errors
+            # Use returncode as the success signal so noise on stderr (e.g.
+            # urllib3 deprecation warnings under externalbrowser auth) can't
+            # masquerade as the result of a successful command. On failure
+            # we surface stderr too, since some dbt errors only appear there.
             if process.returncode != 0:
-                if "dbt found" in output and "resource" in output:
-                    return f"Error: dbt-codegen package may not be installed. Run 'dbt deps' to install it.\n{output}"
-                return f"Error running dbt-codegen macro: {output}"
-
-            return output or "OK"
+                parts = []
+                if stdout:
+                    parts.append(f"--- stdout ---\n{stdout.rstrip()}")
+                if stderr:
+                    parts.append(f"--- stderr ---\n{stderr.rstrip()}")
+                combined = "\n".join(parts)
+                detail = combined or f"exit code {process.returncode} (no output)"
+                if "dbt found" in combined and "resource" in combined:
+                    return (
+                        "Error: dbt-codegen package may not be installed. "
+                        f"Run 'dbt deps' to install it.\n{detail}"
+                    )
+                return f"Error running dbt-codegen macro: {detail}"
+
+            return stdout or "OK"
 
         except subprocess.TimeoutExpired:
             return f"Timeout: dbt-codegen operation took longer than {config.dbt_cli_timeout} seconds."

tests/unit/dbt_cli/test_cli_integration.py

@@ -15,6 +15,7 @@ def test_dbt_command_execution(self, mock_popen):
         """
         # Mock setup
         mock_process = MagicMock()
+        mock_process.returncode = 0
         mock_process.communicate.return_value = ("command output", None)
         mock_popen.return_value = mock_process
 

tests/unit/dbt_cli/test_manifest_encoding.py

@@ -10,8 +10,10 @@
 @pytest.fixture
 def mock_process():
     class MockProcess:
+        returncode = 0
+
         def communicate(self, timeout=None):
-            return "command output", None
+            return "command output", ""
 
     return MockProcess()
 

tests/unit/dbt_cli/test_manifest_non_ascii.py

@@ -11,8 +11,10 @@
 @pytest.fixture
 def mock_process():
     class MockProcess:
+        returncode = 0
+
         def communicate(self, timeout=None):
-            return "command output", None
+            return "command output", ""
 
     return MockProcess()
 

tests/unit/dbt_cli/test_tools.py

@@ -14,8 +14,10 @@
 @pytest.fixture
 def mock_process():
     class MockProcess:
+        returncode = 0
+
         def communicate(self, timeout=None):
-            return "command output", None
+            return "command output", ""
 
     return MockProcess()
 
@@ -515,6 +517,135 @@ def mock_popen(args, **kwargs):
     assert "--selector" not in args_list
 
 
+def test_success_returns_ok_even_with_stderr_noise(
+    monkeypatch: MonkeyPatch, mock_fastmcp
+):
+    """A successful command (returncode=0) returns 'OK' even when stderr has
+    noise like urllib3 deprecation warnings — stderr is dropped on success."""
+
+    class MockProcessSuccessWithStderrWarning:
+        returncode = 0
+
+        def communicate(self, timeout=None):
+            return "", "urllib3 v2.0 only supports OpenSSL 1.1.1+: NotOpenSSLWarning"
+
+    def mock_popen(args, **kwargs):
+        return MockProcessSuccessWithStderrWarning()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, tools = mock_fastmcp
+    register_dbt_cli_tools(
+        fastmcp,
+        mock_dbt_cli_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+
+    assert tools["build"]() == "OK"
+
+
+def test_failure_surfaces_stderr_when_stdout_is_empty(
+    monkeypatch: MonkeyPatch, mock_fastmcp
+):
+    """A failed command (returncode!=0) surfaces stderr — some dbt errors
+    only appear there, e.g. authentication or connection problems."""
+
+    class MockProcessFailureStderrOnly:
+        returncode = 1
+
+        def communicate(self, timeout=None):
+            return "", "Database Error: could not connect to server"
+
+    def mock_popen(args, **kwargs):
+        return MockProcessFailureStderrOnly()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, tools = mock_fastmcp
+    register_dbt_cli_tools(
+        fastmcp,
+        mock_dbt_cli_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+
+    result = tools["build"]()
+
+    assert "Database Error" in result
+    assert result != "OK"
+
+
+def test_failure_with_no_output_surfaces_exit_code(
+    monkeypatch: MonkeyPatch, mock_fastmcp
+):
+    """A failed command with empty stdout AND stderr must NOT return 'OK' —
+    surface the exit code so the LLM can tell the call actually failed."""
+
+    class MockProcessFailureNoOutput:
+        returncode = 137  # e.g. OOM-killed
+
+        def communicate(self, timeout=None):
+            return "", ""
+
+    def mock_popen(args, **kwargs):
+        return MockProcessFailureNoOutput()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, tools = mock_fastmcp
+    register_dbt_cli_tools(
+        fastmcp,
+        mock_dbt_cli_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+
+    result = tools["build"]()
+
+    assert result != "OK"
+    assert "137" in result
+    assert "exit code" in result.lower()
+
+
+def test_failure_combines_stdout_and_stderr(monkeypatch: MonkeyPatch, mock_fastmcp):
+    """When both streams have content on failure, both are surfaced."""
+
+    class MockProcessFailureBothStreams:
+        returncode = 1
+
+        def communicate(self, timeout=None):
+            return "Compilation Error in model my_model", "Stack trace here"
+
+    def mock_popen(args, **kwargs):
+        return MockProcessFailureBothStreams()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, tools = mock_fastmcp
+    register_dbt_cli_tools(
+        fastmcp,
+        mock_dbt_cli_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+
+    result = tools["build"]()
+
+    assert "--- stdout ---" in result
+    assert "Compilation Error" in result
+    assert "--- stderr ---" in result
+    assert "Stack trace" in result
+
+
 def test_clone_command_binary_state_path_logic(
     monkeypatch: MonkeyPatch,
     mock_process,

tests/unit/dbt_codegen/test_tools.py

@@ -285,6 +285,126 @@ def mock_popen(args, **kwargs):
     assert "Some other error occurred" in result
 
 
+def test_codegen_success_drops_stderr_noise(monkeypatch: MonkeyPatch, mock_fastmcp):
+    """A successful codegen run returns just stdout, ignoring stderr noise
+    (e.g. urllib3 deprecation warnings under externalbrowser auth)."""
+
+    class MockProcessSuccessWithStderrWarning:
+        returncode = 0
+
+        def communicate(self, timeout=None):
+            return (
+                "models:\n  - name: my_model",
+                "urllib3 v2.0 only supports OpenSSL 1.1.1+: NotOpenSSLWarning",
+            )
+
+    def mock_popen(args, **kwargs):
+        return MockProcessSuccessWithStderrWarning()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, _ = mock_fastmcp
+    register_dbt_codegen_tools(
+        fastmcp,
+        mock_dbt_codegen_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+    generate_source_tool = fastmcp.tools["generate_source"]
+
+    result = generate_source_tool(
+        schema_name="test_schema",
+        database_name=None,
+        table_names=None,
+        generate_columns=False,
+        include_descriptions=False,
+    )
+
+    # Stdout is preserved, stderr warning is dropped.
+    assert "my_model" in result
+    assert "urllib3" not in result
+
+
+def test_codegen_failure_surfaces_stderr(monkeypatch: MonkeyPatch, mock_fastmcp):
+    """A failed codegen run surfaces stderr in the error message even when
+    stdout is empty — some dbt errors only appear on stderr."""
+
+    class MockProcessFailureStderrOnly:
+        returncode = 1
+
+        def communicate(self, timeout=None):
+            return "", "Database Error: relation does not exist"
+
+    def mock_popen(args, **kwargs):
+        return MockProcessFailureStderrOnly()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, _ = mock_fastmcp
+    register_dbt_codegen_tools(
+        fastmcp,
+        mock_dbt_codegen_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+    generate_source_tool = fastmcp.tools["generate_source"]
+
+    result = generate_source_tool(
+        schema_name="test_schema",
+        database_name=None,
+        table_names=None,
+        generate_columns=False,
+        include_descriptions=False,
+    )
+
+    assert "Error running dbt-codegen macro" in result
+    assert "Database Error" in result
+
+
+def test_codegen_failure_with_no_output_surfaces_exit_code(
+    monkeypatch: MonkeyPatch, mock_fastmcp
+):
+    """A failed codegen run with no output on either stream must still surface
+    the exit code so the LLM can distinguish failure from an empty response."""
+
+    class MockProcessFailureNoOutput:
+        returncode = 2
+
+        def communicate(self, timeout=None):
+            return "", ""
+
+    def mock_popen(args, **kwargs):
+        return MockProcessFailureNoOutput()
+
+    monkeypatch.setattr("subprocess.Popen", mock_popen)
+
+    fastmcp, _ = mock_fastmcp
+    register_dbt_codegen_tools(
+        fastmcp,
+        mock_dbt_codegen_config,
+        disabled_tools=set(),
+        enabled_tools=None,
+        enabled_toolsets=set(),
+        disabled_toolsets=set(),
+    )
+    generate_source_tool = fastmcp.tools["generate_source"]
+
+    result = generate_source_tool(
+        schema_name="test_schema",
+        database_name=None,
+        table_names=None,
+        generate_columns=False,
+        include_descriptions=False,
+    )
+
+    assert "Error running dbt-codegen macro" in result
+    assert "exit code 2" in result
+
+
 def test_codegen_timeout_handling(monkeypatch: MonkeyPatch, mock_fastmcp):
     """Test timeout handling for long-running operations."""