mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 12:13:30 +00:00
🛟 fix: Report Skill Sync Files Whose Paths Cannot Be Mirrored (#15067)
* 🛟 fix: Report Skill Sync Files Whose Paths Cannot Be Mirrored `discoverSkills` dropped any file whose path failed `isSafeRelativePath` with no warning, no count, and no record. The skill published, reported `succeeded`, and was missing files — invisible from the mirrored copy. Real case: NVIDIA/skills has two such files (spaces in the filename), so that repository syncs "cleanly" while silently losing them. Matches what zip import already does — record the file, keep the skill — and mirrors the existing `skippedSkills` shape into `skippedFiles` / `skippedFileCount` on the sync status. Dropped files now make a run `partial`, since a run reporting `succeeded` while dropping content is the bug. Only dropped *skills* can still make a run `failed`: a source that published everything it found is a real mirror even if a file inside one skill could not come along. * 🔧 fix: Charge Dropped Files to the Skill That Published Them Codex review: the up-front accounting counted a skill's unsupported files whether or not that skill went on to publish. Two consequences — the status described a skipped skill as published-but-incomplete, and enough failed skills could consume the 20-entry sample and crowd out drops from skills that actually published, which is the case the record exists for. Now recorded at the two points a skill is counted as synced, matching what `ISkillSyncSkippedFile` already documented ("the skill itself is live"). Also replaces `Array.prototype.at` in the new tests: it is outside this package's lib target, so `tsc` rejected it even though jest ran it fine.
This commit is contained in:
parent
17a02ac804
commit
757fbebc37
10 changed files with 398 additions and 31 deletions
|
|
@ -37,6 +37,7 @@ function createSourceStatus(overrides: Partial<SourceStatus> = {}): SourceStatus
|
|||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
skippedSkillCount: 0,
|
||||
skippedFileCount: 0,
|
||||
errorCode: undefined,
|
||||
errorMessage: undefined,
|
||||
startedAt: undefined,
|
||||
|
|
|
|||
|
|
@ -168,6 +168,9 @@ function serializeSourceStatus(
|
|||
/* The per-skill entries name repository paths, so they follow the same
|
||||
visibility rule as owner/repo/paths rather than the bare count. */
|
||||
skippedSkills: includePrivateSourceMetadata ? status.skippedSkills : undefined,
|
||||
skippedFileCount: status.skippedFileCount ?? 0,
|
||||
/* Same rule: `path`/`skillPath` name repository paths, unlike the count. */
|
||||
skippedFiles: includePrivateSourceMetadata ? status.skippedFiles : undefined,
|
||||
createdAt: toIso(status.createdAt),
|
||||
updatedAt: toIso(status.updatedAt),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -271,6 +271,8 @@ function createDeps(
|
|||
deletedFileCount: input.deletedFileCount ?? 0,
|
||||
skippedSkillCount: input.skippedSkillCount ?? 0,
|
||||
skippedSkills: input.skippedSkills,
|
||||
skippedFileCount: input.skippedFileCount ?? 0,
|
||||
skippedFiles: input.skippedFiles,
|
||||
};
|
||||
statuses.push(status);
|
||||
return status;
|
||||
|
|
@ -3420,23 +3422,23 @@ describe('createGitHubSkillSyncRunner', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('repository adapter seam', () => {
|
||||
/** Stands in for any provider: a flat repository held in memory. */
|
||||
function createFakeAdapter(files: Record<string, string>): GitRepoAdapter {
|
||||
const entries: RepoTreeEntry[] = Object.entries(files).map(([path, content]) => ({
|
||||
path,
|
||||
type: 'blob',
|
||||
id: `${path}@1`,
|
||||
size: Buffer.byteLength(content),
|
||||
}));
|
||||
return {
|
||||
resolveCommit: async () => ({ id: 'fake-commit', treeId: 'fake-tree' }),
|
||||
fetchTreeEntries: async (_commit, { pathPrefix }) =>
|
||||
entries.filter((entry) => !pathPrefix || entry.path.startsWith(`${pathPrefix}/`)),
|
||||
fetchFileContent: async (_commit, entry) => Buffer.from(files[entry.path]),
|
||||
};
|
||||
}
|
||||
/** Stands in for any provider: a flat repository held in memory. */
|
||||
function createFakeAdapter(files: Record<string, string>): GitRepoAdapter {
|
||||
const entries: RepoTreeEntry[] = Object.entries(files).map(([path, content]) => ({
|
||||
path,
|
||||
type: 'blob',
|
||||
id: `${path}@1`,
|
||||
size: Buffer.byteLength(content),
|
||||
}));
|
||||
return {
|
||||
resolveCommit: async () => ({ id: 'fake-commit', treeId: 'fake-tree' }),
|
||||
fetchTreeEntries: async (_commit, { pathPrefix }) =>
|
||||
entries.filter((entry) => !pathPrefix || entry.path.startsWith(`${pathPrefix}/`)),
|
||||
fetchFileContent: async (_commit, entry) => Buffer.from(files[entry.path]),
|
||||
};
|
||||
}
|
||||
|
||||
describe('repository adapter seam', () => {
|
||||
it('publishes skills read through any repository client, with no provider requests', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
|
|
@ -3504,3 +3506,182 @@ describe('repository adapter seam', () => {
|
|||
expect(deps.upsertSkillFile).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('files whose paths cannot be mirrored', () => {
|
||||
const skillMarkdown = '---\nname: research\ndescription: Research things\n---\nBody';
|
||||
|
||||
it('publishes the skill but reports the run partial and names the dropped file', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
createFakeAdapter({
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
'skills/research/scripts/run.sh': 'echo hi',
|
||||
'skills/research/Skill Card Generator Card': 'card',
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(result.status).toBe('completed');
|
||||
expect(deps.createSkill).toHaveBeenCalledTimes(1);
|
||||
expect(deps.upsertSkillFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ relativePath: 'scripts/run.sh' }),
|
||||
);
|
||||
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
status: 'partial',
|
||||
syncedSkillCount: 1,
|
||||
skippedSkillCount: 0,
|
||||
skippedFileCount: 1,
|
||||
skippedFiles: [
|
||||
{
|
||||
path: 'skills/research/Skill Card Generator Card',
|
||||
skillPath: 'skills/research',
|
||||
errorCode: 'SKILL_FILE_PATH_UNSUPPORTED',
|
||||
errorMessage: expect.stringContaining('cannot represent'),
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('never mirrors the unsupported file itself', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
createFakeAdapter({
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
'skills/research/Skill Card Generator Card': 'card',
|
||||
}),
|
||||
});
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(deps.upsertSkillFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not downgrade a source that mirrored everything it found', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
createFakeAdapter({
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
'skills/research/scripts/run.sh': 'echo hi',
|
||||
}),
|
||||
});
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({ status: 'succeeded', skippedFileCount: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not charge a dropped file to a skill that never published', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
createFakeAdapter({
|
||||
'skills/broken/SKILL.md': '---\nname: [\n---\nBody',
|
||||
'skills/broken/bad name': 'x',
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
}),
|
||||
});
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
status: 'partial',
|
||||
skippedSkillCount: 1,
|
||||
skippedFileCount: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the recorded sample for skills that published, not skills that were skipped', async () => {
|
||||
const files: Record<string, string> = {
|
||||
'skills/broken/SKILL.md': '---\nname: [\n---\nBody',
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
'skills/research/bad name': 'x',
|
||||
};
|
||||
for (let i = 0; i < 25; i++) {
|
||||
files[`skills/broken/bad name ${i}`] = 'x';
|
||||
}
|
||||
const deps = createDeps({ createAdapter: () => createFakeAdapter(files) });
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
const statusCalls = (deps.upsertStatus as jest.Mock).mock.calls;
|
||||
const status = statusCalls[statusCalls.length - 1][0] as SkillSyncStatusInput;
|
||||
expect(status.skippedFileCount).toBe(1);
|
||||
expect(status.skippedFiles).toEqual([
|
||||
expect.objectContaining({ path: 'skills/research/bad name', skillPath: 'skills/research' }),
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps counting past the recorded sample so the total stays truthful', async () => {
|
||||
const files: Record<string, string> = { 'skills/research/SKILL.md': skillMarkdown };
|
||||
for (let i = 0; i < 25; i++) {
|
||||
files[`skills/research/bad name ${i}`] = 'x';
|
||||
}
|
||||
const deps = createDeps({ createAdapter: () => createFakeAdapter(files) });
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
const statusCalls = (deps.upsertStatus as jest.Mock).mock.calls;
|
||||
const status = statusCalls[statusCalls.length - 1][0] as SkillSyncStatusInput;
|
||||
expect(status.skippedFileCount).toBe(25);
|
||||
expect(status.skippedFiles).toHaveLength(20);
|
||||
});
|
||||
|
||||
it('records an empty skill path for a skill mirrored from the repository root', async () => {
|
||||
const deps = createDeps({
|
||||
getConfig: () => ({
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
id: 'librechat-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: [''],
|
||||
credentialKey: 'github-skills-prod',
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
createAdapter: () => createFakeAdapter({ 'SKILL.md': skillMarkdown, 'bad name': 'x' }),
|
||||
});
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
skippedFileCount: 1,
|
||||
skippedFiles: [expect.objectContaining({ path: 'bad name', skillPath: '' })],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('attributes a dropped file to the nested skill that owns it', async () => {
|
||||
const deps = createDeps({
|
||||
createAdapter: () =>
|
||||
createFakeAdapter({
|
||||
'skills/research/SKILL.md': skillMarkdown,
|
||||
'skills/research/nested/SKILL.md':
|
||||
'---\nname: nested\ndescription: Nested things\n---\nBody',
|
||||
'skills/research/nested/bad name': 'x',
|
||||
}),
|
||||
});
|
||||
|
||||
await createGitHubSkillSyncRunner(deps).runOnce();
|
||||
|
||||
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({
|
||||
skippedFileCount: 1,
|
||||
skippedFiles: [expect.objectContaining({ skillPath: 'skills/research/nested' })],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import type {
|
|||
ISkill,
|
||||
ISkillFile,
|
||||
ValidationIssue,
|
||||
ISkillSyncSkippedFile,
|
||||
ISkillSyncSkippedSkill,
|
||||
CreateSkillInput,
|
||||
UpdateSkillInput,
|
||||
|
|
@ -53,6 +54,11 @@ const PROVIDER: SkillSyncProvider = 'github';
|
|||
const LOCK_LEASE_MS = 30 * 60 * 1000;
|
||||
/** Keeps a pathological source from writing an unbounded status document. */
|
||||
const MAX_RECORDED_SKIPPED_SKILLS = 20;
|
||||
/** Same bound for files, which a single malformed source can produce far more of. */
|
||||
const MAX_RECORDED_SKIPPED_FILES = 20;
|
||||
const UNSUPPORTED_FILE_PATH_CODE = 'SKILL_FILE_PATH_UNSUPPORTED';
|
||||
const UNSUPPORTED_FILE_PATH_MESSAGE =
|
||||
'File path uses characters that skill file paths cannot represent';
|
||||
/** Shared cap for skipped-skill and successful-skill validation warning logs. */
|
||||
const MAX_LOGGED_PER_SKILL_WARNINGS = 20;
|
||||
const SKIP_PATH_MAX = 500;
|
||||
|
|
@ -74,12 +80,20 @@ type SyncCounters = {
|
|||
deletedSkillCount: number;
|
||||
deletedFileCount: number;
|
||||
skippedSkillCount: number;
|
||||
skippedFileCount: number;
|
||||
};
|
||||
|
||||
type DiscoveredSkill = {
|
||||
rootPath: string;
|
||||
skillMd: RepoTreeEntry;
|
||||
files: RepoTreeEntry[];
|
||||
/**
|
||||
* Repository paths under the skill root that exist upstream but cannot be
|
||||
* mirrored, because their path is not representable as a skill file path.
|
||||
* Dropping them silently would publish a skill that looks complete while
|
||||
* missing files, so they are carried out to the sync status instead.
|
||||
*/
|
||||
unsupportedFiles: string[];
|
||||
};
|
||||
|
||||
type UpsertRemoteSkillResult = {
|
||||
|
|
@ -676,21 +690,30 @@ function discoverSkills(
|
|||
}
|
||||
return rootPath ? candidate.startsWith(`${rootPath}/`) : true;
|
||||
});
|
||||
const files = tree.filter((entry) => {
|
||||
const files: RepoTreeEntry[] = [];
|
||||
const unsupportedFiles: string[] = [];
|
||||
for (const entry of tree) {
|
||||
if (entry.type !== 'blob') {
|
||||
return false;
|
||||
continue;
|
||||
}
|
||||
const normalized = normalizeRepoPath(entry.path);
|
||||
if (!normalized.startsWith(prefix) || normalized === skillMd.path) {
|
||||
return false;
|
||||
continue;
|
||||
}
|
||||
if (childSkillRoots.some((childRoot) => normalized.startsWith(`${childRoot}/`))) {
|
||||
return false;
|
||||
continue;
|
||||
}
|
||||
const relativePath = prefix ? normalized.slice(prefix.length) : normalized;
|
||||
return isSafeRelativePath(relativePath) && relativePath.toUpperCase() !== 'SKILL.MD';
|
||||
});
|
||||
return { rootPath, skillMd, files };
|
||||
if (relativePath.toUpperCase() === 'SKILL.MD') {
|
||||
continue;
|
||||
}
|
||||
if (!isSafeRelativePath(relativePath)) {
|
||||
unsupportedFiles.push(normalized);
|
||||
continue;
|
||||
}
|
||||
files.push(entry);
|
||||
}
|
||||
return { rootPath, skillMd, files, unsupportedFiles };
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -724,6 +747,7 @@ function makeStatusInput(params: {
|
|||
errorMessage?: string;
|
||||
counts?: Partial<SyncCounters>;
|
||||
skippedSkills?: ISkillSyncSkippedSkill[];
|
||||
skippedFiles?: ISkillSyncSkippedFile[];
|
||||
}): SkillSyncStatusInput {
|
||||
return {
|
||||
provider: PROVIDER,
|
||||
|
|
@ -745,6 +769,8 @@ function makeStatusInput(params: {
|
|||
deletedFileCount: params.counts?.deletedFileCount ?? 0,
|
||||
skippedSkillCount: params.counts?.skippedSkillCount ?? 0,
|
||||
skippedSkills: params.skippedSkills,
|
||||
skippedFileCount: params.counts?.skippedFileCount ?? 0,
|
||||
skippedFiles: params.skippedFiles,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -1437,8 +1463,10 @@ async function syncSource(params: {
|
|||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
skippedSkillCount: 0,
|
||||
skippedFileCount: 0,
|
||||
};
|
||||
const skippedSkills: ISkillSyncSkippedSkill[] = [];
|
||||
const skippedFiles: ISkillSyncSkippedFile[] = [];
|
||||
await deps.upsertStatus(makeStatusInput({ source, status: 'running', startedAt }));
|
||||
try {
|
||||
assertNotCancelled();
|
||||
|
|
@ -1636,6 +1664,27 @@ async function syncSource(params: {
|
|||
discoveredUpstreamIds,
|
||||
});
|
||||
|
||||
/**
|
||||
* Only a live skill's dropped files are worth reporting. A skill that was
|
||||
* skipped outright is already accounted for in `skippedSkills`, so charging
|
||||
* its files here would both misdescribe it as published-but-incomplete and
|
||||
* let it crowd genuinely invisible drops out of the recorded sample.
|
||||
*/
|
||||
const recordUnsupportedFiles = (discovered: DiscoveredSkill): void => {
|
||||
for (const unsupportedPath of discovered.unsupportedFiles) {
|
||||
counts.skippedFileCount++;
|
||||
if (skippedFiles.length >= MAX_RECORDED_SKIPPED_FILES) {
|
||||
continue;
|
||||
}
|
||||
skippedFiles.push({
|
||||
path: truncateSkipPath(unsupportedPath),
|
||||
skillPath: truncateSkipPath(discovered.rootPath),
|
||||
errorCode: UNSUPPORTED_FILE_PATH_CODE,
|
||||
errorMessage: UNSUPPORTED_FILE_PATH_MESSAGE,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
const syncPreparedSkill = async ({
|
||||
discovered,
|
||||
prepared,
|
||||
|
|
@ -1760,6 +1809,7 @@ async function syncSource(params: {
|
|||
counts.syncedSkillCount++;
|
||||
counts.syncedFileCount += fileCounts.syncedFileCount;
|
||||
counts.deletedFileCount += fileCounts.deletedFileCount;
|
||||
recordUnsupportedFiles(discovered);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -1780,6 +1830,7 @@ async function syncSource(params: {
|
|||
counts.syncedSkillCount++;
|
||||
counts.syncedFileCount += fileCounts.syncedFileCount;
|
||||
counts.deletedFileCount += fileCounts.deletedFileCount;
|
||||
recordUnsupportedFiles(discovered);
|
||||
} catch (error) {
|
||||
const rolledBack = await deleteSyncedSkill(deps, skill)
|
||||
.then(() => true)
|
||||
|
|
@ -1852,7 +1903,7 @@ async function syncSource(params: {
|
|||
counts.deletedSkillCount++;
|
||||
}
|
||||
|
||||
if (counts.skippedSkillCount === 0) {
|
||||
if (counts.skippedSkillCount === 0 && counts.skippedFileCount === 0) {
|
||||
logSuppressedPerSkillWarningSummaries();
|
||||
return deps.upsertStatus(
|
||||
makeStatusInput({
|
||||
|
|
@ -1867,11 +1918,14 @@ async function syncSource(params: {
|
|||
/* Nothing published and something skipped means the source produced no
|
||||
usable mirror at all, which is a failure however it is spelled. The
|
||||
first skip carries the reason so the status is actionable. */
|
||||
const publishedNothing = counts.syncedSkillCount === 0;
|
||||
/* Only dropped *skills* can make a run a failure. A run that published
|
||||
every skill it found is still a real mirror, even if some file inside
|
||||
one of them could not come along. */
|
||||
const publishedNothing = counts.syncedSkillCount === 0 && counts.skippedSkillCount > 0;
|
||||
const firstSkip = skippedSkills[0];
|
||||
logSuppressedPerSkillWarningSummaries();
|
||||
logger.warn(
|
||||
`[GitHubSkillSync] Source "${source.id}" synced ${counts.syncedSkillCount} skill(s) and skipped ${counts.skippedSkillCount}`,
|
||||
`[GitHubSkillSync] Source "${source.id}" synced ${counts.syncedSkillCount} skill(s), skipped ${counts.skippedSkillCount} skill(s) and ${counts.skippedFileCount} file(s)`,
|
||||
);
|
||||
return deps.upsertStatus(
|
||||
makeStatusInput({
|
||||
|
|
@ -1881,6 +1935,7 @@ async function syncSource(params: {
|
|||
finishedAt: new Date(),
|
||||
counts,
|
||||
skippedSkills,
|
||||
skippedFiles: skippedFiles.length > 0 ? skippedFiles : undefined,
|
||||
errorCode: publishedNothing ? firstSkip?.errorCode : undefined,
|
||||
errorMessage: publishedNothing ? firstSkip?.errorMessage : undefined,
|
||||
}),
|
||||
|
|
@ -1900,8 +1955,10 @@ async function syncSource(params: {
|
|||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
skippedSkillCount: counts.skippedSkillCount,
|
||||
skippedFileCount: counts.skippedFileCount,
|
||||
},
|
||||
skippedSkills: skippedSkills.length > 0 ? skippedSkills : undefined,
|
||||
skippedFiles: skippedFiles.length > 0 ? skippedFiles : undefined,
|
||||
errorCode: sanitized.code,
|
||||
errorMessage: sanitized.message,
|
||||
}),
|
||||
|
|
@ -1998,6 +2055,8 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps): GitHubSk
|
|||
deletedFileCount: stored?.deletedFileCount ?? 0,
|
||||
skippedSkillCount: stored?.skippedSkillCount ?? 0,
|
||||
skippedSkills: stored?.skippedSkills,
|
||||
skippedFileCount: stored?.skippedFileCount ?? 0,
|
||||
skippedFiles: stored?.skippedFiles,
|
||||
createdAt: stored?.createdAt,
|
||||
updatedAt: stored?.updatedAt,
|
||||
} satisfies ISkillSyncStatus & { credentialPresent: boolean };
|
||||
|
|
|
|||
|
|
@ -58,6 +58,7 @@ function statusFromConfig(
|
|||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
skippedSkillCount: 0,
|
||||
skippedFileCount: 0,
|
||||
errorCode: undefined,
|
||||
errorMessage: undefined,
|
||||
startedAt: undefined,
|
||||
|
|
|
|||
|
|
@ -237,6 +237,14 @@ export type TGitHubSkillSyncSkippedSkill = {
|
|||
errorMessage: string;
|
||||
};
|
||||
|
||||
/** One upstream file a sync run published a skill without, and why. */
|
||||
export type TGitHubSkillSyncSkippedFile = {
|
||||
path: string;
|
||||
skillPath: string;
|
||||
errorCode: string;
|
||||
errorMessage: string;
|
||||
};
|
||||
|
||||
export type TGitHubSkillSyncSourceStatus = {
|
||||
provider: 'github';
|
||||
sourceId: string;
|
||||
|
|
@ -261,6 +269,8 @@ export type TGitHubSkillSyncSourceStatus = {
|
|||
deletedFileCount: number;
|
||||
skippedSkillCount: number;
|
||||
skippedSkills?: TGitHubSkillSyncSkippedSkill[];
|
||||
skippedFileCount: number;
|
||||
skippedFiles?: TGitHubSkillSyncSkippedFile[];
|
||||
updatedAt?: string;
|
||||
createdAt?: string;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import type {
|
|||
ISkillSyncStatus,
|
||||
SkillSyncProvider,
|
||||
SkillSyncRunStatus,
|
||||
ISkillSyncSkippedFile,
|
||||
ISkillSyncSkippedSkill,
|
||||
ISkillSyncStatusDocument,
|
||||
ISkillSyncCredential,
|
||||
|
|
@ -49,6 +50,8 @@ export type SkillSyncStatusInput = {
|
|||
deletedFileCount?: number;
|
||||
skippedSkillCount?: number;
|
||||
skippedSkills?: ISkillSyncSkippedSkill[];
|
||||
skippedFileCount?: number;
|
||||
skippedFiles?: ISkillSyncSkippedFile[];
|
||||
};
|
||||
|
||||
export type SkillSyncLockInput = {
|
||||
|
|
@ -238,6 +241,8 @@ export function createSkillSyncMethods(mongoose: typeof import('mongoose')): Ski
|
|||
deletedFileCount: input.deletedFileCount ?? 0,
|
||||
skippedSkillCount: input.skippedSkillCount ?? 0,
|
||||
skippedSkills: input.skippedSkills ?? [],
|
||||
skippedFileCount: input.skippedFileCount ?? 0,
|
||||
skippedFiles: input.skippedFiles ?? [],
|
||||
...(success ? { lastSuccessAt: input.finishedAt ?? now } : {}),
|
||||
...(failure ? { lastFailureAt: input.finishedAt ?? now } : {}),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -28,3 +28,42 @@ describe('skillSyncStatusSchema', () => {
|
|||
expect(status.validateSync()?.errors['skippedSkills.0.path']?.message).toBe('Path is required');
|
||||
});
|
||||
});
|
||||
|
||||
describe('skillSyncStatusSchema skipped files', () => {
|
||||
it('accepts an empty skill path for a file dropped from a repository-root skill', () => {
|
||||
const status = new SkillSyncStatus({
|
||||
provider: 'github',
|
||||
sourceId: 'root-skills',
|
||||
status: 'partial',
|
||||
skippedFileCount: 1,
|
||||
skippedFiles: [
|
||||
{
|
||||
path: 'bad name',
|
||||
skillPath: '',
|
||||
errorCode: 'SKILL_FILE_PATH_UNSUPPORTED',
|
||||
errorMessage: 'File path uses characters that skill file paths cannot represent',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(status.validateSync()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('still rejects a skipped file without a path', () => {
|
||||
const status = new SkillSyncStatus({
|
||||
provider: 'github',
|
||||
sourceId: 'root-skills',
|
||||
status: 'partial',
|
||||
skippedFileCount: 1,
|
||||
skippedFiles: [
|
||||
{
|
||||
skillPath: 'skills/research',
|
||||
errorCode: 'SKILL_FILE_PATH_UNSUPPORTED',
|
||||
errorMessage: 'File path uses characters that skill file paths cannot represent',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(status.validateSync()?.errors['skippedFiles.0.path']?.message).toBe('Path is required');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import { Schema } from 'mongoose';
|
||||
import type { ISkillSyncSkippedSkill, ISkillSyncStatusDocument } from '~/types/skillSync';
|
||||
import type {
|
||||
ISkillSyncSkippedFile,
|
||||
ISkillSyncSkippedSkill,
|
||||
ISkillSyncStatusDocument,
|
||||
} from '~/types/skillSync';
|
||||
|
||||
const skippedSkillSchema = new Schema<ISkillSyncSkippedSkill>(
|
||||
{
|
||||
|
|
@ -30,6 +34,43 @@ const skippedSkillSchema = new Schema<ISkillSyncSkippedSkill>(
|
|||
{ _id: false },
|
||||
);
|
||||
|
||||
const skippedFileSchema = new Schema<ISkillSyncSkippedFile>(
|
||||
{
|
||||
path: {
|
||||
type: String,
|
||||
default: null,
|
||||
maxlength: 500,
|
||||
validate: {
|
||||
validator: (value: unknown) => typeof value === 'string',
|
||||
message: 'Path is required',
|
||||
},
|
||||
},
|
||||
/* A skill mirrored from the repository root has an empty root path, so this
|
||||
is validated for presence rather than marked `required`, which rejects the
|
||||
empty string. Same reason `skippedSkills.path` is written this way. */
|
||||
skillPath: {
|
||||
type: String,
|
||||
default: null,
|
||||
maxlength: 500,
|
||||
validate: {
|
||||
validator: (value: unknown) => typeof value === 'string',
|
||||
message: 'Skill path is required',
|
||||
},
|
||||
},
|
||||
errorCode: {
|
||||
type: String,
|
||||
required: true,
|
||||
maxlength: 64,
|
||||
},
|
||||
errorMessage: {
|
||||
type: String,
|
||||
required: true,
|
||||
maxlength: 500,
|
||||
},
|
||||
},
|
||||
{ _id: false },
|
||||
);
|
||||
|
||||
const skillSyncStatusSchema: Schema<ISkillSyncStatusDocument> = new Schema(
|
||||
{
|
||||
provider: {
|
||||
|
|
@ -117,6 +158,15 @@ const skillSyncStatusSchema: Schema<ISkillSyncStatusDocument> = new Schema(
|
|||
type: [skippedSkillSchema],
|
||||
default: undefined,
|
||||
},
|
||||
skippedFileCount: {
|
||||
type: Number,
|
||||
default: 0,
|
||||
min: 0,
|
||||
},
|
||||
skippedFiles: {
|
||||
type: [skippedFileSchema],
|
||||
default: undefined,
|
||||
},
|
||||
lockOwner: {
|
||||
type: String,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -2,10 +2,11 @@ import type { Document, Types } from 'mongoose';
|
|||
|
||||
export type SkillSyncProvider = 'github';
|
||||
/**
|
||||
* `partial` means the source published at least one skill while dropping
|
||||
* others: a single unusable `SKILL.md` must not hide the skills that synced
|
||||
* fine, and a run that quietly reported `succeeded` would hide the ones that
|
||||
* did not.
|
||||
* `partial` means the source published at least one skill while dropping some
|
||||
* of what it was asked to mirror — an unusable `SKILL.md`, or a file whose path
|
||||
* cannot be represented as a skill file path. A single bad skill must not hide
|
||||
* the ones that synced fine, and a run that quietly reported `succeeded` would
|
||||
* hide whatever it dropped.
|
||||
*/
|
||||
export type SkillSyncRunStatus =
|
||||
| 'idle'
|
||||
|
|
@ -25,6 +26,20 @@ export interface ISkillSyncSkippedSkill {
|
|||
errorMessage: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* One upstream file a run published a skill without. Unlike a skipped skill,
|
||||
* the skill itself is live — it is just missing this file, which is invisible
|
||||
* from the mirrored copy alone and so has to be recorded here.
|
||||
*/
|
||||
export interface ISkillSyncSkippedFile {
|
||||
/** Repository path of the file that was dropped. */
|
||||
path: string;
|
||||
/** Repository path of the skill root it belongs to. */
|
||||
skillPath: string;
|
||||
errorCode: string;
|
||||
errorMessage: string;
|
||||
}
|
||||
|
||||
export interface ISkillSyncCredential {
|
||||
provider: SkillSyncProvider;
|
||||
credentialKey: string;
|
||||
|
|
@ -61,6 +76,9 @@ export interface ISkillSyncStatus {
|
|||
skippedSkillCount: number;
|
||||
/** Capped sample of the skipped skills; `skippedSkillCount` is the full total. */
|
||||
skippedSkills?: ISkillSyncSkippedSkill[];
|
||||
skippedFileCount: number;
|
||||
/** Capped sample of the skipped files; `skippedFileCount` is the full total. */
|
||||
skippedFiles?: ISkillSyncSkippedFile[];
|
||||
lockOwner?: string;
|
||||
lockExpiresAt?: Date;
|
||||
createdAt?: Date;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue