From d219832b9940387b57aede9b054846021458bc71 Mon Sep 17 00:00:00 2001 From: hanlinyu1030 Date: Tue, 11 Aug 2026 15:49:52 +0800 Subject: [PATCH 1/3] test(flow-chat): mock updateSessionMetadata in SessionModule test --- .../flow_chat/services/flow-chat-manager/SessionModule.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/SessionModule.test.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/SessionModule.test.ts index 38442ed547..1f7c16bd5c 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/SessionModule.test.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/SessionModule.test.ts @@ -47,6 +47,7 @@ const persistenceMocks = vi.hoisted(() => ({ touchSessionActivity: vi.fn(), cleanupSaveState: vi.fn(), cleanupSessionBuffers: vi.fn(), + updateSessionMetadata: vi.fn().mockResolvedValue(undefined), })); const stateMachineMocks = vi.hoisted(() => ({ @@ -101,6 +102,7 @@ vi.mock('@/infrastructure/services/business/workspaceManager', () => ({ vi.mock('./PersistenceModule', () => ({ touchSessionActivity: persistenceMocks.touchSessionActivity, cleanupSaveState: persistenceMocks.cleanupSaveState, + updateSessionMetadata: persistenceMocks.updateSessionMetadata, })); vi.mock('./TextChunkModule', () => ({ From 5c3ad3512f7f4ba3ca6c2cc79f380610dfd9e552 Mon Sep 17 00:00:00 2001 From: hanlinyu1030 Date: Mon, 24 Aug 2026 16:27:16 +0800 Subject: [PATCH 2/3] feat(skills): native Rust download + proxy-aware listing and offset pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace npx/bun skill install with pure-Rust GitHub tarball download + gzip/tar extract + atomic install. Works on HarmonyOS (where the bundled bun binary V8-aborts, exit code -1) without Node/npx/bun. Locates the skill by SKILL.md frontmatter `name` (== skills.sh slug) so repos whose folder name differs from the slug (vercel-labs/agent-skills) and nested layouts both install. - Listing: per-call build_market_client reads BitFun ProxyConfig live (HarmonyOS connectivity — otherwise skills.sh direct times out) + UA; real offset pagination (over-fetch+slice, no grow-limit); blocks on description fill so cards appear with descriptions. No aggressive per-request timeout (skills.sh is international; a slow-but-reachable direct connection can take 8-20s and a tight connect_timeout would cut it). Each per-skill description fetch is bounded by the 4s per-page timeout. - New get_skill_descriptions command + fetch_descriptions_for_ids helper. - Filter reqwest/rustls/rustls_platform_verifier/hyper to Warn (stop per-connection DEBUG log flood). Known limit: per-page listing ~4-8s cold is inherent (skills.sh legacy search has no description field; v1 API needs Vercel OIDC auth BitFun cannot obtain; per-skill HTML scrape is the only no-auth source). Process cache speeds repeat views. --- src/apps/desktop/src/api/mod.rs | 1 + .../src/api/remote_workspace_policy.rs | 4 + src/apps/desktop/src/api/skill_api.rs | 339 ++++++---- .../src/api/skill_market_downloader.rs | 632 ++++++++++++++++++ src/apps/desktop/src/lib.rs | 1 + src/apps/desktop/src/logging.rs | 7 + .../skills/hooks/useSkillMarket.test.tsx | 3 + .../app/scenes/skills/hooks/useSkillMarket.ts | 48 +- .../api/service-api/ConfigAPI.ts | 20 +- 9 files changed, 912 insertions(+), 143 deletions(-) create mode 100644 src/apps/desktop/src/api/skill_market_downloader.rs diff --git a/src/apps/desktop/src/api/mod.rs b/src/apps/desktop/src/api/mod.rs index 437954a58b..b6eb6b5b12 100644 --- a/src/apps/desktop/src/api/mod.rs +++ b/src/apps/desktop/src/api/mod.rs @@ -52,6 +52,7 @@ pub mod search_api; pub mod session_api; pub mod session_storage_path; pub mod skill_api; +pub mod skill_market_downloader; pub mod snapshot_service; pub mod speech_api; pub mod ssh_api; diff --git a/src/apps/desktop/src/api/remote_workspace_policy.rs b/src/apps/desktop/src/api/remote_workspace_policy.rs index f1531e116a..2055feea2b 100644 --- a/src/apps/desktop/src/api/remote_workspace_policy.rs +++ b/src/apps/desktop/src/api/remote_workspace_policy.rs @@ -778,6 +778,10 @@ pub const REMOTE_WORKSPACE_COMMAND_POLICIES: &[(&str, RemoteWorkspacePolicy)] = RemoteWorkspacePolicy::LegacyUnaudited, ), ("get_skill_configs", RemoteWorkspacePolicy::LegacyUnaudited), + ( + "get_skill_descriptions", + RemoteWorkspacePolicy::WorkspaceAgnostic, + ), ( "get_snapshot_sessions", RemoteWorkspacePolicy::LegacyUnaudited, diff --git a/src/apps/desktop/src/api/skill_api.rs b/src/apps/desktop/src/api/skill_api.rs index a0d8e05bec..b3c393b1ed 100644 --- a/src/apps/desktop/src/api/skill_api.rs +++ b/src/apps/desktop/src/api/skill_api.rs @@ -1,6 +1,7 @@ //! Skill Management API use crate::api::app_state::RemoteWorkspace; +use crate::api::skill_market_downloader::install_skill_from_market; use log::info; use regex::Regex; use reqwest::Client; @@ -8,7 +9,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; -use std::process::Stdio; use std::sync::OnceLock; use tauri::State; use tokio::sync::RwLock; @@ -31,22 +31,80 @@ use bitfun_core::infrastructure::get_path_manager_arc; use bitfun_core::service::config::agent_profile_project_store::{ deserialize_project_agent_profiles_document, serialize_project_agent_profiles_document, }; +use bitfun_core::service::config::types::ProxyConfig; use bitfun_core::service::remote_ssh::workspace_state::is_remote_path; use bitfun_core::service::remote_ssh::{get_remote_workspace_manager, RemoteWorkspaceEntry}; -use bitfun_core::service::runtime::RuntimeManager; -use bitfun_core::util::process_manager; const SKILLS_SEARCH_API_BASE: &str = "https://skills.sh"; const DEFAULT_MARKET_QUERY: &str = "skill"; const DEFAULT_MARKET_LIMIT: u32 = 12; const MAX_MARKET_LIMIT: u32 = 500; -const MAX_OUTPUT_PREVIEW_CHARS: usize = 2000; const MARKET_DESC_FETCH_TIMEOUT_SECS: u64 = 4; -const MARKET_DESC_FETCH_CONCURRENCY: usize = 6; +const MARKET_DESC_FETCH_CONCURRENCY: usize = 12; const MARKET_DESC_MAX_LEN: usize = 220; +const MARKET_DESC_FETCH_DEADLINE_SECS: u64 = 15; static MARKET_DESCRIPTION_CACHE: OnceLock>> = OnceLock::new(); +/// Build a per-call HTTP client for the skill market listing/description +/// paths. Uses BitFun's configured `ProxyConfig` when enabled (read live so +/// changing the AI proxy takes effect without restart), falling back to the +/// standard HTTPS_PROXY/HTTP_PROXY/ALL_PROXY env vars (reqwest default), then +/// direct. No aggressive connect/overall timeout is set here: skills.sh is an +/// international domain and a slow-but-reachable direct connection can take +/// 8-20s; a tight connect_timeout would cut it off and turn "works" into +/// "timed out". The per-skill description fetch keeps its own per-page bound +/// in fetch_description_from_skill_page. +fn build_market_client(proxy: Option<&ProxyConfig>) -> Result { + let mut builder = Client::builder() + .user_agent(concat!("BitFun/", env!("CARGO_PKG_VERSION"))); + if let Some(proxy_url) = resolve_proxy_url(proxy) { + match reqwest::Proxy::all(&proxy_url) { + Ok(p) => builder = builder.proxy(p), + Err(e) => log::warn!("Failed to apply skill market proxy: {}", e), + } + } + builder + .build() + .map_err(|e| format!("Failed to build HTTP client: {}", e)) +} + +/// Resolve the proxy URL: BitFun's `ProxyConfig` if enabled, else startup env. +fn resolve_proxy_url(proxy: Option<&ProxyConfig>) -> Option { + if let Some(p) = proxy { + if p.enabled { + let url = p.url.trim(); + if !url.is_empty() { + return Some(normalize_proxy_url(url)); + } + } + } + read_startup_proxy_env() +} + +/// Prefix `http://` to bare `host:port` proxy URLs (mirrors the AI adapter). +fn normalize_proxy_url(url: &str) -> String { + if url.contains("://") { + url.to_string() + } else { + format!("http://{}", url) + } +} + +/// Read a proxy URL from the standard env vars (HTTPS_PROXY / HTTP_PROXY / +/// ALL_PROXY, case-insensitive). Returns the first one set. +fn read_startup_proxy_env() -> Option { + for key in ["HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"] { + if let Ok(value) = std::env::var(key) { + let trimmed = value.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + None +} + fn can_delete_owned_skill(source_id: &str, source_slot: &str, is_builtin: bool) -> bool { if is_builtin { return false; @@ -82,6 +140,8 @@ pub struct SkillValidationResult { pub struct SkillMarketListRequest { pub query: Option, pub limit: Option, + #[serde(default)] + pub offset: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -89,6 +149,8 @@ pub struct SkillMarketListRequest { pub struct SkillMarketSearchRequest { pub query: String, pub limit: Option, + #[serde(default)] + pub offset: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1075,7 +1137,7 @@ mod skill_delete_policy_tests { #[tauri::command] pub async fn list_skill_market( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: SkillMarketListRequest, ) -> Result, String> { let query = request @@ -1085,12 +1147,14 @@ pub async fn list_skill_market( .filter(|v| !v.is_empty()) .unwrap_or(DEFAULT_MARKET_QUERY); let limit = normalize_market_limit(request.limit); - fetch_skill_market(query, limit).await + let offset = normalize_market_offset(request.offset); + let proxy = configured_proxy_for_skills(&state).await?; + fetch_skill_market(query, limit, offset, proxy.as_ref()).await } #[tauri::command] pub async fn search_skill_market( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: SkillMarketSearchRequest, ) -> Result, String> { let query = request.query.trim(); @@ -1098,12 +1162,47 @@ pub async fn search_skill_market( return Ok(Vec::new()); } let limit = normalize_market_limit(request.limit); - fetch_skill_market(query, limit).await + let offset = normalize_market_offset(request.offset); + let proxy = configured_proxy_for_skills(&state).await?; + fetch_skill_market(query, limit, offset, proxy.as_ref()).await +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SkillDescriptionRequest { + pub ids: Vec, +} + +/// Lazily fetch skill descriptions for a set of skill ids. The listing path no +/// longer blocks on description scraping; the frontend calls this after the +/// list renders so the marketplace appears immediately and descriptions fill +/// in progressively. Returns whatever could be fetched (including from the +/// process-level cache) when the overall deadline elapses. +#[tauri::command] +pub async fn get_skill_descriptions( + state: State<'_, AppState>, + request: SkillDescriptionRequest, +) -> Result, String> { + if request.ids.is_empty() { + return Ok(HashMap::new()); + } + + let api_base = + std::env::var("SKILLS_API_URL").unwrap_or_else(|_| SKILLS_SEARCH_API_BASE.into()); + let base_url = api_base.trim_end_matches('/').to_string(); + let proxy = configured_proxy_for_skills(&state).await?; + + let fetched = timeout( + Duration::from_secs(MARKET_DESC_FETCH_DEADLINE_SECS), + fetch_descriptions_for_ids(&request.ids, &base_url, proxy.as_ref()), + ) + .await; + Ok(fetched.unwrap_or_default()) } #[tauri::command] pub async fn download_skill_market( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: SkillMarketDownloadRequest, ) -> Result { let package = request.package.trim().to_string(); @@ -1134,64 +1233,19 @@ pub async fn download_skill_market( .map(|skill| skill.name) .collect(); - let runtime_manager = RuntimeManager::new() - .map_err(|e| format!("Failed to initialize runtime manager: {}", e))?; - let resolved_npx = runtime_manager.resolve_command("npx").ok_or_else(|| { - "Command 'npx' is not available. Install Node.js or configure BitFun runtimes.".to_string() - })?; - - let mut command = process_manager::create_tokio_command(&resolved_npx.command); - command - .arg("-y") - .arg("skills") - .arg("add") - .arg(&package) - .arg("-y") - .arg("-a") - .arg("universal"); - - if level == SkillLocation::User { - command.arg("-g"); - } - - if let Some(path) = workspace_path.as_ref() { - command.current_dir(path); - } - - let current_path = std::env::var("PATH").ok(); - if let Some(merged_path) = runtime_manager.merged_path_env(current_path.as_deref()) { - command.env("PATH", &merged_path); - #[cfg(windows)] - { - command.env("Path", &merged_path); - } - } - - command.stdout(Stdio::piped()); - command.stderr(Stdio::piped()); - - let output = command - .output() - .await - .map_err(|e| format!("Failed to execute skills installer: {}", e))?; - - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - - if !output.status.success() { - let exit_code = output.status.code().unwrap_or(-1); - let detail = if !stderr.trim().is_empty() { - truncate_preview(stderr.trim()) - } else if !stdout.trim().is_empty() { - truncate_preview(stdout.trim()) - } else { - "Unknown installer error".to_string() - }; - return Err(format!( - "Failed to download skill package '{}' (exit code {}): {}", - package, exit_code, detail - )); - } + let proxy = configured_proxy_for_skills(&state).await?; + let outcome = install_skill_from_market( + &package, + level, + workspace_path.as_deref(), + proxy.as_ref(), + ) + .await?; + let install_summary = format!( + "Installed skill '{}' to {}", + package, + outcome.target_dir.display() + ); registry .refresh_for_workspace(workspace_path.as_deref()) @@ -1217,26 +1271,59 @@ pub async fn download_skill_market( package, level, installed_skills, - output: summarize_command_output(&stdout, &stderr), + output: install_summary, }) } +/// Read the BitFun AI proxy config (`global_config.ai.proxy`) for use by the +/// native skill downloader. Mirrors `commands.rs::configured_ai_proxy` inline +/// to keep the skill market download path self-contained; the duplication is a +/// known follow-up cleanup (extract to a shared `api::proxy` module). +async fn configured_proxy_for_skills( + state: &State<'_, AppState>, +) -> Result, String> { + let global_config: bitfun_core::service::config::GlobalConfig = state + .config_service + .get_config(None) + .await + .map_err(|e| format!("Failed to get configuration: {}", e))?; + Ok(global_config.ai.proxy.enabled.then_some(global_config.ai.proxy)) +} + fn normalize_market_limit(value: Option) -> u32 { value .unwrap_or(DEFAULT_MARKET_LIMIT) .clamp(1, MAX_MARKET_LIMIT) } -async fn fetch_skill_market(query: &str, limit: u32) -> Result, String> { +fn normalize_market_offset(value: Option) -> u32 { + value.unwrap_or(0) +} + +async fn fetch_skill_market( + query: &str, + limit: u32, + offset: u32, + proxy: Option<&ProxyConfig>, +) -> Result, String> { let api_base = std::env::var("SKILLS_API_URL").unwrap_or_else(|_| SKILLS_SEARCH_API_BASE.into()); let base_url = api_base.trim_end_matches('/'); let endpoint = format!("{}/api/search", base_url); - let client = Client::new(); + // Over-fetch by `offset` so we can slice the requested page out of the + // prefix without depending on whether skills.sh's legacy /api/search + // honors an `offset`/`page` parameter. The legacy endpoint does not + // document one, so requesting `limit + offset` and slicing locally is the + // safe, correct fallback. + let fetch_limit = offset + .saturating_add(limit) + .min(MAX_MARKET_LIMIT); + + let client = build_market_client(proxy)?; let response = client .get(&endpoint) - .query(&[("q", query), ("limit", &limit.to_string())]) + .query(&[("q", query), ("limit", &fetch_limit.to_string())]) .send() .await .map_err(|e| format!("Failed to query skill market: {}", e))?; @@ -1283,68 +1370,74 @@ async fn fetch_skill_market(query: &str, limit: u32) -> Result String { - let primary = if !stdout.trim().is_empty() { - stdout.trim() - } else { - stderr.trim() - }; - - if primary.is_empty() { - return "Skill downloaded successfully.".to_string(); - } - - truncate_preview(primary) -} - -fn truncate_preview(text: &str) -> String { - if text.chars().count() <= MAX_OUTPUT_PREVIEW_CHARS { - return text.to_string(); + // Slice the requested page. + let start = (offset as usize).min(items.len()); + let end = (start.saturating_add(limit as usize)).min(items.len()); + let mut page: Vec = items.into_iter().skip(start).take(end - start).collect(); + + // Block on description fill so cards appear WITH their descriptions (no + // title-only placeholders that pop in later). skills.sh's legacy search has + // no description field and its v1 API needs Vercel OIDC auth, so per-skill + // HTML scrape is the only no-auth source — this is the unavoidable + // ~few-second cost of a cold page (the process cache speeds up repeat + // views of the same skills). Each per-skill fetch is bounded by + // MARKET_DESC_FETCH_TIMEOUT_SECS, so this cannot hang indefinitely. + if !page.is_empty() { + let ids: Vec = page.iter().map(|i| i.id.clone()).collect(); + let descs = fetch_descriptions_for_ids(&ids, base_url, proxy).await; + for item in page.iter_mut() { + if item.description.trim().is_empty() { + if let Some(desc) = descs.get(&item.id) { + item.description = desc.clone(); + } + } + } } - let truncated: String = text.chars().take(MAX_OUTPUT_PREVIEW_CHARS).collect(); - format!("{}...", truncated) + Ok(page) } fn market_description_cache() -> &'static RwLock> { MARKET_DESCRIPTION_CACHE.get_or_init(|| RwLock::new(HashMap::new())) } -async fn fill_market_descriptions(client: &Client, base_url: &str, items: &mut [SkillMarketItem]) { +async fn fetch_descriptions_for_ids( + ids: &[String], + base_url: &str, + proxy: Option<&ProxyConfig>, +) -> HashMap { let cache = market_description_cache(); + let mut result: HashMap = HashMap::new(); + // 1. Serve whatever is already in the process-level cache. { let reader = cache.read().await; - for item in items.iter_mut() { - if !item.description.trim().is_empty() { - continue; - } - if let Some(cached) = reader.get(&item.id) { - item.description = cached.clone(); + for id in ids { + if let Some(cached) = reader.get(id) { + result.insert(id.clone(), cached.clone()); } } } - let mut missing_ids = Vec::new(); - for item in items.iter() { - if item.description.trim().is_empty() { - missing_ids.push(item.id.clone()); - } - } + let missing: Vec = ids + .iter() + .filter(|id| !result.contains_key(*id)) + .cloned() + .collect(); - if missing_ids.is_empty() { - return; + if missing.is_empty() { + return result; } + // 2. Concurrent HTML scrape for uncached ids. Concurrency and per-page + // timeout are bounded by MARKET_DESC_FETCH_CONCURRENCY and + // MARKET_DESC_FETCH_TIMEOUT_SECS respectively; the caller also wraps + // the whole call in an overall deadline. + let client = build_market_client(proxy).unwrap_or_else(|_| Client::new()); let mut join_set = JoinSet::new(); let mut fetched = HashMap::new(); - for skill_id in missing_ids { + for skill_id in missing { let client_clone = client.clone(); let page_url = format!("{}/{}", base_url, skill_id.trim_start_matches('/')); @@ -1360,30 +1453,24 @@ async fn fill_market_descriptions(client: &Client, base_url: &str, items: &mut [ } } - while let Some(result) = join_set.join_next().await { - if let Ok((skill_id, Some(desc))) = result { + while let Some(result_entry) = join_set.join_next().await { + if let Ok((skill_id, Some(desc))) = result_entry { fetched.insert(skill_id, desc); } } - if fetched.is_empty() { - return; - } - - { + // 3. Write back to cache and merge into the result map. + if !fetched.is_empty() { let mut writer = cache.write().await; for (skill_id, desc) in &fetched { writer.insert(skill_id.clone(), desc.clone()); } } - - for item in items.iter_mut() { - if item.description.trim().is_empty() { - if let Some(desc) = fetched.get(&item.id) { - item.description = desc.clone(); - } - } + for (id, desc) in fetched { + result.insert(id, desc); } + + result } async fn fetch_description_from_skill_page(client: &Client, page_url: &str) -> Option { diff --git a/src/apps/desktop/src/api/skill_market_downloader.rs b/src/apps/desktop/src/api/skill_market_downloader.rs new file mode 100644 index 0000000000..546fc5bbe7 --- /dev/null +++ b/src/apps/desktop/src/api/skill_market_downloader.rs @@ -0,0 +1,632 @@ +//! Native skill market downloader. +//! +//! Replaces the previous `npx -y skills add ` shell-out. The `skills` +//! npm CLI bundles a prebuilt bun binary that aborts (V8 `__errno_location` +//! assertion) on HarmonyOS PC, so BitFun downloads GitHub-hosted skills itself +//! with pure-Rust crates (`reqwest` + `flate2` + `tar`). This works identically +//! on Windows and HarmonyOS without Node/npx/bun. +//! +//! Flow: +//! 1. Parse `install_id` (`org/repo@subdir`) into GitHub `org/repo` + subdir. +//! 2. Download `https://api.github.com/repos/{org}/{repo}/tarball` (no auth; +//! GitHub serves tarballs anonymously, ~60 req/hour per IP). +//! 3. Gzip-decode + walk the tar, stripping the top-level `-/` +//! folder, and extract only entries under `/` into a staging dir. +//! 4. Verify `/SKILL.md` exists. +//! 5. Atomic swap: remove existing target, `rename(staging, target)`. +//! +//! Cross-process safety uses a unique staging dir per install plus an atomic +//! rename; the `fs2` advisory lock from `skills/builtin.rs` is intentionally +//! omitted here to avoid adding a workspace dep and because `fs2`'s `flock` +//! behavior on HarmonyOS is unverified. Concurrent installs of different +//! skills cannot collide (uuid staging names); concurrent installs of the +//! same skill resolve to last-writer-wins via the atomic rename. + +use std::io::Read; +use std::path::{Path, PathBuf}; + +use flate2::read::GzDecoder; +use log::{info, warn}; +use reqwest::Client; +use tar::Archive; +use tokio::fs; + +use bitfun_core::agentic::tools::implementations::skills::SkillLocation; +use bitfun_core::infrastructure::get_path_manager_arc; +use bitfun_core::service::config::types::ProxyConfig; + +/// Hard cap on downloaded tarball size. Skill repos are tiny (< 1 MiB typically); +/// 50 MiB is a generous ceiling to reject runaway downloads. +const MAX_SKILL_TARBALL_BYTES: usize = 50 * 1024 * 1024; + +/// Result of a successful skill market install. +pub struct InstallOutcome { + pub skill_dir_name: String, + pub target_dir: PathBuf, +} + +/// Install a skill from the market by `install_id` (`org/repo@subdir`). +/// +/// `workspace_path` is required for project-level installs and ignored for +/// user-level installs. `proxy`, when enabled, is applied to the HTTP client. +pub async fn install_skill_from_market( + package: &str, + level: SkillLocation, + workspace_path: Option<&Path>, + proxy: Option<&ProxyConfig>, +) -> Result { + let trimmed = package.trim(); + if trimmed.is_empty() { + return Err("Skill package cannot be empty".to_string()); + } + + // 1. Parse install_id -> (org/repo, subdir) + let (org_repo, subdir) = parse_install_id(trimmed)?; + let skill_dir_name = subdir_dir_name(&subdir)?; + + // 2. Resolve target parent + staging dir (sibling of target -> same volume) + let target_parent = resolve_target_parent(level, workspace_path)?; + fs::create_dir_all(&target_parent) + .await + .map_err(|e| format!("Failed to create skills directory: {}", e))?; + let target_dir = target_parent.join(&skill_dir_name); + let staging_dir = target_parent.join(format!(".installing-{}", uuid::Uuid::new_v4().simple())); + fs::create_dir_all(&staging_dir) + .await + .map_err(|e| format!("Failed to create staging directory: {}", e))?; + + // 3-6. Download + extract + verify, into staging + let work = install_to_staging(trimmed, &org_repo, &subdir, &staging_dir, proxy).await; + if let Err(err) = work { + let _ = fs::remove_dir_all(&staging_dir).await; + info!( + "Skill market install failed: package={}, staging={}, error={}", + trimmed, + staging_dir.display(), + err + ); + return Err(err); + } + + // 7. Atomic swap + if target_dir.exists() { + if let Err(e) = fs::remove_dir_all(&target_dir).await { + let _ = fs::remove_dir_all(&staging_dir).await; + return Err(format!("Failed to remove existing skill dir: {}", e)); + } + } + if let Err(e) = fs::rename(&staging_dir, &target_dir).await { + // Keep staging for diagnostics; do not delete. + return Err(format!( + "Failed to finalize skill install (rename {} -> {}): {}", + staging_dir.display(), + target_dir.display(), + e + )); + } + + info!( + "Skill market install completed: package={}, target={}", + trimmed, + target_dir.display() + ); + + Ok(InstallOutcome { skill_dir_name, target_dir }) +} + +/// Parse `org/repo@subdir` into `(org/repo, subdir)`. +/// +/// Rejects anything without a `/` before the first `@` (non-GitHub sources like +/// `id@name` are not supported by the native installer). +pub(crate) fn parse_install_id(package: &str) -> Result<(String, String), String> { + let at_index = package + .find('@') + .ok_or_else(|| unsupported_source_error(package))?; + let left = package[..at_index].trim(); + let right = package[at_index + 1..].trim(); + if !left.contains('/') || right.is_empty() { + return Err(unsupported_source_error(package)); + } + Ok((left.to_string(), right.to_string())) +} + +fn unsupported_source_error(package: &str) -> String { + format!( + "Unsupported skill source '{}': native installer only supports GitHub org/repo@subdir", + package + ) +} + +/// Derive the on-disk directory name from the subdir (last path segment). +pub(crate) fn subdir_dir_name(subdir: &str) -> Result { + subdir + .rsplit('/') + .next() + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()) + .ok_or_else(|| format!("Invalid skill subdir '{}': empty directory name", subdir)) +} + +/// Resolve the parent directory under which the skill dir lands. +fn resolve_target_parent( + level: SkillLocation, + workspace_path: Option<&Path>, +) -> Result { + match level { + SkillLocation::Project => { + let workspace_root = workspace_path.ok_or_else(|| { + "No workspace open, cannot add project-level Skill".to_string() + })?; + Ok(workspace_root.join(".bitfun").join("skills")) + } + SkillLocation::User => Ok(get_path_manager_arc().user_skills_dir()), + } +} + +async fn install_to_staging( + package: &str, + org_repo: &str, + subdir: &str, + staging: &Path, + proxy: Option<&ProxyConfig>, +) -> Result<(), String> { + // 5. HTTP download + let tarball_url = format!("https://api.github.com/repos/{}/tarball", org_repo); + let client = build_download_client(proxy)?; + let bytes = download_tarball(&client, &tarball_url).await?; + + // 6. Extract subdir into staging (also verifies SKILL.md presence and + // reports sample archive paths on failure for diagnostics). + extract_subdir_from_tarball(&bytes, subdir, staging) + .map_err(|e| format!("Failed to extract skill '{}': {}", package, e))?; + Ok(()) +} + +fn build_download_client(proxy: Option<&ProxyConfig>) -> Result { + let mut builder = Client::builder() + .connect_timeout(std::time::Duration::from_secs(10)) + .timeout(std::time::Duration::from_secs(60)) + .user_agent(concat!("BitFun/", env!("CARGO_PKG_VERSION"))); + if let Some(proxy) = proxy { + if proxy.enabled { + let url = proxy.url.trim(); + if !url.is_empty() { + let normalized = normalize_proxy_url(url); + match reqwest::Proxy::all(&normalized) { + Ok(p) => builder = builder.proxy(p), + Err(e) => warn!("Failed to configure proxy for skill download: {}", e), + } + } + } + } + builder + .build() + .map_err(|e| format!("Failed to build HTTP client: {}", e)) +} + +/// Prefix `http://` to bare `host:port` proxy URLs (mirrors the AI adapter). +fn normalize_proxy_url(url: &str) -> String { + if url.contains("://") { + url.to_string() + } else { + format!("http://{}", url) + } +} + +async fn download_tarball(client: &Client, url: &str) -> Result, String> { + let response = client + .get(url) + .send() + .await + .map_err(|e| format!("Failed to download skill tarball from {}: {}", url, e))?; + if !response.status().is_success() { + return Err(format!( + "Failed to download skill tarball: HTTP {} from {}", + response.status(), + url + )); + } + let bytes = response + .bytes() + .await + .map_err(|e| format!("Failed to read skill tarball body: {}", e))?; + if bytes.len() > MAX_SKILL_TARBALL_BYTES { + return Err(format!( + "Skill tarball too large: {} bytes (max {})", + bytes.len(), + MAX_SKILL_TARBALL_BYTES + )); + } + Ok(bytes.to_vec()) +} + +/// Walk the gzip+tar archive, locate the requested skill's folder, and extract +/// only that folder's contents into `staging` (so `staging/SKILL.md` exists). +/// +/// Locating the skill is necessary because the skills.sh slug (`subdir`) does +/// NOT always equal the repo folder name — e.g. `vercel-labs/agent-skills` +/// has folder `react-native-skills` but slug `vercel-react-native-skills`. The +/// SKILL.md frontmatter `name` field IS the slug, so match on that first; +/// fall back to a folder-name match, then a single SKILL.md if the repo has +/// only one skill. The archive is read twice (it is in-memory); pass 1 finds +/// the skill folder, pass 2 extracts only its contents (clean staging, no +/// sibling-skill junk). +fn extract_subdir_from_tarball( + bytes: &[u8], + subdir: &str, + staging: &Path, +) -> std::io::Result<()> { + let skill_folder = find_skill_folder_in_archive(bytes, subdir)? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "did not contain a skill matching '{}'. The slug did not match any \ + SKILL.md frontmatter name, any skill folder name, and the archive had \ + more than one SKILL.md.", + subdir + ), + ) + })?; + + extract_skill_folder_contents(bytes, &skill_folder, staging)?; + + if !staging.join("SKILL.md").exists() { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "did not contain SKILL.md at resolved skill folder '{}'", + skill_folder + ), + )); + } + Ok(()) +} + +/// Pass 1: scan the archive and return the skill folder path relative to the +/// top-level wrapper folder (e.g. `skills/react-native-skills`, +/// `frontend-design`, or `""` for a SKILL.md at the repo root). +fn find_skill_folder_in_archive(bytes: &[u8], subdir: &str) -> std::io::Result> { + let gz = GzDecoder::new(bytes); + let mut archive = Archive::new(gz); + + let mut folder_name_match: Option = None; + let mut frontmatter_match: Option = None; + let mut skill_md_folders: Vec = Vec::new(); + + for entry in archive.entries()? { + let mut entry = entry?; + let entry_path = entry.path()?.into_owned(); + let entry_path_str = entry_path.to_string_lossy().replace('\\', "/"); + let Some(rest) = strip_top_level_dir(&entry_path_str) else { + continue; + }; + if rest.is_empty() { + continue; + } + + let is_skill_md = rest == "SKILL.md" || rest.ends_with("/SKILL.md"); + if !is_skill_md { + continue; + } + + // Skill folder = parent path of the SKILL.md (or "" at repo root). + let folder = match rest.rfind('/') { + Some(idx) => rest[..idx].to_string(), + None => String::new(), + }; + skill_md_folders.push(folder.clone()); + + // Cheap check first: folder's last path segment == slug? + if folder_name_match.is_none() { + let last_segment = folder.rsplit('/').next().unwrap_or(""); + if last_segment == subdir { + folder_name_match = Some(folder.clone()); + } + } + + // Most reliable: SKILL.md frontmatter `name` == slug. Only parse if we + // haven't matched yet (avoids reading every SKILL.md in large repos). + if frontmatter_match.is_none() { + if let Some(name) = read_skill_md_frontmatter_name(&mut entry) { + if name == subdir { + frontmatter_match = Some(folder); + } + } + } + } + + // Prefer frontmatter match (slug == frontmatter name), then folder-name + // match, then a single SKILL.md (repo with one skill). + if let Some(f) = frontmatter_match { + return Ok(Some(f)); + } + if let Some(f) = folder_name_match { + return Ok(Some(f)); + } + if skill_md_folders.len() == 1 { + return Ok(skill_md_folders.into_iter().next()); + } + Ok(None) +} + +/// Read the `name:` field from a SKILL.md YAML frontmatter. Reads the whole +/// entry so the tar iterator cleanly advances to the next entry (SKILL.md +/// files are small). +fn read_skill_md_frontmatter_name(reader: &mut R) -> Option { + let mut buf = String::new(); + reader.read_to_string(&mut buf).ok()?; + parse_frontmatter_name(&buf) +} + +fn parse_frontmatter_name(content: &str) -> Option { + // Frontmatter sits between leading `---` lines. + let after_open = if let Some(rest) = content.strip_prefix("---\n") { + rest + } else if let Some(rest) = content.strip_prefix("---\r\n") { + rest + } else { + content + }; + for line in after_open.lines() { + let line = line.trim_end_matches('\r'); + if line == "---" { + break; + } + if let Some(rest) = line.strip_prefix("name:") { + let val = rest.trim().trim_matches('"').trim_matches('\'').to_string(); + if !val.is_empty() { + return Some(val); + } + } + } + None +} + +/// Pass 2: extract entries under `{top-level}/{skill_folder}/` to `staging` +/// root (so SKILL.md lands at `staging/SKILL.md`). +fn extract_skill_folder_contents( + bytes: &[u8], + skill_folder: &str, + staging: &Path, +) -> std::io::Result<()> { + let gz = GzDecoder::new(bytes); + let mut archive = Archive::new(gz); + let folder_prefix = if skill_folder.is_empty() { + String::new() + } else { + format!("{}/", skill_folder) + }; + + for entry in archive.entries()? { + let mut entry = entry?; + let entry_path = entry.path()?.into_owned(); + let entry_path_str = entry_path.to_string_lossy().replace('\\', "/"); + let Some(rest) = strip_top_level_dir(&entry_path_str) else { + continue; + }; + if rest.is_empty() { + continue; + } + + // Keep only entries under the resolved skill folder. + let relative = if skill_folder.is_empty() { + // SKILL.md at the repo root: the whole repo is the skill. Keep all + // entries (rare; the common case has a non-empty folder). + rest + } else if rest == skill_folder { + continue; + } else if let Some(stripped) = rest.strip_prefix(&folder_prefix) { + stripped.to_string() + } else { + continue; + }; + + let safe_relative = validate_relative_path(&relative) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?; + let target = staging.join(&safe_relative); + + if entry.header().entry_type().is_dir() { + std::fs::create_dir_all(&target)?; + } else { + if let Some(parent) = target.parent() { + std::fs::create_dir_all(parent)?; + } + entry.unpack(&target)?; + } + } + Ok(()) +} + +/// Strip the first path segment (the GitHub `-` folder). Returns the +/// remainder without a leading slash, or `None` if the entry is the top-level +/// folder itself or has no sub-path. +fn strip_top_level_dir(path: &str) -> Option { + // splitn(2, '/') yields [first_segment, rest_after_first_slash]; the + // remainder retains any subsequent slashes, which is what we want when + // locating `/...` under the top-level `-/` folder. + let mut parts = path.splitn(2, '/'); + let first = parts.next()?; + if first.is_empty() { + return None; + } + let rest = parts.next()?; + if rest.is_empty() { + return None; + } + Some(rest.to_string()) +} + +/// Reject path traversal and absolute paths inside the extracted subdir. +fn validate_relative_path(path: &str) -> Result { + let normalized = path.replace('\\', "/"); + if normalized.starts_with('/') { + return Err(format!("Refusing to extract absolute path: {}", normalized)); + } + for segment in normalized.split('/') { + if segment == ".." { + return Err(format!("Refusing to extract path with '..': {}", normalized)); + } + } + Ok(normalized) +} + +#[cfg(test)] +mod tests { + use super::*; + use flate2::Compression; + use std::io::Write; + + fn build_tarball(entries: &[(&str, &str)]) -> Vec { + // entries: (path, contents); path is the full path inside the tar, + // already including the top-level `-/` folder. + let mut buf = Vec::new(); + { + let encoder = flate2::write::GzEncoder::new(&mut buf, Compression::none()); + let mut builder = tar::Builder::new(encoder); + for (path, contents) in entries { + let mut header = tar::Header::new_gnu(); + header.set_size(contents.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder.append_data(&mut header, path, std::io::Cursor::new(contents.as_bytes())) + .expect("append file"); + } + builder.finish().expect("finish tar"); + } + buf + } + + #[test] + fn parse_install_id_accepts_github_form() { + let (org_repo, subdir) = parse_install_id("anthropics/skills@frontend-design").unwrap(); + assert_eq!(org_repo, "anthropics/skills"); + assert_eq!(subdir, "frontend-design"); + } + + #[test] + fn parse_install_id_accepts_nested_subdir() { + let (org_repo, subdir) = parse_install_id("org/repo@foo/bar/baz").unwrap(); + assert_eq!(org_repo, "org/repo"); + assert_eq!(subdir, "foo/bar/baz"); + } + + #[test] + fn parse_install_id_rejects_no_at() { + let err = parse_install_id("anthropics/skills").unwrap_err(); + assert!(err.contains("only supports GitHub org/repo@subdir")); + } + + #[test] + fn parse_install_id_rejects_no_slash_before_at() { + let err = parse_install_id("anthropics@frontend-design").unwrap_err(); + assert!(err.contains("only supports GitHub org/repo@subdir")); + } + + #[test] + fn parse_install_id_rejects_empty_subdir() { + let err = parse_install_id("anthropics/skills@").unwrap_err(); + assert!(err.contains("only supports GitHub org/repo@subdir")); + } + + #[test] + fn subdir_dir_name_takes_last_segment() { + assert_eq!(subdir_dir_name("frontend-design").unwrap(), "frontend-design"); + assert_eq!(subdir_dir_name("foo/bar/baz").unwrap(), "baz"); + } + + #[test] + fn subdir_dir_name_rejects_trailing_slash() { + assert!(subdir_dir_name("foo/").is_err()); + } + + #[test] + fn validate_relative_path_accepts_normal() { + assert_eq!(validate_relative_path("SKILL.md").unwrap(), "SKILL.md"); + assert_eq!(validate_relative_path("examples/app.ts").unwrap(), "examples/app.ts"); + } + + #[test] + fn validate_relative_path_rejects_parent_traversal() { + let err = validate_relative_path("../escape.md").unwrap_err(); + assert!(err.contains("'..'")); + let err = validate_relative_path("foo/../../escape.md").unwrap_err(); + assert!(err.contains("'..'")); + } + + #[test] + fn validate_relative_path_rejects_absolute() { + let err = validate_relative_path("/etc/passwd").unwrap_err(); + assert!(err.contains("absolute")); + } + + #[test] + fn extract_subdir_from_tarball_extracts_only_subdir() { + // Top-level folder emulates GitHub's `-/` wrapper. + let tarball = build_tarball(&[ + ("repo-abc123/frontend-design/SKILL.md", "---\nname: x\n---\nbody"), + ("repo-abc123/frontend-design/examples/app.ts", "// code"), + ("repo-abc123/other-skill/SKILL.md", "should be skipped"), + ("repo-abc123/README.md", "repo root, skipped"), + ]); + let tmp = tempfile::tempdir().expect("tempdir"); + let staging = tmp.path().join("staging"); + std::fs::create_dir_all(&staging).unwrap(); + + extract_subdir_from_tarball(&tarball, "frontend-design", &staging).unwrap(); + + assert!(staging.join("SKILL.md").exists()); + assert!(staging.join("examples").join("app.ts").exists()); + // other-skill and repo root must NOT leak in. + assert!(!staging.join("other-skill").exists()); + assert!(!staging.join("README.md").exists()); + } + + #[test] + fn extract_subdir_from_tarball_rejects_traversal_entry() { + // A malicious archive that places a `..` segment under the subdir. + let tarball = build_tarball(&[ + ("repo-abc123/frontend-design/SKILL.md", "---\nname: x\n---\nbody"), + ("repo-abc123/frontend-design/../escape.md", "pwn"), + ]); + let tmp = tempfile::tempdir().expect("tempdir"); + let staging = tmp.path().join("staging"); + std::fs::create_dir_all(&staging).unwrap(); + + let err = extract_subdir_from_tarball(&tarball, "frontend-design", &staging) + .expect_err("traversal must be rejected"); + let msg = format!("{}", err); + assert!(msg.contains("..") || msg.contains("traversal") || msg.contains("'..'")); + } + + #[test] + fn extract_subdir_matches_by_frontmatter_name_when_folder_differs() { + // skills.sh slug (`vercel-react-native-skills`) != repo folder + // (`react-native-skills`); the SKILL.md frontmatter `name` IS the slug. + // Mirrors the real vercel-labs/agent-skills layout. + let skill_md = "---\nname: vercel-react-native-skills\ndescription: rn\n---\n# RN"; + let other_md = "---\nname: some-other-skill\ndescription: x\n---\n"; + let tarball = build_tarball(&[ + ("repo-abc/skills/react-native-skills/SKILL.md", skill_md), + ("repo-abc/skills/react-native-skills/rules/foo.md", "rule"), + ("repo-abc/skills/other-skill/SKILL.md", other_md), + ("repo-abc/README.md", "repo root"), + ]); + let tmp = tempfile::tempdir().expect("tempdir"); + let staging = tmp.path().join("staging"); + std::fs::create_dir_all(&staging).unwrap(); + + extract_subdir_from_tarball(&tarball, "vercel-react-native-skills", &staging) + .expect("frontmatter match must resolve the skill"); + + assert!(staging.join("SKILL.md").exists()); + assert!(staging.join("rules").join("foo.md").exists()); + // The other skill and repo root must NOT leak into staging. + assert!(!staging.join("other-skill").exists()); + assert!(!staging.join("skills").exists()); + assert!(!staging.join("README.md").exists()); + let installed = std::fs::read_to_string(staging.join("SKILL.md")).unwrap(); + assert!(installed.contains("vercel-react-native-skills")); + } +} diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index 5cba54f4a3..f43ce086eb 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -1568,6 +1568,7 @@ pub async fn _run() { list_skill_market, search_skill_market, download_skill_market, + get_skill_descriptions, set_global_skill_disabled, set_mode_skill_disabled, replace_mode_skill_selection, diff --git a/src/apps/desktop/src/logging.rs b/src/apps/desktop/src/logging.rs index 88efd553e7..61b1f992e5 100644 --- a/src/apps/desktop/src/logging.rs +++ b/src/apps/desktop/src/logging.rs @@ -644,6 +644,13 @@ fn configured_log_builder(log_targets: Vec) -> tauri_plugin_log::Builder ) .level_for("hyper_util", log::LevelFilter::Info) .level_for("h2", log::LevelFilter::Info) + // reqwest/rustls log every new connection and CA-store load at DEBUG, + // which floods the log during skill market listing/description fetches. + // Keep WARN/ERROR (real failures) but drop the mechanical connect noise. + .level_for("reqwest", log::LevelFilter::Warn) + .level_for("rustls", log::LevelFilter::Warn) + .level_for("rustls_platform_verifier", log::LevelFilter::Warn) + .level_for("hyper", log::LevelFilter::Warn) .level_for("portable_pty", log::LevelFilter::Info) .level_for("russh", log::LevelFilter::Info) .level_for("grep_searcher", log::LevelFilter::Warn) diff --git a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx index 1d33e338e2..8235581101 100644 --- a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx +++ b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx @@ -9,6 +9,7 @@ import { useSkillMarket } from './useSkillMarket'; const listSkillMarketMock = vi.hoisted(() => vi.fn()); const searchSkillMarketMock = vi.hoisted(() => vi.fn()); const downloadSkillMarketMock = vi.hoisted(() => vi.fn()); +const getSkillDescriptionsMock = vi.hoisted(() => vi.fn()); const installedChangedMock = vi.hoisted(() => vi.fn()); const notificationMocks = vi.hoisted(() => ({ success: vi.fn(), @@ -24,6 +25,7 @@ vi.mock('@/infrastructure/api', () => ({ listSkillMarket: listSkillMarketMock, searchSkillMarket: searchSkillMarketMock, downloadSkillMarket: downloadSkillMarketMock, + getSkillDescriptions: getSkillDescriptionsMock, }, })); vi.mock('@/infrastructure/hooks/useWorkspaceManagerSync', () => ({ @@ -61,6 +63,7 @@ describe('useSkillMarket', () => { listSkillMarketMock.mockReset().mockResolvedValue([]); searchSkillMarketMock.mockReset().mockResolvedValue([]); downloadSkillMarketMock.mockReset(); + getSkillDescriptionsMock.mockReset().mockResolvedValue({}); installedChangedMock.mockReset(); notificationMocks.success.mockReset(); notificationMocks.warning.mockReset(); diff --git a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts index e7b7dcfe24..e22260ebbc 100644 --- a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts +++ b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts @@ -59,11 +59,11 @@ export function useSkillMarket({ capabilityRef.current.enabled && capabilityRef.current.epoch === epoch ), []); - const fetchSkills = useCallback(async (query: string | undefined, limit: number) => { + const fetchSkills = useCallback(async (query: string | undefined, limit: number, offset: number) => { const normalized = query?.trim(); return normalized - ? await configAPI.searchSkillMarket(normalized, limit) - : await configAPI.listSkillMarket(undefined, limit); + ? await configAPI.searchSkillMarket(normalized, limit, offset) + : await configAPI.listSkillMarket(undefined, limit, offset); }, []); const loadFirstPage = useCallback(async (query?: string) => { @@ -77,7 +77,7 @@ export function useSkillMarket({ setMarketError(null); setCurrentPage(0); try { - const skillList = await fetchSkills(query, pageSize); + const skillList = await fetchSkills(query, pageSize, 0); if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { return; } @@ -122,6 +122,8 @@ export function useSkillMarket({ installed: installedSkillNames.has(skill.name), })); + // Sort by install count (popular first), then original fetch order for a + // stable position. Installed skills are prioritized to the front. entries.sort((a, b) => { if (a.installed !== b.installed) { return a.installed ? -1 : 1; @@ -156,12 +158,12 @@ export function useSkillMarket({ if (capabilityEpoch === null) { return; } - const requestId = ++marketRequestIdRef.current; const nextPage = currentPage + 1; - const neededCount = Math.min((nextPage + 1) * pageSize, MAX_TOTAL_SKILLS); + const nextOffset = nextPage * pageSize; - if (displayMarketSkills.length >= neededCount) { + // If the next page is already loaded locally, just advance the view. + if (displayMarketSkills.length >= nextOffset + pageSize) { setCurrentPage(nextPage); return; } @@ -170,22 +172,46 @@ export function useSkillMarket({ return; } + const requestId = ++marketRequestIdRef.current; + // Advance the page immediately so the user sees the page turn (with + // skeletons) right away, instead of freezing on the current page. setCurrentPage(nextPage); try { setLoadingMore(true); - const skillList = await fetchSkills(searchQuery || undefined, neededCount); + // Real offset pagination: fetch only the next page slice from the + // backend; no re-fetching of previously loaded items, no grow-limit. + const remainingBudget = Math.max(0, MAX_TOTAL_SKILLS - displayMarketSkills.length); + if (remainingBudget < pageSize) { + setHasMore(false); + return; + } + const skillList = await fetchSkills(searchQuery || undefined, pageSize, nextOffset); if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { return; } - setMarketSkills(skillList); - const hitCap = neededCount >= MAX_TOTAL_SKILLS; - setHasMore(!hitCap && skillList.length >= neededCount); + if (skillList.length > 0) { + setMarketSkills((prev) => { + // Append new items, deduping by installId to avoid duplicate cards + // if the backend returned overlap. + const seen = new Set(prev.map((s) => s.installId)); + const fresh = skillList.filter((s) => { + if (seen.has(s.installId)) { + return false; + } + seen.add(s.installId); + return true; + }); + return [...prev, ...fresh]; + }); + } + setHasMore(skillList.length >= pageSize); } catch (err) { if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { return; } log.error('Failed to load more skills', err); + // Roll back to the previous page on failure. setCurrentPage(currentPage); } finally { if (requestId === marketRequestIdRef.current && capabilityIsCurrent(capabilityEpoch)) { diff --git a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts index c25e5ecf16..6335efe9fe 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts @@ -436,23 +436,31 @@ export class ConfigAPI { } } - async listSkillMarket(query?: string, limit?: number): Promise { + async listSkillMarket(query?: string, limit?: number, offset?: number): Promise { try { return await api.invoke('list_skill_market', { - request: { query, limit } + request: { query, limit, offset } }); } catch (error) { - throw createTauriCommandError('list_skill_market', error, { query, limit }); + throw createTauriCommandError('list_skill_market', error, { query, limit, offset }); } } - async searchSkillMarket(query: string, limit?: number): Promise { + async searchSkillMarket(query: string, limit?: number, offset?: number): Promise { try { return await api.invoke('search_skill_market', { - request: { query, limit } + request: { query, limit, offset } }); } catch (error) { - throw createTauriCommandError('search_skill_market', error, { query, limit }); + throw createTauriCommandError('search_skill_market', error, { query, limit, offset }); + } + } + + async getSkillDescriptions(ids: string[]): Promise> { + try { + return await api.invoke('get_skill_descriptions', { request: { ids } }); + } catch (error) { + throw createTauriCommandError('get_skill_descriptions', error, { ids }); } } From 9662404922823f3a9968b1e5bfb32bff1e72130f Mon Sep 17 00:00:00 2001 From: hanlinyu1030 Date: Tue, 25 Aug 2026 09:57:46 +0800 Subject: [PATCH 3/3] fix(skills): disable project-level install when assistant workspace is active When no real project is open, BitFun auto-activates an assistant workspace as currentWorkspace. hasWorkspace only checked non-null (not workspaceKind), so project-level skill install landed in /.bitfun/skills/ and 'disappeared' when the workspace switched (scan root changed; files still on disk). Add isAssistantWorkspace to useWorkspaceManagerSync; in useSkillMarket.handleDownload and useInstalledSkills.handleAdd, block project-level when assistant is current (no auto-fallback to user). SkillsScene: market card download button disabled, detail-modal project button hidden + a noWorkspace hint, manual-add project level option disabled + projectDisabled label. User-level install stays available. Reuses existing messages.noWorkspace text (no new i18n key). Matches the existing pickWorkspaceForProjectChatSession pattern that already excludes Assistant. --- src/web-ui/src/app/scenes/skills/SkillsScene.scss | 9 +++++++++ src/web-ui/src/app/scenes/skills/SkillsScene.tsx | 12 +++++++++--- .../app/scenes/skills/hooks/useInstalledSkills.ts | 10 +++++++--- .../app/scenes/skills/hooks/useSkillMarket.test.tsx | 1 + .../src/app/scenes/skills/hooks/useSkillMarket.ts | 10 +++++++--- .../infrastructure/hooks/useWorkspaceManagerSync.ts | 4 +++- 6 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/web-ui/src/app/scenes/skills/SkillsScene.scss b/src/web-ui/src/app/scenes/skills/SkillsScene.scss index 0f394ed525..8ac91b4616 100644 --- a/src/web-ui/src/app/scenes/skills/SkillsScene.scss +++ b/src/web-ui/src/app/scenes/skills/SkillsScene.scss @@ -1163,6 +1163,15 @@ padding-top: $size-gap-3; border-top: 1px solid var(--bf-appearance-token-border-subtle); } + + // Hint shown in the skill detail modal when project-level install is + // unavailable (assistant or remote workspace is active). + &__modal-project-hint { + margin: 0; + font-size: var(--bf-appearance-token-font-size-xs); + color: var(--bf-appearance-token-color-text-muted); + opacity: 0.85; + } } // ══════════════════════════════════════════════════════════════════════════════ diff --git a/src/web-ui/src/app/scenes/skills/SkillsScene.tsx b/src/web-ui/src/app/scenes/skills/SkillsScene.tsx index 25c08ed51c..cc83e65626 100644 --- a/src/web-ui/src/app/scenes/skills/SkillsScene.tsx +++ b/src/web-ui/src/app/scenes/skills/SkillsScene.tsx @@ -683,6 +683,7 @@ const SkillsScene: React.FC = () => { isDownloading || !market.hasWorkspace || market.isRemoteWorkspace + || market.isAssistantWorkspace || isInstalled, tone: isInstalled ? 'success' : 'primary', onClick: () => void market.handleDownload(skill, 'project'), @@ -809,7 +810,7 @@ const SkillsScene: React.FC = () => { ) : ( <> - {!market.isRemoteWorkspace && ( + {!market.isRemoteWorkspace && !market.isAssistantWorkspace && ( )} + {(market.isRemoteWorkspace || market.isAssistantWorkspace) && ( +

+ {t('messages.noWorkspace')} +

+ )}