feat: Phase 2 — browser skill-management HTTP endpoints (+ groups plumbing)

crates/fastskill-cli/src/commands/add/mod.rs

@@ -534,39 +534,12 @@ pub async fn execute_add(service: &FastSkillService, args: AddArgs, global: bool
     } else {
         AddMode::Fresh
     };
+    let groups = args.group.clone().map(|g| vec![g]).unwrap_or_default();
     let outcome = service
-        .add_from_origin(origin, mode)
+        .add_from_origin(origin, mode, groups)
         .await
         .map_err(CliError::Service)?;
 
-    // Core-seam gap: `add_from_origin`'s manifest/lock upsert has no `groups`
-    // parameter (always writes `groups: None` / `Vec::new()`), so `--group` is
-    // reapplied here, same as the update path.
-    if let Some(group) = &args.group {
-        let current_dir = env::current_dir()
-            .map_err(|e| CliError::Config(format!("Failed to get current directory: {}", e)))?;
-        let project_file_result = resolve_project_file(&current_dir);
-        let lock_path = project_file_result
-            .path
-            .parent()
-            .map(|p| p.join("skills.lock"))
-            .unwrap_or_else(|| PathBuf::from("skills.lock"));
-        if let Err(e) = crate::utils::manifest_utils::reapply_groups_after_seam(
-            &project_file_result.path,
-            &lock_path,
-            &outcome.id,
-            vec![group.clone()],
-        ) {
-            eprintln!(
-                "{}",
-                crate::utils::messages::error(&format!(
-                    "Added {} but failed to record its group: {}",
-                    outcome.id, e
-                ))
-            );
-        }
-    }
-
     // `AddOutcome` only carries the skill `id`, not its display `name`; look the
     // freshly-registered skill back up for a nicer message, falling back to the
     // id (which is always a valid, if less friendly, thing to print).

crates/fastskill-cli/src/commands/serve.rs

@@ -99,7 +99,8 @@ impl FromArgValueMap for ServeArgs {
 }
 
 pub async fn execute_serve(
-    service: std::sync::Arc<fastskill_core::FastSkillService>,
+    global: bool,
+    skills_dir: Option<std::path::PathBuf>,
     args: ServeArgs,
 ) -> CliResult<()> {
     info!(
@@ -120,6 +121,29 @@ pub async fn execute_serve(
         println!("  Write endpoints: disabled (read-only); pass --enable-write to enable");
     }
 
+    // Build the served service directly (rather than reusing the CLI's cached
+    // singleton via `FsState::service_with`) so this exact instance can carry
+    // BOTH the edge-injected embedding provider + repository manager
+    // (`inject_edge_services`, same as every other command) AND the served
+    // project's root (ADR-0005 install seam): `add_from_origin`/`preflight`-driven
+    // writes (via `POST /skills/install`, `/skills/update`) must land in the
+    // served project's `skill-project.toml`/`skills.lock`, not wherever the
+    // server process happens to have cwd set.
+    let cfg = crate::config::create_service_config(global, skills_dir)?;
+    let mut service = fastskill_core::FastSkillService::new(cfg)
+        .await
+        .map_err(CliError::Service)?;
+    service.initialize().await.map_err(CliError::Service)?;
+    let mut service = crate::config::inject_edge_services(service)?;
+
+    if let Ok(current_dir) = std::env::current_dir() {
+        if let Ok(project_config) = fastskill_core::core::load_project_config(&current_dir) {
+            service = service.with_project_root(project_config.project_root);
+        }
+    }
+
+    let service = std::sync::Arc::new(service);
+
     let server =
         fastskill_core::http::server::FastSkillServer::from_ref(&service, &args.host, args.port)
             .enable_write(args.enable_write);

crates/fastskill-cli/src/commands/update.rs

@@ -2,7 +2,7 @@
 
 use crate::config::create_service_config;
 use crate::error::{manifest_required_message, CliError, CliResult};
-use crate::utils::{manifest_utils, messages};
+use crate::utils::messages;
 use cli_framework::command::{FromArgValueMap, IntoCommandSpec};
 use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality};
 use cli_framework::spec::command_tree::CommandSpec;
@@ -417,28 +417,12 @@ async fn execute_update_project(args: UpdateArgs) -> CliResult<()> {
         println!("  Updating {}...", entry.id);
         match service.preflight(&entry.origin).await {
             Ok(UpdatePreflight::Updatable) => {
+                // Pass the existing groups so update preserves group membership.
                 match service
-                    .add_from_origin(entry.origin.clone(), AddMode::Update)
+                    .add_from_origin(entry.origin.clone(), AddMode::Update, entry.groups.clone())
                     .await
                 {
-                    Ok(outcome) => {
-                        // Core-seam gap workaround: `add_from_origin`'s manifest/lock
-                        // upsert has no `groups` parameter (see
-                        // `reapply_groups_after_seam`'s doc comment).
-                        if let Err(e) = manifest_utils::reapply_groups_after_seam(
-                            &project_file_path,
-                            &lock_path,
-                            &outcome.id,
-                            entry.groups.clone(),
-                        ) {
-                            eprintln!(
-                                "  {}",
-                                messages::error(&format!(
-                                    "Updated {} but failed to reapply groups: {}",
-                                    entry.id, e
-                                ))
-                            );
-                        }
+                    Ok(_outcome) => {
                         updated_count += 1;
                         println!("  {}", messages::ok(&format!("Updated {}", entry.id)));
                     }

crates/fastskill-cli/src/main.rs

@@ -528,7 +528,6 @@ fn build_app(builder: AppBuilder, state: Arc<FsState>) -> anyhow::Result<AppBuil
         let state_reindex = Arc::clone(&state);
         let state_remove = Arc::clone(&state);
         let state_search = Arc::clone(&state);
-        let state_serve = Arc::clone(&state);
         let state_doctor = Arc::clone(&state);
         builder
             .register(path!["reindex"], move |ctx, args: reindex::ReindexArgs| {
@@ -567,10 +566,12 @@ fn build_app(builder: AppBuilder, state: Arc<FsState>) -> anyhow::Result<AppBuil
             .register(path!["serve"], move |ctx, args: serve::ServeArgs| {
                 let global = ctx_global(ctx);
                 let skills_dir = ctx_skills_dir(ctx);
-                let state = Arc::clone(&state_serve);
                 async move {
-                    let svc = state.service_with(global, skills_dir).await?;
-                    serve::execute_serve(svc, args)
+                    // `serve` builds its own service (rather than going through
+                    // `FsState::service_with`) so it can inject the served
+                    // project's root alongside the usual edge services; see
+                    // `serve::execute_serve`.
+                    serve::execute_serve(global, skills_dir, args)
                         .await
                         .map_err(anyhow::Error::from)
                 }

crates/fastskill-cli/src/utils/manifest_utils.rs

@@ -145,64 +145,6 @@ pub fn remove_from_global_lock_file(skill_id: &str) -> Result<(), Box<dyn std::e
     Ok(())
 }
 
-/// Re-apply `groups` onto both the skill-project.toml dependency entry and the
-/// skills.lock entry for `skill_id`, after a core-seam `add_from_origin` /
-/// `preflight` install (ADR-0005).
-///
-/// Core-seam gap: `add_from_origin`'s shared `commit` pipeline has no `groups`
-/// parameter — its manifest upsert always writes `groups: None` and its lock
-/// upsert always writes `groups: Vec::new()` (`ProjectSkillsLock::update_skill_with_depth`
-/// hardcodes it). This restores the CLI's poetry-style `--group` / dependency-group
-/// UX on top of the seam without touching core. A no-op when `groups` is empty
-/// (the common case), so it never turns a clean write into a dirty one.
-pub fn reapply_groups_after_seam(
-    project_file_path: &Path,
-    lock_path: &Path,
-    skill_id: &str,
-    groups: Vec<String>,
-) -> Result<(), Box<dyn std::error::Error>> {
-    if groups.is_empty() {
-        return Ok(());
-    }
-
-    if project_file_path.exists() {
-        let mut project = SkillProjectToml::load_from_file(project_file_path)
-            .map_err(|e| format!("Failed to load skill-project.toml: {}", e))?;
-        let mut changed = false;
-        if let Some(deps) = project.dependencies.as_mut() {
-            if let Some(DependencySpec::Inline { groups: g, .. }) =
-                deps.dependencies.get_mut(skill_id)
-            {
-                *g = Some(groups.clone());
-                changed = true;
-            }
-        }
-        if changed {
-            project
-                .save_to_file(project_file_path)
-                .map_err(|e| format!("Failed to save skill-project.toml: {}", e))?;
-        }
-    }
-
-    if lock_path.exists() {
-        let sidecar = sidecar_path(lock_path);
-        let _guard = acquire_advisory_lock(&sidecar)
-            .map_err(|e| format!("Failed to acquire lock on skills.lock: {}", e))?;
-
-        let mut lock = ProjectSkillsLock::load_from_file(lock_path)
-            .map_err(|e| format!("Failed to load lock file: {}", e))?;
-        if let Some(entry) = lock.skills.iter_mut().find(|s| s.id == skill_id) {
-            entry.groups = groups;
-            lock.save_to_file(lock_path)
-                .map_err(|e| format!("Failed to save lock file: {}", e))?;
-        }
-
-        let _ = std::fs::remove_file(&sidecar);
-    }
-
-    Ok(())
-}
-
 /// T028: Add skill to skill-project.toml [dependencies] section.
 /// Fails if skill-project.toml is not found in the hierarchy (we never auto-create it).
 ///