fix(workflows): render gate show_file contents in the interactive prompt by doquanghuy · Pull Request #2810 · github/spec-kit (original) (raw)
The gate step read and recorded show_file but never displayed its
contents at the interactive prompt, so the operator approved/rejected
without seeing the referenced file. Render the file inside the prompt
when stdin is a TTY, with a graceful notice for missing/unreadable
files. Non-interactive PAUSED behaviour, exit codes, resume semantics,
and no-show_file output are unchanged.
Closes github#2809.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
…le reads
The gate prompt rendered show_file by passing it as a third positional argument to _prompt. A test that stubs _prompt with a two-argument lambda (test_gate_abort_still_halts_with_continue_on_error) then failed once the branch caught up to main, because the call site passed three arguments to the two-argument stub.
Compose the show_file material into the displayed message in execute() and keep _prompt to its (message, options) contract. Display data no longer widens the interactive seam, so stubbing _prompt stays stable and future review material can be added without breaking callers. _prompt now renders a multi-line message inside the gate box.
Also catch ValueError in _read_show_file so a path the OS rejects outright (e.g. an embedded NUL byte) degrades to a notice instead of crashing the prompt, matching the helper's stated contract.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
The multi-line render loop split the message on newlines, which assumes a str. A non-string message (e.g. a YAML numeric literal) previously rendered fine through the old f-string and would now raise on .split. Coerce with str() to preserve that tolerance, and add a regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
… typing
Address review feedback on the gate tests and helper:
- Swap the gate module's sys.stdin for a fixed-isatty stub (shared _StubStdin / _force_gate_stdin helpers) instead of setattr on sys.stdin.isatty, which is not assignable under some pytest capture modes. This also forces the non-interactive tests to a non-TTY so they cannot block on input() when run in a real terminal.
- The non-interactive show_file test now hard-fails if _read_show_file is called, proving the file is not read on the PAUSED path.
- _compose_prompt accepts a non-string message (e.g. a YAML numeric literal) and always returns str via str(message), keeping its annotation and docstring accurate; the redundant coercion in _prompt is removed.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
…s non-TTY
Address review feedback:
- _read_show_file strips C0 control characters (except tab) from each line, so a show_file containing ANSI escape sequences (e.g. \x1b[2J) cannot clear the screen or spoof the prompt/options when rendered to a terminal.
- Add an autouse fixture on TestGateStep that defaults every gate test to a non-TTY stdin, so no test can drop into the interactive prompt and block on input() when the suite runs under a real TTY. Interactive tests opt back in via _force_gate_stdin(tty=True); the now-redundant explicit non-TTY calls were removed.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
_force_gate_stdin rebinds the gate module's sys name to a stand-in whose
stdin has a fixed isatty() and which delegates every other attribute to the
real sys, instead of mutating the process-wide sys.stdin. This keeps the
patch local to the gate module and leaves real stdin untouched. The gate
abort test, which used the same process-wide swap, now shares the helper, so
the pattern exists in exactly one place.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
…content
Control characters were stripped from show_file contents but the path was
still printed verbatim as the header (f"{show_file}:") and echoed in the
read-error notice, so a show_file path containing ANSI escapes could still
inject terminal sequences. Centralize stripping in _sanitize_for_display
and apply it to every show_file-derived string that reaches the terminal —
the displayed path, each file line, and the error notice — while still
opening the file with the original path. Add a test for path sanitization.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Reuse the existing _CONTROL_CHARS regex directly at the three display sites instead of wrapping it in a one-line helper.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
…play
The control-char class skipped LF (so an embedded newline in a show_file path could break the boxed layout) and the C1 range (so \x9b CSI and other 8-bit controls survived). Widen the class to [\x00-\x08\x0a-\x1f\x7f-\x9f] (still keeping tab).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})