Skip to content

Commit 1746289

Browse files
authored
fix: address three unresolved review comments in InitStep
- Use `with os.scandir(...)` context manager so the iterator is always closed even when `any()` short-circuits, preventing file-descriptor leaks in long-running workflow runs. - Guard `os.chdir(prev_cwd)` in the `finally` block with a try/except so an `OSError` (e.g. directory deleted) doesn't bypass returning the captured `StepResult`. - Reject non-string `script` values in `validate()` with a clear error message, rather than silently passing them through to become `--script True` at runtime.
1 parent 86333ab commit 1746289

1 file changed

Lines changed: 12 additions & 3 deletions

File tree

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
100100
if targets_current_dir and not force:
101101
base = context.project_root or os.getcwd()
102102
try:
103-
not_empty = any(os.scandir(base))
103+
with os.scandir(base) as it:
104+
not_empty = any(it)
104105
except OSError:
105106
not_empty = False
106107
if not_empty:
@@ -202,7 +203,10 @@ def _run_init(
202203
try:
203204
result = runner.invoke(app, argv, catch_exceptions=True)
204205
finally:
205-
os.chdir(prev_cwd)
206+
try:
207+
os.chdir(prev_cwd)
208+
except OSError:
209+
pass
206210

207211
stdout = result.output or ""
208212
# click >= 8.2 captures stderr separately; older versions mix it into
@@ -221,7 +225,12 @@ def _run_init(
221225
def validate(self, config: dict[str, Any]) -> list[str]:
222226
errors = super().validate(config)
223227
script = config.get("script")
224-
if (
228+
if script is not None and not isinstance(script, str):
229+
errors.append(
230+
f"Init step {config.get('id', '?')!r}: 'script' must be a string "
231+
f"({' or '.join(repr(s) for s in VALID_SCRIPT_TYPES)})."
232+
)
233+
elif (
225234
isinstance(script, str)
226235
and "{{" not in script
227236
and script not in VALID_SCRIPT_TYPES

0 commit comments

Comments
 (0)