From bdb2964e34ebf5ccb5ce6cc324f14f69dd3e5cd7 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:12:34 -0400 Subject: [PATCH] fix(sync): honor per-spoon auto-sync settings --- packages/backend/convex/github.ts | 2 +- packages/backend/convex/githubSync.ts | 21 ++- packages/backend/convex/migrations.ts | 25 +++ packages/backend/convex/syncGating.ts | 19 ++ .../backend/tests/unit/sync-gating.test.ts | 167 ++++++++++++++++++ 5 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 packages/backend/convex/migrations.ts create mode 100644 packages/backend/convex/syncGating.ts create mode 100644 packages/backend/tests/unit/sync-gating.test.ts diff --git a/packages/backend/convex/github.ts b/packages/backend/convex/github.ts index 38f91ce..c7886f1 100644 --- a/packages/backend/convex/github.ts +++ b/packages/backend/convex/github.ts @@ -190,7 +190,7 @@ export const createForkSpoonRecord = internalMutation({ ownerId: args.ownerId, autoRefreshEnabled: true, autoReviewEnabled: true, - autoSyncEnabled: false, + autoSyncEnabled: true, requireAiLowRiskForSync: true, requireCleanCompareForSync: true, ignoredFilePatterns: [], diff --git a/packages/backend/convex/githubSync.ts b/packages/backend/convex/githubSync.ts index 8515326..9f07e28 100644 --- a/packages/backend/convex/githubSync.ts +++ b/packages/backend/convex/githubSync.ts @@ -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, { diff --git a/packages/backend/convex/migrations.ts b/packages/backend/convex/migrations.ts new file mode 100644 index 0000000..e86db72 --- /dev/null +++ b/packages/backend/convex/migrations.ts @@ -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 }; + }, +}); diff --git a/packages/backend/convex/syncGating.ts b/packages/backend/convex/syncGating.ts new file mode 100644 index 0000000..17eb489 --- /dev/null +++ b/packages/backend/convex/syncGating.ts @@ -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'; diff --git a/packages/backend/tests/unit/sync-gating.test.ts b/packages/backend/tests/unit/sync-gating.test.ts new file mode 100644 index 0000000..78525f1 --- /dev/null +++ b/packages/backend/tests/unit/sync-gating.test.ts @@ -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, + { 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); + }); +});