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
4 changes: 2 additions & 2 deletions src/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
listRepoFiles,
pullRepoFile,
type RemoteFile,
formatSyncFailure,
} from './sync/git-sync';
import { logError } from './lib/logging';
import { useReadOnlyFiles } from './data/useReadOnlyFiles';
Expand Down Expand Up @@ -450,10 +451,9 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput
setStatusMessage(parts.length ? `Synced: ${parts.join(', ')}` : 'Up to date');
} catch (error) {
logError(error);
setStatusMessage('Sync failed');
setStatusMessage(formatSyncFailure(error));
}
};

// click on a file in the sidebar
const selectFile = async (path: string | undefined) => {
await selectReadOnlyFile(path);
Expand Down
68 changes: 63 additions & 5 deletions src/data/data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ type SyncMocks = {
syncBidirectional: ReturnType<typeof vi.fn>;
};

type LoggingMocks = {
logError: ReturnType<typeof vi.fn>;
};

const authModule = vi.hoisted<AuthMocks>(() => ({
signInWithGitHubApp: vi.fn(),
getSessionToken: vi.fn(),
Expand All @@ -57,6 +61,10 @@ const syncModule = vi.hoisted<SyncMocks>(() => ({
syncBidirectional: vi.fn(),
}));

const loggingModule = vi.hoisted<LoggingMocks>(() => ({
logError: vi.fn(),
}));

vi.mock('../auth/app-auth', () => ({
signInWithGitHubApp: authModule.signInWithGitHubApp,
getSessionToken: authModule.getSessionToken,
Expand All @@ -73,13 +81,21 @@ vi.mock('../lib/backend', () => ({
revokeShareLink: backendModule.revokeShareLink,
}));

vi.mock('../sync/git-sync', () => ({
buildRemoteConfig: syncModule.buildRemoteConfig,
listRepoFiles: syncModule.listRepoFiles,
pullRepoFile: syncModule.pullRepoFile,
syncBidirectional: syncModule.syncBidirectional,
vi.mock('../lib/logging', () => ({
logError: loggingModule.logError,
}));

vi.mock('../sync/git-sync', async () => {
const actual = await vi.importActual<typeof import('../sync/git-sync')>('../sync/git-sync');
return {
...actual,
buildRemoteConfig: syncModule.buildRemoteConfig,
listRepoFiles: syncModule.listRepoFiles,
pullRepoFile: syncModule.pullRepoFile,
syncBidirectional: syncModule.syncBidirectional,
};
});

let useRepoData: typeof import('../data').useRepoData;

beforeAll(async () => {
Expand All @@ -101,6 +117,7 @@ const mockBuildRemoteConfig = syncModule.buildRemoteConfig;
const mockListRepoFiles = syncModule.listRepoFiles;
const mockPullRepoFile = syncModule.pullRepoFile;
const mockSyncBidirectional = syncModule.syncBidirectional;
const mockLogError = loggingModule.logError;

const writableMeta: RepoMetadata = {
isPrivate: true,
Expand Down Expand Up @@ -189,6 +206,7 @@ describe('useRepoData', () => {
mockListRepoFiles.mockReset();
mockPullRepoFile.mockReset();
mockSyncBidirectional.mockReset();
mockLogError.mockReset();

mockGetSessionToken.mockReturnValue(null);
mockGetSessionUser.mockReturnValue(null);
Expand Down Expand Up @@ -413,6 +431,46 @@ describe('useRepoData', () => {
expect(result.current.state.user).toEqual(expect.objectContaining({ login: 'hubot' }));
});

test('sync surfaces detailed message when GitHub returns 422', async () => {
const slug = 'acme/docs';
const recordRecent = vi.fn<RecordRecentFn>();

mockGetSessionToken.mockReturnValue('session-token');
mockGetSessionUser.mockReturnValue({
login: 'hubot',
name: null,
avatarUrl: 'https://example.com/hubot.png',
});
markRepoLinked(slug);

const ghError = Object.assign(new Error('GitHub request failed (422)'), {
status: 422,
path: '/repos/acme/docs/git/refs/heads/main',
body: { message: 'Update is not a fast-forward' },
syncContexts: [{ operation: 'delete', paths: ['Ready.md'] }],
});
mockSyncBidirectional.mockRejectedValue(ghError);

const { result } = renderRepoData({
slug,
route: { kind: 'repo', owner: 'acme', repo: 'docs' },
recordRecent,
});

await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready'));
await waitFor(() => expect(result.current.state.canSync).toBe(true));

await act(async () => {
await result.current.actions.syncNow();
});

expect(mockSyncBidirectional).toHaveBeenCalledWith(expect.any(LocalStore), slug);
expect(result.current.state.statusMessage).toBe(
'Sync failed: GitHub returned 422 while updating refs/heads/main for Ready.md (Update is not a fast-forward). Please report this bug.'
);
expect(mockLogError).toHaveBeenCalledWith(ghError);
});

// Read-only repos should list remote notes and refresh on selection.
test('read-only repos surface notes and refresh on selection', async () => {
const slug = 'octo/wiki';
Expand Down
1 change: 1 addition & 0 deletions src/storage/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ export function moveFilePath(slug: string, id: string, toPath: string) {
};
});
rebuildFolderIndex(slug);
emitRepoChange(slug);
}

function loadIndexForSlug(slug: string): FileMeta[] {
Expand Down
77 changes: 77 additions & 0 deletions src/sync/git-sync-stale.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { LocalStore, listTombstones } from '../storage/local';
import { MockRemoteRepo } from '../test/mock-remote';

const authModule = vi.hoisted(() => ({
ensureFreshAccessToken: vi.fn().mockResolvedValue('test-token'),
}));

vi.mock('../auth/app-auth', () => authModule);

const globalAny = globalThis as { fetch?: typeof fetch };

describe('syncBidirectional with stale ref reads enabled', () => {
let store: LocalStore;
let remote: MockRemoteRepo;
let syncBidirectional: typeof import('./git-sync').syncBidirectional;

beforeEach(async () => {
authModule.ensureFreshAccessToken.mockReset();
authModule.ensureFreshAccessToken.mockResolvedValue('test-token');
remote = new MockRemoteRepo();
remote.configure('user', 'repo');
remote.allowToken('test-token');
remote.enableStaleReads({ enabled: true, windowMs: 5_000 });
const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => remote.handleFetch(input, init));
globalAny.fetch = fetchMock as unknown as typeof fetch;
const mod = await import('./git-sync');
syncBidirectional = mod.syncBidirectional;
store = new LocalStore('user/repo');
});

test('second consecutive edit survives stale ref reads', async () => {
store.createFile('Note.md', 'v1');
await syncBidirectional(store, 'user/repo');

store.saveFile('Note.md', 'v2');
await syncBidirectional(store, 'user/repo');

store.saveFile('Note.md', 'v3');
const summary = await syncBidirectional(store, 'user/repo');
expect(summary).toEqual({
pulled: 0,
pushed: 1,
deletedRemote: 0,
deletedLocal: 0,
merged: 0,
});
});

test('rename then edit succeeds despite stale reads', async () => {
store.createFile('Draft.md', 'first');
await syncBidirectional(store, 'user/repo');

store.renameFile('Draft.md', 'Draft v2');
const renameSummary = await syncBidirectional(store, 'user/repo');
expect(renameSummary).toEqual({
pulled: 0,
pushed: 1,
deletedRemote: 1,
deletedLocal: 0,
merged: 0,
});
expect([...remote.snapshot().keys()]).toEqual(['Draft v2.md']);

store.saveFile('Draft v2.md', 'second');
const editSummary = await syncBidirectional(store, 'user/repo');
expect(editSummary).toEqual({
pulled: 0,
pushed: 1,
deletedRemote: 0,
deletedLocal: 0,
merged: 0,
});
expect([...remote.snapshot().keys()]).toEqual(['Draft v2.md']);
expect(listTombstones(store.slug)).toHaveLength(0);
});
});
54 changes: 53 additions & 1 deletion src/sync/git-sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,26 @@ const globalAny = globalThis as {
fetch?: typeof fetch;
};

describe('syncBidirectional', () => {
const remoteScenarios: Array<{
label: string;
configure(remote: MockRemoteRepo): void;
}> = [
{
label: 'fresh remote responses',
configure(remote) {
remote.enableStaleReads({ enabled: false });
},
},
{
label: 'stale remote responses with random delay',
configure(remote) {
const windowMs = Math.floor(Math.random() * 901) + 100;
remote.enableStaleReads({ enabled: true, windowMs });
},
},
];

describe.each(remoteScenarios)('syncBidirectional: $label', ({ configure }) => {
let store: LocalStore;
let remote: MockRemoteRepo;
let syncBidirectional: typeof import('./git-sync').syncBidirectional;
Expand All @@ -24,6 +43,7 @@ describe('syncBidirectional', () => {
remote = new MockRemoteRepo();
remote.configure('user', 'repo');
remote.allowToken('test-token');
configure(remote);
const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) =>
remote.handleFetch(input, init)
);
Expand Down Expand Up @@ -110,6 +130,38 @@ describe('syncBidirectional', () => {
expectParity(store, remote);
});

test('rename followed by local edit pushes updated content under new path', async () => {
store.createFile('Draft.md', 'initial body');
await syncBidirectional(store, 'user/repo');
expect([...remote.snapshot().keys()]).toEqual(['Draft.md']);

const nextPath = store.renameFile('Draft.md', 'Ready');
expect(nextPath).toBe('Ready.md');
store.saveFile('Ready.md', 'edited after rename');

await syncBidirectional(store, 'user/repo');

const remoteFiles = [...remote.snapshot().entries()];
expect(remoteFiles).toEqual([['Ready.md', 'edited after rename']]);
const readyMeta = store.listFiles().find((file) => file.path === 'Ready.md');
const readyDoc = readyMeta ? store.loadFileById(readyMeta.id) : null;
expect(readyDoc?.content).toBe('edited after rename');
expect(listTombstones(store.slug)).toHaveLength(0);
});

test('surface 422 when branch head advances during push', async () => {
store.createFile('Lonely.md', 'seed text');
await syncBidirectional(store, 'user/repo');

store.saveFile('Lonely.md', 'edited locally');
remote.advanceHeadOnNextUpdate();

await expect(syncBidirectional(store, 'user/repo')).rejects.toMatchObject({
status: 422,
path: expect.stringContaining('/git/refs/heads/'),
});
});

test('pulls new remote notes', async () => {
remote.setFile('Remote.md', '# remote');
await syncBidirectional(store, 'user/repo');
Expand Down
Loading