From df4ee06e2f613649f8331cc18a1b587e6146f8a8 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:55:43 -0500 Subject: [PATCH] feat(skills): select and validate tracked GitHub branches (#16288) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Skills can track a branch in an authorized GitHub repository. > - Sources previously offered only a default-branch repair action. > - People need to select a listed branch or enter a branch that GitHub has not listed. > - The server must validate the selected ref with the caller's authorized connection. > - This pull request connects the approved branch picker to production APIs. > - Installed skills remain usable while the source changes its tracked ref. ## Linked Issues or Issue Description **Subsystem affected** Skills UI, shared API contracts, GitHub source services, and agent API discovery. **Problem or motivation** The default-branch repair action does not let people choose another branch. The settings service also validates the old ref when a caller supplies a new ref other than HEAD. A missing tracked branch can prevent connection repair even when the selected account has repository access. **Proposed solution** Load a bounded branch list with the authorized connection. Mark the default branch. Search listed branches or accept a typed ref. Validate the selected ref before saving. Keep the chosen value after a load error, invalid ref, or concurrent update. Save the selected connection and branch together during connection repair. **Alternatives considered** A default-branch button does not support a named branch. A text field alone does not help people find the repository default or available branches. **Roadmap alignment** This improves the existing Skills source manager. It does not add a new skill import system. **Additional context** Refs #16066 and #16256. This is the production implementation of the approved branch-selection Storybook journeys. ## What Changed - Add an authorized branch-list endpoint and shared response/query types. Register it in agent API discovery. - Validate the requested ref, including the returned commit SHA. Preserve source leases, revision checks, project ownership, skill identities, selections, and installed versions. - Add the searchable branch picker to Sources and connection repair. Allow typed refs during branch discovery and after discovery fails. - Keep save errors in the dialog. Retain the selected branch through a concurrent source update. - Use the production components and API client in Storybook. Add keyboard, touch, loading, retry, invalid-ref, connection-repair, and concurrent-update browser journeys. - Update product documentation and the feature/access maps. ## Verification - Red/green: two new database regressions fail against the previous service and pass against the implementation. - Linux database/service suite: 85 tests passed. This includes source visibility, human/agent authorization, selected connections, immutable versions, and invalid refs. - Rebased branch and review fixes: 94 focused UI, route, branch-validation, and fixture tests passed. Two conflict-message regressions and the stale-retry fixture regression fail before the fix and pass after it. - Storybook build passed. All 26 focused browser journeys passed. - Repository typecheck and build passed. Final UI typecheck passed after the review fixes. - The full local Vitest run was interrupted when the checkout and its dependencies disappeared. It reported missing working-directory/Postgres files and worker startup errors. Its two timed-out tests pass after the checkout was restored. The complete CI suite passed on the final head, `cf5d4c8f0a`: [CI run](https://github.com/paperclipai/paperclip/actions/runs/38571068332). One unchanged workspace timestamp test required a single rerun. Its release-minus-one-millisecond value can equal the original confirmation time. The rerun passed. - Live acceptance passed on implementation head `5dd935bf14` in an isolated macOS arm64 instance with Node 24.21.0 and an authorized private GitHub repository. The UI loaded the actual default branch, saved branch and selected connection together, and preserved the installed skill/version IDs after reload. An invalid typed ref retained the draft and showed the provider error. A valid typed HEAD ref then saved successfully. The owned runtime and database were stopped after the test. Focused unit, Linux database, and browser suites passed again after the error-handling review fixes. - Greptile reviewed final head `cf5d4c8f0a` at 5/5. Both review threads are resolved; the fresh review has no remaining findings. - Token gates and map/catalog checks passed. - Review entry points: `Skills / Project repository skills / 03 Branch selection components` and `04 Branch selection journeys`. Run `pnpm storybook` to inspect them. - Browser command: `pnpm exec playwright test --config tests/storybook-visual/playwright.config.ts tests/storybook-visual/skill-source-branch-selection.spec.ts tests/storybook-visual/project-repository-skills.spec.ts`. - Manual check: open an independent GitHub source, choose a new branch, select the default or type a ref, save, and reload. Try an invalid ref and confirm the selection remains in the dialog. For a project-bound source, change its ref in project settings. ## Risks - GitHub discovery is bounded to 100 listed branches. The default branch is always included. Typed refs remain available. - A source can change while the dialog is open. The API rejects a stale revision and the UI keeps the draft for retry. - A ref change schedules the existing refresh pipeline. Installed versions remain available until a successful refresh imports new content. - No database migration or credential storage change is required. Existing HEAD requests remain supported. ## Model Used OpenAI Codex, GPT-6 family. The serving model's exact identifier and context-window size are not exposed in this session. Used reasoning, repository tools, code execution, and browser tools. ## 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 --- doc/DATABASE.md | 8 +- doc/PRODUCT.md | 5 + maps/agent-access-map/skills.md | 8 +- maps/feature-map/skills.md | 7 +- packages/shared/src/types/skill-source.ts | 11 +- .../shared/src/validators/skill-source.ts | 3 +- .../__tests__/company-skills-routes.test.ts | 15 +- .../__tests__/project-skill-sources.test.ts | 6 + .../__tests__/skill-source-branches.test.ts | 36 ++++ server/src/__tests__/skill-sources.test.ts | 50 ++++++ server/src/routes/company-skills.ts | 11 +- server/src/routes/openapi.ts | 5 +- server/src/services/skill-source-branches.ts | 19 +++ server/src/services/skill-sources.ts | 23 ++- .../project-repository-skills.spec.ts | 7 +- .../skill-source-branch-selection.spec.ts | 160 ++++++++++++++++++ ui/src/api/skillSources.ts | 2 + ui/src/components/RepositoryBranchSelect.tsx | 54 ++++++ .../skill-studio/SkillSourceBranchRepair.tsx | 36 ++++ .../SkillSourceBranchRepairDialog.tsx | 47 +++++ .../SkillSourceConnectionRepair.tsx | 15 +- ui/src/pages/SkillSources.test.tsx | 40 ++++- ui/src/pages/SkillSources.tsx | 12 +- .../project-repository-skills/README.md | 11 ++ .../branch-selection-components.stories.tsx | 31 ++++ .../branch-selection-journeys.stories.tsx | 52 ++++++ .../support/projectRepositorySkills.ts | 9 +- .../skillSourceBranchSelection.test.ts | 24 +++ .../support/skillSourceBranchSelection.ts | 57 +++++++ 29 files changed, 722 insertions(+), 42 deletions(-) create mode 100644 server/src/__tests__/skill-source-branches.test.ts create mode 100644 server/src/services/skill-source-branches.ts create mode 100644 tests/storybook-visual/skill-source-branch-selection.spec.ts create mode 100644 ui/src/components/RepositoryBranchSelect.tsx create mode 100644 ui/src/components/skill-studio/SkillSourceBranchRepair.tsx create mode 100644 ui/src/components/skill-studio/SkillSourceBranchRepairDialog.tsx create mode 100644 ui/storybook/stories/skills/project-repository-skills/branch-selection-components.stories.tsx create mode 100644 ui/storybook/stories/skills/project-repository-skills/branch-selection-journeys.stories.tsx create mode 100644 ui/storybook/support/skillSourceBranchSelection.test.ts create mode 100644 ui/storybook/support/skillSourceBranchSelection.ts diff --git a/doc/DATABASE.md b/doc/DATABASE.md index b3d09d77103..74400ba9625 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -814,11 +814,15 @@ source identity even if it currently resolves to that default branch. API-key agents without an authenticated run may attach repositories; their durable import jobs have no setup person and require adoption by an authorized person. -Independent legacy sources with a missing branch can explicitly switch to `HEAD` -through source settings. Validate the repository identity and default-branch read +Independent sources can change their tracked branch or revision through source +settings, including `HEAD` to follow the default branch. Validate the repository +identity and the requested commit read before changing the ref; preserve installed skill IDs, versions and selection. Active project bindings must change their ref through the project configuration. `HEAD` remains the default-branch alias after refresh, including for independent sources. +Branch changes use the source revision and lease fence, reject duplicate repository/ref +sources, keep installed version rows unchanged, and enqueue the normal refresh path. +Branch listing uses the caller's authorized connection, not the stored setup identity. ### Task workspace records (2026-10-09) diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index f6cbbb3a35f..0ed960f9926 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -106,6 +106,11 @@ using the setup person's authorized GitHub connection. New skills import automat unless excluded. Existing projects can adopt their repositories from Skills → Sources; existing manually imported sources preserve their selections and gain automatic updates. Refresh remains available, and operators can pause automatic updates. +Independent skill sources have a searchable branch picker with the repository's +default branch marked. People can also enter a branch name that is not listed. +Saving checks GitHub access and the selected ref before changing tracking; failed +saves keep the selection and installed content. Project refs remain owned by project +configuration so changing a source cannot redirect another project's tracking. If a tracked repository grows beyond the supported package size, its installed repository skills and shared dependencies remain available until a safe refresh can succeed. Initial oversized imports can use self-contained skill packages; diff --git a/maps/agent-access-map/skills.md b/maps/agent-access-map/skills.md index 15f2d881b80..eac0b245e01 100644 --- a/maps/agent-access-map/skills.md +++ b/maps/agent-access-map/skills.md @@ -146,8 +146,12 @@ admitted skill-test JWT scope. Project-source API additions: GET `/api/companies/{companyId}/skill-sources/project-repositories`, POST `/api/companies/{companyId}/skill-sources/adopt-project` with `workspaceId`, and PATCH `/api/companies/{companyId}/skill-sources/{sourceId}/settings` with expected revision -and update/connection preferences. `trackingRef: "HEAD"` explicitly restores an independent -source's default-branch tracking; active project bindings must be edited through their +and update/connection preferences. `trackingRef` accepts a branch or explicit ref; +`"HEAD"` keeps default-branch tracking. GET `/api/companies/{companyId}/skill-sources/{sourceId}/branches` +returns a bounded branch list and the default branch, with optional `connectionId` for +connection repair. It requires source access and `skills.edit`, and reads only through +the caller's authorized GitHub grant. Saving validates the requested ref before changing +an independent source; active project bindings must be edited through their project configuration. Responses identify branch, connection, or retry repair and distinguish queued imports from imports under an active worker lease. Native API discovery uses the OpenAPI registration. diff --git a/maps/feature-map/skills.md b/maps/feature-map/skills.md index 77d3433a604..902649f3781 100644 --- a/maps/feature-map/skills.md +++ b/maps/feature-map/skills.md @@ -81,8 +81,11 @@ must be verified separately; inspect resolved version IDs across an upstream upd ### Connection repair during an update A missing tracked branch is a branch repair, not a credential failure. On an independent -source, choose Use default branch explicitly; installed identities, selections and version -pins remain. Project-tracked branches are changed in project settings. A saved connection +source, choose a new branch from the searchable list or enter its name. The repository's +default branch is marked and included even outside the first 100 listed branches. +Loading or a failed list request does not prevent manual entry; saving validates the +requested ref. Installed identities, selections and version pins remain. Project-tracked +branches are changed in project settings. A saved connection is validated against both the repository and tracked revision before repair succeeds. Open Choose connection while a source is being refreshed. Select an authorized account and save. If the source changes, the dialog must retain that selection, load the current revision, and allow a second save without reopening. The [project repository browser suite](../../tests/storybook-visual/project-repository-skills.spec.ts) covers this recovery with simulated upstream data. diff --git a/packages/shared/src/types/skill-source.ts b/packages/shared/src/types/skill-source.ts index 7714ff33ac8..f92df553fb8 100644 --- a/packages/shared/src/types/skill-source.ts +++ b/packages/shared/src/types/skill-source.ts @@ -86,8 +86,15 @@ export interface SkillSourceSettingsRequest { automaticUpdates?: boolean; importNewSkills?: boolean; connectionId?: string | null; - /** Explicitly resume following the default branch; existing version pins remain intact. */ - trackingRef?: 'HEAD'; + /** Follow this branch/ref, or HEAD for the default branch. Version pins remain intact. */ + trackingRef?: string; +} +export interface SkillSourceBranchesQuery { connectionId?: string } +export interface SkillSourceBranches { + defaultBranch: string; + branches: Array<{ name: string; isDefault: boolean }>; + /** The bounded list may omit branches; an unlisted name can still be entered. */ + truncated: boolean; } export interface ProjectSkillRepository { projectId: string; diff --git a/packages/shared/src/validators/skill-source.ts b/packages/shared/src/validators/skill-source.ts index a1355aae3f0..94e418769a4 100644 --- a/packages/shared/src/validators/skill-source.ts +++ b/packages/shared/src/validators/skill-source.ts @@ -30,8 +30,9 @@ export const skillSourceSettingsSchema = z.object({ automaticUpdates: z.boolean().optional(), importNewSkills: z.boolean().optional(), connectionId: z.string().uuid().nullable().optional(), - trackingRef: z.literal('HEAD').optional(), + trackingRef: z.string().trim().min(1).max(255).refine(value => !/[\x00-\x1f\x7f]/.test(value), 'Enter a valid branch or revision.').optional(), }); +export const skillSourceBranchesQuerySchema = z.object({ connectionId: z.string().uuid().optional() }); export const projectSkillSourceAdoptSchema = z.object({ workspaceId: z.string().uuid(), }); diff --git a/server/src/__tests__/company-skills-routes.test.ts b/server/src/__tests__/company-skills-routes.test.ts index 18e395ec232..f40bfa0f8ce 100644 --- a/server/src/__tests__/company-skills-routes.test.ts +++ b/server/src/__tests__/company-skills-routes.test.ts @@ -5,7 +5,7 @@ import { hoistModuleGraph } from "./helpers/hoist-module-graph.js"; const mockCaptureRunIdentity = vi.hoisted(() => vi.fn()); -const mockSkillSourceService = vi.hoisted(() => ({ sourceForSkill: vi.fn(), importFromUrl: vi.fn(), discover: vi.fn() })); +const mockSkillSourceService = vi.hoisted(() => ({ sourceForSkill: vi.fn(), importFromUrl: vi.fn(), discover: vi.fn(), branches: vi.fn() })); const mockAgentService = vi.hoisted(() => ({ getById: vi.fn(), @@ -1344,6 +1344,17 @@ describe("company skill mutation permissions", () => { } }); + it('lists authorized source branches with a validated connection override', async () => { + const app = createApp({ type: 'board', userId: 'member', companyIds: ['company-1'], source: 'session' }); + const connectionId = '00000000-0000-4000-8000-000000000042'; + const data = { defaultBranch: 'master', branches: [{ name: 'master', isDefault: true }], truncated: false }; + mockSkillSourceService.branches.mockResolvedValue(data); + const path = '/api/companies/company-1/skill-sources/source-id/branches'; + expect((await request(app).get(path).query({ connectionId }).expect(200)).body).toEqual(data); + expect(mockSkillSourceService.branches).toHaveBeenCalledWith('company-1', 'source-id', { connectionId }, expect.objectContaining({ read: expect.any(Function) }), expect.any(AbortSignal)); + await request(app).get(path).query({ connectionId: 'invalid' }).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" }, @@ -1352,7 +1363,7 @@ describe("company skill mutation permissions", () => { const app = createApp(actor); const base = "/api/companies/company-2/skill-sources"; const responses = await Promise.all([ - request(app).get(base), request(app).get(`${base}/repositories`), request(app).get(`${base}/source-id`), + request(app).get(base), request(app).get(`${base}/repositories`), request(app).get(`${base}/source-id`), request(app).get(`${base}/source-id/branches`), request(app).post(`${base}/discover`).send({ repositoryUrl: "https://github.com/acme/skills" }), request(app).post(`${base}/discover`).set('Accept', 'application/x-ndjson').send({ repositoryUrl: "https://github.com/acme/skills" }), request(app).post(`${base}/preview`).send({ repositoryUrl: "https://github.com/acme/skills", commitSha: "a".repeat(40), skillPath: "SKILL.md", filePath: "SKILL.md" }), diff --git a/server/src/__tests__/project-skill-sources.test.ts b/server/src/__tests__/project-skill-sources.test.ts index 9f0c2fa60ef..ddc9e2573c7 100644 --- a/server/src/__tests__/project-skill-sources.test.ts +++ b/server/src/__tests__/project-skill-sources.test.ts @@ -466,6 +466,9 @@ describe.skipIf(!support.supported)('automatic project skill sources', () => { await expect(companySkillService(db).createLocalSkill(companyId, { name: 'Unsafe publication', forkedFromSkillId: skillId, sharingScope: 'public_link' })).rejects.toThrow(/public link/); }); await withSkillActor(actor('bob'), async () => { + const branchRead = vi.fn(context.read(null)); + await expect(skillSourceService(db).branches(companyId, source.id, {}, { ...context, read: () => branchRead })).rejects.toThrow(/not found/); + expect(branchRead).not.toHaveBeenCalled(); await expect(companySkillService(db).importFromSource(companyId, privateSnapshotPath)).rejects.toThrow(/not found/); expect(await companySkillService(db).getById(companyId, skillId)).toBeNull(); expect(await companySkillService(db).readFile(companyId, skillId, 'SKILL.md')).toBeNull(); @@ -487,6 +490,9 @@ describe.skipIf(!support.supported)('automatic project skill sources', () => { }); for (const denied of [agentActor(reader!.id, 'bob'), agentActor(outsider!.id, 'alice'), { ...agentActor(reader!.id, 'alice'), companyId: randomUUID() }]) { await withSkillActor(denied, async () => { + const branchRead = vi.fn(context.read(null)); + await expect(skillSourceService(db).branches(companyId, source.id, {}, { ...context, read: () => branchRead })).rejects.toThrow(/not found/); + expect(branchRead).not.toHaveBeenCalled(); expect(await companySkillService(db).readFile(companyId, skillId, 'SKILL.md')).toBeNull(); expect((await companySkillService(db).listRuntimeSkillEntries(companyId)).some(entry => entry.key.includes('private'))).toBe(false); }); diff --git a/server/src/__tests__/skill-source-branches.test.ts b/server/src/__tests__/skill-source-branches.test.ts new file mode 100644 index 00000000000..3555b205fdc --- /dev/null +++ b/server/src/__tests__/skill-source-branches.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it, vi } from 'vitest'; +import { skillSourceSettingsSchema } from '@paperclipai/shared'; +import { listSkillSourceBranches } from '../services/skill-source-branches.js'; + +const source = { fullName: 'acme/skills', repositoryId: '42' }; +function reader(branches: unknown, defaultBranch = 'master', id = 42) { + return vi.fn(async (request: string) => request === '/repos/acme/skills' ? { id, default_branch: defaultBranch } : branches); +} +describe('skill source branch discovery', () => { + it('includes the default even when it is outside the bounded first page', async () => { + const read = reader(Array.from({ length: 100 }, (_, index) => ({ name: `feature/${index}` }))); + const result = await listSkillSourceBranches(source, read); + expect(result.branches).toHaveLength(101); + expect(result.branches[0]).toEqual({ name: 'master', isDefault: true }); + expect(result.truncated).toBe(true); + expect(read).toHaveBeenLastCalledWith('/repos/acme/skills/branches?per_page=100&page=1', undefined); + }); + it('deduplicates branch names and preserves case and slashes', async () => { + expect(await listSkillSourceBranches(source, reader([{ name: 'master' }, { name: 'feature/New' }, { name: 'feature/New' }]))).toEqual({ + defaultBranch: 'master', branches: [{ name: 'master', isDefault: true }, { name: 'feature/New', isDefault: false }], truncated: false, + }); + }); + it('stops before listing when the repository identity changed', async () => { + const read = reader([], 'master', 99); + await expect(listSkillSourceBranches(source, read)).rejects.toMatchObject({ status: 409 }); + expect(read).toHaveBeenCalledTimes(1); + }); + it.each([{ value: null }, { value: {} }, { value: [{ name: 42 }] }, { value: [{ name: 'bad\0branch' }] }])('rejects malformed provider lists without returning their contents', async ({ value }) => { + await expect(listSkillSourceBranches(source, reader(value))).rejects.toMatchObject({ status: 422, message: 'GitHub returned an invalid branch list. Try again.' }); + }); + it('accepts a typed ref and the existing HEAD alias but rejects empty and control-character values', () => { + expect(skillSourceSettingsSchema.parse({ revision: 2, trackingRef: ' feature/New ' }).trackingRef).toBe('feature/New'); + expect(skillSourceSettingsSchema.parse({ revision: 2, trackingRef: 'HEAD' }).trackingRef).toBe('HEAD'); + for (const trackingRef of ['', ' ', 'bad\0ref', 'bad\nref']) expect(skillSourceSettingsSchema.safeParse({ revision: 2, trackingRef }).success).toBe(false); + }); +}); diff --git a/server/src/__tests__/skill-sources.test.ts b/server/src/__tests__/skill-sources.test.ts index 026ce8519cb..da9906625c9 100644 --- a/server/src/__tests__/skill-sources.test.ts +++ b/server/src/__tests__/skill-sources.test.ts @@ -61,6 +61,56 @@ describe.skipIf(!support.supported)('skill source persistence', () => { expect(refreshed.source.trackingRef).toBe('HEAD'); expect(refreshed.source.entries[0]?.skillId).toBe(created.source.entries[0]?.skillId); }); + it('validates the selected branch and retains installed content until a successful refresh', async () => { + const isolatedCompany = randomUUID(); + await db.insert(companies).values({ id: isolatedCompany, name: 'Selected branch', issuePrefix: 'SBR' }); + const service = skillSourceService(db); + const created = await service.create(isolatedCompany, { repositoryUrl: 'https://github.com/acme/skills', trackingRef: 'main', commitSha: sha, selectedPaths: ['deep/one/SKILL.md'], excludedFolders: ['elsewhere'] }, context); + const versions = await db.select().from(companySkillVersions).where(eq(companySkillVersions.companyId, isolatedCompany)); + const read = vi.fn(githubFixture(initial)); + const repaired = await service.settings(isolatedCompany, created.source.id, { revision: created.source.revision, trackingRef: 'feature/New' }, { ...context, read: () => read }); + expect(read).toHaveBeenCalledWith('/repos/acme/skills/commits/feature%2FNew'); + expect(repaired.trackingRef).toBe('feature/New'); + expect(repaired.entries).toEqual(created.source.entries); + expect(repaired.excludedFolders).toEqual(['elsewhere']); + expect(repaired.lastScanCommit).toBe(created.source.lastScanCommit); + await expect(service.settings(isolatedCompany, created.source.id, { revision: created.source.revision, automaticUpdates: false }, context)) + .rejects.toMatchObject({ status: 409, details: { code: 'skill_source_settings_changed' } }); + expect(await db.select().from(companySkillVersions).where(eq(companySkillVersions.companyId, isolatedCompany))).toEqual(versions); + const changed = githubFixture({ 'deep/one/SKILL.md': md('new branch version') }, {}, 'f'.repeat(40)); + await service.refresh(isolatedCompany, repaired.id, { ...context, read: () => changed }); + const skillId = repaired.entries.find(entry => entry.skillId)!.skillId!; + const current = await companySkillService(db).getById(isolatedCompany, skillId); + expect(current!.sourceRef).toBe('f'.repeat(40)); + expect(current!.currentVersionId).not.toBe(versions[0]!.id); + expect(await companySkillService(db).getVersion(isolatedCompany, skillId, versions[0]!.id)).not.toBeNull(); + }); + it('rejects an unavailable selected branch without changing the source or recording a successful audit', async () => { + const isolatedCompany = randomUUID(); + await db.insert(companies).values({ id: isolatedCompany, name: 'Unavailable branch', issuePrefix: 'UBR' }); + const service = skillSourceService(db); + const created = await service.create(isolatedCompany, { repositoryUrl: 'https://github.com/acme/skills', trackingRef: 'main', commitSha: sha, selectedPaths: ['deep/one/SKILL.md'] }, context); + const original = githubFixture(initial), audit = vi.fn(); + const read = async (request: string) => { + if (request.endsWith('/commits/feature%2Fmissing')) throw unprocessable('Unavailable branch.', { code: 'skill_source_ref_unavailable' }); + if (request.endsWith('/commits/..')) return { id: 42, default_branch: 'main' }; + return original(request); + }; + for (const trackingRef of ['feature/missing', '..']) await expect(service.settings(isolatedCompany, created.source.id, { revision: created.source.revision, trackingRef }, { ...context, audit, read: () => read })).rejects.toMatchObject({ details: { code: 'skill_source_ref_unavailable' } }); + expect(await service.detail(isolatedCompany, created.source.id)).toEqual(created.source); + expect(audit).not.toHaveBeenCalled(); + }); + it('requires source edit authorization before reading branches and uses only the requested connection', async () => { + const service = skillSourceService(db); + const created = await service.create(companyId, { repositoryUrl: 'https://github.com/acme/skills', trackingRef: 'branch-list', commitSha: sha, selectedPaths: [] }, context); + const read = vi.fn(async (request: string) => request === '/repos/acme/skills' ? { id: 42, default_branch: 'main' } : [{ name: 'main' }]); + const connection = randomUUID(), choose = vi.fn(() => read); + const result = await service.branches(companyId, created.source.id, { connectionId: connection }, { ...context, read: choose }); + expect(result.defaultBranch).toBe('main'); expect(choose).toHaveBeenCalledWith(connection); + read.mockClear(); + await expect(service.branches(companyId, created.source.id, {}, { ...context, read: choose, authorize: async () => { throw unprocessable('Editing denied.'); } })).rejects.toThrow('Editing denied'); + expect(read).not.toHaveBeenCalled(); + }); it('checks a new commit with an unchanged tree without downloading or replacing versions', async () => { const originalFiles = files, originalCommit = commit; try { diff --git a/server/src/routes/company-skills.ts b/server/src/routes/company-skills.ts index a5f78735051..93a396fcb9c 100644 --- a/server/src/routes/company-skills.ts +++ b/server/src/routes/company-skills.ts @@ -4,7 +4,7 @@ import { skillAccessRequestContext, skillAccessService } from "../services/skill 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, skillSourceSettingsSchema, projectSkillSourceAdoptSchema, skillSourceDisconnectSchema } from "@paperclipai/shared"; +import { skillSourceCreateSchema, skillSourceDiscoverySchema, skillSourcePreviewSchema, skillSourceSelectionSchema, skillSourceSettingsSchema, skillSourceBranchesQuerySchema, projectSkillSourceAdoptSchema, skillSourceDisconnectSchema } from "@paperclipai/shared"; import type { ActivityPublication } from "../services/activity-log.js"; import { once } from "node:events"; import type { SkillSourceDiscoveryEvent } from "@paperclipai/shared"; @@ -347,6 +347,15 @@ export function companySkillRoutes(db: Db) { const companyId = String(req.params.companyId); res.json(await sourceOperation(req, companyId, context => sourceSvc.settings(companyId, String(req.params.sourceId), req.body, context))); }); + router.get("/companies/:companyId/skill-sources/:sourceId/branches", async (req, res) => { + const companyId = String(req.params.companyId); + const input = skillSourceBranchesQuerySchema.parse({ connectionId: firstQueryString(req.query.connectionId) }); + const controller = new AbortController(); + const stop = () => controller.abort(); + res.once("close", stop); + try { res.json(await sourceOperation(req, companyId, context => sourceSvc.branches(companyId, String(req.params.sourceId), input, context, controller.signal))); } + finally { res.off("close", stop); } + }); router.get("/companies/:companyId/skill-sources/repositories", async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 1f6a9f8d414..d84b733334a 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -174,6 +174,7 @@ import { skillSourceCreateSchema, skillSourceSelectionSchema, skillSourceSettingsSchema, + skillSourceBranchesQuerySchema, projectSkillSourceAdoptSchema, companySkillCreateSchema, companySkillFileDeleteSchema, @@ -12452,6 +12453,7 @@ for (const [method, path, summary] of [ ["get", "/api/companies/{companyId}/skill-sources/project-repositories", "List accessible project repositories and skill import status"], ["post", "/api/companies/{companyId}/skill-sources/adopt-project", "Import all skills from an accessible project repository"], ["patch", "/api/companies/{companyId}/skill-sources/{sourceId}/settings", "Set automatic skill updates and authorized refresh connection"], + ["get", "/api/companies/{companyId}/skill-sources/{sourceId}/branches", "List authorized repository branches, including the default branch"], ["get", "/api/companies/{companyId}/skill-sources/repositories", "Browse authorized GitHub repositories for skills"], ["get", "/api/companies/{companyId}/skill-sources/{sourceId}", "Get a skill source and entries"], ["post", "/api/companies/{companyId}/skill-sources/discover", "Discover and validate repository skills"], @@ -12462,7 +12464,8 @@ for (const [method, path, summary] of [ ["delete", "/api/companies/{companyId}/skill-sources/{sourceId}", "Disconnect a skill source and retain installed skills"], ] as const) registerCurrentRoute({ method, path, tags: ["skills"], summary, - ...(method === "patch" && path.endsWith("/settings") ? { body: skillSourceSettingsSchema } + ...(method === "get" && path.endsWith("/branches") ? { query: skillSourceBranchesQuerySchema } + : method === "patch" && path.endsWith("/settings") ? { body: skillSourceSettingsSchema } : method === "post" && path.endsWith("/adopt-project") ? { body: projectSkillSourceAdoptSchema } : method === "patch" ? { body: skillSourceSelectionSchema } : method === "post" && path.endsWith("/discover") ? { body: skillSourceDiscoverySchema } diff --git a/server/src/services/skill-source-branches.ts b/server/src/services/skill-source-branches.ts new file mode 100644 index 00000000000..8b53b99014b --- /dev/null +++ b/server/src/services/skill-source-branches.ts @@ -0,0 +1,19 @@ +import type { SkillSource, SkillSourceBranches } from '@paperclipai/shared'; +import { conflict, unprocessable } from '../errors.js'; +import type { GitHubRead } from './github-skill-source.js'; + +const LIMIT = 100; +function branchName(value: unknown): value is string { + return typeof value === 'string' && value.length > 0 && value.length <= 255 && !/[\x00-\x1f\x7f]/.test(value); +} + +/** One bounded GitHub list request. The repository default is always included. */ +export async function listSkillSourceBranches(source: Pick, read: GitHubRead, signal?: AbortSignal): Promise { + const repository = await read(`/repos/${source.fullName}`, signal) as { id: number; default_branch: unknown }; + if (source.repositoryId && String(repository.id) !== source.repositoryId) throw conflict('The repository at this URL has changed identity. Add it as a new source.'); + if (!branchName(repository.default_branch)) throw unprocessable('GitHub returned an invalid default branch. Try again.'); + const result = await read(`/repos/${source.fullName}/branches?per_page=${LIMIT}&page=1`, signal); + if (!Array.isArray(result) || result.some(branch => !branch || !branchName(branch.name))) throw unprocessable('GitHub returned an invalid branch list. Try again.'); + const names = new Set([repository.default_branch, ...result.slice(0, LIMIT).map(branch => branch.name as string)]); + return { defaultBranch: repository.default_branch, branches: [...names].map(name => ({ name, isDefault: name === repository.default_branch })), truncated: result.length >= LIMIT }; +} diff --git a/server/src/services/skill-sources.ts b/server/src/services/skill-sources.ts index 92a1143b2cd..ae1c17bd080 100644 --- a/server/src/services/skill-sources.ts +++ b/server/src/services/skill-sources.ts @@ -1,5 +1,6 @@ import { reserveAutomaticSkillWork, settleAutomaticSkillWork } from './skill-source-refresh-budget.js'; import { skillSourceRepairAction } from './skill-source-problems.js'; +import { listSkillSourceBranches } from './skill-source-branches.js'; import { createHash, randomUUID } from 'node:crypto'; import { skillSourceRetryAt } from './skill-source-retry.js'; import path from 'node:path'; @@ -7,7 +8,7 @@ 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, projectSkillSources, projects, type Db, authUsers } from '@paperclipai/db'; -import type { CompanySkill, SkillSource, SkillSourceCreateRequest, SkillSourceDiscoveryRequest, SkillSourceSelectionRequest, SkillSourceRefreshResult, SkillSourcePreviewRequest, SkillSourceSettingsRequest, SkillSourceDisconnectRequest, SkillSourceDisconnectResult } from '@paperclipai/shared'; +import type { CompanySkill, SkillSource, SkillSourceCreateRequest, SkillSourceDiscoveryRequest, SkillSourceSelectionRequest, SkillSourceRefreshResult, SkillSourcePreviewRequest, SkillSourceSettingsRequest, SkillSourceDisconnectRequest, SkillSourceDisconnectResult, SkillSourceBranchesQuery } from '@paperclipai/shared'; import { normalizeAgentUrlKey } from '@paperclipai/shared'; import { conflict, notFound, unprocessable } from '../errors.js'; import { companySkillService, parseSkillImportSourceInput } from './company-skills.js'; @@ -295,7 +296,14 @@ export function skillSourceService(db: Db) { throw error; } } + async function branches(companyId: string, id: string, input: SkillSourceBranchesQuery, context: SkillSourceContext, signal?: AbortSignal) { + await skillAccessService(db).assertSource(companyId, id); + const source = await row(companyId, id); + await context.authorize('skills.edit', { sourceType: 'git', sourceLocator: source.repositoryUrl }); + return listSkillSourceBranches(source, context.read(input.connectionId ?? source.connectionId), signal); + } async function settings(companyId: string, id: string, input: SkillSourceSettingsRequest, context: SkillSourceContext) { + await skillAccessService(db).assertSource(companyId, id); const source = await row(companyId, id); await context.authorize('skills.edit', { sourceType: 'git', sourceLocator: source.repositoryUrl }); // Validate a replacement connection before binding unattended work to it. @@ -309,8 +317,11 @@ export function skillSourceService(db: Db) { reader = context.read(input.connectionId !== undefined ? input.connectionId : source.connectionId); const repository = await reader(`/repos/${source.fullName}`) as { id: number; default_branch: string }; if (source.repositoryId && String(repository.id) !== source.repositoryId) throw conflict('The repository at this URL has changed identity. Add it as a new source.'); - const ref = (input.trackingRef ?? source.trackingRef) === 'HEAD' ? repository.default_branch : source.trackingRef; - await reader(`/repos/${source.fullName}/commits/${encodeURIComponent(ref)}`); + const requestedRef = input.trackingRef ?? source.trackingRef; + const ref = requestedRef === 'HEAD' ? repository.default_branch : requestedRef; + if (typeof ref !== 'string' || !ref) throw unprocessable('The selected branch or revision is unavailable.', { code: 'skill_source_ref_unavailable' }); + const commit = await reader(`/repos/${source.fullName}/commits/${encodeURIComponent(ref)}`) as { sha?: unknown } | null; + if (!commit || typeof commit.sha !== 'string' || !/^[a-f0-9]{40}$/i.test(commit.sha)) throw unprocessable('The selected branch or revision is unavailable.', { code: 'skill_source_ref_unavailable' }); } return db.transaction(async tx => { if (input.trackingRef !== undefined) { @@ -321,7 +332,7 @@ export function skillSourceService(db: Db) { if (binding) throw unprocessable('Update the repository branch in its project settings.'); const [duplicate] = await tx.select({ id: sources.id }).from(sources).where(and( eq(sources.companyId, companyId), eq(sources.repositoryUrl, source.repositoryUrl), eq(sources.trackingRef, input.trackingRef), isNull(sources.removedAt), sql`${sources.id} <> ${id}`)); - if (duplicate) throw conflict('This repository’s default branch is already in Sources. Use that source; installed skills were kept.'); + if (duplicate) throw conflict('This repository and branch are already in Sources. Use that source; installed skills were kept.'); } const [updated] = await tx.update(sources).set({ ...(input.automaticUpdates !== undefined ? { automaticUpdates: input.automaticUpdates } : {}), @@ -332,7 +343,7 @@ export function skillSourceService(db: Db) { ...(input.trackingRef !== undefined ? { trackingRef: input.trackingRef, lastError: null, failureCount: 0 } : {}), revision: sql`${sources.revision} + 1`, nextSyncAt: new Date(), }).where(and(scope(companyId, id), eq(sources.revision, input.revision), or(isNull(sources.leaseToken), lt(sources.leaseExpiresAt, new Date())))).returning(); - if (!updated) throw conflict('This source is refreshing or changed. Reload before saving.'); + if (!updated) throw conflict('This source is refreshing or changed. Reload before saving.', { code: 'skill_source_settings_changed' }); await context.audit(tx, id, 'company.skill_source_settings_updated', { automaticUpdates: updated.automaticUpdates, importNewSkills: updated.importNewSkills, connectionChanged: input.connectionId !== undefined, ...(input.trackingRef !== undefined ? { previousRef: source.trackingRef, trackingRef: updated.trackingRef } : {}) }); return detail(companyId, id, tx); @@ -435,5 +446,5 @@ export function skillSourceService(db: Db) { const imported = await Promise.all(result.source.entries.filter(entry => selectedPaths.includes(entry.path) && entry.skillId && !entry.error).map(entry => skills.getById(companyId, entry.skillId!))); return { imported: imported.filter((skill): skill is CompanySkill => Boolean(skill)), warnings: result.warnings }; } - return { list, detail, sourceForSkill, discover, preview, create, refresh, settings, disconnect, importFromUrl }; + return { list, detail, sourceForSkill, discover, preview, create, refresh, branches, settings, disconnect, importFromUrl }; } diff --git a/tests/storybook-visual/project-repository-skills.spec.ts b/tests/storybook-visual/project-repository-skills.spec.ts index 9e14a3826b1..01b4a56270c 100644 --- a/tests/storybook-visual/project-repository-skills.spec.ts +++ b/tests/storybook-visual/project-repository-skills.spec.ts @@ -15,9 +15,12 @@ test('groups matching project repositories and keeps progress visible through ad test('repairs a missing branch without prompting for another connection', async ({ page }) => { await page.goto(story('missing-branch')); await expect(page.getByRole('button', { name: 'Choose connection', exact: true })).toHaveCount(0); - await page.getByRole('button', { name: 'Use default branch', exact: true }).click(); + await page.getByRole('button', { name: 'Choose a new branch', exact: true }).click(); + await page.getByRole('combobox', { name: 'Branch', exact: true }).click(); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await page.getByRole('button', { name: 'Change branch', exact: true }).click(); await expect(page.getByRole('alert')).toHaveCount(0); - await expect(page.getByText(/Default branch · Updated/)).toBeVisible(); + await expect(page.getByText(/master · Updated/)).toBeVisible(); await expect(page.getByRole('region', { name: 'acme/team-skills', exact: true }).getByText('Code review', { exact: true })).toBeVisible(); await expect(page.getByRole('button', { name: 'More actions for acme/team-skills', exact: true })).toBeFocused(); }); diff --git a/tests/storybook-visual/skill-source-branch-selection.spec.ts b/tests/storybook-visual/skill-source-branch-selection.spec.ts new file mode 100644 index 00000000000..f10ef740ac4 --- /dev/null +++ b/tests/storybook-visual/skill-source-branch-selection.spec.ts @@ -0,0 +1,160 @@ +import { expect, test, type Page } from '@playwright/test'; + +const story = (name = 'choose-branch', theme = 'dark') => `/iframe.html?id=skills-project-repository-skills-04-branch-selection-journeys--${name}&viewMode=story&globals=theme:${theme}`; + +async function openRepair(page: Page) { + await page.getByRole('button', { name: 'Choose a new branch', exact: true }).click(); + const dialog = page.getByRole('dialog', { name: 'Choose a new branch', exact: true }); + await expect(dialog).toBeVisible(); + return dialog; +} + +async function search(page: Page, name: string) { + const picker = page.getByRole('combobox', { name: 'Branch', exact: true }); + if (await picker.getAttribute('aria-expanded') !== 'true') await picker.click(); + await page.getByPlaceholder('Search or enter a branch name…', { exact: true }).fill(name); +} + +test('marks the default branch and saves a searched listed branch without replacing skills', async ({ page }) => { + await page.goto(story()); + const source = page.getByRole('region', { name: 'acme/team-skills', exact: true }); + const skill = source.getByRole('link', { name: /^Code review / }); + const skillHref = await skill.getAttribute('href'); + const dialog = await openRepair(page); + await expect(dialog.getByRole('button', { name: 'Change branch', exact: true })).toBeDisabled(); + await search(page, ''); + await expect(page.getByRole('option', { name: 'master Default', exact: true })).toBeVisible(); + await search(page, 'release'); + await page.getByRole('option', { name: 'release/2026.10', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog.getByRole('button', { name: 'Changing branch…', exact: true })).toBeDisabled(); + await expect(dialog).toHaveCount(0); + await expect(source.getByText(/release\/2026\.10 · Updated/)).toBeVisible(); + await expect(skill).toHaveAttribute('href', skillHref!); + await expect(page.getByRole('button', { name: 'More actions for acme/team-skills', exact: true })).toBeFocused(); +}); + +test('allows an unlisted branch while discovery is still loading', async ({ page }) => { + await page.goto(story('slow-branch-list')); + const dialog = await openRepair(page); + await expect(dialog.getByRole('status')).toContainText('Loading branches'); + await search(page, 'feature/not-in-the-list'); + await page.getByRole('option', { name: 'Use “feature/not-in-the-list”', exact: true }).click(); + await expect(dialog.getByRole('combobox', { name: 'Branch', exact: true })).toContainText('feature/not-in-the-list'); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/feature\/not-in-the-list · Updated/)).toBeVisible(); +}); + +test('retains manual entry when discovery fails', async ({ page }) => { + await page.goto(story('branch-list-unavailable')); + const dialog = await openRepair(page); + await expect(dialog.getByRole('alert')).toContainText('You can still enter a branch name'); + await search(page, 'private/release'); + await page.getByRole('option', { name: 'Use “private/release”', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/private\/release · Updated/)).toBeVisible(); +}); + +test('retries discovery and loads the default branch', async ({ page }) => { + await page.goto(story('branch-list-unavailable')); + const dialog = await openRepair(page); + await expect(dialog.getByRole('alert')).toBeVisible(); + await dialog.getByRole('button', { name: 'Try again', exact: true }).click(); + await expect(dialog.getByRole('alert')).toHaveCount(0); + await search(page, 'master'); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(page.getByText(/master · Updated/)).toBeVisible(); +}); + +test('keeps an invalid selection and permits correction after a save error', async ({ page }) => { + await page.goto(story('validation-failure')); + const dialog = await openRepair(page); + await search(page, 'feature/missing'); + await page.getByRole('option', { name: 'Use “feature/missing”', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog.getByRole('alert')).toContainText('GitHub could not find that branch or revision'); + await expect(dialog.getByRole('combobox', { name: 'Branch', exact: true })).toContainText('feature/missing'); + await expect(page.getByText(/main · Updated/)).toBeVisible(); + await search(page, 'master'); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await expect(dialog.getByRole('alert')).toHaveCount(0); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/master · Updated/)).toBeVisible(); +}); + +test('keyboard input accepts an unlisted branch and Escape closes only the picker', async ({ page }) => { + await page.goto(story('empty-branch-list')); + const dialog = await openRepair(page); + await search(page, 'feature/keyboard'); + const input = page.getByPlaceholder('Search or enter a branch name…', { exact: true }); + await input.press('Escape'); + await expect(dialog).toBeVisible(); + await search(page, 'feature/keyboard'); + await input.press('ArrowDown'); + await input.press('Enter'); + await expect(dialog.getByRole('combobox', { name: 'Branch', exact: true })).toContainText('feature/keyboard'); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).press('Enter'); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/feature\/keyboard · Updated/)).toBeVisible(); +}); + +test('cancel preserves the tracked branch and clears the next draft', async ({ page }) => { + await page.goto(story()); + const dialog = await openRepair(page); + await search(page, 'master'); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await dialog.getByRole('button', { name: 'Cancel', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/main · Updated/)).toBeVisible(); + await openRepair(page); + await expect(dialog.getByRole('combobox', { name: 'Branch', exact: true })).toContainText('Choose a branch'); + await expect(dialog.getByRole('button', { name: 'Change branch', exact: true })).toBeDisabled(); +}); + +test('retains a selection across a concurrent source update and retries with the new revision', async ({ page }) => { + await page.goto(story('save-conflict')); + const dialog = await openRepair(page); + await search(page, 'master'); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog.getByRole('alert')).toContainText('Your branch is still selected'); + await expect(dialog.getByRole('combobox', { name: 'Branch', exact: true })).toContainText('master'); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(/master · Updated/)).toBeVisible(); +}); + +test('uses the selected connection when repairing an unavailable branch', async ({ page }) => { + await page.goto(story('connection-repair')); + await page.getByRole('button', { name: 'Choose connection', exact: true }).click(); + await page.getByRole('button', { name: 'Save connection', exact: true }).click(); + await page.getByRole('button', { name: 'Choose a new branch', exact: true }).click(); + const dialog = page.getByRole('dialog', { name: 'Choose a new branch', exact: true }); + await search(page, 'master'); + await page.getByRole('option', { name: 'master Default', exact: true }).click(); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByRole('dialog', { name: 'Choose GitHub connection', exact: true })).toHaveCount(0); + await expect(page.getByText(/master · Updated/)).toBeVisible(); +}); + +test.describe('mobile', () => { + test.use({ viewport: { width: 390, height: 844 }, hasTouch: true }); + test('selects a long typed branch using touch without horizontal overflow', async ({ page }) => { + await page.goto(story('choose-branch', 'light')); + await page.getByRole('button', { name: 'Choose a new branch', exact: true }).tap(); + const dialog = page.getByRole('dialog', { name: 'Choose a new branch', exact: true }); + const branch = 'feature/platform-services-new-skill-refresh-and-accessibility'; + await search(page, branch); + await page.getByRole('option', { name: `Use “${branch}”`, exact: true }).tap(); + await expect(dialog.getByRole('button', { name: 'Change branch', exact: true })).toBeInViewport(); + expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true); + await dialog.getByRole('button', { name: 'Change branch', exact: true }).tap(); + await expect(dialog).toHaveCount(0); + await expect(page.getByText(new RegExp(`${branch} · Updated`))).toBeVisible(); + }); +}); diff --git a/ui/src/api/skillSources.ts b/ui/src/api/skillSources.ts index 1b04a2563fc..d541520eef2 100644 --- a/ui/src/api/skillSources.ts +++ b/ui/src/api/skillSources.ts @@ -1,11 +1,13 @@ import type { ProjectRepositoryOptions, ProjectSkillRepository, SkillSourceSettingsRequest, SkillSourceFilePreview, SkillSourcePreviewRequest, SkillSource, SkillSourceCreateRequest, SkillSourceDiscovery, SkillSourceDiscoveryRequest, SkillSourceRefreshResult, SkillSourceSelectionRequest, SkillSourceScanUpdate, SkillSourceDiscoveryEvent, SkillSourceDisconnectRequest, SkillSourceDisconnectResult } from '@paperclipai/shared'; import { api, ApiError, requestResponse } from './client'; +import type { SkillSourceBranches, SkillSourceBranchesQuery } from '@paperclipai/shared'; const base = (companyId: string) => `/companies/${companyId}/skill-sources`; export const skillSourcesApi = { list: (companyId: string) => api.get(base(companyId)), projectRepositories: (companyId: string) => api.get(`${base(companyId)}/project-repositories`), adoptProject: (companyId: string, workspaceId: string) => api.post<{ bindingId: string }>(`${base(companyId)}/adopt-project`, { workspaceId }), settings: (companyId: string, id: string, input: SkillSourceSettingsRequest) => api.patch(`${base(companyId)}/${id}/settings`, input), + branches: (companyId: string, id: string, input: SkillSourceBranchesQuery = {}, signal?: AbortSignal) => api.get(`${base(companyId)}/${id}/branches${input.connectionId ? `?connectionId=${encodeURIComponent(input.connectionId)}` : ''}`, { signal }), repositories: (companyId: string) => api.get(`${base(companyId)}/repositories`), discover: (companyId: string, input: SkillSourceDiscoveryRequest) => api.post(`${base(companyId)}/discover`, input), discoverStream: async (companyId: string, input: SkillSourceDiscoveryRequest, onProgress: (event: SkillSourceScanUpdate) => void, signal?: AbortSignal): Promise => { diff --git a/ui/src/components/RepositoryBranchSelect.tsx b/ui/src/components/RepositoryBranchSelect.tsx new file mode 100644 index 00000000000..239f52fab58 --- /dev/null +++ b/ui/src/components/RepositoryBranchSelect.tsx @@ -0,0 +1,54 @@ +import { GitBranch, LoaderCircle } from 'lucide-react'; +import { SearchableSelect, type SearchableSelectGroup, type SearchableSelectOption } from '@/components/SearchableSelect'; +import { Button } from '@/components/ui/button'; + +export interface RepositoryBranch { + name: string; + isDefault?: boolean; +} + +interface BranchOption extends SearchableSelectOption { + isDefault?: boolean; + entered?: boolean; +} + +export function RepositoryBranchSelect({ value, branches, loading, error, disabled, onChange, onRetry }: { + value: string; + branches: readonly RepositoryBranch[]; + loading?: boolean; + error?: string | null; + disabled?: boolean; + onChange: (branch: string) => void; + onRetry?: () => void; +}) { + const uniqueBranches = [...new Map(branches.map(branch => [branch.name, branch])).values()]; + const options: BranchOption[] = uniqueBranches.map(branch => ({ key: branch.name, value: branch.name, label: branch.name, searchText: branch.isDefault ? 'default branch' : undefined, isDefault: branch.isDefault })); + if (value && !options.some(option => option.value === value)) options.push({ key: value, value, label: value }); + const groups: SearchableSelectGroup[] = [ + { id: 'default', label: 'Default branch', options: options.filter(option => option.isDefault) }, + { id: 'branches', label: 'Branches', options: options.filter(option => !option.isDefault) }, + ]; + return
+ + value={value} groups={groups} disabled={disabled} triggerAriaLabel="Branch" mobileTitle="Choose a branch" + placeholder="Choose a branch" searchPlaceholder="Search or enter a branch name…" + emptyMessage={loading ? 'Branches are loading. You can still enter a name.' : 'No matching branches. Enter a branch name to use it.'} + onValueChange={branch => onChange(branch)} + deriveGroups={(query, current) => { + const name = query.trim(); + return name && !options.some(option => option.value === name) + ? [...current, { id: 'entered', options: [{ key: `entered:${name}`, value: name, label: name, entered: true }] }] + : current; + }} + renderOption={option => <> + + {option.entered ? `Use “${option.value}”` : option.label} + {option.isDefault && Default} + } + /> + {loading &&

Loading branches…

} + {error &&

{error} You can still enter a branch name.

+ {onRetry && } +
} +
; +} diff --git a/ui/src/components/skill-studio/SkillSourceBranchRepair.tsx b/ui/src/components/skill-studio/SkillSourceBranchRepair.tsx new file mode 100644 index 00000000000..f266a735761 --- /dev/null +++ b/ui/src/components/skill-studio/SkillSourceBranchRepair.tsx @@ -0,0 +1,36 @@ +import { useQuery, useQueryClient } from '@tanstack/react-query'; +import type { SkillSource } from '@paperclipai/shared'; +import { skillSourcesApi } from '@/api/skillSources'; +import { ApiError } from '@/api/client'; +import { queryKeys } from '@/lib/queryKeys'; +import { SkillSourceBranchRepairDialog } from './SkillSourceBranchRepairDialog'; + +export function SkillSourceBranchRepair({ companyId, source, connectionId, onSaved, onClose, onReturnFocus }: { + companyId: string; source: SkillSource; connectionId?: string; onSaved: () => Promise; + onClose: () => void; onReturnFocus?: () => void; +}) { + const client = useQueryClient(); + const branches = useQuery({ + queryKey: ['skill-source-branches', companyId, source.id, connectionId ?? source.connectionId], + queryFn: ({ signal }) => skillSourcesApi.branches(companyId, source.id, connectionId ? { connectionId } : {}, signal), + retry: false, refetchOnMount: 'always', + }); + async function save(branch: string) { + try { + await skillSourcesApi.settings(companyId, source.id, { revision: source.revision, trackingRef: branch, ...(connectionId ? { connectionId } : {}) }); + } catch (error) { + if (error instanceof ApiError && error.status === 409 && (error.body as { code?: string } | null)?.code === 'skill_source_settings_changed') { + await client.invalidateQueries({ queryKey: queryKeys.skillSources.all(companyId) }); + throw new Error('This source is refreshing or changed. Your branch is still selected. Try saving again.'); + } + if (error instanceof ApiError && (error.body as { code?: string } | null)?.code === 'skill_source_ref_unavailable') { + throw new Error('GitHub could not find that branch or revision. Check the name or choose another branch.'); + } + throw error; + } + await onSaved(); + } + return void branches.refetch()} onSave={save} onClose={onClose} onReturnFocus={onReturnFocus} />; +} diff --git a/ui/src/components/skill-studio/SkillSourceBranchRepairDialog.tsx b/ui/src/components/skill-studio/SkillSourceBranchRepairDialog.tsx new file mode 100644 index 00000000000..ac6a0300d1d --- /dev/null +++ b/ui/src/components/skill-studio/SkillSourceBranchRepairDialog.tsx @@ -0,0 +1,47 @@ +import { useRef, useState } from 'react'; +import { LoaderCircle } from 'lucide-react'; +import { RepositoryBranchSelect, type RepositoryBranch } from '@/components/RepositoryBranchSelect'; +import { Button } from '@/components/ui/button'; +import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog'; + +/** Presentational branch repair. The caller owns branch discovery and saving. */ +export function SkillSourceBranchRepairDialog({ repository, currentBranch, branches, loading, loadError, onRetry, onSave, onClose, onReturnFocus }: { + repository: string; + currentBranch: string; + branches: readonly RepositoryBranch[]; + loading?: boolean; + loadError?: string | null; + onRetry?: () => void; + onSave: (branch: string) => Promise; + onClose: () => void; + onReturnFocus?: () => void; +}) { + const [branch, setBranch] = useState(''); + const [pending, setPending] = useState(false); + const [error, setError] = useState(null); + const title = useRef(null); + async function save() { + setPending(true); setError(null); + try { await onSave(branch); onClose(); } + catch (problem) { setError(problem instanceof Error ? problem.message : 'Could not change the branch. Try again.'); } + finally { setPending(false); } + } + return { if (!open && !pending) onClose(); }}> + { event.preventDefault(); title.current?.focus(); }} + onCloseAutoFocus={event => { if (onReturnFocus) { event.preventDefault(); onReturnFocus(); } }}> + + Choose a new branch + {repository} · Currently tracking {currentBranch === 'HEAD' ? 'the default branch' : currentBranch} + +
{ event.preventDefault(); if (branch && !pending) void save(); }}> + { setBranch(value); setError(null); }} onRetry={onRetry} /> + {error &&

{error}

} +
+ + +
+ +
+ ; +} diff --git a/ui/src/components/skill-studio/SkillSourceConnectionRepair.tsx b/ui/src/components/skill-studio/SkillSourceConnectionRepair.tsx index c5163bf87c6..cb144471f04 100644 --- a/ui/src/components/skill-studio/SkillSourceConnectionRepair.tsx +++ b/ui/src/components/skill-studio/SkillSourceConnectionRepair.tsx @@ -8,18 +8,19 @@ import { Link } from '@/lib/router'; import { Button } from '@/components/ui/button'; import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog'; import { RadioCardGroup } from '@/components/ui/radio-card'; +import { SkillSourceBranchRepair } from './SkillSourceBranchRepair'; export function SkillSourceConnectionRepair({ companyId, source, onClose, onSaved, onReturnFocus }: { companyId: string; source: SkillSource; onClose: () => void; onSaved: () => Promise; onReturnFocus?: () => void; }) { const [connectionId, setConnectionId] = useState(source.connectionId ?? ''); + const [chooseBranch, setChooseBranch] = useState(false); const client = useQueryClient(); const query = useQuery({ queryKey: queryKeys.skillSources.repositories(companyId), queryFn: () => skillSourcesApi.repositories(companyId), refetchOnMount: 'always', retry: false }); - const save = useMutation({ mutationFn: (useDefaultBranch: boolean) => skillSourcesApi.settings(companyId, source.id, { revision: source.revision, connectionId, - ...(useDefaultBranch ? { trackingRef: 'HEAD' as const } : {}) }), + const save = useMutation({ mutationFn: () => skillSourcesApi.settings(companyId, source.id, { revision: source.revision, connectionId }), onError: async error => { - if (error instanceof ApiError && error.status === 409) { + if (error instanceof ApiError && error.status === 409 && (error.body as { code?: string } | null)?.code === 'skill_source_settings_changed') { await client.invalidateQueries({ queryKey: queryKeys.skillSources.all(companyId) }); } }, @@ -27,6 +28,8 @@ export function SkillSourceConnectionRepair({ companyId, source, onClose, onSave const connections = query.data?.connections ?? []; const branchMissing = save.error instanceof ApiError && (save.error.body as { code?: string } | null)?.code === 'skill_source_ref_unavailable'; const canSave = connections.some(connection => connection.id === connectionId); + if (chooseBranch) return setChooseBranch(false)} onReturnFocus={onReturnFocus} onSaved={async () => { await onSaved(); onClose(); }} />; return { if (!open && !save.isPending) onClose(); }}> { if (onReturnFocus) { event.preventDefault(); onReturnFocus(); } }}> @@ -41,13 +44,13 @@ export function SkillSourceConnectionRepair({ companyId, source, onClose, onSave options={connections.map(connection => ({ value: connection.id, title: connection.name }))} /> {query.isSuccess && connections.length === 0 &&

