refactor(source-control): responsibility cleanup (PR2)

e2e-tests/provider/suites/sync-manager.e2e.test.ts

@@ -8,6 +8,7 @@ import { SyncManagerWorkspace } from '../../../src/logic/sync/SyncWorkspace';
 import { SourceControlActionService } from '../../../src/logic/source-control/SourceControlActionService';
 import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository';
 import { OperationState } from '../../../src/logic/source-control/OperationState';
+import { SyncSelectionStore } from '../../../src/logic/source-control/SyncSelectionStore';
 import { toChangeId } from '../../../src/logic/source-control/types';
 // `import type` deliberately, not a value import: src/settings.ts also
 // exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest ->
@@ -261,7 +262,7 @@ describe('SyncManager E2E', () => {
             normalizePath: p => p,
             app: {} as App,
         });
-        const actionService = new SourceControlActionService(repository, operations, workspace);
+        const actionService = new SourceControlActionService(repository, new SyncSelectionStore(), operations, workspace);
 
         await actionService.deleteRemote([changeId]);
 

e2e-tests/provider/support/source-control-scenarios.ts

@@ -211,7 +211,7 @@ export class SourceControlScenario {
                 getDiff: (): Promise<FileDiff> => Promise.resolve({ path: '', kind: 'text' } as FileDiff),
             },
         );
-        const actionService = new SourceControlActionService(repository, operations, workspace);
+        const actionService = new SourceControlActionService(repository, selection, operations, workspace);
         return { repository, selection, operations, actionService, workspace };
     }
 }

e2e-tests/provider/support/two-client-sync-scenario.ts

@@ -14,6 +14,7 @@ import { GitignoreManager } from '../../../src/logic/gitignore-manager';
 import { ensureSyncWorkspaceRuntime } from '../../../src/logic/sync/SyncWorkspace';
 import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository';
 import { OperationState } from '../../../src/logic/source-control/OperationState';
+import { SyncSelectionStore } from '../../../src/logic/source-control/SyncSelectionStore';
 import { SourceControlActionService } from '../../../src/logic/source-control/SourceControlActionService';
 import { toSyncChanges } from '../../../src/logic/source-control/FileStatusAdapter';
 import {
@@ -67,6 +68,7 @@ export class TwoClient {
      */
     private readonly statuses: SyncStatusService;
     private readonly repository = new ChangeRepository();
+    private readonly selection = new SyncSelectionStore();
     private readonly operations = new OperationState();
     private readonly refreshService: SyncStatusRefreshService;
     private readonly actionService: SourceControlActionService;
@@ -110,7 +112,7 @@ export class TwoClient {
             sync: this.manager,
             getNormalizedPath: path => path,
         }, this.statuses);
-        this.actionService = new SourceControlActionService(this.repository, this.operations, workspace);
+        this.actionService = new SourceControlActionService(this.repository, this.selection, this.operations, workspace);
     }
 
     // --- local vault ops --------------------------------------------------

progress.md

@@ -5,14 +5,18 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
 ## Current State
 
 **Last Updated:** 2026-09-01
-**Active Feature:** restore explicit per-file sync actions (no tracked issue number). All 7 planned commits landed and each individually passes lint/tests/build.
-**Branch / PR:** `claude/fix-source-control-explicit-sync-intent`, based on `claude/fix-mobile-diff-rendering-and-responsive-layout` (itself 1 commit ahead of `main`). Not yet pushed or opened as a PR.
+**Active Feature:** PR2 responsibility cleanup, item 5 done — provider contract cleanup, partial (no tracked issue number; an ad-hoc follow-up plan on top of `origin/1.6.1`, not in `feature_list.json`).
+**Branch / PR:** `claude/pr2-source-control-boundary`, branched from `origin/1.6.1` (commit `69e5540`). Pushed; opened as [PR #154](https://github.com/firstsun-dev/git-files-sync/pull/154) against `1.6.1` (covers items 1-4; item 5 below lands as a follow-up commit on the same branch/PR).
 
-**Scope:** `SyncSelectionStore`/`ChangeActionPolicy` (per-change action overrides + resolution), `SourceControlViewModel` (resolved `syncAction`/`hasActionOverride` projection), Sync Queue grouping/row controls, `SourceControlActionService.sync()` (now takes `SyncIntentRequest[]`), a new Repository Changes row "⋯" menu, and `SyncPlanModal` per-row direction icons. Deliberately did not touch `DiffViewer.ts`, mobile diff lifecycle, or E2E cleanup — that's the base branch's prior work.
+**Scope (item 5, per the PR2 plan):** Moved `ConnectionTestResult` out of `git-service-base.ts` into `git-service-interface.ts` — it's a contract type consumed by `GitServiceInterface.testConnection`, so it belongs with the interface, not the base implementation class. `git-service-base.ts` now imports it back for its own `abstract testConnection` signature; `github-service.ts`/`gitlab-service.ts`/`gitea-service.ts`/`main.ts`/`GitLabSyncSettingTab.ts`/`tests/ui/SettingsConnectionStatus.test.ts` updated to import from the new location. Reviewed `updateConfig(...args: unknown[])` on `GitServiceInterface` per the plan's ask, but did **not** convert it to a typed discriminated union: every actual call site (`main.ts` `initializeGitService()`, 3 branches) already calls `updateConfig` on the concrete class (`GitLabService`/`GiteaService`/`GitHubService`), never through the loose interface type, so the untyped signature isn't causing a real type-safety gap today. A discriminated union would mean reshaping the interface, all three services' `updateConfig` bodies, and all three `main.ts` call sites into config-object form for no functional benefit — exactly the "touches too much, leave for later" case the plan calls out, so left as-is.
 
-**Next:** push the branch and open the PR (title `fix(source-control): restore explicit per-file sync actions`); no further planned work outstanding.
+**Next:** PR2 plan is now fully worked through (items 1-5). Nothing further planned here; watch PR #154 for review feedback.
 
-Below that: the previous "Outstanding Items"/"Verification Evidence" entries track separate, still-open work on PR #129 / `claude/source-control-foundation` and Issue #143 — not superseded by this entry, carried over from the base branch history.
+Below that: the previous "Outstanding Items"/"Verification Evidence" entries track separate, still-open work on PR #129 / `claude/source-control-foundation`, Issue #143, and `claude/fix-source-control-explicit-sync-intent` — not superseded by this entry, carried over from the base branch history.
+
+- `npx eslint .` — 0 errors.
+- `npx vitest run` — 76 files / 953 tests passed (unchanged count; pure type-relocation, no new tests needed).
+- `npm run build` (tsc + Obsidian 1.11.0 compat typecheck + esbuild) — passed.
 
 ## Outstanding Items
 

src/logic/source-control/SourceControlActionService.ts

@@ -3,6 +3,8 @@ import { type SyncExecutionResult, type SyncResultNotificationPort } from './Syn
 import type { ChangeRepository } from './ChangeRepository';
 import type { OperationState } from './OperationState';
 import type { SourceControlItem } from './SourceControlViewModel';
+import type { SyncSelectionStore } from './SyncSelectionStore';
+import { defaultSyncAction, type SyncAction } from './ChangeActionPolicy';
 import { SyncIntentExecutor } from './SyncIntentExecutor';
 import type { SyncIntentRequest } from './SyncIntent';
 import type { ChangeId, SyncChange } from './types';
@@ -29,12 +31,18 @@ export interface SourceControlDiffContent {
  *
  * Neither layer talks to a Git provider directly; SyncWorkspace remains the
  * execution boundary.
+ *
+ * Also owns the Sync Queue selection/action-override mutation boundary
+ * (select/deselect, set/clear a row's action override) on behalf of
+ * SyncSelectionStore, so the UI never reaches past this facade into that
+ * store directly.
  */
 export class SourceControlActionService {
     private readonly syncIntentExecutor: SyncIntentExecutor;
 
     constructor(
         private readonly changes: ChangeRepository,
+        private readonly selection: SyncSelectionStore,
         private readonly operations: OperationState,
         private readonly workspace: SyncWorkspace,
         private readonly syncResultNotifier: SyncResultNotificationPort = { notify: () => {} },
@@ -47,6 +55,48 @@ export class SourceControlActionService {
         );
     }
 
+    /** Adds one change to the Sync Queue. */
+    selectForSync(changeId: ChangeId): void {
+        this.selection.selectForSync(changeId);
+    }
+
+    /** Removes one change from the Sync Queue, clearing any action override with it. */
+    deselectFromSync(changeId: ChangeId): void {
+        this.selection.deselectFromSync(changeId);
+    }
+
+    /** Adds several changes to the Sync Queue in one batch (e.g. a folder checkbox). */
+    selectMany(changeIds: readonly ChangeId[]): void {
+        this.selection.selectMany(changeIds);
+    }
+
+    /** Removes several changes from the Sync Queue in one batch. */
+    deselectMany(changeIds: readonly ChangeId[]): void {
+        this.selection.deselectMany(changeIds);
+    }
+
+    /**
+     * Sets a Sync Queue row's explicit action override. Picking the kind's
+     * own default clears the override instead of storing a redundant one, so
+     * `SourceControlItem.hasActionOverride` only means "the user chose
+     * something other than the default".
+     */
+    setSyncAction(changeId: ChangeId, action: SyncAction): void {
+        const change = this.changes.getById(changeId);
+        if (!change) return;
+
+        if (action === defaultSyncAction(change.kind)) {
+            this.selection.clearActionOverride(changeId);
+        } else {
+            this.selection.setActionOverride(changeId, action);
+        }
+    }
+
+    /** Clears a Sync Queue row's explicit action override, reverting it to the kind default. */
+    clearSyncAction(changeId: ChangeId): void {
+        this.selection.clearActionOverride(changeId);
+    }
+
     /** Pushes one or more changes (single push and batch push share this path). */
     async push(changeIds: readonly ChangeId[]): Promise<void> {
         const targets = this.resolve(changeIds);

src/logic/source-control/SourceControlViewModel.ts

@@ -35,10 +35,11 @@ export interface SourceControlViewState {
  * Read-only projection of repository, selection, operation, and refresh state
  * into UI-ready snapshots.
  *
- * The constructor wires selection-intent reconciliation to authoritative
- * ChangeRepository replacements. Cleanup therefore happens on the write-side
- * repository lifecycle, while repeated getState() calls remain observational
- * and never mutate queue intent.
+ * Purely observational: getState() never mutates queue intent, and this
+ * class exposes no selection mutation surface of its own. Selection-intent
+ * reconciliation against authoritative ChangeRepository replacements is
+ * wired by the runtime composition root (createSyncRuntime), not here, and
+ * mutation goes through SourceControlActionService instead of this class.
  */
 export class SourceControlViewModel {
     constructor(
@@ -47,15 +48,7 @@ export class SourceControlViewModel {
         private readonly operations: OperationState,
         private readonly refreshSource: () => Promise<unknown>,
         private readonly refreshState: RefreshState,
-    ) {
-        this.changes.subscribe(changes => this.selectionStore.reconcile(changes));
-    }
-
-    /**
-     * Existing UI mutation boundary for queue selection. Kept for this PR to
-     * avoid mixing a renderer API redesign into the intent/execution cleanup.
-     */
-    get selection(): SyncSelectionStore { return this.selectionStore; }
+    ) {}
 
     getState(filter: SourceControlFilter = 'all', showSynced = false): SourceControlViewState {
         const all = this.changes.getAll();
@@ -75,6 +68,19 @@ export class SourceControlViewModel {
         };
     }
 
+    /**
+     * Projects a single change by id, independent of any filter -- the sole
+     * projection path for callers (e.g. a diff pane host) that need one
+     * row's current selection/operation/syncAction state without hand-rolling
+     * a SourceControlItem themselves. Returns undefined once the change is no
+     * longer in the repository (e.g. it synced and dropped out, or was
+     * deleted).
+     */
+    getItem(id: ChangeId): SourceControlItem | undefined {
+        const change = this.changes.getById(id);
+        return change ? this.toItem(change) : undefined;
+    }
+
     /**
      * Triggers a view-wide refresh through the injected source and records
      * only its presentation lifecycle. Repository population still happens

src/logic/sync/PullCoordinator.ts

@@ -62,6 +62,25 @@ export class PullCoordinator {
         return this.processBatch(files, onProgress, tree, options);
     }
 
+    /**
+     * Plans one already-fetched remote file exactly like batch pull's own
+     * per-file classification without a prefetched tree (`planFromRemote`) --
+     * the shared decision step `SyncManager.pullFile()` delegates to, so
+     * single- and batch-pull planning semantics (baseline resolution,
+     * exists/content/sha comparison) can't silently drift apart.
+     *
+     * Deliberately NOT unified: interactive conflict handling ("resolve
+     * conflict" opens a modal for single-file pull, but is skipped/aggregated
+     * for batch pull), per-call confirmation (single confirms every call;
+     * batch confirms once for the whole plan), and notification (single
+     * always reports "up to date"; batch only summarizes). Those differences
+     * are deliberate UX, not accidental drift, and stay owned by each caller.
+     */
+    async planSingleFile(file: TFile | string, remote: GitFile): Promise<PlannedFileAction> {
+        const { path, isString } = this.dependencies.scanner.fileInfo(file);
+        return this.planFromRemote(file, path, isString, remote);
+    }
+
     async planPullBatch(files: Array<TFile | string>, remoteTree?: GitTreeEntry[]): Promise<SyncPlan> {
         const tree = remoteTree ? new Map(remoteTree.map(entry => [entry.path, entry])) : undefined;
         const plan: SyncPlan = { additions: [], modifications: [], deletions: [], moves: [] };

src/logic/sync/SyncManager.ts

@@ -10,8 +10,6 @@ import {
     isSyncPlanEmpty,
 } from './types';
 import { logger } from '../../utils/logger';
-import { contentsEqual, isBinaryPath } from '../../utils/path';
-import { gitBlobSha } from '../../utils/git-blob-sha';
 import { SyncStatusService } from '../sync-status-service';
 import { PushExecutor } from './PushExecutor';
 import { PullExecutor } from './PullExecutor';
@@ -21,7 +19,6 @@ import { ConflictResolver } from './ConflictResolver';
 import { SyncExecutor } from './SyncExecutor';
 import { PullCoordinator } from './PullCoordinator';
 import { PushCoordinator } from './PushCoordinator';
-import { SyncPlanner } from './SyncPlanner';
 import {
     HeadlessSyncInteraction,
     type ConflictDiffLoader,
@@ -41,7 +38,6 @@ export class SyncManager {
     private readonly scanner: SyncScanner;
     private readonly pullCoordinator: PullCoordinator;
     private readonly pushCoordinator: PushCoordinator;
-    private readonly planner = new SyncPlanner();
     private readonly interaction: SyncInteractionPort;
     /** Optional progressive +/- diff-stat source handed to the batch conflict modal. */
     private diffStatLoader?: ConflictDiffStatLoader;
@@ -211,23 +207,12 @@ export class SyncManager {
 
             const exists = await this.fileExists(fileOrPath);
             const localContent = exists ? await this.getFileContent(fileOrPath) : null;
-            const lastSynced = this.settings.syncMetadata[path];
-            const kind = isBinaryPath(path) ? 'binary' : 'text';
-            const baseline = lastSynced?.lastSyncedSha === remote.revision ? remote.sha : lastSynced?.lastSyncedSha;
-            let localSha: string | undefined;
-            if (localContent !== null) {
-                localSha = contentsEqual(localContent, remote.content) ? remote.sha : await gitBlobSha(localContent);
-            }
-            const decision = this.planner.planFor('pull', {
-                local: {
-                    path,
-                    exists,
-                    blobSha: localSha,
-                    kind,
-                },
-                remote: { path, repoPath, exists: true, blobSha: remote.sha, kind },
-                base: { blobSha: baseline },
-            });
+            // Shared with batch pull's own no-prefetched-tree classification
+            // (PullCoordinator.planFromRemote), so single- and batch-pull
+            // planning semantics can't silently drift apart. Interactive
+            // conflict handling, confirmation, and notification stay separate
+            // below -- see PullCoordinator.planSingleFile's doc comment.
+            const decision = await this.pullCoordinator.planSingleFile(fileOrPath, remote);
 
             if (decision.action === 'none') {
                 await this.updateMetadata(path, remote.sha);

src/main.ts

@@ -3,8 +3,7 @@ import { DEFAULT_SETTINGS, GitLabFilesPushSettings, GitLabSyncSettingTab, getSer
 import { GitLabService } from './services/gitlab-service';
 import { GitHubService } from './services/github-service';
 import { GiteaService } from './services/gitea-service';
-import { GitServiceInterface, GitTreeEntry } from './services/git-service-interface';
-import { ConnectionTestResult } from './services/git-service-base';
+import { ConnectionTestResult, GitServiceInterface, GitTreeEntry } from './services/git-service-interface';
 import type { SyncManager } from './logic/sync-manager';
 import { SourceControlItemView, SOURCE_CONTROL_VIEW_TYPE } from './ui/source-control/SourceControlItemView';
 import { DiffTabView, SOURCE_CONTROL_DIFF_VIEW_TYPE, type DiffTabContent } from './ui/source-control/DiffTabView';
@@ -64,7 +63,7 @@ export default class GitLabFilesPush extends Plugin {
 
 	async onload() {
 		await this.loadSettings();
-		this.addSettingTab(new GitLabSyncSettingTab(this.app, this));
+		this.addSettingTab(new GitLabSyncSettingTab(this.app, this, this));
 
 		this.registerView(
 			SOURCE_CONTROL_VIEW_TYPE,

src/runtime/createSyncRuntime.ts

@@ -110,18 +110,27 @@ export function createSyncRuntime(deps: SyncRuntimeDependencies): SyncRuntime {
     );
     const sourceControlActions = new SourceControlActionService(
         changeRepository,
+        syncSelectionStore,
         operationState,
         syncWorkspace,
         new SyncResultNotifier(deps.notify),
     );
 
+    // Selection-intent reconciliation is wired here, at the composition
+    // root, rather than inside SourceControlViewModel: it is a write-side
+    // lifecycle concern (stale selections/overrides get dropped whenever the
+    // repository publishes an authoritative snapshot), not part of the
+    // ViewModel's read-only projection.
+    const unsubscribeSelectionReconciliation = changeRepository.subscribe(changes => syncSelectionStore.reconcile(changes));
+
     // Keeps ChangeRepository (and therefore the Source Control view) in sync
     // with the same SyncStatusService instance the sync domain already
-    // publishes to -- no separate refresh/polling path.
+    // publishes to -- no separate refresh/polling path. SyncSelectionStore
+    // cleanup is handled by the reconciliation subscription above, which
+    // ChangeRepository.replace() below triggers, so it isn't repeated here.
     const unsubscribeChangeRepository = sync.status.subscribe((statuses) => {
         const changes = toSyncChanges([...statuses.values()]);
         changeRepository.replace(changes);
-        syncSelectionStore.refresh(changes.map(change => change.id));
     });
 
     return {
@@ -135,6 +144,9 @@ export function createSyncRuntime(deps: SyncRuntimeDependencies): SyncRuntime {
         refreshState,
         sourceControlViewModel,
         sourceControlActions,
-        dispose: () => unsubscribeChangeRepository(),
+        dispose: () => {
+            unsubscribeChangeRepository();
+            unsubscribeSelectionReconciliation();
+        },
     };
 }