diff --git a/doc/idle-sleep-safety.md b/doc/idle-sleep-safety.md index a4d5197cc84..e535edcd569 100644 --- a/doc/idle-sleep-safety.md +++ b/doc/idle-sleep-safety.md @@ -449,9 +449,10 @@ sleep eligibility for a configured Muse connection. ## Skill package maintenance The package delivery worker automatically converts legacy snapshots and versions. -Unfinished `migration_at` rows block idle sleep until their durable contents are -verified and conversion finishes. Package/object/native-asset retention timestamps -are earliest cleanup eligibility, not promised delivery deadlines; optional cleanup -may wait for the next ingress wake. Workers reconstruct persisted rows and filesystem +Legacy contents remain authoritative until conversion is verified. Unfinished or +failed conversions, including rows retained while package storage is disabled, +may wait for the next ingress wake and do not block idle sleep. Their persisted +retry dates and package/object/native-asset retention timestamps are earliest +maintenance eligibility, not promised delivery deadlines. Workers reconstruct persisted rows and filesystem access leases on startup, and use the existing coordinator's drain/standby/shutdown controls. No external Cloud alarm or provider suspension is established by these tests. diff --git a/doc/plans/2026-10-28-skill-package-storage.md b/doc/plans/2026-10-28-skill-package-storage.md index f7302c9b9ff..c3ec1d26fc0 100644 --- a/doc/plans/2026-10-28-skill-package-storage.md +++ b/doc/plans/2026-10-28-skill-package-storage.md @@ -342,8 +342,8 @@ warm package preparation. Native access timestamps are coalesced hourly on disk. Lifecycle ownership remains in the package/native asset services. `index.ts` only registers them with the existing coordinator. `migration_at`/`maintenance_at` and the existing deletion receipts are durable truth; no additional worker queue table was -introduced. Unfinished conversion blocks idle sleep. Optional retention may wait for -next ingress wake. This does not establish deployed sleep or a Cloud alarm contract. +introduced. Conversion retains the original bytes and may wait for ingress without +blocking idle sleep. Optional retention may wait for the next ingress wake. This does not establish deployed sleep or a Cloud alarm contract. ## Delivery slices and proof diff --git a/doc/skill-package-storage.md b/doc/skill-package-storage.md index 2c899c829cc..e63af366628 100644 --- a/doc/skill-package-storage.md +++ b/doc/skill-package-storage.md @@ -76,9 +76,10 @@ restore deadlines on startup, pause in drain/standby, and stop on shutdown. With pending conversion, retry, or eligible cache deadline they perform no recurring work. The supported live writer is the owning server; independent SQL writers/other servers need restart reconstruction or an explicit wake. No permanent DB listener is added. -Conversion is an idle-sleep blocker while unfinished. Cache/deletion times are earliest -eligibility, may wait until the next ingress wake, and do not claim an external Cloud -alarm or measured host suspension. +Conversion retains authoritative legacy bytes until verification succeeds. Conversion +retries and cache/deletion dates are optional maintenance eligibility; they may wait +until the next ingress wake, including with package storage disabled. They do not +block idle sleep or claim an external Cloud alarm or measured host suspension. Each object-deletion receipt has a 30-second storage deadline. Shutdown cancels pending delete/existence requests; incomplete cleanup retains the receipt for retry. @@ -267,3 +268,15 @@ The larger restore recipe stays out of those queries. Ten-skill cold preparation slower on this busy host; the raw receipts retain that result as well as warm samples. The earlier full repository test attempt remains incomplete; this follow-up does not claim a clean full-suite result, deployed Railway measurements, or a live agent run. + +### Administrative version reads + +Version lists, current-version summaries, and test-run detail return file names/kinds +without bodies. Diff and restore fetch only the selected version through its existing +single-version endpoint. Archive reads for those operations and retention checks use +isolated scratch directories, so storage latency does not hold the company preparation +lock. Support inspection reads the requested stored version without Git recovery. + +Package preparation renews a 24-hour publication grace. Publication and retirement +lock the same package row and recheck its receipt/current references before commit; +a prepared receipt retired before publication cannot silently become current. diff --git a/maps/agent-access-map/skills.md b/maps/agent-access-map/skills.md index 55ce78c0295..100a06f8921 100644 --- a/maps/agent-access-map/skills.md +++ b/maps/agent-access-map/skills.md @@ -91,6 +91,12 @@ support a narrower shape than full API. ## `versions` +Version lists and current/test-run version summaries contain file names and kinds. +Use `GET /api/companies/{companyId}/skills/{skillId}/versions/{versionId}` for the +selected version's file contents. Authorization remains company/skill scoped; support +inspection reads only stored bytes for the requested version and does not recover Git. + + Capability ID: `skills/versions`. Review: `baseline`. diff --git a/maps/feature-map/skills.md b/maps/feature-map/skills.md index d98ad2d38e3..279b7d95344 100644 --- a/maps/feature-map/skills.md +++ b/maps/feature-map/skills.md @@ -53,6 +53,12 @@ Automated: [Studio](../../ui/src/pages/SkillStudio.test.tsx) covers UI behavior; Manual: Create or fork a disposable skill, edit instructions, save, and reload. Run a bounded test, respond to any question, and inspect the output. Verify the saved version matches what was tested and failed saves retain the draft. +Version history lists file metadata without downloading archives. Opening a diff +fetches only its selected versions; restoring fetches the chosen version. The +[library UI regression](../../ui/src/pages/CompanySkills.test.tsx) checks deferred +loads; the [package regression](../../server/src/__tests__/skill-package-store.test.ts) +checks that slow archive verification cannot hold up a warm agent preparation. + ### `agent-enablement` Automated: [policy service](../../server/src/__tests__/company-skill-policy-service.test.ts) and [agent skills state](../../ui/src/pages/agent-skills/AgentSkillsTab.test.ts) cover policy/state contracts. diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 6b149bc00e6..32f14cb47b3 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -784,6 +784,7 @@ export type { CompanySkillListQuery, CompanySkillCategoryCount, CompanySkillVersion, + CompanySkillVersionSummary, CompanySkillVersionCreateRequest, CompanySkillStarResult, CompanySkillComment, diff --git a/packages/shared/src/types/company-skill.ts b/packages/shared/src/types/company-skill.ts index 9ab84634f21..0f24c5646b5 100644 --- a/packages/shared/src/types/company-skill.ts +++ b/packages/shared/src/types/company-skill.ts @@ -138,7 +138,7 @@ export interface CompanySkillDetail extends CompanySkill { sourceLabel: string | null; sourceBadge: CompanySkillSourceBadge; sourcePath: string | null; - currentVersion: CompanySkillVersion | null; + currentVersion: CompanySkillVersionSummary | null; starredByCurrentActor: boolean; } @@ -174,6 +174,11 @@ export interface CompanySkillVersion { createdAt: Date; } +/** Version listings carry file names/kinds; fetch one version to read its bytes. */ +export type CompanySkillVersionSummary = Omit & { + fileInventory: CompanySkillFileInventoryEntry[]; +}; + export interface CompanySkillVersionCreateRequest { label?: string | null; } @@ -608,7 +613,7 @@ export interface CompanySkillTestRunHarnessContent { } export interface CompanySkillTestRunDetail extends CompanySkillTestRun { - skillVersion: CompanySkillVersion; + skillVersion: CompanySkillVersionSummary; outputBody: string; harnessContent: CompanySkillTestRunHarnessContent; harnessIssue: { diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 8f2e9711dea..fdf551452dd 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -168,6 +168,7 @@ export type { CompanySkillListQuery, CompanySkillCategoryCount, CompanySkillVersion, + CompanySkillVersionSummary, CompanySkillVersionCreateRequest, CompanySkillStarResult, CompanySkillComment, diff --git a/server/src/__tests__/company-skills-service.test.ts b/server/src/__tests__/company-skills-service.test.ts index 5d7f1675048..a88634af5e5 100644 --- a/server/src/__tests__/company-skills-service.test.ts +++ b/server/src/__tests__/company-skills-service.test.ts @@ -898,7 +898,8 @@ describeEmbeddedPostgres("companySkillService.list", () => { releasedAt: new Date("2026-07-21T00:00:00.000Z"), }); if (!champion) throw new Error("Expected seeded v7-roster release"); - const championHashes = Object.fromEntries(champion.fileInventory.map((entry) => [ + const championVersion = (await svc.getVersion(companyId, paperclipSkill.id, champion.id))!; + const championHashes = Object.fromEntries(championVersion.fileInventory.map((entry) => [ entry.path, createHash("sha256").update(entry.content).digest("hex"), ])); @@ -2705,7 +2706,8 @@ describeEmbeddedPostgres("companySkillService.list", () => { versions = await svc.listVersions(companyId, skill.id); expect(versions).toHaveLength(2); expect(versions[0]).toMatchObject({ revisionNumber: 2, authorUserId: "board" }); - expect(versions[0]!.fileInventory).toEqual([ + expect(versions[0]!.fileInventory).toEqual([{ path: "SKILL.md", kind: "skill" }]); + expect((await svc.getVersion(companyId, skill.id, versions[0]!.id))!.fileInventory).toEqual([ expect.objectContaining({ path: "SKILL.md", content: editedMarkdown }), ]); await expect(svc.getById(companyId, skill.id)).resolves.toMatchObject({ diff --git a/server/src/__tests__/idle-sleep-safety.test.ts b/server/src/__tests__/idle-sleep-safety.test.ts index a05af7c7a96..3d4bb27077c 100644 --- a/server/src/__tests__/idle-sleep-safety.test.ts +++ b/server/src/__tests__/idle-sleep-safety.test.ts @@ -5,7 +5,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest import { agentInstructionWorkingCopies, agents, agentApiKeys, agentWakeupRequests, budgetPolicies, companies, companySecretProposals, createDb, adapterAuthSessions, environments, environmentLeases, executionWorkspaces, - feedbackExports, feedbackVotes, + feedbackExports, feedbackVotes, companySkills, companySkillVersions, companySkillRepositorySnapshots, heartbeatRuns, issues, issueWatchdogs, issueThreadInteractions, issueQuestionResponseDeliveries, projects, projectWorkspaces, routines, routineTriggers, toolApplications, toolConnections, workspaceOperations, executionWorkspaceRuntimeLeases, type Db, } from "@paperclipai/db"; @@ -187,6 +187,23 @@ if (!support.supported) console.warn(`Skipping idle sleep Postgres tests: ${supp return { companyId, agentId, runId }; } + it.each(["true", "false"])("lets optional conversion retries sleep with package storage %s", async enabled => { + const previous = process.env.PAPERCLIP_SKILL_PACKAGE_STORAGE_ENABLED; + process.env.PAPERCLIP_SKILL_PACKAGE_STORAGE_ENABLED = enabled; + try { + const { companyId } = await seed(); + const [skill] = await db.insert(companySkills).values({ companyId, key: "legacy", slug: "legacy", name: "Legacy", markdown: "original" }).returning(); + await db.insert(companySkillVersions).values({ companyId, companySkillId: skill!.id, revisionNumber: 1, + fileInventory: [{ path: "SKILL.md", kind: "skill", content: "original" }], migrationAt: new Date(Date.now() + 60 * 60_000) }); + await db.insert(companySkillRepositorySnapshots).values({ companyId, contentHash: "a".repeat(64), + fileInventory: [{ path: "SKILL.md", kind: "skill", content: "original" }], migrationAt: new Date(0) }); + expect(await read()).toEqual(none); + } finally { + if (previous === undefined) delete process.env.PAPERCLIP_SKILL_PACKAGE_STORAGE_ENABLED; + else process.env.PAPERCLIP_SKILL_PACKAGE_STORAGE_ENABLED = previous; + } + }); + it("retains committed budget stop intent until its exact generation is delivered", async () => { const f = await seed(); const [policy] = await db.insert(budgetPolicies).values({ companyId: f.companyId, scopeType: "company", scopeId: f.companyId, diff --git a/server/src/__tests__/skill-package-store.test.ts b/server/src/__tests__/skill-package-store.test.ts index 8bf57b02915..d32432eae11 100644 --- a/server/src/__tests__/skill-package-store.test.ts +++ b/server/src/__tests__/skill-package-store.test.ts @@ -8,7 +8,7 @@ import path from "node:path"; import { and, eq } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { companies, companySkills, companySkillVersions, companySkillRepositorySnapshots as snapshots, - skillPackageDeletions, createDb } from "@paperclipai/db"; + skillPackageDeletions, createDb, withDedicatedDbConnection } from "@paperclipai/db"; import type { CompanySkillVersionFileInventoryEntry } from "@paperclipai/shared"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import * as storageModule from "../storage/index.js"; @@ -20,6 +20,8 @@ import { skillPackageMaintenance } from "../services/skill-package-maintenance.j import { skillPackageManifest, validateSkillPackageManifest, writeSkillPackageArchive, extractSkillPackageArchive, verifySkillPackageDirectory } from "../services/skill-package-files.js"; import { skillFileBytes, skillSnapshotHash } from "../services/skill-snapshot.js"; +import { readCustomerSuccessResource } from "../services/customer-success-inspection.js"; +import { repositorySkillProjection } from "../services/skill-repository-package.js"; import { hasRestorableSnapshot } from "../services/skill-version-cache.js"; const support = await getEmbeddedPostgresTestSupport(); @@ -69,7 +71,7 @@ describe.skipIf(!support.supported)("external skill packages", () => { name, markdown: files.find(file => file.path === `skills/${name}/SKILL.md`)!.content, sourceType: "github", metadata: { skillSourceId: randomUUID(), snapshotHash: "saved" } }).returning(); const [version] = await db.insert(companySkillVersions).values({ companyId, companySkillId: skill!.id, revisionNumber: 1, - repositorySnapshotId: id, repositorySkillPath: `skills/${name}/SKILL.md`, packageSnapshotId: id, + repositorySnapshotId: id, repositorySkillPath: `skills/${name}/SKILL.md`, packageSnapshotId: id, migrationAt: null, packageFileMap: [{ path: "SKILL.md", kind: "skill", sourcePath: `skills/${name}/SKILL.md` }] }).returning(); await db.update(companySkills).set({ currentVersionId: version!.id }).where(eq(companySkills.id, skill!.id)); skills.push({ skill: skill!, version: version! }); @@ -79,6 +81,111 @@ describe.skipIf(!support.supported)("external skill packages", () => { return { companyId, id, store, skills, service, resolve }; } + it("forks a packaged repository inside an outer transaction with a one-connection pool", async () => { + const f = await fixture(); + const fork = await db.transaction(tx => companySkillService(tx as unknown as typeof db, { packageDb: db }) + .createLocalSkill(f.companyId, { name: "Idempotent fork", slug: "idempotent-fork", forkedFromSkillId: f.skills[0]!.skill.id })); + const saved = await f.service.getVersion(f.companyId, fork.id, fork.currentVersionId!); + expect(saved!.fileInventory).toEqual(expect.arrayContaining([ + expect.objectContaining({ path: ".paperclip-repository/shared/run.sh", content: files[2]!.content }), + ])); + }, 15_000); + + it("lists version metadata without reading archives, while inspection reads the requested historical bytes", async () => { + const f = await fixture(); + const { skill, version } = f.skills[0]!; + await db.update(companySkills).set({ markdown: "New current markdown" }).where(eq(companySkills.id, skill.id)); + const read = vi.spyOn(storage, "getObject"); + const history = await f.service.listVersions(f.companyId, skill.id); + expect(history[0]!.fileInventory).toEqual([{ path: "SKILL.md", kind: "skill" }]); + expect(read.mock.calls.filter(call => call[1].endsWith("files.tar.gz"))).toHaveLength(0); + await db.update(companySkillVersions).set({ packageFileMap: [ + ...version.packageFileMap!, { path: "run.sh", kind: "script", sourcePath: "shared/run.sh" }, + ] }).where(eq(companySkillVersions.id, version.id)); + const script = await readCustomerSuccessResource(db, storage, { + operation: "skills.file", companyId: f.companyId, resourceId: skill.id, versionId: version.id, path: "run.sh", + }) as { content: string }; + expect(Buffer.from(script.content, "base64").toString()).toBe(files[2]!.content); + const result = await readCustomerSuccessResource(db, storage, { + operation: "skills.file", companyId: f.companyId, resourceId: skill.id, versionId: version.id, path: "SKILL.md", + }) as { content: string }; + expect(Buffer.from(result.content, "base64").toString()).toBe(files[0]!.content); + const missing = await readCustomerSuccessResource(db, storage, { + operation: "skills.file", companyId: f.companyId, resourceId: skill.id, versionId: version.id, path: "not-saved.md", + }); + expect(missing).toEqual({ available: false, reason: "skill_file_not_snapshotted" }); + }); + + it("does not block a warm turn behind a paused archive read for retention", async () => { + const f = await fixture(); + await f.resolve(); + await fs.rm(skillPackagePaths(f.companyId, f.id).directory, { recursive: true, force: true }); + let release!: () => void; + let entered!: () => void; + const blocked = new Promise(resolve => { release = resolve; }); + const reading = new Promise(resolve => { entered = resolve; }); + const original = storage.getObject.bind(storage); + vi.spyOn(storage, "getObject").mockImplementation(async (companyId, key, options) => { + if (key.endsWith("files.tar.gz")) { entered(); await blocked; } + return original(companyId, key, options); + }); + const cleanup = hasRestorableSnapshot(db, f.companyId, f.skills[0]!.version.id); + try { + await reading; + const warm = await f.resolve(); + expect(warm.map(entry => entry.sourceStatus)).toEqual(["available", "available"]); + } finally { release(); await cleanup; } + expect(await fs.stat(skillPackagePaths(f.companyId, f.id).directory).catch(() => null)).toBeNull(); + }, 15_000); + + it("skips an in-flight publication without waiting on its row or the warm preparation lock", async () => { + const f = await fixture(true); + await db.update(companySkills).set({ currentVersionId: null }).where(eq(companySkills.companyId, f.companyId)); + await db.update(snapshots).set({ createdAt: new Date(0), maintenanceAt: new Date(0) }).where(eq(snapshots.id, f.id)); + let retirementStarted!: () => void; + const waiting = new Promise(resolve => { retirementStarted = resolve; }); + const debug = db.$client.options.debug; + db.$client.options.debug = (_connection, query) => { + if (query.includes('company_skill_repository_snapshots') && query.includes('for update')) retirementStarted(); + }; + let sweep: Promise | undefined; + try { + await withDedicatedDbConnection(db, connection => connection.transaction(async tx => { + await tx.select({ id: snapshots.id }).from(snapshots).where(eq(snapshots.id, f.id)).for("update"); + await tx.update(companySkills).set({ currentVersionId: f.skills[0]!.version.id }).where(eq(companySkills.id, f.skills[0]!.skill.id)); + sweep = skillPackageWork(db, storage).tick(new AbortController().signal); + await waiting; + await sweep; + })); + await sweep; + expect((await f.store.descriptor(f.companyId, f.id)).storage).not.toBeNull(); + } finally { db.$client.options.debug = debug; await sweep; } + }, 15_000); + + it("refuses to publish a package retired before the publication row lock", async () => { + const f = await fixture(true); + await db.update(companySkills).set({ currentVersionId: null }).where(eq(companySkills.companyId, f.companyId)); + await db.update(snapshots).set({ storage: null }).where(eq(snapshots.id, f.id)); + await expect(f.service.createVersion(f.companyId, f.skills[0]!.skill.id, {}, null, { + skill: f.skills[0]!.skill, skipInventoryRefresh: true, fileInventory: files, + packageSnapshotId: f.id, repositorySnapshotId: f.id, repositorySkillPath: files[0]!.path, + })).rejects.toThrow(/changed during publication/); + const [skill] = await db.select().from(companySkills).where(eq(companySkills.id, f.skills[0]!.skill.id)); + expect(skill!.currentVersionId).toBeNull(); + }); + + it("converts a legacy fork with the scanner's deepest accepted repository path", async () => { + const companyId = await company(); + const deep = [...Array(63).fill("d"), "SKILL.md"].join("/"); + const projected = repositorySkillProjection([{ ...files[0]!, path: deep }], deep); + const [skill] = await db.insert(companySkills).values({ companyId, key: "deep", slug: "deep", name: "Deep", markdown: projected[0]!.content }).returning(); + const [version] = await db.insert(companySkillVersions).values({ companyId, companySkillId: skill!.id, revisionNumber: 1, fileInventory: projected }).returning(); + await skillPackageMaintenance(db, storage).migrateVersions({ id: version!.id, apply: true, clearLegacy: true }); + const [saved] = await db.select().from(companySkillVersions).where(eq(companySkillVersions.id, version!.id)); + expect(saved!.migrationAt).toBeNull(); + expect(await skillPackageStore(db, storage).versionFiles(saved!)).toEqual(projected); + }); + it("automatically converts legacy rows, clears verified payloads and preserves version identities", async () => { const companyId = await company(); const [snapshot] = await db.insert(snapshots).values({ companyId, contentHash: skillSnapshotHash(files), fileInventory: files }).returning(); diff --git a/server/src/__tests__/skill-repository-package.test.ts b/server/src/__tests__/skill-repository-package.test.ts index 8c1a602dcf5..91eef93caba 100644 --- a/server/src/__tests__/skill-repository-package.test.ts +++ b/server/src/__tests__/skill-repository-package.test.ts @@ -4,6 +4,7 @@ import os from 'node:os'; import path from 'node:path'; import { materializePaperclipSkillCopy } from '@paperclipai/adapter-utils/server-utils'; import { repositorySkillProjection } from '../services/skill-repository-package.js'; +import { skillPackageManifest } from '../services/skill-package-files.js'; import { snapshotFile, skillFileBytes } from '../services/skill-snapshot.js'; const md = '---\nname: main\ndescription: Work\ndisable-model-invocation: true\n---\nRead [runtime](../runtime/SKILL.md) and ../../agents/worker.md.\n'; @@ -37,6 +38,15 @@ describe('repository skill delivery', () => { expect(await fs.readFile(path.join(sandbox, 'agents/openai.yaml'), 'utf8')).toContain('allow_implicit_invocation: false'); } finally { await fs.rm(root, { recursive: true, force: true }); } }); + it('accepts scanner-limit paths after adding the portable repository prefix', () => { + const deep = [...Array(63).fill('d'), 'SKILL.md'].join('/'); + const long = `${'d'.repeat(4096 - '/SKILL.md'.length)}/SKILL.md`; + for (const name of [deep, long]) { + const original = [{ path: name, kind: 'skill' as const, content: md }]; + expect(() => skillPackageManifest(repositorySkillProjection(original, name))).not.toThrow(); + expect(() => skillPackageManifest([{ ...original[0]!, path: `extra/${name}` }])).toThrow(); + } + }); it('rejects escaped and absent entrypoints', () => { expect(() => repositorySkillProjection([], '../SKILL.md')).toThrow(/Invalid/); expect(() => repositorySkillProjection([], 'missing/SKILL.md')).toThrow(/missing/); diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index cbdafa76186..d78e6799f8f 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -31,6 +31,7 @@ import { companySkillTestRuns, companySkillVersions, withDedicatedDbConnection, companySkills, + companySkillRepositorySnapshots, costEvents, documents, issueAttachments, @@ -105,6 +106,7 @@ import type { CompanySkillUpdateHoldReason, CompanySkillUsageAgent, CompanySkillVersion, + CompanySkillVersionSummary, CompanySkillVersionCreateRequest, CompanySkillVersionFileInventoryEntry, IssueAttachment, @@ -2802,7 +2804,7 @@ function enrichSkill( skill: CompanySkill, attachedAgentCount: number, usedByAgents: CompanySkillUsageAgent[] = [], - currentVersion: CompanySkillVersion | null = null, + currentVersion: CompanySkillVersionSummary | null = null, starredByCurrentActor = false, existingForks: CompanySkillForkSummary[] = [], ) { @@ -2976,6 +2978,8 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo const packageDb = options.packageDb ?? db; const packages = skillPackageStore(packageDb); const packageRuntime = skillPackageRuntime(packageDb); + // Admin reads keep SQL on the caller's transaction, which may own the pool's only slot. + const packageReader = db === packageDb ? packages : skillPackageStore(db); const visibility = (database: DbOrTransaction = db) => skillAccessService(database as Db, options.actor); const folderSvc = folderService(db); const agents = agentService(db); @@ -3486,11 +3490,19 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo async function getVersion(companyId: string, skillId: string, versionId: string): Promise { if (!await getById(companyId, skillId)) return null; const row = await readVersionMetadata(companyId, skillId, versionId); - return row ? toCompanySkillVersion({ ...row, fileInventory: await packages.versionFiles(row) }) : null; + return row ? toCompanySkillVersion({ ...row, fileInventory: await packageReader.versionFiles(row) }) : null; } - async function getCurrentVersion(skill: CompanySkill): Promise { - return skill.currentVersionId ? getVersion(skill.companyId, skill.id, skill.currentVersionId) : null; + async function versionSummary(row: StoredSkillVersion): Promise { + return { ...toCompanySkillVersion(row), fileInventory: await packageReader.versionInventory(row) }; + } + async function getVersionSummary(companyId: string, skillId: string, versionId: string) { + if (!await getById(companyId, skillId)) return null; + const row = await readVersionMetadata(companyId, skillId, versionId); + return row ? versionSummary(row) : null; + } + async function getCurrentVersion(skill: CompanySkill): Promise { + return skill.currentVersionId ? getVersionSummary(skill.companyId, skill.id, skill.currentVersionId) : null; } async function isStarredByActor(companyId: string, skillId: string, actor: SkillActor | null | undefined) { @@ -3681,7 +3693,7 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo return out; } - async function listVersions(companyId: string, skillId: string): Promise { + async function listVersions(companyId: string, skillId: string): Promise { const skill = await getById(companyId, skillId); if (!skill) throw notFound("Skill not found"); const rows = await db @@ -3689,8 +3701,8 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo .from(companySkillVersions) .where(and(eq(companySkillVersions.companyId, companyId), eq(companySkillVersions.companySkillId, skillId))) .orderBy(desc(companySkillVersions.revisionNumber)); - const result: CompanySkillVersion[] = []; - for (const row of rows) result.push(toCompanySkillVersion({ ...row, fileInventory: await packages.versionFiles(row) })); + const result: CompanySkillVersionSummary[] = []; + for (const row of rows) result.push(await versionSummary(row)); return result; } @@ -3707,7 +3719,7 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo const fileInventory = serializeVersionFileInventory( options.fileInventory ?? await collectVersionFileInventory(companyId, skill), ); - const previousVersion = options.fileInventory === undefined ? await getCurrentVersion(skill) : null; + const previousVersion = options.fileInventory === undefined && skill.currentVersionId ? await readVersionMetadata(companyId, skill.id, skill.currentVersionId) : null; // Shipped release rows retain their existing validation contract. All user // snapshots use the package store; source imports prepare before their transaction. let packageFileMap = options.packageFileMap ?? null; @@ -3726,6 +3738,12 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo } const persist = async (tx: DbOrTransaction) => { await notifyDeliveryWork(tx, DELIVERY_QUEUES.skillPackages); + if (packageSnapshotId) { + const [saved] = await tx.select({ storage: companySkillRepositorySnapshots.storage }) + .from(companySkillRepositorySnapshots).where(and(eq(companySkillRepositorySnapshots.companyId, companyId), + eq(companySkillRepositorySnapshots.id, packageSnapshotId))).for("update"); + if (!saved?.storage) throw conflict("Skill package changed during publication; retry the operation."); + } await tx.execute(sql` select ${companySkills.id} from ${companySkills} @@ -4445,19 +4463,13 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo const normalizedPath = normalizePortablePath(relativePath || "SKILL.md"); if (skill.metadata?.skillSourceId && skill.metadata?.snapshotHash && skill.currentVersionId) { const version = await readVersionMetadata(skill.companyId, skill.id, skill.currentVersionId); - let file: CompanySkillVersionFileInventoryEntry | null | undefined; - if (version?.packageSnapshotId) { - const mapping = version.packageFileMap?.find(file => file.path === normalizedPath); - if (mapping) file = await packages.withPackage(skill.companyId, version.packageSnapshotId, async (root, manifest) => { - const entry = manifest.files.find(file => file.path === mapping.sourcePath); - if (!entry) return null; - const encoding = mapping.encoding ?? entry.encoding; - return { path: mapping.path, kind: mapping.kind, content: (await fs.readFile(path.join(root, entry.path))).toString(encoding), - encoding, executable: entry.executable }; - }); - else if (!version.repositorySnapshotId) file = await packageRuntime.readFile(version, normalizedPath); - } else if (version) file = (await packages.versionFiles(version)).find(file => file.path === normalizedPath); - if (!file && version?.repositorySnapshotId) file = await packageRuntime.readFile(version, normalizedPath); + let file: CompanySkillVersionFileInventoryEntry | undefined; + if (version) { + file = await packageReader.readVersionFile(version, normalizedPath); + if (!file && version.repositorySnapshotId && version.repositorySkillPath) { + file = (await runtimeVersionInventory(skill.companyId, toCompanySkillVersion(version))).find(file => file.path === normalizedPath); + } + } if (!file) throw notFound("Installed skill snapshot is missing a file."); return { ...file, skillId: skill.id, language: file.encoding === "base64" ? null : inferLanguageFromPath(normalizedPath), markdown: file.encoding !== "base64" && isMarkdownPath(normalizedPath), editable: false }; @@ -6063,11 +6075,13 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo return true; } - async function runtimeVersionInventory(companyId: string, version: CompanySkillVersion) { - if (!version.repositorySnapshotId) return version.fileInventory.length ? version.fileInventory - : packages.versionFiles((await readVersionMetadata(companyId, version.companySkillId, version.id))!); + async function runtimeVersionInventory(companyId: string, version: CompanySkillVersionSummary) { + if (!version.repositorySnapshotId) { + const row = (await readVersionMetadata(companyId, version.companySkillId, version.id))!; + return packageReader.versionFiles(row); + } if (!version.repositorySkillPath) throw unprocessable("Installed repository snapshot is unavailable."); - return repositorySkillProjection(await packages.files(companyId, version.repositorySnapshotId), version.repositorySkillPath); + return repositorySkillProjection(await packageReader.files(companyId, version.repositorySnapshotId, true), version.repositorySkillPath); } async function materializeVersionSnapshot(companyId: string, skill: CompanySkill, version: StoredSkillVersion): Promise { @@ -6816,7 +6830,7 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo throw unprocessable("Cannot run a skill test for a skill with zero files."); } - const currentVersion = await getCurrentVersion(skill); + const currentVersion = skill.currentVersionId ? await getVersion(companyId, skill.id, skill.currentVersionId) : null; if (!currentVersion || !versionInventorySnapshotEqual(currentVersion.fileInventory, currentSnapshot)) { return createVersion(companyId, skill.id, { label: "Auto version for test run" }, actor); } @@ -7063,7 +7077,7 @@ export function companySkillService(db: Db, options: { actor?: AuthorizationActo if (!run) return null; const harnessIssueGone = Boolean(row.harnessIssueDeletedAt); const [version, issue, documentRows, interactionRows, attachmentRows, workProductRows] = await Promise.all([ - getVersion(companyId, skillId, row.skillVersionId), + getVersionSummary(companyId, skillId, row.skillVersionId), harnessIssueGone ? Promise.resolve(null) : db diff --git a/server/src/services/customer-success-inspection.ts b/server/src/services/customer-success-inspection.ts index 2f9a031e82a..e112d1ad71c 100644 --- a/server/src/services/customer-success-inspection.ts +++ b/server/src/services/customer-success-inspection.ts @@ -19,6 +19,7 @@ import { } from "./workspace-file-resources.js"; import { deriveBundleState } from "./agent-instructions.js"; import { readInstructionBytes } from "./agent-instruction-files.js"; +import { skillPackageStore } from "./skill-package-store.js"; import { getRunLogStore } from "./run-log-store.js"; // This is a reviewed catalog of readers, not a proxy for existing GET routes. @@ -281,7 +282,8 @@ export async function readCustomerSuccessResource( assertWorkspaceFilePathAllowed(relative); const versionId = q.versionId ?? skill.currentVersionId; // Installed version snapshots are authoritative. Reading must not invoke - // inventory reconciliation, checkout, fetching, adoption or materialization. + // inventory reconciliation or Git recovery. Stored packages are verified in + // an isolated scratch directory, outside the agent preparation lock. if (versionId) { const version = await ownRow( tx, @@ -290,18 +292,15 @@ export async function readCustomerSuccessResource( String(versionId), ); if (version.companySkillId !== skill.id) throw notFound("Skill version not found"); - const files = version.fileInventory as Array<{ - path: string; - content?: string; - encoding?: string; - }>; - const file = files.find((f) => f.path === relative); + const file = await skillPackageStore(tx, storage).readVersionFile( + version as typeof schema.companySkillVersions.$inferSelect, relative, false); if (file?.content !== undefined) { const bytes = Buffer.from(file.content, file.encoding === "base64" ? "base64" : "utf8"); if (bytes.length > MAX_INSPECTION_BYTES) throw unprocessable("Skill snapshot exceeds inspection limit"); return binary(bytes, bytes.length); } + return unavailable("skill_file_not_snapshotted"); } if (relative === "SKILL.md") return binary( diff --git a/server/src/services/idle-sleep-safety.ts b/server/src/services/idle-sleep-safety.ts index daa093682d8..b9ee6483e52 100644 --- a/server/src/services/idle-sleep-safety.ts +++ b/server/src/services/idle-sleep-safety.ts @@ -213,11 +213,9 @@ const WORK_CHECKS = [ // label requires its completion stamp and a cleared failure reason. `SELECT 1 FROM feedback_exports WHERE (status = 'local_only' OR (status = 'sent' AND exported_at IS NOT NULL AND failure_reason IS NULL)) IS NOT TRUE`, - // Conversion owes a verified durable copy before dropping legacy payloads. - // Cache/deletion retention times are earliest eligibility, allowed to wait for - // the next ingress wake; they are not a promised external alarm deadline. - `SELECT 1 FROM company_skill_repository_snapshots WHERE migration_at IS NOT NULL`, - `SELECT 1 FROM company_skill_versions WHERE migration_at IS NOT NULL`, + // Skill package conversion and retention are optional maintenance. Legacy + // contents remain authoritative until verified conversion; retries may wait + // for ingress, including when external package storage is disabled. `SELECT 1 FROM project_skill_sources b JOIN companies c ON c.id = b.company_id WHERE c.status = 'active' AND b.active AND b.source_id IS NULL`, `SELECT 1 FROM company_skill_sources s JOIN companies c ON c.id = s.company_id diff --git a/server/src/services/skill-package-files.ts b/server/src/services/skill-package-files.ts index 9f82ac248ea..b93fb176be6 100644 --- a/server/src/services/skill-package-files.ts +++ b/server/src/services/skill-package-files.ts @@ -6,6 +6,7 @@ import { pipeline } from "node:stream/promises"; import { createGzip, createGunzip } from "node:zlib"; import { extract, pack } from "tar-stream"; import type { CompanySkillVersionFileInventoryEntry } from "@paperclipai/shared"; +import { REPOSITORY_CONTENT_DIR } from "./skill-repository-package.js"; import { assertSkillSnapshotPath, skillFileBytes, skillSnapshotHash, MAX_SKILL_SNAPSHOT_BYTES, MAX_SKILL_SNAPSHOT_FILES } from "./skill-snapshot.js"; export const MAX_SKILL_PACKAGE_BYTES = MAX_SKILL_SNAPSHOT_BYTES; @@ -38,7 +39,9 @@ export function validateSkillPackageManifest(value: unknown): SkillPackageManife let total = 0; for (const file of manifest.files) { assertSkillSnapshotPath(file.path); - if (file.path.length > 4096 || file.path.split("/").length > 64 || paths.has(file.path) + // A portable repository fork adds one generated directory to scanner-accepted paths. + const sourcePath = file.path.startsWith(`${REPOSITORY_CONTENT_DIR}/`) ? file.path.slice(REPOSITORY_CONTENT_DIR.length + 1) : file.path; + if (sourcePath.length > 4096 || sourcePath.split("/").length > 64 || paths.has(file.path) || !kinds.has(file.kind) || !["utf8", "base64"].includes(file.encoding) || typeof file.executable !== "boolean" || !Number.isSafeInteger(file.size) || file.size < 0 || !/^[a-f0-9]{64}$/.test(file.sha256)) throw new Error("Invalid skill package file"); paths.add(file.path); diff --git a/server/src/services/skill-package-store.ts b/server/src/services/skill-package-store.ts index 9df7181d351..ef9327c8393 100644 --- a/server/src/services/skill-package-store.ts +++ b/server/src/services/skill-package-store.ts @@ -237,6 +237,63 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { return manifest; }); } + async function restoreInto(row: Descriptor, manifest: SkillPackageManifest, staging: string, + archivePath: string, signal: AbortSignal, allowRecovery = true) { + let legacy: CompanySkillVersionFileInventoryEntry[] | null = null; + if (!row.storage && !row.restorable) legacy = await legacyFiles(row); + try { + if (row.storage) { + if (row.storage.provider !== storage().provider) throw new Error("Skill package storage provider changed"); + const object = await storage().getObject(row.companyId, row.storage.archiveKey, { signal }); + const digest = boundedHashStream(row.storage.archiveBytes); + await pipeline(object.stream, digest.stream, createWriteStream(archivePath, { flags: "wx", mode: 0o600 }), { signal }); + const actual = digest.result(); + if (actual.bytes !== row.storage.archiveBytes || actual.sha256 !== row.storage.archiveSha256) throw new Error("Skill package archive checksum mismatch"); + await extractSkillPackageArchive(archivePath, staging, manifest, signal); + } + } catch (error) { + signal.throwIfAborted(); + // A missing/corrupt archive must not hide a still-authoritative legacy + // body. Only then use the saved, authorized immutable Git recipe. + const [saved] = await db.select({ files: snapshots.fileInventory }).from(snapshots).where(scope(row.companyId, row.id)); + if (saved?.files.length && skillPackageManifest(saved.files).contentHash === row.contentHash) legacy = saved.files; + else if (row.restorable && allowRecovery) legacy = await acquireFiles(row, signal); + else throw error; + await fs.rm(staging, { recursive: true, force: true }); + await fs.mkdir(staging); + } + if (!row.storage && row.restorable && !legacy) { + if (!allowRecovery) throw notFound("Skill package bytes are not stored"); + legacy = await acquireFiles(row, signal); + } + if (legacy) { + for (const file of legacy) { + signal.throwIfAborted(); + const target = path.join(staging, assertSkillSnapshotPath(file.path)); + await fs.mkdir(path.dirname(target), { recursive: true, mode: 0o700 }); + await fs.writeFile(target, skillFileBytes(file), { flag: "wx", mode: file.executable ? 0o555 : 0o444 }); + } + } + if (!await verifySkillPackageDirectory(staging, manifest.files)) throw new Error("Restored skill package does not match its manifest"); + } + // Admin reads and retention verification own a scratch directory. They never + // wait for or hold the lock used to prepare and pin an agent's warm files. + async function withIsolatedPackage(companyId: string, id: string, + consume: (root: string, manifest: SkillPackageManifest) => Promise, allowRecovery = true) { + const row = await descriptor(companyId, id); + const manifest = await manifestFor(row); + const signal = AbortSignal.timeout(180_000); + return withPackageIo(async () => { + const parent = skillPackagePaths(companyId, id).parent; + await ensureSkillCacheDirectory(parent, true); + const staging = await fs.mkdtemp(path.join(parent, ".verify-")); + const archive = `${staging}.tar.gz`; + try { + await restoreInto(row, manifest, staging, archive, signal, allowRecovery); + return await consume(staging, manifest); + } finally { await fs.rm(staging, { recursive: true, force: true }); await fs.rm(archive, { force: true }); } + }, signal); + } async function materialize(row: Descriptor, manifest: SkillPackageManifest) { const paths = skillPackagePaths(row.companyId, row.id); return flight(db, `files:${paths.directory}`, async () => { @@ -247,39 +304,7 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { const staging = await fs.mkdtemp(path.join(paths.parent, ".package-")); const archivePath = `${staging}.tar.gz`; try { - let legacy: CompanySkillVersionFileInventoryEntry[] | null = null; - if (!row.storage && !row.restorable) legacy = await legacyFiles(row); - try { - if (row.storage) { - if (row.storage.provider !== storage().provider) throw new Error("Skill package storage provider changed"); - const object = await storage().getObject(row.companyId, row.storage.archiveKey, { signal }); - const digest = boundedHashStream(row.storage.archiveBytes); - await pipeline(object.stream, digest.stream, createWriteStream(archivePath, { flags: "wx", mode: 0o600 }), { signal }); - const actual = digest.result(); - if (actual.bytes !== row.storage.archiveBytes || actual.sha256 !== row.storage.archiveSha256) throw new Error("Skill package archive checksum mismatch"); - await extractSkillPackageArchive(archivePath, staging, manifest, signal); - } - } catch (error) { - signal.throwIfAborted(); - // A missing/corrupt archive must not hide a still-authoritative legacy - // body. Only then use the saved, authorized immutable Git recipe. - const [saved] = await db.select({ files: snapshots.fileInventory }).from(snapshots).where(scope(row.companyId, row.id)); - if (saved?.files.length && skillPackageManifest(saved.files).contentHash === row.contentHash) legacy = saved.files; - else if (row.restorable) legacy = await acquireFiles(row, signal); - else throw error; - await fs.rm(staging, { recursive: true, force: true }); - await fs.mkdir(staging); - } - if (!row.storage && row.restorable && !legacy) legacy = await acquireFiles(row, signal); - if (legacy) { - for (const file of legacy) { - signal.throwIfAborted(); - const target = path.join(staging, assertSkillSnapshotPath(file.path)); - await fs.mkdir(path.dirname(target), { recursive: true, mode: 0o700 }); - await fs.writeFile(target, skillFileBytes(file), { flag: "wx", mode: file.executable ? 0o555 : 0o444 }); - } - } - if (!await verifySkillPackageDirectory(staging, manifest.files)) throw new Error("Restored skill package does not match its manifest"); + await restoreInto(row, manifest, staging, archivePath, signal); // All consumers hold the company cache lock, including processes sharing a volume. await fs.rm(paths.directory, { recursive: true, force: true }); await fs.rename(staging, paths.directory); @@ -300,8 +325,8 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { return consume(restored.directory, manifest); }); } - async function files(companyId: string, id: string): Promise { - return withPackage(companyId, id, async (root, manifest) => { + async function files(companyId: string, id: string, isolated = false): Promise { + return (isolated ? withIsolatedPackage : withPackage)(companyId, id, async (root, manifest) => { const result: CompanySkillVersionFileInventoryEntry[] = []; for (const entry of manifest.files) result.push({ path: entry.path, kind: entry.kind, content: (await fs.readFile(path.join(root, entry.path))).toString(entry.encoding), ...(entry.encoding === "base64" ? { encoding: entry.encoding } : {}), ...(entry.executable ? { executable: true } : {}) }); @@ -385,17 +410,22 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { } }, signal); }); - if (!restore) { - const [retained] = await db.update(snapshots).set({ storage: sql`jsonb_set(${snapshots.storage}, '{durable}', 'true'::jsonb)` }) + { + const [retained] = await db.update(snapshots).set({ maintenanceAt: new Date(Date.now() + 24 * 60 * 60_000), + ...(!restore ? { storage: sql`jsonb_set(${snapshots.storage}, '{durable}', 'true'::jsonb)` } : {}) }) .where(and(scope(companyId, preparedId), sql`${snapshots.storage} is not null`)).returning({ id: snapshots.id }); - if (!retained) return prepare(companyId, files, true, cancellation); + if (!retained) return prepare(companyId, files, true, cancellation, restore); } return preparedId; } - async function verifyDurable(companyId: string, id: string, cancellation?: AbortSignal): Promise { + async function verifyDurable(companyId: string, id: string, cancellation?: AbortSignal, + validate?: (root: string, manifest: SkillPackageManifest) => Promise): Promise { const action = async () => { const row = await descriptor(companyId, id); - if (!row.storage) { await manifestFor(row); if (!row.storage) return true; } + if (!row.storage) { + await manifestFor(row); + if (!row.storage) return validate ? withIsolatedPackage(companyId, id, validate, false) : true; + } if (row.storage.provider !== storage().provider) throw new Error("Skill package storage provider changed; migrate its objects before switching storage"); const signal = cancellation ? AbortSignal.any([cancellation, AbortSignal.timeout(180_000)]) : AbortSignal.timeout(180_000); return withPackageIo(async () => { @@ -416,28 +446,27 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { const restored = path.join(staging, "files"); await fs.mkdir(restored); await extractSkillPackageArchive(archive, restored, manifest, signal); - return true; + return validate ? await validate(restored, manifest) : true; } finally { await fs.rm(staging, { recursive: true, force: true }); } }, signal); }; const context = readScope.getStore(); const key = `${companyId}:${id}`; - if (context?.db !== db) return action(); + if (context?.db !== db || validate) return action(); if (!context.durable.has(key)) context.durable.set(key, action()); return context.durable.get(key)!; } - async function versionFiles(version: StoredSkillVersion): Promise { + async function versionFiles(version: StoredSkillVersion, allowRecovery = true): Promise { if (!version.packageSnapshotId) { const [row] = await db.select({ ...skillVersionMetadataColumns, files: companySkillVersions.fileInventory }).from(companySkillVersions).where(and( eq(companySkillVersions.companyId, version.companyId), eq(companySkillVersions.id, version.id))); - if (row?.packageSnapshotId) return versionFiles(row); + if (row?.packageSnapshotId) return versionFiles(row, allowRecovery); return row?.files ?? []; } - if (!version.packageFileMap) return files(version.companyId, version.packageSnapshotId); - return withPackage(version.companyId, version.packageSnapshotId, async (root, manifest) => { + return withIsolatedPackage(version.companyId, version.packageSnapshotId, async (root, manifest) => { const byPath = new Map(manifest.files.map(file => [file.path, file])); const result: CompanySkillVersionFileInventoryEntry[] = []; - for (const mapping of version.packageFileMap!) { + for (const mapping of version.packageFileMap ?? manifest.files.map(file => ({ ...file, sourcePath: file.path }))) { assertSkillSnapshotPath(mapping.path); assertSkillSnapshotPath(mapping.sourcePath); const file = byPath.get(mapping.sourcePath); @@ -448,8 +477,53 @@ export function skillPackageStore(db: Db, suppliedStorage?: StorageService) { ...(encoding === "base64" ? { encoding } : {}), ...(file.executable ? { executable: true } : {}) }); } return result; - }); + }, allowRecovery); } - return { descriptor, prepare, files, version, prefetch, versionFiles, withPackage, verifyDurable, + async function readVersionFile(version: StoredSkillVersion, relativePath: string, allowRecovery = true) { + assertSkillSnapshotPath(relativePath); + if (!version.packageSnapshotId) return (await versionFiles(version, allowRecovery)).find(file => file.path === relativePath); + const id = version.packageSnapshotId; + const manifest = await manifestFor(await descriptor(version.companyId, id)); + const mapping = version.packageFileMap?.find(file => file.path === relativePath); + if (version.packageFileMap && !mapping) return undefined; + const entry = manifest.files.find(file => file.path === (mapping?.sourcePath ?? relativePath)); + if (!entry) return undefined; + const read = async (root: string) => { + const filename = path.join(root, entry.path); + await ensureSkillCacheDirectory(path.dirname(filename)); + const handle = await fs.open(filename, constants.O_RDONLY | constants.O_NOFOLLOW); + try { + const stat = await handle.stat(); + if (!stat.isFile() || stat.size !== entry.size) throw new Error("Invalid cached skill file"); + const bytes = Buffer.alloc(entry.size); + let offset = 0; + while (offset < bytes.length) { + const { bytesRead } = await handle.read(bytes, offset, bytes.length - offset, offset); + if (!bytesRead) throw new Error("Invalid cached skill file"); + offset += bytesRead; + } + if (packageSha256(bytes) !== entry.sha256) throw new Error("Invalid cached skill file"); + const encoding = mapping?.encoding ?? entry.encoding; + return { path: relativePath, kind: mapping?.kind ?? entry.kind, content: bytes.toString(encoding), + encoding, executable: entry.executable }; + } finally { await handle.close(); } + }; + // Immutable cache files can be read without the preparation lock. An open + // descriptor survives eviction; missing/racing files fall back to storage. + try { return await read(skillPackagePaths(version.companyId, id).directory); } + catch { return withIsolatedPackage(version.companyId, id, read, allowRecovery); } + } + async function versionInventory(version: StoredSkillVersion) { + if (version.packageSnapshotId) { + const index = version.packageFileMap ?? (await manifestFor(await descriptor(version.companyId, version.packageSnapshotId))).files; + return index.map(({ path, kind }) => ({ path, kind })); + } + const [row] = await db.select({ files: sql>` + coalesce((select jsonb_agg(jsonb_build_object('path', f->>'path', 'kind', f->>'kind')) + from jsonb_array_elements(${companySkillVersions.fileInventory}) f), '[]'::jsonb)` }) + .from(companySkillVersions).where(and(eq(companySkillVersions.companyId, version.companyId), eq(companySkillVersions.id, version.id))); + return row?.files ?? []; + } + return { descriptor, prepare, files, version, prefetch, versionFiles, versionInventory, readVersionFile, withPackage, withIsolatedPackage, verifyDurable, manifest: async (companyId: string, id: string) => manifestFor(await descriptor(companyId, id)) }; } diff --git a/server/src/services/skill-package-work.ts b/server/src/services/skill-package-work.ts index 8b9334a8519..7bdf92ff92e 100644 --- a/server/src/services/skill-package-work.ts +++ b/server/src/services/skill-package-work.ts @@ -72,29 +72,41 @@ export function skillPackageWork(db: Db, storage?: StorageService) { const manifest = validateSkillPackageManifest(row.restore.manifest); if (row.restore.version !== 1 || manifest.contentHash !== row.contentHash || packageSha256(skillPackageManifestBytes(manifest)) !== row.storage.manifestSha256) throw new Error("Invalid skill restore recipe; keep durable bytes"); - await withSkillVersionCacheLock(db, row.companyId, async () => { - const referenced = await db.select({ id: versions.id, repositorySnapshotId: versions.repositorySnapshotId }).from(versions).where(and(eq(versions.companyId, row.companyId), - sql`(${versions.repositorySnapshotId} = ${id} or ${versions.packageSnapshotId} = ${id})`)); - if (referenced.some(version => version.repositorySnapshotId !== id)) return; - const [current] = await db.execute(sql`select 1 from company_skills s where s.company_id = ${row.companyId} - and s.current_version_id in (select id from company_skill_versions where repository_snapshot_id = ${id} or package_snapshot_id = ${id}) - union all select 1 from company_skill_test_runs t where t.company_id = ${row.companyId} - and t.status in ('queued', 'running') and t.skill_version_id in - (select id from company_skill_versions where repository_snapshot_id = ${id} or package_snapshot_id = ${id}) limit 1`); - if (current) return; - const pinned = new Set(); - for (const agent of await db.select({ config: agents.adapterConfig }).from(agents).where(eq(agents.companyId, row.companyId))) { - for (const entry of readPaperclipSkillSyncPreference(agent.config ?? {}).desiredSkillEntries) if (entry.versionId) pinned.add(entry.versionId); - } - const active = await protectedVersions(db, row.companyId); - if (!active || referenced.some(version => active.has(version.id) || pinned.has(version.id))) return; - signal.throwIfAborted(); - // Metadata/recipe remain. Triggered deletion receipts retain the old - // object keys through the rollback grace; future reads reacquire the SHA. - const retired = await db.update(snapshots).set({ storage: null }).where(and(eq(snapshots.id, id), isNull(snapshots.upload), - sql`${snapshots.storage}->>'durable' is distinct from 'true'`)).returning({ id: snapshots.id }); - if (retired.length) await removeLocal(); - }, true); + const publishing = await db.transaction(async tx => { + // Publication takes this row lock before making a version current. + // Skip active publishers before taking the warm preparation lock. + const [latest] = await tx.select({ storage: snapshots.storage, upload: snapshots.upload, + migrationAt: snapshots.migrationAt, maintenanceAt: snapshots.maintenanceAt }) + .from(snapshots).where(eq(snapshots.id, id)).for("update", { skipLocked: true }); + if (!latest) return true; + if (!latest.storage || latest.storage.durable || latest.upload || latest.migrationAt + || !latest.maintenanceAt || latest.maintenanceAt.getTime() > Date.now()) return; + await withSkillVersionCacheLock(db, row.companyId, async () => { + const referenced = await tx.select({ id: versions.id, repositorySnapshotId: versions.repositorySnapshotId }).from(versions).where(and(eq(versions.companyId, row.companyId), + sql`(${versions.repositorySnapshotId} = ${id} or ${versions.packageSnapshotId} = ${id})`)); + if (referenced.some(version => version.repositorySnapshotId !== id)) return; + const [current] = await tx.execute(sql`select 1 from company_skills s where s.company_id = ${row.companyId} + and s.current_version_id in (select id from company_skill_versions where repository_snapshot_id = ${id} or package_snapshot_id = ${id}) + union all select 1 from company_skill_test_runs t where t.company_id = ${row.companyId} + and t.status in ('queued', 'running') and t.skill_version_id in + (select id from company_skill_versions where repository_snapshot_id = ${id} or package_snapshot_id = ${id}) limit 1`); + if (current) return; + const pinned = new Set(); + for (const agent of await tx.select({ config: agents.adapterConfig }).from(agents).where(eq(agents.companyId, row.companyId))) { + for (const entry of readPaperclipSkillSyncPreference(agent.config ?? {}).desiredSkillEntries) if (entry.versionId) pinned.add(entry.versionId); + } + const active = await protectedVersions(tx as unknown as Db, row.companyId); + if (!active || referenced.some(version => active.has(version.id) || pinned.has(version.id))) return; + signal.throwIfAborted(); + // Metadata/recipe remain. Triggered deletion receipts retain the old + // object keys through the rollback grace; future reads reacquire the SHA. + const retired = await tx.update(snapshots).set({ storage: null }).where(and(eq(snapshots.id, id), isNull(snapshots.upload), + sql`${snapshots.storage}->>'durable' is distinct from 'true'`)).returning({ id: snapshots.id }); + if (retired.length) await removeLocal(); + }, true); + return false; + }); + if (publishing) return; } } await db.update(snapshots).set({ maintenanceAt: next }).where(and(eq(snapshots.id, id), lte(snapshots.maintenanceAt, new Date()))); diff --git a/server/src/services/skill-version-cache.ts b/server/src/services/skill-version-cache.ts index f5dbbc37985..0357c05e3f5 100644 --- a/server/src/services/skill-version-cache.ts +++ b/server/src/services/skill-version-cache.ts @@ -177,10 +177,11 @@ export async function hasRestorableSnapshot(db: Db, companyId: string, versionId // Git packages intentionally permit byte eviction. Their immutable recipe, // not a network probe during cleanup, is the restoration contract. if ((await store.descriptor(companyId, packageId)).restorable) return true; - if (version.repositorySnapshotId && (entry.encoding === "base64" || !await store.withPackage(companyId, packageId, - async root => repositorySkillFrontmatter(await fs.readFile(path.join(root, entrypoint), "utf8"))))) return false; - // Never evict the only known-good bytes based on a permanent process cache. - return store.verifyDurable(companyId, packageId); + if (version.repositorySnapshotId && entry.encoding === "base64") return false; + // Validate original frontmatter inside the same isolated storage verification, + // outside the preparation lock and without rebuilding the shared cache. + return store.verifyDurable(companyId, packageId, undefined, version.repositorySnapshotId + ? async root => Boolean(repositorySkillFrontmatter(await fs.readFile(path.join(root, entrypoint), "utf8"))) : undefined); } const files = await store.versionFiles(version); return validateInventory(files) && files.some(file => file.path === "SKILL.md"); diff --git a/ui/src/api/companySkills.ts b/ui/src/api/companySkills.ts index 726d7eb9585..9b671d57038 100644 --- a/ui/src/api/companySkills.ts +++ b/ui/src/api/companySkills.ts @@ -38,6 +38,7 @@ import type { CompanySkillUpdateRequest, CompanySkillUpdateStatus, CompanySkillVersion, + CompanySkillVersionSummary, CompanySkillVersionCreateRequest, } from "@paperclipai/shared"; import { api } from "./client"; @@ -66,7 +67,7 @@ export const companySkillsApi = { `/companies/${encodeURIComponent(companyId)}/skills/${encodeURIComponent(skillId)}`, ), versions: (companyId: string, skillId: string) => - api.get( + api.get( `/companies/${encodeURIComponent(companyId)}/skills/${encodeURIComponent(skillId)}/versions`, ), version: (companyId: string, skillId: string, versionId: string) => diff --git a/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx b/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx index 439e0ce92c1..c4e747ae4db 100644 --- a/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx +++ b/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx @@ -6,7 +6,7 @@ import type { AgentDesiredSkillEntry, CompanySkillDetail, CompanySkillUsageAgent, - CompanySkillVersion, + CompanySkillVersionSummary, } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { agentsApi } from "@/api/agents"; @@ -130,7 +130,7 @@ export function AgentsUsingSkillDialog({ if (!open) setConfirmRemoveId(null); }, [open]); - const versions = useMemo( + const versions = useMemo( () => [...(versionsQuery.data ?? [])].sort((a, b) => b.revisionNumber - a.revisionNumber), [versionsQuery.data], ); @@ -308,7 +308,7 @@ function AgentRow({ icon: string | null; canManage: boolean; hasVersions: boolean; - versions: CompanySkillVersion[]; + versions: CompanySkillVersionSummary[]; latestRevision: number | null; pinnedRevision: number | null; busy: boolean; diff --git a/ui/src/pages/CompanySkills.production.tsx b/ui/src/pages/CompanySkills.production.tsx index c0b9ab9a129..c01590d10f2 100644 --- a/ui/src/pages/CompanySkills.production.tsx +++ b/ui/src/pages/CompanySkills.production.tsx @@ -23,7 +23,7 @@ import type { CompanySkillTrustLevel, CompanySkillUpdateRequest, CompanySkillUpdateStatus, - CompanySkillVersion, + CompanySkillVersionSummary, } from "@paperclipai/shared"; import { companySkillsApi } from "../api/companySkills"; import { foldersApi } from "../api/folders"; @@ -2277,7 +2277,7 @@ function AttachAgentsPopover({ }: { agents: AttachAgentOption[]; attachedAgentIds: string[]; - versions: CompanySkillVersion[]; + versions: CompanySkillVersionSummary[]; selectedVersionId: string | null; pending: boolean; onSubmit: (nextIds: string[], versionId: string | null) => void; @@ -2571,12 +2571,12 @@ function currentVersionSelection(detail: CompanySkillDetail | null | undefined) return selected ?? null; } -function versionLabel(version: CompanySkillVersion | null | undefined) { +function versionLabel(version: CompanySkillVersionSummary | null | undefined) { if (!version) return "Latest"; return `v${version.revisionNumber}${version.label ? ` · ${version.label}` : ""}`; } -export function getSkillVersionDiffSelection(versions: CompanySkillVersion[], targetVersionId?: string | null) { +export function getSkillVersionDiffSelection(versions: CompanySkillVersionSummary[], targetVersionId?: string | null) { const sorted = [...versions].sort((a, b) => b.revisionNumber - a.revisionNumber); const right = targetVersionId ? sorted.find((version) => version.id === targetVersionId) ?? null @@ -2601,15 +2601,27 @@ function SkillVersionDiffDialog({ }: { open: boolean; onOpenChange: (open: boolean) => void; - versions: CompanySkillVersion[]; + versions: CompanySkillVersionSummary[]; leftVersionId: string | null; rightVersionId: string | null; onLeftVersionChange: (id: string | null) => void; onRightVersionChange: (id: string | null) => void; }) { const sorted = [...versions].sort((a, b) => b.revisionNumber - a.revisionNumber); - const left = sorted.find((version) => version.id === leftVersionId) ?? null; - const right = sorted.find((version) => version.id === rightVersionId) ?? null; + const selectedLeft = sorted.find((version) => version.id === leftVersionId); + const selectedRight = sorted.find((version) => version.id === rightVersionId); + const leftQuery = useQuery({ + queryKey: [...queryKeys.companySkills.versions(selectedLeft?.companyId ?? "", selectedLeft?.companySkillId ?? ""), leftVersionId], + queryFn: () => companySkillsApi.version(selectedLeft!.companyId, selectedLeft!.companySkillId, selectedLeft!.id), + enabled: open && Boolean(selectedLeft), + }); + const rightQuery = useQuery({ + queryKey: [...queryKeys.companySkills.versions(selectedRight?.companyId ?? "", selectedRight?.companySkillId ?? ""), rightVersionId], + queryFn: () => companySkillsApi.version(selectedRight!.companyId, selectedRight!.companySkillId, selectedRight!.id), + enabled: open && Boolean(selectedRight), + }); + const left = leftQuery.data ?? null; + const right = rightQuery.data ?? null; const allPaths = useMemo(() => { const paths = new Set(); for (const file of left?.fileInventory ?? []) paths.add(file.path); @@ -2651,6 +2663,8 @@ function SkillVersionDiffDialog({
Diff · skill files + {(leftQuery.error || rightQuery.error) &&

{(leftQuery.error ?? rightQuery.error)?.message}

} + {(leftQuery.isLoading || rightQuery.isLoading) &&

Loading selected versions…

}