feat(new): harden skill revisions

AgentCompany-New/src/main/app/ipc-handlers.ts

@@ -26,7 +26,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
   const approvals = new ApprovalService(deps.connection.db);
   const chat = new ChatService(deps.connection.db);
   const providers = new ProviderService(deps.connection.db);
-  const skills = new SkillService(deps.connection.db);
+  const skills = new SkillService(deps.connection.db, deps.artifactRoot);
   const social = new SocialService(deps.connection.db);
   const files = new FileService(deps.connection.db, deps.artifactRoot);
   const migrations = new MigrationService(deps.connection.db);

AgentCompany-New/src/main/database/migrations/007-skill-hardening.ts

@@ -0,0 +1,4 @@
+export const skillHardeningSchema = `
+  create unique index if not exists idx_skill_runtime_snapshots_agent_run
+    on skill_runtime_snapshots(agent_id, run_id);
+`;

AgentCompany-New/src/main/database/schema.ts

@@ -5,6 +5,7 @@ import { onboardingExecutionSchema } from "./migrations/003-onboarding";
 import { filesPinnedAndScheduledPosts } from "./migrations/004-files-posts";
 import { chatReadState } from "./migrations/005-chat-read-state";
 import { appStateSchema } from "./migrations/006-app-state";
+import { skillHardeningSchema } from "./migrations/007-skill-hardening";
 
 const MIGRATIONS: Array<{ version: number; sql: string }> = [
   { version: 1, sql: initialEnterpriseSchema },
@@ -13,6 +14,7 @@ const MIGRATIONS: Array<{ version: number; sql: string }> = [
   { version: 4, sql: filesPinnedAndScheduledPosts },
   { version: 5, sql: chatReadState },
   { version: 6, sql: appStateSchema },
+  { version: 7, sql: skillHardeningSchema },
 ];
 
 const SCHEMA_VERSION = MIGRATIONS[MIGRATIONS.length - 1].version;

AgentCompany-New/src/main/services/skill-artifacts.ts

@@ -0,0 +1,85 @@
+import { copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, statSync } from "node:fs";
+import { createHash } from "node:crypto";
+import { dirname, isAbsolute, join, normalize, relative, resolve, sep } from "node:path";
+
+export interface CopiedSkillFile {
+  relativePath: string;
+  objectKey: string;
+  sha256: string;
+  sizeBytes: number;
+}
+
+export function copySkillRevision(sourceDir: string, artifactRoot: string, revisionId: string): CopiedSkillFile[] {
+  const sourceRoot = resolve(sourceDir);
+  if (!existsSync(sourceRoot) || !lstatSync(sourceRoot).isDirectory()) {
+    throw new Error("Skill directory was not found.");
+  }
+  const targetRoot = join(artifactRoot, "skills", revisionId);
+  const files = listSkillFiles(sourceRoot);
+  mkdirSync(targetRoot, { recursive: true });
+  return files.map((relativePath) => {
+    const sourcePath = pathFromRelative(sourceRoot, relativePath);
+    const targetPath = pathFromRelative(targetRoot, relativePath);
+    mkdirSync(dirname(targetPath), { recursive: true });
+    copyFileSync(sourcePath, targetPath);
+    return {
+      relativePath,
+      objectKey: targetPath,
+      sha256: fileChecksum(targetPath),
+      sizeBytes: statSync(targetPath).size,
+    };
+  });
+}
+
+export function recursiveSkillChecksum(sourceDir: string): string {
+  const sourceRoot = resolve(sourceDir);
+  const hash = createHash("sha256");
+  for (const relativePath of listSkillFiles(sourceRoot)) {
+    hash.update(relativePath);
+    hash.update(readFileSync(pathFromRelative(sourceRoot, relativePath)));
+  }
+  return hash.digest("hex");
+}
+
+export function assertSafeSkillPath(relativePath: string): string {
+  const clean = relativePath.trim().replaceAll("\\", "/");
+  if (!clean || isAbsolute(clean)) throw new Error(`Invalid skill artifact path: ${relativePath}`);
+  const normalized = normalize(clean).replaceAll("\\", "/");
+  if (normalized === "." || normalized.startsWith("../") || normalized.includes("/../")) {
+    throw new Error(`Invalid skill artifact path: ${relativePath}`);
+  }
+  return normalized;
+}
+
+function listSkillFiles(root: string): string[] {
+  const files: string[] = [];
+  walk(root, root, files);
+  return files.sort();
+}
+
+function walk(root: string, currentDir: string, files: string[]): void {
+  for (const name of readdirSync(currentDir).sort()) {
+    if (name.startsWith(".")) continue;
+    const path = join(currentDir, name);
+    const stat = lstatSync(path);
+    if (stat.isSymbolicLink()) throw new Error("Skill directories must not contain symbolic links.");
+    if (stat.isDirectory()) {
+      walk(root, path, files);
+      continue;
+    }
+    if (stat.isFile()) files.push(relativePath(root, path));
+  }
+}
+
+function relativePath(root: string, path: string): string {
+  return relative(root, path).split(sep).join("/");
+}
+
+function pathFromRelative(root: string, relativePath: string): string {
+  const safe = assertSafeSkillPath(relativePath);
+  return join(root, ...safe.split("/"));
+}
+
+function fileChecksum(path: string): string {
+  return createHash("sha256").update(readFileSync(path)).digest("hex");
+}

AgentCompany-New/src/main/services/skill-manifest.ts

@@ -0,0 +1,76 @@
+import { existsSync, lstatSync, readFileSync } from "node:fs";
+import { join } from "node:path";
+import { RISK_LEVEL, type RiskLevel } from "../../shared/domain";
+import { assertSafeSkillPath } from "./skill-artifacts";
+
+export interface SkillManifest {
+  name: string;
+  version: string;
+  entrypoint: string;
+  capabilities: string[];
+  requiredTools: string[];
+  compatibleModels: string[];
+  riskLevel: RiskLevel;
+  tests: string[];
+  inputs: unknown;
+  outputs: unknown;
+}
+
+export function loadSkillManifest(directoryPath: string, requestedVersion: string): SkillManifest {
+  const manifestPath = join(directoryPath, "skill.manifest.json");
+  if (!existsSync(manifestPath)) throw new Error("skill.manifest.json is required.");
+  const raw = JSON.parse(readFileSync(manifestPath, "utf8")) as Record<string, unknown>;
+  const manifest = normalizeManifest(raw);
+  if (manifest.version !== requestedVersion) {
+    throw new Error("Skill manifest version must match the requested version.");
+  }
+  verifyDeclaredFile(directoryPath, manifest.entrypoint);
+  for (const testPath of manifest.tests) verifyDeclaredFile(directoryPath, testPath);
+  return manifest;
+}
+
+function normalizeManifest(raw: Record<string, unknown>): SkillManifest {
+  const manifest: SkillManifest = {
+    name: readString(raw.name, "name"),
+    version: readSemver(raw.version),
+    entrypoint: assertSafeSkillPath(readString(raw.entrypoint, "entrypoint")),
+    capabilities: readStringArray(raw.capabilities, "capabilities"),
+    requiredTools: readStringArray(raw.required_tools ?? raw.requiredTools, "required_tools"),
+    compatibleModels: readStringArray(raw.compatible_models ?? raw.compatibleModels, "compatible_models"),
+    riskLevel: readRisk(raw.risk_level ?? raw.riskLevel),
+    tests: readStringArray(raw.tests, "tests").map(assertSafeSkillPath),
+    inputs: raw.inputs ?? {},
+    outputs: raw.outputs ?? {},
+  };
+  return manifest;
+}
+
+function readString(value: unknown, field: string): string {
+  if (typeof value !== "string" || !value.trim()) throw new Error(`Skill manifest field ${field} is required.`);
+  return value.trim();
+}
+
+function readSemver(value: unknown): string {
+  const version = readString(value, "version");
+  if (!/^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/.test(version)) {
+    throw new Error("Skill manifest version must be valid semver.");
+  }
+  return version;
+}
+
+function readStringArray(value: unknown, field: string): string[] {
+  if (!Array.isArray(value)) throw new Error(`Skill manifest field ${field} must be an array.`);
+  return value.map((item) => readString(item, field));
+}
+
+function readRisk(value: unknown): RiskLevel {
+  if (value === RISK_LEVEL.LOW || value === RISK_LEVEL.MEDIUM || value === RISK_LEVEL.HIGH) return value;
+  throw new Error("Skill manifest risk_level must be low, medium, or high.");
+}
+
+function verifyDeclaredFile(directoryPath: string, relativePath: string): void {
+  const path = join(directoryPath, ...relativePath.split("/"));
+  if (!existsSync(path) || !lstatSync(path).isFile()) {
+    throw new Error(`Skill manifest file was not found: ${relativePath}`);
+  }
+}

AgentCompany-New/src/main/services/skill-service.ts

@@ -1,79 +1,39 @@
-import { createHash } from "node:crypto";
-import { existsSync, readFileSync, readdirSync, statSync } from "node:fs";
-import { basename, join } from "node:path";
+import { existsSync, readFileSync } from "node:fs";
 import type { DatabaseSync } from "node:sqlite";
 import type { BindSkillInput, ImportSkillDirectoryInput, ResolveAgentSkillsInput } from "../../shared/commands";
 import { createId } from "../../shared/ids";
 import { AuditService } from "./audit-service";
-
-interface SkillManifest {
-  name: string;
-  version: string;
-  entrypoint: string;
-  capabilities: string[];
-  requiredTools: string[];
-  compatibleModels: string[];
-  riskLevel: string;
-}
+import { copySkillRevision, recursiveSkillChecksum } from "./skill-artifacts";
+import { loadSkillManifest, type SkillManifest } from "./skill-manifest";
 
 function slugify(value: string): string {
   return value.trim().toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/^-|-$/g, "");
 }
 
-function parseSkillName(markdown: string, directoryPath: string): string {
-  const frontmatterName = markdown.match(/^---[\s\S]*?\nname:\s*["']?([^"'\n]+)["']?/m)?.[1];
-  if (frontmatterName) return frontmatterName.trim();
-  const heading = markdown.match(/^#\s+(.+)$/m)?.[1];
-  return heading?.trim() || basename(directoryPath);
-}
-
-function hashFiles(directoryPath: string): string {
-  const hash = createHash("sha256");
-  const files = readdirSync(directoryPath).filter((name) => !name.startsWith(".")).sort();
-  for (const file of files) {
-    const path = join(directoryPath, file);
-    const stats = statSync(path);
-    if (stats.isFile()) {
-      hash.update(file);
-      hash.update(readFileSync(path));
-    }
-  }
-  return hash.digest("hex");
-}
-
 export class SkillService {
   private readonly audit: AuditService;
 
-  constructor(private readonly db: DatabaseSync) {
+  constructor(private readonly db: DatabaseSync, private readonly artifactRoot: string) {
     this.audit = new AuditService(db);
   }
 
   importDirectory(input: ImportSkillDirectoryInput): string {
-    const skillPath = join(input.directoryPath, "SKILL.md");
-    if (!existsSync(skillPath)) throw new Error("SKILL.md was not found in the selected directory.");
-    const markdown = readFileSync(skillPath, "utf8");
-    const name = parseSkillName(markdown, input.directoryPath);
-    const slug = slugify(name);
+    const manifest = loadSkillManifest(input.directoryPath, input.versionSemver);
+    const slug = slugify(manifest.name);
     if (!slug) throw new Error("Skill name could not be derived.");
     const now = new Date().toISOString();
     const skillId = createId("skl");
     const revisionId = createId("skr");
-    const checksum = hashFiles(input.directoryPath);
-    const manifest: SkillManifest = {
-      name,
-      version: input.versionSemver,
-      entrypoint: "SKILL.md",
-      capabilities: [],
-      requiredTools: [],
-      compatibleModels: ["openai/*", "anthropic/*", "xai/*"],
-      riskLevel: "medium",
-    };
+    const checksum = recursiveSkillChecksum(input.directoryPath);
+    const copiedFiles = copySkillRevision(input.directoryPath, this.artifactRoot, revisionId);
+    const copiedEntrypoint = copiedFiles.find((file) => file.relativePath === manifest.entrypoint);
+    if (!copiedEntrypoint) throw new Error("Skill manifest entrypoint was not found.");
 
     this.db.prepare(`
       insert into skill_definitions(id, org_id, slug, name, owner_type, status, created_at)
       values (?, ?, ?, ?, ?, ?, ?)
       on conflict(org_id, slug) do update set name = excluded.name, status = 'active'
-    `).run(skillId, input.orgId, slug, name, "organization", "active", now);
+    `).run(skillId, input.orgId, slug, manifest.name, "organization", "active", now);
 
     const existing = this.db.prepare("select id from skill_definitions where org_id = ? and slug = ?")
       .get(input.orgId, slug) as { id: string };
@@ -85,17 +45,25 @@ export class SkillService {
     `).run(
       revisionId,
       existing.id,
-      input.versionSemver,
+      manifest.version,
       JSON.stringify(manifest),
       "local_directory",
       input.directoryPath,
       now,
       checksum,
     );
-    this.db.prepare(`
-      insert into skill_artifacts(id, revision_id, artifact_type, object_key, sha256, size_bytes)
-      values (?, ?, ?, ?, ?, ?)
-    `).run(createId("ska"), revisionId, "markdown", skillPath, checksum, Buffer.byteLength(markdown));
+    for (const file of copiedFiles) {
+      this.db.prepare(`
+        insert into skill_artifacts(id, revision_id, artifact_type, object_key, sha256, size_bytes)
+        values (?, ?, ?, ?, ?, ?)
+      `).run(createId("ska"), revisionId, artifactType(file.relativePath, manifest), file.objectKey, file.sha256, file.sizeBytes);
+    }
+    for (const testPath of manifest.tests) {
+      this.db.prepare(`
+        insert into skill_tests(id, revision_id, test_type, expected_output_ref, last_result, last_run_at)
+        values (?, ?, ?, ?, ?, ?)
+      `).run(createId("skt"), revisionId, "manifest", testPath, null, null);
+    }
 
     this.audit.record({
       orgId: input.orgId,
@@ -104,18 +72,28 @@ export class SkillService {
       actorType: "system",
       actorId: null,
       action: "published",
-      payload: { slug, version: input.versionSemver },
+      payload: { slug, version: manifest.version },
     });
     return revisionId;
   }
 
   bind(input: BindSkillInput): string {
-    const id = createId("skb");
-    this.db.prepare(`
-      insert into skill_bindings(id, org_id, agent_id, skill_id, pinned_revision_id, binding_mode, priority, enabled)
-      values (?, ?, ?, ?, ?, ?, ?, ?)
-      on conflict(agent_id, skill_id) do update set pinned_revision_id = excluded.pinned_revision_id, enabled = 1
-    `).run(id, input.orgId, input.agentId, input.skillId, input.revisionId, "pinned", 100, 1);
+    this.verifyBindingTargets(input);
+    const existing = this.db.prepare("select id from skill_bindings where agent_id = ? and skill_id = ?")
+      .get(input.agentId, input.skillId) as { id: string } | undefined;
+    const id = existing?.id ?? createId("skb");
+    if (existing) {
+      this.db.prepare(`
+        update skill_bindings
+        set pinned_revision_id = ?, binding_mode = ?, priority = ?, enabled = ?
+        where id = ? and org_id = ?
+      `).run(input.revisionId, "pinned", 100, 1, id, input.orgId);
+    } else {
+      this.db.prepare(`
+        insert into skill_bindings(id, org_id, agent_id, skill_id, pinned_revision_id, binding_mode, priority, enabled)
+        values (?, ?, ?, ?, ?, ?, ?, ?)
+      `).run(id, input.orgId, input.agentId, input.skillId, input.revisionId, "pinned", 100, 1);
+    }
     this.audit.record({
       orgId: input.orgId,
       entityType: "skill_binding",
@@ -132,6 +110,9 @@ export class SkillService {
     const agent = this.db.prepare("select id from agents where id = ? and org_id = ?")
       .get(input.agentId, input.orgId) as { id: string } | undefined;
     if (!agent) throw new Error("Agent was not found.");
+    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) throw new Error("Skill runtime snapshot already exists for this run.");
     const rows = this.db.prepare(`
       select
         sr.id as revision_id,
@@ -140,7 +121,7 @@ export class SkillService {
         sa.object_key
       from skill_bindings sb
       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 = 'markdown'
+      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
       order by sb.priority asc, sr.published_at asc
     `).all(input.orgId, input.agentId) as unknown as RuntimeSkillRow[];
@@ -187,6 +168,25 @@ export class SkillService {
     });
     return snapshotId;
   }
+
+  private verifyBindingTargets(input: BindSkillInput): void {
+    const agent = this.db.prepare("select id from agents where id = ? and org_id = ?")
+      .get(input.agentId, input.orgId) as { id: string } | undefined;
+    if (!agent) throw new Error("Agent was not found.");
+    const skill = this.db.prepare("select id from skill_definitions where id = ? and org_id = ?")
+      .get(input.skillId, input.orgId) as { id: string } | undefined;
+    if (!skill) throw new Error("Skill was not found.");
+    const revision = this.db.prepare("select id from skill_revisions where id = ? and skill_id = ?")
+      .get(input.revisionId, input.skillId) as { id: string } | undefined;
+    if (!revision) throw new Error("Skill revision was not found.");
+  }
+}
+
+function artifactType(relativePath: string, manifest: SkillManifest): string {
+  if (relativePath === "skill.manifest.json") return "manifest";
+  if (relativePath === manifest.entrypoint) return "entrypoint";
+  if (manifest.tests.includes(relativePath)) return "test";
+  return "asset";
 }
 
 interface RuntimeSkillRow {

AgentCompany-New/tests/unit/domain-services.test.ts

@@ -1,4 +1,4 @@
-import { mkdirSync, writeFileSync } from "node:fs";
+import { mkdirSync, rmSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { randomUUID } from "node:crypto";
@@ -81,11 +81,25 @@ describe("domain services", () => {
   test("imports a versioned skill revision from a local directory", () => {
     const connection = createConnection();
     const organization = new OrganizationService(connection.db);
-    const skills = new SkillService(connection.db);
+    const skills = new SkillService(connection.db, join(tmpdir(), `agentcompany-skill-artifacts-${randomUUID()}`));
     const snapshot = new SnapshotService(connection.db);
     const skillDir = join(tmpdir(), `agentcompany-skill-${randomUUID()}`);
     mkdirSync(skillDir, { recursive: true });
-    writeFileSync(join(skillDir, "SKILL.md"), "---\nname: review-routes\n---\n# Review Routes\n");
+    mkdirSync(join(skillDir, "tests"), { recursive: true });
+    writeFileSync(join(skillDir, "SKILL.md"), "# Review Routes\n");
+    writeFileSync(join(skillDir, "tests", "happy_path.yaml"), "input: review\n");
+    writeFileSync(join(skillDir, "skill.manifest.json"), JSON.stringify({
+      name: "review-routes",
+      version: "1.0.0",
+      entrypoint: "SKILL.md",
+      capabilities: ["architecture-review"],
+      required_tools: ["file_search"],
+      compatible_models: ["openai/*", "anthropic/*"],
+      risk_level: "medium",
+      inputs: { diff: "markdown" },
+      outputs: { recommendation: "markdown" },
+      tests: ["tests/happy_path.yaml"],
+    }));
     let orgId = "";
     let agentId = "";
     let revisionId = "";
@@ -111,6 +125,7 @@ describe("domain services", () => {
       const skill = connection.db.prepare("select id from skill_definitions where org_id = ? and slug = ?")
         .get(orgId, "review-routes") as { id: string };
       skills.bind({ orgId, agentId, skillId: skill.id, revisionId });
+      rmSync(skillDir, { recursive: true, force: true });
       skills.resolveRuntimeSnapshot({ orgId, agentId, runId: "run-review-1" });
     });
 
@@ -118,11 +133,57 @@ describe("domain services", () => {
     expect(state.skillDefinitions[0]?.slug).toBe("review-routes");
     expect(state.skillRevisions[0]?.versionSemver).toBe("1.0.0");
     expect(state.skillRevisions[0]?.immutableChecksum).toHaveLength(64);
+    expect(state.skillRevisions[0]?.manifestJson).toContain("architecture-review");
     expect(state.skillRuntimeSnapshots[0]?.sourceRevisionIds).toContain(revisionId);
     expect(state.skillRuntimeSnapshots[0]?.compiledPrompt).toContain("# Review Routes");
     connection.close();
   });
 
+  test("rejects cross-organization skill binding and duplicate runtime snapshots", () => {
+    const connection = createConnection();
+    const organization = new OrganizationService(connection.db);
+    const skills = new SkillService(connection.db, join(tmpdir(), `agentcompany-skill-artifacts-${randomUUID()}`));
+    const skillDir = join(tmpdir(), `agentcompany-skill-${randomUUID()}`);
+    mkdirSync(skillDir, { recursive: true });
+    writeFileSync(join(skillDir, "SKILL.md"), "# Review Routes\n");
+    writeFileSync(join(skillDir, "skill.manifest.json"), JSON.stringify({
+      name: "review-routes",
+      version: "1.0.0",
+      entrypoint: "SKILL.md",
+      capabilities: [],
+      required_tools: [],
+      compatible_models: ["openai/*"],
+      risk_level: "low",
+      inputs: {},
+      outputs: {},
+      tests: [],
+    }));
+    let orgA = "";
+    let orgB = "";
+    let agentA = "";
+    let agentB = "";
+    let skillId = "";
+    let revisionId = "";
+
+    connection.transaction(() => {
+      orgA = organization.createOrganization({ organizationName: "Acme A", employeeName: "Dana Lee", mission: "A", goal: "A" });
+      orgB = organization.createOrganization({ organizationName: "Acme B", employeeName: "Riley Chen", mission: "B", goal: "B" });
+      agentA = organization.createAgent({ orgId: orgA, name: "Ada", role: "Reviewer", department: "Engineering", modelProviderId: null, modelId: null, budgetCents: 10000, managerAgentId: null });
+      agentB = organization.createAgent({ orgId: orgB, name: "Ben", role: "Reviewer", department: "Engineering", modelProviderId: null, modelId: null, budgetCents: 10000, managerAgentId: null });
+      revisionId = skills.importDirectory({ orgId: orgA, directoryPath: skillDir, versionSemver: "1.0.0" });
+      skillId = (connection.db.prepare("select id from skill_definitions where org_id = ? and slug = ?")
+        .get(orgA, "review-routes") as { id: string }).id;
+      expect(() => skills.bind({ orgId: orgA, agentId: agentB, skillId, revisionId })).toThrow("Agent was not found.");
+      skills.bind({ orgId: orgA, agentId: agentA, skillId, revisionId });
+      skills.resolveRuntimeSnapshot({ orgId: orgA, agentId: agentA, runId: "run-review-1" });
+      expect(() => skills.resolveRuntimeSnapshot({ orgId: orgA, agentId: agentA, runId: "run-review-1" }))
+        .toThrow("Skill runtime snapshot already exists for this run.");
+    });
+
+    expect(agentA).not.toBe(agentB);
+    connection.close();
+  });
+
   test("records collaborative chat semantics for threads, commands, mentions, and files", () => {
     const connection = createConnection();
     const organization = new OrganizationService(connection.db);