diff --git a/CLAUDE.md b/CLAUDE.md index 91607d5a..d20bf5da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - **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**: `src/ui/SyncStatusView.ts` renders the sync status side panel; `src/ui/components/` holds its sub-views. +- **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." - **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. diff --git a/docs/source-control-refactor/phase-1-viewmodel-foundation.md b/docs/source-control-refactor/phase-1-viewmodel-foundation.md index ca14c94d..cc23b501 100644 --- a/docs/source-control-refactor/phase-1-viewmodel-foundation.md +++ b/docs/source-control-refactor/phase-1-viewmodel-foundation.md @@ -1,5 +1,8 @@ # Phase 1 — Source Control ViewModel Foundation +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 建立 Source Control UI 與 Sync domain 之間的 ViewModel layer。 diff --git a/docs/source-control-refactor/phase-2-action-unification.md b/docs/source-control-refactor/phase-2-action-unification.md index f9623bdd..11b09ad9 100644 --- a/docs/source-control-refactor/phase-2-action-unification.md +++ b/docs/source-control-refactor/phase-2-action-unification.md @@ -1,5 +1,8 @@ # Phase 2 — Sync Action Unification +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 統一 Source Control、Context Menu、Single File 操作的 pipeline。 diff --git a/docs/source-control-refactor/phase-3-source-control-ui.md b/docs/source-control-refactor/phase-3-source-control-ui.md index f9299175..1f89e9ec 100644 --- a/docs/source-control-refactor/phase-3-source-control-ui.md +++ b/docs/source-control-refactor/phase-3-source-control-ui.md @@ -1,5 +1,8 @@ # Phase 3 — Source Control UI +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 建立 VS Code style Source Control workflow。 diff --git a/docs/source-control-refactor/phase-4-legacy-cleanup.md b/docs/source-control-refactor/phase-4-legacy-cleanup.md index a1da7103..56b4db12 100644 --- a/docs/source-control-refactor/phase-4-legacy-cleanup.md +++ b/docs/source-control-refactor/phase-4-legacy-cleanup.md @@ -1,5 +1,8 @@ # Phase 4 — Legacy Cleanup +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 移除舊 Source Control orchestration，保留同步核心能力。 diff --git a/docs/source-control-refactor/roadmap.md b/docs/source-control-refactor/roadmap.md index a928ff3e..d5cc3fd8 100644 --- a/docs/source-control-refactor/roadmap.md +++ b/docs/source-control-refactor/roadmap.md @@ -1,8 +1,12 @@ # Source Control Refactor — Roadmap (v2) +> **Historical migration roadmap. Do not use as current implementation +> guidance.** The migration this document tracked has landed on `main`; for +> the current architecture see `docs/source-control.md`. + > Supersedes `phase-1..4-*.md`. Those phase docs are kept only as historical -> design notes; this file is the authoritative current plan, grounded in the -> actual branch state as of 2026-08-22. +> design notes; this file was the authoritative current plan as of +> 2026-08-22, before the migration it tracked landed on `main`. ## Where we actually are diff --git a/docs/source-control.md b/docs/source-control.md new file mode 100644 index 00000000..93ed5fb4 --- /dev/null +++ b/docs/source-control.md @@ -0,0 +1,44 @@ +# Source Control — Current Architecture + +The Source Control side panel is the plugin's only sync UI. There is no +separate "sync status" view; `docs/source-control-refactor/` describes the +historical migration into this architecture and is not current guidance. + +## Call chain + +``` +SourceControlItemView (src/ui/source-control/SourceControlItemView.ts) + └─ SourceControlView (src/ui/source-control/SourceControlView.ts) + └─ SourceControlActionService (src/logic/source-control/SourceControlActionService.ts) + └─ SyncWorkspace (src/logic/sync/SyncWorkspace.ts) + └─ SyncManager + executors (src/logic/sync/, e.g. PushExecutor, + PullExecutor, RemoteDeleteExecutor) +``` + +- `SourceControlItemView` is the `ItemView` Obsidian mounts; it owns no + rendering logic itself and delegates to `SourceControlView`. +- `SourceControlView` renders the change tree, Sync Queue, and diff surfaces + (`src/ui/components/`, `src/ui/source-control/DiffTabView.ts`), and turns + clicks into calls on `SourceControlActionService`. +- `SourceControlActionService` converts Source Control intent (push / pull / + delete-remote / delete-local / resolve-conflict) into `SyncWorkspace` calls + and reports outcome via `OperationState`. It never talks to a git provider + directly. +- `SyncWorkspace` is the execution boundary: it drives the real `SyncManager` + and provider-mutating executors (`PushExecutor`, `PullExecutor`, + `RemoteDeleteExecutor`, etc.), which in turn call `GitServiceInterface` + (`src/services/`). + +## Compatibility identifiers (do not remove) + +- `SOURCE_CONTROL_VIEW_TYPE = 'sync-status-view'` — kept so pinned leaves and + saved workspace layouts from before the Source Control migration resolve to + the current `SourceControlItemView` instead of breaking. +- The `open-sync-status` command id — same reason; it already routes to + `activateSourceControlView()`. + +## Legacy surface (removed, do not reintroduce) + +`SyncStatusView` and `ui/sync-status/*` were the pre-migration UI and no +longer exist in `src/`. An ESLint `no-restricted-imports` rule +(`eslint.config.*`) blocks reintroducing imports from those paths. diff --git a/e2e-tests/provider/suites/sync-manager.e2e.test.ts b/e2e-tests/provider/suites/sync-manager.e2e.test.ts index 57f16cce..718c0688 100644 --- a/e2e-tests/provider/suites/sync-manager.e2e.test.ts +++ b/e2e-tests/provider/suites/sync-manager.e2e.test.ts @@ -4,12 +4,20 @@ import { SyncPlanModal, SyncPlanDirection } from '../../../src/ui/SyncPlanModal' import { BatchConflictResolutionModal } from '../../../src/ui/BatchConflictResolutionModal'; import { ObsidianSyncInteraction } from '../../../src/ui/ObsidianSyncInteraction'; import { describePushResult } from '../support/push-result-diagnostic'; +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 { toChangeId } from '../../../src/logic/source-control/types'; // `import type` deliberately, not a value import: src/settings.ts also // exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest -> // AbstractInputSuggest etc.) which pulls in far more of `obsidian` than this // suite's minimal runtime shim provides. A type-only import is erased // entirely, so none of that module ever loads. import type { GitLabFilesPushSettings } from '../../../src/settings'; +import type { SyncStatusRefreshService } from '../../../src/logic/sync/SyncStatusRefreshService'; +import type { SyncDiffService } from '../../../src/logic/sync/SyncDiffService'; +import type { App } from 'obsidian'; import { TFile as ObsidianTFile } from 'obsidian'; import { GitVerifier } from '../support/git-verifier'; import { FakeVault, fakeApp, type TFileLike, type TFileCtor } from '../shim/fake-vault'; @@ -222,9 +230,14 @@ describe('SyncManager E2E', () => { expect(headAfterParent).toBe(headBefore); }); - it('deletes a file via the real service, verified independently', async () => { - // Deletion isn't a SyncManager method -- src/ui/SyncStatusView.ts calls - // gitService.deleteFile directly, so this reproduces that real path. + it('deletes a file via the current Source Control application path, verified independently', async () => { + // Deletion isn't a SyncManager method -- the production call chain is + // SourceControlActionService.deleteRemote() -> SyncWorkspace.deleteRemote() + // -> RemoteDeleteExecutor -> gitService.deleteFile(), not a direct + // provider call, so this exercises that full chain instead of + // bypassing it. `refreshService`/`diffService`/`app` are stubbed -- + // deleteRemote() never touches them -- the same pattern + // tests/logic/sync/SyncWorkspace.test.ts uses for its deleteRemote suite. const filePath = path('to-delete.md'); const vault = new FakeVault(TFile); vault.writeLocal(filePath, 'delete me'); @@ -235,9 +248,24 @@ describe('SyncManager E2E', () => { expect(initialPush.failed, describePushResult(initialPush)).toBe(0); expect(await verifier.fileMissing(filePath, branch)).toBe(false); - await service.deleteFile(filePath, branch, 'e2e: delete file'); - await manager.clearMetadata(filePath); + const changeId = toChangeId(filePath); + const repository = new ChangeRepository(); + repository.replace([{ id: changeId, path: filePath, kind: 'remote-only' }]); + const operations = new OperationState(); + const workspace = new SyncManagerWorkspace({ + manager: () => manager, + gitService: () => service, + settings: () => settings, + refreshService: {} as SyncStatusRefreshService, + diffService: {} as SyncDiffService, + normalizePath: p => p, + app: {} as App, + }); + const actionService = new SourceControlActionService(repository, operations, workspace); + + await actionService.deleteRemote([changeId]); + expect(operations.get(changeId)).toBe('success'); expect(await verifier.fileMissing(filePath, branch)).toBe(true); expect(settings.syncMetadata[filePath]).toBeUndefined(); }); diff --git a/tests/ci-workflow.test.ts b/tests/ci-workflow.test.ts index ce85aea6..4d3975fe 100644 --- a/tests/ci-workflow.test.ts +++ b/tests/ci-workflow.test.ts @@ -7,6 +7,7 @@ const harness = readFileSync('scripts/e2e-harness.sh', 'utf8'); const runner = readFileSync('scripts/run-e2e.sh', 'utf8'); const vitestE2eConfig = readFileSync('vitest.e2e.config.ts', 'utf8'); const eslintConfig = readFileSync('eslint.config.mts', 'utf8'); +const syncManagerE2eSuite = readFileSync('e2e-tests/provider/suites/sync-manager.e2e.test.ts', 'utf8'); describe('CI workflow contracts', () => { it('retries transient provider failures three times', () => { @@ -131,4 +132,29 @@ describe('E2E scanner-boundary contracts (e2e-tests/provider, no runtime generat expect(eslintConfig).toContain('"e2e-tests/**/*.ts"'); expect(eslintConfig).not.toContain('"e2e/**/*.ts"'); }); + + it('still blocks src/ imports of the removed legacy sync-status presentation layer', () => { + // Architecture regression guard for the SyncStatusView -> Source + // Control migration (see docs/source-control.md): a future refactor + // must not silently drop this no-restricted-imports rule and let + // ui/sync-status or SyncStatusView get re-wired back in. + expect(eslintConfig).toContain('"**/ui/sync-status"'); + expect(eslintConfig).toContain('"**/ui/sync-status/*"'); + expect(eslintConfig).toContain('"**/SyncStatusView"'); + expect(eslintConfig).toContain('"**/ui/SyncStatusView"'); + expect(eslintConfig).toContain('no-restricted-imports'); + }); + + it('exercises remote delete through the Source Control application path, not a direct provider bypass', () => { + // The remote-delete E2E used to call `service.deleteFile()` directly, + // reproducing what the removed SyncStatusView UI used to do. The + // current production path is SourceControlActionService.deleteRemote() + // -> SyncWorkspace.deleteRemote() -> RemoteDeleteExecutor -> + // gitService.deleteFile() -- a future edit must keep exercising that + // chain instead of quietly reverting to the raw provider call. + expect(syncManagerE2eSuite).not.toMatch(/\bservice\.deleteFile\(/); + expect(syncManagerE2eSuite).toContain('actionService.deleteRemote('); + expect(syncManagerE2eSuite).toContain("from '../../../src/logic/source-control/SourceControlActionService'"); + expect(syncManagerE2eSuite).toContain("from '../../../src/logic/sync/SyncWorkspace'"); + }); });