Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
69 changes: 39 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 { withSessionRuntime, type SessionRouteContext } 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,31 @@ 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) =>
handleAttachmentPost(
c.var.admitted.request,
c.env,
c.req.param(),
withSessionRuntime(c.env, c.var.admitted.ctx)
)
);

sessionAttachmentRoutes.get(
"/sessions/:id/attachments/:attachmentId",
admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }),
(c) =>
handleAttachmentGet(
c.var.admitted.request,
c.env,
c.req.param(),
withSessionRuntime(c.env, c.var.admitted.ctx)
)
);
33 changes: 21 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 { withSessionRuntime, type SessionRouteContext } 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,22 @@ 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) =>
handleSpawnChild(
c.var.admitted.request,
c.env,
c.req.param(),
withSessionRuntime(c.env, c.var.admitted.ctx)
)
);
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