Fix: No configs available break upload

[knielsen404]

This PR makes sure that machines are uploaded even if there are no client configs available on that machine. It also adds a small message to let the user know that no configs were found.

[qodo-merge-etso]

Review Summary by Qodo

Allow machine upload when no MCP configs found

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Remove early return when results empty, allowing upload with no configs
• Refactor inspect_pipeline to handle missing clients gracefully
• Add user info to upload payload even when no scan results exist
• Add tests for empty results and missing client scenarios

Diagram

flowchart LR
  A["inspect_pipeline"] -->|"handles missing clients"| B["scan_path_results"]
  B -->|"includes errors"| C["upload function"]
  C -->|"sends payload"| D["control server"]
  E["empty results"] -->|"no early return"| C

Loading

File Changes

1. src/agent_scan/pipelines.py 🐞 Bug fix +35/-38

Refactor inspect pipeline to handle missing clients

• Refactored client discovery to build scan_path_results during collection instead of
 post-processing
• Added explicit handling for missing clients with file_not_found error results
• Removed unreachable code that checked for None clients in inspection loop
• Added user-facing message when no MCP configs found on machine
• Simplified detected_usernames filtering by removing unnecessary None check

src/agent_scan/pipelines.py

---

2. src/agent_scan/upload.py 🐞 Bug fix +0/-4

Remove early return for empty results

• Removed early return guard that prevented upload when results list is empty
• Allows machines with no configs to be registered on control server

src/agent_scan/upload.py

---

3. tests/unit/test_control_server.py 🧪 Tests +24/-0

Test upload with empty results

• Added test verifying upload sends payload with user_info when results are empty
• Validates that control server receives machine registration even without scan results

tests/unit/test_control_server.py

---

View more (1)

4. tests/unit/test_inspect.py 🧪 Tests +43/-0

Test inspect pipeline missing client scenarios

• Added test for inspect_pipeline returning empty results when no clients installed
• Added test for explicit path returning file_not_found error when path missing
• Validates graceful handling of missing client configurations

tests/unit/test_inspect.py

---

[qodo-merge-etso]

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)

🐞\ ≡ Correctness (1)

1. Permission mislabeled file_not_found 🐞 ≡

Description

In --paths mode, inspect_pipeline converts any empty return from client_to_inspect_from_path into a
ScanError(category="file_not_found", is_failure=False). If the underlying scan returns empty due to
PermissionError while checking existence, the user gets a misleading “not found” non-failure instead
of a permission-related failure.

Code

src/agent_scan/pipelines.py[R66-82]

+        for path in inspect_args.paths:
+            ctis = await client_to_inspect_from_path(path, True, home_dirs_with_users, inspect_args.scan_skills)
+            if ctis:
+                clients_to_inspect.extend(ctis)
+            else:
+                scan_path_results.append(
+                    ScanPathResult(
+                        path=path,
+                        client=path,
+                        servers=[],
+                        issues=[],
+                        labels=[],
+                        error=ScanError(
+                            message="File or folder not found", is_failure=False, category="file_not_found"
+                        ),
+                    )
+                )

Evidence

get_mcp_config_per_home_directory() catches PermissionError during Path.exists() checks and
returns None (leading to an empty client list). inspect_pipeline() then treats the resulting
empty list as a non-failure file_not_found error, collapsing permission-denied into not-found.

src/agent_scan/pipelines.py[65-82]
src/agent_scan/inspect.py[69-83]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`inspect_pipeline()` maps any `ctis == []` from `client_to_inspect_from_path()` to a `file_not_found` error with `is_failure=False`. But the lower-level existence check can yield an empty result due to `PermissionError`, which should not be reported as “file not found” and usually should be a failure.

### Issue Context
`get_mcp_config_per_home_directory()` catches `PermissionError` during `path_expanded.exists()` and continues, which can leave `client_path` unset and return `None`.

### Fix Focus Areas
- src/agent_scan/pipelines.py[65-82]
- src/agent_scan/inspect.py[69-83]

### Suggested approach
- Propagate permission information upward (e.g., have `get_mcp_config_per_home_directory()` return a structured error object or raise a specific exception that `inspect_pipeline()` can catch).
- Add an explicit error category for permission issues (e.g., extend `ErrorCategory` with `"permission_denied"`) and set `is_failure=True`.
- If you don’t want to change types, add a targeted check in `inspect_pipeline()` before emitting `file_not_found` (e.g., `os.path.exists()` + `os.access()` heuristics) to classify permission-denied separately.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. inspect_pipeline() too long ☑ 📘 ☼

Description

inspect_pipeline is ~41+ non-blank lines, exceeding the ≤40 SLOC limit and making the scan flow
harder to maintain. This increases the risk of future bugs when modifying path discovery, username
selection, and inspection behavior in one place.

Code

src/agent_scan/pipelines.py[R62-111]

+    # fetch clients to inspect, building scan_path_results for paths that don't exist
+    scan_path_results: list[ScanPathResult] = []
+    clients_to_inspect: list[ClientToInspect] = []
    if inspect_args.paths:
-        clients_to_inspect = [
-            cti
-            for path in inspect_args.paths
-            for cti in await client_to_inspect_from_path(path, True, home_dirs_with_users, inspect_args.scan_skills)
-        ]
+        for path in inspect_args.paths:
+            ctis = await client_to_inspect_from_path(path, True, home_dirs_with_users, inspect_args.scan_skills)
+            if ctis:
+                clients_to_inspect.extend(ctis)
+            else:
+                scan_path_results.append(
+                    ScanPathResult(
+                        path=path,
+                        client=path,
+                        servers=[],
+                        issues=[],
+                        labels=[],
+                        error=ScanError(
+                            message="File or folder not found", is_failure=False, category="file_not_found"
+                        ),
+                    )
+                )
    else:
-        clients_to_inspect = [
-            cti
-            for client in get_well_known_clients()
-            for cti in await get_mcp_config_per_client(client, home_dirs_with_users)
-        ]
+        for client in get_well_known_clients():
+            ctis = await get_mcp_config_per_client(client, home_dirs_with_users)
+            if ctis:
+                clients_to_inspect.extend(ctis)
+            else:
+                logger.info(f"Client {client.name} does not exist on this machine. {client.client_exists_paths}")
+        if not clients_to_inspect:
+            rich.print("No MCP client configurations found on this machine.")

    # Only report usernames where an agent was detected in their home directory.
    # When no usernames were associated with detected agents:
    #   - Discovery mode with --scan-all-users: fall back to all readable usernames.
    #   - Otherwise (explicit paths or single-user mode): fall back to the current OS user only,
    #     to avoid disclosing unrelated usernames on the machine.
-    detected_usernames: list[str] = sorted(
-        {cti.username for cti in clients_to_inspect if cti is not None and cti.username is not None}
-    )
+    detected_usernames: list[str] = sorted({cti.username for cti in clients_to_inspect if cti.username is not None})
    if detected_usernames:
        scanned_usernames = detected_usernames
    elif not inspect_args.paths and inspect_args.all_users:
        scanned_usernames = all_usernames
    else:
        scanned_usernames = [getpass.getuser()]
    # inspect
