mirror of
https://github.com/openclaw/clawhub.git
synced 2026-08-14 00:47:57 +00:00
fix: remove skill slug alias quotas (#3218)
This commit is contained in:
@@ -782,8 +782,10 @@ describe("skills ownership", () => {
|
||||
build(chainEq(constraints));
|
||||
if (name === "by_skill") {
|
||||
return {
|
||||
collect: async () =>
|
||||
aliases.filter((alias) => alias.skillId === constraints.skillId),
|
||||
take: async (limit: number) =>
|
||||
aliases
|
||||
.filter((alias) => alias.skillId === constraints.skillId)
|
||||
.slice(0, limit),
|
||||
};
|
||||
}
|
||||
if (name === "by_slug") {
|
||||
@@ -1039,8 +1041,10 @@ describe("skills ownership", () => {
|
||||
build(chainEq(constraints));
|
||||
if (name === "by_skill") {
|
||||
return {
|
||||
collect: async () =>
|
||||
aliases.filter((alias) => alias.skillId === constraints.skillId),
|
||||
take: async (limit: number) =>
|
||||
aliases
|
||||
.filter((alias) => alias.skillId === constraints.skillId)
|
||||
.slice(0, limit),
|
||||
};
|
||||
}
|
||||
if (name === "by_owner_publisher_slug") {
|
||||
@@ -1186,7 +1190,7 @@ describe("skills ownership", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("allows publisher admins to rename publisher-owned skills", async () => {
|
||||
it("allows publisher admins to rename beyond the former historical alias quota", async () => {
|
||||
const patch = vi.fn(async () => {});
|
||||
const insert = vi.fn(async () => "skillSlugAliases:old");
|
||||
const skill = {
|
||||
@@ -1205,6 +1209,13 @@ describe("skills ownership", () => {
|
||||
createdAt: 1_700_000_000_000,
|
||||
softDeletedAt: undefined,
|
||||
};
|
||||
const aliases = Array.from({ length: 25 }, (_, index) => ({
|
||||
_id: `skillSlugAliases:old-${index}`,
|
||||
slug: `historical-name-${index}`,
|
||||
skillId: "skills:source",
|
||||
ownerUserId: "users:creator",
|
||||
ownerPublisherId: "publishers:org",
|
||||
}));
|
||||
|
||||
const result = await renameOwnedSkillInternalHandler(
|
||||
{
|
||||
@@ -1259,8 +1270,8 @@ describe("skills ownership", () => {
|
||||
if (name === "by_owner_publisher_slug") return { unique: async () => null };
|
||||
if (name === "by_owner_slug") return { unique: async () => null };
|
||||
if (name === "by_slug") return { take: async () => [], unique: async () => null };
|
||||
if (name === "by_skill") return { collect: async () => [] };
|
||||
if (name === "by_owner_publisher") return { take: async () => [] };
|
||||
if (name === "by_skill") return { collect: async () => aliases };
|
||||
if (name === "by_owner_publisher") return { take: async () => aliases };
|
||||
throw new Error(`unexpected skillSlugAliases index ${name}`);
|
||||
},
|
||||
};
|
||||
@@ -2415,7 +2426,7 @@ describe("skills ownership", () => {
|
||||
expect(patch).not.toHaveBeenCalledWith("skills:source", expect.anything());
|
||||
});
|
||||
|
||||
it("rejects merges that would reserve too many historical slugs for one skill", async () => {
|
||||
it("bounds aliases rewritten by a single merge transaction", async () => {
|
||||
const patch = vi.fn(async () => {});
|
||||
const insert = vi.fn(async () => "auditLogs:1");
|
||||
const skills = [
|
||||
@@ -2436,7 +2447,7 @@ describe("skills ownership", () => {
|
||||
softDeletedAt: undefined,
|
||||
},
|
||||
];
|
||||
const aliases = Array.from({ length: 5 }, (_, index) => ({
|
||||
const aliases = Array.from({ length: 201 }, (_, index) => ({
|
||||
_id: `skillSlugAliases:target-${index}`,
|
||||
slug: `target-old-${index}`,
|
||||
skillId: "skills:target",
|
||||
@@ -2469,9 +2480,7 @@ describe("skills ownership", () => {
|
||||
};
|
||||
}
|
||||
if (name === "by_owner_publisher_slug") {
|
||||
return {
|
||||
unique: async () => null,
|
||||
};
|
||||
return { unique: async () => null };
|
||||
}
|
||||
if (name === "by_owner_slug") {
|
||||
return {
|
||||
@@ -2485,9 +2494,6 @@ describe("skills ownership", () => {
|
||||
.slice(0, 25),
|
||||
};
|
||||
}
|
||||
if (name === "by_canonical" || name === "by_fork_of") {
|
||||
return { collect: async () => [] };
|
||||
}
|
||||
throw new Error(`unexpected skills index ${name}`);
|
||||
},
|
||||
};
|
||||
@@ -2499,22 +2505,20 @@ describe("skills ownership", () => {
|
||||
build(chainEq(constraints));
|
||||
if (name === "by_skill") {
|
||||
return {
|
||||
collect: async () =>
|
||||
aliases.filter((alias) => alias.skillId === constraints.skillId),
|
||||
take: async (limit: number) =>
|
||||
aliases
|
||||
.filter((alias) => alias.skillId === constraints.skillId)
|
||||
.slice(0, limit),
|
||||
};
|
||||
}
|
||||
if (name === "by_slug") {
|
||||
return {
|
||||
take: async () =>
|
||||
aliases.filter((alias) => alias.slug === constraints.slug).slice(0, 2),
|
||||
unique: async () =>
|
||||
aliases.find((alias) => alias.slug === constraints.slug) ?? null,
|
||||
};
|
||||
}
|
||||
if (name === "by_owner_publisher_slug") {
|
||||
return {
|
||||
unique: async () => null,
|
||||
};
|
||||
return { unique: async () => null };
|
||||
}
|
||||
if (name === "by_owner_slug") {
|
||||
return {
|
||||
@@ -2528,12 +2532,6 @@ describe("skills ownership", () => {
|
||||
.slice(0, 25),
|
||||
};
|
||||
}
|
||||
if (name === "by_owner") {
|
||||
return {
|
||||
take: async () =>
|
||||
aliases.filter((alias) => alias.ownerUserId === constraints.ownerUserId),
|
||||
};
|
||||
}
|
||||
throw new Error(`unexpected skillSlugAliases index ${name}`);
|
||||
},
|
||||
};
|
||||
@@ -2550,7 +2548,7 @@ describe("skills ownership", () => {
|
||||
targetSlug: "merge-target",
|
||||
},
|
||||
),
|
||||
).rejects.toThrow(/Too many historical slugs/);
|
||||
).rejects.toThrow(/cannot be merged in one transaction/);
|
||||
|
||||
expect(patch).not.toHaveBeenCalled();
|
||||
expect(insert).not.toHaveBeenCalled();
|
||||
|
||||
+12
-120
@@ -217,8 +217,7 @@ const SLUG_RESERVATION_DAYS = 90;
|
||||
const SLUG_RESERVATION_MS = SLUG_RESERVATION_DAYS * RATE_LIMIT_DAY_MS;
|
||||
const UNPUBLISHED_SLUG_RESERVATION_DAYS = 30;
|
||||
const UNPUBLISHED_SLUG_RESERVATION_MS = UNPUBLISHED_SLUG_RESERVATION_DAYS * RATE_LIMIT_DAY_MS;
|
||||
const MAX_SKILL_SLUG_ALIASES_PER_SKILL = 5;
|
||||
const MAX_SKILL_SLUG_ALIASES_PER_OWNER = 25;
|
||||
const MAX_SKILL_SLUG_ALIASES_PER_MERGE = 200;
|
||||
const MAX_MANUAL_OVERRIDE_NOTE_LENGTH = 1200;
|
||||
const DEFAULT_STAFF_AUDIT_LOG_LIMIT = 10;
|
||||
const MAX_STAFF_AUDIT_LOG_LIMIT = 50;
|
||||
@@ -1291,14 +1290,20 @@ async function getSkillSlugAliasBySlug(ctx: Pick<QueryCtx | MutationCtx, "db">,
|
||||
return resolved.alias;
|
||||
}
|
||||
|
||||
async function listSkillSlugAliasesForSkill(
|
||||
async function listSkillSlugAliasesForMerge(
|
||||
ctx: Pick<QueryCtx | MutationCtx, "db">,
|
||||
skillId: Id<"skills">,
|
||||
) {
|
||||
return ctx.db
|
||||
const aliases = await ctx.db
|
||||
.query("skillSlugAliases")
|
||||
.withIndex("by_skill", (q) => q.eq("skillId", skillId))
|
||||
.collect();
|
||||
.take(MAX_SKILL_SLUG_ALIASES_PER_MERGE + 1);
|
||||
if (aliases.length > MAX_SKILL_SLUG_ALIASES_PER_MERGE) {
|
||||
throw new ConvexError(
|
||||
`A skill with more than ${MAX_SKILL_SLUG_ALIASES_PER_MERGE} historical slugs cannot be merged in one transaction. Contact a ClawHub maintainer for a batched migration.`,
|
||||
);
|
||||
}
|
||||
return aliases;
|
||||
}
|
||||
|
||||
function sameSkillSlugAliasOwner(
|
||||
@@ -1312,75 +1317,6 @@ function sameSkillSlugAliasOwner(
|
||||
);
|
||||
}
|
||||
|
||||
async function countSkillSlugAliasesForOwnerQuota(
|
||||
ctx: Pick<QueryCtx | MutationCtx, "db">,
|
||||
ownerUserId: Id<"users">,
|
||||
ownerPublisherId: Id<"publishers"> | undefined,
|
||||
) {
|
||||
if (ownerPublisherId) {
|
||||
const aliases = await ctx.db
|
||||
.query("skillSlugAliases")
|
||||
.withIndex("by_owner_publisher", (q) => q.eq("ownerPublisherId", ownerPublisherId))
|
||||
.take(MAX_SKILL_SLUG_ALIASES_PER_OWNER + 1);
|
||||
return aliases.length;
|
||||
}
|
||||
|
||||
const aliases = await ctx.db
|
||||
.query("skillSlugAliases")
|
||||
.withIndex("by_owner", (q) => q.eq("ownerUserId", ownerUserId))
|
||||
.take(MAX_SKILL_SLUG_ALIASES_PER_OWNER + 1);
|
||||
return aliases.length;
|
||||
}
|
||||
|
||||
async function assertSkillSlugAliasQuota(
|
||||
ctx: Pick<QueryCtx | MutationCtx, "db">,
|
||||
params: {
|
||||
targetSkillId: Id<"skills">;
|
||||
ownerUserId: Id<"users">;
|
||||
ownerPublisherId: Id<"publishers"> | undefined;
|
||||
currentSkillAliasCount?: number;
|
||||
addedSkillAliases: number;
|
||||
removedSkillAliases?: number;
|
||||
addedOwnerAliases: number;
|
||||
removedOwnerAliases?: number;
|
||||
},
|
||||
) {
|
||||
const addedSkillAliases = Math.max(0, params.addedSkillAliases);
|
||||
const removedSkillAliases = Math.max(0, params.removedSkillAliases ?? 0);
|
||||
const addedOwnerAliases = Math.max(0, params.addedOwnerAliases);
|
||||
const removedOwnerAliases = Math.max(0, params.removedOwnerAliases ?? 0);
|
||||
|
||||
const currentSkillAliasCount =
|
||||
params.currentSkillAliasCount ??
|
||||
(await listSkillSlugAliasesForSkill(ctx, params.targetSkillId)).length;
|
||||
const nextSkillAliasCount =
|
||||
Math.max(0, currentSkillAliasCount - removedSkillAliases) + addedSkillAliases;
|
||||
if (nextSkillAliasCount > MAX_SKILL_SLUG_ALIASES_PER_SKILL) {
|
||||
throw new ConvexError(
|
||||
"Too many historical slugs are already reserved for this skill. " +
|
||||
`A skill can keep at most ${MAX_SKILL_SLUG_ALIASES_PER_SKILL} old slug redirects. ` +
|
||||
"Contact support@openclaw.ai if this is a legitimate migration.",
|
||||
);
|
||||
}
|
||||
|
||||
if (addedOwnerAliases === 0 && removedOwnerAliases === 0) return;
|
||||
|
||||
const currentOwnerAliasCount = await countSkillSlugAliasesForOwnerQuota(
|
||||
ctx,
|
||||
params.ownerUserId,
|
||||
params.ownerPublisherId,
|
||||
);
|
||||
const nextOwnerAliasCount =
|
||||
Math.max(0, currentOwnerAliasCount - removedOwnerAliases) + addedOwnerAliases;
|
||||
if (nextOwnerAliasCount > MAX_SKILL_SLUG_ALIASES_PER_OWNER) {
|
||||
throw new ConvexError(
|
||||
"Too many historical slugs are already reserved by this owner. " +
|
||||
`An owner can keep at most ${MAX_SKILL_SLUG_ALIASES_PER_OWNER} old slug redirects. ` +
|
||||
"Contact support@openclaw.ai if this is a legitimate migration.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function releaseExpiredUnpublishedSkillSlug(
|
||||
ctx: MutationCtx,
|
||||
skill: Doc<"skills">,
|
||||
@@ -11163,32 +11099,7 @@ async function renameOwnedSkillByActor(
|
||||
throw new ConvexError(formatReservedSlugCooldownMessage(newSlug, reservation.expiresAt));
|
||||
}
|
||||
|
||||
const aliasesForSkill = await listSkillSlugAliasesForSkill(ctx, skill._id);
|
||||
const aliasRemovedForNewSlug =
|
||||
existingAlias && existingAlias.skillId === skill._id ? existingAlias : null;
|
||||
const previousAlias = await getSkillSlugAliasBySlugForPublisher(ctx, skill.slug, skillOwner);
|
||||
const addedSkillAliases = previousAlias?.skillId === skill._id ? 0 : 1;
|
||||
const removedSkillAliases = aliasRemovedForNewSlug ? 1 : 0;
|
||||
const addedOwnerAliases = previousAlias
|
||||
? sameSkillSlugAliasOwner(previousAlias, skill.ownerUserId, skill.ownerPublisherId)
|
||||
? 0
|
||||
: 1
|
||||
: 1;
|
||||
const removedOwnerAliases =
|
||||
aliasRemovedForNewSlug &&
|
||||
sameSkillSlugAliasOwner(aliasRemovedForNewSlug, skill.ownerUserId, skill.ownerPublisherId)
|
||||
? 1
|
||||
: 0;
|
||||
await assertSkillSlugAliasQuota(ctx, {
|
||||
targetSkillId: skill._id,
|
||||
ownerUserId: skill.ownerUserId,
|
||||
ownerPublisherId: skill.ownerPublisherId,
|
||||
currentSkillAliasCount: aliasesForSkill.length,
|
||||
addedSkillAliases,
|
||||
removedSkillAliases,
|
||||
addedOwnerAliases,
|
||||
removedOwnerAliases,
|
||||
});
|
||||
|
||||
if (existingAlias && existingAlias.skillId === skill._id) {
|
||||
await ctx.db.delete(existingAlias._id);
|
||||
@@ -11292,9 +11203,9 @@ async function mergeOwnedSkillIntoCanonicalByActor(
|
||||
: null;
|
||||
const targetCanonicalSkillId = target.canonicalSkillId ?? target._id;
|
||||
|
||||
const targetAliases = await listSkillSlugAliasesForSkill(ctx, target._id);
|
||||
const targetAliases = await listSkillSlugAliasesForMerge(ctx, target._id);
|
||||
const targetAliasSlugs = new Set(targetAliases.map((alias) => alias.slug));
|
||||
const aliases = await listSkillSlugAliasesForSkill(ctx, source._id);
|
||||
const aliases = await listSkillSlugAliasesForMerge(ctx, source._id);
|
||||
const targetPublisher = await getOwnerPublisher(ctx, {
|
||||
ownerPublisherId: target.ownerPublisherId,
|
||||
ownerUserId: target.ownerUserId,
|
||||
@@ -11358,27 +11269,8 @@ async function mergeOwnedSkillIntoCanonicalByActor(
|
||||
}
|
||||
}
|
||||
|
||||
await assertSkillSlugAliasQuota(ctx, {
|
||||
targetSkillId: target._id,
|
||||
ownerUserId: target.ownerUserId,
|
||||
ownerPublisherId: target.ownerPublisherId,
|
||||
currentSkillAliasCount: targetAliases.length,
|
||||
addedSkillAliases: addedSkillAliasSlugs.size,
|
||||
addedOwnerAliases: sourceOwnerMatchesTargetOwner ? addedOwnerAliasSlugs.size : 0,
|
||||
});
|
||||
|
||||
const willInsertSourceAlias =
|
||||
!sourceAlias && (!sourceOwnerMatchesTargetOwner || source.slug !== target.slug);
|
||||
if (!sourceOwnerMatchesTargetOwner && willInsertSourceAlias) {
|
||||
await assertSkillSlugAliasQuota(ctx, {
|
||||
targetSkillId: target._id,
|
||||
ownerUserId: source.ownerUserId,
|
||||
ownerPublisherId: source.ownerPublisherId,
|
||||
currentSkillAliasCount: targetAliases.length,
|
||||
addedSkillAliases: 0,
|
||||
addedOwnerAliases: 1,
|
||||
});
|
||||
}
|
||||
|
||||
for (const alias of aliases) {
|
||||
if (sourceOwnerMatchesTargetOwner && alias.slug === target.slug) {
|
||||
|
||||
@@ -135,10 +135,12 @@ protected affixes such as `openclaw-`, `-openclaw`, `official-`, or
|
||||
`-official` are blocked unless an internal/admin path explicitly bypasses the
|
||||
reserved list for a controlled migration.
|
||||
|
||||
Historical slug redirects are also bounded. Rename and merge may preserve old
|
||||
slugs as aliases, but a single skill can keep at most five historical slug
|
||||
redirects and an owner/publisher can keep at most 25. These limits prevent
|
||||
alias-hoarding while preserving ordinary rename and duplicate-merge redirects.
|
||||
Rename and merge preserve historical slugs as aliases in the owning publisher's
|
||||
namespace. Historical aliases are not quota-limited because they do not reserve
|
||||
names across other publishers. Collision checks still prevent two skills or
|
||||
aliases from claiming the same slug within one publisher namespace.
|
||||
The one-shot merge mutation keeps a separate operational ceiling on aliases it
|
||||
can rewrite in one transaction; larger histories require a batched migration.
|
||||
|
||||
Owner-initiated unpublishes must not reserve a slug forever. When an owner
|
||||
soft-deletes a skill, the slug remains reserved for 30 days so they can restore
|
||||
|
||||
Reference in New Issue
Block a user