diff --git a/src/main/services/builtin-skills.ts b/src/main/services/builtin-skills.ts index 137c0b2a..09e3cbeb 100644 --- a/src/main/services/builtin-skills.ts +++ b/src/main/services/builtin-skills.ts @@ -80,7 +80,7 @@ export function stageBuiltinSkillsLibrary(sourceRoot: string, libraryRoot: strin * the org does not carry yet, restores retired skills the bundle ships again, * 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. + * ships. Retired skills leave list_skills, stop being grantable, and stop loading into runs. */ export function importStagedSkills(skills: SkillService, orgId: string, staged: StagedBuiltinSkill[]): BuiltinSkillSyncReport { const report: BuiltinSkillSyncReport = { imported: 0, upgraded: 0, restored: 0, skipped: 0, retired: 0, failures: [] }; diff --git a/src/main/services/skill-service.ts b/src/main/services/skill-service.ts index 0d3c0019..50096de4 100644 --- a/src/main/services/skill-service.ts +++ b/src/main/services/skill-service.ts @@ -57,9 +57,9 @@ export class SkillService { /** * Retires every active skill whose slug is not in `keepSlugs`, so the org - * catalogue mirrors the shipped bundle. Bindings pin revisions directly - * and keep resolving; retired skills leave list_skills and cannot be - * granted anew. Returns the retired slugs. + * catalogue mirrors the shipped bundle. Retired skills leave list_skills, + * cannot be granted, and stop loading into runs; their bindings stay so a + * restore brings them back. Returns the retired slugs. */ retireSkillsNotIn(orgId: string, keepSlugs: string[]): string[] { const keep = new Set(keepSlugs); @@ -245,9 +245,10 @@ export class SkillService { */ hasRuntimeSkills(orgId: string, agentId: string): boolean { if (this.forcedSkillRows(orgId).length > 0) return true; - return Boolean(this.db.prepare( - "select id from skill_bindings where org_id = ? and agent_id = ? and enabled = 1 limit 1", - ).get(orgId, agentId)); + return Boolean(this.db.prepare(` + select sb.id from skill_bindings sb join skill_definitions sd on sd.id = sb.skill_id and sd.status = 'active' + where sb.org_id = ? and sb.agent_id = ? and sb.enabled = 1 limit 1 + `).get(orgId, agentId)); } /** @@ -279,6 +280,14 @@ export class SkillService { return []; } + private carriesRetiredSkill(revisionIds: string[]): boolean { + if (revisionIds.length === 0) return false; + return Boolean(this.db.prepare(` + select 1 from skill_revisions sr join skill_definitions sd on sd.id = sr.skill_id + where sd.status <> 'active' and sr.id in (${revisionIds.map(() => "?").join(", ")}) limit 1 + `).get(...revisionIds)); + } + private appLanguage(): string { const row = this.db.prepare("select language from app_state where id = 1").get() as { language: string } | undefined; return row?.language === "zh" ? "zh" : "en"; @@ -292,12 +301,13 @@ export class SkillService { * prompt instead of silently overflowing the model's context. */ resolveRuntimeSnapshot(input: ResolveAgentSkillsInput): string { - // Idempotent per run: a run interrupted after resolving (crash, quit) is - // re-queued by startup recovery and must reuse its frozen skill set, not - // die on a uniqueness error that burns real retry budget. - const existingSnapshot = this.db.prepare("select id from skill_runtime_snapshots where agent_id = ? and run_id = ?") - .get(input.agentId, input.runId) as { id: string } | undefined; - if (existingSnapshot) return existingSnapshot.id; + // Idempotent per run: a re-queued run reuses its frozen skill set unless a skill in it was retired since. + const existingSnapshot = this.db.prepare("select id, source_revision_ids from skill_runtime_snapshots where agent_id = ? and run_id = ?") + .get(input.agentId, input.runId) as { id: string; source_revision_ids: string } | undefined; + if (existingSnapshot) { + if (!this.carriesRetiredSkill(JSON.parse(existingSnapshot.source_revision_ids) as string[])) return existingSnapshot.id; + this.db.prepare("delete from skill_runtime_snapshots where id = ?").run(existingSnapshot.id); + } const forcedRows = this.forcedSkillRows(input.orgId); const boundRows = this.db.prepare(` select @@ -307,6 +317,7 @@ export class SkillService { sr.manifest_json, sa.object_key from skill_bindings sb + join skill_definitions sd on sd.id = sb.skill_id and sd.status = 'active' join skill_revisions sr on sr.id = sb.pinned_revision_id join skill_artifacts sa on sa.revision_id = sr.id and sa.artifact_type in ('entrypoint', 'markdown') where sb.org_id = ? and sb.agent_id = ? and sb.enabled = 1 diff --git a/tests/unit/builtin-skills.test.ts b/tests/unit/builtin-skills.test.ts index a1ecc900..5128373b 100644 --- a/tests/unit/builtin-skills.test.ts +++ b/tests/unit/builtin-skills.test.ts @@ -154,22 +154,45 @@ describe("built-in skills", () => { h.connection.close(); }); - test("a skill the bundle drops and later ships again comes back active", () => { + test("a skill the bundle drops stops applying to bound agents, and comes back when shipped again", () => { const h = harness(); const sync = () => h.connection.transaction(() => importStagedSkills(h.skills, h.orgId, stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot))); sync(); + const skill = h.connection.db.prepare(` + select sd.id, sr.id as revision_id from skill_definitions sd join skill_revisions sr on sr.skill_id = sd.id + where sd.org_id = ? and sd.slug = 'engineering-backend-architect' + `).get(h.orgId) as { id: string; revision_id: string }; + 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: skill.id, revisionId: skill.revision_id }); + }); + const prompt = (runId: string) => { + const id = h.connection.transaction(() => h.skills.resolveRuntimeSnapshot({ orgId: h.orgId, agentId, runId, maxTokens: SKILL_TOKENS, taskText: "" })); + return (h.connection.db.prepare("select compiled_prompt from skill_runtime_snapshots where id = ?").get(id) as { compiled_prompt: string }).compiled_prompt; + }; + expect(prompt("run_before")).toContain("# Skill: Backend Architect"); + const architectPath = join(h.sourceRoot, "engineering", "engineering-backend-architect.md"); rmSync(architectPath); expect(sync()).toMatchObject({ retired: 1 }); + expect(prompt("run_retired")).not.toContain("# Skill: Backend Architect"); + expect(prompt("run_retired")).toContain("# Skill: humanizer"); + expect(prompt("run_before")).not.toContain("# Skill: Backend Architect"); + writeFileSync(architectPath, ARCHITECT_MD, "utf8"); expect(sync()).toMatchObject({ imported: 0, restored: 1, upgraded: 0, retired: 0, failures: [] }); + expect(prompt("run_restored")).toContain("# Skill: Backend Architect"); expect(h.connection.db.prepare("select status from skill_definitions where org_id = ? and slug = 'engineering-backend-architect'").get(h.orgId)) .toEqual({ status: "active" }); expect(sync()).toMatchObject({ restored: 0, skipped: 3 }); h.connection.close(); }); - test("legacy catalogue entries retire so every install mirrors the bundle, while their bindings keep working", () => { + test("legacy catalogue entries retire so every install mirrors the bundle", () => { const h = harness(); const staged = stageBuiltinSkillsLibrary(h.sourceRoot, h.libraryRoot); h.connection.transaction(() => importStagedSkills(h.skills, h.orgId, staged));