feat(git-sync): per-space toggle for conflict-marker handling on push (#13)
Red-team #13 (conflict markers reaching Docmost) is now a per-space policy exposed as a UI toggle, instead of a hardcoded behavior. New boolean `gitSync.autoMergeConflicts` (default FALSE), mirroring the existing per-space `gitSync.enabled` flag end-to-end (jsonb space settings -> update-space DTO -> space.service -> client types -> space settings form switch): - OFF (default, safe): a page whose committed body still has unresolved git conflict markers is NOT pushed — it is recorded as a per-page push FAILURE ("unresolved conflict markers — resolve in git first"). Recording a failure (not a soft skip) deliberately HOLDS refs/docmost/last-pushed so the conflict commit is never marked pushed and a later pull cannot clobber the user's in-progress resolution; the page retries until the conflict is resolved in git. - ON: the marker lines are stripped and both sides' content is pushed (the prior behavior), so the conflict becomes visible/fixable inside Docmost. The engine Settings carries `autoMergeConflicts`; runPush threads it into the update AND create paths. The orchestrator's buildSettings reads the per-space flag from jsonb (strict opt-in like `enabled`, default false). Tests: redteam-push-cycle #13 rewritten (default -> not pushed + failure + refs held; ON -> strip-and-push); space.service + edit-space-form + orchestrator specs extended. git-sync vitest 618, server jest space+git-sync 163, client edit-space-form 11, server/client tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -80,13 +80,19 @@ function renderForm(props: { space: ISpace; readOnly?: boolean }) {
|
||||
);
|
||||
}
|
||||
|
||||
// The git-sync toggle is the only switch on the form. Mantine renders it as an
|
||||
// <input type="checkbox" role="switch">; its label text lives in a sibling
|
||||
// wrapper, so query by role and assert the visible label is present alongside.
|
||||
// The form now renders TWO switches (git-sync enable + auto-merge-conflicts) in
|
||||
// that DOM order. Mantine renders each as an <input type="checkbox"
|
||||
// role="switch"> but does NOT expose its label as the accessible name, so we
|
||||
// disambiguate by DOM order (index 0 = enable, 1 = auto-merge) and assert the
|
||||
// human-readable label text is present alongside.
|
||||
function getToggle(): HTMLInputElement {
|
||||
// Sanity: the human-readable label is rendered.
|
||||
screen.getByText("Enable Git sync");
|
||||
return screen.getByRole("switch") as HTMLInputElement;
|
||||
return screen.getAllByRole("switch")[0] as HTMLInputElement;
|
||||
}
|
||||
|
||||
function getAutoMergeToggle(): HTMLInputElement {
|
||||
screen.getByText("Auto-merge conflicts on push");
|
||||
return screen.getAllByRole("switch")[1] as HTMLInputElement;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
@@ -169,3 +175,66 @@ describe("EditSpaceForm git-sync toggle", () => {
|
||||
expect(getToggle().disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("EditSpaceForm auto-merge-conflicts toggle", () => {
|
||||
it("derives initial checked state from space.settings.gitSync.autoMergeConflicts (true -> checked)", () => {
|
||||
renderForm({
|
||||
space: makeSpace({
|
||||
settings: { gitSync: { autoMergeConflicts: true } },
|
||||
}),
|
||||
});
|
||||
expect(getAutoMergeToggle().checked).toBe(true);
|
||||
});
|
||||
|
||||
it("defaults to unchecked when autoMergeConflicts is missing (SAFE default)", () => {
|
||||
renderForm({ space: makeSpace() });
|
||||
expect(getAutoMergeToggle().checked).toBe(false);
|
||||
});
|
||||
|
||||
it("fires the mutation with { spaceId, autoMergeConflicts } and optimistically flips on", async () => {
|
||||
mutateAsync.mockResolvedValue(undefined);
|
||||
renderForm({ space: makeSpace() });
|
||||
|
||||
const toggle = getAutoMergeToggle();
|
||||
expect(toggle.checked).toBe(false);
|
||||
|
||||
fireEvent.click(toggle);
|
||||
|
||||
// Optimistic update.
|
||||
expect(toggle.checked).toBe(true);
|
||||
expect(mutateAsync).toHaveBeenCalledTimes(1);
|
||||
expect(mutateAsync).toHaveBeenCalledWith({
|
||||
spaceId: "space-1",
|
||||
autoMergeConflicts: true,
|
||||
});
|
||||
|
||||
await waitFor(() => expect(toggle.checked).toBe(true));
|
||||
});
|
||||
|
||||
it("rolls back to its prior state when the mutation rejects", async () => {
|
||||
mutateAsync.mockRejectedValue(new Error("network"));
|
||||
renderForm({
|
||||
space: makeSpace({
|
||||
settings: { gitSync: { autoMergeConflicts: false } },
|
||||
}),
|
||||
});
|
||||
|
||||
const toggle = getAutoMergeToggle();
|
||||
expect(toggle.checked).toBe(false);
|
||||
|
||||
fireEvent.click(toggle);
|
||||
|
||||
expect(toggle.checked).toBe(true);
|
||||
expect(mutateAsync).toHaveBeenCalledWith({
|
||||
spaceId: "space-1",
|
||||
autoMergeConflicts: true,
|
||||
});
|
||||
|
||||
await waitFor(() => expect(toggle.checked).toBe(false));
|
||||
});
|
||||
|
||||
it("disables the toggle when readOnly", () => {
|
||||
renderForm({ space: makeSpace(), readOnly: true });
|
||||
expect(getAutoMergeToggle().disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -42,6 +42,10 @@ export function EditSpaceForm({ space, readOnly }: EditSpaceFormProps) {
|
||||
space?.settings?.gitSync?.enabled ?? false,
|
||||
);
|
||||
|
||||
const [autoMergeConflicts, setAutoMergeConflicts] = useState<boolean>(
|
||||
space?.settings?.gitSync?.autoMergeConflicts ?? false,
|
||||
);
|
||||
|
||||
const handleGitSyncToggle = async (value: boolean) => {
|
||||
const previous = gitSyncEnabled;
|
||||
setGitSyncEnabled(value); // optimistic update
|
||||
@@ -58,6 +62,20 @@ export function EditSpaceForm({ space, readOnly }: EditSpaceFormProps) {
|
||||
}
|
||||
};
|
||||
|
||||
const handleAutoMergeConflictsToggle = async (value: boolean) => {
|
||||
const previous = autoMergeConflicts;
|
||||
setAutoMergeConflicts(value); // optimistic update
|
||||
try {
|
||||
await updateSpaceMutation.mutateAsync({
|
||||
spaceId: space.id,
|
||||
autoMergeConflicts: value,
|
||||
});
|
||||
} catch (err) {
|
||||
setAutoMergeConflicts(previous); // revert on failure
|
||||
console.error("Failed to toggle git-sync auto-merge-conflicts", err);
|
||||
}
|
||||
};
|
||||
|
||||
const form = useForm<FormValues>({
|
||||
validate: zod4Resolver(formSchema),
|
||||
initialValues: {
|
||||
@@ -145,6 +163,19 @@ export function EditSpaceForm({ space, readOnly }: EditSpaceFormProps) {
|
||||
handleGitSyncToggle(event.currentTarget.checked)
|
||||
}
|
||||
/>
|
||||
|
||||
<Switch
|
||||
mt="md"
|
||||
label={t("Auto-merge conflicts on push")}
|
||||
description={t(
|
||||
"When off (recommended), a page whose content still has unresolved Git conflict markers is skipped on push until you resolve the conflict in Git. When on, the markers are stripped and both sides' content is pushed.",
|
||||
)}
|
||||
checked={autoMergeConflicts}
|
||||
disabled={readOnly || updateSpaceMutation.isPending}
|
||||
onChange={(event) =>
|
||||
handleAutoMergeConflictsToggle(event.currentTarget.checked)
|
||||
}
|
||||
/>
|
||||
</Box>
|
||||
</>
|
||||
);
|
||||
|
||||
@@ -15,6 +15,7 @@ export interface ISpaceCommentsSettings {
|
||||
|
||||
export interface ISpaceGitSyncSettings {
|
||||
enabled?: boolean;
|
||||
autoMergeConflicts?: boolean;
|
||||
}
|
||||
|
||||
export interface ISpaceSettings {
|
||||
@@ -41,6 +42,7 @@ export interface ISpace {
|
||||
disablePublicSharing?: boolean;
|
||||
allowViewerComments?: boolean;
|
||||
gitSyncEnabled?: boolean;
|
||||
autoMergeConflicts?: boolean;
|
||||
}
|
||||
|
||||
interface IMembership {
|
||||
|
||||
@@ -19,4 +19,8 @@ export class UpdateSpaceDto extends PartialType(CreateSpaceDto) {
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
gitSyncEnabled?: boolean;
|
||||
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
autoMergeConflicts?: boolean;
|
||||
}
|
||||
|
||||
@@ -151,5 +151,70 @@ describe('SpaceService', () => {
|
||||
expect(spaceRepo.updateGitSyncSettings).toHaveBeenCalledTimes(1);
|
||||
expect(auditService.log).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// --- autoMergeConflicts: a SECOND key in the SAME `gitSync` jsonb object,
|
||||
// persisted the same way as `enabled` (the repo's jsonb-merge keeps siblings).
|
||||
it('persists autoMergeConflicts via updateGitSyncSettings(autoMergeConflicts)', async () => {
|
||||
const { svc, spaceRepo } = buildService({});
|
||||
|
||||
await svc.updateSpace(
|
||||
{ spaceId, autoMergeConflicts: true } as any,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(spaceRepo.updateGitSyncSettings).toHaveBeenCalledWith(
|
||||
spaceId,
|
||||
workspaceId,
|
||||
'autoMergeConflicts',
|
||||
true,
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not call updateGitSyncSettings when autoMergeConflicts is undefined', async () => {
|
||||
const { svc, spaceRepo } = buildService({});
|
||||
|
||||
await svc.updateSpace({ spaceId } as any, workspaceId);
|
||||
|
||||
expect(spaceRepo.updateGitSyncSettings).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('writes a SPACE_UPDATED audit delta on a REAL autoMergeConflicts change (false -> true)', async () => {
|
||||
// Prior persisted state: gitSync.autoMergeConflicts = false; flip it on.
|
||||
const { svc, auditService } = buildService({
|
||||
gitSync: { autoMergeConflicts: false },
|
||||
});
|
||||
|
||||
await svc.updateSpace(
|
||||
{ spaceId, autoMergeConflicts: true } as any,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(auditService.log).toHaveBeenCalledTimes(1);
|
||||
expect(auditService.log).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
resourceId: spaceId,
|
||||
spaceId,
|
||||
changes: {
|
||||
before: expect.objectContaining({ autoMergeConflicts: false }),
|
||||
after: expect.objectContaining({ autoMergeConflicts: true }),
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('does NOT write an audit delta on a no-op autoMergeConflicts (same value true -> true)', async () => {
|
||||
const { svc, spaceRepo, auditService } = buildService({
|
||||
gitSync: { autoMergeConflicts: true },
|
||||
});
|
||||
|
||||
await svc.updateSpace(
|
||||
{ spaceId, autoMergeConflicts: true } as any,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(spaceRepo.updateGitSyncSettings).toHaveBeenCalledTimes(1);
|
||||
expect(auditService.log).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -229,6 +229,25 @@ export class SpaceService {
|
||||
);
|
||||
}
|
||||
|
||||
if (typeof updateSpaceDto.autoMergeConflicts !== 'undefined') {
|
||||
const prev = settingsBefore?.gitSync?.autoMergeConflicts ?? false;
|
||||
if (prev !== updateSpaceDto.autoMergeConflicts) {
|
||||
before.autoMergeConflicts = prev;
|
||||
after.autoMergeConflicts = updateSpaceDto.autoMergeConflicts;
|
||||
}
|
||||
|
||||
// Merges into the SAME `gitSync` jsonb object as `enabled` (the repo's
|
||||
// jsonb-merge preserves sibling keys), so toggling one never clobbers the
|
||||
// other.
|
||||
await this.spaceRepo.updateGitSyncSettings(
|
||||
updateSpaceDto.spaceId,
|
||||
workspaceId,
|
||||
'autoMergeConflicts',
|
||||
updateSpaceDto.autoMergeConflicts,
|
||||
trx,
|
||||
);
|
||||
}
|
||||
|
||||
updatedSpace = await this.spaceRepo.updateSpace(
|
||||
{
|
||||
name: updateSpaceDto.name,
|
||||
|
||||
@@ -122,7 +122,19 @@ function build(opts: BuildOptions = {}): Built {
|
||||
};
|
||||
const redisService = { getOrThrow: jest.fn(() => redis) };
|
||||
|
||||
const db = {};
|
||||
// Chainable Kysely stub. `buildSettings` reads the space's
|
||||
// `gitSync.autoMergeConflicts` flag via
|
||||
// `selectFrom('spaces').select(...).where('id','=',id).executeTakeFirst()`;
|
||||
// default it to the SAFE off value. `enabledSpaces` uses `.execute()`.
|
||||
const db = (() => {
|
||||
const builder: any = {
|
||||
select: () => builder,
|
||||
where: () => builder,
|
||||
executeTakeFirst: async () => ({ autoMergeConflicts: false }),
|
||||
execute: async () => [],
|
||||
};
|
||||
return { selectFrom: () => builder };
|
||||
})();
|
||||
|
||||
// The REAL SpaceLockService, constructed against the mock redis above, so all
|
||||
// existing lock assertions (lock-held, in-progress, leader lock, release CAS,
|
||||
|
||||
@@ -107,11 +107,24 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
|
||||
* datasource writes in-process — so they are placeholders; only `vaultPath`,
|
||||
* `gitRemote`, and the tunables are load-bearing.
|
||||
*/
|
||||
private buildSettings(spaceId: string): Settings {
|
||||
private async buildSettings(spaceId: string): Promise<Settings> {
|
||||
const remoteTemplate = this.environmentService.getGitSyncRemoteTemplate();
|
||||
const gitRemote = remoteTemplate
|
||||
? remoteTemplate.replace(/\{spaceId\}/g, spaceId)
|
||||
: undefined;
|
||||
// Per-space PUSH policy for still-conflicted page bodies (SPEC §9): read the
|
||||
// `gitSync.autoMergeConflicts` flag from the space's jsonb settings. STRICT
|
||||
// opt-in like `enabled` — anything other than the literal 'true' (absent, null,
|
||||
// 'false') resolves to the SAFE default (skip a conflicted page, do not push).
|
||||
const row = await this.db
|
||||
.selectFrom('spaces')
|
||||
.select(
|
||||
sql<boolean>`settings->'gitSync'->>'autoMergeConflicts' = 'true'`.as(
|
||||
'autoMergeConflicts',
|
||||
),
|
||||
)
|
||||
.where('id', '=', spaceId)
|
||||
.executeTakeFirst();
|
||||
return {
|
||||
docmostApiUrl: 'http://native.local',
|
||||
docmostEmail: 'native@local',
|
||||
@@ -122,6 +135,7 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
|
||||
pollIntervalMs: this.environmentService.getGitSyncPollIntervalMs(),
|
||||
debounceMs: this.environmentService.getGitSyncDebounceMs(),
|
||||
logLevel: 'info',
|
||||
autoMergeConflicts: row?.autoMergeConflicts ?? false,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -249,7 +263,7 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
|
||||
signal?: AbortSignal,
|
||||
): Promise<GitSyncRunStatus> {
|
||||
const { runCycle } = await loadGitSync();
|
||||
const settings = this.buildSettings(spaceId);
|
||||
const settings = await this.buildSettings(spaceId);
|
||||
const vault = await this.vaultRegistry.getVault(spaceId);
|
||||
const client = this.dataSource.bind({ workspaceId, userId: serviceUserId });
|
||||
const maxDeletes = this.environmentService.getGitSyncMaxDeletesPerCycle();
|
||||
|
||||
Reference in New Issue
Block a user