vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

feat(git-sync): remove the per-cycle delete cap; deletes apply + are logged every cycle

Browse Source 
The delete cap (GIT_SYNC_MAX_DELETES_PER_CYCLE, default 5) was a defense-in-depth
guard that SUPPRESSED a cycle's deletions when the planned count exceeded the
limit. In practice it was a crutch over engine correctness that also blocked
legitimate deletes: deleting a folder with many child pages is a normal action,
and git-sync deletes are SOFT (Trash, reversible), so a blocking limit has little
upside and real downside. There is also no user-facing surface to "confirm" a
large delete from a background sync — the only channel is the operator log.

So: drop the cap entirely. Deletes apply unconditionally; every cycle already
logs its full push plan, per-action `delete: <pageId>` lines, and completion
counts through the engine `log`, so what was deleted (and what was skipped) is
always recorded. Engine correctness (the reconcile/layout/round-trip tests) is
what prevents phantom deletions — not a blocking cap.

Removed: orchestrator `resolveApplyClient` cap hook + `maxDeletes`,
`getGitSyncMaxDeletesPerCycle`, the `GIT_SYNC_MAX_DELETES_PER_CYCLE` env/validation/.env.example,
and the cap tests. (The engine's generic optional `resolveApplyClient` hook is
left as an unused extension point.)

server tsc clean, git-sync + environment jest 174.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-26 03:59:12 +03:00
parent afaafe11e1
commit 0580b2a107
6 changed files with 9 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.env.example
View File

@@ -235,7 +235,3 @@ MCP_DOCMOST_PASSWORD=
# after this deadline so it cannot hold the per-space lock forever.
# GIT_SYNC_BACKEND_TIMEOUT_MS=120000
#
# Defense-in-depth absolute cap on soft-deletes applied per push cycle
# (default: 5). A non-convergent / phantom-absence cycle can never trash more
# than this many pages without an explicit override.
# GIT_SYNC_MAX_DELETES_PER_CYCLE=5

21
apps/server/src/integrations/environment/environment.service.spec.ts
View File

