From 2756b7e1e23b0eb85540c2d4816e2e3b0fad8364 Mon Sep 17 00:00:00 2001 From: uiharuayako <2451602772@qq.com> Date: Wed, 28 Oct 2026 14:51:53 +0800 Subject: [PATCH 1/2] fix(skills): clarify remote availability and sync user packages --- docs/features/remote-workspaces.md | 19 + scripts/check-core-boundaries.test.mjs | 71 +++ .../cargo-dependency-boundaries.mjs | 1 + .../core-boundaries/rules/feature-rules.mjs | 27 +- scripts/core-boundaries/self-test.mjs | 10 + src/apps/desktop/README.md | 32 + src/crates/assembly/core/AGENTS.md | 11 + src/crates/assembly/core/Cargo.toml | 1 + .../src/agentic/deep_review/capabilities.rs | 236 ++++++- .../tools/implementations/skill_tool.rs | 92 ++- .../tools/implementations/skills/mod.rs | 3 + .../tools/implementations/skills/registry.rs | 160 ++++- .../skills/remote_skill_progress.rs | 36 ++ .../skills/remote_user_skills.rs | 540 ++++++++++++++++ .../contracts/config-contracts/src/types.rs | 59 ++ .../execution/agent-runtime/src/skills/mod.rs | 4 +- .../agent-runtime/src/skills/types.rs | 10 + .../skill_contracts.rs | 1 + src/crates/services/services-core/AGENTS.md | 14 + src/crates/services/services-core/Cargo.toml | 12 + src/crates/services/services-core/src/lib.rs | 2 + .../services-core/src/skill_package_sync.py | 199 ++++++ .../services-core/src/skill_package_sync.rs | 586 ++++++++++++++++++ .../src/skill_package_sync_tests.rs | 430 +++++++++++++ .../services/services-integrations/AGENTS.md | 7 + .../services/services-integrations/Cargo.toml | 1 + .../manager/tests/workspace_sftp.rs | 181 ++++++ .../RemoteUserSkillsSection.test.tsx | 108 ++++ .../development/RemoteUserSkillsSection.tsx | 98 +++ .../development/WorkspaceGitSettingsPage.tsx | 2 + .../shared/SettingsPageComposition.test.ts | 2 +- .../app/scenes/settings/settingsRegistry.ts | 8 +- .../src/app/scenes/settings/settingsTypes.ts | 1 + .../tool-cards/SkillDisplay.test.tsx | 71 +++ .../src/flow_chat/tool-cards/SkillDisplay.tsx | 19 +- .../tool-cards/TaskToolDisplay.test.tsx | 30 + .../flow_chat/tool-cards/TaskToolDisplay.tsx | 10 +- .../src/infrastructure/config/types/index.ts | 4 + src/web-ui/src/locales/en-US/flow-chat.json | 4 +- src/web-ui/src/locales/en-US/settings.json | 3 +- .../src/locales/en-US/settings/runtime.json | 15 + src/web-ui/src/locales/zh-CN/flow-chat.json | 4 +- src/web-ui/src/locales/zh-CN/settings.json | 3 +- .../src/locales/zh-CN/settings/runtime.json | 15 + src/web-ui/src/locales/zh-TW/flow-chat.json | 4 +- src/web-ui/src/locales/zh-TW/settings.json | 3 +- .../src/locales/zh-TW/settings/runtime.json | 15 + 47 files changed, 3096 insertions(+), 68 deletions(-) create mode 100644 src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_skill_progress.rs create mode 100644 src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_user_skills.rs create mode 100644 src/crates/services/services-core/src/skill_package_sync.py create mode 100644 src/crates/services/services-core/src/skill_package_sync.rs create mode 100644 src/crates/services/services-core/src/skill_package_sync_tests.rs create mode 100644 src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.test.tsx create mode 100644 src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.tsx diff --git a/docs/features/remote-workspaces.md b/docs/features/remote-workspaces.md index eb8ba9e759..bf366b2c99 100644 --- a/docs/features/remote-workspaces.md +++ b/docs/features/remote-workspaces.md @@ -234,6 +234,25 @@ working execution and text-query paths and receive an explicit upgrade instructi for a missing binary capability. Existing Remote Connect file commands and persisted session records remain compatible; the file-chunk revision is additive. +## User Skill resources + +User Skill availability is controlled by the runtime host's +`workspace.remote_user_skills` preference: `skill_md_only` (the upgrade-compatible +default), `disabled`, or `full`. It applies to remote discovery, prompt listings +and explicit invocation, without changing remote project Skills. See the +[Desktop guide](../../src/apps/desktop/README.md#user-skills-in-remote-workspaces) +for prerequisites and behavior. + +Full loading deploys the selected directory through the bound filesystem and +shell providers, never through a local fallback. Deterministic file hashes and +permissions identify package versions. Target verification checks the complete +tree before reuse; otherwise one streamed archive is validated in a staging +directory before publication. Returned Skill paths use target POSIX semantics, +and references rooted in the original package are mapped to that deployment. +The runtime reads instructions back from the deployed snapshot before reporting +success. This copies package resources; it does not install interpreters, +relocate executable hooks, or resolve external machine-specific dependencies. + ## Upgrade compatibility Existing SSH profiles remain plain SSH targets because the new `proxyJump` and diff --git a/scripts/check-core-boundaries.test.mjs b/scripts/check-core-boundaries.test.mjs index 96efc90eab..5bc199986f 100644 --- a/scripts/check-core-boundaries.test.mjs +++ b/scripts/check-core-boundaries.test.mjs @@ -706,6 +706,7 @@ test('Services Core feature-free dependencies stay behind exact text and async I 'workspace-runtime', 'workspace-text-runtime', 'workspace-transfer', + 'skill-package-sync', ]), ); }); @@ -4315,6 +4316,7 @@ test('Services Core accepts only the reviewed feature-owned Tokio runtime graph' ], 'session-git': ['local-storage'], 'workspace-transfer': ['dep:tokio', 'tokio/fs', 'tokio/io-util', 'tokio/rt', 'tokio/sync'], + 'skill-package-sync': ['dep:tokio', 'tokio/fs', 'tokio/io-util', 'tokio/rt'], }, }; @@ -5068,6 +5070,75 @@ test('workspace transfer admits IO without process or networking capabilities', v.message.includes('workspace-transfer has unexpected effective Tokio capabilities'))); }); +test('Skill package synchronization keeps injected workspace IO without local processes or broad runtime ports', async () => { + const manifestPath = 'src/crates/services/services-core/Cargo.toml'; + const manifest = await readFile(new URL(`../${manifestPath}`, import.meta.url), 'utf8'); + const features = parseManifestFeatures(manifest); + const expected = [ + 'dep:anyhow', 'dep:openbitfun-runtime-ports', + 'openbitfun-runtime-ports/workspace-ports', 'dep:sha2', 'dep:zip', 'dep:uuid', + 'dep:tokio', 'tokio/fs', 'tokio/io-util', 'tokio/rt', + ]; + const profile = coreClosedFeatureProfileRules.find(rule => + rule.manifestPath === manifestPath && rule.featureName === 'skill-package-sync'); + assert.equal(profile?.exact, true); + assert.deepEqual(profile.requiredFeatureRefs, expected); + assert.deepEqual(features['skill-package-sync'], expected); + + const pkg = { + name: 'openbitfun-services-core', + manifest_path: manifestPath, + dependencies: [{ name: 'tokio', kind: null, optional: true, features: [] }], + features, + }; + assert.deepEqual(findTokioDependencyFeatureViolations([pkg]), []); + for (const added of ['tokio/process', 'tokio/net', 'workspace-runtime']) { + const mutated = structuredClone(pkg); + mutated.features['skill-package-sync'].push(added); + assert.ok(findTokioDependencyFeatureViolations([mutated]).some(violation => + violation.message.includes('skill-package-sync has unexpected effective Tokio capabilities')), added); + } + + const { findCapabilityContractConsumerViolations } = await import( + './core-boundaries/cargo-dependency-boundaries.mjs' + ); + const runtimePorts = capabilityPackage( + 'openbitfun-runtime-ports', + 'src/crates/contracts/runtime-ports/Cargo.toml', + RUNTIME_PORT_FEATURE_PROFILES, + ); + const consumer = { + ...packageAt('openbitfun-services-core', manifestPath, [ + pathDependency('src/crates/contracts/runtime-ports', { + name: 'openbitfun-runtime-ports', optional: true, usesDefaultFeatures: false, + }), + ]), + features, + }; + const rules = capabilityContractDependencyRules.filter(rule => rule.packageName === 'openbitfun-runtime-ports'); + assert.deepEqual(findTestCapabilityViolations(findCapabilityContractConsumerViolations, [runtimePorts, consumer], rules), []); + for (const added of ['runtime-event-port', 'remote-exec-port', 'tool-runtime-handles']) { + const mutated = structuredClone(consumer); + mutated.features['skill-package-sync'].push(`openbitfun-runtime-ports/${added}`); + assert.ok(findTestCapabilityViolations(findCapabilityContractConsumerViolations, [runtimePorts, mutated], rules) + .some(violation => violation.message.includes('skill-package-sync') && violation.message.includes(added)), added); + } +}); + +test('remote workspace feature selects the Skill package service only at its owning surface', async () => { + const manifestPath = 'src/crates/assembly/core/Cargo.toml'; + const manifest = await readFile(new URL(`../${manifestPath}`, import.meta.url), 'utf8'); + const expected = [ + 'workspace-runtime', 'dep:openbitfun-services-integrations', + 'openbitfun-services-integrations/remote-ssh', 'openbitfun-services-core/skill-package-sync', + ]; + const profile = coreClosedFeatureProfileRules.find(rule => + rule.manifestPath === manifestPath && rule.featureName === 'remote-workspace'); + assert.equal(profile?.exact, true); + assert.deepEqual(profile.requiredFeatureRefs, expected); + assert.deepEqual(new Set(parseManifestFeatures(manifest)['remote-workspace']), new Set(expected)); +}); + test('core loopback WebSocket fixture cannot broaden the runtime dependency', () => { const runtime = { name: 'tokio-tungstenite', kind: null, optional: true, uses_default_features: true, features: [] }; diff --git a/scripts/core-boundaries/cargo-dependency-boundaries.mjs b/scripts/core-boundaries/cargo-dependency-boundaries.mjs index eb1a6863a8..4ff1aac8c6 100644 --- a/scripts/core-boundaries/cargo-dependency-boundaries.mjs +++ b/scripts/core-boundaries/cargo-dependency-boundaries.mjs @@ -188,6 +188,7 @@ const SERVICES_CORE_TOKIO_FEATURES = new Map([ ['workspace-text-runtime', ['rt']], ['workspace-runtime', ['fs', 'io-util', 'process', 'rt', 'sync', 'time']], ['workspace-transfer', ['fs', 'io-util', 'rt', 'sync']], + ['skill-package-sync', ['fs', 'io-util', 'rt']], ]); const SERVICES_CORE_BASE_TOKIO_FEATURES = []; const SERVICES_INTEGRATIONS_TOKIO_AGGREGATES = new Set(['product-full']); diff --git a/scripts/core-boundaries/rules/feature-rules.mjs b/scripts/core-boundaries/rules/feature-rules.mjs index f75ebe5c05..d87d4b2184 100644 --- a/scripts/core-boundaries/rules/feature-rules.mjs +++ b/scripts/core-boundaries/rules/feature-rules.mjs @@ -45,10 +45,10 @@ export const optionalDependencyFeatureOwnerRules = [ 'services-core optional implementation dependencies must stay behind their exact owner capability', dependencies: [ { depName: 'image', ownerFeatures: ['pet-packages'] }, - { depName: 'zip', ownerFeatures: ['pet-packages'] }, - { depName: 'uuid', ownerFeatures: ['pet-packages', 'resource-capacity'] }, + { depName: 'zip', ownerFeatures: ['pet-packages', 'skill-package-sync'] }, + { depName: 'uuid', ownerFeatures: ['pet-packages', 'resource-capacity', 'skill-package-sync'] }, { depName: 'aes-gcm', ownerFeatures: ['credential-vault'] }, - { depName: 'anyhow', ownerFeatures: ['credential-vault', 'dispatch-workspace', 'workspace-runtime', 'workspace-transfer', 'session-recovery'] }, + { depName: 'anyhow', ownerFeatures: ['credential-vault', 'dispatch-workspace', 'workspace-runtime', 'workspace-transfer', 'session-recovery', 'skill-package-sync'] }, { depName: 'async-trait', ownerFeatures: ['permission', 'workspace-runtime'] }, { depName: 'base64', ownerFeatures: ['pet-packages', 'credential-vault', 'filesystem'] }, { @@ -56,7 +56,7 @@ export const optionalDependencyFeatureOwnerRules = [ ownerFeatures: ['filesystem', 'local-storage', 'product-identity', 'workspace-persistence'], }, { depName: 'openbitfun-events', ownerFeatures: ['local-storage', 'session-event-format'] }, - { depName: 'openbitfun-runtime-ports', ownerFeatures: ['permission', 'workspace-runtime', 'workspace-persistence', 'workspace-transfer'] }, + { depName: 'openbitfun-runtime-ports', ownerFeatures: ['permission', 'workspace-runtime', 'workspace-persistence', 'workspace-transfer', 'skill-package-sync'] }, { depName: 'chrono', ownerFeatures: ['filesystem', 'local-storage', 'workspace-persistence'] }, { depName: 'chrono-tz', ownerFeatures: ['token-usage-statistics'] }, { depName: 'dunce', ownerFeatures: ['pet-packages', 'runtime-ownership', 'workspace-identity', 'workspace-runtime'] }, @@ -92,6 +92,7 @@ export const optionalDependencyFeatureOwnerRules = [ 'resource-capacity', 'workspace-identity', 'workspace-transfer', + 'skill-package-sync', ], }, { depName: 'which', ownerFeatures: ['process-runtime'] }, @@ -112,6 +113,7 @@ export const optionalDependencyFeatureOwnerRules = [ 'workspace-runtime', 'workspace-text-runtime', 'workspace-transfer', + 'skill-package-sync', ], }, ], @@ -517,8 +519,9 @@ export const capabilityContractDependencyRules = [ capabilityForwarder('workspace-runtime', 'runtime-event-port'), capabilityForwarder('workspace-runtime', 'workspace-ports'), capabilityForwarder('workspace-transfer', 'workspace-ports'), + capabilityForwarder('skill-package-sync', 'workspace-ports'), ], - ['permission', 'workspace-runtime', 'workspace-persistence', 'workspace-transfer'], + ['permission', 'workspace-runtime', 'workspace-persistence', 'workspace-transfer', 'skill-package-sync'], ['session-recovery'], )], ['openbitfun-services-integrations', capabilityConsumer( @@ -1672,6 +1675,17 @@ export const coreClosedFeatureProfileRules = [ exact: true, reason: 'workspace-transfer owns bounded upload IO and optimistic commit through injected workspace ports, without process or transport implementations', }, + { + manifestPath: 'src/crates/services/services-core/Cargo.toml', + featureName: 'skill-package-sync', + requiredFeatureRefs: [ + 'dep:anyhow', 'dep:openbitfun-runtime-ports', + 'openbitfun-runtime-ports/workspace-ports', 'dep:sha2', 'dep:zip', 'dep:uuid', + 'dep:tokio', 'tokio/fs', 'tokio/io-util', 'tokio/rt', + ], + exact: true, + reason: 'skill-package-sync owns package hashing, archive transfer and verification through injected workspace ports, without local process or transport implementations', + }, { manifestPath: 'src/crates/services/services-core/Cargo.toml', featureName: 'workspace-runtime', @@ -1776,10 +1790,11 @@ export const coreClosedFeatureProfileRules = [ 'workspace-runtime', 'dep:openbitfun-services-integrations', 'openbitfun-services-integrations/remote-ssh', + 'openbitfun-services-core/skill-package-sync', ], allowedTransitiveFeatureRefs: ['filesystem', 'local-storage', 'process-runtime'], exact: true, - reason: 'openbitfun-core remote-workspace must add only the remote workspace service surface', + reason: 'openbitfun-core remote-workspace must add only SSH workspace services and on-demand user Skill package deployment', }, { manifestPath: 'src/crates/assembly/core/Cargo.toml', diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 93804e6636..3642ac4877 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -513,6 +513,15 @@ export function runManifestParserSelfTest({ ], ], [servicesCoreManifest, 'session-git', ['local-storage', 'dep:git2']], + [ + servicesCoreManifest, + 'skill-package-sync', + [ + 'dep:anyhow', 'dep:openbitfun-runtime-ports', + 'openbitfun-runtime-ports/workspace-ports', 'dep:sha2', 'dep:zip', 'dep:uuid', + 'dep:tokio', 'tokio/fs', 'tokio/io-util', 'tokio/rt', + ], + ], [ servicesCoreManifest, 'token-usage-statistics', @@ -555,6 +564,7 @@ export function runManifestParserSelfTest({ 'workspace-runtime', 'dep:openbitfun-services-integrations', 'openbitfun-services-integrations/remote-ssh', + 'openbitfun-services-core/skill-package-sync', ], ], [ diff --git a/src/apps/desktop/README.md b/src/apps/desktop/README.md index 27aeaa9ac0..5b46a2e683 100644 --- a/src/apps/desktop/README.md +++ b/src/apps/desktop/README.md @@ -642,6 +642,38 @@ setup. Detached Dispatch does not provision WSL connections. SSH port forwarding is unavailable for native WSL targets; use Windows WSL networking to reach a Linux service. +## User Skills in remote workspaces + +In **Settings → Workspace & Git → User Skills in remote workspaces**, choose: + +- **Only inject SKILL.md** (default): load local user instructions without copying + their resources. The loaded Skill and its completed tool card explicitly report + that supporting files have not been synced. +- **Do not run**: omit host-local user Skills from remote session listings and + reject their invocation. Remote project Skills remain available. +- **Full**: on invocation, check the complete package against its remote copy; + upload changed packages as one compressed archive and expose a verified POSIX + resource path. The Skill card shows preparation, upload and verification progress. + Its completed state confirms package availability, not that runtime dependencies + are installed or that the Skill has executed successfully. + +Full mode requires Python 3 and a streaming file-upload provider on the POSIX +target, plus any runtime dependencies the Skill itself needs. Packages are cached +under `/.openbitfun/cache/user-skills`; exclude that cache from +version control if your project tracks `.openbitfun`. Many small files or large +packages can take time to hash and transfer. Nested symbolic links and special +files are rejected with an explicit error; files outside the Skill directory are +not copied. Existing verified package versions remain available to active turns. +Project file operations and commands continue to run on the remote host. +Standalone Markdown Skills in a shared directory must first be converted into +an isolated `name/SKILL.md` package for Full mode; OpenBitFun never copies their +entire shared parent directory implicitly. + +This preference belongs to the OpenBitFun runtime host. A mobile or peer +controller uses that host's setting and Skills; it does not upload its own local +Skill library. Older hosts without this setting show an unsupported state in the +settings UI. + ## Remote SSH file handle errors If writing files and browsing directories both start failing with diff --git a/src/crates/assembly/core/AGENTS.md b/src/crates/assembly/core/AGENTS.md index 2441d2ae9c..58451fdde2 100644 --- a/src/crates/assembly/core/AGENTS.md +++ b/src/crates/assembly/core/AGENTS.md @@ -192,6 +192,17 @@ Narrower local guides already exist for some subtrees: ## Verification +For remote user Skill availability, Markdown-only warnings, package path mapping, +and on-demand deployment, use: + +```bash +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git,remote-workspace --lib remote_user_skills +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git,remote-workspace --lib skill_tool::tests +``` + +These owner fixtures use local providers or mocks. The live SSH package resource +fixture is documented in `services-integrations/AGENTS.md`. + For smart routing, unchanged subagent model policy and resumed subagent choices: ```bash diff --git a/src/crates/assembly/core/Cargo.toml b/src/crates/assembly/core/Cargo.toml index 0e2d10985c..56bd13174c 100644 --- a/src/crates/assembly/core/Cargo.toml +++ b/src/crates/assembly/core/Cargo.toml @@ -475,6 +475,7 @@ workspace-runtime = [ workspace-watch = ["workspace-runtime", "dep:notify"] remote-workspace = [ "workspace-runtime", + "openbitfun-services-core/skill-package-sync", "dep:openbitfun-services-integrations", "openbitfun-services-integrations/remote-ssh", ] diff --git a/src/crates/assembly/core/src/agentic/deep_review/capabilities.rs b/src/crates/assembly/core/src/agentic/deep_review/capabilities.rs index d62c0ce035..1c035dc025 100644 --- a/src/crates/assembly/core/src/agentic/deep_review/capabilities.rs +++ b/src/crates/assembly/core/src/agentic/deep_review/capabilities.rs @@ -4,7 +4,7 @@ use crate::agentic::agents::{ use crate::agentic::tools::framework::ToolUseContext; use crate::agentic::tools::implementations::skills::get_skill_registry; use crate::util::errors::{OpenBitFunError, OpenBitFunResult}; -use openbitfun_agent_runtime::skills::{SkillData, SkillInfo}; +use openbitfun_agent_runtime::skills::{SkillData, SkillInfo, SkillLocation}; use sha2::{Digest, Sha256}; const CAPABILITY_CATALOG_LIMIT: usize = 24; @@ -202,15 +202,48 @@ pub async fn resolve_review_capability( }); } if let Some(skill_key) = key.strip_prefix("skill:") { + let remote_user_source = if context.is_remote() { + implicitly_invocable_skills(context) + .await + .into_iter() + .find(|skill| skill.key == skill_key && skill.level == SkillLocation::User) + } else { + None + }; + if let Some(info) = &remote_user_source { + let preview = load_discovered_review_skill(context, info).await?; + verify_review_skill_fingerprint(key, &preview, fingerprint)?; + // Remote command hooks are unsupported. Check the same contract as + // SkillTool before spending time deploying an unusable package. + crate::native_hooks::activate_skill_hooks(&preview, context).await?; + } let skill = load_review_skill(context, skill_key).await?; if !is_compatible_review_skill(&skill.dir_name) { return Err(capability_changed_error()); } let guidance = bounded_selected_guidance(&skill.content)?; - ensure_capability_fingerprint( - &ReviewCapabilityDescriptor::new(key, &skill.name, &skill.description, &guidance), + verify_selected_review_skill_fingerprint( + context, + key, + &skill, + remote_user_source.as_ref(), fingerprint, - )?; + ) + .await?; + let guidance = if let Some(info) = remote_user_source { + if info.path != skill.path { + format!( + "Skill resources are relative to '{}'. Use that package root for scripts, references and templates; project operations still use the remote workspace.\n\n{guidance}", + skill.path, + ) + } else if !skill.compatibility_warnings.is_empty() { + format!("{}\n\n{guidance}", skill.compatibility_warnings.join("\n")) + } else { + guidance + } + } else { + guidance + }; return Ok(ResolvedReviewCapability { guidance, preferred_model: None, @@ -266,6 +299,37 @@ pub async fn resolve_review_capability( )) } +fn verify_review_skill_fingerprint( + key: &str, + skill: &SkillData, + fingerprint: &str, +) -> OpenBitFunResult<()> { + let guidance = bounded_selected_guidance(&skill.content)?; + ensure_capability_fingerprint( + &ReviewCapabilityDescriptor::new(key, &skill.name, &skill.description, &guidance), + fingerprint, + ) +} + +async fn verify_selected_review_skill_fingerprint( + context: &ToolUseContext, + key: &str, + skill: &SkillData, + source: Option<&SkillInfo>, + fingerprint: &str, +) -> OpenBitFunResult<()> { + if let Some(info) = source.filter(|info| info.path != skill.path) { + // The admitted catalog fingerprints source guidance. Verify the raw + // deployed snapshot before comparing its path-mapped instructions; + // rereading the local source here would race the completed upload. + let markdown = read_remote_review_skill_markdown(context, &skill.path, info).await?; + let deployed_preview = parse_review_skill_preview(info, &markdown)?; + verify_review_skill_fingerprint(key, &deployed_preview, fingerprint) + } else { + verify_review_skill_fingerprint(key, skill, fingerprint) + } +} + fn explicit_preferred_model(agent: &AgentInfo) -> Option { agent .model @@ -303,11 +367,13 @@ async fn load_review_skill( .map(|workspace| workspace.root_path_string()) .unwrap_or_default(); registry - .find_and_load_skill_by_key_for_remote_workspace( + .find_and_load_skill_by_key_for_remote_workspace_with_progress( skill_key, fs, &root, context.agent_type.as_deref(), + context.ws_shell(), + Some(crate::agentic::tools::implementations::skills::remote_skill_progress::skill_load_progress(context, "AgentSpawn")), ) .await } else { @@ -325,28 +391,42 @@ async fn load_discovered_review_skill( context: &ToolUseContext, info: &SkillInfo, ) -> OpenBitFunResult { - let skill_file = format!("{}/SKILL.md", info.path.trim_end_matches(['/', '\\'])); - let markdown = if context.is_remote() { - context - .ws_fs() - .ok_or_else(|| { - OpenBitFunError::tool("Remote review skill loading is unavailable".to_string()) - })? - .read_file_text(&skill_file) - .await - .map_err(|error| { - OpenBitFunError::tool(format!("Failed to read review skill: {error}")) - })? + let entry_file = info.entry_file.as_deref().unwrap_or("SKILL.md"); + // Catalog construction previews guidance; deployment happens only when a + // capability is selected through the registry above. + let markdown = if context.is_remote() && info.level == SkillLocation::Project { + read_remote_review_skill_markdown(context, &info.path, info).await? } else { - tokio::fs::read_to_string(&skill_file) + tokio::fs::read_to_string(std::path::Path::new(&info.path).join(entry_file)) .await .map_err(|error| { OpenBitFunError::tool(format!("Failed to read review skill: {error}")) })? }; + parse_review_skill_preview(info, &markdown) +} + +async fn read_remote_review_skill_markdown( + context: &ToolUseContext, + path: &str, + info: &SkillInfo, +) -> OpenBitFunResult { + let entry_file = info.entry_file.as_deref().unwrap_or("SKILL.md"); + let skill_file = format!("{}/{}", path.trim_end_matches('/'), entry_file); + context + .ws_fs() + .ok_or_else(|| { + OpenBitFunError::tool("Remote review skill loading is unavailable".to_string()) + })? + .read_file_text(&skill_file) + .await + .map_err(|error| OpenBitFunError::tool(format!("Failed to read review skill: {error}"))) +} + +fn parse_review_skill_preview(info: &SkillInfo, markdown: &str) -> OpenBitFunResult { let mut data = SkillData::from_markdown_for_source_slot( - info.path.clone(), - &markdown, + info.parser_path(), + markdown, info.level, true, info.parser_source_slot(), @@ -556,6 +636,122 @@ mod tests { assert!(!rendered.contains("full guidance")); } + #[tokio::test] + async fn remote_catalog_previews_local_user_markdown_without_remote_io_or_deployment() { + let temp = tempfile::tempdir().unwrap(); + std::fs::write( + temp.path().join("custom-entry.md"), + "---\nname: review-local\ndescription: User guidance\n---\nCheck scripts/check.py.\n", + ) + .unwrap(); + let info: SkillInfo = serde_json::from_value(serde_json::json!({ + "key": "user::openbitfun-user::code-review-local", + "name": "review-local", + "description": "User guidance", + "path": temp.path().to_str().unwrap(), + "entryFile": "custom-entry.md", + "level": "user", + "sourceSlot": "openbitfun-user", + "dirName": "code-review-local" + })) + .unwrap(); + let identity = crate::service::remote_ssh::workspace_state::workspace_session_identity( + "/remote/project", + Some("catalog-test-connection"), + Some("catalog-test-host"), + ) + .unwrap(); + let mut context = local_tool_context(temp.path().to_path_buf()); + context.workspace = Some(WorkspaceBinding::new_remote( + None, + "/remote/project".into(), + "catalog-test-connection".into(), + "Remote".into(), + identity, + )); + assert!(context.ws_fs().is_none()); + let data = load_discovered_review_skill(&context, &info).await.unwrap(); + assert_eq!(data.content.trim(), "Check scripts/check.py."); + assert_eq!(data.key, info.key); + let mut project = info; + project.level = SkillLocation::Project; + assert!(load_discovered_review_skill(&context, &project) + .await + .is_err()); + } + + #[tokio::test] + async fn deployed_review_guidance_verifies_raw_snapshot_before_path_mapping() { + let temp = tempfile::tempdir().unwrap(); + let source_dir = temp.path().join("source"); + let remote_dir = temp.path().join("remote"); + std::fs::create_dir(&source_dir).unwrap(); + std::fs::create_dir(&remote_dir).unwrap(); + let source_path = source_dir.to_string_lossy().to_string(); + let remote_path = remote_dir.to_string_lossy().to_string(); + let info: SkillInfo = serde_json::from_value(serde_json::json!({ + "key": "user::openbitfun-user::code-review-local", + "name": "review-local", + "description": "User guidance", + "path": source_path, + "level": "user", + "sourceSlot": "openbitfun-user", + "dirName": "code-review-local" + })) + .unwrap(); + let markdown = format!( + "---\nname: review-local\ndescription: User guidance\n---\nRead {}/references/guide.md.\n", + info.path + ); + std::fs::write(source_dir.join("SKILL.md"), &markdown).unwrap(); + std::fs::write(remote_dir.join("SKILL.md"), &markdown).unwrap(); + let preview = parse_review_skill_preview(&info, &markdown).unwrap(); + let key = format!("skill:{}", info.key); + let descriptor = ReviewCapabilityDescriptor::new( + &key, + &preview.name, + &preview.description, + &bounded_selected_guidance(&preview.content).unwrap(), + ); + let mut loaded = preview; + loaded.content = loaded.content.replace(&info.path, &remote_path); + loaded.path = remote_path.clone(); + assert!(verify_review_skill_fingerprint(&key, &loaded, descriptor.fingerprint()).is_err()); + + let mut context = local_tool_context(temp.path().to_path_buf()); + context.runtime_handles = openbitfun_runtime_ports::ToolRuntimeHandles::new( + Some(crate::agentic::workspace::local_workspace_services( + remote_path, + )), + None, + ); + // A later local edit does not change the snapshot already deployed. + std::fs::write(source_dir.join("SKILL.md"), "changed local source").unwrap(); + verify_selected_review_skill_fingerprint( + &context, + &key, + &loaded, + Some(&info), + descriptor.fingerprint(), + ) + .await + .unwrap(); + std::fs::write( + remote_dir.join("SKILL.md"), + markdown.replace("Read ", "Execute "), + ) + .unwrap(); + assert!(verify_selected_review_skill_fingerprint( + &context, + &key, + &loaded, + Some(&info), + descriptor.fingerprint(), + ) + .await + .is_err()); + } + #[tokio::test] async fn catalog_uses_discovered_claude_skill_and_restores_it_after_import_undo() { let temp = tempfile::tempdir().expect("temporary workspace"); diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs index 4b706fc6a1..dcc256c5eb 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs @@ -12,6 +12,8 @@ use log::debug; use openbitfun_services_core::markdown::expand_prompt_template_arguments_with_names; use serde_json::{json, Value}; +use super::skills::remote_skill_progress::skill_load_progress; + // Use skills module use super::skills::{get_skill_registry, render_loaded_skill_for_assistant}; @@ -65,9 +67,7 @@ Important: ) .await } else { - registry - .get_resolved_skills_xml_for_workspace(None, ctx.agent_type.as_deref()) - .await + vec!["Remote workspace Skill loading is unavailable because its filesystem provider is missing. No local fallback is available.".to_string()] } } Some(ctx) => { @@ -161,7 +161,8 @@ impl Tool for SkillTool { "description": {"type": "string"}, "location": {"type": "string"}, "content": {"type": "string"}, - "compatibility_warnings": {"type": "array", "items": {"type": "string"}} + "compatibility_warnings": {"type": "array", "items": {"type": "string"}}, + "resource_availability": {"type": ["string", "null"], "enum": ["markdown_only", "remote_package", null]} }, "required": ["success", "skill_name", "skill_key", "content"], "additionalProperties": false @@ -236,7 +237,7 @@ impl Tool for SkillTool { fn render_tool_use_message(&self, input: &Value, _options: &ToolRenderOptions) -> String { if let Some(command) = input.get("command").and_then(|v| v.as_str()) { - format!("The \"{}\" skill is loaded.", command) + format!("Loading the \"{}\" skill...", command) } else { "Loading skill...".to_string() } @@ -258,6 +259,7 @@ impl Tool for SkillTool { let registry = get_skill_registry(); let use_stable_key = skill_name.starts_with("user::") || skill_name.starts_with("project::"); + let progress = skill_load_progress(context, "Skill"); let mut skill_data = if context.is_remote() { if let Some(ws_fs) = context.ws_fs() { let root = context @@ -267,41 +269,31 @@ impl Tool for SkillTool { .unwrap_or_default(); if use_stable_key { registry - .find_and_load_skill_by_key_for_remote_workspace( + .find_and_load_skill_by_key_for_remote_workspace_with_progress( skill_name, ws_fs, &root, context.agent_type.as_deref(), + context.ws_shell(), + Some(progress.clone()), ) .await? } else { registry - .find_and_load_skill_for_remote_workspace( + .find_and_load_skill_for_remote_workspace_with_progress( skill_name, ws_fs, &root, context.agent_type.as_deref(), + context.ws_shell(), + Some(progress.clone()), ) .await? } } else { - if use_stable_key { - registry - .find_and_load_skill_by_key_for_workspace( - skill_name, - None, - context.agent_type.as_deref(), - ) - .await? - } else { - registry - .find_and_load_skill_for_workspace( - skill_name, - None, - context.agent_type.as_deref(), - ) - .await? - } + return Err(OpenBitFunError::tool( + "Remote workspace Skill loading requires its bound filesystem provider; no local fallback is available.".to_string(), + )); } } else { if use_stable_key { @@ -346,6 +338,7 @@ impl Tool for SkillTool { "location": location_str, "content": skill_data.content, "compatibility_warnings": skill_data.compatibility_warnings, + "resource_availability": skill_data.resource_availability, "success": true }), result_for_assistant: Some(result_for_assistant), @@ -1131,8 +1124,15 @@ Use the remote project skill. ), }; - let results = SkillTool::new() - .call_impl(&json!({ "command": "create-agent" }), &context) + use super::super::skills::remote_user_skills::TEST_REMOTE_USER_SKILLS_MODE; + use crate::service::config::types::RemoteUserSkillsMode; + let tool = SkillTool::new(); + let input = json!({ "command": "create-agent" }); + let results = TEST_REMOTE_USER_SKILLS_MODE + .scope( + RemoteUserSkillsMode::SkillMdOnly, + tool.call_impl(&input, &context), + ) .await .expect("explicit create-agent invocation should load the local built-in skill"); @@ -1146,6 +1146,7 @@ Use the remote project skill. }; assert_eq!(data["skill_name"], "create-agent"); assert_eq!(data["location"], "user"); + assert_eq!(data["resource_availability"], "markdown_only"); assert!(data["content"] .as_str() .unwrap_or_default() @@ -1155,6 +1156,45 @@ Use the remote project skill. assert!(assistant.contains("\n")); assert!(assistant.contains("# Create an OpenBitFun Agent")); assert!(!assistant.contains("from stable key")); + assert!(assistant.contains("NOT available in this remote workspace")); + + TEST_REMOTE_USER_SKILLS_MODE + .scope(RemoteUserSkillsMode::Disabled, async { + let listing = SkillTool::build_available_skills_context_section(Some(&context)) + .await + .unwrap(); + assert!(listing.contains("remote-only-skill-for-test")); + assert!(!listing.contains("user::")); + assert!(tool.call_impl(&input, &context).await.is_err()); + let error = tool + .call_impl( + &json!({"command": "user::openbitfun-system::create-agent"}), + &context, + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("disabled")); + }) + .await; + + let error = TEST_REMOTE_USER_SKILLS_MODE + .scope(RemoteUserSkillsMode::Full, tool.call_impl(&input, &context)) + .await + .unwrap_err(); + #[cfg(feature = "remote-workspace")] + assert!(error + .to_string() + .contains("Failed to deploy complete user Skill package")); + #[cfg(not(feature = "remote-workspace"))] + assert!(error + .to_string() + .contains("does not support complete user Skill deployment")); + + let mut missing_provider = context; + missing_provider.runtime_handles = + openbitfun_runtime_ports::ToolRuntimeHandles::new(None, None); + let error = tool.call_impl(&input, &missing_provider).await.unwrap_err(); + assert!(error.to_string().contains("bound filesystem provider")); } #[tokio::test] diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/mod.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/mod.rs index 394bd6da63..4aca6dc42b 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/mod.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/mod.rs @@ -7,6 +7,9 @@ pub mod catalog; pub mod mode_overrides; pub mod policy; pub mod registry; +pub(crate) mod remote_skill_progress; +pub(crate) mod remote_user_skills; +pub use remote_user_skills::SkillLoadProgress; pub mod resolver; mod runtime_settings; #[cfg(feature = "file-watch")] diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs index 1d22cd3a06..e2ae6c084a 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs @@ -8,19 +8,24 @@ use super::mode_overrides::{ load_globally_disabled_project_skills, load_globally_disabled_user_skills, load_user_mode_skill_overrides, SkillPolicyWorkspace, UserModeSkillOverrides, }; +use super::remote_user_skills::{ + check_remote_user_skill_mode, prepare_remote_user_skill, remote_user_skills_mode, + SkillLoadProgress, +}; use super::runtime_settings::apply_builtin_runtime_settings; #[cfg(feature = "file-watch")] use super::source_cache::{LocalSkillWatchMonitor, LocalSkillWatchRoot, VersionedSnapshotCache}; use super::types::{ ModeSkillInfo, SkillData, SkillInfo, SkillLocation, SkillScanDiagnostic, SkillScanReport, }; -use crate::agentic::workspace::WorkspaceFileSystem; +use crate::agentic::workspace::{WorkspaceFileSystem, WorkspaceShell}; #[cfg(feature = "external-sources")] use crate::external_sources::{ opencode_configured_skill_roots, pi_configured_skill_roots, LocalConfiguredSkillRootContribution, }; use crate::infrastructure::get_path_manager_arc; +use crate::service::config::types::RemoteUserSkillsMode; use crate::util::errors::{OpenBitFunError, OpenBitFunResult}; use futures::{stream, StreamExt}; use log::{debug, error, warn}; @@ -1516,10 +1521,44 @@ impl SkillRegistry { fs: &dyn WorkspaceFileSystem, remote_root: &str, ) -> SkillCandidateScan { - let (user, project) = tokio::join!( - self.scan_skill_candidates_with_diagnostics_for_workspace(None), + let mode = remote_user_skills_mode().await; + self.scan_remote_skill_candidates_with_mode(fs, remote_root, mode) + .await + } + + async fn scan_remote_skill_candidates_with_mode( + &self, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + mode: OpenBitFunResult, + ) -> SkillCandidateScan { + let (user, mut project) = tokio::join!( + async { + match &mode { + Ok(RemoteUserSkillsMode::SkillMdOnly | RemoteUserSkillsMode::Full) => { + self.scan_skill_candidates_with_diagnostics_for_workspace(None) + .await + } + _ => SkillCandidateScan::default(), + } + }, Self::scan_remote_project_skills(fs, remote_root), ); + let notice = match &mode { + Err(error) => Some(format!("Local user Skills are unavailable: {error}")), + Ok(RemoteUserSkillsMode::Unsupported(value)) => Some(format!( + "Local user Skills are unavailable: unsupported remote user Skills mode '{value}'" + )), + _ => None, + }; + if let Some(message) = notice { + project.diagnostics.push(SkillScanDiagnostic { + path: "workspace.remote_user_skills".into(), + source_id: "openbitfun".into(), + message, + unsupported_field: None, + }); + } Self::merge_remote_skill_scans(user, project) } @@ -2052,6 +2091,26 @@ impl SkillRegistry { fs: &dyn WorkspaceFileSystem, remote_root: &str, agent_type: Option<&str>, + ) -> OpenBitFunResult { + self.find_and_load_skill_for_remote_workspace_with_progress( + skill_name, + fs, + remote_root, + agent_type, + None, + None, + ) + .await + } + + pub async fn find_and_load_skill_for_remote_workspace_with_progress( + &self, + skill_name: &str, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + agent_type: Option<&str>, + shell: Option<&dyn WorkspaceShell>, + progress: Option, ) -> OpenBitFunResult { let info = self .find_skill_info_for_explicit_invocation_remote_workspace( @@ -2071,6 +2130,7 @@ impl SkillRegistry { info.parser_source_slot(), ) .map_err(|error| OpenBitFunError::tool(error.to_string()))?; + let parser_source_slot = info.parser_source_slot().to_string(); data.path = info.path; data.entry_file = info.entry_file; data.key = info.key; @@ -2078,6 +2138,15 @@ impl SkillRegistry { data.source_id = info.source_id; data.source_label = info.source_label; data.dir_name = info.dir_name; + prepare_remote_user_skill( + &mut data, + fs, + remote_root, + shell, + progress, + &parser_source_slot, + ) + .await?; apply_builtin_runtime_settings(&mut data).await?; Ok(data) } @@ -2089,6 +2158,29 @@ impl SkillRegistry { remote_root: &str, agent_type: Option<&str>, ) -> OpenBitFunResult { + self.find_and_load_skill_by_key_for_remote_workspace_with_progress( + skill_key, + fs, + remote_root, + agent_type, + None, + None, + ) + .await + } + + pub async fn find_and_load_skill_by_key_for_remote_workspace_with_progress( + &self, + skill_key: &str, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + agent_type: Option<&str>, + shell: Option<&dyn WorkspaceShell>, + progress: Option, + ) -> OpenBitFunResult { + if skill_key.starts_with("user::") { + check_remote_user_skill_mode(&remote_user_skills_mode().await?)?; + } let candidates = self .scan_skill_candidates_for_remote_workspace(fs, remote_root) .await; @@ -2115,6 +2207,7 @@ impl SkillRegistry { info.parser_source_slot(), ) .map_err(|error| OpenBitFunError::tool(error.to_string()))?; + let parser_source_slot = info.parser_source_slot().to_string(); data.path = info.path; data.entry_file = info.entry_file; data.key = info.key; @@ -2122,6 +2215,15 @@ impl SkillRegistry { data.source_id = info.source_id; data.source_label = info.source_label; data.dir_name = info.dir_name; + prepare_remote_user_skill( + &mut data, + fs, + remote_root, + shell, + progress, + &parser_source_slot, + ) + .await?; apply_builtin_runtime_settings(&mut data).await?; Ok(data) } @@ -2195,6 +2297,58 @@ impl SkillRegistry { } } +#[cfg(test)] +mod remote_user_skills_policy_tests { + use super::*; + use crate::agentic::workspace::LocalWorkspaceFs; + + #[tokio::test] + async fn remote_user_skills_disabled_or_unavailable_keeps_only_project_candidates() { + let temp = tempfile::tempdir().unwrap(); + let project_skill = temp.path().join(".openbitfun/skills/project-guide"); + std::fs::create_dir_all(&project_skill).unwrap(); + std::fs::write( + project_skill.join("SKILL.md"), + "---\nname: project-guide\ndescription: Remote project guide\n---\nRead the project.", + ) + .unwrap(); + let registry = SkillRegistry::new(); + for mode in [ + Ok(RemoteUserSkillsMode::Disabled), + Ok(RemoteUserSkillsMode::Unsupported("future".into())), + Err(OpenBitFunError::config("unreadable preference")), + ] { + let should_warn = !matches!(mode, Ok(RemoteUserSkillsMode::Disabled)); + let scan = registry + .scan_remote_skill_candidates_with_mode( + &LocalWorkspaceFs, + &temp.path().to_string_lossy(), + mode, + ) + .await; + assert_eq!(scan.candidates.len(), 1); + assert!(scan + .candidates + .iter() + .all(|candidate| candidate.info.level == SkillLocation::Project)); + assert_eq!(scan.candidates[0].info.name, "project-guide"); + let prompt: String = scan + .candidates + .iter() + .map(|candidate| candidate.info.to_xml_desc()) + .collect(); + assert!(prompt.contains("project-guide")); + assert!(!prompt.contains("user::")); + assert_eq!( + scan.diagnostics + .iter() + .any(|notice| notice.path == "workspace.remote_user_skills"), + should_warn + ); + } + } +} + #[cfg(all(test, feature = "external-sources"))] mod opencode_configured_skill_tests { use super::{SkillRegistry, SkillRootEntry}; diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_skill_progress.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_skill_progress.rs new file mode 100644 index 0000000000..ca0bbb1f24 --- /dev/null +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_skill_progress.rs @@ -0,0 +1,36 @@ +//! Forward package preparation status through the invoking tool's existing stream. + +use super::SkillLoadProgress; +use crate::agentic::tools::framework::ToolUseContext; +use crate::infrastructure::events::event_system::{get_global_event_system, BackendEvent}; +use crate::util::types::event::ToolExecutionProgressInfo; +use std::sync::Arc; + +pub(crate) fn skill_load_progress( + context: &ToolUseContext, + tool_name: &'static str, +) -> SkillLoadProgress { + let tool_call_id = context.tool_call_id.clone(); + // Package scanning invokes this callback on a blocking worker. Capture the + // runtime here so that those updates can use the same event transport. + let runtime = tokio::runtime::Handle::current(); + Arc::new(move |progress_message, percentage| { + let Some(tool_use_id) = tool_call_id.clone() else { + return; + }; + let event_system = get_global_event_system(); + let event = BackendEvent::ToolExecutionProgress(ToolExecutionProgressInfo { + tool_use_id, + tool_name: tool_name.to_string(), + progress_message, + percentage, + timestamp: std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(), + }); + runtime.spawn(async move { + let _ = event_system.emit(event).await; + }); + }) +} diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_user_skills.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_user_skills.rs new file mode 100644 index 0000000000..0fb7b1afce --- /dev/null +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/remote_user_skills.rs @@ -0,0 +1,540 @@ +//! Execution-host preference and on-demand deployment of local user Skills. + +use super::types::{SkillData, SkillLocation}; +use crate::agentic::workspace::{WorkspaceFileSystem, WorkspaceShell}; +use crate::service::config::global::GlobalConfigManager; +use crate::service::config::types::{RemoteUserSkillsMode, WorkspaceConfig}; +use crate::util::errors::{OpenBitFunError, OpenBitFunResult}; +use openbitfun_agent_runtime::skills::SkillResourceAvailability; +use std::sync::Arc; + +pub type SkillLoadProgress = Arc) + Send + Sync>; + +#[cfg(test)] +tokio::task_local! { + // Keep per-call policy fixtures isolated from the process-global settings. + pub(crate) static TEST_REMOTE_USER_SKILLS_MODE: RemoteUserSkillsMode; +} + +pub(super) async fn remote_user_skills_mode() -> OpenBitFunResult { + #[cfg(test)] + if let Ok(mode) = TEST_REMOTE_USER_SKILLS_MODE.try_with(Clone::clone) { + return Ok(mode); + } + // Read the runtime host's preference, including when a peer/mobile drives it. + let service = GlobalConfigManager::get_service().await?; + let config: WorkspaceConfig = service.get_config(Some("workspace")).await?; + Ok(config.remote_user_skills) +} + +pub(super) fn check_remote_user_skill_mode(mode: &RemoteUserSkillsMode) -> OpenBitFunResult<()> { + match mode { + RemoteUserSkillsMode::Disabled => Err(OpenBitFunError::tool( + "Local user Skills are disabled in remote workspaces. Change Settings > Workspace & Git to enable them.".to_string(), + )), + RemoteUserSkillsMode::Unsupported(value) => Err(OpenBitFunError::tool(format!( + "Unsupported remote user Skills mode '{value}'. Select a supported mode in Settings > Workspace & Git." + ))), + _ => Ok(()), + } +} + +pub(super) async fn prepare_remote_user_skill( + skill: &mut SkillData, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + shell: Option<&dyn WorkspaceShell>, + progress: Option, + parser_source_slot: &str, +) -> OpenBitFunResult<()> { + if skill.location == SkillLocation::Project { + return Ok(()); + } + let mode = remote_user_skills_mode().await?; + prepare_remote_user_skill_with_mode( + skill, + fs, + remote_root, + shell, + progress, + &mode, + parser_source_slot, + ) + .await +} + +async fn prepare_remote_user_skill_with_mode( + skill: &mut SkillData, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + shell: Option<&dyn WorkspaceShell>, + progress: Option, + mode: &RemoteUserSkillsMode, + parser_source_slot: &str, +) -> OpenBitFunResult<()> { + check_remote_user_skill_mode(mode)?; + if *mode == RemoteUserSkillsMode::SkillMdOnly { + skill.resource_availability = Some(SkillResourceAvailability::MarkdownOnly); + skill.compatibility_warnings.push( + "Only SKILL.md was loaded from the runtime host. Its local scripts, references and templates are NOT available in this remote workspace. Do not read or execute those local paths remotely. Select Full in Settings > Workspace & Git to deploy the package before using its resources.".to_string(), + ); + return Ok(()); + } + if skill + .entry_file + .as_deref() + .is_some_and(|entry| entry != "SKILL.md") + { + return Err(OpenBitFunError::tool( + "Full remote loading is unsupported for standalone Markdown Skills without an isolated package directory. Move this Skill and its resources into a dedicated directory with SKILL.md, or select Only inject SKILL.md. The shared parent directory was not copied.", + )); + } + #[cfg(feature = "remote-workspace")] + { + use openbitfun_services_core::skill_package_sync::{ + sync_local_skill_package, SkillPackageSyncPhase, + }; + let shell = shell.ok_or_else(|| OpenBitFunError::tool( + "Full remote user Skill loading requires the bound remote shell; no local fallback is available.".to_string(), + ))?; + let notify = progress.map(|callback| { + Arc::new(move |state: openbitfun_services_core::skill_package_sync::SkillPackageSyncProgress| { + let stage = match state.phase { + SkillPackageSyncPhase::Scanning => "Preparing local Skill package", + SkillPackageSyncPhase::Checking => "Checking remote Skill package", + SkillPackageSyncPhase::Uploading => "Uploading Skill package", + SkillPackageSyncPhase::Extracting => "Verifying and publishing remote Skill package", + SkillPackageSyncPhase::Complete => "Remote Skill package ready", + }; + let percentage = (state.phase == SkillPackageSyncPhase::Uploading && state.total_bytes > 0) + .then(|| (state.transferred_bytes as f32 / state.total_bytes as f32 * 100.0).min(100.0)); + let transfer = if state.phase == SkillPackageSyncPhase::Uploading { + format!(", {} bytes uploaded", state.transferred_bytes) + } else { String::new() }; + callback(format!("{stage} ({} files, {} bytes{transfer}); large packages may take a while", state.file_count, state.total_bytes), percentage); + }) as Arc + }); + let cache_root = format!( + "{}/.openbitfun/cache/user-skills", + remote_root.trim_end_matches('/') + ); + let deployment = sync_local_skill_package( + std::path::Path::new(&skill.path), + &cache_root, + remote_root, + &skill.key, + fs, + shell, + notify, + ) + .await + .map_err(|error| { + OpenBitFunError::tool(format!( + "Failed to deploy complete user Skill package: {error:#}" + )) + })?; + + // Read the deployed snapshot so the instructions and resources agree even + // if the source directory changed during preparation. Paths stay POSIX. + let entry = skill.entry_file.as_deref().unwrap_or("SKILL.md"); + let markdown = fs + .read_file_text(&format!("{}/{entry}", deployment.path)) + .await + .map_err(|error| { + OpenBitFunError::tool(format!("Failed to read deployed Skill: {error}")) + })?; + // Dialects can derive an omitted name from the original directory/file. + // The hash-named deployment is only an IO location, never Skill identity. + let parser_path = if entry == "SKILL.md" { + skill.path.clone() + } else { + format!("{}/{entry}", skill.path.trim_end_matches(['/', '\\'])) + }; + let mut deployed = SkillData::from_markdown_for_source_slot( + parser_path, + &markdown, + skill.location, + true, + parser_source_slot, + ) + .map_err(|error| OpenBitFunError::tool(error.to_string()))?; + deployed.content = map_package_paths(&deployed.content, &skill.path, &deployment.path); + skill.content = deployed.content; + skill.name = deployed.name; + skill.description = deployed.description; + skill.hooks = deployed.hooks; + skill.argument_names = deployed.argument_names; + skill.argument_hint = deployed.argument_hint; + skill.allow_implicit_invocation = deployed.allow_implicit_invocation; + skill.allow_user_invocation = deployed.allow_user_invocation; + skill.compatibility_warnings = deployed.compatibility_warnings; + skill.path = deployment.path; + skill.resource_availability = Some(SkillResourceAvailability::RemotePackage); + skill.compatibility_warnings.push( + "The complete Skill package is available at the remote path above. Use POSIX paths there; commands and project operations still run on the remote host. Runtime dependencies and files outside this Skill package are not installed or copied.".to_string(), + ); + Ok(()) + } + #[cfg(not(feature = "remote-workspace"))] + { + let _ = (skill, fs, remote_root, shell, progress, parser_source_slot); + Err(OpenBitFunError::tool( + "This runtime build does not support complete user Skill deployment to remote workspaces.".to_string(), + )) + } +} + +/// Rewrite references rooted in the source package, including Windows paths. +/// Other absolute paths remain unchanged: they are external dependencies. +#[cfg(any(test, feature = "remote-workspace"))] +fn map_package_paths(content: &str, source: &str, target: &str) -> String { + let windows_source = (source.as_bytes().get(1) == Some(&b':') + && source + .as_bytes() + .first() + .is_some_and(u8::is_ascii_alphabetic)) + || source.starts_with("\\\\"); + let mut prefixes = vec![source.to_string(), source.replace('\\', "/")]; + prefixes.retain(|prefix| !prefix.is_empty()); + prefixes.sort(); + prefixes.dedup(); + let mut mapped = String::new(); + let mut offset = 0; + while let Some((index, prefix)) = prefixes + .iter() + .filter_map(|prefix| { + content[offset..] + .find(prefix) + .map(|index| (offset + index, prefix)) + }) + .min_by_key(|(index, _)| *index) + { + mapped.push_str(&content[offset..index]); + let after_start = index + prefix.len(); + let after = &content[after_start..]; + let before = &content[..index]; + if before.chars().next_back().is_some_and(|c| { + !c.is_whitespace() && !matches!(c, '\'' | '"' | '`' | '(' | '<' | '[' | '=' | ':') + }) { + mapped.push_str(prefix); + offset = after_start; + continue; + } + let boundary = |c: char| { + c.is_whitespace() || matches!(c, '\'' | '"' | '`' | ')' | ']' | '>' | ';' | ',') + }; + if after.starts_with('/') || (windows_source && after.starts_with('\\')) { + mapped.push_str(target); + // Quoted resource names may contain spaces in any path component. + let delimiter = enclosing_path_quote(before).or_else(|| { + before + .chars() + .next_back() + .filter(|c| matches!(c, '\'' | '"' | '`' | '(' | '<')) + }); + let end = match delimiter { + Some('(') => after.find(')'), + Some('<') => after.find('>'), + Some(quote) => after.find(quote), + None => after.find(boundary), + } + .unwrap_or(after.len()); + if windows_source { + mapped.push_str(&after[..end].replace('\\', "/")); + } else { + // POSIX command text may escape spaces with backslashes. They + // are shell syntax, not Windows directory separators. + mapped.push_str(&after[..end]); + } + offset = after_start + end; + } else if after.is_empty() || after.starts_with(boundary) { + mapped.push_str(target); + offset = after_start; + } else { + mapped.push_str(prefix); + offset = after_start; + } + } + mapped.push_str(&content[offset..]); + mapped +} + +#[cfg(any(test, feature = "remote-workspace"))] +fn enclosing_path_quote(before: &str) -> Option { + let mut quote = None; + let mut inline_code = false; + let mut escaped = false; + let mut previous = None::; + let mut chars = before + .rsplit('\n') + .next() + .unwrap_or_default() + .chars() + .peekable(); + while let Some(current) = chars.next() { + if escaped { + escaped = false; + } else if quote == Some('\'') { + if current == '\'' { + quote = None; + } + } else if current == '\\' { + escaped = true; + } else if let Some(active) = quote { + if current == active { + quote = None; + } + } else if matches!(current, '\'' | '"') { + // A possessive apostrophe in prose does not open a command token. + if current != '\'' + || !previous.is_some_and(char::is_alphanumeric) + || !chars.peek().is_some_and(|next| next.is_alphanumeric()) + { + quote = Some(current); + } + } else if current == '`' { + inline_code = !inline_code; + } + previous = Some(current); + } + quote.or(inline_code.then_some('`')) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn user_skill(path: &str, markdown: &str, source: &str) -> SkillData { + SkillData::from_markdown_for_source_slot( + path.into(), + markdown, + SkillLocation::User, + true, + source, + ) + .unwrap() + } + + #[tokio::test] + async fn markdown_only_warns_without_accessing_or_deploying_resources() { + let mut skill = user_skill( + "/nonexistent/local-skill", + "---\nname: demo\ndescription: demo\n---\nRun scripts/hello.py", + "openbitfun", + ); + let original = skill.content.clone(); + prepare_remote_user_skill_with_mode( + &mut skill, + &crate::agentic::workspace::LocalWorkspaceFs, + "/nonexistent/remote", + None, + None, + &RemoteUserSkillsMode::SkillMdOnly, + "openbitfun", + ) + .await + .unwrap(); + assert_eq!(skill.content, original); + assert_eq!(skill.path, "/nonexistent/local-skill"); + assert_eq!( + skill.resource_availability, + Some(SkillResourceAvailability::MarkdownOnly) + ); + assert!(skill + .compatibility_warnings + .iter() + .any(|warning| warning.contains("NOT available"))); + let error = prepare_remote_user_skill_with_mode( + &mut skill, + &crate::agentic::workspace::LocalWorkspaceFs, + "/nonexistent/remote", + None, + None, + &RemoteUserSkillsMode::Disabled, + "openbitfun", + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("disabled")); + } + + #[tokio::test] + async fn full_mode_rejects_flat_markdown_without_copying_shared_parent() { + let mut skill = user_skill( + "/user/shared-skills", + "---\nname: standalone\ndescription: standalone\n---\nRead references/shared.md", + "openbitfun", + ); + skill.entry_file = Some("standalone.md".into()); + let error = prepare_remote_user_skill_with_mode( + &mut skill, + &crate::agentic::workspace::LocalWorkspaceFs, + "/unavailable/remote", + None, + None, + &RemoteUserSkillsMode::Full, + "openbitfun", + ) + .await + .unwrap_err(); + assert!(error + .to_string() + .contains("shared parent directory was not copied")); + assert_eq!(skill.path, "/user/shared-skills"); + } + + #[cfg(all(unix, feature = "remote-workspace"))] + #[tokio::test] + async fn full_user_skill_loads_deployed_snapshot_and_preserves_dialect_identity() { + use crate::agentic::workspace::{LocalWorkspaceFs, LocalWorkspaceShell}; + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("local user/review-package"); + let target = temp.path().join("remote project"); + std::fs::create_dir_all(source.join("scripts")).unwrap(); + std::fs::create_dir_all(source.join("references")).unwrap(); + std::fs::create_dir_all(&target).unwrap(); + let original = format!("---\ndescription: Review package\narguments: target\n---\nRead {}/references/guide.txt and run scripts/hello.py for $target.", source.display()); + std::fs::write(source.join("SKILL.md"), &original).unwrap(); + std::fs::write(source.join("references/guide.txt"), "deployed reference").unwrap(); + std::fs::write( + source.join("scripts/hello.py"), + "import os; print(os.getcwd())", + ) + .unwrap(); + let mut skill = user_skill(&source.to_string_lossy(), &original, "claude"); + let key = skill.key.clone(); + let shell = LocalWorkspaceShell::new(target.to_string_lossy().into_owned()); + let error = prepare_remote_user_skill_with_mode( + &mut skill, + &LocalWorkspaceFs, + &target.to_string_lossy(), + None, + None, + &RemoteUserSkillsMode::Full, + "claude", + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("bound remote shell")); + assert!(!target.join(".openbitfun").exists()); + prepare_remote_user_skill_with_mode( + &mut skill, + &LocalWorkspaceFs, + &target.to_string_lossy(), + Some(&shell), + None, + &RemoteUserSkillsMode::Full, + "claude", + ) + .await + .unwrap(); + assert_eq!(skill.name, "review-package"); + assert_eq!( + skill.resource_availability, + Some(SkillResourceAvailability::RemotePackage) + ); + assert_eq!(skill.key, key); + assert_eq!(skill.argument_names, ["target"]); + assert!(skill + .path + .starts_with(&target.to_string_lossy().to_string())); + assert!(!skill.path.contains('\\')); + assert!(skill + .content + .contains(&format!("{}/references/guide.txt", skill.path))); + assert!(!skill + .content + .contains(&source.to_string_lossy().to_string())); + assert_eq!( + LocalWorkspaceFs + .read_file_text(&format!("{}/references/guide.txt", skill.path)) + .await + .unwrap(), + "deployed reference" + ); + let (stdout, stderr, exit) = shell + .exec(&format!("python3 '{}/scripts/hello.py'", skill.path), None) + .await + .unwrap(); + assert_eq!(exit, 0, "{stderr}"); + assert_eq!( + stdout.trim(), + std::fs::canonicalize(&target).unwrap().to_string_lossy() + ); + } + + #[test] + fn remote_user_skill_modes_fail_closed_for_disabled_and_future_values() { + assert!(check_remote_user_skill_mode(&RemoteUserSkillsMode::Disabled).is_err()); + assert!( + check_remote_user_skill_mode(&RemoteUserSkillsMode::Unsupported("future".to_string())) + .is_err() + ); + assert!(check_remote_user_skill_mode(&RemoteUserSkillsMode::SkillMdOnly).is_ok()); + assert!(check_remote_user_skill_mode(&RemoteUserSkillsMode::Full).is_ok()); + } + + #[test] + fn source_package_paths_become_posix_without_rewriting_external_paths() { + assert_eq!( + map_package_paths( + "python \"--config=C:\\skills\\demo\\my config\\settings.json\"", + "C:\\skills\\demo", + "/remote/demo", + ), + "python \"--config=/remote/demo/my config/settings.json\"" + ); + assert_eq!( + map_package_paths( + "`python \"--config=C:/skills/demo/my config\\settings.json\"`", + "C:/skills/demo", + "/remote/demo", + ), + "`python \"--config=/remote/demo/my config/settings.json\"`" + ); + assert_eq!( + map_package_paths( + "python /local/skill/my\\ scripts/hello.py", + "/local/skill", + "/remote/skill", + ), + "python /remote/skill/my\\ scripts/hello.py" + ); + assert_eq!( + map_package_paths( + "Read /backup/home/me/skill/references/x and /home/me/skill/references/x", + "/home/me/skill", + "/remote/cache", + ), + "Read /backup/home/me/skill/references/x and /remote/cache/references/x" + ); + assert_eq!( + map_package_paths( + "python \"C:\\skills\\demo\\my scripts\\hello.py\"", + "C:\\skills\\demo", + "/remote/demo", + ), + "python \"/remote/demo/my scripts/hello.py\"" + ); + assert_eq!( + map_package_paths( + "Read /local/skill/references/a.md", + "/local/skill", + "/local/skill/cache", + ), + "Read /local/skill/cache/references/a.md" + ); + assert_eq!(map_package_paths( + "python \"C:\\Users\\Alice\\my skill\\scripts\\hello.py\"; see `C:/Users/Alice/my skill/references/doc.md`", + "C:\\Users\\Alice\\my skill", "/remote/my-skill", + ), "python \"/remote/my-skill/scripts/hello.py\"; see `/remote/my-skill/references/doc.md`"); + assert_eq!( + map_package_paths( + "Read /local/skill/references/a.md and /local/skill-extra/x and /other/file", + "/local/skill", + "/remote/skill", + ), + "Read /remote/skill/references/a.md and /local/skill-extra/x and /other/file" + ); + } +} diff --git a/src/crates/contracts/config-contracts/src/types.rs b/src/crates/contracts/config-contracts/src/types.rs index 025a54b777..624091f342 100644 --- a/src/crates/contracts/config-contracts/src/types.rs +++ b/src/crates/contracts/config-contracts/src/types.rs @@ -812,6 +812,8 @@ pub struct TerminalConfig { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default)] pub struct WorkspaceConfig { + /// Availability and resource deployment of host-local user Skills in remote workspaces. + pub remote_user_skills: RemoteUserSkillsMode, pub exclude_patterns: Vec, pub include_patterns: Vec, pub watch_ignore: Vec, @@ -823,6 +825,39 @@ pub struct WorkspaceConfig { pub insert_final_newline: bool, } +/// Keep unknown future values intact on disk; runtime callers must reject them. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(from = "String", into = "String")] +pub enum RemoteUserSkillsMode { + Disabled, + #[default] + SkillMdOnly, + Full, + Unsupported(String), +} + +impl From for RemoteUserSkillsMode { + fn from(value: String) -> Self { + match value.as_str() { + "disabled" => Self::Disabled, + "skill_md_only" => Self::SkillMdOnly, + "full" => Self::Full, + _ => Self::Unsupported(value), + } + } +} + +impl From for String { + fn from(value: RemoteUserSkillsMode) -> Self { + match value { + RemoteUserSkillsMode::Disabled => "disabled".into(), + RemoteUserSkillsMode::SkillMdOnly => "skill_md_only".into(), + RemoteUserSkillsMode::Full => "full".into(), + RemoteUserSkillsMode::Unsupported(value) => value, + } + } +} + /// Model capability type (a model can have multiple capabilities). #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] #[serde(rename_all = "snake_case")] @@ -2260,6 +2295,7 @@ impl Default for TerminalConfig { impl Default for WorkspaceConfig { fn default() -> Self { Self { + remote_user_skills: RemoteUserSkillsMode::default(), exclude_patterns: vec![ "**/node_modules/**".to_string(), "**/target/**".to_string(), @@ -2492,6 +2528,29 @@ impl AIModelConfig { #[cfg(test)] mod tests { + #[test] + fn remote_user_skills_preserves_legacy_workspace_settings_and_future_modes() { + use super::{RemoteUserSkillsMode, WorkspaceConfig}; + let legacy = serde_json::json!({ + "exclude_patterns": ["legacy/**"], "encoding": "utf8", "max_file_size": 1234 + }); + let config: WorkspaceConfig = serde_json::from_value(legacy.clone()).unwrap(); + assert_eq!(config.remote_user_skills, RemoteUserSkillsMode::SkillMdOnly); + let saved = serde_json::to_value(config).unwrap(); + for (key, value) in legacy.as_object().unwrap() { + assert_eq!(&saved[key], value); + } + for mode in ["skill_md_only", "disabled", "full", "future_mode"] { + let mut payload = legacy.clone(); + payload["remote_user_skills"] = mode.into(); + let config: WorkspaceConfig = serde_json::from_value(payload).unwrap(); + let saved = serde_json::to_value(&config).unwrap(); + assert_eq!(saved["remote_user_skills"], mode); + let restored: WorkspaceConfig = serde_json::from_value(saved).unwrap(); + assert_eq!(restored.remote_user_skills, config.remote_user_skills); + } + } + #[test] fn model_routing_summary_defaults_to_automatic_and_preserves_old_switch_values() { let legacy: super::ModelRoutingConfig = diff --git a/src/crates/execution/agent-runtime/src/skills/mod.rs b/src/crates/execution/agent-runtime/src/skills/mod.rs index 49406af68f..2d64d50ea8 100644 --- a/src/crates/execution/agent-runtime/src/skills/mod.rs +++ b/src/crates/execution/agent-runtime/src/skills/mod.rs @@ -46,6 +46,6 @@ pub use selection::{ }; pub use types::{ render_loaded_skill_for_assistant, ModeSkillInfo, ModeSkillStateReason, SkillData, - SkillImportOrigin, SkillInfo, SkillLocation, SkillParseError, SkillScanDiagnostic, - SkillScanReport, + SkillImportOrigin, SkillInfo, SkillLocation, SkillParseError, SkillResourceAvailability, + SkillScanDiagnostic, SkillScanReport, }; diff --git a/src/crates/execution/agent-runtime/src/skills/types.rs b/src/crates/execution/agent-runtime/src/skills/types.rs index 686ac6675d..0a1712c330 100644 --- a/src/crates/execution/agent-runtime/src/skills/types.rs +++ b/src/crates/execution/agent-runtime/src/skills/types.rs @@ -231,6 +231,8 @@ pub struct SkillData { pub location: SkillLocation, pub path: String, pub entry_file: Option, + /// Resource availability for this load; absent for local/project/legacy loads. + pub resource_availability: Option, pub source_slot: String, pub source_id: String, pub source_label: String, @@ -245,6 +247,13 @@ pub struct SkillData { pub hooks: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SkillResourceAvailability { + MarkdownOnly, + RemotePackage, +} + fn default_allow_implicit_invocation() -> bool { true } @@ -619,6 +628,7 @@ impl SkillData { location, path, entry_file: None, + resource_availability: None, source_slot: String::new(), source_id: String::new(), source_label: String::new(), diff --git a/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs b/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs index 9d0ed05ef6..2996413e20 100644 --- a/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs +++ b/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs @@ -835,6 +835,7 @@ Use the pdf workflow. assert_eq!(data.description, "Work with PDF files."); assert_eq!(data.dir_name, "pdf"); assert_eq!(data.content, "Use the pdf workflow.\n"); + assert_eq!(data.resource_availability, None); let assistant = render_loaded_skill_for_assistant(&data, false); assert!(assistant.contains("Skill 'pdf' loaded successfully.")); diff --git a/src/crates/services/services-core/AGENTS.md b/src/crates/services/services-core/AGENTS.md index 9c3f91c5ce..3246711329 100644 --- a/src/crates/services/services-core/AGENTS.md +++ b/src/crates/services/services-core/AGENTS.md @@ -219,6 +219,20 @@ For bounded workspace upload ownership, offset recovery, conflict detection, and cargo test -p openbitfun-services-core --no-default-features --features workspace-transfer,workspace-runtime --lib workspace_transfer::tests ``` +For complete local Skill package deployment, cache integrity, source changes, +executable modes and unsupported targets: + +```bash +cargo test --locked -p openbitfun-services-core --no-default-features --features skill-package-sync,workspace-runtime --lib skill_package_sync::tests +``` + +These tests use local POSIX workspace providers. The real loopback SSH/SFTP +exercise is documented in the services-integrations guide. Full deployment +requires Python 3 on the POSIX target and rejects nested symlinks or reparse +points; a symlinked package root is canonicalized before scanning. +The selected remote workspace may have symlink ancestors; only that trusted +root is resolved. Managed cache components below it must remain plain directories. + Workspace catalog identity and upgrade deserialization (without live filesystem/SSH activation): ```bash diff --git a/src/crates/services/services-core/Cargo.toml b/src/crates/services/services-core/Cargo.toml index e11052106a..b4f6a43525 100644 --- a/src/crates/services/services-core/Cargo.toml +++ b/src/crates/services/services-core/Cargo.toml @@ -158,6 +158,18 @@ workspace-transfer = [ "tokio/rt", "tokio/sync", ] +skill-package-sync = [ + "dep:anyhow", + "dep:openbitfun-runtime-ports", + "openbitfun-runtime-ports/workspace-ports", + "dep:sha2", + "dep:zip", + "dep:uuid", + "dep:tokio", + "tokio/fs", + "tokio/io-util", + "tokio/rt", +] workspace-runtime = [ "dep:anyhow", "dep:async-trait", diff --git a/src/crates/services/services-core/src/lib.rs b/src/crates/services/services-core/src/lib.rs index d17bdcd365..9bd7bf691d 100644 --- a/src/crates/services/services-core/src/lib.rs +++ b/src/crates/services/services-core/src/lib.rs @@ -61,6 +61,8 @@ pub mod resource_capacity; pub mod routing_state; #[cfg(feature = "session-event-format")] pub mod session_projection_format; +#[cfg(feature = "skill-package-sync")] +pub mod skill_package_sync; pub mod storage_error; #[cfg(feature = "workspace-persistence")] pub mod workspace_persistence; diff --git a/src/crates/services/services-core/src/skill_package_sync.py b/src/crates/services/services-core/src/skill_package_sync.py new file mode 100644 index 0000000000..51680fa2af --- /dev/null +++ b/src/crates/services/services-core/src/skill_package_sync.py @@ -0,0 +1,199 @@ +"""POSIX target helper embedded by the workspace package transfer service.""" +import contextlib +import fcntl +import hashlib +import json +import os +import shutil +import stat +import sys +import uuid +import zipfile + +MANIFEST = ".openbitfun-package-manifest.json" + + +def plain_directory(path): + if not os.path.lexists(path): + os.mkdir(path, 0o700) + if not stat.S_ISDIR(os.lstat(path).st_mode): + raise ValueError("Skill cache path must be a directory without symbolic links: " + path) + + +def cache_directory(path, workspace): + # The selected workspace may have ordinary symlink ancestors (for example + # macOS /var and /tmp). Resolve only that trusted boundary, never a + # repository-controlled component of the managed cache below it. + workspace_parts = [part for part in workspace.split("/") if part] + cache_parts = [part for part in path.split("/") if part] + if (len(cache_parts) <= len(workspace_parts) + or cache_parts[:len(workspace_parts)] != workspace_parts): + raise ValueError("Skill package cache must be inside the selected remote workspace") + current = os.path.realpath(workspace) + if not os.path.isdir(current): + raise ValueError("Selected remote workspace is not a directory") + for component in cache_parts[len(workspace_parts):]: + current = os.path.join(current, component) + plain_directory(current) + if os.stat(current).st_uid != os.geteuid(): + raise ValueError("Skill cache must be owned by the target user") + + +def manifest_entries(raw, expected_hash): + if hashlib.sha256(raw).hexdigest() != expected_hash: + raise ValueError("Skill package manifest checksum mismatch") + data = json.loads(raw) + if data.get("version") != 1: + raise ValueError("Unsupported Skill package manifest version") + entries = {} + for entry in data["entries"]: + name = entry["path"] + if (not isinstance(name, str) or not name or name == MANIFEST + or "\\" in name or "\0" in name or name.startswith("/") + or any(part in ("", ".", "..") for part in name.split("/"))): + raise ValueError("Invalid Skill package relative path") + if name in entries or entry["kind"] not in ("file", "directory"): + raise ValueError("Invalid or duplicate Skill package entry") + if not isinstance(entry["mode"], int) or not 0 <= entry["mode"] <= 0o777: + raise ValueError("Invalid Skill package permissions") + entries[name] = entry + return entries + + +def verify(root, digest): + try: + if not stat.S_ISDIR(os.lstat(root).st_mode): + return False + manifest_path = os.path.join(root, MANIFEST) + if not stat.S_ISREG(os.lstat(manifest_path).st_mode): + return False + with open(manifest_path, "rb") as stream: + entries = manifest_entries(stream.read(), digest) + seen = set() + def unreadable(error): + raise error + for directory, dirs, files in os.walk(root, followlinks=False, onerror=unreadable): + for name in dirs + files: + full = os.path.join(directory, name) + relative = os.path.relpath(full, root).replace(os.sep, "/") + if relative == MANIFEST: + continue + expected = entries.get(relative) + if expected is None: + return False + metadata = os.lstat(full) + if stat.S_IMODE(metadata.st_mode) != expected["mode"]: + return False + if expected["kind"] == "directory": + if not stat.S_ISDIR(metadata.st_mode): + return False + else: + if not stat.S_ISREG(metadata.st_mode) or metadata.st_size != expected["size"]: + return False + content_hash = hashlib.sha256() + with open(full, "rb") as stream: + for chunk in iter(lambda: stream.read(1024 * 1024), b""): + content_hash.update(chunk) + if content_hash.hexdigest() != expected["sha256"]: + return False + seen.add(relative) + return seen == set(entries) + except (OSError, ValueError, KeyError, TypeError): + return False + + +def unpack(archive, staging, digest): + with zipfile.ZipFile(archive) as package: + raw = package.read(MANIFEST) + entries = manifest_entries(raw, digest) + expected_names = {MANIFEST} | { + name + ("/" if entry["kind"] == "directory" else "") + for name, entry in entries.items() + } + names = package.namelist() + if len(names) != len(expected_names) or set(names) != expected_names: + raise ValueError("Skill package archive entries do not match its manifest") + os.mkdir(staging, 0o700) + for name, entry in sorted(entries.items()): + path = os.path.join(staging, name) + if entry["kind"] == "directory": + os.makedirs(path, exist_ok=True) + else: + os.makedirs(os.path.dirname(path), exist_ok=True) + with package.open(name) as source, open(path, "xb") as destination: + shutil.copyfileobj(source, destination, 1024 * 1024) + os.chmod(path, entry["mode"]) + with open(os.path.join(staging, MANIFEST), "xb") as stream: + stream.write(raw) + for name, entry in sorted(entries.items(), reverse=True): + if entry["kind"] == "directory": + os.chmod(os.path.join(staging, name), entry["mode"]) + if not verify(staging, digest): + raise ValueError("Extracted Skill package failed full integrity verification") + + +def main(): + action, cache, workspace, package_id, digest, upload_name = sys.argv[1:] + for path in (cache, workspace): + if (not path.startswith("/") or "\0" in path + or any(part in (".", "..") for part in path.split("/"))): + raise ValueError("Skill cache and workspace require absolute POSIX paths") + for identifier in (package_id, digest): + if len(identifier) != 64 or any(char not in "0123456789abcdef" for char in identifier): + raise ValueError("Invalid Skill package identity") + cache_directory(cache, workspace) + parent = os.path.join(cache, package_id) + plain_directory(parent) + target = os.path.join(parent, digest) + # The package lock serializes atomic publication across runtime processes. + lock_path = os.path.join(parent, ".lock") + lock_fd = os.open(lock_path, os.O_CREAT | os.O_RDWR | os.O_NOFOLLOW, 0o600) + with os.fdopen(lock_fd, "a") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + if verify(target, digest): + print(json.dumps({"path": target, "reused": True})) + return + if action == "check": + print(json.dumps({"path": target, "reused": False})) + return + if action != "publish" or not upload_name.startswith(".upload-") or "/" in upload_name: + raise ValueError("Invalid Skill package publication request") + archive = os.path.join(parent, upload_name) + if not stat.S_ISREG(os.lstat(archive).st_mode): + raise ValueError("Skill package upload must be a regular file") + staging = os.path.join(parent, ".staging-" + uuid.uuid4().hex) + previous = os.path.join(parent, ".previous-" + uuid.uuid4().hex) + moved_previous = False + try: + unpack(archive, staging, digest) + if os.path.lexists(target): + if not stat.S_ISDIR(os.lstat(target).st_mode): + raise ValueError("Existing Skill deployment is not a plain directory") + os.rename(target, previous) + moved_previous = True + try: + os.rename(staging, target) + except BaseException: + if moved_previous: + os.rename(previous, target) + moved_previous = False + raise + if moved_previous: + # Publication already committed. Cleanup must not turn a valid + # deployment into a reported failure (for example read-only dirs). + with contextlib.suppress(OSError): + shutil.rmtree(previous) + finally: + if os.path.lexists(staging): + with contextlib.suppress(OSError): + shutil.rmtree(staging) + with contextlib.suppress(OSError): + os.unlink(archive) + print(json.dumps({"path": target, "reused": False})) + + +try: + main() +except Exception as error: + print("Skill package synchronization failed: " + str(error), file=sys.stderr) + sys.exit(1) diff --git a/src/crates/services/services-core/src/skill_package_sync.rs b/src/crates/services/services-core/src/skill_package_sync.rs new file mode 100644 index 0000000000..d01cb0805a --- /dev/null +++ b/src/crates/services/services-core/src/skill_package_sync.rs @@ -0,0 +1,586 @@ +//! Verified, staged deployment of a local directory through workspace IO ports. +//! +//! The caller owns Skill selection and the cache location. This owner supplies +//! the concrete archive, transfer, integrity and publication behavior. + +use anyhow::{bail, Context, Result}; +use openbitfun_runtime_ports::{WorkspaceCommandOptions, WorkspaceFileSystem, WorkspaceShell}; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use std::{ + fs::{self, File}, + io::{Read, Write}, + path::{Path, PathBuf}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, + }, + time::{Duration, Instant}, +}; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use zip::{write::SimpleFileOptions, CompressionMethod, ZipWriter}; + +const MANIFEST_NAME: &str = ".openbitfun-package-manifest.json"; +const REMOTE_HELPER: &str = include_str!("skill_package_sync.py"); + +#[cfg(test)] +#[path = "skill_package_sync_tests.rs"] +mod tests; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SkillPackageSyncPhase { + Scanning, + Checking, + Uploading, + Extracting, + Complete, +} + +#[derive(Clone, Debug)] +pub struct SkillPackageSyncProgress { + pub phase: SkillPackageSyncPhase, + pub file_count: usize, + pub total_bytes: u64, + pub transferred_bytes: u64, +} + +pub type SkillPackageProgressCallback = Arc; + +#[derive(Clone, Debug)] +pub struct SkillPackageDeployment { + /// Absolute POSIX path on the target, independent of the controller OS. + pub path: String, + pub content_hash: String, + pub file_count: usize, + pub total_bytes: u64, + pub reused: bool, +} + +#[derive(Serialize)] +struct Manifest { + version: u8, + entries: Vec, +} + +#[derive(Serialize)] +struct ManifestEntry { + path: String, + kind: &'static str, + mode: u32, + size: u64, + sha256: String, +} + +struct LocalArchive { + path: PathBuf, + root: PathBuf, + manifest: Manifest, + manifest_bytes: Vec, + package_id: String, + content_hash: String, + file_count: usize, + total_bytes: u64, + archive_bytes: u64, + archive_created: bool, + cancelled: Arc, +} + +struct CancelLocalWork(Arc); + +impl Drop for CancelLocalWork { + fn drop(&mut self) { + self.0.store(true, Ordering::Relaxed); + } +} + +fn check_cancelled(cancelled: &AtomicBool) -> Result<()> { + if cancelled.load(Ordering::Relaxed) { + bail!("Local Skill package preparation was cancelled"); + } + Ok(()) +} + +impl Drop for LocalArchive { + fn drop(&mut self) { + if self.archive_created { + let _ = fs::remove_file(&self.path); + } + } +} + +fn report( + callback: &Option, + phase: SkillPackageSyncPhase, + file_count: usize, + total_bytes: u64, + transferred_bytes: u64, +) { + if let Some(callback) = callback { + callback(SkillPackageSyncProgress { + phase, + file_count, + total_bytes, + transferred_bytes, + }); + } +} + +/// Copy a complete local package only when the target does not contain a +/// byte-for-byte, mode-for-mode matching tree. No workspace file IO is redirected +/// to the local host. Python 3 on the POSIX target is an explicit prerequisite. +/// The selected workspace is the trusted boundary for symlink resolution; +/// the managed cache must be nested inside it and contain no symlink components. +/// +/// Files are compressed once and sent in one streamed upload, avoiding one SSH +/// round trip per small file. Target verification also runs in one command. +pub async fn sync_local_skill_package( + local_root: &Path, + remote_cache_root: &str, + remote_workspace_root: &str, + package_key: &str, + fs: &dyn WorkspaceFileSystem, + shell: &dyn WorkspaceShell, + progress: Option, +) -> Result { + validate_remote_cache_location(remote_cache_root, remote_workspace_root)?; + // Dropping the async invocation stops the blocking scan/packing worker at + // its next entry/chunk rather than leaving a large package running. + let cancelled = Arc::new(AtomicBool::new(false)); + let _cancel_local_work = CancelLocalWork(cancelled.clone()); + let progress: Option = progress.map(|callback| { + let last = Mutex::new(None::<(SkillPackageSyncPhase, Instant)>); + Arc::new(move |event: SkillPackageSyncProgress| { + let now = Instant::now(); + let mut last = last.lock().unwrap_or_else(|error| error.into_inner()); + if last.is_some_and(|(phase, time)| { + phase == event.phase && now.duration_since(time) < Duration::from_millis(250) + }) { + return; + } + *last = Some((event.phase, now)); + drop(last); + callback(event); + }) as SkillPackageProgressCallback + }); + report(&progress, SkillPackageSyncPhase::Scanning, 0, 0, 0); + let root = local_root.to_owned(); + let key = package_key.to_owned(); + let callback = progress.clone(); + let archive = + tokio::task::spawn_blocking(move || scan_package(&root, &key, &callback, cancelled)) + .await + .context("Local Skill package scan failed")??; + + report( + &progress, + SkillPackageSyncPhase::Checking, + archive.file_count, + archive.total_bytes, + 0, + ); + let check = remote_command( + shell, + "check", + remote_cache_root, + remote_workspace_root, + &archive, + "", + ) + .await?; + let destination = check.path; + if check.reused { + report( + &progress, + SkillPackageSyncPhase::Complete, + archive.file_count, + archive.total_bytes, + 0, + ); + return Ok(deployment(&archive, destination, true)); + } + + // Compression and temporary archive IO are unnecessary on a cache hit. + // Re-read and hash while packing so a source edit cannot pair stale + // instructions with newer resources. + report( + &progress, + SkillPackageSyncPhase::Scanning, + archive.file_count, + archive.total_bytes, + 0, + ); + let archive = tokio::task::spawn_blocking(move || build_archive(archive)) + .await + .context("Local Skill package preparation failed")??; + let upload_name = format!(".upload-{}.zip", uuid::Uuid::new_v4()); + let remote_archive = format!( + "{}/{}/{}", + remote_cache_root.trim_end_matches('/'), + archive.package_id, + upload_name + ); + let transfer = async { + report( + &progress, + SkillPackageSyncPhase::Uploading, + archive.file_count, + archive.total_bytes, + 0, + ); + let mut output = fs + .open_write_new(&remote_archive) + .await + .context("Target does not support streaming Skill package uploads")?; + let mut input = tokio::fs::File::open(&archive.path).await?; + let mut buffer = vec![0; 64 * 1024]; + let mut transferred = 0; + loop { + let read = input.read(&mut buffer).await?; + if read == 0 { + break; + } + output.write_all(&buffer[..read]).await?; + transferred += read as u64; + report( + &progress, + SkillPackageSyncPhase::Uploading, + archive.file_count, + archive.archive_bytes, + transferred, + ); + } + output.shutdown().await?; + drop(output); + report( + &progress, + SkillPackageSyncPhase::Extracting, + archive.file_count, + archive.total_bytes, + transferred, + ); + remote_command( + shell, + "publish", + remote_cache_root, + remote_workspace_root, + &archive, + &upload_name, + ) + .await + } + .await; + // Never remove a published directory when an observer or transfer fails. + // A hard disconnect can leave only an unreferenced upload/staging sibling. + let _ = fs.remove_file(&remote_archive).await; + let published = transfer?; + report( + &progress, + SkillPackageSyncPhase::Complete, + archive.file_count, + archive.total_bytes, + archive.archive_bytes, + ); + Ok(deployment(&archive, published.path, published.reused)) +} + +fn deployment(archive: &LocalArchive, path: String, reused: bool) -> SkillPackageDeployment { + SkillPackageDeployment { + path, + content_hash: archive.content_hash.clone(), + file_count: archive.file_count, + total_bytes: archive.total_bytes, + reused, + } +} + +fn validate_remote_root(root: &str) -> Result<()> { + if !root.starts_with('/') + || root.contains('\0') + || root.split('/').any(|part| matches!(part, "." | "..")) + { + bail!("Skill package cache requires an absolute POSIX target path without dot segments"); + } + Ok(()) +} + +fn validate_remote_cache_location(cache: &str, workspace: &str) -> Result<()> { + validate_remote_root(cache)?; + validate_remote_root(workspace)?; + let workspace: Vec<_> = workspace + .split('/') + .filter(|part| !part.is_empty()) + .collect(); + let cache: Vec<_> = cache.split('/').filter(|part| !part.is_empty()).collect(); + if cache.len() <= workspace.len() || !cache.starts_with(&workspace) { + bail!("Skill package cache must be inside the selected remote workspace"); + } + Ok(()) +} + +fn quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) +} + +#[derive(Deserialize)] +struct RemoteResult { + path: String, + reused: bool, +} + +async fn remote_command( + shell: &dyn WorkspaceShell, + action: &str, + cache_root: &str, + workspace_root: &str, + archive: &LocalArchive, + upload_name: &str, +) -> Result { + let arguments = [ + REMOTE_HELPER, + action, + cache_root, + workspace_root, + &archive.package_id, + &archive.content_hash, + upload_name, + ] + .map(quote) + .join(" "); + let result = shell + .exec_with_options( + &format!( + "command -v python3 >/dev/null 2>&1 || {{ printf '%s\\n' 'Full user Skill sync requires Python 3 on the remote target' >&2; exit 127; }}; python3 -c {arguments}" + ), + // Hashing many files and extracting an archive may legitimately + // take minutes. The transport and turn owner handle cancellation. + WorkspaceCommandOptions::default(), + ) + .await + .context("Remote Skill package synchronization failed")?; + if result.exit_code != 0 || result.interrupted || result.timed_out { + bail!( + "Remote Skill package {action} failed: {}", + result.combined_output().trim() + ); + } + let result: RemoteResult = serde_json::from_str(result.stdout.trim()) + .context("Remote Skill package helper returned an invalid result")?; + let expected = format!( + "{}/{}/{}", + cache_root.trim_end_matches('/'), + archive.package_id, + archive.content_hash + ); + if result.path != expected { + bail!("Remote Skill package helper returned an unexpected deployment path"); + } + Ok(result) +} + +fn scan_package( + local_root: &Path, + key: &str, + progress: &Option, + cancelled: Arc, +) -> Result { + check_cancelled(&cancelled)?; + let root = fs::canonicalize(local_root) + .with_context(|| format!("Cannot read local Skill directory {}", local_root.display()))?; + if !root.is_dir() { + bail!("Local Skill package is not a directory: {}", root.display()); + } + let package_id = format!( + "{:x}", + Sha256::digest(serde_json::to_vec(&(key, root.to_string_lossy()))?) + ); + let archive_path = std::env::temp_dir().join(format!( + "openbitfun-skill-package-{}.zip", + uuid::Uuid::new_v4() + )); + let mut archive = LocalArchive { + path: archive_path, + root: root.clone(), + manifest: Manifest { + version: 1, + entries: Vec::new(), + }, + manifest_bytes: Vec::new(), + package_id, + content_hash: String::new(), + file_count: 0, + total_bytes: 0, + archive_bytes: 0, + archive_created: false, + cancelled, + }; + let mut manifest = Manifest { + version: 1, + entries: Vec::new(), + }; + scan_directory(&root, "", &mut manifest.entries, &mut archive, progress)?; + manifest.entries.sort_by(|a, b| a.path.cmp(&b.path)); + archive.manifest_bytes = serde_json::to_vec(&manifest)?; + archive.content_hash = format!("{:x}", Sha256::digest(&archive.manifest_bytes)); + archive.manifest = manifest; + Ok(archive) +} + +fn scan_directory( + directory: &Path, + prefix: &str, + manifest: &mut Vec, + archive: &mut LocalArchive, + progress: &Option, +) -> Result<()> { + let mut children = fs::read_dir(directory)?.collect::>>()?; + children.sort_by_key(|entry| entry.file_name()); + for child in children { + check_cancelled(&archive.cancelled)?; + let name = child + .file_name() + .into_string() + .map_err(|_| anyhow::anyhow!("Full user Skill sync requires UTF-8 file names"))?; + if name.contains('\\') { + bail!("Full user Skill sync cannot map a file name containing a backslash: {name}"); + } + let relative = if prefix.is_empty() { + name + } else { + format!("{prefix}/{name}") + }; + if relative == MANIFEST_NAME { + bail!("Skill package contains the reserved file {MANIFEST_NAME}"); + } + let path = child.path(); + let metadata = fs::symlink_metadata(&path)?; + if crate::bounded_fs::is_symlink_or_reparse(&metadata) { + bail!( + "Full user Skill sync does not support nested symbolic links: {relative}. Replace the link with package-local files" + ); + } + let mode = local_mode(&metadata); + let mut entry = ManifestEntry { + path: relative.clone(), + kind: "directory", + mode, + size: 0, + sha256: String::new(), + }; + if metadata.is_dir() { + scan_directory(&path, &relative, manifest, archive, progress)?; + } else if metadata.is_file() { + entry.kind = "file"; + let mut input = File::open(&path)?; + let mut digest = Sha256::new(); + let mut buffer = [0; 64 * 1024]; + loop { + check_cancelled(&archive.cancelled)?; + let read = input.read(&mut buffer)?; + if read == 0 { + break; + } + digest.update(&buffer[..read]); + entry.size += read as u64; + } + if input.metadata()?.modified().ok() != metadata.modified().ok() + || entry.size != metadata.len() + { + bail!("Local Skill file changed while preparing its package: {relative}. Try loading the Skill again"); + } + entry.sha256 = format!("{:x}", digest.finalize()); + archive.file_count += 1; + archive.total_bytes += entry.size; + report( + progress, + SkillPackageSyncPhase::Scanning, + archive.file_count, + archive.total_bytes, + 0, + ); + } else { + bail!("Full user Skill sync only supports regular files and directories: {relative}"); + } + manifest.push(entry); + } + Ok(()) +} + +fn build_archive(mut archive: LocalArchive) -> Result { + let mut options = File::options(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let file = options.open(&archive.path)?; + archive.archive_created = true; + let mut zip = ZipWriter::new(file); + for entry in &archive.manifest.entries { + check_cancelled(&archive.cancelled)?; + let path = archive.root.join(&entry.path); + let metadata = fs::symlink_metadata(&path)?; + if crate::bounded_fs::is_symlink_or_reparse(&metadata) + || local_mode(&metadata) != entry.mode + || !fs::canonicalize(&path)?.starts_with(&archive.root) + { + bail!("Local Skill package changed while preparing its archive: {}. Try loading the Skill again", entry.path); + } + let options = SimpleFileOptions::default() + .compression_method(CompressionMethod::Deflated) + .unix_permissions(entry.mode); + if entry.kind == "directory" { + if !metadata.is_dir() { + bail!( + "Local Skill directory changed while preparing its archive: {}", + entry.path + ); + } + zip.add_directory(format!("{}/", entry.path), options)?; + continue; + } + if !metadata.is_file() { + bail!( + "Local Skill file changed while preparing its archive: {}", + entry.path + ); + } + let mut input = File::open(&path)?; + let mut digest = Sha256::new(); + let mut size = 0; + zip.start_file(&entry.path, options)?; + let mut buffer = [0; 64 * 1024]; + loop { + check_cancelled(&archive.cancelled)?; + let read = input.read(&mut buffer)?; + if read == 0 { + break; + } + zip.write_all(&buffer[..read])?; + digest.update(&buffer[..read]); + size += read as u64; + } + if size != entry.size || format!("{:x}", digest.finalize()) != entry.sha256 { + bail!("Local Skill file changed while preparing its archive: {}. Try loading the Skill again", entry.path); + } + } + zip.start_file(MANIFEST_NAME, SimpleFileOptions::default())?; + zip.write_all(&archive.manifest_bytes)?; + archive.archive_bytes = zip.finish()?.metadata()?.len(); + Ok(archive) +} + +#[cfg(unix)] +fn local_mode(metadata: &fs::Metadata) -> u32 { + use std::os::unix::fs::PermissionsExt; + metadata.permissions().mode() & 0o777 +} + +#[cfg(not(unix))] +fn local_mode(metadata: &fs::Metadata) -> u32 { + if metadata.is_dir() { + 0o755 + } else { + 0o644 + } +} diff --git a/src/crates/services/services-core/src/skill_package_sync_tests.rs b/src/crates/services/services-core/src/skill_package_sync_tests.rs new file mode 100644 index 0000000000..478c06ba50 --- /dev/null +++ b/src/crates/services/services-core/src/skill_package_sync_tests.rs @@ -0,0 +1,430 @@ +use super::*; + +#[test] +fn remote_cache_paths_use_posix_syntax_on_every_controller() { + assert!(validate_remote_root("/home/user/project/cache").is_ok()); + assert!(validate_remote_root("/home/user/a\\b/cache").is_ok()); + for path in [ + "C:\\Users\\me\\cache", + "relative/cache", + "/tmp/../escape", + "/tmp/./cache", + ] { + assert!(validate_remote_root(path).is_err(), "{path}"); + } + assert_eq!(quote("a'b\n$HOME`x`"), "'a'\\''b\n$HOME`x`'"); + assert!(validate_remote_cache_location("/workspace/.openbitfun/cache", "/workspace").is_ok()); + assert!( + validate_remote_cache_location("/workspace/.openbitfun/cache", "/workspace-other").is_err() + ); + assert!(validate_remote_cache_location("/workspace", "/workspace").is_err()); + assert!(validate_remote_cache_location("/outside/cache", "/workspace").is_err()); +} + +#[test] +fn scanning_skips_compression_and_packing_rejects_source_changes() { + let temp = tempfile::tempdir().unwrap(); + fs::write(temp.path().join("SKILL.md"), b"before").unwrap(); + let package = scan_package( + temp.path(), + "user::fixture", + &None, + Arc::new(AtomicBool::new(false)), + ) + .unwrap(); + assert!( + !package.path.exists(), + "cache checking must not create a compressed archive" + ); + fs::write(temp.path().join("SKILL.md"), b"after!").unwrap(); + let error = build_archive(package) + .err() + .expect("changed source must fail"); + assert!( + error.to_string().contains("changed while preparing"), + "{error:#}" + ); +} + +#[cfg(all(unix, feature = "workspace-runtime"))] +mod posix { + use super::*; + use crate::workspace::{LocalWorkspaceFs, LocalWorkspaceShell}; + use openbitfun_runtime_ports::{WorkspaceCommandResult, WorkspaceDirEntry}; + use std::{ + os::unix::fs::{symlink, PermissionsExt}, + sync::{ + atomic::{AtomicUsize, Ordering}, + Mutex, + }, + }; + + struct CountingShell { + inner: LocalWorkspaceShell, + calls: AtomicUsize, + } + + #[async_trait::async_trait] + impl WorkspaceShell for CountingShell { + async fn exec_with_options( + &self, + command: &str, + options: WorkspaceCommandOptions, + ) -> Result { + self.calls.fetch_add(1, Ordering::Relaxed); + self.inner.exec_with_options(command, options).await + } + } + + fn write(root: &Path, name: &str, bytes: &[u8]) { + let path = root.join(name); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, bytes).unwrap(); + } + + #[tokio::test] + async fn deploys_complete_package_in_two_commands_and_preserves_executables() { + let temp = tempfile::tempdir().unwrap(); + let local = temp.path().join("local user's skill"); + let remote = temp.path().join("remote workspace"); + fs::create_dir(&remote).unwrap(); + write( + &local, + "SKILL.md", + b"Run scripts/hello.sh, read references/guide.md", + ); + write( + &local, + "references/guide 'one' \u{4e2d}.md", + b"full reference\n", + ); + write(&local, "templates/example.bin", &[0, 255, 1, 2]); + write( + &local, + "scripts/hello.sh", + b"#!/bin/sh\nprintf '%s' remote-script-ok\n", + ); + fs::set_permissions( + local.join("scripts/hello.sh"), + fs::Permissions::from_mode(0o751), + ) + .unwrap(); + for number in 0..200 { + write(&local, &format!("many/{number}.txt"), b"tiny file"); + } + let shell = CountingShell { + inner: LocalWorkspaceShell::new(remote.to_string_lossy().into_owned()), + calls: AtomicUsize::new(0), + }; + let phases = Arc::new(Mutex::new(Vec::new())); + let observed = phases.clone(); + let result = sync_local_skill_package( + &local, + remote.join("cache 'target'\n$literal").to_str().unwrap(), + remote.to_str().unwrap(), + "user::test", + &LocalWorkspaceFs, + &shell, + Some(Arc::new(move |progress| { + observed.lock().unwrap().push(progress.phase) + })), + ) + .await + .unwrap(); + assert!(!result.reused); + assert_eq!(result.file_count, 204); + assert_eq!( + shell.calls.load(Ordering::Relaxed), + 2, + "file count must not multiply SSH commands" + ); + assert_eq!( + fs::read(Path::new(&result.path).join("references/guide 'one' \u{4e2d}.md")).unwrap(), + b"full reference\n" + ); + assert_eq!( + fs::read(Path::new(&result.path).join("templates/example.bin")).unwrap(), + [0, 255, 1, 2] + ); + let (output, _, status) = shell + .exec("e(&format!("{}/scripts/hello.sh", result.path)), None) + .await + .unwrap(); + assert_eq!(status, 0); + assert_eq!(output, "remote-script-ok"); + assert_eq!( + fs::metadata(Path::new(&result.path).join("scripts/hello.sh")) + .unwrap() + .permissions() + .mode() + & 0o777, + 0o751 + ); + let phases = phases.lock().unwrap(); + for phase in [ + SkillPackageSyncPhase::Scanning, + SkillPackageSyncPhase::Checking, + SkillPackageSyncPhase::Uploading, + SkillPackageSyncPhase::Extracting, + SkillPackageSyncPhase::Complete, + ] { + assert!(phases.contains(&phase)); + } + } + + #[tokio::test] + async fn cache_reuse_verifies_bytes_modes_missing_and_extra_files_then_tracks_local_changes() { + let temp = tempfile::tempdir().unwrap(); + let local = temp.path().join("local"); + write(&local, "SKILL.md", b"skill body"); + write(&local, "references/guide.md", b"original"); + write(&local, "templates/deleted.txt", b"delete locally later"); + let cache = temp.path().join("cache").to_string_lossy().into_owned(); + let shell = LocalWorkspaceShell::new(temp.path().to_string_lossy().into_owned()); + let deploy = || { + sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None, + ) + }; + let original = deploy().await.unwrap(); + assert!(!original.reused); + let again = deploy().await.unwrap(); + assert!(again.reused); + assert_eq!(again.path, original.path); + let guide = Path::new(&original.path).join("references/guide.md"); + fs::write(&guide, b"tampered").unwrap(); + assert!(!deploy().await.unwrap().reused); + assert_eq!(fs::read(&guide).unwrap(), b"original"); + fs::remove_file(&guide).unwrap(); + assert!(!deploy().await.unwrap().reused); + assert!(guide.is_file()); + let extra = Path::new(&original.path).join("extra.txt"); + fs::write(&extra, b"not in source").unwrap(); + assert!(!deploy().await.unwrap().reused); + assert!(!extra.exists()); + fs::set_permissions(&guide, fs::Permissions::from_mode(0o700)).unwrap(); + assert!(!deploy().await.unwrap().reused); + write(&local, "references/guide.md", b"updated locally"); + fs::remove_file(local.join("templates/deleted.txt")).unwrap(); + let updated = deploy().await.unwrap(); + assert!(!updated.reused); + assert_ne!(updated.content_hash, original.content_hash); + assert_eq!( + fs::read(Path::new(&updated.path).join("references/guide.md")).unwrap(), + b"updated locally" + ); + assert!(!Path::new(&updated.path) + .join("templates/deleted.txt") + .exists()); + assert_eq!( + fs::read(&guide).unwrap(), + b"original", + "older content-addressed deployments stay intact" + ); + } + + #[tokio::test] + async fn accepts_symlinked_package_root_but_rejects_nested_links_without_success() { + let temp = tempfile::tempdir().unwrap(); + let local = temp.path().join("local"); + write(&local, "SKILL.md", b"skill body"); + let link = temp.path().join("linked-root"); + symlink(&local, &link).unwrap(); + let cache = temp.path().join("cache").to_string_lossy().into_owned(); + let shell = LocalWorkspaceShell::new(temp.path().to_string_lossy().into_owned()); + let first = sync_local_skill_package( + &link, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None, + ) + .await + .unwrap(); + assert!(!first.reused); + assert!( + sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None + ) + .await + .unwrap() + .reused + ); + symlink(temp.path(), local.join("escaping-link")).unwrap(); + let error = sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None, + ) + .await + .unwrap_err(); + assert!( + error.to_string().contains("nested symbolic links"), + "{error:#}" + ); + assert!(Path::new(&first.path).join("SKILL.md").is_file()); + } + + #[tokio::test] + async fn supports_symlinked_workspace_ancestors_but_rejects_managed_cache_links() { + let temp = tempfile::tempdir().unwrap(); + let local = temp.path().join("local"); + write(&local, "SKILL.md", b"skill body"); + write(&local, "references/guide.md", b"deployed reference"); + let real = temp.path().join("real"); + fs::create_dir_all(real.join("workspace")).unwrap(); + let alias = temp.path().join("alias"); + symlink(&real, &alias).unwrap(); + let workspace = alias.join("workspace"); + let cache = workspace.join(".openbitfun/cache/user-skills"); + let shell = LocalWorkspaceShell::new(workspace.to_string_lossy().into_owned()); + let deploy = || { + sync_local_skill_package( + &local, + cache.to_str().unwrap(), + workspace.to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None, + ) + }; + let first = deploy().await.unwrap(); + assert!(first.path.starts_with(cache.to_str().unwrap())); + assert_eq!( + fs::read(Path::new(&first.path).join("references/guide.md")).unwrap(), + b"deployed reference" + ); + assert!(deploy().await.unwrap().reused); + + // Resolving the trusted workspace must not authorize a repository's + // .openbitfun/cache link to escape to an unrelated directory. + fs::remove_dir_all(workspace.join(".openbitfun")).unwrap(); + fs::create_dir(workspace.join(".openbitfun")).unwrap(); + let outside = temp.path().join("outside"); + fs::create_dir(&outside).unwrap(); + write(&outside, "sentinel", b"untouched"); + symlink(&outside, workspace.join(".openbitfun/cache")).unwrap(); + let error = deploy().await.unwrap_err(); + assert!( + error.to_string().contains("without symbolic links"), + "{error:#}" + ); + assert_eq!(fs::read(outside.join("sentinel")).unwrap(), b"untouched"); + assert!(!outside.join("user-skills").exists()); + } + + struct NoPython; + #[async_trait::async_trait] + impl WorkspaceShell for NoPython { + async fn exec_with_options( + &self, + _: &str, + _: WorkspaceCommandOptions, + ) -> Result { + Ok(WorkspaceCommandResult { + stdout: String::new(), + stderr: "Full user Skill sync requires Python 3 on the remote target".into(), + exit_code: 127, + interrupted: false, + timed_out: false, + }) + } + } + + struct NoUploads; + #[async_trait::async_trait] + impl WorkspaceFileSystem for NoUploads { + async fn read_file(&self, _: &str) -> Result> { + bail!("unexpected read") + } + async fn read_file_text(&self, _: &str) -> Result { + bail!("unexpected read") + } + async fn write_file(&self, _: &str, _: &[u8]) -> Result<()> { + bail!("unexpected write") + } + async fn exists(&self, _: &str) -> Result { + bail!("unexpected exists") + } + async fn is_file(&self, _: &str) -> Result { + bail!("unexpected stat") + } + async fn is_dir(&self, _: &str) -> Result { + bail!("unexpected stat") + } + async fn read_dir(&self, _: &str) -> Result> { + bail!("unexpected listing") + } + } + + #[tokio::test] + async fn unsupported_targets_and_failed_uploads_never_return_a_loaded_package() { + let temp = tempfile::tempdir().unwrap(); + let local = temp.path().join("local"); + write(&local, "SKILL.md", b"original"); + let cache = temp.path().join("cache").to_string_lossy().into_owned(); + let error = sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &NoUploads, + &NoPython, + None, + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("requires Python 3")); + assert!(!Path::new(&cache).exists()); + let shell = LocalWorkspaceShell::new(temp.path().to_string_lossy().into_owned()); + let original = sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &LocalWorkspaceFs, + &shell, + None, + ) + .await + .unwrap(); + write(&local, "SKILL.md", b"updated locally"); + let error = sync_local_skill_package( + &local, + &cache, + temp.path().to_str().unwrap(), + "user::fixture", + &NoUploads, + &shell, + None, + ) + .await + .unwrap_err(); + assert!(error + .to_string() + .contains("streaming Skill package uploads")); + assert_eq!( + fs::read(Path::new(&original.path).join("SKILL.md")).unwrap(), + b"original" + ); + } +} diff --git a/src/crates/services/services-integrations/AGENTS.md b/src/crates/services/services-integrations/AGENTS.md index ee2fe4fb37..384fa2ac79 100644 --- a/src/crates/services/services-integrations/AGENTS.md +++ b/src/crates/services/services-integrations/AGENTS.md @@ -179,6 +179,13 @@ advertise a small handle limit and are included in the existing CI the same command with the filter ending in `workspace_sftp::openssh_real_files_over_loopback_ssh -- --ignored`. +For full user Skill deployment, resource reads, executable scripts, content reuse +and stale-copy repair over the same real loopback SSH / OpenSSH SFTP fixture, +run that command with the filter ending in +`workspace_sftp::openssh_skill_package_sync_reads_executes_and_repairs_remote_resources -- --ignored`. +The fixture executes a POSIX shell through SSH against test-owned directories; +it does not establish external-host, ProxyJump, Docker or Windows-client coverage. + For directory symlink browsing and workspace-path validation, use the same package/features with `--lib directory_symlink_browsing -- --include-ignored` and `OPENBITFUN_TEST_SFTP_SERVER` set. This runs real OpenSSH SFTP over loopback diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index b3e73a73cf..9230783df5 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -514,6 +514,7 @@ ssh_config = ["dep:ssh_config"] [dev-dependencies] tempfile = { workspace = true } +openbitfun-services-core = { path = "../services-core", features = ["skill-package-sync"] } tokio = { workspace = true, features = ["io-util", "macros", "net", "rt", "test-util"] } # Used by contract tests that implement the `EventEmitter` port to observe # emitted product events. diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs index 90c0ff6570..f2b5a26029 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs @@ -227,6 +227,31 @@ impl russh::server::Handler for SshServer { self.channels.insert(channel.id(), channel); Ok(true) } + #[cfg(unix)] + async fn exec_request( + &mut self, + channel: russh::ChannelId, + command: &[u8], + session: &mut russh::server::Session, + ) -> Result<(), Self::Error> { + // This loopback fixture runs commands only against test-owned paths. + // Keep the SSH execution path real while using the local POSIX shell. + session.channel_success(channel); + let output = process_manager::create_tokio_command("sh") + .arg("-c") + .arg(std::str::from_utf8(command).unwrap()) + .stdin(Stdio::null()) + .kill_on_drop(true) + .output() + .await + .unwrap(); + session.data(channel, russh::CryptoVec::from_slice(&output.stdout)); + session.extended_data(channel, 1, russh::CryptoVec::from_slice(&output.stderr)); + session.exit_status_request(channel, output.status.code().unwrap_or(1) as u32); + session.eof(channel); + session.close(channel); + Ok(()) + } async fn subsystem_request( &mut self, id: russh::ChannelId, @@ -1028,3 +1053,159 @@ async fn openssh_streaming_upload_create_is_exclusive_and_closes_handle() { assert_eq!(data.len(), chunk.len() * 32); assert!(data.iter().all(|byte| *byte == 29)); } + +#[cfg(unix)] +#[tokio::test] +#[ignore = "Set OPENBITFUN_TEST_SFTP_SERVER to the OpenSSH sftp-server executable"] +async fn openssh_skill_package_sync_reads_executes_and_repairs_remote_resources() { + use crate::remote_ssh::workspace_services::{RemoteWorkspaceFs, RemoteWorkspaceShell}; + use openbitfun_runtime_ports::{WorkspaceFileSystem, WorkspaceShell}; + use openbitfun_services_core::skill_package_sync::sync_local_skill_package; + use std::os::unix::fs::PermissionsExt; + + let executable = std::env::var("OPENBITFUN_TEST_SFTP_SERVER") + .expect("OpenSSH sftp-server executable required"); + let fixture = Fixture::with_openssh(Some(executable)).await; + let local_root = fixture._dir.path().join("local user skill"); + let remote_root = fixture._dir.path().join("remote workspace"); + let cache_root = remote_root.join(".skill-cache"); + for directory in ["scripts", "references", "templates"] { + std::fs::create_dir_all(local_root.join(directory)).unwrap(); + } + std::fs::create_dir(&remote_root).unwrap(); + std::fs::write( + local_root.join("SKILL.md"), + "Read references/guide.md and run scripts/hello.py.", + ) + .unwrap(); + std::fs::write(local_root.join("references/guide.md"), "remote reference").unwrap(); + std::fs::write(local_root.join("templates/binary.dat"), [0, 255, 1, 13, 10]).unwrap(); + std::fs::write( + local_root.join("references/quoted ' Unicode 你好.txt"), + "quoted path", + ) + .unwrap(); + let script = local_root.join("scripts/hello.py"); + std::fs::write(&script, "#!/usr/bin/env python3\nfrom pathlib import Path\nprint(Path(__file__).resolve().parents[1].joinpath('references/guide.md').read_text())\nprint(Path.cwd())\n").unwrap(); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let file_service = crate::remote_ssh::RemoteFileService::new(Arc::new( + tokio::sync::RwLock::new(Some(fixture.manager.as_ref().clone())), + )); + let fs = RemoteWorkspaceFs::new("sftp-test".into(), file_service); + let shell = RemoteWorkspaceShell::new( + "sftp-test".into(), + fixture.manager.as_ref().clone(), + remote_root.to_str().unwrap().into(), + ); + let cache = cache_root.to_str().unwrap(); + let first = sync_local_skill_package( + &local_root, + cache, + remote_root.to_str().unwrap(), + "user::test::hello", + &fs, + &shell, + None, + ) + .await + .unwrap(); + assert!(!first.reused); + assert!(first.path.starts_with(cache)); + assert!(!first.path.contains("local user skill")); + let remote_reference = format!("{}/references/guide.md", first.path); + let remote_template = format!("{}/templates/binary.dat", first.path); + assert_eq!( + fs.read_file_text(&remote_reference).await.unwrap(), + "remote reference" + ); + assert_eq!( + fs.read_file(&remote_template).await.unwrap(), + [0, 255, 1, 13, 10] + ); + assert_eq!( + fs.read_file_text(&format!( + "{}/references/quoted ' Unicode 你好.txt", + first.path + )) + .await + .unwrap(), + "quoted path" + ); + let script_command = + crate::remote_ssh::shell::quote_arg(&format!("{}/scripts/hello.py", first.path)); + let (stdout, stderr, status) = shell.exec(&script_command, Some(30_000)).await.unwrap(); + assert_eq!(status, 0, "{stderr}"); + assert_eq!( + stdout, + format!( + "remote reference\n{}\n", + remote_root.canonicalize().unwrap().display() + ) + ); + + let unchanged = sync_local_skill_package( + &local_root, + cache, + remote_root.to_str().unwrap(), + "user::test::hello", + &fs, + &shell, + None, + ) + .await + .unwrap(); + assert!(unchanged.reused); + assert_eq!(unchanged.path, first.path); + fs.write_file(&remote_reference, b"remote corruption") + .await + .unwrap(); + let repaired = sync_local_skill_package( + &local_root, + cache, + remote_root.to_str().unwrap(), + "user::test::hello", + &fs, + &shell, + None, + ) + .await + .unwrap(); + assert!(!repaired.reused); + assert_eq!( + fs.read_file_text(&format!("{}/references/guide.md", repaired.path)) + .await + .unwrap(), + "remote reference" + ); + + std::fs::write( + local_root.join("references/guide.md"), + "updated local reference", + ) + .unwrap(); + std::fs::remove_file(local_root.join("templates/binary.dat")).unwrap(); + let updated = sync_local_skill_package( + &local_root, + cache, + remote_root.to_str().unwrap(), + "user::test::hello", + &fs, + &shell, + None, + ) + .await + .unwrap(); + assert!(!updated.reused); + assert_ne!(updated.content_hash, first.content_hash); + assert_eq!( + fs.read_file_text(&format!("{}/references/guide.md", updated.path)) + .await + .unwrap(), + "updated local reference" + ); + assert!(!fs + .exists(&format!("{}/templates/binary.dat", updated.path)) + .await + .unwrap()); +} diff --git a/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.test.tsx b/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.test.tsx new file mode 100644 index 0000000000..7f410ff569 --- /dev/null +++ b/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.test.tsx @@ -0,0 +1,108 @@ +// @vitest-environment jsdom +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; +import RemoteUserSkillsSection from './RemoteUserSkillsSection'; + +const mocks = vi.hoisted(() => ({ get: vi.fn(), set: vi.fn(), error: vi.fn() })); +vi.mock('@/infrastructure/config/services/ConfigManager', () => ({ configManager: { + hasCachedConfig: () => false, getCachedConfig: () => undefined, + getConfig: mocks.get, setConfig: mocks.set, +} })); +vi.mock('@openbitfun/ui', async original => ({ + ...await original(), + Select: ({ value, disabled, options, onValueChange }: { + value: string; disabled: boolean; options: Array<{ value: string; label: string }>; + onValueChange: (value: string) => void; + }) => , +})); +vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) })); +vi.mock('@/shared/notification-system', () => ({ notificationService: { error: mocks.error } })); + +describe('remote user Skill settings', () => { + let root: Root; + let container: HTMLDivElement; + const select = () => container.querySelector('select')!; + const choose = async (value: string) => act(async () => { + select().value = value; + select().dispatchEvent(new Event('change', { bubbles: true })); + }); + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true; + activateSurface('local'); + vi.clearAllMocks(); + mocks.get.mockReset().mockResolvedValue({ remote_user_skills: 'skill_md_only' }); + mocks.set.mockReset().mockResolvedValue(undefined); + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); + }); + afterEach(() => { act(() => root.unmount()); container.remove(); activateSurface('local'); }); + + it('preserves the markdown-only default and saves each choice without replacing other workspace settings', async () => { + await act(async () => root.render()); + expect(select().value).toBe('skill_md_only'); + expect(container.textContent).toContain('remoteUserSkills.skillMdOnlyDescription'); + await choose('full'); + expect(mocks.set).toHaveBeenLastCalledWith('workspace.remote_user_skills', 'full'); + expect(container.textContent).toContain('remoteUserSkills.syncHint'); + await choose('disabled'); + expect(mocks.set).toHaveBeenLastCalledWith('workspace.remote_user_skills', 'disabled'); + expect(container.textContent).not.toContain('remoteUserSkills.syncHint'); + await choose('skill_md_only'); + expect(mocks.set).toHaveBeenLastCalledWith('workspace.remote_user_skills', 'skill_md_only'); + expect(select().value).toBe('skill_md_only'); + }); + + it('locks an older host or unknown future mode instead of writing unsupported configuration', async () => { + mocks.get.mockResolvedValue({ remote_user_skills: 'future_mode' }); + await act(async () => root.render()); + expect(select().disabled).toBe(true); + expect(container.textContent).toContain('remoteUserSkills.unsupported'); + expect(mocks.set).not.toHaveBeenCalled(); + }); + + it('keeps older hosts without the field explicitly unsupported', async () => { + mocks.get.mockResolvedValue({}); + await act(async () => root.render()); + expect(select().disabled).toBe(true); + expect(container.textContent).toContain('remoteUserSkills.unsupported'); + expect(mocks.set).not.toHaveBeenCalled(); + }); + + it('preserves the persisted mode after a failed write', async () => { + mocks.set.mockRejectedValue(new Error('Host unavailable')); + await act(async () => root.render()); + await choose('full'); + expect(select().value).toBe('skill_md_only'); + expect(mocks.error).toHaveBeenCalledWith('remoteUserSkills.saveFailed'); + expect(select().disabled).toBe(false); + }); + + it('locks a failed read and retries without writing a default', async () => { + mocks.get.mockRejectedValueOnce(new Error('Host unavailable')); + await act(async () => root.render()); + expect(select()).toBeNull(); + expect(container.textContent).toContain('remoteUserSkills.loadFailed'); + expect(mocks.set).not.toHaveBeenCalled(); + await act(async () => container.querySelector('button')!.click()); + expect(select().value).toBe('skill_md_only'); + expect(mocks.set).not.toHaveBeenCalled(); + }); + + it('ignores a completed write from the previously selected device', async () => { + let finishSave!: () => void; + mocks.set.mockImplementation(() => new Promise(resolve => { finishSave = resolve; })); + await act(async () => root.render()); + await choose('full'); + expect(select().disabled).toBe(true); + activateSurface('peer-device'); + await act(async () => finishSave()); + expect(select().value).toBe('skill_md_only'); + expect(mocks.error).not.toHaveBeenCalled(); + }); +}); diff --git a/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.tsx b/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.tsx new file mode 100644 index 0000000000..c0884d68c9 --- /dev/null +++ b/src/web-ui/src/app/scenes/settings/pages/development/RemoteUserSkillsSection.tsx @@ -0,0 +1,98 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { Select } from '@openbitfun/ui'; +import { + ConfigLoadingState, ConfigMessage, ConfigPageRow, ConfigPageSection, ConfigRetryState, +} from '@/infrastructure/config/components/common'; +import { useConfigSeed } from '@/infrastructure/config/hooks/useConfigSeed'; +import { configManager } from '@/infrastructure/config/services/ConfigManager'; +import type { RemoteUserSkillsMode, WorkspaceConfig } from '@/infrastructure/config/types'; +import { useI18n } from '@/infrastructure/i18n'; +import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { notificationService } from '@/shared/notification-system'; +import { createLogger } from '@/shared/utils/logger'; +import { useSettingsSectionAnchor } from '../shared/useSettingsSectionAnchor'; + +const log = createLogger('RemoteUserSkillsSection'); + +function supportedMode(value: unknown): RemoteUserSkillsMode | undefined { + return value === 'skill_md_only' || value === 'disabled' || value === 'full' ? value : undefined; +} + +export default function RemoteUserSkillsSection() { + const { t } = useI18n('settings/runtime'); + const seed = useConfigSeed(['workspace']); + const [mode, setMode] = useState(() => supportedMode(seed.get>('workspace', {}).remote_user_skills)); + const [loading, setLoading] = useState(!seed.loaded); + const [loadFailed, setLoadFailed] = useState(false); + const [saving, setSaving] = useState(false); + const saveInFlight = useRef(false); + const sectionAnchor = useSettingsSectionAnchor('remote-user-skills', !loading); + + const load = useCallback(async () => { + const scope = getActiveSurfaceScope(); + setLoading(true); + setLoadFailed(false); + try { + const workspace = await configManager.getConfig('workspace'); + if (scope.isCurrent()) setMode(supportedMode(workspace.remote_user_skills)); + } catch (reason) { + if (scope.isCurrent()) { + log.error('Failed to load remote user Skill settings', reason); + setLoadFailed(true); + } + } finally { + if (scope.isCurrent()) setLoading(false); + } + }, []); + + useEffect(() => { if (!seed.loaded) void load(); }, [load, seed.loaded]); + + const save = async (next: RemoteUserSkillsMode) => { + if (saveInFlight.current || mode === undefined || next === mode) return; + const scope = getActiveSurfaceScope(); + saveInFlight.current = true; + setSaving(true); + try { + await configManager.setConfig('workspace.remote_user_skills', next); + if (scope.isCurrent()) setMode(next); + } catch (reason) { + if (scope.isCurrent()) { + log.error('Failed to save remote user Skill settings', reason); + notificationService.error(t('remoteUserSkills.saveFailed')); + } + } finally { + saveInFlight.current = false; + if (scope.isCurrent()) setSaving(false); + } + }; + + const description = mode === 'disabled' ? t('remoteUserSkills.disabledDescription') + : mode === 'full' ? t('remoteUserSkills.fullDescription') + : mode === 'skill_md_only' ? t('remoteUserSkills.skillMdOnlyDescription') + : t('remoteUserSkills.unsupported'); + + return ( + + {loading ? + : loadFailed ? void load()} /> + : <> + +