From ebd52f2113782065fadabfb6b54848a2004b07bb Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 11:49:41 -0500 Subject: [PATCH] tui: stop CI deciding whether the golden capture has colour The TUI golden job has never passed. Not "started failing" -- every run on record, back to 2026-08-30, fails the same way: each committed golden carries a "-- cell styles --" block and the fresh capture carries none. The rendered text matches exactly; only the styling is missing. The cause is one line in termenv. Output.isTTY() returns false whenever CI is set in the environment, before it looks at the file descriptor at all: if len(o.environ.Getenv("CI")) > 0 { return false } GitHub Actions sets CI=true. The capture harness passed the host environment through to the child, so lipgloss resolved the Ascii profile, the binary emitted no SGR, pyte found no styled cells, and render_snapshot omitted the whole section. Locally, where CI is unset, the same capture produces TrueColor spans -- which is how goldens containing styles came to be committed against a check that could never reproduce them. So the harness now builds the child environment itself and drops CI along with NO_COLOR. It owns a real PTY and has already set the size on it, so "this is a terminal" is the truthful answer; letting the host argue otherwise is the one thing a golden harness must not permit. CLICOLOR_FORCE would not have fixed it: it only lifts Ascii to 16-colour ANSI, while the goldens record TrueColor. Two things stop this recurring. The environment decision moves into child_env(), a pure function that is unit-tested rather than reachable only through a PTY the harness refuses to open on Windows. And assert_styled() now fails a capture that contains no styled cells at all, because that is not a snapshot of this TUI, it is a snapshot of colour being off -- which previously failed silently in both directions, reporting every golden as changed under `check` and overwriting reviewed goldens under `update`. The goldens themselves are unchanged. Co-Authored-By: Claude Opus 5 --- scripts/tui_capture.py | 56 ++++++++++++++++++++++++++++++++++--- scripts/tui_capture_test.py | 33 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/scripts/tui_capture.py b/scripts/tui_capture.py index dd41f6b..cf58c60 100755 --- a/scripts/tui_capture.py +++ b/scripts/tui_capture.py @@ -149,6 +149,11 @@ def assert_protocol_safety(raw): assert_balanced_mode(changes, 2004, "bracketed-paste") +# The marker that separates rendered text from the style spans below it. +# scripts/tui_golden.sh splits captures on this string too. +STYLE_HEADER = "-- cell styles --" + + def cell_style(cell): """Return a stable description of non-default pyte cell attributes.""" attributes = [] @@ -185,7 +190,7 @@ def render_snapshot(screen): text = "\n".join(lines) + ("\n" if lines else "") if spans: - text += "\n-- cell styles --\n" + "\n".join(spans) + "\n" + text += "\n" + STYLE_HEADER + "\n" + "\n".join(spans) + "\n" return text @@ -203,6 +208,50 @@ def assert_semantic_text(raw, expected_values, post_resize_raw=None): ) +def child_env(base=None): + """Build the captured process's environment. + + The harness owns a real PTY and has already set its size, so the child + is genuinely attached to a terminal and should render exactly as it + would for a developer. Anything in the host environment that would + argue otherwise is removed rather than trusted, because a golden + harness whose output depends on where it runs is not a golden harness. + + NO_COLOR counts by presence, even when set to "0". + + CI is the one that actually bit: termenv answers isTTY() with a flat + "no" whenever CI is set, before it ever looks at the file descriptor, + so lipgloss falls back to the Ascii profile and the render carries no + SGR at all. Every captured style span disappears and each golden looks + changed for no visible reason. CLICOLOR_FORCE is not a substitute: it + only lifts Ascii to 16-colour ANSI, while the goldens record TrueColor. + """ + env = dict(os.environ if base is None else base, + TERM="xterm-256color", COLORTERM="truecolor", CLICOLOR="1") + for name in ("NO_COLOR", "CI"): + env.pop(name, None) + return env + + +def assert_styled(cells, target, scenario): + """Require at least one styled cell in a captured snapshot. + + A capture that renders every cell in the default style is not a + snapshot of this TUI, it is a snapshot of colour having been switched + off -- which is what happens when something in the environment + convinces termenv it is not writing to a terminal. Without this check + the failure is silent in both directions: `check` reports every golden + as changed with no hint why, and `update` will happily overwrite the + reviewed goldens with colourless ones. + """ + if STYLE_HEADER not in cells: + raise RuntimeError( + f"{target}/{scenario}: capture contains no styled cells, so the " + "render was produced with colour disabled. Check that nothing in " + "the environment (CI, NO_COLOR, TERM) is suppressing it." + ) + + def capture(command, width, height, keys, settle, timeout, resize): if os.name == "nt": raise RuntimeError( @@ -213,9 +262,7 @@ def capture(command, width, height, keys, settle, timeout, resize): master, slave = pty.openpty() # TIOCSWINSZ takes rows, columns. fcntl.ioctl(slave, termios.TIOCSWINSZ, struct.pack("HHHH", height, width, 0, 0)) - env = dict(os.environ, TERM="xterm-256color", COLORTERM="truecolor", CLICOLOR="1") - # NO_COLOR is enabled by presence, even when its value is "0". - env.pop("NO_COLOR", None) + env = child_env() proc = subprocess.Popen(command, stdin=slave, stdout=slave, stderr=slave, env=env, start_new_session=True, close_fds=True) os.close(slave) @@ -308,6 +355,7 @@ def main(): if args.protocol_check: assert_protocol_safety(raw) assert_semantic_text(raw, args.expect_text, post_resize_raw) + assert_styled(cells, args.target, args.scenario) geometry = f"{initial_width}x{initial_height}" if args.resize is not None: geometry += f"-to-{width}x{height}" diff --git a/scripts/tui_capture_test.py b/scripts/tui_capture_test.py index cb57395..4d07ff4 100644 --- a/scripts/tui_capture_test.py +++ b/scripts/tui_capture_test.py @@ -81,5 +81,38 @@ def test_non_default_cell_styles_are_serialized(self): self.assertIn("bold", snapshot) +class ChildEnvTests(unittest.TestCase): + def test_ci_is_removed_so_termenv_sees_a_terminal(self): + # termenv reports "not a TTY" whenever CI is set, whatever the file + # descriptor actually is, which strips every style span from the + # capture and makes every golden look changed. + env = tui_capture.child_env({"CI": "true", "PATH": "/usr/bin"}) + self.assertNotIn("CI", env) + self.assertEqual(env["PATH"], "/usr/bin") + + def test_no_color_is_removed_even_when_zero(self): + env = tui_capture.child_env({"NO_COLOR": "0"}) + self.assertNotIn("NO_COLOR", env) + + def test_colour_capability_is_pinned(self): + env = tui_capture.child_env({"TERM": "dumb", "COLORTERM": ""}) + self.assertEqual(env["TERM"], "xterm-256color") + self.assertEqual(env["COLORTERM"], "truecolor") + self.assertEqual(env["CLICOLOR"], "1") + + +class AssertStyledTests(unittest.TestCase): + def test_styled_capture_is_accepted(self): + snapshot = "hi" + chr(10) + chr(10) + "-- cell styles --" + chr(10) + "1:1-2 fg=010203" + chr(10) + tui_capture.assert_styled(snapshot, "packetcode", "plan") + + def test_styleless_capture_is_rejected(self): + # Colour switched off yields text and no spans. Without this guard + # `update` would promote that into the reviewed goldens. + with self.assertRaises(RuntimeError) as caught: + tui_capture.assert_styled("hi" + chr(10), "packetcode", "plan") + self.assertIn("packetcode/plan", str(caught.exception)) + self.assertIn("colour disabled", str(caught.exception)) + if __name__ == "__main__": unittest.main()