Skip to content

feat(skills): add skill editors #5705

Open
icecrasher321 wants to merge 32 commits into
stagingfrom
staging-v2
Open

feat(skills): add skill editors #5705
icecrasher321 wants to merge 32 commits into
stagingfrom
staging-v2

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds way to set skill editors so only certain people can edit skills. Derived access to skills applies (so workspace, org admins are always skill editors for all skills in the workspace.

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

icecrasher321 and others added 4 commits July 16, 2026 00:50
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
Same DDL as the dropped 0261 (skill_member table, enums, indexes, skill.workspace_shared)
plus the hand-written write-user backfill, renumbered after staging's 0261.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 24, 2026 2:33am

Request Review

icecrasher321 and others added 2 commits July 16, 2026 12:12
# Conflicts:
#	apps/sim/app/api/mothership/execute/route.ts
#	apps/sim/executor/handlers/agent/skills-resolver.test.ts
#	apps/sim/lib/copilot/chat/payload.ts
#	apps/sim/lib/copilot/chat/process-contents.test.ts
#	apps/sim/lib/copilot/chat/process-contents.ts
#	apps/sim/lib/copilot/vfs/workspace-vfs.ts
#	apps/sim/lib/mothership/inbox/executor.ts
#	apps/sim/lib/mothership/skills.test.ts
#	apps/sim/lib/mothership/skills.ts
#	packages/db/migrations/meta/0262_snapshot.json
#	packages/db/migrations/meta/_journal.json
#	scripts/check-api-validation-contracts.ts
Staging claimed 0262 (strong_storm); same DDL plus the hand-written
write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
icecrasher321 and others added 5 commits July 17, 2026 22:26
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
Staging claimed 0263 (workflow_fork_sync_excluded); same DDL plus the
hand-written write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@icecrasher321
icecrasher321 marked this pull request as ready for review July 18, 2026 21:13
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Skill mutations and a new members API change authorization behavior across the product, but runtime skill use stays workspace-wide. Member-removal cleanup and editor gating need correct handling to avoid lockouts or stale access.

Overview
Introduces per-skill editors while keeping skills visible and runnable for every workspace member. New GET/POST/DELETE on /api/skills/[id]/members manages the roster (workspace admins are implicit editors and cannot be added/removed as explicit rows). Skill create/update/delete now checks editor access instead of blanket workspace write; lists return canEdit via listSkillsForUser. Departing workspace members lose skill editor rows through removeWorkspaceSkillMembershipsTx on member-removal paths.

Skills UI moves off modals onto /skills/new and /skills/[skillId] detail pages (Share, editors card, unsaved guards). The gallery redirects legacy ?skillId= links; GitHub URL import and /api/skills/import are removed in favor of file/paste import. Docs cover editors, the new create/detail flow, and Chat / skill tags.

Also refactors credential “add people” onto shared AddPeopleModal / MemberRow, adds a full-page Custom Tools editor in Settings (deep-linkable custom-tool-id), and extracts shared custom-tool editor fields. RichMarkdownField can grow with content on full pages when maxHeight is omitted.

Reviewed by Cursor Bugbot for commit b35f0cf. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds skill editor permissions and new skill management pages. The main changes are:

  • Adds explicit skill editors with derived access for workspace admins.
  • Adds APIs, database records, audit events, and member-removal cleanup.
  • Adds skill create, detail, sharing, import, and editor management views.
  • Updates skill execution, workspace copying, and management tools.
  • Refactors shared permission and custom-tool editor components.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the updated code.
  • The resolver changes preserve the documented execution behavior.

Important Files Changed

Filename Overview
apps/sim/executor/handlers/agent/skills-resolver.ts Narrows database projections while preserving the resolver response shapes.
apps/sim/executor/handlers/agent/skills-resolver.test.ts Updates fixtures and adds coverage for metadata and ID-based content resolution.
apps/sim/executor/handlers/pi/context.ts Simplifies Pi skill resolution by using the resolver's null-on-error behavior.

Reviews (7): Last reviewed commit: "fix(tools): read-only editors, design-sy..." | Re-trigger Greptile

Comment thread apps/sim/app/api/skills/[id]/members/route.ts Outdated
Comment thread apps/sim/executor/handlers/agent/skills-resolver.ts Outdated
icecrasher321 and others added 3 commits July 22, 2026 10:30
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	apps/sim/executor/handlers/pi/pi-handler.ts
#	apps/sim/lib/copilot/vfs/workspace-vfs.ts
#	scripts/check-api-validation-contracts.ts
Staging claimed 0264 (fat_ikaris); same DDL plus the hand-written
write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves the rich-markdown and custom-tool editing surfaces out of modals and
onto full-page surfaces, and collapses the duplicated chrome behind shared
components.

Skills
- Add /skills/new, a full-page create surface mirroring the skill detail page
  (CredentialDetailLayout + DetailSection + unsaved-changes guard). "Add to
  Sim" navigates there instead of opening a modal.
- Import moves to a header action (SkillImportButton) backed by a shared
  readSkillFile helper; the GitHub-URL import and its /api/skills/import route
  are removed.
- Skill name validation is now one shared validateSkillName, replacing three
  copies of the kebab-case rule and its messages.
- The skill editor roster renders through the shared MemberRow instead of
  re-deriving its identity block, with a locked role control and a lock-reason
  tooltip explaining inherited workspace-admin access.

Custom tools
- Extract the canvas modal's schema/code editors into a shared
  custom-tool-editor module (fields, wand generation, schema helpers), cutting
  custom-tool-modal.tsx by ~900 lines.
- Settings > Custom tools gains a full-page detail sub-view (SettingsPanel +
  SettingsSection + saveDiscardActions), deep-linkable via ?custom-tool-id.
  Rows are clickable; delete now lives only in the detail view.
- Replace legacy Button/Input/Badge/Label with the chip family, move chip-field
  chrome into CodeEditor behind an error prop, and delete its dead wand button.

Rich markdown field
- maxHeight is now opt-in: omit it on a page and the editor grows with its
  content so the page owns the only scrollbar. Modals pass explicit caps.
- The field variant drops to font-weight 400 to match adjacent chip fields.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

Comment thread apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx
Comment thread apps/sim/app/api/skills/[id]/members/route.ts
… editor audit

- Skill create navigated using the first element of the upsert response, but
  that endpoint returns the caller's whole skill list (built-ins prepended) —
  match the new skill by its workspace-unique name instead.
- The suggested-skill 409 toast claimed the skill existed but was not shared
  and told the user to ask a skill admin. Every workspace member can already
  see and use every skill, so a 409 only means the name is taken.
- Adding an editor emitted the skill_shared event and SKILL_MEMBER_ADDED audit
  even when onConflictDoNothing skipped the insert on a concurrent add. Gate
  both on the insert actually returning a row.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

Comment thread apps/sim/app/api/skills/route.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 11120d1. Configure here.

…ng, error routing

Two real regressions introduced while simplifying the extracted editor:

- The schema-param autocomplete's trigger was rewritten to match a trailing
  identifier, but the completion still split on separators. The two disagreed,
  so typing `data.ci` opened the menu and selecting replaced `data.ci` whole —
  eating the member-access prefix. Both now share one SCHEMA_PARAM_WORD regex.
- The uncapped markdown field measured its height only on value change while
  always setting overflow-hidden, so any width change that re-wrapped lines
  clipped the tail with no scrollbar to reach it. Now re-measures via
  ResizeObserver.

Also from the audit:
- Generation writes bypass the code field's change handler, so an open
  autocomplete stayed over a disabled streaming editor; close it on busy.
- Delete failures rendered in the Schema section's error slot on both custom
  tool surfaces; route them to a toast instead.
- Skill create navigated away while still dirty, stranding the unsaved-changes
  guard's history sentinel so Back landed on an empty create form.
- The Description field on skill create never received its error border.
- Drop a double-applied opacity-50 (the editor already dims when disabled), a
  dead try/catch around a non-throwing call that also shadowed the error prop,
  and a stale reference to a /tools page that does not exist.
- Docs still described the removed GitHub-URL import and the old Add Skill
  dialog; rewrite for the create page and file/paste import.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 01c614d. Configure here.

…p dead wand prompt API

- Viewers without edit rights could not open a custom tool at all, while the
  equivalent skill and custom-block surfaces both offer a read-only view. The
  detail page now takes `readOnly`: editors inert, no Save/Discard/Delete, no
  Generate. Creating still requires edit rights.
- Creating a tool bounced back to the list while creating a skill lands on the
  new skill. Tools now do the same. The upsert returns the workspace's whole
  tool list (newest first) rather than just the new row, so the id is matched
  by title instead of by index — the same trap that produced the skill-create
  navigation bug.
- Remove `openPrompt`/`closePrompt` from useWand. `closePrompt`'s last callers
  went away with the custom-tool-modal extraction and `openPrompt` had none
  before it; nothing reads `isPromptVisible` any more either.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

…tool identity

- readOnly never reached the editors: the prop gated actions and Generate but
  the schema and code fields were still typable for viewers without edit rights.
  Wire disabled through both fields into CodeEditor.
- The row icon used lucide's Wrench (strokeWidth 2) where @sim/emcn/icons ships
  one drawn for this system (1.55, tuned viewBox), and it inherited body text
  colour instead of --text-icon. Swap it.
- Give the tool detail page the same identity heading as skill detail: tile,
  name, and description at the top left, instead of only a header title.
- Extract ResourceTile so the skills and tools tiles share one definition
  (SkillTile now composes it), and add an opt-in `iconFilled` to
  SettingsResourceRow so the tools list tile matches the skills gallery. Both
  default to today's behaviour for every existing consumer.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b35f0cf. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants