Skip to content

feat: allow owners to edit skill summary #1411

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

Open
SylvanXiao wants to merge 1 commit into
base: main
Choose a base branch
from
+181 −2
Open

feat: allow owners to edit skill summary #1411

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
25 changes: 24 additions & 1 deletion convex/skills.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4780,7 +4780,7 @@ export const deleteTags = mutation({
const nextTags = { ...skill.tags };
let changed = false;
for (const tag of args.tags) {
if (tag === "latest") continue; // protect the latest tag from deletion
if (tag === "latest") continue;
if (tag in nextTags) {
delete nextTags[tag];
changed = true;
Expand All @@ -4796,6 +4796,29 @@ export const deleteTags = mutation({
},
});

export const updateSummary = mutation({
args: {
skillId: v.id("skills"),
summary: v.string(),
},
Comment on lines +4800 to +4803
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No maximum length validation on summary

v.string() imposes no upper bound, so a client can submit an arbitrarily long string. Consider capping the summary (e.g. 500 characters) both in the validator and with a maxLength attribute on the textarea:

In the handler, a quick guard:

if (args.summary.length > 500) throw new Error("Summary must be 500 characters or fewer.");

And in the textarea:

<textarea maxLength={500} ... />
Prompt To Fix With AI 
This is a comment left during a code review.
Path: convex/skills.ts
Line: 4800-4803

Comment:
**No maximum length validation on summary**

`v.string()` imposes no upper bound, so a client can submit an arbitrarily long string. Consider capping the summary (e.g. 500 characters) both in the validator and with a `maxLength` attribute on the textarea:

In the handler, a quick guard:
```ts
if (args.summary.length > 500) throw new Error("Summary must be 500 characters or fewer.");
```

And in the textarea:
```tsx
<textarea maxLength={500} ... />
```

How can I resolve this? If you propose a fix, please make it concise.

handler: async (ctx, args) => {
const { user } = await requireUser(ctx);
const skill = await ctx.db.get(args.skillId);
if (!skill) throw new Error("Skill not found");
if (skill.ownerUserId !== user._id) {
assertModerator(user);
}
Comment on lines +4808 to +4810
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allow publisher owners to update summaries

This mutation only authorizes skill.ownerUserId (or moderators), but org-owned skills are managed by publisher members in the UI (canManage becomes true from ownerPublisherId membership), so those legitimate owners can open the editor and then always hit an authorization failure on save. As written, the new summary-edit feature is broken for publisher-owned skills unless a moderator performs the edit.

Useful? React with 👍 / 👎.


const now = Date.now();
const patch: Partial<Doc<"skills">> = {
summary: args.summary,
updatedAt: now,
};

await ctx.db.patch(skill._id, patch);
},
});
Comment on lines +4799 to +4820
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Publisher members can see "Edit summary" but are blocked by the backend

The edit button is shown to anyone where canManage is true, which includes members of the publisher that owns the skill (via myPublisherIds.has(skill.ownerPublisherId)). However, updateSummary only permits skill.ownerUserId === user._id or a moderator — publisher members are rejected.

When a publisher member (who is not the ownerUserId and not a moderator) hits "Save", they will receive an error and see window.alert("Failed to update summary. Please try again.").

Either extend the backend to accept publisher members, or restrict the edit button to isOwner only:

Option A — extend backend to also allow publisher members:

if (skill.ownerUserId !== user._id) {
  let isPublisherMember = false;
  if (skill.ownerPublisherId) {
    const membership = await ctx.db
      .query("publisherMembers")
      .withIndex("by_publisher_user", (q) =>
        q.eq("publisherId", skill.ownerPublisherId!).eq("userId", user._id),
      )
      .unique();
    isPublisherMember = membership !== null;
  }
  if (!isPublisherMember) {
    assertModerator(user);
  }
}

Option B — gate the button behind isOwner instead of canManage (simpler, matches the current backend check).

Prompt To Fix With AI 
This is a comment left during a code review.
Path: convex/skills.ts
Line: 4799-4820

Comment:
**Publisher members can see "Edit summary" but are blocked by the backend**

The edit button is shown to anyone where `canManage` is true, which includes members of the publisher that owns the skill (via `myPublisherIds.has(skill.ownerPublisherId)`). However, `updateSummary` only permits `skill.ownerUserId === user._id` or a moderator — publisher members are rejected.

When a publisher member (who is not the `ownerUserId` and not a moderator) hits "Save", they will receive an error and see `window.alert("Failed to update summary. Please try again.")`.

Either extend the backend to accept publisher members, or restrict the edit button to `isOwner` only:

**Option A — extend backend to also allow publisher members:**
```ts
if (skill.ownerUserId !== user._id) {
  let isPublisherMember = false;
  if (skill.ownerPublisherId) {
    const membership = await ctx.db
      .query("publisherMembers")
      .withIndex("by_publisher_user", (q) =>
        q.eq("publisherId", skill.ownerPublisherId!).eq("userId", user._id),
      )
      .unique();
    isPublisherMember = membership !== null;
  }
  if (!isPublisherMember) {
    assertModerator(user);
  }
}
```

**Option B — gate the button behind `isOwner` instead of `canManage`** (simpler, matches the current backend check).

How can I resolve this? If you propose a fix, please make it concise.


export const setRedactionApproved = mutation({
args: { skillId: v.id("skills"), approved: v.boolean() },
handler: async (ctx, args) => {
Expand Down
38 changes: 38 additions & 0 deletions src/components/SkillDetailPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export function SkillDetailPage({
const reportSkill = useMutation(api.skills.report);
const updateTags = useMutation(api.skills.updateTags);
const deleteTags = useMutation(api.skills.deleteTags);
const updateSummary = useMutation(api.skills.updateSummary);
const getReadme = useAction(api.skills.getReadme);
const myPublishers = useQuery(api.publishers.listMine) as
| Array<{ publisher: { _id: Id<"publishers"> }; role: string }>
Expand All @@ -99,6 +100,9 @@ export function SkillDetailPage({
const [reportReason, setReportReason] = useState("");
const [reportError, setReportError] = useState<string | null>(null);
const [isSubmittingReport, setIsSubmittingReport] = useState(false);
const [summary, setSummary] = useState("");
const [isSummaryEditing, setIsSummaryEditing] = useState(false);
const [isSummarySubmitting, setIsSummarySubmitting] = useState(false);

const isLoadingSkill = isStaff ? staffResult === undefined : result === undefined;
const skill = result?.skill;
Expand Down Expand Up @@ -267,6 +271,12 @@ export function SkillDetailPage({
}
}, [latestVersion, tagVersionId]);

useEffect(() => {
if (skill && !isSummaryEditing) {
setSummary(skill.summary ?? "");
}
}, [skill, isSummaryEditing]);

const closeReportDialog = () => {
setIsReportDialogOpen(false);
setReportReason("");
Expand Down Expand Up @@ -299,6 +309,28 @@ export function SkillDetailPage({
});
};

const submitSummary = async () => {
if (!skill) return;
setIsSummarySubmitting(true);
try {
await updateSummary({
skillId: skill._id,
summary: summary,
});
setIsSummaryEditing(false);
} catch (error) {
console.error("Failed to update summary", error);
window.alert("Failed to update summary. Please try again.");
} finally {
setIsSummarySubmitting(false);
}
};

const startSummaryEdit = () => {
setSummary(skill?.summary ?? "");
setIsSummaryEditing(true);
};

const submitReport = async () => {
if (!skill) return;

Expand Down Expand Up @@ -386,6 +418,12 @@ export function SkillDetailPage({
tagVersions={versions ?? []}
clawdis={clawdis}
osLabels={osLabels}
summary={summary}
onSummaryChange={setSummary}
onSummarySubmit={submitSummary}
isSummaryEditing={isSummaryEditing}
onSummaryEditToggle={isSummaryEditing ? () => setIsSummaryEditing(false) : startSummaryEdit}
isSummarySubmitting={isSummarySubmitting}
/>

{isOwner && skill ? (
Expand Down
63 changes: 62 additions & 1 deletion src/components/SkillHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ type SkillHeaderProps = {
tagVersions: Doc<"skillVersions">[];
clawdis: ClawdisSkillMetadata | undefined;
osLabels: string[];
summary: string;
onSummaryChange: (value: string) => void;
onSummarySubmit: () => void;
isSummaryEditing: boolean;
onSummaryEditToggle: () => void;
isSummarySubmitting: boolean;
};

export function SkillHeader({
Expand Down Expand Up @@ -115,6 +121,12 @@ export function SkillHeader({
tagVersions,
clawdis,
osLabels,
summary,
onSummaryChange,
onSummarySubmit,
isSummaryEditing,
onSummaryEditToggle,
isSummarySubmitting,
}: SkillHeaderProps) {
const convexSiteUrl = getRuntimeEnv("VITE_CONVEX_SITE_URL") ?? "https://clawhub.ai";
const formattedStats = formatSkillStatsTriplet(skill.stats);
Expand Down Expand Up @@ -199,7 +211,56 @@ export function SkillHeader({
</h1>
{nixPlugin ? <span className="tag tag-accent">Plugin bundle (nix)</span> : null}
</div>
<p className="section-subtitle">{skill.summary ?? "No summary provided."}</p>
<p className="section-subtitle">
{isSummaryEditing ? (
<form
onSubmit={(event) => {
Comment on lines +214 to +217
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Move summary edit form out of paragraph container

When edit mode is active, this renders a <form> as a child of <p className="section-subtitle">, which is invalid HTML content nesting. Browsers will auto-correct that structure, which can cause SSR hydration warnings and inconsistent header layout/behavior when toggling summary edit mode. Wrapping the editable block in a non-<p> container avoids this.

Useful? React with 👍 / 👎.

event.preventDefault();
onSummarySubmit();
}}
className="summary-edit-form"
>
<textarea
className="search-input summary-textarea"
value={summary}
onChange={(event) => onSummaryChange(event.target.value)}
placeholder="Enter a brief summary..."
rows={2}
/>
<div className="summary-edit-actions">
<button
className="btn"
type="submit"
disabled={isSummarySubmitting}
>
{isSummarySubmitting ? "Saving..." : "Save"}
</button>
<button
className="btn btn-ghost"
type="button"
onClick={onSummaryEditToggle}
disabled={isSummarySubmitting}
>
Cancel
</button>
</div>
</form>
) : (
<>
{skill.summary ?? "No summary provided."}
{canManage && (
<button
className="edit-summary-btn"
type="button"
onClick={onSummaryEditToggle}
title="Edit summary"
>
✎
</button>
)}
</>
)}
</p>
Comment on lines +214 to +263
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 <form> nested inside <p> is invalid HTML

A <form> (flow content) cannot be a child of <p> (which only permits phrasing content). When browsers parse this, they implicitly close the <p> before the <form>, producing a broken DOM that causes layout issues — e.g. the form floats outside the intended container and .section-subtitle styles don't apply to the edit view.

Replace the outer <p> with a <div> (keeping the section-subtitle class) so both the static text and the form share the same valid parent:

Suggested change
<p className="section-subtitle">
{isSummaryEditing ? (
<form
onSubmit={(event) => {
event.preventDefault();
onSummarySubmit();
}}
className="summary-edit-form"
>
<textarea
className="search-input summary-textarea"
value={summary}
onChange={(event) => onSummaryChange(event.target.value)}
placeholder="Enter a brief summary..."
rows={2}
/>
<div className="summary-edit-actions">
<button
className="btn"
type="submit"
disabled={isSummarySubmitting}
>
{isSummarySubmitting ? "Saving..." : "Save"}
</button>
<button
className="btn btn-ghost"
type="button"
onClick={onSummaryEditToggle}
disabled={isSummarySubmitting}
>
Cancel
</button>
</div>
</form>
) : (
<>
{skill.summary ?? "No summary provided."}
{canManage && (
<button
className="edit-summary-btn"
type="button"
onClick={onSummaryEditToggle}
title="Edit summary"
>
✎
</button>
)}
</>
)}
</p>
<div className="section-subtitle">
{isSummaryEditing ? (
<form
onSubmit={(event) => {
event.preventDefault();
onSummarySubmit();
}}
className="summary-edit-form"
>
<textarea
className="search-input summary-textarea"
value={summary}
onChange={(event) => onSummaryChange(event.target.value)}
placeholder="Enter a brief summary..."
rows={2}
/>
<div className="summary-edit-actions">
<button
className="btn"
type="submit"
disabled={isSummarySubmitting}
>
{isSummarySubmitting ? "Saving..." : "Save"}
</button>
<button
className="btn btn-ghost"
type="button"
onClick={onSummaryEditToggle}
disabled={isSummarySubmitting}
>
Cancel
</button>
</div>
</form>
) : (
<>
{skill.summary ?? "No summary provided."}
{canManage && (
<button
className="edit-summary-btn"
type="button"
onClick={onSummaryEditToggle}
title="Edit summary"
>
✎
</button>
)}
</>
)}
</div>
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/components/SkillHeader.tsx
Line: 214-263

Comment:
**`<form>` nested inside `<p>` is invalid HTML**

A `<form>` (flow content) cannot be a child of `<p>` (which only permits phrasing content). When browsers parse this, they implicitly close the `<p>` before the `<form>`, producing a broken DOM that causes layout issues — e.g. the form floats outside the intended container and `.section-subtitle` styles don't apply to the edit view.

Replace the outer `<p>` with a `<div>` (keeping the `section-subtitle` class) so both the static text and the form share the same valid parent:

```suggestion
              <div className="section-subtitle">
                {isSummaryEditing ? (
                  <form
                    onSubmit={(event) => {
                      event.preventDefault();
                      onSummarySubmit();
                    }}
                    className="summary-edit-form"
                  >
                    <textarea
                      className="search-input summary-textarea"
                      value={summary}
                      onChange={(event) => onSummaryChange(event.target.value)}
                      placeholder="Enter a brief summary..."
                      rows={2}
                    />
                    <div className="summary-edit-actions">
                      <button
                        className="btn"
                        type="submit"
                        disabled={isSummarySubmitting}
                      >
                        {isSummarySubmitting ? "Saving..." : "Save"}
                      </button>
                      <button
                        className="btn btn-ghost"
                        type="button"
                        onClick={onSummaryEditToggle}
                        disabled={isSummarySubmitting}
                      >
                        Cancel
                      </button>
                    </div>
                  </form>
                ) : (
                  <>
                    {skill.summary ?? "No summary provided."}
                    {canManage && (
                      <button
                        className="edit-summary-btn"
                        type="button"
                        onClick={onSummaryEditToggle}
                        title="Edit summary"
                      >
                        ✎
                      </button>
                    )}
                  </>
                )}
              </div>
```

How can I resolve this? If you propose a fix, please make it concise.


{isStaff && staffModerationNote ? (
<div className="skill-hero-note">{staffModerationNote}</div>
Expand Down
57 changes: 57 additions & 0 deletions src/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,63 @@ code {
margin-bottom: 24px;
}

.edit-summary-btn {
background: none;
border: none;
cursor: pointer;
font-size: 1rem;
margin-left: 8px;
opacity: 0.6;
transition: opacity 0.2s;
}

.edit-summary-btn:hover {
opacity: 1;
}

.summary-edit-form {
display: flex;
flex-direction: column;
gap: 8px;
margin-bottom: 16px;
}

.summary-edit-actions {
display: flex;
gap: 8px;
}

form.summary-edit-form .summary-textarea {
resize: vertical;
min-height: 60px;
font-family: inherit;
font-size: inherit;
flex: none !important;
width: 100%;
box-sizing: border-box;
border: 1px solid rgba(29, 59, 78, 0.22);
border-radius: var(--radius-sm);
padding: 10px 12px;
color: var(--ink);
background: rgba(255, 255, 255, 0.94);
}

form.summary-edit-form .summary-textarea:focus {
outline: none;
border-color: color-mix(in srgb, var(--accent) 70%, white);
box-shadow: 0 0 0 3px color-mix(in srgb, var(--accent) 22%, transparent);
}

[data-theme="dark"] form.summary-edit-form .summary-textarea {
border-color: rgba(255, 255, 255, 0.12);
background: rgba(14, 28, 37, 0.84);
}

[data-theme="dark"] form.summary-edit-form .summary-textarea:focus {
border-color: rgba(255, 131, 95, 0.75);
box-shadow: 0 0 0 3px rgba(255, 131, 95, 0.2);
}

.grid {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(260px, 1fr));
Expand Down