fix(sync): honor per-spoon auto-sync settings
This commit is contained in:
@@ -190,7 +190,7 @@ export const createForkSpoonRecord = internalMutation({
|
||||
ownerId: args.ownerId,
|
||||
autoRefreshEnabled: true,
|
||||
autoReviewEnabled: true,
|
||||
autoSyncEnabled: false,
|
||||
autoSyncEnabled: true,
|
||||
requireAiLowRiskForSync: true,
|
||||
requireCleanCompareForSync: true,
|
||||
ignoredFilePatterns: [],
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
listPullRequests,
|
||||
syncForkBranch,
|
||||
} from './githubClient';
|
||||
import { shouldAutoSync, shouldCreateReviewThread } from './syncGating';
|
||||
|
||||
const getRequiredUserId = async (ctx: ActionCtx) => {
|
||||
const userId = await getAuthUserId(ctx);
|
||||
@@ -69,6 +70,16 @@ const refreshOwnedSpoon = async (
|
||||
'GitHub refresh is only available for GitHub Spoons.',
|
||||
);
|
||||
}
|
||||
const settings = await ctx.runQuery(internal.spoonSettings.getInternal, {
|
||||
spoonId,
|
||||
ownerId,
|
||||
});
|
||||
const autoSyncEnabled = settings?.autoSyncEnabled ?? false;
|
||||
const autoReviewEnabled = settings?.autoReviewEnabled ?? true;
|
||||
const requireCleanCompareForSync =
|
||||
settings?.requireCleanCompareForSync ?? true;
|
||||
// requireAiLowRiskForSync applies to the future AI review-to-auto-merge path,
|
||||
// which is deliberately deferred until Phase 4.
|
||||
const connection = await ctx.runQuery(internal.github.getConnectionForUser, {
|
||||
userId: ownerId,
|
||||
});
|
||||
@@ -202,7 +213,13 @@ const refreshOwnedSpoon = async (
|
||||
summary: `GitHub refresh complete: ${upstreamCompare.aheadBy} upstream commit(s), ${forkCompare.aheadBy} fork-only commit(s).`,
|
||||
});
|
||||
|
||||
if (status === 'behind' && forkCompare.aheadBy === 0 && allowAutoSync) {
|
||||
if (
|
||||
allowAutoSync &&
|
||||
shouldAutoSync(
|
||||
{ autoSyncEnabled, requireCleanCompareForSync },
|
||||
{ status, forkAheadBy: forkCompare.aheadBy },
|
||||
)
|
||||
) {
|
||||
try {
|
||||
await syncForkBranch(octokit, {
|
||||
forkOwner,
|
||||
@@ -258,7 +275,7 @@ const refreshOwnedSpoon = async (
|
||||
}
|
||||
}
|
||||
|
||||
if (status === 'diverged') {
|
||||
if (shouldCreateReviewThread({ autoReviewEnabled }, { status })) {
|
||||
const threadId = await ctx.runMutation(
|
||||
internal.threads.createMaintenanceThread,
|
||||
{
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import { v } from 'convex/values';
|
||||
|
||||
import { internalMutation } from './_generated/server';
|
||||
|
||||
// One-shot backfill: preserve existing automatic fast-forward behavior for
|
||||
// GitHub spoons now that refreshes honor autoSyncEnabled. Idempotent.
|
||||
export const backfillAutoSyncDefaults = internalMutation({
|
||||
args: {},
|
||||
returns: v.object({ updated: v.number() }),
|
||||
handler: async (ctx) => {
|
||||
const settings = await ctx.db.query('spoonSettings').collect();
|
||||
let updated = 0;
|
||||
for (const row of settings) {
|
||||
if (row.autoSyncEnabled) continue;
|
||||
const spoon = await ctx.db.get(row.spoonId);
|
||||
if (spoon?.provider !== 'github') continue;
|
||||
await ctx.db.patch(row._id, {
|
||||
autoSyncEnabled: true,
|
||||
updatedAt: Date.now(),
|
||||
});
|
||||
updated += 1;
|
||||
}
|
||||
return { updated };
|
||||
},
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
export const shouldAutoSync = (
|
||||
settings: {
|
||||
autoSyncEnabled: boolean;
|
||||
requireCleanCompareForSync: boolean;
|
||||
},
|
||||
ctx: { status: string; forkAheadBy: number },
|
||||
): boolean => {
|
||||
if (!settings.autoSyncEnabled) return false;
|
||||
if (ctx.status !== 'behind') return false;
|
||||
if (settings.requireCleanCompareForSync && ctx.forkAheadBy !== 0) {
|
||||
return false;
|
||||
}
|
||||
return ctx.forkAheadBy === 0;
|
||||
};
|
||||
|
||||
export const shouldCreateReviewThread = (
|
||||
settings: { autoReviewEnabled: boolean },
|
||||
ctx: { status: string },
|
||||
): boolean => settings.autoReviewEnabled && ctx.status === 'diverged';
|
||||
@@ -0,0 +1,167 @@
|
||||
import { convexTest } from 'convex-test';
|
||||
import { makeFunctionReference } from 'convex/server';
|
||||
import { describe, expect, test } from 'vitest';
|
||||
|
||||
import { internal } from '../../convex/_generated/api.js';
|
||||
import schema from '../../convex/schema';
|
||||
import {
|
||||
shouldAutoSync,
|
||||
shouldCreateReviewThread,
|
||||
} from '../../convex/syncGating';
|
||||
|
||||
const modules = import.meta.glob('../../convex/**/*.*s');
|
||||
const backfillAutoSyncDefaults = makeFunctionReference<
|
||||
'mutation',
|
||||
Record<string, never>,
|
||||
{ updated: number }
|
||||
>('migrations:backfillAutoSyncDefaults');
|
||||
|
||||
describe('syncGating', () => {
|
||||
test('auto-sync requires the flag and a clean behind compare', () => {
|
||||
expect(
|
||||
shouldAutoSync(
|
||||
{ autoSyncEnabled: false, requireCleanCompareForSync: true },
|
||||
{ status: 'behind', forkAheadBy: 0 },
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldAutoSync(
|
||||
{ autoSyncEnabled: true, requireCleanCompareForSync: true },
|
||||
{ status: 'behind', forkAheadBy: 0 },
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldAutoSync(
|
||||
{ autoSyncEnabled: true, requireCleanCompareForSync: true },
|
||||
{ status: 'behind', forkAheadBy: 2 },
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldAutoSync(
|
||||
{ autoSyncEnabled: true, requireCleanCompareForSync: true },
|
||||
{ status: 'diverged', forkAheadBy: 0 },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test('review thread requires autoReviewEnabled and diverged', () => {
|
||||
expect(
|
||||
shouldCreateReviewThread(
|
||||
{ autoReviewEnabled: false },
|
||||
{ status: 'diverged' },
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldCreateReviewThread(
|
||||
{ autoReviewEnabled: true },
|
||||
{ status: 'diverged' },
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldCreateReviewThread(
|
||||
{ autoReviewEnabled: true },
|
||||
{ status: 'behind' },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test('backfill enables auto-sync only for disabled GitHub spoons', async () => {
|
||||
const t = convexTest(schema, modules);
|
||||
const seeded = await t.mutation(async (ctx) => {
|
||||
const now = Date.now();
|
||||
const ownerId = await ctx.db.insert('users', {
|
||||
email: 'owner@example.com',
|
||||
name: 'Owner',
|
||||
});
|
||||
const createSpoon = async (
|
||||
name: string,
|
||||
provider: 'github' | 'gitea',
|
||||
autoSyncEnabled: boolean,
|
||||
) => {
|
||||
const spoonId = await ctx.db.insert('spoons', {
|
||||
ownerId,
|
||||
name,
|
||||
provider,
|
||||
upstreamOwner: 'upstream',
|
||||
upstreamRepo: name,
|
||||
upstreamDefaultBranch: 'main',
|
||||
upstreamUrl: `https://example.com/upstream/${name}`,
|
||||
forkOwner: 'owner',
|
||||
forkRepo: name,
|
||||
forkUrl: `https://example.com/owner/${name}`,
|
||||
visibility: 'private',
|
||||
maintenanceMode: 'watch',
|
||||
syncCadence: 'daily',
|
||||
productionRefStrategy: 'default_branch',
|
||||
status: 'active',
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
const settingsId = await ctx.db.insert('spoonSettings', {
|
||||
spoonId,
|
||||
ownerId,
|
||||
autoRefreshEnabled: true,
|
||||
autoReviewEnabled: true,
|
||||
autoSyncEnabled,
|
||||
requireAiLowRiskForSync: true,
|
||||
requireCleanCompareForSync: true,
|
||||
ignoredFilePatterns: [],
|
||||
importantFilePatterns: [],
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
return settingsId;
|
||||
};
|
||||
|
||||
return {
|
||||
disabledGithub: await createSpoon('disabled-github', 'github', false),
|
||||
enabledGithub: await createSpoon('enabled-github', 'github', true),
|
||||
disabledGitea: await createSpoon('disabled-gitea', 'gitea', false),
|
||||
};
|
||||
});
|
||||
|
||||
const result = await t.mutation(backfillAutoSyncDefaults, {});
|
||||
const settings = await t.run(async (ctx) => ({
|
||||
disabledGithub: await ctx.db.get(seeded.disabledGithub),
|
||||
enabledGithub: await ctx.db.get(seeded.enabledGithub),
|
||||
disabledGitea: await ctx.db.get(seeded.disabledGitea),
|
||||
}));
|
||||
|
||||
expect(result).toEqual({ updated: 1 });
|
||||
expect(settings.disabledGithub?.autoSyncEnabled).toBe(true);
|
||||
expect(settings.enabledGithub?.autoSyncEnabled).toBe(true);
|
||||
expect(settings.disabledGitea?.autoSyncEnabled).toBe(false);
|
||||
});
|
||||
|
||||
test('new GitHub spoons default auto-sync on', async () => {
|
||||
const t = convexTest(schema, modules);
|
||||
const ownerId = await t.mutation(async (ctx) => {
|
||||
return await ctx.db.insert('users', {
|
||||
email: 'new-owner@example.com',
|
||||
name: 'New owner',
|
||||
});
|
||||
});
|
||||
|
||||
const spoonId = await t.mutation(internal.github.createForkSpoonRecord, {
|
||||
ownerId,
|
||||
name: 'github-spoon',
|
||||
upstreamOwner: 'upstream',
|
||||
upstreamRepo: 'repo',
|
||||
upstreamDefaultBranch: 'main',
|
||||
upstreamUrl: 'https://github.com/upstream/repo',
|
||||
forkOwner: 'new-owner',
|
||||
forkRepo: 'repo',
|
||||
forkDefaultBranch: 'main',
|
||||
forkUrl: 'https://github.com/new-owner/repo',
|
||||
visibility: 'private',
|
||||
});
|
||||
const settings = await t.run(async (ctx) => {
|
||||
return await ctx.db
|
||||
.query('spoonSettings')
|
||||
.withIndex('by_spoon', (q) => q.eq('spoonId', spoonId))
|
||||
.unique();
|
||||
});
|
||||
|
||||
expect(settings?.autoSyncEnabled).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user