-
Notifications
You must be signed in to change notification settings - Fork 315
Jminnetian/askskills #1410
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.
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
base: main
Are you sure you want to change the base?
Jminnetian/askskills #1410
Changes from all commits
23f9b0e
05ae0bd
fdc083b
d92be1b
bf25c64
99cd4ea
344255a
a2031b7
c91ba89
b1263e9
aa0676a
1945a3c
0a76bbe
fe48eba
40cf743
3d5a1e3
2533ae9
7907830
c79ee2b
5579e08
8fdfc92
8550993
0da351b
6bd7aea
20f5b03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -576,6 +576,9 @@ | |
| }, | ||
| "externalWebUrl": { | ||
| "type": "string" | ||
| }, | ||
| "blobSha": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we compress into a single migration? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| -- CreateEnum | ||
| CREATE TYPE "AgentSkillVisibility" AS ENUM ('PERSONAL', 'SHARED'); | ||
|
|
||
| -- CreateTable | ||
| CREATE TABLE "AgentSkill" ( | ||
| "id" TEXT NOT NULL, | ||
| "visibility" "AgentSkillVisibility" NOT NULL, | ||
| "scopeId" TEXT NOT NULL, | ||
| "slug" TEXT NOT NULL, | ||
| "name" TEXT NOT NULL, | ||
| "description" TEXT NOT NULL, | ||
| "instructions" TEXT NOT NULL, | ||
| "createdById" TEXT NOT NULL, | ||
| "updatedById" TEXT, | ||
| "orgId" INTEGER NOT NULL, | ||
| "enabled" BOOLEAN NOT NULL DEFAULT true, | ||
| "featured" BOOLEAN NOT NULL DEFAULT false, | ||
| "autoEnrolled" BOOLEAN NOT NULL DEFAULT false, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "updatedAt" TIMESTAMP(3) NOT NULL, | ||
|
|
||
| CONSTRAINT "AgentSkill_pkey" PRIMARY KEY ("id") | ||
| ); | ||
|
|
||
| -- CreateTable | ||
| CREATE TABLE "AgentSkillAdoption" ( | ||
| "id" TEXT NOT NULL, | ||
| "userId" TEXT NOT NULL, | ||
| "orgId" INTEGER NOT NULL, | ||
| "agentSkillId" TEXT NOT NULL, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "removedAt" TIMESTAMP(3), | ||
|
|
||
| CONSTRAINT "AgentSkillAdoption_pkey" PRIMARY KEY ("id") | ||
| ); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_createdById_idx" ON "AgentSkill"("createdById"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_updatedById_idx" ON "AgentSkill"("updatedById"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_orgId_idx" ON "AgentSkill"("orgId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_orgId_visibility_scopeId_idx" ON "AgentSkill"("orgId", "visibility", "scopeId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_shared_catalog_idx" ON "AgentSkill"("orgId", "visibility", "scopeId", "enabled", "featured" DESC, "updatedAt" DESC, "name"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "AgentSkill_orgId_visibility_scopeId_slug_key" ON "AgentSkill"("orgId", "visibility", "scopeId", "slug"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkillAdoption_userId_orgId_removedAt_idx" ON "AgentSkillAdoption"("userId", "orgId", "removedAt"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkillAdoption_agentSkillId_idx" ON "AgentSkillAdoption"("agentSkillId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkillAdoption_orgId_idx" ON "AgentSkillAdoption"("orgId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "AgentSkillAdoption_orgId_userId_agentSkillId_key" ON "AgentSkillAdoption"("orgId", "userId", "agentSkillId"); | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkill" ADD CONSTRAINT "AgentSkill_createdById_fkey" FOREIGN KEY ("createdById") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkill" ADD CONSTRAINT "AgentSkill_updatedById_fkey" FOREIGN KEY ("updatedById") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkill" ADD CONSTRAINT "AgentSkill_orgId_fkey" FOREIGN KEY ("orgId") REFERENCES "Org"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkillAdoption" ADD CONSTRAINT "AgentSkillAdoption_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkillAdoption" ADD CONSTRAINT "AgentSkillAdoption_orgId_fkey" FOREIGN KEY ("orgId") REFERENCES "Org"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AgentSkillAdoption" ADD CONSTRAINT "AgentSkillAdoption_agentSkillId_fkey" FOREIGN KEY ("agentSkillId") REFERENCES "AgentSkill"("id") ON DELETE CASCADE ON UPDATE CASCADE; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| -- DropIndex | ||
| DROP INDEX "AgentSkill_shared_catalog_idx"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "AgentSkill" DROP COLUMN "featured"; | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AgentSkill_shared_catalog_idx" ON "AgentSkill"("orgId", "visibility", "scopeId", "enabled", "updatedAt" DESC, "name"); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| -- AlterTable | ||
| ALTER TABLE "AgentSkill" ADD COLUMN "sourceBlobSha" TEXT, | ||
| ADD COLUMN "sourceFilePath" TEXT, | ||
| ADD COLUMN "sourceImportedAt" TIMESTAMP(3), | ||
| ADD COLUMN "sourceRepoName" TEXT, | ||
| ADD COLUMN "sourceRevision" TEXT; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -334,6 +334,8 @@ model Org { | |||||||||||||||||||||||||||||
| repoVisits RepoVisit[] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| mcpServers McpServer[] | ||||||||||||||||||||||||||||||
| agentSkills AgentSkill[] | ||||||||||||||||||||||||||||||
| agentSkillAdoptions AgentSkillAdoption[] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| license License? | ||||||||||||||||||||||||||||||
| servicePingEvents ServicePingEvent[] | ||||||||||||||||||||||||||||||
|
|
@@ -519,6 +521,9 @@ model User { | |||||||||||||||||||||||||||||
| sessionVersion Int @default(0) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| userMcpServers UserMcpServer[] | ||||||||||||||||||||||||||||||
| createdAgentSkills AgentSkill[] @relation("AgentSkillCreatedBy") | ||||||||||||||||||||||||||||||
| updatedAgentSkills AgentSkill[] @relation("AgentSkillUpdatedBy") | ||||||||||||||||||||||||||||||
| agentSkillAdoptions AgentSkillAdoption[] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| createdAt DateTime @default(now()) | ||||||||||||||||||||||||||||||
| updatedAt DateTime @updatedAt | ||||||||||||||||||||||||||||||
|
|
@@ -741,6 +746,93 @@ model ChatAccess { | |||||||||||||||||||||||||||||
| @@unique([chatId, userId]) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| enum AgentSkillVisibility { | ||||||||||||||||||||||||||||||
| PERSONAL | ||||||||||||||||||||||||||||||
| SHARED | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| model AgentSkill { | ||||||||||||||||||||||||||||||
| id String @id @default(cuid()) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Namespace identity for slash-command uniqueness and lookup. | ||||||||||||||||||||||||||||||
| // This is not a foreign key and should not be used as an authorization check by itself. | ||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||
| // Every skill belongs to an org (orgId is always set). | ||||||||||||||||||||||||||||||
| // PERSONAL: scopeId is the user id; the skill is private to that user within the org. | ||||||||||||||||||||||||||||||
| // SHARED: scopeId is String(orgId); the skill is visible to the whole org (opt-in via adoptions). | ||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||
| // Future sharing/grants should treat this as the skill's home namespace, not as the ACL. | ||||||||||||||||||||||||||||||
| visibility AgentSkillVisibility | ||||||||||||||||||||||||||||||
| scopeId String | ||||||||||||||||||||||||||||||
| slug String | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| name String | ||||||||||||||||||||||||||||||
| description String | ||||||||||||||||||||||||||||||
| instructions String | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // The user who authored the skill. This is audit metadata, separate from | ||||||||||||||||||||||||||||||
| // the namespace that owns the slash-command slug. | ||||||||||||||||||||||||||||||
| createdBy User @relation("AgentSkillCreatedBy", fields: [createdById], references: [id], onDelete: Cascade) | ||||||||||||||||||||||||||||||
| createdById String | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| updatedBy User? @relation("AgentSkillUpdatedBy", fields: [updatedById], references: [id], onDelete: SetNull) | ||||||||||||||||||||||||||||||
| updatedById String? | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+773
to
+780
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
This contradicts the stated intent and risks unexpected data loss on a routine admin action (user removal). 🛠️ Suggested fix- createdBy User `@relation`("AgentSkillCreatedBy", fields: [createdById], references: [id], onDelete: Cascade)
- createdById String
+ createdBy User? `@relation`("AgentSkillCreatedBy", fields: [createdById], references: [id], onDelete: SetNull)
+ createdById String?Note: this requires updating all 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As of current this was actually intentional, but it's unclear if it should be this way or if a shared skill truly becomes an org resource (but then it's weird that the creator is the sole person who can then remove everybodies access to it).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Given this is a genuine product/design decision rather than a clear-cut bug, I'll leave it to you and the team to decide the intended semantics. If you'd like, I can open a follow-up issue to track "clarify/rework AgentSkill ownership semantics for org offboarding" so it doesn't get lost — just let me know. ✏️ Learnings added
🧠 Learnings used |
||||||||||||||||||||||||||||||
| // The owning org. Both PERSONAL and SHARED skills are scoped to an org, so a | ||||||||||||||||||||||||||||||
| // user's personal skills change when they switch orgs. | ||||||||||||||||||||||||||||||
| org Org @relation(fields: [orgId], references: [id], onDelete: Cascade) | ||||||||||||||||||||||||||||||
| orgId Int | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| enabled Boolean @default(true) | ||||||||||||||||||||||||||||||
| autoEnrolled Boolean @default(false) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Provenance for skills imported from an indexed repository file. All null for | ||||||||||||||||||||||||||||||
| // skills created manually or imported from a local file. When set, the skill's | ||||||||||||||||||||||||||||||
| // content is a read-only mirror of the source file: edits happen in the repo and | ||||||||||||||||||||||||||||||
| // are pulled in via "Update from source". sourceBlobSha (the git blob OID at | ||||||||||||||||||||||||||||||
| // import) is the comparison key used to detect when the indexed file has changed. | ||||||||||||||||||||||||||||||
| // Sync is personal-only; publishing to shared snapshots the content and drops | ||||||||||||||||||||||||||||||
| // these fields. | ||||||||||||||||||||||||||||||
| sourceRepoName String? | ||||||||||||||||||||||||||||||
| sourceFilePath String? | ||||||||||||||||||||||||||||||
| sourceRevision String? | ||||||||||||||||||||||||||||||
| sourceBlobSha String? | ||||||||||||||||||||||||||||||
| sourceImportedAt DateTime? | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| createdAt DateTime @default(now()) | ||||||||||||||||||||||||||||||
| updatedAt DateTime @updatedAt | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| adoptions AgentSkillAdoption[] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @@unique([orgId, visibility, scopeId, slug]) | ||||||||||||||||||||||||||||||
| @@index([createdById]) | ||||||||||||||||||||||||||||||
| @@index([updatedById]) | ||||||||||||||||||||||||||||||
| @@index([orgId]) | ||||||||||||||||||||||||||||||
| @@index([orgId, visibility, scopeId]) | ||||||||||||||||||||||||||||||
| @@index([orgId, visibility, scopeId, enabled, updatedAt(sort: Desc), name], map: "AgentSkill_shared_catalog_idx") | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| model AgentSkillAdoption { | ||||||||||||||||||||||||||||||
| id String @id @default(cuid()) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | ||||||||||||||||||||||||||||||
| userId String | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| org Org @relation(fields: [orgId], references: [id], onDelete: Cascade) | ||||||||||||||||||||||||||||||
| orgId Int | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| agentSkill AgentSkill @relation(fields: [agentSkillId], references: [id], onDelete: Cascade) | ||||||||||||||||||||||||||||||
| agentSkillId String | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| createdAt DateTime @default(now()) | ||||||||||||||||||||||||||||||
| removedAt DateTime? | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @@unique([orgId, userId, agentSkillId]) | ||||||||||||||||||||||||||||||
| @@index([userId, orgId, removedAt]) | ||||||||||||||||||||||||||||||
| @@index([agentSkillId]) | ||||||||||||||||||||||||||||||
| @@index([orgId]) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // OAuth2 Authorization Server models | ||||||||||||||||||||||||||||||
| // @see: https://datatracker.ietf.org/doc/html/rfc6749 | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,62 @@ | ||
| import type { User, Account } from ".prisma/client"; | ||
| import type { Account, Prisma, User } from ".prisma/client"; | ||
| export type UserWithAccounts = User & { accounts: Account[] }; | ||
| export * from ".prisma/client"; | ||
| export * from ".prisma/client"; | ||
|
|
||
| // Personal skills are scoped to the (user, org) pair: a user's personal skills | ||
| // change when they switch orgs. scopeId is the userId; orgId binds it to the org. | ||
| export const personalAgentSkillScope = (userId: string, orgId: number) => ({ | ||
| visibility: "PERSONAL" as const, | ||
| scopeId: userId, | ||
| orgId, | ||
| }); | ||
|
|
||
| // Shared skills are visible to the whole org. scopeId is String(orgId) so every | ||
| // shared skill in an org occupies one slug-uniqueness namespace. | ||
| export const sharedAgentSkillScope = (orgId: number) => ({ | ||
| visibility: "SHARED" as const, | ||
| scopeId: String(orgId), | ||
| orgId, | ||
| }); | ||
|
|
||
| export const personalAgentSkillAuthScope = (userId: string, orgId: number) => ({ | ||
| ...personalAgentSkillScope(userId, orgId), | ||
| createdById: userId, | ||
| }); | ||
|
|
||
| export const sharedAgentSkillAuthScope = (orgId: number) => ({ | ||
| ...sharedAgentSkillScope(orgId), | ||
| }); | ||
|
|
||
| export const sharedAgentSkillVisibleToUserWhere = (userId: string, orgId: number) => ({ | ||
| ...sharedAgentSkillAuthScope(orgId), | ||
| enabled: true, | ||
| AND: [ | ||
| { | ||
| OR: [ | ||
| { autoEnrolled: true }, | ||
| { | ||
| adoptions: { | ||
| some: { | ||
| userId, | ||
| orgId, | ||
| removedAt: null, | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| NOT: { | ||
| adoptions: { | ||
| some: { | ||
| userId, | ||
| orgId, | ||
| removedAt: { | ||
| not: null, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }) satisfies Prisma.AgentSkillWhereInput; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add missing PR link to changelog entry.
This is the only
[Unreleased]entry without a trailing PR link; every sibling entry (lines 16-22) ends with[#<id>](<url>).📝 Proposed fix
As per path instructions, "Update CHANGELOG.md with an entry under
[Unreleased]linking to the new PR" and "entries must include the GitHub pull request id at the end of the line, formatted as[#<id>](<url>)".📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Path instructions