Connect GitHub to continue.

} - {save.error &&

{save.error instanceof ApiError && save.error.status === 409 + {save.error &&

{save.error instanceof ApiError && save.error.status === 409 && (save.error.body as { code?: string } | null)?.code === 'skill_source_settings_changed' ? 'This source was updated. Your connection is still selected. Save again to apply it.' : save.error.message}

} - {branchMissing && source.accessScope !== 'projects' && !source.projects?.some(project => project.active) && } + {branchMissing && source.accessScope !== 'projects' && !source.projects?.some(project => project.active) && }
- +
; diff --git a/ui/src/pages/SkillSources.test.tsx b/ui/src/pages/SkillSources.test.tsx index 0acef241b55..ffc16d6f7b3 100644 --- a/ui/src/pages/SkillSources.test.tsx +++ b/ui/src/pages/SkillSources.test.tsx @@ -16,7 +16,7 @@ vi.mock('@/lib/router', () => ({ useNavigate: () => context.navigate, useParams: () => ({ sourceId: context.sourceId }), })); -vi.mock('@/api/skillSources', () => ({ skillSourcesApi: { list: vi.fn(), projectRepositories: vi.fn(), adoptProject: vi.fn(), settings: vi.fn(), repositories: vi.fn(), discoverStream: vi.fn(), create: vi.fn(), select: vi.fn(), refresh: vi.fn(), disconnect: vi.fn() } })); +vi.mock('@/api/skillSources', () => ({ skillSourcesApi: { list: vi.fn(), projectRepositories: vi.fn(), adoptProject: vi.fn(), settings: vi.fn(), branches: vi.fn(), repositories: vi.fn(), discoverStream: vi.fn(), create: vi.fn(), select: vi.fn(), refresh: vi.fn(), disconnect: vi.fn() } })); const repos = { connections: [{ id: 'personal', name: 'Personal' }, { id: 'shared', name: 'Engineering' }], @@ -52,6 +52,7 @@ beforeEach(() => { vi.mocked(skillSourcesApi.list).mockResolvedValue([]); vi.mocked(skillSourcesApi.projectRepositories).mockResolvedValue([]); vi.mocked(skillSourcesApi.repositories).mockResolvedValue(structuredClone(repos)); + vi.mocked(skillSourcesApi.branches).mockResolvedValue({ defaultBranch: 'master', branches: [{ name: 'master', isDefault: true }], truncated: false }); vi.mocked(skillSourcesApi.discoverStream).mockResolvedValue(discovery); context.sourceId = 'new'; sessionStorage.clear(); @@ -90,11 +91,32 @@ describe('GitHub skill source import', () => { context.sourceId = ''; const source = { ...sourceFixture(), trackingRef: 'main', lastError: 'The tracked GitHub branch or revision is unavailable. Use the repository’s default branch or update its configured ref.' }; vi.mocked(skillSourcesApi.list).mockResolvedValue([{ ...source, repairAction: 'branch' }]); - vi.mocked(skillSourcesApi.settings).mockResolvedValue({ ...source, trackingRef: 'HEAD', lastError: null }); + vi.mocked(skillSourcesApi.settings).mockResolvedValue({ ...source, trackingRef: 'master', lastError: null }); await mount(); expect(button('Choose connection')).toBeUndefined(); - await act(async () => button('Use default branch').click()); await flush(); - expect(skillSourcesApi.settings).toHaveBeenCalledWith('company-1', source.id, { revision: source.revision, trackingRef: 'HEAD' }); + await act(async () => button('Choose a new branch').click()); await flush(); + await act(async () => document.querySelector('button[aria-label="Branch"]')!.click()); await flush(); + await act(async () => (document.querySelector('[cmdk-item][data-value="master"]') as HTMLElement).click()); await flush(); + await act(async () => button('Change branch').click()); await flush(); + expect(skillSourcesApi.branches).toHaveBeenCalledWith('company-1', source.id, {}, expect.any(AbortSignal)); + expect(skillSourcesApi.settings).toHaveBeenCalledWith('company-1', source.id, { revision: source.revision, trackingRef: 'master' }); + }); + + it.each([ + 'This repository and branch are already in Sources. Use that source; installed skills were kept.', + 'The repository at this URL has changed identity. Add it as a new source.', + ])('preserves actionable branch conflict advice: %s', async message => { + context.sourceId = ''; + const source = { ...sourceFixture(), trackingRef: 'main', lastError: 'Unavailable branch.', repairAction: 'branch' as const }; + vi.mocked(skillSourcesApi.list).mockResolvedValue([source]); + vi.mocked(skillSourcesApi.settings).mockRejectedValue(new ApiError(message, 409, {})); + await mount(); + await act(async () => button('Choose a new branch').click()); await flush(); + await act(async () => document.querySelector('button[aria-label="Branch"]')!.click()); await flush(); + await act(async () => (document.querySelector('[cmdk-item][data-value="master"]') as HTMLElement).click()); await flush(); + await act(async () => button('Change branch').click()); await flush(); + expect(document.querySelector('[role="dialog"] [role="alert"]')?.textContent).toBe(message); + expect(document.querySelector('button[aria-label="Branch"]')?.textContent).toContain('master'); }); it('keeps pending project links inside the installed source instead of duplicating its repository', async () => { @@ -129,13 +151,17 @@ describe('GitHub skill source import', () => { const source = { ...sourceFixture(), connectionId: 'personal', lastError: 'Choose a GitHub connection to enable automatic updates.', repairAction: 'connection' as const }; vi.mocked(skillSourcesApi.list).mockResolvedValue([source]); vi.mocked(skillSourcesApi.settings).mockRejectedValueOnce(new ApiError('The tracked GitHub branch or revision is unavailable.', 422, { code: 'skill_source_ref_unavailable' })) - .mockResolvedValue({ ...source, trackingRef: 'HEAD', lastError: null }); + .mockResolvedValue({ ...source, trackingRef: 'master', lastError: null }); await mount(); await act(async () => button('Choose connection').click()); await flush(); await act(async () => button('Save connection').click()); await flush(); expect(skillSourcesApi.settings).toHaveBeenLastCalledWith('company-1', source.id, { revision: source.revision, connectionId: 'personal' }); - await act(async () => button('Use default branch').click()); await flush(); - expect(skillSourcesApi.settings).toHaveBeenLastCalledWith('company-1', source.id, { revision: source.revision, connectionId: 'personal', trackingRef: 'HEAD' }); + await act(async () => button('Choose a new branch').click()); await flush(); + await act(async () => document.querySelector('button[aria-label="Branch"]')!.click()); await flush(); + await act(async () => (document.querySelector('[cmdk-item][data-value="master"]') as HTMLElement).click()); await flush(); + await act(async () => button('Change branch').click()); await flush(); + expect(skillSourcesApi.branches).toHaveBeenLastCalledWith('company-1', source.id, { connectionId: 'personal' }, expect.any(AbortSignal)); + expect(skillSourcesApi.settings).toHaveBeenLastCalledWith('company-1', source.id, { revision: source.revision, connectionId: 'personal', trackingRef: 'master' }); expect(document.querySelector('[role="dialog"]')).toBeNull(); }); it('shows measured Git download progress before skill discovery and retains the automatically chosen connection', async () => { diff --git a/ui/src/pages/SkillSources.tsx b/ui/src/pages/SkillSources.tsx index 4f6d0045b61..ec21881480c 100644 --- a/ui/src/pages/SkillSources.tsx +++ b/ui/src/pages/SkillSources.tsx @@ -1,5 +1,6 @@ import { ProjectSkillRepositoryRow } from "@/components/skill-studio/ProjectSkillRepositoryRow"; import { SkillSourceConnectionRepair } from "@/components/skill-studio/SkillSourceConnectionRepair"; +import { SkillSourceBranchRepair } from "@/components/skill-studio/SkillSourceBranchRepair"; import { SkillSourceSyncControls } from "@/components/skill-studio/SkillSourceSyncControls"; import { GithubIcon } from "@/components/icons/github-icon"; import { useEffect, useRef, useState } from 'react'; @@ -38,11 +39,13 @@ export function SkillSources() { const { setBreadcrumbs } = useBreadcrumbs(); const [results, setResults] = useState>({}); const [repairSourceId, setRepairSourceId] = useState(null); + const [branchSourceId, setBranchSourceId] = useState(null); const [disconnectSource, setDisconnectSource] = useState(null); const companyId = selectedCompanyId ?? ''; useEffect(() => { setBreadcrumbs([{ label: 'Skills', href: '/skills' }, { label: 'Sources' }]); }, [setBreadcrumbs]); const query = useQuery({ queryKey: sourceKey(companyId), queryFn: () => skillSourcesApi.list(companyId), enabled: Boolean(companyId), refetchInterval: 5000 }); const repairSource = query.data?.find(source => source.id === repairSourceId); + const branchSource = query.data?.find(source => source.id === branchSourceId); const projectRepositories = useQuery({ queryKey: queryKeys.skillSources.projects(companyId), queryFn: () => skillSourcesApi.projectRepositories(companyId), enabled: Boolean(companyId), refetchInterval: 5000 }); async function invalidate() { await Promise.all([client.invalidateQueries({ queryKey: queryKeys.skillSources.projects(companyId) }), client.invalidateQueries({ queryKey: sourceKey(companyId) }), client.invalidateQueries({ queryKey: queryKeys.companySkills.list(companyId) })]); @@ -56,9 +59,6 @@ export function SkillSources() { const settings = useMutation({ mutationFn: ({ source, enabled }: { source: SkillSource; enabled: boolean }) => skillSourcesApi.settings(companyId, source.id, { revision: source.revision, automaticUpdates: enabled }), onSuccess: async (_result, variables) => { setResults(prev => ({ ...prev, [variables.source.id]: null })); await invalidate(); }, onError: (error, variables) => setResults(prev => ({ ...prev, [variables.source.id]: { message: error.message, failedAt: Date.now(), operation: 'settings' } })) }); - const repairBranch = useMutation({ mutationFn: (source: SkillSource) => skillSourcesApi.settings(companyId, source.id, { revision: source.revision, trackingRef: 'HEAD' }), - onSuccess: async (_result, source) => { setResults(prev => ({ ...prev, [source.id]: null })); await invalidate(); sourceActionButtons.current[source.id]?.focus(); }, - onError: (error, source) => { setResults(prev => ({ ...prev, [source.id]: { message: error.message, failedAt: Date.now(), operation: 'settings' } })); void invalidate(); } }); const availableProjects = groupProjectSkillRepositories(Array.isArray(projectRepositories.data) ? projectRepositories.data.filter(repo => !repo.sourceId) : []); const groupSource = (group: typeof availableProjects[number]) => query.data?.find(source => source.repositoryUrl.toLowerCase() === group.repository.repositoryUrl.toLowerCase() && source.trackingRef === group.repository.trackingRef); const uninstalledProjects = availableProjects.filter(group => !groupSource(group)); @@ -131,6 +131,7 @@ export function SkillSources() { onCheckedChange={enabled => settings.mutate({ source, enabled })}>Automatic updates navigate(`/skills/sources/${source.id}`)}>Select skills setRepairSourceId(source.id)}>Choose connection + {source.accessScope !== 'projects' && !source.projects?.some(project => project.active) && setBranchSourceId(source.id)}>Choose a new branch} { disconnect.reset(); reloadRemoval.reset(); setDisconnectSource(source); }}>{source.enabled ? 'Disconnect source' : 'Remove source'} @@ -157,12 +158,13 @@ export function SkillSources() { {source.lastError && (source.repairAction === 'connection' || (!source.repairAction && !source.connectionId)) && } {source.lastError && source.repairAction === 'branch' && (source.accessScope === 'projects' || source.projects?.some(project => project.active) ?
{source.projects?.filter(project => project.active).map(project => Update {project.name} repository)}
- : )} + : )} {source.connectionId && Manage GitHub connection} ; })} + {branchSource && setBranchSourceId(null)} + onReturnFocus={() => sourceActionButtons.current[branchSource.id]?.focus()} onSaved={async () => { setResults(prev => ({ ...prev, [branchSource.id]: null })); await invalidate(); }} />} {repairSource && setRepairSourceId(null)} onReturnFocus={() => sourceActionButtons.current[repairSource.id]?.focus()} onSaved={async () => { setResults(prev => ({ ...prev, [repairSource.id]: null })); await invalidate(); }} />} diff --git a/ui/storybook/stories/skills/project-repository-skills/README.md b/ui/storybook/stories/skills/project-repository-skills/README.md index c4d61d27ca1..e75ccdd71cf 100644 --- a/ui/storybook/stories/skills/project-repository-skills/README.md +++ b/ui/storybook/stories/skills/project-repository-skills/README.md @@ -12,6 +12,16 @@ Open **Agents and reusable capabilities → Skills and Skill Studio → Project Connection repair chooses an existing authorized account without changing skill selections. Also inspect removed skills, invalid upstream content, two private projects and the large-list story. +- **03 Branch selection components** and **04 Branch selection journeys**: shared + production branch selector and Sources dialog. Choose a new branch, search the fetched + list, or type an unlisted branch name and select **Use “name”**. The repository's + default branch is marked. Loading or a failed list request does not prevent + entering a name. Try retry, an empty list, many branches, and mobile/light views. + In **Validation failure**, enter `feature/missing`, save, then choose `master`. + The selected value stays visible after an error; existing installed skills remain. + Provider responses are simulated at the production API boundary. Also try + concurrent-update recovery and branch repair with the selected GitHub connection. + Active project refs remain managed through project configuration. All GitHub, import, scheduled update and routine execution responses are scoped in-memory fixtures, reset between stories. They do not prove live synchronization, @@ -19,3 +29,4 @@ credential refresh or model execution. Production acceptance evidence is recorde in `doc/plans/2026-10-12-project-repository-skills.md`. Browser checks: `tests/storybook-visual/project-repository-skills.spec.ts`. +Branch selection checks: `tests/storybook-visual/skill-source-branch-selection.spec.ts`. diff --git a/ui/storybook/stories/skills/project-repository-skills/branch-selection-components.stories.tsx b/ui/storybook/stories/skills/project-repository-skills/branch-selection-components.stories.tsx new file mode 100644 index 00000000000..79e91a57ff8 --- /dev/null +++ b/ui/storybook/stories/skills/project-repository-skills/branch-selection-components.stories.tsx @@ -0,0 +1,31 @@ +import { useState } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { RepositoryBranchSelect } from '@/components/RepositoryBranchSelect'; +import { repositoryBranches } from '../../../support/skillSourceBranchSelection'; + +function BranchPicker({ state = 'ready', selected = '' }: { state?: 'ready' | 'loading' | 'error' | 'empty'; selected?: string }) { + const [value, setValue] = useState(selected); + const [retried, setRetried] = useState(false); + return
+ setRetried(true)} /> +
; +} +const meta = { + id: 'skills-project-repository-skills-03-branch-selection-components', + title: 'Agents and reusable capabilities/Skills and Skill Studio/Project repository skills/03 Branch selection components', + component: BranchPicker, + parameters: { + layout: 'fullscreen', + paperclip: { capabilities: ['skills/discovery', 'projects/repositories'], status: 'current', implementation: 'production', boundary: 'fixtures', notes: 'Shared production branch selector with default, unlisted, loading, failed-list and long-name states. Branch discovery responses use fixtures; Sources owns real API loading and saving.' }, + }, +} satisfies Meta; +export default meta; +type Story = StoryObj; +export const SearchBranches: Story = {}; +export const DefaultBranch: Story = { args: { selected: 'master' } }; +export const UnlistedBranch: Story = { args: { selected: 'feature/my-new-workflow' } }; +export const LoadingBranches: Story = { args: { state: 'loading' } }; +export const BranchListUnavailable: Story = { args: { state: 'error' } }; +export const NoBranches: Story = { args: { state: 'empty' } }; +export const LongBranch: Story = { args: { selected: repositoryBranches[5]!.name } }; diff --git a/ui/storybook/stories/skills/project-repository-skills/branch-selection-journeys.stories.tsx b/ui/storybook/stories/skills/project-repository-skills/branch-selection-journeys.stories.tsx new file mode 100644 index 00000000000..ab75fa97e91 --- /dev/null +++ b/ui/storybook/stories/skills/project-repository-skills/branch-selection-journeys.stories.tsx @@ -0,0 +1,52 @@ +import { useEffect, useRef, useState } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { useQueryClient } from '@tanstack/react-query'; +import { Layout } from '@/components/Layout'; +import { SkillSources } from '@/pages/SkillSources'; +import { CompanySkills } from '@/pages/CompanySkills'; +import { PluginLauncherProvider } from '@/plugins/launchers'; +import { Route, Routes, useNavigate } from '@/lib/router'; +import { installBranchSelectionFixtures, type BranchSelectionScenario } from '../../../support/skillSourceBranchSelection'; + +function BranchSelectionJourney({ scenario = 'ready' }: { scenario?: BranchSelectionScenario }) { + const client = useQueryClient(), navigate = useNavigate(); + const initialNavigate = useRef(navigate); + const [ready, setReady] = useState(false); + useEffect(() => { + const boundary = installBranchSelectionFixtures(scenario, client); + initialNavigate.current('/skills/sources', { replace: true }); + setReady(true); + return boundary.cleanup; + }, [client, scenario]); + if (!ready) return null; + return + }> + } /> + } /> + } /> + + ; +} +const meta = { + id: 'skills-project-repository-skills-04-branch-selection-journeys', + title: 'Agents and reusable capabilities/Skills and Skill Studio/Project repository skills/04 Branch selection journeys', + component: BranchSelectionJourney, + tags: ['!autodocs'], + parameters: { + layout: 'fullscreen', + docs: { description: { component: 'Production Sources page and branch dialog in the app shell. Choose a new branch, search listed branches or enter any name, then Change branch. Provider loading, validation and saves use scoped API fixtures. In Validation failure, enter feature/missing, then choose master. Active project refs remain managed through project settings.' } }, + paperclip: { capabilities: ['skills/discovery', 'skills/versions', 'projects/repositories'], status: 'current', implementation: 'production', boundary: 'fixtures', notes: 'Production shell, Sources page and API-connected branch dialog. Default branch, search, typed input during loading/list failure, retry, validation, concurrent-update recovery and connection repair. Provider responses are simulated; live GitHub saves are verified separately. Active project refs retain project configuration ownership.' }, + }, +} satisfies Meta; +export default meta; +type Story = StoryObj; +export const ChooseBranch: Story = { name: 'Choose a listed or unlisted branch' }; +export const SlowBranchList: Story = { name: 'Enter a name while branches load', args: { scenario: 'loading' } }; +export const BranchListUnavailable: Story = { name: 'Use a name or retry when branch loading fails', args: { scenario: 'load-error' } }; +export const EmptyBranchList: Story = { name: 'Enter a name with no listed branches', args: { scenario: 'empty' } }; +export const ValidationFailure: Story = { name: 'Keep the selected branch after validation fails', args: { scenario: 'save-error' } }; +export const ManyBranches: Story = { name: 'Search a large branch list', args: { scenario: 'many' } }; +export const SaveConflict: Story = { name: 'Retain the branch after a concurrent update', args: { scenario: 'save-conflict' } }; +export const ConnectionRepair: Story = { name: 'Repair the branch with the selected connection', args: { scenario: 'connection-repair' } }; +export const Mobile: Story = { globals: { viewport: { value: 'mobile1', isRotated: false } } }; +export const Light: Story = { globals: { theme: 'light' } }; diff --git a/ui/storybook/support/projectRepositorySkills.ts b/ui/storybook/support/projectRepositorySkills.ts index 16f213f5a40..2191c7c825e 100644 --- a/ui/storybook/support/projectRepositorySkills.ts +++ b/ui/storybook/support/projectRepositorySkills.ts @@ -34,7 +34,7 @@ export function installProjectSkillFixtures(scenario: Scenario, client: QueryCli const timers: ReturnType[] = []; let projectCreated = scenario !== 'new-project'; const updatedSources = new Set(); - const sourcePreferences = new Map(); + const sourcePreferences = new Map(); let live = true, automaticUpdates = scenario !== 'paused', failed = ['error', 'repair', 'repair-conflict', 'missing-branch'].includes(scenario); let sourceRevision = sourceFixture().revision, repairConflict = scenario === 'repair-conflict'; const privateProject = scenario === 'private'; @@ -101,13 +101,16 @@ export function installProjectSkillFixtures(scenario: Scenario, client: QueryCli await adopt(row); return { bindingId: `binding-${workspaceId}` }; }; skillSourcesApi.refresh = refresh; + skillSourcesApi.branches = async () => ({ defaultBranch: scenario === 'missing-branch' ? 'master' : 'main', branches: [ + { name: scenario === 'missing-branch' ? 'master' : 'main', isDefault: true }, { name: 'release/2026.10', isDefault: false }, + ], truncated: false }); skillSourcesApi.settings = async (_companyId, id, input) => { if (input.connectionId !== undefined && repairConflict) { repairConflict = false; sourceRevision++; - throw new ApiError('The source was updated while choosing a connection.', 409, null); + throw new ApiError('The source was updated while choosing a connection.', 409, { code: 'skill_source_settings_changed' }); } if (input.revision !== undefined && input.revision < sourceRevision) { - throw new ApiError('The source revision changed.', 409, null); + throw new ApiError('The source revision changed.', 409, { code: 'skill_source_settings_changed' }); } sourcePreferences.set(id, { ...sourcePreferences.get(id), ...(input.automaticUpdates === undefined ? {} : { automaticUpdates: input.automaticUpdates }), diff --git a/ui/storybook/support/skillSourceBranchSelection.test.ts b/ui/storybook/support/skillSourceBranchSelection.test.ts new file mode 100644 index 00000000000..bea0717ea01 --- /dev/null +++ b/ui/storybook/support/skillSourceBranchSelection.test.ts @@ -0,0 +1,24 @@ +// @vitest-environment jsdom +import { QueryClient } from '@tanstack/react-query'; +import { expect, it } from 'vitest'; +import { skillSourcesApi } from '@/api/skillSources'; +import { COMPANY_ID } from '../fixtures/githubSkillSources'; +import { installBranchSelectionFixtures } from './skillSourceBranchSelection'; + +it('requires the newly returned revision before the concurrent-update journey can save', async () => { + const client = new QueryClient(); + const fixture = installBranchSelectionFixtures('save-conflict', client); + try { + const [source] = await skillSourcesApi.list(COMPANY_ID); + const input = { revision: source!.revision, trackingRef: 'develop' }; + await expect(skillSourcesApi.settings(COMPANY_ID, source!.id, input)).rejects.toMatchObject({ status: 409 }); + const [updated] = await skillSourcesApi.list(COMPANY_ID); + expect(updated!.revision).toBeGreaterThan(source!.revision); + await expect(skillSourcesApi.settings(COMPANY_ID, source!.id, input)).rejects.toMatchObject({ status: 409 }); + const saved = await skillSourcesApi.settings(COMPANY_ID, source!.id, { ...input, revision: updated!.revision }); + expect(saved.trackingRef).toBe('develop'); + expect(saved.revision).toBeGreaterThan(updated!.revision); + } finally { + fixture.cleanup(); client.clear(); + } +}); diff --git a/ui/storybook/support/skillSourceBranchSelection.ts b/ui/storybook/support/skillSourceBranchSelection.ts new file mode 100644 index 00000000000..e488baaadaf --- /dev/null +++ b/ui/storybook/support/skillSourceBranchSelection.ts @@ -0,0 +1,57 @@ +import type { QueryClient } from '@tanstack/react-query'; +import { skillSourcesApi } from '@/api/skillSources'; +import type { RepositoryBranch } from '@/components/RepositoryBranchSelect'; +import { ApiError } from '@/api/client'; +import { installProjectSkillFixtures } from './projectRepositorySkills'; + +export const repositoryBranches: RepositoryBranch[] = [ + { name: 'master', isDefault: true }, + { name: 'develop' }, + { name: 'release/2026.10' }, + { name: 'feature/project-skills' }, + { name: 'feature/branch-selection' }, + { name: 'feature/platform-services-repository-skill-refresh-and-accessibility-improvements' }, +]; +export type BranchSelectionScenario = 'ready' | 'loading' | 'load-error' | 'empty' | 'save-error' | 'many' | 'save-conflict' | 'connection-repair'; + +/** Production API boundary with scoped in-memory provider responses. */ +export function installBranchSelectionFixtures(scenario: BranchSelectionScenario, client: QueryClient) { + const cleanup = installProjectSkillFixtures(scenario === 'connection-repair' ? 'repair' : 'missing-branch', client); + const originalList = skillSourcesApi.list; + const originalBranches = skillSourcesApi.branches, originalSettings = skillSourcesApi.settings; + const selections = new Map(); + const timers: ReturnType[] = []; + const delay = (milliseconds: number) => new Promise(resolve => { timers.push(setTimeout(resolve, milliseconds)); }); + let attempts = 0, revision = 0, conflict = scenario === 'save-conflict'; + skillSourcesApi.list = async companyId => (await originalList(companyId)).map(source => { + const selection = selections.get(source.id); + const visible = scenario === 'connection-repair' ? { ...source, accessScope: 'company' as const, projects: [] } : source; + return selection === undefined ? { ...visible, revision: source.revision + revision, + ...(scenario === 'connection-repair' ? {} : { lastError: 'The tracked branch is unavailable. Choose a new branch.' }) } + : { ...visible, trackingRef: selection.branch, ...(selection.connectionId === undefined ? {} : { connectionId: selection.connectionId }), lastError: null, repairAction: null, syncState: 'ready', revision: source.revision + revision }; + }); + skillSourcesApi.branches = async () => { + await delay(scenario === 'loading' ? 4000 : 350); + if (scenario === 'load-error' && attempts++ === 0) throw new Error('Could not load branches from GitHub.'); + const branches: RepositoryBranch[] = scenario === 'empty' ? [] : scenario === 'many' + ? [...repositoryBranches, ...Array.from({ length: 80 }, (_, index) => ({ name: `feature/workflow-${index + 1}` }))] : repositoryBranches; + return { defaultBranch: 'master', branches: branches.map(branch => ({ ...branch, isDefault: Boolean(branch.isDefault) })), truncated: false }; + }; + skillSourcesApi.settings = async (companyId, sourceId, input) => { + if (input.trackingRef === undefined) { + if (scenario === 'connection-repair' && input.connectionId !== undefined) throw new ApiError('The tracked branch is unavailable.', 422, { code: 'skill_source_ref_unavailable' }); + return originalSettings(companyId, sourceId, input); + } + await delay(900); + if (scenario === 'save-error' && input.trackingRef === 'feature/missing') throw new ApiError('The branch is unavailable.', 422, { code: 'skill_source_ref_unavailable' }); + const current = (await skillSourcesApi.list(companyId)).find(source => source.id === sourceId)!; + if (input.revision !== current.revision) throw new ApiError('The source changed.', 409, { code: 'skill_source_settings_changed' }); + if (conflict) { conflict = false; revision++; throw new ApiError('The source changed.', 409, { code: 'skill_source_settings_changed' }); } + await originalSettings(companyId, sourceId, input); + selections.set(sourceId, { branch: input.trackingRef, connectionId: input.connectionId }); revision++; + return (await skillSourcesApi.list(companyId)).find(source => source.id === sourceId)!; + }; + return { + cleanup() { timers.forEach(clearTimeout); skillSourcesApi.list = originalList; skillSourcesApi.branches = originalBranches; skillSourcesApi.settings = originalSettings; cleanup(); }, + }; +}