Skip to content

fix: prevent cross-process stale writes to maxRevision.json #877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
WillieRuemmele wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: prevent cross-process stale writes to maxRevision.json #877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 30 additions & 0 deletions src/shared/remote/fileOperations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,31 @@ export const upgradeFileContents = (contents: ContentsV0): ContentsV1 => ({
),
});

/**
* Prevents cross-process stale writes by merging in-memory state with on-disk state.
* For each member present in both, takes the higher lastRetrievedFromServer (never regresses).
*/
const mergeWithDiskState = async (
filePath: string,
members: Map<string, MemberRevision>,
maxCounter: number
): Promise<number> => {
const diskContents = await readFileContents(filePath);
if (!('serverMaxRevisionCounter' in diskContents) || !diskContents.sourceMembers) {
return maxCounter;
}

for (const [key, diskMember] of Object.entries(diskContents.sourceMembers)) {
const memMember = members.get(key);
if (memMember && (diskMember.lastRetrievedFromServer ?? -1) > (memMember.lastRetrievedFromServer ?? -1)) {
members.set(key, { ...memMember, lastRetrievedFromServer: diskMember.lastRetrievedFromServer });
}
}

const diskMax = diskContents.serverMaxRevisionCounter;
return Number.isFinite(diskMax) ? Math.max(maxCounter, diskMax) : maxCounter;
};

export const writeTrackingFile = async ({
filePath,
maxCounter,
Expand All @@ -96,6 +121,11 @@ export const writeTrackingFile = async ({
members: Map<string, MemberRevision>;
}): Promise<void> => {
const lockResult = await lockInit(filePath);

// Merge with on-disk state to prevent cross-process stale overwrites.
// readFileContents returns {} for missing/unreadable files, which mergeWithDiskState handles.
maxCounter = await mergeWithDiskState(filePath, members, maxCounter);

const CURRENT_FILE_VERSION_ENV = env.getNumber('SF_SOURCE_TRACKING_FILE_VERSION') ?? 0;
const contents =
CURRENT_FILE_VERSION_ENV === 1
Expand Down
165 changes: 165 additions & 0 deletions test/unit/remote/fileOperations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,171 @@ describe('writing file version based on env', () => {
});
});

describe('cross-process merge on write', () => {
const fakeOrgId = '00DFakeFakeFakeMrg';
const filePath = getFilePath(fakeOrgId);

afterEach(async () => {
delete process.env.SF_SOURCE_TRACKING_FILE_VERSION;
try {
await fs.promises.unlink(filePath);
} catch {
// may not exist
}
});

it('preserves higher lastRetrievedFromServer from disk when in-memory is stale', async () => {
const diskMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 50,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 50,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 50, members: diskMembers });

const staleMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 30,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 50,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 50, members: staleMembers });

const contents = await fs.promises.readFile(filePath, 'utf8');
const parsed = parseJsonMap<ContentsV0>(contents, filePath);
expect(parsed.sourceMembers['ApexClass__MyClass'].lastRetrievedFromServer).to.equal(50);
});

it('preserves higher lastRetrievedFromServer from memory when disk is stale', async () => {
const diskMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 30,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 50,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 30, members: diskMembers });

const freshMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 50,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 50,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 50, members: freshMembers });

const contents = await fs.promises.readFile(filePath, 'utf8');
const parsed = parseJsonMap<ContentsV0>(contents, filePath);
expect(parsed.sourceMembers['ApexClass__MyClass'].lastRetrievedFromServer).to.equal(50);
});

it('does not regress null over a valid lastRetrievedFromServer on disk', async () => {
const diskMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 26,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 26,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 26, members: diskMembers });

const staleMembers = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: undefined,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 26,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 26, members: staleMembers });

const contents = await fs.promises.readFile(filePath, 'utf8');
const parsed = parseJsonMap<ContentsV0>(contents, filePath);
expect(parsed.sourceMembers['ApexClass__MyClass'].lastRetrievedFromServer).to.equal(26);
});

it('takes the max serverMaxRevisionCounter between memory and disk', async () => {
const members = new Map<string, MemberRevision>([
[
'ApexClass###MyClass',
{
MemberIdOrName: '01p000000000001',
ChangedBy: 'user1',
lastRetrievedFromServer: 10,
MemberType: 'ApexClass',
IsNameObsolete: false,
RevisionCounter: 10,
MemberName: 'MyClass',
IsNewMember: false,
LastModifiedDate: '2026-01-01',
},
],
]);
await writeTrackingFile({ filePath, maxCounter: 100, members });
await writeTrackingFile({ filePath, maxCounter: 80, members });

const contents = await fs.promises.readFile(filePath, 'utf8');
const parsed = parseJsonMap<ContentsV0>(contents, filePath);
expect(parsed.serverMaxRevisionCounter).to.equal(100);
});
});

describe('upgrading undefined file version to v1 file', () => {
it('returns new file version even if file is not versioned', () => {
const oldFile = {
Expand Down
Loading