change record: --files rejects the list of paths its name and usage string promise #128

Closed
opened 2026-07-28 19:47:18 +01:00 by allod-agent · 0 comments
Member

Make allod change record's --files accept the list of paths its name and its usage string both promise, so --files a b stages both instead of aborting on the second.

Primary goals:

  • A plural flag takes plural values--files consumes every following path up to the next option, and repeating the flag still accumulates, so both spellings work and combine.
  • The documented form works[--files <file>...] is what the usage string has always said; the parser is the half that disagrees with it.
  • The prescribed workflow works as written — naming files when recording in a shared checkout is the documented defense against sweeping another agent's edits, so its invocation form should not be a trap.
  • No new ambiguitychange record takes no positional arguments, so consuming up to the next --prefixed token cannot swallow anything else.

Current state

allod change record -m <message> [--files <file>...] (allod:37, and identically for -M at :38). The trailing ... reads as "this option takes one or more values", which is what a flag named --files should do.

The parser consumes exactly one:

-f|--files)
  require_option_value "$1" "$#"
  files+=("$2")
  shift 2
  ;;

at allod:530-533. A second path therefore reaches the catch-all at allod:542-543 and aborts with unexpected argument for change record: <path>. Repeating the flag works, because files+= accumulates across occurrences:

allod change record -m msg --files a --files b        # works
allod change record -m msg --files a b                # dies on `b`

Observed while recording a two-file change in allod/nexus.

Why it is worth fixing

git-workflow.md instructs agents to name files when recording in a shared checkout, because record otherwise runs git add -u (allod:552-556), which stages another agent's tracked edits — the concern behind allod/tools#118. So the one invocation shape the memory prescribes as the safe path is also the shape whose documented form is rejected.

The failure itself is benign and loud: it dies before git add, touches nothing, and names the offending argument. This is a usability defect, not a safety one, and should not be triaged as though staging were at risk.

Change

Have -f|--files consume paths until the next --prefixed token, so --files a b, --files a --files b, and -f a b --files c all name the same set. Order stays free, because a following option ends the list: --files a b -m msg and -m msg --files a b are equivalent. A list that consumed nothing keeps failing with the existing requires a value wording rather than falling through to the git add -u sweep, and -- ends option parsing so a path beginning with - stays reachable. The usage strings at :37 and :38 are already accurate and stay as they are.

This reverses the direction first recorded in this issue, which named correcting the usage string as the default and the variadic parser as the alternative. That was backwards. The usage string described what a flag named --files should do; the parser was the defect, and editing the documentation to match it would have documented the bug.

Scope

In: the -f|--files parser for change record, the -- terminator it needs, and tests covering the variadic form, the repeated form, their combination, flag order in both directions, an empty list, and a path beginning with -. Out: the -f/--files short-flag spelling itself, which is fine and matches forge; the git add -u fallback behavior; and the shared-checkout sweeping concern in allod/tools#118, which this only touches through its file-naming flag.

Residual risk

R0. Argument parsing for one subcommand, failing loud today and before any git mutation. Every invocation that works today keeps working — a single --files a and the repeated --files a --files b parse exactly as before — and the forms this adds all abort today, so nothing can regress from working to broken.

Make `allod change record`'s `--files` accept the list of paths its name and its usage string both promise, so `--files a b` stages both instead of aborting on the second. Primary goals: - **A plural flag takes plural values** — `--files` consumes every following path up to the next option, and repeating the flag still accumulates, so both spellings work and combine. - **The documented form works** — `[--files <file>...]` is what the usage string has always said; the parser is the half that disagrees with it. - **The prescribed workflow works as written** — naming files when recording in a shared checkout is the documented defense against sweeping another agent's edits, so its invocation form should not be a trap. - **No new ambiguity** — `change record` takes no positional arguments, so consuming up to the next `-`-prefixed token cannot swallow anything else. ### Current state `allod change record -m <message> [--files <file>...]` (`allod:37`, and identically for `-M` at `:38`). The trailing `...` reads as "this option takes one or more values", which is what a flag named `--files` should do. The parser consumes exactly one: -f|--files) require_option_value "$1" "$#" files+=("$2") shift 2 ;; at `allod:530-533`. A second path therefore reaches the catch-all at `allod:542-543` and aborts with `unexpected argument for change record: <path>`. Repeating the flag works, because `files+=` accumulates across occurrences: allod change record -m msg --files a --files b # works allod change record -m msg --files a b # dies on `b` Observed while recording a two-file change in `allod/nexus`. ### Why it is worth fixing `git-workflow.md` instructs agents to name files when recording in a shared checkout, because `record` otherwise runs `git add -u` (`allod:552-556`), which stages another agent's tracked edits — the concern behind allod/tools#118. So the one invocation shape the memory prescribes as the safe path is also the shape whose documented form is rejected. The failure itself is benign and loud: it dies before `git add`, touches nothing, and names the offending argument. This is a usability defect, not a safety one, and should not be triaged as though staging were at risk. ### Change Have `-f|--files` consume paths until the next `-`-prefixed token, so `--files a b`, `--files a --files b`, and `-f a b --files c` all name the same set. Order stays free, because a following option ends the list: `--files a b -m msg` and `-m msg --files a b` are equivalent. A list that consumed nothing keeps failing with the existing `requires a value` wording rather than falling through to the `git add -u` sweep, and `--` ends option parsing so a path beginning with `-` stays reachable. The usage strings at `:37` and `:38` are already accurate and stay as they are. This reverses the direction first recorded in this issue, which named correcting the usage string as the default and the variadic parser as the alternative. That was backwards. The usage string described what a flag named `--files` should do; the parser was the defect, and editing the documentation to match it would have documented the bug. ### Scope In: the `-f|--files` parser for `change record`, the `--` terminator it needs, and tests covering the variadic form, the repeated form, their combination, flag order in both directions, an empty list, and a path beginning with `-`. Out: the `-f`/`--files` short-flag spelling itself, which is fine and matches `forge`; the `git add -u` fallback behavior; and the shared-checkout sweeping concern in allod/tools#118, which this only touches through its file-naming flag. ### Residual risk R0. Argument parsing for one subcommand, failing loud today and before any git mutation. Every invocation that works today keeps working — a single `--files a` and the repeated `--files a --files b` parse exactly as before — and the forms this adds all abort today, so nothing can regress from working to broken.
allod-agent changed title from change record: --files usage string implies a variadic list the parser rejects to change record: --files rejects the list of paths its name and usage string promise 2026-07-28 20:57:00 +01:00
vnprc closed this issue 2026-07-28 21:14:18 +01:00
Sign in to join this conversation.
No description provided.