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
5 changes: 2 additions & 3 deletions packages/control-plane/src/router.create-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
TEST_BACKGROUND_TASK_CONTEXT,
TEST_SERVICE_SECRETS,
} from "./router.test-support";
import { sessionCreateRoutes } from "./routes/session-create";
import { handleCreateSession } from "./routes/session-create";
import { HttpError, resolveRepoOrError } from "./routes/shared";
import { SessionInternalPaths } from "./session/contracts";
import { resolveManagedSkills } from "./session/skill-resolution";
Expand Down Expand Up @@ -619,7 +619,7 @@ describe("handleCreateSession D1 ordering", () => {
const testEnv: Record<string, unknown> = createEnv(initFetch);
testEnv.SCM_PROVIDER = "gitlab";

const response = await sessionCreateRoutes[0].handler(
const response = await handleCreateSession(
new Request("https://test.local/sessions", {
method: "POST",
headers: { "Content-Type": "application/json" },
Expand All @@ -634,7 +634,6 @@ describe("handleCreateSession D1 ordering", () => {
}),
}),
testEnv as never,
[] as unknown as RegExpMatchArray,
{
request_id: "test-request",
trace_id: "test-trace",
Expand Down
15 changes: 4 additions & 11 deletions packages/control-plane/src/routes/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ import { reposRoutes } from "./repos";
import { scmSettingsRoutes } from "./scm-settings";
import { secretsRoutes } from "./secrets";
import { sessionRoutes } from "./sessions";
import { handleSlackNotify } from "./slack-notify";
import { slackNotifyRoutes } from "./slack-notify";
import { signInProviderRoutes } from "./sign-in-providers";
import { skillRoutes } from "./skills";
import { defineRoute, GITHUB_SANDBOX_FALLBACK_ROUTE, requirePermission } from "./shared";

/**
* Registration order is the precedence order. A Hono sub-app is mounted where
Expand All @@ -42,15 +41,9 @@ export const catalog: RouteCatalogEntry[] = [
...browserAuthRoutes,
signInProviderRoutes,

// Session management
...sessionRoutes,
// Agent-initiated Slack notification (sandbox-authenticated)
defineRoute(GITHUB_SANDBOX_FALLBACK_ROUTE, {
method: "POST",
path: "/sessions/:id/slack-notify",
authorization: requirePermission("sessions.collaborate"),
handler: handleSlackNotify,
}),
// Session management, then the agent-initiated Slack notification
sessionRoutes,
slackNotifyRoutes,

// Repository management
...reposRoutes,
Expand Down
39 changes: 18 additions & 21 deletions packages/control-plane/src/routes/session-attachments.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { describe, expect, it, vi } from "vitest";
import { SESSION_ATTACHMENT_MAX_REQUEST_BYTES } from "../media";
import type { Env } from "../types";
import { sessionAttachmentRoutes } from "./session-attachments";
import { handleAttachmentPost } from "./session-attachments";
import type { RequestContext } from "./shared";
import type { SqlDatabase } from "../db/sql-database";
import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { withSessionRuntime } from "./session-route";

const PNG_BYTES = Uint8Array.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);

Expand Down Expand Up @@ -65,28 +66,16 @@ function oversizedStreamingUploadRequest(): Request {
} as RequestInit & { duplex: "half" });
}

function getUploadRoute() {
const path = "/sessions/session-1/attachments";
const route = sessionAttachmentRoutes.find(
(candidate) => candidate.method === "POST" && path.match(routePathPattern(candidate.path))
);
if (!route) throw new Error("Attachment upload route not found");
const match = path.match(routePathPattern(route.path));
if (!match) throw new Error("Attachment upload route did not match");
return { route, match };
}

describe("session attachment routes", () => {
it("bounds streamed requests when Content-Length is unavailable", async () => {
const fetch = vi.fn(async () => Response.json({ status: "ok" }));
const { env, put } = createEnv(fetch);
const { route, match } = getUploadRoute();

const response = await route.handler(
const response = await handleAttachmentPost(
oversizedStreamingUploadRequest(),
env,
match,
createContext()
{ id: "session-1" },
withSessionRuntime(env, createContext())
);

expect(response.status).toBe(413);
Expand All @@ -105,9 +94,13 @@ describe("session attachment routes", () => {
Response.json({ error: message }, { status: registryStatus })
);
const { env, put } = createEnv(fetch);
const { route, match } = getUploadRoute();

const response = await route.handler(attachmentUploadRequest(), env, match, createContext());
const response = await handleAttachmentPost(
attachmentUploadRequest(),
env,
{ id: "session-1" },
withSessionRuntime(env, createContext())
);

expect(response.status).toBe(routeStatus);
await expect(response.json()).resolves.toEqual({ error: message });
Expand All @@ -133,9 +126,13 @@ describe("session attachment routes", () => {
});
const { env, put, remove } = createEnv(fetch);
remove.mockRejectedValue(new Error("R2 unavailable"));
const { route, match } = getUploadRoute();

const response = await route.handler(attachmentUploadRequest(), env, match, createContext());
const response = await handleAttachmentPost(
attachmentUploadRequest(),
env,
{ id: "session-1" },
withSessionRuntime(env, createContext())
);

expect(response.status).toBe(503);
await expect(response.json()).resolves.toEqual({
Expand Down
57 changes: 27 additions & 30 deletions packages/control-plane/src/routes/session-attachments.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { Hono } from "hono";
import { admit } from "../routing/admit";
import type { ControlPlaneHonoEnv } from "../routing/hono-env";
/**
* Session image attachments added through the chat composer.
*
Expand Down Expand Up @@ -45,15 +48,13 @@ import {
createStoredObjectResponse,
} from "./responses/stored-object-response";
import {
defineRoute,
error,
GITHUB_SANDBOX_FALLBACK_ROUTE,
GITHUB_USER_OR_SERVICE_ROUTE,
json,
requirePermission,
type Route,
} from "./shared";
import { sessionRoute, type SessionRouteContext } from "./session-route";
import { type SessionRouteContext, dispatchSession } from "./session-route";

const logger = createLogger("router:session-attachments");

Expand All @@ -76,13 +77,13 @@ function attachmentStorageErrorResponse(cause: SessionAttachmentStorageError): R
}
}

async function handleAttachmentPost(
export async function handleAttachmentPost(
request: Request,
env: Env,
match: RegExpMatchArray,
params: { id: string },
ctx: SessionRouteContext
): Promise<Response> {
const sessionId = match.groups?.id;
const sessionId = params.id;
if (!sessionId) return error("Session ID required");
if (sessionAttachmentRequestExceedsLimit(request)) {
return error("Attachment request is too large", 413);
Expand Down Expand Up @@ -179,14 +180,14 @@ async function handleAttachmentPost(
);
}

async function handleAttachmentGet(
export async function handleAttachmentGet(
request: Request,
env: Env,
match: RegExpMatchArray,
params: { id: string; attachmentId: string },
ctx: SessionRouteContext
): Promise<Response> {
const sessionId = match.groups?.id;
const attachmentId = match.groups?.attachmentId;
const sessionId = params.id;
const attachmentId = params.attachmentId;
if (!sessionId || !attachmentId) {
return error("Session ID and attachment ID are required", 400);
}
Expand Down Expand Up @@ -237,23 +238,19 @@ async function handleAttachmentGet(
: createStoredObjectResponse(body, metadata, contentType);
}

export const sessionAttachmentRoutes: Route[] = [
defineRoute(
GITHUB_USER_OR_SERVICE_ROUTE,
sessionRoute({
method: "POST",
path: "/sessions/:id/attachments",
authorization: requirePermission("sessions.collaborate"),
handler: handleAttachmentPost,
})
),
defineRoute(
GITHUB_SANDBOX_FALLBACK_ROUTE,
sessionRoute({
method: "GET",
path: "/sessions/:id/attachments/:attachmentId",
authorization: requirePermission("sessions.read"),
handler: handleAttachmentGet,
})
),
];
export const sessionAttachmentRoutes = new Hono<ControlPlaneHonoEnv>();

sessionAttachmentRoutes.post(
"/sessions/:id/attachments",
admit({
...GITHUB_USER_OR_SERVICE_ROUTE,
authorization: requirePermission("sessions.collaborate"),
}),
(c) => dispatchSession(c, handleAttachmentPost)
);

sessionAttachmentRoutes.get(
"/sessions/:id/attachments/:attachmentId",
admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }),
(c) => dispatchSession(c, handleAttachmentGet)
);
27 changes: 15 additions & 12 deletions packages/control-plane/src/routes/session-child-spawn.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { Hono } from "hono";
import { admit } from "../routing/admit";
import type { ControlPlaneHonoEnv } from "../routing/hono-env";
import { spawnChildSessionRequestSchema } from "@open-inspect/shared/types/session-api";
import {
DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS,
Expand Down Expand Up @@ -28,15 +31,13 @@ import {
import { spawnContextSchema } from "../session/spawn-context";
import type { Env } from "../types";
import {
defineRoutes,
error,
GITHUB_SANDBOX_FALLBACK_ROUTE,
json,
permissionRequirement,
requireAll,
type Route,
} from "./shared";
import { sessionRoute, type SessionRouteContext } from "./session-route";
import { type SessionRouteContext, dispatchSession } from "./session-route";
import { DEFAULT_BASE_BRANCH } from "../repos/default-branch";
import { authorizeSessionTarget } from "./session-target-authorization";

Expand All @@ -47,13 +48,13 @@ function isJsonRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);
}

async function handleSpawnChild(
export async function handleSpawnChild(
request: Request,
env: Env,
match: RegExpMatchArray,
params: { id: string },
ctx: SessionRouteContext
): Promise<Response> {
const parentId = match.groups?.id;
const parentId = params.id;
if (!parentId) return error("Parent session ID required");

const parsedBody = spawnChildSessionRequestSchema.safeParse(await request.json());
Expand Down Expand Up @@ -350,14 +351,16 @@ async function handleSpawnChild(
return json({ sessionId: childId, status: "created" }, 201);
}

export const sessionChildSpawnRoutes: Route[] = defineRoutes(GITHUB_SANDBOX_FALLBACK_ROUTE, [
sessionRoute({
method: "POST",
path: "/sessions/:id/children",
export const sessionChildSpawnRoutes = new Hono<ControlPlaneHonoEnv>();

sessionChildSpawnRoutes.post(
"/sessions/:id/children",
admit({
...GITHUB_SANDBOX_FALLBACK_ROUTE,
authorization: requireAll(
permissionRequirement("sessions.create"),
permissionRequirement("sessions.collaborate")
),
handler: handleSpawnChild,
}),
]);
(c) => dispatchSession(c, handleSpawnChild)
);
6 changes: 3 additions & 3 deletions packages/control-plane/src/routes/session-children.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ vi.mock("../session/integration-settings-resolution", () => ({
resolveSandboxSettings: vi.fn(),
}));

function routeMatch(path: string, pattern: string): RegExpMatchArray {
function routeMatch(path: string, pattern: string): { id: string; childId: string } {
const match = path.match(routePathPattern(pattern));
if (!match) throw new Error("Expected route match");
return match;
if (!match?.groups?.id || !match.groups.childId) throw new Error("Expected route match");
return { id: match.groups.id, childId: match.groups.childId };
}

const defaultPromptAuthor: ActivePromptAuthor = {
Expand Down
Loading
Loading