fix(cli): reject flags that were accepted and silently ignored

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

@@ -62,14 +62,6 @@ pub struct ReindexArgs {
     /// Force re-indexing of all skills (ignore existing hashes)
     pub force: bool,
 
-    /// Maximum number of concurrent embedding requests
-    ///
-    /// NOTE (core-seam gap): the core `reindex` seam processes skills
-    /// sequentially and takes no concurrency parameter, so this is currently
-    /// accepted but has no effect. Kept for CLI/arg compatibility.
-    #[allow(dead_code)]
-    pub max_concurrent: usize,
-
     /// Show progress bars and processing details
     pub progress: bool,
 
@@ -108,17 +100,6 @@ impl IntoCommandSpec for ReindexArgs {
                     default: None,
                     ..Default::default()
                 },
-                ArgSpec {
-                    name: "max-concurrent",
-                    long: Some("max-concurrent"),
-                    short: None,
-                    help: "Maximum concurrent embedding requests",
-                    kind: ArgKind::Option,
-                    value_type: ArgValueType::Int,
-                    cardinality: Cardinality::Optional,
-                    default: Some(ArgValue::Int(5)),
-                    ..Default::default()
-                },
                 ArgSpec {
                     name: "progress",
                     long: Some("progress"),
@@ -158,16 +139,6 @@ impl FromArgValueMap for ReindexArgs {
                 }
             }),
             force: matches!(map.get("force"), Some(ArgValue::Bool(true))),
-            max_concurrent: map
-                .get("max-concurrent")
-                .and_then(|v| {
-                    if let ArgValue::Int(n) = v {
-                        Some(*n as usize)
-                    } else {
-                        None
-                    }
-                })
-                .unwrap_or(5),
             progress: matches!(map.get("progress"), Some(ArgValue::Bool(true))),
             no_progress: matches!(map.get("no-progress"), Some(ArgValue::Bool(true))),
         }
@@ -344,7 +315,6 @@ Test skill content"#,
         let args = ReindexArgs {
             skills_dir: None,
             force: false,
-            max_concurrent: 5,
             progress: false,
             no_progress: false,
         };
@@ -378,7 +348,6 @@ Test skill content"#,
         let args = ReindexArgs {
             skills_dir: Some(nonexistent_dir),
             force: false,
-            max_concurrent: 5,
             progress: false,
             no_progress: false,
         };
@@ -413,7 +382,6 @@ Test skill content"#,
         let args = ReindexArgs {
             skills_dir: Some(skills_dir),
             force: true,
-            max_concurrent: 5,
             progress: false,
             no_progress: false,
         };
@@ -458,7 +426,6 @@ Test skill content"#,
         let args = ReindexArgs {
             skills_dir: Some(skills_dir.clone()),
             force: false,
-            max_concurrent: 2,
             progress: true,
             no_progress: false,
         };
@@ -469,7 +436,6 @@ Test skill content"#,
         let force_args = ReindexArgs {
             skills_dir: Some(skills_dir),
             force: true,
-            max_concurrent: 2,
             progress: false,
             no_progress: true,
         };
@@ -494,7 +460,6 @@ Test skill content"#,
         let args = ReindexArgs {
             skills_dir: None,
             force: false,
-            max_concurrent: 5,
             progress: true,
             no_progress: true,
         };

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

@@ -43,7 +43,7 @@ pub struct SearchArgs {
     pub json: bool,
 
     /// Use embedding search: true, false, or auto
-    /// Only applies to local search; ignored for remote search
+    /// Only valid with --local; rejected for remote search
     pub embedding: Option<String>,
 
     /// Skills directory path (overrides default discovery)
@@ -151,7 +151,7 @@ impl IntoCommandSpec for SearchArgs {
                     name: "embedding",
                     long: Some("embedding"),
                     short: None,
-                    help: "Use embedding search: true, false, or auto",
+                    help: "Use embedding search (requires --local): true, false, or auto",
                     kind: ArgKind::Option,
                     value_type: ArgValueType::String,
                     cardinality: Cardinality::Optional,
@@ -412,6 +412,12 @@ fn validate_search_args(args: &SearchArgs) -> CliResult<()> {
         ));
     }
 
