Skip to content

refactor: strip tool-version probing and discarded home-dir enumeration from bootstrap#361

Merged
iamcristi merged 2 commits into
mainfrom
refactor/remove-bootstrap-tool-probe
Jun 10, 2026
Merged

refactor: strip tool-version probing and discarded home-dir enumeration from bootstrap#361
iamcristi merged 2 commits into
mainfrom
refactor/remove-bootstrap-tool-probe

Conversation

@iamcristi

@iamcristi iamcristi commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Removes two pieces of best-effort work the bootstrap handshake did at startup, neither of which the control server depends on:

  1. Tool-version probing
  2. Discarded home-directory enumeration

The bootstrap handshake previously shelled out to `<tool> --version` for
python/node/npx/uvx/docker plus the stdio-local-proxy shim deps
(bash/shasum/grep) and reported the results in the payload's
`host.runtimes` dict. Remove the probing end to end:

- drop `_probe_tool_version`, `get_tool_versions`, and their constants
  (`_TOOL_VERSION_PROBE_TIMEOUT`, `_DEFAULT_PROBED_TOOLS`) from utils,
  along with the now-unused `asyncio` / `Iterable` imports
- drop the `runtimes` field from `HostInfo` so the bootstrap payload no
  longer carries it
- collapse the `_build_request` gather to the home-dir enumeration alone
  (call retained for its access-probe logging side effects)
- remove the probe and payload tests covering the feature
`_build_request` called `get_readable_home_directories` and threw the
result away — home directories were never part of the bootstrap payload
(there's a test asserting `"paths" not in data`). The call only produced
access-probe log lines and, on Windows, spawned PowerShell + wsl.exe
subprocesses. Remove it and the dead plumbing it pulled in:

- drop the `get_readable_home_directories` call and import from bootstrap
- drop the stale `_HOME_DIRECTORIES_LIMIT` constant
- remove the now-dead `scan_all_users` parameter from
  `bootstrap_first_control_server` / `_impl` / `_build_request`, and stop
  forwarding it from the CLI bootstrap wrapper (the scan/inspect
  discovery flow in run_scan still uses --scan-all-users, unchanged)
- correct the security-review allowlist comment to match the fields the
  payload actually carries
- remove the obsolete bootstrap tests (slow/failing home enumeration,
  to_thread usage), the autouse home-dir mock, the scan_all_users
  forwarding tests, and the now-pointless home-dir monkeypatch stubs

`get_readable_home_directories` itself is unchanged — it remains in use
by the scan/inspect pipeline via pipelines.py.
@iamcristi iamcristi requested a review from a team as a code owner June 10, 2026 16:16
@qodo-merge-etso

Copy link
Copy Markdown

PR Summary by Qodo

Refactor bootstrap: remove tool probing and discarded home-dir enumeration
✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

Walkthroughs

Description
• Remove bootstrap subprocess probing of tool versions and drop host.runtimes from payload.
• Stop enumerating readable home directories during bootstrap and remove dead scan_all_users
  plumbing.
• Update request models and prune/adjust unit tests to match the slimmer handshake contract.
Diagram
graph TD
  A["CLI bootstrap wrapper"] --> B["bootstrap_first_control_server"] --> C["_build_request"] --> D["ClientBootstrapRequest"]
  C --> E["utils host signals"]
  B -->|POST JSON| F{{"Control server /bootstrap"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep runtimes telemetry behind an opt-in/feature flag
  • ➕ Preserves toolchain observability for diagnostics/telemetry when explicitly enabled
  • ➕ Avoids breaking any server-side assumptions by keeping the key available when needed
  • ➖ Still requires subprocess execution (risk of hangs/slowdowns) when enabled
  • ➖ Adds flag/config surface area and testing matrix complexity
2. Defer tool probing to a later, non-blocking telemetry path
  • ➕ Avoids adding startup latency or bootstrap flakiness while still collecting versions
  • ➕ Can be rate-limited/cached independently of bootstrap retries
  • ➖ Requires building a separate telemetry channel and server support
  • ➖ Collected data may be less correlated with the exact bootstrap event timing

Recommendation: The PR’s approach (remove probing and discarded home-dir enumeration entirely) is the best fit if the control server does not depend on these fields: it reduces startup latency, eliminates fragile subprocess behavior (notably on Windows), and narrows the bootstrap payload’s security surface. If telemetry is still desired later, prefer a deferred/async telemetry path rather than reintroducing subprocess work into the handshake.

Grey Divider

File Changes

Refactor (4)
bootstrap.py Slim bootstrap payload and remove scan_all_users parameter +6/-26

Slim bootstrap payload and remove scan_all_users parameter

• Removes tool-version probing and readable-home enumeration from bootstrap request building, and deletes the unused '_HOME_DIRECTORIES_LIMIT'. Drops 'scan_all_users' from the bootstrap API surface and updates the payload allowlist comment to match the fields actually sent.

src/agent_scan/bootstrap.py


cli.py Stop forwarding scan_all_users into bootstrap wrapper +0/-1

Stop forwarding scan_all_users into bootstrap wrapper

• Updates the CLI bootstrap wrapper to no longer pass 'scan_all_users' to 'bootstrap_first_control_server', since bootstrap no longer enumerates home directories. Leaves the scan/inspect pipeline’s '--scan-all-users' behavior intact.

src/agent_scan/cli.py


models.py Remove HostInfo.runtimes from bootstrap contract +0/-7

Remove HostInfo.runtimes from bootstrap contract

• Deletes the 'runtimes' field (and its documentation) from 'HostInfo', reflecting the removal of tool-version probing from the bootstrap payload.

src/agent_scan/models.py


utils.py Delete tool-version probing helpers +0/-78

Delete tool-version probing helpers

• Removes '_probe_tool_version', 'get_tool_versions', and associated constants and imports. This eliminates subprocess-based version probing for bootstrap telemetry.

src/agent_scan/utils.py


Tests (4)
test_bootstrap.py Remove tests tied to home enumeration/to_thread behavior +0/-79

Remove tests tied to home enumeration/to_thread behavior

• Deletes the autouse home-dir mock and removes bootstrap tests that asserted slow/failing home enumeration behavior and thread offloading. Keeps remaining bootstrap behavioral tests focused on HTTP/retry and response handling.

tests/unit/test_bootstrap.py


test_bootstrap_payload.py Update payload assertions for removed runtimes field +5/-90

Update payload assertions for removed runtimes field

• Removes monkeypatching of home-directory enumeration and deletes tests asserting the 'host.runtimes' dict shape. Adds/updates assertions that 'runtimes' is absent from the host payload.

tests/unit/test_bootstrap_payload.py


test_cli_bootstrap_wrapper.py Drop scan_all_users forwarding tests for bootstrap wrapper +0/-43

Drop scan_all_users forwarding tests for bootstrap wrapper

• Removes unit tests that verified 'scan_all_users' was passed through the bootstrap wrapper, since the bootstrap API no longer accepts the parameter.

tests/unit/test_cli_bootstrap_wrapper.py


test_utils.py Remove tool probing unit tests +0/-131

Remove tool probing unit tests

• Deletes tests for '_probe_tool_version' and 'get_tool_versions' (including concurrency/timeout behavior) now that tool-version probing has been removed from utils.

tests/unit/test_utils.py


Grey Divider

Qodo Logo

@iamcristi iamcristi merged commit d033b1b into main Jun 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants