Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions packages/backend/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { SINGLE_TENANT_ORG_ID } from './constants.js';
import { isGitHubRateLimitError, isNotFound } from './errors.js';
import { PromClient } from './promClient.js';
import { createGitHubRepoRecord } from './repoCompileUtils.js';
import type { JobManager } from './types.js';
import type { JobManager, Settings } from './types.js';

const logger = createLogger('api');

Expand All @@ -35,6 +35,7 @@ export class Api {
private prisma: PrismaClient,
private jobManager: JobManager,
redis: Redis,
private settings: Settings,
) {
const app = express();
app.use(express.json());
Expand Down Expand Up @@ -138,14 +139,11 @@ export class Api {
create: record,
});

const jobId = await this.jobManager.trigger(
'repo-index',
{
repoId: repo.id,
type: RepoIndexingJobType.INDEX,
},
{ priority: JOB_PRIORITIES.INTERACTIVE },
);
const jobId = await scheduleAndTriggerRepoIndexing({
jobManager: this.jobManager,
repoId: repo.id,
reindexIntervalMs: this.settings.reindexIntervalMs,
});
Comment thread
brendan-kellam marked this conversation as resolved.

res.status(200).json({ jobId, repoId: repo.id });
}
Expand All @@ -162,3 +160,33 @@ export class Api {
});
}
}

const scheduleAndTriggerRepoIndexing = async ({
jobManager,
repoId,
reindexIntervalMs,
}: {
jobManager: JobManager;
repoId: number;
reindexIntervalMs: number;
}): Promise<string> => {
await jobManager.upsertJobScheduler(
"repo-index",
`repo-index-v1-${repoId}`,
reindexIntervalMs,
{
repoId,
type: RepoIndexingJobType.INDEX,
},
{ priority: JOB_PRIORITIES.SCHEDULED },
);

return jobManager.trigger(
"repo-index",
{
repoId,
type: RepoIndexingJobType.INDEX,
},
{ priority: JOB_PRIORITIES.INTERACTIVE },
);
};
2 changes: 1 addition & 1 deletion packages/backend/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ jobManager.register(repoPermissionSyncWorkload);
jobManager.register(attachmentPruneWorkload);
jobManager.register(auditLogPruneWorkload);

const api = new Api(promClient, prisma, jobManager, redis);
const api = new Api(promClient, prisma, jobManager, redis, settings);

await cleanupOrphanedRepoResources(prisma);

Expand Down
51 changes: 48 additions & 3 deletions packages/backend/src/reconcileJobSchedulers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,16 @@ describe("reconcileJobSchedulers", () => {
});
expect(mocks.repoFindMany).toHaveBeenCalledWith({
where: {
connections: {
some: {},
},
OR: [
{
connections: {
some: {},
},
},
{
isAutoCleanupDisabled: true,
},
],
},
select: { id: true },
});
Expand Down Expand Up @@ -205,6 +212,44 @@ describe("reconcileJobSchedulers", () => {
).toBeLessThan(mocks.trigger.mock.invocationCallOrder[0]);
});

test("keeps index schedulers for repos with automatic cleanup disabled", async () => {
mocks.repoFindMany.mockImplementation(async ({ where }) => {
if (where?.isAutoCleanupDisabled === false) {
return [];
}
if (where?.isPublic === false) {
return [];
}
return [{ id: 42 }, { id: 84 }];
});
mocks.getJobSchedulerIds.mockImplementation(async (workloadName) =>
workloadName === "repo-index" ? ["repo-index-v1-84"] : [],
);

await reconcileJobSchedulers({
db,
jobManager,
settings: {
resyncConnectionIntervalMs: 86_400_000,
reindexIntervalMs: 3_600_000,
userDrivenPermissionSyncIntervalMs: 43_200_000,
repoDrivenPermissionSyncIntervalMs: 21_600_000,
},
});

expect(mocks.upsertJobScheduler).toHaveBeenCalledWith(
"repo-index",
"repo-index-v1-84",
3_600_000,
{ repoId: 84, type: "INDEX" },
{ priority: 10 },
);
expect(mocks.removeJobScheduler).not.toHaveBeenCalledWith(
"repo-index",
"repo-index-v1-84",
);
});

test("removes permission schedulers when permission syncing is disabled", async () => {
isPermissionSyncEnabled.mockResolvedValue(false);
mocks.getJobSchedulerIds.mockImplementation(async (workloadName) => {
Expand Down
13 changes: 10 additions & 3 deletions packages/backend/src/reconcileJobSchedulers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,16 @@ export const reconcileJobSchedulers = async ({
}),
db.repo.findMany({
where: {
connections: {
some: {},
},
OR: [
{
connections: {
some: {},
},
},
{
isAutoCleanupDisabled: true,
},
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete scheduler retention fix

Medium Severity

This change keeps repo-index schedulers for isAutoCleanupDisabled repos during startup reconcile, and the experimental endpoint now creates those schedulers, but connection sync still treats connectionless orphans as removable without checking that flag. After a temporary link/unlink (or connection delete), reconcileRepoIndexWork can drop the scheduler while CLEANUP is skipped, so recurring reindex stops until the next process restart.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7a7167d. Configure here.

},
select: {
id: true,
Expand Down
Loading