+    if args.embedding.is_some() && !args.local {
+        return Err(CliError::Config(
+            "--embedding requires --local; remote search does not use embeddings".to_string(),
+        ));
+    }
+
     if args.paths && !args.local {
         return Err(CliError::Config(
             "--paths requires --local; use 'fastskill skill search --local --paths <query>'"
@@ -653,6 +659,26 @@ mod tests {
         assert!(result.is_ok());
     }
 
+    #[test]
+    fn test_validate_search_args_rejects_embedding_for_remote() {
+        let args = SearchArgs {
+            query: "test".to_string(),
+            local: false,
+            remote: false,
+            repository: None,
+            limit: 10,
+            format: None,
+            json: false,
+            embedding: Some("true".to_string()),
+            skills_dir: None,
+            paths: false,
+            content: None,
+        };
+
+        let error = validate_search_args(&args).unwrap_err();
+        assert!(error.to_string().contains("--embedding requires --local"));
+    }
+
     #[test]
     fn test_validate_search_args_valid_remote_with_repository() {
         let args = SearchArgs {

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

@@ -114,7 +114,6 @@ pub async fn maybe_auto_reindex(
     let args = crate::commands::reindex::ReindexArgs {
         skills_dir: None,
         force: false,
-        max_concurrent: 5,
         progress: false,
         no_progress: true,
     };

docs/qa/smoke-test-plan.md

@@ -369,7 +369,7 @@ embedding_model = "text-embedding-3-small"
 |---|---|---|---|---|---|
 | 8.1 | `fastskill cli doctor` | `embedding_config` = Pass ("configuration found"); `api_key` = Pass ("OPENAI_API_KEY is set"). | 🤖 | doctor | ☐P ☐F ☐G |
 | 8.2 | `fastskill --skills-dir "$SKILLS_DIR" index rebuild` | Actually builds the index (not skipped), exit 0, reports a count. | 🤖 | configuration | ☐P ☐F ☐G |
-| 8.3 | `fastskill --skills-dir "$SKILLS_DIR" skill search "edit documents" --embedding true` | Returns semantically-ranked results, exit 0. | 🧑 | search-command | ☐P ☐F ☐G |
+| 8.3 | `fastskill --skills-dir "$SKILLS_DIR" skill search --local "edit documents" --embedding true` | Returns semantically-ranked results, exit 0. | 🧑 | search-command | ☐P ☐F ☐G |
 | 8.4 | `fastskill --skills-dir "$SKILLS_DIR" analysis matrix` | Pairwise similarity output, exit 0. | 🤖 | analysis | ☐P ☐F ☐G |
 | 8.5 | `fastskill --skills-dir "$SKILLS_DIR" analysis duplicates` | Duplicate/near-duplicate pairs (or none), exit 0. | 🤖 | analysis | ☐P ☐F ☐G |
 | 8.6 | `fastskill --skills-dir "$SKILLS_DIR" analysis cluster --num-clusters 2` | Clusters the skills, exit 0. | 🤖 | analysis | ☐P ☐F ☐G |
@@ -386,13 +386,13 @@ embedding_model = "<model your gateway serves>"
 | # | Command | Expected observable | Mode | Src | Result |
 |---|---|---|---|---|---|
 | 8.8 | `fastskill --skills-dir "$SKILLS_DIR" index rebuild` | Reindex succeeds **against the gateway** — confirm via gateway logs that the request hit `<gateway>/v1/embeddings`. **This is the real proof the override works.** | 🧑 | embedding.rs | ☐P ☐F ☐G |
-| 8.9 | `fastskill --skills-dir "$SKILLS_DIR" skill search "edit documents" --embedding true` | Semantic results via the gateway, exit 0. | 🧑 | — | ☐P ☐F ☐G |
+| 8.9 | `fastskill --skills-dir "$SKILLS_DIR" skill search --local "edit documents" --embedding true` | Semantic results via the gateway, exit 0. | 🧑 | — | ☐P ☐F ☐G |
 
 **Variant C — graceful skip (no provider).** Temporarily unset the key:
 | # | Command | Expected observable | Mode | Src | Result |
 |---|---|---|---|---|---|
 | 8.10 | `OPENAI_API_KEY= fastskill --skills-dir "$SKILLS_DIR" index rebuild` | "Reindex skipped: … Run 'fastskill cli doctor' …", **exit 0**. | 🧑 | reindex | ☐P ☐F ☐G |
-| 8.11 | `OPENAI_API_KEY= fastskill --skills-dir "$SKILLS_DIR" skill search "x"` (no `--embedding`) | Falls back to text/fuzzy search, exit 0. | 🧑 | search local | ☐P ☐F ☐G |
+| 8.11 | `OPENAI_API_KEY= fastskill --skills-dir "$SKILLS_DIR" skill search --local "x"` (no `--embedding`) | Falls back to text/fuzzy search, exit 0. | 🧑 | search local | ☐P ☐F ☐G |
 
 ### Section 9 — Eval
 

tests/cli/reindex_e2e_tests.rs

@@ -131,39 +131,21 @@ embedding_model = "text-embedding-3-small"
     assert_snapshot_with_settings("reindex_force", &result.stdout, &cli_snapshot_settings());
 }
 
+/// `--max-concurrent` was accepted and silently ignored (indexing is
+/// sequential); it is now an unknown argument so callers learn it did nothing.
 #[test]
-fn test_reindex_max_concurrent() {
+fn test_reindex_rejects_max_concurrent() {
     let temp_dir = TempDir::new().unwrap();
-    let skills_dir = temp_dir.path().join(".skills");
-    fs::create_dir_all(&skills_dir).unwrap();
-
-    // Create config for reindex
-    let config_content = r#"[dependencies]
-
-[tool.fastskill]
-skills_directory = ".skills"
-
-[tool.fastskill.embedding]
-openai_base_url = "https://api.openai.com/v1"
-embedding_model = "text-embedding-3-small"
-"#;
-    fs::write(temp_dir.path().join("skill-project.toml"), config_content).unwrap();
-
-    // Set OPENAI_API_KEY to avoid config requirement
-    let env_vars = vec![("OPENAI_API_KEY", "test-key")];
-    let result = run_fastskill_command_with_env(
+    let result = run_fastskill_command(
         &["index", "rebuild", "--max-concurrent", "2"],
-        &env_vars,
         Some(temp_dir.path()),
     );
 
-    assert!(result.success);
-    // Should succeed with concurrency limit
-
-    assert_snapshot_with_settings(
-        "reindex_max_concurrent",
-        &result.stdout,
-        &cli_snapshot_settings(),
+    assert!(!result.success, "--max-concurrent must be rejected");
+    assert!(
+        result.stderr.contains("unknown argument"),
+        "stderr: {}",
+        result.stderr
     );
 }
 

tests/cli/snapshots/cli_tests__cli__snapshot_helpers__search_command_help.snap

@@ -21,7 +21,7 @@ Options:
   -l, --limit <limit>            Maximum number of results [default: 10]
   -f, --format <format>          Output format: table, json, grid, xml
       --json                     Shorthand for --format json
-      --embedding <embedding>    Use embedding search: true, false, or auto
+      --embedding <embedding>    Use embedding search (requires --local): true, false, or auto
       --paths                    Output resolved file paths instead of search results (--local only)
       --content <content>        Content to include in JSON output: none, preview, full (--local
                                  --paths only)

webdocs/cli-reference/reindex-command.mdx

@@ -46,14 +46,6 @@ fastskill index rebuild --skills-dir .claude/skills/ --force
 - When embeddings seem outdated
 - To rebuild corrupted index
 
-### --max-concurrent `<NUMBER>`
-
-<Callout type="warning">
-This flag is currently a **no-op**. It is accepted for compatibility but has no effect:
-`index rebuild` embeds skills **sequentially**, one at a time. Passing a value does not change
-indexing speed or the number of concurrent API requests.
-</Callout>
-
 ## Configuration Requirements
 
 ### 1. Embedding Configuration

webdocs/cli-reference/search-command.mdx

@@ -29,7 +29,7 @@ to search only installed skills.
 | `-l, --limit <NUMBER>` | Return between 1 and 1,000 results. | `10` |
 | `-f, --format <FORMAT>` | Render `table`, `json`, `grid`, or `xml`. | `table` |
 | `--json` | Shorthand for `--format json`; mutually exclusive with `--format`. | `false` |
-| `--embedding <MODE>` | Local search mode: `true`, `false`, or `auto`. | `auto` |
+| `--embedding <MODE>` | Local search mode: `true`, `false`, or `auto`. Requires `--local`; rejected for remote search. | `auto` |
 | `--paths` | Return resolved paths as JSON; requires `--local`. | `false` |
 | `--content <MODE>` | Include `none`, `preview`, or `full`; requires `--local --paths`. | `none` |
 | `--skills-dir <PATH>` | Override the installation directory for this invocation. | project setting |