fix(extensions): keep a disabled extension disabled across upgrade and rollback

docs/reference/extensions.md

@@ -302,7 +302,11 @@ names the exact repair command.
 For a separately distributed provider, pass `--manifest <extension.toml>`.
 `upgrade` validates and probes the new manifest before changing the active
 revision. `rollback` probes the previous revision before switching back. A
-failed probe leaves the current revision untouched. Activation state contains
+failed probe leaves the current revision untouched. Neither operation changes
+the enablement decision: a disabled extension stays disabled across an upgrade
+or rollback until an explicit `enable`. The operation result reports the
+enablement state it committed, so an `enable` or `disable` that completes while
+the probe runs shows in the result as well as in `list`. Activation state contains
 validated manifest snapshots and revision ids in the private LoopX runtime
 root; it does not contain provider output or credentials.
 

loopx/extensions/runtime.py

@@ -239,6 +239,10 @@ def install_extension(
         previous_revision = (
             str(existing.get("active_revision")) if isinstance(existing, dict) else None
         )
+        # A fresh install activates the provider. An upgrade changes only the
+        # active revision; the operator's enable/disable decision is separate
+        # state and survives the revision change.
+        enabled = bool(existing.get("enabled")) if isinstance(existing, dict) else True
         if execute:
             revisions = (
                 list(existing.get("revisions") or [])
@@ -257,7 +261,7 @@ def install_extension(
             )
             extensions[extension_id] = {
                 "id": extension_id,
-                "enabled": True,
+                "enabled": enabled,
                 "active_revision": revision,
                 "rollback_revision": previous_revision,
                 "revisions": revisions,
@@ -275,7 +279,7 @@ def install_extension(
         "version": provider["version"],
         "revision": revision,
         "previous_revision": previous_revision,
-        "enabled": True if execute else None,
+        "enabled": enabled if execute else None,
         "doctor": doctor,
         "rollback_available": previous_revision is not None,
     }
@@ -420,6 +424,9 @@ def rollback_extension(
             f"extension `{extension_id}` rollback doctor is not ready: {doctor['status']}"
         )
     previous_revision = str(entry.get("active_revision") or "")
+    # Dry-run previews the enablement read before the probe; the execute path
+    # replaces it with the value committed under the lock below.
+    enabled = bool(entry.get("enabled"))
     if execute:
         with exclusive_file_lock(path):
             current_state = _read_state(path)
@@ -438,7 +445,11 @@ def rollback_extension(
             current_entry["active_revision"] = target_revision
             current_entry["rollback_revision"] = previous_revision
             _record_doctor(current_entry, target_revision, doctor)
-            current_entry["enabled"] = True
+            # Rollback switches the active revision only; it does not undo an
+            # operator's enable/disable decision, including one that completed
+            # while the probe ran. The result must report what was committed
+            # here, not the pre-probe snapshot.
+            enabled = bool(current_entry.get("enabled"))
             _write_state(path, current_state)
     return {
         "ok": True,
@@ -450,7 +461,7 @@ def rollback_extension(
         "version": target.get("version"),
         "revision": target_revision,
         "previous_revision": previous_revision,
-        "enabled": True if execute else bool(entry.get("enabled")),
+        "enabled": enabled,
         "doctor": doctor,
     }
 

tests/extensions/test_extension_runtime.py

@@ -1605,6 +1605,204 @@ def test_declared_manifest_for_an_installed_id_must_repeat_the_active_revision(
         extension_catalog_entries([v2], state_file=state_file)
 
 
+def test_upgrade_and_rollback_preserve_a_disabled_extension(tmp_path: Path) -> None:
+    """Changing the active revision must not undo an operator's disable.
+
+    Before this guard, both `upgrade` and `rollback` wrote `enabled: True`
+    unconditionally, so a provider the operator had switched off came back
+    on as a side effect of a revision change.
+    """
+    provider = _provider(tmp_path / "provider")
+    v1 = _manifest(tmp_path / "v1.toml", entrypoint=provider, version="1.0.0")
+    v2 = _manifest(tmp_path / "v2.toml", entrypoint=provider, version="2.0.0")
+    state_file = tmp_path / "runtime" / "extensions.json"
+
+    installed = install_extension(v1, state_file=state_file, execute=True)
+    assert installed["enabled"] is True
+    assert disable_extension(
+        "test-semantic-extension", state_file=state_file, execute=True
+    )["changed"] is True
+
+    upgraded = install_extension(
+        v2, state_file=state_file, operation="upgrade", execute=True
+    )
+    assert upgraded["enabled"] is False
+    assert upgraded["revision"] != installed["revision"]
+    entry = json.loads(state_file.read_text(encoding="utf-8"))["extensions"][
+        "test-semantic-extension"
+    ]
+    assert entry["enabled"] is False
+    assert entry["active_revision"] == upgraded["revision"]
+    with pytest.raises(ValueError, match="is disabled"):
+        resolve_extension_activation("test-semantic-extension", state_file=state_file)
+
+    rolled_back = rollback_extension(
+        "test-semantic-extension", state_file=state_file, execute=True
+    )
+    assert rolled_back["enabled"] is False
+    assert rolled_back["revision"] == installed["revision"]
+    entry = json.loads(state_file.read_text(encoding="utf-8"))["extensions"][
+        "test-semantic-extension"
+    ]
+    assert entry["enabled"] is False
+    assert extension_status(state_file=state_file)["extensions"][0]["enabled"] is False
+
+    # The explicit enable path remains the only way back on.
+    assert enable_extension(
+        "test-semantic-extension", state_file=state_file, execute=True
+    )["changed"] is True
+    assert resolve_extension_activation(
+        "test-semantic-extension", state_file=state_file
+    )["revision"] == installed["revision"]
+
+
+def test_upgrade_and_rollback_keep_an_enabled_extension_enabled(tmp_path: Path) -> None:
+    provider = _provider(tmp_path / "provider")
+    v1 = _manifest(tmp_path / "v1.toml", entrypoint=provider, version="1.0.0")
+    v2 = _manifest(tmp_path / "v2.toml", entrypoint=provider, version="2.0.0")
+    state_file = tmp_path / "runtime" / "extensions.json"
+
+    install_extension(v1, state_file=state_file, execute=True)
+    upgraded = install_extension(
+        v2, state_file=state_file, operation="upgrade", execute=True
+    )
+    assert upgraded["enabled"] is True
+    rolled_back = rollback_extension(
+        "test-semantic-extension", state_file=state_file, execute=True
+    )
+    assert rolled_back["enabled"] is True
+    assert extension_status(state_file=state_file)["extensions"][0]["enabled"] is True
+
+
+def _barrier_provider(path: Path, *, gate: Path, started: Path, release: Path) -> Path:
+    """A provider whose doctor blocks at an observable barrier.
+
+    Without ``gate`` the doctor passes at once, so install and upgrade are
+    unaffected. Once ``gate`` exists the doctor writes ``started`` and then
+    waits for ``release``, which lets a test complete another real CLI call
+    while the probe is in flight.
+    """
+    path.write_text(
+        f"""#!{sys.executable}
+import sys
+import time
+from pathlib import Path
+
+if "--doctor" not in sys.argv:
+    raise SystemExit(1)
+if Path({str(gate)!r}).exists():
+    Path({str(started)!r}).write_text("started", encoding="utf-8")
+    deadline = time.monotonic() + 30
+    while not Path({str(release)!r}).exists():
+        if time.monotonic() > deadline:
+            raise SystemExit(3)
+        time.sleep(0.02)
+raise SystemExit(0)
+""",
+        encoding="utf-8",
+    )
+    path.chmod(0o755)
+    return path
+
+
+@pytest.mark.parametrize(
+    ("concurrent_command", "enabled_before", "enabled_after"),
+    [("disable", True, False), ("enable", False, True)],
+)
+def test_rollback_result_reports_the_enablement_committed_under_the_lock(
+    tmp_path: Path,
+    concurrent_command: str,
+    enabled_before: bool,
+    enabled_after: bool,
+) -> None:
+    """An enable/disable that lands during the rollback probe is preserved
+    and reported.
+
+    The rollback doctor runs outside the state lock. The committed state
+    already kept the concurrent decision, but the result carried the
+    pre-probe snapshot, so a caller of the real CLI saw `enabled` disagree
+    with `extension list` and could not tell whether a follow-up
+    enable/disable was needed. Both directions use real `loopx.cli`
+    subprocesses and a doctor that blocks at a file barrier.
+    """
+    extension_id = "test-semantic-extension"
+    gate, started, release = (
+        tmp_path / name for name in ("gate", "doctor-started", "release")
+    )
+    blocking = _barrier_provider(
+        tmp_path / "blocking-provider", gate=gate, started=started, release=release
+    )
+    v1 = _manifest(tmp_path / "v1.toml", entrypoint=blocking, version="1.0.0")
+    v1.write_text(
+        v1.read_text(encoding="utf-8").replace(
+            "timeout_seconds = 5", "timeout_seconds = 60"
+        ),
+        encoding="utf-8",
+    )
+    # The active revision keeps an unblocked doctor so a concurrent `enable`
+    # (which probes the active revision) cannot wait on the same barrier.
+    v2 = _manifest(
+        tmp_path / "v2.toml", entrypoint=_provider(tmp_path / "plain-provider"),
+        version="2.0.0",
+    )
+    state_file = tmp_path / "runtime" / "extensions.json"
+    installed = install_extension(v1, state_file=state_file, execute=True)
+    upgraded = install_extension(
+        v2, state_file=state_file, operation="upgrade", execute=True
+    )
+    if not enabled_before:
+        disable_extension(extension_id, state_file=state_file, execute=True)
+
+    def cli(*arguments: str) -> list[str]:
+        return [
+            sys.executable, "-m", "loopx.cli", "--format", "json", "extension",
+            *arguments, "--state-file", str(state_file),
+        ]
+
+    gate.write_text("", encoding="utf-8")
+    with subprocess.Popen(
+        cli("rollback", extension_id, "--execute"),
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
+    ) as rollback:
+        try:
+            deadline = time.monotonic() + 30
+            while not started.exists():
+                assert rollback.poll() is None, rollback.communicate()[1]
+                assert time.monotonic() < deadline, "rollback doctor never reached the barrier"
+                time.sleep(0.02)
+            # The rollback is inside its probe and does not hold the lock.
+            concurrent = subprocess.run(
+                cli(concurrent_command, extension_id, "--execute"),
+                capture_output=True, text=True, check=False, timeout=60,
+            )
+            assert concurrent.returncode == 0, concurrent.stderr
+            concurrent_payload = json.loads(concurrent.stdout)
+            assert concurrent_payload["changed"] is True
+            assert concurrent_payload["enabled"] is enabled_after
+            assert concurrent_payload["active_revision"] == upgraded["revision"]
+        finally:
+            release.write_text("", encoding="utf-8")
+        stdout, stderr = rollback.communicate(timeout=60)
+    assert rollback.returncode == 0, stderr
+    payload = json.loads(stdout)
+    assert payload["ok"] is True
+    assert payload["operation"] == "rollback"
+    assert payload["revision"] == installed["revision"]
+    assert payload["previous_revision"] == upgraded["revision"]
+    assert payload["enabled"] is enabled_after
+
+    entry = json.loads(state_file.read_text(encoding="utf-8"))["extensions"][extension_id]
+    assert entry["active_revision"] == installed["revision"]
+    assert entry["enabled"] is enabled_after
+    listing = subprocess.run(
+        cli("list"), capture_output=True, text=True, check=False, timeout=60,
+    )
+    assert listing.returncode == 0, listing.stderr
+    [listed] = json.loads(listing.stdout)["extensions"]
+    assert listed["active_revision"] == installed["revision"]
+    assert listed["enabled"] is payload["enabled"] is enabled_after
+
+
 def test_failed_upgrade_keeps_the_active_revision(tmp_path: Path) -> None:
     ready = _provider(tmp_path / "ready-provider")
     broken = _provider(tmp_path / "broken-provider", doctor_exit=2)