Q34: built-in skill updates reach existing orgs and their bindings
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
66de4ef015
commit
da072f9a9b
5 files changed
+102
-16
No files matched your search
+2
-2
@@ -102,8 +102,8 @@ function syncAllOrganizationSkills(connection: DatabaseConnection, artifactRoot:
|
||||
const orgs = connection.db.prepare("select id from organizations").all() as Array<{ id: string }>;
|
||||
for (const org of orgs) {
|
||||
const report = connection.transaction(() => importStagedSkills(skills, org.id, staged));
|
||||
if (report.imported > 0 || report.retired > 0 || report.failures.length > 0) {
|
||||
console.info(`[skills] org ${org.id}: imported ${report.imported}, skipped ${report.skipped}, retired ${report.retired}, failures ${report.failures.length}`);
|
||||
if (report.imported > 0 || report.upgraded > 0 || report.retired > 0 || report.failures.length > 0) {
|
||||
console.info(`[skills] org ${org.id}: imported ${report.imported}, upgraded ${report.upgraded}, skipped ${report.skipped}, retired ${report.retired}, failures ${report.failures.length}`);
|
||||
for (const failure of report.failures.slice(0, 5)) console.warn(`[skills] ${failure}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ export function listSkillsTool(deps: AgentToolDeps, ctx: AgentToolContext): stri
|
||||
select sd.id, sd.name, sd.slug, sr.manifest_json
|
||||
from skill_definitions sd
|
||||
left join skill_revisions sr on sr.id = (
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, id desc limit 1
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, rowid desc limit 1
|
||||
)
|
||||
where sd.org_id = ? and sd.status = 'active'
|
||||
order by sd.slug asc
|
||||
@@ -69,7 +69,7 @@ export function grantSkillTool(deps: AgentToolDeps, ctx: AgentToolContext, args:
|
||||
const skillService = new SkillService(deps.db, "");
|
||||
if (mode === "grant") {
|
||||
const revision = deps.db.prepare(
|
||||
"select id from skill_revisions where skill_id = ? order by published_at desc, id desc limit 1",
|
||||
"select id from skill_revisions where skill_id = ? order by published_at desc, rowid desc limit 1",
|
||||
).get(skill.id) as { id: string } | undefined;
|
||||
if (!revision) return `ERROR: The "${skillSlug}" skill has no published revision.`;
|
||||
skillService.bind({ orgId: ctx.orgId, agentId: target.id, skillId: skill.id, revisionId: revision.id });
|
||||
@@ -94,7 +94,7 @@ export function bindSkillSlugs(deps: AgentToolDeps, orgId: string, agentId: stri
|
||||
select sd.id as skill_id, sd.name, sr.id as revision_id
|
||||
from skill_definitions sd
|
||||
join skill_revisions sr on sr.id = (
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, id desc limit 1
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, rowid desc limit 1
|
||||
)
|
||||
where sd.org_id = ? and sd.slug = ? and sd.status = 'active'
|
||||
`).get(orgId, slug) as { skill_id: string; name: string; revision_id: string } | undefined;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { cpSync, existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { cpSync, existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { RISK_LEVEL } from "../../shared/domain";
|
||||
import { loadSkillManifest, type SkillManifest } from "./skill-manifest";
|
||||
@@ -17,7 +17,7 @@ import type { SkillService } from "./skill-service";
|
||||
* Staging normalizes both into one library layout the import machinery
|
||||
* understands — `<libraryRoot>/<slug>/SKILL.md` — a real copy on disk, never a
|
||||
* reference back into the bundle. Import then publishes every staged skill the
|
||||
* org does not carry yet into the immutable artifact store.
|
||||
* org does not carry yet, or whose files changed, into the immutable artifact store.
|
||||
*/
|
||||
|
||||
export interface StagedBuiltinSkill {
|
||||
@@ -28,6 +28,8 @@ export interface StagedBuiltinSkill {
|
||||
|
||||
export interface BuiltinSkillSyncReport {
|
||||
imported: number;
|
||||
/** Existing skills that got a new revision because the bundle changed them. */
|
||||
upgraded: number;
|
||||
skipped: number;
|
||||
/** Active skills retired because the bundle no longer carries their slug. */
|
||||
retired: number;
|
||||
@@ -73,17 +75,18 @@ export function stageBuiltinSkillsLibrary(sourceRoot: string, libraryRoot: strin
|
||||
|
||||
/**
|
||||
* Makes the org catalogue mirror the bundle: publishes every staged skill
|
||||
* the org does not carry yet, then retires active skills whose slug the
|
||||
* bundle no longer ships (legacy ~/.agents imports, removed skills).
|
||||
* Existing bindings pin revisions directly and keep working; retired skills
|
||||
* simply leave list_skills and stop being grantable.
|
||||
* the org does not carry yet, upgrades skills whose files changed (moving
|
||||
* bindings onto the new revision), then retires active skills whose slug
|
||||
* the bundle no longer ships. Retired skills leave list_skills and stop
|
||||
* being grantable.
|
||||
*/
|
||||
export function importStagedSkills(skills: SkillService, orgId: string, staged: StagedBuiltinSkill[]): BuiltinSkillSyncReport {
|
||||
const report: BuiltinSkillSyncReport = { imported: 0, skipped: 0, retired: 0, failures: [] };
|
||||
const report: BuiltinSkillSyncReport = { imported: 0, upgraded: 0, skipped: 0, retired: 0, failures: [] };
|
||||
for (const unit of staged) {
|
||||
try {
|
||||
if (skills.hasSkill(orgId, unit.slug)) {
|
||||
report.skipped += 1;
|
||||
if (skills.upgradeIfChanged(orgId, unit.directory, unit.manifest, unit.slug)) report.upgraded += 1;
|
||||
else report.skipped += 1;
|
||||
continue;
|
||||
}
|
||||
skills.importManifest(orgId, unit.directory, unit.manifest, unit.slug);
|
||||
@@ -99,7 +102,8 @@ export function importStagedSkills(skills: SkillService, orgId: string, staged:
|
||||
function stageDirectorySkill(sourceDir: string, dirName: string, libraryRoot: string, seen: Set<string>): StagedBuiltinSkill {
|
||||
const slug = uniqueSlug(slugify(dirName), seen, sourceDir);
|
||||
const target = join(libraryRoot, slug);
|
||||
mkdirSync(target, { recursive: true });
|
||||
// Clear first so files the bundle dropped do not linger in the library.
|
||||
rmSync(target, { recursive: true, force: true });
|
||||
cpSync(sourceDir, target, { recursive: true });
|
||||
const manifest = existsSync(join(target, "skill.manifest.json"))
|
||||
? loadSkillManifest(target)
|
||||
@@ -119,6 +123,7 @@ function stageCategorySkills(categoryDir: string, libraryRoot: string, seen: Set
|
||||
const stem = entry.name.replace(/\.md$/i, "");
|
||||
const slug = uniqueSlug(slugify(stem), seen, join(categoryDir, entry.name));
|
||||
const target = join(libraryRoot, slug);
|
||||
rmSync(target, { recursive: true, force: true });
|
||||
mkdirSync(target, { recursive: true });
|
||||
writeFileSync(join(target, "SKILL.md"), body, "utf8");
|
||||
staged.push({
|
||||
|
||||
@@ -83,6 +83,39 @@ export class SkillService {
|
||||
return retired;
|
||||
}
|
||||
|
||||
/** Publishes a new revision when the files changed and moves bindings on the latest revision onto it. */
|
||||
upgradeIfChanged(orgId: string, directoryPath: string, manifest: SkillManifest, slug: string): boolean {
|
||||
const latest = this.db.prepare(`
|
||||
select sr.id, sr.immutable_checksum from skill_definitions sd
|
||||
join skill_revisions sr on sr.skill_id = sd.id
|
||||
where sd.org_id = ? and sd.slug = ?
|
||||
order by sr.published_at desc, sr.rowid desc limit 1
|
||||
`).get(orgId, slug) as { id: string; immutable_checksum: string } | undefined;
|
||||
if (!latest) throw new Error(`Skill "${slug}" has no revision to upgrade.`);
|
||||
const checksum = recursiveSkillChecksum(directoryPath);
|
||||
if (latest.immutable_checksum === checksum) return false;
|
||||
const revisions = this.db.prepare(`
|
||||
select sr.version_semver from skill_revisions sr join skill_definitions sd on sd.id = sr.skill_id
|
||||
where sd.org_id = ? and sd.slug = ?
|
||||
`).all(orgId, slug) as Array<{ version_semver: string }>;
|
||||
// Same declared version, new content: tag the build so revisions stay distinct.
|
||||
const versionTaken = revisions.some((row) => row.version_semver === manifest.version);
|
||||
const published = versionTaken ? { ...manifest, version: `${manifest.version}+rev${revisions.length + 1}` } : manifest;
|
||||
const revisionId = this.importManifest(orgId, directoryPath, published, slug);
|
||||
const moved = this.db.prepare("update skill_bindings set pinned_revision_id = ? where org_id = ? and pinned_revision_id = ?")
|
||||
.run(revisionId, orgId, latest.id);
|
||||
this.audit.record({
|
||||
orgId,
|
||||
entityType: "skill_revision",
|
||||
entityId: revisionId,
|
||||
actorType: "system",
|
||||
actorId: null,
|
||||
action: "upgraded",
|
||||
payload: { slug, fromRevisionId: latest.id, version: published.version, bindingsMoved: Number(moved.changes) },
|
||||
});
|
||||
return true;
|
||||
}
|
||||
|
||||
importManifest(orgId: string, directoryPath: string, manifest: SkillManifest, explicitSlug?: string): string {
|
||||
const slug = explicitSlug ?? slugify(manifest.name);
|
||||
if (!slug) throw new Error("Skill name could not be derived.");
|
||||
@@ -216,7 +249,7 @@ export class SkillService {
|
||||
sa.object_key
|
||||
from skill_definitions sd
|
||||
join skill_revisions sr on sr.id = (
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, id desc limit 1
|
||||
select id from skill_revisions where skill_id = sd.id order by published_at desc, rowid desc limit 1
|
||||
)
|
||||
join skill_artifacts sa on sa.revision_id = sr.id and sa.artifact_type in ('entrypoint', 'markdown')
|
||||
where sd.org_id = ? and sd.status = 'active' and sd.slug in (${candidates.map(() => "?").join(", ")})
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { existsSync, mkdirSync, writeFileSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, test } from "vitest";
|
||||
@@ -106,6 +106,54 @@ describe("built-in skills", () => {
|
||||
h.connection.close();
|
||||
});
|
||||
|
||||
test("an app update that changes a built-in skill reaches existing orgs and their bindings", () => {
|
||||
const h = harness();
|
||||
h.connection.transaction(() => importStagedSkills(h.skills, h.orgId, stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot)));
|
||||
const skillId = (h.connection.db.prepare("select id from skill_definitions where org_id = ? and slug = 'engineering-backend-architect'").get(h.orgId) as { id: string }).id;
|
||||
const oldRevision = (h.connection.db.prepare("select id from skill_revisions where skill_id = ?").get(skillId) as { id: string }).id;
|
||||
let agentId = "";
|
||||
h.connection.transaction(() => {
|
||||
agentId = h.organization.createAgent({
|
||||
orgId: h.orgId, name: "Ada", role: "Engineer", department: "Engineering",
|
||||
modelProviderId: null, modelId: null, budgetCents: 1000, managerAgentId: null,
|
||||
});
|
||||
h.skills.bind({ orgId: h.orgId, agentId, skillId, revisionId: oldRevision });
|
||||
});
|
||||
|
||||
writeFileSync(join(h.sourceRoot, "engineering", "engineering-backend-architect.md"), ARCHITECT_MD.replace("hold everything up", "scale to millions"), "utf8");
|
||||
rmSync(join(h.sourceRoot, "humanizer", "README.md"));
|
||||
const report = h.connection.transaction(() => importStagedSkills(h.skills, h.orgId, stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot)));
|
||||
|
||||
expect(report).toMatchObject({ imported: 0, upgraded: 2, skipped: 1, retired: 0, failures: [] });
|
||||
const bound = h.connection.db.prepare(`
|
||||
select sa.object_key from skill_bindings sb join skill_artifacts sa on sa.revision_id = sb.pinned_revision_id and sa.artifact_type = 'entrypoint'
|
||||
where sb.agent_id = ?
|
||||
`).get(agentId) as { object_key: string };
|
||||
expect(readFileSync(bound.object_key, "utf8")).toContain("scale to millions");
|
||||
const humanizerFiles = h.connection.db.prepare(`
|
||||
select sa.object_key from skill_definitions sd
|
||||
join skill_revisions sr on sr.id = (select id from skill_revisions where skill_id = sd.id order by published_at desc, id desc limit 1)
|
||||
join skill_artifacts sa on sa.revision_id = sr.id
|
||||
where sd.org_id = ? and sd.slug = 'humanizer'
|
||||
`).all(h.orgId) as Array<{ object_key: string }>;
|
||||
const architectPath = join(h.sourceRoot, "engineering", "engineering-backend-architect.md");
|
||||
for (const body of [ARCHITECT_MD.replace("hold everything up", "stay up"), ARCHITECT_MD.replace("hold everything up", "scale to millions")]) {
|
||||
writeFileSync(architectPath, body, "utf8");
|
||||
const again = h.connection.transaction(() => importStagedSkills(h.skills, h.orgId, stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot)));
|
||||
expect(again).toMatchObject({ upgraded: 1, failures: [] });
|
||||
}
|
||||
const versions = (h.connection.db.prepare("select version_semver from skill_revisions where skill_id = ? order by published_at, rowid").all(skillId) as Array<{ version_semver: string }>)
|
||||
.map((row) => row.version_semver);
|
||||
expect(versions).toEqual(["1.0.0", "1.0.0+rev2", "1.0.0+rev3", "1.0.0+rev4"]);
|
||||
const rebound = h.connection.db.prepare(`
|
||||
select sa.object_key from skill_bindings sb join skill_artifacts sa on sa.revision_id = sb.pinned_revision_id and sa.artifact_type = 'entrypoint'
|
||||
where sb.agent_id = ?
|
||||
`).get(agentId) as { object_key: string };
|
||||
expect(readFileSync(rebound.object_key, "utf8")).toContain("scale to millions");
|
||||
expect(humanizerFiles.map((row) => row.object_key.replaceAll("\\", "/").split("/").at(-1))).toEqual(["SKILL.md"]);
|
||||
h.connection.close();
|
||||
});
|
||||
|
||||
test("legacy catalogue entries retire so every install mirrors the bundle, while their bindings keep working", () => {
|
||||
const h = harness();
|
||||
const staged = stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot);
|
||||
|
||||
Reference in new issue
Block a user