feat: make GitHub Skill Sync refreshes version-safe (#3229)

This commit is contained in:
Patrick Erichsen
2026-07-29 23:26:05 -07:00
committed by GitHub
parent 9784710147
commit 49771f5a69
16 changed files with 2682 additions and 196 deletions
+3 -3
View File
@@ -326,12 +326,12 @@ describe("Agent Skills discovery HTTP handler", () => {
})
.mockResolvedValueOnce({
githubSourceId: "githubSkillSources:demo",
repo: "openclaw/openclaw",
contentHash,
commit: "def456",
commit: "abc123",
path: "skills/demo",
status: "clean",
})
.mockResolvedValueOnce({ repo: "openclaw/openclaw", defaultBranch: "main" });
});
const response = await agentSkillsHttpHandler(
makeCtx({ runQuery, storage: { get: vi.fn() } }),
+5 -8
View File
@@ -202,10 +202,12 @@ async function resolveSkill(
internal.githubSkillSync.getArchiveScanBySkillAndContentHashInternal,
{
skillId: skill._id,
commit: archivePin.commit,
contentHash: archivePin.contentHash,
},
)) as {
githubSourceId: Id<"githubSkillSources">;
repo: string;
contentHash: string;
commit: string;
path: string;
@@ -213,17 +215,12 @@ async function resolveSkill(
} | null;
if (
!scan ||
scan.commit !== archivePin.commit ||
scan.contentHash !== archivePin.contentHash ||
(scan.status !== "clean" && scan.status !== "suspicious")
) {
return { ok: false, status: 404, message: "GitHub skill archive not available" };
}
const source = (await ctx.runQuery(internal.githubSkillSources.getByIdInternal, {
sourceId: scan.githubSourceId,
})) as InstallResolverSource | null;
if (!source) {
return { ok: false, status: 404, message: "GitHub skill archive not available" };
}
const moderationBlock = getPublicSkillFileAccessBlock(publicResult.moderationInfo);
if (moderationBlock) {
return {
@@ -237,11 +234,11 @@ async function resolveSkill(
slug: skill.slug,
installKind: "github",
github: {
repo: source.repo,
repo: scan.repo,
path: scan.path,
commit: archivePin.commit,
contentHash: scan.contentHash,
sourceUrl: `https://github.com/${source.repo}/tree/${archivePin.commit}/${scan.path}`,
sourceUrl: `https://github.com/${scan.repo}/tree/${archivePin.commit}/${scan.path}`,
},
};
} else {
+14 -5
View File
@@ -469,10 +469,19 @@ describe("catalog feed projection", () => {
it("projects current GitHub-backed skills into public GitHub install candidates", async () => {
const result = (await listOfficialSkillEntriesHandler(
makeCtx([makeGitHubSkill({ slug: "aiq-deploy", displayName: "AIQ Deploy" })], {
"publishers:1": { _id: "publishers:1", kind: "org", handle: "nvidia" },
"githubSkillSources:1": makeGitHubSource(),
}),
makeCtx(
[
makeGitHubSkill({
slug: "aiq-deploy",
displayName: "AIQ Deploy",
githubCurrentRepo: "NVIDIA/skills-archive",
}),
],
{
"publishers:1": { _id: "publishers:1", kind: "org", handle: "nvidia" },
"githubSkillSources:1": makeGitHubSource({ repo: "NVIDIA/renamed-skills" }),
},
),
{ publisherId: "publishers:1", cursor: null },
)) as { entries: unknown[]; isDone: boolean };
@@ -494,7 +503,7 @@ describe("catalog feed projection", () => {
version: "1111111111111111111111111111111111111111",
integrity: "sha256:hash-aiq-deploy",
github: {
repo: "NVIDIA/skills",
repo: "NVIDIA/skills-archive",
path: "skills/aiq-deploy",
commit: "1111111111111111111111111111111111111111",
contentHash: "hash-aiq-deploy",
+1 -1
View File
@@ -295,7 +295,7 @@ async function buildSkillEntry(
const source = await ctx.db.get(skill.githubSourceId);
if (!source || source.ownerPublisherId !== skill.ownerPublisherId) return null;
const repo = source.repo.trim();
const repo = (skill.githubCurrentRepo ?? source.repo).trim();
const path = skill.githubPath.trim();
const commit = skill.githubCurrentCommit.trim();
const contentHash = skill.githubCurrentContentHash.trim();
+35 -11
View File
@@ -40,7 +40,11 @@ type WrappedHandler<TArgs, TResult = unknown> = {
const listForManageableOfficialPublishersHandler = (
listForManageableOfficialPublishers as unknown as WrappedHandler<
Record<string, never>,
Array<{ _id: string; repo: string; ownerPublisher: { handle: string } | null }>
Array<{
_id: string;
repo: string;
ownerPublisher: { handle: string } | null;
}>
>
)._handler;
@@ -210,7 +214,9 @@ function makeAliasSkill(id: string, githubPath = "skills/html"): Row {
describe("githubSkillSources.deleteForPublisherHandler", () => {
beforeEach(() => {
vi.mocked(requireUser).mockResolvedValue({ userId: "users:owner" } as never);
vi.mocked(requireUser).mockResolvedValue({
userId: "users:owner",
} as never);
vi.mocked(requirePublisherRole).mockResolvedValue(undefined as never);
});
@@ -240,7 +246,7 @@ describe("githubSkillSources.deleteForPublisherHandler", () => {
expect(scheduler.runAfter).not.toHaveBeenCalled();
});
it("deletes a source and removes only GitHub-backed skills from that source", async () => {
it("disconnects a source and removes only GitHub-backed skills from that source", async () => {
const { db, tables } = createDb({
githubSkillSources: [
{
@@ -248,7 +254,7 @@ describe("githubSkillSources.deleteForPublisherHandler", () => {
repo: "mattpocock/skills",
ownerPublisherId: "publishers:openclaw",
createdAt: 1,
updatedAt: 2,
updatedAt: 123,
},
],
githubSkillContents: [
@@ -280,7 +286,9 @@ describe("githubSkillSources.deleteForPublisherHandler", () => {
githubPath: "skills/hosted-candidate",
githubCommit: "c".repeat(40),
githubContentHash: "hash-hosted-candidate",
scanStatus: "pending",
scanStatus: "failed",
lifecycleStatus: "failed",
failedAt: 100,
},
],
skills: [
@@ -367,13 +375,27 @@ describe("githubSkillSources.deleteForPublisherHandler", () => {
allowed: ["admin"],
}),
);
expect(tables.githubSkillSources).toHaveLength(0);
expect(tables.githubSkillSources).toEqual([
expect.objectContaining({
_id: "githubSkillSources:matt",
disconnectedOwnerPublisherId: "publishers:openclaw",
authorizationStatus: "revoked",
authorizationCheckedAt: 123,
updatedAt: 124,
}),
]);
expect(tables.githubSkillSources[0]).not.toHaveProperty("ownerPublisherId");
expect(tables.githubSkillContents).toHaveLength(0);
expect(tables.githubSkillCandidates).toHaveLength(0);
expect(tables.githubSkillCandidates).toEqual([
expect.objectContaining({
_id: "githubSkillCandidates:hosted",
lifecycleStatus: "canceled",
canceledAt: 123,
cancellationReason: "github.source.disconnected",
}),
]);
expect(tables.githubSkillScans).toHaveLength(2);
expect(scheduler.runAfter).toHaveBeenCalledWith(0, expect.anything(), {
sourceId: "githubSkillSources:matt",
});
expect(scheduler.runAfter).not.toHaveBeenCalled();
const deletedSkill = tables.skills.find((skill) => skill._id === "skills:github");
expect(deletedSkill).toMatchObject({
softDeletedAt: 123,
@@ -469,7 +491,9 @@ describe("githubSkillSources.deleteForPublisherHandler", () => {
expect(tables.skillScanRequests?.[0]).not.toHaveProperty("githubSkillScanId");
expect(tables.skillScanRequests?.[0]).not.toHaveProperty("securityScanJobId");
expect(tables.skillScanRequests?.[0]?.expiresAt).toBeLessThan(Number.MAX_SAFE_INTEGER);
expect(scheduler.runAfter).toHaveBeenCalledWith(0, expect.anything(), { batchSize: 10 });
expect(scheduler.runAfter).toHaveBeenCalledWith(0, expect.anything(), {
batchSize: 10,
});
});
it("rejects deleting a source from another publisher", async () => {
+37 -10
View File
@@ -71,6 +71,7 @@ export const getSkillsShAliasTargetInternal = internalQuery({
const matches = skills.filter(
(skill) =>
skill.githubPath === path &&
(skill.githubCurrentRepo ?? source.repo).toLowerCase() === repo &&
skill.installKind === "github" &&
skill.githubCurrentStatus === "present" &&
(skill.githubScanStatus === "clean" || skill.githubScanStatus === "suspicious") &&
@@ -231,6 +232,7 @@ export async function deleteForPublisherHandler(
assertGenericGitHubSkillSyncEnabled(source.repo);
const now = args.now ?? Date.now();
const sourceUpdatedAt = Math.max(now, source.updatedAt + 1);
const contents = await ctx.db
.query("githubSkillContents")
.withIndex("by_github_source", (q) => q.eq("githubSourceId", args.sourceId))
@@ -238,10 +240,27 @@ export async function deleteForPublisherHandler(
for (const content of contents) {
await ctx.db.delete(content._id);
}
const candidates = await ctx.db
.query("githubSkillCandidates")
.withIndex("by_github_source", (q) => q.eq("githubSourceId", args.sourceId))
.collect();
const [pendingCandidates, failedCandidates, legacyCandidates] = await Promise.all([
ctx.db
.query("githubSkillCandidates")
.withIndex("by_github_source_and_lifecycle_status", (q) =>
q.eq("githubSourceId", args.sourceId).eq("lifecycleStatus", "pending"),
)
.collect(),
ctx.db
.query("githubSkillCandidates")
.withIndex("by_github_source_and_lifecycle_status", (q) =>
q.eq("githubSourceId", args.sourceId).eq("lifecycleStatus", "failed"),
)
.collect(),
ctx.db
.query("githubSkillCandidates")
.withIndex("by_github_source_and_lifecycle_status", (q) =>
q.eq("githubSourceId", args.sourceId).eq("lifecycleStatus", undefined),
)
.collect(),
]);
const candidates = [...pendingCandidates, ...failedCandidates, ...legacyCandidates];
for (const candidate of candidates) {
const skill = await ctx.db.get(candidate.skillId);
if (skill?.githubPendingCandidateId === candidate._id) {
@@ -249,13 +268,14 @@ export async function deleteForPublisherHandler(
githubPendingCandidateId: undefined,
updatedAt: now,
});
await ctx.db.patch(candidate._id, {
lifecycleStatus: "canceled",
canceledAt: now,
cancellationReason: "github.source.disconnected",
updatedAt: now,
});
}
await ctx.db.delete(candidate._id);
}
await ctx.scheduler.runAfter(0, internal.githubSkillSources.cleanupDeletedSourceScansInternal, {
sourceId: args.sourceId,
});
const skills = await ctx.db
.query("skills")
.withIndex("by_github_source", (q) => q.eq("githubSourceId", args.sourceId))
@@ -286,7 +306,14 @@ export async function deleteForPublisherHandler(
if (publicSkillDelta !== 0) {
await adjustGlobalPublicSkillsCount(ctx, publicSkillDelta, now);
}
await ctx.db.delete(args.sourceId);
await ctx.db.patch(args.sourceId, {
ownerPublisherId: undefined,
disconnectedOwnerPublisherId: args.ownerPublisherId,
authorizationStatus: "revoked",
authorizationCheckedAt: now,
authorizationError: "GitHub source disconnected by publisher.",
updatedAt: sourceUpdatedAt,
});
return { ok: true as const, deletedSkills };
}
File diff suppressed because it is too large Load Diff
+803 -89
View File
File diff suppressed because it is too large Load Diff
+5 -3
View File
@@ -13,6 +13,7 @@ export type InstallResolverSkill = {
displayName: string;
latestVersionSummary?: { version: string } | null;
installKind?: "github";
githubCurrentRepo?: string;
githubPath?: string;
githubCurrentCommit?: string;
githubCurrentContentHash?: string;
@@ -106,7 +107,8 @@ export function buildSkillInstallResolution({
if (isSecurityScanStatusBlockedFromPublic(skill.githubScanStatus)) {
return block(skill.slug, "github_scan_failed", 403);
}
if (!source || !skill.githubPath) {
const repo = skill.githubCurrentRepo ?? source?.repo;
if (!repo || !skill.githubPath) {
return block(skill.slug, "github_source_missing", 409);
}
if (
@@ -128,11 +130,11 @@ export function buildSkillInstallResolution({
slug: skill.slug,
installKind: "github",
github: {
repo: source.repo,
repo,
path: skill.githubPath,
commit: skill.githubCurrentCommit,
contentHash: skill.githubCurrentContentHash,
sourceUrl: buildGitHubTreeUrl(source.repo, skill.githubCurrentCommit, skill.githubPath),
sourceUrl: buildGitHubTreeUrl(repo, skill.githubCurrentCommit, skill.githubPath),
},
};
}
+9
View File
@@ -59,6 +59,15 @@ describe("retention policies", () => {
});
});
it("keeps GitHub Skill Sync candidates and verdicts as durable rollback history", () => {
expect(getRetentionPolicy("githubSkillCandidates")).toMatchObject({
classification: "permanent",
});
expect(getRetentionPolicy("githubSkillScans")).toMatchObject({
classification: "permanent",
});
});
it("documents publisher abuse signals as durable review evidence", () => {
expect(getRetentionPolicy("publisherAbuseSignals")).toMatchObject({
classification: "permanent",
+4 -2
View File
@@ -108,8 +108,10 @@ export const RETENTION_POLICIES = {
officialPublishers: permanent("Manual official publisher assignments."),
githubSkillSources: permanent("Tracked GitHub source configuration."),
githubSkillContents: derived("Cached GitHub source content snapshots.", "githubSkillSources"),
githubSkillCandidates: derived("Pending exact GitHub source candidates.", "githubSkillSources"),
githubSkillScans: derived("Cached GitHub source scan state.", "githubSkillSources"),
githubSkillCandidates: permanent(
"Immutable GitHub source candidate, promotion, rejection, and rollback history.",
),
githubSkillScans: permanent("Durable exact-content GitHub security verdict history."),
skills: permanent("Canonical skill records."),
skillSlugAliases: permanent("Historical slug routing aliases."),
packages: permanent("Canonical package records."),
+39 -1
View File
@@ -384,6 +384,9 @@ const githubSkillSourceIssueValidator = v.object({
const githubSkillSources = defineTable({
repo: v.string(),
ownerPublisherId: v.optional(v.id("publishers")),
// Retains the last owner across disconnects so generic sync cannot silently
// transfer a source by reconnecting its ownerless row.
disconnectedOwnerPublisherId: v.optional(v.id("publishers")),
githubRepositoryId: v.optional(v.string()),
githubOwnerId: v.optional(v.string()),
authorizationStatus: v.optional(v.union(v.literal("active"), v.literal("revoked"))),
@@ -503,6 +506,8 @@ const githubSkillCurrentStatusValidator = v.union(
const githubSkillCandidates = defineTable({
skillId: v.id("skills"),
githubSourceId: v.id("githubSkillSources"),
// Optional only for rows created before version-safe candidate history landed.
githubRepo: v.optional(v.string()),
githubPath: v.string(),
githubHasSkillCard: v.boolean(),
githubCommit: v.string(),
@@ -516,12 +521,43 @@ const githubSkillCandidates = defineTable({
skillCardMarkdownPath: v.optional(v.string()),
skillCardMarkdown: v.optional(v.string()),
scanStatus: githubSkillScanStatusValidator,
lifecycleStatus: v.optional(
v.union(
v.literal("pending"),
v.literal("promoted"),
v.literal("superseded"),
v.literal("rejected"),
v.literal("failed"),
v.literal("canceled"),
v.literal("rolled_back"),
),
),
verdictSourceScanId: v.optional(v.id("githubSkillScans")),
previousCandidateId: v.optional(v.id("githubSkillCandidates")),
supersededByCandidateId: v.optional(v.id("githubSkillCandidates")),
promotedAt: v.optional(v.number()),
supersededAt: v.optional(v.number()),
rejectedAt: v.optional(v.number()),
failedAt: v.optional(v.number()),
canceledAt: v.optional(v.number()),
rolledBackAt: v.optional(v.number()),
cancellationReason: v.optional(v.string()),
createdAt: v.number(),
updatedAt: v.number(),
})
.index("by_skill", ["skillId"])
.index("by_skill_and_content_hash", ["skillId", "githubContentHash"])
.index("by_github_source", ["githubSourceId"]);
.index("by_skill_and_commit_and_content_hash", ["skillId", "githubCommit", "githubContentHash"])
.index("by_skill_and_repo_source_commit_path_hash", [
"skillId",
"githubRepo",
"githubSourceId",
"githubCommit",
"githubPath",
"githubContentHash",
])
.index("by_github_source", ["githubSourceId"])
.index("by_github_source_and_lifecycle_status", ["githubSourceId", "lifecycleStatus"]);
const githubSkillScans = defineTable({
skillId: v.id("skills"),
@@ -842,6 +878,7 @@ const skills = defineTable({
forkOf: forkOfValidator,
installKind: v.optional(v.literal("github")),
githubSourceId: v.optional(v.id("githubSkillSources")),
githubCurrentRepo: v.optional(v.string()),
githubPath: v.optional(v.string()),
githubHasSkillCard: v.optional(v.boolean()),
githubCurrentCommit: v.optional(v.string()),
@@ -850,6 +887,7 @@ const skills = defineTable({
githubCurrentCheckedAt: v.optional(v.number()),
githubScanStatus: v.optional(githubSkillScanStatusValidator),
githubRemovedAt: v.optional(v.number()),
githubCurrentCandidateId: v.optional(v.id("githubSkillCandidates")),
githubPendingCandidateId: v.optional(v.id("githubSkillCandidates")),
latestVersionId: v.optional(v.id("skillVersions")),
latestVersionSummary: v.optional(
+8
View File
@@ -1824,6 +1824,9 @@ describe("securityScan", () => {
"githubSkillScans:github": {
_id: "githubSkillScans:github",
skillId: "skills:github",
githubSourceId: "githubSkillSources:github",
commit: "a".repeat(40),
path: "skills/github-skill",
contentHash: "content-hash",
status: "pending",
skillScanRequestId: "skillScanRequests:github",
@@ -2815,12 +2818,17 @@ describe("securityScan", () => {
"githubSkillScans:github": {
_id: "githubSkillScans:github",
skillId: "skills:github",
githubSourceId: "githubSkillSources:github",
contentHash: "content-hash",
commit: "a".repeat(40),
path: "skills/github-skill",
status: "failed",
},
"skills:github": {
_id: "skills:github",
installKind: "github",
githubSourceId: "githubSkillSources:github",
githubPath: "skills/github-skill",
githubCurrentStatus: "present",
githubCurrentCommit: "a".repeat(40),
githubCurrentContentHash: "content-hash",
+2
View File
@@ -2335,6 +2335,7 @@ export const recordGitHubSkillScanResultInternal = internalMutation({
return await applyGitHubSkillVerificationResultHandler(ctx, {
skillId: scan.skillId,
contentHash: scan.contentHash,
githubSkillScanId: scan._id,
scanStatus: args.scanStatus,
now,
});
@@ -3385,6 +3386,7 @@ export const requeueFailedSecurityScanJobsInternal = internalMutation({
await applyGitHubSkillVerificationResultHandler(ctx, {
skillId: scan.skillId,
contentHash: scan.contentHash,
githubSkillScanId: scan._id,
scanStatus: "pending",
now,
});
+7 -3
View File
@@ -2675,7 +2675,7 @@ export const getBySlug = query({
const forkOf = await loadPublicSkillReference(ctx, skill.forkOf?.skillId);
const canonical = await loadPublicSkillReference(ctx, skill.canonicalSkillId);
const githubSource = skill.githubSourceId ? await ctx.db.get(skill.githubSourceId) : null;
const githubSourceRepo = githubSource?.repo;
const githubSourceRepo = skill.githubCurrentRepo ?? githubSource?.repo;
const publicSkill = toPublicSkill({ ...skill, badges });
@@ -2844,7 +2844,7 @@ export const getGitHubDownloadTargetInternal = internalQuery({
return {
installKind: "github" as const,
repo: source?.repo ?? null,
repo: skill.githubCurrentRepo ?? source?.repo ?? null,
path: skill.githubPath ?? null,
commit: skill.githubCurrentCommit ?? null,
contentHash: skill.githubCurrentContentHash ?? null,
@@ -10289,7 +10289,11 @@ export const getGitHubSkillContent = query({
const source = await ctx.db.get(content.githubSourceId);
const resultSource = source
? buildGitHubMarkdownSourceBaseUrl(source.repo, content.githubCommit, content.githubPath)
? buildGitHubMarkdownSourceBaseUrl(
skill.githubCurrentRepo ?? source.repo,
content.githubCommit,
content.githubPath,
)
: undefined;
if (args.kind === "skill-card") {
+32 -13
View File
@@ -50,18 +50,21 @@ skills:
This table is not an install artifact store. OpenClaw must not install from
`githubSkillContents`.
`githubSkillCandidates` stores an exact pending replacement for a canonical
skill while its currently allowed source remains active:
`githubSkillCandidates` stores immutable exact replacement history for a
canonical skill while its currently allowed source remains active:
- canonical `skillId`
- immutable source repository, path, commit, and folder content hash
- bounded display Markdown fetched from that exact commit
- the ClawHub scan state for that exact content
- the ClawHub scan state and durable verdict-source scan identity for that
exact content
- lifecycle state for pending, promoted, superseded, rejected, failed,
canceled, and rolled-back observations
Candidates do not create `skillVersions`. A clean or suspicious exact verdict
promotes the candidate onto the existing skill row and then deletes the
candidate. Failed, malicious, stale, removed, or disconnected candidates never
replace the active source.
promotes only the active candidate bound to that verdict onto the existing skill
row. Candidate rows are retained for audit and rollback. Failed, malicious,
stale, removed, or disconnected candidates never replace the active source.
## Dark skills.sh discovery metadata
@@ -111,6 +114,7 @@ duplicated alias/adoption row is created for GitHub-backed content.
- `installKind: "github"`
- `githubSourceId`
- `githubCurrentRepo`
- `githubPath`
- `githubHasSkillCard`
- `githubCurrentCommit`
@@ -271,6 +275,17 @@ allowed. Promotion patches the existing canonical skill row in place, preserving
its slug, routes, ClawHub metrics, bookmarks, prior hosted versions, and audit
relationships.
Repository redirects, commit-only moves, and path-only moves use the same
candidate boundary even when the folder content hash is unchanged. The active
skill keeps its previously allowed repository/path/commit until the replacement
candidate can reuse or receive an allowed verdict. Source mutations carry the
source row version observed before fetching, so a slower stale observation
cannot overwrite a newer synchronized state. Before the first replacement is
created, an older allowed GitHub pointer is materialized into retained candidate
history so it remains available for audit, pinned archive resolution, and
rollback. Late content persistence also rechecks the active pointer before
writing, preventing same-content races from restoring an older commit.
When verification succeeds cleanly:
- persist the completed ClawScan, SkillSpector, and static findings on a durable
@@ -321,12 +336,13 @@ If the upstream path disappears:
The row may remain for audit/history, but users must not silently install an old
ClawHub-cached revision after upstream removed or changed it.
Removing a selected repository also removes its pending candidates before
deleting the source, so an in-flight callback cannot promote disconnected
content. Active source-backed skills become missing and hidden. Re-enrollment or
upstream reappearance may revive the same canonical skill row, but the
reappeared exact content returns to pending and must pass scanning before
installation.
Disconnecting a selected repository cancels its pending candidates and revokes
the source, so an in-flight callback cannot promote disconnected content. The
source, candidates, and verdict rows remain as audit history while the publisher
ownership link is cleared. Active source-backed skills become missing and
hidden. Re-enrollment or upstream reappearance may revive the same canonical
skill row; unchanged previously allowed content can reactivate idempotently,
while changed content returns through the candidate gate before installation.
## Install Resolver
@@ -354,7 +370,10 @@ returns:
```
OpenClaw downloads the GitHub archive for that commit and extracts only the skill
path. The local lock/origin version is the commit SHA.
path. The local lock/origin version is the commit SHA. Current descriptors and
detail links use the skill's promoted repository pointer, not a newer repository
name still waiting on candidate promotion. Pinned historical archives resolve
repository and path from retained promoted candidate history.
Controlled unclaimed skills.sh catalog entries use the repository-qualified
reference `skills-sh/<owner>/<repo>/<slug>`. The colon form