feat(git-sync): remove the per-cycle delete cap; deletes apply + are logged every cycle
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:
@@ -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({
|
||||
|
||||
@@ -411,20 +411,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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user