Skip to content

fix: isolate profile variation submissions #1614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Cedric Vidal (cedricvidal) wants to merge 1 commit into
base: main
Choose a base branch
from
+503 −52
Draft

fix: isolate profile variation submissions #1614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ config/ # Benchmark definitions (YAML)

For package architecture and data models, see [docs/architecture/app-design.md](docs/architecture/app-design.md).

## Cross-client domain ownership

The API owns domain validation, orchestration, fan-out, defaulting, and
resolution rules shared by Portal, CLI, and direct API consumers. Portal and CLI
are thin clients: they collect user intent and may provide early validation, but
must not duplicate or materialize API-owned business logic. When one user action
expands into multiple domain objects, each object is derived independently by
the API from the shared intent so client or sibling state cannot leak between
them.

## Key Components

### API (`apps/api/`)
Expand Down
154 changes: 154 additions & 0 deletions apps/api/src/endpoints.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,160 @@ describe("API Endpoints", () => {
expect(res.body.error).toContain("missing a prompt");
expect(mocks.collection.insertMany).not.toHaveBeenCalled();
});

it("rejects shared resources in variation mode before resolving profiles", async () => {
const resourceRevision = {
_id: "resource-revision-1",
resourceId: "resource-1",
projectId: TEST_PROJECT_ID,
slug: "github-simulator",
revisionNumber: 1,
ref: "github-simulator@r1",
setup: { sh: "echo ready" },
exports: [],
parameters: [{ name: "REPO", required: true }],
contentSha256: "sha",
createdAt: new Date(),
};
(mocks.resourceStore.getBySlug as any).mockResolvedValue({
_id: "resource-1",
projectId: TEST_PROJECT_ID,
slug: "github-simulator",
latestRevisionId: resourceRevision._id,
});
(mocks.resourceRevisionStore.getLatest as any).mockResolvedValue(resourceRevision);
(mocks.profileVersionCollection.findOne as any).mockImplementation(
async (q: { profileId: string }) => ({
_id: `pv-${q.profileId}`,
profileId: q.profileId,
version: 1,
workerType: "coder-acp-copilot",
model: q.profileId === "base-profile" ? "m1" : "m2",
mcpServers: [],
skillRevisions: [],
extensions: [],
resources: [{
ref: "github-simulator",
params: { REPO: "microsoft/scope" },
}],
}),
);

const res = await request(testServer())
.post(`/api/v1/requests?projectId=${TEST_PROJECT_ID}`)
.send({
scenario: { task: "Build something", criteria: ["works"] },
profileId: "base-profile",
profileVariations: ["var-profile"],
resources: [{
ref: "github-simulator",
params: { REPO: "microsoft/scope" },
}],
});

expect(res.status).toBe(400);
expect(res.body.error).toContain("resources");
expect(mocks.collection.insertMany).not.toHaveBeenCalled();
});

it.each([
["model", "m1"],
["reasoningEffort", "high"],
["agentVersion", "v1"],
["mcpServers", ["server-a"]],
["skills", ["skill-a"]],
["extensions", ["publisher.extension"]],
])("rejects shared %s in variation mode", async (field, value) => {
const res = await request(testServer())
.post(`/api/v1/requests?projectId=${TEST_PROJECT_ID}`)
.send({
scenario: { task: "Build something", criteria: ["works"] },
profileId: "base-profile",
profileVariations: ["var-profile"],
[field]: value,
});

expect(res.status).toBe(400);
expect(res.body.error).toContain(field);
expect(mocks.collection.insertMany).not.toHaveBeenCalled();
});

it("rejects a shared worker query in variation mode", async () => {
const res = await request(testServer())
.post(`/api/v1/requests?projectId=${TEST_PROJECT_ID}&worker=coder-acp-copilot`)
.send({
scenario: { task: "Build something", criteria: ["works"] },
profileId: "base-profile",
profileVariations: ["var-profile"],
});

expect(res.status).toBe(400);
expect(res.body.error).toContain("worker");
expect(mocks.collection.insertMany).not.toHaveBeenCalled();
});

