From 06b077db40795049330d5679c38d0a5ee981e533 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 16 Aug 2026 04:16:14 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20pos=20media=20sync=20=E2=80=94=20strip?= =?UTF-8?q?=20all=20trailing=20slashes=20+=20mark=20partial=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENT_TODO.md | 1 + DOC/AGENT_Context_Project.md | 2 +- bin/pos-media-sync | 29 ++++++++++++++++++++--------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/AGENT_TODO.md b/AGENT_TODO.md index c437d1b..b12ba62 100644 --- a/AGENT_TODO.md +++ b/AGENT_TODO.md @@ -42,6 +42,7 @@ summary (newest last). ## Done +- **2026-08-16** — `pos media sync` review follow-up (on ba12a41): (1) **strip ALL trailing slashes** — `SRC="${SRC%/}"` removed only ONE slash, and GNU find preserves a doubled one on the starting point (`find -H /x// -type f` emits `/x//Album/a.mp3`), so `--source /x//` / `MEDIA_SYNC_SOURCE=…//` still hit the original nesting bug (prefix `/x//` never matched); now `while [[ "$SRC" == */ ]]; do SRC="${SRC%/}"; done`, which also collapses a lone `/` or `//` to empty → guard errs instead of mirroring the filesystem root. (2) **partial-sync marker** — the "find reported problems" condition is computed once (`find_ok=1/0` at the warn block) and reused: when set, the final `ok "Sync complete: …"` and `notify_send "Music sync completed: …"` both append ` (partial — find reported problems)` so the success signal can't contradict the warning (dry-run line untouched). (3) **clear empty-source error** — the post-normalization guard now says `Source path is empty` instead of `Source not found: ` with a blank value; the `-d` guard keeps `Source not found: $SRC` for non-empty missing paths. Docs untouched (ba12a41 wording kept). Verified: harness extended (`NOTIFY_LOG` seam: telegram-sender stub appends its argv, run_sync passes `NOTIFY_LOG`; new §7 `--source …//`/`…///` + env `…//` → correct rel placement, no `Music/tmp` nesting; §8 `--source /`, `//`, `''` → exit 1 + "Source path is empty"; §3 now asserts the partial marker on both the ok line and the notify log; §1 negative: no marker on clean runs) — full suite **51/51 green** on the fixed tool, and against a ba12a41 snapshot it fails ONLY the follow-up assertions (double/triple-slash nesting ×5, partial markers ×2, empty-source message ×2; 8b `--source //` excluded from the pre-run because the old code would have mirrored `/`). `make gen && make check` green, `make lint` 0 FAIL / 0 WARN. - **2026-08-16** — `pos media sync` hardening (3 approved fixes): (1) **trailing-slash source** — with `--source /x/` (or `MEDIA_SYNC_SOURCE=…/`) GNU find normalizes the slash on the starting point, so the rel prefix `"${f#"$SRC/"}"` became `/x//` which never matched and every file silently nested under `/Music//data/Music/...` each run; `SRC` is now `%/`-normalized with a non-empty guard right after option parsing, before the `-d` check (covers flag + env forms). (2) **silent find failure → false "Sync complete"** — both `find -H` invocations ran through process substitution, which hides find's exit code and stderr from `set -e`/`pipefail`: an unreadable subdir made find exit 1 with "Permission denied" yet the tool announced a full success on a partial tree. find now runs ONCE into a temp list + temp stderr (`|| find_rc=$?`, sorted in place with `sort -o`, temp files removed via `trap EXIT`); rc != 0 or non-empty stderr prints a visible `warn` ("results may be incomplete") with the find stderr lines indented (spawn-style) instead of continuing silently; both the space scan and the copy loop read the same captured list, so the double find scan is gone and the file set is identical. (3) **inner symlinks silently skipped** — `find -H` follows only the command-line source symlink, so symlinks inside the tree never synced; an inner-symlink count (`find -H "$SRC" -mindepth 1 -type l`, root symlink excluded) now warns "N symlink(s) inside the source are not followed (find -H) — their targets will not be synced". `find -H` kept (no `-L`); `needs_copy` mtime logic untouched (FAT32 granularity deferred). Docs: howto/media.md symlink paragraph notes inner symlinks are skipped with a count warning; AGENT_Context filetable regenerated (pos-media-sync 164→202). Verified: stub harness `/tmp/opencode/media-sync-fix-test/` (lsblk JSON fixture with rm=true/type=part/mountpoint/TRAN=usb, `y` confirm, `MEDIA_SYNC_SOURCE`/`MEDIA_SYNC_DEST`/`USB_BYID` seams, HOME isolation, telegram-sender stub) 32/32 green — trailing-slash `--source` and env forms land under `/Music/` with no absolute-path nesting, unreadable subdir → visible "results may be incomplete" warn + indented find stderr + exit 0 + accessible files still copied, inner symlinks → count warning, symlink root still works with NO inner-symlink warning (23d69b7 regression incl. trailing-slash combo), re-run after success → `0 added, 0 updated, 2 unchanged`; the same suite against the pre-fix script fails exactly those assertions (24/32). `make gen && make check` green, `make lint` 0 FAIL / 0 WARN. - **2026-08-15** — `pos docker stack` (`bin/pos-docker-stack`) — containers grouped by their Docker Compose project. Each stack is a section (project name, sorted) with lines `container-name status ports`; containers with no compose project land in a `Standalone` section at the end; ends with `Stacks: N containers: N standalone: N`. Running only by default, `-a|--all` includes stopped/exited (like `docker ps -a`). Status colored on a terminal (`Up*` green, `Exited*`/`Dead*`/`Created*` red, `Paused*`/`Restarting*` yellow); exit 0 also when no containers. Data via `docker ps` with `--format '{{.Names}}{{"\u001f"}}{{.Label "com.docker.compose.project"}}{{"\u001f"}}{{.Status}}{{"\u001f"}}{{.Ports}}'` (compose v2 sets the project label; `{{"\u001f"}}` escapes in the Go template), parsed with `awk -F'\x1f'` + `IFS=$'\x1f' read` everywhere — tab/pipe delimiters are IFS whitespace or inside values, so `\x1f` (DEV.md:213 gotcha); dash padding via `sed` not `tr` (tr corrupts multi-byte `─`). Deps guard (`docker`) before `--help`; no stdin → not in `INTERACTIVE_CMDS`; `# POS_FLAGS: -a --all`. Docs: POS.md docker row + detail, howto/docker.md table + section, `bin/pos` usage EXAMPLES, AGENT_Context §14 row. Verified: stub-PATH suite `/tmp/opencode/docker-stack-test/run-tests.sh` 23/23 (grouping, sorted stacks, `-a` shows exited, standalone, empty daemon rc=0, colored status, missing docker rc=1, `--help` after deps guard); live runs against the real daemon (affine/audiobookshelf/convertx/gitea stacks, `affine_migration_job Exited (0)` + `lab1 Exited (137)` under `-a`); dispatch via `pos docker stack`; `make gen && make check`, `make lint` 0 FAIL / 0 WARN. - **2026-08-15** — Fix `pos media sync` offering a Ventoy stick's **EFI partition** as the sync target: with the data partition unmounted, the 32 MB `VTOYEFI` ESP was the only mounted USB partition, `usb_detect` offered it with no context, and `cp` died mid-copy with `No space left on device` (live-box report). `usb_detect` now fetches `FSTYPE`/`PARTTYPENAME` and excludes EFI system partitions (Ventoy `VTOYEFI`, `/boot/efi`) from **both** the mounted list and the mount-offer list; `USB_MOUNTED` entries carry `mp|label|size|model|fs` and `usb_pick_root` shows that in the single-stick confirm and the multi-stick/partition picker (`1) /media/Ventoy (1.1T, Ventoy, exfat)`), while `USB_ROOT` stays a bare mountpoint (`${root%|*}`) so `pos system backup` (`${root%/}/backups`) is unaffected. `pos-media-sync` gained a pre-flight space check (measures exactly what `needs_copy` would copy vs `df -Pk`, `err`/`warn` before any copy) — no more mid-copy ENOSPC. Docs: howto/media.md target-picking note, SCRIPTS.md usb-lib paragraph, AGENT_Context hand-maintained lib row (194→205). Verified: new stub harness `/tmp/opencode/vtoyefi-run.sh` (ESP filtered from mounted + mount-offer, multi-pick shows only the data partition, space fit/too-small/dry-run-warn) green; `/tmp/opencode/backup-test` still green; live check `printf 'n\ns\n' | bash bin/pos-media-sync --mp3` no longer offers VTOYEFI (offers unmounted `sda1` Ventoy instead); `make gen && make check`, `make lint` 0 FAIL / 0 WARN. diff --git a/DOC/AGENT_Context_Project.md b/DOC/AGENT_Context_Project.md index 3560c98..23d4302 100644 --- a/DOC/AGENT_Context_Project.md +++ b/DOC/AGENT_Context_Project.md @@ -602,7 +602,7 @@ Use conventional prefixes: `feat:`, `fix:`, `docs:`, `refactor:`, `chore:` | `bin/pos-entertainment-status` | 62 | Show enabled plugins and scheduler state | | `bin/pos-media-mp3` | 86 | Download audio as MP3 (yt-dlp) | | `bin/pos-media-mp4` | 132 | Download video as MP4 (smart/interactive format select) | -| `bin/pos-media-sync` | 202 | Incremental Music → USB sync (mp3/mp4, add/update only) | +| `bin/pos-media-sync` | 213 | Incremental Music → USB sync (mp3/mp4, add/update only) | | `bin/pos-network-checkport` | 496 | Check TCP/UDP port reachability (nmap, or bash/nc fallback) + local interface view | | `bin/pos-network-download` | 951 | aria2 RPC daemon + queue control (add/torrent/metalink, watch, limits) | | `bin/pos-network-hotspot` | 93 | Wi-Fi hotspot via create_ap + wihotspot-gui | diff --git a/bin/pos-media-sync b/bin/pos-media-sync index c5582c8..fb474cf 100755 --- a/bin/pos-media-sync +++ b/bin/pos-media-sync @@ -69,13 +69,15 @@ while [[ $# -gt 0 ]]; do done [ "$MP3" -eq 1 ] || [ "$MP4" -eq 1 ] || { MP3=1; MP4=1; } -# Normalize a trailing slash on the source: GNU find strips it on the starting -# point (find -H /x/ -type f emits /x/a.mp3, not /x//a.mp3), so with -# SRC=/x/ the rel prefix "${f#"$SRC/"}" becomes /x// which never matches and -# files would nest under /Music//x/... instead of mirroring the tree. -# Covers both --source /x/ and MEDIA_SYNC_SOURCE=.../. -SRC="${SRC%/}" -[ -n "$SRC" ] || err "Source not found: $SRC" +# Normalize trailing slashes on the source: GNU find normalizes ONE trailing +# slash on the starting point (find -H /x/ -type f emits /x/a.mp3) but keeps +# a doubled one (find -H /x// -type f emits /x//a.mp3), so with SRC=/x// the +# rel prefix "${f#"$SRC/"}" never matches and files would nest under +# /Music//x/... instead of mirroring the tree. Strip ALL trailing +# slashes — a lone "/" or "//" ends up empty and hits the guard below. +# Covers both --source /x// and MEDIA_SYNC_SOURCE=...//. +while [[ "$SRC" == */ ]]; do SRC="${SRC%/}"; done +[ -n "$SRC" ] || err "Source path is empty" [ -d "$SRC" ] || err "Source not found: $SRC" @@ -143,7 +145,12 @@ trap 'rm -f "$find_list" "$find_err"' EXIT find_rc=0 find -H "$SRC" "${find_expr[@]}" >"$find_list" 2>"$find_err" || find_rc=$? sort -o "$find_list" "$find_list" +# One "find reported problems" condition, computed once here and reused at the +# final success messages (below) so a partial tree is never announced as fully +# synced. +find_ok=1 if [ "$find_rc" -ne 0 ] || [ -s "$find_err" ]; then + find_ok=0 warn "find of the source reported problems — results may be incomplete:" if [ -s "$find_err" ]; then sed 's/^/ /' "$find_err" @@ -194,9 +201,13 @@ while IFS= read -r f; do fi done < "$find_list" +# Mark the success line + notification as partial when find reported +# problems, so the success signal can't contradict the warning above. +partial_suffix="" +[ "$find_ok" -eq 0 ] && partial_suffix=" (partial — find reported problems)" if [ "$DRY_RUN" -eq 1 ]; then echo "DRY RUN — nothing copied. Would sync: ${added} new, ${updated} updated, ${unchanged} unchanged → $dest_root" else - ok "Sync complete: ${added} added, ${updated} updated, ${unchanged} unchanged → $dest_root" - notify_send "Music sync completed: ${added} added, ${updated} updated → $dest_root" + ok "Sync complete: ${added} added, ${updated} updated, ${unchanged} unchanged → $dest_root$partial_suffix" + notify_send "Music sync completed: ${added} added, ${updated} updated → $dest_root$partial_suffix" fi