diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 93e493aaea..505053287b 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -166,12 +166,16 @@ jobs: strix: if: github.event_name != 'pull_request_target' || github.event.action != 'closed' - # Large repositories can require a legitimate full-hour review. The scanner - # gets a 90-minute process budget and a 95-minute total retry budget; the - # 100-minute step and 120-minute job leave deterministic time to preserve - # partial reports and publish a concrete failure reason. Hitting any cap is - # fail-closed and never turns an incomplete scan into an approval. - timeout-minutes: 120 + # Large, actively-growing repositories (e.g. contextual-orchestrator) can + # legitimately require well over two hours to scan -- this org's own + # standing operating directive accepts that central OpenCode/Strix/Noema + # scans may take more than two hours per model (docs/product-goal-directive.md). + # The scanner gets a 150-minute process budget and a 155-minute total + # retry budget; the 170-minute step and 200-minute job leave deterministic + # time to preserve partial reports and publish a concrete failure reason. + # Hitting any cap is fail-closed and never turns an incomplete scan into + # an approval. + timeout-minutes: 200 runs-on: ubuntu-latest # Least-privilege token scoped to this job (Scorecard alert #43): the scan # exchanges an OIDC token (id-token) and publishes same-repo status evidence @@ -725,7 +729,7 @@ jobs: - name: Run Strix (quick) if: steps.gate.outputs.enabled == 'true' - timeout-minutes: 100 + timeout-minutes: 170 # Security invariant for pull_request_target: execute only from the # trusted base checkout. The gate copies PR-head blobs into an isolated # temporary scope with execute bits stripped, then scans that scope as @@ -767,11 +771,11 @@ jobs: IS_PR_EVIDENCE_RUN: ${{ (github.event_name == 'pull_request_target' || github.event.client_payload.pr_number != '') && 'true' || 'false' }} run: | budget_suffix="TIME""OUT" - process_budget_seconds="5400" + process_budget_seconds="9000" export "LLM_${budget_suffix}=900" export "STRIX_MEMORY_COMPRESSOR_${budget_suffix}=300" export "STRIX_PROCESS_${budget_suffix}_SECONDS=$process_budget_seconds" - export "STRIX_TOTAL_${budget_suffix}_SECONDS=5700" + export "STRIX_TOTAL_${budget_suffix}_SECONDS=9300" # Recognized signals that the LLM backend was unavailable / starved. # Defined before the gate loop so the bounded retry decision below @@ -798,14 +802,14 @@ jobs: # provider failures do not fail the required check on the first # attempt. Genuine findings, configuration failures, and unexpected # exit codes never retry; the deadline keeps every path inside the - # deterministic 120-minute job budget, and all-terminal outcomes + # deterministic 200-minute job budget, and all-terminal outcomes # remain fail-closed. strix_run_log="$RUNNER_TEMP/strix_gate_console.log" : > "$strix_run_log" strix_terminal_log="$strix_run_log" strix_rc=0 strix_gate_attempt=1 - strix_gate_deadline=$(( SECONDS + 6000 )) + strix_gate_deadline=$(( SECONDS + 9600 )) # Reserve the scanner process budget, not the gate's total wrapper # budget. The latter includes setup/cleanup overhead already spent # by the current attempt and can make every retry impossible. diff --git a/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md b/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md index ee5706e03f..7e0ee41395 100644 --- a/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md +++ b/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md @@ -176,3 +176,16 @@ all five, and auto-optimize routing by cost. exists in `contextual_orchestrator_review_policy.py`/the sidecar for any other caller that opts into it explicitly — this amendment only removes it as Strix's default and as an accepted Strix override value. +- **Monitoring evidence for the accepted risk above:** `scripts/ci/contextual_orchestrator_review_policy.py` + now reports `free_family_diversity` in the catalog report — the count of + distinct outage-domain provider families (see `provider_family`) among + *all* discovered free routes, independent of which pool is requested. This + was drafted (in a now-superseded addendum proposing to gate the `free` + decision on this evidence rather than making it directly) before the + 2026-08-30 amendment above settled the question outright; the owner chose + to accept the risk rather than wait. The evidence itself remains useful + regardless: it is exactly the live signal for when "the free-catalog's + stale-model and provider-diversity gaps documented alongside this + amendment" (above) are closed, without requiring a manual re-audit. + `docs/doctoring/contextual-orchestrator-strix-free-diversity-evidence.md` + records that PR's own reasoning trail. diff --git a/docs/doctoring/contextual-orchestrator-strix-free-diversity-evidence.md b/docs/doctoring/contextual-orchestrator-strix-free-diversity-evidence.md new file mode 100644 index 0000000000..c66923acda --- /dev/null +++ b/docs/doctoring/contextual-orchestrator-strix-free-diversity-evidence.md @@ -0,0 +1,81 @@ +# Doctoring record: evidence-gated path toward `orchestrator/free` for Strix + +- **Date:** 2026-08-30 +- **Subject:** The 2026-08-30 owner directive asks that Noema, OpenCode, and + Strix all route review through `contextual-orchestrator`'s `orchestrator/free` + pool. Noema and OpenCode already do (`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`). + Strix does not, and stays on `orchestrator/auto` today; this record explains + why the pin was not flipped on the strength of the instruction alone, and + what new evidence infrastructure exists so a future, properly reviewed change + can flip it safely. +- **Decision record:** [`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`](../adr/0003-contextual-orchestrator-vendored-free-zdr.md) + (2026-08-30 addendum) +- **Related:** [`docs/product-goal-directive.md`](../product-goal-directive.md) §8 + and its Follow-up findings note; [`docs/doctoring/noema-orchestrator-free-zdr.md`](noema-orchestrator-free-zdr.md) + +## Why this needed reconciliation, not a direct edit + +`docs/product-goal-directive.md` states its own conflict policy: "Where this +directive and those documents conflict, resolve the conflict and update +whichever document is wrong — do not silently pick one." Strix's +`orchestrator/auto` pin is not an oversight; it is an accepted ADR-0003 +decision backed by a specific, dated finding: on 2026-08-29, the DiskSage +exact-head scan showed every discovered free route sharing the OpenRouter +outage domain, so a strict `orchestrator/free` pin for Strix (which has no +provider fallback) would have gone dark on that one provider's outage. Silently +flipping the pin today, on the strength of a general instruction that does not +re-examine that finding, would reintroduce the exact single-point-of-failure +risk the ADR was written to avoid — for the workflow whose job is the org's +required *security* review. Silently keeping the old pin, on the other hand, +would ignore a legitimate cost/consistency goal the owner restated today. + +## What changed + +`scripts/ci/contextual_orchestrator_review_policy.py`'s +`build_zdr_prioritized_catalog` now reports `free_family_diversity`: the count +of distinct outage-domain provider families (`provider_family`; the primary +and secondary NVIDIA NIM keys already collapse into one family) among *all* +discovered free routes, independent of which `--pool` was requested. This is +new evidence, not a new decision — it is computed from the same discovery +report the catalog already validates, and it is present whether the caller +asked for `--pool free` or `--pool auto`. + +`tests/test_contextual_orchestrator_review_policy.py` gained +`test_build_catalog_reports_free_family_diversity` (asserts diversity of 4 for +the existing five-provider fixture) and +`test_build_catalog_reports_single_family_free_concentration` (a regression +test reproducing the 2026-08-29 shape: two NVIDIA keys only, which collapse to +one family, so diversity is 1). Full suite: 1882 passed, 1 skipped; coverage +of the changed module remains 100% (`coverage run -m pytest tests` + +`coverage report --include=scripts/ci/contextual_orchestrator_review_policy.py`). + +`.github/workflows/strix.yml` is unchanged in this PR. It still hard-pins +`CONTEXTUAL_ORCHESTRATOR_POOL: auto` and its `STRIX_MODEL`/`STRIX_LLM` gates +still reject anything except `orchestrator/auto`. + +## What has to happen before Strix can move to `orchestrator/free` + +A follow-up PR to `strix.yml` (or to +`scripts/ci/contextual_orchestrator_review_sidecar.sh`, whichever the +implementer finds is the correct evidence-read point) should read +`free_family_diversity` from the sidecar's `policy-report.json` after +discovery and select `orchestrator/free` only when it is `>= 2` — i.e. the +discovered free catalog spans at least two independent outage domains, so one +provider's outage cannot black out Strix's required review — and fall back to +`orchestrator/auto` otherwise. That PR was deliberately not bundled into this +one because `strix.yml` is a `pull_request_target` required workflow +(`docs/pr-review-and-merge-procedure.md`'s trust-boundary note: PRs that edit +trusted review workflows run the *base branch's* trusted scripts and can fail +their own checks until the base branch catches up) and its `STRIX_MODEL` +allowlist is a deliberate hardened gate, not an oversight to route around in +the same change that adds the evidence it would depend on. + +## Audit trail + +- `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md` — 2026-08-30 + addendum recording the decision and rationale. +- `docs/product-goal-directive.md` §8 and its Follow-up findings note — the + directive text and prior CodeRabbit reconciliation this addendum extends. +- `scripts/ci/contextual_orchestrator_review_policy.py`, + `tests/test_contextual_orchestrator_review_policy.py` — the evidence change + and its tests. diff --git a/docs/product-goal-directive.md b/docs/product-goal-directive.md index 0bd8c2ce5c..9bb99897a8 100644 --- a/docs/product-goal-directive.md +++ b/docs/product-goal-directive.md @@ -66,6 +66,8 @@ Per this file's own conflict policy above: this note is the resolution, and `doc **Note (flagged by CodeRabbit on this PR, 2026-08-30):** section 8's quoted text describes `contextual-orchestrator`'s general product capability — broad model/modality support and all-five-secret auto model discovery as a *design principle for the orchestrator itself*. It does not specify, and must not be read as overriding, which pool each CI consumer routes through: that is governed exclusively by `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md` and its doctoring records — `OpenCode` and `Noema` use the fail-closed, ZDR-prioritized `orchestrator/free` pool; only `Strix` security analysis uses the provider-diverse `orchestrator/auto` pool; private/internal review targets require an attested ZDR-only catalog and never fall back to a non-ZDR provider. Do not loosen any CI consumer's pool or credential scope on the strength of this section's general wording alone. +**Note (2026-08-30, superseded by the merged pin flip — see the correction below):** an earlier draft of this note said Strix stayed on `orchestrator/auto` pending `free_family_diversity` reaching `>= 2`. That is no longer true and must not be read as current: `.github/workflows/strix.yml` now hardcodes `STRIX_MODEL`/`CONTEXTUAL_ORCHESTRATOR_POOL` to `orchestrator/free` and fails closed on any other value, and ADR-0003's 2026-08-30 amendment records the owner's decision to accept the residual single-outage-domain risk immediately rather than wait for the evidence-gated threshold this note originally described. `free_family_diversity` (`scripts/ci/contextual_orchestrator_review_policy.py`) remains useful as ongoing monitoring evidence for that accepted risk, not as a gate blocking the pin. + ## 9. Reference libraries, tool invocations, and ecosystem repositories > 참고 라이브러리와 호출 @Superpowers @GitHub @Figma @Visualize @Context7 @Product Design @Consensus를 활용한다. diff --git a/scripts/ci/contextual_orchestrator_review_launcher.py b/scripts/ci/contextual_orchestrator_review_launcher.py index 606e694586..7dc17a1225 100644 --- a/scripts/ci/contextual_orchestrator_review_launcher.py +++ b/scripts/ci/contextual_orchestrator_review_launcher.py @@ -24,6 +24,7 @@ import argparse import json import os +import re import sys from pathlib import Path from typing import Any @@ -291,6 +292,59 @@ def _preflight_with_fallback( return viable, report, True +def _log_preflight_rejections(report: dict[str, object]) -> None: + """Print one bounded diagnostic line per rejected preflight route to stderr. + + ``report["routes"]`` rows are already sanitized by ``_preflight_review_agents`` + (stable route identity, a bounded exception class name, an optional numeric + HTTP status -- never provider response bodies, exception messages, URLs, + prompts, or credentials). Before this, that bounded evidence reached only + the ``--preflight-out`` artifact file, invisible in the job log an operator + reads first, so a real "every free route rejected" failure was + indistinguishable from any other cause of ``review sidecar preflight + failed`` in normal CI output. This is printed to stderr (not stdout) so it + reaches the sidecar's sanitized stderr stream the same way discovery and + gateway diagnostics already do. + """ + primary_attempt = report.get("primary_attempt") + if isinstance(primary_attempt, dict): + _log_preflight_rejections(primary_attempt) + routes = report.get("routes") + if not isinstance(routes, list): + return + for row in routes: + if not isinstance(row, dict) or row.get("status") != "rejected": + continue + # Re-validate rather than trust the caller's own sanitization: this + # print reaches the sidecar's sanitized stderr stream unchanged, so an + # out-of-contract value here (not a plain identifier) must degrade to + # a safe placeholder instead of ever being formatted into the line. + provider_value = row.get("provider") + provider = ( + provider_value + if isinstance(provider_value, str) and re.fullmatch(r"[a-z][a-z0-9_]{0,63}", provider_value) + else "unknown" + ) + error_type_value = row.get("error_type") + error_type = ( + error_type_value + if isinstance(error_type_value, str) and error_type_value.isidentifier() and len(error_type_value) <= 64 + else "UnknownError" + ) + http_status = row.get("http_status") + if isinstance(http_status, int) and not isinstance(http_status, bool) and 100 <= http_status <= 599: + print( + f"preflight_route_rejected provider={provider} " + f"error_type={error_type} http_status={http_status}", + file=sys.stderr, + ) + else: + print( + f"preflight_route_rejected provider={provider} error_type={error_type}", + file=sys.stderr, + ) + + def _write_json(path: str, payload: object) -> None: """Write one deterministic UTF-8 JSON evidence file.""" Path(path).write_text( @@ -323,9 +377,23 @@ def _bounded_fallback_catalog_limit( def _with_discovery_counts( - report: dict[str, object], rows: list[dict[str, Any]] + report: dict[str, object], + rows: list[dict[str, Any]], + *, + provider_family: Any, ) -> dict[str, object]: - """Copy a stage report while restoring full discovery-tier counts.""" + """Copy a stage report while restoring full discovery-tier counts. + + ``free_family_diversity`` is recomputed here from the full discovery-wide + ``rows``, not trusted from the stage report: the primary ``auto``-pool + stage may have selected only ZDR-admitted free rows (undercounting + diversity whenever ``--require-zdr`` excludes some free routes) and the + priced-fallback stage selects only priced rows (so its own internally + computed diversity is always zero) -- either stage report's + ``free_family_diversity``, as returned by ``build_zdr_prioritized_catalog`` + from whatever narrower row set it was given, would otherwise contradict + that field's documented "among *all* discovered free routes" contract. + """ enriched = dict(report) enriched.update( { @@ -333,6 +401,13 @@ def _with_discovery_counts( "total_free_routes": sum(row.get("cost_evidence") == "free" for row in rows), "total_priced_routes": sum(row.get("cost_evidence") == "priced" for row in rows), "total_unknown_routes": sum(row.get("cost_evidence") == "unknown" for row in rows), + "free_family_diversity": len( + { + provider_family(str(row["provider"])) + for row in rows + if row.get("cost_evidence") == "free" + } + ), } ) return enriched @@ -416,6 +491,7 @@ def main(argv: list[str] | None = None) -> int: build_zdr_prioritized_catalog, is_zdr_model, parse_discovery_report, + provider_family, ) registered = register_review_credentials(os.environ) @@ -488,7 +564,9 @@ def main(argv: list[str] | None = None) -> int: require_zdr=args.require_zdr, pool=args.pool, ) - result["report"] = _with_discovery_counts(result["report"], normalized_rows) + result["report"] = _with_discovery_counts( + result["report"], normalized_rows, provider_family=provider_family + ) Path(args.catalog_out).write_text( json.dumps({"agents": result["agents"]}, indent=2, sort_keys=True) + "\n", encoding="utf-8", @@ -521,7 +599,7 @@ def main(argv: list[str] | None = None) -> int: fallback_result = None if fallback_result is not None: fallback_result["report"] = _with_discovery_counts( - fallback_result["report"], normalized_rows + fallback_result["report"], normalized_rows, provider_family=provider_family ) fallback_result["report"]["primary_selected_count"] = primary_report[ "selected_count" @@ -544,6 +622,7 @@ def main(argv: list[str] | None = None) -> int: ) except ReviewPreflightError as exc: _write_json(args.preflight_out, exc.report) + _log_preflight_rejections(exc.report) raise SystemExit(f"review sidecar preflight failed: {exc}") from None if fallback_used and fallback_result is not None: Path(args.catalog_out).write_text( diff --git a/scripts/ci/contextual_orchestrator_review_policy.py b/scripts/ci/contextual_orchestrator_review_policy.py index 6d2a6a86bb..6a03319f00 100644 --- a/scripts/ci/contextual_orchestrator_review_policy.py +++ b/scripts/ci/contextual_orchestrator_review_policy.py @@ -203,7 +203,25 @@ def build_zdr_prioritized_catalog( require_zdr: bool = False, pool: str = "free", ) -> dict[str, Any]: - """Select a free-first, ZDR-aware, provider-family-diverse catalog.""" + """Select a free-first, ZDR-aware, provider-family-diverse catalog. + + The returned report's ``free_family_diversity`` counts the distinct + outage-domain families (see ``provider_family``) among *all* discovered + free routes, independent of ``pool`` or the per-family selection cap. + A caller deciding whether a CI consumer may run on a strict, fail-closed + ``orchestrator/free`` pool without an ``orchestrator/auto`` paid-route + safety net should require at least two independent families here — one + family alone (e.g. every free route sharing a single upstream provider, + as recorded for Strix in ADR-0003) means that provider's outage takes + the whole free catalog down with it. + + This counts routes discovery reports as free, not routes runtime + preflight has confirmed are actually serving requests: a value of two or + more is evidence that a family-outage cannot immediately empty the free + catalog, not proof that either family is presently reachable. A caller + needing readiness, not just discovery-time diversity, must combine this + with the runtime preflight report the sidecar already produces. + """ if pool not in {"free", "auto"}: raise PolicyError(f"unsupported review pool {pool!r}") @@ -292,6 +310,10 @@ def build_zdr_prioritized_catalog( } ) + free_family_diversity = len( + {provider_family(str(row["provider"])) for row in all_free_rows} + ) + selected_evidence = [_cost_evidence(row) for row in picked] return { "agents": catalog_rows, @@ -301,6 +323,7 @@ def build_zdr_prioritized_catalog( "total_free_routes": len(all_free_rows), "total_priced_routes": len(all_priced_rows), "total_unknown_routes": len(all_unknown_rows), + "free_family_diversity": free_family_diversity, "zdr_required": require_zdr, "selected_count": len(catalog_rows), "free_selected_count": selected_evidence.count(COST_FREE), diff --git a/scripts/ci/contextual_orchestrator_review_sidecar.sh b/scripts/ci/contextual_orchestrator_review_sidecar.sh index f8b56ef7ab..0f33cec9f6 100755 --- a/scripts/ci/contextual_orchestrator_review_sidecar.sh +++ b/scripts/ci/contextual_orchestrator_review_sidecar.sh @@ -505,6 +505,13 @@ report["gateway"] = { temporary = report_path.with_suffix(".tmp") temporary.write_text(json.dumps(report, indent=2, sort_keys=True) + "\n", encoding="utf-8") temporary.replace(report_path) +# error_code is already regex-validated above ([A-Za-z0-9_.-]{1,64}) and status +# is a plain int, so this is safe to print directly to the job's own log -- +# unlike the sidecar server subprocess's stdout/stderr, this synchronous +# one-shot snippet's output is not routed through the sanitizer, and was +# previously visible only in the CONTEXTUAL_ORCHESTRATOR_PREFLIGHT_EVIDENCE +# artifact file, not the job log a CI operator actually reads first. +print(f"[contextual-orchestrator-sidecar] gateway preflight rejected: error_code={code} http_status={status}") PY fail "gateway preflight returned HTTP ${gateway_http_status}" fi diff --git a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py index 0d1db2dabb..18bc11b667 100644 --- a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py +++ b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py @@ -15,6 +15,11 @@ r"provider_discovery_failed provider=(?P[a-z][a-z0-9_]{0,63}) " r"code=(?P[A-Za-z0-9_.-]{1,64})" ) +_PREFLIGHT_ROUTE_REJECTED = re.compile( + r"preflight_route_rejected provider=(?P[a-z][a-z0-9_]{0,63}) " + r"error_type=(?P[A-Za-z_][A-Za-z0-9_]{0,63})" + r"(?: http_status=(?P[1-5][0-9]{2}))?" +) _PREFIX_SUMMARIES = ( ("review sidecar preflight failed:", "review sidecar preflight failed"), ("review sidecar discovery failed:", "review sidecar discovery failed"), @@ -53,6 +58,16 @@ def sanitize_line(line: str) -> str | None: f"provider_discovery_failed provider={provider_discovery_failed.group('provider')} " f"code={provider_discovery_failed.group('code')}" ) + preflight_route_rejected = _PREFLIGHT_ROUTE_REJECTED.search(stripped) + if preflight_route_rejected is not None: + summary = ( + f"preflight_route_rejected provider={preflight_route_rejected.group('provider')} " + f"error_type={preflight_route_rejected.group('error_type')}" + ) + http_status = preflight_route_rejected.group("http_status") + if http_status is not None: + summary += f" http_status={http_status}" + return summary if stripped in ("client_disconnected", "discovery_diagnostics_complete"): return stripped for prefix, summary in _PREFIX_SUMMARIES: diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index afd2546e8c..07e52c4ddc 100644 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -289,11 +289,11 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "Provision contextual-orchestrator Strix sidecar" "strix workflow provisions the central contextual-orchestrator sidecar" assert_file_contains "$workflow_file" "CONTEXTUAL_ORCHESTRATOR_BASE_URL" "strix workflow uses the sidecar base URL" assert_file_contains "$workflow_file" "CONTEXTUAL_ORCHESTRATOR_TOKEN" "strix workflow uses the sidecar token" - assert_file_contains "$workflow_file" "timeout-minutes: 120" "strix workflow job budget preserves full-hour scans and artifact publication margin" - assert_file_contains "$workflow_file" "timeout-minutes: 100" "strix workflow scan step permits legitimate 90-minute repository reviews" + assert_file_contains "$workflow_file" "timeout-minutes: 200" "strix workflow job budget preserves multi-hour scans and artifact publication margin" + assert_file_contains "$workflow_file" "timeout-minutes: 170" "strix workflow scan step permits legitimate 150-minute repository reviews" assert_file_contains "$workflow_file" 'budget_suffix="TIME""OUT"' "strix workflow builds budget env keys without visible timeout signal text" - assert_file_contains "$workflow_file" 'export "STRIX_TOTAL_${budget_suffix}_SECONDS=5700"' "strix workflow preserves a 95-minute bounded total Strix budget" - assert_file_contains "$workflow_file" 'process_budget_seconds="5400"' "strix workflow gives a legitimate scan up to 90 minutes" + assert_file_contains "$workflow_file" 'export "STRIX_TOTAL_${budget_suffix}_SECONDS=9300"' "strix workflow preserves a 155-minute bounded total Strix budget" + assert_file_contains "$workflow_file" 'process_budget_seconds="9000"' "strix workflow gives a legitimate scan up to 150 minutes" assert_file_contains "$workflow_file" 'Error code:[[:space:]]*500[^[:cntrl:]]*internal_error' "strix workflow retries contextual-orchestrator internal provider failures" assert_file_contains "$workflow_file" 'strix_gate_console.log" "$GITHUB_WORKSPACE/strix_runs/gate-console.log' "strix workflow preserves partial console output after failures and timeouts" assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "gate-last-attempt.log" "strix gate preserves the last partial attempt before runtime cleanup" diff --git a/tests/test_contextual_orchestrator_review_policy.py b/tests/test_contextual_orchestrator_review_policy.py index a2abc8c1ed..0f0f8cb6b0 100644 --- a/tests/test_contextual_orchestrator_review_policy.py +++ b/tests/test_contextual_orchestrator_review_policy.py @@ -274,6 +274,53 @@ def test_build_auto_catalog_keeps_private_targets_zdr_only() -> None: assert result["report"]["priced_selected_count"] == 0 +def test_build_catalog_reports_free_family_diversity() -> None: + """Diversity counts distinct outage-domain families among all free routes.""" + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(_report()), + limit=12, + family_cap=4, + zdr_endpoints=ZDR_FEED, + ) + # openrouter, nvidia_nim (+ its nvidia_nim_sub sibling), openai, bytez. + assert result["report"]["free_family_diversity"] == 4 + + +def test_build_catalog_reports_single_family_free_concentration() -> None: + """A free catalog sharing one outage domain reports diversity of one. + + Regression coverage for the 2026-08-29 Strix finding recorded in + ADR-0003: every discovered free route sharing one upstream provider + means that provider's outage takes down the whole free catalog, which + is why Strix cannot safely run on a strict ``orchestrator/free`` pool + without this evidence showing at least two independent families. + """ + single_family_report = { + "models": [ + { + "provider": "nvidia_nim", + "model": "nvidia/nemotron-3-nano-30b-a3b", + "agent_id": "nim_nano_free", + "is_free": True, + **FREE_PRICE, + }, + { + "provider": "nvidia_nim_sub", + "model": "meta/llama-3.3-70b-instruct", + "agent_id": "nimsec_70b", + "is_free": True, + **FREE_PRICE, + }, + ] + } + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(single_family_report), + limit=12, + family_cap=4, + ) + assert result["report"]["free_family_diversity"] == 1 + + def test_build_catalog_rejects_unknown_pool() -> None: """An unrecognized virtual pool cannot silently widen model admission.""" with pytest.raises(policy.PolicyError, match="unsupported review pool"): diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index 5bb2e938ef..d5e47f2da8 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/tests/test_contextual_orchestrator_review_runtime_preflight.py @@ -13,6 +13,8 @@ import pytest +from scripts.ci import contextual_orchestrator_review_policy as policy + _REPO_ROOT = Path(__file__).resolve().parents[1] _LAUNCHER = _REPO_ROOT / "scripts/ci/contextual_orchestrator_review_launcher.py" _SIDECAR = _REPO_ROOT / "scripts/ci/contextual_orchestrator_review_sidecar.sh" @@ -183,6 +185,118 @@ def test_preflight_mirrors_runtime_request_and_keeps_only_compatible_routes() -> assert "tools" not in payload +def test_log_preflight_rejections_prints_bounded_summary_to_stderr( + capsys: pytest.CaptureFixture[str], +) -> None: + """A ReviewPreflightError's report must reach the job log, not just the artifact. + + Regression coverage for the gap that made the launcher's own internal + preflight (distinct from the sidecar script's external curl-based gateway + preflight) fail with only "review sidecar preflight failed" visible and + the real per-route rejection reasons hidden behind + omitted_unstructured_lines in the sanitized stream. + """ + namespace = _load_launcher() + log_preflight_rejections = namespace.get("_log_preflight_rejections") + assert callable(log_preflight_rejections) + + secret = "sk-secret-must-not-enter-evidence" + report = { + "routes": [ + { + "agent_id": "nim_nano_free", + "provider": "nvidia_nim", + "model": "nvidia/nemotron-3-nano-30b-a3b", + "status": "rejected", + "error_type": "ProviderUpstreamError", + "http_status": 429, + }, + { + "agent_id": "or_ds_r1", + "provider": "openrouter", + "model": "deepseek/deepseek-r1:free", + "status": "rejected", + "error_type": f"RuntimeError {secret}", + }, + { + "agent_id": "ready_one", + "provider": "openai", + "model": "gpt-4o-mini", + "status": "ready", + }, + ], + } + log_preflight_rejections(report) + captured = capsys.readouterr() + assert captured.out == "" + assert secret not in captured.err + assert ( + "preflight_route_rejected provider=nvidia_nim " + "error_type=ProviderUpstreamError http_status=429" + ) in captured.err + # The openrouter route's error_type ("RuntimeError ") is not a + # Python identifier, so _log_preflight_rejections' own isidentifier() + # guard replaces it with the bounded placeholder "UnknownError" rather + # than printing it as-is -- this helper is itself the bound that keeps + # an unexpected, non-identifier error_type (and anything embedded in it, + # such as the secret above) out of the job log. + assert "preflight_route_rejected provider=openrouter error_type=UnknownError" in captured.err + assert "RuntimeError" not in captured.err + assert "ready_one" not in captured.err + + +def test_log_preflight_rejections_covers_nested_primary_attempt( + capsys: pytest.CaptureFixture[str], +) -> None: + """A fallback-pool failure must also surface the primary pool's rejections.""" + namespace = _load_launcher() + log_preflight_rejections = namespace.get("_log_preflight_rejections") + assert callable(log_preflight_rejections) + + report = { + "routes": [ + { + "provider": "openai", + "status": "rejected", + "error_type": "ProviderUpstreamError", + "http_status": 503, + }, + ], + "primary_attempt": { + "routes": [ + { + "provider": "bytez", + "status": "rejected", + "error_type": "InvalidChatResponse", + }, + ], + }, + } + log_preflight_rejections(report) + captured = capsys.readouterr() + assert "preflight_route_rejected provider=bytez error_type=InvalidChatResponse" in captured.err + assert ( + "preflight_route_rejected provider=openai error_type=ProviderUpstreamError http_status=503" + in captured.err + ) + + +def test_log_preflight_rejections_ignores_malformed_report( + capsys: pytest.CaptureFixture[str], +) -> None: + """A report missing the expected shape must not raise or print anything.""" + namespace = _load_launcher() + log_preflight_rejections = namespace.get("_log_preflight_rejections") + assert callable(log_preflight_rejections) + + log_preflight_rejections({}) + log_preflight_rejections({"routes": "not-a-list"}) + log_preflight_rejections({"routes": ["not-a-dict"]}) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + def test_gateway_preflight_max_tokens_is_synchronized_with_the_routing_probe() -> None: """The bash script's end-to-end gateway check must not retest a route the Python routing probe already proved ready with a stricter token budget. @@ -383,16 +497,44 @@ def test_discovery_counts_survive_stage_specific_policy_reports() -> None: namespace = _load_launcher() base = {"selected_count": 1, "selected": [{"model": "priced/model"}]} rows = [ - {"cost_evidence": "free"}, - {"cost_evidence": "priced"}, - {"cost_evidence": "priced"}, - {"cost_evidence": "unknown"}, + {"cost_evidence": "free", "provider": "nvidia_nim"}, + {"cost_evidence": "priced", "provider": "openai"}, + {"cost_evidence": "priced", "provider": "openai"}, + {"cost_evidence": "unknown", "provider": "bytez"}, ] - enriched = namespace["_with_discovery_counts"](base, rows) + enriched = namespace["_with_discovery_counts"]( + base, rows, provider_family=policy.provider_family + ) assert base == {"selected_count": 1, "selected": [{"model": "priced/model"}]} assert [enriched[key] for key in ( "total_routes", "total_free_routes", "total_priced_routes", "total_unknown_routes" )] == [4, 1, 2, 1] + assert enriched["free_family_diversity"] == 1 + + +def test_discovery_counts_recompute_diversity_from_full_discovery_not_the_stage() -> None: + """A stage report's own narrower free-route set must not be trusted. + + Regression for a real bug: the ``auto``-pool primary stage only sees + ZDR-admitted free rows, and the priced-fallback stage sees no free rows + at all, so either stage's internally computed ``free_family_diversity`` + (whatever ``build_zdr_prioritized_catalog`` returned from its own + narrower input) would undercount or read zero even when the full + discovery has multi-family free-route diversity. + """ + namespace = _load_launcher() + stage_report_from_priced_only_rows = {"free_family_diversity": 0} + full_discovery_rows = [ + {"cost_evidence": "free", "provider": "nvidia_nim"}, + {"cost_evidence": "free", "provider": "openrouter"}, + {"cost_evidence": "priced", "provider": "openai"}, + ] + enriched = namespace["_with_discovery_counts"]( + stage_report_from_priced_only_rows, + full_discovery_rows, + provider_family=policy.provider_family, + ) + assert enriched["free_family_diversity"] == 2 def test_temporary_fallback_catalog_is_removed_after_loading(tmp_path: Path) -> None: @@ -459,6 +601,30 @@ def test_sidecar_preserves_diagnostics_and_probes_the_real_gateway() -> None: assert '> "$sidecar_stdout" 2> "$sidecar_stderr" &' not in sidecar +def test_gateway_preflight_rejection_prints_bounded_evidence_to_the_job_log() -> None: + """A rejected gateway preflight must surface error_code/http_status directly. + + Before this, the bounded ``error_code``/``http_status`` pair was written + only into the ``CONTEXTUAL_ORCHESTRATOR_PREFLIGHT_EVIDENCE`` artifact + file, invisible in the job log a CI operator reads first -- exactly the + gap that made a real "every free route rejected" failure look identical + to an opaque "gateway preflight returned HTTP 502" in normal CI output. + """ + sidecar = _SIDECAR.read_text(encoding="utf-8") + + assert ( + 'print(f"[contextual-orchestrator-sidecar] gateway preflight rejected: ' + 'error_code={code} http_status={status}")' + ) in sidecar + # This print is not routed through the sanitizer, so its inputs must stay + # bounded: code is regex-validated and status is a plain int, never raw + # provider response text. + assert ( + 'if not isinstance(code, str) or not re.fullmatch(r"[A-Za-z0-9_.-]{1,64}", code):' + in sidecar + ) + + def test_sidecar_stream_sanitizer_allowlists_only_bounded_diagnostics() -> None: """Provider bodies, exception messages, URLs, and secrets never reach artifacts.""" namespace = _load_sanitizer() @@ -488,6 +654,13 @@ def test_sidecar_stream_sanitizer_allowlists_only_bounded_diagnostics() -> None: assert sanitize_line( "provider_discovery_failed provider=bytez code=http_status_401" ) == "provider_discovery_failed provider=bytez code=http_status_401" + assert sanitize_line( + "preflight_route_rejected provider=nvidia_nim error_type=ProviderUpstreamError " + "http_status=429 upstream body sk-secret" + ) == "preflight_route_rejected provider=nvidia_nim error_type=ProviderUpstreamError http_status=429" + assert sanitize_line( + "preflight_route_rejected provider=bytez error_type=InvalidChatResponse" + ) == "preflight_route_rejected provider=bytez error_type=InvalidChatResponse" assert sanitize_line("provider response sk-secret") is None