Fix dry_run bypass in install/update/uninstall and correct README

[pirate]

Summary

• Fix dry_run short-circuit in BinProvider.install(), update(), uninstall(): Move the dry_run check before calling _call_handler_for_action so providers that call subprocess.run directly (like AnsibleProvider) don't execute real commands during dry runs. Previously, the check happened after the handler ran, causing 120s timeouts in ansible dry-run tests.
• Fix README inaccuracies: Replace non-existent field references (ansible_installer_module, ansible_playbook_template, pyinfra_installer_module, pyinfra_installer_kwargs) with comments documenting the defaults. Fix duplicate text in CargoProvider section.

Test plan

• All lint/type checks pass (uv run prek run --all-files — 26/26 passed)
• Core tests pass (test_semver, test_module_api, test_security_controls, test_envprovider, test_binprovider, test_binary — 54/54 passed)
• Ansible dry_run test now completes instantly instead of timing out at 120s
• Full lifecycle test suite (CI matrix covers all providers across OS/Python versions)

https://claude.ai/code/session_01RR9j9JutDnrVqB7qE7iPgS

---

---

Summary by cubic

Prevent real command execution in dry_run for AnsibleProvider and PyinfraProvider, while preserving dry-run logging for other providers. Also correct README defaults and clarify Cargo install root wording.

• Bug Fixes
  • AnsibleProvider/PyinfraProvider: add early dry_run checks in install/update/uninstall handlers; install/update return a DRY RUN message, uninstall returns True.
  • Preserve dry-run behavior for other providers via _run_subprocess_as_provider (e.g., keeps BunProvider logging intact).
  • README: replace non-existent fields with comments documenting defaults; clarify install_root aliasing (cargo_root) and <install_root>/bin path.

^{Written for commit 7abe208. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 2 files

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[cubic-dev-ai]

2 issues found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="abxpkg/binprovider.py">

<violation number="1" location="abxpkg/binprovider.py:2128">
P1: Dry-run checks are executed after handler calls, so install/update/uninstall still run provider code during dry runs.</violation>
</file>

<file name="abxpkg/binprovider_ansible.py">

<violation number="1" location="abxpkg/binprovider_ansible.py:376">
P2: The new dry-run check still runs after `INSTALLER_BINARY(...)`, so dry runs execute provider-loading work before returning. Move the dry-run short-circuit before resolving `ansible_playbook`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Dry-run checks are executed after handler calls, so install/update/uninstall still run provider code during dry runs.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At abxpkg/binprovider.py, line 2128:

<comment>Dry-run checks are executed after handler calls, so install/update/uninstall still run provider code during dry runs.</comment>

<file context>
@@ -2146,6 +2125,19 @@ def install(
         finally:
             ACTIVE_EXEC_LOG_PREFIX.reset(exec_log_prefix_token)
 
+        if self.dry_run:
+            return ShallowBinary.model_construct(
+                name=bin_name,
</file context>

[cubic-dev-ai]

P2: The new dry-run check still runs after INSTALLER_BINARY(...), so dry runs execute provider-loading work before returning. Move the dry-run short-circuit before resolving ansible_playbook.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At abxpkg/binprovider_ansible.py, line 376:

<comment>The new dry-run check still runs after `INSTALLER_BINARY(...)`, so dry runs execute provider-loading work before returning. Move the dry-run short-circuit before resolving `ansible_playbook`.</comment>

<file context>
@@ -373,6 +373,14 @@ def default_install_handler(
         ansible_playbook = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath
         assert ansible_playbook
 
+        if self.dry_run:
+            logger.info(
+                "DRY RUN (%s): ansible-playbook install %s",
</file context>

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

[devin-ai-integration]

🔴 Moving dry_run check after handler call in uninstall() causes real file deletions during dry_run

The base class BinProvider.uninstall() previously short-circuited before calling the handler when self.dry_run is true (abxpkg/binprovider.py:2380, old code). This PR moved the check to after the handler runs at line 2410. While self.exec() has its own dry_run guard (line 1699), multiple providers' uninstall handlers perform direct file operations that bypass exec() and will now execute even during dry_run:

• GoGet (abxpkg/binprovider_goget.py:255): Path(abspath).unlink() deletes the binary
• Docker (abxpkg/binprovider_docker.py:280-281): deletes wrapper script and metadata JSON
• Puppeteer (abxpkg/binprovider_puppeteer.py:702,707): deletes symlink and shutil.rmtree() on browser dir
• Playwright (abxpkg/binprovider_playwright.py:726,736): deletes symlink and shutil.rmtree() on browser dirs
• ChromeWebstore (abxpkg/binprovider_chromewebstore.py:264-269): deletes cache, CRX, and unpacked extension

None of these uninstall handlers have their own dry_run checks, unlike the Ansible/Pyinfra handlers that this PR explicitly patched. A provider.uninstall(bin_name, dry_run=True) call will now irreversibly delete files.

(Refers to lines 2410-2411)

Prompt for agents

The dry_run guard in BinProvider.uninstall() was moved from before _call_handler_for_action to after it (line 2410). This breaks dry_run for any provider whose uninstall handler performs direct file operations (Path.unlink, shutil.rmtree) instead of going through self.exec() (which has its own dry_run guard at line 1699). Affected providers: GoGet (binprovider_goget.py:242-264), Docker (binprovider_docker.py:261-291), Puppeteer (binprovider_puppeteer.py:690-708), Playwright (binprovider_playwright.py:716-737), ChromeWebstore (binprovider_chromewebstore.py:251-270). Fix either by (a) moving the dry_run check in BinProvider.uninstall() back before the handler call (between setup_PATH and the handler invocation, around line 2381), or (b) adding dry_run early-return checks to each of those five uninstall handlers, matching the pattern used for Ansible/Pyinfra in this PR.

---

Was this helpful? React with 👍 or 👎 to provide feedback.