Skip to content

Desktop & multi-machine session sync: three overlapping PRs need a direction decision #91

Description

@tawanorg

Three open PRs implement overlapping desktop/multi-machine session sync, by three different authors, totalling roughly 8,300 added lines. They cannot all land, and the order matters, so this issue records the decision rather than leaving it implicit in the PR queue.

The overlap

PR Author Size State
#77 @sjalife +2472 / −35, 18 files mergeable, no reviews yet
#84 @d-jiao +2131 / −4, 21 files mergeable, 5 unresolved review threads
#86 @austinc3030 +3683 / −68, 24 files conflicts with main

#86 strictly contains #77. Its first seven commits are #77's, by SHA:

43fee299  fix(sync): resolve append-only transcript conflicts…
e6e6d125  fix(sync): never sync per-machine debris…
f84476ed  feat(storage): typed ErrNotFound…
7ff6e931  fix(sync): union-merge history.jsonl…
710ef084  feat(sync): union-merge genuinely diverged session transcripts
84a0659a  feat(sync): sync Claude Code Desktop session records
65324021  fix(cli): report per-file sync errors to stderr in quiet mode

plus 16 of its own. So #86 is #77 extended, not an alternative to it. #86 also carries ad31e936 sync: JSON-escape substituted paths in .json/.jsonl content, an independent fix for the bug already fixed differently in #82 — that will conflict, and one implementation has to win.

#84 is largely independent, touching the other two only at cmd/claude-sync/main.go, internal/sync/state.go and internal/sync/sync.go.

Decision

Land #77 first, then have #86 rebase onto it. #77 is the only one that currently merges clean and is the shared foundation; landing it reduces #86 to its own 16 commits, which is reviewable, and keeps attribution with both authors. #84 proceeds on its own track.

Why none of them is merging yet

All three restructure the most data-sensitive code in the repo — conflict resolution, history merging, session merging, state tracking. That is a different risk class from an additive fix, and it wants a focused review rather than a backlog sweep. For scale: #82 was 4 files and 324 lines, and still took three review rounds and seven confirmed defects — two of which would have corrupted users' transcripts — before it was safe to merge.

#84 specifically should not merge until its outstanding review findings are addressed. Paraphrasing the five unresolved threads:

  • internal/desktop/record.go:42 — Record.Save uses os.WriteFile, which follows an existing symlink, so a remote-controlled sessionId can make desktop pull write outside the index directory. Needs a no-follow open or a refusal on symlink destinations.
  • internal/desktop/apply.go:92 — deduplication only checks cliSessionId, so a remote record whose sessionId collides with another local record's filename silently overwrites that record.
  • internal/sync/desktop.go:133 — PushDesktop replaces the whole remote index, so a machine that pushes before pulling removes the other machine's sidebar entries.
  • internal/sync/desktop.go:315 — pushExcluder feeds DetectChanges, which treats a tracked path missing from localFiles as a deletion, so push --skip-archived schedules already-uploaded archives for DeleteBatch.
  • internal/sync/desktop.go:338 — remoteKeyExists passes a full object key as a List prefix, but the WebDAV adapter appends / to any non-empty prefix, so desktop pull never downloads the index on WebDAV.

Three of those are data-loss paths and one is a write-outside-the-target-directory escape, which is why this is a hold rather than a nitpick.

Next steps

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestneeds-decisionBlocked on a maintainer decision about direction, not on code

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions