Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ jobs:
E2E_PROVIDER: ${{ matrix.provider }}
with:
timeout_minutes: 15
max_attempts: 2
max_attempts: 3
retry_wait_seconds: 15
command: |
set -a
Expand Down Expand Up @@ -251,26 +251,37 @@ jobs:
# "Determine whether this provider leg should run" step above; this gate
# only asks "did whatever ran, pass?"). A gated-off leg's steps are all
# skipped without failing the job, so it still reports "success" here.
# `if: always()` so a real provider-e2e failure/cancellation is caught
# here and blocks CI/release, instead of GitHub Actions silently treating
# an upstream failure as "this job never needed to run".
# `if: always()` so a real provider-e2e failure is caught here and blocks
# CI/release. A cancelled matrix means a newer run in the same branch/provider
# concurrency group replaced this duplicate run; report that as neutral and
# do not start another copy of downstream CI.
e2e-gate:
name: E2E gate
needs: provider-e2e
if: always()
runs-on: ubuntu-latest
outputs:
run-ci: ${{ steps.check.outputs.run-ci }}
steps:
- name: Check provider-e2e result
id: check
run: |
result="${{ needs.provider-e2e.result }}"
echo "provider-e2e result: $result"
echo "run-ci=true" >> "$GITHUB_OUTPUT"
if [ "$result" = "cancelled" ]; then
echo "run-ci=false" >> "$GITHUB_OUTPUT"
echo "::notice::provider-e2e was replaced by a newer run in the same concurrency group."
exit 0
fi
if [ "$result" != "success" ] && [ "$result" != "skipped" ]; then
echo "::error::provider-e2e failed or was cancelled ($result) -- blocking CI/release."
echo "::error::provider-e2e failed ($result) -- blocking CI/release."
exit 1
fi

CI:
needs: e2e-gate
if: needs.e2e-gate.outputs.run-ci == 'true'
uses: firstsun-dev/.github/.github/workflows/obsidian-plugin-ci.yml@v1
with:
plugin-id: "git-file-sync"
Expand Down
10 changes: 7 additions & 3 deletions docs/testing/real-provider-e2e.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ sandbox and starved each other (observed as real GitLab API timeouts under that
Keying by branch name alone means the later of the two cancels the earlier instead. The two
cleanup workflows below share this same group naming for the same branch, with
`cancel-in-progress: false`, so cleanup queues behind rather than races an active run.
The cancelled duplicate's `e2e-gate` reports the replacement as neutral and sets `run-ci=false`,
so it neither leaves a misleading aggregate failure nor starts a second copy of downstream CI.
The surviving run remains responsible for the real provider result and release gate.
**Cancellation is not a cleanup mechanism.** A cancelled run's `cleanup` step may never execute, or
may be mid-delete when the runner is terminated; the next run is still safe because it always
allocates a brand-new `run-<run-id>-<run-attempt>` branch rather than deleting and reusing the old
Expand Down Expand Up @@ -263,9 +266,10 @@ changes -> provider-e2e [github | gitlab | gitea, parallel] -> e2e-gate -> CI (s
```

`e2e-gate` runs with `if: always()` and treats `provider-e2e`'s aggregate result as pass-through
on `success` or `skipped` (the latter covers path-filtered-out runs), and a hard failure on
anything else — so a real provider regression blocks the release instead of shipping and being
caught after the fact.
on `success` or `skipped` (the latter covers path-filtered-out runs), a neutral replacement on
`cancelled` (with downstream CI suppressed for that duplicate run), and a hard failure on any
other result. A real provider regression therefore still blocks the release instead of shipping
and being caught after the fact.

**Branch protection** (not something this repo checkout can change — a GitHub repo-settings
change, left for whoever has admin access): add `E2E / gitea` as a required status check.
Expand Down
32 changes: 23 additions & 9 deletions e2e/suites/sync-manager.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import { SyncManager, BatchPushConflict, ConflictResolution } from '../../src/logic/sync-manager';
import { SyncPlanModal, SyncPlanDirection } from '../../src/ui/SyncPlanModal';
import { BatchConflictResolutionModal } from '../../src/ui/BatchConflictResolutionModal';
import { ObsidianSyncInteraction } from '../../src/ui/ObsidianSyncInteraction';
import { describePushResult } from '../support/push-result-diagnostic';
// `import type` deliberately, not a value import: src/settings.ts also
// exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest ->
// AbstractInputSuggest etc.) which pulls in far more of `obsidian` than this
Expand Down Expand Up @@ -94,7 +96,8 @@
}, timeouts.containerReadyMs + 30_000);

function newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager {
return new SyncManager(fakeApp(vault), service, settings, undefined, () => false);
const app = fakeApp(vault);
return new SyncManager(app, service, settings, undefined, () => false, undefined, new ObsidianSyncInteraction(app));
}

it('pushes a new local file, verified independently of the service', async () => {
Expand All @@ -106,7 +109,8 @@

const result = await manager.pushFiles([filePath]);

expect(result.success).toBe(1);
expect(result.success, describePushResult(result)).toBe(1);
expect(result.failed, describePushResult(result)).toBe(0);
const pushedSha = result.syncedPaths.find(p => p.path === filePath)?.sha;
expect(pushedSha).toBeTruthy();
const remote = await verifier.getFile(filePath, branch);
Expand All @@ -121,7 +125,9 @@
vault.writeLocal(filePath, 'steady state');
const settings = makeSettings(branch);
const manager = newManager(vault, settings);
await manager.pushFiles([filePath]);
const initialPush = await manager.pushFiles([filePath]);
expect(initialPush.success, describePushResult(initialPush)).toBe(1);
expect(initialPush.failed, describePushResult(initialPush)).toBe(0);

const shasBefore = await verifier.listCommitShas(branch);
const result = await manager.pushFiles([filePath]);
Expand Down Expand Up @@ -164,7 +170,9 @@
vault.writeLocal(filePath, 'baseline');
const settings = makeSettings(branch);
const manager = newManager(vault, settings);
await manager.pushFiles([filePath]);
const initialPush = await manager.pushFiles([filePath]);
expect(initialPush.success, describePushResult(initialPush)).toBe(1);
expect(initialPush.failed, describePushResult(initialPush)).toBe(0);
const baselineMeta = settings.syncMetadata[filePath];

// Diverge both sides from the synced baseline.
Expand All @@ -176,7 +184,7 @@
const conflictCallsBefore = vi.mocked(BatchConflictResolutionModal).mock.calls.length;
const result = await manager.pushFiles([filePath]);

expect(vi.mocked(BatchConflictResolutionModal).mock.calls.length).toBe(conflictCallsBefore + 1);

Check failure on line 187 in e2e/suites/sync-manager.e2e.test.ts

View workflow job for this annotation

GitHub Actions / E2E / github

e2e/suites/sync-manager.e2e.test.ts > SyncManager E2E > does not overwrite the remote or falsely mark synced when both sides changed

AssertionError: expected +0 to be 1 // Object.is equality - Expected + Received - 1 + 0 ❯ e2e/suites/sync-manager.e2e.test.ts:187:75
expect(result.skippedConflicts).toBeGreaterThanOrEqual(1);
const remoteAfter = await verifier.getFile(filePath, branch);
expect(remoteAfter?.content).toBe('remote edit');
Expand All @@ -190,7 +198,9 @@
vault.writeLocal(oldPath, 'move me');
const settings = makeSettings(branch);
const manager = newManager(vault, settings);
await manager.pushFiles([oldPath]);
const initialPush = await manager.pushFiles([oldPath]);
expect(initialPush.success, describePushResult(initialPush)).toBe(1);
expect(initialPush.failed, describePushResult(initialPush)).toBe(0);

vault.renameLocal(oldPath, newPath);
await manager.trackRename(newPath, oldPath);
Expand All @@ -204,7 +214,9 @@
// `!isString && fileOrPath instanceof TFile` before consulting
// `renamedFrom`).
const newFile: TFileLike = vault.fileAt(newPath);
await manager.pushFiles([newFile as unknown as string]);
const moveResult = await manager.pushFiles([newFile as unknown as string]);
expect(moveResult.success, describePushResult(moveResult)).toBe(1);
expect(moveResult.failed, describePushResult(moveResult)).toBe(0);

expect(await verifier.fileMissing(oldPath, branch)).toBe(true);
const remote = await verifier.getFile(newPath, branch);
Expand All @@ -222,7 +234,9 @@
vault.writeLocal(filePath, 'delete me');
const settings = makeSettings(branch);
const manager = newManager(vault, settings);
await manager.pushFiles([filePath]);
const initialPush = await manager.pushFiles([filePath]);
expect(initialPush.success, describePushResult(initialPush)).toBe(1);
expect(initialPush.failed, describePushResult(initialPush)).toBe(0);
expect(await verifier.fileMissing(filePath, branch)).toBe(false);

await service.deleteFile(filePath, branch, 'e2e: delete file');
Expand All @@ -242,8 +256,8 @@

const results = await manager.pushFiles(paths);

expect(results.success).toBe(paths.length);
expect(results.failed).toBe(0);
expect(results.success, describePushResult(results)).toBe(paths.length);
expect(results.failed, describePushResult(results)).toBe(0);
for (const p of paths) {
const remote = await verifier.getFile(p, branch);
expect(remote?.content).toBe(`content for ${p}`);
Expand Down
9 changes: 9 additions & 0 deletions e2e/support/push-result-diagnostic.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
interface PushResultDiagnostic {
success: number;
failed: number;
errors: ReadonlyArray<unknown>;
}

export function describePushResult(result: PushResultDiagnostic): string {
return `push result: success=${result.success}, failed=${result.failed}, errors=${JSON.stringify(result.errors)}`;
}
10 changes: 9 additions & 1 deletion feature_list.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
{
"_note": "GitHub Issues (firstsun-dev/git-files-sync, Project #6) is the source of truth for the full backlog and priority/estimate fields. This file mirrors only the active feature and the next few candidates so an agent session has a local, offline checkpoint — sync it against `gh issue list --repo firstsun-dev/git-files-sync --state open` at the start of a session rather than treating it as authoritative.",
"_lastSync": "2026-08-07: Archived feat-001 through feat-025 (complete). Backlog candidates below.",
"_lastSync": "2026-08-19: Synced against open GitHub issues; issue #105 is the active architecture refactor.",
"features": [
{
"id": "feat-026",
"name": "refactor(sync): separate planning, execution, conflicts, metadata, and UI (issue #105)",
"description": "Preserve sync behavior while extracting SyncStatusView presentation state/controller boundaries and SyncManager scanner/planner/executor/workspace boundaries with regression and integration coverage.",
"dependencies": [],
"status": "in-progress",
"evidence": "Commits dff95db/948df28 on refactor/sync-domain-pipeline: unified sync decisions and CI hardening are covered; 613 tests, local Gitea E2E, and real CI run 32338116598 are green; desktop/mobile smoke pending."
},
{
"id": "feat-004",
"name": "fix: resolve sonarqube issues (issue #45)",
Expand Down
Loading
Loading