refactor(source-control): unify UI state into SourceControlState

progress.md

@@ -5,7 +5,7 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
 ## Current State
 
 **Last Updated:** 2026-08-22
-**Active Feature:** feat-026 / issue #105 — Source Control UI refactor on `feat/source-control-integration` (plan: `docs/source-control-refactor/roadmap.md`). **PR130 done (uncommitted)**: execution-result + three-state (success/conflict/failed) semantics in the Source Control logic layer — `OperationStatus` gained `'conflict'`; new `ExecutionResult` thin projection (not a new sync model); `SourceControlActionService` actions return `ExecutionResult` and `push` maps `PushResults.conflictedPaths` → `'conflict'`; `SourceControlViewModel` exposes `lastOperationResult`. No UI, no sync-domain changes. Gate green: `npx eslint .` 0 errors; `npm run build` PASS (Obsidian 1.11 compat); `npx vitest run` 66 files / 700 tests. Separately, the integration WIP (view-wiring + legacy deletion) is green in worktree `bridge-cse_01S28SbagdQFNToUPv8peeau` (531 tests) but awaits manual Obsidian verification + commit; disjoint from PR130.
+**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

session-handoff.md

@@ -1,81 +1,68 @@
 # Session Handoff
 
 **Date:** 2026-08-22
-**Branch:** `feat/source-control-integration` (worktree `/tmp/kilo/pr130`,
-based on `origin/claude/source-control-foundation` @ `ec44025`)
-**Active Feature:** feat-026 / issue #105 — Source Control UI refactor, PR130
+**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 — PR130 (A1–A4)
+## Completed this session — PR131 (Phase 2: unify state model)
 
-Landed the execution-result + three-state (success/conflict/failed) semantics
-in the Source Control logic layer. **No UI work** (per the user's directive —
-UI already exists; it lacked this three-state model). **No sync-domain
-changes** — reused existing executor conflict semantics.
+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.
 
-- **A1 `OperationState.ts`**: `OperationStatus` now includes `'conflict'`; added
-  `conflict(changeId)`. Conflict is a distinct lifecycle from `'failed'`
-  (needs-resolution, resolvable, not an error).
-- **A2 `ExecutionResult.ts` (new)**: thin UI projection
-  `{ completed: ChangeId[]; conflicts: ChangeId[]; failed: ChangeId[] }` +
-  `emptyExecutionResult()`. Derived in `SourceControlActionService` from
-  `PushResults`/`SyncResult`/`RemoteDeleteResult` — **not** a new sync model.
-- **A2 `SourceControlActionService.ts`**: `push`/`pull`/`deleteRemote`/
-  `deleteLocal`/`resolveConflict` now return `Promise<ExecutionResult>`.
-  `push` reads `PushResults.conflictedPaths` → marks those `'conflict'`
-  (conflict takes precedence over error). New `classify()` helper maps
-  per-path outcome → `OperationStatus` + `ExecutionResult` in one pass.
-- **A3 `SourceControlViewModel.ts`**: `SourceControlViewState` gains
-  `lastOperationResult: ExecutionResult | null`; `setOperationResult` /
-  `clearOperationResult` so the UI can render a batch summary. The `'conflicts'`
-  filter stays change-model-driven (`change.kind === 'conflict'`); operation
-  `'conflict'` status is the per-change push outcome — complementary, not
-  overlapping.
-- **A4 tests**: new `ExecutionResult.test.ts`; extended `OperationState.test.ts`
-  (conflict lifecycle), `SourceControlActionService.test.ts` (conflict mapping,
-  mixed 7/3/1 batch, conflict-precedence, throw → all-failed, pull/deleteRemote/
-  resolveConflict return values), `SourceControlViewModel.test.ts` (operation-
-  result exposure).
-
-### Known asymmetry (by design, not a gap)
-
-`PushResults` exposes per-path `conflictedPaths`; `SyncResult` (pull) carries
-only a `conflicts` count. So push maps conflicts to `ChangeId`s; pull conflicts
-surface through change-model reclassification (`kind: 'conflict'`) on the next
-repository refresh, not through this projection. Documented in code + tests.
+- 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.
 
 ## Verification evidence
 
 ```text
 npx eslint .   -> 0 errors
 npm run build  -> PASS (tsc + Obsidian 1.11.0 compat + esbuild)
-npx vitest run -> 66 files / 700 tests PASS
-  (source-control logic: 7 files / 60 tests PASS)
+npx vitest run -> 59 files / 553 tests
 ```
 
 ## Exact next step
 
-1. Review/commit PR130 on `feat/source-control-integration` (uncommitted now;
-   user has not requested a commit). PR targets `claude/source-control-foundation`.
-2. **Separately land the integration WIP** in worktree
-   `bridge-cse_01S28SbagdQFNToUPv8peeau` (Phase A view-wiring + Phase E legacy
-   deletion — already green there: eslint 0, build PASS, 531 tests). It touches
-   `main.ts`/`SourceControlItemView.ts`/deletions — **disjoint** from PR130's
-   `src/logic/source-control/*` files, so the two merge independently. That WIP
-   still needs **manual Obsidian verification** before commit (DoD for UI
-   surfaces).
-3. **Phase B (PR131) — batch conflict workflow**: `ConflictClassifier`
-   (content / delete-modify / rename / binary) + conflict section in ViewModel
-   (`conflicts: ChangeViewModel[]`) driven by the change model. Reuses
-   `src/logic/sync/ConflictResolver.ts` semantics — do not recreate.
-
-After B: Phase C (PR132, conflict resolution UX — `ConflictPanel`,
-`DiffLayoutSelector`, Accept Local/Remote/Manual Merge, Resolve All), then
-Phase D (PR133, context menu + command palette → `SourceControlActionService`),
-then Phase E (PR134, remove any remaining legacy paths).
+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).
 
 ## Local working tree
 
-- Worktree `/tmp/kilo/pr130` is session-local; the branch ref
-  `feat/source-control-integration` persists. `node_modules` was installed via
-  `npm ci` for the gate.
+- `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).

src/logic/source-control/SourceControlActionService.ts

@@ -1,7 +1,7 @@
 import type { SyncWorkspace } from '../sync/SyncWorkspace';
 import type { ChangeRepository } from './ChangeRepository';
 import { emptyExecutionResult, type ExecutionResult } from './ExecutionResult';
-import type { OperationState } from './OperationState';
+import type { OperationState } from './state/OperationState';
 import type { SourceControlItem } from './SourceControlViewModel';
 import type { ChangeId, SyncChange } from './types';
 

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';

src/logic/source-control/SourceControlViewModel.ts

@@ -1,8 +1,8 @@
-import type { ChangeRepository } from './ChangeRepository';
 import type { ExecutionResult } from './ExecutionResult';
-import type { OperationState, OperationStatus } from './OperationState';
-import type { PushSelectionStore } from './PushSelectionStore';
 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. */
@@ -27,35 +27,60 @@ export interface SourceControlViewState {
 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 {
     private lastOperationResult: ExecutionResult | null = null;
 
-    constructor(
-        private readonly changes: ChangeRepository,
-        private readonly selection: PushSelectionStore,
-        private readonly operations: OperationState,
-    ) {}
+    constructor(private readonly state: SourceControlState) {}
 
-    getState(filter: SourceControlFilter = 'all'): SourceControlViewState {
-        const all = this.changes.getAll();
+    // --- 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, lastOperationResult: this.lastOperationResult };
     }
 
-    /**
-     * Stores the most recent batch outcome so the UI can render a summary
-     * ("7 completed / 3 conflicts / 1 failed"). Set by the action flow after
-     * `SourceControlActionService` returns; cleared by `clearOperationResult`
-     * (e.g. when the user dismisses the summary or starts a new action).
-     */
+    // --- 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<string> { 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;
     }
@@ -70,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<SourceControlFilter, number> {
         const counts = {} as Record<SourceControlFilter, number>;
         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;
     }
-}
+}

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<SourceControlFilter, 'all'>;
+
+/**
+ * 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<SectionFilter>();
+    private readonly collapsedFolders = new Set<string>();
+
+    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<string> {
+        return this.collapsedFolders;
+    }
+}

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;
+    }
+}

src/logic/source-control/OperationState.ts → src/logic/source-control/state/OperationState.ts

@@ -1,4 +1,4 @@
-import type { ChangeId } from './types';
+import type { ChangeId } from '../types';
 
 export type OperationStatus = 'idle' | 'running' | 'success' | 'failed' | 'conflict';
 
@@ -46,4 +46,4 @@ export class OperationState {
     clear(): void {
         this.status.clear();
     }
-}
+}