From 16f31ce834b434a0a2d6e7a4c4c7a22aa3552c93 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Mon, 20 Jul 2026 20:50:32 +0530 Subject: [PATCH 1/8] UN-3636 [FEAT] Merge overlay manifests via UNSTRACT_RIG_EXTRA_MANIFESTS load_groups() now merges extra group manifests listed in the env var (os.pathsep-separated, REPO_ROOT-relative) onto the base tests/groups.yaml before validation, so cross-manifest depends_on and the platform-gate invariant are checked over the union. Name collisions are an error. Lets a downstream repo (the cloud build) contribute its own test groups by copying a groups.cloud.yaml into the merged tree, without editing the OSS manifest. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --- tests/rig/groups.py | 45 ++++++++++++++++++++++++++++ tests/rig/tests/test_groups.py | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/tests/rig/groups.py b/tests/rig/groups.py index 07b70283ce..62c345f564 100644 --- a/tests/rig/groups.py +++ b/tests/rig/groups.py @@ -9,6 +9,7 @@ from __future__ import annotations import graphlib +import os from collections.abc import Mapping from dataclasses import dataclass, field from pathlib import Path @@ -26,6 +27,11 @@ REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_MANIFEST = REPO_ROOT / "tests" / "groups.yaml" +# os.pathsep-separated overlay manifests merged on top of the base groups.yaml. +# Lets a downstream repo (e.g. the cloud build, which copies its plugins into +# this tree) contribute groups without editing the OSS manifest. +EXTRA_MANIFESTS_ENV = "UNSTRACT_RIG_EXTRA_MANIFESTS" + @dataclass(frozen=True) class GroupDefinition: @@ -127,6 +133,11 @@ def load_groups(path: Path | None = None) -> GroupManifest: for name, spec in (raw["groups"] or {}).items(): groups[name] = _build_group(name, spec, defaults) + # Overlay manifests are merged before validation, so cross-manifest + # `depends_on` and the platform-gate invariant are checked over the union. + for extra in _extra_manifest_paths(): + _merge_manifest(groups, extra, defaults) + _validate_no_cycles(groups) _validate_dep_targets_exist(groups) _validate_paths(groups) @@ -135,6 +146,40 @@ def load_groups(path: Path | None = None) -> GroupManifest: return GroupManifest(groups=groups) +def _extra_manifest_paths() -> list[Path]: + """Overlay manifest paths from ``UNSTRACT_RIG_EXTRA_MANIFESTS``. + + Relative paths resolve against ``REPO_ROOT`` so a downstream repo can point + at a manifest it copied into this tree (e.g. ``tests/groups.cloud.yaml``). + """ + raw = os.environ.get(EXTRA_MANIFESTS_ENV, "").strip() + if not raw: + return [] + paths: list[Path] = [] + for entry in filter(None, (e.strip() for e in raw.split(os.pathsep))): + p = Path(entry) + paths.append(p if p.is_absolute() else REPO_ROOT / p) + return paths + + +def _merge_manifest( + groups: dict[str, GroupDefinition], manifest_path: Path, base_defaults: dict[str, Any] +) -> None: + """Merge an overlay manifest into ``groups`` in place. Overlay groups inherit + the base ``defaults`` unless they declare their own; a name collision is an + error rather than a silent override.""" + raw = yaml.safe_load(manifest_path.read_text()) + if not isinstance(raw, dict) or "groups" not in raw: + raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") + defaults = {**base_defaults, **(raw.get("defaults") or {})} + for name, spec in (raw["groups"] or {}).items(): + if name in groups: + raise ValueError( + f"{manifest_path}: group {name!r} already defined in a prior manifest" + ) + groups[name] = _build_group(name, spec, defaults) + + def _build_group( name: str, spec: dict[str, Any], defaults: dict[str, Any] ) -> GroupDefinition: diff --git a/tests/rig/tests/test_groups.py b/tests/rig/tests/test_groups.py index eabd5b7f05..8ca2c8409d 100644 --- a/tests/rig/tests/test_groups.py +++ b/tests/rig/tests/test_groups.py @@ -232,3 +232,58 @@ def test_group_env_is_frozen() -> None: assert group.env["A"] == "1" with pytest.raises(TypeError): group.env["B"] = "2" # type: ignore[index] + + +def _base_manifest(tmp_path: Path) -> Path: + return _write_manifest( + tmp_path, + """ + version: 1 + groups: + unit-base: + tier: unit + paths: [x] + optional: true + """, + ) + + +def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + base = _base_manifest(tmp_path) + overlay = tmp_path / "groups.cloud.yaml" + overlay.write_text( + """ + version: 1 + groups: + unit-cloud: + tier: unit + paths: [y] + depends_on: [unit-base] + optional: true + """ + ) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) + manifest = load_groups(base) + assert "unit-cloud" in manifest.names() + # Cross-manifest depends_on resolves against the merged set. + assert "unit-base" in manifest.expand(["unit-cloud"]) + + +def test_extra_manifest_name_collision_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + base = _base_manifest(tmp_path) + overlay = tmp_path / "groups.cloud.yaml" + overlay.write_text( + """ + version: 1 + groups: + unit-base: + tier: unit + paths: [y] + optional: true + """ + ) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) + with pytest.raises(ValueError, match="already defined"): + load_groups(base) From b6da5a69fc822a5f580699b3f8ad7f2c00fbe27c Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Tue, 21 Jul 2026 19:10:36 +0530 Subject: [PATCH 2/8] UN-3636 [FIX] Skip org lookup when no org in context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserContext.get_organization() ran Organization.objects.get() even with no org id in StateStore (import time, or management commands with no request), catching only DoesNotExist/ProgrammingError — a DB-less/unmigrated setup hit an uncaught OperationalError. Short-circuit when there's no org id: no query, so serializers/managers that reference org-scoped querysets at class-def can be imported during DB-free test collection. --- backend/utils/tests/test_user_context.py | 30 ++++++++++++++++++++++++ backend/utils/user_context.py | 4 ++++ 2 files changed, 34 insertions(+) create mode 100644 backend/utils/tests/test_user_context.py diff --git a/backend/utils/tests/test_user_context.py b/backend/utils/tests/test_user_context.py new file mode 100644 index 0000000000..72e12a192e --- /dev/null +++ b/backend/utils/tests/test_user_context.py @@ -0,0 +1,30 @@ +"""Regression tests for utils.user_context.UserContext.get_organization. + +Pins the no-org short-circuit: with no organization in context (import time, +management commands with no request), the org lookup must return None without +touching the DB, so evaluating it on a DB-less/unmigrated setup can't fail. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from utils.user_context import UserContext + + +class TestGetOrganizationNoContext: + def test_returns_none_without_hitting_db(self): + with ( + patch("utils.user_context.StateStore.get", return_value=None), + patch("utils.user_context.Organization.objects.get") as mock_get, + ): + assert UserContext.get_organization() is None + mock_get.assert_not_called() + + def test_empty_identifier_short_circuits(self): + with ( + patch("utils.user_context.StateStore.get", return_value=""), + patch("utils.user_context.Organization.objects.get") as mock_get, + ): + assert UserContext.get_organization() is None + mock_get.assert_not_called() diff --git a/backend/utils/user_context.py b/backend/utils/user_context.py index 71d63806b5..df72e12136 100644 --- a/backend/utils/user_context.py +++ b/backend/utils/user_context.py @@ -18,6 +18,10 @@ def set_organization_identifier(organization_identifier: str) -> None: @staticmethod def get_organization() -> Organization | None: organization_id = StateStore.get(Account.ORGANIZATION_ID) + # No org in context (import time, or management commands with no request): + # skip the query so evaluating this on a DB-less/unmigrated setup can't fail. + if not organization_id: + return None try: organization: Organization = Organization.objects.get( organization_id=organization_id From d48d1dcd863a3c0f60e2726d9a05fff618f52e86 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Jul 2026 13:13:56 +0530 Subject: [PATCH 3/8] UN-3636 [FIX] Scope overlay manifests to the default manifest Address review feedback on the UNSTRACT_RIG_EXTRA_MANIFESTS overlay: - Overlays now apply only when loading the default manifest, so an explicit `load_groups(path)` (test fixture, ad-hoc manifest) can no longer absorb a downstream repo's ambient overlay. - `_merge_manifest` returns the merged defaults so an overlay can rename `platform_gate_group` instead of having it silently ignored. - A bad path in the env var raises a ValueError naming the variable rather than a bare FileNotFoundError/IsADirectoryError. - Extract `_load_manifest_dict` to single-source manifest parsing and its error message. - Tests: drive the real default-manifest path; cover overlay isolation, overlay defaults, malformed overlay, and a missing overlay path. - Pin the truthy branch of UserContext.get_organization so inverting the guard can't pass unnoticed. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --- backend/utils/tests/test_user_context.py | 12 +++ tests/rig/groups.py | 42 ++++++++--- tests/rig/tests/test_groups.py | 93 ++++++++++++++++++++++-- 3 files changed, 127 insertions(+), 20 deletions(-) diff --git a/backend/utils/tests/test_user_context.py b/backend/utils/tests/test_user_context.py index 72e12a192e..cceffc6466 100644 --- a/backend/utils/tests/test_user_context.py +++ b/backend/utils/tests/test_user_context.py @@ -28,3 +28,15 @@ def test_empty_identifier_short_circuits(self): ): assert UserContext.get_organization() is None mock_get.assert_not_called() + + def test_identifier_present_looks_up_organization(self): + """Pins the complement, so inverting the guard can't pass unnoticed.""" + sentinel = object() + with ( + patch("utils.user_context.StateStore.get", return_value="org-123"), + patch( + "utils.user_context.Organization.objects.get", return_value=sentinel + ) as mock_get, + ): + assert UserContext.get_organization() is sentinel + mock_get.assert_called_once_with(organization_id="org-123") diff --git a/tests/rig/groups.py b/tests/rig/groups.py index 62c345f564..2e5309dbb0 100644 --- a/tests/rig/groups.py +++ b/tests/rig/groups.py @@ -124,9 +124,7 @@ def expand(self, selected: list[str]) -> list[str]: def load_groups(path: Path | None = None) -> GroupManifest: """Parse the YAML manifest and validate it.""" manifest_path = path or DEFAULT_MANIFEST - raw = yaml.safe_load(manifest_path.read_text()) - if not isinstance(raw, dict) or "groups" not in raw: - raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") + raw = _load_manifest_dict(manifest_path) defaults = raw.get("defaults") or {} groups: dict[str, GroupDefinition] = {} @@ -135,8 +133,11 @@ def load_groups(path: Path | None = None) -> GroupManifest: # Overlay manifests are merged before validation, so cross-manifest # `depends_on` and the platform-gate invariant are checked over the union. - for extra in _extra_manifest_paths(): - _merge_manifest(groups, extra, defaults) + # Only the default manifest takes overlays — an explicit `path` (test fixture + # or ad-hoc manifest) must stay isolated from the ambient env var. + if manifest_path == DEFAULT_MANIFEST: + for extra in _extra_manifest_paths(): + defaults = _merge_manifest(groups, extra, defaults) _validate_no_cycles(groups) _validate_dep_targets_exist(groups) @@ -158,19 +159,35 @@ def _extra_manifest_paths() -> list[Path]: paths: list[Path] = [] for entry in filter(None, (e.strip() for e in raw.split(os.pathsep))): p = Path(entry) - paths.append(p if p.is_absolute() else REPO_ROOT / p) + p = p if p.is_absolute() else REPO_ROOT / p + # Name the env var here: a bare FileNotFoundError from read_text() gives + # no hint about where the bad path came from. + if not p.is_file(): + raise ValueError( + f"{EXTRA_MANIFESTS_ENV}: {entry!r} is not a file (resolved to {p})" + ) + paths.append(p) return paths -def _merge_manifest( - groups: dict[str, GroupDefinition], manifest_path: Path, base_defaults: dict[str, Any] -) -> None: - """Merge an overlay manifest into ``groups`` in place. Overlay groups inherit - the base ``defaults`` unless they declare their own; a name collision is an - error rather than a silent override.""" +def _load_manifest_dict(manifest_path: Path) -> dict[str, Any]: + """Parse a manifest YAML, rejecting anything that is not a ``groups:`` mapping.""" + if not manifest_path.is_file(): + raise ValueError(f"{manifest_path}: manifest file not found") raw = yaml.safe_load(manifest_path.read_text()) if not isinstance(raw, dict) or "groups" not in raw: raise ValueError(f"{manifest_path}: expected top-level `groups:` mapping") + return raw + + +def _merge_manifest( + groups: dict[str, GroupDefinition], manifest_path: Path, base_defaults: dict[str, Any] +) -> dict[str, Any]: + """Merge an overlay manifest into ``groups`` in place and return the merged + defaults, so an overlay can also rename the platform gate. Overlay groups + inherit the base ``defaults`` unless they declare their own; a name collision + is an error rather than a silent override.""" + raw = _load_manifest_dict(manifest_path) defaults = {**base_defaults, **(raw.get("defaults") or {})} for name, spec in (raw["groups"] or {}).items(): if name in groups: @@ -178,6 +195,7 @@ def _merge_manifest( f"{manifest_path}: group {name!r} already defined in a prior manifest" ) groups[name] = _build_group(name, spec, defaults) + return defaults def _build_group( diff --git a/tests/rig/tests/test_groups.py b/tests/rig/tests/test_groups.py index 8ca2c8409d..ba362bae41 100644 --- a/tests/rig/tests/test_groups.py +++ b/tests/rig/tests/test_groups.py @@ -248,10 +248,26 @@ def _base_manifest(tmp_path: Path) -> Path: ) -def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +def _overlay_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, body: str +) -> Path: + """Point the loader's default manifest at a fixture and register an overlay. + + Overlays only apply to the default manifest, so the tests drive that path + rather than passing an explicit one. + """ base = _base_manifest(tmp_path) overlay = tmp_path / "groups.cloud.yaml" - overlay.write_text( + overlay.write_text(body) + monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) + return base + + +def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _overlay_env( + tmp_path, + monkeypatch, """ version: 1 groups: @@ -260,10 +276,9 @@ def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) paths: [y] depends_on: [unit-base] optional: true - """ + """, ) - monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) - manifest = load_groups(base) + manifest = load_groups() assert "unit-cloud" in manifest.names() # Cross-manifest depends_on resolves against the merged set. assert "unit-base" in manifest.expand(["unit-cloud"]) @@ -272,18 +287,80 @@ def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) def test_extra_manifest_name_collision_rejected( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + groups: + unit-base: + tier: unit + paths: [y] + optional: true + """, + ) + with pytest.raises(ValueError, match="already defined"): + load_groups() + + +def test_explicit_manifest_ignores_overlays( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An ad-hoc manifest must not absorb a downstream repo's overlay.""" base = _base_manifest(tmp_path) overlay = tmp_path / "groups.cloud.yaml" overlay.write_text( """ version: 1 groups: - unit-base: + unit-cloud: tier: unit paths: [y] optional: true """ ) monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay)) - with pytest.raises(ValueError, match="already defined"): - load_groups(base) + # `base` is deliberately not the default manifest here. + assert "unit-cloud" not in load_groups(base).names() + + +def test_extra_manifest_defaults_applied( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An overlay's own `defaults` reach its groups instead of being dropped.""" + _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + defaults: + parallel: false + timeout_seconds: 42 + groups: + unit-cloud: + tier: unit + paths: [y] + optional: true + """, + ) + group = load_groups().get("unit-cloud") + assert group.parallel is False + assert group.timeout_seconds == 42 + + +def test_extra_manifest_malformed_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _overlay_env(tmp_path, monkeypatch, "- not: a-mapping\n") + with pytest.raises(ValueError, match="expected top-level `groups:` mapping"): + load_groups() + + +def test_extra_manifest_missing_path_names_env_var( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + base = _base_manifest(tmp_path) + monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base) + monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(tmp_path / "nope.yaml")) + with pytest.raises(ValueError, match="UNSTRACT_RIG_EXTRA_MANIFESTS"): + load_groups() From 793b32f0cd2db9ae94833d287739c66c3c5080eb Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Jul 2026 13:15:40 +0530 Subject: [PATCH 4/8] UN-3636 [FIX] Stop re-running cross-tier deps in every tier leg `--tier X` expanded each selected group's `depends_on` transitively, with no tier bound. `integration-workflow-execution` and `e2e-smoke` both declare `depends_on: [unit-sdk1, unit-workers]`, so those two unit groups ran again in the integration leg and a third time in the e2e leg. Tiers run as separate CI legs and the unit leg already covers them, so dep expansion is now bounded to the requested tier. Explicitly named groups are never dropped, and intra-tier deps (e2e-smoke -> e2e-login) still expand and order as before. Unrun deps do not weaken gating: `blocked_by` intersects with groups that failed in the same run. Measured on the last main run: ~88s of unit-workers and ~48s of unit-sdk1 re-executed per run. On the cloud CI runner the same duplication costs ~350s. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --- tests/rig/selection.py | 17 ++++++++++- tests/rig/tests/test_selection.py | 48 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/tests/rig/selection.py b/tests/rig/selection.py index a1daac001a..e67945d0dd 100644 --- a/tests/rig/selection.py +++ b/tests/rig/selection.py @@ -7,6 +7,10 @@ The literal ``all`` expands to every group in the manifest. When the result is empty, callers should treat that as "do nothing" rather than silently running everything — the CLI surfaces a clear error. + +With ``--tier``, dep expansion never reaches outside that tier: each tier runs as +its own CI leg, so a cross-tier ``depends_on`` would otherwise re-run the same +group in every leg that transitively depends on it. """ from __future__ import annotations @@ -66,7 +70,18 @@ def resolve( if changed_only: selected.update(_groups_for_changed_paths(manifest, base=changed_base)) - return manifest.expand(sorted(selected)) if selected else [] + if not selected: + return [] + + requested = sorted(selected) + expanded = manifest.expand(requested) + if tier is not None: + # A tier run never pulls in another tier's groups. Tiers run as separate + # legs, so a cross-tier `depends_on` would re-run the same group once per + # leg; the owning tier's leg covers it. Explicitly requested groups stay. + keep = set(requested) + expanded = [n for n in expanded if n in keep or manifest.get(n).tier == tier] + return expanded def _groups_for_changed_paths(manifest: GroupManifest, *, base: str) -> set[str]: diff --git a/tests/rig/tests/test_selection.py b/tests/rig/tests/test_selection.py index d0ee7ef410..468a66667d 100644 --- a/tests/rig/tests/test_selection.py +++ b/tests/rig/tests/test_selection.py @@ -76,3 +76,51 @@ def test_tier_only_selects_that_tier_and_deps(tmp_path: Path) -> None: manifest = load_groups(_manifest(tmp_path)) ordered = resolve(manifest, positional=[], tier="unit") assert set(ordered) == {"unit-a", "unit-b"} + + +def _cross_tier_manifest(tmp_path: Path) -> Path: + p = tmp_path / "groups.yaml" + p.write_text( + """ + version: 1 + groups: + unit-a: + tier: unit + paths: [x] + optional: true + e2e-smoke: + tier: e2e + paths: [x] + requires_platform: true + optional: true + e2e-cross: + tier: e2e + paths: [x] + requires_platform: true + depends_on: [e2e-smoke, unit-a] + optional: true + """ + ) + return p + + +def test_tier_run_does_not_pull_in_other_tiers(tmp_path: Path) -> None: + """Each tier is its own CI leg; a cross-tier dep must not re-run there.""" + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=[], tier="e2e") + assert set(ordered) == {"e2e-smoke", "e2e-cross"} + # Intra-tier ordering still holds. + assert ordered.index("e2e-smoke") < ordered.index("e2e-cross") + + +def test_explicitly_named_group_survives_tier_filter(tmp_path: Path) -> None: + """The filter only drops dep-expanded groups, never an explicit request.""" + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=["unit-a"], tier="e2e") + assert set(ordered) == {"unit-a", "e2e-smoke", "e2e-cross"} + + +def test_cross_tier_dep_still_expands_without_tier_filter(tmp_path: Path) -> None: + manifest = load_groups(_cross_tier_manifest(tmp_path)) + ordered = resolve(manifest, positional=["e2e-cross"]) + assert set(ordered) == {"unit-a", "e2e-smoke", "e2e-cross"} From da2a5cec378e2b48620e3a1cbd05f30bb96d390d Mon Sep 17 00:00:00 2001 From: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:09:11 +0530 Subject: [PATCH 5/8] UN-3636 [MISC] Drop the ENVIRONMENT gate on the LLM mock (#2191) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * UN-3636 [FIX] Drop the ENVIRONMENT gate on the LLM mock It did not defend the case it was added for. The threat was a worker env block copied out of the test overlay into a real deployment, but the gate was written into that same block, so a copy carries it. Base compose also sets ENVIRONMENT=development on both workers that run the injection, so any deployment derived from it satisfied the gate regardless. That left one real case -- the mock var set alone somewhere that sets no ENVIRONMENT at all -- which holds by accident rather than design, in exchange for depending on a variable nothing else in the codebase reads. What actually guards the hatch is unchanged: it is off unless someone sets UNSTRACT_LLM_MOCK_RESPONSE, and it warns once per process while active. Making mocked spend distinguishable downstream is the defence worth having, and it belongs on the usage record rather than in a config check. Co-Authored-By: Claude Opus 4.8 * UN-3636 [MISC] Drop the unread ENVIRONMENT variable from compose Nothing reads it: no service, worker, frontend or plugin looks the variable up, and the one consumer it ever had — the LLM mock gate — was removed in the previous commit. Dropping it everywhere keeps a dead knob from looking load-bearing to the next reader. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Ryg9chVDJQggCybpq3YoY3 --------- Co-authored-by: Claude Opus 4.8 --- docker/docker-compose.yaml | 15 -------- tests/README.md | 2 +- tests/compose/docker-compose.test.yaml | 12 +----- unstract/sdk1/src/unstract/sdk1/llm.py | 21 ----------- unstract/sdk1/tests/test_mock_response.py | 46 ----------------------- 5 files changed, 2 insertions(+), 94 deletions(-) diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index aa36cbf961..e85b9bf22c 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -39,7 +39,6 @@ services: - ./workflow_data:/data - ${TOOL_REGISTRY_CONFIG_SRC_PATH}:/data/tool_registry_config environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-backend labels: - traefik.enable=true @@ -60,7 +59,6 @@ services: - rabbitmq - db environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-metrics labels: @@ -83,7 +81,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-ide-callback - WORKER_TYPE=ide_callback - WORKER_NAME=ide-callback-worker @@ -107,7 +104,6 @@ services: ports: - "5555:5555" environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-celery-flower volumes: - unstract_data:/data @@ -129,7 +125,6 @@ services: - db - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-celery-beat # Frontend React app @@ -143,7 +138,6 @@ services: - backend - reverse-proxy environment: - - ENVIRONMENT=development # Running platform version, surfaced on the profile page. Reuses the same # ${VERSION} that tags the image, so it always matches what's deployed # (and updates on RC->stable promotion, which only retags — doesn't rebuild). @@ -219,7 +213,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-api-deployment-v2 - WORKER_TYPE=api_deployment - CELERY_QUEUES_API_DEPLOYMENT=${CELERY_QUEUES_API_DEPLOYMENT:-celery_api_deployments} @@ -252,7 +245,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-callback-v2 - WORKER_TYPE=callback - WORKER_NAME=callback-worker-v2 @@ -294,7 +286,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-file-processing-v2 - WORKER_TYPE=file_processing - WORKER_MODE=oss @@ -332,7 +323,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-general-v2 - WORKER_TYPE=general - WORKER_NAME=general-worker-v2 @@ -360,7 +350,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-notification-v2 - WORKER_TYPE=notification - WORKER_NAME=notification-worker-v2 @@ -409,7 +398,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-log-consumer-v2 - WORKER_TYPE=log_consumer - WORKER_NAME=log-consumer-worker-v2 @@ -458,7 +446,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-log-history-scheduler-v2 # Scheduler interval in seconds - LOG_HISTORY_CONSUMER_INTERVAL=${LOG_HISTORY_CONSUMER_INTERVAL:-5} @@ -483,7 +470,6 @@ services: - redis - rabbitmq environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-scheduler-v2 - WORKER_TYPE=scheduler - WORKER_NAME=scheduler-worker-v2 @@ -529,7 +515,6 @@ services: - rabbitmq - platform-service environment: - - ENVIRONMENT=development - APPLICATION_NAME=unstract-worker-executor-v2 - WORKER_TYPE=executor - WORKER_NAME=executor-worker-v2 diff --git a/tests/README.md b/tests/README.md index 982b12f553..3628cd82c1 100644 --- a/tests/README.md +++ b/tests/README.md @@ -158,7 +158,7 @@ The `platform` pytest fixture in `tests/e2e/conftest.py` reads those env vars; e Execute-path e2e tests must not call a real provider, so the rig sets `UNSTRACT_LLM_MOCK_RESPONSE` (default `MOCK_LLM_OK`) before boot, for any runtime and treating an exported empty string as unset. The test overlay forwards it into the workers, and `unstract.sdk1.llm` passes it to litellm as `mock_response`: for the non-streaming completion path litellm returns the string verbatim with fixed usage (10 prompt / 20 completion / 30 total), so both the answer and the token counts are exact-assertable. Streaming (`stream_complete`) goes through a different litellm path whose usage differs, so don't assume 10/20/30 there. Sentinels like `litellm.RateLimitError` force error paths. Unset (the production default) the hook is a no-op. -Mocking needs two conditions, not one: `ENVIRONMENT` must also be `test` or `development`. Production sets neither variable, so a stray mock var alone cannot fake completions and their billing — and a refusal is logged rather than silent, since setting the var at all means someone expected mocking. `development` is allowed because that is what base compose sets on the workers that run the injection; the test overlay pins `ENVIRONMENT=test` on those same two workers explicitly, so the tier can't lose its mock to a base-compose edit. +The variable being set is the only condition, deliberately. A second gate on `ENVIRONMENT` was tried and removed: it did not defend against the case it was written for — a worker env block copied out of this overlay carries both variables together — and compose declared `ENVIRONMENT` on every service anyway, so any deployment derived from it satisfied the gate. Nothing read that variable, so it was dropped everywhere along with the gate. What remains is that the hatch is off unless someone sets it, and that it warns once per process while active. Making fake spend safe to *detect* rather than trying to prevent the config wants the usage record itself to say it was mocked. A CI/dev override wins (the rig only fills an unset value). Running these tests under the rig **fails** if the var is missing; running **without** the rig just skips the execute-path tests — export it on both sides (your shell and the workers) if you boot the stack yourself. diff --git a/tests/compose/docker-compose.test.yaml b/tests/compose/docker-compose.test.yaml index feb46d6eda..8b18a3ca4b 100644 --- a/tests/compose/docker-compose.test.yaml +++ b/tests/compose/docker-compose.test.yaml @@ -7,33 +7,23 @@ services: # contributor run without setting it. image: unstract/backend:${UNSTRACT_TEST_VERSION:-latest} environment: - - ENVIRONMENT=test # Workers ask the backend for this, so this is the value that normally # takes effect for fan-out. - MAX_PARALLEL_FILE_BATCHES=${MAX_PARALLEL_FILE_BATCHES:-3} platform-service: image: unstract/platform-service:${UNSTRACT_TEST_VERSION:-latest} - environment: - - ENVIRONMENT=test runner: image: unstract/runner:${UNSTRACT_TEST_VERSION:-latest} - environment: - - ENVIRONMENT=test - # Execute-path e2e must never reach a real provider. The delay gives each - # mocked completion a known cost so per-file durations stay comparable, and - # ENVIRONMENT is the mock's second condition — set here rather than inherited - # so the tier can't lose its mock to a base-compose edit. + # Execute-path e2e must never reach a real provider. worker-executor-v2: environment: - - ENVIRONMENT=test - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} worker-file-processing-v2: environment: - - ENVIRONMENT=test - UNSTRACT_LLM_MOCK_RESPONSE=${UNSTRACT_LLM_MOCK_RESPONSE:-} # Only the worker's fallback if it can't reach the backend; kept in step so it diff --git a/unstract/sdk1/src/unstract/sdk1/llm.py b/unstract/sdk1/src/unstract/sdk1/llm.py index 1befc96eb3..b0a712b49f 100644 --- a/unstract/sdk1/src/unstract/sdk1/llm.py +++ b/unstract/sdk1/src/unstract/sdk1/llm.py @@ -35,10 +35,6 @@ # Lets tests force a deterministic completion without a provider or a secret. # Unset in production, where this is a no-op. _MOCK_RESPONSE_ENV = "UNSTRACT_LLM_MOCK_RESPONSE" -# Second condition on the hatch, so a stray mock var alone can't fake -# completions and their billing. Deployments that set neither fail closed. -_ENVIRONMENT_ENV = "ENVIRONMENT" -_MOCK_ALLOWED_ENVIRONMENTS = frozenset({"test", "development"}) @lru_cache(maxsize=1) @@ -52,27 +48,10 @@ def _warn_mock_active() -> None: ) -@lru_cache(maxsize=1) -def _warn_mock_refused(environment: str) -> None: - # Loud rather than silent: the var being set at all means someone expected - # mocking, and they need to know why the bill is real. - logger.warning( - "%s is set but %s=%r is not one of %s — calling the provider for real.", - _MOCK_RESPONSE_ENV, - _ENVIRONMENT_ENV, - environment, - sorted(_MOCK_ALLOWED_ENVIRONMENTS), - ) - - def _inject_mock_response(completion_kwargs: dict[str, object]) -> None: mock = os.getenv(_MOCK_RESPONSE_ENV) if not mock or "mock_response" in completion_kwargs: return - environment = os.getenv(_ENVIRONMENT_ENV, "").strip().lower() - if environment not in _MOCK_ALLOWED_ENVIRONMENTS: - _warn_mock_refused(environment) - return _warn_mock_active() completion_kwargs["mock_response"] = mock diff --git a/unstract/sdk1/tests/test_mock_response.py b/unstract/sdk1/tests/test_mock_response.py index 6a4e0b1a59..807d7d05f1 100644 --- a/unstract/sdk1/tests/test_mock_response.py +++ b/unstract/sdk1/tests/test_mock_response.py @@ -38,13 +38,6 @@ def _inject(kwargs: dict[str, object]) -> dict[str, object]: @pytest.fixture(autouse=True) def _reset_warn_cache() -> None: _load_llm_module()._warn_mock_active.cache_clear() - _load_llm_module()._warn_mock_refused.cache_clear() - - -@pytest.fixture(autouse=True) -def _allowed_environment(monkeypatch: pytest.MonkeyPatch) -> None: - """The hatch needs a permitted ENVIRONMENT; the guard itself is tested below.""" - monkeypatch.setenv("ENVIRONMENT", "test") def test_inject_is_noop_when_env_unset(monkeypatch: pytest.MonkeyPatch) -> None: @@ -67,45 +60,6 @@ def test_inject_sets_mock_response_when_env_set( assert _inject({"model": "gpt-4o"})["mock_response"] == "canned answer" -@pytest.mark.parametrize("environment", ["production", "staging", "", " "]) -def test_mock_refused_outside_allowed_environments( - monkeypatch: pytest.MonkeyPatch, environment: str -) -> None: - # The whole point of the guard: a stray mock var in a real deployment must - # not fake completions and their billing. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", environment) - assert _inject({"model": "gpt-4o"}) == {"model": "gpt-4o"} - - -def test_mock_refused_when_environment_unset(monkeypatch: pytest.MonkeyPatch) -> None: - # Production k8s sets no ENVIRONMENT at all, so unset must fail closed. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.delenv("ENVIRONMENT", raising=False) - assert _inject({"model": "gpt-4o"}) == {"model": "gpt-4o"} - - -def test_refusal_is_warned_so_a_real_bill_is_never_silent( - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture -) -> None: - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", "production") - with caplog.at_level("WARNING", logger="unstract.sdk1.llm"): - _inject({"model": "gpt-4o"}) - assert any("not one of" in r.message for r in caplog.records), caplog.text - - -@pytest.mark.parametrize("environment", ["test", "development", "TEST", " Development "]) -def test_mock_applies_in_allowed_environments( - monkeypatch: pytest.MonkeyPatch, environment: str -) -> None: - # `development` is what base compose sets on the workers that run the - # injection; a guard that only accepted `test` would kill the local stack. - monkeypatch.setenv("UNSTRACT_LLM_MOCK_RESPONSE", "canned") - monkeypatch.setenv("ENVIRONMENT", environment) - assert _inject({"model": "gpt-4o"})["mock_response"] == "canned" - - def test_inject_does_not_clobber_explicit_mock_response( monkeypatch: pytest.MonkeyPatch, ) -> None: From d23587d6f2b9e5d3160f101700db89d0205ae445 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Jul 2026 16:17:30 +0530 Subject: [PATCH 6/8] UN-3636 [MISC] Tighten code comments Drop lines that restate the code, trim session-specific detail, and merge comments that duplicated each other across a module and its test. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --- backend/utils/tests/test_user_context.py | 7 +++---- backend/utils/user_context.py | 3 +-- tests/rig/groups.py | 16 +++++++--------- tests/rig/selection.py | 9 +++------ tests/rig/tests/test_groups.py | 5 ++--- tests/rig/tests/test_selection.py | 1 - 6 files changed, 16 insertions(+), 25 deletions(-) diff --git a/backend/utils/tests/test_user_context.py b/backend/utils/tests/test_user_context.py index cceffc6466..09d2c82d0a 100644 --- a/backend/utils/tests/test_user_context.py +++ b/backend/utils/tests/test_user_context.py @@ -1,8 +1,7 @@ -"""Regression tests for utils.user_context.UserContext.get_organization. +"""Regression tests for UserContext.get_organization. -Pins the no-org short-circuit: with no organization in context (import time, -management commands with no request), the org lookup must return None without -touching the DB, so evaluating it on a DB-less/unmigrated setup can't fail. +Pins the no-org short-circuit: the lookup must return None without touching the +DB, so it stays evaluable on a DB-less/unmigrated setup. """ from __future__ import annotations diff --git a/backend/utils/user_context.py b/backend/utils/user_context.py index df72e12136..e0f2d4fc12 100644 --- a/backend/utils/user_context.py +++ b/backend/utils/user_context.py @@ -18,8 +18,7 @@ def set_organization_identifier(organization_identifier: str) -> None: @staticmethod def get_organization() -> Organization | None: organization_id = StateStore.get(Account.ORGANIZATION_ID) - # No org in context (import time, or management commands with no request): - # skip the query so evaluating this on a DB-less/unmigrated setup can't fail. + # Skip the query so this stays evaluable on a DB-less/unmigrated setup. if not organization_id: return None try: diff --git a/tests/rig/groups.py b/tests/rig/groups.py index 2e5309dbb0..bee525d590 100644 --- a/tests/rig/groups.py +++ b/tests/rig/groups.py @@ -27,9 +27,8 @@ REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_MANIFEST = REPO_ROOT / "tests" / "groups.yaml" -# os.pathsep-separated overlay manifests merged on top of the base groups.yaml. -# Lets a downstream repo (e.g. the cloud build, which copies its plugins into -# this tree) contribute groups without editing the OSS manifest. +# os.pathsep-separated overlay manifests, so a downstream repo can contribute +# groups without editing the base manifest. EXTRA_MANIFESTS_ENV = "UNSTRACT_RIG_EXTRA_MANIFESTS" @@ -131,10 +130,9 @@ def load_groups(path: Path | None = None) -> GroupManifest: for name, spec in (raw["groups"] or {}).items(): groups[name] = _build_group(name, spec, defaults) - # Overlay manifests are merged before validation, so cross-manifest - # `depends_on` and the platform-gate invariant are checked over the union. - # Only the default manifest takes overlays — an explicit `path` (test fixture - # or ad-hoc manifest) must stay isolated from the ambient env var. + # Merge before validation so cross-manifest `depends_on` and the platform + # gate are checked over the union. An explicit `path` stays isolated from + # the ambient env var. if manifest_path == DEFAULT_MANIFEST: for extra in _extra_manifest_paths(): defaults = _merge_manifest(groups, extra, defaults) @@ -160,8 +158,8 @@ def _extra_manifest_paths() -> list[Path]: for entry in filter(None, (e.strip() for e in raw.split(os.pathsep))): p = Path(entry) p = p if p.is_absolute() else REPO_ROOT / p - # Name the env var here: a bare FileNotFoundError from read_text() gives - # no hint about where the bad path came from. + # Name the env var — a bare FileNotFoundError won't say where the path + # came from. if not p.is_file(): raise ValueError( f"{EXTRA_MANIFESTS_ENV}: {entry!r} is not a file (resolved to {p})" diff --git a/tests/rig/selection.py b/tests/rig/selection.py index e67945d0dd..518330767d 100644 --- a/tests/rig/selection.py +++ b/tests/rig/selection.py @@ -8,9 +8,8 @@ empty, callers should treat that as "do nothing" rather than silently running everything — the CLI surfaces a clear error. -With ``--tier``, dep expansion never reaches outside that tier: each tier runs as -its own CI leg, so a cross-tier ``depends_on`` would otherwise re-run the same -group in every leg that transitively depends on it. +With ``--tier``, dep expansion stays inside that tier: tiers run as separate CI +legs, so a cross-tier ``depends_on`` would re-run the same group in every leg. """ from __future__ import annotations @@ -76,9 +75,7 @@ def resolve( requested = sorted(selected) expanded = manifest.expand(requested) if tier is not None: - # A tier run never pulls in another tier's groups. Tiers run as separate - # legs, so a cross-tier `depends_on` would re-run the same group once per - # leg; the owning tier's leg covers it. Explicitly requested groups stay. + # Only dep-expanded groups are dropped; an explicit request survives. keep = set(requested) expanded = [n for n in expanded if n in keep or manifest.get(n).tier == tier] return expanded diff --git a/tests/rig/tests/test_groups.py b/tests/rig/tests/test_groups.py index ba362bae41..37fcbbc81c 100644 --- a/tests/rig/tests/test_groups.py +++ b/tests/rig/tests/test_groups.py @@ -251,10 +251,9 @@ def _base_manifest(tmp_path: Path) -> Path: def _overlay_env( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, body: str ) -> Path: - """Point the loader's default manifest at a fixture and register an overlay. + """Point the default manifest at a fixture and register an overlay. - Overlays only apply to the default manifest, so the tests drive that path - rather than passing an explicit one. + Overlays only apply to the default manifest, so tests must drive that path. """ base = _base_manifest(tmp_path) overlay = tmp_path / "groups.cloud.yaml" diff --git a/tests/rig/tests/test_selection.py b/tests/rig/tests/test_selection.py index 468a66667d..4007c1963c 100644 --- a/tests/rig/tests/test_selection.py +++ b/tests/rig/tests/test_selection.py @@ -109,7 +109,6 @@ def test_tier_run_does_not_pull_in_other_tiers(tmp_path: Path) -> None: manifest = load_groups(_cross_tier_manifest(tmp_path)) ordered = resolve(manifest, positional=[], tier="e2e") assert set(ordered) == {"e2e-smoke", "e2e-cross"} - # Intra-tier ordering still holds. assert ordered.index("e2e-smoke") < ordered.index("e2e-cross") From 8d1442d07e377a87daa59ab2e161aa1442132a08 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Jul 2026 16:31:28 +0530 Subject: [PATCH 7/8] UN-3636 [FIX] Gate overlay merging on an omitted path, not its value `load_groups(DEFAULT_MANIFEST)` merged overlays even though the caller named a manifest explicitly, because the check compared path values. Path equality is also spelling-sensitive, so the same file relative and absolute behaved differently. Key on `path is None` instead: an explicit path loads exactly what it names. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Ryg9chVDJQggCybpq3YoY3 --- tests/rig/groups.py | 9 +++++---- tests/rig/tests/test_groups.py | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/tests/rig/groups.py b/tests/rig/groups.py index bee525d590..3a2c7b25d4 100644 --- a/tests/rig/groups.py +++ b/tests/rig/groups.py @@ -131,9 +131,9 @@ def load_groups(path: Path | None = None) -> GroupManifest: groups[name] = _build_group(name, spec, defaults) # Merge before validation so cross-manifest `depends_on` and the platform - # gate are checked over the union. An explicit `path` stays isolated from - # the ambient env var. - if manifest_path == DEFAULT_MANIFEST: + # gate are checked over the union. Keyed on `path` being omitted rather than + # on its value: an explicit path means "load exactly this", however spelled. + if path is None: for extra in _extra_manifest_paths(): defaults = _merge_manifest(groups, extra, defaults) @@ -184,7 +184,8 @@ def _merge_manifest( """Merge an overlay manifest into ``groups`` in place and return the merged defaults, so an overlay can also rename the platform gate. Overlay groups inherit the base ``defaults`` unless they declare their own; a name collision - is an error rather than a silent override.""" + is an error rather than a silent override. + """ raw = _load_manifest_dict(manifest_path) defaults = {**base_defaults, **(raw.get("defaults") or {})} for name, spec in (raw["groups"] or {}).items(): diff --git a/tests/rig/tests/test_groups.py b/tests/rig/tests/test_groups.py index 37fcbbc81c..a8e55a26c2 100644 --- a/tests/rig/tests/test_groups.py +++ b/tests/rig/tests/test_groups.py @@ -253,7 +253,8 @@ def _overlay_env( ) -> Path: """Point the default manifest at a fixture and register an overlay. - Overlays only apply to the default manifest, so tests must drive that path. + Overlays only apply when `load_groups` is called with no path, so tests must + drive that call. """ base = _base_manifest(tmp_path) overlay = tmp_path / "groups.cloud.yaml" @@ -323,6 +324,25 @@ def test_explicit_manifest_ignores_overlays( assert "unit-cloud" not in load_groups(base).names() +def test_explicit_default_path_ignores_overlays( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Isolation keys on `path` being omitted, not on which file it names.""" + base = _overlay_env( + tmp_path, + monkeypatch, + """ + version: 1 + groups: + unit-cloud: + tier: unit + paths: [y] + optional: true + """, + ) + assert "unit-cloud" not in load_groups(base).names() + + def test_extra_manifest_defaults_applied( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From ecc64b9ce6b4ec9273326a24ae2d5ec10366ac4e Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Jul 2026 16:38:25 +0530 Subject: [PATCH 8/8] UN-3636 [FIX] Stop ide_prompt_complete tests from hitting the network TestIdePromptComplete drives the full success path, which reaches client_plugin_registry.get_client_plugin("subscription_usage"). In OSS no such plugin is installed, so the lookup returns None instantly. In a tree with the cloud plugins copied in, it resolves to a real plugin that POSTs to the backend; with no backend running the call only fails after a multi-second connect timeout, and _track_subscription_usage swallows the error so the tests still pass. That accounted for ~190s of the cloud unit-workers run. The file already declared _PATCH_GET_PLUGIN but never applied it outside the dedicated subscription-usage classes. Apply it as a class-scoped fixture so the lookup is pinned to the OSS answer. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz --- workers/tests/test_ide_callback.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/workers/tests/test_ide_callback.py b/workers/tests/test_ide_callback.py index eb0b2f8c8d..02c5e10b46 100644 --- a/workers/tests/test_ide_callback.py +++ b/workers/tests/test_ide_callback.py @@ -95,6 +95,17 @@ def failure_result(): } +@pytest.fixture +def no_client_plugins(): + """Pin plugin lookup to the OSS answer for callbacks that aren't testing it. + + Left live, a tree with the plugins installed makes a real HTTP call that only + fails after a multi-second connect timeout. + """ + with patch(_PATCH_GET_PLUGIN, return_value=None): + yield + + # --------------------------------------------------------------------------- # TestIdeIndexComplete # --------------------------------------------------------------------------- @@ -346,6 +357,7 @@ def test_exception_does_not_crash(self, mock_get_client, mock_emit_ws, mock_api, # --------------------------------------------------------------------------- +@pytest.mark.usefixtures("no_client_plugins") class TestIdePromptComplete: """Tests for ide_prompt_complete task."""