@@ -15,27 +15,6 @@ describe('EnvironmentService', () => {
expect(service).toBeDefined();
});
describe('getGitSyncMaxDeletesPerCycle', () => {
const withEnv = (value?: string) =>
new EnvironmentService({
get: (_key: string, fallback?: string) => value ?? fallback,
} as any);
it('defaults to 5 when unset', () => {
expect(withEnv().getGitSyncMaxDeletesPerCycle()).toBe(5);
});
it('parses a valid positive int', () => {
expect(withEnv('12').getGitSyncMaxDeletesPerCycle()).toBe(12);
});
it('falls back to 5 for non-positive or unparseable values', () => {
expect(withEnv('0').getGitSyncMaxDeletesPerCycle()).toBe(5);
expect(withEnv('-3').getGitSyncMaxDeletesPerCycle()).toBe(5);
expect(withEnv('not-a-number').getGitSyncMaxDeletesPerCycle()).toBe(5);
});
});
describe('getGitSyncPollIntervalMs', () => {
const withEnv = (value?: string) =>
new EnvironmentService({

14
apps/server/src/integrations/environment/environment.service.ts
View File

@@ -399,20 +399,6 @@ export class EnvironmentService {
return Number.isFinite(parsed) && parsed > 0 ? parsed : 2000;
}
/**
* Defense-in-depth absolute cap on how many pages a single push cycle may
* soft-delete (default 5). A non-convergent / phantom-absence cycle whose push
* plan would delete more than this is forced to skip deletions that cycle (the
* orchestrator logs a WARNING). A non-positive or unparseable value falls back
* to the default 5 so the cap can never be silently disabled.
*/
getGitSyncMaxDeletesPerCycle(): number {
const parsed = parseInt(
this.configService.get<string>('GIT_SYNC_MAX_DELETES_PER_CYCLE', '5'),
10,
);
return Number.isFinite(parsed) && parsed > 0 ? parsed : 5;
}
/**
* The service user id git-sync writes are attributed to. Required when sync is

6
apps/server/src/integrations/environment/environment.validation.ts
View File

@@ -209,12 +209,6 @@ export class EnvironmentVariables {
@IsString()
GIT_SYNC_BACKEND_TIMEOUT_MS: string;
// Defense-in-depth absolute cap on soft-deletes per push cycle (default 5): a
// non-convergent / phantom-absence cycle can never trash more than this many
// pages without an explicit override. Optional int (validated as a string env).
@IsOptional()
@IsString()
GIT_SYNC_MAX_DELETES_PER_CYCLE: string;
// Required when git-sync is enabled: the service user create/move/rename/delete
// are attributed to (issue #194 §7.2). Optional otherwise.

66
apps/server/src/integrations/git-sync/services/git-sync.orchestrator.spec.ts
View File

@@ -1,7 +1,7 @@
// Unit tests for the git-sync control plane. The engine's `runCycle`
// (which owns the PULL->PUSH branch choreography) is mocked so we exercise ONLY
// the orchestrator's wiring: gating, the Redis leader lock + in-process mutex
// (via SpaceLockService), the delete-cap POLICY it injects as `resolveApplyClient`,
// (via SpaceLockService),
// the remote-template substitution in the settings it hands the engine, the
// external-push ingest, and the idempotent interval lifecycle. The cycle
// mechanics themselves are covered by the engine's own cycle round-trip spec.
@@ -48,7 +48,6 @@ interface BuildOptions {
/** Env tunables (only the load-bearing ones are surfaced as overrides). */
enabled?: boolean;
serviceUserId?: string | undefined;
maxDeletes?: number;
remoteTemplate?: string | undefined;
dataDir?: string;
pollIntervalMs?: number;
@@ -73,7 +72,6 @@ interface Built {
function build(opts: BuildOptions = {}): Built {
const {
enabled = true,
maxDeletes = 100,
remoteTemplate = undefined,
dataDir = '/vaults',
pollIntervalMs = 15000,
@@ -87,7 +85,6 @@ function build(opts: BuildOptions = {}): Built {
const env: Record<string, AnyMock> = {
isGitSyncEnabled: jest.fn(() => enabled),
getGitSyncServiceUserId: jest.fn(() => serviceUserId),
getGitSyncMaxDeletesPerCycle: jest.fn(() => maxDeletes),
getGitSyncRemoteTemplate: jest.fn(() => remoteTemplate),
getGitSyncDataDir: jest.fn(() => dataDir),
getGitSyncPollIntervalMs: jest.fn(() => pollIntervalMs),
@@ -177,15 +174,6 @@ function primeEngineHappyPath(): void {
runCycleMock.mockResolvedValue(OK_CYCLE);
}
/** Pull the `resolveApplyClient` hook out of the (single) runCycle call args. */
function lastResolveApplyClient(): (
plannedDeletes: number,
client: unknown,
) => unknown {
const calls = runCycleMock.mock.calls;
return calls[calls.length - 1][0].resolveApplyClient;
}
beforeEach(() => {
jest.clearAllMocks();
primeEngineHappyPath();
@@ -295,7 +283,6 @@ describe('GitSyncOrchestrator', () => {
expect(deps.vault).toBe(built.vault);
expect(deps.client).toBe(built.client);
expect(deps.settings.vaultPath).toBe('/vaults/space-1');
expect(typeof deps.resolveApplyClient).toBe('function');
// The bound datasource identity is the (workspace, service-user) pair.
expect(built.dataSource.bind).toHaveBeenCalledWith({
workspaceId: 'ws-1',
@@ -390,57 +377,6 @@ describe('GitSyncOrchestrator', () => {
});
});
describe('delete cap (anti-data-loss)', () => {
// The cap is now a POLICY the orchestrator injects as runCycle's
// `resolveApplyClient` hook; the engine calls it with the dry-run's planned
// delete count. We pull the hook out of the runCycle args and exercise it.
it('suppresses deletePage (by throwing) when planned deletes exceed the cap', async () => {
jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
const built = build({ maxDeletes: 5 });
await built.orchestrator.runOnce('space-1', 'ws-1');
const resolveApplyClient = lastResolveApplyClient();
const applyClient = resolveApplyClient(9, built.client) as any;
// deletePage is still a function (the engine calls it)...
expect(typeof applyClient.deletePage).toBe('function');
// ...but it THROWS, so the engine records a per-page failure and holds
// `last-pushed` (a resolving no-op would advance past the dropped deletes
// and never replay them — PR #119 review). The real deletePage is NOT hit.
await expect(applyClient.deletePage('p1')).rejects.toThrow(/suppress/i);
expect(built.client.deletePage).not.toHaveBeenCalled();
// Creates/updates pass through to the real client.
expect(applyClient.createPage).toBe(built.client.createPage);
});
it('fails safe: an Infinity planned-delete count (dry-run failed) is suppressed', async () => {
jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
const built = build({ maxDeletes: 5 });
await built.orchestrator.runOnce('space-1', 'ws-1');
// runCycle passes Number.POSITIVE_INFINITY when its dry-run planning threw;
// Infinity > cap → suppress (same throwing path).
const resolveApplyClient = lastResolveApplyClient();
const applyClient = resolveApplyClient(
Number.POSITIVE_INFINITY,
built.client,
) as any;
await expect(applyClient.deletePage('p1')).rejects.toThrow(/suppress/i);
expect(built.client.deletePage).not.toHaveBeenCalled();
});
it('passes through the original client when planned deletes are within the cap', async () => {
const built = build({ maxDeletes: 5 });
await built.orchestrator.runOnce('space-1', 'ws-1');
const resolveApplyClient = lastResolveApplyClient();
const applyClient = resolveApplyClient(3, built.client) as any;
// The ORIGINAL client is used (deletePage forwards to the real one).
expect(applyClient).toBe(built.client);
await applyClient.deletePage('p1');
expect(built.client.deletePage).toHaveBeenCalledWith('p1');
});
});
describe('remote template substitution', () => {
it('substitutes {spaceId} into the gitRemote settings handed to the engine', async () => {
const built = build({ remoteTemplate: 'git@h:vault-{spaceId}.git' });

39
apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts
View File

@@ -253,8 +253,8 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
* Drive ONE reconcile cycle for a space. The PULL->PUSH branch choreography
* lives in the engine's `runCycle` (so it can never drift from the engine it
* ships with); the orchestrator owns only the lock (its caller) and the
* gitmost-specific delete-cap POLICY, injected here as the `resolveApplyClient`
* hook.
* service binding. There is no delete cap — deletes apply unconditionally (they
* are soft/reversible) and every cycle logs what it deleted via `log`.
*/
private async driveCycle(
spaceId: string,
@@ -266,7 +266,6 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
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();
const result = await runCycle({
// Cooperative-abort signal from the per-space lock: if a heartbeat refresh
@@ -284,35 +283,13 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
mkdir: (absDir) => mkdir(absDir, { recursive: true }).then(() => undefined),
rm: (absPath) => rm(absPath, { force: true }),
},
// Every cycle logs its full push plan + per-action lines + completion
// counts (created/updated/deleted/skipped/failures) through this `log`, so
// what was deleted (and what was not) is always recorded. There is no
// delete cap: deletes are soft (Trash, reversible), so a blocking limit
// only got in the way of legitimate deletes; engine correctness (covered by
// the reconcile/layout tests) is what prevents phantom deletions.
log: (line: string) => this.logger.log(`git-sync[${spaceId}] ${line}`),
// DEFENSE-IN-DEPTH delete cap (gitmost-specific policy). A non-convergent
// vault (e.g. empty/duplicate titles -> colliding paths) can compute
// PHANTOM absence-deletions. When the push's planned delete count exceeds
// GIT_SYNC_MAX_DELETES_PER_CYCLE (or planning failed -> Infinity), suppress
// deletes by making deletePage THROW: the engine records each as a per-page
// failure, which keeps `refs/docmost/last-pushed` from advancing past the
// dropped-file commit, so the deletion is RETRIED next cycle rather than
// silently dropped (a no-op that resolved would advance the ref and a pull
// would then recreate the user's deleted files). See PR #119 review.
resolveApplyClient: (plannedDeletes, c) => {
if (plannedDeletes <= maxDeletes) return c;
this.logger.warn(
`git-sync[${spaceId}]: push delete count ${plannedDeletes} exceeds ` +
`GIT_SYNC_MAX_DELETES_PER_CYCLE=${maxDeletes}; suppressing deletions ` +
`this cycle (possible non-convergence / collision). Investigate vault ` +
`layout.`,
);
return {
...c,
deletePage: async () => {
throw new Error(
'git-sync: delete suppressed this cycle ' +
'(over GIT_SYNC_MAX_DELETES_PER_CYCLE) — refs intentionally held ' +
'so the deletion is retried, not dropped',
);
},
};
},
});
return { spaceId, ...result };