fleetd #635 follow-up: fix the stale --restore not-found message (defect 6, criterion 14)
CI / shell-tests (pull_request) Failing after 9s
CI / contract (pull_request) Successful in 55s
CI / build (pull_request) Failing after 2m48s

The --restore "no backup found" message still printed the old beside-the-config glob
(${CONFIG}.bak.*) even though newest_backup had already moved to searching the managed
.config-backups/ directory. The message was left behind when the search moved — the search
itself was already correct (ticket comment 17664). Fix is reporting-only: the message now
names the directory actually searched (via backup_dir_for), and separately says that a
backup written the old way, directly beside the config, is not searched any more, with the
one-line cp to recover one by hand. No search fallback was added — reading backups from
outside the managed directory stays unsupported, as instructed.

Acceptance criterion 14 proves both directions: the not-found message names the real
directory (confirmed red on the pre-fix code, green after), and a restore with a real backup
present in .config-backups/ still succeeds (confirmed this catches an "always not-found"
regression that direction 1 alone would miss).

All 14 criteria plus 3 extras pass in scripts/test-config-edit.sh.
This commit is contained in:
Dai Ha
2026-10-01 18:25:08 +02:00
parent 4eb720029c
commit 096f08c866
2 changed files with 57 additions and 2 deletions
+15 -2
View File
@@ -612,9 +612,22 @@ dry_run_diff() {
restore_mode() {
[ -f "$CONFIG" ] || die "no config at $CONFIG to restore onto"
local backup
local backup dir base
backup="$(newest_backup "$CONFIG")"
[ -n "$backup" ] || die "no backup found matching ${CONFIG}.bak.* — nothing to restore"
if [ -z "$backup" ]; then
# fleetd #635 follow-up (ticket comment 17664) — this message must name the directory the
# code actually searches (backup_dir_for, same as newest_backup), not the old beside-the-
# config glob. A backup written the OLD way is real and NOT searched any more — say so and
# give the one-line recovery command — but do NOT make the search itself look there; that
# would be a behaviour change nobody asked for. The message is the only thing being fixed.
dir="$(backup_dir_for "$CONFIG")"
base="$(basename "$CONFIG")"
die "no backup found matching ${dir}/${base}.bak.* — nothing to restore.
A backup written the OLD way, directly beside the config (${CONFIG}.bak.*), is NOT
searched — that location was retired so a backup of a file that must never be committed
cannot sit next to a tracked directory. If one exists there, recover it by hand:
cp ${CONFIG}.bak.<timestamp>.<pid> $CONFIG"
fi
[ -f "$backup" ] || die "backup candidate $backup vanished"
say "restore"
+42
View File
@@ -342,6 +342,46 @@ test_file_mode_survives_edit_and_restore() {
done
}
# ----------------------------------------- acceptance criterion 14: restore message names the real directory
# fleetd #635 follow-up (ticket comment 17664, defect 6) — the --restore "no backup found"
# message used to print the OLD beside-the-config glob even though newest_backup had already
# moved to searching the managed directory. Proves BOTH directions: the not-found message names
# the directory actually searched (not merely that it says SOMETHING), and that a real backup
# sitting in that directory still lets --restore succeed — otherwise the fix could regress into
# a message that is always printed regardless of whether a backup exists.
test_restore_message_names_the_searched_directory() {
local dir rc=0
# Direction 1: no backup anywhere — the message must name .config-backups/, not the bare
# beside-the-config glob the OLD code printed.
dir="$(new_fixture)"
"$EDIT" --restore --config "$dir/fleetd.yaml" --log "$dir/fleetd.out" --wait-seconds 2 \
> "$dir/stdout.log" 2>&1 || rc=$?
RUN_OUTPUT="$(cat "$dir/stdout.log")"
assert_equals 1 "$rc" "--restore with no backup anywhere exit code"
assert_contains ".config-backups/fleetd.yaml.bak.*" "$RUN_OUTPUT" \
"the not-found message must name the directory actually searched, not the old beside-the-config glob"
# Direction 2: a real backup IS present in .config-backups/ — --restore must still succeed, so
# the message fix cannot have turned into one that prints regardless of whether a backup exists.
dir="$(new_fixture)"
cp "$dir/fleetd.yaml" "$dir/pre-edit.yaml"
start_run "$dir" 5 --set '.profiles.sonnet.weight=77'
sleep 1
printf 'config reloaded\n' >> "$dir/fleetd.out"
collect_run "$dir"
assert_equals 0 "$RUN_RC" "setup edit exit code for criterion 14's second half"
start_run "$dir" 5 --restore
sleep 1
printf 'config reloaded\n' >> "$dir/fleetd.out"
collect_run "$dir"
assert_equals 0 "$RUN_RC" "--restore with a real backup present must still succeed"
cmp -s "$dir/fleetd.yaml" "$dir/pre-edit.yaml" \
|| fail "--restore with a real backup present must put the file back byte for byte"
}
# dry-run must never touch the live file and must still redact.
test_dry_run_never_installs_and_redacts() {
local dir before
@@ -409,6 +449,8 @@ echo "== acceptance criterion 11: a backup is never committable =="
test_backup_is_never_committable
echo "== acceptance criterion 12: the file mode survives an edit and a restore =="
test_file_mode_survives_edit_and_restore
echo "== acceptance criterion 14: the restore message names the directory actually searched =="
test_restore_message_names_the_searched_directory
echo "== extra: dry-run never installs, and redacts =="
test_dry_run_never_installs_and_redacts
echo "== extra: --check is read-only and always exits 0 =="