Skip to content
Open
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
108 changes: 108 additions & 0 deletions apps/bff/src/routes/internal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { beforeEach, describe, expect, it, mock } from "bun:test";
import { Hono } from "hono";

const mockRepositorySettingsFindUnique = mock(() => Promise.resolve(null as unknown));
const mockGetOrgQuotaPool = mock(() => Promise.resolve({ total: 0, used: 0, hasUnlimited: false }));

mock.module("../db", () => ({
prisma: {
repositorySettings: {
findUnique: mockRepositorySettingsFindUnique,
},
},
}));

mock.module("../middleware/organization", () => ({
getOrgQuotaPool: mockGetOrgQuotaPool,
}));

mock.module("../services/notifications", () => ({
createNotification: mock(() => Promise.resolve()),
}));

const { internalRoutes } = await import("./internal");

function createApp() {
const app = new Hono();
app.route("/api/internal", internalRoutes);
return app;
}

function requestSettings(owner = "antiptrn", repo = "opendiff") {
return createApp().fetch(
new Request(`http://localhost/api/internal/settings/${owner}/${repo}`, {
headers: { "X-API-Key": "test-key" },
})
);
}

describe("internal settings routes", () => {
beforeEach(() => {
process.env.REVIEW_AGENT_API_KEY = "test-key";
mockRepositorySettingsFindUnique.mockReset();
mockRepositorySettingsFindUnique.mockResolvedValue(null);
mockGetOrgQuotaPool.mockReset();
mockGetOrgQuotaPool.mockResolvedValue({ total: 0, used: 0, hasUnlimited: false });
});

it("disables repository settings when quota is exhausted before review work starts", async () => {
mockRepositorySettingsFindUnique.mockResolvedValue({
owner: "antiptrn",
repo: "opendiff",
enabled: true,
autofixEnabled: true,
sensitivity: 50,
organizationId: "org-1",
});
mockGetOrgQuotaPool.mockResolvedValue({ total: 0, used: 0, hasUnlimited: false });

const response = await requestSettings();
const body = await response.json();

expect(response.status).toBe(200);
expect(body.enabled).toBe(true);
expect(body.effectiveEnabled).toBe(false);
expect(body.disabledReason).toBe("quota_exhausted");
expect(body.quota).toEqual({ total: 0, used: 0, hasUnlimited: false });
expect(mockGetOrgQuotaPool).toHaveBeenCalledWith("org-1");
});

it("keeps repository settings effective when quota remains", async () => {
mockRepositorySettingsFindUnique.mockResolvedValue({
owner: "antiptrn",
repo: "opendiff",
enabled: true,
autofixEnabled: true,
sensitivity: 50,
organizationId: "org-1",
});
mockGetOrgQuotaPool.mockResolvedValue({ total: 100, used: 99, hasUnlimited: false });

const response = await requestSettings();
const body = await response.json();

expect(response.status).toBe(200);
expect(body.effectiveEnabled).toBe(true);
expect(body.disabledReason).toBeUndefined();
expect(body.quota).toEqual({ total: 100, used: 99, hasUnlimited: false });
});

it("disables repository settings that are not attached to an organization", async () => {
mockRepositorySettingsFindUnique.mockResolvedValue({
owner: "antiptrn",
repo: "opendiff",
enabled: true,
autofixEnabled: true,
sensitivity: 50,
organizationId: null,
});

const response = await requestSettings();
const body = await response.json();

expect(response.status).toBe(200);
expect(body.effectiveEnabled).toBe(false);
expect(body.disabledReason).toBe("missing_organization");
expect(mockGetOrgQuotaPool).not.toHaveBeenCalled();
});
});
60 changes: 51 additions & 9 deletions apps/bff/src/routes/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,36 @@ function safeCompare(a: string, b: string): boolean {

const internalRoutes = new Hono();

type InternalSettingsDisabledReason =
| "repository_not_configured"
| "repository_disabled"
| "missing_organization"
| "quota_exhausted";

type InternalSettingsQuota = {
total: number;
used: number;
hasUnlimited: boolean;
};

function buildDisabledSettings(
owner: string,
repo: string,
reason: InternalSettingsDisabledReason,
quota?: InternalSettingsQuota
) {
return {
owner,
repo,
enabled: false,
effectiveEnabled: false,
autofixEnabled: false,
sensitivity: 50,
disabledReason: reason,
...(quota ? { quota } : {}),
};
}

// Check if a user has an active seat for a given repo
internalRoutes.get("/check-seat/:owner/:repo", async (c) => {
const apiKey = c.req.header("X-API-Key");
Expand Down Expand Up @@ -105,14 +135,24 @@ internalRoutes.get("/settings/:owner/:repo", async (c) => {
});

if (!settings) {
return c.json({
owner,
repo,
enabled: false,
effectiveEnabled: false,
autofixEnabled: false,
sensitivity: 50,
});
return c.json(buildDisabledSettings(owner, repo, "repository_not_configured"));
}

let effectiveEnabled = settings.enabled;
let disabledReason: InternalSettingsDisabledReason | undefined;
let quota: InternalSettingsQuota | undefined;

if (!settings.enabled) {
disabledReason = "repository_disabled";
} else if (!settings.organizationId) {
effectiveEnabled = false;
disabledReason = "missing_organization";
} else {
quota = await getOrgQuotaPool(settings.organizationId);
if (quota.total !== -1 && quota.used >= quota.total) {
effectiveEnabled = false;
disabledReason = "quota_exhausted";
}
}

return c.json({
Expand All @@ -121,7 +161,9 @@ internalRoutes.get("/settings/:owner/:repo", async (c) => {
enabled: settings.enabled,
autofixEnabled: settings.autofixEnabled,
sensitivity: settings.sensitivity,
effectiveEnabled: settings.enabled,
effectiveEnabled,
...(disabledReason ? { disabledReason } : {}),
...(quota ? { quota } : {}),
});
});

Expand Down
2 changes: 1 addition & 1 deletion apps/bff/src/routes/reviews/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ localRoutes.post("/reviews/local", requireAuth(), async (c) => {
}

const quotaPool = await getOrgQuotaPool(orgId);
if (quotaPool.total !== -1 && quotaPool.used > quotaPool.total) {
if (quotaPool.total !== -1 && quotaPool.used >= quotaPool.total) {
return c.json(
{
error: "Token quota exceeded",
Expand Down
2 changes: 1 addition & 1 deletion apps/review-agent/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ app.post("/webhook", async (c) => {

if (!settings.effectiveEnabled) {
console.log(
`Reviews disabled for ${owner}/${repo} (enabled: ${settings.enabled}, effectiveEnabled: ${settings.effectiveEnabled})`
`Reviews disabled for ${owner}/${repo} (enabled: ${settings.enabled}, effectiveEnabled: ${settings.effectiveEnabled}, reason: ${settings.disabledReason ?? "unknown"})`
);
return c.json({ status: "skipped", reason: "disabled" });
}
Expand Down
Binary file modified bun.lockb
Binary file not shown.
10 changes: 10 additions & 0 deletions packages/shared/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,14 @@ export interface RepositorySettings {
effectiveEnabled: boolean;
autofixEnabled: boolean;
sensitivity: number;
disabledReason?:
| "repository_not_configured"
| "repository_disabled"
| "missing_organization"
| "quota_exhausted";
quota?: {
total: number;
used: number;
hasUnlimited: boolean;
};
}