Repository navigation
fix(source): skip unchanged pushes and give watch pushes the real budget - #154
Merged
Merged
Conversation
A watch-mode source re-sent the full envelope to every sink on each 30s baseline tick, even when nothing had changed, and each watch-mode push was capped at 60s, below the 5-minute SyncClient timeout, so slow links failed with context deadline exceeded. The source now remembers, per sink, the hash of the last envelope it delivered (Sequence and cookie access times excluded) and skips a sink that already holds the same payload, re-sending at least every 10 minutes as a heartbeat. Only a successful POST counts as delivered, so a failed sink is always retried. Watch-mode pushes get the same budget as --once (SyncClient timeout plus 30s), and the watcher skips a trigger while a push is still running so slow pushes cannot pile up. Reported by @Dannyxlm in #149. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
The source's 30s re-push used to re-inject cookies into a Linux sink's live Chrome as a side effect, which covered browser contexts opened since the last sync and a Chrome that restarted. Now that unchanged pushes are skipped, the sink holds the cookies from its last /sync and re-injects them locally every 30s, applying its current cookie policy. Failures log once and recovery logs once. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
…ct logging Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
- Forget a sink's remembered delivery when a POST is sent and fails, so a later payload is never skipped on an older acknowledgement. - Stamp each delivery when its POST succeeds, so a slow POST does not bring the next heartbeat forward. - Serialize the live CDP re-inject tick with /sync applies so a newer /sync set always lands after an in-flight tick. - Use liveCDPEndpointFor everywhere, and move the re-injector and the dedup log into sink_reinject.go and source_dedup.go. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
[Medium impact] The PR appears safe to merge; no new blocking issue was found. SummaryThe source skips unchanged deliveries per sink until a 10-minute heartbeat and gives watch pushes the same budget as one-shot pushes.
|
…status Addresses review feedback on #154. An older /sync that finishes after a newer one from the same source no longer replaces the held cookie set, and each re-inject tick that attempts an inject records its outcome in the sink state, so status reflects the latest live CDP result. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
Addresses review feedback on #154. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN
# Conflicts: # CHANGELOG.md # docs/faq.md # internal/cli/source.go # internal/cli/source_test.go
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
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.
Summary
An idle watch-mode source stops re-sending the same envelope to every sink every 30 seconds, and slow links stop failing at a hardcoded 60-second push budget. On the reporter's install, cookie-only pushes still cost about 1.8 MB per 30 s (about 5 GB a day) with nothing changing. On uplinks slower than about 12 Mbit/s, each push hit
context deadline exceeded(6,356 times in one log), well under the 5-minute SyncClient timeout v1.1.0 set.Sequenceand cookie access times are left out of the hash, so Chrome bumpinglast_access_utcon open tabs does not count as a change. The sent payload still carries the access times.--once(SyncClient timeout plus 30 s). The watcher skips a trigger while a push is still running, so slow pushes cannot pile up; the next 30 s tick picks up anything skipped./syncinto the running Chrome every 30 s, locally, applying its current cookie policy. The source's 30 s re-push used to do this as a side effect, covering browser contexts an agent opened since the last sync and a Chrome that restarted. Without it, dedup would leave those logged out until the heartbeat. Re-injects are serialized with/sync, so a newer set always lands last.Reported by @Dannyxlm in #149 (items 2 and 3).
Related: #149
Verified
go build ./...go vet ./...go test -race ./...golangci-lintv2.11.4 (the CI pin): 0 issuesDedup tests drive the real fan-out on a stub clock:
last_access_utcdoes notSequencestrictly increases across real POSTsWatcher tests check that the push deadline is the configured budget and that only one push runs at a time. Re-injector tests cover re-inject after
/syncand nothing before it, no ticker when live CDP is off or on--dry-run, failure and recovery each logged once, the current policy applied each tick, and a newer/synclanding after an in-flight tick.Code review:
ce-code-reviewat full depth, with correctness, reliability, testing and maintainability reviewers plus an independent Codex adversarial read (gpt-6-luna, independence verified)./syncrace, forgetting a delivery after a failed POST, timestamping deliveries per sink, the shared endpoint helper, splitting the new code intosource_dedup.goandsink_reinject.go, and a dry-run guard test.Notes
doctoronly warns after 24 h without a push, so status stays fresh.--skip-local-storagechange in fix(source): cookie-only source with --skip-local-storage and --skip-secrets #153, and item 4 is fix(config): honor live_cdp.enabled: false on Linux sinks #152, so Source ships ~90 MB of Local Storage per push to Linux sinks (plus three related sync-path findings) #149 can close once all three merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN