refactor(architecture): enforce module boundaries

CLAUDE.md

@@ -18,15 +18,27 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 - Test: `npm run test` (runs vitest suite)
 - Version bump: `npm run version` (updates manifest.json and versions.json via script)
 
-## Code Architecture
-- **Type**: Obsidian Plugin (TypeScript) that syncs vault files with a GitLab or GitHub repository.
-- **Entry Point**: `src/main.ts` contains the main `GitLabFilesPush` class extending `Plugin`.
+## Architecture Contract
+
+Before production code changes:
+- Read `docs/architecture.md`.
+- For bug fixes also read `docs/bug-fix-guidelines.md`.
+- Identify the owning module before editing.
+- Preserve dependency direction.
+- Do not bypass `SyncWorkspace`.
+- Do not move domain/provider logic into UI.
+- Do not duplicate status/conflict/rename/action policy.
+
+`docs/architecture.md` is the canonical module map (layers, ownership table, MUST/MUST NOT rules, current hotspots) — this file does not duplicate it. A short summary:
+
+- **Type**: Obsidian Plugin (TypeScript) that syncs vault files with a GitHub, GitLab, or Gitea repository.
+- **Entry point**: `src/main.ts` owns only Obsidian lifecycle (settings load/save, command/view/ribbon/vault-event registration); the sync/Source Control constructor graph is wired by `src/runtime/createSyncRuntime.ts`.
 - **Settings**: `src/settings.ts` defines `GitLabFilesPushSettings` interface, `DEFAULT_SETTINGS` object, and `GitLabSyncSettingTab` for the Obsidian UI.
-- **Services**: `src/services/` abstracts the git provider behind `GitServiceInterface`, with `GitHubService` and `GitLabService` implementations sharing common logic via `BaseGitService`.
-- **Sync logic**: `src/logic/sync-manager.ts` handles push/pull, conflict detection, and rename detection; `src/logic/gitignore-manager.ts` merges local and remote `.gitignore` rules.
-- **UI**: the production Source Control surface is `SourceControlItemView` (`src/ui/source-control/SourceControlItemView.ts`), which renders `SourceControlView` (`src/ui/source-control/SourceControlView.ts`). User intent (push/pull/delete-remote/resolve-conflict) flows through `SourceControlActionService` (`src/logic/source-control/SourceControlActionService.ts`) into `SyncWorkspace` (`src/logic/sync/SyncWorkspace.ts`), which drives `SyncManager` and its executors (`PushExecutor`, `PullExecutor`, `RemoteDeleteExecutor`, etc. in `src/logic/sync/`). `src/ui/components/` holds shared diff/change presentation pieces used by this surface.
-  - Do not reintroduce `SyncStatusView` or `ui/sync-status/*` — that legacy presentation layer was replaced by the Source Control surface above and is blocked by an ESLint `no-restricted-imports` rule (`eslint.config.*`). The historical migration docs live in `docs/source-control-refactor/` and are marked as such; they are not current implementation guidance.
-  - `SOURCE_CONTROL_VIEW_TYPE` (`'sync-status-view'`) and the `open-sync-status` command id are intentionally kept as-is for pinned-leaf/workspace-layout compatibility — they resolve to the current `SourceControlItemView`, not a leftover of the old UI. Do not rename them as "cleanup."
+
+Two compatibility gotchas not covered by `docs/architecture.md`:
+- Do not reintroduce `SyncStatusView` or `ui/sync-status/*` — that legacy presentation layer was replaced by the Source Control surface (`SourceControlItemView`/`SourceControlView`) and is blocked by an ESLint `no-restricted-imports` rule (`eslint.config.*`). The historical migration docs live in `docs/source-control-refactor/` and are marked as such; they are not current implementation guidance.
+- `SOURCE_CONTROL_VIEW_TYPE` (`'sync-status-view'`) and the `open-sync-status` command id are intentionally kept as-is for pinned-leaf/workspace-layout compatibility — they resolve to the current `SourceControlItemView`, not a leftover of the old UI. Do not rename them as "cleanup."
+
 - **Bundling**: Uses `esbuild.config.mjs` for compilation from TypeScript to a single `main.js` file.
 - **Deployment**: Relies on `manifest.json` for plugin metadata and `versions.json` for version mapping/compatibility.
 

docs/architecture.md

@@ -26,7 +26,8 @@ The dependency direction should normally flow downward. Results and state flow b
 
 | Layer | Module | Owns | Interacts with | Must not own |
 | --- | --- | --- | --- | --- |
-| Plugin runtime | `src/main.ts` | Obsidian lifecycle, command/view/event registration, top-level wiring | settings, UI, sync runtime | sync planning, conflict algorithms, provider-specific workflow |
+| Plugin runtime | `src/main.ts` | Obsidian lifecycle, command/view/event registration | settings, `createSyncRuntime`, UI | sync/Source Control constructor graph, sync planning, conflict algorithms, provider-specific workflow |
+| Plugin runtime | `createSyncRuntime` (`src/runtime/createSyncRuntime.ts`) | wires `SyncManager`, `SyncStatusRefreshService`, `SyncDiffService`, `SyncWorkspace`, and the Source Control application layer together | the constructors it composes | Obsidian lifecycle events, commands, views, ribbons |
 | UI | `src/ui/source-control/*` | rendering, user interaction, Source Control composition | `SourceControlViewModel`, `SourceControlActionService` | provider API calls, sync classification rules |
 | Application | `ChangeRepository` | authoritative Source Control `SyncChange` snapshot | `FileStatusAdapter`, ViewModel, action services | remote Git or filesystem access |
 | Application | `SyncSelectionStore` | queued selection and explicit per-change action override | ViewModel, current repository snapshot | sync execution |
@@ -43,10 +44,14 @@ The dependency direction should normally flow downward. Results and state flow b
 | Sync domain | `PushExecutor` | provider-side batch mutations | `GitServiceInterface`, metadata | UI state |
 | Sync domain | `PullExecutor` | local file application for pulls | Obsidian vault, metadata | Source Control UI state |
 | Sync domain | `RemoteDeleteExecutor` | remote deletion execution | `GitServiceInterface` | UI |
-| Sync domain | `SyncStatusRefreshService` | local/remote discovery, status refresh, incremental file event reconciliation, out-of-band move reconciliation | vault, provider, gitignore, status store, metadata | Source Control rendering |
+| Sync domain | `SyncStatusRefreshService` | orchestrates discovery → resolve → reconcile → publish, plus incremental create/modify/delete/rename event handling | `SyncFileDiscovery`, `SyncStatusResolver`, `RenameReconciler`, status store | Source Control rendering, the three algorithms below (delegates to their owning class) |
+| Sync domain | `SyncFileDiscovery` | vault/hidden-file/remote-tree/gitignore/symlink discovery, remote-only vs local-deleted classification | vault, provider, gitignore, status store | status resolution, rename reconciliation |
+| Sync domain | `SyncStatusResolver` | local-vs-remote status resolution: SHA/content comparison, baseline diff direction, `FileStatus` classification | provider, sync manager, status store | discovery, rename reconciliation |
+| Sync domain | `RenameReconciler` | out-of-band (external) rename detection by orphan/candidate blob-sha matching | sync manager, status store | discovery, status resolution |
 | Sync domain | `SyncStatusService` | observable `FileStatus` store and status classification | refresh/sync domain, adapters | UI orchestration |
 | Sync domain | `SyncMetadataStore` | last-synced SHA and rename metadata persistence | manager/executors/coordinators | presentation |
-| Sync domain | `SyncDiffService` | diff content/stat loading and cache | status store, blob loader, workspace | sync orchestration |
+| Sync domain | `SyncDiffService` | diff content/stat loading and cache | status store, blob loader, workspace, `DiffStat` | sync orchestration |
+| Sync domain | `DiffStat` (`src/logic/sync/DiffStat.ts`) | pure +/- diff-stat computation and the `DiffStatLoadResult` contract | diff utilities | UI rendering, provider calls |
 | Interaction boundary | `SyncInteractionPort` | domain-facing confirmation/conflict interaction contract | domain, Obsidian adapter | provider implementation |
 | UI adapter | `ObsidianSyncInteraction` | Obsidian modal/notice implementation of interaction port | `SyncInteractionPort`, modal UI | sync algorithms |
 | Infrastructure | `GitServiceInterface` | provider abstraction used by the sync domain | concrete provider services | UI/application state |
@@ -157,9 +162,9 @@ If the owning module cannot fix a bug without crossing a forbidden boundary, imp
 
 Some current modules have high responsibility density. That is not permission to bypass them, and file size alone is not a reason to split them.
 
-- `main.ts`: composition and Obsidian lifecycle are still concentrated here.
-- `SyncStatusRefreshService`: currently owns several related refresh concerns (discovery, classification, incremental events, rename reconciliation).
-- `SourceControlView`: currently contains substantial Source Control presentation/composition logic.
+- `main.ts`: reduced to Obsidian lifecycle (settings load/save, command/view/ribbon/vault-event registration). The sync/Source Control constructor graph now lives in `createSyncRuntime` (`src/runtime/createSyncRuntime.ts`).
+- `SyncStatusRefreshService`: reduced to orchestration (discovery → resolve → reconcile → publish) plus the incremental create/modify/delete/rename handlers. Discovery, status resolution, and rename reconciliation each now have a single owner: `SyncFileDiscovery`, `SyncStatusResolver`, `RenameReconciler`.
+- `SourceControlView`: reduced by extracting the "Sync Queue" and "Repository Changes" regions into `SyncQueueSection`/`RepositoryChangesSection` (`src/ui/source-control/`, pure state+callbacks render functions matching `FilterMenu`/`SourceControlHeader`). Still owns the diff pane, scroll-state management, and section composition.
 - `PushCoordinator`: large, but still centered on one batch-push use case; split only when a stable responsibility boundary is identified.
 
 Future refactors should reduce these hotspots while preserving the dependency direction in this document.

eslint.config.mts

@@ -50,6 +50,75 @@ export default tseslint.config(
 			],
 		},
 	},
+	{
+		// Architecture regression guard (docs/architecture.md): the UI layer
+		// must reach the sync domain only through SyncWorkspace / the Source
+		// Control application services -- never a concrete Git provider or a
+		// push/pull coordinator/executor directly.
+		files: ["src/ui/**/*.ts", "src/ui/**/*.tsx"],
+		rules: {
+			"no-restricted-imports": [
+				"error",
+				{
+					patterns: [
+						{
+							group: ["**/services/github-service", "**/services/gitlab-service", "**/services/gitea-service"],
+							message: "UI must not depend on a concrete Git provider; go through SyncWorkspace instead.",
+						},
+						{
+							group: ["**/logic/sync/PushCoordinator", "**/logic/sync/PullCoordinator", "**/logic/sync/PushExecutor", "**/logic/sync/PullExecutor"],
+							message: "UI must not bypass SyncWorkspace to reach a push/pull coordinator or executor directly.",
+						},
+					],
+				},
+			],
+		},
+	},
+	{
+		// Architecture regression guard (docs/architecture.md): the Source
+		// Control application layer (ChangeRepository, SourceControlActionService,
+		// SyncIntentExecutor, ...) must reach the sync domain only through
+		// SyncWorkspace -- never a concrete Git provider or a push/pull
+		// coordinator/executor directly.
+		files: ["src/logic/source-control/**/*.ts"],
+		rules: {
+			"no-restricted-imports": [
+				"error",
+				{
+					patterns: [
+						{
+							group: ["**/services/github-service", "**/services/gitlab-service", "**/services/gitea-service"],
+							message: "Source Control must not depend on a concrete Git provider; go through SyncWorkspace instead.",
+						},
+						{
+							group: ["**/logic/sync/PushCoordinator", "**/logic/sync/PullCoordinator", "**/logic/sync/PushExecutor", "**/logic/sync/PullExecutor"],
+							message: "Source Control must not bypass SyncWorkspace to reach a push/pull coordinator or executor directly.",
+						},
+					],
+				},
+			],
+		},
+	},
+	{
+		// Architecture regression guard (docs/architecture.md): sync-domain
+		// modules (SyncManager, coordinators, executors, status resolution)
+		// must not depend on the Source Control presentation layer -- the
+		// dependency direction runs UI -> application -> domain, never back.
+		files: ["src/logic/sync/**/*.ts"],
+		rules: {
+			"no-restricted-imports": [
+				"error",
+				{
+					patterns: [
+						{
+							group: ["**/ui/source-control", "**/ui/source-control/*"],
+							message: "Sync-domain modules must not depend on the Source Control UI; the dependency direction runs UI -> domain, never back.",
+						},
+					],
+				},
+			],
+		},
+	},
 	{
 		files: ["src/**/*.ts", "src/**/*.tsx"],
 		...sonarjs.configs.recommended,

src/logic/sync/DiffStat.ts

@@ -0,0 +1,71 @@
+import { computeSideBySideDiff } from '../../utils/diff';
+
+/** Additions/deletions for a single change's diff, the +/- stat a row shows. */
+export interface ChangeStat {
+    additions: number;
+    deletions: number;
+}
+
+/**
+ * What a diff-stat load resolved to for one row. The distinction matters
+ * because a cache treats the three outcomes differently:
+ * - `ready` — cached as a usable stat.
+ * - `unavailable` — permanent (binary, symlink, no two sides to diff);
+ *   cached so the row is never retried.
+ * - `pending` — the backing content simply isn't in memory yet (e.g. a
+ *   `local-only` row whose `localContent` hasn't been read). NOT cached:
+ *   the next load pass retries the row, so a late-arriving stat still lands.
+ */
+export type DiffStatLoadResult =
+    | { status: 'ready'; stat: ChangeStat }
+    | { status: 'pending' }
+    | { status: 'unavailable' };
+
+/**
+ * +/- stat for a two-sided diff (local-modified / remote-only /
+ * remote-modified / moved / conflict), reusing the existing LCS op logic in
+ * `utils/diff.ts`. Additions = added ops, deletions = removed ops.
+ */
+export function computeDiffStat(remote: string, local: string): ChangeStat {
+    const rows = computeSideBySideDiff(remote, local);
+    let additions = 0;
+    let deletions = 0;
+    for (const row of rows) {
+        if (row.right.type === 'added') additions++;
+        if (row.left.type === 'removed') deletions++;
+    }
+    return { additions, deletions };
+}
+
+/**
+ * Cheap stat for a `local-only` change: additions only (the local line
+ * count), no deletions and no remote/provider call. A trailing newline
+ * doesn't add a phantom line.
+ */
+export function cheapLocalStat(local: string): ChangeStat {
+    return { additions: countLines(local), deletions: 0 };
+}
+
+/**
+ * Stat for a one-sided change whose only content is the ADDED side: every
+ * line is an addition, no deletions. Used for `local-only` (A) and
+ * `remote-only` (↓) — both show +N, not the -N a content-vs-'' diff would
+ * produce for the download direction.
+ */
+export function addedContentStat(content: string): ChangeStat {
+    return { additions: countLines(content), deletions: 0 };
+}
+
+/**
+ * Stat for a one-sided DELETION: the content existed remotely and is gone
+ * locally, so every line is a deletion. Used for `local-deleted` (D).
+ */
+export function deletedContentStat(content: string): ChangeStat {
+    return { additions: 0, deletions: countLines(content) };
+}
+
+function countLines(s: string): number {
+    if (s === '') return 0;
+    const lines = s.replace(/\r\n/g, '\n').replace(/\r/g, '\n').split('\n');
+    return lines[lines.length - 1] === '' ? lines.length - 1 : lines.length;
+}

src/logic/sync/RenameReconciler.ts

@@ -0,0 +1,86 @@
+import { isSyncMetadataAtPath, type GitLabFilesPushSettings } from '../../settings';
+import type { SyncStatusService } from '../sync-status-service';
+import type { GitTreeEntry } from '../../services/git-service-interface';
+import { gitBlobSha } from '../../utils/git-blob-sha';
+import type { SyncManager } from './SyncManager';
+
+export interface RenameReconcilerDependencies {
+    settings: () => GitLabFilesPushSettings;
+    syncManager: () => SyncManager;
+    /** Republishes a single path's status after it's been reconciled as a rename target. */
+    refreshFileStatus(path: string, remoteEntry: GitTreeEntry | undefined): Promise<void>;
+}
+
+/**
+ * Reconciles renames performed outside the plugin (e.g. via git directly):
+ * matches an orphaned tracked path against an unsynced local file by blob
+ * sha, and relocates the sync metadata. Deliberately owns no discovery or
+ * status-resolution logic.
+ */
+export class RenameReconciler {
+    constructor(
+        private readonly dependencies: RenameReconcilerDependencies,
+        private readonly statuses: SyncStatusService,
+    ) {}
+
+    async reconcileOutOfBandMoves(remoteMap: Map<string, GitTreeEntry>): Promise<void> {
+        const orphansBySha = this.orphanedMoveSourcesBySha(remoteMap);
+        if (orphansBySha.size === 0) return;
+        const candidatesBySha = await this.unsyncedMoveDestinationsBySha(remoteMap, orphansBySha);
+
+        for (const [sha, orphanPaths] of orphansBySha) {
+            if (orphanPaths.length !== 1) continue;
+            const newPaths = candidatesBySha.get(sha);
+            if (!newPaths || newPaths.length !== 1) continue;
+            const oldPath = orphanPaths[0] as string;
+            const newPath = newPaths[0] as string;
+            await this.dependencies.syncManager().trackRename(newPath, oldPath);
+            this.statuses.delete(oldPath);
+            await this.dependencies.refreshFileStatus(newPath, remoteMap.get(newPath));
+        }
+    }
+
+    pendingMoveOldPaths(): Set<string> {
+        const paths = new Set<string>();
+        for (const metadata of Object.values(this.dependencies.settings().syncMetadata ?? {})) {
+            if (metadata.renamedFrom) paths.add(metadata.renamedFrom);
+        }
+        return paths;
+    }
+
+    private orphanedMoveSourcesBySha(remoteMap: Map<string, GitTreeEntry>): Map<string, string[]> {
+        const metadata = this.dependencies.settings().syncMetadata ?? {};
+        const orphansBySha = new Map<string, string[]>();
+        for (const [path, status] of this.statuses) {
+            // A tracked-then-deleted file is now classified `local-deleted`
+            // (not `remote-only`), so both qualify as an orphaned move
+            // source: the remote entry still exists, sync metadata is
+            // present for the path, and it isn't itself a pending move.
+            if (status.status !== 'remote-only' && status.status !== 'local-deleted') continue;
+            const pathMetadata = metadata[path];
+            if (!isSyncMetadataAtPath(pathMetadata, path) || pathMetadata.renamedFrom) continue;
+            const entry = remoteMap.get(path);
+            if (!entry || entry.symlink || !entry.sha) continue;
+            const paths = orphansBySha.get(entry.sha) ?? [];
+            paths.push(path);
+            orphansBySha.set(entry.sha, paths);
+        }
+        return orphansBySha;
+    }
+
+    private async unsyncedMoveDestinationsBySha(
+        remoteMap: Map<string, GitTreeEntry>,
+        orphansBySha: Map<string, string[]>,
+    ): Promise<Map<string, string[]>> {
+        const candidatesBySha = new Map<string, string[]>();
+        for (const [path, status] of this.statuses) {
+            if (status.status !== 'unsynced' || status.localContent === undefined || remoteMap.has(path)) continue;
+            const sha = await gitBlobSha(status.localContent);
+            if (!orphansBySha.has(sha)) continue;
+            const paths = candidatesBySha.get(sha) ?? [];
+            paths.push(path);
+            candidatesBySha.set(sha, paths);
+        }
+        return candidatesBySha;
+    }
+}

src/logic/sync/SyncDiffService.ts

@@ -1,8 +1,7 @@
 import type { SyncStatusService } from '../sync-status-service';
 import { isBinaryPath } from '../../utils/path';
 import type { FileDiff } from './types';
-import { computeDiffStat } from '../../ui/source-control/ChangePresentation';
-import type { DiffStatLoadResult } from '../../ui/source-control/DiffStatProvider';
+import { computeDiffStat, type DiffStatLoadResult } from './DiffStat';
 
 export type BlobReader = (sha: string, path: string) => Promise<{ content: string | ArrayBuffer }>;