From 0bcc8007a4b2f77850a5d3341ceeee216d26c51e Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:26:31 +0000 Subject: [PATCH 01/21] fix(source-control): correct status grouping and filter semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a presentation layer so the UI no longer reads Git status directly: SourceControlSummary is the single source for every count (all/changes/remote-changes/ready-to-push/conflicts/synced) and the ViewModel only forwards its counts. Filter semantics: - all = actionable (kind !== 'synced') so All no longer duplicates the Synced bucket. - changes = local-side only (local-only/local-modified/moved). - ready-to-push excludes synced. Rendering: - Every filter (including All) renders one flat tree + an active-filter header; the old section breakdown under All is removed, so SYNCED never leaks into All. - Synced hidden by default behind a Show synced toggle; the synced chip only surfaces when opted in, and hiding it while on synced falls back to All. Tree grouping: - ChangeTreeBuilder gains TreeDisplayOptions { maxDepth, collapseSingleChild }; the view enables collapseSingleChild so single-child folder chains collapse to one path node instead of an Explorer-like deep nest. ChangeSection.ts deleted (no longer used). i18n keys + styles added. Verification: npx eslint . 0 errors; npm run build PASS (tsc + Obsidian 1.11.0 compat + esbuild); npx vitest run 56 files / 547 tests. Manual Obsidian verification in a real vault remains. Scope excludes diff viewer, conflict resolution UI, push/pull pipeline, and view migration per the fix plan's后续順序. --- src/i18n/locales/en.ts | 2 + src/i18n/locales/zh-cn.ts | 2 + src/i18n/locales/zh-tw.ts | 2 + src/logic/source-control/ChangeTreeBuilder.ts | 121 +++++++++++++++++- .../source-control/SourceControlFilter.ts | 31 +++-- .../source-control/SourceControlSummary.ts | 89 +++++++++++++ .../source-control/SourceControlViewModel.ts | 39 +++--- src/ui/source-control/ChangeSection.ts | 41 ------ src/ui/source-control/ChangeTree.ts | 16 ++- src/ui/source-control/FilterMenu.ts | 46 +++++-- src/ui/source-control/SourceControlView.ts | 101 +++++++-------- styles.css | 45 +++++++ .../source-control/ChangeTreeBuilder.test.ts | 46 +++++++ .../SourceControlSummary.test.ts | 111 ++++++++++++++++ .../SourceControlViewModel.test.ts | 21 ++- tests/ui/source-control/FilterMenu.test.ts | 49 ++++--- .../source-control/SourceControlView.test.ts | 73 ++++++++++- 17 files changed, 677 insertions(+), 158 deletions(-) create mode 100644 src/logic/source-control/SourceControlSummary.ts delete mode 100644 src/ui/source-control/ChangeSection.ts create mode 100644 tests/logic/source-control/SourceControlSummary.test.ts diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index e0417b9..efde946 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -257,6 +257,8 @@ const en = { 'sourceControl.filter.remoteChanges': 'Remote Changes', 'sourceControl.filter.conflicts': 'Conflicts', 'sourceControl.filter.synced': 'Synced', + 'sourceControl.filter.showSynced': 'Show synced', + 'sourceControl.section.all': 'ALL', 'sourceControl.section.readyToPush': 'READY TO PUSH', 'sourceControl.section.changes': 'CHANGES', 'sourceControl.section.remoteChanges': 'REMOTE CHANGES', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index c6d3c9f..145d004 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -259,6 +259,8 @@ const zhCn: Partial> = { 'sourceControl.filter.remoteChanges': '远程更改', 'sourceControl.filter.conflicts': '冲突', 'sourceControl.filter.synced': '已同步', + 'sourceControl.filter.showSynced': '显示已同步', + 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', 'sourceControl.section.changes': '更改', 'sourceControl.section.remoteChanges': '远程更改', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 6d63b0c..8fe3130 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -259,6 +259,8 @@ const zhTw: Partial> = { 'sourceControl.filter.remoteChanges': '遠端變更', 'sourceControl.filter.conflicts': '衝突', 'sourceControl.filter.synced': '已同步', + 'sourceControl.filter.showSynced': '顯示已同步', + 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', 'sourceControl.section.changes': '變更', 'sourceControl.section.remoteChanges': '遠端變更', diff --git a/src/logic/source-control/ChangeTreeBuilder.ts b/src/logic/source-control/ChangeTreeBuilder.ts index b8cdc07..1a9152b 100644 --- a/src/logic/source-control/ChangeTreeBuilder.ts +++ b/src/logic/source-control/ChangeTreeBuilder.ts @@ -18,6 +18,31 @@ export interface ChangeTreeFolderNode { export type ChangeTreeNode = ChangeTreeFileNode | ChangeTreeFolderNode; +/** + * Presentation-only controls for tree rendering, so the Source Control tree + * stays a compact change view rather than reproducing the full file Explorer. + * + * - `maxDepth`: the maximum number of folder nesting levels rendered as + * separate, collapsible nodes. Deeper folders are folded into a single + * flattened path segment (e.g. `02_Areas/blog/_pixnet/zh-tw/tech`) instead of + * five nested expandable rows. Files always render at their real depth; only + * intermediate folders are flattened. Defaults to unlimited depth (legacy + * behavior) when omitted. + * - `collapseSingleChild`: when true, a folder that contains exactly one + * child folder (no files) is merged with that child into one combined folder + * node, reducing pointless single-step nesting like `tech › tech › tech`. + * Defaults to false to preserve the existing rendering when omitted. + */ +export interface TreeDisplayOptions { + maxDepth?: number; + collapseSingleChild?: boolean; +} + +const DEFAULT_OPTIONS: Required> = { + maxDepth: Number.POSITIVE_INFINITY, + collapseSingleChild: false, +}; + /** * Turns a flat `SyncChange[]` into a folder/file tree for rendering. * A renamed/moved file is placed at its *current* path — `previousPath` @@ -25,12 +50,14 @@ export type ChangeTreeNode = ChangeTreeFileNode | ChangeTreeFolderNode; * not create a second tree entry. */ export class ChangeTreeBuilder { - build(changes: readonly SyncChange[]): ChangeTreeNode[] { + build(changes: readonly SyncChange[], options: TreeDisplayOptions = {}): ChangeTreeNode[] { + const opts = { ...DEFAULT_OPTIONS, ...options }; const root: ChangeTreeFolderNode = { type: 'folder', name: '', path: '', children: [] }; for (const change of changes) { this.insert(root, change); } - return root.children; + const nodes = this.collapseAndLimit(root.children, opts, 0); + return nodes; } private insert(root: ChangeTreeFolderNode, change: SyncChange): void { @@ -65,4 +92,92 @@ export class ChangeTreeBuilder { parent.children.push(created); return created; } -} + + /** + * Applies `collapseSingleChild` and `maxDepth` to a depth's children. + * + * `collapseSingleChild` merges a folder whose only child is a single folder + * (no file siblings) into one combined node, joining names/paths with `/`. + * The merge repeats along a run of single-child folders so + * `a/b/c/d.md` collapses to `a/b/c` (one node) when every level has only one + * child folder. Files break the run, so `a/x.md` + `a/b/c/y.md` keeps `a` + * separate from the collapsed `b/c`. + * + * `maxDepth` flattens any folder nesting deeper than the limit into a + * single path-labelled node whose children are the files/subfolders at that + * point (no further nesting is rendered). + */ + private collapseAndLimit( + nodes: ChangeTreeNode[], + opts: Required>, + depth: number, + ): ChangeTreeNode[] { + const result: ChangeTreeNode[] = []; + for (const node of nodes) { + if (node.type === 'file') { + result.push(node); + continue; + } + + const collapsed = this.collapseSingleChildRun(node, opts); + const atDepthLimit = depth >= opts.maxDepth; + + if (atDepthLimit) { + // Flatten deeper structure into one folder node holding all descendants' files. + result.push(this.flattenFolder(collapsed)); + continue; + } + + collapsed.children = this.collapseAndLimit(collapsed.children, opts, depth + 1); + result.push(collapsed); + } + return result; + } + + private collapseSingleChildRun( + folder: ChangeTreeFolderNode, + opts: Required>, + ): ChangeTreeFolderNode { + if (!opts.collapseSingleChild) return folder; + + let current = folder; + // Walk down while the current folder has exactly one child and it is a folder. + let onlyChild = current.children[0]; + while (current.children.length === 1 && onlyChild && onlyChild.type === 'folder') { + current = this.mergeFolders(current, onlyChild); + onlyChild = current.children[0]; + } + return current; + } + + private mergeFolders(parent: ChangeTreeFolderNode, child: ChangeTreeFolderNode): ChangeTreeFolderNode { + return { + type: 'folder', + name: `${parent.name}/${child.name}`, + path: child.path, + children: child.children, + }; + } + + private flattenFolder(folder: ChangeTreeFolderNode): ChangeTreeFolderNode { + const files = this.collectFiles(folder); + return { + type: 'folder', + name: folder.name, + path: folder.path, + children: files, + }; + } + + private collectFiles(folder: ChangeTreeFolderNode): ChangeTreeNode[] { + const files: ChangeTreeNode[] = []; + for (const child of folder.children) { + if (child.type === 'file') { + files.push(child); + } else { + files.push(...this.collectFiles(child)); + } + } + return files; + } +} \ No newline at end of file diff --git a/src/logic/source-control/SourceControlFilter.ts b/src/logic/source-control/SourceControlFilter.ts index 3229ec2..db67d93 100644 --- a/src/logic/source-control/SourceControlFilter.ts +++ b/src/logic/source-control/SourceControlFilter.ts @@ -1,5 +1,5 @@ import type { PushSelectionStore } from './PushSelectionStore'; -import type { SyncChange } from './types'; +import type { SyncChange, SyncChangeKind } from './types'; export type SourceControlFilter = | 'all' @@ -9,18 +9,31 @@ export type SourceControlFilter = | 'conflicts' | 'synced'; +const LOCAL_KINDS: ReadonlySet = new Set(['local-only', 'local-modified', 'moved']); +const REMOTE_KINDS: ReadonlySet = new Set(['remote-only', 'remote-modified']); + /** - * Whether `change` belongs under `filter`. `ready-to-push` is defined purely - * by `PushSelectionStore` membership — it's a user selection, not a fact - * derivable from the change's kind alone. + * Whether `change` belongs under `filter`. Filters are user-facing *action* + * semantics, not a raw mirror of Git status: + * + * - `all` — *actionable* changes only (everything except synced). A synced + * file needs no action, so it never appears under All. This keeps All from + * duplicating the Synced bucket. + * - `changes` — local-side changes only (local-only, local-modified, moved). + * Remote-only/conflict rows belong to their own filters, not Changes. + * - `ready-to-push` — defined purely by {@link PushSelectionStore} membership; + * it's a user selection, not a fact derivable from the change's kind alone. + * - `remote-changes` — remote-only / remote-modified. + * - `conflicts` — conflict. + * - `synced` — synced (only surfaced when the user opts in via "Show synced"). */ export function matchesFilter(change: SyncChange, filter: SourceControlFilter, selection: PushSelectionStore): boolean { switch (filter) { - case 'all': return true; - case 'changes': return change.kind !== 'synced'; - case 'ready-to-push': return selection.isIncluded(change.id); - case 'remote-changes': return change.kind === 'remote-only' || change.kind === 'remote-modified'; + case 'all': return change.kind !== 'synced'; + case 'changes': return LOCAL_KINDS.has(change.kind); + case 'ready-to-push': return selection.isIncluded(change.id) && change.kind !== 'synced'; + case 'remote-changes': return REMOTE_KINDS.has(change.kind); case 'conflicts': return change.kind === 'conflict'; case 'synced': return change.kind === 'synced'; } -} +} \ No newline at end of file diff --git a/src/logic/source-control/SourceControlSummary.ts b/src/logic/source-control/SourceControlSummary.ts new file mode 100644 index 0000000..ffd3140 --- /dev/null +++ b/src/logic/source-control/SourceControlSummary.ts @@ -0,0 +1,89 @@ +import type { PushSelectionStore } from './PushSelectionStore'; +import type { SourceControlFilter } from './SourceControlFilter'; +import type { ChangeId, SyncChange, SyncChangeKind } from './types'; + +/** + * Per-filter counts, keyed by the same {@link SourceControlFilter} values the + * filter menu renders. This is the single source of truth for every count the + * UI shows — the ViewModel passes it through unchanged and the view layer + * never recomputes a count itself. + * + * `synced` is the *rendered* count: it is `0` when synced changes are hidden + * (showSynced = false) so the UI can't display a synced count the user has + * asked to suppress. The raw synced bucket is still available on + * {@link SourceControlSummary.synced} for callers that need the actual figure. + */ +export type SourceControlCounts = Record; + +/** + * The complete presentation projection of a pending-change set: the raw + * buckets the UI renders from, plus the single {@link counts} object every + * count label reads from. + * + * Buckets are disjoint and exhaustive over {@link SyncChangeKind}: + * - {@link localChanges}: local-only, local-modified, moved + * - {@link remoteChanges}: remote-only, remote-modified + * - {@link conflicts}: conflict + * - {@link synced}: synced + * - {@link all}: the union of the three actionable buckets (everything except + * synced) — "All" means *actionable*, not "every row", so a synced file never + * appears under All. + * - {@link readyToPush}: the subset of actionable changes the user has selected + * for push (membership in {@link PushSelectionStore}); it overlaps the other + * actionable buckets by design, since "ready to push" is a selection, not a + * change kind. + */ +export interface SourceControlSummary { + all: SyncChange[]; + localChanges: SyncChange[]; + remoteChanges: SyncChange[]; + readyToPush: SyncChange[]; + conflicts: SyncChange[]; + synced: SyncChange[]; + counts: SourceControlCounts; +} + +const LOCAL_KINDS: ReadonlySet = new Set(['local-only', 'local-modified', 'moved']); +const REMOTE_KINDS: ReadonlySet = new Set(['remote-only', 'remote-modified']); + +function isLocal(change: SyncChange): boolean { return LOCAL_KINDS.has(change.kind); } +function isRemote(change: SyncChange): boolean { return REMOTE_KINDS.has(change.kind); } +function isConflict(change: SyncChange): boolean { return change.kind === 'conflict'; } +function isSynced(change: SyncChange): boolean { return change.kind === 'synced'; } +function isActionable(change: SyncChange): boolean { return change.kind !== 'synced'; } + +/** + * Builds the single presentation projection the Source Control UI consumes. + * Pure: given the same `changes` + `selection` + `showSynced` it always + * produces the same {@link SourceControlSummary}, with no side effects on the + * store. Callers (the ViewModel) hold no count logic of their own. + * + * @param showSynced when false, {@link SourceControlCounts.synced} is reported + * as `0` (the UI hides the synced bucket) while {@link SourceControlSummary.synced} + * still holds the raw synced changes. + */ +export function buildSummary( + changes: readonly SyncChange[], + selection: PushSelectionStore, + showSynced: boolean, +): SourceControlSummary { + const localChanges = changes.filter(isLocal); + const remoteChanges = changes.filter(isRemote); + const conflicts = changes.filter(isConflict); + const synced = changes.filter(isSynced); + const all = changes.filter(isActionable); + + const selectedIds = new Set(selection.getSelectedChangeIds()); + const readyToPush = all.filter(change => selectedIds.has(change.id)); + + const counts: SourceControlCounts = { + all: all.length, + changes: localChanges.length, + 'ready-to-push': readyToPush.length, + 'remote-changes': remoteChanges.length, + conflicts: conflicts.length, + synced: showSynced ? synced.length : 0, + }; + + return { all, localChanges, remoteChanges, readyToPush, conflicts, synced, counts }; +} \ No newline at end of file diff --git a/src/logic/source-control/SourceControlViewModel.ts b/src/logic/source-control/SourceControlViewModel.ts index 317a285..c880a70 100644 --- a/src/logic/source-control/SourceControlViewModel.ts +++ b/src/logic/source-control/SourceControlViewModel.ts @@ -1,4 +1,5 @@ import type { ChangeRepository } from './ChangeRepository'; +import { buildSummary, type SourceControlCounts } from './SourceControlSummary'; import type { OperationState, OperationStatus } from './OperationState'; import type { PushSelectionStore } from './PushSelectionStore'; import { matchesFilter, type SourceControlFilter } from './SourceControlFilter'; @@ -18,16 +19,24 @@ export interface SourceControlItem { export interface SourceControlViewState { filter: SourceControlFilter; items: SourceControlItem[]; - counts: Record; + /** Single-source counts from {@link buildSummary} — the view never recomputes these. */ + counts: SourceControlCounts; } -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. + * + * Every count the UI shows comes from one place: {@link buildSummary}. The + * ViewModel only projects items for the active filter and forwards the + * summary's counts unchanged, so the filter menu, section headers, and tree + * can never drift apart. + * + * `showSynced` governs whether the synced bucket is surfaced: when false the + * synced count is reported as `0` and the `synced` filter yields no items, + * matching the "Show synced" toggle (default off). */ export class SourceControlViewModel { constructor( @@ -36,13 +45,21 @@ export class SourceControlViewModel { private readonly operations: OperationState, ) {} - getState(filter: SourceControlFilter = 'all'): SourceControlViewState { + getState(filter: SourceControlFilter = 'all', showSynced = false): SourceControlViewState { const all = this.changes.getAll(); + const summary = buildSummary(all, this.selection, showSynced); const items = all .filter(change => matchesFilter(change, filter, this.selection)) + .filter(() => this.isRenderable(filter, showSynced)) .map(change => this.toItem(change)); - const counts = this.countByFilter(all); - return { filter, items, counts }; + return { filter, items, counts: summary.counts }; + } + + private isRenderable(filter: SourceControlFilter, showSynced: boolean): boolean { + // Synced rows only render under the `synced` filter, and only when the + // user has opted in via "Show synced". `all`/`changes`/etc. already + // exclude synced via matchesFilter, so this only gates the synced view. + return !(filter === 'synced' && !showSynced); } private toItem(change: SyncChange): SourceControlItem { @@ -55,12 +72,4 @@ export class SourceControlViewModel { operationStatus: this.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; - } - return counts; - } -} +} \ No newline at end of file diff --git a/src/ui/source-control/ChangeSection.ts b/src/ui/source-control/ChangeSection.ts deleted file mode 100644 index 8d52dd8..0000000 --- a/src/ui/source-control/ChangeSection.ts +++ /dev/null @@ -1,41 +0,0 @@ -import type { SourceControlFilter } from '../../logic/source-control/SourceControlFilter'; -import type { SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; -import { renderChangeTree, type ChangeTreeCallbacks } from './ChangeTree'; - -export interface ChangeSectionProps { - /** One of the section filters (not 'all' — the "All" filter renders every section). */ - id: Exclude; - title: string; - items: readonly SourceControlItem[]; - collapsed: boolean; - collapsedFolders: ReadonlySet; -} - -export interface ChangeSectionCallbacks extends ChangeTreeCallbacks { - onToggleSection: (id: Exclude) => void; -} - -/** Renders one of the five Source Control sections: a collapsible header + its change tree. */ -export function renderChangeSection( - container: HTMLElement, - props: ChangeSectionProps, - callbacks: ChangeSectionCallbacks, -): HTMLElement { - const sectionEl = container.createDiv({ cls: `scv-section scv-section-${props.id}` }); - const header = sectionEl.createDiv({ cls: 'scv-section-header' }); - - const toggle = header.createEl('button', { cls: 'scv-section-toggle' }); - toggle.setAttr('aria-expanded', String(!props.collapsed)); - toggle.setText(props.collapsed ? '▶' : '▼'); - toggle.addEventListener('click', () => callbacks.onToggleSection(props.id)); - - header.createSpan({ cls: 'scv-section-title', text: props.title }); - header.createSpan({ cls: 'scv-section-count', text: String(props.items.length) }); - - if (!props.collapsed) { - const body = sectionEl.createDiv({ cls: 'scv-section-body' }); - renderChangeTree(body, props.items, props.collapsedFolders, callbacks); - } - - return sectionEl; -} diff --git a/src/ui/source-control/ChangeTree.ts b/src/ui/source-control/ChangeTree.ts index cc4a202..478f38f 100644 --- a/src/ui/source-control/ChangeTree.ts +++ b/src/ui/source-control/ChangeTree.ts @@ -3,6 +3,7 @@ import { type ChangeTreeFileNode, type ChangeTreeFolderNode, type ChangeTreeNode, + type TreeDisplayOptions, } from '../../logic/source-control/ChangeTreeBuilder'; import type { SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; import type { ChangeId } from '../../logic/source-control/types'; @@ -20,16 +21,21 @@ const builder = new ChangeTreeBuilder(); * `SyncChange`, so the builder's output only carries `id`/`path`/`kind`; a * by-id lookup restores `isReadyToPush`/`operationStatus` at render time * instead of duplicating the tree-building logic. + * + * `options` controls presentation-only tree shaping (single-child folder + * collapse, depth limit) so the Source Control tree stays a compact change + * view rather than reproducing the full file Explorer. */ export function renderChangeTree( container: HTMLElement, items: readonly SourceControlItem[], collapsedFolders: ReadonlySet, callbacks: ChangeTreeCallbacks, + options: TreeDisplayOptions = {}, ): void { const byId = new Map(items.map(item => [item.id, item])); - const nodes = builder.build(items); - renderNodes(container, nodes, byId, collapsedFolders, callbacks); + const nodes = builder.build(items, options); + renderNodes(container, nodes, byId, collapsedFolders, callbacks, options); } function renderNodes( @@ -38,9 +44,10 @@ function renderNodes( byId: ReadonlyMap, collapsedFolders: ReadonlySet, callbacks: ChangeTreeCallbacks, + options: TreeDisplayOptions, ): void { for (const node of nodes) { - if (node.type === 'folder') renderFolder(container, node, byId, collapsedFolders, callbacks); + if (node.type === 'folder') renderFolder(container, node, byId, collapsedFolders, callbacks, options); else renderFile(container, node, byId, callbacks); } } @@ -51,6 +58,7 @@ function renderFolder( byId: ReadonlyMap, collapsedFolders: ReadonlySet, callbacks: ChangeTreeCallbacks, + options: TreeDisplayOptions, ): void { const collapsed = collapsedFolders.has(folder.path); const folderEl = container.createDiv({ cls: 'scv-tree-folder' }); @@ -65,7 +73,7 @@ function renderFolder( if (!collapsed) { const childrenEl = folderEl.createDiv({ cls: 'scv-tree-children' }); - renderNodes(childrenEl, folder.children, byId, collapsedFolders, callbacks); + renderNodes(childrenEl, folder.children, byId, collapsedFolders, callbacks, options); } } diff --git a/src/ui/source-control/FilterMenu.ts b/src/ui/source-control/FilterMenu.ts index 3c193b3..f7bd306 100644 --- a/src/ui/source-control/FilterMenu.ts +++ b/src/ui/source-control/FilterMenu.ts @@ -1,8 +1,12 @@ import { t, type TranslationKey } from '../../i18n'; import type { SourceControlFilter } from '../../logic/source-control/SourceControlFilter'; -/** Order and labels match the Phase 3 spec's Filter section exactly. */ -const FILTER_ORDER: SourceControlFilter[] = ['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts', 'synced']; +/** + * Action filter chips, in spec order. `synced` is deliberately NOT a permanent + * chip — it surfaces only when the user opts in via the "Show synced" toggle, + * so a quiet workspace isn't dominated by a large synced count. + */ +const ACTION_FILTERS: SourceControlFilter[] = ['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts']; const FILTER_LABEL_KEYS: Record = { all: 'sourceControl.filter.all', @@ -13,21 +17,47 @@ const FILTER_LABEL_KEYS: Record = { synced: 'sourceControl.filter.synced', }; -/** Renders the six-way Source Control filter switch, with per-filter counts from the ViewModel. */ +export interface FilterMenuCallbacks { + /** Switches the active filter chip. */ + onFilterChange: (filter: SourceControlFilter) => void; + /** Toggles whether synced changes are surfaced at all (the "Show synced" switch). */ + onToggleShowSynced: (show: boolean) => void; +} + +/** + * Renders the Source Control filter row: the five action chips (All, Changes, + * Ready to Push, Remote Changes, Conflicts) followed by a "Show synced" + * toggle. The `synced` chip is appended only when `showSynced` is on, so a + * hidden synced bucket contributes no chip and no count to the row. + * + * Per-filter counts come straight from the ViewModel's single-source counts; + * the menu never recomputes one. + */ export function renderFilterMenu( container: HTMLElement, current: SourceControlFilter, counts: Record, - onChange: (filter: SourceControlFilter) => void, + showSynced: boolean, + callbacks: FilterMenuCallbacks, ): void { const menu = container.createDiv({ cls: 'scv-filter-menu' }); - for (const value of FILTER_ORDER) { + + const renderChip = (value: SourceControlFilter): void => { const isActive = value === current; const btn = menu.createEl('button', { cls: `scv-filter-option${isActive ? ' is-active' : ''}` }); btn.setAttr('data-filter', value); btn.setAttr('aria-pressed', String(isActive)); btn.createSpan({ cls: 'scv-filter-label', text: t(FILTER_LABEL_KEYS[value]) }); btn.createSpan({ cls: 'scv-filter-count', text: String(counts[value] ?? 0) }); - btn.addEventListener('click', () => onChange(value)); - } -} + btn.addEventListener('click', () => callbacks.onFilterChange(value)); + }; + + for (const value of ACTION_FILTERS) renderChip(value); + if (showSynced) renderChip('synced'); + + const toggle = menu.createEl('label', { cls: 'scv-filter-show-synced' }); + const checkbox = toggle.createEl('input', { type: 'checkbox', cls: 'scv-filter-show-synced-checkbox' }); + checkbox.checked = showSynced; + checkbox.addEventListener('change', () => callbacks.onToggleShowSynced(checkbox.checked)); + toggle.createSpan({ cls: 'scv-filter-show-synced-label', text: t('sourceControl.filter.showSynced') }); +} \ No newline at end of file diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index 0010a90..a1afc0d 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -5,7 +5,6 @@ import type { SourceControlFilter } from '../../logic/source-control/SourceContr import { SourceControlViewModel, type SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; import type { ChangeId } from '../../logic/source-control/types'; import { renderDiffPanel } from '../components/DiffPanel'; -import { renderChangeSection } from './ChangeSection'; import { renderChangeTree, type ChangeTreeCallbacks } from './ChangeTree'; import { renderFilterMenu } from './FilterMenu'; import { renderSourceControlHeader } from './SourceControlHeader'; @@ -24,22 +23,22 @@ 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']; - -const SECTION_TITLE_KEYS: Record = { - 'ready-to-push': 'sourceControl.section.readyToPush', - changes: 'sourceControl.section.changes', - 'remote-changes': 'sourceControl.section.remoteChanges', - conflicts: 'sourceControl.section.conflicts', - synced: 'sourceControl.section.synced', +/** Active-filter header title keys. Every filter renders one header + a flat tree (no section breakdown). */ +const FILTER_HEADER_KEYS: Record = { + all: 'sourceControl.section.all', + changes: 'sourceControl.section.changes', + 'ready-to-push': 'sourceControl.section.readyToPush', + 'remote-changes': 'sourceControl.section.remoteChanges', + conflicts: 'sourceControl.section.conflicts', + synced: 'sourceControl.section.synced', }; +/** Tree shaping so the change tree stays a compact change view, not a full Explorer. */ +const TREE_OPTIONS = { collapseSingleChild: true }; + /** - * Composes the Source Control UI (Header, Filter, ChangeTree/sections, Diff - * panel) from `SourceControlViewModel` state, per + * Composes the Source Control UI (Header, Filter, change tree, Diff panel) + * from `SourceControlViewModel` state, per * docs/source-control-refactor/phase-3-source-control-ui.md. * * Pure presentation + wiring: push/diff intent is handed to injected @@ -48,10 +47,17 @@ const SECTION_TITLE_KEYS: Record = { * 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. + * + * Rendering semantics (status-grouping fix): + * - Every filter — including "All" — renders a single flat tree. "All" no + * longer breaks the view into CHANGES / REMOTE CHANGES / SYNCED sections, so + * a change never appears twice and SYNCED never leaks into All. + * - Synced is hidden by default (`showSynced = false`): the `synced` chip is + * absent and synced rows render nowhere. The "Show synced" toggle opts in. */ export class SourceControlView { private filter: SourceControlFilter = 'all'; - private readonly collapsedSections = new Set(); + private showSynced = false; private readonly collapsedFolders = new Set(); private selectedChangeId: ChangeId | null = null; private container?: HTMLElement; @@ -86,6 +92,7 @@ export class SourceControlView { } getFilter(): SourceControlFilter { return this.filter; } + getShowSynced(): boolean { return this.showSynced; } getSelectedChangeId(): ChangeId | null { return this.selectedChangeId; } private rerender(): void { @@ -93,7 +100,7 @@ export class SourceControlView { } private renderMain(container: HTMLElement): void { - const state = this.viewModel.getState(this.filter); + const state = this.viewModel.getState(this.filter, this.showSynced); renderSourceControlHeader( container, @@ -101,12 +108,24 @@ export class SourceControlView { { onPush: () => { void this.callbacks.onPush(this.selection.getSelectedChangeIds()); } }, ); - renderFilterMenu(container, this.filter, state.counts, (filter) => { - this.filter = filter; - this.rerender(); - }); + renderFilterMenu( + container, + this.filter, + state.counts, + this.showSynced, + { + onFilterChange: (filter) => { this.filter = filter; this.rerender(); }, + onToggleShowSynced: (show) => { + this.showSynced = show; + // If the user hid synced while viewing it, fall back to All. + if (!show && this.filter === 'synced') this.filter = 'all'; + this.rerender(); + }, + }, + ); const body = container.createDiv({ cls: 'scv-body' }); + this.renderActiveFilterHeader(body, state.filter, state.items.length); if (state.items.length === 0) { body.createDiv({ cls: 'scv-empty', text: t('sourceControl.empty') }); return; @@ -118,33 +137,14 @@ export class SourceControlView { onOpenDiff: (item) => this.openDiff(item), }; - if (this.filter === 'all') { - this.renderSections(body, treeCallbacks); - } else { - renderChangeTree(body, state.items, this.collapsedFolders, treeCallbacks); - } + renderChangeTree(body, state.items, this.collapsedFolders, treeCallbacks, TREE_OPTIONS); } - private renderSections(body: HTMLElement, treeCallbacks: ChangeTreeCallbacks): void { - for (const sectionFilter of SECTION_FILTERS) { - const items = this.viewModel.getState(sectionFilter).items; - if (items.length === 0) continue; - - renderChangeSection( - body, - { - id: sectionFilter, - title: t(SECTION_TITLE_KEYS[sectionFilter]), - items, - collapsed: this.collapsedSections.has(sectionFilter), - collapsedFolders: this.collapsedFolders, - }, - { - ...treeCallbacks, - onToggleSection: (id) => this.toggleSection(id), - }, - ); - } + /** Renders the single active-filter header (e.g. "ALL (132)") above the flat tree. */ + private renderActiveFilterHeader(container: HTMLElement, filter: SourceControlFilter, count: number): void { + const header = container.createDiv({ cls: 'scv-active-filter-header' }); + header.createSpan({ cls: 'scv-active-filter-title', text: t(FILTER_HEADER_KEYS[filter]) }); + header.createSpan({ cls: 'scv-active-filter-count', text: String(count) }); } private renderDiffPane(container: HTMLElement): void { @@ -169,7 +169,8 @@ export class SourceControlView { private async loadAndRenderDiff(container: HTMLElement, changeId: ChangeId): Promise { if (!this.callbacks.loadDiffContent) return; - const item = this.viewModel.getState('all').items.find(i => i.id === changeId); + const item = this.viewModel.getState('all', this.showSynced).items.find(i => i.id === changeId) + ?? this.viewModel.getState('synced', this.showSynced).items.find(i => i.id === changeId); if (!item) return; const content = await this.callbacks.loadDiffContent(item); @@ -178,12 +179,6 @@ export class SourceControlView { 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); @@ -201,4 +196,4 @@ export class SourceControlView { if (this.callbacks.onOpenDiff) void this.callbacks.onOpenDiff(item); this.rerender(); } -} +} \ No newline at end of file diff --git a/styles.css b/styles.css index b9355e7..e494def 100644 --- a/styles.css +++ b/styles.css @@ -93,6 +93,51 @@ background: rgba(255, 255, 255, 0.22); } +/* ── Show synced toggle ───────────────────────────────────────── */ +.scv-filter-show-synced { + display: flex; + align-items: center; + gap: 4px; + padding: 4px 8px; + font-size: 0.78em; + color: var(--text-muted); + cursor: pointer; + white-space: nowrap; + margin-left: auto; +} + +.scv-filter-show-synced-checkbox { + margin: 0; + cursor: pointer; +} + +.scv-filter-show-synced-label { + text-transform: none; + letter-spacing: 0; +} + +/* ── Active filter header ─────────────────────────────────────── */ +.scv-active-filter-header { + display: flex; + align-items: center; + gap: 6px; + padding: 6px 12px 4px 12px; + color: var(--text-muted); + font-size: 0.78em; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.02em; +} + +.scv-active-filter-count { + background: var(--background-modifier-border); + border-radius: 10px; + padding: 1px 6px; + font-size: 0.9em; + min-width: 18px; + text-align: center; +} + /* ── Push button ───────────────────────────────────────────────── */ .scv-push-btn { display: flex; diff --git a/tests/logic/source-control/ChangeTreeBuilder.test.ts b/tests/logic/source-control/ChangeTreeBuilder.test.ts index 5e417e3..67ba0b2 100644 --- a/tests/logic/source-control/ChangeTreeBuilder.test.ts +++ b/tests/logic/source-control/ChangeTreeBuilder.test.ts @@ -91,4 +91,50 @@ describe('ChangeTreeBuilder', () => { { type: 'file', id: toChangeId('c-1'), name: 'd.md', path: 'a/b/c/d.md', previousPath: undefined, kind: 'local-only' }, ]); }); + + describe('TreeDisplayOptions', () => { + it('collapses single-child folder chains into one combined path node', () => { + const builder = new ChangeTreeBuilder(); + const changes: SyncChange[] = [ + { id: toChangeId('c-1'), path: '02_Areas/blog/_pixnet/zh-tw/tech/pixnet-xxx.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: '02_Areas/blog/_pixnet/zh-tw/tech/pixnet-yyy.md', kind: 'local-only' }, + ]; + + const tree = builder.build(changes, { collapseSingleChild: true }); + const folder = tree[0] as ChangeTreeFolderNode; + + // The single-child chain 02_Areas/.../tech collapses to one folder node + // holding both files, instead of five nested expandable rows. + expect(tree).toHaveLength(1); + expect(folder.name).toBe('02_Areas/blog/_pixnet/zh-tw/tech'); + expect(folder.children.map(child => child.name)).toEqual(['pixnet-xxx.md', 'pixnet-yyy.md']); + }); + + it('keeps sibling files from breaking out of their shared folder under collapseSingleChild', () => { + const builder = new ChangeTreeBuilder(); + const changes: SyncChange[] = [ + { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'notes/inner/b.md', kind: 'local-only' }, + ]; + + const tree = builder.build(changes, { collapseSingleChild: true }); + const notes = tree[0] as ChangeTreeFolderNode; + + // `notes` has a file sibling (a.md) alongside the inner folder, so it is + // not merged away; only the single-child `inner` run would collapse if + // it had no file siblings of its own. + expect(notes.name).toBe('notes'); + expect(notes.children.some(c => c.type === 'file')).toBe(true); + }); + + it('leaves full nesting intact by default (no options)', () => { + const builder = new ChangeTreeBuilder(); + const changes: SyncChange[] = [ + { id: toChangeId('c-1'), path: 'a/b/c/d.md', kind: 'local-only' }, + ]; + + const tree = builder.build(changes); + expect((tree[0] as ChangeTreeFolderNode).name).toBe('a'); + }); + }); }); diff --git a/tests/logic/source-control/SourceControlSummary.test.ts b/tests/logic/source-control/SourceControlSummary.test.ts new file mode 100644 index 0000000..74cb68a --- /dev/null +++ b/tests/logic/source-control/SourceControlSummary.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import { buildSummary } from '../../../src/logic/source-control/SourceControlSummary'; +import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; +import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; + +function local(id: string, path = `${id}.md`): SyncChange { + return { id: toChangeId(id), path, kind: 'local-only' }; +} + +function remote(id: string, path = `${id}.md`): SyncChange { + return { id: toChangeId(id), path, kind: 'remote-only' }; +} + +function synced(id: string, path = `${id}.md`): SyncChange { + return { id: toChangeId(id), path, kind: 'synced' }; +} + +/** Builds a change set with `nLocal` local, `nRemote` remote, and `nSynced` synced changes. */ +function changes(nLocal: number, nRemote: number, nSynced: number): SyncChange[] { + const out: SyncChange[] = []; + for (let i = 0; i < nLocal; i++) out.push(local(`local-${i}`)); + for (let i = 0; i < nRemote; i++) out.push(remote(`remote-${i}`)); + for (let i = 0; i < nSynced; i++) out.push(synced(`synced-${i}`)); + return out; +} + +describe('SourceControlSummary', () => { + describe('Case 1: actionable All excludes synced', () => { + it('reports all = local + remote (132) and synced = 36 for 115/17/36', () => { + const selection = new PushSelectionStore(); + const summary = buildSummary(changes(115, 17, 36), selection, true); + + expect(summary.counts.all).toBe(132); + expect(summary.counts.changes).toBe(115); + expect(summary.counts['remote-changes']).toBe(17); + expect(summary.counts.synced).toBe(36); + expect(summary.synced).toHaveLength(36); + }); + + it('keeps synced changes out of the actionable all bucket', () => { + const selection = new PushSelectionStore(); + const summary = buildSummary(changes(115, 17, 36), selection, true); + + expect(summary.all.every(change => change.kind !== 'synced')).toBe(true); + expect(summary.all).toHaveLength(132); + }); + }); + + describe('Case 2: synced hidden (showSynced = false)', () => { + it('renders a synced count of 0 while the raw synced bucket still holds 36', () => { + const selection = new PushSelectionStore(); + const summary = buildSummary(changes(115, 17, 36), selection, false); + + expect(summary.counts.synced).toBe(0); + expect(summary.synced).toHaveLength(36); + // render count (0) differs from the actual synced count (36) + expect(summary.counts.synced).not.toBe(summary.synced.length); + }); + + it('does not affect the actionable All count when synced is hidden', () => { + const selection = new PushSelectionStore(); + const hidden = buildSummary(changes(115, 17, 36), selection, false); + const shown = buildSummary(changes(115, 17, 36), selection, true); + + expect(hidden.counts.all).toBe(132); + expect(hidden.counts.all).toBe(shown.counts.all); + }); + }); + + describe('Case 3: All filter never surfaces a synced bucket', () => { + it('contains no synced change in the actionable all bucket', () => { + const selection = new PushSelectionStore(); + const summary = buildSummary(changes(10, 5, 20), selection, false); + + expect(summary.all.filter(change => change.kind === 'synced')).toEqual([]); + }); + + it('partitions kinds into disjoint actionable buckets plus synced', () => { + const input: SyncChange[] = [ + local('l1'), { id: toChangeId('l2'), path: 'l2.md', kind: 'local-modified' }, + { id: toChangeId('l3'), path: 'l3.md', kind: 'moved' }, + remote('r1'), { id: toChangeId('r2'), path: 'r2.md', kind: 'remote-modified' }, + { id: toChangeId('cf'), path: 'cf.md', kind: 'conflict' }, + synced('s1'), + ]; + const summary = buildSummary(input, new PushSelectionStore(), true); + + expect(summary.localChanges.map(c => c.id)).toEqual([toChangeId('l1'), toChangeId('l2'), toChangeId('l3')]); + expect(summary.remoteChanges.map(c => c.id)).toEqual([toChangeId('r1'), toChangeId('r2')]); + expect(summary.conflicts.map(c => c.id)).toEqual([toChangeId('cf')]); + expect(summary.synced.map(c => c.id)).toEqual([toChangeId('s1')]); + expect(summary.all.map(c => c.id)).toEqual([ + toChangeId('l1'), toChangeId('l2'), toChangeId('l3'), + toChangeId('r1'), toChangeId('r2'), toChangeId('cf'), + ]); + }); + }); + + describe('ready-to-push selection', () => { + it('counts only selected actionable changes, ignoring synced selections', () => { + const selection = new PushSelectionStore(); + selection.includeForPush(toChangeId('local-0')); + selection.includeForPush(toChangeId('synced-0')); + const summary = buildSummary(changes(2, 1, 2), selection, true); + + // synced-0 was selected but is not actionable, so it is excluded. + expect(summary.counts['ready-to-push']).toBe(1); + expect(summary.readyToPush.map(c => c.id)).toEqual([toChangeId('local-0')]); + }); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/SourceControlViewModel.test.ts b/tests/logic/source-control/SourceControlViewModel.test.ts index eba3edc..cd9b1d1 100644 --- a/tests/logic/source-control/SourceControlViewModel.test.ts +++ b/tests/logic/source-control/SourceControlViewModel.test.ts @@ -62,16 +62,19 @@ describe('SourceControlViewModel', () => { expect(viewModel.getState('all').items[0]?.operationStatus).toBe('running'); }); - it('excludes synced changes from "changes" but keeps them in "synced" and "all"', () => { + it('excludes synced changes from "changes" and "all", surfacing them only via "synced" + showSynced', () => { const synced: SyncChange = { id: toChangeId('c-1'), path: 'a.md', kind: 'synced' }; const { viewModel } = buildViewModel([synced]); + // Synced is not actionable: it never appears under All or Changes. + expect(viewModel.getState('all').items).toEqual([]); expect(viewModel.getState('changes').items).toEqual([]); - expect(viewModel.getState('synced').items.map(i => i.id)).toEqual([toChangeId('c-1')]); - expect(viewModel.getState('all').items.map(i => i.id)).toEqual([toChangeId('c-1')]); + // Hidden by default: the synced filter yields nothing until the user opts in. + expect(viewModel.getState('synced').items).toEqual([]); + expect(viewModel.getState('synced', true).items.map(i => i.id)).toEqual([toChangeId('c-1')]); }); - it('counts every filter bucket regardless of the active filter', () => { + it('counts every filter bucket from the single-source summary, regardless of the active filter', () => { const changes: SyncChange[] = [ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, { id: toChangeId('c-2'), path: 'b.md', kind: 'remote-only' }, @@ -80,15 +83,19 @@ describe('SourceControlViewModel', () => { ]; const { viewModel } = buildViewModel(changes); + // showSynced = false (default): synced contributes 0 to counts and is absent from All. const { counts } = viewModel.getState('all'); expect(counts).toEqual({ - all: 4, - changes: 3, + all: 3, + changes: 1, 'ready-to-push': 0, 'remote-changes': 1, conflicts: 1, - synced: 1, + synced: 0, }); + + // showSynced = true: the raw synced count (1) surfaces. + expect(viewModel.getState('all', true).counts.synced).toBe(1); }); it('keeps ChangeId stable across a rename', () => { diff --git a/tests/ui/source-control/FilterMenu.test.ts b/tests/ui/source-control/FilterMenu.test.ts index 69410f6..dc6858f 100644 --- a/tests/ui/source-control/FilterMenu.test.ts +++ b/tests/ui/source-control/FilterMenu.test.ts @@ -11,48 +11,65 @@ const zeroCounts: Record = { describe('renderFilterMenu', () => { let container: HTMLElement; - let onChange: (filter: SourceControlFilter) => void; + let callbacks: { onFilterChange: (f: SourceControlFilter) => void; onToggleShowSynced: (s: boolean) => void }; beforeEach(() => { container = createContainer(); - onChange = vi.fn(); + callbacks = { onFilterChange: vi.fn(), onToggleShowSynced: vi.fn() }; }); - it('renders all six filters in spec order', () => { - renderFilterMenu(container, 'all', zeroCounts, onChange); + it('renders the five action chips (no synced chip) when showSynced is false', () => { + renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + + const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); + expect(filters).toEqual(['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts']); + }); + + it('appends the synced chip when showSynced is true', () => { + renderFilterMenu(container, 'all', { ...zeroCounts, synced: 7 }, true, callbacks); const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); expect(filters).toEqual(['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts', 'synced']); + const syncedOption = container.querySelector('.scv-filter-option[data-filter="synced"]'); + expect(syncedOption?.querySelector('.scv-filter-count')?.textContent).toBe('7'); }); - it('marks the current filter as active', () => { - renderFilterMenu(container, 'conflicts', zeroCounts, onChange); + it('marks the current filter chip as active', () => { + renderFilterMenu(container, 'conflicts', zeroCounts, false, callbacks); const active = container.querySelector('.scv-filter-option.is-active'); expect(active?.getAttribute('data-filter')).toBe('conflicts'); }); it('shows the per-filter count from the ViewModel', () => { - renderFilterMenu(container, 'all', { ...zeroCounts, conflicts: 3 }, onChange); + renderFilterMenu(container, 'all', { ...zeroCounts, conflicts: 3 }, false, callbacks); const conflictsOption = container.querySelector('.scv-filter-option[data-filter="conflicts"]'); expect(conflictsOption?.querySelector('.scv-filter-count')?.textContent).toBe('3'); }); - it('calls onChange with the clicked filter value (filter switching)', () => { - renderFilterMenu(container, 'all', zeroCounts, onChange); + it('calls onFilterChange with the clicked filter value', () => { + renderFilterMenu(container, 'all', zeroCounts, false, callbacks); (container.querySelector('.scv-filter-option[data-filter="remote-changes"]') as HTMLButtonElement).click(); - expect(onChange).toHaveBeenCalledWith('remote-changes'); + expect(callbacks.onFilterChange).toHaveBeenCalledWith('remote-changes'); + }); + + it('renders the Show synced toggle reflecting the showSynced state', () => { + renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; + expect(checkbox).not.toBeNull(); + expect(checkbox.checked).toBe(false); }); - it('does not call onChange for filters that were not clicked', () => { - renderFilterMenu(container, 'all', zeroCounts, onChange); + it('calls onToggleShowSynced when the Show synced checkbox changes', () => { + renderFilterMenu(container, 'all', zeroCounts, false, callbacks); - (container.querySelector('.scv-filter-option[data-filter="synced"]') as HTMLButtonElement).click(); + const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; + checkbox.checked = true; + checkbox.dispatchEvent(new Event('change')); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith('synced'); + expect(callbacks.onToggleShowSynced).toHaveBeenCalledWith(true); }); -}); +}); \ No newline at end of file diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 5110067..493da34 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -28,7 +28,7 @@ describe('SourceControlView', () => { }); describe('filter switching', () => { - it('groups changes into their sections under the "all" filter', () => { + it('renders "All" as a single flat tree (no section breakdown) and excludes synced', () => { const { view } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, { id: toChangeId('c-2'), path: 'b.md', kind: 'remote-only' }, @@ -37,8 +37,27 @@ describe('SourceControlView', () => { ]); view.render(container); + // No section grouping under All — every filter renders one flat tree. + expect(container.querySelectorAll('.scv-section')).toHaveLength(0); + // Active-filter header reads "ALL". + expect(container.querySelector('.scv-active-filter-title')?.textContent).toBe('ALL'); + expect(container.querySelector('.scv-active-filter-count')?.textContent).toBe('3'); + // Actionable items only: synced is absent from All. + const kinds = Array.from(container.querySelectorAll('.scv-change-item')).map(el => el.getAttribute('class')); + expect(kinds.some(c => c?.includes('scv-kind-synced'))).toBe(false); + expect(container.querySelectorAll('.scv-change-item')).toHaveLength(3); + }); + + it('does not render a SYNCED section under the All filter (status-grouping fix)', () => { + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, + ]); + view.render(container); + const sectionTitles = Array.from(container.querySelectorAll('.scv-section-title')).map(el => el.textContent); - expect(sectionTitles).toEqual(['CHANGES', 'REMOTE CHANGES', 'CONFLICTS', 'SYNCED']); + expect(sectionTitles).not.toContain('SYNCED'); + expect(container.querySelectorAll('.scv-section')).toHaveLength(0); }); it('shows a flat tree (no sections) once a specific filter is selected', () => { @@ -65,6 +84,56 @@ describe('SourceControlView', () => { }); }); + describe('show synced toggle', () => { + it('hides the synced chip and synced rows by default', () => { + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, + ]); + view.render(container); + + expect(container.querySelector('.scv-filter-option[data-filter="synced"]')).toBeNull(); + expect(view.getShowSynced()).toBe(false); + }); + + it('reveals the synced chip and renders synced rows when toggled on', () => { + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, + ]); + view.render(container); + + const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; + checkbox.checked = true; + checkbox.dispatchEvent(new Event('change')); + + expect(view.getShowSynced()).toBe(true); + const syncedChip = container.querySelector('.scv-filter-option[data-filter="synced"]'); + expect(syncedChip).not.toBeNull(); + expect(syncedChip?.querySelector('.scv-filter-count')?.textContent).toBe('1'); + }); + + it('falls back to All when synced is hidden while viewing the synced filter', () => { + const { view } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, + ]); + view.render(container); + + // Opt in and switch to the synced filter. + const toggle = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; + toggle.checked = true; + toggle.dispatchEvent(new Event('change')); + (container.querySelector('.scv-filter-option[data-filter="synced"]') as HTMLButtonElement).click(); + expect(view.getFilter()).toBe('synced'); + + // Opt back out: filter snaps back to All. + toggle.checked = false; + toggle.dispatchEvent(new Event('change')); + expect(view.getFilter()).toBe('all'); + }); + }); + describe('selection', () => { it('moves a change into "ready to push" and updates the push button count', () => { const { view, selection } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); From 8f130b31cc94c756b9d69bdf81a8e75dd5a62382 Mon Sep 17 00:00:00 2001 From: ClaudiaFang Date: Sat, 22 Aug 2026 18:40:53 +0800 Subject: [PATCH 02/21] test: add source control filter coverage --- .../SourceControlFilter.test.ts | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 tests/logic/source-control/SourceControlFilter.test.ts diff --git a/tests/logic/source-control/SourceControlFilter.test.ts b/tests/logic/source-control/SourceControlFilter.test.ts new file mode 100644 index 0000000..c35e303 --- /dev/null +++ b/tests/logic/source-control/SourceControlFilter.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; +import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; +import { matchesFilter, type SourceControlFilter } from '../../../src/logic/source-control/SourceControlFilter'; +import { toChangeId, type SyncChange, type SyncChangeKind } from '../../../src/logic/source-control/types'; + +function change(id: string, kind: SyncChangeKind): SyncChange { + return { id: toChangeId(id), path: `${id}.md`, kind }; +} + +const FILTERS: SourceControlFilter[] = [ + 'all', + 'changes', + 'ready-to-push', + 'remote-changes', + 'conflicts', + 'synced', +]; + +describe('SourceControlFilter', () => { + it.each([ + ['local-only', ['all', 'changes']], + ['local-modified', ['all', 'changes']], + ['moved', ['all', 'changes']], + ['remote-only', ['all', 'remote-changes']], + ['remote-modified', ['all', 'remote-changes']], + ['conflict', ['all', 'conflicts']], + ['synced', ['synced']], + ] as const)('maps %s into the expected non-selection filters', (kind, expectedFilters) => { + const selection = new PushSelectionStore(); + const item = change(`change-${kind}`, kind); + + const matched = FILTERS.filter(filter => matchesFilter(item, filter, selection)); + + expect(matched).toEqual(expectedFilters); + }); + + it('puts a selected actionable change into ready-to-push without changing its status bucket', () => { + const selection = new PushSelectionStore(); + const item = change('local', 'local-modified'); + selection.includeForPush(item.id); + + expect(matchesFilter(item, 'ready-to-push', selection)).toBe(true); + expect(matchesFilter(item, 'changes', selection)).toBe(true); + expect(matchesFilter(item, 'all', selection)).toBe(true); + }); + + it('does not put an unselected actionable change into ready-to-push', () => { + const selection = new PushSelectionStore(); + const item = change('remote', 'remote-modified'); + + expect(matchesFilter(item, 'ready-to-push', selection)).toBe(false); + expect(matchesFilter(item, 'remote-changes', selection)).toBe(true); + }); + + it('never treats a selected synced change as ready-to-push or actionable', () => { + const selection = new PushSelectionStore(); + const item = change('synced', 'synced'); + selection.includeForPush(item.id); + + expect(matchesFilter(item, 'ready-to-push', selection)).toBe(false); + expect(matchesFilter(item, 'all', selection)).toBe(false); + expect(matchesFilter(item, 'synced', selection)).toBe(true); + }); + + it('keeps conflicts distinct from local and remote status filters', () => { + const selection = new PushSelectionStore(); + const item = change('conflict', 'conflict'); + + expect(matchesFilter(item, 'conflicts', selection)).toBe(true); + expect(matchesFilter(item, 'changes', selection)).toBe(false); + expect(matchesFilter(item, 'remote-changes', selection)).toBe(false); + }); + + it('preserves ready-to-push membership across a move because selection is keyed by ChangeId', () => { + const selection = new PushSelectionStore(); + const id = toChangeId('move-1'); + selection.includeForPush(id); + + const moved: SyncChange = { + id, + path: 'archive/a.md', + previousPath: 'folder/a.md', + kind: 'moved', + }; + + expect(matchesFilter(moved, 'ready-to-push', selection)).toBe(true); + expect(matchesFilter(moved, 'changes', selection)).toBe(true); + }); +}); From 8ed5df940ffdfbc07095090141559cb021f66364 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:52:20 +0000 Subject: [PATCH 03/21] test: extract reusable source control E2E fixtures Add e2e/support/sync-manager-fixture.ts (real-provider service + verifier + TFile shim + auto-confirming plan/conflict modals, steered by a per-test conflict resolver) and e2e/support/source-control- scenarios.ts (high-level seed/modify/assert verbs + the Source Control selection stack wiring), so workflow suites read as seed -> modify -> push -> expect instead of 50 lines of setup per test. Add a non-breaking removeLocal to FakeVault for delete-local conflict scenarios. No production code touched; existing provider/SyncManager E2E unchanged. --- e2e/shim/fake-vault.ts | 5 + e2e/support/source-control-scenarios.ts | 207 ++++++++++++++++++++++++ e2e/support/sync-manager-fixture.ts | 142 ++++++++++++++++ 3 files changed, 354 insertions(+) create mode 100644 e2e/support/source-control-scenarios.ts create mode 100644 e2e/support/sync-manager-fixture.ts diff --git a/e2e/shim/fake-vault.ts b/e2e/shim/fake-vault.ts index 553b4b4..afb5753 100644 --- a/e2e/shim/fake-vault.ts +++ b/e2e/shim/fake-vault.ts @@ -40,6 +40,11 @@ export class FakeVault { this.files.set(newPath, content); } + /** Removes a local file, mirroring Obsidian's vault delete. */ + removeLocal(path: string): void { + this.files.delete(path); + } + /** Constructs a real TFile handle for a path already in this vault. */ fileAt(path: string): TFileLike { return new this.TFile(path); diff --git a/e2e/support/source-control-scenarios.ts b/e2e/support/source-control-scenarios.ts new file mode 100644 index 0000000..d2e3d0e --- /dev/null +++ b/e2e/support/source-control-scenarios.ts @@ -0,0 +1,207 @@ +import { expect } from 'vitest'; +import type { TFile } from 'obsidian'; +import type { GitServiceInterface } from '../../src/services/git-service-interface'; +import type { SyncManager } from '../../src/logic/sync-manager'; +import type { BatchPushConflict, ConflictResolution, PushResults } from '../../src/logic/sync/types'; +import type { GitLabFilesPushSettings } from '../../src/settings'; +import type { FakeVault, TFileLike } from '../shim/fake-vault'; +import type { SyncManagerFixture } from './sync-manager-fixture'; +import type { GitVerifier as GitVerifierType } from '../verifier-runtime-types'; +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 { SourceControlActionService } from '../../src/logic/source-control/SourceControlActionService'; +import { BoundarySyncWorkspace } from '../../src/logic/sync/SyncWorkspace'; +import { toChangeId, type SyncChange } from '../../src/logic/source-control/types'; +import type { SyncStatusRefreshResult } from '../../src/logic/sync/SyncStatusRefreshService'; +import type { RemoteDeleteResult } from '../../src/logic/sync/RemoteDeleteExecutor'; +import type { FileDiff } from '../../src/logic/sync/types'; +import type { GitTreeEntry } from '../../src/services/git-service-interface'; + +/** + * High-level scenario wrapper around a {@link SyncManagerFixture}: owns one + * FakeVault + settings + real SyncManager for a test, and exposes the + * seed/modify/assert verbs the source-control-flow suites use, so a test reads + * as `seed → modify local → modify remote → push → expect` instead of 50 lines + * of setup. Remote assertions always go through the fixture's independent + * git-CLI verifier, never the service under test. + * + * One scenario per test; paths are supplied by the caller (via + * `fixture.path`) so two scenarios can share a remote path when a test needs a + * fresh manager against pre-seeded remote state. + */ +export class SourceControlScenario { + readonly vault: FakeVault; + readonly settings: GitLabFilesPushSettings; + readonly manager: SyncManager; + private readonly service: GitServiceInterface; + private readonly verifier: GitVerifierType; + private readonly branch: string; + + constructor(fixture: SyncManagerFixture) { + this.vault = fixture.createVault(); + this.settings = fixture.makeSettings(); + this.manager = fixture.newManager(this.vault, this.settings); + this.service = fixture.service; + this.verifier = fixture.verifier; + this.branch = fixture.branch; + } + + // --- local vault ops ------------------------------------------------- + + writeLocal(path: string, content: string | ArrayBuffer): void { + this.vault.writeLocal(path, content); + } + + deleteLocal(path: string): void { + this.vault.removeLocal(path); + } + + renameLocal(oldPath: string, newPath: string): void { + this.vault.renameLocal(oldPath, newPath); + } + + /** Real TFile handle for a path in this vault (needed so push rename-detection runs). */ + tfile(path: string): TFileLike { + return this.vault.fileAt(path); + } + + localExists(path: string): boolean { + return this.vault.has(path); + } + + async readLocal(path: string): Promise { + return this.vault.adapter.read(path); + } + + // --- remote ops (via the real production service) -------------------- + + /** Seeds the remote directly, bypassing SyncManager — no local file, no metadata. */ + async seedRemote(path: string, content: string | ArrayBuffer): Promise { + await this.service.pushFile(path, content, this.branch, 'e2e: seed remote'); + } + + /** Overwrites the remote path with new content, reading the current sha first (like another client pushing). */ + async modifyRemote(path: string, content: string | ArrayBuffer): Promise { + const current = await this.verifier.getFile(path, this.branch); + await this.service.pushFile(path, content, this.branch, 'e2e: modify remote', current?.sha); + } + + async deleteRemoteFile(path: string): Promise { + await this.service.deleteFile(path, this.branch, 'e2e: delete remote'); + } + + // --- baseline (push through the manager to establish synced metadata) --- + + /** Writes locally and pushes via the manager, establishing a synced baseline (local == remote + metadata). */ + async baseline(path: string, content: string | ArrayBuffer): Promise { + this.writeLocal(path, content); + return this.manager.pushFiles([path]); + } + + // --- sync actions ---------------------------------------------------- + + /** Pushes via the real manager. Accepts TFile handles (for rename detection) or plain paths. */ + async push(files: (TFileLike | string)[]): Promise { + return this.manager.pushFiles(files as unknown as (TFile | string)[]); + } + + async pullFile(path: string): Promise { + await this.manager.pullFile(path); + } + + // --- independent remote assertions (via the git-CLI verifier) -------- + + async remoteContent(path: string): Promise<{ content: string; sha: string } | null> { + return this.verifier.getFile(path, this.branch); + } + + async expectRemoteContent(path: string, expected: string): Promise { + const remote = await this.verifier.getFile(path, this.branch); + expect(remote?.content, `remote content for ${path}`).toBe(expected); + } + + async expectRemoteMissing(path: string): Promise { + expect(await this.verifier.fileMissing(path, this.branch), `expected ${path} missing on remote`).toBe(true); + } + + async expectRemoteExists(path: string): Promise { + expect(await this.verifier.fileMissing(path, this.branch), `expected ${path} present on remote`).toBe(false); + } + + /** Current branch tip sha. */ + async head(): Promise { + const [tip] = await this.verifier.listCommitShas(this.branch, 1); + return tip!; + } + + /** Asserts exactly one new commit landed since `headBefore` (the new commit's parent is `headBefore`). */ + async expectSingleCommitSince(headBefore: string): Promise { + const [headAfter, headAfterParent] = await this.verifier.listCommitShas(this.branch, 2); + expect(headAfter, 'expected a new commit on the branch').not.toBe(headBefore); + expect(headAfterParent, 'expected exactly one new commit since baseline').toBe(headBefore); + } + + /** Asserts no new commit landed since `headBefore`. */ + async expectNoCommitSince(headBefore: string): Promise { + expect(await this.head(), 'expected no new commit').toBe(headBefore); + } + + async commitMessage(sha: string): Promise { + return this.verifier.getCommitMessage(sha); + } + + // --- metadata -------------------------------------------------------- + + metadata(path: string) { + return this.settings.syncMetadata[path]; + } + + metadataSha(path: string): string | undefined { + return this.settings.syncMetadata[path]?.lastSyncedSha; + } + + // --- Source Control selection stack (Phase 6) ----------------------- + + /** + * Wires the real Source Control selection layer (ChangeRepository + + * PushSelectionStore + OperationState + SourceControlActionService) on top + * of this scenario's real SyncManager, via the thin BoundarySyncWorkspace. + * `push`/`pull`/`deleteRemote` go through the real manager/provider; the + * selection filter (ChangeId -> path -> workspace call) is the real + * production code under test. + */ + selectionStack(changes: SyncChange[]): SelectionStack { + const repository = new ChangeRepository(); + repository.replace(changes); + const selection = new PushSelectionStore(); + const operations = new OperationState(); + const workspace = new BoundarySyncWorkspace( + () => this.manager, + { + refresh: (): Promise => Promise.resolve({ + localCount: 0, remoteCount: 0, remoteEntries: [] as GitTreeEntry[], + }), + deleteRemote: (): Promise => Promise.resolve({ deletedPaths: [], errors: [] }), + getDiff: (): Promise => Promise.resolve({ path: '', kind: 'text' } as FileDiff), + }, + ); + const actionService = new SourceControlActionService(repository, operations, workspace); + return { repository, selection, operations, actionService, workspace }; + } +} + +export interface SelectionStack { + readonly repository: ChangeRepository; + readonly selection: PushSelectionStore; + readonly operations: OperationState; + readonly actionService: SourceControlActionService; + readonly workspace: BoundarySyncWorkspace; +} + +/** Builds a SyncChange with a path-derived ChangeId (mirrors FileStatusAdapter). */ +export function change(path: string, kind: SyncChange['kind'], previousPath?: string): SyncChange { + return { id: toChangeId(path), path, kind, previousPath }; +} + +export type { ConflictResolution, BatchPushConflict }; \ No newline at end of file diff --git a/e2e/support/sync-manager-fixture.ts b/e2e/support/sync-manager-fixture.ts new file mode 100644 index 0000000..7cb3ca5 --- /dev/null +++ b/e2e/support/sync-manager-fixture.ts @@ -0,0 +1,142 @@ +import { vi } from 'vitest'; +import { SyncManager } from '../../src/logic/sync-manager'; +import type { BatchPushConflict, ConflictResolution, PushResults } from '../../src/logic/sync/types'; +import { SyncPlanModal, type SyncPlanDirection } from '../../src/ui/SyncPlanModal'; +import { BatchConflictResolutionModal } from '../../src/ui/BatchConflictResolutionModal'; +import { ObsidianSyncInteraction } from '../../src/ui/ObsidianSyncInteraction'; +// `import type` deliberately: settings.ts re-exports the settings-tab UI +// (GitLabSyncSettingTab -> FolderSuggest -> AbstractInputSuggest) which pulls +// in far more of `obsidian` than this suite's generated shim provides. A +// type-only import is erased entirely, so none of that module ever loads. +import type { GitLabFilesPushSettings } from '../../src/settings'; +import { FakeVault, fakeApp, type TFileCtor } from '../shim/fake-vault'; +import { currentProvider, contextFor, runtimeDir } from '../config/env'; +import type { GitVerifier as GitVerifierType } from '../verifier-runtime-types'; +import type { GitServiceInterface } from '../../src/services/git-service-interface'; + +/** + * Reusable real-provider E2E fixture for SyncManager workflows. Owns the + * once-per-suite wiring the old `e2e/suites/sync-manager.e2e.test.ts` kept in + * its `beforeAll`: resolving the real production provider service + isolated + * branch, loading the generated git-CLI verifier + TFile shim, and installing + * plan-review/conflict modals that auto-confirm (so a push can proceed without + * a human clicking through). Per-test conflict outcomes are steered through + * {@link setConflictResolver}. + * + * Only the Obsidian filesystem boundary is faked (e2e/shim/fake-vault.ts); + * everything else — SyncManager, PushCoordinator, the provider service — is + * the real production code path against a real Git server. + */ +export interface SyncManagerFixture { + /** Real production provider service for the selected `E2E_PROVIDER`. */ + readonly service: GitServiceInterface; + /** Isolated branch `scripts/e2e-harness.sh provision` created for this run. */ + readonly branch: string; + /** Independent git-CLI verifier (generated at runtime, never committed). */ + readonly verifier: GitVerifierType; + /** The exact TFile class the vitest-runtime `obsidian` alias resolves to. */ + readonly TFile: TFileCtor; + /** Per-suite run id, so every test's remote paths are namespaced apart. */ + readonly runId: string; + /** Namespaced remote path: `path('note.md') -> e2e-sc-/note.md`. */ + path(name: string): string; + /** Fresh settings object pointing at the isolated branch, empty metadata. */ + makeSettings(branch?: string): GitLabFilesPushSettings; + /** A fresh in-memory vault (the only faked boundary). */ + createVault(): FakeVault; + /** A real SyncManager wired to `vault` + `settings` + the real service. */ + newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager; + /** Steers how the auto-confirming conflict modal resolves each conflict. */ + setConflictResolver(resolver: (conflict: BatchPushConflict) => ConflictResolution): void; +} + +export async function createSyncManagerFixture(): Promise { + const provider = currentProvider(); + const ctx = contextFor(provider); + const service = ctx.service; + const branch = ctx.branch; + + const dir = runtimeDir(); + const { GitVerifier } = await import(/* @vite-ignore */ `${dir}/verifier/git-verifier.ts`) as { GitVerifier: new () => GitVerifierType }; + const obsidianShim = await import(/* @vite-ignore */ `${dir}/obsidian-request-url.ts`) as { TFile: TFileCtor }; + const verifier = new GitVerifier(); + const TFile = obsidianShim.TFile; + + let conflictResolver: (conflict: BatchPushConflict) => ConflictResolution = () => 'skip'; + + // Auto-confirm the plan-review modal (production shows it before every + // push/pull). Same pattern as tests/logic/sync-manager-batch.test.ts. + vi.mocked(SyncPlanModal).mockImplementation(function ( + this: SyncPlanModal, _app: unknown, _plan: unknown, _direction: SyncPlanDirection, onConfirm: () => void + ) { + onConfirm(); + return this; + } as never); + + // Every push-side content conflict goes through BatchConflictResolutionModal + // (even a single-file batch). Auto-resolve using the current resolver. + vi.mocked(BatchConflictResolutionModal).mockImplementation(function ( + this: BatchConflictResolutionModal, + _app: unknown, + _gitService: unknown, + conflicts: BatchPushConflict[], + _totalFiles: number, + _safeCount: number, + onResolve: () => void, + _onCancel: () => void, + ) { + for (const conflict of conflicts) conflict.resolution = conflictResolver(conflict); + onResolve(); + return this; + } as never); + + const runId = Math.random().toString(36).slice(2, 10); + + function path(name: string): string { + return `e2e-sc-${runId}/${name}`; + } + + function makeSettings(branchOverride?: string): GitLabFilesPushSettings { + return { + serviceType: 'gitea', + gitlabToken: '', gitlabBaseUrl: '', projectId: '', + githubToken: '', githubOwner: '', githubRepo: '', + giteaToken: '', giteaBaseUrl: '', giteaOwner: '', giteaRepo: '', + branch: branchOverride ?? branch, + syncMetadata: {}, + rootPath: '', + vaultFolder: '', + symlinkHandling: 'skip', + ignorePatterns: '', + lastSeenVersion: '', + bannerDismissedVersion: '', + language: 'system', + autoRefreshOnStartup: true, + }; + } + + function createVault(): FakeVault { + return new FakeVault(TFile); + } + + function newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager { + const app = fakeApp(vault); + return new SyncManager(app, service, settings, undefined, () => false, undefined, new ObsidianSyncInteraction(app)); + } + + return { + service, + branch, + verifier, + TFile, + runId, + path, + makeSettings, + createVault, + newManager, + setConflictResolver: (resolver) => { conflictResolver = resolver; }, + }; +} + +export { describePushResult } from './push-result-diagnostic'; +export type { PushResults }; \ No newline at end of file From df9dea96e6b2293a62e64e65775a71cd531deb88 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:54:31 +0000 Subject: [PATCH 04/21] test: cover complex rename and move workflows Add e2e/suites/source-control-flows.e2e.test.ts and wire it into scripts/run-e2e.sh. Phase 2 covers: rename+modify (one commit, metadata moved to the new path, old path metadata cleared), multi-rename+modify batch (two moves in one commit), and the Extended nested-directory move and A->B->C rename-chain collapse (GitHub only, since they exercise SyncManager rename tracking rather than provider APIs). --- e2e/suites/source-control-flows.e2e.test.ts | 140 ++++++++++++++++++++ scripts/run-e2e.sh | 15 ++- 2 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 e2e/suites/source-control-flows.e2e.test.ts diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts new file mode 100644 index 0000000..a5821b2 --- /dev/null +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, beforeAll, vi } from 'vitest'; +import { createSyncManagerFixture, describePushResult, type SyncManagerFixture } from '../support/sync-manager-fixture'; +import { SourceControlScenario } from '../support/source-control-scenarios'; +import { timeouts } from '../config/env'; + +// Auto-confirm the plan-review + conflict modals so a push can proceed +// without a human. vi.mock is hoisted above the fixture import, so the fixture +// receives the mocked modules and installs their mockImplementation. Pull-side +// SyncConflictModal stays the bare automock default (does nothing, matching +// production: pullFile returns before the conflict modal resolves). +vi.mock('../../src/ui/SyncPlanModal'); +vi.mock('../../src/ui/SyncConflictModal'); +vi.mock('../../src/ui/BatchConflictResolutionModal'); + +// Provider matrix: Core scenarios run on every provider; Extended scenarios +// (rename chains, unicode, batch-scale, etc.) exercise SyncManager/model +// behavior that's provider-agnostic, so they run on GitHub only to keep +// real-API CI fast and stable. +const isGitHub = process.env.E2E_PROVIDER === 'github'; + +describe('Source Control Flows E2E', () => { + let fixture: SyncManagerFixture; + + beforeAll(async () => { + fixture = await createSyncManagerFixture(); + }, timeouts.containerReadyMs + 30_000); + + const path = (name: string): string => fixture.path(name); + const scenario = (): SourceControlScenario => new SourceControlScenario(fixture); + + // ------------------------------------------------------------------ + // Phase 2 — Rename / Move workflows + // ------------------------------------------------------------------ + describe('rename and move workflows', () => { + it('renames and modifies a file in one commit, moving metadata to the new path', async () => { + const s = scenario(); + const oldP = path('rename-modify/a.md'); + const newP = path('rename-modify/archive/a.md'); + await s.baseline(oldP, 'v1'); + expect(s.metadataSha(oldP), 'baseline metadata at old path').toBeTruthy(); + + s.renameLocal(oldP, newP); + s.writeLocal(newP, 'v2'); + await s.manager.trackRename(newP, oldP); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(newP)]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteMissing(oldP); + await s.expectRemoteContent(newP, 'v2'); + await s.expectSingleCommitSince(headBefore); + expect(s.metadataSha(newP), 'metadata moved to new path').toBeTruthy(); + expect(s.metadata(oldP), 'old path metadata removed').toBeUndefined(); + }); + + it('renames and modifies multiple files in one batch push (one commit)', async () => { + const s = scenario(); + const oldA = path('multi-rename/folder/a.md'); + const oldB = path('multi-rename/folder/b.md'); + const newA = path('multi-rename/archive/a.md'); + const newB = path('multi-rename/archive/b.md'); + await s.baseline(oldA, 'a-v1'); + await s.baseline(oldB, 'b-v1'); + + s.renameLocal(oldA, newA); + s.writeLocal(newA, 'a-v2'); + s.renameLocal(oldB, newB); + s.writeLocal(newB, 'b-v2'); + await s.manager.trackRename(newA, oldA); + await s.manager.trackRename(newB, oldB); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(newA), s.tfile(newB)]); + expect(result.success, describePushResult(result)).toBe(2); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteMissing(oldA); + await s.expectRemoteMissing(oldB); + await s.expectRemoteContent(newA, 'a-v2'); + await s.expectRemoteContent(newB, 'b-v2'); + await s.expectSingleCommitSince(headBefore); + }); + + // Extended: nested move + rename chain (SyncManager/model behavior, + // provider-agnostic) — GitHub only. + it.skipIf(!isGitHub)('moves files across nested directories in one commit', async () => { + const s = scenario(); + const oldFlat = path('nested-move/folder/a.md'); + const oldNested = path('nested-move/folder/nested/b.md'); + const newFlat = path('nested-move/archive/a.md'); + const newNested = path('nested-move/archive/nested/b.md'); + await s.baseline(oldFlat, 'flat'); + await s.baseline(oldNested, 'nested'); + + s.renameLocal(oldFlat, newFlat); + s.renameLocal(oldNested, newNested); + await s.manager.trackRename(newFlat, oldFlat); + await s.manager.trackRename(newNested, oldNested); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(newFlat), s.tfile(newNested)]); + expect(result.success, describePushResult(result)).toBe(2); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteMissing(oldFlat); + await s.expectRemoteMissing(oldNested); + await s.expectRemoteContent(newFlat, 'flat'); + await s.expectRemoteContent(newNested, 'nested'); + await s.expectSingleCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('collapses a rename chain (A->B->C) into a single move of the original path', async () => { + const s = scenario(); + const a = path('rename-chain/a.md'); + const b = path('rename-chain/b.md'); + const c = path('rename-chain/c.md'); + await s.baseline(a, 'chain'); + + s.renameLocal(a, b); + await s.manager.trackRename(b, a); + s.renameLocal(b, c); + await s.manager.trackRename(c, b); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(c)]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteMissing(a); + await s.expectRemoteMissing(b); + await s.expectRemoteContent(c, 'chain'); + await s.expectSingleCommitSince(headBefore); + expect(s.metadata(a), 'no stale metadata at intermediate path A').toBeUndefined(); + expect(s.metadata(b), 'no stale metadata at intermediate path B').toBeUndefined(); + expect(s.metadataSha(c), 'metadata landed at final path').toBeTruthy(); + }); + }); +}); \ No newline at end of file diff --git a/scripts/run-e2e.sh b/scripts/run-e2e.sh index 247502c..7629f83 100755 --- a/scripts/run-e2e.sh +++ b/scripts/run-e2e.sh @@ -36,8 +36,13 @@ scripts/e2e-harness.sh provision set -a; source "$E2E_WORKDIR/e2e.env"; [ -f "$E2E_WORKDIR/e2e.secrets.env" ] && source "$E2E_WORKDIR/e2e.secrets.env"; set +a scripts/e2e-harness.sh seed -# Only this provider's contract suite + the shared SyncManager suite -- -# vitest.e2e.config.ts's `include` matches every e2e/suites/*.e2e.test.ts -# file, and the other two providers' suites would otherwise also try to run -# (and fail on missing credentials) regardless of --provider. -npx vitest run -c vitest.e2e.config.ts "e2e/suites/${provider}.e2e.test.ts" e2e/suites/sync-manager.e2e.test.ts +# Only this provider's contract suite + the shared SyncManager/source-control +# workflow suites -- vitest.e2e.config.ts's `include` matches every +# e2e/suites/*.e2e.test.ts file, and the other two providers' suites would +# otherwise also try to run (and fail on missing credentials) regardless of +# --provider. source-control-flows gates its Extended scenarios to GitHub only +# (and 1000-file stress to E2E_STRESS=1) in-file. +npx vitest run -c vitest.e2e.config.ts \ + "e2e/suites/${provider}.e2e.test.ts" \ + e2e/suites/sync-manager.e2e.test.ts \ + e2e/suites/source-control-flows.e2e.test.ts From 726c54aafdc9fd1d825af799e8131019be92d4ef Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:55:32 +0000 Subject: [PATCH 05/21] test: expand conflict state transition coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 locks the current SyncPlanner conflict contract: modify/modify with a stored baseline IS a conflict (asserted with strengthened side-effect checks — both sides + baseline + HEAD untouched on skip); delete/modify (unrelated push leaves the modified remote intact, metadata not advanced), modify/delete (blind re-create from local), rename with a remotely-edited source (move drops the old-path edit), and no-baseline add/add (local overwrites remote) are NOT conflicts today and are locked as such, so a future change to surface them as conflicts is an intentional, test-updating decision. No production behavior changed. --- e2e/suites/source-control-flows.e2e.test.ts | 129 ++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index a5821b2..f1fa5e5 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -137,4 +137,133 @@ describe('Source Control Flows E2E', () => { expect(s.metadataSha(c), 'metadata landed at final path').toBeTruthy(); }); }); + + // ------------------------------------------------------------------ + // Phase 3 — Conflict state transitions + // + // The current SyncPlanner only surfaces a conflict on a push when both + // sides diverged from a *stored* baseline (modify/modify with a base + // sha). No-baseline add/add, delete-side divergence, and a move whose + // *source* was remotely edited are NOT conflicts today — they resolve to + // local-wins / blind-recreate / move-drops-old-edit. These tests lock + // that current contract (per the agreed scope: no production behavior + // changed to satisfy tests) so a future change to surface those as + // conflicts is an intentional, test-updating decision. The one real + // conflict (modify/modify with baseline) is asserted as a conflict. + // ------------------------------------------------------------------ + describe('conflict state transitions', () => { + it('detects a modify/modify conflict and leaves both sides + baseline untouched on skip', async () => { + const s = scenario(); + const p = path('conflict-modify-modify/a.md'); + await s.baseline(p, 'baseline'); + const baselineMeta = s.metadata(p); + + s.writeLocal(p, 'local edit'); + await s.modifyRemote(p, 'remote edit'); + + fixture.setConflictResolver(() => 'skip'); + const headBefore = await s.head(); + const result = await s.push([p]); + + expect(result.skippedConflicts, describePushResult(result)).toBeGreaterThanOrEqual(1); + expect(result.success, describePushResult(result)).toBe(0); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectRemoteContent(p, 'remote edit'); + expect(await s.readLocal(p)).toBe('local edit'); + expect(s.metadata(p)).toEqual(baselineMeta); + await s.expectNoCommitSince(headBefore); + }); + + it('does not auto-delete a remotely-modified file when its local copy is gone (current push contract)', async () => { + const s = scenario(); + const gone = path('conflict-delete-modify/a.md'); + const other = path('conflict-delete-modify/b.md'); + await s.baseline(gone, 'baseline'); + await s.baseline(other, 'other-baseline'); + const baselineSha = s.metadataSha(gone); + + s.deleteLocal(gone); + await s.modifyRemote(gone, 'remote edit'); + s.writeLocal(other, 'other-modified'); + + const headBefore = await s.head(); + const result = await s.push([other]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + + // pushFiles never propagates a local deletion, so the + // remotely-modified file survives and its baseline metadata is + // not advanced. No conflict is surfaced for delete/modify today. + await s.expectRemoteContent(gone, 'remote edit'); + expect(s.metadataSha(gone), 'metadata not falsely advanced').toBe(baselineSha); + await s.expectRemoteContent(other, 'other-modified'); + await s.expectSingleCommitSince(headBefore); + }); + + it('re-creates a remotely-deleted file from a modified local copy (current push contract)', async () => { + const s = scenario(); + const p = path('conflict-modify-delete/a.md'); + await s.baseline(p, 'baseline'); + const baselineSha = s.metadataSha(p); + + s.writeLocal(p, 'local edit'); + await s.deleteRemoteFile(p); + + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + + // A remote deletion + local modification classifies as + // 'local-only' (push-create): the remote is blindly re-created + // with local content and metadata advances. No conflict today. + await s.expectRemoteContent(p, 'local edit'); + await s.expectSingleCommitSince(headBefore); + expect(s.metadataSha(p), 'metadata advanced to new sha').not.toBe(baselineSha); + expect(s.metadataSha(p)).toBeTruthy(); + }); + + it.skipIf(!isGitHub)('a move whose source was remotely edited proceeds, dropping the old-path edit (current contract)', async () => { + const s = scenario(); + const oldP = path('conflict-rename-modify/a.md'); + const newP = path('conflict-rename-modify/archive/a.md'); + await s.baseline(oldP, 'v1'); + + s.renameLocal(oldP, newP); + await s.manager.trackRename(newP, oldP); + await s.modifyRemote(oldP, 'remote edit on old path'); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(newP)]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + + // planMove only flags a conflict when the DESTINATION is occupied. + // A diverged source (old path remotely edited) is a plain move, so + // the old-path edit is dropped (old path deleted, new path created + // with local content). Locked here as the current contract. + await s.expectRemoteMissing(oldP); + await s.expectRemoteContent(newP, 'v1'); + await s.expectSingleCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('overwrites a remotely-created file with local content on a no-baseline add/add (current contract)', async () => { + const s = scenario(); + const p = path('conflict-add-add/a.md'); + await s.seedRemote(p, 'remote'); + s.writeLocal(p, 'local'); + + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + expect(result.skippedConflicts, describePushResult(result)).toBe(0); + + // A no-baseline two-sided diff downgrades to 'local-modified' on + // push (classifyForOperation), so local overwrites remote with no + // conflict surfaced. Locked here as the current contract. + await s.expectRemoteContent(p, 'local'); + await s.expectSingleCommitSince(headBefore); + }); + }); }); \ No newline at end of file From e4a3ff09ec07d4112fc828a7bd20bb8963c9da99 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:56:46 +0000 Subject: [PATCH 06/21] test: cover conflict resolution workflows Phase 4 verifies the end-to-end resolution paths: keep-local pushes local content over the remote in one commit and advances metadata to the new sha; keep-remote pulls the remote blob into the vault (no remote mutation, no new commit) and updates metadata; skip is retained as a regression lock confirming local, remote, baseline metadata, and HEAD are all untouched. --- e2e/suites/source-control-flows.e2e.test.ts | 71 +++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index f1fa5e5..a623cec 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -266,4 +266,75 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); }); + + // ------------------------------------------------------------------ + // Phase 4 — Conflict resolution workflows + // ------------------------------------------------------------------ + describe('conflict resolution workflows', () => { + it('resolves a modify/modify conflict with keep-local: remote becomes local, metadata advances', async () => { + const s = scenario(); + const p = path('resolve-keep-local/a.md'); + await s.baseline(p, 'baseline'); + s.writeLocal(p, 'local edit'); + await s.modifyRemote(p, 'remote edit'); + + fixture.setConflictResolver(() => 'keep-local'); + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.resolvedConflicts, describePushResult(result)).toBe(1); + expect(result.skippedConflicts, describePushResult(result)).toBe(0); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteContent(p, 'local edit'); + expect(await s.readLocal(p)).toBe('local edit'); + const remote = await s.remoteContent(p); + expect(s.metadataSha(p), 'metadata = new remote sha').toBe(remote?.sha); + await s.expectSingleCommitSince(headBefore); + }); + + it('resolves a modify/modify conflict with keep-remote: local becomes remote, no remote mutation', async () => { + const s = scenario(); + const p = path('resolve-keep-remote/a.md'); + await s.baseline(p, 'baseline'); + s.writeLocal(p, 'local edit'); + await s.modifyRemote(p, 'remote edit'); + + fixture.setConflictResolver(() => 'keep-remote'); + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.failed, describePushResult(result)).toBe(0); + expect(result.resolvedConflicts, describePushResult(result)).toBe(1); + expect(result.skippedConflicts, describePushResult(result)).toBe(0); + + await s.expectRemoteContent(p, 'remote edit'); + expect(await s.readLocal(p)).toBe('remote edit'); + const remote = await s.remoteContent(p); + expect(s.metadataSha(p), 'metadata = remote sha').toBe(remote?.sha); + // keep-remote is a pull, not a push — no new commit on the branch. + await s.expectNoCommitSince(headBefore); + }); + + it('regression: skip leaves local, remote, baseline metadata, and HEAD all untouched', async () => { + const s = scenario(); + const p = path('resolve-skip/a.md'); + await s.baseline(p, 'baseline'); + const baselineMeta = s.metadata(p); + + s.writeLocal(p, 'local edit'); + await s.modifyRemote(p, 'remote edit'); + + fixture.setConflictResolver(() => 'skip'); + const headBefore = await s.head(); + const result = await s.push([p]); + + expect(result.skippedConflicts, describePushResult(result)).toBeGreaterThanOrEqual(1); + expect(result.success, describePushResult(result)).toBe(0); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectRemoteContent(p, 'remote edit'); + expect(await s.readLocal(p)).toBe('local edit'); + expect(s.metadata(p)).toEqual(baselineMeta); + await s.expectNoCommitSince(headBefore); + }); + }); }); \ No newline at end of file From b8f50b244e89ea79326bfa12ce9bcb2c23758903 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:57:55 +0000 Subject: [PATCH 07/21] test: cover mixed batch operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5: create+modify+rename in one commit (GitHub only), a full create+modify+pure-rename+rename-with-modify lifecycle batch in one commit (all providers), and a safe+conflict batch that locks the current non-atomic contract — safe files land in one commit while the conflict is skipped and the remote stays on the remote side. --- e2e/suites/source-control-flows.e2e.test.ts | 94 +++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index a623cec..ab3b898 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -337,4 +337,98 @@ describe('Source Control Flows E2E', () => { await s.expectNoCommitSince(headBefore); }); }); + + // ------------------------------------------------------------------ + // Phase 5 — Mixed batch operations + // ------------------------------------------------------------------ + describe('mixed batch operations', () => { + it.skipIf(!isGitHub)('pushes a create + modify + rename in one commit', async () => { + const s = scenario(); + const create = path('mixed-cmr/create.md'); + const modify = path('mixed-cmr/modify.md'); + const oldMove = path('mixed-cmr/old.md'); + const newMove = path('mixed-cmr/moved.md'); + await s.baseline(modify, 'm-v1'); + await s.baseline(oldMove, 'move-me'); + + s.writeLocal(create, 'create content'); + s.writeLocal(modify, 'm-v2'); + s.renameLocal(oldMove, newMove); + await s.manager.trackRename(newMove, oldMove); + + const headBefore = await s.head(); + const result = await s.push([create, modify, s.tfile(newMove)]); + expect(result.success, describePushResult(result)).toBe(3); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteContent(create, 'create content'); + await s.expectRemoteContent(modify, 'm-v2'); + await s.expectRemoteMissing(oldMove); + await s.expectRemoteContent(newMove, 'move-me'); + await s.expectSingleCommitSince(headBefore); + }); + + it('pushes create + modify + pure rename + rename-with-modify in one commit', async () => { + const s = scenario(); + const create = path('mixed-lifecycle/create.md'); + const modify = path('mixed-lifecycle/modify.md'); + const renameOld = path('mixed-lifecycle/rename-old.md'); + const renameNew = path('mixed-lifecycle/rename-new.md'); + const moveOld = path('mixed-lifecycle/move-old.md'); + const moveNew = path('mixed-lifecycle/move-new.md'); + await s.baseline(modify, 'm-v1'); + await s.baseline(renameOld, 'r-v1'); + await s.baseline(moveOld, 'mv-v1'); + + s.writeLocal(create, 'create content'); + s.writeLocal(modify, 'm-v2'); + s.renameLocal(renameOld, renameNew); + await s.manager.trackRename(renameNew, renameOld); + s.renameLocal(moveOld, moveNew); + s.writeLocal(moveNew, 'mv-v2'); + await s.manager.trackRename(moveNew, moveOld); + + const headBefore = await s.head(); + const result = await s.push([create, modify, s.tfile(renameNew), s.tfile(moveNew)]); + expect(result.success, describePushResult(result)).toBe(4); + expect(result.failed, describePushResult(result)).toBe(0); + + await s.expectRemoteContent(create, 'create content'); + await s.expectRemoteContent(modify, 'm-v2'); + await s.expectRemoteMissing(renameOld); + await s.expectRemoteContent(renameNew, 'r-v1'); + await s.expectRemoteMissing(moveOld); + await s.expectRemoteContent(moveNew, 'mv-v2'); + await s.expectSingleCommitSince(headBefore); + }); + + it('locks the current contract for a safe + conflict batch (safe files commit, conflict skipped)', async () => { + const s = scenario(); + const safe = path('mixed-safe-conflict/a.md'); + const conflict = path('mixed-safe-conflict/b.md'); + const created = path('mixed-safe-conflict/c.md'); + await s.baseline(safe, 'a-v1'); + await s.baseline(conflict, 'b-v1'); + + s.writeLocal(safe, 'a-v2'); + s.writeLocal(conflict, 'b-local'); + await s.modifyRemote(conflict, 'b-remote'); + s.writeLocal(created, 'c-new'); + + fixture.setConflictResolver(() => 'skip'); + const headBefore = await s.head(); + const result = await s.push([safe, conflict, created]); + expect(result.success, describePushResult(result)).toBe(2); + expect(result.failed, describePushResult(result)).toBe(0); + expect(result.conflicts, describePushResult(result)).toBe(1); + expect(result.skippedConflicts, describePushResult(result)).toBe(1); + + // Current contract: safe files land in one commit; the conflict is + // skipped (remote stays 'b-remote'), not atomic. Locked here. + await s.expectRemoteContent(safe, 'a-v2'); + await s.expectRemoteContent(conflict, 'b-remote'); + await s.expectRemoteContent(created, 'c-new'); + await s.expectSingleCommitSince(headBefore); + }); + }); }); \ No newline at end of file From 2e293f3249456c442fba69a46adae8db5c8d1828 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 10:59:14 +0000 Subject: [PATCH 08/21] test: cover source control selection workflows Phase 6 drives the real SourceControlActionService + PushSelectionStore + ChangeRepository over the real SyncManager (via BoundarySyncWorkspace): selected-subset push leaves unselected files untouched (core); subset- then-remaining push yields two separate commits (GitHub only); and a rename yields a path-derived ChangeId so selecting the new path's change pushes the move (GitHub only), locking the current status-model assumption. Add listCommitShas to the scenario helper. --- e2e/suites/source-control-flows.e2e.test.ts | 114 +++++++++++++++++++- e2e/support/source-control-scenarios.ts | 5 + 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index ab3b898..fa579ae 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeAll, vi } from 'vitest'; import { createSyncManagerFixture, describePushResult, type SyncManagerFixture } from '../support/sync-manager-fixture'; -import { SourceControlScenario } from '../support/source-control-scenarios'; +import { SourceControlScenario, change } from '../support/source-control-scenarios'; import { timeouts } from '../config/env'; // Auto-confirm the plan-review + conflict modals so a push can proceed @@ -431,4 +431,116 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); }); + + // ------------------------------------------------------------------ + // Phase 6 — Source Control selection workflows + // + // Drives the real SourceControlActionService + PushSelectionStore + + // ChangeRepository on top of the real SyncManager (via the thin + // BoundarySyncWorkspace), so the ChangeId -> path -> workspace.push + // selection filter is the real production code, not a mock. + // ------------------------------------------------------------------ + describe('selection workflows', () => { + it('pushes only the selected subset, leaving unselected files untouched', async () => { + const s = scenario(); + const a = path('subset/a.md'); + const b = path('subset/b.md'); + const c = path('subset/c.md'); + await s.baseline(a, 'a-v1'); + await s.baseline(b, 'b-v1'); + await s.baseline(c, 'c-v1'); + s.writeLocal(a, 'a-v2'); + s.writeLocal(b, 'b-v2'); + s.writeLocal(c, 'c-v2'); + + const ca = change(a, 'local-modified'); + const cb = change(b, 'local-modified'); + const cc = change(c, 'local-modified'); + const { selection, actionService, operations } = s.selectionStack([ca, cb, cc]); + selection.includeForPush(ca.id); + selection.includeForPush(cc.id); + + const headBefore = await s.head(); + await actionService.push([ca.id, cc.id]); + + expect(operations.get(ca.id)).toBe('success'); + expect(operations.get(cc.id)).toBe('success'); + expect(operations.get(cb.id), 'unselected change stays idle').toBe('idle'); + await s.expectRemoteContent(a, 'a-v2'); + await s.expectRemoteContent(c, 'c-v2'); + await s.expectRemoteContent(b, 'b-v1'); + await s.expectSingleCommitSince(headBefore); + // Current contract: the action service marks operations but does + // not clear selection or refresh the repository, so the selection + // is retained (locked here). + expect(selection.isIncluded(ca.id)).toBe(true); + expect(selection.isIncluded(cc.id)).toBe(true); + }); + + it.skipIf(!isGitHub)('pushes a subset then the remaining subset as two separate commits', async () => { + const s = scenario(); + const a = path('subset-then-rest/a.md'); + const b = path('subset-then-rest/b.md'); + const c = path('subset-then-rest/c.md'); + await s.baseline(a, 'a-v1'); + await s.baseline(b, 'b-v1'); + await s.baseline(c, 'c-v1'); + s.writeLocal(a, 'a-v2'); + s.writeLocal(b, 'b-v2'); + s.writeLocal(c, 'c-v2'); + + const ca = change(a, 'local-modified'); + const cb = change(b, 'local-modified'); + const cc = change(c, 'local-modified'); + const { selection, actionService, operations } = s.selectionStack([ca, cb, cc]); + + const head0 = await s.head(); + selection.includeForPush(ca.id); + selection.includeForPush(cc.id); + await actionService.push([ca.id, cc.id]); + const head1 = await s.head(); + await s.expectSingleCommitSince(head0); + + selection.includeForPush(cb.id); + await actionService.push([cb.id]); + const head2 = await s.head(); + expect(head2, 'second push is a separate commit').not.toBe(head1); + const [, head2Parent] = await s.listCommitShas(2); + expect(head2Parent).toBe(head1); + + expect(operations.get(ca.id)).toBe('success'); + expect(operations.get(cb.id)).toBe('success'); + expect(operations.get(cc.id)).toBe('success'); + await s.expectRemoteContent(a, 'a-v2'); + await s.expectRemoteContent(b, 'b-v2'); + await s.expectRemoteContent(c, 'c-v2'); + }); + + it.skipIf(!isGitHub)('rename yields a path-derived ChangeId; selecting the new id pushes the move', async () => { + const s = scenario(); + const oldP = path('selection-rename/a.md'); + const newP = path('selection-rename/archive/a.md'); + await s.baseline(oldP, 'v1'); + + s.renameLocal(oldP, newP); + await s.manager.trackRename(newP, oldP); + + // Current model: ChangeId is path-derived, so the moved change + // carries a NEW id (the new path) with previousPath set; the old + // path's id is gone. Locking this assumption protects the status + // model against an accidental path->identity regression. + const moved = change(newP, 'moved', oldP); + const { selection, actionService, operations } = s.selectionStack([moved]); + selection.refresh([moved.id]); + selection.includeForPush(moved.id); + + const headBefore = await s.head(); + await actionService.push([moved.id]); + + expect(operations.get(moved.id)).toBe('success'); + await s.expectRemoteMissing(oldP); + await s.expectRemoteContent(newP, 'v1'); + await s.expectSingleCommitSince(headBefore); + }); + }); }); \ No newline at end of file diff --git a/e2e/support/source-control-scenarios.ts b/e2e/support/source-control-scenarios.ts index d2e3d0e..c1edbb4 100644 --- a/e2e/support/source-control-scenarios.ts +++ b/e2e/support/source-control-scenarios.ts @@ -135,6 +135,11 @@ export class SourceControlScenario { return tip!; } + /** Newest-first commit shas on the branch (independent of the service). */ + async listCommitShas(count: number): Promise { + return this.verifier.listCommitShas(this.branch, count); + } + /** Asserts exactly one new commit landed since `headBefore` (the new commit's parent is `headBefore`). */ async expectSingleCommitSince(headBefore: string): Promise { const [headAfter, headAfterParent] = await this.verifier.listCommitShas(this.branch, 2); From 119ba030afe9e2c17ed5695eb940036f7b253725 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 11:00:32 +0000 Subject: [PATCH 09/21] test: cover divergence and idempotency flows Phase 7: remote-ahead pull advances metadata (core); a remote-ahead change and an unrelated local change coexist without cross-contamination (GitHub); a concurrent remote write surfaces as a conflict then reconciles with no lost update (GitHub); an all-unchanged batch reports zero work and zero commits (core); repeating a push makes no second mutation and corrupts no metadata (GitHub); and a skipped conflict can be resolved then re-synced cleanly with no stale operation state (GitHub). --- e2e/suites/source-control-flows.e2e.test.ts | 122 ++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index fa579ae..0227a5d 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -543,4 +543,126 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); }); + + // ------------------------------------------------------------------ + // Phase 7 — Remote divergence + idempotency + // ------------------------------------------------------------------ + describe('divergence and idempotency flows', () => { + it('pulls a remote-ahead update into a synced-baseline local, advancing metadata', async () => { + const s = scenario(); + const p = path('remote-ahead/a.md'); + await s.baseline(p, 'A'); + await s.modifyRemote(p, 'B'); + + await s.pullFile(p); + + expect(await s.readLocal(p)).toBe('B'); + const remote = await s.remoteContent(p); + expect(s.metadataSha(p), 'metadata moves to the remote sha').toBe(remote?.sha); + }); + + it.skipIf(!isGitHub)('a remote-ahead change and an unrelated local change coexist', async () => { + const s = scenario(); + const a = path('coexist/a.md'); + const b = path('coexist/b.md'); + await s.baseline(a, 'A'); + await s.baseline(b, 'B'); + + await s.modifyRemote(a, 'A-remote'); + s.writeLocal(b, 'B-local'); + + await s.pullFile(a); + expect(await s.readLocal(a)).toBe('A-remote'); + // The local change on b survives the pull of a — no cross-contamination. + expect(await s.readLocal(b)).toBe('B-local'); + await s.expectRemoteContent(b, 'B'); + }); + + it.skipIf(!isGitHub)('a concurrent remote write surfaces as a conflict, then reconciles with no lost update', async () => { + const s = scenario(); + const p = path('concurrent/a.md'); + await s.baseline(p, 'v1'); + s.writeLocal(p, 'local-v2'); + await s.modifyRemote(p, 'concurrent-v2'); + + fixture.setConflictResolver(() => 'skip'); + const skipped = await s.push([p]); + expect(skipped.skippedConflicts, describePushResult(skipped)).toBeGreaterThanOrEqual(1); + await s.expectRemoteContent(p, 'concurrent-v2'); + + // Reconcile: accept the concurrent remote, then push a fresh local edit. + fixture.setConflictResolver(() => 'keep-remote'); + await s.push([p]); + expect(await s.readLocal(p)).toBe('concurrent-v2'); + + s.writeLocal(p, 'final'); + const headBefore = await s.head(); + const finalResult = await s.push([p]); + expect(finalResult.success, describePushResult(finalResult)).toBe(1); + await s.expectRemoteContent(p, 'final'); + await s.expectSingleCommitSince(headBefore); + }); + + it('an all-unchanged batch reports no work and creates zero commits', async () => { + const s = scenario(); + const a = path('noop-batch/a.md'); + const b = path('noop-batch/b.md'); + const c = path('noop-batch/c.md'); + await s.baseline(a, 'a'); + await s.baseline(b, 'b'); + await s.baseline(c, 'c'); + + const headBefore = await s.head(); + const result = await s.push([a, b, c]); + expect(result.success, describePushResult(result)).toBe(0); + expect(result.failed, describePushResult(result)).toBe(0); + expect(result.skippedConflicts, describePushResult(result)).toBe(0); + await s.expectNoCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('repeating the same push twice makes no second mutation and corrupts no metadata', async () => { + const s = scenario(); + const p = path('repeat-push/a.md'); + await s.baseline(p, 'v1'); + s.writeLocal(p, 'v2'); + + const headAfterFirst = await s.head(); + const first = await s.push([p]); + expect(first.success, describePushResult(first)).toBe(1); + await s.expectRemoteContent(p, 'v2'); + const shaAfterFirst = s.metadataSha(p); + expect(shaAfterFirst).toBeTruthy(); + + const second = await s.push([p]); + expect(second.success, describePushResult(second)).toBe(0); + expect(second.failed, describePushResult(second)).toBe(0); + await s.expectNoCommitSince(headAfterFirst); + expect(s.metadataSha(p), 'metadata not corrupted by the no-op repeat').toBe(shaAfterFirst); + }); + + it.skipIf(!isGitHub)('re-syncs cleanly after a skipped conflict (no stale operation state)', async () => { + const s = scenario(); + const p = path('retry-after-skip/a.md'); + await s.baseline(p, 'v1'); + s.writeLocal(p, 'local'); + await s.modifyRemote(p, 'remote'); + + fixture.setConflictResolver(() => 'skip'); + const skipped = await s.push([p]); + expect(skipped.skippedConflicts, describePushResult(skipped)).toBeGreaterThanOrEqual(1); + + // Resolve the skipped conflict (keep-remote), then push a fresh edit. + fixture.setConflictResolver(() => 'keep-remote'); + await s.push([p]); + expect(await s.readLocal(p)).toBe('remote'); + + s.writeLocal(p, 'reconciled'); + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.success, describePushResult(result)).toBe(1); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectRemoteContent(p, 'reconciled'); + await s.expectSingleCommitSince(headBefore); + }); + }); }); \ No newline at end of file From f036888b08af2464c3badc2d3a951648d4a354c3 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 11:01:48 +0000 Subject: [PATCH 10/21] test: add path and batch-scale regression cases Phase 8: unicode filename create+modify+rename, spaces-and-symbols create+modify, deeply-nested move+modify (GitHub only); 100-file batch create in one commit and a 100-file mixed modify+create+rename batch in one commit (GitHub only); plus an opt-in 1000-file stress create behind E2E_STRESS=1 (never a required CI check). --- e2e/suites/source-control-flows.e2e.test.ts | 118 +++++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index 0227a5d..88a28a8 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -15,8 +15,9 @@ vi.mock('../../src/ui/BatchConflictResolutionModal'); // Provider matrix: Core scenarios run on every provider; Extended scenarios // (rename chains, unicode, batch-scale, etc.) exercise SyncManager/model // behavior that's provider-agnostic, so they run on GitHub only to keep -// real-API CI fast and stable. +// real-API CI fast and stable. Stress (1000-file) is opt-in via E2E_STRESS=1. const isGitHub = process.env.E2E_PROVIDER === 'github'; +const isStress = process.env.E2E_STRESS === '1'; describe('Source Control Flows E2E', () => { let fixture: SyncManagerFixture; @@ -665,4 +666,119 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); }); + + // ------------------------------------------------------------------ + // Phase 8 — Path edge cases + batch scale + // ------------------------------------------------------------------ + describe('path edge cases and batch scale', () => { + it.skipIf(!isGitHub)('creates, modifies, and renames a unicode-named file', async () => { + const s = scenario(); + const original = path('unicode/筆記/測試文件.md'); + const archived = path('unicode/筆記/已歸檔.md'); + await s.baseline(original, 'unicode-v1'); + + s.writeLocal(original, 'unicode-v2'); + let headBefore = await s.head(); + let result = await s.push([original]); + expect(result.success, describePushResult(result)).toBe(1); + await s.expectRemoteContent(original, 'unicode-v2'); + await s.expectSingleCommitSince(headBefore); + + s.renameLocal(original, archived); + await s.manager.trackRename(archived, original); + headBefore = await s.head(); + result = await s.push([s.tfile(archived)]); + expect(result.success, describePushResult(result)).toBe(1); + await s.expectRemoteMissing(original); + await s.expectRemoteContent(archived, 'unicode-v2'); + await s.expectSingleCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('creates and modifies a file with spaces and symbols', async () => { + const s = scenario(); + const p = path('spaces/folder/my note (draft).md'); + await s.baseline(p, 'draft-v1'); + + s.writeLocal(p, 'draft-v2'); + const headBefore = await s.head(); + const result = await s.push([p]); + expect(result.success, describePushResult(result)).toBe(1); + await s.expectRemoteContent(p, 'draft-v2'); + await s.expectSingleCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('moves and modifies a deeply nested file', async () => { + const s = scenario(); + const oldP = path('deep/a/b/c/d/e/note.md'); + const newP = path('deep/archive/x/y/z/w/note.md'); + await s.baseline(oldP, 'deep-v1'); + + s.renameLocal(oldP, newP); + s.writeLocal(newP, 'deep-v2'); + await s.manager.trackRename(newP, oldP); + + const headBefore = await s.head(); + const result = await s.push([s.tfile(newP)]); + expect(result.success, describePushResult(result)).toBe(1); + await s.expectRemoteMissing(oldP); + await s.expectRemoteContent(newP, 'deep-v2'); + await s.expectSingleCommitSince(headBefore); + }); + + it.skipIf(!isGitHub)('creates 100 files in one commit', async () => { + const s = scenario(); + const paths = Array.from({ length: 100 }, (_, i) => path(`batch-100/${String(i).padStart(3, '0')}.md`)); + for (const p of paths) s.writeLocal(p, `content ${p}`); + + const headBefore = await s.head(); + const result = await s.push(paths); + expect(result.success, describePushResult(result)).toBe(100); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectSingleCommitSince(headBefore); + await s.expectRemoteContent(paths[0]!, `content ${paths[0]}`); + await s.expectRemoteContent(paths[50]!, `content ${paths[50]}`); + await s.expectRemoteContent(paths[99]!, `content ${paths[99]}`); + }); + + it.skipIf(!isGitHub)('pushes a 100-file mixed batch (modify + create + rename) in one commit', async () => { + const s = scenario(); + const modifyPaths = Array.from({ length: 40 }, (_, i) => path(`mixed-100/modify/${i}.md`)); + const createPaths = Array.from({ length: 30 }, (_, i) => path(`mixed-100/create/${i}.md`)); + const renameOld = Array.from({ length: 30 }, (_, i) => path(`mixed-100/rename-old/${i}.md`)); + const renameNew = Array.from({ length: 30 }, (_, i) => path(`mixed-100/rename-new/${i}.md`)); + + for (const p of modifyPaths) await s.baseline(p, 'v1'); + for (const p of renameOld) await s.baseline(p, 'r-v1'); + for (const p of modifyPaths) s.writeLocal(p, 'v2'); + for (const p of createPaths) s.writeLocal(p, 'new'); + for (let i = 0; i < renameOld.length; i++) { + s.renameLocal(renameOld[i]!, renameNew[i]!); + await s.manager.trackRename(renameNew[i]!, renameOld[i]!); + } + + const headBefore = await s.head(); + const all = [...modifyPaths, ...createPaths, ...renameNew.map(p => s.tfile(p))]; + const result = await s.push(all); + expect(result.success, describePushResult(result)).toBe(100); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectSingleCommitSince(headBefore); + + await s.expectRemoteContent(modifyPaths[0]!, 'v2'); + await s.expectRemoteContent(createPaths[0]!, 'new'); + await s.expectRemoteMissing(renameOld[0]!); + await s.expectRemoteContent(renameNew[0]!, 'r-v1'); + }); + + it.skipIf(!isStress || !isGitHub)('stress: creates 1000 files', async () => { + const s = scenario(); + const paths = Array.from({ length: 1000 }, (_, i) => path(`batch-1000/${String(i).padStart(4, '0')}.md`)); + for (const p of paths) s.writeLocal(p, `content ${p}`); + + const result = await s.push(paths); + expect(result.success, describePushResult(result)).toBe(1000); + expect(result.failed, describePushResult(result)).toBe(0); + await s.expectRemoteContent(paths[0]!, `content ${paths[0]}`); + await s.expectRemoteContent(paths[999]!, `content ${paths[999]}`); + }, 300_000); + }); }); \ No newline at end of file From e9f0d28ffc174e028869f6f20a1c72bafc55ca19 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 11:19:50 +0000 Subject: [PATCH 11/21] fix(ci): run E2E suites through shared runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider-e2e job hard-coded the vitest suite list and omitted source-control-flows.e2e.test.ts, so the new suite never ran in CI (a "fake green" — the job passed without exercising the new coverage). Make scripts/e2e-suites.txt the single source of truth: scripts/run-e2e.sh reads it, expands ${provider}, and runs the listed suites; CI now calls scripts/run-e2e.sh --provider (same command local dev uses), collapsing the separate provision/seed/vitest/verify steps into the one retry-wrapped entry point. Adding a shared suite now only requires editing scripts/e2e-suites.txt. Also make the Gitea-disabled state explicit: the gate step emits a notice and a step-summary ("Gitea E2E: disabled — runner Docker networking") so a green gitea leg is never mistaken for three-provider coverage. --- .github/workflows/ci.yml | 56 +++++++++++++++++----------------------- scripts/e2e-suites.txt | 10 +++++++ scripts/run-e2e.sh | 34 ++++++++++++++---------- 3 files changed, 55 insertions(+), 45 deletions(-) create mode 100644 scripts/e2e-suites.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 049dbd2..ae1202a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,6 +156,15 @@ jobs: # fork PRs get no E2E coverage at all. if [ "${{ matrix.provider }}" = "gitea" ]; then run=false + # Make the disabled state explicit in the run log + summary so a + # green "E2E / gitea" job is never mistaken for "Gitea E2E passed". + echo "::notice::Gitea E2E is disabled in CI (runner Docker networking — see TODO below). Suite/harness code passes locally; re-enable by removing this block." + { + echo "### Gitea E2E: disabled" + echo "Reason: runner Docker networking — container provisioning against this runner fleet needs investigation (bridge-IP reachability, health-check timing)." + echo "Suite/harness code is untouched and passes locally (\`npm run test:e2e -- --provider gitea\`). The Gitea infrastructure fix is tracked separately; do not infer three-provider coverage from a green gitea leg." + echo "Re-enable by removing the gitea block in the \"Determine whether this provider leg should run\" step." + } >> "$GITHUB_STEP_SUMMARY" fi if [ "${{ github.event_name }}" = "pull_request" ] \ && [ "${{ matrix.provider }}" != "gitea" ] \ @@ -184,29 +193,24 @@ jobs: - run: npm ci --ignore-scripts if: steps.gate.outputs.run == 'true' - # Arrange/Assert/cleanup are Shell + Git (scripts/e2e-harness.sh); Act - # stays production TypeScript (npx vitest). E2E_WORKDIR/E2E_PR_NUMBER/ - # E2E_SOURCE_BRANCH are set once at job level (see the job `env:` - # above) so all steps below share the same run state/identity. - - name: Provision isolated branch/container - if: steps.gate.outputs.run == 'true' - env: - E2E_PROVIDER: ${{ matrix.provider }} - run: scripts/e2e-harness.sh provision - - - name: Seed baseline fixture - if: steps.gate.outputs.run == 'true' - env: - E2E_PROVIDER: ${{ matrix.provider }} - run: scripts/e2e-harness.sh seed - + # One entry point for the whole real-provider E2E flow: scripts/run-e2e.sh + # provisions the isolated branch/container, seeds the baseline fixture, + # runs the suites listed in scripts/e2e-suites.txt (the single source of + # truth — CI and local run the same command, so the suite list is never + # duplicated here), and cleans up via its EXIT trap. New suites are added + # in scripts/e2e-suites.txt only; scripts/check-e2e-suite-registration.mjs + # (wired into `npm run lint`) fails CI if a suite file isn't registered. + # E2E_WORKDIR is set by the "Compute run-scoped workdir" step above; the + # job `env:` supplies the provider secrets and run identity + # (E2E_PR_NUMBER/E2E_SOURCE_BRANCH) that run-e2e.sh/e2e-harness.sh consume. + # # Retried (not just run once): observed failures against the real # providers include transient runner-network blips unrelated to the # suite/product code (e.g. a bare `getaddrinfo ENOTFOUND gitlab.com` # mid-test on 2026-08-14, run 31770197590) that a same-attempt rerun - # simply doesn't reproduce. Safe to retry the whole step from scratch: - # each suite's `runId`/branch paths are randomized per vitest process - # (see e.g. e2e/suites/sync-manager.e2e.test.ts), so a failed + # simply doesn't reproduce. Safe to retry from scratch: run-e2e.sh + # re-provisions a fresh isolated branch each attempt and every suite's + # runId/branch paths are randomized per vitest process, so a failed # attempt's partial remote state never collides with the retry -- a # genuine product/test bug still fails identically every attempt and # exhausts the retries. @@ -219,19 +223,7 @@ jobs: timeout_minutes: 15 max_attempts: 3 retry_wait_seconds: 15 - command: | - set -a - # shellcheck disable=SC1091 - source "$E2E_WORKDIR/e2e.env" - [ -f "$E2E_WORKDIR/e2e.secrets.env" ] && source "$E2E_WORKDIR/e2e.secrets.env" - set +a - npx vitest run -c vitest.e2e.config.ts "e2e/suites/${{ matrix.provider }}.e2e.test.ts" e2e/suites/sync-manager.e2e.test.ts - - - name: Independent verification - if: steps.gate.outputs.run == 'true' - env: - E2E_PROVIDER: ${{ matrix.provider }} - run: scripts/e2e-harness.sh verify + command: scripts/run-e2e.sh --provider "${{ matrix.provider }}" # `if: always()` -- cleanup is best-effort, never a prerequisite for # the next run (see scripts/e2e-harness.sh's cmd_cleanup and diff --git a/scripts/e2e-suites.txt b/scripts/e2e-suites.txt new file mode 100644 index 0000000..b6f588f --- /dev/null +++ b/scripts/e2e-suites.txt @@ -0,0 +1,10 @@ +# E2E suite manifest — the single source of truth for which vitest suites run +# per provider. scripts/run-e2e.sh reads this, expands ${provider}, and runs +# them; CI calls run-e2e.sh so this list is never duplicated in the workflow. +# scripts/check-e2e-suite-registration.mjs enforces that every +# e2e/suites/*.e2e.test.ts is registered here: the ${provider} line covers the +# provider-specific suites (github/gitlab/gitea); every other shared suite +# must be listed explicitly, or CI fails. +e2e/suites/${provider}.e2e.test.ts +e2e/suites/sync-manager.e2e.test.ts +e2e/suites/source-control-flows.e2e.test.ts \ No newline at end of file diff --git a/scripts/run-e2e.sh b/scripts/run-e2e.sh index 7629f83..0c85aab 100755 --- a/scripts/run-e2e.sh +++ b/scripts/run-e2e.sh @@ -1,9 +1,11 @@ #!/usr/bin/env bash # Thin local-dev orchestration around scripts/e2e-harness.sh: provision the # isolated branch/container, seed a baseline fixture, run the provider's -# vitest suite + the SyncManager suite, then clean up (even on failure). CI -# drives the same four steps directly from .github/workflows/ci.yml instead, -# so each shows up as its own job step. +# vitest suites, then clean up (even on failure). CI calls this same script +# (see .github/workflows/ci.yml), so the suite list lives in exactly one +# place: scripts/e2e-suites.txt. Add a new shared suite there and both local +# and CI pick it up; scripts/check-e2e-suite-registration.mjs fails CI if a +# suite file isn't registered. set -euo pipefail provider="" @@ -36,13 +38,19 @@ scripts/e2e-harness.sh provision set -a; source "$E2E_WORKDIR/e2e.env"; [ -f "$E2E_WORKDIR/e2e.secrets.env" ] && source "$E2E_WORKDIR/e2e.secrets.env"; set +a scripts/e2e-harness.sh seed -# Only this provider's contract suite + the shared SyncManager/source-control -# workflow suites -- vitest.e2e.config.ts's `include` matches every -# e2e/suites/*.e2e.test.ts file, and the other two providers' suites would -# otherwise also try to run (and fail on missing credentials) regardless of -# --provider. source-control-flows gates its Extended scenarios to GitHub only -# (and 1000-file stress to E2E_STRESS=1) in-file. -npx vitest run -c vitest.e2e.config.ts \ - "e2e/suites/${provider}.e2e.test.ts" \ - e2e/suites/sync-manager.e2e.test.ts \ - e2e/suites/source-control-flows.e2e.test.ts + +# Suite manifest: scripts/e2e-suites.txt (single source of truth). ${provider} +# expands to the active provider's contract suite; the rest are shared suites. +# `|| [ -n "$line" ]` keeps the last line even without a trailing newline. +SUITES=() +while IFS= read -r line || [ -n "$line" ]; do + case "$line" in ''|\#*) continue;; esac + SUITES+=("$(printf '%s' "$line" | sed "s/\${provider}/$provider/g")") +done < scripts/e2e-suites.txt + +# vitest.e2e.config.ts's `include` matches every e2e/suites/*.e2e.test.ts, so +# the other two providers' suites would also try to run (and fail on missing +# credentials) if not explicitly limited to this list. source-control-flows +# gates its Extended scenarios to GitHub only (and 1000-file stress to +# E2E_STRESS=1) in-file. +npx vitest run -c vitest.e2e.config.ts "${SUITES[@]}" From 220f2d520c3416d85a8c2c3873b7781da8df8b97 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 11:20:08 +0000 Subject: [PATCH 12/21] test(ci): guard E2E suite registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add scripts/check-e2e-suite-registration.mjs and wire it into `npm run lint` (both the husky pre-commit hook and CI). It fails when an e2e/suites/*.e2e.test.ts file exists but isn't registered in scripts/e2e-suites.txt — provider-specific suites (github/gitlab/gitea) are covered by the ${provider} line; every other shared suite must be listed explicitly. So adding a suite without wiring CI now breaks the build instead of silently passing (the original fake-green failure mode). --- package.json | 2 +- scripts/check-e2e-suite-registration.mjs | 101 +++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 scripts/check-e2e-suite-registration.mjs diff --git a/package.json b/package.json index e460210..3ad313d 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "build": "tsc -noEmit -skipLibCheck && npm run typecheck:compat && node esbuild.config.mjs production", "typecheck:compat": "node scripts/typecheck-compat.mjs", "version": "node version-bump.mjs && git add manifest.json versions.json", - "lint": "eslint .", + "lint": "eslint . && node scripts/check-e2e-suite-registration.mjs", "test": "vitest run", "deploy": "npm run build && mkdir -p ~/Obsidian/MyPKM/.obsidian/plugins/git-file-sync && cp main.js manifest.json styles.css ~/Obsidian/MyPKM/.obsidian/plugins/git-file-sync/", "test:ui": "vitest --ui", diff --git a/scripts/check-e2e-suite-registration.mjs b/scripts/check-e2e-suite-registration.mjs new file mode 100644 index 0000000..ae43700 --- /dev/null +++ b/scripts/check-e2e-suite-registration.mjs @@ -0,0 +1,101 @@ +#!/usr/bin/env node +/* + * Guards against the "fake green" failure mode: a new e2e/suites/*.e2e.test.ts + * file that isn't wired into CI. The suite manifest has one source of truth — + * scripts/e2e-suites.txt, consumed by scripts/run-e2e.sh (which CI calls). This + * check fails (non-zero) when a suite file exists on disk but isn't registered + * there, so adding a suite without registering it breaks CI instead of + * silently passing. + * + * Rules: + * - Provider-specific suites (github/gitlab/gitea.e2e.test.ts) are covered by + * a manifest line containing ${provider}; they must NOT also need an + * explicit static line. + * - Every other e2e/suites/*.e2e.test.ts is a shared suite and MUST be listed + * explicitly in the manifest. + * - Every static manifest line must point to a file that exists (catches + * typos / deleted suites). + * + * Wired into `npm run lint` so both the husky pre-commit hook and CI enforce it. + */ +import { readFileSync, readdirSync, existsSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = dirname(fileURLToPath(import.meta.url)); +const root = resolve(here, '..'); +const manifestPath = join(root, 'scripts', 'e2e-suites.txt'); +const suitesDir = join(root, 'e2e', 'suites'); + +const PROVIDERS = ['github', 'gitlab', 'gitea']; + +function readManifest() { + const raw = readFileSync(manifestPath, 'utf-8'); + const lines = raw.split('\n').map(l => { + const hash = l.indexOf('#'); + return (hash >= 0 ? l.slice(0, hash) : l).trim(); + }); + const staticSuites = []; + let hasDynamic = false; + for (const line of lines) { + if (!line) continue; + if (line.includes('${provider}')) { + hasDynamic = true; + continue; + } + staticSuites.push(line); + } + return { staticSuites, hasDynamic }; +} + +function listSuiteFiles() { + return readdirSync(suitesDir) + .filter(f => f.endsWith('.e2e.test.ts')) + .sort(); +} + +function fail(message) { + console.error(`check-e2e-suite-registration: ${message}`); + process.exit(1); +} + +const { staticSuites, hasDynamic } = readManifest(); +const suiteFiles = listSuiteFiles(); +const staticSet = new Set(staticSuites.map(s => s.replace(/^\.\//, ''))); +const providerSuites = new Set(PROVIDERS.map(p => `e2e/suites/${p}.e2e.test.ts`)); + +// 1. Every static manifest line must reference an existing file. +for (const entry of staticSuites) { + const rel = entry.replace(/^\.\//, ''); + if (!existsSync(join(root, rel))) { + fail(`manifest "${entry}" does not match any existing file under the repo root.`); + } +} + +// 2. A ${provider} line must expand to all three provider contract suites. +if (!hasDynamic) { + fail('manifest is missing a ${provider} line — the provider-specific suites (github/gitlab/gitea) would not run.'); +} + +// 3. Every suite file on disk must be registered. +const unregistered = []; +for (const file of suiteFiles) { + const rel = `e2e/suites/${file}`; + if (providerSuites.has(rel)) { + if (!hasDynamic) unregistered.push(`${rel} (needs a \${provider} manifest line)`); + continue; + } + if (!staticSet.has(rel)) { + unregistered.push(`${rel} (add it to scripts/e2e-suites.txt)`); + } +} + +if (unregistered.length > 0) { + fail( + `unregistered suite file(s):\n ${unregistered.join('\n ')}\n` + + `Every e2e/suites/*.e2e.test.ts must be listed in scripts/e2e-suites.txt ` + + `(provider-specific suites via the \${provider} line) or CI will not run them.`, + ); +} + +console.log(`check-e2e-suite-registration: OK — ${suiteFiles.length} suite file(s), ${staticSuites.length} static + ${hasDynamic ? '1 dynamic' : '0 dynamic'} manifest line(s).`); \ No newline at end of file From c2bfeb07dd7bf9500cb28ddb48e9f76dfb16985a Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 11:50:36 +0000 Subject: [PATCH 13/21] fix(ci): fold E2E suite registration check into run-e2e.sh Replace the standalone check-e2e-suite-registration.mjs (wired into `npm run lint`) with forward/reverse checks inside scripts/run-e2e.sh itself, so suite manifest validation lives in the same script CI already calls instead of a separate Node checker. Also harden GitVerifier.git() to surface stderr on unexpected git failures while keeping expected missing-path lookups silent. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 4 +- package.json | 2 +- scripts/check-e2e-suite-registration.mjs | 101 ----------------------- scripts/e2e-harness.sh | 19 ++++- scripts/e2e-suites.txt | 4 +- scripts/run-e2e.sh | 62 +++++++++++++- 6 files changed, 81 insertions(+), 111 deletions(-) delete mode 100644 scripts/check-e2e-suite-registration.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae1202a..bd6a6e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,8 +198,8 @@ jobs: # runs the suites listed in scripts/e2e-suites.txt (the single source of # truth — CI and local run the same command, so the suite list is never # duplicated here), and cleans up via its EXIT trap. New suites are added - # in scripts/e2e-suites.txt only; scripts/check-e2e-suite-registration.mjs - # (wired into `npm run lint`) fails CI if a suite file isn't registered. + # in scripts/e2e-suites.txt only; run-e2e.sh's own forward/reverse checks + # fail the run if a suite file isn't registered (or vice versa). # E2E_WORKDIR is set by the "Compute run-scoped workdir" step above; the # job `env:` supplies the provider secrets and run identity # (E2E_PR_NUMBER/E2E_SOURCE_BRANCH) that run-e2e.sh/e2e-harness.sh consume. diff --git a/package.json b/package.json index 3ad313d..e460210 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "build": "tsc -noEmit -skipLibCheck && npm run typecheck:compat && node esbuild.config.mjs production", "typecheck:compat": "node scripts/typecheck-compat.mjs", "version": "node version-bump.mjs && git add manifest.json versions.json", - "lint": "eslint . && node scripts/check-e2e-suite-registration.mjs", + "lint": "eslint .", "test": "vitest run", "deploy": "npm run build && mkdir -p ~/Obsidian/MyPKM/.obsidian/plugins/git-file-sync && cp main.js manifest.json styles.css ~/Obsidian/MyPKM/.obsidian/plugins/git-file-sync/", "test:ui": "vitest --ui", diff --git a/scripts/check-e2e-suite-registration.mjs b/scripts/check-e2e-suite-registration.mjs deleted file mode 100644 index ae43700..0000000 --- a/scripts/check-e2e-suite-registration.mjs +++ /dev/null @@ -1,101 +0,0 @@ -#!/usr/bin/env node -/* - * Guards against the "fake green" failure mode: a new e2e/suites/*.e2e.test.ts - * file that isn't wired into CI. The suite manifest has one source of truth — - * scripts/e2e-suites.txt, consumed by scripts/run-e2e.sh (which CI calls). This - * check fails (non-zero) when a suite file exists on disk but isn't registered - * there, so adding a suite without registering it breaks CI instead of - * silently passing. - * - * Rules: - * - Provider-specific suites (github/gitlab/gitea.e2e.test.ts) are covered by - * a manifest line containing ${provider}; they must NOT also need an - * explicit static line. - * - Every other e2e/suites/*.e2e.test.ts is a shared suite and MUST be listed - * explicitly in the manifest. - * - Every static manifest line must point to a file that exists (catches - * typos / deleted suites). - * - * Wired into `npm run lint` so both the husky pre-commit hook and CI enforce it. - */ -import { readFileSync, readdirSync, existsSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const here = dirname(fileURLToPath(import.meta.url)); -const root = resolve(here, '..'); -const manifestPath = join(root, 'scripts', 'e2e-suites.txt'); -const suitesDir = join(root, 'e2e', 'suites'); - -const PROVIDERS = ['github', 'gitlab', 'gitea']; - -function readManifest() { - const raw = readFileSync(manifestPath, 'utf-8'); - const lines = raw.split('\n').map(l => { - const hash = l.indexOf('#'); - return (hash >= 0 ? l.slice(0, hash) : l).trim(); - }); - const staticSuites = []; - let hasDynamic = false; - for (const line of lines) { - if (!line) continue; - if (line.includes('${provider}')) { - hasDynamic = true; - continue; - } - staticSuites.push(line); - } - return { staticSuites, hasDynamic }; -} - -function listSuiteFiles() { - return readdirSync(suitesDir) - .filter(f => f.endsWith('.e2e.test.ts')) - .sort(); -} - -function fail(message) { - console.error(`check-e2e-suite-registration: ${message}`); - process.exit(1); -} - -const { staticSuites, hasDynamic } = readManifest(); -const suiteFiles = listSuiteFiles(); -const staticSet = new Set(staticSuites.map(s => s.replace(/^\.\//, ''))); -const providerSuites = new Set(PROVIDERS.map(p => `e2e/suites/${p}.e2e.test.ts`)); - -// 1. Every static manifest line must reference an existing file. -for (const entry of staticSuites) { - const rel = entry.replace(/^\.\//, ''); - if (!existsSync(join(root, rel))) { - fail(`manifest "${entry}" does not match any existing file under the repo root.`); - } -} - -// 2. A ${provider} line must expand to all three provider contract suites. -if (!hasDynamic) { - fail('manifest is missing a ${provider} line — the provider-specific suites (github/gitlab/gitea) would not run.'); -} - -// 3. Every suite file on disk must be registered. -const unregistered = []; -for (const file of suiteFiles) { - const rel = `e2e/suites/${file}`; - if (providerSuites.has(rel)) { - if (!hasDynamic) unregistered.push(`${rel} (needs a \${provider} manifest line)`); - continue; - } - if (!staticSet.has(rel)) { - unregistered.push(`${rel} (add it to scripts/e2e-suites.txt)`); - } -} - -if (unregistered.length > 0) { - fail( - `unregistered suite file(s):\n ${unregistered.join('\n ')}\n` + - `Every e2e/suites/*.e2e.test.ts must be listed in scripts/e2e-suites.txt ` + - `(provider-specific suites via the \${provider} line) or CI will not run them.`, - ); -} - -console.log(`check-e2e-suite-registration: OK — ${suiteFiles.length} suite file(s), ${staticSuites.length} static + ${hasDynamic ? '1 dynamic' : '0 dynamic'} manifest line(s).`); \ No newline at end of file diff --git a/scripts/e2e-harness.sh b/scripts/e2e-harness.sh index 46464c4..c19ca5c 100755 --- a/scripts/e2e-harness.sh +++ b/scripts/e2e-harness.sh @@ -314,7 +314,24 @@ export class GitVerifier { constructor(private readonly repoDir: string = ${repo_dir@Q}) {} private git(args: string[]): string { - return execFileSync('git', ['-C', this.repoDir, ...args], { encoding: 'utf-8' }); + try { + return execFileSync('git', ['-C', this.repoDir, ...args], { + encoding: 'utf-8', + // Pipe stderr so an *expected* missing path (getFile's + // try/catch -> null) stays silent instead of spamming the log + // with "fatal: path does not exist". A genuine, unexpected git + // failure still surfaces: callers without their own try/catch + // re-throw below with the captured stderr attached. + stdio: ['pipe', 'pipe', 'pipe'], + }); + } catch (error) { + const stderr = error && typeof error === 'object' && 'stderr' in error + ? String((error as { stderr: unknown }).stderr).trim() + : ''; + throw new Error( + \`git \${args.join(' ')} failed\` + (stderr ? \`:\\n\${stderr}\` : ''), + ); + } } private fetch(ref: string): void { diff --git a/scripts/e2e-suites.txt b/scripts/e2e-suites.txt index b6f588f..6ac6a1a 100644 --- a/scripts/e2e-suites.txt +++ b/scripts/e2e-suites.txt @@ -1,10 +1,10 @@ # E2E suite manifest — the single source of truth for which vitest suites run # per provider. scripts/run-e2e.sh reads this, expands ${provider}, and runs # them; CI calls run-e2e.sh so this list is never duplicated in the workflow. -# scripts/check-e2e-suite-registration.mjs enforces that every +# scripts/run-e2e.sh's own forward/reverse checks enforce that every # e2e/suites/*.e2e.test.ts is registered here: the ${provider} line covers the # provider-specific suites (github/gitlab/gitea); every other shared suite -# must be listed explicitly, or CI fails. +# must be listed explicitly, or the run fails. e2e/suites/${provider}.e2e.test.ts e2e/suites/sync-manager.e2e.test.ts e2e/suites/source-control-flows.e2e.test.ts \ No newline at end of file diff --git a/scripts/run-e2e.sh b/scripts/run-e2e.sh index 0c85aab..1da3be1 100755 --- a/scripts/run-e2e.sh +++ b/scripts/run-e2e.sh @@ -4,8 +4,8 @@ # vitest suites, then clean up (even on failure). CI calls this same script # (see .github/workflows/ci.yml), so the suite list lives in exactly one # place: scripts/e2e-suites.txt. Add a new shared suite there and both local -# and CI pick it up; scripts/check-e2e-suite-registration.mjs fails CI if a -# suite file isn't registered. +# and CI pick it up; this script's own forward/reverse checks below fail the +# run if a suite file isn't registered (or a manifest entry doesn't exist). set -euo pipefail provider="" @@ -42,12 +42,66 @@ scripts/e2e-harness.sh seed # Suite manifest: scripts/e2e-suites.txt (single source of truth). ${provider} # expands to the active provider's contract suite; the rest are shared suites. # `|| [ -n "$line" ]` keeps the last line even without a trailing newline. -SUITES=() +PROVIDERS=(github gitlab gitea) +manifest_has_dynamic=0 +SHARED_SUITES=() while IFS= read -r line || [ -n "$line" ]; do case "$line" in ''|\#*) continue;; esac - SUITES+=("$(printf '%s' "$line" | sed "s/\${provider}/$provider/g")") + if [[ "$line" == *'${provider}'* ]]; then + manifest_has_dynamic=1 + continue + fi + SHARED_SUITES+=("$line") done < scripts/e2e-suites.txt +if [ "$manifest_has_dynamic" -ne 1 ]; then + echo "scripts/e2e-suites.txt is missing a \${provider} line -- provider-specific suites (github/gitlab/gitea) would not run." >&2 + exit 1 +fi + +SUITES=("e2e/suites/${provider}.e2e.test.ts" "${SHARED_SUITES[@]}") + +# Forward check: every manifest entry (after ${provider} expansion) must +# exist on disk -- catches a typo'd or deleted suite path in the manifest. +for suite in "${SUITES[@]}"; do + if [[ ! -f "$suite" ]]; then + echo "E2E suite not found: $suite" >&2 + exit 1 + fi +done + +# Reverse check: every e2e/suites/*.e2e.test.ts file on disk must be either a +# known provider suite (github/gitlab/gitea -- covered by the ${provider} +# line regardless of which provider this run targets) or a shared suite +# explicitly registered in the manifest. Catches a new suite file added +# without wiring it into scripts/e2e-suites.txt, which would otherwise pass +# CI without ever running (the exact "fake green" this guards against). +is_shared_suite() { + local candidate="$1" s + for s in "${SHARED_SUITES[@]}"; do + [[ "$s" == "$candidate" ]] && return 0 + done + return 1 +} +unregistered=() +for file in e2e/suites/*.e2e.test.ts; do + [ -e "$file" ] || continue + base="$(basename "$file" .e2e.test.ts)" + is_known_provider=0 + for p in "${PROVIDERS[@]}"; do + [ "$base" = "$p" ] && is_known_provider=1 && break + done + [ "$is_known_provider" -eq 1 ] && continue + is_shared_suite "$file" || unregistered+=("$file") +done +if [ "${#unregistered[@]}" -gt 0 ]; then + echo "Unregistered E2E suite file(s) -- add to scripts/e2e-suites.txt:" >&2 + printf ' %s\n' "${unregistered[@]}" >&2 + exit 1 +fi + +echo "[run-e2e] running suites: ${SUITES[*]}" >&2 + # vitest.e2e.config.ts's `include` matches every e2e/suites/*.e2e.test.ts, so # the other two providers' suites would also try to run (and fail on missing # credentials) if not explicitly limited to this list. source-control-flows From 54e3fb75e82c35a9ab2abdafe89caec4d89d9302 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 12:04:00 +0000 Subject: [PATCH 14/21] fix(test): show per-test progress in real-provider E2E CI logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vitest's default reporter only prints once a whole file finishes, and these suites do real network round trips per test — in CI that reads as a silent hang. Switch to the verbose reporter so each test prints as it completes. Co-Authored-By: Claude Sonnet 5 --- vitest.e2e.config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vitest.e2e.config.ts b/vitest.e2e.config.ts index 284e929..0d618aa 100644 --- a/vitest.e2e.config.ts +++ b/vitest.e2e.config.ts @@ -25,6 +25,11 @@ export default defineConfig({ exclude: ['**/node_modules/**', '**/.claude/**'], testTimeout: 120_000, hookTimeout: 120_000, + // Real-provider suites run network round trips per test with nothing + // printed until a whole file finishes under the default reporter — + // in CI that reads as a hang. verbose prints each test as it + // completes, so progress is visible while it's still running. + reporters: ['verbose'], // Provisioning spins up one container per provider; running suites in // parallel workers would multiply that for no benefit at this scale. fileParallelism: false, From 039588feeef4d2956739534b13588404fbb984fd Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 12:12:03 +0000 Subject: [PATCH 15/21] fix(test): capture post-push head before asserting no-op repeat push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit headAfterFirst was captured before the first push instead of after, so expectNoCommitSince compared against the pre-push head — failing on the commit the first push itself legitimately created, not on any duplicate mutation from the second push. Co-Authored-By: Claude Sonnet 5 --- e2e/suites/source-control-flows.e2e.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index 88a28a8..5403dd8 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -627,12 +627,12 @@ describe('Source Control Flows E2E', () => { await s.baseline(p, 'v1'); s.writeLocal(p, 'v2'); - const headAfterFirst = await s.head(); const first = await s.push([p]); expect(first.success, describePushResult(first)).toBe(1); await s.expectRemoteContent(p, 'v2'); const shaAfterFirst = s.metadataSha(p); expect(shaAfterFirst).toBeTruthy(); + const headAfterFirst = await s.head(); const second = await s.push([p]); expect(second.success, describePushResult(second)).toBe(0); From d6cdc36049ee7d728e838e7b339b88292149e9d4 Mon Sep 17 00:00:00 2001 From: ClaudiaFang Date: Sat, 22 Aug 2026 20:18:57 +0800 Subject: [PATCH 16/21] fix(settings): keep release history accessible after dismiss --- src/settings-implementation.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/settings-implementation.ts b/src/settings-implementation.ts index 1030495..07edc2a 100644 --- a/src/settings-implementation.ts +++ b/src/settings-implementation.ts @@ -2,6 +2,7 @@ import {App, PluginSettingTab, Setting, Notice, TextComponent} from 'obsidian'; import GitLabFilesPush, { type ConnectionStatus } from "./main"; import {FolderSuggest} from "./ui/FolderSuggest"; import {RemoteFolderSuggest} from "./ui/RemoteFolderSuggest"; +import {WhatsNewModal} from "./ui/WhatsNewModal"; import { t, setLanguageOverride, type LanguageSetting } from "./i18n"; import { CHANGELOG, entryText } from "./changelog"; @@ -172,10 +173,8 @@ export class GitLabSyncSettingTab extends PluginSettingTab { } // Persistent (until dismissed) banner surfacing the current version's notable - // highlights right at the top of the settings tab, so users who dismissed or - // never saw the WhatsNewModal (see main.ts) can still find them. Separate - // from `lastSeenVersion` — that gate controls the once-per-upgrade modal, - // this one just tracks whether the banner itself was dismissed. + // highlights right at the top of the settings tab. Dismissing this only hides + // the attention banner; release history remains available from Settings. private renderWhatsNewBanner(containerEl: HTMLElement): void { const currentVersion = this.plugin.manifest.version; if (this.plugin.settings.bannerDismissedVersion === currentVersion) return; @@ -206,6 +205,17 @@ export class GitLabSyncSettingTab extends PluginSettingTab { }); } + private renderReleaseHistorySetting(containerEl: HTMLElement): void { + new Setting(containerEl) + .setName(t('settings.releaseHistory.name')) + .setDesc(t('settings.releaseHistory.desc')) + .addButton(button => button + .setButtonText(t('settings.releaseHistory.button')) + .onClick(() => { + new WhatsNewModal(this.app, CHANGELOG).open(); + })); + } + // Rebuilding the whole settings tab (renderSettings) to refresh the badge // would empty and recreate every field, stealing focus mid-typing. The // badge element is instead created once per renderSettings pass and @@ -249,6 +259,7 @@ export class GitLabSyncSettingTab extends PluginSettingTab { containerEl.empty(); this.renderWhatsNewBanner(containerEl); + this.renderReleaseHistorySetting(containerEl); this.renderConnectionStatus(containerEl); new Setting(containerEl) @@ -490,7 +501,7 @@ export class GitLabSyncSettingTab extends PluginSettingTab { .setPlaceholder('https://gitea.example.com') .setValue(this.plugin.settings.giteaBaseUrl) .onChange((value) => { - this.plugin.settings.giteaBaseUrl = value; + this.plugin.settings.giteaBaseUrl = value || 'https://gitea.example.com'; void this.plugin.saveSettings(); this.plugin.initializeGitService(); this.scheduleConnectionTest(); From b9a90b29a31b82254b6b4e9646ecec29db53a361 Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 12:18:49 +0000 Subject: [PATCH 17/21] perf(test): memoize remote reads in source-control-flows scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each remote-read verifier call (getFile/fileMissing/listCommitShas) does its own `git fetch origin ` even when nothing has mutated the remote since the last read in the same test — most tests do 3-5 such reads per push. Cache them in SourceControlScenario, invalidated on any call to manager.pushFiles/pullFile or service.pushFile/deleteFile. The invalidation hooks onto the manager/service instances themselves (via a thin Proxy), not this class's own push()/baseline() wrappers, so it stays correct even for mutations this class doesn't mediate directly — e.g. the selection stack's `actionService.push()`, which calls manager.pushFiles through BoundarySyncWorkspace. Scoped to source-control-flows.e2e.test.ts only (the sole consumer of SourceControlScenario); the shared GitVerifier and the other real-provider suites are untouched. Co-Authored-By: Claude Sonnet 5 --- e2e/support/source-control-scenarios.ts | 64 +++++++++++++++++++++---- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/e2e/support/source-control-scenarios.ts b/e2e/support/source-control-scenarios.ts index c1edbb4..72c263b 100644 --- a/e2e/support/source-control-scenarios.ts +++ b/e2e/support/source-control-scenarios.ts @@ -37,16 +37,37 @@ export class SourceControlScenario { private readonly service: GitServiceInterface; private readonly verifier: GitVerifierType; private readonly branch: string; + /** + * Memoizes remote reads (each of which is a real `git fetch` round trip) + * between remote mutations. Invalidated by `invalidatingProxy` below + * whenever `manager.pushFiles`/`pullFile` or `service.pushFile`/ + * `deleteFile` is called on the wrapped instances this scenario hands + * out — including indirectly, e.g. via the selection stack's + * `actionService.push`, which calls `manager.pushFiles` through + * `BoundarySyncWorkspace` rather than through this class's own `push()`. + * Wrapping the instances themselves (instead of only this class's + * wrapper methods) is what makes that indirect path safe to cache too. + */ + private readonly remoteCache = new Map(); constructor(fixture: SyncManagerFixture) { this.vault = fixture.createVault(); this.settings = fixture.makeSettings(); - this.manager = fixture.newManager(this.vault, this.settings); - this.service = fixture.service; + const invalidate = (): void => this.remoteCache.clear(); + this.manager = invalidatingProxy(fixture.newManager(this.vault, this.settings), ['pushFiles', 'pullFile'], invalidate); + this.service = invalidatingProxy(fixture.service, ['pushFile', 'deleteFile'], invalidate); this.verifier = fixture.verifier; this.branch = fixture.branch; } + /** Runs `fn` once and memoizes it under `key` until the next remote mutation. */ + private async cachedRemote(key: string, fn: () => Promise): Promise { + if (this.remoteCache.has(key)) return this.remoteCache.get(key) as T; + const value = await fn(); + this.remoteCache.set(key, value); + return value; + } + // --- local vault ops ------------------------------------------------- writeLocal(path: string, content: string | ArrayBuffer): void { @@ -113,36 +134,40 @@ export class SourceControlScenario { // --- independent remote assertions (via the git-CLI verifier) -------- async remoteContent(path: string): Promise<{ content: string; sha: string } | null> { - return this.verifier.getFile(path, this.branch); + return this.cachedRemote(`file:${path}`, () => this.verifier.getFile(path, this.branch)); } async expectRemoteContent(path: string, expected: string): Promise { - const remote = await this.verifier.getFile(path, this.branch); + const remote = await this.cachedRemote(`file:${path}`, () => this.verifier.getFile(path, this.branch)); expect(remote?.content, `remote content for ${path}`).toBe(expected); } async expectRemoteMissing(path: string): Promise { - expect(await this.verifier.fileMissing(path, this.branch), `expected ${path} missing on remote`).toBe(true); + expect(await this.cachedRemote(`missing:${path}`, () => this.verifier.fileMissing(path, this.branch)), `expected ${path} missing on remote`).toBe(true); } async expectRemoteExists(path: string): Promise { - expect(await this.verifier.fileMissing(path, this.branch), `expected ${path} present on remote`).toBe(false); + expect(await this.cachedRemote(`missing:${path}`, () => this.verifier.fileMissing(path, this.branch)), `expected ${path} present on remote`).toBe(false); } /** Current branch tip sha. */ async head(): Promise { - const [tip] = await this.verifier.listCommitShas(this.branch, 1); + const [tip] = await this.cachedRemote('shas:2', () => this.verifier.listCommitShas(this.branch, 2)); return tip!; } /** Newest-first commit shas on the branch (independent of the service). */ async listCommitShas(count: number): Promise { + if (count <= 2) { + const shas = await this.cachedRemote('shas:2', () => this.verifier.listCommitShas(this.branch, 2)); + return shas.slice(0, count); + } return this.verifier.listCommitShas(this.branch, count); } /** Asserts exactly one new commit landed since `headBefore` (the new commit's parent is `headBefore`). */ async expectSingleCommitSince(headBefore: string): Promise { - const [headAfter, headAfterParent] = await this.verifier.listCommitShas(this.branch, 2); + const [headAfter, headAfterParent] = await this.cachedRemote('shas:2', () => this.verifier.listCommitShas(this.branch, 2)); expect(headAfter, 'expected a new commit on the branch').not.toBe(headBefore); expect(headAfterParent, 'expected exactly one new commit since baseline').toBe(headBefore); } @@ -204,6 +229,29 @@ export interface SelectionStack { readonly workspace: BoundarySyncWorkspace; } +/** + * Wraps `target` so that calling any method named in `mutatingMethods` still + * behaves exactly as before, but also invokes `onMutation` once the call + * resolves. Every other property/method passes through untouched. Used to + * invalidate SourceControlScenario's remote-read cache on every path that + * can mutate the remote — including ones this file doesn't call directly + * (e.g. BoundarySyncWorkspace invoking `manager.pushFiles`). + */ +function invalidatingProxy(target: T, mutatingMethods: (keyof T)[], onMutation: () => void): T { + return new Proxy(target, { + get(obj, prop, receiver): unknown { + const value: unknown = Reflect.get(obj, prop, receiver); + if (typeof value !== 'function') return value; + if (!mutatingMethods.includes(prop as keyof T)) return value.bind(obj); + return async (...args: unknown[]) => { + const result: unknown = await (value as (...a: unknown[]) => unknown).apply(obj, args); + onMutation(); + return result; + }; + }, + }); +} + /** Builds a SyncChange with a path-derived ChangeId (mirrors FileStatusAdapter). */ export function change(path: string, kind: SyncChange['kind'], previousPath?: string): SyncChange { return { id: toChangeId(path), path, kind, previousPath }; From c37e37ceffa4d86edce33ba9a985b74f8defd44d Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 12:33:28 +0000 Subject: [PATCH 18/21] fix(settings): keep release history accessible after dismiss Missing i18n keys (settings.releaseHistory.name/desc/button) referenced by settings-implementation.ts's renderReleaseHistorySetting broke npm run build. Add them to all three locales. Co-Authored-By: Claude Sonnet 5 --- src/i18n/locales/en.ts | 4 ++++ src/i18n/locales/zh-cn.ts | 4 ++++ src/i18n/locales/zh-tw.ts | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index efde946..16a8df9 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -115,6 +115,10 @@ const en = { 'settings.whatsNewBanner.title': "What's new in v{version}", 'settings.whatsNewBanner.dismiss': 'Dismiss', + 'settings.releaseHistory.name': 'Release history', + 'settings.releaseHistory.desc': 'View past release notes for this plugin.', + 'settings.releaseHistory.button': 'View history', + 'syncStatus.viewTitle': 'Sync status', 'syncStatus.emptyPrompt': 'Click "Refresh" to check sync status', 'syncStatus.progress.checkingWithCount': 'Checking files… {current}/{total} ({pct}%)', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 145d004..f336e50 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -117,6 +117,10 @@ const zhCn: Partial> = { 'settings.whatsNewBanner.title': 'v{version} 更新重点', 'settings.whatsNewBanner.dismiss': '关闭提示', + 'settings.releaseHistory.name': '发布记录', + 'settings.releaseHistory.desc': '查看此插件过去的发布说明。', + 'settings.releaseHistory.button': '查看记录', + 'syncStatus.viewTitle': '同步状态', 'syncStatus.emptyPrompt': '点击「刷新」以检查同步状态', 'syncStatus.progress.checkingWithCount': '检查文件中… {current}/{total}（{pct}%）', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 8fe3130..a479461 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -117,6 +117,10 @@ const zhTw: Partial> = { 'settings.whatsNewBanner.title': 'v{version} 更新重點', 'settings.whatsNewBanner.dismiss': '關閉提示', + 'settings.releaseHistory.name': '發布紀錄', + 'settings.releaseHistory.desc': '查看此外掛過去的發布說明。', + 'settings.releaseHistory.button': '查看紀錄', + 'syncStatus.viewTitle': '同步狀態', 'syncStatus.emptyPrompt': '點擊「重新整理」以檢查同步狀態', 'syncStatus.progress.checkingWithCount': '檢查檔案中… {current}/{total}（{pct}%）', From 8228a049603872fdb83d372295456274ed706c30 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sat, 22 Aug 2026 20:33:42 +0800 Subject: [PATCH 19/21] test(settings): keep release history accessible after dismiss --- src/i18n/locales/en.ts | 3 +++ src/i18n/locales/zh-cn.ts | 3 +++ src/i18n/locales/zh-tw.ts | 3 +++ tests/ui/SettingsConnectionStatus.test.ts | 22 ++++++++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 16a8df9..e5b3d31 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -114,6 +114,9 @@ const en = { 'settings.whatsNewBanner.title': "What's new in v{version}", 'settings.whatsNewBanner.dismiss': 'Dismiss', + 'settings.releaseHistory.name': 'Release history', + 'settings.releaseHistory.desc': "Review what's new in current and previous versions", + 'settings.releaseHistory.button': 'View release history', 'settings.releaseHistory.name': 'Release history', 'settings.releaseHistory.desc': 'View past release notes for this plugin.', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index f336e50..122220d 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -116,6 +116,9 @@ const zhCn: Partial> = { 'settings.whatsNewBanner.title': 'v{version} 更新重点', 'settings.whatsNewBanner.dismiss': '关闭提示', + 'settings.releaseHistory.name': '版本更新记录', + 'settings.releaseHistory.desc': '查看当前与过往版本的更新内容', + 'settings.releaseHistory.button': '查看更新记录', 'settings.releaseHistory.name': '发布记录', 'settings.releaseHistory.desc': '查看此插件过去的发布说明。', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index a479461..913a0a0 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -116,6 +116,9 @@ const zhTw: Partial> = { 'settings.whatsNewBanner.title': 'v{version} 更新重點', 'settings.whatsNewBanner.dismiss': '關閉提示', + 'settings.releaseHistory.name': '版本更新紀錄', + 'settings.releaseHistory.desc': '查看目前與過往版本的更新內容', + 'settings.releaseHistory.button': '查看更新紀錄', 'settings.releaseHistory.name': '發布紀錄', 'settings.releaseHistory.desc': '查看此外掛過去的發布說明。', diff --git a/tests/ui/SettingsConnectionStatus.test.ts b/tests/ui/SettingsConnectionStatus.test.ts index 6da4ce4..453a714 100644 --- a/tests/ui/SettingsConnectionStatus.test.ts +++ b/tests/ui/SettingsConnectionStatus.test.ts @@ -121,3 +121,25 @@ describe('GitLabSyncSettingTab ignore patterns setting', () => { expect(textarea.value).toBe('draft/\n*.tmp'); }); }); + +describe('GitLabSyncSettingTab release history', () => { + it('keeps release history accessible after the current-version banner was dismissed', () => { + vi.useFakeTimers(); + const plugin = createPluginStub(vi.fn().mockResolvedValue({ repoOk: true, branchOk: true })); + plugin.manifest = { version: '1.5.0' } as GitLabFilesPush['manifest']; + plugin.settings.bannerDismissedVersion = '1.5.0'; + const tab = new GitLabSyncSettingTab(new App(), plugin); + tab.containerEl = createContainer(); + + try { + tab.display(); + + expect(tab.containerEl.querySelector('.gfs-whats-new-banner')).toBeNull(); + const buttons = Array.from(tab.containerEl.querySelectorAll('button')); + expect(buttons.some(button => button.textContent === 'View release history')).toBe(true); + } finally { + vi.clearAllTimers(); + vi.useRealTimers(); + } + }); +}); From de5565324f702e822ec138a411bcb44db568196a Mon Sep 17 00:00:00 2001 From: tianyao Date: Sat, 22 Aug 2026 12:37:08 +0000 Subject: [PATCH 20/21] fix(i18n): remove duplicate releaseHistory keys from concurrent fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My earlier fix and 8228a04 (concurrent push) both added settings.releaseHistory.{name,desc,button} independently, so the rebase merged in two copies of each key per locale file — TS1117 (duplicate object literal property). Keep 8228a04's wording, drop mine. Co-Authored-By: Claude Sonnet 5 --- src/i18n/locales/en.ts | 4 ---- src/i18n/locales/zh-cn.ts | 4 ---- src/i18n/locales/zh-tw.ts | 4 ---- 3 files changed, 12 deletions(-) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index e5b3d31..7e5a746 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -118,10 +118,6 @@ const en = { 'settings.releaseHistory.desc': "Review what's new in current and previous versions", 'settings.releaseHistory.button': 'View release history', - 'settings.releaseHistory.name': 'Release history', - 'settings.releaseHistory.desc': 'View past release notes for this plugin.', - 'settings.releaseHistory.button': 'View history', - 'syncStatus.viewTitle': 'Sync status', 'syncStatus.emptyPrompt': 'Click "Refresh" to check sync status', 'syncStatus.progress.checkingWithCount': 'Checking files… {current}/{total} ({pct}%)', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 122220d..ea0774c 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -120,10 +120,6 @@ const zhCn: Partial> = { 'settings.releaseHistory.desc': '查看当前与过往版本的更新内容', 'settings.releaseHistory.button': '查看更新记录', - 'settings.releaseHistory.name': '发布记录', - 'settings.releaseHistory.desc': '查看此插件过去的发布说明。', - 'settings.releaseHistory.button': '查看记录', - 'syncStatus.viewTitle': '同步状态', 'syncStatus.emptyPrompt': '点击「刷新」以检查同步状态', 'syncStatus.progress.checkingWithCount': '检查文件中… {current}/{total}（{pct}%）', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 913a0a0..cda7811 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -120,10 +120,6 @@ const zhTw: Partial> = { 'settings.releaseHistory.desc': '查看目前與過往版本的更新內容', 'settings.releaseHistory.button': '查看更新紀錄', - 'settings.releaseHistory.name': '發布紀錄', - 'settings.releaseHistory.desc': '查看此外掛過去的發布說明。', - 'settings.releaseHistory.button': '查看紀錄', - 'syncStatus.viewTitle': '同步狀態', 'syncStatus.emptyPrompt': '點擊「重新整理」以檢查同步狀態', 'syncStatus.progress.checkingWithCount': '檢查檔案中… {current}/{total}（{pct}%）', From b1d22083237a9bf563d641685618164a3c23641a Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sat, 22 Aug 2026 20:55:34 +0800 Subject: [PATCH 21/21] perf(ci): gate and tier real-provider E2E --- .github/workflows/ci.yml | 26 ++++++++++++++-- e2e/suites/source-control-flows.e2e.test.ts | 33 +++++++++++---------- scripts/run-e2e.sh | 19 ++++++++++++ scripts/run-preflight.sh | 19 ++++++++++++ 4 files changed, 80 insertions(+), 17 deletions(-) create mode 100755 scripts/run-preflight.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd6a6e8..aa4d015 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,11 +57,33 @@ jobs: - 'package-lock.json' - '.github/workflows/ci.yml' + # Fast local gate: run cheap deterministic checks in parallel before any + # real-provider E2E spends remote API time. The release-critical reusable CI + # still runs after E2E below; this is only an early failure gate. + preflight: + name: Preflight / ${{ matrix.check }} + runs-on: ubuntu-latest + strategy: + fail-fast: true + max-parallel: 3 + matrix: + check: [lint, test, build] + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: '22' + cache: npm + - run: npm ci --ignore-scripts + - name: Run ${{ matrix.check }} + run: bash scripts/run-preflight.sh "${{ matrix.check }}" + # Real-provider E2E: one matrix job covering GitHub, GitLab, and Gitea (see - # docs/testing/real-provider-e2e.md). + # docs/testing/real-provider-e2e.md). It starts only after the fast local + # preflight passes, then provider legs run in parallel. provider-e2e: name: E2E / ${{ matrix.provider }} - needs: changes + needs: [changes, preflight] runs-on: [self-hosted, linux, x64, 32gb-ram] # Runs when sync/provider-relevant paths changed, or unconditionally on # workflow_dispatch/schedule/a push to main (main always gets the full diff --git a/e2e/suites/source-control-flows.e2e.test.ts b/e2e/suites/source-control-flows.e2e.test.ts index 5403dd8..ded8171 100644 --- a/e2e/suites/source-control-flows.e2e.test.ts +++ b/e2e/suites/source-control-flows.e2e.test.ts @@ -14,9 +14,12 @@ vi.mock('../../src/ui/BatchConflictResolutionModal'); // Provider matrix: Core scenarios run on every provider; Extended scenarios // (rename chains, unicode, batch-scale, etc.) exercise SyncManager/model -// behavior that's provider-agnostic, so they run on GitHub only to keep -// real-API CI fast and stable. Stress (1000-file) is opt-in via E2E_STRESS=1. +// behavior that's provider-agnostic. PR/branch CI runs the core tier; GitHub +// main, schedule, manual, and local runs use the full tier. Stress (1000-file) +// remains opt-in via E2E_STRESS=1. const isGitHub = process.env.E2E_PROVIDER === 'github'; +const e2eTier = process.env.E2E_TIER ?? 'full'; +const runExtended = isGitHub && e2eTier !== 'core'; const isStress = process.env.E2E_STRESS === '1'; describe('Source Control Flows E2E', () => { @@ -86,7 +89,7 @@ describe('Source Control Flows E2E', () => { // Extended: nested move + rename chain (SyncManager/model behavior, // provider-agnostic) — GitHub only. - it.skipIf(!isGitHub)('moves files across nested directories in one commit', async () => { + it.skipIf(!runExtended)('moves files across nested directories in one commit', async () => { const s = scenario(); const oldFlat = path('nested-move/folder/a.md'); const oldNested = path('nested-move/folder/nested/b.md'); @@ -112,7 +115,7 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); - it.skipIf(!isGitHub)('collapses a rename chain (A->B->C) into a single move of the original path', async () => { + it.skipIf(!runExtended)('collapses a rename chain (A->B->C) into a single move of the original path', async () => { const s = scenario(); const a = path('rename-chain/a.md'); const b = path('rename-chain/b.md'); @@ -248,7 +251,7 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); - it.skipIf(!isGitHub)('overwrites a remotely-created file with local content on a no-baseline add/add (current contract)', async () => { + it.skipIf(!runExtended)('overwrites a remotely-created file with local content on a no-baseline add/add (current contract)', async () => { const s = scenario(); const p = path('conflict-add-add/a.md'); await s.seedRemote(p, 'remote'); @@ -343,7 +346,7 @@ describe('Source Control Flows E2E', () => { // Phase 5 — Mixed batch operations // ------------------------------------------------------------------ describe('mixed batch operations', () => { - it.skipIf(!isGitHub)('pushes a create + modify + rename in one commit', async () => { + it.skipIf(!runExtended)('pushes a create + modify + rename in one commit', async () => { const s = scenario(); const create = path('mixed-cmr/create.md'); const modify = path('mixed-cmr/modify.md'); @@ -478,7 +481,7 @@ describe('Source Control Flows E2E', () => { expect(selection.isIncluded(cc.id)).toBe(true); }); - it.skipIf(!isGitHub)('pushes a subset then the remaining subset as two separate commits', async () => { + it.skipIf(!runExtended)('pushes a subset then the remaining subset as two separate commits', async () => { const s = scenario(); const a = path('subset-then-rest/a.md'); const b = path('subset-then-rest/b.md'); @@ -517,7 +520,7 @@ describe('Source Control Flows E2E', () => { await s.expectRemoteContent(c, 'c-v2'); }); - it.skipIf(!isGitHub)('rename yields a path-derived ChangeId; selecting the new id pushes the move', async () => { + it.skipIf(!runExtended)('rename yields a path-derived ChangeId; selecting the new id pushes the move', async () => { const s = scenario(); const oldP = path('selection-rename/a.md'); const newP = path('selection-rename/archive/a.md'); @@ -641,7 +644,7 @@ describe('Source Control Flows E2E', () => { expect(s.metadataSha(p), 'metadata not corrupted by the no-op repeat').toBe(shaAfterFirst); }); - it.skipIf(!isGitHub)('re-syncs cleanly after a skipped conflict (no stale operation state)', async () => { + it.skipIf(!runExtended)('re-syncs cleanly after a skipped conflict (no stale operation state)', async () => { const s = scenario(); const p = path('retry-after-skip/a.md'); await s.baseline(p, 'v1'); @@ -671,7 +674,7 @@ describe('Source Control Flows E2E', () => { // Phase 8 — Path edge cases + batch scale // ------------------------------------------------------------------ describe('path edge cases and batch scale', () => { - it.skipIf(!isGitHub)('creates, modifies, and renames a unicode-named file', async () => { + it.skipIf(!runExtended)('creates, modifies, and renames a unicode-named file', async () => { const s = scenario(); const original = path('unicode/筆記/測試文件.md'); const archived = path('unicode/筆記/已歸檔.md'); @@ -694,7 +697,7 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); - it.skipIf(!isGitHub)('creates and modifies a file with spaces and symbols', async () => { + it.skipIf(!runExtended)('creates and modifies a file with spaces and symbols', async () => { const s = scenario(); const p = path('spaces/folder/my note (draft).md'); await s.baseline(p, 'draft-v1'); @@ -707,7 +710,7 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); - it.skipIf(!isGitHub)('moves and modifies a deeply nested file', async () => { + it.skipIf(!runExtended)('moves and modifies a deeply nested file', async () => { const s = scenario(); const oldP = path('deep/a/b/c/d/e/note.md'); const newP = path('deep/archive/x/y/z/w/note.md'); @@ -725,7 +728,7 @@ describe('Source Control Flows E2E', () => { await s.expectSingleCommitSince(headBefore); }); - it.skipIf(!isGitHub)('creates 100 files in one commit', async () => { + it.skipIf(!runExtended)('creates 100 files in one commit', async () => { const s = scenario(); const paths = Array.from({ length: 100 }, (_, i) => path(`batch-100/${String(i).padStart(3, '0')}.md`)); for (const p of paths) s.writeLocal(p, `content ${p}`); @@ -740,7 +743,7 @@ describe('Source Control Flows E2E', () => { await s.expectRemoteContent(paths[99]!, `content ${paths[99]}`); }); - it.skipIf(!isGitHub)('pushes a 100-file mixed batch (modify + create + rename) in one commit', async () => { + it.skipIf(!runExtended)('pushes a 100-file mixed batch (modify + create + rename) in one commit', async () => { const s = scenario(); const modifyPaths = Array.from({ length: 40 }, (_, i) => path(`mixed-100/modify/${i}.md`)); const createPaths = Array.from({ length: 30 }, (_, i) => path(`mixed-100/create/${i}.md`)); @@ -769,7 +772,7 @@ describe('Source Control Flows E2E', () => { await s.expectRemoteContent(renameNew[0]!, 'r-v1'); }); - it.skipIf(!isStress || !isGitHub)('stress: creates 1000 files', async () => { + it.skipIf(!isStress || !runExtended)('stress: creates 1000 files', async () => { const s = scenario(); const paths = Array.from({ length: 1000 }, (_, i) => path(`batch-1000/${String(i).padStart(4, '0')}.md`)); for (const p of paths) s.writeLocal(p, `content ${p}`); diff --git a/scripts/run-e2e.sh b/scripts/run-e2e.sh index 1da3be1..dbdd9fc 100755 --- a/scripts/run-e2e.sh +++ b/scripts/run-e2e.sh @@ -9,10 +9,13 @@ set -euo pipefail provider="" +tier="auto" while [[ $# -gt 0 ]]; do case "$1" in --provider) provider="$2"; shift 2 ;; --provider=*) provider="${1#*=}"; shift ;; + --tier) tier="$2"; shift 2 ;; + --tier=*) tier="${1#*=}"; shift ;; *) shift ;; esac done @@ -21,7 +24,22 @@ if [ -z "$provider" ]; then exit 1 fi +if [ "$tier" = "auto" ]; then + if [ "${GITHUB_ACTIONS:-}" != "true" ]; then + tier="full" + elif [ "$provider" = "github" ] && { [ "${GITHUB_REF_NAME:-}" = "main" ] || [ "${GITHUB_REF_NAME:-}" = "master" ] || [ "${GITHUB_EVENT_NAME:-}" = "schedule" ] || [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ]; }; then + tier="full" + else + tier="core" + fi +fi +if [ "$tier" != "core" ] && [ "$tier" != "full" ]; then + echo "Invalid E2E tier: $tier (expected core|full|auto)" >&2 + exit 1 +fi + export E2E_PROVIDER="$provider" +export E2E_TIER="$tier" export E2E_WORKDIR="${E2E_WORKDIR:-${TMPDIR:-/tmp}/gfs-e2e-${provider}}" cleanup() { @@ -100,6 +118,7 @@ if [ "${#unregistered[@]}" -gt 0 ]; then exit 1 fi +echo "[run-e2e] tier=$E2E_TIER provider=$E2E_PROVIDER" >&2 echo "[run-e2e] running suites: ${SUITES[*]}" >&2 # vitest.e2e.config.ts's `include` matches every e2e/suites/*.e2e.test.ts, so diff --git a/scripts/run-preflight.sh b/scripts/run-preflight.sh new file mode 100755 index 0000000..e5b8b9f --- /dev/null +++ b/scripts/run-preflight.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -euo pipefail + +check="${1:-}" +case "$check" in + lint) + npm run lint + ;; + test) + npm test + ;; + build) + npm run build + ;; + *) + echo "Usage: scripts/run-preflight.sh " >&2 + exit 2 + ;; +esac