Use one duration parser and fix the --older-than months example (closes #123) #140

Merged
clawbot merged 1 commits from issue-123-single-duration-parser into next 2026-09-21 20:58:31 +02:00
Collaborator

Closes #123.

Two parseDuration functions existed with different grammars. snapshot purge --older-than and --keep-newer-than (which routes through the same path) both parsed via the copy in internal/vaultik/helpers.go; the richer copy in internal/cli/duration.go was reachable only from its own test. I kept the live-path parser and deleted the unused one, so no flag's accepted grammar changes.

The trap the issue names: README.md documented 6m as the months example for --older-than, but m is minutes, so that command removed every snapshot older than six minutes — a destructive default. Corrected to 6mo, and both flags' --help now carry the same example list stating m is minutes and mo is months.

The surviving parser now rejects negative durations, which it previously accepted (-5h) or silently made positive (-5d); the issue's definition of done requires that rejection. Table-driven tests cover every unit, 6m as six minutes, 6mo as 180 days, and rejection of a bare number, an unknown unit, and a negative.

Disclosure: this arrived as one pushed commit from a stopped worker. I rebased it onto current next (keeping all TODO.md entries), reviewed it against the definition of done, and confirmed the negative-rejection tests fail without the source change. make check green.

model: claude-opus-4-8

Closes https://git.eeqj.de/sneak/vaultik/issues/123. Two `parseDuration` functions existed with different grammars. `snapshot purge --older-than` and `--keep-newer-than` (which routes through the same path) both parsed via the copy in `internal/vaultik/helpers.go`; the richer copy in `internal/cli/duration.go` was reachable only from its own test. I kept the live-path parser and deleted the unused one, so no flag's accepted grammar changes. The trap the issue names: `README.md` documented `6m` as the months example for `--older-than`, but `m` is minutes, so that command removed every snapshot older than six minutes — a destructive default. Corrected to `6mo`, and both flags' `--help` now carry the same example list stating `m` is minutes and `mo` is months. The surviving parser now rejects negative durations, which it previously accepted (`-5h`) or silently made positive (`-5d`); the issue's definition of done requires that rejection. Table-driven tests cover every unit, `6m` as six minutes, `6mo` as 180 days, and rejection of a bare number, an unknown unit, and a negative. Disclosure: this arrived as one pushed commit from a stopped worker. I rebased it onto current `next` (keeping all `TODO.md` entries), reviewed it against the definition of done, and confirmed the negative-rejection tests fail without the source change. `make check` green. model: claude-opus-4-8
clawbot self-assigned this 2026-09-21 20:08:01 +02:00
clawbot added 1 commit 2026-09-21 20:08:02 +02:00
Two functions named parseDuration existed with different grammars. --older-than and --keep-newer-than both already parsed through the one in internal/vaultik; the richer copy in internal/cli/duration.go was reachable only from its own test. Kept the live-path parser and deleted the unused one, so no flag's accepted grammar changes. README documented 6m as the months example for --older-than, but m is minutes, so that command deleted every snapshot older than six minutes; corrected it to 6mo and put both flags' help on one example list stating m is minutes and mo is months. The parser now rejects negatives it used to accept (-5h) or silently made positive (-5d).

Model: opus-4-8
clawbot added the needs-review label 2026-09-21 20:08:10 +02:00
Author
Collaborator

PASS

model: claude-opus-4-8

PASS model: claude-opus-4-8
clawbot merged commit 89ebfc78e2 into next 2026-09-21 20:58:31 +02:00
clawbot deleted branch issue-123-single-duration-parser 2026-09-21 20:58:31 +02:00
Sign in to join this conversation.