-    scan_path_results: list[ScanPathResult] = []
-    for i, client_to_inspect in enumerate(clients_to_inspect):
-        if client_to_inspect is None and inspect_args.paths:
-            scan_path_results.append(
-                ScanPathResult(
-                    path=inspect_args.paths[i],
-                    client=inspect_args.paths[i],
-                    servers=[],
-                    issues=[],
-                    labels=[],
-                    error=ScanError(message="File or folder not found", is_failure=False, category="file_not_found"),
-                )
-            )
-            continue
-        elif client_to_inspect is None:
-            logger.info(
-                f"Client {get_well_known_clients()[i].name} does not exist os this machine. {get_well_known_clients()[i].client_exists_paths}"
-            )
-            continue
-        else:
-            inspected_client = await inspect_client(
-                client_to_inspect, inspect_args.timeout, inspect_args.tokens, inspect_args.scan_skills
-            )
-            scan_path_results.append(inspected_client_to_scan_path_result(inspected_client))
+    for client_to_inspect in clients_to_inspect:
+        inspected_client = await inspect_client(
+            client_to_inspect, inspect_args.timeout, inspect_args.tokens, inspect_args.scan_skills
+        )
+        scan_path_results.append(inspected_client_to_scan_path_result(inspected_client))
    return scan_path_results, scanned_usernames

Evidence

PR Compliance ID 45 requires functions to be ≤40 lines; the updated inspect_pipeline spans well
beyond this threshold in the changed hunk, combining client discovery, missing-path handling,
username selection, and inspection in one function.

Rule 45: Limit function length to ≤ 40 lines (SLOC)
src/agent_scan/pipelines.py[55-111]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`inspect_pipeline()` exceeds the 40-line function-length limit, mixing multiple responsibilities (client discovery, missing-path result creation, username selection, inspection loop).

## Issue Context
The PR added additional control flow for paths that don’t exist and for the "no configs" case, increasing the function size.

## Fix Focus Areas
- src/agent_scan/pipelines.py[55-111]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. No-config message misleading ☑ 🐞 ≡

Description

inspect_pipeline prints "No MCP client configurations found" only when clients_to_inspect is empty,
but get_mcp_config_per_client can return ClientToInspect even when all MCP config files are missing
(mcp_configs stays empty), so the message won’t appear in the “client exists but no configs”
scenario. This contradicts the PR’s stated intent to notify users when no configs are found.

Code

src/agent_scan/pipelines.py[R84-91]

+        for client in get_well_known_clients():
+            ctis = await get_mcp_config_per_client(client, home_dirs_with_users)
+            if ctis:
+                clients_to_inspect.extend(ctis)
+            else:
+                logger.info(f"Client {client.name} does not exist on this machine. {client.client_exists_paths}")
+        if not clients_to_inspect:
+            rich.print("No MCP client configurations found on this machine.")

Evidence

The message is gated on clients_to_inspect being empty, but get_mcp_config_per_client returns a
ClientToInspect as long as a client exists, even if no config paths exist and
create_file_not_found_error is False—meaning clients_to_inspect can be non-empty while still having
zero configs discovered.

src/agent_scan/pipelines.py[83-91]
src/agent_scan/pipelines.py[84-88]
src/agent_scan/inspect.py[84-101]
src/agent_scan/inspect.py[136-141]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`inspect_pipeline()` prints the “No MCP client configurations found…” message only when `clients_to_inspect` is empty. However, `get_mcp_config_per_home_directory()` returns a `ClientToInspect` whenever the client exists, even if all MCP config files are missing (so `mcp_configs` can be empty). This means the message can be suppressed in the very “no configs” condition the PR aims to communicate.

### Issue Context
- In discovery mode, a client can exist without any readable/available MCP config files.
- The PR’s message should align with “no configs found” rather than “no clients_to_inspect objects returned”.

### Fix Focus Areas
- src/agent_scan/pipelines.py[83-111]
- src/agent_scan/inspect.py[84-141]

### Suggested approach
- After building `scan_path_results` (or after inspecting clients), compute whether any discovered scan result actually contains servers or any path-level error that indicates a discovered config/skill.
- Print the message when there are **no discovered MCP configs/skills/servers** (e.g., no servers across results and no meaningful errors), rather than when `clients_to_inspect` is empty.
- Alternatively (often cleaner): move/duplicate the message to `upload()` after `results_with_servers` filtering; if `results_with_servers` is empty, print a single “No configs found; uploading machine metadata anyway” message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[mmilanta]

Minor change, then it is great

[mmilanta]

lgtm