From 7fea1cdc0df5cff08ea48b51c18807303cc118ed Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 20:48:23 +0000 Subject: [PATCH 1/3] fix(ci): exclude non-text-input models from review sidecar catalog contextual_orchestrator_review_launcher.py's model-selection loop only checked output modality (_has_text_output), never input modality, so a model whose input-modality evidence declares e.g. "image" -- NVIDIA NIM's meta/llama-3.2-90b-vision-instruct -- could still be admitted to the plain-text review catalog. The provider then rejects Strix's plain-text security-review request with a non-retryable HTTP 400, which Strix does not discover until it has already burned its ~2-hour job budget failing over through the rest of the pool (see ContextualWisdomLab/.github#1415's strix job 99583819068). contextual-orchestrator itself already closed the equivalent gap in its own runtime selection path -- chat_capability.requires_non_text_input, whose docstring cites this exact incident class (ContextualWisdomLab/.github#1198) -- but this repo's independently vendored catalog-builder never called it, and the sidecar's emitted agent rows carry no `input:` tag for orchestrator.py's own runtime `_is_general_free_agent` gate to catch either. Now calls requires_non_text_input(model.input_modalities) alongside the existing output-modality check. Verification: PYTHONPATH=. coverage run -m pytest tests -- 2126 passed, 1 skipped; coverage report -- 100%; interrogate -- 100%. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- CHANGELOG.md | 15 +++++++++++++++ .../contextual_orchestrator_review_launcher.py | 16 +++++++++++++++- ...xtual_orchestrator_review_sidecar_contract.py | 9 ++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c61c142b..3d6a1fdf8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Exclude non-text-input (vision/audio/video) models from the review sidecar's + plain-text catalog: `contextual_orchestrator_review_launcher.py`'s model + loop checked only output modality (`_has_text_output`), so a model whose + *input* modality evidence declares e.g. `image` (NVIDIA NIM's + `meta/llama-3.2-90b-vision-instruct`) could still be selected for Strix's + plain-text security-review prompts, which the provider then rejects with a + non-retryable HTTP 400 — burning the full multi-hour Strix job budget + before failing closed (#1415). `contextual-orchestrator` itself already + fixed the equivalent gap in its own runtime selection path (its + `chat_capability.requires_non_text_input`, cited there against this exact + incident class), but this repo's independently vendored catalog-builder + never called it and its emitted agent rows carry no `input:` tag for that + runtime gate to catch either. Now calls + `chat_capability.requires_non_text_input(model.input_modalities)` + alongside the existing output-modality check. - Harden the review sidecar's per-account catalog cap against silent drift: `contextual_orchestrator_review_launcher.py`'s two `build_zdr_prioritized_catalog` call sites now source their diff --git a/scripts/ci/contextual_orchestrator_review_launcher.py b/scripts/ci/contextual_orchestrator_review_launcher.py index f115ef2b88..3e7c08564d 100644 --- a/scripts/ci/contextual_orchestrator_review_launcher.py +++ b/scripts/ci/contextual_orchestrator_review_launcher.py @@ -796,7 +796,10 @@ def main(argv: list[str] | None = None) -> int: args = parser.parse_args(argv) from contextual_orchestrator.credentials import get_credential - from contextual_orchestrator.chat_capability import is_general_chat_agent_model_id + from contextual_orchestrator.chat_capability import ( + is_general_chat_agent_model_id, + requires_non_text_input, + ) from contextual_orchestrator.model_discovery import discover_all_models, free_discovered_models from contextual_orchestrator.orchestrator import ModelClient, TaskOrchestrator, load_agents from contextual_orchestrator.review_gateway import ( @@ -837,6 +840,17 @@ def main(argv: list[str] | None = None) -> int: model_id = getattr(model, "model_id", "") if not is_general_chat_agent_model_id(model_id) or not _has_text_output(model): continue + if requires_non_text_input(getattr(model, "input_modalities", ()) or ()): + # Excludes vision/audio/video-input-only deployments (e.g. NVIDIA NIM's + # meta/llama-3.2-90b-vision-instruct) from this plain-text review catalog. + # `_has_text_output` above only checks output modality; contextual-orchestrator's + # own runtime `_is_general_free_agent` gate (orchestrator.py) reads an `input:` + # agent tag this sidecar's own catalog rows never carry, so it cannot catch this + # here -- the same NVIDIA NIM 400 this fixed once already in that repo (see + # `contextual_orchestrator.model_discovery._requires_non_text_input`'s docstring, + # citing ContextualWisdomLab/.github#1198) recurred via this independently + # rebuilt catalog. See ContextualWisdomLab/.github#1415's strix failure. + continue if args.pool == "free" and _route_identity(model) not in free_route_identities: continue selected_models.append(model) diff --git a/tests/test_contextual_orchestrator_review_sidecar_contract.py b/tests/test_contextual_orchestrator_review_sidecar_contract.py index 0a63356dad..611cbace74 100644 --- a/tests/test_contextual_orchestrator_review_sidecar_contract.py +++ b/tests/test_contextual_orchestrator_review_sidecar_contract.py @@ -279,7 +279,9 @@ def test_launcher_registers_secrets_into_the_kv_once() -> None: def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None: """Discovery, price evidence, and serving come from the vendored library.""" text = _read(LAUNCHER) - assert "from contextual_orchestrator.chat_capability import is_general_chat_agent_model_id" in text + assert "from contextual_orchestrator.chat_capability import (" in text + assert "is_general_chat_agent_model_id," in text + assert "requires_non_text_input," in text assert "from contextual_orchestrator.model_discovery import discover_all_models, free_discovered_models" in text assert "routable_discovered = _routable_discovered_models(discovered)" in text assert "free_discovered_models(routable_discovered)" in text @@ -289,6 +291,11 @@ def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None: assert '"text" in {str(modality).casefold() for modality in modalities}' in text assert "not _has_text_output(model)" in text assert 'model_id = getattr(model, "model_id", "")' in text + # Output modality alone is not enough: a vision/audio/video-input-only model can still + # declare a text output modality (e.g. NVIDIA NIM's meta/llama-3.2-90b-vision-instruct), + # so the catalog must separately exclude any model whose *input* modality evidence is + # non-text-only -- the exact recurrence this closes is ContextualWisdomLab/.github#1415. + assert 'requires_non_text_input(getattr(model, "input_modalities", ()) or ())' in text launcher = runpy.run_path(str(LAUNCHER)) has_text_output = launcher["_has_text_output"] From 14f2d1bc192a993a95a80c29f1703b2665dca474 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 13:13:16 +0900 Subject: [PATCH 2/3] fix(ci): verify text review catalog capability --- CHANGELOG.md | 20 ++---- ...contextual_orchestrator_review_launcher.py | 41 ++++++++---- ...al_orchestrator_review_sidecar_contract.py | 65 +++++++++++++++++-- 3 files changed, 91 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc6c2cb20c..848cec87a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,21 +5,11 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] -- Exclude non-text-input (vision/audio/video) models from the review sidecar's - plain-text catalog: `contextual_orchestrator_review_launcher.py`'s model - loop checked only output modality (`_has_text_output`), so a model whose - *input* modality evidence declares e.g. `image` (NVIDIA NIM's - `meta/llama-3.2-90b-vision-instruct`) could still be selected for Strix's - plain-text security-review prompts, which the provider then rejects with a - non-retryable HTTP 400 — burning the full multi-hour Strix job budget - before failing closed (#1415). `contextual-orchestrator` itself already - fixed the equivalent gap in its own runtime selection path (its - `chat_capability.requires_non_text_input`, cited there against this exact - incident class), but this repo's independently vendored catalog-builder - never called it and its emitted agent rows carry no `input:` tag for that - runtime gate to catch either. Now calls - `chat_capability.requires_non_text_input(model.input_modalities)` - alongside the existing output-modality check. +- Exclude models with declared non-text input requirements from the Contextual + Orchestrator review catalog before cost, privacy, and account selection. + Plain-text Chat Completions, Responses, and structured-output review paths + remain eligible, while the existing multi-hour per-model review budgets are + unchanged (#1415). - Fail closed when the first top-level Noema JSON candidate is malformed, preventing a later approval object from overriding malformed preface data; multiple-object output remains supported when its first object is valid. diff --git a/scripts/ci/contextual_orchestrator_review_launcher.py b/scripts/ci/contextual_orchestrator_review_launcher.py index 3e7c08564d..f6d2bbffd5 100644 --- a/scripts/ci/contextual_orchestrator_review_launcher.py +++ b/scripts/ci/contextual_orchestrator_review_launcher.py @@ -105,6 +105,29 @@ def _has_text_output(model: object) -> bool: return not modalities or "text" in {str(modality).casefold() for modality in modalities} +def _is_text_review_candidate( + model: object, + *, + is_general_chat_agent_model_id: Any, + requires_non_text_input: Any, +) -> bool: + """Return whether discovery evidence permits plain-text review traffic. + + Unknown input modality remains eligible because absence of catalog evidence + is not evidence of a non-text requirement. Any declared non-text input is + excluded conservatively before cost, privacy, or account selection can + admit the route. Chat and structured-output capability metadata otherwise + remain untouched for the gateway's Responses and Chat Completions paths. + """ + model_id = getattr(model, "model_id", "") + input_modalities = getattr(model, "input_modalities", ()) or () + return bool( + is_general_chat_agent_model_id(model_id) + and _has_text_output(model) + and not requires_non_text_input(input_modalities) + ) + + _DISCOVERY_DIAGNOSTICS_COMPLETE_SENTINEL = "discovery_diagnostics_complete" @@ -837,19 +860,11 @@ def main(argv: list[str] | None = None) -> int: free_route_identities = frozenset(_route_identity(model) for model in free_models) selected_models = [] for model in routable_discovered: - model_id = getattr(model, "model_id", "") - if not is_general_chat_agent_model_id(model_id) or not _has_text_output(model): - continue - if requires_non_text_input(getattr(model, "input_modalities", ()) or ()): - # Excludes vision/audio/video-input-only deployments (e.g. NVIDIA NIM's - # meta/llama-3.2-90b-vision-instruct) from this plain-text review catalog. - # `_has_text_output` above only checks output modality; contextual-orchestrator's - # own runtime `_is_general_free_agent` gate (orchestrator.py) reads an `input:` - # agent tag this sidecar's own catalog rows never carry, so it cannot catch this - # here -- the same NVIDIA NIM 400 this fixed once already in that repo (see - # `contextual_orchestrator.model_discovery._requires_non_text_input`'s docstring, - # citing ContextualWisdomLab/.github#1198) recurred via this independently - # rebuilt catalog. See ContextualWisdomLab/.github#1415's strix failure. + if not _is_text_review_candidate( + model, + is_general_chat_agent_model_id=is_general_chat_agent_model_id, + requires_non_text_input=requires_non_text_input, + ): continue if args.pool == "free" and _route_identity(model) not in free_route_identities: continue diff --git a/tests/test_contextual_orchestrator_review_sidecar_contract.py b/tests/test_contextual_orchestrator_review_sidecar_contract.py index 611cbace74..80c1f4e449 100644 --- a/tests/test_contextual_orchestrator_review_sidecar_contract.py +++ b/tests/test_contextual_orchestrator_review_sidecar_contract.py @@ -289,13 +289,9 @@ def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None: assert 'getattr(model, "output_modalities", None)' in text assert 'isinstance(modalities, str)' in text assert '"text" in {str(modality).casefold() for modality in modalities}' in text - assert "not _has_text_output(model)" in text - assert 'model_id = getattr(model, "model_id", "")' in text - # Output modality alone is not enough: a vision/audio/video-input-only model can still - # declare a text output modality (e.g. NVIDIA NIM's meta/llama-3.2-90b-vision-instruct), - # so the catalog must separately exclude any model whose *input* modality evidence is - # non-text-only -- the exact recurrence this closes is ContextualWisdomLab/.github#1415. - assert 'requires_non_text_input(getattr(model, "input_modalities", ()) or ())' in text + assert "_is_text_review_candidate(" in text + assert 'input_modalities = getattr(model, "input_modalities", ()) or ()' in text + assert "not requires_non_text_input(input_modalities)" in text launcher = runpy.run_path(str(LAUNCHER)) has_text_output = launcher["_has_text_output"] @@ -330,6 +326,61 @@ def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None: assert "from scripts.ci import zdr_policy" in text +def test_launcher_admits_only_plain_text_review_candidates() -> None: + """Review discovery keeps text protocols while excluding multimodal rows.""" + launcher = runpy.run_path(str(LAUNCHER)) + is_candidate = launcher["_is_text_review_candidate"] + + def general_chat(model_id: str) -> bool: + return model_id.startswith("chat-") + + def needs_non_text(modalities: object) -> bool: + return any(str(value).casefold() != "text" for value in modalities) + + compatible = SimpleNamespace( + model_id="chat-structured", + input_modalities=("text",), + output_modalities=("text",), + capabilities=("chat", "response_format"), + ) + unknown_input = SimpleNamespace( + model_id="chat-undocumented", + input_modalities=(), + output_modalities=("text",), + ) + multimodal = SimpleNamespace( + model_id="chat-multimodal", + input_modalities=("text", "image"), + output_modalities=("text",), + ) + media_output = SimpleNamespace( + model_id="chat-media-output", + input_modalities=("text",), + output_modalities=("image",), + ) + + assert is_candidate( + compatible, + is_general_chat_agent_model_id=general_chat, + requires_non_text_input=needs_non_text, + ) + assert is_candidate( + unknown_input, + is_general_chat_agent_model_id=general_chat, + requires_non_text_input=needs_non_text, + ) + assert not is_candidate( + multimodal, + is_general_chat_agent_model_id=general_chat, + requires_non_text_input=needs_non_text, + ) + assert not is_candidate( + media_output, + is_general_chat_agent_model_id=general_chat, + requires_non_text_input=needs_non_text, + ) + + def test_launcher_wraps_catalog_for_vendored_load_agents() -> None: """Persist the catalog envelope expected by the pinned orchestrator loader.""" text = _read(LAUNCHER) From c352014a163d2555ada6150a2452bdfbebb71ce6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 13:29:38 +0900 Subject: [PATCH 3/3] test(ci): cover latest scheduler merge branches --- tests/test_pr_review_fix_scheduler.py | 10 +++++ ...ew_fix_scheduler_rest_workflow_identity.py | 39 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 3b4416bdc3..0120a4a108 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -1142,6 +1142,16 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch): """Inspect and queue logic report skip, wait, dispatch-limit, and errors.""" args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) assert fix.inspect_pr("owner/repo", make_pr(isDraft=True), args) == ("skip", ("draft PR",)) + assert fix.inspect_pr( + "owner/repo", + _approved_dirty_pr(isDraft=True), + args, + ) == ("skip", ("draft PR",)) + monkeypatch.setattr(fix, "needs_conflict_resolution", lambda pr, **kwargs: (False, ())) + assert fix.inspect_pr("owner/repo", _approved_dirty_pr(), args) == ( + "skip", + ("merge conflict is not authorized for repair",), + ) assert fix.inspect_pr("owner/repo", make_pr(baseRefName="develop"), args)[1][0].startswith("base branch") wildcard_args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "*"]) monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ())) diff --git a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py index c24cfb05f9..2c96afd0b4 100644 --- a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +++ b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py @@ -10,6 +10,45 @@ from scripts.ci import pr_review_merge_scheduler as merge +def test_workflow_name_rest_fallback_paginates_and_skips_incomplete_rows( + monkeypatch: Any, +) -> None: + """Workflow identity pagination ignores rows without a complete binding.""" + calls: list[str] = [] + + def fake_api(path: str) -> Any: + calls.append(path) + if path.endswith("&page=1"): + return { + "workflow_runs": [ + {"check_suite_id": 7, "name": "Required OpenCode Review"}, + *({"check_suite_id": None, "name": ""} for _ in range(99)), + ] + } + return {"workflow_runs": []} + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + assert merge.fetch_workflow_names_by_check_suite_rest( + "owner/repo", "a" * 40 + ) == {7: "Required OpenCode Review"} + assert len(calls) == 2 + assert "page=2" in calls[-1] + + +def test_workflow_name_rest_fallback_propagates_unexpected_errors( + monkeypatch: Any, +) -> None: + """Only permission denial degrades to unknown workflow identity.""" + def fail(_path: str) -> Any: + raise RuntimeError("unexpected transport failure") + + monkeypatch.setattr(merge, "gh_api_json", fail) + + with pytest.raises(RuntimeError, match="unexpected transport failure"): + merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "b" * 40) + + def test_rest_fallback_preserves_renamed_opencode_workflow_identity( monkeypatch: Any, ) -> None: