Fix rimport called on a directory - #31
Merged
Merged
Conversation
Holds the subagent-driven-development ledger and per-task artifacts for the in-flight plan. Scratch, not source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
walk_files does not exist yet; these fail by design. They pin the enumeration contract: recursive, sorted for determinism, symlinks yielded as entries, no descent through a directory symlink (which would let the walk leave the named tree or loop on a cycle), dotfiles included, and an unreadable directory returned as a Skip rather than raised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
expand_directories does not exist yet; these fail by design. They pin named-vs-discovered provenance, which is what lets a discovered failure warn-and-skip while a path the user typed still aborts the batch. Also pinned: a named symlink-to-directory is not expanded, duplicates collapse with named winning, an empty directory warns without becoming a skip, and the expansion count line only appears when something actually expanded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fail by design. Today an empty name anchors to cwd and yields the cwd itself, which was harmless only because a directory was then rejected. Once directories expand, `rimport ""` from an unset shell variable would recursively publish the subtree the user is standing in -- the hazard named in 75c79cd. Pin the rejection before the behavior that needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fail by design. Pin the asymmetry the design turns on: a path the user typed is still fatal and aborts everything, while a bad file found by a walk warns, is skipped, and lets its neighbours publish. Also pin exit 3 for completed-with-skips, precedence 1 > 3 so a staging failure is never masked, --check sharing the same codes, and the skip being reported twice -- inline on stdout, then repeated on stderr at the very end. Swap the empty directory in the pre-flight gate test for a broken symlink: an empty directory is no longer a failure once directories expand, so leaving it there would quietly weaken the test to a single failure mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hing test
test_named_failure_still_aborts_everything_including_discovered_files passed
today for the wrong reason: a named directory is currently rejected outright,
so it and the named `missing` path both fail pre-flight ("2 of 2"), giving
the same externally-observable result the test already checked for. That
made the test pass both before and after directory enumeration, pinning
nothing.
Add an assertion on the pre-flight count itself: after enumeration only the
named `missing` path is fatal, while good.nc (discovered under subdir) is
not counted against it, so the message must read "1 of 2", not "2 of 2".
This makes the test red now and green only once the named/discovered split
is implemented.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_discovered_failure_warns_skips_and_returns_3 checked that good.nc's content landed under staging_root, but never that the original was replaced with a symlink pointing at it. Staging has two halves -- copy, then relink -- and an implementation that copied but forgot to relink would have passed. Red at HEAD by design: the directory guard still rejects a directory argument outright. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fail by design. rimport's --help is meant to stand on its own (69b3e47), so the new behavior has to be discoverable there: that a directory argument is enumerated recursively, that a symlink to a directory is the carve-out and is not expanded, and that there are now four exit codes rather than three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tests The two initial red tests had loose assertions: - test_help_documents_directory_expansion only required "director" (already present in 'inputdata directory') and "recursiv" separately, allowing either to pass independently without documenting recursive enumeration together. - test_help_documents_all_four_exit_codes looked for "0:", "1:", "2:", "3:" and "skipped" anywhere in the help text, not anchored to an "exit codes:" section. Both defects defeated the point of these red tests: to force later implementation of actual help text. Fixed: - test_help_documents_directory_expansion now checks both "directory" and "enumerated recursively" appear together via whitespace-normalized text (to account for argparse line wrapping). - test_help_documents_all_four_exit_codes now anchors all four codes and "skipped" to the "exit codes:" section. Both tests still fail (red by design) because today's help contains neither "enumerated recursively" nor an "exit codes:" section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fail by design. Flip the three tests that asserted the old directory-is-an-error contract, keeping the assertion that matters from 75c79cd: the directory itself is still never renamed, symlinked away, or left with a failed-rollback '.tmp'. That was always the real invariant; the error was only how it was enforced. The empty-string test keeps failing but for the new reason, and the mixed-validity list swaps its empty directory for a broken symlink, since an empty directory is no longer a failure. New coverage: recursion into subdirectories, the skip summary landing last on stderr and surviving -q, and the expansion count printing before anything is staged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turns tests/rimport/test_walk_files.py green. The walk never descends through a symlink, so it cannot leave the tree the user named and cannot loop on a cycle; a symlink is always a leaf entry and validate_source_path decides what it means. Unlike relink's walker there is no owner filter and symlinks are not skipped -- rimport runs as the staging owner and needs to see every entry. Per-level sorting makes output and tests deterministic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turns tests/rimport/test_expand_directories.py green. Expansion deliberately does no validation, so a nonexistent named path still reaches the pre-flight gate and gets its usual message. A symlink to a directory is left as one named entry, matching walk_files. When a path is both named and discovered, named wins: demoting a path the user typed would turn its failure from fatal into a skip. The expansion count is logged here rather than in main -- this is the only place holding both numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The n_expanded_files counter was summing len(found) from each walk before the dict deduplication, which overstates the file count in the log message. When expand_directories receives overlapping directory arguments like [d, d], it would report "expanded 2 director(ies) to 4 file(s)" when only 2 distinct files would be staged. Since the expansion count's only purpose is to show the blast radius before staging, accuracy matters at the moment the operator reads it. Fix: use a set to track distinct found files; count len(expanded_files) in the log line instead of a running sum. This preserves the honest count of n_dirs (a directory argument named twice really is two arguments) while fixing the file count to match deduplication. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turns the new tests in tests/rimport/test_get_files_to_process.py green. An empty name anchors to cwd and resolves to the cwd itself. That was survivable only because a directory was then rejected. Directory enumeration removes that accidental safety net, so `rimport "$unset_var"` would recursively publish the subtree the user is standing in -- the hazard 75c79cd called out. Refuse an empty name before anchoring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turns tests/rimport/test_main.py and the e2e tests in test_cmdline.py green. Pointing rimport at a directory now publishes the files beneath it. Pre-flight splits on provenance. A named bad path keeps today's fatal all-or-nothing behavior, so a batch containing one is never half completed. A discovered bad path warns, is skipped, and lets its neighbours publish -- one broken symlink in a large tree blocking thousands of good files would make directory arguments useless. Skips are reported inline where they happen, then repeated on stderr at the very end, and earn the new exit code 3. The is_dir() guard in validate_source_path stays. After expansion a directory should never reach it, which is exactly what makes it worth keeping as a backstop against the 75c79cd corruption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main's docstring described only exit codes 0-2, leaving 3 (completed with skips) undocumented and making the 0 entry read as though a run with skips were unqualified success. Also annotates named_failures and to_stage, matching the file's existing convention of typing local collections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turns tests/rimport/test_build_parser.py green. rimport's --help is meant to stand on its own (69b3e47). Say that a directory argument is enumerated recursively and that a symlink to a directory is the carve-out, list all four exit codes now that 3 exists, and narrow --check's all-or-nothing claim to the names the user gave -- a file found by enumeration is skipped individually, not fatal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ilog line breaks RawDescriptionHelpFormatter preserves the epilog's exit-code list, but it also stops re-wrapping the description, which renders as one long line on a narrow terminal. Replace it with a subclass that only raw-formats the epilog. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rimport <unreadable-dir> <ok-dir>` exits 3 and publishes ok-dir anyway. A path the user NAMED that fails validation must be fatal -- exit 2, nothing published -- so this turns a hard stop into a partial publish. Skip carries no provenance, so main's gate cannot tell a named unreadable directory from one discovered inside a named tree. The second test pins what must NOT change: an unreadable directory discovered BENEATH a named one stays a warn-and-skip, so the fix cannot over-reach. It passes today and must keep passing. Red at HEAD by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tory Seeding named_failures from skips while leaving the message's denominator at len(entries) reports "1 of 0 file(s) failed pre-flight validation" for a lone unreadable directory, because a walk skip never produced an Entry. The exact wording is already treated as contract by three other tests, so pin it before the fix rather than discovering it afterwards. Also widens the nothing-published assertion to the whole staging tree, matching its neighbour, so a fix that publishes anything at all is caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A directory the user names that cannot be read comes back from walk_files as a Skip, and Skip carries no provenance, so main's gate could not see it as named. The run exited 3 and published every other named argument, turning a hard stop into a partial publish. Recognise it in main, which still holds the list of names the user gave. An unreadable directory discovered BENEATH a named one is untouched: the user did not name it, so it stays a warn-and-skip. Count it in the pre-flight message too. That denominator was len(entries), and a directory that could not be read never became an entry, so a lone one reported "1 of 0". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every skip should be reported twice: where it happened, and again in the end-of-run summary. Walk skips get neither -- nothing reports them inline, and the summary is unreachable past the fatal return -- so a skip during an aborted run is reported zero times. The same function also claims "no files found" for a directory it could not read, contradicting the Permission denied printed for that path. The third test pins what must NOT change: a NAMED unreadable directory is fatal, so it must not also be warned about as "skipping". It passes today and must keep passing. Red at HEAD by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every skip should be reported twice: where it happened, and again in the end-of-run summary. Walk skips got neither -- nothing reported them inline, and the summary is unreachable past the fatal return -- so an unreadable subdirectory went unmentioned in any run that aborted on a named failure. Warn about a discovered walk skip in place, matching main's wording so both halves of a skip report read the same. A skip whose path is the directory being walked is the one the user named, which main reports as fatal, so it is left alone: calling it "skipping" would contradict "nothing was published" about the same path. Also stops claiming "no files found under X" for a directory that could not be read. A genuinely empty directory produces no skips and still gets the warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard on walk skips asks "is this skip the directory this walk was rooted at?". The question that matters is "did the user name this path anywhere in the batch?" -- which is what main asks. Name a tree and an unreadable directory inside it, and the walk of the tree warns "skipping 'X'" for a path main then reports under "nothing was published": the contradiction the guard exists to prevent, reached from the other side. The second test pins the ordering property. Emitting warnings inside the walk loop pushes the expansion count -- the blast radius -- down one line per unreadable directory. Both fail at HEAD by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard on walk skips compared each one against the directory being walked. That is the right question only when a path is discovered or named, never both. Name a tree and an unreadable directory inside it and the walk of the tree fired "skipping 'X'" for a path main then listed under "nothing was published" -- the contradiction the guard was added to prevent, arrived at from the other side. It reproduced in both argument orders, so ordering could not fix it. Ask instead the question main asks: was this path named? Hoisting the loop out of the per-path walk makes that possible, and puts the expansion count -- the blast radius -- back at the top, where one warning per unreadable directory no longer pushes it down the screen. `paths` is now read twice, so it is materialised; the signature still says Iterable and a generator would otherwise be consumed by the first pass. Suite: 328 passed. Verified through the real CLI: naming both tree and tree/locked exits 2 with no "skipping" line in either order; naming tree alone still warns inline and exits 3; naming tree/locked alone exits 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main's docstring described exit 3 as discovered paths that "failed pre-flight validation". It also covers an unreadable directory beneath a named one, either cause alone being enough, which is what the code has done since walk skips were introduced. Verified: a directory whose only problem is an unreadable subdirectory exits 3. Also states the provenance rule the docstring left implicit: only discovered paths are skipped; the same failure on a named path is fatal and exits 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Path.is_dir() propagates EACCES rather than returning False -- it ignores only ENOENT, ENOTDIR, EBADF and ELOOP -- so naming a file under an unreadable directory escapes main as a traceback and exits 1, where --help promises exit 2. Red at HEAD by design. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Path.is_dir() propagates EACCES rather than returning False, so naming a file under an unreadable directory escaped main as a traceback and exited 1, where --help promises exit 2. The probe now catches OSError and records it as a Skip, letting main decide how to route it. Every path reaching expand_directories was NAMED by the user, so main's provenance split sends it to the fatal pre-flight block: the user gets exit 2 and a message naming the path and the reason. Suite: 329 passed. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The guard on is_dir() was justified on the grounds that Path.is_dir()
swallows EACCES on 3.9-3.12 and only 3.13 re-raises, making the crash
invisible to CI. That is false. pathlib._IGNORED_ERRNOS is
{ENOENT, ENOTDIR, EBADF, ELOOP} on 3.11.9 and 3.13.2 alike; EACCES is in
neither, and the pre-fix code reproduces the traceback and exit 1 under
both.
The fix itself was already right and is unchanged. What changes is the
reason recorded beside it: a maintainer reading "Python 3.13's is_dir()"
in a repo whose CI pins 3.9-3.12 would reasonably read the guard as a
local-interpreter workaround CI cannot exercise, and delete it. The
truth is better news -- the bug affects every supported version, and the
test is red without the fix across the whole CI matrix.
Corrected in the code comment and the test docstring. Also fixes
expand_directories' Returns docstring, which described skips as only
walk failures; it also carries a named path whose is_dir() probe raised.
Suite: 329 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Directory arguments are enumerated recursively, with the directory itself never staged or symlinked. A symlink to a directory is not expanded. Discovered failures are skipped and summarised while a name you gave directly is still fatal. This commit also removes "a directory" from the pre-flight failure list—which this branch made false—and replaces the prose exit code documentation with a structured table. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The directory-arguments section used -- where the rest of the README uses an em dash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
expand_directories' Returns docstring said "Both are named, so main reports them as fatal pre-flight failures". That is false for the dominant case: a directory that could not be read DURING A WALK is discovered unless the user also typed it, and stays a warn-and-skip with exit 3, which is what test_unreadable_subdirectory_stays_a_skip pins. Verified: --check on a tree with an unreadable subdirectory exits 3. The README also still promised all-or-nothing over every file processed. That is false for discovered files. The equivalent --check blurb in --help had already been rescoped to named paths; its README counterpart was missed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
expand_directories de-duplicates the files it discovers but not the skips, and it re-walks a directory named twice. So naming the same directory twice, or a directory alongside its own parent, records the same unreadable subdirectory twice: warned twice inline, listed twice in the end-of-run summary, counted twice in "N file(s) skipped", and -- when the user named the path -- counted twice in the fatal block's numerator and denominator. n_dirs has the same shape of bug: it counts walks, so a directory named twice reports "expanded 2 director(ies)". Four tests: the walk count, the duplicated skip by both routes, and the inflated fatal count end to end. Red at HEAD by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Skip gains a `named` field, defaulting False so walk_files can still build one with two arguments. expand_directories is the only place that knows the whole batch, so it is the only place that fills it in: it collapses skips by path and stamps each with whole-batch membership. main then reads the field instead of rebuilding `set(paths)` for itself. That second copy was the hazard. The inline-warning guard was correct only because expand_directories' set and main's set were guaranteed identical, an invariant nothing enforced and no test would have caught breaking. Deriving it once removes the possibility. Duplicate arguments now collapse before the walk, so a directory named twice is walked once rather than reporting "expanded 2 director(ies)" and finding everything beneath it twice. Verified through the real CLI: `<tree> <tree>` and `<tree> <tree/sub>` each warn once, list once, and say "1 file(s) skipped" instead of two of each; naming an unreadable directory twice reports "1 of 2 file(s) failed" listing it once, where it said "2 of 3" and printed the path twice. Suite: 333 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The epilog described exit 2 as "a name you gave failed validation", which is one of several triggers -- a bad command line, a missing or empty --list file and an undeterminable working directory all produce it too, as main's own docstring has always said. And nothing in --help stated that the codes are not mutually exclusive or which wins, though the README documents 2 > 1 > 3 > 0. The epilog is where a script author looks, and exit 3 is new and scriptable. Adds the named-vs-discovered rule in one line, since that is the mental model the exit codes turn on. Also rewraps the three help strings that ran past 100 characters, which is why they are in this commit rather than left to a formatting pass: they are the same strings this change is about. Rendered output is unchanged -- argparse rewraps them anyway -- and no line of --help exceeds 78 columns at COLUMNS=80, verified on 3.13.2 and on 3.11.9 inside the CI range. test_help_documents_all_four_exit_codes still passes: it asserts the section exists and lists 0: through 3:, not the wording. Suite: 333 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--file's and --list's help text both promise that a name may be a directory, but only the positional channel was tested. All three funnel through get_files_to_process -> normalize_paths -> expand_directories, so the other two work by construction -- but nothing stopped a future change to read_filelist or to anchoring from silently breaking the --list half. These pass at HEAD rather than starting red, because the behaviour is already correct and the gap is coverage. So they were mutation-tested instead: with expansion disabled in a scratch copy, both fail. The --list case uses a relative entry, which also pins that a directory entry anchors to the list file's own directory exactly as a file entry does. Both assert the directory itself is still a real directory afterwards, the invariant 75c79cd was added to protect. Suite: 335 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A named file outside the inputdata root exits 2. A named directory outside it is expanded first, so its contents become discovered failures and the run exits 3 -- a softer verdict for the same user error, and one that contradicts the README's "a bad name you gave directly is still fatal". It is also walked before being rejected, so a mistyped `rimport ~` recurses an arbitrary tree and reports every file in it. Red at HEAD by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A named directory outside the inputdata root was expanded and walked before anything rejected it, so the user's own bad argument became a heap of discovered skips and the run "finished" with exit 3. A named file outside the root has always exited 2. Same error, two verdicts, and the softer one contradicts the README. expand_directories now takes the root and declines to expand anything outside it, leaving the path as a named entry for the pre-flight gate that already knows how to judge it. So `rimport ~` no longer recurses an arbitrary tree as cesmdata before rejecting every file in it. validate_source_path checks containment before the is-a-directory backstop, so an out-of-root directory now names the reason the user can act on rather than "source is a directory, not a file", which invites the reply that directories are supported now. The backstop itself is unchanged for a directory inside the tree, and the messages for a missing file, a file outside the root, and a file already under staging are all byte-identical. Verified through the real CLI across those five shapes. The two scoping tests were mutation-tested: deferring the check until after the walk fails both. Suite: 339 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reorder of validate_source_path -- the riskiest part of the previous change, since that function gates every path on every run -- was pinned by nothing: reverting it left the whole suite green. The existing directory test uses a directory inside the root, so it exercises the backstop but cannot tell the two orderings apart. A directory outside the root now has its own test, and the main-level test asserts the reason string rather than just the exit code. Both fail against the reverted order. The resolve-rather-than-lexical scope test was likewise unpinned; a directory reached through a symlinked parent now covers it. README and the positional help both still promised that any name may be a directory and everything beneath it is enumerated, which stopped being true when expansion was scoped to the tree. The --file help already carried the containment qualifier, so the two channels documented the same behaviour differently. validate_source_path's docstring listed its checks in the pre-reorder order. Suite: 341 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 99.14% 99.31% +0.16%
==========================================
Files 28 31 +3
Lines 2937 4243 +1306
==========================================
+ Hits 2912 4214 +1302
- Misses 25 29 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The README stated the carve-out without its reasons, so it read as an arbitrary exception rather than a consequence of what a symlink means in a published inputdata tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
samsrabin
force-pushed
the
fix-rimport-on-dir
branch
from
September 9, 2026 19:00
a300baf to
8d7705b
Compare
samsrabin
commented
Sep 10, 2026
`is_real_dir` stopped describing its variable once an out-of-root directory started clearing it: it is true only for a real directory inside the tree, which is what "expandable" says. Reverts an unrelated rename of the staging loop's `p`, which had no reason beyond churn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A directory can be the thing that failed pre-flight or was skipped, so the fatal line, the skip summary and the two --help strings that describe them now say "item(s)". The expansion count keeps "file(s)": it only ever counts files, since a walk yields no directories. Reasons now render through reason_text(), which uses OSError.strerror rather than str(). Both messages already name the path, and OSError's str() appends it again, so every unreadable-directory line carried it twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test docstrings had become a running commentary on the branch's own development -- "the counterpart", references to task numbers and commit hashes, and descriptions of behaviour that used to be wrong. None of that helps someone meeting a test cold, and "the counterpart" had no referent at all once the tests moved apart. Each now says, in the present tense, what the test pins. One was outright false: a docstring still explained that Skip carries no provenance, which stopped being true when it gained a `named` field. Also renames test_directory_named_twice_is_walked_once to ..._is_reported_once, and drops a docstring's list of which paths in a fixture are invalid, which would drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_expansion_count_is_logged_before_staging asserted only that the count line appeared, so it passed regardless of where it appeared; it now compares its position against the first staged-file line. The --check-on-a-directory test said in a comment that the directory itself is never described, and checked nothing of the sort. It now asserts on the per-item header, which is the only form that can distinguish the directory from the file beneath it -- the directory's name appears in that file's path either way. The named-unreadable-directory test had one good file, so its denominator matched the argument count and could not tell the two apart. A second good file separates them. Adds the reverse argument order for named-wins-over-discovered, which takes a different route through the accumulator, and gives the first-seen test a fixture where order is actually observable. Suite: 342 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--check had one test, for exit 3, sitting well away from the run it mirrors, and its docstring claimed to cover "the same exit codes" while checking one. Each of the four exit-code paths now has a --check twin immediately after its non-check equivalent, so a divergence between the two modes shows up as an adjacent pair disagreeing. The exit-0 and exit-3 twins assert nothing was written, which is what makes them catch --check being ignored. The exit-2 twin instead pins that the good file is never reported on, since the gate runs before any checking; the exit-1 twin pins that precedence 1 > 3 holds when nothing was being written at all. Suite: 345 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The table had been sitting inside "Directory arguments" because it replaced a bullet in that part of the file, but it describes every run. It now stands on its own, says "items" where a directory can be the thing skipped, and carries the named-vs-discovered rule that makes 2 and 3 predictable to a script. Drops the symlink bullet that explained the carve-out by reference to how things used to work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
reason_text changed the text of every skip and every pre-flight failure line, and nothing asserted the new form: reverting it to str() left the suite green. The walk-skip test now pins that the reason names the error without repeating the path the line already carries. The --check twin for exit 1 mocks stage_data, so --check was swallowed by the mock and the test passed identically without the flag. It now asserts the flag reaches stage_data, which is the only place it changes anything. The exit-3 table says "item(s)" because a directory can be the thing skipped, but every sentence explaining exit 3 still said "file" -- including one added two lines under that table. They now say "anything found by expanding a directory", and name the unreadable-subdirectory case that motivated it. reason_text's docstring claimed more than it delivers: the RuntimeErrors from validate_source_path carry no strerror and several write the path into their own text, so those lines are unchanged. Restores, in the README, what naming a symlink to a directory actually does, which was lost with the bullet that explained it by history. Suite: 345 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
samsrabin
force-pushed
the
fix-rimport-on-dir
branch
from
September 10, 2026 02:09
ad0be5b to
d1d8992
Compare
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
2 participants
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.
I previously added a guard to
rimportthat would throw an error if a directory was included because I didn't like the behavior. That's now removed: A directory argument is now expanded recursively into the files beneath it, and each of those files goes through the existing per-file publishing path. This differs from before, when the entire directory would be copied to staging and symlinked.Also makes it so that skipped files are summarized at the end of the printout (stderr).
Remaining tasks: