Skip to content

fix(sutando-migrate): use uname-s branch for preflight byte count (closes #1474) - #1476

Merged
qingyun-wu merged 1 commit into
sonichi:staging-workspace-revampfrom
JasonOA888:fix/1474-stat-cross-platform
Jun 6, 2026
Merged

qingyun-wu merged 1 commit into
sonichi:staging-workspace-revampfrom
JasonOA888:fix/1474-stat-cross-platform

Conversation

@JasonOA888

Copy link
Copy Markdown
Contributor

Summary — closes #1474

Replace the BSD stat -f '%z' + heuristic Linux fallback with an explicit uname -s case branch in preflight_summary().

Bug

On Linux, stat -f '%z' means "filesystem stat" (GNU semantics), not file size. The xargs -0 stat -f '%z' pipeline produces non-empty, non-zero output (filesystem block info), so the [ -z "$bytes" ] || [ "$bytes" = "0" ] fallback trigger never fires. Result: wrong byte count → misleading ETA estimate.

Fix

Branch by uname -s — exactly as proposed in #1474:

  • Darwin: stat -f '%z' (BSD file size via xargs pipeline)
  • Linux|\*: find -printf '%s\\n' (GNU, no stat needed)

This is the same cross-platform pattern already used by stage-readiness.sh and catchup-after-startup.sh.

Why the other stat -f calls are fine

The other 15+ stat -f calls in sutando-migrate.sh all use the stat -f %z ... || stat -c %s ... short-circuit pattern: on Linux the first command exits non-zero (error), so || triggers the GNU fallback correctly. Only the preflight byte count used xargs piping, which produces wrong-but-non-empty output on Linux.

Test

  • Added Test 8: asserts uname -s branch exists, Darwin) and Linux|*) cases present, old heuristic fallback removed
  • All 8 tests pass: ALL TESTS PASS

Scope

…oses sonichi#1474)

Replace the BSD stat -f '%z' + heuristic Linux fallback with an explicit
uname -s case branch in preflight_summary().

Bug: on Linux, stat -f '%z' means 'filesystem stat' (GNU), not file size.
The xargs pipeline produces non-empty, non-zero output (filesystem info),
so the [ -z "$bytes" ] fallback trigger never fires → wrong byte count
→ misleading ETA estimate.

Fix: branch by uname -s — Darwin uses stat -f '%z' (BSD file size),
Linux/* uses find -printf '%s\n' (GNU). Same approach as stage-readiness.sh
and catchup-after-startup.sh already use for cross-platform stat.

Test: added Test 8 asserting the uname-s branch exists and the old
heuristic fallback is removed.

Cosmetic severity only — data integrity and copy correctness are unaffected.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Explicit uname -s branch replaces the unreliable BSD-stat-fallback pattern. Linux|*) default is safe. Test guards regression. Closes #1474. Stand: Echo Act IV Pro

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cold-review from Studio (qingyun-sutando, the bot that filed the original #1474). LGTM with one tiny style nit (non-blocking).

What's right:

  1. Code matches my proposed fix in #1474 byte-for-byte. The case "$(uname -s)" shape + Darwin/Linux|* arms + drop of the heuristic [ -z "$bytes" ] || [ "$bytes" = "0" ] fallback are exactly what I suggested. Net effect on Linux: deterministic byte-count via find -printf '%s\n' instead of a guessing-game fallback that doesn't trigger when GNU stat -f produces non-empty garbage.

  2. Linux|*) default arm correctly catches FreeBSD-without-Darwin, OpenBSD, NetBSD, WSL, etc. — anywhere with GNU find. The same fall-through-to-GNU pattern is used in scripts/stage-readiness.sh and skills/catchup-after-startup/scripts/catchup-after-startup.sh.

  3. Test 8 has both positive (new branch present) AND negative (old fallback comment + trigger removed) assertions — guards against a future revert that silently re-introduces the BSD-stat-on-Linux bug.

  4. No collateral damage to the other ~15 stat -f %z … || stat -c %s calls in sutando-migrate.sh: those use the short-circuit || pattern which is portable (BSD stat exits non-zero on Linux → falls through to stat -c). The preflight call was unique in chaining via xargs -0 which broke the short-circuit. PR body's note on this is accurate.

Non-blocking nit: the Linux|*) branch could in principle drop the 2>/dev/null on find -printf since GNU find supports it natively — but matching the pre-existing error-suppression idiom in the same function is consistent (the Darwin arm also has 2>/dev/null on the BSD path), so leave as-is. Purely style.

Net: ship it. Sonichi's approval already covers the architectural call; this comment is for the record from the issue-filer.

— Studio (qingyun-sutando)

@qingyun-wu
qingyun-wu merged commit 043ab19 into sonichi:staging-workspace-revamp Jun 6, 2026
4 checks passed
qingyun-wu added a commit that referenced this pull request Jun 6, 2026
…rator checklist + workspace-sync node_modules

Per Chi's PR #1472 comments (#design 2026-06-06):

**CHANGELOG.md v0.3.0 additions:**
- Fixed → Migration subsection: Lucy's Maddy v0.8 5-bug sweep (#1475 #1478 #1482 #1483 #1484)
- Fixed → Cross-platform: #1476 (uname -s branch for preflight byte count)
- Fixed → Security: #1452 #1462 #1466 (execSync → execFileSync sweep)
- Added: #1467 (bridges inject skill instructions into owner task files)
- Workspace contract rollup → M1: also reference #1473 (pre-flight scan + per-file progress reporter)

**KNOWN_ISSUES.md additions** (from migration-thread digest 1512359988834730014):
- Pre-migration operator checklist (7 items) — vault URL, remove shell-rc env var,
  grep .git/info/exclude, confirm wsId, stagger hosts, restart services,
  post-migration sanity check
- Identity-collision rule — exactly one host at a time can hold a Discord token
  (also applies to Twilio + other per-identity creds)

**docs/workspace-sync.md** pre-flight checklist:
- Added `node_modules/` row per Chi's PR #1472 comment (covers vault-sync side;
  #1482 already handles the backup-side via `_BACKUP_DEFAULT_EXCLUDES`)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants