fix: show pending skill page to owners instead of "Skill not found"#136
Merged
fix: show pending skill page to owners instead of "Skill not found"#136
Conversation
When a skill owner uploads a skill that's pending VirusTotal scan, they now see their skill page with a pending banner instead of "Skill not found". The banner explains the scan is in progress. Changes: - Modified getBySlug query to return skill data for owners even when moderationStatus is 'hidden' with reason 'pending.scan' - Added pendingReview flag to query response - Added pending banner component to SkillDetailPage - Added CSS for pending banner using existing ClawHub gold theme
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
516
to
528
| const skillData = publicSkill ?? { | ||
| _id: skill._id, | ||
| _creationTime: skill._creationTime, | ||
| slug: skill.slug, | ||
| displayName: skill.displayName, | ||
| summary: skill.summary, | ||
| ownerUserId: skill.ownerUserId, | ||
| canonicalSkillId: skill.canonicalSkillId, | ||
| forkOf: skill.forkOf, | ||
| latestVersionId: skill.latestVersionId, | ||
| tags: skill.tags, | ||
| badges: skill.badges, | ||
| stats: skill.stats, |
There was a problem hiding this comment.
Incorrect badges source
In the owner/pending fallback (skillData = publicSkill ?? { ... }), badges is set to skill.badges (convex/skills.ts:527), but this query already computed the up-to-date badge map via getSkillBadgeMap (convex/skills.ts:500) and uses it for toPublicSkill({ ...skill, badges }) (convex/skills.ts:510). When publicSkill is null (pending hidden skill), the owner will get the legacy/stale skill.badges instead of the computed badge map, which can make the UI render wrong badges for pending skills. Use the computed badges variable in the fallback object.
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/skills.ts
Line: 516:528
Comment:
**Incorrect badges source**
In the owner/pending fallback (`skillData = publicSkill ?? { ... }`), `badges` is set to `skill.badges` (`convex/skills.ts:527`), but this query already computed the up-to-date badge map via `getSkillBadgeMap` (`convex/skills.ts:500`) and uses it for `toPublicSkill({ ...skill, badges })` (`convex/skills.ts:510`). When `publicSkill` is null (pending hidden skill), the owner will get the legacy/stale `skill.badges` instead of the computed badge map, which can make the UI render wrong badges for pending skills. Use the computed `badges` variable in the fallback object.
How can I resolve this? If you propose a fix, please make it concise.Extended the list query to include pending skills when the requester is viewing their own dashboard. Added "Scanning" badge with gold theme to indicate skills pending VirusTotal review.
- Owners see their blocked/removed skills with explanatory banners - Red banner for malware-blocked and removed skills - Gold banner for pending scan - Download button hidden for blocked/removed skills - Added security disclaimer: "Like a lobster shell, security has layers" - Fixed badges bug (use computed badges, not stale skill.badges)
Blocked skills are now visible to everyone via direct URL: - Shows red banner with "security issue detected" - Displays VT scan results - No download button - Still hidden from listings/search Sends a strong transparency signal about security enforcement.
Contributor
Author
|
Here are the states that will be visible.
|
|
Thank you! |
Collaborator
|
Landed via temp rebase onto main. Thanks @orlyjamie! |
|
Seems like this still doesn't fix the skill disappearing issue. Or is the CI/CD taking longer than 8 mins? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a skill owner uploads a skill that's pending VirusTotal scan, they now see their skill page with a pending banner instead of "Skill not found". The banner explains the scan is in progress.
Changes:
Test plan
Greptile Overview
Greptile Summary
This PR updates the
getBySlugConvex query to allow skill owners to view their own skills while they’re hidden forpending.scan, returning an extrapendingReviewflag. The frontendSkillDetailPageconsumes that flag to display a “Security scan in progress” banner, and new CSS styles the banner using the existing gold theme tokens.Key interaction change:
getBySlugnow conditionally returns skill data even whentoPublicSkillwould normally hide it, but only for the owner, so non-owners still get the existing “not found” behavior.Confidence Score: 4/5
skill.badgesinstead of the computed badge map already fetched in the query, which can lead to incorrect badges being shown for pending skills.Context used:
dashboard- AGENTS.md (source)