fix(sync): log every delivery, not just the ones that went wrong - #69
Merged
Conversation
sync.log gated its success line on `attempt or (ip, port) != addrs[0]` (text) and `(ip, port) != addrs[0]` (media), so a send that landed on the first try to the primary mDNS address wrote nothing at all. The log only ever spoke up about retries, address fallbacks and failures. That makes a healthy idle sync and a completely dead one look identical on disk, and the silence actively invites the wrong reading: twice in one debugging session an empty log was taken as "no send was even attempted", sending the investigation after `_seen_has()` suppression and swallowed exceptions in mac_app.on_change() — when in fact every clip was being delivered. netstat showed the truth: a fresh 192.168.1.60.<ephemeral> -> 192.168.1.17.47823 TIME_WAIT per copy. Both log lines are now unconditional. `_log` self-bounds sync.log at 512 KB (truncating to the last 256 KB), so the extra volume is already capped. Guarded by a new case 6 in scripts/sync_delivery_test.py: deliver to a single good address — attempt 0, addrs[0], the exact case that used to be silent — and assert the line is there. It fails on the old code with `got []`. The text path is covered; the media guard was identical in shape and removed alongside it, but streaming a blob is not exercised here. The three sync self-tests now point _SYNC_LOG at a temp dir. They were writing into the real <data>/sync.log, which this change would have made noisier — the `delivered to macbookpro-1.home via 127.0.0.1:48051` pairs in a live log are self-test residue, not traffic. Suite now leaves it untouched (delta 0 lines). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
sync.loggated its success line onattempt or (ip, port) != addrs[0](text) and(ip, port) != addrs[0](media). A send that landed on the first attempt to the primary mDNS address — the healthy path, i.e. nearly every send — wrote nothing. The log only ever recorded retries, address fallbacks and outright failures.So a working sync and a dead one produce byte-identical logs, and the silence reads as absence of activity. In one debugging session that cost two wrong diagnoses: an empty log was taken as "no send was even attempted", which sent the investigation after
_seen_has()echo-suppression and the swallowed exceptions aroundengine.broadcast_id(eid)inmac_app.on_change(). Every clip had in fact been delivered.netstatis what settled it — a fresh192.168.1.60.<ephemeral> -> 192.168.1.17.47823 TIME_WAITper copy, three for three.The change
Both log lines are unconditional. Volume is already bounded:
_logtruncatessync.logto the last 256 KB once it passes 512 KB.Regression guard
New case 6 in
scripts/sync_delivery_test.pydelivers to a single good address —attempt == 0,addrs[0], precisely the case that used to be silent — and asserts the line exists. Against the pre-fixsync.pyit fails with:Scope note: the guard covers the text path. The media guard was identical in shape and is removed alongside it, but streaming a blob isn't exercised by this test.
Self-tests stop polluting the real log
All three sync self-tests were writing into the live
<data>/sync.log, and this change would have made them noisier — successes now log too. They now pointsync._SYNC_LOGat a temp dir. Worth knowing when reading an existing log:delivered to macbookpro-1.home via 127.0.0.1:48051followed by127.0.0.1:9 refusedis self-test residue, not traffic.Verification
python -m compileall clippy scripts— OKscripts/mac_selector_test.py— OK across 9 PyObjC classes in 32 sourcessync_selftest.py,sync_drift_test.py,sync_delivery_test.py— all pass~/.local/share/clippy/sync.logafter a full suite run: 626 lines before, 626 after🤖 Generated with Claude Code