it("resolves each variation profile's resources without leaking base values", async () => {
const resourceRevision = {
_id: "resource-revision-1",
resourceId: "resource-1",
projectId: TEST_PROJECT_ID,
slug: "github-simulator",
revisionNumber: 1,
ref: "github-simulator@r1",
setup: { sh: "echo ready" },
exports: [],
parameters: [{ name: "REPO", required: true }],
contentSha256: "sha",
createdAt: new Date(),
};
(mocks.resourceStore.getBySlug as any).mockResolvedValue({
_id: "resource-1",
projectId: TEST_PROJECT_ID,
slug: "github-simulator",
latestRevisionId: resourceRevision._id,
});
(mocks.resourceRevisionStore.getLatest as any).mockResolvedValue(resourceRevision);
(mocks.profileVersionCollection.findOne as any).mockImplementation(
async (q: { profileId: string }) => ({
_id: `pv-${q.profileId}`,
ref: `${q.profileId}@1`,
profileId: q.profileId,
version: 1,
workerType: "coder-acp-copilot",
model: q.profileId === "base-profile" ? "m1" : "m2",
mcpServers: [],
skillRevisions: [],
extensions: [],
resources: [{
ref: "github-simulator",
params: {
REPO: q.profileId === "base-profile"
? "baseline/repository"
: "comparison/repository",
},
}],
}),
);

const res = await request(testServer())
.post(`/api/v1/requests?projectId=${TEST_PROJECT_ID}`)
.send({
scenario: { task: "Build something", criteria: ["works"] },
profileId: "base-profile",
profileVariations: ["var-profile"],
});

expect(res.status).toBe(201);
const docs = (mocks.collection.insertMany as any).mock.calls[0][0];
expect(docs).toHaveLength(2);
expect(docs.map((doc: { profileId: string; resources: Array<{ params: Record<string, string> }> }) => ({
profileId: doc.profileId,
repo: doc.resources[0].params.REPO,
}))).toEqual([
{ profileId: "base-profile", repo: "baseline/repository" },
{ profileId: "var-profile", repo: "comparison/repository" },
]);
});
});

// ===================================================================
Expand Down
87 changes: 45 additions & 42 deletions apps/api/src/routes/requests/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,44 @@ apiRoute(ctx.app, ctx.registry, {
const { scenario: scenarioObj, persona: personaObj, maxIterations, personaInstructions, count = 1, model: requestedModel, reasoningEffort: requestedReasoningEffort, mcpServers: mcpServerSlugs, skills: skillSlugs, extensions: extensionIds, agentVersion: requestedAgentVersion, profileId: requestedProfileSpec, profileVariations, priority: requestedPriority, agentsMd: requestedAgentsMd, agentsMdParentIds: requestedAgentsMdParentIds, gates: requestedGates, codebase: codebaseSpec, codebaseRevisionId: requestedCodebaseRevisionId, resources: resourceSpecs } = req.body;
let worker = req.query.worker as string | undefined;

const typedProfileVariations: string[] = Array.isArray(profileVariations)
? profileVariations.filter((v: unknown): v is string => typeof v === "string")
: [];
const isVariationSubmit = typedProfileVariations.length > 0;

if (isVariationSubmit) {
if (!requestedProfileSpec) {
res.status(400).json({ error: "profileId (the base profile) is required when profileVariations are provided" });
return;
}

// Profile fan-out starts from shared run intent only. Every controlled
// value must be resolved independently from each profile.
const requestControlledFields: Array<[string, unknown]> = [
["model", requestedModel],
["reasoningEffort", requestedReasoningEffort],
["agentVersion", requestedAgentVersion],
["mcpServers", mcpServerSlugs],
["skills", skillSlugs],
["extensions", extensionIds],
["resources", resourceSpecs],
];
const providedControlledFields = requestControlledFields.filter(([, value]) => value !== undefined);
if (providedControlledFields.length > 0) {
res.status(400).json({
error: `In variation mode, request-level ${providedControlledFields.map(([field]) => field).join("/")} must be omitted; each profile supplies its own controlled configuration.`,
});
return;
}

if (worker !== undefined) {
res.status(400).json({
error: "The ?worker query parameter is not allowed in variation mode; worker is derived from each variation profile.",
});
return;
}
}

// AGENTS.md body + lineage (for any caller that wants to attach an
// AGENTS.md instruction file to a run).
const agentsMdText =
Expand Down Expand Up @@ -520,34 +558,7 @@ apiRoute(ctx.app, ctx.registry, {
label?: string;
};

const typedProfileVariations: string[] = Array.isArray(profileVariations)
? profileVariations.filter((v: unknown): v is string => typeof v === "string")
: [];
const isVariationSubmit = typedProfileVariations.length > 0;

if (isVariationSubmit) {
if (!requestedProfileSpec) {
res.status(400).json({ error: "profileId (the base profile) is required when profileVariations are provided" });
return;
}

// In variation mode, controlled fields come from the variation profiles themselves.
if (requestedModel !== undefined || mcpServerSlugs !== undefined || skillSlugs !== undefined || extensionIds !== undefined) {
res.status(400).json({
error: "In variation mode, model/mcpServers/skills/extensions must be set via the variation profile.",
});
return;
}

// In variation mode, the worker is derived per-variation from each profile's workerType.
// Reject the ?worker= query param so callers don't think it has any effect.
if (worker) {
res.status(400).json({
error: "The ?worker query parameter is not allowed in variation mode; worker is derived from each variation profile.",
});
return;
}

if (!scenarioObj || typeof scenarioObj !== "object" || !scenarioObj.task || typeof scenarioObj.task !== "string") {
res.status(400).json({ error: "scenario.task is required and must be a string" });
return;
Expand All @@ -569,7 +580,7 @@ apiRoute(ctx.app, ctx.registry, {
}

const baseParsed = (() => {
try { return parseProfileSpec(requestedProfileSpec); }
try { return parseProfileSpec(requestedProfileSpec!); }
catch (err) { res.status(400).json({ error: (err as Error).message }); return null; }
})();
if (!baseParsed) return;
Expand Down Expand Up @@ -673,25 +684,17 @@ apiRoute(ctx.app, ctx.registry, {
const effectiveMcpServers = variationProfileVersion.mcpServers ?? undefined;
const effectiveSkills = variationProfileVersion.skillRevisions ?? undefined;
const effectiveExtensions = variationProfileVersion.extensions ?? undefined;
const requestedVariationAgentVersion = variationProfileVersion.agentVersion ?? requestedAgentVersion;
const targetCheck = await validateAgentTarget(ctx.agentCollection, {
workerType: variationWorkerType,
requestedVersion: requestedVariationAgentVersion,
requestedVersion: variationProfileVersion.agentVersion,
model: variationProfileVersion.model,
requireModel: true,
requirements: requestedAgentCapabilities({
reasoningEffort:
variationProfileVersion.reasoningEffort ?? requestedReasoningEffort,
reasoningEffort: variationProfileVersion.reasoningEffort,
mcpServers: effectiveMcpServers,
skillRevisions: effectiveSkills,
extensions: effectiveExtensions,
// Mirrors resolveResourceBindings' precedence (profile wins). Uses the
// specs rather than resolved bindings because the capability check only
// needs to know whether any resource was requested, and resolution
// happens after this point.
resources:
normalizeResourceBindingSpecs(variationProfileVersion.resources)
?? requestedResourceSpecs,
resources: normalizeResourceBindingSpecs(variationProfileVersion.resources),
}),
strictCapabilities: ctx.strictAgentCapabilities,
});
Expand Down Expand Up @@ -772,7 +775,7 @@ apiRoute(ctx.app, ctx.registry, {
const resourceResult = await resolveResourceBindings(
ctx,
projectId,
requestedResourceSpecs,
undefined,
normalizeResourceBindingSpecs(variationProfileVersion.resources),
);
if (resourceResult.errors.length > 0) {
Expand Down Expand Up @@ -833,8 +836,8 @@ apiRoute(ctx.app, ctx.registry, {
createdAt: new Date(),
priority: requestedPriority ?? 0,
...(r.model ? { model: r.model } : {}),
...((r.profileVersion.reasoningEffort ?? requestedReasoningEffort)
? { reasoningEffort: r.profileVersion.reasoningEffort ?? requestedReasoningEffort }
...(r.profileVersion.reasoningEffort
? { reasoningEffort: r.profileVersion.reasoningEffort }
: {}),
...(maxIterations ? { maxIterations } : {}),
...(personaInstructions ? { personaInstructions } : {}),
Expand Down
Loading
Loading