mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
Fix skill source removal with a keep-or-delete choice (#16150)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - GitHub sources import skills and keep their updates together.
> - Disconnect only disabled a source and kept its read-only imports.
> - A disabled source could not be removed or imported again.
> - Removing a source needs an explicit decision about its imported
skills.
> - This pull request removes the source after the user keeps editable
copies or deletes the imports.
> - Existing disabled sources use the same removal flow.
## Linked Issues or Issue Description
Refs #15731, #14713.
**What happened?**
Disconnect left the repository on Sources. Its Disconnect action was
then disabled. The saved repository still blocked a new import.
**Expected behavior**
Remove the repository from Sources. Require a choice to keep editable
copies or delete its imports. Allow removal of sources that are already
disconnected.
**Steps to reproduce**
1. Import a repository in Skills → Sources.
2. Choose Disconnect source.
3. Observe the disconnected row and disabled removal action.
4. Try to import the repository again.
**Paperclip version or commit**
d67e3507b2.
## What Changed
- Add one confirmation with a required keep-or-delete choice.
- Remove the source and entries, including legacy disabled sources.
- Keep complete editable copies in My Skills, including shared
repository files and executable modes. Preserve skill IDs, keys,
versions, and agent assignments.
- Delete imported skills and source in one transaction with its activity
record. Check policy for every linked skill, including deselected
entries. Reject deletion of skills still used by agents.
- Fence removal with the source revision. Reload stale confirmations and
require a new choice. Prevent an in-flight refresh from restoring
removed imports.
- Reject incomplete legacy copies with a recovery message. Lock copied
skill rows against concurrent deletion and check publication succeeded.
- Add regression tests and production-page Storybook examples. Make an
initially unselected radio-card group keyboard accessible.
## Verification
- Workspace typecheck and build passed. Final affected server/UI
typechecks passed.
- Focused server, UI, and API suites passed: 114 tests initially. After
review fixes, all 21 source persistence and 58 route authorization tests
passed, including the complete repository copy regression; affected
server/UI typechecks and token gates passed again.
- Design token gates passed.
- Real isolated app: imported a public repository, assigned bro to the
CEO, verified that deletion reports the assignment, kept an editable
copy, observed the source disappear, opened the copy in My Skills, saved
an edit, and refreshed. The assignment remained. Re-imported the same
repository, deleted its new unused import, and verified the original
editable copy and assignment remained. No model tasks ran.
- Full local Vitest was attempted but terminated with exit 137 before
completion; it is not counted as a pass. Current-head sharded CI is
pending. Greptile reviewed the current head at 5/5 with no new
actionable comments. The final workspace build passed again.
- Storybook catalog aligned. The legacy removal example was inspected in
the browser; the Delete choice enables removal.
- Storybook: Skills / GitHub-synced skills / Sources · Disconnect source
and Sources · Remove disconnected source.
## Risks
- DELETE of a skill source now requires the saved revision and an
explicit copy/delete choice. Old callers must update their request body.
- Deletion removes imported skills and their records. Assigned skills
must be detached first.
- Incomplete legacy versions must be refreshed before keeping copies;
removal does not silently discard supporting files.
- Copies intentionally stop receiving upstream updates. Their IDs and
keys remain stable for assigned agents.
- No database migration. Copy writes use new managed directories and
clean up on failure. Delete restores quarantined files before releasing
publication locks on failure.
## Model Used
OpenAI GPT-6 through Codex, configured model `gpt-6-astra`. The runtime
does not expose the context-window size. Used shell tools, code editing,
tests, and browser interaction.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
df06df2306
commit
2ad36f4ff2
18 files changed
+505
-101
No files matched your search
+5
-1
@@ -85,7 +85,11 @@ skills through their existing GitHub connection and manually refresh installed
|
||||
snapshots. The source records repository, branch, path, and installed commit;
|
||||
the connection supplies caller-authorized access. Originals remain viewable and
|
||||
testable, with independent editable copies. New upstream skills require selection,
|
||||
and deselection, source disconnection, or upstream removal retains installed content.
|
||||
and deselection or upstream removal retains installed content. Disconnecting removes
|
||||
the source after an explicit choice: keep independent editable copies in My Skills,
|
||||
or delete its imported skills. Copies preserve agent assignments; skills still
|
||||
used by agents must be detached before deletion. Already-disconnected sources
|
||||
can be removed through the same flow.
|
||||
|
||||
An explicit restricted policy may deny selected operations or switch to a default-deny preset with explicit allow rules. Core exposes a stable versioned policy API so EE and other administrative clients configure and simulate the same evaluator used by skill mutation routes. Core Skill Studio only needs to perform normal skill work, explain an explicit denial, and point administrators to EE when its richer policy UI is available; it must not recreate a partial enterprise permission editor.
|
||||
|
||||
|
||||
@@ -2077,8 +2077,17 @@ unavailable. Preserve current ownership and newer-work fences. See
|
||||
runs use local installed snapshots; pinned/active runs retain their versions.
|
||||
- Additions require reviewed selection, with new candidates checked by default and
|
||||
explicit folder exclusions retained. Upstream deletion keeps installed content with
|
||||
removed status; moves are removal plus discovery. Deselect/disconnect retains content
|
||||
removed status; moves are removal plus discovery. Deselect retains content
|
||||
and assignments. Originals are read-only, with the existing independent fork workflow.
|
||||
- Source disconnection requires a saved revision and an explicit copy/delete choice.
|
||||
Remove the source and all entries to allow a fresh repository import, including
|
||||
legacy disabled sources. Copy materializes complete local files in the caller's
|
||||
My Skills folder, retaining skill IDs, keys, versions, and agent assignments.
|
||||
Delete applies skills.remove to every linked skill (including deselected entries),
|
||||
rejects skills still used by agents, and removes skills and source atomically with
|
||||
the activity record. Copy applies skills.edit and skills.create to every linked
|
||||
skill; failed writes or audit persistence leave the source and imported skills intact.
|
||||
|
||||
- Adopt recognizable existing GitHub imports without provider calls or content changes;
|
||||
resolve missing repository IDs and full snapshots on successful refresh. Exclude bundled,
|
||||
catalog, local/project, skills.sh, and unsupported hosts. Existing GitHub import and update
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
[Contract and maintenance](README.md) · [Review findings](REVIEW.md)
|
||||
|
||||
223 story files; 2210 indexed stories. Counts describe inventory, not passed tests or complete feature coverage.
|
||||
223 story files; 2212 indexed stories. Counts describe inventory, not passed tests or complete feature coverage.
|
||||
|
||||
Status describes product alignment; implementation describes the rendered subject; boundary describes simulated data. Sources and limitations are in catalog.json. Related capabilities cross-list one canonical story file.
|
||||
|
||||
@@ -254,7 +254,7 @@ Discovery, sources, authoring, revisions, tests, and agent policies.
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skill Studio/EditACopy](../../ui/storybook/stories/skills/fork-skill-flow.stories.tsx) | `skills/authoring` | unreviewed / production / fixtures | 6 / 0 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Product/Skill Studio · FrontmatterPanel](../../ui/storybook/stories/skills/frontmatter-panel.stories.tsx) | `skills/authoring` | unreviewed / production / fixtures | 13 / 0 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skills/GitHub sync journey](../../ui/storybook/stories/skills/github-skills-journey.stories.tsx) | `skills/versions`, `skills/discovery` | unreviewed / production / fixtures | 14 / 3 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skills/GitHub synced skills](../../ui/storybook/stories/skills/github-synced-skills.stories.tsx) | `skills/versions`, `skills/discovery` | unreviewed / production / fixtures | 19 / 6 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skills/GitHub synced skills](../../ui/storybook/stories/skills/github-synced-skills.stories.tsx) | `skills/versions`, `skills/discovery` | unreviewed / production / fixtures | 21 / 8 | Production Sources page with fixture APIs. Disconnect and legacy removal stories show a required keep-or-delete choice; the real copy, delete, and reimport flows were also tested locally. Other routes remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skills/Repository packages](../../ui/storybook/stories/skills/repository-skill-packages/repository-skill-packages.stories.tsx) | `skills/versions`, `skills/discovery` | unreviewed / production / fixtures | 9 / 3 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Skills/Import progress](../../ui/storybook/stories/skills/skill-import-progress.stories.tsx) | `skills/versions`, `skills/discovery` | unreviewed / production / fixtures | 6 / 0 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
| [Agents and reusable capabilities/Skills and Skill Studio/Product/Skills/Policy surfaces](../../ui/storybook/stories/skills/skill-policy-surfaces.stories.tsx) | `skills/policy` | unreviewed / production / fixtures | 2 / 0 | Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed. |
|
||||
|
||||
@@ -2619,7 +2619,7 @@
|
||||
"implementation": "production",
|
||||
"boundary": "fixtures",
|
||||
"notes": "Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed.",
|
||||
"reviewedAt": "2026-10-12",
|
||||
"reviewedAt": "2026-10-13",
|
||||
"sources": [
|
||||
"maps/feature-map/skills.md",
|
||||
"ui/src/components/Layout.tsx",
|
||||
@@ -2630,7 +2630,7 @@
|
||||
"ui/storybook/fixtures/paperclipData.ts",
|
||||
"ui/storybook/stories/skills/github-skills-journey.stories.tsx"
|
||||
],
|
||||
"fingerprint": "dcb8e4ce80ddc71f25542b9d84cdf1ce20e552da2848404e62d61f0687e3fc3f",
|
||||
"fingerprint": "04b12de08d13813c1332d46f9bed8164f329aaf73ea0c57cbf83ecb6d3a4c6b1",
|
||||
"stableId": "skills-github-sync-journey",
|
||||
"sidebarTitle": "Agents and reusable capabilities/Skills and Skill Studio/Skills/GitHub sync journey"
|
||||
},
|
||||
@@ -2644,8 +2644,8 @@
|
||||
"status": "unreviewed",
|
||||
"implementation": "production",
|
||||
"boundary": "fixtures",
|
||||
"notes": "Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed.",
|
||||
"reviewedAt": "2026-10-12",
|
||||
"notes": "Production Sources page with fixture APIs. Disconnect and legacy removal stories show a required keep-or-delete choice; the real copy, delete, and reimport flows were also tested locally. Other routes remain unreviewed.",
|
||||
"reviewedAt": "2026-10-13",
|
||||
"sources": [
|
||||
"maps/feature-map/skills.md",
|
||||
"ui/src/pages/SkillSources.tsx",
|
||||
@@ -2653,7 +2653,7 @@
|
||||
"ui/storybook/fixtures/paperclipData.ts",
|
||||
"ui/storybook/stories/skills/github-synced-skills.stories.tsx"
|
||||
],
|
||||
"fingerprint": "f87c8ee2574a097ef4e4828589618ad196f170fb652db022bc7488b3fb7971e2",
|
||||
"fingerprint": "c14509e4b57e7587f560824f417c8439dbb6bc3d303d37dec169ab819c867d36",
|
||||
"stableId": "skills-github-synced-skills",
|
||||
"sidebarTitle": "Agents and reusable capabilities/Skills and Skill Studio/Skills/GitHub synced skills"
|
||||
},
|
||||
@@ -3669,7 +3669,7 @@
|
||||
"implementation": "production",
|
||||
"boundary": "fixtures",
|
||||
"notes": "Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed.",
|
||||
"reviewedAt": "2026-10-12",
|
||||
"reviewedAt": "2026-10-13",
|
||||
"sources": [
|
||||
"ui/src/components/ui/alert-dialog.tsx",
|
||||
"ui/src/components/ui/avatar.tsx",
|
||||
@@ -3685,7 +3685,7 @@
|
||||
"ui/storybook/stories/foundations/primitives-coverage.stories.tsx"
|
||||
],
|
||||
"exception": "Shared foundations or contributor guide; no single business capability owns this suite.",
|
||||
"fingerprint": "b8a0afd9a2fefb2e89e3215ca86a7dea3673f254d361f7d821d43c13c2a795bc",
|
||||
"fingerprint": "2fba85501507d80dae7b3c202c083d9ac32c796ee8e234a4fecc6a80b33fa35c",
|
||||
"stableId": "foundations-primitive-coverage",
|
||||
"sidebarTitle": "Foundations/Primitive Coverage"
|
||||
},
|
||||
@@ -4404,7 +4404,7 @@
|
||||
"implementation": "production",
|
||||
"boundary": "fixtures",
|
||||
"notes": "Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed.",
|
||||
"reviewedAt": "2026-10-12",
|
||||
"reviewedAt": "2026-10-13",
|
||||
"sources": [
|
||||
"maps/feature-map/skills.md",
|
||||
"ui/src/components/Layout.tsx",
|
||||
@@ -4416,7 +4416,7 @@
|
||||
"ui/storybook/stories/skills/github-skills-journey.stories.tsx",
|
||||
"ui/storybook/stories/skills/repository-skill-packages/repository-skill-packages.stories.tsx"
|
||||
],
|
||||
"fingerprint": "a90fb6c9377034d3645b851711ddc84eb3b9945dce0cdea4496cb9a4050fb8ad",
|
||||
"fingerprint": "d8af830b23aa96f0a92b898b16c090e76fe4e9b986517fbf72d9cdedf840e7e4",
|
||||
"stableId": "skills-repository-packages",
|
||||
"sidebarTitle": "Agents and reusable capabilities/Skills and Skill Studio/Skills/Repository packages"
|
||||
},
|
||||
@@ -4749,7 +4749,7 @@
|
||||
"implementation": "production",
|
||||
"boundary": "fixtures",
|
||||
"notes": "Shipped subjects identified through imports and local wrappers; full state, fixture-contract and route parity remain unreviewed.",
|
||||
"reviewedAt": "2026-10-12",
|
||||
"reviewedAt": "2026-10-13",
|
||||
"sources": [
|
||||
"maps/feature-map/skills.md",
|
||||
"ui/src/pages/skills/SkillImportProgress.tsx",
|
||||
@@ -4757,7 +4757,7 @@
|
||||
"ui/storybook/fixtures/paperclipData.ts",
|
||||
"ui/storybook/stories/skills/skill-import-progress.stories.tsx"
|
||||
],
|
||||
"fingerprint": "55477a243eb0cd940e20e47b0c2e388af26268930be5ee516043a8812f3ab0e5",
|
||||
"fingerprint": "4f517ad1f4e9a390e29d2f5f31bd78209179043f4253817b5a2788c8a4824a6c",
|
||||
"stableId": "skills-import-progress",
|
||||
"sidebarTitle": "Agents and reusable capabilities/Skills and Skill Studio/Skills/Import progress"
|
||||
},
|
||||
|
||||
@@ -141,3 +141,13 @@ export interface SkillSourceRefreshResult {
|
||||
unchanged: number;
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
export interface SkillSourceDisconnectRequest {
|
||||
revision: number;
|
||||
skills: 'copy' | 'delete';
|
||||
}
|
||||
export interface SkillSourceDisconnectResult {
|
||||
id: string;
|
||||
copiedCount: number;
|
||||
deletedCount: number;
|
||||
}
|
||||
@@ -25,3 +25,8 @@ export const skillSourceSelectionSchema = z.object({
|
||||
excludedFolders: z.array(repoPath).max(10000),
|
||||
connectionId: z.string().uuid().nullable().optional(),
|
||||
});
|
||||
|
||||
export const skillSourceDisconnectSchema = z.object({
|
||||
revision: z.number().int().nonnegative(),
|
||||
skills: z.enum(['copy', 'delete']),
|
||||
});
|
||||
@@ -1293,6 +1293,14 @@ describe("company skill mutation permissions", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("requires an explicit skill disposition and revision when removing a source", async () => {
|
||||
const app = createApp({ type: "board", userId: "member", companyIds: ["company-1"], source: "session" });
|
||||
const path = "/api/companies/company-1/skill-sources/source-id";
|
||||
for (const body of [{}, { revision: 0 }, { skills: "delete" }, { revision: 0, skills: "ignore" }]) {
|
||||
await request(app).delete(path).send(body).expect(400);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects cross-company source reads and mutations for board users and agents", async () => {
|
||||
for (const actor of [
|
||||
{ type: "board", userId: "member", companyIds: ["company-1"], isInstanceAdmin: true, source: "session" },
|
||||
|
||||
@@ -5,7 +5,7 @@ import { promises as fs } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import { companies, companySkills, companySkillVersions, companySkillRepositorySnapshots, createDb } from '@paperclipai/db';
|
||||
import { agents, folders, companySkillSources, companies, companySkills, companySkillVersions, companySkillRepositorySnapshots, createDb } from '@paperclipai/db';
|
||||
import { eq, sql } from 'drizzle-orm';
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from './helpers/embedded-postgres.js';
|
||||
import { unprocessable } from '../errors.js';
|
||||
@@ -81,8 +81,9 @@ describe.skipIf(!support.supported)('skill source persistence', () => {
|
||||
expect((await promisify(execFile)(path.join(staged.source, 'scripts/run.sh'))).stdout.trim()).toBe('hello');
|
||||
} finally { fetchSpy.mockRestore(); }
|
||||
await expect(sources.refresh(companyId, result.source.id, context, { revision: 0, selectedPaths: [], excludedFolders: [] })).rejects.toThrow(/changed/);
|
||||
const disconnected = await sources.disconnect(companyId, result.source.id, context);
|
||||
expect(disconnected.enabled).toBe(false);
|
||||
const disconnected = await sources.disconnect(companyId, result.source.id, context, { revision: (await sources.detail(companyId, result.source.id)).revision, skills: 'copy' });
|
||||
expect(disconnected.copiedCount).toBe(2);
|
||||
await expect(sources.detail(companyId, result.source.id)).rejects.toThrow(/not found/);
|
||||
expect(await skills.getById(companyId, skill.id)).not.toBeNull();
|
||||
await expect(sources.detail(randomUUID(), result.source.id)).rejects.toThrow(/not found/);
|
||||
});
|
||||
@@ -186,12 +187,37 @@ describe.skipIf(!support.supported)('skill source persistence', () => {
|
||||
const provider = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('Existing reference'));
|
||||
try { expect((await skills.readFile(legacyCompany, legacy!.id, 'reference.md'))?.content).toBe('Existing reference'); }
|
||||
finally { provider.mockRestore(); }
|
||||
const revision = (await service.detail(legacyCompany, adopted[0]!.id)).revision;
|
||||
await expect(service.disconnect(legacyCompany, adopted[0]!.id, context, { revision, skills: 'copy' })).rejects.toThrow(/saved files are incomplete.*Refresh the source/);
|
||||
expect((await skills.getById(legacyCompany, legacy!.id))?.sourceType).toBe('github');
|
||||
expect((await service.detail(legacyCompany, adopted[0]!.id)).revision).toBe(revision);
|
||||
await db.update(companySkills).set({ currentVersionId: null }).where(eq(companySkills.id, legacy!.id));
|
||||
await expect(service.disconnect(legacyCompany, adopted[0]!.id, context, { revision, skills: 'copy' })).rejects.toThrow(/saved files are incomplete/);
|
||||
await db.update(companySkills).set({ currentVersionId: partial!.id }).where(eq(companySkills.id, legacy!.id));
|
||||
files = { 'legacy/SKILL.md': md('updated name') };
|
||||
const refreshed = await service.refresh(legacyCompany, adopted[0]!.id, context);
|
||||
expect(refreshed.updated[0]).toMatchObject({ id: legacy!.id, key: legacy!.key, slug: legacy!.slug });
|
||||
expect((await db.select().from(companySkills).where(eq(companySkills.companyId, legacyCompany))).filter(skill => skill.key === legacy!.key)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not report a copy when an imported skill is deleted before publication', async () => {
|
||||
files = { 'SKILL.md': md('copy-race') }; commit = sha;
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await service.create(companyId, { repositoryUrl: 'https://github.com/acme/skills', trackingRef: 'copy-race', commitSha: sha, selectedPaths: ['SKILL.md'] }, context);
|
||||
let release!: () => void, entered!: () => void;
|
||||
const started = new Promise<void>(resolve => { entered = resolve; });
|
||||
const blocked = new Promise<void>(resolve => { release = resolve; });
|
||||
const pending = service.disconnect(companyId, created.source.id, { ...context, authorize: async action => {
|
||||
if (action === 'skills.create') { entered(); await blocked; }
|
||||
} }, { revision: created.source.revision, skills: 'copy' });
|
||||
await started;
|
||||
await skills.deleteSkill(companyId, created.imported[0]!.id);
|
||||
release();
|
||||
await expect(pending).rejects.toThrow(/imported skill changed/);
|
||||
expect((await service.detail(companyId, created.source.id)).id).toBe(created.source.id);
|
||||
expect(await skills.getById(companyId, created.imported[0]!.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('resolves legacy commit-only imports to the default branch without duplicating skills', async () => {
|
||||
const legacyCompany = randomUUID();
|
||||
await db.insert(companies).values({ id: legacyCompany, name: 'Old imports', issuePrefix: 'OLD' });
|
||||
@@ -411,4 +437,110 @@ describe.skipIf(!support.supported)('skill source persistence', () => {
|
||||
expect(await db.select().from(companySkillVersions).where(eq(companySkillVersions.companyId, temporaryCompany))).toHaveLength(0);
|
||||
});
|
||||
|
||||
async function removalFixture(mode: 'skills' | 'repository' = 'repository') {
|
||||
files = { 'one/SKILL.md': md('one') + '\nRead [reference](reference.md)', 'one/reference.md': 'Local reference',
|
||||
'two/SKILL.md': md('two'), 'shared/check.sh': '#!/bin/sh\necho preserved',
|
||||
'shared/image.png': Buffer.from([0, 255, 137, 80]) };
|
||||
commit = sha;
|
||||
return skillSourceService(db).create(companyId, { repositoryUrl: 'https://github.com/acme/skills',
|
||||
trackingRef: `remove-${randomUUID()}`, packageMode: mode, commitSha: sha, selectedPaths: ['one/SKILL.md', 'two/SKILL.md'] }, context);
|
||||
}
|
||||
|
||||
it('removes legacy disconnected sources and all linked imports, including deselected skills, and allows re-import', async () => {
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await removalFixture('skills');
|
||||
const changed = await service.refresh(companyId, created.source.id, context, {
|
||||
revision: created.source.revision, selectedPaths: ['one/SKILL.md'], excludedFolders: [] });
|
||||
await db.update(companySkillSources).set({ enabled: false }).where(eq(companySkillSources.id, created.source.id));
|
||||
const audit = vi.fn();
|
||||
const result = await service.disconnect(companyId, created.source.id, { ...context, audit }, { revision: changed.source.revision, skills: 'delete' });
|
||||
expect(result).toMatchObject({ deletedCount: 2, copiedCount: 0 });
|
||||
await expect(service.detail(companyId, created.source.id)).rejects.toThrow(/not found/);
|
||||
for (const skill of created.imported) expect(await skills.getById(companyId, skill.id)).toBeNull();
|
||||
expect(audit).toHaveBeenCalledWith(expect.anything(), created.source.id, 'company.skill_source_disconnected', expect.objectContaining({ deletedCount: 2 }));
|
||||
const reimported = await service.create(companyId, { repositoryUrl: created.source.repositoryUrl, trackingRef: created.source.trackingRef,
|
||||
commitSha: sha, selectedPaths: ['one/SKILL.md'] }, context);
|
||||
expect(reimported.imported).toHaveLength(1);
|
||||
expect(reimported.imported[0]!.id).not.toBe(created.imported[0]!.id);
|
||||
});
|
||||
|
||||
it('keeps complete editable personal copies and assigned identities without requiring GitHub', async () => {
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await removalFixture();
|
||||
const original = created.imported[0]!;
|
||||
const [agent] = await db.insert(agents).values({ companyId, name: 'Copy user', role: 'engineer',
|
||||
adapterType: 'codex_local', adapterConfig: { paperclipSkillSync: { desiredSkills: [{ key: original.key }] } } }).returning();
|
||||
const result = await service.disconnect(companyId, created.source.id, { ...context, read: () => { throw new Error('GitHub unavailable'); } }, { revision: created.source.revision, skills: 'copy' });
|
||||
expect(result).toMatchObject({ copiedCount: 2, deletedCount: 0 });
|
||||
await expect(service.detail(companyId, created.source.id)).rejects.toThrow(/not found/);
|
||||
const copy = (await skills.detail(companyId, original.id))!;
|
||||
expect(copy).toMatchObject({ id: original.id, key: original.key, sourceType: 'local_path', sourceRef: null, editable: true });
|
||||
expect(copy.metadata?.skillSourceId).toBeUndefined();
|
||||
expect((await skills.readFile(companyId, original.id, '.paperclip-repository/one/reference.md'))?.content).toBe('Local reference');
|
||||
const [personal] = await db.select().from(folders).where(eq(folders.id, copy.folderId!));
|
||||
expect(personal?.systemKey).toBe('my:board');
|
||||
expect((await skills.readFile(companyId, original.id, '.paperclip-repository/shared/image.png'))?.content).toBe(Buffer.from([0,255,137,80]).toString('base64'));
|
||||
const script = await skills.readFile(companyId, original.id, '.paperclip-repository/shared/check.sh');
|
||||
expect(script).toMatchObject({ content: '#!/bin/sh\necho preserved', executable: true });
|
||||
await skills.updateFile(companyId, original.id, '.paperclip-repository/shared/check.sh', '#!/bin/sh\necho edited', context.actor);
|
||||
expect((await skills.readFile(companyId, original.id, '.paperclip-repository/shared/check.sh'))?.content).toBe('#!/bin/sh\necho edited');
|
||||
expect((await db.select().from(agents).where(eq(agents.id, agent!.id)))[0]?.adapterConfig).toEqual(agent!.adapterConfig);
|
||||
await db.delete(agents).where(eq(agents.id, agent!.id));
|
||||
});
|
||||
|
||||
it('does not partly delete a source when an imported skill is assigned to an agent', async () => {
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await removalFixture();
|
||||
const [agent] = await db.insert(agents).values({ companyId, name: 'Needs this skill', role: 'engineer', adapterType: 'codex_local',
|
||||
adapterConfig: { paperclipSkillSync: { desiredSkills: [{ key: created.imported[1]!.key }] } } }).returning();
|
||||
try {
|
||||
await expect(service.disconnect(companyId, created.source.id, context, { revision: created.source.revision, skills: 'delete' })).rejects.toThrow(/Needs this skill/);
|
||||
expect((await service.detail(companyId, created.source.id)).enabled).toBe(true);
|
||||
for (const skill of created.imported) expect(await skills.getById(companyId, skill.id)).not.toBeNull();
|
||||
} finally { await db.delete(agents).where(eq(agents.id, agent!.id)); }
|
||||
});
|
||||
|
||||
it('rejects stale revisions, other-company targets, and removal denied for a deselected skill', async () => {
|
||||
const service = skillSourceService(db);
|
||||
const created = await removalFixture();
|
||||
const changed = await service.refresh(companyId, created.source.id, context, { revision: created.source.revision, selectedPaths: ['one/SKILL.md'], excludedFolders: [] });
|
||||
await expect(service.disconnect(companyId, created.source.id, context, { revision: created.source.revision, skills: 'delete' })).rejects.toThrow(/changed/);
|
||||
await expect(service.disconnect(randomUUID(), created.source.id, context, { revision: changed.source.revision, skills: 'delete' })).rejects.toThrow(/not found/);
|
||||
const excluded = changed.source.entries.find(entry => entry.selection === 'excluded')!;
|
||||
await expect(service.disconnect(companyId, created.source.id, { ...context, authorize: async (action, resource) => {
|
||||
if (action === 'skills.remove' && resource.skillId === excluded.skillId) throw new Error('Protected skill');
|
||||
} }, { revision: changed.source.revision, skills: 'delete' })).rejects.toThrow('Protected skill');
|
||||
expect((await service.detail(companyId, created.source.id)).entries.filter(entry => entry.skillId)).toHaveLength(2);
|
||||
});
|
||||
|
||||
it.each(['copy', 'delete'] as const)('rolls back source and skill changes when %s activity persistence fails', async choice => {
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await removalFixture();
|
||||
await expect(service.disconnect(companyId, created.source.id, { ...context, audit: async () => { throw new Error('Audit unavailable'); } },
|
||||
{ revision: created.source.revision, skills: choice })).rejects.toThrow('Audit unavailable');
|
||||
expect((await service.detail(companyId, created.source.id)).revision).toBe(created.source.revision);
|
||||
for (const skill of created.imported) {
|
||||
expect(await skills.getById(companyId, skill.id)).toMatchObject({ sourceType: 'github', currentVersionId: skill.currentVersionId });
|
||||
expect(await skills.readFile(companyId, skill.id, '.paperclip-repository/shared/image.png')).not.toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('removes a source while its refresh is in flight without resurrecting imports', async () => {
|
||||
const service = skillSourceService(db), skills = companySkillService(db);
|
||||
const created = await removalFixture();
|
||||
let entered!: () => void, release!: () => void;
|
||||
const started = new Promise<void>(resolve => { entered = resolve; });
|
||||
const blocked = new Promise<void>(resolve => { release = resolve; });
|
||||
const reader = context.read(null);
|
||||
const refreshing = service.refresh(companyId, created.source.id, { ...context, read: () => Object.assign(async (request: string) => {
|
||||
entered(); await blocked; return reader(request);
|
||||
}, { openSnapshot: reader.openSnapshot }) }).then(() => null, error => error);
|
||||
await started;
|
||||
await service.disconnect(companyId, created.source.id, context, { revision: created.source.revision, skills: 'delete' });
|
||||
release();
|
||||
expect(await refreshing).toBeInstanceOf(Error);
|
||||
await expect(service.detail(companyId, created.source.id)).rejects.toThrow(/not found/);
|
||||
for (const skill of created.imported) expect(await skills.getById(companyId, skill.id)).toBeNull();
|
||||
});
|
||||
|
||||
});
|
||||
@@ -1,7 +1,7 @@
|
||||
import { skillSourceService, type SkillSourceContext } from "../services/skill-sources.js";
|
||||
import { skillSourceGitHubReader } from "../services/skill-source-github-access.js";
|
||||
import { toolAccessService } from "../services/tool-access.js";
|
||||
import { skillSourceCreateSchema, skillSourceDiscoverySchema, skillSourcePreviewSchema, skillSourceSelectionSchema } from "@paperclipai/shared";
|
||||
import { skillSourceDisconnectSchema, skillSourceCreateSchema, skillSourceDiscoverySchema, skillSourcePreviewSchema, skillSourceSelectionSchema } from "@paperclipai/shared";
|
||||
import type { ActivityPublication } from "../services/activity-log.js";
|
||||
import { once } from "node:events";
|
||||
import type { SkillSourceDiscoveryEvent } from "@paperclipai/shared";
|
||||
@@ -390,9 +390,12 @@ export function companySkillRoutes(db: Db) {
|
||||
const companyId = req.params.companyId as string;
|
||||
res.json(await sourceOperation(req, companyId, context => sourceSvc.refresh(companyId, req.params.sourceId as string, context)));
|
||||
});
|
||||
router.delete("/companies/:companyId/skill-sources/:sourceId", async (req, res) => {
|
||||
router.delete("/companies/:companyId/skill-sources/:sourceId", (req, _res, next) => {
|
||||
assertCompanyAccess(req, req.params.companyId as string);
|
||||
next();
|
||||
}, validate(skillSourceDisconnectSchema), async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
res.json(await sourceOperation(req, companyId, context => sourceSvc.disconnect(companyId, req.params.sourceId as string, context)));
|
||||
res.json(await sourceOperation(req, companyId, context => sourceSvc.disconnect(companyId, req.params.sourceId as string, context, req.body)));
|
||||
});
|
||||
|
||||
router.get("/skills/catalog", async (req, res) => {
|
||||
|
||||
@@ -7159,45 +7159,87 @@ export function companySkillService(db: Db) {
|
||||
return { pruned: rows.length };
|
||||
}
|
||||
|
||||
async function deleteSkill(companyId: string, skillId: string): Promise<CompanySkill | null> {
|
||||
const initial = await getById(companyId, skillId);
|
||||
if (!initial) return null;
|
||||
async function makeImportedSkillEditable(
|
||||
companyId: string, skillId: string, actor: SkillActor, folderId: string | null,
|
||||
database: DbOrTransaction, createdDirectories: string[],
|
||||
) {
|
||||
const [locked] = await database.select({ id: companySkills.id }).from(companySkills)
|
||||
.where(and(eq(companySkills.companyId, companyId), eq(companySkills.id, skillId))).for("update");
|
||||
if (!locked) throw conflict("An imported skill changed. Reload before disconnecting.");
|
||||
const skill = (await getById(companyId, skillId, database))!;
|
||||
const version = await getCurrentVersion(skill);
|
||||
const recovery = `Cannot keep "${skill.name}" because its saved files are incomplete. Refresh the source before keeping editable copies, or choose Delete skills.`;
|
||||
if (!version) throw unprocessable(recovery);
|
||||
const files = await runtimeVersionInventory(companyId, version);
|
||||
const savedPaths = new Set(files.map(file => file.path));
|
||||
const savedSkillPath = (relativePath: string) => version.repositorySnapshotId && version.repositorySkillPath
|
||||
? `${REPOSITORY_CONTENT_DIR}/${path.posix.join(path.posix.dirname(version.repositorySkillPath), relativePath)}`
|
||||
: relativePath;
|
||||
if (!savedPaths.has("SKILL.md") || skill.fileInventory.some(file => !savedPaths.has(savedSkillPath(file.path)))) {
|
||||
throw unprocessable(recovery);
|
||||
}
|
||||
const managedRoot = resolveManagedSkillsRoot(companyId);
|
||||
await fs.mkdir(managedRoot, { recursive: true });
|
||||
const directory = await fs.mkdtemp(path.join(managedRoot, `${skill.slug}-copy-`));
|
||||
createdDirectories.push(directory);
|
||||
for (const file of files) {
|
||||
assertSkillSnapshotPath(file.path);
|
||||
const target = path.join(directory, file.path);
|
||||
await fs.mkdir(path.dirname(target), { recursive: true });
|
||||
await fs.writeFile(target, skillFileBytes(file), { mode: file.executable ? 0o755 : 0o644 });
|
||||
}
|
||||
const updated = await database.update(companySkills).set({
|
||||
sourceType: "local_path", sourceLocator: directory, sourceRef: null, folderId,
|
||||
fileInventory: files.map(({ path, kind }) => ({ path, kind })),
|
||||
metadata: {
|
||||
sourceKind: "managed_local",
|
||||
copiedFromRepository: { url: skill.sourceLocator, ref: skill.sourceRef, path: skill.metadata?.skillSourcePath },
|
||||
forkedByUserId: actor.type === "user" ? actor.userId ?? null : null,
|
||||
forkedByAgentId: actor.type === "agent" ? actor.agentId ?? null : null,
|
||||
},
|
||||
updatedAt: new Date(),
|
||||
}).where(and(eq(companySkills.companyId, companyId), eq(companySkills.id, skillId))).returning({ id: companySkills.id });
|
||||
if (updated.length !== 1) throw conflict("An imported skill changed. Reload before disconnecting.");
|
||||
}
|
||||
|
||||
async function deleteSkills(
|
||||
companyId: string, skillIds: string[],
|
||||
onDelete?: (tx: DbTransaction, deleted: CompanySkill[]) => Promise<void>,
|
||||
beforeDelete?: (tx: DbTransaction) => Promise<void>,
|
||||
): Promise<CompanySkill[]> {
|
||||
const initial = (await Promise.all([...new Set(skillIds)].sort().map(id => getById(companyId, id))))
|
||||
.filter((skill): skill is CompanySkill => Boolean(skill));
|
||||
async function assertUnused(skill: CompanySkill) {
|
||||
const usedByAgents = await usage(companyId, skill.key);
|
||||
if (usedByAgents.length === 0) return;
|
||||
const agentNames = usedByAgents.map((agent) => agent.name).sort((left, right) => left.localeCompare(right));
|
||||
throw unprocessable(
|
||||
`Cannot delete skill "${skill.name}" while it is still used by ${agentNames.join(", ")}. Detach it from those agents first.`,
|
||||
{
|
||||
skillId: skill.id, skillKey: skill.key,
|
||||
usedByAgents: usedByAgents.map((agent) => ({
|
||||
id: agent.id, name: agent.name, urlKey: agent.urlKey, adapterType: agent.adapterType,
|
||||
})),
|
||||
},
|
||||
`Cannot delete skill "${skill.name}" while it is still used by ${usedByAgents.map(agent => agent.name).sort().join(", ")}. Detach it from those agents first.`,
|
||||
{ skillId: skill.id, skillKey: skill.key, usedByAgents: usedByAgents.map(({ id, name, urlKey, adapterType }) => ({ id, name, urlKey, adapterType })) },
|
||||
);
|
||||
}
|
||||
await assertUnused(initial);
|
||||
for (const skill of initial) await assertUnused(skill);
|
||||
const managedRoot = resolveManagedSkillsRoot(companyId);
|
||||
let movedSource: { source: string; quarantine: string } | undefined;
|
||||
let deleted: CompanySkill | null = null;
|
||||
async function restoreSource() {
|
||||
if (!movedSource) return;
|
||||
await fs.rename(movedSource.quarantine, movedSource.source);
|
||||
movedSource = undefined;
|
||||
const moved: Array<{ source: string; quarantine: string }> = [];
|
||||
async function restoreSources() {
|
||||
for (const item of moved.reverse()) await fs.rename(item.quarantine, item.source);
|
||||
moved.length = 0;
|
||||
}
|
||||
// Keep the cache lifecycle lock through database commit. A publisher must
|
||||
// not see the old row after cache removal and republish a deleted skill.
|
||||
await removeRuntimeSkillCache(managedRoot, initial.id, async () => {
|
||||
async function withCacheLocks(index: number): Promise<CompanySkill[]> {
|
||||
if (index < initial.length) {
|
||||
let result: CompanySkill[] = [];
|
||||
await removeRuntimeSkillCache(managedRoot, initial[index]!.id, async () => { result = await withCacheLocks(index + 1); });
|
||||
return result;
|
||||
}
|
||||
try {
|
||||
deleted = await db.transaction(async (tx) => {
|
||||
// Share creation's name lock and re-read the ID, so a second delete
|
||||
// cannot remove a newly recreated skill with the same name.
|
||||
await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${`${companyId}:${initial.slug}`}, 0))`);
|
||||
const skill = await getById(companyId, skillId, tx);
|
||||
if (!skill) return null;
|
||||
if (skill.slug !== initial.slug) throw conflict("Skill changed during deletion. Retry the request.");
|
||||
await assertUnused(skill);
|
||||
try {
|
||||
return await db.transaction(async tx => {
|
||||
await beforeDelete?.(tx);
|
||||
const deleted: CompanySkill[] = [];
|
||||
for (const before of initial) {
|
||||
await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${`${companyId}:${before.slug}`}, 0))`);
|
||||
const skill = await getById(companyId, before.id, tx);
|
||||
if (!skill) continue;
|
||||
if (skill.slug !== before.slug) throw conflict("Skill changed during deletion. Retry the request.");
|
||||
await assertUnused(skill);
|
||||
await fs.rm(resolveRuntimeSkillMaterializedPath(companyId, skill), { recursive: true, force: true });
|
||||
if (isPaperclipManagedRenameTarget(skill)) {
|
||||
const source = normalizeSkillDirectory(skill)!;
|
||||
@@ -7209,35 +7251,36 @@ export function companySkillService(db: Db) {
|
||||
const quarantineRoot = path.join(managedRoot, ".deleted");
|
||||
await fs.mkdir(quarantineRoot, { recursive: true });
|
||||
const quarantine = path.join(quarantineRoot, `${skill.id}-${randomUUID()}`);
|
||||
// Do not follow symlinks or delete external/project sources.
|
||||
// Retain bytes until the database deletion commits.
|
||||
await fs.rename(source, quarantine);
|
||||
movedSource = { source, quarantine };
|
||||
moved.push({ source, quarantine });
|
||||
}
|
||||
}
|
||||
await tx.delete(companySkills).where(and(eq(companySkills.id, skillId), eq(companySkills.companyId, companyId)));
|
||||
} catch (error) {
|
||||
// Restore before releasing the name lock on a failed write.
|
||||
await restoreSource();
|
||||
throw error;
|
||||
await tx.delete(companySkills).where(and(eq(companySkills.id, skill.id), eq(companySkills.companyId, companyId)));
|
||||
deleted.push(skill);
|
||||
}
|
||||
return skill;
|
||||
await onDelete?.(tx, deleted);
|
||||
return deleted;
|
||||
});
|
||||
} catch (error) {
|
||||
await restoreSource();
|
||||
await restoreSources();
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
// Only remove the unique quarantine path after commit. Cleanup must not
|
||||
// remove a replacement skill or report a committed delete as failed.
|
||||
if (movedSource) {
|
||||
await fs.rm(movedSource.quarantine, { recursive: true, force: true }).catch((error) => {
|
||||
logger.warn({ err: error, companyId, skillId }, "Skill deleted; quarantined source cleanup failed");
|
||||
}
|
||||
let deleted: CompanySkill[];
|
||||
try { deleted = await withCacheLocks(0); }
|
||||
catch (error) { await restoreSources(); throw error; }
|
||||
for (const item of moved) {
|
||||
await fs.rm(item.quarantine, { recursive: true, force: true }).catch(error => {
|
||||
logger.warn({ err: error, companyId }, "Skill deleted; quarantined source cleanup failed");
|
||||
});
|
||||
}
|
||||
return deleted;
|
||||
}
|
||||
|
||||
async function deleteSkill(companyId: string, skillId: string): Promise<CompanySkill | null> {
|
||||
return (await deleteSkills(companyId, [skillId]))[0] ?? null;
|
||||
}
|
||||
|
||||
return {
|
||||
list,
|
||||
listFull,
|
||||
@@ -7277,6 +7320,8 @@ export function companySkillService(db: Db) {
|
||||
deleteFile,
|
||||
createLocalSkill,
|
||||
deleteSkill,
|
||||
deleteSkills,
|
||||
makeImportedSkillEditable,
|
||||
listTestInputs,
|
||||
createTestInput,
|
||||
updateTestInput,
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { createHash, randomUUID } from 'node:crypto';
|
||||
import path from 'node:path';
|
||||
import { promises as fs } from 'node:fs';
|
||||
import { folderService } from './folders.js';
|
||||
import { and, eq, isNull, lt, or, sql } from 'drizzle-orm';
|
||||
import { companySkillSources as sources, companySkillSourceEntries as entries, companySkills, companySkillRepositorySnapshots, type Db } from '@paperclipai/db';
|
||||
import type { CompanySkill, SkillSource, SkillSourceCreateRequest, SkillSourceDiscoveryRequest, SkillSourceSelectionRequest, SkillSourceRefreshResult, SkillSourcePreviewRequest } from '@paperclipai/shared';
|
||||
import { companySkillSources as sources, companySkillSourceEntries as entries, companySkills, companySkillRepositorySnapshots, authUsers, type Db } from '@paperclipai/db';
|
||||
import type { CompanySkill, SkillSourceDisconnectRequest, SkillSourceDisconnectResult, SkillSource, SkillSourceCreateRequest, SkillSourceDiscoveryRequest, SkillSourceSelectionRequest, SkillSourceRefreshResult, SkillSourcePreviewRequest } from '@paperclipai/shared';
|
||||
import { normalizeAgentUrlKey } from '@paperclipai/shared';
|
||||
import { conflict, notFound, unprocessable } from '../errors.js';
|
||||
import { companySkillService, parseSkillImportSourceInput } from './company-skills.js';
|
||||
@@ -14,7 +16,7 @@ type SourceRow = typeof sources.$inferSelect;
|
||||
export type SkillSourceContext = {
|
||||
actor: { type: 'user' | 'agent' | 'system'; userId?: string | null; agentId?: string | null };
|
||||
read: (connectionId: string | null) => GitHubRead;
|
||||
authorize: (action: 'skills.import' | 'skills.update' | 'skills.edit', resource: { skillId?: string; skillKey?: string; sourceType: 'git'; sourceLocator: string }) => Promise<void>;
|
||||
authorize: (action: 'skills.import' | 'skills.update' | 'skills.edit' | 'skills.remove' | 'skills.create', resource: { skillId?: string; skillKey?: string; sourceType: 'git'; sourceLocator: string }) => Promise<void>;
|
||||
audit: (tx: Tx, sourceId: string, action: string, details: Record<string, unknown>) => Promise<void>;
|
||||
};
|
||||
const scope = (companyId: string, id: string) => and(eq(sources.companyId, companyId), eq(sources.id, id));
|
||||
@@ -217,21 +219,47 @@ export function skillSourceService(db: Db) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
async function disconnect(companyId: string, id: string, context: SkillSourceContext) {
|
||||
const source = await row(companyId, id);
|
||||
async function disconnect(companyId: string, id: string, context: SkillSourceContext, input: SkillSourceDisconnectRequest): Promise<SkillSourceDisconnectResult> {
|
||||
const source = await detail(companyId, id);
|
||||
if (source.revision !== input.revision) throw conflict('This source changed. Reload before disconnecting.');
|
||||
await context.authorize('skills.edit', { sourceType: 'git', sourceLocator: source.repositoryUrl });
|
||||
await authorizeSelection(source, [], context);
|
||||
return db.transaction(async tx => {
|
||||
await tx.select().from(sources).where(scope(companyId, id)).for('update');
|
||||
await tx.update(sources).set({ enabled: false, revision: sql`${sources.revision} + 1`, leaseToken: null, leaseExpiresAt: null }).where(scope(companyId, id));
|
||||
for (const entry of (await detail(companyId, id, tx)).entries) {
|
||||
if (!entry.skillId) continue;
|
||||
const skill = await skills.getById(companyId, entry.skillId, tx);
|
||||
if (skill) await tx.update(companySkills).set({ metadata: { ...skill.metadata, skillSourceId: id, skillSourceState: 'not_syncing' } }).where(and(eq(companySkills.companyId, companyId), eq(companySkills.id, skill.id)));
|
||||
}
|
||||
await context.audit(tx, id, 'company.skill_source_disconnected', {});
|
||||
return detail(companyId, id, tx);
|
||||
});
|
||||
const skillIds = [...new Set(source.entries.flatMap(entry => entry.skillId ? [entry.skillId] : []))];
|
||||
for (const skillId of skillIds) {
|
||||
const skill = await skills.getById(companyId, skillId);
|
||||
if (!skill) continue;
|
||||
const resource = { sourceType: 'git' as const, sourceLocator: source.repositoryUrl, skillId, skillKey: skill.key };
|
||||
await context.authorize(input.skills === 'delete' ? 'skills.remove' : 'skills.edit', resource);
|
||||
if (input.skills === 'copy') await context.authorize('skills.create', resource);
|
||||
}
|
||||
async function lock(tx: Tx) {
|
||||
const [locked] = await tx.select().from(sources).where(scope(companyId, id)).for('update');
|
||||
if (!locked || locked.revision !== input.revision) throw conflict('This source changed. Reload before disconnecting.');
|
||||
}
|
||||
async function remove(tx: Tx, copiedCount: number, deletedCount: number) {
|
||||
await tx.delete(sources).where(scope(companyId, id));
|
||||
await context.audit(tx, id, 'company.skill_source_disconnected', { skills: input.skills, skillIds, copiedCount, deletedCount });
|
||||
}
|
||||
if (input.skills === 'delete') {
|
||||
const deleted = await skills.deleteSkills(companyId, skillIds, (tx, deleted) => remove(tx, 0, deleted.length), lock);
|
||||
return { id, copiedCount: 0, deletedCount: deleted.length };
|
||||
}
|
||||
let folderId: string | null = null;
|
||||
if (skillIds.length && context.actor.type === 'user' && context.actor.userId) {
|
||||
const [user] = await db.select({ name: authUsers.name }).from(authUsers).where(eq(authUsers.id, context.actor.userId));
|
||||
folderId = (await folderService(db).ensureMyFolder(companyId, context.actor.userId, user?.name ?? null)).id;
|
||||
}
|
||||
const directories: string[] = [];
|
||||
try {
|
||||
return await db.transaction(async tx => {
|
||||
await lock(tx);
|
||||
for (const skillId of skillIds) await skills.makeImportedSkillEditable(companyId, skillId, context.actor, folderId, tx, directories);
|
||||
await remove(tx, skillIds.length, 0);
|
||||
return { id, copiedCount: skillIds.length, deletedCount: 0 };
|
||||
});
|
||||
} catch (error) {
|
||||
await Promise.all(directories.map(directory => fs.rm(directory, { recursive: true, force: true })));
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
async function importFromUrl(companyId: string, input: string, context: SkillSourceContext) {
|
||||
const parsed = parseSkillImportSourceInput(input);
|
||||
|
||||
@@ -53,4 +53,14 @@ describe('live skill source discovery', () => {
|
||||
await expect(skillSourcesApi.discoverStream('company-1', input, updates, controller.signal)).rejects.toMatchObject({ name: 'AbortError' });
|
||||
expect(fetchMock.mock.calls[0]![1].signal).toBe(controller.signal);
|
||||
});
|
||||
it('sends the required disposition and source revision with the delete request', async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue(Response.json({ id: 'source-1', copiedCount: 0, deletedCount: 2 }));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
await skillSourcesApi.disconnect('company-1', 'source-1', { revision: 7, skills: 'delete' });
|
||||
const [url, init] = fetchMock.mock.calls[0]!;
|
||||
expect(url).toBe('/api/companies/company-1/skill-sources/source-1');
|
||||
expect(init.method).toBe('DELETE');
|
||||
expect(JSON.parse(init.body)).toEqual({ revision: 7, skills: 'delete' });
|
||||
});
|
||||
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { ProjectRepositoryOptions, SkillSourceFilePreview, SkillSourcePreviewRequest, SkillSource, SkillSourceCreateRequest, SkillSourceDiscovery, SkillSourceDiscoveryRequest, SkillSourceRefreshResult, SkillSourceSelectionRequest, SkillSourceScanUpdate, SkillSourceDiscoveryEvent } from '@paperclipai/shared';
|
||||
import type { ProjectRepositoryOptions, SkillSourceDisconnectRequest, SkillSourceDisconnectResult, SkillSourceFilePreview, SkillSourcePreviewRequest, SkillSource, SkillSourceCreateRequest, SkillSourceDiscovery, SkillSourceDiscoveryRequest, SkillSourceRefreshResult, SkillSourceSelectionRequest, SkillSourceScanUpdate, SkillSourceDiscoveryEvent } from '@paperclipai/shared';
|
||||
import { api, ApiError, requestResponse } from './client';
|
||||
const base = (companyId: string) => `/companies/${companyId}/skill-sources`;
|
||||
export const skillSourcesApi = {
|
||||
@@ -41,5 +41,5 @@ export const skillSourcesApi = {
|
||||
create: (companyId: string, input: SkillSourceCreateRequest) => api.post<SkillSourceRefreshResult>(base(companyId), input),
|
||||
select: (companyId: string, id: string, input: SkillSourceSelectionRequest) => api.patch<SkillSourceRefreshResult>(`${base(companyId)}/${id}`, input),
|
||||
refresh: (companyId: string, id: string) => api.post<SkillSourceRefreshResult>(`${base(companyId)}/${id}/refresh`, {}),
|
||||
disconnect: (companyId: string, id: string) => api.delete<SkillSource>(`${base(companyId)}/${id}`),
|
||||
disconnect: (companyId: string, id: string, input: SkillSourceDisconnectRequest) => api.delete<SkillSourceDisconnectResult>(`${base(companyId)}/${id}`, input),
|
||||
};
|
||||
@@ -97,7 +97,7 @@ export function RadioCardGroup({
|
||||
const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
|
||||
if (disabled) return;
|
||||
const idx = options.findIndex((option) => option.value === value);
|
||||
if (idx === -1) return;
|
||||
|
||||
let step: number | null = null;
|
||||
if (event.key === "ArrowDown" || event.key === "ArrowRight") {
|
||||
step = 1;
|
||||
@@ -110,9 +110,11 @@ export function RadioCardGroup({
|
||||
// choice the viewer cannot make would select it.
|
||||
const len = options.length;
|
||||
for (let hop = 1; hop <= len; hop++) {
|
||||
const candidate = options[(((idx + step * hop) % len) + len) % len];
|
||||
const next = idx === -1 ? (step === 1 ? hop - 1 : len - hop) : (((idx + step * hop) % len) + len) % len;
|
||||
const candidate = options[next];
|
||||
if (!candidate.disabled) {
|
||||
onValueChange(candidate.value);
|
||||
event.currentTarget.querySelectorAll<HTMLButtonElement>('[role="radio"]')[next]?.focus();
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -135,7 +137,7 @@ export function RadioCardGroup({
|
||||
tooltip={option.tooltip}
|
||||
aria-label={option.accessibleLabel}
|
||||
disabled={disabled || option.disabled}
|
||||
tabIndex={option.value === value ? 0 : -1}
|
||||
tabIndex={option.value === value || (!options.some(item => item.value === value && !item.disabled) && option === options.find(item => !item.disabled)) ? 0 : -1}
|
||||
onClick={() => onValueChange(option.value)}
|
||||
/>
|
||||
))}
|
||||
|
||||
@@ -5,6 +5,8 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { SkillSources } from './SkillSources';
|
||||
import { skillSourcesApi } from '@/api/skillSources';
|
||||
import { ApiError } from '@/api/client';
|
||||
import { sourceFixture } from '../../storybook/fixtures/githubSkillSources';
|
||||
|
||||
const context = vi.hoisted(() => ({ navigate: vi.fn(), breadcrumbs: vi.fn(), sourceId: 'new' }));
|
||||
vi.mock('@/context/CompanyContext', () => ({ useCompany: () => ({ selectedCompanyId: 'company-1', selectedCompany: { name: 'Acme' } }) }));
|
||||
@@ -241,4 +243,73 @@ describe('GitHub skill source import', () => {
|
||||
expect(document.body.textContent).not.toContain('Include repository files');
|
||||
});
|
||||
|
||||
async function openRemoval(disconnected = false) {
|
||||
context.sourceId = '';
|
||||
const source = { ...sourceFixture(), companyId: 'company-1', enabled: !disconnected };
|
||||
vi.mocked(skillSourcesApi.list).mockResolvedValue([source]);
|
||||
await mount();
|
||||
const trigger = document.querySelector<HTMLButtonElement>(`button[aria-label="More actions for ${source.fullName}"]`)!;
|
||||
await act(async () => trigger.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })));
|
||||
await flush();
|
||||
const menu = [...document.querySelectorAll<HTMLElement>('[role="menuitem"]')].find(el => el.textContent === (disconnected ? 'Remove source' : 'Disconnect source'))!;
|
||||
await act(async () => menu.click()); await flush();
|
||||
return source;
|
||||
}
|
||||
it('requires a keep-or-delete choice and cancellation leaves the source intact', async () => {
|
||||
await openRemoval();
|
||||
expect(button('Disconnect source').disabled).toBe(true);
|
||||
expect(document.querySelector('[role="radio"][tabindex="0"]')?.textContent).toContain('Keep editable copies');
|
||||
expect(skillSourcesApi.disconnect).not.toHaveBeenCalled();
|
||||
await act(async () => button('Cancel').click());
|
||||
expect(document.querySelector('[role="dialog"]')).toBeNull();
|
||||
expect(document.body.textContent).toContain('acme/team-skills');
|
||||
});
|
||||
it('removes an already-disconnected source and sends the explicit delete choice and revision', async () => {
|
||||
const source = await openRemoval(true);
|
||||
vi.mocked(skillSourcesApi.disconnect).mockImplementation(async () => {
|
||||
vi.mocked(skillSourcesApi.list).mockResolvedValue([]);
|
||||
return { id: source.id, copiedCount: 0, deletedCount: 2 };
|
||||
});
|
||||
const choice = [...document.querySelectorAll<HTMLButtonElement>('[role="radio"]')].find(el => el.textContent?.startsWith('Delete skills'))!;
|
||||
await act(async () => choice.click());
|
||||
await act(async () => button('Remove source').click()); await flush(); await flush();
|
||||
expect(skillSourcesApi.disconnect).toHaveBeenCalledWith('company-1', source.id, { revision: source.revision, skills: 'delete' });
|
||||
expect(document.querySelector('[role="dialog"]')).toBeNull();
|
||||
expect(document.body.textContent).not.toContain('acme/team-skills');
|
||||
});
|
||||
it('reloads a stale confirmation and requires a new choice for the updated skill count', async () => {
|
||||
const source = await openRemoval();
|
||||
vi.mocked(skillSourcesApi.disconnect).mockRejectedValueOnce(new ApiError('Source changed. Reload before disconnecting.', 409, {}));
|
||||
await act(async () => document.querySelector<HTMLButtonElement>('[role="radio"]')!.click());
|
||||
await act(async () => button('Disconnect source').click()); await flush();
|
||||
expect(button('Disconnect source').disabled).toBe(true);
|
||||
vi.mocked(skillSourcesApi.list).mockResolvedValue([{ ...source, revision: source.revision + 1, entries: source.entries.slice(0, 1) }]);
|
||||
await act(async () => button('Reload source').click()); await flush(); await flush();
|
||||
expect(document.querySelector('[role="dialog"]')?.textContent).toContain('1 imported skill');
|
||||
expect(document.querySelector('[role="dialog"] [role="alert"]')).toBeNull();
|
||||
expect(button('Disconnect source').disabled).toBe(true);
|
||||
await act(async () => document.querySelector<HTMLButtonElement>('[role="radio"]')!.click());
|
||||
vi.mocked(skillSourcesApi.disconnect).mockImplementation(async () => {
|
||||
vi.mocked(skillSourcesApi.list).mockResolvedValue([]);
|
||||
return { id: source.id, copiedCount: 1, deletedCount: 0 };
|
||||
});
|
||||
await act(async () => button('Disconnect source').click()); await flush(); await flush();
|
||||
expect(skillSourcesApi.disconnect).toHaveBeenLastCalledWith('company-1', source.id, { revision: source.revision + 1, skills: 'copy' });
|
||||
expect(document.querySelector('[role="dialog"]')).toBeNull();
|
||||
});
|
||||
it('sends the keep-copy choice and keeps failures in the confirmation for retry', async () => {
|
||||
const source = await openRemoval();
|
||||
vi.mocked(skillSourcesApi.disconnect).mockRejectedValue(new Error('Source changed. Reload before disconnecting.'));
|
||||
const choice = [...document.querySelectorAll<HTMLButtonElement>('[role="radio"]')].find(el => el.textContent?.startsWith('Keep editable copies'))!;
|
||||
await act(async () => choice.click());
|
||||
await act(async () => button('Disconnect source').click()); await flush();
|
||||
expect(skillSourcesApi.disconnect).toHaveBeenCalledWith('company-1', source.id, { revision: source.revision, skills: 'copy' });
|
||||
expect(document.querySelector('[role="dialog"] [role="alert"]')?.textContent).toContain('Source changed');
|
||||
expect(button('Disconnect source').disabled).toBe(false);
|
||||
expect(document.querySelector('section[aria-label="acme/team-skills"]')).not.toBeNull();
|
||||
const other = [...document.querySelectorAll<HTMLButtonElement>('[role="radio"]')].find(el => el.textContent?.startsWith('Delete skills'))!;
|
||||
await act(async () => other.click());
|
||||
expect(document.querySelector('[role="dialog"] [role="alert"]')).toBeNull();
|
||||
});
|
||||
|
||||
});
|
||||
@@ -2,16 +2,18 @@ import { GithubIcon } from "@/components/icons/github-icon";
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
|
||||
import { RefreshCw, Plus, ExternalLink, Check, Lock, GitBranch, FileText, MoreVertical } from 'lucide-react';
|
||||
import { parseGitHubSkillRepositoryUrl, type SkillSource, type SkillSourceDiscovery, type SkillSourceRefreshResult, type SkillSourceScanProgress } from '@paperclipai/shared';
|
||||
import { parseGitHubSkillRepositoryUrl, type SkillSourceDisconnectRequest, type SkillSource, type SkillSourceDiscovery, type SkillSourceRefreshResult, type SkillSourceScanProgress } from '@paperclipai/shared';
|
||||
import { Link, useNavigate, useParams } from '@/lib/router';
|
||||
import { useCompany } from '@/context/CompanyContext';
|
||||
import { useBreadcrumbs } from '@/context/BreadcrumbContext';
|
||||
import { queryKeys } from '@/lib/queryKeys';
|
||||
import { skillSourcesApi } from '@/api/skillSources';
|
||||
import { ApiError } from '@/api/client';
|
||||
import { Command, CommandInput, CommandList, CommandEmpty, CommandGroup, CommandItem } from '@/components/ui/command';
|
||||
import { appSourceConnectHref } from './apps/app-connect-policy';
|
||||
import { Button } from '@/components/ui/button';
|
||||
import { Input } from '@/components/ui/input';
|
||||
import { RadioCardGroup } from '@/components/ui/radio-card';
|
||||
import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog';
|
||||
import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from '@/components/ui/dropdown-menu';
|
||||
import { SkillImportProgress, type FoundSkill } from './skills/SkillImportProgress';
|
||||
@@ -29,6 +31,7 @@ export function SkillSources() {
|
||||
const navigate = useNavigate();
|
||||
const client = useQueryClient();
|
||||
const { setBreadcrumbs } = useBreadcrumbs();
|
||||
const [disconnectSource, setDisconnectSource] = useState<SkillSource | null>(null);
|
||||
const [results, setResults] = useState<Record<string, string>>({});
|
||||
const companyId = selectedCompanyId ?? '';
|
||||
useEffect(() => { setBreadcrumbs([{ label: 'Skills', href: '/skills' }, { label: 'Sources' }]); }, [setBreadcrumbs]);
|
||||
@@ -39,7 +42,24 @@ export function SkillSources() {
|
||||
const refresh = useMutation({ mutationFn: (id: string) => skillSourcesApi.refresh(companyId, id), onSuccess: async result => {
|
||||
setResults(prev => ({ ...prev, [result.source.id]: '' })); await invalidate();
|
||||
}, onError: (error, id) => { setResults(prev => ({ ...prev, [id]: error.message })); void invalidate(); } });
|
||||
const disconnect = useMutation({ mutationFn: (id: string) => skillSourcesApi.disconnect(companyId, id), onSuccess: invalidate });
|
||||
const disconnect = useMutation({
|
||||
mutationFn: ({ source, skills }: { source: SkillSource; skills: SkillSourceDisconnectRequest['skills'] }) =>
|
||||
skillSourcesApi.disconnect(companyId, source.id, { revision: source.revision, skills }),
|
||||
onSuccess: async result => {
|
||||
client.setQueryData<SkillSource[]>(sourceKey(companyId), previous => previous?.filter(source => source.id !== result.id));
|
||||
await invalidate();
|
||||
await client.invalidateQueries({ queryKey: queryKeys.folders.list(companyId, 'skill') });
|
||||
setDisconnectSource(null);
|
||||
},
|
||||
});
|
||||
const reloadRemoval = useMutation({
|
||||
mutationFn: async () => {
|
||||
const result = await query.refetch({ throwOnError: true });
|
||||
return result.data?.find(source => source.id === disconnectSource?.id) ?? null;
|
||||
},
|
||||
onSuccess: source => { setDisconnectSource(source); disconnect.reset(); },
|
||||
});
|
||||
const staleRemoval = disconnect.error instanceof ApiError && disconnect.error.status === 409;
|
||||
const activeSource = query.data?.find(source => source.id === sourceId);
|
||||
if (!companyId) return <p className="p-6 text-sm text-muted-foreground">Select a company to manage skill sources.</p>;
|
||||
return <div className="mx-auto flex w-full max-w-5xl flex-col gap-6 p-4 md:p-6">
|
||||
@@ -50,7 +70,6 @@ export function SkillSources() {
|
||||
<Link to="/skills" className="text-sm text-muted-foreground hover:text-foreground">Installed skills</Link>
|
||||
{query.isPending && <p role="status" className="text-sm text-muted-foreground">Loading sources…</p>}
|
||||
{query.error && <p role="alert" className="text-sm text-destructive">{query.error.message} <Button variant="ghost" size="sm" onClick={() => void query.refetch()}>Try again</Button></p>}
|
||||
{disconnect.error && <p role="alert" className="text-sm text-destructive">{disconnect.error.message}</p>}
|
||||
{query.data?.length === 0 && <div className="flex flex-col items-start gap-3 py-8"><p className="text-sm text-muted-foreground">No repositories added yet. Import your skills to make them available in {selectedCompany?.name ?? 'this company'}.</p><Button variant="outline" onClick={() => navigate('/skills/sources/new')}>Import from GitHub</Button></div>}
|
||||
<div className="divide-y divide-border">
|
||||
{query.data?.map(source => {
|
||||
@@ -71,7 +90,7 @@ export function SkillSources() {
|
||||
<DropdownMenuContent align="end">
|
||||
<DropdownMenuItem disabled={!source.enabled || refresh.isPending} onSelect={() => refresh.mutate(source.id)}>{refresh.isPending && refresh.variables === source.id ? 'Refreshing…' : 'Refresh'}</DropdownMenuItem>
|
||||
<DropdownMenuItem onSelect={() => navigate(`/skills/sources/${source.id}`)}>Select skills</DropdownMenuItem>
|
||||
<DropdownMenuItem variant="destructive" disabled={!source.enabled || disconnect.isPending} onSelect={() => disconnect.mutate(source.id)}>Disconnect source</DropdownMenuItem>
|
||||
<DropdownMenuItem variant="destructive" disabled={disconnect.isPending} onSelect={() => { disconnect.reset(); reloadRemoval.reset(); setDisconnectSource(source); }}>{source.enabled ? 'Disconnect source' : 'Remove source'}</DropdownMenuItem>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</div>
|
||||
@@ -92,6 +111,11 @@ export function SkillSources() {
|
||||
</section>;
|
||||
})}
|
||||
</div>
|
||||
{disconnectSource && <DisconnectSourceDialog key={`${disconnectSource.id}:${disconnectSource.revision}`} source={disconnectSource}
|
||||
pending={disconnect.isPending || reloadRemoval.isPending} error={reloadRemoval.error?.message ?? disconnect.error?.message}
|
||||
stale={staleRemoval} onReload={() => reloadRemoval.mutate()} onChoiceChange={() => { if (!staleRemoval) disconnect.reset(); }}
|
||||
onClose={() => setDisconnectSource(null)}
|
||||
onConfirm={skills => disconnect.mutate({ source: disconnectSource, skills })} />}
|
||||
{sourceId && (sourceId === 'new' || activeSource) && <SourceDialog key={`${companyId}:${sourceId}`} companyId={companyId} source={activeSource} onClose={() => navigate('/skills/sources')} onSaved={async result => {
|
||||
setResults(prev => ({ ...prev, [result.source.id]: '' })); await invalidate(); navigate('/skills/sources');
|
||||
}} />}
|
||||
@@ -99,6 +123,39 @@ export function SkillSources() {
|
||||
</div>;
|
||||
}
|
||||
|
||||
function DisconnectSourceDialog({ source, pending, error, stale, onReload, onClose, onConfirm, onChoiceChange }: {
|
||||
source: SkillSource; pending: boolean; error?: string; stale: boolean; onReload: () => void; onClose: () => void;
|
||||
onConfirm: (skills: SkillSourceDisconnectRequest['skills']) => void; onChoiceChange: () => void;
|
||||
}) {
|
||||
const count = new Set(source.entries.flatMap(entry => entry.skillId ? [entry.skillId] : [])).size;
|
||||
const [choice, setChoice] = useState<SkillSourceDisconnectRequest['skills'] | ''>('');
|
||||
const action = source.enabled ? 'Disconnect source' : 'Remove source';
|
||||
return <Dialog open onOpenChange={open => { if (!open && !pending) onClose(); }}>
|
||||
<DialogContent>
|
||||
<DialogHeader>
|
||||
<DialogTitle>{source.enabled ? 'Disconnect' : 'Remove'} {source.fullName}?</DialogTitle>
|
||||
<DialogDescription>{count ? `Remove this repository from Sources. What should happen to its ${count} imported ${count === 1 ? 'skill' : 'skills'}?` : 'Remove this repository from Sources.'}</DialogDescription>
|
||||
</DialogHeader>
|
||||
{count > 0 && <RadioCardGroup ariaLabel="Imported skills" value={choice}
|
||||
onValueChange={value => { setChoice(value as SkillSourceDisconnectRequest['skills']); onChoiceChange(); }} disabled={pending}
|
||||
options={[
|
||||
{ value: 'copy', title: 'Keep editable copies', description: 'Move them to My Skills. Assigned agents keep using them.' },
|
||||
{ value: 'delete', title: 'Delete skills', description: 'Remove them from the library. Detach skills used by agents first.' },
|
||||
]} />}
|
||||
|
||||
{error && <div role="alert" className="text-sm text-destructive">{error}
|
||||
{stale && <Button variant="ghost" size="sm" disabled={pending} onClick={onReload}>Reload source</Button>}
|
||||
</div>}
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<Button variant="outline" disabled={pending} onClick={onClose}>Cancel</Button>
|
||||
<Button variant={choice === 'delete' ? 'destructive' : 'default'} disabled={pending || stale || Boolean(count && !choice)} onClick={() => onConfirm(choice || 'copy')}>
|
||||
{pending ? 'Removing…' : action}
|
||||
</Button>
|
||||
</div>
|
||||
</DialogContent>
|
||||
</Dialog>;
|
||||
}
|
||||
|
||||
function SourceDialog({ companyId, source, onClose, onSaved }: {
|
||||
companyId: string; source?: SkillSource; onClose: () => void; onSaved: (result: SkillSourceRefreshResult) => Promise<void>;
|
||||
}) {
|
||||
|
||||
@@ -91,7 +91,7 @@ export function importedSkill(source: SkillSource, entry: SkillSourceEntry): Com
|
||||
export type RepositoryScenario = 'single' | 'multiple' | 'none' | 'empty' | 'partial-error' | 'error' | 'loading';
|
||||
|
||||
/** Scoped API fixtures keep the production page interactive without contacting GitHub. */
|
||||
export function installFixtures(empty: boolean, needsConnection: boolean, options: { packageScenario?: PackageScenario; journey?: boolean; refreshed?: boolean; assigned?: boolean; repositories?: RepositoryScenario; scan?: "live" | "large" | "interrupted"; saving?: boolean } = {}) {
|
||||
export function installFixtures(empty: boolean, needsConnection: boolean, options: { packageScenario?: PackageScenario; journey?: boolean; refreshed?: boolean; assigned?: boolean; repositories?: RepositoryScenario; scan?: "live" | "large" | "interrupted"; saving?: boolean; disconnected?: boolean } = {}) {
|
||||
const fixtureController = new AbortController();
|
||||
const wait = (ms: number, signal = fixtureController.signal) => new Promise<void>((resolve, reject) => {
|
||||
signal.throwIfAborted();
|
||||
@@ -284,10 +284,12 @@ export function installFixtures(empty: boolean, needsConnection: boolean, option
|
||||
source.lastSuccessAt = new Date();
|
||||
return structuredClone({ source, imported: [], updated: [], unchanged: source.entries.filter(entry => entry.selection === "selected").length, warnings: [] });
|
||||
};
|
||||
skillSourcesApi.disconnect = async (_companyId, id) => {
|
||||
if (options.disconnected) sources.forEach(source => { source.enabled = false; });
|
||||
skillSourcesApi.disconnect = async (_companyId, id, input) => {
|
||||
const source = sources.find(item => item.id === id)!;
|
||||
source.enabled = false;
|
||||
return structuredClone(source);
|
||||
const count = source.entries.filter(entry => entry.skillId).length;
|
||||
sources.splice(sources.findIndex(item => item.id === id), 1);
|
||||
return { id, copiedCount: input.skills === 'copy' ? count : 0, deletedCount: input.skills === 'delete' ? count : 0 };
|
||||
};
|
||||
return () => {
|
||||
fixtureController.abort();
|
||||
|
||||
@@ -5,13 +5,13 @@ import { COMPANY_ID, SOURCE_ID, installFixtures, type RepositoryScenario } from
|
||||
import { Link, Route, Routes, useNavigate } from "@/lib/router";
|
||||
import { SkillSources } from "@/pages/SkillSources";
|
||||
|
||||
function SourcesStory({ view = "sources", empty = false, needsConnection = false, repositories = "single", repositoryUrl = "", scan }: {
|
||||
view?: "sources" | "import" | "manage"; empty?: boolean; needsConnection?: boolean; repositories?: RepositoryScenario; repositoryUrl?: string; scan?: "live" | "large" | "interrupted";
|
||||
function SourcesStory({ view = "sources", empty = false, needsConnection = false, repositories = "single", repositoryUrl = "", scan, disconnected = false }: {
|
||||
disconnected?: boolean; view?: "sources" | "import" | "manage"; empty?: boolean; needsConnection?: boolean; repositories?: RepositoryScenario; repositoryUrl?: string; scan?: "live" | "large" | "interrupted";
|
||||
}) {
|
||||
const [ready, setReady] = useState(false);
|
||||
const navigate = useNavigate();
|
||||
useEffect(() => {
|
||||
const cleanup = installFixtures(empty, needsConnection, { repositories, scan });
|
||||
const cleanup = installFixtures(empty, needsConnection, { repositories, scan, disconnected });
|
||||
for (const id of ["new", SOURCE_ID]) sessionStorage.removeItem(`paperclip.skill-source-draft:${COMPANY_ID}:${id}`);
|
||||
if (repositoryUrl) sessionStorage.setItem(`paperclip.skill-source-draft:${COMPANY_ID}:new`, JSON.stringify({ repositoryUrl }));
|
||||
navigate(`/skills/sources${view === "import" ? "/new" : view === "manage" ? `/${SOURCE_ID}` : ""}`, { replace: true });
|
||||
@@ -19,7 +19,7 @@ function SourcesStory({ view = "sources", empty = false, needsConnection = false
|
||||
return cleanup;
|
||||
// Install each story's fixtures only once; route changes belong to the user.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [empty, needsConnection, view, repositories, repositoryUrl, scan]);
|
||||
}, [empty, needsConnection, view, repositories, repositoryUrl, scan, disconnected]);
|
||||
if (!ready) return null;
|
||||
return <Routes>
|
||||
<Route path="/:companyPrefix/skills/sources/:sourceId" element={<SkillSources />} />
|
||||
@@ -98,3 +98,21 @@ export const LargeRepository: Story = {
|
||||
export const InterruptedScan: Story = {
|
||||
name: 'Import · Interrupted scan', args: { view: 'import', empty: true, repositoryUrl: 'https://github.com/acme/team-skills', scan: 'interrupted' }, play: startScan,
|
||||
};
|
||||
|
||||
export const DisconnectSource: Story = {
|
||||
name: 'Sources · Disconnect source',
|
||||
play: async ({ canvasElement }) => {
|
||||
const page = within(canvasElement.ownerDocument.body);
|
||||
await userEvent.click(await page.findByRole('button', { name: 'More actions for acme/team-skills' }));
|
||||
await userEvent.click(await page.findByRole('menuitem', { name: 'Disconnect source' }));
|
||||
},
|
||||
};
|
||||
export const RemoveDisconnectedSource: Story = {
|
||||
name: 'Sources · Remove disconnected source', args: { disconnected: true },
|
||||
play: async ({ canvasElement }) => {
|
||||
const page = within(canvasElement.ownerDocument.body);
|
||||
await userEvent.click(await page.findByRole('button', { name: 'More actions for acme/team-skills' }));
|
||||
await userEvent.click(await page.findByRole('menuitem', { name: 'Remove source' }));
|
||||
await userEvent.click(await page.findByRole('radio', { name: /^Delete skills/ }));
|
||||
},
|
||||
};
|
||||
Reference in new issue
Block a user