Skip to content

Commit bf2f25c

Browse files
doquanghuyclaude
andcommitted
fix(workflows): render gate show_file contents in the interactive prompt
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 #2809. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ed10b32 commit bf2f25c

2 files changed

Lines changed: 162 additions & 4 deletions

File tree

src/specify_cli/workflows/steps/gate/__init__.py

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import sys
6+
from pathlib import Path
67
from typing import Any
78

89
from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus
@@ -23,6 +24,10 @@ class GateStep(StepBase):
2324

2425
type_key = "gate"
2526

27+
#: Maximum number of ``show_file`` lines rendered at the prompt, so a
28+
#: large file cannot flood the terminal before the choice.
29+
MAX_SHOW_FILE_LINES = 200
30+
2631
def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
2732
message = config.get("message", "Review required.")
2833
if isinstance(message, str) and "{{" in message:
@@ -32,8 +37,14 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
3237
on_reject = config.get("on_reject", "abort")
3338

3439
show_file = config.get("show_file")
35-
if show_file and isinstance(show_file, str) and "{{" in show_file:
40+
if isinstance(show_file, str) and "{{" in show_file:
3641
show_file = evaluate_expression(show_file, context)
42+
# ``evaluate_expression`` can return a non-string for a single
43+
# expression (e.g. a number from a prior step), and a literal
44+
# non-string is also possible; coerce so it is rendered rather
45+
# than silently skipped at the prompt.
46+
if show_file is not None:
47+
show_file = str(show_file)
3748

3849
output = {
3950
"message": message,
@@ -48,7 +59,7 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
4859
return StepResult(status=StepStatus.PAUSED, output=output)
4960

5061
# Interactive: prompt the user
51-
choice = self._prompt(message, options)
62+
choice = self._prompt(message, options, show_file)
5263
output["choice"] = choice
5364

5465
if choice in ("reject", "abort"):
@@ -68,10 +79,20 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
6879
return StepResult(status=StepStatus.COMPLETED, output=output)
6980

7081
@staticmethod
71-
def _prompt(message: str, options: list[str]) -> str:
72-
"""Display gate message and prompt for a choice."""
82+
def _prompt(message: str, options: list[str], show_file: str | None = None) -> str:
83+
"""Display gate message and prompt for a choice.
84+
85+
When ``show_file`` names a readable file, its contents are shown
86+
before the options so the operator can review the material the
87+
gate refers to.
88+
"""
7389
print("\n ┌─ Gate ─────────────────────────────────────")
7490
print(f" │ {message}")
91+
if show_file:
92+
print(" │")
93+
print(f" │ {show_file}:")
94+
for line in GateStep._read_show_file(show_file):
95+
print(f" │ {line}")
7596
print(" │")
7697
for i, opt in enumerate(options, 1):
7798
print(f" │ [{i}] {opt}")
@@ -90,6 +111,34 @@ def _prompt(message: str, options: list[str]) -> str:
90111
return next(o for o in options if o.lower() == raw.lower())
91112
print(f" Invalid choice. Enter 1-{len(options)} or an option name.")
92113

114+
@staticmethod
115+
def _read_show_file(show_file: str) -> list[str]:
116+
"""Return the lines of ``show_file`` for display.
117+
118+
Reads at most ``MAX_SHOW_FILE_LINES`` lines so a large file cannot
119+
flood the prompt, and returns a short notice instead of raising
120+
when the file is missing or cannot be decoded, so a misconfigured
121+
path never breaks the interactive prompt.
122+
"""
123+
lines: list[str] = []
124+
truncated = False
125+
try:
126+
with Path(show_file).open(encoding="utf-8") as handle:
127+
for line in handle:
128+
if len(lines) >= GateStep.MAX_SHOW_FILE_LINES:
129+
truncated = True
130+
break
131+
lines.append(line.rstrip("\n"))
132+
except (OSError, UnicodeDecodeError) as exc:
133+
return [f"(could not read file: {exc})"]
134+
if not lines and not truncated:
135+
return ["(file is empty)"]
136+
if truncated:
137+
lines.append(
138+
f"… (output truncated at {GateStep.MAX_SHOW_FILE_LINES} lines)"
139+
)
140+
return lines
141+
93142
def validate(self, config: dict[str, Any]) -> list[str]:
94143
errors = super().validate(config)
95144
if "message" not in config:

tests/test_workflows.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,115 @@ def test_validate_invalid_on_reject(self):
822822
})
823823
assert any("on_reject" in e for e in errors)
824824

825+
def test_interactive_prompt_renders_show_file(self, tmp_path, monkeypatch, capsys):
826+
from specify_cli.workflows.steps.gate import GateStep
827+
from specify_cli.workflows.base import StepContext, StepStatus
828+
829+
review = tmp_path / "spec.md"
830+
review.write_text("LINE-ONE\nLINE-TWO\n", encoding="utf-8")
831+
832+
monkeypatch.setattr("sys.stdin.isatty", lambda: True)
833+
monkeypatch.setattr("builtins.input", lambda _prompt="": "1")
834+
835+
step = GateStep()
836+
config = {
837+
"id": "review",
838+
"message": "Review the spec.",
839+
"show_file": str(review),
840+
"options": ["approve", "reject"],
841+
}
842+
result = step.execute(config, StepContext())
843+
out = capsys.readouterr().out
844+
845+
assert "LINE-ONE" in out and "LINE-TWO" in out
846+
assert str(review) in out
847+
assert result.status == StepStatus.COMPLETED
848+
assert result.output["choice"] == "approve"
849+
850+
def test_interactive_prompt_missing_show_file_does_not_crash(
851+
self, tmp_path, monkeypatch, capsys
852+
):
853+
from specify_cli.workflows.steps.gate import GateStep
854+
from specify_cli.workflows.base import StepContext, StepStatus
855+
856+
missing = tmp_path / "does-not-exist.md"
857+
858+
monkeypatch.setattr("sys.stdin.isatty", lambda: True)
859+
monkeypatch.setattr("builtins.input", lambda _prompt="": "1")
860+
861+
step = GateStep()
862+
config = {
863+
"id": "review",
864+
"message": "Review.",
865+
"show_file": str(missing),
866+
"options": ["approve", "reject"],
867+
}
868+
result = step.execute(config, StepContext())
869+
out = capsys.readouterr().out
870+
871+
assert "could not read file" in out
872+
assert result.status == StepStatus.COMPLETED
873+
874+
def test_non_interactive_show_file_still_pauses_without_reading(
875+
self, tmp_path, monkeypatch
876+
):
877+
from specify_cli.workflows.steps.gate import GateStep
878+
from specify_cli.workflows.base import StepContext, StepStatus
879+
880+
review = tmp_path / "spec.md"
881+
review.write_text("CONTENT\n", encoding="utf-8")
882+
883+
monkeypatch.setattr("sys.stdin.isatty", lambda: False)
884+
885+
step = GateStep()
886+
config = {
887+
"id": "review",
888+
"message": "Review.",
889+
"show_file": str(review),
890+
"options": ["approve", "reject"],
891+
}
892+
result = step.execute(config, StepContext())
893+
assert result.status == StepStatus.PAUSED
894+
assert result.output["show_file"] == str(review)
895+
896+
def test_read_show_file_empty(self, tmp_path):
897+
from specify_cli.workflows.steps.gate import GateStep
898+
899+
empty = tmp_path / "empty.md"
900+
empty.write_text("", encoding="utf-8")
901+
assert GateStep._read_show_file(str(empty)) == ["(file is empty)"]
902+
903+
def test_read_show_file_truncates_large_file(self, tmp_path):
904+
from specify_cli.workflows.steps.gate import GateStep
905+
906+
big = tmp_path / "big.md"
907+
big.write_text(
908+
"\n".join(f"line{i}" for i in range(GateStep.MAX_SHOW_FILE_LINES + 50)),
909+
encoding="utf-8",
910+
)
911+
rendered = GateStep._read_show_file(str(big))
912+
# MAX_SHOW_FILE_LINES content lines + one truncation notice line.
913+
assert len(rendered) == GateStep.MAX_SHOW_FILE_LINES + 1
914+
assert "truncated" in rendered[-1]
915+
916+
def test_templated_show_file_resolving_to_non_string_is_coerced(self):
917+
from specify_cli.workflows.steps.gate import GateStep
918+
from specify_cli.workflows.base import StepContext, StepStatus
919+
920+
# A single-expression template can resolve to a non-string (e.g. a
921+
# number from a prior step); it must be coerced to str, not skipped.
922+
step = GateStep()
923+
ctx = StepContext(steps={"prev": {"output": {"ref": 123}}})
924+
config = {
925+
"id": "review",
926+
"message": "Review.",
927+
"show_file": "{{ steps.prev.output.ref }}",
928+
"options": ["approve", "reject"],
929+
}
930+
result = step.execute(config, ctx) # non-interactive -> PAUSED
931+
assert result.status == StepStatus.PAUSED
932+
assert result.output["show_file"] == "123"
933+
825934

826935
class TestIfThenStep:
827936
"""Test the if/then/else step type."""

0 commit comments

Comments
 (0)