diff --git a/docker/platformclaw-runtime/OPERATIONS.ko.md b/docker/platformclaw-runtime/OPERATIONS.ko.md index e175305786..5a67ac3074 100644 --- a/docker/platformclaw-runtime/OPERATIONS.ko.md +++ b/docker/platformclaw-runtime/OPERATIONS.ko.md @@ -705,4 +705,33 @@ DOCKER_HOST="unix:///run/user/$(id -u)/docker.sock" docker ps -a | rootless socket 연결 실패 | `/run/user//docker.sock`, user systemd 상태 | | VM 인증 실패 | host-key 승인, 할당, AD 비밀번호, SafeConnect 연결 검사 | +### Knox 그룹방 스킬 복구 확인 + +배포 후 `/skillhub installed`에서 설치 기록 미완료나 방 정책으로 비활성화된 항목을 확인한다. +Control DB에서 다음 읽기 전용 쿼리로 아직 확인이 필요한 서버 설치 기록을 찾는다. + +```sql +select agent_id, slug +from skill_hub_catalog_installations +where agent_id like 'group-%' + and destination = 'platform_server' + and (state != 'completed' or installed_revision is null); +``` + +모든 미완료 행의 설치본이 검증되면 결과는 0건이다. 파일이 없거나 원본과 일치하지 않거나 +방 정책이 설치를 허용하지 않는 행은 계속 제외된다. 이 경우 Control 로그에서 해당 행의 +복구 제외 사유를 확인한다. + +방의 활성 작업이 끝난 뒤 메시지를 보내고, 서비스 계정의 rootless Docker에서 샌드박스의 +마운트를 확인한다. `sandbox`는 확인할 컨테이너 이름이다. + +```bash +DOCKER_HOST="unix:///run/user/$(id -u)/docker.sock" \ + docker inspect -f '{{range .Mounts}}{{println .Destination .Source}}{{end}}' "$sandbox" +``` + +복구된 `/workspace/skills/`에는 `.excluded-skill-empty` 소스가 없어야 하고, +해당 경로의 `SKILL.md`를 읽을 수 있어야 한다. 방 정책으로 차단된 다른 스킬의 제외 +마운트는 남을 수 있다. + 로그를 공유할 때 AD 비밀번호, API key, token, 직원정보를 제거한다. diff --git a/docs/platformclaw/skill-hub.md b/docs/platformclaw/skill-hub.md index f63655a964..53ed6f8f6e 100644 --- a/docs/platformclaw/skill-hub.md +++ b/docs/platformclaw/skill-hub.md @@ -171,6 +171,11 @@ Deleting an installed copy does not delete its Hub original. Deletion checks the current physical directory and revision; it cannot remove another workspace, follow an arbitrary path, or select a sibling by a frontmatter alias. +In a room, `/skillhub installed` identifies copies disabled by an incomplete +installation record or room policy. If a skill mount changes while the room is +busy, wait for the current task to finish and retry the message. The next idle +dispatch can recreate the room sandbox with the current skill mounts. + ## Natural language in Knox Ask the Agent to find a skill for a task. It searches the available catalog, diff --git a/docs/plugins/hooks.md b/docs/plugins/hooks.md index 0de2549109..296eb42e99 100644 --- a/docs/plugins/hooks.md +++ b/docs/plugins/hooks.md @@ -570,6 +570,11 @@ Use the phase-specific hooks for new plugins: run check rejects permissions that need a mask missing from the prepared view. Workspace masks are confined to the executing Agent's `skills/` and `.agents/skills/` source roots. They do not delete or rewrite installed content. + A managed policy owner can return `reconcileSandboxSkillMounts: true` to allow + a restricted Docker dispatch to refresh an idle, writable agent or session + sandbox. Docker verifies that only the managed skill mounts changed before + replacing the container. Competing work keeps its sandbox and the new dispatch + returns a retryable error. Return `{ excludedPaths: [...] }` to exclude exact candidate paths from this run's skill prompt, environment requirements, Code Mode readers, and native CLI skill materialization. Multiple handlers' exclusions are combined; a diff --git a/extensions/knox/src/inbound-replies.test.ts b/extensions/knox/src/inbound-replies.test.ts index dd94f090ce..eb90f26eb5 100644 --- a/extensions/knox/src/inbound-replies.test.ts +++ b/extensions/knox/src/inbound-replies.test.ts @@ -16,7 +16,6 @@ import { type ChannelInboundTurnPlan, } from "openclaw/plugin-sdk/channel-inbound"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { closeOpenClawStateDatabaseForTest } from "openclaw/plugin-sdk/plugin-state-test-runtime"; import { getReplyPayloadNoVisibleReplyReason, type NoVisibleReplyReason, @@ -29,6 +28,10 @@ import { setReplyPayloadMetadata, } from "openclaw/plugin-sdk/reply-payload-testing"; import type { PluginRuntime } from "openclaw/plugin-sdk/runtime-store"; +import { + closeOpenClawAgentDatabasesForTest, + closeOpenClawStateDatabaseForTest, +} from "openclaw/plugin-sdk/sqlite-runtime-testing"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { dispatchKnoxInbound } from "./inbound.js"; import { setKnoxRuntime } from "./runtime.js"; @@ -207,6 +210,7 @@ afterEach(async () => { } vi.useRealTimers(); resetAgentEventsForTest(); + closeOpenClawAgentDatabasesForTest(); closeOpenClawStateDatabaseForTest(); vi.unstubAllEnvs(); vi.unstubAllGlobals(); @@ -823,6 +827,33 @@ describe("Knox reply delivery through the core dispatch boundary", () => { }, ); + it.each(["thrown", "payload"] as const)( + "delivers a safe Korean retry notice for a %s sandbox skill refresh conflict", + async (kind) => { + const error = Object.assign( + new Error("Recreate first: openclaw sandbox recreate --agent test"), + { + code: "sandbox-skill-refresh-busy", + }, + ); + await runReplyBoundary(async () => { + if (kind === "thrown") { + throw new Error("Sandbox provisioning failed", { cause: error }); + } + return buildKnownAgentRunFailureReplyPayload({ + err: error, + sessionCtx: { ChatType: "group", Provider: "knox", Surface: "knox" }, + resolvedVerboseLevel: "off", + }); + }); + expect(sent).toHaveLength(1); + expect(sent[0]).toMatchObject({ status: "error", final: true }); + expect(sent[0]?.text).toContain("스킬 설정을 반영 중입니다. 잠시 후 다시 시도해 주세요."); + expect(sent[0]?.text).toContain("확인 코드: sandbox-skill-refresh-busy"); + expect(sent[0]?.text).not.toContain("openclaw sandbox recreate"); + }, + ); + it.each(["room", "dm"] as const)( "explains an empty result in %s with one Korean terminal notice", async (conversationType) => { diff --git a/extensions/knox/src/inbound.ts b/extensions/knox/src/inbound.ts index 6f33431408..6e8b615627 100644 --- a/extensions/knox/src/inbound.ts +++ b/extensions/knox/src/inbound.ts @@ -1,6 +1,11 @@ import { createHash } from "node:crypto"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { findAgentRunTerminalOutcome, formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; +import { + collectErrorGraphCandidates, + extractErrorCode, + findAgentRunTerminalOutcome, + formatErrorMessage, +} from "openclaw/plugin-sdk/error-runtime"; import { redactSensitiveText } from "openclaw/plugin-sdk/logging-core"; import { getReplyPayloadAgentRunTerminalOutcome, @@ -43,10 +48,24 @@ function describeSourceFailure( return reason === "runtime-error" ? (knownReason ?? reason) : reason; } -function runtimeFailureReason(error: unknown): "timeout" | "cancelled" | "runtime-error" { +function runtimeFailureReason(error: unknown): KnoxReplyFailureReason { const terminal = findAgentRunTerminalOutcome(error); - if (terminal) { - return terminalFailureReason(terminal); + const terminalReason = terminal ? terminalFailureReason(terminal) : undefined; + if (terminalReason && terminalReason !== "runtime-error") { + return terminalReason; + } + const skillRefreshBusy = "sandbox-skill-refresh-busy"; + if ( + collectErrorGraphCandidates(error, (candidate) => [ + candidate.cause, + candidate.error, + ...(Array.isArray(candidate.errors) ? candidate.errors : []), + ]).some((candidate) => extractErrorCode(candidate) === skillRefreshBusy) + ) { + return skillRefreshBusy; + } + if (terminalReason) { + return terminalReason; } let candidate = error; const seen = new Set(); diff --git a/extensions/knox/src/reply-notice.ts b/extensions/knox/src/reply-notice.ts index f8018b60f3..8cebc5e812 100644 --- a/extensions/knox/src/reply-notice.ts +++ b/extensions/knox/src/reply-notice.ts @@ -31,6 +31,7 @@ const REPLY_FAILURE_TEXT: Record = { cancelled: "완료 전에 작업이 취소되었습니다. 계속하려면 남은 작업을 다시 요청해 주세요.", "runtime-error": "작업 실행 중 오류가 발생했습니다. 운영자에게 아래 확인 코드로 실행 기록 확인을 요청해 주세요.", + "sandbox-skill-refresh-busy": "스킬 설정을 반영 중입니다. 잠시 후 다시 시도해 주세요.", "unrenderable-result": "응답에 본문·선택지·첨부 설명이 없어 Knox 메시지로 변환하지 못했습니다. 운영자에게 응답 형식 확인을 요청해 주세요.", }; diff --git a/extensions/platformclaw-execution/src/backend.test.ts b/extensions/platformclaw-execution/src/backend.test.ts index bf85b36b9b..08dda1f17c 100644 --- a/extensions/platformclaw-execution/src/backend.test.ts +++ b/extensions/platformclaw-execution/src/backend.test.ts @@ -565,7 +565,7 @@ describe("PlatformClaw execution backend", () => { expect(dependencies.createPlatformServerHandle).not.toHaveBeenCalled(); }); - it("materializes Basic workspace skills before creating the Docker handle", async () => { + it("materializes Basic workspace skills and forwards the mount refresh policy", async () => { const dependencies = createDependencies(async () => ({ kind: "platform_server", agentId: "person_one", @@ -573,6 +573,15 @@ describe("PlatformClaw execution backend", () => { targetId: "server-default", })); const params = createParams("person_one"); + const mounts = [ + { hostPath: "/gateway/managed-skills", containerPath: "/opt/platformclaw/skills" }, + ]; + vi.mocked(params.materializeSkills!).mockResolvedValueOnce({ + catalog: { revision: "gateway:test", files: [] }, + mounts, + policyApplied: true, + reconcileSandboxSkillMounts: true, + }); await createPlatformClawExecutionBackendFactory(dependencies)(params); @@ -580,6 +589,15 @@ describe("PlatformClaw execution backend", () => { expect(vi.mocked(params.materializeSkills!).mock.invocationCallOrder[0]).toBeLessThan( vi.mocked(dependencies.createPlatformServerHandle).mock.invocationCallOrder[0]!, ); + expect(dependencies.createPlatformServerHandle).toHaveBeenCalledWith( + expect.objectContaining({ + createParams: expect.objectContaining({ + readOnlySkillMounts: mounts, + requireCurrentConfig: true, + reconcileSandboxSkillMounts: true, + }), + }), + ); }); it("does not create a backend handle when target skill discovery fails", async () => { diff --git a/extensions/platformclaw-execution/src/backend.ts b/extensions/platformclaw-execution/src/backend.ts index 87ce67e2b3..bd520951a0 100644 --- a/extensions/platformclaw-execution/src/backend.ts +++ b/extensions/platformclaw-execution/src/backend.ts @@ -351,6 +351,9 @@ export function createPlatformClawExecutionBackendFactory( requireCurrentConfig: materializedSkills.policyApplied ? true : (createParams.requireCurrentConfig ?? true), + ...(materializedSkills.reconcileSandboxSkillMounts + ? { reconcileSandboxSkillMounts: true as const } + : {}), } : createParams; const effectiveSkillCatalog = skillCatalog ?? materializedSkills?.catalog; diff --git a/extensions/platformclaw-skillhub/src/run-policy.test.ts b/extensions/platformclaw-skillhub/src/run-policy.test.ts index 289daf2b2c..2a21dcfd4a 100644 --- a/extensions/platformclaw-skillhub/src/run-policy.test.ts +++ b/extensions/platformclaw-skillhub/src/run-policy.test.ts @@ -54,10 +54,19 @@ describe("SkillHub managed room skill policy", () => { await expect(f.run()).rejects.toThrow("permissions could not be verified"); }); - it("forwards managed source exclusions independently of discoverable candidates", async () => { - const excludedSourceDirectories = ["/workspace/skills/private"]; - await expect( - fixture({ status: 200, body: { excludedPaths: [], excludedSourceDirectories } }).run(), - ).resolves.toEqual({ excludedPaths: [], excludedSourceDirectories }); - }); + it.each([ + { excludedSourceDirectories: [] }, + { excludedSourceDirectories: ["/workspace/skills/private"] }, + ])( + "opts in to room reconciliation with source exclusions $excludedSourceDirectories", + async ({ excludedSourceDirectories }) => { + await expect( + fixture({ status: 200, body: { excludedPaths: [], excludedSourceDirectories } }).run(), + ).resolves.toEqual({ + excludedPaths: [], + excludedSourceDirectories, + reconcileSandboxSkillMounts: true, + }); + }, + ); }); diff --git a/extensions/platformclaw-skillhub/src/run-policy.ts b/extensions/platformclaw-skillhub/src/run-policy.ts index dcd7c44641..0a464d2055 100644 --- a/extensions/platformclaw-skillhub/src/run-policy.ts +++ b/extensions/platformclaw-skillhub/src/run-policy.ts @@ -45,7 +45,11 @@ export function registerSkillHubRunPolicy( } return { excludedPaths: paths, - ...(directories !== undefined ? { excludedSourceDirectories: sourceDirectories } : {}), + // The authenticated control-plane response includes the managed room + // policy even when its current exclusion set is empty. + ...(directories !== undefined + ? { excludedSourceDirectories: sourceDirectories, reconcileSandboxSkillMounts: true } + : {}), }; }); } diff --git a/packages/platformclaw-control-plane/src/knox-skill-hub-http.test.ts b/packages/platformclaw-control-plane/src/knox-skill-hub-http.test.ts index e73ce409f3..3eb23a8dae 100644 --- a/packages/platformclaw-control-plane/src/knox-skill-hub-http.test.ts +++ b/packages/platformclaw-control-plane/src/knox-skill-hub-http.test.ts @@ -93,17 +93,80 @@ async function fixture(enabled = true) { skillId: f.entry!.id, version: "1.0.0", archiveSha256: "a".repeat(64), - installedRevision: "sha256:1111111111111111", installedAt: 10, }; + const recordPendingOrigin = () => + f.store.skillHubCatalog.recordInstallation({ ...origin, state: "pending" }); const recordOrigin = async () => { - await f.store.skillHubCatalog.recordInstallation({ ...origin, state: "pending" }); - await f.store.skillHubCatalog.completeInstallation({ ...origin, now: 11 }); + await recordPendingOrigin(); + await f.store.skillHubCatalog.completeInstallation({ + ...origin, + installedRevision: "sha256:1111111111111111", + now: 11, + }); }; - return { ...f, room, request, readJsonBody, candidate, recordOrigin }; + return { ...f, room, request, readJsonBody, candidate, recordOrigin, recordPendingOrigin }; } describe("Skill Hub internal HTTP boundaries", () => { + it("explains incomplete room installations without exposing undiscovered skill identities", async () => { + const f = await fixture(); + await f.recordPendingOrigin(); + const roomInstallKey = `${f.room.agentId}/platform-server`; + f.installed.set(roomInstallKey, { + slug: "demo-skill", + version: "1.0.0", + revision: "sha256:1111111111111111", + }); + const installed = (locale: "en" | "ko", room = true) => + f.request( + { + accountId: f.actor.user.accountId, + args: "installed", + locale, + context: { + accountId: "default", + conversationType: room ? "room" : "dm", + conversationId: room ? "room-one" : "42", + agentId: room ? f.room.agentId : f.actor.agentId, + sessionKey: `agent:${room ? f.room.agentId : f.actor.agentId}:main`, + }, + }, + { path: commandPath }, + ); + expect(await installed("ko")).toMatchObject({ + status: 200, + body: { text: "설치된 스킬\ndemo-skill · 설치 기록 미완료로 비활성" }, + }); + expect(await installed("en")).toMatchObject({ + status: 200, + body: { + text: "Installed skills\ndemo-skill · Inactive because the installation record is incomplete", + }, + }); + expect(await f.request({ agentId: f.room.agentId, candidates: [f.candidate] })).toMatchObject({ + status: 200, + body: { + excludedPaths: [f.candidate.filePath], + excludedSourceDirectories: [path.dirname(f.candidate.filePath)], + }, + }); + f.installed.set(`${f.actor.agentId}/platform-server`, { + slug: "demo-skill", + version: "1.0.0", + revision: "sha256:1111111111111111", + }); + expect(await installed("ko", false)).toMatchObject({ + status: 200, + body: { text: "설치된 스킬\ndemo-skill · 1.0.0" }, + }); + f.installed.delete(roomInstallKey); + expect(await installed("ko")).toMatchObject({ + status: 200, + body: { text: "설치된 스킬\n설치 기록이 미완료인 비활성 스킬 1개" }, + }); + }); + it.each(["en", "ko"] as const)( "preserves a linked-account 401 and its diagnostic in %s", async (locale) => { @@ -162,6 +225,15 @@ describe("Skill Hub internal HTTP boundaries", () => { ); expect(status.status).toBe(200); expect(status.body.items).toEqual([expect.not.objectContaining({ hubSkillId: f.entry!.id })]); + expect( + await f.request( + { accountId: f.actor.user.accountId, args: "installed", locale: "ko", context }, + { path: commandPath }, + ), + ).toMatchObject({ + status: 200, + body: { text: "설치된 스킬\ndemo-skill · 공유방 스킬 정책으로 비활성" }, + }); expect(await f.request({ agentId: f.room.agentId, candidates: [f.candidate] })).toMatchObject({ status: 200, body: { diff --git a/packages/platformclaw-control-plane/src/skill-hub-catalog-state.ts b/packages/platformclaw-control-plane/src/skill-hub-catalog-state.ts index 90390b2e6c..5b139cdeef 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-catalog-state.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-catalog-state.ts @@ -68,6 +68,8 @@ export type SkillHubCatalogInstallation = { state: "pending" | "completed"; }; +export type SkillHubCatalogMigration = "sharing-v1" | "legacy-room-installations-v1"; + type CatalogMutation = { skillId: string; actorUserId: string; @@ -109,6 +111,12 @@ export interface SkillHubCatalogStore { > & { installedRevision?: string; now: number }, ): Promise; listInstallations(agentId: string, targetId: string): Promise; + listPendingLegacyRoomInstallations(): Promise; + recoverLegacyRoomInstallation( + origin: SkillHubCatalogInstallation, + installedRevision: string, + expectedCatalogRevision: number, + ): Promise; roomExcludedSlugs(agentId: string): Promise; removeInstallation(params: { agentId: string; @@ -220,6 +228,6 @@ export interface SkillHubCatalogStore { now: number; }): Promise; listLegacyCoordinates(): Promise>; - migrationComplete(): Promise; - finishMigration(now: number): Promise; + migrationComplete(id?: SkillHubCatalogMigration): Promise; + finishMigration(now: number, id?: SkillHubCatalogMigration): Promise; } diff --git a/packages/platformclaw-control-plane/src/skill-hub-command-service.ts b/packages/platformclaw-control-plane/src/skill-hub-command-service.ts index 3ad8335c0a..b95f8dd87d 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-command-service.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-command-service.ts @@ -1,7 +1,8 @@ import { randomUUID } from "node:crypto"; -import type { PlatformUser } from "./contracts.js"; +import { ControlPlaneAuthorizationError, type PlatformUser } from "./contracts.js"; import { localizeSkillHubCommandError } from "./skill-hub-command-errors.js"; import { + assertSkillHubWorkspace, resolveKnoxSkillHubWorkspace, type KnoxSkillHubCommandContext, } from "./skill-hub-command-workspace.js"; @@ -186,12 +187,62 @@ export abstract class SkillHubCommandService extends SkillHubPublicationService } if (action === "installed") { const result = await this.commandInstalled(actor); + const inactive = new Map(); + let unlistedIncomplete = 0; + if (actor.roomBinding) { + const catalog = this.options.store.skillHubCatalog; + const origins = await catalog.listInstallations(actor.agentId, result.targetId); + const incomplete = new Set( + origins + .filter( + (origin) => + origin.destination === result.target && + (origin.state !== "completed" || origin.installedRevision === null), + ) + .map((origin) => origin.slug), + ); + const excluded = await catalog + .roomExcludedSlugs(actor.agentId) + .catch((error: unknown) => { + if (error instanceof ControlPlaneAuthorizationError) { + throw new SkillHubServiceError( + "Knox room workspace changed; retry the command", + 409, + ); + } + throw error; + }); + await assertSkillHubWorkspace(this.options.store, actor); + const listed = new Set(result.items.map((item) => item.skillKey)); + const incompleteReason = ko + ? "설치 기록 미완료로 비활성" + : "Inactive because the installation record is incomplete"; + const policyReason = ko + ? "공유방 스킬 정책으로 비활성" + : "Inactive under the room skill policy"; + for (const slug of excluded) { + if (listed.has(slug)) { + inactive.set(slug, incomplete.has(slug) ? incompleteReason : policyReason); + } else if (incomplete.has(slug)) { + // Count undiscoverable records without disclosing catalog-only identities. + unlistedIncomplete += 1; + } + } + } return { text: [ ko ? "설치된 스킬" : "Installed skills", ...result.items.map( - (item) => `${item.skillKey} · ${item.version ?? (ko ? "버전 정보 없음" : "unknown")}`, + (item) => + `${item.skillKey} · ${inactive.get(item.skillKey) ?? item.version ?? (ko ? "버전 정보 없음" : "unknown")}`, ), + ...(unlistedIncomplete > 0 + ? [ + ko + ? `설치 기록이 미완료인 비활성 스킬 ${unlistedIncomplete}개` + : `Inactive skills with incomplete installation records · ${unlistedIncomplete}`, + ] + : []), ].join("\n"), }; } diff --git a/packages/platformclaw-control-plane/src/skill-hub-command.integration.test.ts b/packages/platformclaw-control-plane/src/skill-hub-command.integration.test.ts index 8ded83310e..a33868a607 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-command.integration.test.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-command.integration.test.ts @@ -11,6 +11,7 @@ import { registerPluginCommand, } from "../../../src/plugin-sdk/plugin-runtime.js"; import type { OpenClawPluginApi } from "../../../src/plugins/types.js"; +import { computeSkillPromptVersion } from "../../../src/skills/loading/skill-version.js"; import { resolveRelativeBundledPluginPublicModuleId } from "../../../src/test-utils/bundled-plugin-public-surface.js"; import { readBrowserJsonBody } from "./browser-http-shared.js"; import { KnoxRoutingService } from "./knox-routing-service.js"; @@ -145,13 +146,14 @@ async function setup(enabled = true, room = false, manager?: "admin" | "owner") async function archive(version = "1.0.0") { const zip = new JSZip(); - zip.file( - "SKILL.md", - `---\nname: demo-skill\ndescription: Demo\nversion: ${version}\n---\nInstructions`, - ); + zip.file("SKILL.md", archiveManifest(version)); return await zip.generateAsync({ type: "nodebuffer", compression: "DEFLATE" }); } +function archiveManifest(version: string): string { + return `---\nname: demo-skill\ndescription: Demo\nversion: ${version}\n---\nInstructions`; +} + describe("Knox Skill Hub command transport", () => { it.each([ ["list knowledge", "Page must be 1 to 25", "페이지는 1부터 25까지"], @@ -375,8 +377,9 @@ describe("Knox Skill Hub command transport", () => { : await listVersions(namespace, slug), ); f.adapterMocks.download.mockResolvedValue(await archive()); + const revision = computeSkillPromptVersion(archiveManifest("1.0.0")); let installed = false; - f.adminRpcCall.mockImplementation(async (method) => { + f.adminRpcCall.mockImplementation(async (method, params) => { if (method === "skills.status") { return { skills: installed @@ -386,7 +389,7 @@ describe("Knox Skill Hub command transport", () => { source: "openclaw-workspace", baseDir: "/workspace/skills/demo-skill", version: "1.0.0", - revision: "sha256:1111111111111111", + revision, }, ] : [], @@ -397,6 +400,7 @@ describe("Knox Skill Hub command transport", () => { } if (method === "skills.install") { installed = true; + return { ok: true, slug: "demo-skill", sha256: params.sha256 }; } if (method === "skills.uninstall") { installed = false; @@ -414,7 +418,7 @@ describe("Knox Skill Hub command transport", () => { expect.objectContaining({ agentId: f.agentId, force: true, - expectedSkillRevision: "sha256:1111111111111111", + expectedSkillRevision: revision, }), ); await writeFile( @@ -441,7 +445,7 @@ describe("Knox Skill Hub command transport", () => { "skills.uninstall", expect.objectContaining({ agentId: f.agentId, - expectedSkillRevision: "sha256:1111111111111111", + expectedSkillRevision: revision, }), ); expect(f.adapterMocks.deleteSkill).not.toHaveBeenCalled(); diff --git a/packages/platformclaw-control-plane/src/skill-hub-room-install-recovery.test.ts b/packages/platformclaw-control-plane/src/skill-hub-room-install-recovery.test.ts new file mode 100644 index 0000000000..c5339d3e15 --- /dev/null +++ b/packages/platformclaw-control-plane/src/skill-hub-room-install-recovery.test.ts @@ -0,0 +1,191 @@ +import { createHash } from "node:crypto"; +import { mkdir, writeFile } from "node:fs/promises"; +import path from "node:path"; +import JSZip from "jszip"; +import { describe, expect, it, vi } from "vitest"; +import { computeSkillPromptVersion } from "../../../src/skills/loading/skill-version.js"; +import { SKILL_HUB_UPLOAD_ENTRIES } from "./skill-hub-service-support.js"; +import { createSkillHubServiceFixture } from "./skill-hub-service.test-fixtures.js"; + +const manifest = "---\nname: demo-skill\ndescription: Demo\nversion: 1.0.0\n---\nInstructions\n"; +const revision = computeSkillPromptVersion(manifest); +const migration = "legacy-room-installations-v1"; + +async function setup(personal = false, archivePaths: readonly string[] = []) { + // The legacy workspace and record exist before authentication starts the worker. + const f = await createSkillHubServiceFixture([], undefined, { authenticate: false }); + const room = await f.store.reserveKnoxRoomAgent({ + accountId: "fixture", + roomId: "recovery-room", + reservedAt: 5, + }); + await f.store.transitionAgent({ bindingId: room.binding.id, state: "active", changedAt: 6 }); + const actor = personal + ? f.actor + : { + ...f.actor, + agentId: room.binding.agentId, + workspaceDir: path.join(f.workspaceRoot, room.binding.agentId), + roomBinding: room.binding, + }; + const zip = new JSZip(); + zip.file("SKILL.md", manifest); + zip.file("helper.txt", "expected"); + zip.folder("empty"); + for (const entry of archivePaths) { + zip.file(entry, "", { createFolders: false }); + } + const archive = await zip.generateAsync({ type: "nodebuffer", compression: "DEFLATE" }); + f.adapterMocks.download.mockResolvedValue(archive); + const entry = f.entry!; + return { + ...f, + actor, + archive, + entry, + archiveSha256: createHash("sha256").update(archive).digest("hex"), + skillDir: path.join(actor.workspaceDir, "skills", entry.slug), + }; +} + +describe("Knox room Skill Hub installation recovery", () => { + it.each([ + "match", + "implicit-directories", + "extra-directory", + "changed-file", + "missing", + "digest-mismatch", + "blocked", + "private", + "personal", + ] as const)("reconciles only the verified legacy room installation (%s)", async (scenario) => { + const archivePaths = + scenario === "implicit-directories" + ? Array.from({ length: Math.floor(SKILL_HUB_UPLOAD_ENTRIES / 2) }, (_, index) => + path.posix.join(`parent-${index}`, "data.txt"), + ) + : []; + const f = await setup(scenario === "personal", archivePaths); + const catalog = f.store.skillHubCatalog; + const filePath = path.join(f.skillDir, "SKILL.md"); + if (scenario !== "missing") { + await mkdir(f.skillDir, { recursive: true }); + await mkdir(path.join(f.skillDir, "empty")); + await writeFile(filePath, manifest); + await writeFile( + path.join(f.skillDir, "helper.txt"), + scenario === "changed-file" ? "tampered" : "expected", + ); + for (const entry of archivePaths) { + const filename = path.join(f.skillDir, entry); + await mkdir(path.dirname(filename), { recursive: true }); + await writeFile(filename, ""); + } + if (scenario === "extra-directory") { + await mkdir(path.join(f.skillDir, "unrecorded")); + } + } + if (scenario === "blocked") { + await catalog.blockVersion({ + skillId: f.entry.id, + version: "1.0.0", + reason: "test", + now: 12, + }); + } + if (scenario === "private") { + await f.service.updateMetadata(f.actor.user, f.entry.id, { + expectedRevision: f.entry.revision, + sharing: { mode: "PRIVATE", userIds: [], scopeIds: [] }, + }); + } + const origin = await catalog.recordInstallation({ + operationId: "legacy-test-installation", + agentId: f.actor.agentId, + destination: "platform_server", + targetId: "platform-server", + slug: f.entry.slug, + skillId: f.entry.id, + version: "1.0.0", + archiveSha256: scenario === "digest-mismatch" ? "1".repeat(64) : f.archiveSha256, + installedAt: 13, + }); + const candidates = [{ name: f.entry.slug, filePath }]; + expect((await f.service.roomSkills(f.actor.agentId, candidates)).excludedPaths).toEqual( + scenario === "personal" ? [] : [filePath], + ); + const warning = vi.spyOn(console, "warn").mockImplementation(() => undefined); + try { + expect(await catalog.migrationComplete(migration)).toBe(false); + await f.service.processGovernanceQueue(); + await vi.waitFor(async () => expect(await catalog.migrationComplete(migration)).toBe(true), { + timeout: scenario === "implicit-directories" ? 30_000 : 1_000, + }); + const recovered = scenario === "match" || scenario === "implicit-directories"; + expect(await catalog.listInstallations(f.actor.agentId, "platform-server")).toEqual([ + { + ...origin, + state: recovered ? "completed" : "pending", + installedRevision: recovered ? revision : null, + }, + ]); + expect((await f.service.roomSkills(f.actor.agentId, candidates)).excludedPaths).toEqual( + recovered || scenario === "personal" ? [] : [filePath], + ); + if (!recovered && scenario !== "personal") { + expect(warning).toHaveBeenCalledWith( + "[PlatformClaw Skill Hub] Legacy room skill remains excluded", + expect.objectContaining({ + agentId: f.actor.agentId, + slug: f.entry.slug, + reason: expect.any(String), + }), + ); + } + const downloads = f.adapterMocks.download.mock.calls.length; + await f.service.processGovernanceQueue(); + expect(f.adapterMocks.download).toHaveBeenCalledTimes(downloads); + } finally { + warning.mockRestore(); + } + }); + + it.each(["match", "wrong digest"] as const)( + "completes from the room install receipt (%s)", + async (digest) => { + const f = await setup(); + f.adminRpcCall.mockImplementation(async (method, params) => { + if (method === "skills.status") { + // Pending records can be absent from discovery even after the writer commits. + return { skills: [] }; + } + if (method === "skills.install" && digest === "wrong digest") { + return { ok: true, slug: params.slug, sha256: "0".repeat(64) }; + } + return await f.gatewayCall(method, params); + }); + const install = f.service.install(f.actor, { + id: f.entry.id, + version: "1.0.0", + destination: "platform_server", + }); + if (digest === "match") { + await expect(install).resolves.toMatchObject({ ok: true }); + } else { + await expect(install).rejects.toThrow("Gateway returned an invalid install result"); + } + expect( + f.adminRpcCall.mock.calls.filter(([method]) => method === "skills.status"), + ).toHaveLength(1); + expect( + await f.store.skillHubCatalog.listInstallations(f.actor.agentId, "platform-server"), + ).toEqual([ + expect.objectContaining({ + state: digest === "match" ? "completed" : "pending", + installedRevision: digest === "match" ? revision : null, + }), + ]); + }, + ); +}); diff --git a/packages/platformclaw-control-plane/src/skill-hub-service-base.ts b/packages/platformclaw-control-plane/src/skill-hub-service-base.ts index 8dc763b3c7..2483b22fda 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-service-base.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-service-base.ts @@ -1,9 +1,11 @@ +import { createHash } from "node:crypto"; import { constants as fsConstants, createWriteStream, type Stats } from "node:fs"; import { lstat, mkdtemp, open, readdir, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { Readable, Transform } from "node:stream"; import { finished, pipeline } from "node:stream/promises"; +import { isRecord } from "@openclaw/normalization-core/record-coerce"; import JSZip from "jszip"; import { ControlPlaneStateError, type PlatformUser } from "./contracts.js"; import { GatewayAdminRpcError } from "./gateway-admin-rpc-client.js"; @@ -21,6 +23,7 @@ import { SKILL_HUB_UPLOAD_ARCHIVE_BYTES, SKILL_HUB_UPLOAD_ENTRY_BYTES, SKILL_HUB_UPLOAD_EXPANDED_BYTES, + type SkillHubArchiveInventory, type SkillHubServiceOptions, type SkillInstallTarget, type SkillHubTargetSnapshot, @@ -29,6 +32,152 @@ import { const MAX_SKILL_DIRECTORY_DEPTH = 16; +type WorkspaceSkillFile = { absolute: string; relative: string; stat: Stats }; + +// ZIP extraction can create parent directories that aren't archive entries. +// Search the sorted paths for an explicit directory or a descendant. +function archiveHasDirectory(entryPaths: readonly string[], relative: string): boolean { + const prefix = `${relative}/`; + let first = 0; + let end = entryPaths.length; + while (first < end) { + const middle = Math.floor((first + end) / 2); + if (entryPaths[middle]! < prefix) { + first = middle + 1; + } else { + end = middle; + } + } + return entryPaths[first]?.startsWith(prefix) ?? false; +} + +async function listWorkspaceSkillFiles( + workspaceDir: string, + skill: string, + archivePaths?: readonly string[], +): Promise { + const skillDir = path.resolve(workspaceDir, "skills", skill); + const skillsDir = path.resolve(workspaceDir, "skills"); + if (!isInside(skillsDir, skillDir) || skillDir === skillsDir) { + throw new SkillHubServiceError("invalid workspace skill path", 400); + } + const rootStat = await lstat(skillDir).catch((error: unknown) => { + if (archivePaths && (!isRecord(error) || error.code !== "ENOENT")) { + throw error; + } + return null; + }); + if (!rootStat?.isDirectory() || rootStat.isSymbolicLink()) { + throw new SkillHubServiceError("workspace skill directory is unavailable", 404); + } + const realRoot = await realpath(skillDir); + if (!isInside(await realpath(skillsDir), realRoot)) { + throw new SkillHubServiceError("workspace skill escapes the skills directory", 400); + } + const files: WorkspaceSkillFile[] = []; + const pending = [{ directory: skillDir, depth: 0 }]; + while (pending.length > 0) { + const { directory: current, depth } = pending.pop()!; + const entries = (await readdir(current, { withFileTypes: true })).toSorted((a, b) => + a.name.localeCompare(b.name), + ); + for (const entry of entries) { + const absolute = path.join(current, entry.name); + const stat = await lstat(absolute); + if (stat.isSymbolicLink()) { + throw new SkillHubServiceError("workspace skill contains a symbolic link", 400); + } + if (!isInside(realRoot, await realpath(absolute))) { + throw new SkillHubServiceError("workspace skill contains an escaping path", 400); + } + if (isExcludedSkillArchivePath(entry.name, stat.isDirectory())) { + if (archivePaths) { + throw new SkillHubServiceError("workspace skill contains a reserved path", 400); + } + continue; + } + const relative = path.relative(skillDir, absolute).split(path.sep).join("/"); + if (stat.isDirectory()) { + if (archivePaths && !archiveHasDirectory(archivePaths, relative)) { + throw new SkillHubServiceError("workspace skill contains an unrecorded directory", 400); + } + if (!archivePaths && depth >= MAX_SKILL_DIRECTORY_DEPTH) { + throw new SkillHubServiceError("workspace skill exceeds the directory depth limit", 400); + } + pending.push({ directory: absolute, depth: depth + 1 }); + continue; + } + if (!stat.isFile() || stat.nlink !== 1) { + throw new SkillHubServiceError("workspace skill contains an unsupported file", 400); + } + files.push({ + absolute, + relative, + stat, + }); + if (files.length > MAX_SKILL_FILES || stat.size > SKILL_HUB_UPLOAD_ENTRY_BYTES) { + throw new SkillHubServiceError("workspace skill exceeds the configured package limit", 400); + } + } + } + if (!files.some((file) => file.relative === "SKILL.md")) { + throw new SkillHubServiceError("workspace skill is missing SKILL.md", archivePaths ? 404 : 400); + } + return files; +} + +async function* readWorkspaceSkillFile(file: WorkspaceSkillFile): AsyncGenerator { + const isManifest = file.relative === "SKILL.md"; + const maxBytes = isManifest ? MAX_SKILL_MD_BYTES : SKILL_HUB_UPLOAD_ENTRY_BYTES; + const flags = + process.platform === "win32" + ? fsConstants.O_RDONLY + : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW; + const handle = await open(file.absolute, flags); + try { + const opened = await handle.stat(); + if ( + !opened.isFile() || + opened.nlink !== 1 || + opened.dev !== file.stat.dev || + opened.ino !== file.stat.ino || + opened.size !== file.stat.size || + opened.mtimeMs !== file.stat.mtimeMs + ) { + throw new SkillHubServiceError("workspace skill changed during packaging", 409); + } + let fileBytes = 0; + while (true) { + const buffer = Buffer.allocUnsafe(64 * 1024); + const { bytesRead } = await handle.read(buffer, 0, buffer.byteLength, null); + if (bytesRead === 0) { + break; + } + fileBytes += bytesRead; + if (fileBytes > maxBytes) { + throw new SkillHubServiceError( + isManifest + ? "SKILL.md exceeds the size limit" + : "workspace skill exceeds the configured package limit", + 400, + ); + } + yield buffer.subarray(0, bytesRead); + } + const after = await handle.stat(); + if ( + fileBytes !== opened.size || + after.size !== opened.size || + after.mtimeMs !== opened.mtimeMs || + after.nlink !== 1 + ) { + throw new SkillHubServiceError("workspace skill changed during packaging", 409); + } + } finally { + await handle.close(); + } +} + export abstract class SkillHubServiceBase { protected readonly workspaceRoot: string; private readonly installLocks = new Map>(); @@ -279,72 +428,62 @@ export abstract class SkillHubServiceBase { ); } - protected async packageWorkspaceSkill( + protected async workspaceSkillMatchesArchive( workspaceDir: string, skill: string, - version: string, - ): Promise<{ path: string; size: number; summary: string; cleanup(): Promise }> { - const skillDir = path.resolve(workspaceDir, "skills", skill); - const skillsDir = path.resolve(workspaceDir, "skills"); - if (!isInside(skillsDir, skillDir) || skillDir === skillsDir) { - throw new SkillHubServiceError("invalid workspace skill path", 400); - } - const rootStat = await lstat(skillDir).catch(() => null); - if (!rootStat?.isDirectory() || rootStat.isSymbolicLink()) { - throw new SkillHubServiceError("workspace skill directory is unavailable", 404); + archive: SkillHubArchiveInventory, + ): Promise { + const realWorkspace = await realpath(workspaceDir); + const realWorkspaceRoot = await realpath(this.workspaceRoot); + const realSkillsDir = await realpath(path.join(workspaceDir, "skills")); + if ( + realWorkspace !== path.join(realWorkspaceRoot, path.basename(workspaceDir)) || + realSkillsDir !== path.join(realWorkspace, "skills") + ) { + return false; } - const realRoot = await realpath(skillDir); - if (!isInside(await realpath(skillsDir), realRoot)) { - throw new SkillHubServiceError("workspace skill escapes the skills directory", 400); + const entryPaths = archive.entryPaths.toSorted(); + const files = await listWorkspaceSkillFiles(workspaceDir, skill, entryPaths); + if (files.length !== archive.files.size) { + return false; } - const files: Array<{ absolute: string; relative: string; stat: Stats }> = []; - const pending = [{ directory: skillDir, depth: 0 }]; - while (pending.length > 0) { - const { directory: current, depth } = pending.pop()!; - const entries = (await readdir(current, { withFileTypes: true })).toSorted((a, b) => - a.name.localeCompare(b.name), - ); - for (const entry of entries) { - const absolute = path.join(current, entry.name); - const stat = await lstat(absolute); - if (stat.isSymbolicLink()) { - throw new SkillHubServiceError("workspace skill contains a symbolic link", 400); - } - if (!isInside(realRoot, await realpath(absolute))) { - throw new SkillHubServiceError("workspace skill contains an escaping path", 400); - } - if (isExcludedSkillArchivePath(entry.name, stat.isDirectory())) { - continue; - } - if (stat.isDirectory()) { - if (depth >= MAX_SKILL_DIRECTORY_DEPTH) { - throw new SkillHubServiceError( - "workspace skill exceeds the directory depth limit", - 400, - ); - } - pending.push({ directory: absolute, depth: depth + 1 }); - continue; - } - if (!stat.isFile() || stat.nlink !== 1) { - throw new SkillHubServiceError("workspace skill contains an unsupported file", 400); - } - files.push({ - absolute, - relative: path.relative(skillDir, absolute).split(path.sep).join("/"), - stat, - }); - if (files.length > MAX_SKILL_FILES || stat.size > SKILL_HUB_UPLOAD_ENTRY_BYTES) { - throw new SkillHubServiceError( - "workspace skill exceeds the configured package limit", - 400, - ); - } + for (const file of files) { + const expected = archive.files.get(file.relative); + if (!expected || expected.bytes !== file.stat.size) { + return false; + } + const digest = createHash("sha256"); + for await (const chunk of readWorkspaceSkillFile(file)) { + digest.update(chunk); + } + if (digest.digest("hex") !== expected.sha256) { + return false; } } - if (!files.some((file) => file.relative === "SKILL.md")) { - throw new SkillHubServiceError("workspace skill is missing SKILL.md", 400); - } + // Catch files added, removed, or swapped while streaming the earlier snapshot. + const after = await listWorkspaceSkillFiles(workspaceDir, skill, entryPaths); + return ( + after.length === files.length && + after.every((file, index) => { + const before = files[index]!; + return ( + file.relative === before.relative && + file.stat.dev === before.stat.dev && + file.stat.ino === before.stat.ino && + file.stat.size === before.stat.size && + file.stat.mtimeMs === before.stat.mtimeMs && + file.stat.ctimeMs === before.stat.ctimeMs + ); + }) + ); + } + + protected async packageWorkspaceSkill( + workspaceDir: string, + skill: string, + version: string, + ): Promise<{ path: string; size: number; summary: string; cleanup(): Promise }> { + const files = await listWorkspaceSkillFiles(workspaceDir, skill); let totalBytes = 0; let summary: string | undefined; @@ -354,76 +493,29 @@ export abstract class SkillHubServiceBase { throw new SkillHubServiceError("workspace skill exceeds the configured package limit", 400); } }; - const readSource = async function* (file: (typeof files)[number]) { - const flags = - process.platform === "win32" - ? fsConstants.O_RDONLY - : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW; - const handle = await open(file.absolute, flags); - try { - const opened = await handle.stat(); - if ( - !opened.isFile() || - opened.nlink !== 1 || - opened.dev !== file.stat.dev || - opened.ino !== file.stat.ino || - opened.size !== file.stat.size || - opened.mtimeMs !== file.stat.mtimeMs - ) { - throw new SkillHubServiceError("workspace skill changed during packaging", 409); - } - const markdown: Buffer[] = []; - let fileBytes = 0; - while (true) { - const buffer = Buffer.allocUnsafe(64 * 1024); - const { bytesRead } = await handle.read(buffer, 0, buffer.byteLength, null); - if (bytesRead === 0) { - break; - } - const chunk = buffer.subarray(0, bytesRead); - fileBytes += bytesRead; - if (fileBytes > SKILL_HUB_UPLOAD_ENTRY_BYTES) { - throw new SkillHubServiceError( - "workspace skill exceeds the configured package limit", - 400, - ); - } - if (file.relative === "SKILL.md") { - if (fileBytes > MAX_SKILL_MD_BYTES) { - throw new SkillHubServiceError("SKILL.md exceeds the size limit", 400); - } - markdown.push(chunk); - } else { - countBytes(bytesRead); - yield chunk; - } + const readSource = async function* (file: WorkspaceSkillFile) { + if (file.relative !== "SKILL.md") { + for await (const chunk of readWorkspaceSkillFile(file)) { + countBytes(chunk.byteLength); + yield chunk; } - const after = await handle.stat(); - if ( - fileBytes !== opened.size || - after.size !== opened.size || - after.mtimeMs !== opened.mtimeMs || - after.nlink !== 1 - ) { - throw new SkillHubServiceError("workspace skill changed during packaging", 409); - } - if (file.relative === "SKILL.md") { - const metadata = parseSkillMarkdown( - Buffer.concat(markdown, fileBytes).toString("utf8"), - skill, - version, - ); - const bytes = Buffer.from(metadata.source, "utf8"); - if (bytes.byteLength > MAX_SKILL_MD_BYTES) { - throw new SkillHubServiceError("SKILL.md exceeds the size limit", 400); - } - summary = metadata.description; - countBytes(bytes.byteLength); - yield bytes; - } - } finally { - await handle.close(); + return; + } + if (file.stat.size > MAX_SKILL_MD_BYTES) { + throw new SkillHubServiceError("SKILL.md exceeds the size limit", 400); + } + const markdown: Buffer[] = []; + for await (const chunk of readWorkspaceSkillFile(file)) { + markdown.push(chunk); + } + const metadata = parseSkillMarkdown(Buffer.concat(markdown).toString("utf8"), skill, version); + const bytes = Buffer.from(metadata.source, "utf8"); + if (bytes.byteLength > MAX_SKILL_MD_BYTES) { + throw new SkillHubServiceError("SKILL.md exceeds the size limit", 400); } + summary = metadata.description; + countBytes(bytes.byteLength); + yield bytes; }; const directory = await mkdtemp(path.join(tmpdir(), `platformclaw-skill-hub-${skill}-`)); const archivePath = path.join(directory, "archive.zip"); diff --git a/packages/platformclaw-control-plane/src/skill-hub-service-installation.ts b/packages/platformclaw-control-plane/src/skill-hub-service-installation.ts index ef98e1ef70..58e124ddf4 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-service-installation.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-service-installation.ts @@ -1,11 +1,13 @@ import { createHash } from "node:crypto"; import type { PlatformUser } from "./contracts.js"; import { SkillHubCatalogService } from "./skill-hub-catalog-service.js"; -import { assertSkillHubWorkspace } from "./skill-hub-command-workspace.js"; +import type { SkillHubCatalogInstallation } from "./skill-hub-catalog-state.js"; +import { assertSkillHubWorkspace, workspaceForAgent } from "./skill-hub-command-workspace.js"; import { compareSemVer, isValidSkillHubVersion, MAX_SKILL_FILES, + SKILL_KEY_PATTERN, SKILL_HUB_UPLOAD_ARCHIVE_BYTES, SKILL_HUB_UPLOAD_ENTRY_BYTES, SKILL_HUB_UPLOAD_EXPANDED_BYTES, @@ -15,11 +17,21 @@ import { validVersion, validateDownloadedArchive, type AuthenticatedWorkspace, + type SkillHubArchiveInventory, type SkillHubAudience, type SkillInstallTarget, type SkillHubTargetSnapshot, } from "./skill-hub-service-support.js"; +const archiveLimits = { + archiveBytes: SKILL_HUB_UPLOAD_ARCHIVE_BYTES, + expandedBytes: SKILL_HUB_UPLOAD_EXPANDED_BYTES, + entryBytes: SKILL_HUB_UPLOAD_ENTRY_BYTES, + files: MAX_SKILL_FILES, + entries: SKILL_HUB_UPLOAD_ENTRIES, +}; +const legacyRoomMigration = "legacy-room-installations-v1"; + type SkillHubInstallResult = { ok: true; noOp: false; @@ -30,6 +42,135 @@ type SkillHubInstallResult = { export abstract class SkillHubInstallationService extends SkillHubCatalogService { private readonly installsInFlight = new Map>(); + private legacyRoomRecovery?: Promise; + + protected recoverLegacyRoomInstallations(): Promise { + this.legacyRoomRecovery ??= this.recoverLegacyRoomInstallationsOnce() + .catch(() => { + console.warn("[PlatformClaw Skill Hub] Legacy room install recovery will retry"); + }) + .finally(() => { + this.legacyRoomRecovery = undefined; + }); + return this.legacyRoomRecovery; + } + + private async recoverLegacyRoomInstallationsOnce(): Promise { + const catalog = this.options.store.skillHubCatalog; + if ( + !(await catalog.migrationComplete()) || + (await catalog.migrationComplete(legacyRoomMigration)) + ) { + return; + } + let retry = false; + for (const origin of await catalog.listPendingLegacyRoomInstallations()) { + if (this.governanceClosed) { + return; + } + let reason: string | undefined; + try { + reason = await this.recoverLegacyRoomInstallation(origin); + retry ||= reason === "state changed during verification"; + } catch { + reason = "verification unavailable, will retry"; + retry = true; + } + if (reason) { + console.warn("[PlatformClaw Skill Hub] Legacy room skill remains excluded", { + agentId: origin.agentId, + slug: origin.slug, + reason, + }); + } + } + if (!retry && !this.governanceClosed) { + await catalog.finishMigration(this.now(), legacyRoomMigration); + } + } + + private async recoverLegacyRoomInstallation( + origin: SkillHubCatalogInstallation, + ): Promise { + const catalog = this.options.store.skillHubCatalog; + if (!SKILL_KEY_PATTERN.test(origin.slug) || !isValidSkillHubVersion(origin.version)) { + return "invalid installation identity"; + } + const entry = await catalog.get(origin.skillId); + if ( + !entry || + entry.sharing.mode !== "PUBLIC" || + entry.deletedAt !== null || + entry.purgedAt !== null || + entry.purgePending || + entry.currentVersion === null + ) { + return "skill is unavailable to rooms"; + } + if ((await catalog.getBlockedVersions(entry.id)).includes(origin.version)) { + return "version is blocked"; + } + const governance = this.options.governance; + if (!governance || entry.registryId === null) { + throw new SkillHubServiceError("Managed artifact access is unavailable", 503); + } + const version = (await this.versionScans(entry)).find( + (item) => item.version === origin.version, + ); + if (!version?.downloadAvailable) { + return version?.scanner.blocked ? "version is blocked" : "recorded artifact is unavailable"; + } + const archive = await governance.downloadArtifact({ + skillId: entry.registryId, + versionId: version.id, + namespace: entry.namespace, + slug: entry.slug, + }); + if (createHash("sha256").update(archive).digest("hex") !== origin.archiveSha256) { + return "recorded archive digest does not match"; + } + const inventory: SkillHubArchiveInventory = { entryPaths: [], files: new Map() }; + let revision: string; + try { + revision = await validateDownloadedArchive( + archive, + origin.slug, + origin.version, + archiveLimits, + inventory, + ); + } catch (error) { + if (error instanceof SkillHubServiceError && error.statusCode === 400) { + return "recorded archive failed validation"; + } + throw error; + } + const workspaceDir = workspaceForAgent(this.workspaceRoot, origin.agentId); + return await this.withWorkspaceMutationLock(origin.agentId, origin.destination, async () => { + try { + if (!(await this.workspaceSkillMatchesArchive(workspaceDir, origin.slug, inventory))) { + return "workspace files do not match the recorded archive"; + } + } catch (error) { + if ( + (error as NodeJS.ErrnoException)?.code === "ENOENT" || + (error instanceof SkillHubServiceError && error.statusCode === 404) + ) { + return "workspace files are missing"; + } + if (error instanceof SkillHubServiceError && error.statusCode === 400) { + return "workspace files are invalid"; + } + throw error; + } + const recovered = await catalog.recoverLegacyRoomInstallation( + origin, + revision, + entry.revision, + ); + return recovered ? undefined : "state changed during verification"; + }); + } async install( actor: AuthenticatedWorkspace, @@ -178,13 +319,7 @@ export abstract class SkillHubInstallationService extends SkillHubCatalogService namespace, slug, }); - await validateDownloadedArchive(archive, slug, version, { - archiveBytes: SKILL_HUB_UPLOAD_ARCHIVE_BYTES, - expandedBytes: SKILL_HUB_UPLOAD_EXPANDED_BYTES, - entryBytes: SKILL_HUB_UPLOAD_ENTRY_BYTES, - files: MAX_SKILL_FILES, - entries: SKILL_HUB_UPLOAD_ENTRIES, - }); + const archiveRevision = await validateDownloadedArchive(archive, slug, version, archiveLimits); const sha256 = createHash("sha256").update(archive).digest("hex"); const begin = await this.gatewayCall<{ uploadId?: string; receivedBytes?: number }>( "skills.upload.begin", @@ -270,20 +405,30 @@ export abstract class SkillHubInstallationService extends SkillHubCatalogService expectedTargetRevision: execution.targetRevision, ...(installedRevision ? { expectedSkillRevision: installedRevision } : {}), }); - if (result.ok !== true || result.slug !== slug) { - throw new SkillHubServiceError("Gateway returned an invalid install result", 503); - } - const installedStatus = await this.workspaceSkills(actor, params.destination, execution); - const installedSkill = installedStatus.items.find((item) => item.skillKey === slug); if ( - !installedSkill?.revision || - !/^sha256:[a-f0-9]{16}$/u.test(installedSkill.revision) || - installedSkill.version !== version + result.ok !== true || + result.slug !== slug || + (actor.roomBinding && result.sha256 !== sha256) ) { - throw new SkillHubServiceError( - "Installation completed but its revision is not verified; refresh the workspace before retrying", - 503, - ); + throw new SkillHubServiceError("Gateway returned an invalid install result", 503); + } + // Room discovery may still hide a pending install. The Gateway just committed + // the exact validated archive, so its manifest already supplies the revision. + let committedRevision = archiveRevision; + if (!actor.roomBinding) { + const installedStatus = await this.workspaceSkills(actor, params.destination, execution); + const installedSkill = installedStatus.items.find((item) => item.skillKey === slug); + if ( + !installedSkill?.revision || + !/^sha256:[a-f0-9]{16}$/u.test(installedSkill.revision) || + installedSkill.version !== version + ) { + throw new SkillHubServiceError( + "Installation completed but its revision is not verified; refresh the workspace before retrying", + 503, + ); + } + committedRevision = installedSkill.revision; } await this.options.store.skillHubCatalog.completeInstallation({ operationId: installation.operationId, @@ -294,7 +439,7 @@ export abstract class SkillHubInstallationService extends SkillHubCatalogService skillId: entry.id, version, archiveSha256: sha256, - installedRevision: installedSkill.revision, + installedRevision: committedRevision, now: this.now(), }); return { @@ -350,13 +495,7 @@ export abstract class SkillHubInstallationService extends SkillHubCatalogService namespace: entry.namespace, slug: entry.slug, }); - await validateDownloadedArchive(archive, entry.slug, version, { - archiveBytes: SKILL_HUB_UPLOAD_ARCHIVE_BYTES, - expandedBytes: SKILL_HUB_UPLOAD_EXPANDED_BYTES, - entryBytes: SKILL_HUB_UPLOAD_ENTRY_BYTES, - files: MAX_SKILL_FILES, - entries: SKILL_HUB_UPLOAD_ENTRIES, - }); + await validateDownloadedArchive(archive, entry.slug, version, archiveLimits); await this.authorizeVersionInstallation(user, id, version, audience); return { archive, filename: `${entry.slug}-${version}.zip` }; } diff --git a/packages/platformclaw-control-plane/src/skill-hub-service-support.ts b/packages/platformclaw-control-plane/src/skill-hub-service-support.ts index e332cd15a3..251d5d2665 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-service-support.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-service-support.ts @@ -1,6 +1,8 @@ +import { createHash } from "node:crypto"; import path from "node:path"; import JSZip, { type JSZipObject } from "jszip"; import { isMap, parseDocument } from "yaml"; +import { computeSkillPromptVersion } from "../../../src/skills/loading/skill-version.js"; import type { BrowserAuthService } from "./browser-auth-service.js"; import type { ControlPlaneAuditWriter, @@ -249,8 +251,10 @@ async function readZipEntryBounded( entry: JSZipObject, extracted: { bytes: number }, limits: { expandedBytes: number; entryBytes: number }, + fileDigests?: Map, ): Promise { const retain = entry.name === "SKILL.md"; + const digest = fileDigests && createHash("sha256"); return await new Promise((resolve, reject) => { const stream = entry.nodeStream("nodebuffer") as NodeJS.ReadableStream & { destroy(): void; @@ -291,6 +295,7 @@ async function readZipEntryBounded( if (retain) { chunks.push(chunk); } + digest?.update(chunk); }); stream.once("error", () => { fail(new SkillHubServiceError("Skill Hub returned an invalid ZIP archive", 400)); @@ -300,11 +305,20 @@ async function readZipEntryBounded( return; } settled = true; + if (digest) { + fileDigests?.set(entry.name, { bytes: entryBytes, sha256: digest.digest("hex") }); + } resolve(retain ? Buffer.concat(chunks, entryBytes) : undefined); }); }); } +type SkillHubArchiveFileDigest = { bytes: number; sha256: string }; +export type SkillHubArchiveInventory = { + entryPaths: string[]; + files: Map; +}; + export async function validateDownloadedArchive( archive: Buffer, expectedSlug: string, @@ -316,7 +330,8 @@ export async function validateDownloadedArchive( files: number; entries: number; }, -): Promise { + inventory?: SkillHubArchiveInventory, +): Promise { if (archive.byteLength === 0 || archive.byteLength > limits.archiveBytes) { throw new SkillHubServiceError("Skill Hub archive exceeds the configured size limit", 400); } @@ -339,14 +354,15 @@ export async function validateDownloadedArchive( const extracted = { bytes: 0 }; let skillMarkdown: Buffer | undefined; for (const entry of entries) { - zipEntryPath(entry); + const entryPath = zipEntryPath(entry); if (isZipSymlink(entry)) { throw new SkillHubServiceError("Skill Hub archive contains a symbolic link", 400); } + inventory?.entryPaths.push(entryPath); if (entry.dir) { continue; } - const content = await readZipEntryBounded(entry, extracted, limits); + const content = await readZipEntryBounded(entry, extracted, limits, inventory?.files); if (content) { skillMarkdown = content; } @@ -354,10 +370,12 @@ export async function validateDownloadedArchive( if (!skillMarkdown) { throw new SkillHubServiceError("Skill Hub archive is missing a valid SKILL.md", 400); } - const metadata = parseSkillMarkdown(skillMarkdown.toString("utf8"), expectedSlug); + const markdown = skillMarkdown.toString("utf8"); + const metadata = parseSkillMarkdown(markdown, expectedSlug); if (metadata.version !== expectedVersion) { throw new SkillHubServiceError("SKILL.md version does not match the requested version", 400, { code: "skill-hub-version-mismatch", }); } + return computeSkillPromptVersion(markdown); } diff --git a/packages/platformclaw-control-plane/src/skill-hub-service.test-fixtures.ts b/packages/platformclaw-control-plane/src/skill-hub-service.test-fixtures.ts index 05d55ea964..d0d20a0f72 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-service.test-fixtures.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-service.test-fixtures.ts @@ -3,6 +3,7 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import JSZip from "jszip"; import { afterEach, vi } from "vitest"; +import { computeSkillPromptVersion } from "../../../src/skills/loading/skill-version.js"; import { createTempDirTracker } from "../../../test/helpers/temp-dir.js"; import { BrowserAuthService, hashBrowserSessionToken } from "./browser-auth-service.js"; import { testHostKey } from "./execution-store.test-support.js"; @@ -32,7 +33,11 @@ afterEach(() => { export async function createSkillHubServiceFixture( groups: string[] = [], governance?: Partial, - options: { registrySkill?: "absent" | "existing"; now?: () => number } = {}, + options: { + registrySkill?: "absent" | "existing"; + now?: () => number; + authenticate?: boolean; + } = {}, ) { const workspaceRoot = tempDirs.make("platformclaw-skill-hub-"); let userId = 0; @@ -316,9 +321,13 @@ export async function createSkillHubServiceFixture( installed.set(`${agentId}/${target}`, { slug: String(params.slug), version, - revision: `sha256:${createHash("sha256").update(bytes).digest("hex").slice(0, 16)}`, + revision: computeSkillPromptVersion(markdown), }); - return { ok: true, slug: params.slug }; + return { + ok: true, + slug: params.slug, + sha256: createHash("sha256").update(bytes).digest("hex"), + }; } return { ok: true }; }; @@ -488,7 +497,8 @@ export async function createSkillHubServiceFixture( }, }); resources.push({ service, store }); - const authenticated = await service.authenticate("session-token"); + const authenticated = + options.authenticate === false ? actor : await service.authenticate("session-token"); if (!authenticated) { throw new Error("Fixture authentication failed"); } diff --git a/packages/platformclaw-control-plane/src/skill-hub-service.ts b/packages/platformclaw-control-plane/src/skill-hub-service.ts index b69961eb0e..e53d57483a 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-service.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-service.ts @@ -417,6 +417,11 @@ export class SkillHubService extends SkillHubCommandService { "Skill Hub cleanup is pending. Existing trash stays unavailable; an administrator can retry cleanup."; } finally { this.governanceProcessing = false; + // Archive verification can wait on the registry. Keep it off startup's + // awaited governance pass and let the existing timer retry outages. + if (!this.governanceClosed) { + void this.recoverLegacyRoomInstallations(); + } this.triggerGovernanceProcessing(30_000); } return { processed }; diff --git a/packages/platformclaw-control-plane/src/skill-hub-tool.integration.test.ts b/packages/platformclaw-control-plane/src/skill-hub-tool.integration.test.ts index ee1ee6068f..2403cc3fa9 100644 --- a/packages/platformclaw-control-plane/src/skill-hub-tool.integration.test.ts +++ b/packages/platformclaw-control-plane/src/skill-hub-tool.integration.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import JSZip from "jszip"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { OpenClawPluginApi, OpenClawPluginToolFactory } from "../../../src/plugins/types.js"; +import { computeSkillPromptVersion } from "../../../src/skills/loading/skill-version.js"; import { resolveRelativeBundledPluginPublicModuleId } from "../../../src/test-utils/bundled-plugin-public-surface.js"; import { readBrowserJsonBody } from "./browser-http-shared.js"; import { KnoxRoutingService } from "./knox-routing-service.js"; @@ -71,10 +72,9 @@ async function setup(room = true, manager?: "admin" | "owner") { { id: 1, version: "1.0.0", status: "PUBLISHED", downloadAvailable: true }, ]); const zip = new JSZip(); - zip.file( - "SKILL.md", - "---\nname: demo-skill\ndescription: Synthetic helper\nversion: 1.0.0\n---\nInstructions", - ); + const markdown = + "---\nname: demo-skill\ndescription: Synthetic helper\nversion: 1.0.0\n---\nInstructions"; + zip.file("SKILL.md", markdown); fixture.adapterMocks.download.mockResolvedValue( await zip.generateAsync({ type: "nodebuffer", compression: "DEFLATE" }), ); @@ -83,14 +83,14 @@ async function setup(room = true, manager?: "admin" | "owner") { source: "openclaw-workspace", baseDir: "/workspace/skills/demo-skill", version: "1.0.0", - revision: "sha256:1111111111111111", + revision: computeSkillPromptVersion(markdown), eligible: true, disabled: false, modelVisible: true, missing: { bins: [], anyBins: [], env: [], config: [], os: [] }, }; let installed = false; - fixture.adminRpcCall.mockImplementation(async (method: string) => { + fixture.adminRpcCall.mockImplementation(async (method, params) => { if (method === "skills.status") { return { skills: installed ? [ready] : [] }; } @@ -99,7 +99,7 @@ async function setup(room = true, manager?: "admin" | "owner") { } if (method === "skills.install") { installed = true; - return { ok: true, slug: "demo-skill" }; + return { ok: true, slug: "demo-skill", sha256: params.sha256 }; } if (method === "skills.uninstall") { installed = false; diff --git a/packages/platformclaw-control-plane/src/sqlite-skill-hub-catalog-artifacts.ts b/packages/platformclaw-control-plane/src/sqlite-skill-hub-catalog-artifacts.ts index d4dd869398..678d2202f2 100644 --- a/packages/platformclaw-control-plane/src/sqlite-skill-hub-catalog-artifacts.ts +++ b/packages/platformclaw-control-plane/src/sqlite-skill-hub-catalog-artifacts.ts @@ -12,11 +12,13 @@ import { takeFirstSync, } from "./kysely-sync.js"; import type { + SkillHubCatalogInstallation, SkillHubCatalogStore, SkillHubPublicationReceipt, SkillHubSharing, } from "./skill-hub-catalog-state.js"; import type { + SkillHubCatalogInstallationRow, SkillHubCatalogPublicationRow, SkillHubCatalogRow, } from "./sqlite-schema-skill-hub-catalog.js"; @@ -37,6 +39,22 @@ function receipt(row: SkillHubCatalogPublicationRow): SkillHubPublicationReceipt }; } +function installation(row: SkillHubCatalogInstallationRow): SkillHubCatalogInstallation { + return { + operationId: row.operation_id, + agentId: row.agent_id, + destination: row.destination, + targetId: row.target_id, + slug: row.slug, + skillId: row.skill_id, + version: row.version, + archiveSha256: row.archive_sha256, + installedRevision: row.installed_revision, + installedAt: row.installed_at, + state: row.state, + }; +} + export class SqliteSkillHubCatalogArtifacts extends SqliteSkillHubCatalogCore { async blockVersion(params: Parameters[0]) { this.ensure(); @@ -163,24 +181,7 @@ export class SqliteSkillHubCatalogArtifacts extends SqliteSkillHubCatalogCore { throw new ControlPlaneStateError("Invalid installed skill revision"); } const completed = Number( - executeSync( - this.db, - this.query - .updateTable("skill_hub_catalog_installations") - .set({ - state: "completed", - installed_revision: params.installedRevision ?? null, - installed_at: params.now, - }) - .where("agent_id", "=", params.agentId) - .where("destination", "=", params.destination) - .where("target_id", "=", params.targetId) - .where("slug", "=", params.slug) - .where("operation_id", "=", params.operationId) - .where("skill_id", "=", params.skillId) - .where("version", "=", params.version) - .where("archive_sha256", "=", params.archiveSha256), - ).numAffectedRows, + executeSync(this.db, this.completeInstallationQuery(params)).numAffectedRows, ); if (completed === 0) { throw new ControlPlaneConflictError( @@ -190,6 +191,26 @@ export class SqliteSkillHubCatalogArtifacts extends SqliteSkillHubCatalogCore { } } + protected completeInstallationQuery( + params: Parameters[0], + ) { + return this.query + .updateTable("skill_hub_catalog_installations") + .set({ + state: "completed", + installed_revision: params.installedRevision ?? null, + installed_at: params.now, + }) + .where("agent_id", "=", params.agentId) + .where("destination", "=", params.destination) + .where("target_id", "=", params.targetId) + .where("slug", "=", params.slug) + .where("operation_id", "=", params.operationId) + .where("skill_id", "=", params.skillId) + .where("version", "=", params.version) + .where("archive_sha256", "=", params.archiveSha256); + } + async listInstallations(agentId: string, targetId: string) { this.ensure(); return executeSync( @@ -201,19 +222,26 @@ export class SqliteSkillHubCatalogArtifacts extends SqliteSkillHubCatalogCore { .where("target_id", "=", targetId) .orderBy("destination") .orderBy("slug"), - ).rows.map((row) => ({ - operationId: row.operation_id, - agentId: row.agent_id, - destination: row.destination, - targetId: row.target_id, - slug: row.slug, - skillId: row.skill_id, - version: row.version, - archiveSha256: row.archive_sha256, - installedRevision: row.installed_revision, - installedAt: row.installed_at, - state: row.state, - })); + ).rows.map(installation); + } + + async listPendingLegacyRoomInstallations() { + this.ensure(); + return executeSync( + this.db, + this.query + .selectFrom("skill_hub_catalog_installations as origin") + .innerJoin("agent_bindings as binding", "binding.agent_id", "origin.agent_id") + .selectAll("origin") + .where("binding.kind", "=", "knox-room") + .where("origin.destination", "=", "platform_server") + .where("origin.target_id", "=", "platform-server") + .where("origin.operation_id", "like", "legacy-%") + .where("origin.state", "=", "pending") + .where("origin.installed_revision", "is", null) + .orderBy("origin.agent_id") + .orderBy("origin.slug"), + ).rows.map(installation); } async removeInstallation(params: Parameters[0]) { @@ -260,53 +288,60 @@ export class SqliteSkillHubCatalogArtifacts extends SqliteSkillHubCatalogCore { if (binding.kind === "personal") { return []; } - const origins = executeSync( - this.db, - this.query - .selectFrom("skill_hub_catalog_installations as origin") - .leftJoin("skill_hub_catalog as skill", "skill.id", "origin.skill_id") - .leftJoin("skill_hub_catalog_blocked_versions as blocked", (join) => - join - .onRef("blocked.skill_id", "=", "origin.skill_id") - .onRef("blocked.version", "=", "origin.version"), - ) - .select([ - "origin.slug", - "origin.state", - "origin.installed_revision", - "skill.id", - "skill.deleted_at", - "skill.purged_at", - "skill.purge_pending", - "skill.sharing_mode", - "skill.current_version", - "blocked.version as blocked_version", - ]) - .where("origin.agent_id", "=", agentId) - .where("origin.destination", "=", "platform_server") - .where("origin.target_id", "=", "platform-server"), - ).rows; - return [ - ...new Set( - origins - .filter( - (origin) => - origin.state !== "completed" || - origin.installed_revision === null || - origin.id === null || - origin.deleted_at !== null || - origin.purged_at !== null || - origin.purge_pending !== 0 || - origin.sharing_mode !== "PUBLIC" || - origin.current_version === null || - origin.blocked_version !== null, - ) - .map((origin) => origin.slug), - ), - ]; + return this.roomInstallationRows(agentId) + .filter( + (origin) => + origin.state !== "completed" || + origin.installed_revision === null || + !this.roomSkillAvailable(origin), + ) + .map((origin) => origin.slug); }); } + protected roomInstallationRows(agentId: string, slug?: string) { + const query = this.query + .selectFrom("skill_hub_catalog_installations as origin") + .leftJoin("skill_hub_catalog as skill", "skill.id", "origin.skill_id") + .leftJoin("skill_hub_catalog_blocked_versions as blocked", (join) => + join + .onRef("blocked.skill_id", "=", "origin.skill_id") + .onRef("blocked.version", "=", "origin.version"), + ) + .select([ + "origin.slug", + "origin.state", + "origin.installed_revision", + "skill.id", + "skill.revision", + "skill.deleted_at", + "skill.purged_at", + "skill.purge_pending", + "skill.sharing_mode", + "skill.current_version", + "blocked.version as blocked_version", + ]) + .where("origin.agent_id", "=", agentId) + .where("origin.destination", "=", "platform_server") + .where("origin.target_id", "=", "platform-server"); + return executeSync(this.db, slug === undefined ? query : query.where("origin.slug", "=", slug)) + .rows; + } + + protected roomSkillAvailable( + origin: ReturnType[number], + ) { + return ( + origin.id !== null && + origin.deleted_at === null && + origin.purged_at === null && + origin.purge_pending === 0 && + origin.sharing_mode === "PUBLIC" && + origin.current_version !== null && + origin.blocked_version === null + ); + } + async listPendingPublications(limit = 20) { this.ensure(); return executeSync( diff --git a/packages/platformclaw-control-plane/src/sqlite-store-skill-hub-catalog.ts b/packages/platformclaw-control-plane/src/sqlite-store-skill-hub-catalog.ts index bceeec7d3b..600496a203 100644 --- a/packages/platformclaw-control-plane/src/sqlite-store-skill-hub-catalog.ts +++ b/packages/platformclaw-control-plane/src/sqlite-store-skill-hub-catalog.ts @@ -1,7 +1,11 @@ import { sql } from "kysely"; import { ControlPlaneAuthorizationError, ControlPlaneStateError } from "./contracts.js"; import { executeSync, runImmediateTransaction, takeFirstSync } from "./kysely-sync.js"; -import type { SkillHubCatalogStore } from "./skill-hub-catalog-state.js"; +import type { + SkillHubCatalogInstallation, + SkillHubCatalogMigration, + SkillHubCatalogStore, +} from "./skill-hub-catalog-state.js"; import type { SkillHubCatalogRow } from "./sqlite-schema-skill-hub-catalog.js"; import { ensureSkillHubStateSchema } from "./sqlite-schema.js"; import { SqliteSkillHubCatalogLifecycle } from "./sqlite-skill-hub-catalog-lifecycle.js"; @@ -491,15 +495,63 @@ export class SqliteSkillHubCatalogStore } } - async migrationComplete() { + async recoverLegacyRoomInstallation( + origin: SkillHubCatalogInstallation, + installedRevision: string, + expectedCatalogRevision: number, + ): Promise { + this.ensure(); + if ( + !/^sha256:[a-f0-9]{16}$/u.test(installedRevision) || + !origin.operationId.startsWith("legacy-") || + origin.destination !== "platform_server" || + origin.targetId !== "platform-server" + ) { + throw new ControlPlaneStateError("Invalid legacy room installation identity"); + } + return runImmediateTransaction(this.db, () => { + // The artifact and filesystem proof happened outside this transaction. + // A concurrent catalog edit, block, or replacement must win over recovery. + const binding = takeFirstSync( + this.db, + this.query + .selectFrom("agent_bindings") + .select("kind") + .where("agent_id", "=", origin.agentId), + ); + const [current] = this.roomInstallationRows(origin.agentId, origin.slug); + if ( + binding?.kind !== "knox-room" || + !current || + current.id !== origin.skillId || + current.revision !== expectedCatalogRevision || + !this.roomSkillAvailable(current) + ) { + return false; + } + return ( + Number( + executeSync( + this.db, + this.completeInstallationQuery({ + ...origin, + installedRevision, + now: origin.installedAt, + }) + .where("state", "=", "pending") + .where("installed_revision", "is", null), + ).numAffectedRows, + ) > 0 + ); + }); + } + + async migrationComplete(id: SkillHubCatalogMigration = "sharing-v1") { this.ensure(); return Boolean( takeFirstSync( this.db, - this.query - .selectFrom("skill_hub_catalog_migrations") - .select("id") - .where("id", "=", "sharing-v1"), + this.query.selectFrom("skill_hub_catalog_migrations").select("id").where("id", "=", id), ), ); } @@ -523,13 +575,13 @@ export class SqliteSkillHubCatalogStore ).rows; } - async finishMigration(now: number) { + async finishMigration(now: number, id: SkillHubCatalogMigration = "sharing-v1") { this.ensure(); executeSync( this.db, this.query .insertInto("skill_hub_catalog_migrations") - .values({ id: "sharing-v1", completed_at: now }) + .values({ id, completed_at: now }) .onConflict((conflict) => conflict.column("id").doNothing()), ); } diff --git a/src/agents/agent-work-admission.test.ts b/src/agents/agent-work-admission.test.ts index 0c41783482..437a71a345 100644 --- a/src/agents/agent-work-admission.test.ts +++ b/src/agents/agent-work-admission.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { createDeferred } from "../shared/deferred.js"; import { beginAgentWorkAdmission, + bindAgentWorkAdmissionContext, hasPendingAgentWork, tryAcquireAgentWorkMutation, withAgentWorkAdmission, @@ -71,4 +72,48 @@ describe("agent work admission", () => { release(); } }); + + it("fences in-band mutations across a queue handoff without ignoring competing claims", () => { + const owner = "refresh-owner"; + const turn = beginAgentWorkAdmission(owner); + const competing = beginAgentWorkAdmission(owner); + const acquire = turn.run(() => + bindAgentWorkAdmissionContext(() => + tryAcquireAgentWorkMutation(owner, { + allowCurrent: true, + createBusyError: () => new Error("Skill refresh busy"), + }), + ), + ); + try { + expect(competing.run(acquire)).toBeNull(); + competing(); + const fence = acquire(); + expect(fence).not.toBeNull(); + try { + expect(() => beginAgentWorkAdmission(owner)).toThrow("Skill refresh busy"); + } finally { + fence?.(); + } + turn(); + const newTurn = beginAgentWorkAdmission(owner); + try { + expect(acquire()).toBeNull(); + } finally { + newTurn(); + } + } finally { + competing(); + turn(); + } + }); + + it("does not treat a nested same-agent execution as the active caller's own work", async () => { + const owner = "nested-execution-owner"; + await withAgentWorkAdmission(owner, async () => { + await withAgentWorkAdmission(owner, async () => { + expect(tryAcquireAgentWorkMutation(owner, { allowCurrent: true })).toBeNull(); + }); + }); + }); }); diff --git a/src/agents/agent-work-admission.ts b/src/agents/agent-work-admission.ts index b1703207d0..28e9706909 100644 --- a/src/agents/agent-work-admission.ts +++ b/src/agents/agent-work-admission.ts @@ -1,11 +1,22 @@ +import { AsyncLocalStorage } from "node:async_hooks"; import { normalizeAgentId } from "../routing/session-key.js"; -import { resolveGlobalMap } from "../shared/global-singleton.js"; +import { resolveGlobalMap, resolveGlobalSingleton } from "../shared/global-singleton.js"; -type AgentWorkState = { admissions: Set; mutation?: object }; +type AgentWorkToken = { execution: boolean }; +type AgentWorkState = { + admissions: Set; + mutation?: { createBusyError?: () => Error }; +}; const states = resolveGlobalMap( Symbol.for("openclaw.agentWorkAdmission"), "close-and-restart", ); +const currentAdmissions = resolveGlobalSingleton( + Symbol.for("openclaw.currentAgentWorkAdmissions"), + () => new AsyncLocalStorage | undefined>(), +); + +type AgentWorkAdmission = (() => void) & { run(run: () => T): T }; function ownerKey(agentId: string): string { if (!agentId.trim()) { @@ -14,8 +25,14 @@ function ownerKey(agentId: string): string { return normalizeAgentId(agentId); } -function releaseToken(agentId: string, state: AgentWorkState, token: object): void { - state.admissions.delete(token); +function releaseToken( + agentId: string, + state: AgentWorkState, + token: AgentWorkToken | NonNullable, +): void { + if ("execution" in token) { + state.admissions.delete(token); + } if (state.mutation === token) { delete state.mutation; } @@ -27,27 +44,54 @@ function releaseToken(agentId: string, state: AgentWorkState, token: object): vo } /** Claims work synchronously, before preparation or queueing can yield. */ -export function beginAgentWorkAdmission(agentId: string): () => void { +export function beginAgentWorkAdmission(agentId: string): AgentWorkAdmission { + return beginAgentWork(agentId, false); +} + +function beginAgentWork(agentId: string, execution: boolean): AgentWorkAdmission { const key = ownerKey(agentId); - const state: AgentWorkState = states.get(key) ?? { admissions: new Set() }; + const state: AgentWorkState = states.get(key) ?? { admissions: new Set() }; if (state.mutation) { - throw new Error( - `Agent "${key}" is changing its execution environment. Retry when it completes.`, + throw ( + state.mutation.createBusyError?.() ?? + new Error(`Agent "${key}" is changing its execution environment. Retry when it completes.`) ); } - const token = {}; + const token = { execution }; state.admissions.add(token); states.set(key, state); - return () => releaseToken(key, state, token); + return Object.assign(() => releaseToken(key, state, token), { + run(run: () => T): T { + // An execution may start another run. Its originating turn is a + // competing owner, even if the new invocation inherits async context. + const claims = [...(currentAdmissions.getStore() ?? [])].filter( + (claim) => !execution || !claim.execution, + ); + return currentAdmissions.run(new Set([...claims, token]), run); + }, + }); +} + +/** Carries only the submitting turn's work claims across a scheduler handoff. */ +export function bindAgentWorkAdmissionContext(run: () => T): () => T { + const admissions = currentAdmissions.getStore(); + return () => currentAdmissions.run(admissions, run); +} + +export function runWithAgentWorkAdmissions( + admissions: Iterable, + run: () => T, +): T { + return Array.from(admissions).reduceRight((next, owner) => () => owner.run(next), run)(); } export async function withAgentWorkAdmission( agentId: string, run: () => Promise, ): Promise { - const release = beginAgentWorkAdmission(agentId); + const release = beginAgentWork(agentId, true); try { - return await run(); + return await release.run(run); } finally { release(); } @@ -57,14 +101,18 @@ export function hasPendingAgentWork(agentId: string): boolean { return Boolean(states.get(ownerKey(agentId))?.admissions.size); } -/** Returns an exclusive fence only when no admitted work or mutation exists. */ -export function tryAcquireAgentWorkMutation(agentId: string): (() => void) | null { +/** An in-band mutation may exclude only its own already admitted turn. */ +export function tryAcquireAgentWorkMutation( + agentId: string, + options?: { allowCurrent?: boolean; createBusyError?: () => Error }, +): (() => void) | null { const key = ownerKey(agentId); - const state: AgentWorkState = states.get(key) ?? { admissions: new Set() }; - if (state.admissions.size || state.mutation) { + const state: AgentWorkState = states.get(key) ?? { admissions: new Set() }; + const current = options?.allowCurrent ? currentAdmissions.getStore() : undefined; + if (state.mutation || [...state.admissions].some((admission) => !current?.has(admission))) { return null; } - const token = {}; + const token = options?.createBusyError ? { createBusyError: options.createBusyError } : {}; state.mutation = token; states.set(key, state); return () => releaseToken(key, state, token); diff --git a/src/agents/embedded-agent-runner/run/lane-controller.ts b/src/agents/embedded-agent-runner/run/lane-controller.ts index 914ab4f1c4..541fa9ab53 100644 --- a/src/agents/embedded-agent-runner/run/lane-controller.ts +++ b/src/agents/embedded-agent-runner/run/lane-controller.ts @@ -10,6 +10,7 @@ import { } from "../../../infra/agent-run-registry.js"; import { enqueueCommandInLane, getCommandLaneSnapshot } from "../../../process/command-queue.js"; import type { CommandQueueEnqueueOptions } from "../../../process/command-queue.types.js"; +import { bindAgentWorkAdmissionContext } from "../../agent-work-admission.js"; import { withSessionPlacementTurnAdmission } from "../../session-placement-admission.js"; import type { EmbeddedAgentRunResult } from "../types.js"; import { @@ -124,7 +125,7 @@ export function createEmbeddedRunLaneController(opti ...opts, priority: sessionQueuePriority, }; - const taskWithCurrentLifecycle = async () => { + const taskWithCurrentLifecycle = bindAgentWorkAdmissionContext(async () => { let params = options.getParams(); params.onLaneWait?.({ waitMs: 0, queuedAhead: 0, waiting: false }); params.replyOperation?.markGlobalLaneWaitEnded(); @@ -178,7 +179,7 @@ export function createEmbeddedRunLaneController(opti }, ), ); - }; + }); const params = options.getParams(); if (params.enqueue) { return params.enqueue(taskWithCurrentLifecycle, withLaneTimeout(withRunLaneWait(globalOpts))); @@ -192,10 +193,10 @@ export function createEmbeddedRunLaneController(opti }; const enqueueSession = (task: () => Promise, opts?: CommandQueueEnqueueOptions) => { const sessionOpts: CommandQueueEnqueueOptions = { ...opts, priority: sessionQueuePriority }; - const taskWithLaneAdmission = () => { + const taskWithLaneAdmission = bindAgentWorkAdmissionContext(() => { options.getParams().onLaneWait?.({ waitMs: 0, queuedAhead: 0, waiting: false }); return task(); - }; + }); const params = options.getParams(); // Session admission, deferred maintenance, and global admission share one queue owner. releaseQueuedRunContext = retainQueuedAgentRunContext( diff --git a/src/agents/sandbox/backend.types.ts b/src/agents/sandbox/backend.types.ts index 9b95a430a1..6613c3f814 100644 --- a/src/agents/sandbox/backend.types.ts +++ b/src/agents/sandbox/backend.types.ts @@ -40,6 +40,7 @@ export type SandboxBackendMaterializedSkills = { excludedSourceDirectories?: readonly string[]; missingSourceDirectories?: readonly string[]; policyApplied?: boolean; + reconcileSandboxSkillMounts?: true; }; /** Optional lifecycle manager for an existing registered sandbox runtime. */ @@ -78,6 +79,7 @@ export type CreateSandboxBackendParams = { readOnlySkillMounts?: readonly SandboxBackendReadOnlySkillMount[]; cfg: SandboxConfig; requireCurrentConfig?: boolean; + reconcileSandboxSkillMounts?: true; }; /** Lets a dynamic backend choose between Gateway skills and a target-owned catalog. */ diff --git a/src/agents/sandbox/container-engine.ts b/src/agents/sandbox/container-engine.ts index 2d662af61f..e55d64fcdd 100644 --- a/src/agents/sandbox/container-engine.ts +++ b/src/agents/sandbox/container-engine.ts @@ -4,7 +4,7 @@ import { createAbortError } from "../../infra/abort-signal.js"; import { toErrorObject } from "../../infra/errors.js"; import { isPlainCommandExitFailure, spawnCommand } from "../../process/exec.js"; -import { SANDBOX_COMMAND_MAX_BUFFER_BYTES } from "./constants.js"; +import { DEFAULT_SANDBOX_IMAGE, SANDBOX_COMMAND_MAX_BUFFER_BYTES } from "./constants.js"; export type ExecContainerRawOptions = { allowFailure?: boolean; @@ -130,3 +130,79 @@ export async function execContainer( code: result.code, }; } + +const DOCKER_DAEMON_UNAVAILABLE_MARKERS = [ + "cannot connect to the docker daemon", + "dial unix", + "docker daemon is not running", + "connection refused", +]; + +export function isDockerDaemonUnavailable(stderr: string): boolean { + return DOCKER_DAEMON_UNAVAILABLE_MARKERS.some((marker) => stderr.toLowerCase().includes(marker)); +} + +export function formatDockerDaemonUnavailableError(stderr: string): string { + const detail = stderr.trim(); + return [ + "Sandbox mode requires Docker, but the Docker daemon is not available.", + "Start Docker, or set `agents.defaults.sandbox.mode=off` to disable sandboxing.", + detail ? `Docker said: ${detail}` : undefined, + ] + .filter((line): line is string => Boolean(line)) + .join(" "); +} + +async function inspectContainerImage( + engine: SandboxContainerEngine, + image: string, +): Promise<"exists" | "missing"> { + const result = await execContainer(engine, ["image", "inspect", image], { + allowFailure: true, + }); + if (result.code === 0) { + return "exists"; + } + const stderr = result.stderr.trim(); + const imageMissing = + engine.id === "docker" + ? stderr.toLowerCase().includes("no such image") + : /no such image|image not known|image .* not found/iu.test(stderr); + if (imageMissing) { + return "missing"; + } + if (engine.id === "docker" && isDockerDaemonUnavailable(stderr)) { + throw new Error(formatDockerDaemonUnavailableError(stderr)); + } + if (engine.id === "docker") { + throw new Error(`Failed to inspect sandbox image: ${stderr}`); + } + throw new Error(`Failed to inspect sandbox image with ${engine.displayName}: ${stderr}`); +} + +export async function ensureDockerImage(image: string) { + await ensureContainerImage(DOCKER_SANDBOX_ENGINE, image); +} + +export async function ensureContainerImage(engine: SandboxContainerEngine, image: string) { + const imageState = await inspectContainerImage(engine, image); + if (imageState === "exists") { + return; + } + if (image === DEFAULT_SANDBOX_IMAGE) { + if (engine.id === "docker") { + throw new Error( + `Sandbox image not found: ${image}. Build it with scripts/sandbox-setup.sh before enabling Docker sandboxing. The default image includes python3 for sandbox write/edit helpers; OpenClaw will not substitute plain debian:bookworm-slim.`, + ); + } + throw new Error( + `Sandbox image not found in ${engine.displayName}: ${image}. Build it with podman build -t ${image} -f scripts/docker/sandbox/Dockerfile . before enabling container sandboxing. The default image includes python3 for sandbox write/edit helpers; OpenClaw will not substitute plain debian:bookworm-slim.`, + ); + } + if (engine.id === "docker") { + throw new Error(`Sandbox image not found: ${image}. Build or pull it first.`); + } + throw new Error( + `Sandbox image not found in ${engine.displayName}: ${image}. Build or pull it first.`, + ); +} diff --git a/src/agents/sandbox/context.ts b/src/agents/sandbox/context.ts index 221e90c7c8..0e2dd15997 100644 --- a/src/agents/sandbox/context.ts +++ b/src/agents/sandbox/context.ts @@ -214,6 +214,7 @@ async function syncSandboxSkillsToWorkspace(params: { excludedSourceDirectories: exclusionView.excludedSourceDirectories, missingSourceDirectories: exclusionView.missingSourceDirectories, policyApplied: synced.policyApplied, + ...(synced.reconcileSandboxSkillMounts ? { reconcileSandboxSkillMounts: true } : {}), }, skillUsagePaths, excludedSkillFiles: synced.excludedSkillFiles, @@ -459,6 +460,9 @@ async function resolveProvisionedSandboxContext( agentWorkspaceDir, skillsWorkspaceDir, ...(syncedSkills.materialized ? { readOnlySkillMounts: syncedSkills.materialized.mounts } : {}), + ...(syncedSkills.materialized?.reconcileSandboxSkillMounts + ? { reconcileSandboxSkillMounts: true } + : {}), ...(backendDeferredSkills ? { materializeSkills } : {}), cfg: resolvedCfg, ...(requiresPolicyConfig diff --git a/src/agents/sandbox/docker-backend.ts b/src/agents/sandbox/docker-backend.ts index cb3d168a02..7e6e5efb59 100644 --- a/src/agents/sandbox/docker-backend.ts +++ b/src/agents/sandbox/docker-backend.ts @@ -64,6 +64,7 @@ async function createContainerSandboxBackend( ...(params.requireCurrentConfig !== undefined ? { requireCurrentConfig: params.requireCurrentConfig } : {}), + ...(params.reconcileSandboxSkillMounts ? { reconcileSandboxSkillMounts: true } : {}), }); return createContainerSandboxBackendHandle({ engine: boundEngine, diff --git a/src/agents/sandbox/docker-skill-refresh.test.ts b/src/agents/sandbox/docker-skill-refresh.test.ts new file mode 100644 index 0000000000..c11f099635 --- /dev/null +++ b/src/agents/sandbox/docker-skill-refresh.test.ts @@ -0,0 +1,280 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { beginAgentWorkAdmission } from "../agent-work-admission.js"; +import { ensureSandboxContainer } from "./docker.js"; +import { collectDockerFlagValues } from "./test-args.js"; +import type { SandboxConfig } from "./types.js"; + +const { spawnState, registryMocks } = vi.hoisted(() => ({ + spawnState: { + calls: [] as Array<{ args: string[] }>, + containerExists: false, + inspectRunning: false, + labelHash: "", + inspectManagedBinds: "", + activeProcessSessions: [] as Array<{ sessionKey: string }>, + }, + registryMocks: { + readRegistryEntry: vi.fn(), + removeRegistryEntry: vi.fn(), + updateRegistry: vi.fn(), + }, +})); + +vi.mock("./registry.js", () => registryMocks); +vi.mock("../bash-process-registry.js", () => ({ + listRunningSessions: () => spawnState.activeProcessSessions, +})); +vi.mock("./container-engine.js", async (importOriginal) => ({ + ...(await importOriginal()), + ensureContainerImage: vi.fn(), + execContainer: vi.fn(async (_engine: unknown, args: string[]) => { + spawnState.calls.push({ args }); + let stdout = ""; + let code = 0; + if (args[0] === "inspect" && args[1] === "--type=container") { + stdout = spawnState.inspectManagedBinds; + } else if (args[0] === "inspect" && args[2] === "{{.State.Running}}") { + code = spawnState.containerExists ? 0 : 1; + stdout = String(spawnState.inspectRunning); + } else if (args[0] === "inspect" && args[2]?.includes("openclaw.configHash")) { + stdout = spawnState.labelHash; + } else if (args[0] === "rm") { + spawnState.containerExists = false; + spawnState.inspectRunning = false; + } else if (args[0] === "create") { + expect(spawnState.containerExists).toBe(false); + spawnState.containerExists = true; + spawnState.labelHash = + collectDockerFlagValues(args, "--label") + .find((label) => label.startsWith("openclaw.configHash=")) + ?.slice("openclaw.configHash=".length) ?? ""; + } else if (args[0] === "start") { + spawnState.inspectRunning = true; + } else { + throw new Error(`Unexpected Docker command ${args[0]}`); + } + return { stdout, stderr: "", code }; + }), +})); + +const tmpDirs: string[] = []; + +function makeTempDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-docker-refresh-")); + tmpDirs.push(dir); + return dir; +} + +function createSandboxConfig( + dns: string[], + binds?: string[], + workspaceAccess: "rw" | "ro" | "none" = "rw", + env: Record = { LANG: "C.UTF-8" }, +): SandboxConfig { + return { + mode: "all", + backend: "docker", + scope: "shared", + workspaceAccess, + workspaceRoot: "~/.openclaw/sandboxes", + dockerTmpfsSource: "default", + docker: { + image: "openclaw-sandbox:test", + containerPrefix: "oc-test-", + workdir: "/workspace", + readOnlyRoot: true, + tmpfs: ["/tmp", "/var/tmp", "/run"], + network: "none", + capDrop: ["ALL"], + env, + dns, + extraHosts: ["host.docker.internal:host-gateway"], + binds: binds ?? ["/tmp/workspace:/workspace:rw"], + dangerouslyAllowReservedContainerTargets: true, + }, + ssh: { + command: "ssh", + workspaceRoot: "/tmp/openclaw-sandboxes", + strictHostKeyChecking: true, + updateHostKeys: true, + }, + browser: { + enabled: false, + image: "openclaw-browser:test", + containerPrefix: "oc-browser-", + network: "openclaw-sandbox-browser", + cdpPort: 9222, + vncPort: 5900, + noVncPort: 6080, + headless: true, + noVncEnabled: false, + allowHostControl: false, + autoStart: false, + autoStartTimeoutMs: 5000, + }, + tools: { allow: [], deny: [] }, + prune: { idleHours: 24, maxAgeDays: 7 }, + }; +} + +async function createMaskedRoomContainer(hasPreviousSkill = true, aliasedSkillRoot = false) { + const workspaceDir = fs.realpathSync(makeTempDir()); + const skillsWorkspaceDir = fs.realpathSync(makeTempDir()); + const mask = path.join(skillsWorkspaceDir, ".excluded-skill-empty"); + const sources = [ + { hostPath: path.join(skillsWorkspaceDir, "managed"), containerPath: "/opt/managed-skills" }, + { hostPath: path.join(skillsWorkspaceDir, "bundled"), containerPath: "/opt/bundled-skills" }, + ]; + for (const mount of sources) { + fs.mkdirSync(mount.hostPath); + } + fs.mkdirSync(path.join(skillsWorkspaceDir, "skills")); + fs.mkdirSync(mask); + const skillRoot = aliasedSkillRoot ? "real-skills" : "skills"; + if (hasPreviousSkill) { + fs.mkdirSync(path.join(workspaceDir, skillRoot, "sheet"), { recursive: true }); + if (aliasedSkillRoot) { + fs.symlinkSync( + process.platform === "win32" ? path.join(workspaceDir, skillRoot) : skillRoot, + path.join(workspaceDir, "skills"), + process.platform === "win32" ? "junction" : "dir", + ); + } + } + const params = { + cfg: { ...createSandboxConfig([], []), scope: "agent" as const }, + workspaceDir, + agentWorkspaceDir: workspaceDir, + skillsWorkspaceDir, + scopeKey: `agent:managed-room:workspace:${"0".repeat(32)}`, + requireCurrentConfig: true, + reconcileSandboxSkillMounts: true as const, + readOnlySkillMounts: sources, + }; + spawnState.containerExists = false; + registryMocks.readRegistryEntry.mockResolvedValue(null); + await ensureSandboxContainer({ + ...params, + readOnlySkillMounts: hasPreviousSkill + ? [{ hostPath: mask, containerPath: `/workspace/${skillRoot}/sheet` }, ...sources] + : sources, + }); + const create = spawnState.calls.find((call) => call.args[0] === "create"); + expect(create).toBeDefined(); + const binds = collectDockerFlagValues(create!.args, "-v"); + spawnState.inspectManagedBinds = [ + "a".repeat(64), + "1", + params.scopeKey, + spawnState.labelHash, + JSON.stringify(binds.toReversed()), + ].join("\n"); + registryMocks.readRegistryEntry.mockResolvedValue({ lastUsedAtMs: Date.now() }); + spawnState.calls.length = 0; + if (!hasPreviousSkill) { + fs.mkdirSync(path.join(workspaceDir, "skills", "sheet"), { recursive: true }); + } + return params; +} + +describe("restricted Docker skill mount reconciliation", () => { + beforeEach(() => { + spawnState.calls.length = 0; + spawnState.containerExists = false; + spawnState.inspectRunning = false; + spawnState.labelHash = ""; + spawnState.inspectManagedBinds = ""; + spawnState.activeProcessSessions = []; + vi.clearAllMocks(); + }); + + afterEach(() => { + for (const dir of tmpDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it.each(["recent", "idle", "stopped", "first-install", "aliased"] as const)( + "recreates a %s room container when its existing hash proves skill-only drift", + async (state) => { + const params = await createMaskedRoomContainer( + state !== "first-install", + state === "aliased", + ); + const turn = beginAgentWorkAdmission("managed-room"); + try { + if (state === "idle" || state === "stopped") { + registryMocks.readRegistryEntry.mockResolvedValue({ lastUsedAtMs: 1 }); + } + spawnState.inspectRunning = state !== "stopped"; + await turn.run(() => ensureSandboxContainer(params)); + } finally { + turn(); + } + expect(spawnState.calls).toEqual( + expect.arrayContaining([expect.objectContaining({ args: ["rm", "-f", "a".repeat(64)] })]), + ); + const createCall = spawnState.calls.find((call) => call.args[0] === "create"); + expect(createCall?.args.some((arg) => arg.includes(".excluded-skill-empty"))).toBe(false); + if (state !== "aliased") { + expect(createCall?.args.some((arg) => arg.endsWith(":/workspace/skills:ro,z"))).toBe(true); + } + }, + ); + + it.each(["admitted", "detached"] as const)( + "defers verified skill refresh with %s work even after the hot window expires", + async (activity) => { + const params = await createMaskedRoomContainer(); + registryMocks.readRegistryEntry.mockResolvedValue({ lastUsedAtMs: 1 }); + const turn = beginAgentWorkAdmission("managed-room"); + const competing = + activity === "admitted" ? beginAgentWorkAdmission("managed-room") : undefined; + if (activity === "detached") { + spawnState.activeProcessSessions = [{ sessionKey: "agent:managed-room:main" }]; + } + try { + await expect(turn.run(() => ensureSandboxContainer(params))).rejects.toMatchObject({ + code: "sandbox-skill-refresh-busy", + }); + } finally { + competing?.(); + turn(); + spawnState.activeProcessSessions = []; + } + expect(spawnState.calls.some((call) => call.args[0] === "rm")).toBe(false); + expect(spawnState.calls.some((call) => call.args[0] === "create")).toBe(false); + }, + ); + + it.each(["personal", "network", "unknown-bind", "browser"] as const)( + "preserves restricted hot mismatch rejection for %s drift", + async (reason) => { + const params = await createMaskedRoomContainer(); + const cfg = { + ...params.cfg, + ...(reason === "network" ? { docker: { ...params.cfg.docker, network: "bridge" } } : {}), + ...(reason === "browser" ? { browser: { ...params.cfg.browser, enabled: true } } : {}), + }; + if (reason === "unknown-bind") { + const lines = spawnState.inspectManagedBinds.split("\n"); + const binds = JSON.parse(lines[4]!) as string[]; + lines[4] = JSON.stringify([...binds, "/tmp/unknown:/unknown:ro,z"]); + spawnState.inspectManagedBinds = lines.join("\n"); + } + const { reconcileSandboxSkillMounts, ...personalParams } = params; + await expect( + ensureSandboxContainer({ + ...personalParams, + cfg, + ...(reason === "personal" ? {} : { reconcileSandboxSkillMounts }), + }), + ).rejects.toThrow("restricted dispatch requires the current container config"); + expect(spawnState.calls.some((call) => call.args[0] === "rm")).toBe(false); + expect(spawnState.calls.some((call) => call.args[0] === "create")).toBe(false); + }, + ); +}); diff --git a/src/agents/sandbox/docker-skill-refresh.ts b/src/agents/sandbox/docker-skill-refresh.ts new file mode 100644 index 0000000000..3a3715a3ee --- /dev/null +++ b/src/agents/sandbox/docker-skill-refresh.ts @@ -0,0 +1,161 @@ +import { tryAcquireAgentWorkMutation } from "../agent-work-admission.js"; +import { listRunningSessions } from "../bash-process-registry.js"; +import { execContainer, type SandboxContainerEngine } from "./container-engine.js"; +import { SandboxSkillRefreshBusyError } from "./provisioning-error.js"; +import { resolveSandboxAgentId } from "./shared.js"; +import type { SandboxConfig } from "./types.js"; +import { + appendReadOnlyWorkspaceSkillMountArgs, + appendWorkspaceMountArgs, + filterBindsConflictingWithProtectedMounts, + resolvePreviousReadOnlyWorkspaceSkillMounts, + resolveProtectedSkillMountContainerPaths, + type ReadOnlyWorkspaceSkillMount, +} from "./workspace-mounts.js"; + +type DockerSkillRefreshLayout = { + workspaceDir: string; + agentWorkspaceDir: string; + skillsWorkspaceDir?: string; + readOnlySkillMounts?: readonly ReadOnlyWorkspaceSkillMount[]; + cfg: SandboxConfig; + requireCurrentConfig?: boolean; + reconcileSandboxSkillMounts?: true; +}; + +export type DockerSkillMountRefresh = { containerId: string; release: () => void }; + +async function findSkillMountReplacementTarget(params: { + engine: SandboxContainerEngine; + containerName: string; + scopeKey: string; + currentHash: string; + createParams: DockerSkillRefreshLayout; + hashConfig: (mounts: readonly ReadOnlyWorkspaceSkillMount[]) => string; +}): Promise { + const result = await execContainer( + params.engine, + [ + "inspect", + "--type=container", + "-f", + [ + "{{.Id}}", + '{{index .Config.Labels "openclaw.sandbox"}}', + '{{index .Config.Labels "openclaw.sessionKey"}}', + '{{index .Config.Labels "openclaw.configHash"}}', + "{{json .HostConfig.Binds}}", + ].join("\n"), + params.containerName, + ], + { allowFailure: true }, + ); + if (result.code !== 0) { + return undefined; + } + const [id, managed, scope, hash, rawBinds] = result.stdout.trim().split(/\r?\n/u); + if ( + !id || + !/^[a-f0-9]{64}$/u.test(id) || + managed !== "1" || + scope !== params.scopeKey || + hash !== params.currentHash || + !rawBinds + ) { + return undefined; + } + let binds: unknown; + try { + binds = JSON.parse(rawBinds); + } catch { + return undefined; + } + if (!Array.isArray(binds) || !binds.every((bind): bind is string => typeof bind === "string")) { + return undefined; + } + const { cfg, workspaceDir, agentWorkspaceDir, skillsWorkspaceDir, readOnlySkillMounts } = + params.createParams; + const layout = { + workspaceDir, + agentWorkspaceDir, + skillsWorkspaceDir, + readOnlySkillMounts, + workdir: cfg.docker.workdir, + workspaceAccess: cfg.workspaceAccess, + }; + const previousMounts = resolvePreviousReadOnlyWorkspaceSkillMounts(layout, binds); + if (params.hashConfig(previousMounts) !== hash) { + return undefined; + } + const bindArgs: string[] = []; + appendWorkspaceMountArgs({ + args: bindArgs, + ...layout, + includeReadOnlyWorkspaceSkillMounts: false, + }); + for (const bind of filterBindsConflictingWithProtectedMounts( + cfg.docker.binds, + resolveProtectedSkillMountContainerPaths(previousMounts), + )) { + bindArgs.push("-v", bind); + } + appendReadOnlyWorkspaceSkillMountArgs({ + args: bindArgs, + readOnlyWorkspaceSkillMounts: previousMounts, + }); + const expectedBinds = bindArgs.filter((_, index) => index % 2 === 1); + const previousBinds = new Set(binds); + if ( + previousBinds.size !== binds.length || + new Set(expectedBinds).size !== expectedBinds.length || + expectedBinds.length !== binds.length || + !expectedBinds.every((bind) => previousBinds.has(bind)) + ) { + return undefined; + } + return id; +} + +export async function prepareDockerSkillMountRefresh( + params: Parameters[0], +): Promise { + const { cfg, requireCurrentConfig, reconcileSandboxSkillMounts } = params.createParams; + // Browser containers have their own mount lifecycle. A primary-only refresh + // cannot prove that a browser-enabled runtime's skill view is current. + if ( + !requireCurrentConfig || + !reconcileSandboxSkillMounts || + params.engine.id !== "docker" || + cfg.browser.enabled || + cfg.workspaceAccess !== "rw" || + cfg.scope === "shared" + ) { + return undefined; + } + const agentId = resolveSandboxAgentId(params.scopeKey); + if (!agentId) { + return undefined; + } + const containerId = await findSkillMountReplacementTarget(params); + if (!containerId) { + return undefined; + } + const release = tryAcquireAgentWorkMutation(agentId, { + allowCurrent: true, + createBusyError: () => new SandboxSkillRefreshBusyError(), + }); + if (!release) { + throw new SandboxSkillRefreshBusyError(); + } + // A detached shell can outlive the model turn that admitted it. + if ( + listRunningSessions().some((session) => { + const key = session.sessionKey ?? session.scopeKey; + return key?.startsWith("agent:") && resolveSandboxAgentId(key) === agentId; + }) + ) { + release(); + throw new SandboxSkillRefreshBusyError(); + } + return { containerId, release }; +} diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index a6523085ae..e1fba8c191 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -6,6 +6,7 @@ import { createSubsystemLogger } from "../../logging/subsystem.js"; import { DOCKER_SANDBOX_ENGINE, + ensureContainerImage, execContainer, execContainerRaw, type ExecContainerRawOptions, @@ -29,6 +30,10 @@ import { export { DOCKER_SANDBOX_ENGINE, + ensureContainerImage, + ensureDockerImage, + formatDockerDaemonUnavailableError, + isDockerDaemonUnavailable, execContainer, execContainerRaw, PODMAN_SANDBOX_ENGINE, @@ -57,9 +62,14 @@ export async function execDockerRaw( import { markOpenClawExecEnv } from "../../infra/openclaw-exec-env.js"; import { KeyedAsyncQueue } from "../../plugin-sdk/keyed-async-queue.js"; +import { bindAgentWorkAdmissionContext } from "../agent-work-admission.js"; import { computeSandboxConfigHash } from "./config-hash.js"; -import { DEFAULT_SANDBOX_IMAGE, SANDBOX_DOCKER_CREATE_ARGS_EPOCH } from "./constants.js"; +import { SANDBOX_DOCKER_CREATE_ARGS_EPOCH } from "./constants.js"; import { handleHotSandboxConfigMismatch } from "./current-config.js"; +import { + prepareDockerSkillMountRefresh, + type DockerSkillMountRefresh, +} from "./docker-skill-refresh.js"; import { readRegistryEntry, removeRegistryEntry, updateRegistry } from "./registry.js"; import { buildSandboxContainerName, slugifySessionKey } from "./shared.js"; import type { SandboxConfig, SandboxDockerConfig, SandboxWorkspaceAccess } from "./types.js"; @@ -153,82 +163,6 @@ export async function readDockerPort(containerName: string, port: number) { return Number.isFinite(mapped) ? mapped : null; } -const DOCKER_DAEMON_UNAVAILABLE_MARKERS = [ - "cannot connect to the docker daemon", - "dial unix", - "docker daemon is not running", - "connection refused", -]; - -export function isDockerDaemonUnavailable(stderr: string): boolean { - return DOCKER_DAEMON_UNAVAILABLE_MARKERS.some((marker) => stderr.toLowerCase().includes(marker)); -} - -export function formatDockerDaemonUnavailableError(stderr: string): string { - const detail = stderr.trim(); - return [ - "Sandbox mode requires Docker, but the Docker daemon is not available.", - "Start Docker, or set `agents.defaults.sandbox.mode=off` to disable sandboxing.", - detail ? `Docker said: ${detail}` : undefined, - ] - .filter((line): line is string => Boolean(line)) - .join(" "); -} - -async function inspectContainerImage( - engine: SandboxContainerEngine, - image: string, -): Promise<"exists" | "missing"> { - const result = await execContainer(engine, ["image", "inspect", image], { - allowFailure: true, - }); - if (result.code === 0) { - return "exists"; - } - const stderr = result.stderr.trim(); - const imageMissing = - engine.id === "docker" - ? stderr.toLowerCase().includes("no such image") - : /no such image|image not known|image .* not found/iu.test(stderr); - if (imageMissing) { - return "missing"; - } - if (engine.id === "docker" && isDockerDaemonUnavailable(stderr)) { - throw new Error(formatDockerDaemonUnavailableError(stderr)); - } - if (engine.id === "docker") { - throw new Error(`Failed to inspect sandbox image: ${stderr}`); - } - throw new Error(`Failed to inspect sandbox image with ${engine.displayName}: ${stderr}`); -} - -export async function ensureDockerImage(image: string) { - await ensureContainerImage(DOCKER_SANDBOX_ENGINE, image); -} - -export async function ensureContainerImage(engine: SandboxContainerEngine, image: string) { - const imageState = await inspectContainerImage(engine, image); - if (imageState === "exists") { - return; - } - if (image === DEFAULT_SANDBOX_IMAGE) { - if (engine.id === "docker") { - throw new Error( - `Sandbox image not found: ${image}. Build it with scripts/sandbox-setup.sh before enabling Docker sandboxing. The default image includes python3 for sandbox write/edit helpers; OpenClaw will not substitute plain debian:bookworm-slim.`, - ); - } - throw new Error( - `Sandbox image not found in ${engine.displayName}: ${image}. Build it with podman build -t ${image} -f scripts/docker/sandbox/Dockerfile . before enabling container sandboxing. The default image includes python3 for sandbox write/edit helpers; OpenClaw will not substitute plain debian:bookworm-slim.`, - ); - } - if (engine.id === "docker") { - throw new Error(`Sandbox image not found: ${image}. Build or pull it first.`); - } - throw new Error( - `Sandbox image not found in ${engine.displayName}: ${image}. Build or pull it first.`, - ); -} - export async function dockerContainerState(name: string) { return await containerState(DOCKER_SANDBOX_ENGINE, name); } @@ -527,11 +461,19 @@ async function createSandboxContainer(params: { }); args.push(cfg.image, "sleep", "infinity"); - await execContainer(engine, args); - await execContainer(engine, ["start", name]); - - if (cfg.setupCommand?.trim()) { - await execContainer(engine, ["exec", "-i", name, "/bin/sh", "-lc", cfg.setupCommand]); + const created = await execContainer(engine, args); + try { + await execContainer(engine, ["start", name]); + if (cfg.setupCommand?.trim()) { + await execContainer(engine, ["exec", "-i", name, "/bin/sh", "-lc", cfg.setupCommand]); + } + } catch (error) { + // A failed start/setup must not leave an apparently current replacement. + const id = created.stdout.trim(); + if (/^[a-f0-9]{64}$/u.test(id)) { + await execContainer(engine, ["rm", "-f", id], { allowFailure: true }); + } + throw error; } } @@ -552,6 +494,7 @@ type EnsureSandboxContainerParams = { readOnlySkillMounts?: readonly ReadOnlyWorkspaceSkillMount[]; cfg: SandboxConfig; requireCurrentConfig?: boolean; + reconcileSandboxSkillMounts?: true; }; export async function ensureSandboxContainer(params: EnsureSandboxContainerParams) { @@ -565,9 +508,10 @@ export async function ensureSandboxContainer(params: EnsureSandboxContainerParam // Independent agent runs can converge on one container resource. Serialize the // full lifecycle so followers re-read state after create, start, or replace. - return await sandboxContainerLifecycleQueue.enqueue(containerName, async () => { - return await ensureSandboxContainerLifecycle(params, containerName); - }); + return await sandboxContainerLifecycleQueue.enqueue( + containerName, + bindAgentWorkAdmissionContext(() => ensureSandboxContainerLifecycle(params, containerName)), + ); } async function ensureSandboxContainerLifecycle( @@ -618,18 +562,18 @@ async function ensureSandboxContainerLifecycle( workspaceAccess: params.cfg.workspaceAccess, readOnlySkillMounts: params.readOnlySkillMounts, }); - const genericConfigHash = computeSandboxConfigHash({ - docker: params.cfg.docker, - dockerEnvPolicyEpoch: resolveDockerEnvPolicyEpoch(params.cfg.docker.env), - workspaceAccess: params.cfg.workspaceAccess, - workspaceDir: params.workspaceDir, - agentWorkspaceDir: params.agentWorkspaceDir, - mountFormatVersion: SANDBOX_MOUNT_FORMAT_VERSION, - createArgsEpoch: SANDBOX_DOCKER_CREATE_ARGS_EPOCH, - readOnlyWorkspaceSkillMounts: formatReadOnlyWorkspaceSkillMountHashState( - readOnlyWorkspaceSkillMounts, - ), - }); + const hashConfig = (mounts: readonly ReadOnlyWorkspaceSkillMount[]) => + computeSandboxConfigHash({ + docker: params.cfg.docker, + dockerEnvPolicyEpoch: resolveDockerEnvPolicyEpoch(params.cfg.docker.env), + workspaceAccess: params.cfg.workspaceAccess, + workspaceDir: params.workspaceDir, + agentWorkspaceDir: params.agentWorkspaceDir, + mountFormatVersion: SANDBOX_MOUNT_FORMAT_VERSION, + createArgsEpoch: SANDBOX_DOCKER_CREATE_ARGS_EPOCH, + readOnlyWorkspaceSkillMounts: formatReadOnlyWorkspaceSkillMountHashState(mounts), + }); + const genericConfigHash = hashConfig(readOnlyWorkspaceSkillMounts); const expectedHash = engine.id === "podman" ? resolvePodmanSandboxConfigHash({ @@ -644,9 +588,11 @@ async function ensureSandboxContainerLifecycle( let running = state.running; let currentHash: string | null = null; let hashMismatch = false; + let skillMountRefresh: DockerSkillMountRefresh | undefined; const registryEntry = existingRegistryEntry ?? undefined; if (hasContainer) { - currentHash = await readContainerConfigHash(engine, containerName); + const actualHash = await readContainerConfigHash(engine, containerName); + currentHash = actualHash; if (!currentHash) { currentHash = registryEntry?.configHash ?? null; } @@ -659,7 +605,20 @@ async function ensureSandboxContainerLifecycle( registryEntry !== undefined && running && (typeof lastUsedAtMs !== "number" || now - lastUsedAtMs < HOT_CONTAINER_WINDOW_MS); - if (isHot) { + if (registryEntry && actualHash && params.reconcileSandboxSkillMounts) { + skillMountRefresh = await prepareDockerSkillMountRefresh({ + engine, + containerName, + scopeKey: params.scopeKey, + currentHash: actualHash, + createParams: params, + hashConfig, + }); + } + if (skillMountRefresh) { + hasContainer = false; + running = false; + } else if (isHot) { handleHotSandboxConfigMismatch({ containerName, scope: params.cfg.scope, @@ -675,35 +634,44 @@ async function ensureSandboxContainerLifecycle( } } } - if (!hasContainer) { - await createSandboxContainer({ - engine, - name: containerName, - cfg: params.cfg.docker, - dockerTmpfsSource: params.cfg.dockerTmpfsSource, - workspaceDir: params.workspaceDir, - workspaceAccess: params.cfg.workspaceAccess, - agentWorkspaceDir: params.agentWorkspaceDir, - skillsWorkspaceDir: params.skillsWorkspaceDir, - scopeKey: params.scopeKey, - configHash: expectedHash, - readOnlyWorkspaceSkillMounts, - podmanRuntimeInfo, + try { + if (skillMountRefresh) { + // The lifecycle queue serializes provisioning while the owner fence also + // excludes new runs. Remove by inspected ID, never a reused name. + await execContainer(engine, ["rm", "-f", skillMountRefresh.containerId]); + } + if (!hasContainer) { + await createSandboxContainer({ + engine, + name: containerName, + cfg: params.cfg.docker, + dockerTmpfsSource: params.cfg.dockerTmpfsSource, + workspaceDir: params.workspaceDir, + workspaceAccess: params.cfg.workspaceAccess, + agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, + scopeKey: params.scopeKey, + configHash: expectedHash, + readOnlyWorkspaceSkillMounts, + podmanRuntimeInfo, + }); + } else if (!running) { + await execContainer(engine, ["start", containerName]); + } + await updateRegistry({ + containerName, + backendId: engine.id, + ...(podmanRuntimeInfo ? { backendTarget: podmanRuntimeInfo.target } : {}), + runtimeLabel: containerName, + sessionKey: params.scopeKey, + createdAtMs: now, + lastUsedAtMs: now, + image: params.cfg.docker.image, + configLabelKind: "Image", + configHash: hashMismatch && running ? (currentHash ?? undefined) : expectedHash, }); - } else if (!running) { - await execContainer(engine, ["start", containerName]); + return containerName; + } finally { + skillMountRefresh?.release(); } - await updateRegistry({ - containerName, - backendId: engine.id, - ...(podmanRuntimeInfo ? { backendTarget: podmanRuntimeInfo.target } : {}), - runtimeLabel: containerName, - sessionKey: params.scopeKey, - createdAtMs: now, - lastUsedAtMs: now, - image: params.cfg.docker.image, - configLabelKind: "Image", - configHash: hashMismatch && running ? (currentHash ?? undefined) : expectedHash, - }); - return containerName; } diff --git a/src/agents/sandbox/provisioning-error.ts b/src/agents/sandbox/provisioning-error.ts index 6cf994d608..844f5f4659 100644 --- a/src/agents/sandbox/provisioning-error.ts +++ b/src/agents/sandbox/provisioning-error.ts @@ -1,6 +1,32 @@ -import { formatErrorMessage } from "../../infra/errors.js"; +import { collectErrorGraphCandidates, formatErrorMessage } from "../../infra/errors.js"; const SANDBOX_PROVISIONING_ERROR_CODE = "sandbox_provisioning"; +const SANDBOX_SKILL_REFRESH_BUSY_CODE = "sandbox-skill-refresh-busy"; + +/** A managed skill mount update must wait for the current sandbox owner to finish. */ +export class SandboxSkillRefreshBusyError extends Error { + readonly code = SANDBOX_SKILL_REFRESH_BUSY_CODE; + + constructor() { + super("Please try again shortly while the sandbox applies the updated skill settings."); + this.name = "SandboxSkillRefreshBusyError"; + } +} + +export function isSandboxSkillRefreshBusyError(error: unknown): boolean { + return collectErrorGraphCandidates(error, (candidate) => [ + candidate.cause, + candidate.error, + ...(Array.isArray(candidate.errors) ? candidate.errors : []), + ]).some( + (candidate) => + candidate instanceof SandboxSkillRefreshBusyError || + (typeof candidate === "object" && + candidate !== null && + "code" in candidate && + candidate.code === SANDBOX_SKILL_REFRESH_BUSY_CODE), + ); +} /** Model-independent sandbox setup failure that must not consume model fallbacks. */ class SandboxProvisioningError extends Error { diff --git a/src/agents/sandbox/workspace-mounts.ts b/src/agents/sandbox/workspace-mounts.ts index dd072fd18a..6906af2c76 100644 --- a/src/agents/sandbox/workspace-mounts.ts +++ b/src/agents/sandbox/workspace-mounts.ts @@ -231,6 +231,7 @@ export function resolveReadOnlyWorkspaceSkillMounts(params: { workdir: string; workspaceAccess: SandboxWorkspaceAccess; readOnlySkillMounts?: readonly ReadOnlyWorkspaceSkillMount[]; + previousReadOnlySkillBinds?: ReadonlySet; }): ReadOnlyWorkspaceSkillMount[] { // RW workspaces mount the project as writable, but skill sources remain read-only so agent // instructions are visible without letting sandbox commands mutate them. @@ -265,30 +266,38 @@ export function resolveReadOnlyWorkspaceSkillMounts(params: { ] : []; - const defaults = mounts - .filter((mount) => - isExistingWorkspaceSkillMountSource({ - rootDir: mount.rootDir, - hostPath: mount.hostPath, - }), - ) - .map(({ hostPath, containerPath, rootDir, workspaceSource }) => { - const canonicalHost = resolveSandboxHostPathViaExistingAncestor(hostPath); - // Docker resolves workspace source aliases in the mounted tree. Use that - // same destination for its hash, nested masks, and host filesystem reads. - const canonicalTarget = workspaceSource - ? containerJoin( - params.workdir, - ...path - .relative( - resolveSandboxHostPathViaExistingAncestor(path.resolve(rootDir)), - canonicalHost, - ) - .split(path.sep), - ) - : containerPath; - return { hostPath: canonicalHost, containerPath: canonicalTarget }; - }); + const defaults = mounts.flatMap(({ hostPath, containerPath, rootDir, workspaceSource }) => { + if ( + !params.previousReadOnlySkillBinds && + !isExistingWorkspaceSkillMountSource({ rootDir, hostPath }) + ) { + return []; + } + const canonicalHost = resolveSandboxHostPathViaExistingAncestor(hostPath); + // Docker resolves workspace source aliases in the mounted tree. Use that + // same destination for its hash, nested masks, and host filesystem reads. + const canonicalTarget = workspaceSource + ? containerJoin( + params.workdir, + ...path + .relative( + resolveSandboxHostPathViaExistingAncestor(path.resolve(rootDir)), + canonicalHost, + ) + .split(path.sep), + ) + : containerPath; + const mount = { hostPath: canonicalHost, containerPath: canonicalTarget }; + if ( + params.previousReadOnlySkillBinds && + !params.previousReadOnlySkillBinds.has( + formatManagedWorkspaceBind({ ...mount, readOnly: true }), + ) + ) { + return []; + } + return [mount]; + }); // One authoritative plan is shared by Docker and the filesystem bridge. const byTarget = new Map( defaults.map((mount) => [normalizeMountContainerPath(mount.containerPath), mount]), @@ -299,6 +308,46 @@ export function resolveReadOnlyWorkspaceSkillMounts(params: { return [...byTarget.values()]; } +/** Reconstructs the managed skill bind order for an existing writable sandbox. */ +export function resolvePreviousReadOnlyWorkspaceSkillMounts( + params: Parameters[0], + binds: readonly string[], +): ReadOnlyWorkspaceSkillMount[] { + const materialized = + params.skillsWorkspaceDir ?? + resolveMaterializedSandboxSkillsWorkspaceDir(params.agentWorkspaceDir); + const maskSource = path.join(materialized, ".excluded-skill-empty"); + const masks = binds.flatMap((bind) => { + const parsed = splitSandboxBindSpec(bind); + if (!parsed || parsed.host !== maskSource || parsed.options !== "ro,z") { + return []; + } + const target = normalizeMountContainerPath(parsed.container); + // A skill root may resolve to an alias within the workspace. The reserved + // mask source and previous full hash prove its role without spelling the root. + return target !== normalizeMountContainerPath(params.workdir) && + isPathInsideContainerPath(params.workdir, target) + ? [target] + : []; + }); + const previousReadOnlySkillBinds = new Set(binds); + const canonicalSources = (params.readOnlySkillMounts ?? []).filter( + (mount) => + mount.hostPath !== maskSource && + previousReadOnlySkillBinds.has(formatManagedWorkspaceBind({ ...mount, readOnly: true })), + ); + // Docker's CLI assembles HostConfig.Binds from a map. Use the same order + // as materialization and creation, not the order returned by inspect. + return resolveReadOnlyWorkspaceSkillMounts({ + ...params, + previousReadOnlySkillBinds, + readOnlySkillMounts: [ + ...masks.toSorted().map((containerPath) => ({ hostPath: maskSource, containerPath })), + ...canonicalSources, + ], + }); +} + /** Returns stable mount state for sandbox config hashes. */ export function formatReadOnlyWorkspaceSkillMountHashState( mounts: readonly ReadOnlyWorkspaceSkillMount[], diff --git a/src/agents/sandbox/workspace-skill-exclusion.test.ts b/src/agents/sandbox/workspace-skill-exclusion.test.ts index 7e544f25b5..1337ea232f 100644 --- a/src/agents/sandbox/workspace-skill-exclusion.test.ts +++ b/src/agents/sandbox/workspace-skill-exclusion.test.ts @@ -50,6 +50,49 @@ async function fixture() { } describe("explicit skill source exclusions", () => { + it("exposes an admitted workspace skill while keeping other exclusions masked", async () => { + const f = await fixture(); + const recoveredFile = path.join(f.agentWorkspaceDir, "skills", "recovered", "SKILL.md"); + const recoveredContent = "---\nname: recovered\ndescription: Recovered skill\n---\nverified"; + await fs.mkdir(path.dirname(recoveredFile), { recursive: true }); + await fs.writeFile(recoveredFile, recoveredContent); + const params = { + ...f, + workspaceDir: f.agentWorkspaceDir, + workspaceAccess: "rw" as const, + workdir: "/workspace", + }; + const bridgeForExclusions = async (excludedSkillFiles: string[]) => { + const { mounts } = await prepareExcludedWorkspaceSkillMounts({ + ...params, + excludedSkillFiles, + }); + const sandbox = createSandboxTestContext({ + overrides: { + workspaceDir: params.workspaceDir, + agentWorkspaceDir: params.agentWorkspaceDir, + readOnlySkillMounts: mounts, + }, + }); + return { mounts, bridge: createSandboxFsBridge({ sandbox }) }; + }; + + const recoveredPath = "/workspace/skills/recovered/SKILL.md"; + const before = await bridgeForExclusions([f.excludedFile, recoveredFile]); + await expect(before.bridge.readFile({ filePath: recoveredPath })).rejects.toThrow(/ENOENT/i); + + const after = await bridgeForExclusions([f.excludedFile]); + expect(after.mounts.map((mount) => mount.containerPath)).toEqual([ + "/workspace/skills/physical-folder", + ]); + await expect(after.bridge.readFile({ filePath: recoveredPath })).resolves.toEqual( + Buffer.from(recoveredContent), + ); + await expect( + after.bridge.readFile({ filePath: "/workspace/skills/physical-folder/SKILL.md" }), + ).rejects.toThrow(/ENOENT/i); + }); + it.each(["rw", "ro", "none"] as const)( "masks only the excluded physical directory for %s access", async (workspaceAccess) => { @@ -119,8 +162,8 @@ describe("explicit skill source exclusions", () => { expect( mounts .filter((mount) => mount.source === "protectedSkill") - .map((mount) => [mount.hostRoot, mount.containerRoot]), - ).toEqual(canonical.map((mount) => [mount.hostPath, mount.containerPath])); + .map((mount) => [path.normalize(mount.hostRoot), mount.containerRoot]), + ).toEqual(canonical.map((mount) => [path.normalize(mount.hostPath), mount.containerPath])); const bridge = createSandboxFsBridge({ sandbox }); for (const filePath of [ "/workspace/skills/physical-folder/SKILL.md", @@ -134,7 +177,7 @@ describe("explicit skill source exclusions", () => { defaultContainerRoot: "/workspace", mounts, }); - expect(resolved.hostPath).toBe(path.join(masks[0]!.hostPath, "SKILL.md")); + expect(path.normalize(resolved.hostPath)).toBe(path.join(masks[0]!.hostPath, "SKILL.md")); await expect(bridge.readFile({ filePath })).rejects.toThrow(/ENOENT|no such file/i); } for (const filePath of ["/workspace/hub-alias/SKILL.md", path.join(alias, "SKILL.md")]) { @@ -189,7 +232,11 @@ describe("explicit skill source exclusions", () => { excludedSkillFiles: [], excludedSourceDirectories: [path.dirname(f.excludedFile)], }); - expect(result).toEqual({ + expect({ + ...result, + excludedSourceDirectories: result.excludedSourceDirectories.map(path.normalize), + missingSourceDirectories: result.missingSourceDirectories.map(path.normalize), + }).toEqual({ mounts: [], excludedSourceDirectories: [path.dirname(f.excludedFile)], missingSourceDirectories: [path.dirname(f.excludedFile)], @@ -296,7 +343,13 @@ describe("explicit skill source exclusions", () => { readOnlySkillMounts: prepared.mounts, }, }); - expect(buildSandboxFsMounts(sandbox)).toEqual( + expect( + buildSandboxFsMounts(sandbox).map(({ hostRoot, containerRoot, writable }) => ({ + hostRoot: path.normalize(hostRoot), + containerRoot, + writable, + })), + ).toEqual( expect.arrayContaining([ expect.objectContaining({ hostRoot: path.join(realAgents, "skills"), @@ -364,7 +417,9 @@ describe("explicit skill source exclusions", () => { const initial = await prepareRunSkillContext(params); await fs.unlink(alias); await link(second); - expect(materialized.excludedSourceDirectories).toEqual([path.join(first, "physical-folder")]); + expect(materialized.excludedSourceDirectories.map(path.normalize)).toEqual([ + path.join(first, "physical-folder"), + ]); expect(materialized.mounts[0]?.containerPath).toBe("/workspace/first-skills/physical-folder"); await expect(prepareRunSkillContext(params)).rejects.toMatchObject({ name: "SkillContextPolicyError", diff --git a/src/auto-reply/reply-payload.ts b/src/auto-reply/reply-payload.ts index 452369bfee..f06b9aaf71 100644 --- a/src/auto-reply/reply-payload.ts +++ b/src/auto-reply/reply-payload.ts @@ -242,6 +242,7 @@ export type NoVisibleReplyReason = | "filtered-result" | "empty-result" | "question-unavailable" + | "sandbox-skill-refresh-busy" | "unknown"; /** WeakMap-backed metadata attached to payload objects without changing wire shape. */ diff --git a/src/auto-reply/reply/agent-runner-failure-reply.ts b/src/auto-reply/reply/agent-runner-failure-reply.ts index edc33da57b..94525a601f 100644 --- a/src/auto-reply/reply/agent-runner-failure-reply.ts +++ b/src/auto-reply/reply/agent-runner-failure-reply.ts @@ -31,6 +31,7 @@ import { import { isMissingProviderAuthError } from "../../agents/model-auth.js"; import { isFallbackSummaryError } from "../../agents/model-fallback-attempt.js"; import { resolveAgentRunErrorLifecycleFields } from "../../agents/run-termination.js"; +import { isSandboxSkillRefreshBusyError } from "../../agents/sandbox/provisioning-error.js"; import { resolveSilentReplyPolicy } from "../../config/silent-reply.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { formatErrorMessage } from "../../infra/errors.js"; @@ -572,6 +573,14 @@ export function buildKnownAgentRunFailureReplyPayload(params: { }): ReplyPayload | undefined { const markFailurePayload = (payload: T) => markAgentRunFailureReplyPayload(payload, resolveAgentRunFailureOutcome(params.err)); + if (isSandboxSkillRefreshBusyError(params.err)) { + return setReplyPayloadMetadata( + markFailurePayload({ + text: "Please try again shortly while the sandbox applies the updated skill settings.", + }), + { noVisibleReplyReason: "sandbox-skill-refresh-busy" }, + ); + } const message = formatErrorMessage(params.err); const isFallbackSummary = isFallbackSummaryError(params.err); const isBilling = isFallbackSummary diff --git a/src/auto-reply/reply/agent-runner-run.ts b/src/auto-reply/reply/agent-runner-run.ts index e0dc06628f..4eb0a92f09 100644 --- a/src/auto-reply/reply/agent-runner-run.ts +++ b/src/auto-reply/reply/agent-runner-run.ts @@ -55,7 +55,11 @@ import * as replyRunState from "./reply-operation-run-state.js"; import { type ReplyOperation, replyRunRegistry } from "./reply-run-registry.js"; import { bindReplyOperationTyping, refreshReplyOperationTyping } from "./reply-run-typing.js"; import { createReplyToModeFilterForChannel, resolveReplyToMode } from "./reply-threading.js"; -import { admitReplyTurn, resolveReplyTurnKind } from "./reply-turn-admission.js"; +import { + admitReplyTurn, + resolveReplyTurnKind, + runWithReplyOperationLifecycleAdmission, +} from "./reply-turn-admission.js"; import { isDuplicateRestartRecoverySource, retireTerminalRestartRecoverySourceClaim, @@ -608,64 +612,66 @@ export async function runReplyAgent( cleanupTranscripts: true, }); try { - return await executePreparedReplyAgentRun({ - activeSessionStore, - admitUserTurn, - agentCfgContextTokens, - applyReplyToMode, - beforeAgentReplyDispatchedForSteer, - beginBeforeAgentReply, - blockReplyChunking, - blockReplyPipeline, - blockStreamingEnabled, - cfg, - checkpointBeforeAgentReply, - confirmRestartRecoveryArmedAfterLeaseLoss, - commandBody, - defaultModel, - followupRun, - getActiveIsNewSession: () => activeIsNewSession, - getActiveSessionEntry: () => activeSessionEntry, - isHeartbeat, - isRestartRecoveryArmed, - opts: runOpts, - pendingToolTasks, - performSessionReset: resetSession, - queueKey, - replyMediaContext, - replyOperation, - replyRouteThreadId, - replyThreadingOverride, - replyToChannel, - replyToMode, - resetSessionAfterRoleOrderingConflict, - resolvedBlockStreamingBreak, - resolvedQueue, - resolvedVerboseLevel, - returnWithQueuedFollowupDrain, - runFollowupTurn, - runtimePolicySessionKey, - sendDirectCompactionNotice, - sessionCtx, - sessionKey, - setActiveSessionEntry: (entry) => { - activeSessionEntry = entry; - }, - setRunFollowupTurn: (runner) => { - runFollowupTurn = runner; - }, - shouldEmitToolOutput, - shouldEmitToolResult, - shouldInjectGroupIntro, - storePath, - toolProgressDetail, - traceAgentPhase, - transcriptCommandBody, - turnAdoptionLifecycle, - typing, - typingMode, - typingSignals, - }); + const run = () => + executePreparedReplyAgentRun({ + activeSessionStore, + admitUserTurn, + agentCfgContextTokens, + applyReplyToMode, + beforeAgentReplyDispatchedForSteer, + beginBeforeAgentReply, + blockReplyChunking, + blockReplyPipeline, + blockStreamingEnabled, + cfg, + checkpointBeforeAgentReply, + confirmRestartRecoveryArmedAfterLeaseLoss, + commandBody, + defaultModel, + followupRun, + getActiveIsNewSession: () => activeIsNewSession, + getActiveSessionEntry: () => activeSessionEntry, + isHeartbeat, + isRestartRecoveryArmed, + opts: runOpts, + pendingToolTasks, + performSessionReset: resetSession, + queueKey, + replyMediaContext, + replyOperation, + replyRouteThreadId, + replyThreadingOverride, + replyToChannel, + replyToMode, + resetSessionAfterRoleOrderingConflict, + resolvedBlockStreamingBreak, + resolvedQueue, + resolvedVerboseLevel, + returnWithQueuedFollowupDrain, + runFollowupTurn, + runtimePolicySessionKey, + sendDirectCompactionNotice, + sessionCtx, + sessionKey, + setActiveSessionEntry: (entry) => { + activeSessionEntry = entry; + }, + setRunFollowupTurn: (runner) => { + runFollowupTurn = runner; + }, + shouldEmitToolOutput, + shouldEmitToolResult, + shouldInjectGroupIntro, + storePath, + toolProgressDetail, + traceAgentPhase, + transcriptCommandBody, + turnAdoptionLifecycle, + typing, + typingMode, + typingSignals, + }); + return await runWithReplyOperationLifecycleAdmission(replyOperation, run); } catch (error) { return await handleReplyAgentRunError(error, { blockReplyPipeline, diff --git a/src/auto-reply/reply/followup-runner.ts b/src/auto-reply/reply/followup-runner.ts index 278c12cd72..a852f1e5a2 100644 --- a/src/auto-reply/reply/followup-runner.ts +++ b/src/auto-reply/reply/followup-runner.ts @@ -19,6 +19,7 @@ import { type FollowupRun, } from "./queue.js"; import type { ReplyOperation } from "./reply-run-registry.js"; +import { runWithReplyOperationLifecycleAdmission } from "./reply-turn-admission.js"; type FollowupDrainDisposition = | { kind: "consumed" } @@ -88,33 +89,35 @@ export function createFollowupRunner( admittedRunId = turn.runId; operation = turn.operation; queuedFollowupAdmitted = true; - const execution = await executeFollowupTurn({ - turn, - defaults, - onExecutionStarted: () => { - executionStarted = true; - }, - onToolResult: async (payload, identity) => { - await deliverFollowupDecision({ - decision: { kind: "deliver", payloads: [payload] }, - turn, - defaults, - runId: identity.runId, - runFollowup, - kind: "tool", - }); - }, - onCompactionNoticePayload: async (payload, identity) => { - await deliverFollowupDecision({ - decision: { kind: "deliver", payloads: [payload] }, - turn, - defaults, - runId: identity.runId, - runFollowup, - kind: "block", - }); - }, - }); + const execute = () => + executeFollowupTurn({ + turn, + defaults, + onExecutionStarted: () => { + executionStarted = true; + }, + onToolResult: async (payload, identity) => { + await deliverFollowupDecision({ + decision: { kind: "deliver", payloads: [payload] }, + turn, + defaults, + runId: identity.runId, + runFollowup, + kind: "tool", + }); + }, + onCompactionNoticePayload: async (payload, identity) => { + await deliverFollowupDecision({ + decision: { kind: "deliver", payloads: [payload] }, + turn, + defaults, + runId: identity.runId, + runFollowup, + kind: "block", + }); + }, + }); + const execution = await runWithReplyOperationLifecycleAdmission(turn.operation, execute); try { await execution.progress.drain(); } catch (error) { diff --git a/src/auto-reply/reply/reply-turn-admission.ts b/src/auto-reply/reply/reply-turn-admission.ts index ab5f1cc38c..affa9eb622 100644 --- a/src/auto-reply/reply/reply-turn-admission.ts +++ b/src/auto-reply/reply/reply-turn-admission.ts @@ -1,4 +1,7 @@ -import { beginAgentWorkAdmission } from "../../agents/agent-work-admission.js"; +import { + beginAgentWorkAdmission, + runWithAgentWorkAdmissions, +} from "../../agents/agent-work-admission.js"; import { scheduleMainSessionRecoveryPendingTarget } from "../../agents/main-session-recovery-owner-release.js"; import { isMainRestartRecoveryCandidate } from "../../agents/main-session-recovery-state.js"; import { @@ -64,6 +67,10 @@ class QueuedFollowupLifecycleInvalidatedError extends Error {} const log = createSubsystemLogger("auto-reply/reply-turn-admission"); const lifecycleAdmissionByOperation = new WeakMap(); +const ownerAdmissionsByOperation = new WeakMap< + ReplyOperation, + Set> +>(); async function releaseReplyRecoveryOwner( lease: MainSessionRecoveryOwnerLease | undefined, @@ -82,7 +89,9 @@ export async function runWithReplyOperationLifecycleAdmission( run: () => Promise, ): Promise { const admission = lifecycleAdmissionByOperation.get(operation); - return admission ? await admission.run(run) : await run(); + return await runWithAgentWorkAdmissions(ownerAdmissionsByOperation.get(operation) ?? [], () => + admission ? admission.run(run) : run(), + ); } function rejectLifecycleInvalidatedWork(params: { @@ -178,7 +187,16 @@ export async function admitReplyTurn( // The reply operation owns final attachment delivery too. Keep its owner // claim across admission waits and until the publication owner clears it. retainReplyOperationUntilComplete(result.operation); - runAfterReplyOperationClear(result.operation, releaseOwner); + const owners = ownerAdmissionsByOperation.get(result.operation) ?? new Set(); + owners.add(releaseOwner); + ownerAdmissionsByOperation.set(result.operation, owners); + runAfterReplyOperationClear(result.operation, () => { + owners.delete(releaseOwner); + if (!owners.size) { + ownerAdmissionsByOperation.delete(result.operation); + } + releaseOwner(); + }); ownerTransferred = true; } else if (releaseOwner && result.status === "skipped" && result.lifecycleAdmission) { void result.lifecycleAdmission.released.then(releaseOwner); diff --git a/src/plugins/hook-skill.types.ts b/src/plugins/hook-skill.types.ts index a22d833358..a2e7289f4d 100644 --- a/src/plugins/hook-skill.types.ts +++ b/src/plugins/hook-skill.types.ts @@ -13,6 +13,8 @@ export type PluginHookBeforeSkillContextResult = { excludedPaths: string[]; /** Managed source directories to hide even when their SKILL.md is not discoverable. */ excludedSourceDirectories?: string[]; + /** Managed policy may refresh an idle sandbox when only skill mount configuration changed. */ + reconcileSandboxSkillMounts?: true; }; export type PluginHookSkillProposalKind = "create" | "update"; diff --git a/src/plugins/hooks.skill-context.test.ts b/src/plugins/hooks.skill-context.test.ts index 3e37f01f59..9cf9ccf0be 100644 --- a/src/plugins/hooks.skill-context.test.ts +++ b/src/plugins/hooks.skill-context.test.ts @@ -92,4 +92,22 @@ describe("before_skill_context", () => { "invalid source directory exclusions", ); }); + + it.each([ + { excludedSourceDirectories: [], reconcileSandboxSkillMounts: false }, + { excludedSourceDirectories: [], reconcileSandboxSkillMounts: "true" }, + { reconcileSandboxSkillMounts: true }, + ])("rejects invalid sandbox mount policy %j", async (result) => { + const runner = createHookRunner( + createMockPluginRegistry([ + { + hookName: "before_skill_context", + handler: () => ({ excludedPaths: [], ...result }), + }, + ]), + ); + await expect(runner.runBeforeSkillContext({ candidates: [] }, {})).rejects.toThrow( + "invalid sandbox skill mount policy", + ); + }); }); diff --git a/src/plugins/hooks.ts b/src/plugins/hooks.ts index b597f150bb..b7ce576eb8 100644 --- a/src/plugins/hooks.ts +++ b/src/plugins/hooks.ts @@ -965,6 +965,14 @@ export function createHookRunner( ) { throw new Error("before_skill_context returned invalid source directory exclusions"); } + // Runtime plugin results can violate the declared literal type. + const reconcileSandboxSkillMounts: unknown = next.reconcileSandboxSkillMounts; + if ( + reconcileSandboxSkillMounts !== undefined && + (reconcileSandboxSkillMounts !== true || next.excludedSourceDirectories === undefined) + ) { + throw new Error("before_skill_context returned invalid sandbox skill mount policy"); + } return { excludedPaths: [ ...new Set([...(previous?.excludedPaths ?? []), ...next.excludedPaths]), @@ -980,6 +988,9 @@ export function createHookRunner( ], } : {}), + ...(previous?.reconcileSandboxSkillMounts || reconcileSandboxSkillMounts + ? { reconcileSandboxSkillMounts: true } + : {}), }; }, }, diff --git a/src/sessions/session-lifecycle-admission.ts b/src/sessions/session-lifecycle-admission.ts index d5dbeeadab..b04d590ead 100644 --- a/src/sessions/session-lifecycle-admission.ts +++ b/src/sessions/session-lifecycle-admission.ts @@ -1,6 +1,9 @@ // Serializes lifecycle mutations and work admission for logical session identities. import { AsyncLocalStorage } from "node:async_hooks"; -import { beginAgentWorkAdmission } from "../agents/agent-work-admission.js"; +import { + beginAgentWorkAdmission, + runWithAgentWorkAdmissions, +} from "../agents/agent-work-admission.js"; import { runExclusiveSessionStoreWrite } from "../config/sessions/store-writer.js"; import { GatewayDrainingError, @@ -458,7 +461,7 @@ export async function beginSessionWorkAdmission(params: { }), ), ]; - const ownerReleases: Array<() => void> = []; + const ownerReleases: Array> = []; const releaseOwnerWork = () => ownerReleases.splice(0).forEach((release) => release()); try { // Hold the owner through preflight, queueing, execution and publication; @@ -521,7 +524,9 @@ export async function beginSessionWorkAdmission(params: { run: async (run: () => Promise) => { const current = new Set(CURRENT_SESSION_WORK_ADMISSIONS.getStore()); current.add(admission); - return await CURRENT_SESSION_WORK_ADMISSIONS.run(current, run); + return await runWithAgentWorkAdmissions(ownerReleases, () => + CURRENT_SESSION_WORK_ADMISSIONS.run(current, run), + ); }, }; const signal = params.signal; diff --git a/src/skills/loading/workspace.ts b/src/skills/loading/workspace.ts index 0171d3d401..03634d58b4 100644 --- a/src/skills/loading/workspace.ts +++ b/src/skills/loading/workspace.ts @@ -1904,6 +1904,7 @@ export async function syncSkillsToWorkspace(params: { excludedSkillFiles: string[]; excludedSourceDirectories: string[]; policyApplied: boolean; + reconcileSandboxSkillMounts?: true; }> { const sourceDir = resolveUserPath(params.sourceWorkspaceDir); const targetDir = resolveUserPath(params.targetWorkspaceDir); @@ -1927,7 +1928,7 @@ export async function syncSkillsToWorkspace(params: { }); // Check before refreshing the cache so revoked candidates never leave behind // a newly materialized copy or enter this run's immutable backend catalog. - const { excludedPaths, excludedSourceDirectories, policyApplied } = + const { excludedPaths, excludedSourceDirectories, policyApplied, reconcileSandboxSkillMounts } = await resolveSkillContextExclusions( candidates.map(({ skill }) => ({ name: skill.name, @@ -2003,6 +2004,7 @@ export async function syncSkillsToWorkspace(params: { excludedSkillFiles, excludedSourceDirectories: [...excludedSourceDirectories], policyApplied, + ...(reconcileSandboxSkillMounts ? { reconcileSandboxSkillMounts } : {}), }; }); } catch (error) { diff --git a/src/skills/runtime/context-policy.ts b/src/skills/runtime/context-policy.ts index a1c2a267c2..b571f981a2 100644 --- a/src/skills/runtime/context-policy.ts +++ b/src/skills/runtime/context-policy.ts @@ -40,6 +40,7 @@ export async function resolveSkillContextExclusions( excludedPaths: ReadonlySet; excludedSourceDirectories: ReadonlySet; policyApplied: boolean; + reconcileSandboxSkillMounts?: true; }> { const runner = getGlobalHookRunner(); if (!runner?.hasHooks("before_skill_context", context)) { @@ -51,6 +52,7 @@ export async function resolveSkillContextExclusions( excludedPaths: new Set(result?.excludedPaths ?? []), excludedSourceDirectories: new Set(result?.excludedSourceDirectories ?? []), policyApplied: true, + ...(result?.reconcileSandboxSkillMounts ? { reconcileSandboxSkillMounts: true } : {}), }; } catch (error) { throw new SkillContextPolicyError(error);