Skip to content

Commit 6214376

Browse files
jfr956claude
andcommitted
fix(models): limit default-change pin check to the outgoing model
Changing a space's default only strands pins in that space on the outgoing model, so the check no longer refuses because of other pins or spaces. Enabled model ids follow the same length rule as the model id. Refs elie222#1370 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 0d8b725 commit 6214376

3 files changed

Lines changed: 98 additions & 49 deletions

File tree

‎apps/api/src/router.test.ts‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3668,7 +3668,7 @@ describe("openai-compatible enabled models", () => {
36683668
userId?: string;
36693669
modelProvider?: string;
36703670
provider?: string;
3671-
modelId?: { notIn?: string[] };
3671+
modelId?: string | { notIn?: string[] };
36723672
};
36733673

36743674
function secretText(enabledModelIds?: string[], reasoning?: boolean) {
@@ -3691,7 +3691,9 @@ describe("openai-compatible enabled models", () => {
36913691
(where.spaceId === undefined || pin.spaceId === where.spaceId) &&
36923692
(where.userId === undefined || pin.userId === where.userId) &&
36933693
!(where.archivedAt === null && pin.archived) &&
3694-
!where.modelId?.notIn?.includes(pin.modelId),
3694+
(typeof where.modelId === "string"
3695+
? pin.modelId === where.modelId
3696+
: !where.modelId?.notIn?.includes(pin.modelId)),
36953697
)
36963698
.map(({ name, spaceId, modelId }) => ({ name, spaceId, modelId }));
36973699
}
@@ -3832,7 +3834,7 @@ describe("openai-compatible enabled models", () => {
38323834
bot: {
38333835
findFirst: vi.fn(async () => bot),
38343836
findMany: vi.fn(async ({ where }: { where: PinWhere }) =>
3835-
where.modelId?.notIn ? pins.bots(where) : [bot],
3837+
where.modelProvider ? pins.bots(where) : [bot],
38363838
),
38373839
update: vi.fn(async ({ data }: { data: Record<string, unknown> }) =>
38383840
Object.assign(
@@ -4161,6 +4163,39 @@ describe("openai-compatible enabled models", () => {
41614163
expect(enabledInDto()).toContain("m-b");
41624164
});
41634165

4166+
it.each([
4167+
["a same-space bot on another non-default id", { name: "Ada", modelId: "m-z" }],
4168+
["a bot in another space", { name: "Elsewhere", modelId: "m-w", spaceId: "space-2" }],
4169+
[
4170+
"another space's bot on the outgoing default",
4171+
{ name: "Far", modelId: "m-a", spaceId: "space-2" },
4172+
],
4173+
])("lets setDefault on a legacy secret ignore %s", async (_case, bot) => {
4174+
const { call, state } = fixture({
4175+
plaintext: secretText(),
4176+
defaultModel: "m-a",
4177+
bots: [bot],
4178+
});
4179+
4180+
const result = await call("models/setDefault", { provider: compatible, modelId: "m-x" });
4181+
4182+
expect(result.status).toBe(200);
4183+
expect(state.preference.modelId).toBe("m-x");
4184+
});
4185+
4186+
it("skips the pin check when the outgoing default stays in the stored list", async () => {
4187+
const { call, prisma } = fixture({
4188+
plaintext: secretText(["m-a", "m-x"]),
4189+
defaultModel: "m-a",
4190+
bots: [{ name: "Ada", modelId: "m-a" }],
4191+
});
4192+
4193+
const result = await call("models/setDefault", { provider: compatible, modelId: "m-x" });
4194+
4195+
expect(result.status).toBe(200);
4196+
expect(prisma.routine.findMany).not.toHaveBeenCalled();
4197+
});
4198+
41644199
it("refuses setDefault on a legacy secret away from a pinned default", async () => {
41654200
const { call, prisma } = fixture({
41664201
plaintext: secretText(),

‎apps/api/src/router.ts‎

Lines changed: 59 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6909,73 +6909,83 @@ function nextRoutineDate(crons: string[], timezone: string): Date {
69096909
return next;
69106910
}
69116911

6912-
/**
6913-
* Names of the actor's active bots and routines (plus "backup models"), in every space, that
6914-
* pin an openai-compatible id outside their own space's effective list once `stored` is the
6915-
* connection's list and `actorDefault` is the actor space's default. The secret is per user
6916-
* and account-wide; each other space keeps its own saved default. Catalog-listed compatible
6917-
* ids pass today's gates without the connection's list, so they are never reported.
6918-
*/
6919-
async function modelPinUsers(
6912+
/** Catalog-listed compatible ids pass today's gates without the connection's list. */
6913+
function catalogCompatibleModelIds() {
6914+
return listPiCatalog()
6915+
.filter((entry) => entry.provider === OPENAI_COMPATIBLE_PROVIDER_ID)
6916+
.map((entry) => entry.id);
6917+
}
6918+
6919+
/** The actor's active compatible bot, routine and backup pins matching `filter`. */
6920+
async function compatibleModelPins(
69206921
deps: RouterDeps,
69216922
actor: Actor,
6922-
stored: string[],
6923-
actorDefault: string,
6923+
filter: { spaceId?: string; modelId: string | { notIn: string[] } },
69246924
) {
69256925
const provider = OPENAI_COMPATIBLE_PROVIDER_ID;
6926-
const excluded = [
6927-
...stored,
6928-
...listPiCatalog()
6929-
.filter((entry) => entry.provider === provider)
6930-
.map((entry) => entry.id),
6931-
];
6932-
const modelId = { notIn: excluded };
6926+
const where = { userId: actor.userId, ...filter };
69336927
const pin = { spaceId: true, modelId: true } as const;
6934-
const [preferences, bots, routines, backups] = await Promise.all([
6935-
deps.prisma.spaceModelPreference.findMany({
6936-
where: { userId: actor.userId, credential: { userId: actor.userId, provider } },
6937-
select: { spaceId: true, modelId: true },
6938-
}),
6928+
const [bots, routines, backups] = await Promise.all([
69396929
deps.prisma.bot.findMany({
6940-
where: { userId: actor.userId, archivedAt: null, modelProvider: provider, modelId },
6930+
where: { ...where, archivedAt: null, modelProvider: provider },
69416931
select: { ...pin, name: true },
69426932
}),
69436933
deps.prisma.routine.findMany({
6944-
where: { userId: actor.userId, modelProvider: provider, modelId },
6934+
where: { ...where, modelProvider: provider },
69456935
select: { ...pin, name: true },
69466936
}),
6947-
deps.prisma.spaceBackupModel.findMany({
6948-
where: { userId: actor.userId, provider, modelId },
6949-
select: pin,
6950-
}),
6937+
deps.prisma.spaceBackupModel.findMany({ where: { ...where, provider }, select: pin }),
69516938
]);
6952-
const defaults = new Map(preferences.map((row) => [row.spaceId, usableModelId(row.modelId)]));
6953-
defaults.set(actor.spaceId, usableModelId(actorDefault));
6954-
const dangling = (row: { spaceId: string; modelId: string | null }) =>
6955-
row.modelId !== null && row.modelId !== defaults.get(row.spaceId);
6956-
const names = [...bots, ...routines].filter(dangling).map((row) => row.name);
6957-
if (backups.some(dangling)) names.push("backup models");
6958-
return [...new Set(names)];
6939+
return { named: [...bots, ...routines], backups };
69596940
}
69606941

6961-
/** Refuse a compatible connection change that would leave one of the actor's pins dangling. */
6942+
type ModelPinRow = { spaceId: string; modelId: string | null };
6943+
6944+
/** Refusal naming the bots and routines (plus "backup models") left on a dropped model. */
6945+
function refuseModelPins(
6946+
pins: { named: Array<ModelPinRow & { name: string }>; backups: ModelPinRow[] },
6947+
dangling: (row: ModelPinRow) => boolean,
6948+
) {
6949+
const names = pins.named.filter(dangling).map((row) => row.name);
6950+
if (pins.backups.some(dangling)) names.push("backup models");
6951+
if (names.length === 0) return;
6952+
throw new ORPCError("BAD_REQUEST", {
6953+
message: `In use by ${[...new Set(names)].join(", ")}. Pick another model for them first.`,
6954+
});
6955+
}
6956+
6957+
/**
6958+
* Refuse a compatible connect whose saved list would leave one of the actor's pins, in any
6959+
* space, outside that space's effective list (`stored` plus the space's own default; the
6960+
* actor's space gets `actorDefault`). The secret is per user and account-wide.
6961+
*/
69626962
async function assertNoDanglingModelPins(
69636963
deps: RouterDeps,
69646964
actor: Actor,
69656965
stored: string[],
69666966
actorDefault: string,
69676967
) {
6968-
const users = await modelPinUsers(deps, actor, stored, actorDefault);
6969-
if (users.length === 0) return;
6970-
throw new ORPCError("BAD_REQUEST", {
6971-
message: `In use by ${users.join(", ")}. Pick another model for them first.`,
6972-
});
6968+
const [preferences, pins] = await Promise.all([
6969+
deps.prisma.spaceModelPreference.findMany({
6970+
where: {
6971+
userId: actor.userId,
6972+
credential: { userId: actor.userId, provider: OPENAI_COMPATIBLE_PROVIDER_ID },
6973+
},
6974+
select: { spaceId: true, modelId: true },
6975+
}),
6976+
compatibleModelPins(deps, actor, {
6977+
modelId: { notIn: [...stored, ...catalogCompatibleModelIds()] },
6978+
}),
6979+
]);
6980+
const defaults = new Map(preferences.map((row) => [row.spaceId, usableModelId(row.modelId)]));
6981+
defaults.set(actor.spaceId, usableModelId(actorDefault));
6982+
refuseModelPins(pins, (row) => row.modelId !== null && row.modelId !== defaults.get(row.spaceId));
69736983
}
69746984

69756985
/**
69766986
* `models.setDefault` on an openai-compatible connection: a listed secret only accepts an
6977-
* effective id, and moving off a default that is effective only through the per-space
6978-
* union (legacy secret, or a default outside the stored list) must not strand its pins.
6987+
* effective id. It changes only this space's default and never the secret, so the only pins
6988+
* it can strand are this space's pins on an outgoing default that is outside the stored list.
69796989
*/
69806990
async function assertCompatibleDefaultChange(
69816991
deps: RouterDeps,
@@ -6990,8 +7000,12 @@ async function assertCompatibleDefaultChange(
69907000
throw new ORPCError("BAD_REQUEST", { message: "Unknown model for that provider" });
69917001
}
69927002
const outgoing = enabled.find((id) => id !== requested && !stored?.includes(id));
6993-
if (!outgoing) return;
6994-
await assertNoDanglingModelPins(deps, actor, stored ?? [], requested);
7003+
if (!outgoing || catalogCompatibleModelIds().includes(outgoing)) return;
7004+
const pins = await compatibleModelPins(deps, actor, {
7005+
spaceId: actor.spaceId,
7006+
modelId: outgoing,
7007+
});
7008+
refuseModelPins(pins, () => true);
69957009
}
69967010

69977011
/** A routine may only name a model the space has connected. */

‎packages/contracts/src/domain.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ export const ModelConnectInputSchema = z
10841084
maxTokens: z.number().int().min(1).max(MAX_MODEL_MAX_TOKENS).nullable().optional(),
10851085
supportsImages: z.boolean().optional(),
10861086
maxImagesPerPrompt: z.number().int().min(1).max(1000).nullable().optional(),
1087-
enabledModelIds: z.array(z.string().trim().min(1).max(256)).max(500).optional(),
1087+
enabledModelIds: z.array(z.string().trim().min(1)).max(500).optional(),
10881088
})
10891089
.superRefine((value, ctx) => {
10901090
if (!modelOutputLeavesInputRoom(value.maxTokens, value.contextWindow)) {

0 commit comments

Comments
 (0)