Skip to content

fix(sync): classify remote-only changes as remote-modified, not local-modified - #145

Merged
ClaudiaFang merged 1 commit into
claude/source-control-foundationfrom
fix/remote-modified-classification
Aug 31, 2026
Merged

ClaudiaFang merged 1 commit into
claude/source-control-foundationfrom
fix/remote-modified-classification

Conversation

@ClaudiaFang

Copy link
Copy Markdown
Member

Summary

Fixes the CI failure on PR #129 (CI / Provider E2E / gitea, run 33355671713): the new two-client-sync.e2e.test.ts P0-1/P0-2 tests caught a real production bug where a client's sync would silently drop a remote-only change to an already-tracked file, because SyncStatusService.classify() had no way to tell "only the remote side changed" apart from "the local side changed" — both collapsed into a direction-blind 'modified', which FileStatusAdapter then mapped to 'local-modified' (push), never pull.

  • SyncStatusService.classify() now takes optional localChanged/remoteChanged facts, derived from each file's tracked lastSyncedSha baseline, and emits the already-modeled but previously unreachable 'remote-modified' status when only the remote side moved.
  • FileStatusAdapter, ChangeActionPolicy.canDownload, and ChangeItem's inline Download button are wired through, so a remote-modified row now routes to pull by default and offers manual download — same as remote-only.
  • Relabeled the Source Control "Remote" filter chip to "Incoming" (all three locales): with remote-modified now reachable, "Remote Changes" read as "an existing file changed remotely" and obscured that the bucket also holds brand-new remote-only files.

Test plan

  • npx eslint . — 0 errors
  • npm run build — clean, including Obsidian 1.11.0 compat typecheck
  • npx vitest run — 862/862 passed (68 files), including 3 pre-existing tests updated for the new, correct behavior and 2 new regression tests
  • Real-provider E2E (gitea) — re-run in CI to confirm the originally-failing P0-1/P0-2 now pass

🤖 Generated with Claude Code

…-modified

SyncStatusService.classify() collapsed every two-sided diff into a
direction-blind 'modified', which FileStatusAdapter then mapped to
'local-modified' regardless of which side actually changed. When only the
remote side had moved (the exact shape a second client produces), the
change was routed to the push bucket instead of pull, so it was silently
skipped/conflicted instead of being pulled — caught by the new
two-client-sync E2E suite (P0-1, P0-2) failing against the disposable
Gitea provider in CI.

classify() now takes optional localChanged/remoteChanged facts (derived
from the tracked lastSyncedSha baseline) so it can tell "only remote
changed" apart from "local changed" or "no baseline on record", and emits
the already-modeled but previously unreachable 'remote-modified' status.
FileStatusAdapter, ChangeActionPolicy.canDownload, and ChangeItem's
inline Download button are wired through so a remote-modified row now
routes to pull by default and offers manual download, same as
remote-only.

Also relabels the Source Control "Remote" filter chip to "Incoming" in
all three locales: with remote-modified now reachable, "Remote Changes"
read as "an existing file changed on the remote" and obscured that the
bucket also holds brand-new remote-only files — "Incoming" covers both
without implying a direction that isn't there.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@ClaudiaFang
ClaudiaFang merged commit b186d35 into claude/source-control-foundation Aug 31, 2026
6 of 20 checks passed
@ClaudiaFang
ClaudiaFang deleted the fix/remote-modified-classification branch August 31, 2026 04:34
@ClaudiaFang

Copy link
Copy Markdown
Member Author

🎉 This PR is included in version 1.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant