Validate and resolve file paths before API work#29
Open
sahilsunny wants to merge 3 commits into
Open
Conversation
Summary Fix path handling for --output-file, --output-dir, and --input-file so ~/ home shortcuts work, missing output directories are created, overwrite conflicts are detected early, and input files are verified before any scrape or batch API call. This prevents wasted credits from scraping first and failing on write, and gives clear REPL-friendly errors instead of literal '~/...' path failures. Details - cli_utils.py:323-325 — add resolve_output_path() to expand ~ via Path.expanduser() so all path helpers share one resolution step. - cli_utils.py:328-349 — add ensure_output_file_ready() to expand ~, mkdir parent dirs, and run overwrite checks before API work. - cli_utils.py:352-360 — add ensure_output_dir_ready() to expand ~ and create --output-dir before batch/crawl writes. - cli_utils.py:363-385 — add ensure_input_file_ready() to expand ~, verify the file exists/is readable, and pass stdin (-) through unchanged. - cli_utils.py:392-403 — update confirm_overwrite() to check the expanded path so ~/Desktop/foo.png correctly detects an existing Desktop file; REPL still raises UsageError with --overwrite hint instead of blocking on click.confirm(). - cli_utils.py:591-601 — call all ensure_* helpers at the end of store_common_options(), after flag validation but before commands hit the API. - cli_utils.py:1902-1923 — harden write_output() with expand/mkdir/open on the resolved path as a safety net when extensions are appended post-validation. - batch.py:220-221 — expand ~ in read_input_file() as a fallback for callers outside store_common_options(). - crawl.py:432,569 — ensure crawl output dirs (explicit --output-dir and default crawl_<timestamp>) exist before the spider starts. - export.py:96-101 — replace confirm_overwrite() with ensure_output_file_ready() so export validates the destination before reading input files. - test_v132_fixes.py:600-860 — unit tests for tilde expansion, early input/output validation, and REPL scrape guards that API must not run on bad paths. - test_repl_pty.py:320-345 — PTY regression that REPL shows overwrite error for existing ~/ output paths before scraping. - test_v132_fixes.py:955-1001 — update batch option tests to create real input files now that store_common_options validates --input-file up front.
Summary CI lint failed on ruff format (test_v132_fixes.py) and ty reported invalid-argument-type for ensure_* calls passing Any | None from obj dict. Narrow path values with isinstance(str) before calling helpers. Details - cli_utils.py:592-603 — use isinstance(path, str) guards before ensure_input_file_ready / ensure_output_file_ready / ensure_output_dir_ready so ty accepts narrowed str arguments from the options dict. - test_v132_fixes.py:634 — ruff format wrap for store_common_options call.
Summary
CI failed on all Windows matrix jobs: the new ~/ path tests set only HOME,
but Path.expanduser() on Windows reads USERPROFILE, so ~ expanded to the
real runner profile (C:\Users\runneradmin) instead of the pytest sandbox.
Add a _set_home() helper that sets both variables and use it everywhere.
Details
- test_v132_fixes.py:28-37 — add _set_home() helper setting HOME and
USERPROFILE so ~ expansion is sandboxed on POSIX and Windows alike.
- test_v132_fixes.py:615-618 — test_resolve_output_path_expands_tilde now
uses tmp_path instead of a hardcoded POSIX /tmp/fakehome expectation.
- test_v132_fixes.py (10 call sites) — replace monkeypatch.setenv("HOME", ...)
with _set_home(monkeypatch, home) in output/input/REPL path tests.
Note: the macOS/Ubuntu failures on test_session_default_skip_warning_on_screen
are a pre-existing flaky PTY test — it also failed on the main branch push
(run 29008646319) before this branch existed.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix path handling for --output-file, --output-dir, and --input-file so ~/ home shortcuts work, missing output directories are created, overwrite conflicts are detected early, and input files are verified before any scrape or batch API call. This prevents wasted credits from scraping first and failing on write, and gives clear REPL-friendly errors instead of literal '~/...' path failures.
Details