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
1,023 changes: 798 additions & 225 deletions package-lock.json

Large diffs are not rendered by default.

92 changes: 89 additions & 3 deletions packages/control-plane/src/routes/repos.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,23 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
import type { SqlDatabase } from "../db/sql-database";
import type { Env } from "../types";
import { reposRoutes } from "./repos";
import type * as SharedRoutes from "./shared";
import type { RequestContext } from "./shared";

const { mockCacheDelete, mockLogger, mockUpsert } = vi.hoisted(() => ({
const {
mockCacheDelete,
mockCacheGet,
mockCachePut,
mockGetBatch,
mockListRepositories,
mockLogger,
mockUpsert,
} = vi.hoisted(() => ({
mockCacheDelete: vi.fn(),
mockCacheGet: vi.fn(),
mockCachePut: vi.fn(),
mockGetBatch: vi.fn(),
mockListRepositories: vi.fn(),
mockLogger: {
debug: vi.fn(),
info: vi.fn(),
Expand All @@ -17,12 +30,16 @@ const { mockCacheDelete, mockLogger, mockUpsert } = vi.hoisted(() => ({

vi.mock("../db/repo-metadata", () => ({
RepoMetadataStore: vi.fn().mockImplementation(function () {
return { upsert: mockUpsert };
return { upsert: mockUpsert, getBatch: mockGetBatch };
}),
}));

vi.mock("@open-inspect/shared/cache-store", () => ({
createKvCacheStore: vi.fn(() => ({ delete: mockCacheDelete })),
createKvCacheStore: vi.fn(() => ({
delete: mockCacheDelete,
get: mockCacheGet,
put: mockCachePut,
})),
}));

vi.mock("../logger", () => ({
Expand All @@ -44,6 +61,26 @@ function createContext(): RequestContext {
};
}

vi.mock("./shared", async () => {
const actual = await vi.importActual<typeof SharedRoutes>("./shared");
return {
...actual,
createRouteSourceControlProvider: vi.fn(() => ({
listRepositories: mockListRepositories,
})),
};
});

function getListHandler() {
const route = reposRoutes.find(
(candidate) => candidate.method === "GET" && candidate.pattern.test("/repos")
);
if (!route) throw new Error("No repository list route found");
const match = "/repos".match(route.pattern);
if (!match) throw new Error("List route did not match /repos");
return { handler: route.handler, match };
}

function getUpdateHandler(path: string) {
const route = reposRoutes.find((candidate) => candidate.method === "PUT");
if (!route) throw new Error("No repository metadata update route found");
Expand All @@ -52,6 +89,55 @@ function getUpdateHandler(path: string) {
return { handler: route.handler, match };
}

describe("repository list route", () => {
beforeEach(() => {
vi.clearAllMocks();
mockCacheGet.mockResolvedValue(null);
mockCachePut.mockResolvedValue(undefined);
mockGetBatch.mockResolvedValue(new Map());
mockListRepositories.mockResolvedValue([
{
id: 1,
owner: "acme",
name: "widgets",
fullName: "acme/widgets",
description: null,
private: true,
archived: false,
defaultBranch: "main",
},
]);
});

it("keeps the cold-cache refresh alive when the client disconnects", async () => {
// A cold cache is populated synchronously. The web proxy aborts at
// CONTROL_PLANE_FETCH_TIMEOUT_MS, which cancels the worker — so unless the
// refresh is registered with waitUntil, the KV write never lands and every
// later request repeats the same slow path against an empty cache.
const waitUntil = vi.fn();
const { handler, match } = getListHandler();
const ctx = createContext();

const response = await handler(
new Request("https://test.local/repos"),
{ REPOS_CACHE: {} as KVNamespace } as Env,
match,
{
...ctx,
executionCtx: {
waitUntil,
passThroughOnException: vi.fn(),
} as unknown as ExecutionContext,
}
);

expect(response.status).toBe(200);
expect(mockCachePut).toHaveBeenCalledTimes(1);
expect(waitUntil).toHaveBeenCalledTimes(1);
await expect(waitUntil.mock.calls[0][0]).resolves.not.toThrow();
});
});

describe("repository metadata routes", () => {
beforeEach(() => {
vi.clearAllMocks();
Expand Down
89 changes: 34 additions & 55 deletions packages/control-plane/src/routes/repos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,29 @@ interface CachedReposList {
freshUntil?: number;
}

type ReposRefreshResult =
| { ok: true; repos: EnrichedRepository[]; cachedAt: string }
| { ok: false; reason: "not_configured" | "fetch_failed" };

/** Times the SCM call when a request context is available; identity otherwise. */
type ScmApiTimer = <T>(fn: () => Promise<T>) => Promise<T>;

/**
* Fetch repos via the source control provider, enrich with D1 metadata, and write to KV cache.
* Runs either in the foreground (cache miss) or background (stale-while-revalidate).
*/
async function refreshReposCache(env: Env, db: SqlDatabase, traceId?: string): Promise<void> {
async function refreshReposCache(
env: Env,
db: SqlDatabase,
traceId?: string,
timeScmApi: ScmApiTimer = (fn) => fn()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Name the identity timer default.

Define the identity ScmApiTimer in a named constant. Use that constant as the parameter default.

Proposed fix
 type ScmApiTimer = <T>(fn: () => Promise<T>) => Promise<T>;
+const IDENTITY_SCM_API_TIMER: ScmApiTimer = (fn) => fn();
 
 async function refreshReposCache(
   env: Env,
   db: SqlDatabase,
   traceId?: string,
-  timeScmApi: ScmApiTimer = (fn) => fn()
+  timeScmApi: ScmApiTimer = IDENTITY_SCM_API_TIMER
 ): Promise<ReposRefreshResult> {

As per coding guidelines, define each default value exactly once in a named constant and import or reuse that constant everywhere.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
timeScmApi: ScmApiTimer = (fn) => fn()
type ScmApiTimer = <T>(fn: () => Promise<T>) => Promise<T>;
const IDENTITY_SCM_API_TIMER: ScmApiTimer = (fn) => fn();
async function refreshReposCache(
env: Env,
db: SqlDatabase,
traceId?: string,
timeScmApi: ScmApiTimer = IDENTITY_SCM_API_TIMER
): Promise<ReposRefreshResult> {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/control-plane/src/routes/repos.ts` at line 57, Define the identity
ScmApiTimer function in a named constant near the route timer definitions, then
update the relevant constructor or function parameter to use that constant as
its default instead of the inline `(fn) => fn()` expression. Reuse the named
constant wherever this identity default is needed.

Source: Coding guidelines

): Promise<ReposRefreshResult> {
const provider = createRouteSourceControlProvider(env);
const cacheStore = createKvCacheStore(env.REPOS_CACHE);

let repos: InstallationRepository[];
try {
repos = await provider.listRepositories();
repos = await timeScmApi(() => provider.listRepositories());

logger.info("Repo fetch completed", {
trace_id: traceId,
Expand All @@ -60,13 +72,13 @@ async function refreshReposCache(env: Env, db: SqlDatabase, traceId?: string): P
logger.warn("SCM provider not configured, skipping repo refresh", {
trace_id: traceId,
});
return;
return { ok: false, reason: "not_configured" };
}
logger.error("Failed to list installation repositories (background refresh)", {
trace_id: traceId,
error: e instanceof Error ? e : String(e),
});
return;
return { ok: false, reason: "fetch_failed" };
}

const metadataStore = new RepoMetadataStore(db);
Expand Down Expand Up @@ -107,6 +119,8 @@ async function refreshReposCache(env: Env, db: SqlDatabase, traceId?: string): P
error: e instanceof Error ? e : String(e),
});
}

return { ok: true, repos: enrichedRepos, cachedAt };
}

/**
Expand Down Expand Up @@ -157,64 +171,29 @@ async function handleListRepos(
});
}

// No cache at all — must fetch synchronously
const provider = createRouteSourceControlProvider(env);

let repos: InstallationRepository[];
try {
repos = await ctx.metrics.time("scm_api", () => provider.listRepositories());
} catch (e) {
if (e instanceof SourceControlProviderError && e.errorType === "permanent" && !e.httpStatus) {
// No cache at all — populate synchronously. The refresh is also registered
// with waitUntil so it outlives this response: a caller that gives up first
// (the web proxy aborts at CONTROL_PLANE_FETCH_TIMEOUT_MS) would otherwise
// cancel the Worker before the KV write, leaving the cache empty so the next
// request repeats the same slow path — a miss that can never self-heal,
// because the stale-while-revalidate branch above needs an entry to exist.
const refresh = refreshReposCache(env, ctx.db, ctx.trace_id, (fn) =>
ctx.metrics.time("scm_api", fn)
);
ctx.executionCtx?.waitUntil(refresh);

const result = await refresh;
if (!result.ok) {
if (result.reason === "not_configured") {
return error("SCM provider not configured", 500);
}
logger.error("Failed to list installation repositories", {
error: e instanceof Error ? e : String(e),
});
return error("Failed to fetch repositories", 500);
}

logger.info("Repo fetch completed", {
trace_id: ctx.trace_id,
total_repos: repos.length,
});

const metadataStore = new RepoMetadataStore(ctx.db);
let metadataMap: Map<string, RepoMetadata>;
try {
metadataMap = await metadataStore.getBatch(
repos.map((r) => ({ owner: r.owner, name: r.name }))
);
} catch (e) {
logger.warn("Failed to fetch repo metadata batch", {
error: e instanceof Error ? e : String(e),
});
metadataMap = new Map();
}

const enrichedRepos: EnrichedRepository[] = repos.map((repo) => {
const key = `${repo.owner.toLowerCase()}/${repo.name.toLowerCase()}`;
const metadata = metadataMap.get(key);
return metadata ? { ...repo, metadata } : repo;
});

const cachedAt = new Date().toISOString();
const freshUntil = Date.now() + REPOS_CACHE_FRESH_MS;
try {
await ctx.metrics.time("kv_write", () =>
cacheStore.put(
REPOS_CACHE_KEY,
JSON.stringify({ repos: enrichedRepos, cachedAt, freshUntil }),
{ expirationTtl: REPOS_CACHE_KV_TTL_SECONDS }
)
);
} catch (e) {
logger.warn("Failed to cache repos list", { error: e instanceof Error ? e : String(e) });
}

return json({
repos: enrichedRepos,
repos: result.repos,
cached: false,
cachedAt,
cachedAt: result.cachedAt,
});
}

Expand Down
39 changes: 34 additions & 5 deletions packages/control-plane/src/webhooks/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,23 @@
import { verifySentrySignature, normalizeSentryEvent } from "@open-inspect/shared/triggers";
import { AutomationStore } from "../db/automation-store";
import { decryptSentrySecret } from "../auth/webhook-key";
import { createLogger } from "../logger";
import type { Route, RequestContext } from "../routes/shared";
import { parsePattern, json, error } from "../routes/shared";
import type { Env } from "../types";

/** Maximum Sentry webhook payload size (256KB — Sentry payloads with stack traces can be large). */
const MAX_PAYLOAD_SIZE = 256 * 1024;
const logger = createLogger("sentry-webhook");

function classifySentryAction(action: unknown): "created" | "critical" | "other" | "missing" {
if (action === "created" || action === "critical") return action;
return typeof action === "string" ? "other" : "missing";
}

function isRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}

async function handleSentryWebhook(
request: Request,
Expand Down Expand Up @@ -66,17 +77,35 @@ async function handleSentryWebhook(
}

// 3. Parse and normalize
let payload: Record<string, unknown>;
let parsedPayload: unknown;
try {
payload = JSON.parse(body) as Record<string, unknown>;
parsedPayload = JSON.parse(body) as unknown;
} catch {
return error("Invalid JSON", 400);
}

const event = normalizeSentryEvent(payload, automationId);
if (!event) {
const payload = isRecord(parsedPayload) ? parsedPayload : {};

const sentryResource = request.headers.get("sentry-hook-resource");
const normalization = normalizeSentryEvent(payload, automationId, sentryResource);
if (normalization.status === "skipped") {
const logData = {
event: "sentry.webhook_skipped",
reason: normalization.reason,
automation_id: automationId,
configured_event_type: automation.event_type,
sentry_resource: sentryResource,
sentry_action: classifySentryAction(payload.action),
request_id: ctx.request_id,
trace_id: ctx.trace_id,
};
if (normalization.reason === "unsupported_action") {
logger.info("Sentry webhook action is not configured for automation", logData);
} else {
logger.warn("Sentry webhook skipped during normalization", logData);
}
return json({ ok: true, skipped: true });
}
const event = normalization.event;

// 4. Forward to SchedulerDO
if (!env.SCHEDULER) {
Expand Down
Loading
Loading