diff --git a/progress.md b/progress.md index bb8a526..73af21a 100644 --- a/progress.md +++ b/progress.md @@ -4,8 +4,8 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont ## Current State -**Last Updated:** 2026-08-20 -**Active Feature:** feat-026 / issue #105 — sync architecture refactor on `refactor/sync-domain-pipeline`. `SyncPlanner` is now the decision source for normal push, batch pull/preview, single pull, and moves. Edited tracked renames with a free destination plan one move instead of being auto-skipped; remote-only changes pull without false conflicts; real two-sided divergence and occupied move destinations remain conflicts. Post-push CI hardening is locally green; real provider CI plus Obsidian desktop/mobile manual verification remain before declaring the feature complete. +**Last Updated:** 2026-08-22 +**Active Feature:** feat-026 / issue #105 — Source Control refactor. Stacked PRs on `claude/source-control-foundation` (PR #129 → main): **#129** foundation + integration (`4e647fb`, legacy UI removed, `SourceControlItemView` wired as sole entry); **#130** execution-result + conflict status (`b3379b7`, `OperationStatus 'conflict'` + `ExecutionResult`); **PR131 state-model unification (uncommitted, this session)** — `src/logic/source-control/state/` with `SourceControlState` container + `SelectionState`/`OperationState`/`FilterState`/`ExpandedNodesState`/`SelectedChangeState`; `SourceControlViewModel` is the single mutation facade; `SourceControlView` holds no state and mutates only through the ViewModel. Pure refactor, no behavior change. Gate green: `npx eslint .` 0 errors; `npm run build` PASS (Obsidian 1.11 compat); `npx vitest run` 59 files / 553 tests. **Manual Obsidian verification remains** for #129/#130 (UI surface) before final merge. Phase 1 string purge intentionally skipped (kept `sync-status-view` view type + `open-sync-status` command id for pinned-leaf migration). **Parallel Work:** PR #87 (4x Dependabot security alerts via npm overrides) and Issue #57 (live-credential smoke test). ## Outstanding Items @@ -17,6 +17,8 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont ## Latest Evidence +- [x] PR130 Source Control execution-result + conflict status (2026-08-22), branch `feat/source-control-integration` (uncommitted): added `'conflict'` to `OperationState.OperationStatus` with a `conflict()` method (distinct lifecycle from `'failed'`); new `ExecutionResult` (`{completed, conflicts, failed}` `ChangeId[]`) as a thin UI projection derived in `SourceControlActionService` from existing `PushResults`/`SyncResult`/`RemoteDeleteResult` — no new sync model; `push`/`pull`/`deleteRemote`/`deleteLocal`/`resolveConflict` now return `Promise`, with `push` reading `PushResults.conflictedPaths` → `'conflict'` (conflict takes precedence over error); `SourceControlViewModel` exposes `lastOperationResult` via `setOperationResult`/`clearOperationResult`. Known asymmetry by design: pull `SyncResult` has only a conflict count (no per-path), so pull conflicts surface via change-model reclassification (`kind:'conflict'`), not this projection. TDD: new `ExecutionResult.test.ts` plus extended OperationState/ActionService/ViewModel tests covering conflict mapping, mixed 7/3/1 batch, conflict-precedence, throw→all-failed, and return-value contracts. Verification: `npx eslint .` — 0 errors; `npm run build` — clean incl. Obsidian 1.11.0 compatibility; `npx vitest run` — 66 files / 700 tests (source-control logic 7 files / 60 tests). No UI, no sync-domain changes, per the user's "先把 execution semantics 補完整，不要先做 UI" directive. + - [x] Issue #105 post-push CI hardening (2026-08-20), commit `948df28`: diagnosed run 32336155736 as two exhausted transient-provider attempts rather than a planner regression (GitHub 503/socket close; GitLab deadline exceeded). Increased provider E2E attempts from 2 to 3. A duplicate matrix cancelled by the shared push/PR concurrency group now produces a neutral aggregate gate with `run-ci=false`, so it neither creates a misleading `E2E gate` failure nor starts duplicate downstream CI; real failures still block. SyncManager E2E push preconditions now include `success`, `failed`, and provider `errors` in assertion diagnostics instead of surfacing only a secondary count mismatch. Added workflow contract and diagnostic unit tests and updated the E2E documentation. Verification: `actionlint v1.7.12 .github/workflows/ci.yml` — 0 errors; `npx eslint .` — 0 errors; `npm run build` — clean including Obsidian 1.11 compatibility; `npx vitest run` — 56 files / 613 tests; `npm run test:e2e -- --provider gitea` — 2 files / 14 tests and container cleanup; `git diff --check` — clean. Real CI run 32338116598 passed GitHub/GitLab production E2E, independent verification, cleanup, aggregate gate, Node 22/24 tests, lint, package, and build/release. The initial disabled-Gitea job landed on offline runner `heavenweb-runner-8`; failed-only rerun completed its skip in 11s and the full run concluded success. Provider API checks found no remaining `e2e/pr/127/**` or branch-source E2E refs. AGENTS-required Haiku was unavailable, so verification ran locally and through real CI. - [x] Issue #105 unified sync decisions and move regression (2026-08-20): added operation-aware `SyncPlanner.planFor(push|pull)`, `MoveFacts`, and the `move` domain action. Normal push, batch pull and preview, single pull, and tracked moves now consume planner decisions instead of reimplementing SHA conflict checks. Removed `PushCoordinator.queueMove`'s stale-metadata gate, so an edited tracked rename with a free destination appears under Moves and commits once; occupied destinations remain conflicts. Fixed the complementary pull false positive: a remote-only change now pulls, while real two-sided divergence still resolves as conflict. Content-fetched text/binary paths normalize equal bytes to the provider blob SHA before planning, preserving binary and GitLab legacy-baseline behavior. Added planner operation matrix, coordinator move regression, batch pull, and single pull coverage. Verification: `npx eslint .` — 0 errors; `npm run build` — clean including Obsidian 1.11 compatibility; `npx vitest run` — 54 files / 610 tests; `git diff --check` — clean. Manual Obsidian verification remains. diff --git a/session-handoff.md b/session-handoff.md index 9a92258..57b26ae 100644 --- a/session-handoff.md +++ b/session-handoff.md @@ -1,44 +1,68 @@ # Session Handoff -**Date:** 2026-08-20 -**Branch:** `refactor/sync-domain-pipeline` (PR #127) -**Active Feature:** feat-026 / issue #105 — sync architecture refactor +**Date:** 2026-08-22 +**Branch:** `refactor/source-control-state-model` (PR131, base +`feat/source-control-integration` PR #130 `b3379b7`) +**Active Feature:** feat-026 / issue #105 — Source Control refactor +(plan: `docs/source-control-refactor/roadmap.md`) -## Completed This Session +## Completed this session — PR131 (Phase 2: unify state model) -Investigated the failed real-provider CI after the unified planner commit. The move paths passed; -GitHub exhausted two attempts on a 503 and `UND_ERR_SOCKET`, while GitLab exhausted two attempts -on provider deadline errors. The tests then surfaced secondary count/existence assertions that -hid those original request failures. +Reorganized Source Control UI state from scattered View-local fields + direct +store access into one `SourceControlState` container, with the ViewModel as the +single facade the View reads from and mutates through. **No behavior change** +— pure refactor; all 553 tests pass. -Hardened CI with three provider attempts, explicit push-result diagnostics in SyncManager E2E, -and workflow contract coverage. When the shared push/PR concurrency group cancels a duplicate -matrix, its aggregate gate now reports the replacement neutrally and emits `run-ci=false`, so it -does not leave an additional aggregate red check or run downstream CI twice. Real failures remain -blocking. Updated the real-provider E2E documentation to match. +- New `src/logic/source-control/state/`: + - `SourceControlState.ts` — thin container composing the change model + every + UI state slice (selection, operation, filter, expanded nodes, selected + change). Not a god-object: each slice keeps its own invariants. + - `SelectionState.ts` — moved/renamed from `PushSelectionStore` (same API). + - `OperationState.ts` — moved from `src/logic/source-control/` (incl. the + `'conflict'` status from PR #130). + - `FilterState.ts` — new (active filter; was View-local). + - `ExpandedNodesState.ts` — new (collapsed sections + folders; was View-local). + - `SelectedChangeState.ts` — new (selected change id; was View-local). +- `SourceControlViewModel` — now constructed from `SourceControlState`; is the + mutation facade (`setFilter`, `toggleSection/Folder`, `selectForPush`/ + `deselectFromPush`, `selectForDiff`/`clearSelection`, `getCollapsedFolders`, + `setOperationResult`/`clear`). The View no longer reaches any store directly. +- `SourceControlView` — thinned: holds **no state** (no `filter`/ + `collapsedSections`/`collapsedFolders`/`selectedChangeId`); reads everything + from the ViewModel and mutates only through it. `getFilter`/`getSelected + ChangeId` moved to the ViewModel. Constructor is now `(viewModel, callbacks)`. +- `main.ts` / `SourceControlItemView` — construct `SourceControlState`; plugin + exposes `sourceControlState` (replaces `pushSelectionStore`/`operationState`). +- `SourceControlFilter.matchesFilter` — selection param type is `SelectionState`. +- Tests: moved `OperationState.test.ts`/`PushSelectionStore.test.ts` into + `state/` (renamed to `SelectionState.test.ts`); new `FilterState.test.ts` / + `ExpandedNodesState.test.ts` / `SelectedChangeState.test.ts`; updated + ViewModel/View/ItemView/ActionService tests for the new construction. -Committed as `948df28` (`fix(ci): harden provider e2e failures`) and pushed to -`origin/refactor/sync-domain-pipeline`. The pre-existing untracked `.codex-gitlab.env` remains -untouched. - -## Verification Evidence +## Verification evidence ```text -npx eslint . -> PASS, 0 errors -npm run build -> PASS, incl. Obsidian 1.11 compatibility -npx vitest run -> PASS, 56 files / 613 tests -npm run test:e2e -- --provider gitea -> PASS, 2 files / 14 tests; container removed -actionlint v1.7.12 .github/workflows/ci.yml -> PASS, 0 errors -git diff --check -> PASS -real CI run 32338116598 -> PASS after failed-only rerun of a disabled Gitea leg assigned to an offline runner -GitHub/GitLab sandbox branch query -> PASS, no e2e/pr/127 or source-branch refs remain +npx eslint . -> 0 errors +npm run build -> PASS (tsc + Obsidian 1.11.0 compat + esbuild) +npx vitest run -> 59 files / 553 tests ``` -The AGENTS-required Haiku verifier was unavailable in this environment, so verification ran -locally in this session. +## Exact next step + +1. Review/commit PR131 (uncommitted now) + push; PR base = PR #130 branch. +2. PR #129 (foundation + integration `4e647fb`) and PR #130 (conflict status + `b3379b7`) still need **manual Obsidian verification** before final merge + (DoD for the UI surface). PR131 doesn't change runtime behavior, so it + shares that same manual-verification requirement once stacked. +3. Remaining Clean-Plan items: Phase 3 (action pipeline — already mostly clean, + verify no direct sync in UI: confirmed clean; single/batch already unified), + Phase 5 (view split — mostly done; `ConflictPanel` is Phase C feature, deferred), + Phase 7 (docs reorg into `docs/source-control/*`), Phase 6 (workflow test + coverage — PR135). The user's directive: clean architecture first, then + conflict UX (Phase C). -## Exact Next Step +## Local working tree -Complete the remaining Obsidian desktop/mobile move smoke tests. Verify moving and editing a -tracked file appears under Moves and applies as one remote move, while an occupied remote -destination remains a skipped conflict. +- `package-lock.json` modified (from `npm ci`); not committed. npm caches were + cleared this session to free disk (`~/.npm/_npx`, `~/.npm/_cacache`). + `/tmp/kilo/pr130/node_modules` was removed to free space (regenerable). \ No newline at end of file diff --git a/src/logic/source-control/ExecutionResult.ts b/src/logic/source-control/ExecutionResult.ts new file mode 100644 index 0000000..e55ce9e --- /dev/null +++ b/src/logic/source-control/ExecutionResult.ts @@ -0,0 +1,28 @@ +import type { ChangeId } from './types'; + +/** + * UI-facing projection of a batch operation's outcome: which targeted changes + * completed, which need conflict resolution, and which hard-failed. + * + * This is a **projection** derived in `SourceControlActionService` from the + * existing sync-domain results (`PushResults`/`SyncResult`/`RemoteDeleteResult`) + * — not a new sync model. It exists so the Source Control UI can render a batch + * summary ("7 completed / 3 conflicts / 1 failed") instead of only a binary + * success/failed. + * + * Per-change persistence lives in `OperationState`; this type is the transient + * aggregate the ViewModel exposes to the UI for one render cycle. + */ +export interface ExecutionResult { + /** Changes that completed without conflict or error. */ + completed: ChangeId[]; + /** Changes the executor reported as conflicts (needs-resolution, not failed). */ + conflicts: ChangeId[]; + /** Changes that hard-failed (network/permission/etc.). */ + failed: ChangeId[]; +} + +/** A fresh, empty result with independent arrays (no shared prototype aliasing). */ +export function emptyExecutionResult(): ExecutionResult { + return { completed: [], conflicts: [], failed: [] }; +} \ No newline at end of file diff --git a/src/logic/source-control/SourceControlActionService.ts b/src/logic/source-control/SourceControlActionService.ts index 567d364..b635a82 100644 --- a/src/logic/source-control/SourceControlActionService.ts +++ b/src/logic/source-control/SourceControlActionService.ts @@ -1,6 +1,7 @@ import type { SyncWorkspace } from '../sync/SyncWorkspace'; import type { ChangeRepository } from './ChangeRepository'; -import type { OperationState } from './OperationState'; +import { emptyExecutionResult, type ExecutionResult } from './ExecutionResult'; +import type { OperationState } from './state/OperationState'; import type { SourceControlItem } from './SourceControlViewModel'; import type { ChangeId, SyncChange } from './types'; @@ -37,63 +38,85 @@ export class SourceControlActionService { private readonly workspace: SyncWorkspace, ) {} - /** Pushes one or more changes (single push and batch push share this path). */ - async push(changeIds: readonly ChangeId[]): Promise { + /** + * Pushes one or more changes (single push and batch push share this path). + * Returns an `ExecutionResult` projecting the batch outcome: paths the + * executor reported as conflicts (via `PushResults.conflictedPaths`) become + * `conflicts` (needs-resolution), not `failed`. + */ + async push(changeIds: readonly ChangeId[]): Promise { const targets = this.resolve(changeIds); - if (targets.length === 0) return; + if (targets.length === 0) return emptyExecutionResult(); this.startAll(targets); try { const results = await this.workspace.push(targets.map(target => target.path)); + const conflicted = new Set(results.conflictedPaths ?? []); const failed = new Set(results.errors.map(error => error.file)); - this.finishAll(targets, path => (failed.has(path) ? 'failed' : 'success')); + return this.classify(targets, path => { + if (conflicted.has(path)) return 'conflict'; + if (failed.has(path)) return 'failed'; + return 'success'; + }); } catch { - this.failAll(targets); + return this.failAll(targets); } } - /** Pulls one or more changes. */ - async pull(changeIds: readonly ChangeId[]): Promise { + /** + * Pulls one or more changes. `SyncResult` carries only a conflict count, + * not per-path conflict info, so pull conflicts cannot be mapped to + * `ChangeId`s here; they surface through change-model reclassification + * (`kind: 'conflict'`) on the next repository refresh. Errors map to + * `failed`; everything else is `completed`. + */ + async pull(changeIds: readonly ChangeId[]): Promise { const targets = this.resolve(changeIds); - if (targets.length === 0) return; + if (targets.length === 0) return emptyExecutionResult(); this.startAll(targets); try { const results = await this.workspace.pull(targets.map(target => target.path)); const failed = new Set(results.errors.map(error => error.file)); - this.finishAll(targets, path => (failed.has(path) ? 'failed' : 'success')); + return this.classify(targets, path => (failed.has(path) ? 'failed' : 'success')); } catch { - this.failAll(targets); + return this.failAll(targets); } } /** Deletes one or more changes from the remote only. */ - async deleteRemote(changeIds: readonly ChangeId[]): Promise { + async deleteRemote(changeIds: readonly ChangeId[]): Promise { const targets = this.resolve(changeIds); - if (targets.length === 0) return; + if (targets.length === 0) return emptyExecutionResult(); this.startAll(targets); try { const result = await this.workspace.deleteRemote(targets.map(target => target.path)); const failed = new Set(result.errors.map(error => error.path)); - this.finishAll(targets, path => (failed.has(path) ? 'failed' : 'success')); + return this.classify(targets, path => (failed.has(path) ? 'failed' : 'success')); } catch { - this.failAll(targets); + return this.failAll(targets); } } - /** Deletes one or more changes from the local vault only. No batch primitive exists on `SyncWorkspace`, so each runs independently and one failure doesn't block the rest. */ - async deleteLocal(changeIds: readonly ChangeId[]): Promise { + /** + * Deletes one or more changes from the local vault only. No batch primitive exists on `SyncWorkspace`, so each runs independently and one failure doesn't block the rest. + */ + async deleteLocal(changeIds: readonly ChangeId[]): Promise { const targets = this.resolve(changeIds); + const result = emptyExecutionResult(); for (const target of targets) { this.operations.start(target.id); try { await this.workspace.deleteLocal(target.path); this.operations.succeed(target.id); + result.completed.push(target.id); } catch { this.operations.fail(target.id); + result.failed.push(target.id); } } + return result; } /** @@ -102,9 +125,9 @@ export class SourceControlActionService { * two primitives every other action uses, so no separate conflict-apply * pathway is introduced. */ - async resolveConflict(changeId: ChangeId, resolution: ConflictResolution): Promise { + async resolveConflict(changeId: ChangeId, resolution: ConflictResolution): Promise { const change = this.changes.getById(changeId); - if (!change) return; + if (!change) return emptyExecutionResult(); this.operations.start(changeId); try { @@ -114,8 +137,10 @@ export class SourceControlActionService { await this.workspace.pullOne(change.path); } this.operations.succeed(changeId); + return { completed: [changeId], conflicts: [], failed: [] }; } catch { this.operations.fail(changeId); + return { completed: [], conflicts: [], failed: [changeId] }; } } @@ -145,14 +170,38 @@ export class SourceControlActionService { for (const target of targets) this.operations.start(target.id); } - private finishAll(targets: readonly SyncChange[], statusFor: (path: string) => 'success' | 'failed'): void { + /** + * Maps each targeted change to a per-change `OperationStatus` and + * accumulates the `ExecutionResult` projection in one pass. Conflict takes + * precedence over failure when a path appears in both executor lists. + */ + private classify( + targets: readonly SyncChange[], + statusFor: (path: string) => 'success' | 'failed' | 'conflict', + ): ExecutionResult { + const result = emptyExecutionResult(); for (const target of targets) { - if (statusFor(target.path) === 'success') this.operations.succeed(target.id); - else this.operations.fail(target.id); + const status = statusFor(target.path); + if (status === 'success') { + this.operations.succeed(target.id); + result.completed.push(target.id); + } else if (status === 'conflict') { + this.operations.conflict(target.id); + result.conflicts.push(target.id); + } else { + this.operations.fail(target.id); + result.failed.push(target.id); + } } + return result; } - private failAll(targets: readonly SyncChange[]): void { - for (const target of targets) this.operations.fail(target.id); + private failAll(targets: readonly SyncChange[]): ExecutionResult { + const result = emptyExecutionResult(); + for (const target of targets) { + this.operations.fail(target.id); + result.failed.push(target.id); + } + return result; } } diff --git a/src/logic/source-control/SourceControlFilter.ts b/src/logic/source-control/SourceControlFilter.ts index 3229ec2..d959f90 100644 --- a/src/logic/source-control/SourceControlFilter.ts +++ b/src/logic/source-control/SourceControlFilter.ts @@ -1,4 +1,4 @@ -import type { PushSelectionStore } from './PushSelectionStore'; +import type { SelectionState } from './state/SelectionState'; import type { SyncChange } from './types'; export type SourceControlFilter = @@ -11,10 +11,10 @@ export type SourceControlFilter = /** * Whether `change` belongs under `filter`. `ready-to-push` is defined purely - * by `PushSelectionStore` membership — it's a user selection, not a fact + * by `SelectionState` membership — it's a user selection, not a fact * derivable from the change's kind alone. */ -export function matchesFilter(change: SyncChange, filter: SourceControlFilter, selection: PushSelectionStore): boolean { +export function matchesFilter(change: SyncChange, filter: SourceControlFilter, selection: SelectionState): boolean { switch (filter) { case 'all': return true; case 'changes': return change.kind !== 'synced'; diff --git a/src/logic/source-control/SourceControlViewModel.ts b/src/logic/source-control/SourceControlViewModel.ts index 317a285..9d490ea 100644 --- a/src/logic/source-control/SourceControlViewModel.ts +++ b/src/logic/source-control/SourceControlViewModel.ts @@ -1,7 +1,8 @@ -import type { ChangeRepository } from './ChangeRepository'; -import type { OperationState, OperationStatus } from './OperationState'; -import type { PushSelectionStore } from './PushSelectionStore'; +import type { ExecutionResult } from './ExecutionResult'; import { matchesFilter, type SourceControlFilter } from './SourceControlFilter'; +import type { SourceControlState } from './state/SourceControlState'; +import type { SectionFilter } from './state/ExpandedNodesState'; +import type { OperationStatus } from './state/OperationState'; import type { ChangeId, SyncChange, SyncChangeKind } from './types'; /** One row of UI-ready state for a change: its own facts plus derived selection/operation status. */ @@ -19,30 +20,73 @@ export interface SourceControlViewState { filter: SourceControlFilter; items: SourceControlItem[]; counts: Record; + /** Transient batch summary from the last action, or `null` if none/cleared. */ + lastOperationResult: ExecutionResult | null; } const ALL_FILTERS: SourceControlFilter[] = ['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts', 'synced']; /** - * Combines `SyncChange[]` (via `ChangeRepository`), `PushSelectionStore`, and - * `OperationState` into a single UI-ready snapshot. Holds no sync behavior of - * its own — it's a pure projection, so `SyncManager`/`SyncPlanner`/`SyncExecutor` - * stay untouched and the UI never needs to reach past this layer. + * The single Source Control UI state facade. Reads from `SourceControlState` + * (change model + selection + operation + filter + expanded nodes + selected + * change) and exposes a UI-ready projection. The View is pure layout + event + * binding and mutates state **only** through this facade — never reaching the + * state slices directly — so there is one source of truth and no parallel + * View-local state that can drift. + * + * It holds no sync behavior of its own: it's a projection + a thin mutation + * facade, so `SyncManager`/`SyncPlanner`/`SyncExecutor` stay untouched and the + * UI never needs to reach past this layer. */ export class SourceControlViewModel { - constructor( - private readonly changes: ChangeRepository, - private readonly selection: PushSelectionStore, - private readonly operations: OperationState, - ) {} - - getState(filter: SourceControlFilter = 'all'): SourceControlViewState { - const all = this.changes.getAll(); + private lastOperationResult: ExecutionResult | null = null; + + constructor(private readonly state: SourceControlState) {} + + // --- Read-side projection --- + + /** Projects the items + counts for `filter` (defaults to the active filter). */ + getState(filter: SourceControlFilter = this.state.filter.get()): SourceControlViewState { + const all = this.state.changes.getAll(); const items = all - .filter(change => matchesFilter(change, filter, this.selection)) + .filter(change => matchesFilter(change, filter, this.state.selection)) .map(change => this.toItem(change)); const counts = this.countByFilter(all); - return { filter, items, counts }; + return { filter, items, counts, lastOperationResult: this.lastOperationResult }; + } + + // --- Active UI state (moved out of the View) --- + + getFilter(): SourceControlFilter { return this.state.filter.get(); } + setFilter(filter: SourceControlFilter): void { this.state.filter.set(filter); } + + getSelectedChangeId(): ChangeId | null { return this.state.selectedChange.get(); } + selectForDiff(changeId: ChangeId): void { this.state.selectedChange.set(changeId); } + clearSelection(): void { this.state.selectedChange.clear(); } + + isSectionCollapsed(section: SectionFilter): boolean { return this.state.expanded.isSectionCollapsed(section); } + toggleSection(section: SectionFilter): void { this.state.expanded.toggleSection(section); } + isFolderCollapsed(path: string): boolean { return this.state.expanded.isFolderCollapsed(path); } + toggleFolder(path: string): void { this.state.expanded.toggleFolder(path); } + /** Snapshot of collapsed folder paths for the tree/section renderers (read-only during one render). */ + getCollapsedFolders(): Set { return this.state.expanded.getCollapsedFolders(); } + + // --- Selection mutation (routed through the facade, not direct to the store) --- + + selectForPush(changeId: ChangeId): void { this.state.selection.includeForPush(changeId); } + deselectFromPush(changeId: ChangeId): void { this.state.selection.excludeFromPush(changeId); } + + /** ChangeIds currently marked ready to push — feeds the toolbar Push action. */ + getSelectedChangeIds(): ChangeId[] { return this.state.selection.getSelectedChangeIds(); } + + // --- Operation-result projection (batch summary) --- + + setOperationResult(result: ExecutionResult): void { + this.lastOperationResult = result; + } + + clearOperationResult(): void { + this.lastOperationResult = null; } private toItem(change: SyncChange): SourceControlItem { @@ -51,16 +95,16 @@ export class SourceControlViewModel { path: change.path, previousPath: change.previousPath, kind: change.kind, - isReadyToPush: this.selection.isIncluded(change.id), - operationStatus: this.operations.get(change.id), + isReadyToPush: this.state.selection.isIncluded(change.id), + operationStatus: this.state.operations.get(change.id), }; } private countByFilter(changes: readonly SyncChange[]): Record { const counts = {} as Record; for (const filter of ALL_FILTERS) { - counts[filter] = changes.filter(change => matchesFilter(change, filter, this.selection)).length; + counts[filter] = changes.filter(change => matchesFilter(change, filter, this.state.selection)).length; } return counts; } -} +} \ No newline at end of file diff --git a/src/logic/source-control/state/ExpandedNodesState.ts b/src/logic/source-control/state/ExpandedNodesState.ts new file mode 100644 index 0000000..a929a3d --- /dev/null +++ b/src/logic/source-control/state/ExpandedNodesState.ts @@ -0,0 +1,37 @@ +import type { SourceControlFilter } from '../SourceControlFilter'; + +/** A filter that maps to a collapsible section (every filter except 'all'). */ +export type SectionFilter = Exclude; + +/** + * Which sections and folders the user has collapsed. UI presentation state + * kept out of the View so collapse state survives a status-driven re-render + * rather than resetting to the default every time. + */ +export class ExpandedNodesState { + private readonly collapsedSections = new Set(); + private readonly collapsedFolders = new Set(); + + isSectionCollapsed(section: SectionFilter): boolean { + return this.collapsedSections.has(section); + } + + toggleSection(section: SectionFilter): void { + if (this.collapsedSections.has(section)) this.collapsedSections.delete(section); + else this.collapsedSections.add(section); + } + + isFolderCollapsed(path: string): boolean { + return this.collapsedFolders.has(path); + } + + toggleFolder(path: string): void { + if (this.collapsedFolders.has(path)) this.collapsedFolders.delete(path); + else this.collapsedFolders.add(path); + } + + /** Snapshot of collapsed folder paths; consumers only read (`.has`) it during one render. */ + getCollapsedFolders(): Set { + return this.collapsedFolders; + } +} \ No newline at end of file diff --git a/src/logic/source-control/state/FilterState.ts b/src/logic/source-control/state/FilterState.ts new file mode 100644 index 0000000..16448c3 --- /dev/null +++ b/src/logic/source-control/state/FilterState.ts @@ -0,0 +1,18 @@ +import type { SourceControlFilter } from '../SourceControlFilter'; + +/** + * The currently active Source Control filter. A single-value UI state slice + * kept out of the View so the ViewModel/state is the single source of truth + * for what the UI is showing (no parallel local copy that can drift). + */ +export class FilterState { + private filter: SourceControlFilter = 'all'; + + get(): SourceControlFilter { + return this.filter; + } + + set(filter: SourceControlFilter): void { + this.filter = filter; + } +} \ No newline at end of file diff --git a/src/logic/source-control/OperationState.ts b/src/logic/source-control/state/OperationState.ts similarity index 69% rename from src/logic/source-control/OperationState.ts rename to src/logic/source-control/state/OperationState.ts index d03869e..047b6ba 100644 --- a/src/logic/source-control/OperationState.ts +++ b/src/logic/source-control/state/OperationState.ts @@ -1,6 +1,6 @@ -import type { ChangeId } from './types'; +import type { ChangeId } from '../types'; -export type OperationStatus = 'idle' | 'running' | 'success' | 'failed'; +export type OperationStatus = 'idle' | 'running' | 'success' | 'failed' | 'conflict'; /** * Tracks in-flight per-change operation status, independent of both the @@ -25,6 +25,16 @@ export class OperationState { this.status.set(changeId, 'failed'); } + /** + * Marks a change as needing resolution. This is a distinct lifecycle from + * `fail`: a conflict is a resolvable outcome (the executor reported both + * sides diverged), not an error. The UI must render it differently from a + * hard failure and offer resolution actions. + */ + conflict(changeId: ChangeId): void { + this.status.set(changeId, 'conflict'); + } + reset(changeId: ChangeId): void { this.status.delete(changeId); } @@ -36,4 +46,4 @@ export class OperationState { clear(): void { this.status.clear(); } -} +} \ No newline at end of file diff --git a/src/logic/source-control/state/SelectedChangeState.ts b/src/logic/source-control/state/SelectedChangeState.ts new file mode 100644 index 0000000..a30aa5d --- /dev/null +++ b/src/logic/source-control/state/SelectedChangeState.ts @@ -0,0 +1,23 @@ +import type { ChangeId } from '../types'; + +/** + * Stable identity of the change currently selected for diff viewing, or + * `null` when nothing is selected. Held as state (not View-local) so a + * re-render driven by a sync-status refresh keeps the selection instead of + * losing it. + */ +export class SelectedChangeState { + private selected: ChangeId | null = null; + + get(): ChangeId | null { + return this.selected; + } + + set(changeId: ChangeId | null): void { + this.selected = changeId; + } + + clear(): void { + this.selected = null; + } +} \ No newline at end of file diff --git a/src/logic/source-control/PushSelectionStore.ts b/src/logic/source-control/state/SelectionState.ts similarity index 93% rename from src/logic/source-control/PushSelectionStore.ts rename to src/logic/source-control/state/SelectionState.ts index 658a8c3..900a7f0 100644 --- a/src/logic/source-control/PushSelectionStore.ts +++ b/src/logic/source-control/state/SelectionState.ts @@ -1,4 +1,4 @@ -import type { ChangeId } from './types'; +import type { ChangeId } from '../types'; /** * Tracks which pending sync changes are "Ready to Push" — independent of the @@ -8,7 +8,7 @@ import type { ChangeId } from './types'; * Keyed by ChangeId rather than path so a rename/move doesn't drop the * selection. */ -export class PushSelectionStore { +export class SelectionState { private readonly selected = new Set(); includeForPush(changeId: ChangeId): void { @@ -36,4 +36,4 @@ export class PushSelectionStore { } } } -} +} \ No newline at end of file diff --git a/src/logic/source-control/state/SourceControlState.ts b/src/logic/source-control/state/SourceControlState.ts new file mode 100644 index 0000000..d9912c2 --- /dev/null +++ b/src/logic/source-control/state/SourceControlState.ts @@ -0,0 +1,30 @@ +import type { ChangeRepository } from '../ChangeRepository'; +import { ExpandedNodesState } from './ExpandedNodesState'; +import { FilterState } from './FilterState'; +import { OperationState } from './OperationState'; +import { SelectedChangeState } from './SelectedChangeState'; +import { SelectionState } from './SelectionState'; + +/** + * The single Source Control UI state boundary: one container composing the + * change model and every UI state slice (selection, operation, filter, + * expanded nodes, selected change). The ViewModel reads from this and the + * View mutates only through the ViewModel — nothing reaches the individual + * slices directly from the UI. + * + * Intentionally a thin composition, not a god-object: each slice keeps its + * own invariants and methods. The container exists so there is one thing to + * construct/wire (in `main.ts` / `SourceControlItemView`) and one place the + * "current UI state" is defined, instead of it being scattered across the + * View's local fields and several unrelated stores. + */ +export class SourceControlState { + constructor( + readonly changes: ChangeRepository, + readonly selection: SelectionState, + readonly operations: OperationState, + readonly filter: FilterState = new FilterState(), + readonly expanded: ExpandedNodesState = new ExpandedNodesState(), + readonly selectedChange: SelectedChangeState = new SelectedChangeState(), + ) {} +} \ No newline at end of file diff --git a/src/main.ts b/src/main.ts index f2af4a3..8af346b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -19,11 +19,12 @@ import { SyncStatusRefreshService } from './logic/sync/SyncStatusRefreshService' import { SyncDiffService } from './logic/sync/SyncDiffService'; import { SyncManagerWorkspace, type SyncWorkspace } from './logic/sync/SyncWorkspace'; import { ChangeRepository } from './logic/source-control/ChangeRepository'; -import { OperationState } from './logic/source-control/OperationState'; -import { PushSelectionStore } from './logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from './logic/source-control/SourceControlViewModel'; import { SourceControlActionService } from './logic/source-control/SourceControlActionService'; import { toSyncChanges } from './logic/source-control/FileStatusAdapter'; +import { SourceControlState } from './logic/source-control/state/SourceControlState'; +import { SelectionState } from './logic/source-control/state/SelectionState'; +import { OperationState } from './logic/source-control/state/OperationState'; export type ConnectionStatusState = 'checking' | 'connected' | 'disconnected'; @@ -40,8 +41,7 @@ export default class GitLabFilesPush extends Plugin { syncStatusRefresh: SyncStatusRefreshService; gitignoreManager: GitignoreManager; changeRepository: ChangeRepository; - pushSelectionStore: PushSelectionStore; - operationState: OperationState; + sourceControlState: SourceControlState; sourceControlViewModel: SourceControlViewModel; sourceControlActions: SourceControlActionService; private unsubscribeChangeRepository?: () => void; @@ -106,16 +106,13 @@ export default class GitLabFilesPush extends Plugin { }); this.changeRepository = new ChangeRepository(); - this.pushSelectionStore = new PushSelectionStore(); - this.operationState = new OperationState(); - this.sourceControlViewModel = new SourceControlViewModel( - this.changeRepository, - this.pushSelectionStore, - this.operationState, - ); + const selection = new SelectionState(); + const operations = new OperationState(); + this.sourceControlState = new SourceControlState(this.changeRepository, selection, operations); + this.sourceControlViewModel = new SourceControlViewModel(this.sourceControlState); this.sourceControlActions = new SourceControlActionService( this.changeRepository, - this.operationState, + operations, this.syncWorkspace, ); // Keeps ChangeRepository (and therefore the Source Control view) in @@ -124,7 +121,7 @@ export default class GitLabFilesPush extends Plugin { this.unsubscribeChangeRepository = this.sync.status.subscribe((statuses) => { const changes = toSyncChanges([...statuses.values()]); this.changeRepository.replace(changes); - this.pushSelectionStore.refresh(changes.map(change => change.id)); + selection.refresh(changes.map(change => change.id)); }); this.statusBarEl = this.addStatusBarItem(); diff --git a/src/ui/source-control/OperationIndicator.ts b/src/ui/source-control/OperationIndicator.ts index 8785db8..bba354d 100644 --- a/src/ui/source-control/OperationIndicator.ts +++ b/src/ui/source-control/OperationIndicator.ts @@ -1,6 +1,6 @@ import { setIcon } from 'obsidian'; import { ICONS } from '../components/icons'; -import type { OperationStatus } from '../../logic/source-control/OperationState'; +import type { OperationStatus } from '../../logic/source-control/state/OperationState'; /** * Renders a small per-change status indicator for an in-flight operation. diff --git a/src/ui/source-control/SourceControlItemView.ts b/src/ui/source-control/SourceControlItemView.ts index 90178e7..be08988 100644 --- a/src/ui/source-control/SourceControlItemView.ts +++ b/src/ui/source-control/SourceControlItemView.ts @@ -34,7 +34,6 @@ export class SourceControlItemView extends ItemView { }; this.view = new SourceControlView( this.plugin.sourceControlViewModel, - this.plugin.pushSelectionStore, callbacks, ); } @@ -71,7 +70,7 @@ export class SourceControlItemView extends ItemView { * through the subscription above; the explicit re-render here is what * covers the failure path, where nothing else republishes status. */ - private runAction(action: Promise): void { + private runAction(action: Promise): void { this.renderView(); void action.finally(() => this.renderView()); } diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index 0010a90..26a66e5 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -1,8 +1,7 @@ import { Platform } from 'obsidian'; import { t, type TranslationKey } from '../../i18n'; -import type { PushSelectionStore } from '../../logic/source-control/PushSelectionStore'; -import type { SourceControlFilter } from '../../logic/source-control/SourceControlFilter'; import { SourceControlViewModel, type SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; +import type { SectionFilter } from '../../logic/source-control/state/ExpandedNodesState'; import type { ChangeId } from '../../logic/source-control/types'; import { renderDiffPanel } from '../components/DiffPanel'; import { renderChangeSection } from './ChangeSection'; @@ -24,8 +23,6 @@ export interface SourceControlViewCallbacks { loadDiffContent?: (item: SourceControlItem) => Promise; } -type SectionFilter = Exclude; - /** The five Source Control sections, in the order the spec lists them. */ const SECTION_FILTERS: SectionFilter[] = ['ready-to-push', 'changes', 'remote-changes', 'conflicts', 'synced']; @@ -38,27 +35,21 @@ const SECTION_TITLE_KEYS: Record = { }; /** - * Composes the Source Control UI (Header, Filter, ChangeTree/sections, Diff - * panel) from `SourceControlViewModel` state, per - * docs/source-control-refactor/phase-3-source-control-ui.md. + * Pure layout + event binding for the Source Control UI (Header, Filter, + * ChangeTree/sections, Diff panel), composed from `SourceControlViewModel` + * state, per docs/source-control-refactor/roadmap.md. * - * Pure presentation + wiring: push/diff intent is handed to injected - * callbacks rather than acted on directly here, so this layer never reaches - * past the ViewModel to `SyncManager`/a Git provider. Selection toggling is - * the one exception — it goes straight to `PushSelectionStore` (Phase 1 - * state), since "ready to push" is just a set membership change, not a sync - * action. + * This layer holds **no state of its own** — the active filter, collapsed + * sections/folders, and selected change all live in `SourceControlState`, + * reached only through the ViewModel. Push/diff intent is handed to injected + * callbacks rather than acted on directly, so this layer never reaches past + * the ViewModel to `SyncManager`/a Git provider. */ export class SourceControlView { - private filter: SourceControlFilter = 'all'; - private readonly collapsedSections = new Set(); - private readonly collapsedFolders = new Set(); - private selectedChangeId: ChangeId | null = null; private container?: HTMLElement; constructor( private readonly viewModel: SourceControlViewModel, - private readonly selection: PushSelectionStore, private readonly callbacks: SourceControlViewCallbacks, ) {} @@ -71,7 +62,7 @@ export class SourceControlView { container.toggleClass('scv-mobile', isMobile); container.toggleClass('scv-desktop', !isMobile); - if (isMobile && this.selectedChangeId !== null) { + if (isMobile && this.viewModel.getSelectedChangeId() !== null) { this.renderDetail(container); return; } @@ -85,24 +76,22 @@ export class SourceControlView { } } - getFilter(): SourceControlFilter { return this.filter; } - getSelectedChangeId(): ChangeId | null { return this.selectedChangeId; } - private rerender(): void { if (this.container) this.render(this.container); } private renderMain(container: HTMLElement): void { - const state = this.viewModel.getState(this.filter); + const filter = this.viewModel.getFilter(); + const state = this.viewModel.getState(filter); renderSourceControlHeader( container, { readyToPushCount: state.counts['ready-to-push'] }, - { onPush: () => { void this.callbacks.onPush(this.selection.getSelectedChangeIds()); } }, + { onPush: () => { void this.callbacks.onPush(this.viewModel.getSelectedChangeIds()); } }, ); - renderFilterMenu(container, this.filter, state.counts, (filter) => { - this.filter = filter; + renderFilterMenu(container, filter, state.counts, (next) => { + this.viewModel.setFilter(next); this.rerender(); }); @@ -113,19 +102,24 @@ export class SourceControlView { } const treeCallbacks: ChangeTreeCallbacks = { - onToggleFolder: (path) => this.toggleFolder(path), - onToggleSelect: (id, selected) => this.toggleSelect(id, selected), + onToggleFolder: (path) => { this.viewModel.toggleFolder(path); this.rerender(); }, + onToggleSelect: (id, selected) => { + if (selected) this.viewModel.selectForPush(id); + else this.viewModel.deselectFromPush(id); + this.rerender(); + }, onOpenDiff: (item) => this.openDiff(item), }; - if (this.filter === 'all') { + if (filter === 'all') { this.renderSections(body, treeCallbacks); } else { - renderChangeTree(body, state.items, this.collapsedFolders, treeCallbacks); + renderChangeTree(body, state.items, this.viewModel.getCollapsedFolders(), treeCallbacks); } } private renderSections(body: HTMLElement, treeCallbacks: ChangeTreeCallbacks): void { + const collapsedFolders = this.viewModel.getCollapsedFolders(); for (const sectionFilter of SECTION_FILTERS) { const items = this.viewModel.getState(sectionFilter).items; if (items.length === 0) continue; @@ -136,35 +130,37 @@ export class SourceControlView { id: sectionFilter, title: t(SECTION_TITLE_KEYS[sectionFilter]), items, - collapsed: this.collapsedSections.has(sectionFilter), - collapsedFolders: this.collapsedFolders, + collapsed: this.viewModel.isSectionCollapsed(sectionFilter), + collapsedFolders, }, { ...treeCallbacks, - onToggleSection: (id) => this.toggleSection(id), + onToggleSection: (id) => { this.viewModel.toggleSection(id); this.rerender(); }, }, ); } } private renderDiffPane(container: HTMLElement): void { - if (!this.selectedChangeId) { + const selected = this.viewModel.getSelectedChangeId(); + if (!selected) { container.createDiv({ cls: 'scv-diff-empty', text: t('sourceControl.diff.selectPrompt') }); return; } - void this.loadAndRenderDiff(container, this.selectedChangeId); + void this.loadAndRenderDiff(container, selected); } private renderDetail(root: HTMLElement): void { const detail = root.createDiv({ cls: 'scv-detail' }); const backBtn = detail.createEl('button', { cls: 'scv-detail-back', text: t('sourceControl.detail.back') }); backBtn.addEventListener('click', () => { - this.selectedChangeId = null; + this.viewModel.clearSelection(); this.rerender(); }); const diffContainer = detail.createDiv({ cls: 'scv-detail-diff' }); - if (this.selectedChangeId) void this.loadAndRenderDiff(diffContainer, this.selectedChangeId); + const selected = this.viewModel.getSelectedChangeId(); + if (selected) void this.loadAndRenderDiff(diffContainer, selected); } private async loadAndRenderDiff(container: HTMLElement, changeId: ChangeId): Promise { @@ -174,31 +170,13 @@ export class SourceControlView { const content = await this.callbacks.loadDiffContent(item); // Stale response guard: the selection may have moved on while awaiting. - if (!content || this.selectedChangeId !== changeId) return; + if (!content || this.viewModel.getSelectedChangeId() !== changeId) return; renderDiffPanel(container, content.remote, content.local); } - private toggleSection(id: SectionFilter): void { - if (this.collapsedSections.has(id)) this.collapsedSections.delete(id); - else this.collapsedSections.add(id); - this.rerender(); - } - - private toggleFolder(path: string): void { - if (this.collapsedFolders.has(path)) this.collapsedFolders.delete(path); - else this.collapsedFolders.add(path); - this.rerender(); - } - - private toggleSelect(id: ChangeId, selected: boolean): void { - if (selected) this.selection.includeForPush(id); - else this.selection.excludeFromPush(id); - this.rerender(); - } - private openDiff(item: SourceControlItem): void { - this.selectedChangeId = item.id; + this.viewModel.selectForDiff(item.id); if (this.callbacks.onOpenDiff) void this.callbacks.onOpenDiff(item); this.rerender(); } -} +} \ No newline at end of file diff --git a/tests/logic/source-control/ExecutionResult.test.ts b/tests/logic/source-control/ExecutionResult.test.ts new file mode 100644 index 0000000..8083dec --- /dev/null +++ b/tests/logic/source-control/ExecutionResult.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { emptyExecutionResult } from '../../../src/logic/source-control/ExecutionResult'; +import { toChangeId } from '../../../src/logic/source-control/types'; + +describe('ExecutionResult', () => { + it('is an empty projection with no completed, conflicts, or failed', () => { + expect(emptyExecutionResult()).toEqual({ + completed: [], + conflicts: [], + failed: [], + }); + }); + + it('returns independent arrays so callers can mutate without aliasing the prototype', () => { + const a = emptyExecutionResult(); + const b = emptyExecutionResult(); + a.completed.push(toChangeId('c-1')); + + expect(b.completed).toEqual([]); + expect(a.completed).toEqual([toChangeId('c-1')]); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/SourceControlActionService.test.ts b/tests/logic/source-control/SourceControlActionService.test.ts index 6931d36..8f22fd2 100644 --- a/tests/logic/source-control/SourceControlActionService.test.ts +++ b/tests/logic/source-control/SourceControlActionService.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; -import { OperationState } from '../../../src/logic/source-control/OperationState'; +import { OperationState } from '../../../src/logic/source-control/state/OperationState'; import { SourceControlActionService } from '../../../src/logic/source-control/SourceControlActionService'; import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; import type { SyncWorkspace } from '../../../src/logic/sync/SyncWorkspace'; @@ -118,6 +118,101 @@ describe('SourceControlActionService', () => { expect(operations.get(toChangeId('c-1'))).toBe('failed'); }); + + it('marks a conflicted path as conflict (not failed) and reports it in the result', async () => { + const push = vi.fn().mockResolvedValue(emptyPushResults({ + syncedPaths: [{ path: 'a.md' }], + conflicts: 1, + conflictedPaths: ['b.md'], + })); + const { service, operations } = buildService( + [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'local-modified' }, + ], + fakeWorkspace({ push }), + ); + + const result = await service.push([toChangeId('c-1'), toChangeId('c-2')]); + + expect(operations.get(toChangeId('c-1'))).toBe('success'); + expect(operations.get(toChangeId('c-2'))).toBe('conflict'); + expect(result).toEqual({ + completed: [toChangeId('c-1')], + conflicts: [toChangeId('c-2')], + failed: [], + }); + }); + + it('classifies a mixed batch into completed, conflicts, and failed', async () => { + const push = vi.fn().mockResolvedValue(emptyPushResults({ + syncedPaths: [{ path: 'a.md' }, { path: 'b.md' }, { path: 'c.md' }], + conflicts: 1, + conflictedPaths: ['d.md'], + errors: [{ file: 'e.md', error: 'boom' }], + })); + const { service, operations } = buildService( + [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'local-only' }, + { id: toChangeId('c-3'), path: 'c.md', kind: 'local-only' }, + { id: toChangeId('c-4'), path: 'd.md', kind: 'local-modified' }, + { id: toChangeId('c-5'), path: 'e.md', kind: 'local-modified' }, + ], + fakeWorkspace({ push }), + ); + + const result = await service.push([ + toChangeId('c-1'), toChangeId('c-2'), toChangeId('c-3'), + toChangeId('c-4'), toChangeId('c-5'), + ]); + + expect(operations.get(toChangeId('c-1'))).toBe('success'); + expect(operations.get(toChangeId('c-4'))).toBe('conflict'); + expect(operations.get(toChangeId('c-5'))).toBe('failed'); + expect(result).toEqual({ + completed: [toChangeId('c-1'), toChangeId('c-2'), toChangeId('c-3')], + conflicts: [toChangeId('c-4')], + failed: [toChangeId('c-5')], + }); + }); + + it('prefers conflict over error when a path appears in both lists', async () => { + const push = vi.fn().mockResolvedValue(emptyPushResults({ + conflicts: 1, + conflictedPaths: ['a.md'], + errors: [{ file: 'a.md', error: 'also errored' }], + })); + const { service, operations } = buildService( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }], + fakeWorkspace({ push }), + ); + + const result = await service.push([toChangeId('c-1')]); + + expect(operations.get(toChangeId('c-1'))).toBe('conflict'); + expect(result.conflicts).toEqual([toChangeId('c-1')]); + expect(result.failed).toEqual([]); + }); + + it('returns a fully-failed ExecutionResult when SyncWorkspace throws', async () => { + const push = vi.fn().mockRejectedValue(new Error('network down')); + const { service } = buildService( + [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'local-only' }, + ], + fakeWorkspace({ push }), + ); + + const result = await service.push([toChangeId('c-1'), toChangeId('c-2')]); + + expect(result).toEqual({ + completed: [], + conflicts: [], + failed: [toChangeId('c-1'), toChangeId('c-2')], + }); + }); }); describe('pull', () => { @@ -145,6 +240,30 @@ describe('SourceControlActionService', () => { expect(operations.get(toChangeId('c-1'))).toBe('failed'); }); + + it('returns an ExecutionResult classifying pulled changes as completed or failed', async () => { + const pull = vi.fn().mockResolvedValue(emptySyncResult({ + errors: [{ file: 'b.md', error: 'boom' }], + })); + const { service } = buildService( + [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'remote-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'remote-modified' }, + ], + fakeWorkspace({ pull }), + ); + + const result = await service.pull([toChangeId('c-1'), toChangeId('c-2')]); + + // SyncResult carries only a conflict count, not per-path conflict + // info, so pull conflicts surface through change-model reclassification + // (kind: 'conflict') rather than this projection. + expect(result).toEqual({ + completed: [toChangeId('c-1')], + conflicts: [], + failed: [toChangeId('c-2')], + }); + }); }); describe('deleteRemote / deleteLocal', () => { @@ -219,6 +338,22 @@ describe('SourceControlActionService', () => { expect(operations.get(toChangeId('c-1'))).toBe('failed'); }); + + it('returns an ExecutionResult for a successful resolution', async () => { + const push = vi.fn().mockResolvedValue(emptyPushResults()); + const { service } = buildService( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'conflict' }], + fakeWorkspace({ push }), + ); + + const result = await service.resolveConflict(toChangeId('c-1'), 'local'); + + expect(result).toEqual({ + completed: [toChangeId('c-1')], + conflicts: [], + failed: [], + }); + }); }); describe('invalid ChangeId', () => { diff --git a/tests/logic/source-control/SourceControlViewModel.test.ts b/tests/logic/source-control/SourceControlViewModel.test.ts index eba3edc..c47c266 100644 --- a/tests/logic/source-control/SourceControlViewModel.test.ts +++ b/tests/logic/source-control/SourceControlViewModel.test.ts @@ -1,17 +1,20 @@ import { describe, expect, it } from 'vitest'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; -import { OperationState } from '../../../src/logic/source-control/OperationState'; -import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; +import { emptyExecutionResult } from '../../../src/logic/source-control/ExecutionResult'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; +import { SourceControlState } from '../../../src/logic/source-control/state/SourceControlState'; +import { OperationState } from '../../../src/logic/source-control/state/OperationState'; +import { SelectionState } from '../../../src/logic/source-control/state/SelectionState'; import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; function buildViewModel(changes: SyncChange[]) { const repository = new ChangeRepository(); repository.replace(changes); - const selection = new PushSelectionStore(); + const selection = new SelectionState(); const operations = new OperationState(); - const viewModel = new SourceControlViewModel(repository, selection, operations); - return { viewModel, selection, operations }; + const state = new SourceControlState(repository, selection, operations); + const viewModel = new SourceControlViewModel(state); + return { viewModel, selection, operations, state }; } describe('SourceControlViewModel', () => { @@ -105,4 +108,37 @@ describe('SourceControlViewModel', () => { expect(item?.path).toBe('new.md'); expect(item?.previousPath).toBe('old.md'); }); + + it('exposes no operation result until one is set', () => { + const { viewModel } = buildViewModel([]); + + expect(viewModel.getState().lastOperationResult).toBeNull(); + }); + + it('exposes the last operation result so the UI can render a batch summary', () => { + const { viewModel } = buildViewModel([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + ]); + + viewModel.setOperationResult({ + completed: [toChangeId('c-1'), toChangeId('c-2'), toChangeId('c-3')], + conflicts: [toChangeId('c-4')], + failed: [toChangeId('c-5')], + }); + + expect(viewModel.getState().lastOperationResult).toEqual({ + completed: [toChangeId('c-1'), toChangeId('c-2'), toChangeId('c-3')], + conflicts: [toChangeId('c-4')], + failed: [toChangeId('c-5')], + }); + }); + + it('clears the operation result back to null', () => { + const { viewModel } = buildViewModel([]); + viewModel.setOperationResult(emptyExecutionResult()); + + viewModel.clearOperationResult(); + + expect(viewModel.getState().lastOperationResult).toBeNull(); + }); }); diff --git a/tests/logic/source-control/state/ExpandedNodesState.test.ts b/tests/logic/source-control/state/ExpandedNodesState.test.ts new file mode 100644 index 0000000..f949d31 --- /dev/null +++ b/tests/logic/source-control/state/ExpandedNodesState.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { ExpandedNodesState } from '../../../../src/logic/source-control/state/ExpandedNodesState'; + +describe('ExpandedNodesState', () => { + describe('sections', () => { + it('sections are expanded by default', () => { + const state = new ExpandedNodesState(); + expect(state.isSectionCollapsed('changes')).toBe(false); + }); + + it('toggles a section collapsed and back', () => { + const state = new ExpandedNodesState(); + state.toggleSection('conflicts'); + expect(state.isSectionCollapsed('conflicts')).toBe(true); + + state.toggleSection('conflicts'); + expect(state.isSectionCollapsed('conflicts')).toBe(false); + }); + + it('tracks sections independently', () => { + const state = new ExpandedNodesState(); + state.toggleSection('changes'); + state.toggleSection('conflicts'); + + expect(state.isSectionCollapsed('changes')).toBe(true); + expect(state.isSectionCollapsed('conflicts')).toBe(true); + expect(state.isSectionCollapsed('synced')).toBe(false); + }); + }); + + describe('folders', () => { + it('folders are expanded by default', () => { + const state = new ExpandedNodesState(); + expect(state.isFolderCollapsed('blog')).toBe(false); + }); + + it('toggles a folder collapsed and exposes the collapsed set', () => { + const state = new ExpandedNodesState(); + state.toggleFolder('blog/posts'); + state.toggleFolder('notes'); + + expect(state.isFolderCollapsed('blog/posts')).toBe(true); + expect(state.isFolderCollapsed('notes')).toBe(true); + expect(state.getCollapsedFolders()).toEqual(new Set(['blog/posts', 'notes'])); + }); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/state/FilterState.test.ts b/tests/logic/source-control/state/FilterState.test.ts new file mode 100644 index 0000000..a2b19f2 --- /dev/null +++ b/tests/logic/source-control/state/FilterState.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest'; +import { FilterState } from '../../../../src/logic/source-control/state/FilterState'; + +describe('FilterState', () => { + it('defaults to "all"', () => { + expect(new FilterState().get()).toBe('all'); + }); + + it('holds the last set filter', () => { + const state = new FilterState(); + state.set('conflicts'); + expect(state.get()).toBe('conflicts'); + state.set('ready-to-push'); + expect(state.get()).toBe('ready-to-push'); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/OperationState.test.ts b/tests/logic/source-control/state/OperationState.test.ts similarity index 70% rename from tests/logic/source-control/OperationState.test.ts rename to tests/logic/source-control/state/OperationState.test.ts index f34aa92..a06bb1c 100644 --- a/tests/logic/source-control/OperationState.test.ts +++ b/tests/logic/source-control/state/OperationState.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { OperationState } from '../../../src/logic/source-control/OperationState'; -import { toChangeId } from '../../../src/logic/source-control/types'; +import { OperationState } from '../../../../src/logic/source-control/state/OperationState'; +import { toChangeId } from '../../../../src/logic/source-control/types'; describe('OperationState', () => { it('defaults to idle for an untracked change', () => { @@ -23,6 +23,28 @@ describe('OperationState', () => { expect(state.get(toChangeId('change-a'))).toBe('failed'); }); + it('marks a change as conflict, distinct from failed (needs-resolution, not an error)', () => { + const state = new OperationState(); + + state.start(toChangeId('change-a')); + state.conflict(toChangeId('change-a')); + + expect(state.get(toChangeId('change-a'))).toBe('conflict'); + }); + + it('treats conflict and failed as independent lifecycles', () => { + const state = new OperationState(); + + state.start(toChangeId('conflicted')); + state.conflict(toChangeId('conflicted')); + + state.start(toChangeId('errored')); + state.fail(toChangeId('errored')); + + expect(state.get(toChangeId('conflicted'))).toBe('conflict'); + expect(state.get(toChangeId('errored'))).toBe('failed'); + }); + it('tracks multiple changes independently', () => { const state = new OperationState(); diff --git a/tests/logic/source-control/state/SelectedChangeState.test.ts b/tests/logic/source-control/state/SelectedChangeState.test.ts new file mode 100644 index 0000000..c99eb1f --- /dev/null +++ b/tests/logic/source-control/state/SelectedChangeState.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest'; +import { SelectedChangeState } from '../../../../src/logic/source-control/state/SelectedChangeState'; +import { toChangeId } from '../../../../src/logic/source-control/types'; + +describe('SelectedChangeState', () => { + it('defaults to null', () => { + expect(new SelectedChangeState().get()).toBeNull(); + }); + + it('holds the selected change id and clears back to null', () => { + const state = new SelectedChangeState(); + state.set(toChangeId('c-1')); + expect(state.get()).toBe(toChangeId('c-1')); + + state.clear(); + expect(state.get()).toBeNull(); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/PushSelectionStore.test.ts b/tests/logic/source-control/state/SelectionState.test.ts similarity index 81% rename from tests/logic/source-control/PushSelectionStore.test.ts rename to tests/logic/source-control/state/SelectionState.test.ts index 9b382da..1069c57 100644 --- a/tests/logic/source-control/PushSelectionStore.test.ts +++ b/tests/logic/source-control/state/SelectionState.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest'; -import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; -import { toChangeId } from '../../../src/logic/source-control/types'; +import { SelectionState } from '../../../../src/logic/source-control/state/SelectionState'; +import { toChangeId } from '../../../../src/logic/source-control/types'; -describe('PushSelectionStore', () => { +describe('SelectionState', () => { it('includes a change for push', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-a')); @@ -13,7 +13,7 @@ describe('PushSelectionStore', () => { }); it('excludes a change from push', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-a')); store.excludeFromPush(toChangeId('change-a')); @@ -23,7 +23,7 @@ describe('PushSelectionStore', () => { }); it('tracks multiple changes independently', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-a')); store.includeForPush(toChangeId('change-b')); @@ -35,7 +35,7 @@ describe('PushSelectionStore', () => { }); it('keeps selection across a refresh when the change is still present', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-a')); store.refresh([toChangeId('change-a'), toChangeId('change-b')]); @@ -44,7 +44,7 @@ describe('PushSelectionStore', () => { }); it('clears selection for a change removed by refresh', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-a')); store.includeForPush(toChangeId('change-b')); @@ -56,7 +56,7 @@ describe('PushSelectionStore', () => { }); it('keeps selection when path changes but change id stays', () => { - const store = new PushSelectionStore(); + const store = new SelectionState(); store.includeForPush(toChangeId('change-1')); // old.md renamed to new.md, but the change id is stable diff --git a/tests/ui/source-control/SourceControlItemView.test.ts b/tests/ui/source-control/SourceControlItemView.test.ts index 1701a55..8541271 100644 --- a/tests/ui/source-control/SourceControlItemView.test.ts +++ b/tests/ui/source-control/SourceControlItemView.test.ts @@ -2,9 +2,10 @@ import { beforeAll, describe, expect, it, vi } from 'vitest'; import { WorkspaceLeaf } from 'obsidian'; import { SourceControlItemView, SOURCE_CONTROL_VIEW_TYPE } from '../../../src/ui/source-control/SourceControlItemView'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; -import { OperationState } from '../../../src/logic/source-control/OperationState'; -import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; +import { SourceControlState } from '../../../src/logic/source-control/state/SourceControlState'; +import { OperationState } from '../../../src/logic/source-control/state/OperationState'; +import { SelectionState } from '../../../src/logic/source-control/state/SelectionState'; import { toChangeId } from '../../../src/logic/source-control/types'; import { SyncStatusService } from '../../../src/logic/sync-status-service'; import type GitLabFilesPush from '../../../src/main'; @@ -15,17 +16,17 @@ beforeAll(() => { setupObsidianDOM(); }); function buildPlugin() { const repository = new ChangeRepository(); repository.replace([{ id: toChangeId('a.md'), path: 'a.md', kind: 'local-only' }]); - const selection = new PushSelectionStore(); + const selection = new SelectionState(); const operations = new OperationState(); - const viewModel = new SourceControlViewModel(repository, selection, operations); + const state = new SourceControlState(repository, selection, operations); + const viewModel = new SourceControlViewModel(state); const push = vi.fn().mockResolvedValue(undefined); const loadDiffContent = vi.fn().mockResolvedValue(null); const status = new SyncStatusService(); const plugin = { changeRepository: repository, - pushSelectionStore: selection, - operationState: operations, + sourceControlState: state, sourceControlViewModel: viewModel, sourceControlActions: { push, loadDiffContent }, sync: { status }, diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 5110067..304b4a9 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -1,9 +1,10 @@ import { describe, expect, it, vi, beforeAll, beforeEach } from 'vitest'; import { SourceControlView, type SourceControlViewCallbacks } from '../../../src/ui/source-control/SourceControlView'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; -import { OperationState } from '../../../src/logic/source-control/OperationState'; -import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; +import { SourceControlState } from '../../../src/logic/source-control/state/SourceControlState'; +import { OperationState } from '../../../src/logic/source-control/state/OperationState'; +import { SelectionState } from '../../../src/logic/source-control/state/SelectionState'; import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; import { setupObsidianDOM, createContainer } from '../setup-dom'; @@ -12,12 +13,13 @@ beforeAll(() => { setupObsidianDOM(); }); function buildView(changes: SyncChange[], callbacks: Partial = {}) { const repository = new ChangeRepository(); repository.replace(changes); - const selection = new PushSelectionStore(); + const selection = new SelectionState(); const operations = new OperationState(); - const viewModel = new SourceControlViewModel(repository, selection, operations); + const state = new SourceControlState(repository, selection, operations); + const viewModel = new SourceControlViewModel(state); const onPush = callbacks.onPush ?? vi.fn(); - const view = new SourceControlView(viewModel, selection, { onPush, ...callbacks }); - return { view, selection, operations, onPush }; + const view = new SourceControlView(viewModel, { onPush, ...callbacks }); + return { view, selection, operations, viewModel, onPush }; } describe('SourceControlView', () => { @@ -42,7 +44,7 @@ describe('SourceControlView', () => { }); it('shows a flat tree (no sections) once a specific filter is selected', () => { - const { view } = buildView([ + const { view, viewModel } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'conflict' }, { id: toChangeId('c-2'), path: 'b.md', kind: 'local-only' }, ]); @@ -52,7 +54,7 @@ describe('SourceControlView', () => { expect(container.querySelectorAll('.scv-section')).toHaveLength(0); expect(container.querySelectorAll('.scv-change-item')).toHaveLength(1); - expect(view.getFilter()).toBe('conflicts'); + expect(viewModel.getFilter()).toBe('conflicts'); }); it('shows the empty state when the active filter has no items', () => { @@ -173,21 +175,21 @@ describe('SourceControlView', () => { describe('rename stability', () => { it('keeps the selected ChangeId set after a rename changes the path', () => { - const { view } = buildView([ + const { view, viewModel } = buildView([ { id: toChangeId('c-1'), path: 'old.md', kind: 'local-modified' }, ]); view.render(container); (container.querySelector('.scv-change-item') as HTMLElement).click(); - expect(view.getSelectedChangeId()).toBe(toChangeId('c-1')); + expect(viewModel.getSelectedChangeId()).toBe(toChangeId('c-1')); // Simulate a rename being reflected in a fresh ViewModel snapshot for the same ChangeId. - const { view: renamedView } = buildView([ + const { view: renamedView, viewModel: renamedViewModel } = buildView([ { id: toChangeId('c-1'), path: 'new.md', previousPath: 'old.md', kind: 'moved' }, ]); renamedView.render(container); (container.querySelector('.scv-change-item') as HTMLElement).click(); - expect(renamedView.getSelectedChangeId()).toBe(toChangeId('c-1')); + expect(renamedViewModel.getSelectedChangeId()).toBe(toChangeId('c-1')); expect(container.querySelector('.scv-change-rename-from')?.textContent).toBe('old.md'); }); });