fix: report-write failures should not look like findings in check and lint

src/sqlquality/cli.py

@@ -110,6 +110,27 @@ def read_sql_file(path: Path) -> str:
         raise typer.Exit(code=2)
 
 
+def _write_report_or_exit(path: Path, text: str, flag: str) -> None:
+    """Write a rendered report, or exit 2 naming the flag that pointed at the bad path.
+
+    Two failures are folded together here because both used to escape as exit **1**, and
+    exit 1 is how `check` and `lint` report real findings — so an unwritable path was
+    indistinguishable from a failed gate, and CI would block on a healthy run.
+
+    * ``encoding="utf-8"`` because the default is platform-dependent and these reports
+      carry non-ASCII (the gate verdict is an emoji), so an ASCII locale made every
+      ``--markdown`` write a guaranteed crash.
+    * ``UnicodeError`` alongside ``OSError`` because ``UnicodeEncodeError`` is a
+      ``ValueError``: pinning the encoding makes it unlikely, not unreachable, since a lone
+      surrogate in an identifier is unencodable in any codec.
+    """
+    try:
+        path.write_text(text, encoding="utf-8")
+    except (OSError, UnicodeError) as exc:
+        typer.echo(f"Could not write {flag} to {path}: {exc}", err=True)
+        raise typer.Exit(code=2)
+
+
 def _validate_dialect_or_exit(name: str) -> str:
     """Normalize a dialect name or print the friendly error and exit 2."""
     try:
@@ -300,11 +321,12 @@ def check(
     deltas, skipped = compute_deltas(baseline, candidate, changeset.changed, resolved_dialect)
     report = evaluate_gate(deltas, cfg)
 
+    # Rendered first, then written, so a renderer bug cannot be reported as a write failure.
     if html is not None:
-        Path(html).write_text(render_html(report, skipped))
+        _write_report_or_exit(Path(html), render_html(report, skipped), "--html")
 
     if markdown is not None:
-        Path(markdown).write_text(render_markdown(report, skipped))
+        _write_report_or_exit(Path(markdown), render_markdown(report, skipped), "--markdown")
 
     if json_out:
         typer.echo(
@@ -377,7 +399,11 @@ def lint(
         if fix:
             fixed_sql = fix_sql(sql, dialect, excl, config_path)
             if fixed_sql != sql:
-                path.write_text(fixed_sql)
+                # `--fix` rewrites the user's own source. read_sql_file already reads as
+                # UTF-8, so writing without an encoding could round-trip a non-ASCII
+                # comment into mojibake, or raise and exit 1 — which `lint` also uses for
+                # findings.
+                _write_report_or_exit(path, fixed_sql, "--fix")
                 changed = True
         # INFO (unresolved-Jinja) findings are advisory and never gate the commit.
         gating = gating or any(f.severity in (Severity.WARNING, Severity.ERROR) for f in findings)

tests/test_check_gate.py

@@ -229,3 +229,87 @@ def test_check_old_schema_warns(tmp_path):
     # Warning only: no behavior change, still exits normally.
     assert result.exit_code == 0
     assert "v12" in result.stderr and "v9" in result.stderr
+
+
+def test_check_html_write_failure_exits_2_not_1(tmp_path):
+    """A bad --html path must not be mistaken for a failed gate.
+
+    `check` uses exit 1 legitimately, for a regression over the threshold, so an unwritable
+    output path escaping as exit 1 is indistinguishable from a real finding — CI would
+    report a healthy run as a blocked one.
+    """
+    proj, state = _project_with_baseline(tmp_path)
+    with _mock_changed():
+        result = runner.invoke(
+            app,
+            [
+                "check",
+                "--project-dir",
+                str(proj),
+                "--state",
+                str(state),
+                "--html",
+                str(tmp_path / "nope" / "missing.html"),
+            ],
+        )
+    assert result.exit_code == 2, result.output
+    assert "--html" in result.output
+
+
+def test_check_markdown_write_failure_exits_2_not_1(tmp_path):
+    proj, state = _project_with_baseline(tmp_path)
+    a_directory = tmp_path / "adir"
+    a_directory.mkdir()
+    with _mock_changed():
+        result = runner.invoke(
+            app,
+            [
+                "check",
+                "--project-dir",
+                str(proj),
+                "--state",
+                str(state),
+                "--markdown",
+                str(a_directory),
+            ],
+        )
+    assert result.exit_code == 2, result.output
+    assert "--markdown" in result.output
+
+
+def test_check_writes_reports_as_utf8(tmp_path, monkeypatch):
+    """Both report writes must name their encoding.
+
+    The verdict line carries an emoji, so a platform-encoded write is a guaranteed crash on
+    an ASCII locale — and UnicodeEncodeError is a ValueError, so it escapes the OSError
+    handler and exits 1, which `check` also uses for a failed gate. Asserting the argument
+    rather than emulating a locale, because this Python resolves the default encoding
+    somewhere the locale module cannot be patched from.
+    """
+    seen: list[str | None] = []
+    real = Path.write_text
+
+    def spy(self, data, encoding=None, *args, **kwargs):
+        seen.append(encoding)
+        return real(self, data, encoding=encoding or "utf-8", *args, **kwargs)
+
+    proj, state = _project_with_baseline(tmp_path)
+    monkeypatch.setattr(Path, "write_text", spy)
+    seen.clear()  # the fixture writes manifests of its own; only the reports matter here
+    with _mock_changed():
+        result = runner.invoke(
+            app,
+            [
+                "check",
+                "--project-dir",
+                str(proj),
+                "--state",
+                str(state),
+                "--markdown",
+                str(tmp_path / "r.md"),
+                "--html",
+                str(tmp_path / "r.html"),
+            ],
+        )
+    assert result.exit_code in (0, 1), result.output
+    assert seen == ["utf-8", "utf-8"], f"reports written without an explicit encoding: {seen}"

tests/test_lint_cli.py

@@ -220,3 +220,29 @@ def test_lint_fix_bad_second_path_leaves_first_unmodified(tmp_path):
     result = runner.invoke(app, ["lint", "--fix", str(good), str(missing)])
     assert result.exit_code == 2
     assert good.read_text() == before  # untouched
+
+
+def test_lint_fix_writes_utf8(tmp_path, monkeypatch):
+    """--fix rewrites the user's own source file.
+
+    read_sql_file already reads as UTF-8; writing without an encoding meant a non-ASCII
+    comment could come back as mojibake, or raise UnicodeEncodeError — which escapes as
+    exit 1, the code `lint` also uses for findings.
+    """
+    from pathlib import Path as _P
+
+    seen: list[str | None] = []
+    real = _P.write_text
+
+    def spy(self, data, encoding=None, *args, **kwargs):
+        seen.append(encoding)
+        return real(self, data, encoding=encoding or "utf-8", *args, **kwargs)
+
+    monkeypatch.setattr(_P, "write_text", spy)
+    f = tmp_path / "m.sql"
+    f.write_text("-- \u2705 v\u00e9rifi\u00e9\nselect a from t as t\n", encoding="utf-8")
+    seen.clear()
+    result = runner.invoke(app, ["lint", str(f), "--fix"])
+    assert result.exit_code in (0, 1), result.output
+    assert seen == ["utf-8"], f"--fix wrote the source file without an encoding: {seen}"
+    assert "v\u00e9rifi\u00e9" in f.read_text(encoding="utf-8")