Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 119 additions & 2 deletions src/cli/commands/plugin-skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ import {
skillsAddMeta,
} from '../metadata/plugin-skills.js';
import { getHomeDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../constants.js';
import { isGitHubUrl, parseGitHubUrl } from '../../utils/plugin-path.js';
import { isGitHubUrl, parseGitHubUrl, stripGitRef } from '../../utils/plugin-path.js';
import { fetchPlugin, getPluginName, seedFetchCache } from '../../core/plugin.js';
import { upsertSyncStateSource } from '../../core/sync-state.js';
import { parseSkillMetadata } from '../../validators/skill.js';
import {
addMarketplace,
Expand Down Expand Up @@ -60,6 +61,60 @@ function resolveScope(cwd: string): 'user' | 'project' {
return 'user';
}

/**
* Record a per-source provenance entry (resolved ref + SHA + pin) into the
* workspace's sync-state. The key is the spec with any `@<ref>` suffix stripped
* so all installs of `owner/repo` map to one entry regardless of pin.
*
* Silently no-ops for non-GitHub sources (local paths, marketplace shorthand)
* since we can't resolve a SHA from them.
*/
async function recordSourceProvenance(opts: {
from: string;
pinnedRef: string | undefined;
workspacePath: string;
isUser: boolean;
}): Promise<void> {
const { from, pinnedRef, workspacePath, isUser } = opts;
if (!isGitHubUrl(from)) return;
const parsed = parseGitHubUrl(from);
if (!parsed) return;

// The fetch ran during install so this hits the cache and returns the
// resolvedSha without another git operation.
const fetchResult = await fetchPlugin(from, {
...(parsed.branch && { branch: parsed.branch }),
});
if (!fetchResult.success || !fetchResult.resolvedSha) return;

const targetPath = isUser ? getHomeDir() : workspacePath;
const key = stripGitRef(`${parsed.owner}/${parsed.repo}`);
await upsertSyncStateSource(targetPath, key, {
pluginSpec: key,
resolvedRef: fetchResult.resolvedRef ?? parsed.branch ?? 'HEAD',
resolvedSha: fetchResult.resolvedSha,
...(pinnedRef && { pinnedRef }),
});
}

/**
* Extract the inline `@<ref>` suffix from a plugin source spec, if present.
* Only matches owner/repo-style sources (must have a slash before the `@`),
* so `plugin@marketplace` returns undefined.
*/
function extractInlineRef(spec: string): string | undefined {
const slashIdx = spec.indexOf('/');
if (slashIdx === -1) return undefined;
const atIdx = spec.indexOf('@', slashIdx);
if (atIdx === -1) return undefined;
const ref = spec.slice(atIdx + 1);
// If the @ref contains another slash, it's the subpath portion; the ref is
// the chunk between @ and the next slash.
const nextSlash = ref.indexOf('/');
const cleanRef = nextSlash === -1 ? ref : ref.slice(0, nextSlash);
return cleanRef.length > 0 ? cleanRef : undefined;
}

/**
* If the skill argument is a GitHub URL, extract the skill name and return
* it along with the URL as the plugin source. Returns null if not a URL.
Expand Down Expand Up @@ -905,6 +960,11 @@ const addCmd = command({
short: 'f',
description: 'Plugin source to install if the skill is not already available',
}),
pin: option({
type: optional(string),
long: 'pin',
description: 'Pin the plugin to a specific Git ref (tag, branch, or SHA). Mutually exclusive with inline @ref in --from.',
}),
list: flag({
long: 'list',
short: 'l',
Expand All @@ -915,8 +975,43 @@ const addCmd = command({
description: 'Install every skill from --from',
}),
},
handler: async ({ skill: skillArg, scope, plugin, from: fromArg, list, all }) => {
handler: async ({ skill: skillArg, scope, plugin, from: fromArg, pin, list, all }) => {
try {
// Resolve --pin together with inline @ref. Three legal states:
// • --pin only → splice into fromArg
// • inline @ref → leave fromArg alone, remember pinnedRef
// • neither → no pin
// Mutex: --pin combined with inline @ref is rejected.
let pinnedRef: string | undefined;
if (pin || fromArg) {
const inlineRef = fromArg ? extractInlineRef(fromArg) : undefined;
if (pin && inlineRef) {
const error = 'Cannot combine inline @version in --from with --pin. Use one or the other.';
if (isJsonMode()) {
jsonOutput({ success: false, command: 'skill add', error });
process.exit(1);
}
console.error(`Error: ${error}`);
process.exit(1);
}
if (pin && fromArg) {
// Splice the pin into the source string so downstream parseGitHubUrl
// picks it up as the branch/tag.
fromArg = `${fromArg}@${pin}`;
pinnedRef = pin;
} else if (inlineRef) {
pinnedRef = inlineRef;
} else if (pin && !fromArg) {
const error = '--pin requires --from to specify a plugin source.';
if (isJsonMode()) {
jsonOutput({ success: false, command: 'skill add', error });
process.exit(1);
}
console.error(`Error: ${error}`);
process.exit(1);
}
}

// --list: dry-run discovery, no workspace changes
if (list) {
if (skillArg) {
Expand Down Expand Up @@ -1028,6 +1123,14 @@ const addCmd = command({
process.exit(1);
}

// Record source provenance for the --all install path too.
await recordSourceProvenance({
from: fromArg,
pinnedRef,
workspacePath: workspacePathAll,
isUser: isUserAll,
});

if (isJsonMode()) {
jsonOutput({
success: true,
Expand All @@ -1039,6 +1142,7 @@ const addCmd = command({
copied: installResult.syncResult.totalCopied,
failed: installResult.syncResult.totalFailed,
},
...(pinnedRef && { pinnedRef }),
},
});
return;
Expand Down Expand Up @@ -1121,6 +1225,15 @@ const addCmd = command({
process.exit(1);
}

// Record source provenance (resolved ref + SHA + optional pin) for
// drift detection on subsequent syncs.
await recordSourceProvenance({
from,
pinnedRef,
workspacePath,
isUser,
});

if (isJsonMode()) {
jsonOutput({
success: true,
Expand All @@ -1129,11 +1242,15 @@ const addCmd = command({
skill,
plugin: installFromResult.pluginName,
syncResult: installFromResult.syncResult,
...(pinnedRef && { pinnedRef }),
},
});
return;
}

if (pinnedRef) {
console.log(`Pinned to ${pinnedRef}.`);
}
console.log('Sync complete.');
return;
}
Expand Down
13 changes: 13 additions & 0 deletions src/core/marketplace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1243,12 +1243,25 @@ async function autoRegisterMarketplace(

/**
* Check if a spec is in plugin@marketplace format
*
* `plugin@marketplace` → true (plain marketplace shorthand)
* `plugin@owner/repo[/sub]` → true (marketplace by GitHub repo)
* `owner/repo@ref[/sub]` → false (GitHub plugin with `@ref` version pin)
*
* The disambiguation: when the segment before the last `@` itself contains a
* `/`, the spec is an `owner/repo` GitHub URL with a `@ref` pin, not a
* plugin@marketplace pair. Plugin names in marketplaces are bare identifiers
* without slashes.
*/
export function isPluginSpec(spec: string): boolean {
const atIndex = spec.lastIndexOf('@');
if (atIndex === -1 || atIndex === 0 || atIndex === spec.length - 1) {
return false;
}
const beforeAt = spec.slice(0, atIndex);
if (beforeAt.includes('/')) {
return false;
}
return true;
}

Expand Down
57 changes: 52 additions & 5 deletions src/core/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mkdir, readdir, stat } from 'node:fs/promises';
import { existsSync } from 'node:fs';
import { basename, dirname, join, resolve } from 'node:path';
import simpleGit from 'simple-git';
import {
parseGitHubUrl,
getPluginCachePath,
Expand Down Expand Up @@ -28,6 +29,10 @@ export interface FetchResult {
error?: string;
/** Duration of the git operation in milliseconds */
durationMs?: number;
/** The ref (branch/tag) the fetch resolved against, if known. */
resolvedRef?: string;
/** Resolved commit SHA of the cached working tree, if known. */
resolvedSha?: string;
}

/**
Expand All @@ -50,6 +55,21 @@ export interface FetchDeps {
pull?: typeof pull;
}

/**
* Resolve the HEAD commit SHA of a local repository. Returns undefined if the
* directory isn't a git repo or rev-parse fails (e.g., a cached marketplace
* subdirectory that was copied rather than cloned).
*/
async function resolveHeadSha(repoPath: string): Promise<string | undefined> {
try {
const sha = await simpleGit(repoPath).revparse(['HEAD']);
const trimmed = sha.trim();
return trimmed.length > 0 ? trimmed : undefined;
} catch {
return undefined;
}
}

// Deduplicates git operations for the same cache directory within a sync session.
// Both concurrent and sequential callers targeting the same repo reuse the
// result of the first git operation. Call `resetFetchCache()` between sync
Expand Down Expand Up @@ -197,9 +217,24 @@ async function doFetchPlugin(
const pullStart = performance.now();
await pullFn(cachePath);
const pullMs = Math.round(performance.now() - pullStart);
return { success: true, action: 'updated', cachePath, durationMs: pullMs };
const sha = await resolveHeadSha(cachePath);
return {
success: true,
action: 'updated',
cachePath,
durationMs: pullMs,
...(branch && { resolvedRef: branch }),
...(sha && { resolvedSha: sha }),
};
} catch {
return { success: true, action: 'skipped', cachePath };
const sha = await resolveHeadSha(cachePath);
return {
success: true,
action: 'skipped',
cachePath,
...(branch && { resolvedRef: branch }),
...(sha && { resolvedSha: sha }),
};
}
}

Expand All @@ -212,12 +247,15 @@ async function doFetchPlugin(
const cloneStart = performance.now();
await cloneToFn(repoUrl, cachePath, branch);
const cloneMs = Math.round(performance.now() - cloneStart);
const sha = await resolveHeadSha(cachePath);

return {
success: true,
action: 'fetched',
cachePath,
durationMs: cloneMs,
...(branch && { resolvedRef: branch }),
...(sha && { resolvedSha: sha }),
};
} catch (error) {
if (error instanceof GitCloneError) {
Expand Down Expand Up @@ -339,12 +377,21 @@ export async function updateCachedPlugins(
}

/**
* Get the plugin name from the directory name
* Get the plugin name from the directory name.
*
* Cache directories for branch/tag-pinned clones use the form
* `<owner>-<repo>@<sanitized-ref>`. The pin suffix is part of the on-disk
* layout for collision avoidance, but the logical plugin name is the
* base — strip the suffix so callers that key workspace.yaml entries by
* plugin name (e.g., setPluginSkillsMode) match against the unpinned form.
*
* @param pluginPath - Resolved path to the plugin directory
* @returns The plugin name (directory basename)
* @returns The plugin name (directory basename, without any `@ref` suffix)
*/
export function getPluginName(pluginPath: string): string {
return basename(pluginPath);
const base = basename(pluginPath);
const atIdx = base.indexOf('@');
return atIdx === -1 ? base : base.slice(0, atIdx);
}

/**
Expand Down
44 changes: 43 additions & 1 deletion src/core/sync-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { readFile, writeFile, mkdir } from 'node:fs/promises';
import { existsSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { CONFIG_DIR, SYNC_STATE_FILE } from '../constants.js';
import { SyncStateSchema, type SyncState } from '../models/sync-state.js';
import {
SyncStateSchema,
type SyncState,
type SyncStateSource,
} from '../models/sync-state.js';
import type { ClientType } from '../models/workspace-config.js';
import { ensureConfigGitignore } from './config-gitignore.js';

Expand All @@ -19,6 +23,7 @@ export interface SyncStateData {
vscodeWorkspaceHash?: string;
vscodeWorkspaceRepos?: string[];
skillsIndex?: string[];
sources?: Record<string, SyncStateSource>;
}

/**
Expand Down Expand Up @@ -85,6 +90,7 @@ export async function saveSyncState(
...(normalizedData.vscodeWorkspaceHash && { vscodeWorkspaceHash: normalizedData.vscodeWorkspaceHash }),
...(normalizedData.vscodeWorkspaceRepos && { vscodeWorkspaceRepos: normalizedData.vscodeWorkspaceRepos }),
...(normalizedData.skillsIndex && normalizedData.skillsIndex.length > 0 && { skillsIndex: normalizedData.skillsIndex }),
...(normalizedData.sources && Object.keys(normalizedData.sources).length > 0 && { sources: normalizedData.sources }),
};

await mkdir(dirname(statePath), { recursive: true });
Expand Down Expand Up @@ -139,3 +145,39 @@ export function getPreviouslySyncedNativePlugins(
if (!state?.nativePlugins) return [];
return state.nativePlugins[client] ?? [];
}

/**
* Upsert a single per-source provenance record into the workspace sync state.
* Preserves every other field of the existing sync-state file.
*
* If no sync-state exists yet, a fresh one is created with only this entry.
*/
export async function upsertSyncStateSource(
workspacePath: string,
key: string,
source: SyncStateSource,
): Promise<void> {
const existing = await loadSyncState(workspacePath);
const sources = {
...(existing?.sources ?? {}),
[key]: source,
};

await saveSyncState(workspacePath, {
files: (existing?.files ?? {}) as Partial<Record<ClientType, string[]>>,
...(existing?.mcpServers && {
mcpServers: existing.mcpServers as Partial<Record<McpScope, string[]>>,
}),
...(existing?.nativePlugins && {
nativePlugins: existing.nativePlugins as Partial<Record<ClientType, string[]>>,
}),
...(existing?.vscodeWorkspaceHash && {
vscodeWorkspaceHash: existing.vscodeWorkspaceHash,
}),
...(existing?.vscodeWorkspaceRepos && {
vscodeWorkspaceRepos: existing.vscodeWorkspaceRepos,
}),
...(existing?.skillsIndex && { skillsIndex: existing.skillsIndex }),
sources,
});
}
Loading