diff --git a/convex/lib/publicBrowse.test.ts b/convex/lib/publicBrowse.test.ts new file mode 100644 index 00000000..cc0217fd --- /dev/null +++ b/convex/lib/publicBrowse.test.ts @@ -0,0 +1,106 @@ +/* @vitest-environment node */ +import { describe, expect, it } from "vitest"; +import { + hasPriorApprovedPublicSkillVersion, + isPubliclyListableSkillVersion, + isSkillPendingPublicReview, + shouldExcludeSkillFromPublicBrowse, +} from "./publicBrowse"; + +describe("publicBrowse", () => { + it("treats scanner review flags as pending public review", () => { + expect( + isSkillPendingPublicReview({ + moderationStatus: "active", + moderationReason: "scanner.llm.review", + moderationFlags: ["flagged.review"], + }), + ).toBe(true); + }); + + it("treats active pending.scan skills as pending public review", () => { + expect( + isSkillPendingPublicReview({ + moderationStatus: "active", + moderationReason: "pending.scan", + moderationFlags: undefined, + }), + ).toBe(true); + }); + + it("excludes first-publish pending review skills from public browse", () => { + expect( + shouldExcludeSkillFromPublicBrowse({ + softDeletedAt: undefined, + moderationStatus: "active", + moderationReason: "pending.scan", + moderationFlags: undefined, + moderationVerdict: "clean", + moderationSourceVersionId: undefined, + latestVersionId: "skillVersions:1", + githubScanStatus: "clean", + stats: { + versions: 1, + downloads: 0, + stars: 0, + installsCurrent: 0, + installsAllTime: 0, + comments: 0, + }, + }), + ).toBe(true); + }); + + it("keeps previously approved skills visible while a newer version is pending review", () => { + expect( + hasPriorApprovedPublicSkillVersion({ + stats: { + versions: 2, + downloads: 0, + stars: 0, + installsCurrent: 0, + installsAllTime: 0, + comments: 0, + }, + }), + ).toBe(true); + expect( + shouldExcludeSkillFromPublicBrowse({ + softDeletedAt: undefined, + moderationStatus: "active", + moderationReason: "pending.scan", + moderationFlags: undefined, + moderationVerdict: "clean", + moderationSourceVersionId: "skillVersions:2", + latestVersionId: "skillVersions:2", + githubScanStatus: "clean", + stats: { + versions: 2, + downloads: 0, + stars: 0, + installsCurrent: 0, + installsAllTime: 0, + comments: 0, + }, + }), + ).toBe(false); + }); + + it("rejects pending-review skill versions from public listing", () => { + expect( + isPubliclyListableSkillVersion({ + _id: "skillVersions:pending", + skillId: "skills:1", + softDeletedAt: undefined, + version: "2.0.0", + createdAt: 1, + changelog: "c", + changelogSource: "user", + parsed: { frontmatter: {}, license: "MIT" }, + vtAnalysis: { status: "pending", checkedAt: 1 }, + llmAnalysis: undefined, + staticScan: undefined, + }), + ).toBe(false); + }); +}); diff --git a/convex/lib/publicBrowse.ts b/convex/lib/publicBrowse.ts new file mode 100644 index 00000000..a36d3fdf --- /dev/null +++ b/convex/lib/publicBrowse.ts @@ -0,0 +1,111 @@ +import type { Doc, Id } from "../_generated/dataModel"; +import type { MutationCtx, QueryCtx } from "../_generated/server"; +import { isPublicSkillDoc } from "./globalStats"; +import { + isSecurityScanStatusBlockedFromPublic, + normalizeSecurityScanStatus, +} from "./securityScanPolicy"; +import { isSkillReviewFlagged, isSkillSuspicious } from "./skillSafety"; + +type SkillPublicBrowseFields = Pick< + Doc<"skills">, + | "softDeletedAt" + | "moderationStatus" + | "moderationReason" + | "moderationFlags" + | "moderationVerdict" + | "moderationSourceVersionId" + | "latestVersionId" + | "githubScanStatus" + | "stats" +>; + +type SkillVersionPublicBrowseFields = Pick< + Doc<"skillVersions">, + | "_id" + | "skillId" + | "softDeletedAt" + | "version" + | "createdAt" + | "changelog" + | "changelogSource" + | "parsed" + | "vtAnalysis" + | "llmAnalysis" + | "staticScan" +>; + +function isPendingSkillModerationReason(reason: string | null | undefined) { + const normalized = reason?.trim().toLowerCase(); + return ( + normalized === "pending.scan" || + normalized === "pending.scan.stale" || + normalized === "scanner.vt.pending" || + normalized === "scanner.llm.pending" + ); +} + +export function isSkillPendingPublicReview( + skill: Pick, "moderationStatus" | "moderationReason" | "moderationFlags">, +) { + if (isSkillReviewFlagged(skill)) return true; + return isPendingSkillModerationReason(skill.moderationReason); +} + +export function hasPriorApprovedPublicSkillVersion(skill: Pick, "stats">) { + return (skill.stats?.versions ?? 0) > 1; +} + +export function shouldExcludeSkillFromPublicBrowse(skill: SkillPublicBrowseFields) { + if (!isPublicSkillDoc(skill)) return true; + if (isSkillSuspicious(skill)) return true; + if (normalizeSecurityScanStatus(skill.githubScanStatus) === "pending") return true; + if (!isSkillPendingPublicReview(skill)) return false; + return !hasPriorApprovedPublicSkillVersion(skill); +} + +export function isPubliclyListableSkillVersion( + version: SkillVersionPublicBrowseFields | null | undefined, +) { + if (!version || version.softDeletedAt) return false; + const statuses = [ + normalizeSecurityScanStatus(version.vtAnalysis?.status), + normalizeSecurityScanStatus(version.llmAnalysis?.verdict ?? version.llmAnalysis?.status), + normalizeSecurityScanStatus(version.staticScan?.status), + ]; + if (statuses.some((status) => status === "pending" || status === "not-run")) return false; + return !statuses.some((status) => isSecurityScanStatusBlockedFromPublic(status)); +} + +export async function resolvePublicBrowseVersionForSkill( + ctx: Pick, + skill: SkillPublicBrowseFields & { _id: Id<"skills"> }, +): Promise | null> { + const latestVersionId = skill.latestVersionId; + if (!latestVersionId) return null; + + if (!isSkillPendingPublicReview(skill)) { + const latestVersion = await ctx.db.get(latestVersionId); + return latestVersion && + latestVersion.skillId === skill._id && + isPubliclyListableSkillVersion(latestVersion) + ? latestVersion + : null; + } + + if (!hasPriorApprovedPublicSkillVersion(skill)) return null; + + const versions = await ctx.db + .query("skillVersions") + .withIndex("by_skill_active_created", (q) => + q.eq("skillId", skill._id).eq("softDeletedAt", undefined), + ) + .order("desc") + .take(24); + + for (const version of versions) { + if (version._id === skill.moderationSourceVersionId) continue; + if (isPubliclyListableSkillVersion(version)) return version; + } + return null; +} diff --git a/convex/search.test.ts b/convex/search.test.ts index 595a6579..17a79851 100644 --- a/convex/search.test.ts +++ b/convex/search.test.ts @@ -708,7 +708,7 @@ describe("search helpers", () => { ); }); - it("preserves suspicious lexical fallback results when nonSuspiciousOnly is unset", async () => { + it("excludes suspicious lexical fallback results from public search", async () => { const clean = makeSkillDoc({ id: "skills:clean", slug: "orf-clean", displayName: "ORF Clean" }); const suspicious = makeSkillDoc({ id: "skills:suspicious", @@ -727,7 +727,7 @@ describe("search helpers", () => { limit: 10, }); - expect(result.map((entry) => entry.skill.slug)).toEqual(["orf-clean", "orf-suspicious"]); + expect(result.map((entry) => entry.skill.slug)).toEqual(["orf-clean"]); expect(ctx.usedIndexes).toEqual( expect.arrayContaining(["by_active_updated", "by_active_created"]), ); diff --git a/convex/search.ts b/convex/search.ts index 1de3c3fa..1410985e 100644 --- a/convex/search.ts +++ b/convex/search.ts @@ -16,6 +16,7 @@ import { generateEmbedding } from "./lib/embeddings"; import { hasOfficialPublisherRow, toPublicPublisherWithOfficial } from "./lib/officialPublishers"; import type { HydratableSkill, PublicPublisher } from "./lib/public"; import { toPublicSkill } from "./lib/public"; +import { shouldExcludeSkillFromPublicBrowse } from "./lib/publicBrowse"; import { getOwnerPublisher } from "./lib/publishers"; import { matchesAllTokens, @@ -270,6 +271,7 @@ function matchesCatalogFilters( } function toPublicSearchSkill(skill: HydratableSkill) { + if (shouldExcludeSkillFromPublicBrowse(skill)) return null; return toPublicSkill({ ...skill, categories: resolveStoredSkillCategories(skill), @@ -576,10 +578,15 @@ export const directPrefixSkillMatches = internalQuery({ ...(digest.categories ?? []), ...(digest.topics ?? []), ]); - const matchesDirectRecallFilters = (digest: Doc<"skillSearchDigest">) => - (!args.highlightedOnly || isSkillHighlighted(digestToHydratableSkill(digest))) && - passesAllQueryTokens(digest) && - matchesCatalogFilters(digest, categorySlug, topic); + const matchesDirectRecallFilters = (digest: Doc<"skillSearchDigest">) => { + const skill = digestToHydratableSkill(digest); + return ( + !shouldExcludeSkillFromPublicBrowse(skill) && + (!args.highlightedOnly || isSkillHighlighted(skill)) && + passesAllQueryTokens(digest) && + matchesCatalogFilters(skill, categorySlug, topic) + ); + }; const needsExpandedRecall = Boolean( categorySlug || topic || args.highlightedOnly || queryTokens.length > 1, ); @@ -950,7 +957,7 @@ export const lexicalFallbackSkills = internalQuery({ .take(MAX_EXACT_SLUG_MATCHES); for (const exactSlugSkill of exactSlugSkills) { if ( - !exactSlugSkill.softDeletedAt && + !shouldExcludeSkillFromPublicBrowse(exactSlugSkill) && (!args.nonSuspiciousOnly || !isSkillSuspicious(exactSlugSkill)) && (!args.excludePendingScan || exactSlugSkill.githubScanStatus !== "pending") && matchesCatalogFilters(exactSlugSkill, categorySlug, topic) @@ -994,6 +1001,7 @@ export const lexicalFallbackSkills = internalQuery({ const matchesFallbackRecallFilters = (digest: Doc<"skillSearchDigest">) => { const skill = digestToHydratableSkill(digest); return ( + !shouldExcludeSkillFromPublicBrowse(skill) && (!args.highlightedOnly || isSkillHighlighted(skill)) && (!args.excludePendingScan || skill.githubScanStatus !== "pending") && matchesCatalogFilters(skill, categorySlug, topic) && diff --git a/convex/skills.listPublicPageV4.test.ts b/convex/skills.listPublicPageV4.test.ts index 059a147a..3207a7e4 100644 --- a/convex/skills.listPublicPageV4.test.ts +++ b/convex/skills.listPublicPageV4.test.ts @@ -87,7 +87,7 @@ describe("skills.listPublicPageV4", () => { }); }); - it("falls back to updated results while recommendation scores are missing", () => { + it("falls back to the recommended rank index while recommendation scores are missing", () => { expect( __test.resolveRecommendedPublicListQuery({ scoreIndexName: "by_active_recommended_score", @@ -99,8 +99,8 @@ describe("skills.listPublicPageV4", () => { hasMissingScores: true, }), ).toEqual({ - sort: "updated", - indexName: "by_active_updated", + sort: "recommended", + indexName: "by_active_recommended_rank", decodedCursor: null, }); }); diff --git a/convex/skills.packageCatalog.test.ts b/convex/skills.packageCatalog.test.ts index 4e2856fc..267d2242 100644 --- a/convex/skills.packageCatalog.test.ts +++ b/convex/skills.packageCatalog.test.ts @@ -428,7 +428,7 @@ describe("skills package catalog queries", () => { expect(indexNames).not.toContain("by_active_topic_updated"); }); - it("falls topic recommendation sorting back to downloads while scores are missing", async () => { + it("keeps topic recommended sorting on the recommendation score index while scores are missing", async () => { const indexNames: string[] = []; const calendarSkill = makeDigest("calendar-skill", { topics: ["calendar"] }); @@ -444,7 +444,7 @@ describe("skills package catalog queries", () => { }, ], isDone: false, - continueCursor: "downloads-next", + continueCursor: "recommended-next", }, ], [calendarSkill], @@ -459,9 +459,9 @@ describe("skills package catalog queries", () => { ); expect(result.page.map((entry) => entry.name)).toEqual(["calendar-skill"]); - expect(indexNames).toContain("by_active_topic_downloads"); - expect(indexNames).not.toContain("by_active_topic_recommended_score"); - expect(result.continueCursor).toContain('"recommendedFallback":"downloads"'); + expect(indexNames).toContain("by_active_topic_recommended_score"); + expect(indexNames).not.toContain("by_active_topic_downloads"); + expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"'); }); it("keeps legacy topic recommendation fallback cursors on the updated index", async () => { @@ -606,12 +606,7 @@ describe("skills package catalog queries", () => { }, ); - expect(indexNames).toEqual([ - "by_active_recommended_score", - "by_active_recommended_score_version", - "by_active_recommended_score_version", - "by_active_recommended_score", - ]); + expect(indexNames).toEqual(["by_active_recommended_score"]); expect(result.page).toEqual([ expect.objectContaining({ name: "recommended-skill", @@ -620,7 +615,7 @@ describe("skills package catalog queries", () => { ]); }); - it("falls back to downloads sort for recommended package catalog rows while scores backfill", async () => { + it("keeps recommended package catalog rows on the recommendation score index while scores backfill", async () => { const indexNames: string[] = []; const result = await listPackageCatalogPageHandler( makeCtx( @@ -628,7 +623,7 @@ describe("skills package catalog queries", () => { { page: [makeDigest("fallback-skill")], isDone: false, - continueCursor: "next-updated-page", + continueCursor: "next-recommended-page", }, ], { @@ -644,13 +639,9 @@ describe("skills package catalog queries", () => { }, ); - expect(indexNames).toEqual([ - "by_active_recommended_score", - "by_active_recommended_score_version", - "by_active_stats_downloads", - ]); + expect(indexNames).toEqual(["by_active_recommended_score"]); expect(result.page).toEqual([expect.objectContaining({ name: "fallback-skill" })]); - expect(result.continueCursor).toContain('"recommendedFallback":"downloads"'); + expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"'); }); it("uses the recommended score index for recommended package catalog rows", async () => { @@ -681,7 +672,7 @@ describe("skills package catalog queries", () => { expect(result.page).toEqual([expect.objectContaining({ name: "recommended-skill" })]); }); - it("falls recommended package catalog rows back to downloads when scores are missing", async () => { + it("keeps recommended package catalog rows on the recommendation score index when scores are missing", async () => { const indexNames: string[] = []; const result = await listPackageCatalogPageHandler( makeCtx( @@ -689,7 +680,7 @@ describe("skills package catalog queries", () => { { page: [makeDigest("download-fallback-skill")], isDone: false, - continueCursor: "updated-next", + continueCursor: "recommended-next", }, ], { indexNames, missingRecommendedScores: true }, @@ -700,9 +691,9 @@ describe("skills package catalog queries", () => { }, ); - expect(indexNames).toEqual(["by_active_recommended_score", "by_active_stats_downloads"]); + expect(indexNames).toEqual(["by_active_recommended_score"]); expect(result.page).toEqual([expect.objectContaining({ name: "download-fallback-skill" })]); - expect(result.continueCursor).toContain('"recommendedFallback":"downloads"'); + expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"'); }); it("keeps recommended package catalog cursors on their original index", async () => { diff --git a/convex/skills.publicListCursor.test.ts b/convex/skills.publicListCursor.test.ts index 5ea2e1c7..88a248ac 100644 --- a/convex/skills.publicListCursor.test.ts +++ b/convex/skills.publicListCursor.test.ts @@ -382,7 +382,7 @@ describe("public skill list deterministic cursors", () => { }); }); - it("keeps topic recommendation fallback cursors on the updated index", async () => { + it("keeps topic recommended browse on the recommended score index when scores are missing", async () => { const firstDigest = makeSearchDigest({ skillId: "skills:calendar-one", slug: "calendar-one", @@ -452,12 +452,12 @@ describe("public skill list deterministic cursors", () => { expect(first.nextCursor).not.toBeNull(); expect(second.nextCursor).toBeNull(); expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({ - index: "by_active_topic_updated", + index: "by_active_topic_recommended_score", startIndexKey: [undefined, "calendar"], startInclusive: true, }); expect(getPageMock.mock.calls[1]?.[1]).toMatchObject({ - index: "by_active_topic_updated", + index: "by_active_topic_recommended_score", startIndexKey: firstIndexKey, startInclusive: false, }); @@ -543,7 +543,7 @@ describe("public skill list deterministic cursors", () => { }); }); - it("falls back to the updated index while recommendation scores are missing", async () => { + it("falls back to the recommended rank index while recommendation scores are missing", async () => { const { ctx, withIndex } = makeMissingRecommendedScoresCtx(); await listPublicPageV4Handler(ctx, { @@ -555,14 +555,14 @@ describe("public skill list deterministic cursors", () => { ]); expect(getPageMock).toHaveBeenCalledTimes(1); expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({ - index: "by_active_updated", + index: "by_active_recommended_rank", startIndexKey: [undefined], endIndexKey: [undefined], startInclusive: true, }); }); - it("falls back to the non-suspicious updated index while recommendation scores are missing", async () => { + it("falls back to the non-suspicious recommended rank index while recommendation scores are missing", async () => { const { ctx, withIndex } = makeMissingRecommendedScoresCtx(); await listPublicApiPageV1Handler(ctx, { @@ -576,7 +576,7 @@ describe("public skill list deterministic cursors", () => { ]); expect(getPageMock).toHaveBeenCalledTimes(1); expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({ - index: "by_nonsuspicious_updated", + index: "by_nonsuspicious_recommended_rank", startIndexKey: [undefined, false], endIndexKey: [undefined, false], startInclusive: true, diff --git a/convex/skills.ts b/convex/skills.ts index 06d4d061..353ce66c 100644 --- a/convex/skills.ts +++ b/convex/skills.ts @@ -87,6 +87,12 @@ import { toPublicSkill, toPublicUser, } from "./lib/public"; +import { + hasPriorApprovedPublicSkillVersion, + isSkillPendingPublicReview, + resolvePublicBrowseVersionForSkill, + shouldExcludeSkillFromPublicBrowse, +} from "./lib/publicBrowse"; import { assertCanManageOwnedResource, canAccessPublisherOwnerScope, @@ -5423,16 +5429,10 @@ export const listPublicPageV4 = query({ } if (officialFirstCursor && categorySlug) { - const sort = - officialFirstCursor.sort ?? - (requestedSort === "recommended" && - (await hasMissingRecommendedScores(ctx, args.nonSuspiciousOnly ?? false, null)) - ? "updated" - : requestedSort); return await listOfficialFirstSkillCategoryPage(ctx, { - state: { ...officialFirstCursor, sort }, - sort, - dir: resolvePublicListDir(sort, args.dir), + state: { ...officialFirstCursor, sort: officialFirstCursor.sort ?? requestedSort }, + sort: officialFirstCursor.sort ?? requestedSort, + dir: resolvePublicListDir(officialFirstCursor.sort ?? requestedSort, args.dir), numItems, topic, categorySlug, @@ -5713,20 +5713,9 @@ async function listSkillTopicFilteredPage( allowLegacyArray: false, }); const recommendedCursor = opts.sort === "recommended" ? decodeTopicCursor("recommended") : null; - const updatedCursor = opts.sort === "recommended" ? decodeTopicCursor("updated") : null; - const useUpdatedRecommendationFallback = - opts.sort === "recommended" && - (Boolean(updatedCursor) || - (!recommendedCursor && - (await hasMissingRecommendedScores(ctx, opts.nonSuspiciousOnly, null)))); - const sort = useUpdatedRecommendationFallback ? "updated" : opts.sort; + const sort = opts.sort; const indexName = getTopicIndexName(sort); - const decodedCursor = - opts.sort === "recommended" - ? sort === "updated" - ? updatedCursor - : recommendedCursor - : decodeTopicCursor(sort); + const decodedCursor = opts.sort === "recommended" ? recommendedCursor : decodeTopicCursor(sort); const items: PublicSkillEntry[] = []; let scanCursor = decodedCursor ?? eqPrefix; let scanInclusive = !decodedCursor; @@ -5986,6 +5975,7 @@ async function buildPublicSkillEntryFromDigest( digest: Doc<"skillSearchDigest">, ): Promise { const hydratable = digestToHydratableSkill(digest); + if (shouldExcludeSkillFromPublicBrowse(hydratable)) return null; const publicSkill = toPublicSkill(hydratable); if (!publicSkill) return null; const ownerInfo = await addOfficialStatusToOwnerInfo( @@ -6021,6 +6011,13 @@ async function loadPublicLatestVersionForDigest( if (digest.latestVersionSkillId !== undefined && digest.latestVersionSkillId !== digest.skillId) { return null; } + const skill = await ctx.db.get(digest.skillId); + if (skill && isSkillPendingPublicReview(skill) && hasPriorApprovedPublicSkillVersion(skill)) { + const version = await resolvePublicBrowseVersionForSkill(ctx, skill); + return version && isPublicSkillVersionAvailableForSkill(version, digest.skillId) + ? version + : null; + } const version = await ctx.db.get(digest.latestVersionId); return isPublicSkillVersionAvailableForSkill(version, digest.skillId) ? version : null; } @@ -6306,6 +6303,7 @@ function skillCatalogMatchesFilters( }, ) { if (!isVisibleSkillCatalogDigest(digest)) return false; + if (shouldExcludeSkillFromPublicBrowse(digestToHydratableSkill(digest))) return false; if (args.channel === "private") return false; const isOfficial = isSkillCatalogOfficial(digest); const channel = getSkillCatalogChannel(digest); @@ -6366,15 +6364,7 @@ async function listSkillPackageCatalogTopicPage( let done = decodedCursor.done; let loops = 0; let remainingScanBudget = MAX_SKILL_CATALOG_SCAN_DOCUMENTS; - const isFreshRecommendedRequest = - args.sort === "recommended" && args.paginationOpts.cursor === null; - const recommendedFallback = - args.sort === "recommended" - ? (decodedCursor.recommendedFallback ?? - (isFreshRecommendedRequest && (await hasMissingRecommendedScores(ctx, false, null)) - ? SKILL_CATALOG_RECOMMENDED_FALLBACK_SORT - : undefined)) - : undefined; + const recommendedFallback = decodedCursor.recommendedFallback; const catalogSort = recommendedFallback ?? args.sort; while ( @@ -6588,15 +6578,7 @@ export const listPackageCatalogPage = query({ let done = decodedCursor.done; let loops = 0; let remainingScanBudget = MAX_SKILL_CATALOG_SCAN_DOCUMENTS; - const isFreshRecommendedRequest = - args.sort === "recommended" && args.paginationOpts.cursor === null; - const recommendedFallback = - args.sort === "recommended" - ? (decodedCursor.recommendedFallback ?? - (isFreshRecommendedRequest && (await hasMissingRecommendedScores(ctx, false, null)) - ? SKILL_CATALOG_RECOMMENDED_FALLBACK_SORT - : undefined)) - : undefined; + const recommendedFallback = decodedCursor.recommendedFallback; const catalogSort = recommendedFallback ?? args.sort; while ( @@ -6925,7 +6907,7 @@ function resolveRecommendedPublicListQuery({ return { sort: "updated", indexName: updatedIndexName, decodedCursor: updatedCursor }; } if (hasMissingScores) { - return { sort: "updated", indexName: updatedIndexName, decodedCursor: null }; + return { sort: "recommended", indexName: rankIndexName, decodedCursor: null }; } return { sort: "recommended", indexName: scoreIndexName, decodedCursor: null }; }