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:
claude code agent 227
2026-06-26 01:56:55 +03:00
parent b751c4bdc5
commit 687482a901
11 changed files with 336 additions and 25 deletions

View File

@@ -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);
});
});

View File

@@ -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>
</>
);

View File

@@ -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 {

View File

@@ -19,4 +19,8 @@ export class UpdateSpaceDto extends PartialType(CreateSpaceDto) {
@IsOptional()
@IsBoolean()
gitSyncEnabled?: boolean;
@IsOptional()
@IsBoolean()
autoMergeConflicts?: boolean;
}

View File

@@ -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();
});
});
});

View File

@@ -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,

View File

@@ -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,

View File

@@ -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();

View File

@@ -488,8 +488,27 @@ export interface ApplyPushDeps {
* folder's `.md` at `refs/docmost/last-pushed`, SPEC §5 path-as-truth).
*/
git: Pick<VaultGit, "updateRef" | "fastForwardBranch" | "showFileAtRef">;
/**
* Per-space PUSH policy for a page body that still carries unresolved git
* conflict markers (SPEC §9). When TRUE, the marker lines are stripped and both
* sides' content is pushed (the legacy `stripConflictMarkers` behavior). When
* FALSE/undefined (the SAFE DEFAULT), the conflicted page is NOT pushed: it is
* recorded as a per-page FAILURE (so the refs are not advanced and the page is
* retried) and the user resolves the git conflict first.
*/
autoMergeConflicts?: boolean;
}
/**
* Reason recorded on a per-page push FAILURE when a page is skipped because its
* body still carries unresolved git conflict markers and `autoMergeConflicts` is
* off (the SAFE default). Recorded as a failure (not a soft skip) on purpose: it
* HOLDS the refs so the conflict commit is never marked as pushed and the page is
* retried until the human resolves the conflict in git (SPEC §9).
*/
export const CONFLICT_MARKERS_FAILURE_REASON =
"unresolved conflict markers — resolve in git first";
/** A file whose meta was rewritten with a freshly-assigned pageId (post-create). */
export interface WrittenBackPage {
path: string;
@@ -665,11 +684,22 @@ export async function applyPushActions(
// Push the CLEAN body only (no `gitmost_id` frontmatter): the frontmatter
// is engine metadata, never page content. The server converts the markdown
// it receives verbatim, so stripping here keeps the id out of Docmost.
// Also strip any git conflict markers — they must NEVER reach Docmost
// (SPEC §9, red-team #13); content on both sides is preserved.
const body = stripConflictMarkers(
parsePageFile(await deps.readFile(u.path)).body,
);
const rawBody = parsePageFile(await deps.readFile(u.path)).body;
// Git conflict markers must NEVER reach Docmost (SPEC §9, red-team #13).
// Per-space policy (`autoMergeConflicts`): when OFF (the SAFE default), a
// still-conflicted body is NOT pushed — record a failure so the refs are
// held and the page is retried once the human resolves the conflict in git.
// When ON, strip the marker lines and push both sides' content.
if (!deps.autoMergeConflicts && hasConflictMarkers(rawBody)) {
failures.push({
kind: "update",
pageId: u.pageId,
path: u.path,
error: CONFLICT_MARKERS_FAILURE_REASON,
});
continue;
}
const body = stripConflictMarkers(rawBody);
// The last-synced version of this file (pre-image) is the common ancestor
// for a 3-way merge against the live page, so concurrent human edits are
// not clobbered (review #5). Null when the file is new at last-pushed. Its
@@ -744,9 +774,21 @@ export async function applyPushActions(
for (const c of orderedCreates) {
try {
const text = await deps.readFile(c.path);
// Conflict markers must never reach Docmost (SPEC §9, red-team #13); strip
// them from the create body too, preserving both sides' content.
const body = stripConflictMarkers(parsePageFile(text).body);
const rawBody = parsePageFile(text).body;
// Conflict markers must never reach Docmost (SPEC §9, red-team #13). Honor
// the per-space `autoMergeConflicts` policy on the create path too: OFF (the
// SAFE default) records a failure (refs held, retried) rather than creating
// a page from conflicted content; ON strips the markers and pushes both
// sides' content.
if (!deps.autoMergeConflicts && hasConflictMarkers(rawBody)) {
failures.push({
kind: "create",
path: c.path,
error: CONFLICT_MARKERS_FAILURE_REASON,
});
continue;
}
const body = stripConflictMarkers(rawBody);
// Derive create args from the PATH (native-Obsidian, SPEC §5): title from
// the filename, parent from the enclosing folder's folder-note, space from
// the run (the vault's space). `parentPageId: null` -> created at ROOT.
@@ -1475,6 +1517,9 @@ export async function runPush(
readFile: deps.readFile,
writeFile: deps.writeFile,
spaceId: settings.docmostSpaceId,
// Per-space PUSH policy for still-conflicted bodies (SPEC §9). Default OFF:
// a conflicted page is skipped (recorded as a failure) instead of pushed.
autoMergeConflicts: settings.autoMergeConflicts ?? false,
},
actions,
pushedCommit,

View File

@@ -17,4 +17,15 @@ export type Settings = {
pollIntervalMs: number;
debounceMs: number;
logLevel: 'debug' | 'info' | 'warn' | 'error';
/**
* Per-space PUSH policy for a page whose committed body still contains
* unresolved git conflict markers (`<<<<<<<` / `=======` / `>>>>>>>`):
* - false (DEFAULT, SAFE): SKIP that page's push (it is recorded as a push
* failure, so refs are NOT advanced) — the user must resolve the git
* conflict first before the page reaches Docmost.
* - true: strip the marker lines and push BOTH sides' content (the
* `stripConflictMarkers` behavior).
* Optional/undefined is treated as false.
*/
autoMergeConflicts?: boolean;
};

View File

@@ -24,9 +24,11 @@ function makeSettings(): Settings {
// ---------------------------------------------------------------------------
// #13 — conflict markers must never reach Docmost (SPEC §9), even when there is
// NO in-progress merge (markers committed on `main` by some other path). The
// push apply reads the body and hands it to importPageMarkdown verbatim; the
// DESIRED behavior is a content scan that prevents a `<<<<<<<` body from being
// pushed. Assert the pushed body does NOT contain a conflict marker.
// behavior is now gated by the per-space `autoMergeConflicts` setting:
// - DEFAULT (off): a still-conflicted page is NOT pushed — it is recorded as a
// per-page FAILURE and the refs are NOT advanced, so the user resolves the
// git conflict first.
// - ON: the marker lines are stripped and both sides' content is pushed.
// ---------------------------------------------------------------------------
function makePushGit(opts: {
changes: { status: 'A' | 'M' | 'D' | 'R' | 'C'; path: string; oldPath?: string }[];
@@ -60,11 +62,14 @@ function makePushGit(opts: {
}
describe('#13 conflict markers reach Docmost', () => {
it('does NOT push a body containing a `<<<<<<< HEAD` conflict marker', async () => {
const conflictBody =
'<<<<<<< HEAD\nmy line\n=======\ntheir line\n>>>>>>> feature\n';
const conflictBody =
'<<<<<<< HEAD\nmy line\n=======\ntheir line\n>>>>>>> feature\n';
function makeConflictDeps(settings: Settings) {
const file = serializePageFile('p-1', conflictBody);
const { git } = makePushGit({ changes: [{ status: 'M', path: 'Doc.md' }] });
const { git, calls } = makePushGit({
changes: [{ status: 'M', path: 'Doc.md' }],
});
const importPageMarkdown = vi.fn(async () => ({ success: true }));
const client = {
@@ -77,7 +82,7 @@ describe('#13 conflict markers reach Docmost', () => {
};
const deps: PushDeps = {
settings: makeSettings(),
settings,
git,
makeClient: () => client as any,
readFile: vi.fn(async (path: string) => {
@@ -87,6 +92,38 @@ describe('#13 conflict markers reach Docmost', () => {
writeFile: vi.fn(async () => {}),
log: () => {},
};
return { deps, importPageMarkdown, calls };
}
it('DEFAULT (autoMergeConflicts off): does NOT push a conflicted page; records a failure and holds the refs', async () => {
// makeSettings() leaves autoMergeConflicts undefined -> the SAFE default.
const { deps, importPageMarkdown, calls } = makeConflictDeps(makeSettings());
const res = await runPush(deps, { dryRun: false });
expect(res.mode).toBe('apply');
// The conflicted page is NOT pushed to Docmost at all.
expect(importPageMarkdown).not.toHaveBeenCalled();
// It is recorded as a per-page failure (so the user resolves the git conflict
// first), and because there is a failure the last-pushed ref is NOT advanced.
expect(res.applied?.failures).toEqual([
expect.objectContaining({
kind: 'update',
pageId: 'p-1',
path: 'Doc.md',
}),
]);
expect(res.applied?.failures[0].error).toMatch(/conflict markers/i);
expect(res.applied?.lastPushedAdvanced).toBe(false);
expect(calls.updateRef).toHaveLength(0);
});
it('autoMergeConflicts on: strips the markers and pushes a clean body', async () => {
const { deps, importPageMarkdown } = makeConflictDeps({
...makeSettings(),
autoMergeConflicts: true,
});
const res = await runPush(deps, { dryRun: false });
expect(res.mode).toBe('apply');
@@ -95,10 +132,12 @@ describe('#13 conflict markers reach Docmost', () => {
expect(importPageMarkdown).toHaveBeenCalledTimes(1);
const pushedBody: string = importPageMarkdown.mock.calls[0][1] as any;
// DESIRED: a content scan gates conflict markers; the body must be clean.
// The marker SYNTAX is stripped; both sides' content survives.
expect(pushedBody).not.toContain('<<<<<<<');
expect(pushedBody).not.toContain('=======');
expect(pushedBody).not.toContain('>>>>>>>');
expect(pushedBody).toContain('my line');
expect(pushedBody).toContain('their line');
});
});