Skip to content

repo-hygiene: clean-batch.sh rejects a --repos-from/--skip-from file whose last line is blank, with "file not found" and exit 2 #3482

Description

@kyle-sexton

Found by the repo-wide /code-tidying:batch-simplify sweep (group G60). Reported, not fixed — the sweep is behavior-preserving, and the fix lives outside the reporting group's file list.

What happens

plugins/repo-hygiene/skills/clean/scripts/clean-batch.sh lines 130 and 140 reject a perfectly valid input file whenever its last line is blank, reporting clean-batch.sh: file not found: <path> and exiting 2 — even though the file exists and every entry in it parsed correctly.

Reproduced:

  • repos file containing a\n\nclean-batch.sh: file not found: …, exit 2
  • repos file containing a\nb\n → proceeds normally

A file that ends with a newline followed by a blank line is an ordinary way to end a text file, and the diagnostic points at the wrong thing entirely: it claims the file is missing.

Root cause

batch_read_lines_into in plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.sh returns the status of the last command executed in its read loop. On a trailing blank line that last command is the [[ -n "" ]] && … test, which is false, so the function returns 1 — despite having read every entry successfully. clean-batch.sh checks that status and treats a non-zero return as "file not found".

Why this looks unintended rather than deliberate

The sibling command is immune to the same input. plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.sh lines 143 and 154 pre-check [[ -f "$2" ]] and then ignore batch_read_lines_into's return value, so it accepts a trailing blank line without complaint. Two commands reading the same list format through the same helper disagree about whether a blank final line is an error.

Suggested direction

The load-bearing question is what batch_read_lines_into should mean by its exit status. Returning the last loop iteration's test result conflates "I could not read this" with "the last line happened to be empty". A fix in the helper — returning success once the file has been read, and signalling unreadability separately — would correct both call sites at once and let git-tree-reset-batch.sh drop its defensive pre-check. Worth checking whether any other caller depends on the current return.

Note that fixing this changes behavior: invocations that currently fail with exit 2 would start succeeding.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions