diff --git a/.github/source-fix-1591-free-identity.trigger b/.github/source-fix-1591-free-identity.trigger new file mode 100644 index 0000000000..0b5696e49f --- /dev/null +++ b/.github/source-fix-1591-free-identity.trigger @@ -0,0 +1,2 @@ +repair central free-only reachability and runtime identity collision +attempt=v1-red-green diff --git a/.github/workflows/source-fix-1591-free-identity.yml b/.github/workflows/source-fix-1591-free-identity.yml new file mode 100644 index 0000000000..165d49182f --- /dev/null +++ b/.github/workflows/source-fix-1591-free-identity.yml @@ -0,0 +1,97 @@ +name: Source fix PR1591 free identity + +on: + push: + branches: [fix/no-heuristic-free-review-admission] + paths: [.github/source-fix-1591-free-identity.trigger] + +permissions: + contents: write + pull-requests: write + +jobs: + repair: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: fix/no-heuristic-free-review-admission + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 + with: + python-version: "3.12" + - name: Install exact test dependencies + run: >- + python -m pip install --disable-pip-version-check --require-hashes + --only-binary=:all: -r requirements-opencode-review-ci-hashes.txt + - name: Verify RED contract and apply owner-side repair + id: patch + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + branch_name='fix/no-heuristic-free-review-admission' + starting_head="$GITHUB_SHA" + echo "starting_head=$starting_head" >>"$GITHUB_OUTPUT" + live_ref="$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/heads/${branch_name}" --jq '.object.sha')" + pr_state="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/1591" --jq '.state')" + pr_head_ref="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/1591" --jq '.head.ref')" + test "$live_ref" = "$starting_head" + test "$pr_state" = open + test "$pr_head_ref" = "$branch_name" + + if python -m pytest -q \ + tests/test_contextual_orchestrator_no_heuristic_admission.py \ + tests/test_contextual_orchestrator_central_free_only.py; then + echo '::error::PR #1591 RED regressions unexpectedly pass before production repair.' + exit 91 + fi + + python scripts/ci/source_fix_1591_free_identity.py + + python -m pytest -q \ + tests/test_contextual_orchestrator_no_heuristic_admission.py \ + tests/test_contextual_orchestrator_central_free_only.py \ + tests/test_contextual_orchestrator_review_policy.py \ + tests/test_contextual_orchestrator_review_runtime_preflight.py \ + tests/test_contextual_orchestrator_review_sidecar_contract.py \ + tests/test_contextual_orchestrator_free_credential_admission.py \ + tests/test_contextual_orchestrator_review_live_discovery_contract.py + python -m coverage run -m pytest tests -q + python -m coverage report --fail-under=100 + python -m interrogate --fail-under=100 scripts/ci + git diff --check + + latest_ref="$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/heads/${branch_name}" --jq '.object.sha')" + latest_state="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/1591" --jq '.state')" + latest_head_ref="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/1591" --jq '.head.ref')" + test "$latest_ref" = "$starting_head" + test "$latest_state" = open + test "$latest_head_ref" = "$branch_name" + + rm -f \ + .github/workflows/source-fix-1591-free-identity.yml \ + .github/source-fix-1591-free-identity.trigger \ + scripts/ci/source_fix_1591_free_identity.py + git add -A + git diff --cached --check + git config user.name 'opencode-agent[bot]' + git config user.email '219766164+opencode-agent[bot]@users.noreply.github.com' + git commit -m 'fix(ci): close central free-pool identity gaps' + + - name: Push repaired exact head + env: + GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }} + STARTING_HEAD: ${{ steps.patch.outputs.starting_head }} + run: | + set -euo pipefail + if [ -z "${GH_TOKEN:-}" ]; then + echo '::error::No event-capable branch-write credential is configured.' + exit 1 + fi + branch_name='fix/no-heuristic-free-review-admission' + remote_head="$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/heads/${branch_name}" --jq '.object.sha')" + test "$remote_head" = "$STARTING_HEAD" + gh auth setup-git + git push origin HEAD:"$branch_name" diff --git a/CHANGELOG.md b/CHANGELOG.md index d7c6d40ae7..ba26530765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Remove the shared first-come-first-served review preflight escalation quota. Every route that emits the explicit budget-starvation signature now receives its own single evidence-bearing escalation, so catalog order cannot deny later eligible routes a viability test; primary/fallback escalation counts remain audit telemetry only. +- Remove the retired Noema/OpenCode catalog cardinality heuristics from the review launcher and sidecar. Evidence-eligible routes are no longer truncated before runtime preflight, auto-mode keeps the full priced fallback set, and legacy `limit`/`account_cap` inputs are accepted only as ignored compatibility arguments. This closes the >12-route startup crash found by Devin Review without making serialization order or provider identity a routing preference. - **Fix `opencode-review.yml` admission gaps around stale/out-of-order events (`#1568`).** Building on the draft-poll exemption's live PR/head validation, Devin Review found two further defects. (1) The concurrency group was keyed only by repository and PR number, so diff --git a/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md b/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md index 7b9ea7e1ac..2b686489fe 100644 --- a/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md +++ b/docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md @@ -1,228 +1,50 @@ -# ADR-0003: Vendored contextual-orchestrator review sidecar with governed gateway pools +# ADR-0003: Vendored contextual-orchestrator review sidecar with governed free pool -- Status: accepted, amended 2026-08-30 (see "2026-08-30 amendment" below — Strix - now uses `orchestrator/free`, not the `orchestrator/auto` this header - originally recorded) +- Status: Accepted; consolidated 2026-09-01 - Date: 2026-08-27 -- Scope: ContextualWisdomLab/.github central review pipelines (OpenCode autofix/dispatch + shared `opencode.jsonc` default + required Noema + Strix review) -- Decision: Route every central CI review write/model execution that touches contracts in this repository through the **vendored** `contextual-orchestrator` gateway, served as a per-runner sidecar. OpenCode, Noema, and (as of the 2026-08-30 amendment) Strix all use the fail-closed zero-cost virtual model id `orchestrator/free`. **Zero Data Retention (ZDR)-compliant routes remain mandatory for private targets.** -- Ownership: `.github` owns control-plane evidence; `ContextualWisdomLab/contextual-orchestrator` owns the gateway. The 2026-08-18 org decision (recorded in `ContextualWisdomLab/contextual-orchestrator` AGENTS.md) already migrated OpenCode/Noema/Strix to the orchestrator backend; this ADR is the org-repo (provider-config) half of that decision. -- Figma File ID: N/A (no customer UI). - -## Context - -Central review paths previously pinned direct provider endpoints and hard-coded -model ids (e.g. `nvidia-nim/mistralai/mistral-small-4-119b-2603` in the PR -autofix writer). Provider keys were consumed from Actions env at the OpenCode -layer, and no path used the org's five-key auto-discovery. The orchestrator's -AGENTS.md (2026-08-18) commits the org to a shared gateway: register -`BYTEZ_API_KEY`, `NVIDIA_NIM_API_KEY`, `NVIDIA_NIM_API_KEY_SUB`, -`OPENROUTER_API_KEY`, `OPENAI_API_KEY` into its KV, auto-discover models across -all five, and auto-optimize routing by cost. - -## Decision - -1. **Vendoring, pinned**: `scripts/ci/contextual_orchestrator_review_sidecar.sh` - clones `ContextualWisdomLab/contextual-orchestrator` at an exact SHA - (`8cd99f139915131ba0239bce12a5d6a5fd85394e` today) into `RUNNER_TEMP`. The - source's `requirements.lock` is installed with `--require-hashes` and - `--no-deps`, so dependency resolution cannot silently move the reviewed - runtime. - runtime entry (`contextual_orchestrator_review_launcher.py`) registers the - five provider secrets plus the gateway bearer token into the process-local - KV in the **same process** that performs model discovery and serves - `/v1/chat/completions` and `/v1/responses` on loopback. Env is bootstrap - transport only; request-time credential reads go through the KV. -2. **Auto model discovery + governed virtual pools**: discovery runs with the - orchestrator's own `discover_all_models()` against the KV credentials. - OpenCode and Noema admit only zero-priced routes. Strix admits two explicit - evidence tiers: zero-priced first, then routes with finite, - nonnegative prompt and completion prices plus an explicit currency. Routes - without a complete published price vector remain counted for audit but are - not admitted to CI review. A missing pair is never relabeled free or - price-attested; a partial price vector, malformed numeric value, conflicting - free marker, or missing currency for a published vector fails closed. The gateway's - `orchestrator/free` virtual id fails closed (`400 invalid_model`) unless an - enabled zero-cost agent exists. Strix uses `orchestrator/auto`; its catalog - may admit priced routes only through this evidence-bearing - policy, never through a direct-provider model identifier. - The auto pool probes the free catalog first. Only when every selected free - route rejects the real runtime request contract does it rebuild once from - fully price-attested routes and record the rejected primary attempt. This is - evidence-triggered failover, not an arbitrary free/paid mixing ratio. - Both stages share one twelve-route startup budget: no more than eight routes - enter the free primary stage and only its remaining capacity may enter priced - fallback. Full discovery counts remain in policy evidence, and the transient - priced catalog is removed immediately after loading. -3. **ZDR-first within each cost tier**: `scripts/ci/zdr_policy.py` defines ZDR - the way OpenRouter does ("a provider will not store your data for any period - of time"; zero retention also implies no training) and is deliberately - conservative: any provider whose zero-retention guarantee cannot be - attested from a machine-readable, dated source is treated as non-ZDR, - mirroring OpenRouter's stance on unascertained policies. The - OpenRouter `/api/v1/endpoints/zdr` feed (documented, auto-updated) is - fetched when egress allows it and is authoritative for the `openrouter` - scope; otherwise the dated static attestation table is used, never a - fabricated policy. - For private targets, ZDR admission is applied before choosing the cost tier. - A discovered but non-ZDR free route therefore cannot suppress an attested - priced route; when an admitted free tier exists it remains the exclusive - primary, and the admitted priced tier remains fallback-only. - `scripts/ci/contextual_orchestrator_review_policy.py` turns the discovery - report into a free-first, cost-evidence-ranked, ZDR-prioritized, - credential-account-diverse agents catalog, capped in size, in the - orchestrator's own `ModelAgent` schema. Every KV credential is an independent - account; vendor or endpoint identity does not imply model equivalence. Only - explicit `model_group` membership may share routing evidence. -4. **Wiring**: `pr-review-autofix.yml` and the Required OpenCode dispatch - provision the sidecar with the five secrets before OpenCode runs and point - every model/diagnosis candidate at `contextual-orchestrator/orchestrator/free`; - the generated dispatch config contains only the gateway provider. The shared - `opencode.jsonc` default `model`/`small_model` is the same gateway route. - `noema-review.yml` retains `orchestrator/free`. `strix.yml` provisions the - same sidecar and uses the loopback chat-completions/API-compatible URL with - `orchestrator/auto`: the 2026-08-29 exact-head DiskSage scan proved that four - discovered free routes all shared the OpenRouter outage domain, which the - gateway correctly collapsed to one provider attempt. Strix therefore uses - the provider-diverse pool supplied by all five configured credentials. - Provider diversity and cost-evidence classification remain delegated to the - gateway rather than embedding a second routing policy in GitHub Actions. - Strix has no external fallback and private targets pass visibility through - to the gateway's ZDR requirement. Noema reviewer identity remains - `NOEMA_REVIEW_TOKEN` / GitHub App / OIDC and is still never `github.token`; - Autofix mutation still requires `PR_REVIEW_MERGE_TOKEN` / - `OPENCODE_APPROVE_TOKEN` / the exchanged OpenCode app token, never - `github.token`; model subprocesses still run with - `GITHUB_TOKEN`/`GH_TOKEN`/OIDC request env stripped. -5. **Evidence**: the sidecar writes a discovery report, the policy report (pool, - total/free/priced/unknown counts, selected counts by admitted cost tier, ZDR - sources, feed-used flag, selected routes), and exports - `CONTEXTUAL_ORCHESTRATOR_EVIDENCE`; these are auditable per run. -6. **Review request envelope**: the library keeps its generic 64 KiB default, - while this loopback, bearer-authenticated, per-job sidecar configures a - 512 MiB ceiling so inline image inputs can reach routing. This follows the - OpenAI image-input limit of 512 MB total payload per request; it is not - treated as a universal JSON default or as the Files API's separate 512 MB - per-file limit. The sidecar startup probe verifies the configured HTTP - boundary before any review model runs. - -## Consequences - -- The autofix/OpenCode review paths no longer hard-code any provider base URL - or model id; upstream model selection is delegated to the orchestrator's - discovery under the zero-cost pool. Strix uses the separately governed auto - pool without treating absent price metadata as either free or paid-route - evidence. -- Strix delegates selection to `orchestrator/auto`. Its correctness-first pool - remains distinct from the zero-cost OpenCode/Noema pool, while private-target - ZDR admission remains fail-closed. Unknown-cost routes remain auditable but - ineligible; free and fully price-attested routes are the only review routes. -- Workers need egress to the five provider model-list hosts and, when reachable, - `https://openrouter.ai/api/v1/endpoints/zdr`; the feed failure path is - graceful (static table). -- A new central dep (vendored repo pinned to a SHA) must be reviewed when the - orchestrator upgrades; the pin is centralized in one script and one contract - test. -- `noema-review.yml`, `strix.yml`, and the Required OpenCode dispatch now - review through the same gateway; direct provider model routes and Strix - external fallbacks are gone from these required workflows. Reviewer and - mutation identities are unchanged. The hourly-review-repair roster is not - collapsed here. - -## References (and ZDR standardization) - -- OpenRouter. (2026, August). *Zero data retention* [Documentation]. https://openrouter.ai/docs/guides/features/zdr -- OpenRouter. (2026, August). *Provider logging: Data retention & logging* [Documentation]. https://openrouter.ai/docs/guides/privacy/provider-logging -- OpenRouter. (n.d.). *List all models and their properties* API reference; the per-model data-retention metadata (`data_retention: crichton | none`) and the ZDR endpoint feed `https://openrouter.ai/api/v1/endpoints/zdr` are consumed at runtime. -- ContextualWisdomLab/contextual-orchestrator. (2026, August 18). *AGENTS.md*, section “Policy change” — org migration of OpenCode/Noema/Strix to the gateway with the five KV credentials and auto-discovery. -- OpenAI. (n.d.). *Images and vision: Image input requirements*. - https://developers.openai.com/api/docs/guides/images-vision -- OpenAI. (n.d.). *Create file* [API reference]. - https://developers.openai.com/api/reference/resources/files/methods/create - -- **Private-target boundary (2026-08-27):** Noema resolves target visibility with - the selected repository-scoped reviewer token. Private/internal repositories - set `CONTEXTUAL_ORCHESTRATOR_REQUIRE_ZDR=true`; the catalog then excludes - every non-ZDR route and fails closed when no attested ZDR route exists in the - selected workflow pool. - -- **2026-08-30 amendment: Strix uses `orchestrator/free`, superseding this - ADR's original `orchestrator/auto` decision.** The org owner explicitly - directed Strix off the paid-inclusive `orchestrator/auto` pool and onto the - same zero-cost `orchestrator/free` pool OpenCode and Noema already use, so - no central review path executes a paid model. This is a deliberate, - informed override of the original decision above, not an oversight of it: - the trade-off the original decision recorded — "the 2026-08-29 exact-head - DiskSage scan proved that four discovered free routes all shared the - OpenRouter outage domain, which the gateway correctly collapsed to one - provider attempt... Strix has no external fallback" — was surfaced to the - owner explicitly, including a live 2026-08-30 reproduction of that same - single-family-collapse pattern (a `strix` run's `orchestrator/auto` - primary/free stage rejected 4/4 candidates — 2 timeouts, 2 HTTP 404s from - retired NVIDIA-hosted models — and only the `auto` pool's paid fallback - kept that run alive; see `docs/product-technical-gap-baseline.md`'s - 2026-08-30 sidecar-preflight entries for the full evidence trail). The - owner's response, verbatim in substance: implement the free-only directive - as originally instructed. **Accepted consequence**: Strix has no external - fallback and can go fully dark (rather than degraded-but-running) during - the exact class of incident this ADR originally used `orchestrator/auto` - to survive, until the free-catalog's stale-model and provider-diversity - gaps documented alongside this amendment are separately closed. This is - the owner's accepted risk, not an unnoticed regression. - `scripts/ci/strix_quick_gate.sh`'s `is_contextual_orchestrator_model` no - longer accepts `orchestrator/auto`; `strix.yml`'s `STRIX_MODEL`/ - `CONTEXTUAL_ORCHESTRATOR_POOL` default to `orchestrator/free`; and - `scripts/ci/strix_required_workflow_smoke.sh`/`AGENTS.md` were updated to - match. The `orchestrator/auto` pool mode itself is unchanged and still - 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_account_diversity` in the catalog report — the count of - independently credentialed accounts (see `provider_account`) 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. -- **2026-08-31 amendment: Noema reviews independently of OpenCode.** Noema no - longer waits for an OpenCode approval, review-thread state, or other check - conclusions before calling the gateway and submitting its current-head - review. A colliding OpenCode reviewer credential fails closed. The Noema LLM - response must bind every formal verdict to exact LEFT/RIGHT changed lines - and publish structured adversarial probes. Executable, test, and workflow - changes require at least two distinct probes; other diffs require one. - `approve` admits only falsified regression hypotheses, while - `request_changes` requires a confirmed probe at a published finding. A - generic no-issues summary can no longer synthesize a green review. -- **2026-08-31 amendment: required OpenCode execution is initiated by the - required check.** The unprivileged `pull_request_target` bootstrap exchanges - GitHub OIDC for the repository-scoped OpenCode App token and requests the - existing central scheduler chain for the exact PR. That chain runs Strix - evidence first and then the privileged OpenCode dispatch; both model paths, - like Noema, provision the pinned contextual-orchestrator sidecar and use - `orchestrator/free`. The bootstrap still checks out no PR code and binds no - Actions secret. -- **2026-08-31 amendment: model inference has no repository- or - application-configured fixed wall-clock timeout.** - OpenCode, Noema, Strix, and their contextual-orchestrator sidecar MUST NOT - impose a fixed wall-clock timeout on model inference, including an initial - completion ping, warm-up, retry, repair verdict, or substantive review call. - A slow reasoning model such as DeepSeek is not unavailable merely because it - takes minutes or hours to produce tokens. Cancellation remains an explicit - operator or superseded-head action. The review bootstrap also MUST NOT impose - fixed wall-clock limits on loopback `/healthz`, DNS/TLS establishment, ZDR - metadata, or provider model-list discovery: those prerequisites can be slow - and a short bound can discard an otherwise usable route before inference. - A hosting platform or runner termination is an external capacity constraint, - not model-unavailability or review evidence. Such an interrupted run is - incomplete and non-authoritative: it MUST NOT approve, merge, or classify the - model as unavailable, and the exact head MUST be retried or resumed on a - runner capable of completing the work. - This amendment supersedes all fixed readiness and inference-attempt budgets - in ADR 0005. +- Scope: central OpenCode, Noema, and Strix review pipelines +- Ownership: `ContextualWisdomLab/.github` owns CI/control-plane wiring; `ContextualWisdomLab/contextual-orchestrator` owns provider discovery, candidate admission, routing, and inference. + +## Current decision + +Every central review model call goes through the vendored `ContextualWisdomLab/contextual-orchestrator` sidecar. OpenCode, Noema, and Strix use the virtual model `orchestrator/free`; private/internal targets additionally require ZDR and fail closed when no eligible ZDR route exists. The current reviewed sidecar source is pinned exactly to contextual-orchestrator commit `8cd99f139915131ba0239bce12a5d6a5fd85394e`; changing that supply-chain identity requires ordinary exact-head review and verification rather than an inferred compatible version. + +All five GitHub Secrets may be supplied to global contextual-orchestrator discovery: `BYTEZ_API_KEY`, `NVIDIA_NIM_API_KEY`, `NVIDIA_NIM_API_KEY_SUB`, `OPENROUTER_API_KEY`, and `OPENAI_API_KEY`. Credential discovery and free-pool candidate admission are separate contracts. `OPENAI_API_KEY` may be registered and may globally discover OpenAI models, but any row sourced through `OPENAI_API_KEY` is excluded from `orchestrator/free` candidate generation, preflight, routing, failover, fallback, serving, and durable free-pool persistence. The eligible provider-account sources for `orchestrator/free` are `BYTEZ_API_KEY`, `NVIDIA_NIM_API_KEY`, `NVIDIA_NIM_API_KEY_SUB`, and `OPENROUTER_API_KEY`, subject to the remaining explicit free/privacy/capability evidence predicates. + +`scripts/ci/contextual_orchestrator_review_policy.py` is an admission boundary, not a router. Every row that satisfies the explicit pool, credential-source, zero-cost, capability, and when required ZDR predicates remains admitted with neutral priority. Serialization order is provenance only. The policy MUST NOT use candidate-count caps, per-account quotas, provider-family quotas, provider/model/cost sorting, hand-authored priorities, arbitrary fallback ratios, model-name inference, or any other heuristic to change candidate membership or preference. Legacy `limit` and `account_cap` parameters may remain temporarily as ignored compatibility inputs, but their values are non-authoritative. + +The historical twelve-route total catalog cap, eight-route primary cap, per-account cap, `priority=-rank`, cost/provider ordering, and Strix `orchestrator/auto` paid-fallback design are superseded. Incident evidence that motivated those controls remains useful for observability and research, but an incident-derived rule is not a valid decision policy without an explicit mathematical/statistical/psychometric model, authoritative standard, experimentally validated evidence, or documented research-backed algorithm with executable provenance. + +No heuristic, rule of thumb, hand-tuned threshold, arbitrary weight, ad-hoc score, undocumented tie break, name-based inference, or magic-number decision rule may determine routing, model selection, test-time-compute allocation, response-quality scoring, RAG evaluation, weighting, thresholding, admission, fallback order, or prioritization. If the required evidence is unavailable, the system fails closed or records unresolved evidence; it does not invent a substitute heuristic. + +Model inference has no repository- or application-configured fixed wall-clock cutoff. OpenCode, Noema, Strix, and the contextual-orchestrator sidecar **MUST NOT impose a fixed wall-clock timeout on model inference**, including initial completion ping, warm-up, retry, repair verdicts, or substantive review calls. A slow reasoning model is not classified as unavailable merely because it runs for minutes or hours. Explicit operator cancellation, exact-head supersession, and an external runner/platform termination remain observable lifecycle events; an externally interrupted run is incomplete evidence and cannot become an approval or availability judgment. The same principle applies to bootstrap discovery/readiness paths when a fixed local deadline would silently convert an otherwise usable provider into a negative routing signal. + +Reference-free/model-response quality evaluation uses the `ContextualWisdomLab/fast-mlsirm` psychometric/statistical boundary where applicable. The GitHub policy layer does not synthesize a model-quality scalar. + +The sidecar retains secret-free discovery, admission, and runtime-preflight evidence. Raw credentials, prompts, and unredacted provider error bodies are never persisted in ordinary evidence. Exact-head GitHub Checks and current review findings remain authoritative for merge. + +## Verification contract + +Executable tests must prove at least that: + +1. all five credentials may be supplied and globally discovered; +2. the four free-eligible credential sources are considered independently; +3. OpenAI may be globally discovered while contributing zero `orchestrator/free` candidates; +4. OpenAI-derived rows cannot enter free-pool preflight, fallback, failover, serving, or durable persistence; +5. more than the historical catalog cap can remain admitted without truncation or launcher failure; +6. legacy cap arguments cannot alter admission, ordering, or priority; +7. private targets cannot bypass ZDR admission; +8. logs and artifacts contain no secret values; +9. the accepted ADR names the exact vendored sidecar commit and forbids fixed wall-clock inference timeouts. + +## References + +Chen, L., Zaharia, M., & Zou, J. (2024). FrugalGPT: How to use large language models while reducing cost and improving performance. *Transactions on Machine Learning Research*. https://arxiv.org/abs/2305.05176 + +Ong, I., Almahairi, A., Wu, V., Chiang, W.-L., Wu, T., Gonzalez, J. E., Kadous, M. W., & Stoica, I. (2024). *RouteLLM: Learning to route LLMs with preference data* [Preprint]. arXiv. https://arxiv.org/abs/2406.18665 + +Xu, J., Sun, Q., Schwendeman, P., Nielsen, S., Cetin, E., & Tang, Y. (2026). *TRINITY: An evolved LLM coordinator* [Preprint]. arXiv. https://doi.org/10.48550/arXiv.2512.04695 + +OpenRouter. (2026). *Zero data retention*. https://openrouter.ai/docs/guides/features/zdr + +OpenRouter. (2026). *Provider logging: Data retention & logging*. https://openrouter.ai/docs/guides/privacy/provider-logging diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 9367d54f67..ccf902b73f 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2475,3 +2475,12 @@ Zhang, S., Yu, Y., Li, Y., Zhao, W., Yang, Y., Zhang, Y., & Liu, T. (2025). *Con Xu, J., Sun, Q., Schwendeman, P., Nielsen, S., Cetin, E., & Tang, Y. (2026). *TRINITY: An evolved LLM coordinator* [Preprint]. arXiv. https://doi.org/10.48550/arXiv.2512.04695 Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A research agenda for multiplexity beyond the average. *PLOS ONE, 16*(9), e0257527. https://doi.org/10.1371/journal.pone.0257527 + + +## 2026-09-01 Noema/OpenCode admission/runtime reconciliation + +Devin Review exposed a contract split in PR #1591: the policy layer correctly stopped truncating evidence-eligible routes, while the launcher still rejected any primary catalog larger than the historical 12-route preflight budget. The causal owner is the central `.github` launcher/sidecar boundary, not a leaf repository. The repair removes catalog cardinality and per-account caps from launcher admission, preserves the full primary and evidence-triggered priced fallback catalogs, and keeps neutral policy priority. Legacy `limit` and `account_cap` inputs remain accepted but are explicitly non-authoritative. Regression coverage includes >12 primary routes, >8 free routes with a priced fallback set, shared escalation evidence across a larger catalog, and arbitrary ignored compatibility values. The former `12 base attempts + 4 escalations = 160s` statement is historical rather than a current admission invariant; startup-latency control must not silently evict eligible routes without an independently justified decision model. + +### 2026-09-01 — Review preflight escalation order removed + +`_preflight_review_agents` no longer uses a shared first-come-first-served escalation quota. A route's explicit budget-starvation evidence authorizes one retry for that route independently of catalog position; primary and fallback escalation counts are retained only as audit telemetry. This removes the order-sensitive admission defect identified on PR #1591 without turning provider identity, route count, or an arbitrary shared quota into routing authority. diff --git a/scripts/ci/contextual_orchestrator_review_launcher.py b/scripts/ci/contextual_orchestrator_review_launcher.py index 2e56809639..a04783f5da 100644 --- a/scripts/ci/contextual_orchestrator_review_launcher.py +++ b/scripts/ci/contextual_orchestrator_review_launcher.py @@ -12,7 +12,7 @@ in-process (so the KV-backed credentials are visible to it), the zero-cost ("free") routes are collected into a report, and ``scripts/ci/contextual_orchestrator_review_policy.py`` turns that report into a -ZDR-prioritized, credential-account-diverse catalog for ``orchestrator/free``. +evidence-admitted catalog for ``orchestrator/free``; routing preference remains owned by the orchestrator's explicit evidence model. Keeping the decision logic in that stdlib-only module lets every branch of the ZDR policy be tested offline in this repository while ``orchestrator/free`` still resolves from authentically zero-priced models discovered by the @@ -42,8 +42,6 @@ # Provider-neutral sampling: several modern endpoints reject non-default # temperatures, while 1.0 is the OpenAI-compatible default. REVIEW_TEMPERATURE = 1.0 -REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES = 12 -REVIEW_PREFLIGHT_PRIMARY_ROUTE_LIMIT = 8 # ADR-0005: a single fixed max_tokens cannot fit every model in a heterogeneous # pool -- some spend internal reasoning tokens before visible content and need # more, others have a real completion ceiling a large budget would exceed. The @@ -59,9 +57,6 @@ # already-proven-working REVIEW_MAX_OUTPUT_TOKENS rather than inventing a new # number. REVIEW_PREFLIGHT_ESCALATED_TOKENS = REVIEW_MAX_OUTPUT_TOKENS -# Shared cap on how many candidates in one preflight run may use the -# escalation retry above. It bounds request count, never model response time. -REVIEW_PREFLIGHT_MAX_ESCALATIONS = 4 class ReviewPreflightError(RuntimeError): @@ -318,7 +313,7 @@ def _response_has_reasoning_without_content(response: object) -> bool: def _preflight_review_agents( - agents: list[object], *, client: Any, escalations_used: int = 0 + agents: list[object], *, client: Any ) -> tuple[list[object], dict[str, object]]: """Probe each route with the runtime request contract and keep ready routes. @@ -334,14 +329,9 @@ def _preflight_review_agents( across the pool, and this is the exact original failure mode PR #1436 responded to) -- that *same* candidate is retried once at a larger, escalated budget (``REVIEW_PREFLIGHT_ESCALATED_TOKENS``) before being - marked rejected -- bounded by a shared ``REVIEW_PREFLIGHT_MAX_ESCALATIONS`` - counter, which the ``escalations_used`` argument carries forward across - calls (not per candidate, and not reset per call): a caller that probes - two stages of the same preflight run (e.g. ``_preflight_with_fallback``'s - primary and fallback stages) must pass the previous stage's ending count - back in here so the two stages share one budget instead of each getting - its own -- otherwise the computed worst-case bound this counter exists to - enforce silently doubles. Every other failure class (transport exception, + marked rejected. The retry decision is local to that candidate and + cannot be exhausted by earlier catalog entries. Every other failure class + (transport exception, non-2xx, or empty content matching neither signature) is not retried: a genuinely-down candidate never reaches the escalation path, so it cannot produce a false "healthy" read. @@ -371,21 +361,17 @@ def _preflight_review_agents( Args: agents: Selected zero-cost model agents. client: Vendored ``ModelClient``-compatible transport. - escalations_used: Escalations already spent earlier in this same - preflight run (e.g. by a prior stage), so the shared budget is - honored across calls rather than restarted at zero. - Returns: A pair of viable agents and a sanitized preflight report. The - report's ``escalations_used`` is the running total including - ``escalations_used``'s starting value, so a caller chaining another - stage can pass it straight back in. + report's ``escalations_used`` is observed telemetry for this + stage only and never an admission quota. Raises: ReviewPreflightError: If no provider route returns usable text. """ viable: list[object] = [] routes: list[dict[str, object]] = [] + escalations_used = 0 for agent in agents: row: dict[str, object] = { "agent_id": str(getattr(agent, "id", "")), @@ -439,28 +425,9 @@ def _preflight_review_agents( reasoning_without_content = _response_has_reasoning_without_content(response) row["reasoning_without_content"] = reasoning_without_content budget_signature = finish_reason == "length" or reasoning_without_content - # KNOWN, ACCEPTED, TRACKED LIMITATION on the escalations_used >= - # REVIEW_PREFLIGHT_MAX_ESCALATIONS branch below, ContextualWisdomLab/.github#1458 - # (originally documented on ADR-0005, docs/adr/0005-sidecar-preflight-token-budget.md): - # escalations_used is one shared, first-come-first-served counter for - # the whole run, consumed in catalog order - # (build_zdr_prioritized_catalog's (cost_evidence_rank, - # zdr_attested_rank, provider, model) sort, not random). A - # later-sorting candidate can be denied its own escalation attempt - # purely because REVIEW_PREFLIGHT_MAX_ESCALATIONS earlier candidates - # already claimed the shared budget -- even if it would have been the - # only one to succeed at REVIEW_PREFLIGHT_ESCALATED_TOKENS. - # Deliberately not reordered (round-robin/random): a fixed-size - # shared budget smaller than the candidate pool always has to deny - # someone an escalation, so reordering only changes who, and picking - # a specific policy without real telemetry on which candidates - # actually need escalation would itself be the kind of unjustified - # heuristic this design rejects elsewhere. - if not budget_signature or escalations_used >= REVIEW_PREFLIGHT_MAX_ESCALATIONS: + if not budget_signature: row["status"] = "rejected" - row["error_type"] = ( - "invalid_chat_response" if not budget_signature else "escalation_budget_exhausted" - ) + row["error_type"] = "invalid_chat_response" routes.append(row) continue escalations_used += 1 @@ -513,7 +480,6 @@ def _preflight_review_agents( "ready_count": len(viable), "rejected_count": len(agents) - len(viable), "escalations_used": escalations_used, - "escalation_budget": REVIEW_PREFLIGHT_MAX_ESCALATIONS, "routes": routes, } if not viable: @@ -528,17 +494,12 @@ def _preflight_with_fallback( ) -> tuple[list[object], dict[str, object], bool]: """Use the priced catalog only after every primary route rejects. - The two stages share ADR-0005's one ``REVIEW_PREFLIGHT_MAX_ESCALATIONS`` - budget for the whole preflight run, not one budget each: the primary - stage's ending ``escalations_used`` is passed as the fallback stage's - starting point, so a run that rejects all 8 primary routes and then - probes 4 fallback routes still spends at most 4 escalations total (12 - base attempts + 4 escalations). This bounds request count, not individual - model response or sidecar readiness time. Both - stages' reports remain in the result: the fallback (or sole) stage's - report carries the run's final, cumulative ``escalations_used``, and - ``primary_attempt`` nests the primary stage's own report -- including its - own ``escalations_used`` -- whenever a fallback stage ran at all. + Each candidate keeps the same two-attempt evidence contract used by + ``_preflight_review_agents``. A candidate that returns the explicit + budget-starvation signature receives exactly one larger-budget retry; + another candidate's earlier position cannot consume or deny that retry. + Primary and fallback reports preserve their own observed escalation counts + for audit without using those counts as admission authority. """ try: viable, report = _preflight_review_agents(primary_agents, client=client) @@ -546,10 +507,9 @@ def _preflight_with_fallback( except ReviewPreflightError as primary_error: if not fallback_agents: raise - escalations_used = int(primary_error.report.get("escalations_used", 0)) try: viable, report = _preflight_review_agents( - fallback_agents, client=client, escalations_used=escalations_used + fallback_agents, client=client ) except ReviewPreflightError as fallback_error: fallback_error.report["primary_attempt"] = primary_error.report @@ -619,60 +579,6 @@ def _write_json(path: str, payload: object) -> None: ) -def _bounded_primary_catalog_limit( - requested_limit: int, *, pool: str, has_free_rows: bool -) -> int: - """Return the primary-stage route limit within one startup budget.""" - if requested_limit < 1: - raise ValueError("ORCHESTRATOR_CATALOG_LIMIT must be positive") - total_limit = min(requested_limit, REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES) - if pool == "auto" and has_free_rows: - return min(total_limit, REVIEW_PREFLIGHT_PRIMARY_ROUTE_LIMIT) - return total_limit - - -def _bounded_fallback_catalog_limit( - requested_limit: int, *, primary_count: int -) -> int: - """Return remaining priced-fallback capacity after primary selection.""" - if requested_limit < 1: - raise ValueError("ORCHESTRATOR_CATALOG_LIMIT must be positive") - total_limit = min(requested_limit, REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES) - if primary_count < 0 or primary_count > total_limit: - raise ValueError("primary route count exceeds the preflight budget") - return total_limit - primary_count - - -def _catalog_account_cap(default: int) -> int: - """Return the configured per-account catalog admission cap. - - ``default`` must be ``scripts.ci.contextual_orchestrator_review_policy``'s - own ``DEFAULT_ACCOUNT_CAP`` -- the single source of truth for how many - routes one credential account may contribute to the bounded preflight - budget. A caller must never substitute a total-routes-scale constant - (e.g. ``REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES``) here: doing so silently - disables per-account diversification and lets one rate-limited account - consume the entire preflight budget. That is not a hypothetical failure - mode -- a sibling in-flight branch's own ``_catalog_family_cap()`` - fell back to exactly ``REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES`` and, in a live - production run, let two NVIDIA NIM credentials sharing one rate-limited - upstream jointly occupy 12/12 preflight slots, of which 10 were then - rejected with 429/404/timeout (see ContextualWisdomLab/.github#1415 and - the "빈 깡통 경로" report it responds to). Routing the default through the - caller-supplied ``policy.DEFAULT_ACCOUNT_CAP`` (rather than hand-typing a - literal here) keeps this module's cap from silently drifting out of sync - with the policy module's own declared intent. - - Args: - default: The cap to use when ``ORCHESTRATOR_CATALOG_ACCOUNT_CAP`` is - unset, always ``policy.DEFAULT_ACCOUNT_CAP``. - - Returns: - The per-account cap to pass to ``build_zdr_prioritized_catalog``. - """ - return int(os.environ.get("ORCHESTRATOR_CATALOG_ACCOUNT_CAP", str(default))) - - def _with_discovery_counts( report: dict[str, object], rows: list[dict[str, Any]], @@ -794,7 +700,6 @@ def main(argv: list[str] | None = None) -> int: ) from contextual_orchestrator.server import SecurityConfig, serve from scripts.ci.contextual_orchestrator_review_policy import ( - DEFAULT_ACCOUNT_CAP, PolicyError, _load_zdr_endpoints, build_zdr_prioritized_catalog, @@ -856,10 +761,6 @@ def main(argv: list[str] | None = None) -> int: zdr_endpoints=zdr_endpoints, checker=is_zdr_model, ) - requested_catalog_limit = int(os.environ.get("ORCHESTRATOR_CATALOG_LIMIT", "12")) - primary_limit = _bounded_primary_catalog_limit( - requested_catalog_limit, pool=args.pool, has_free_rows=bool(admitted_free_rows) - ) primary_rows = ( (admitted_free_rows or admitted_priced_rows) if args.pool == "auto" @@ -867,8 +768,6 @@ def main(argv: list[str] | None = None) -> int: ) result = build_zdr_prioritized_catalog( primary_rows, - limit=primary_limit, - account_cap=_catalog_account_cap(DEFAULT_ACCOUNT_CAP), zdr_endpoints=zdr_endpoints, require_zdr=args.require_zdr, pool=args.pool, @@ -886,20 +785,14 @@ def main(argv: list[str] | None = None) -> int: primary_report = result["report"] fallback_result = None fallback_agents: list[object] = [] - fallback_limit = _bounded_fallback_catalog_limit( - requested_catalog_limit, primary_count=len(result["agents"]) - ) if ( args.pool == "auto" and admitted_free_rows and admitted_priced_rows - and fallback_limit ): try: fallback_result = build_zdr_prioritized_catalog( admitted_priced_rows, - limit=fallback_limit, - account_cap=_catalog_account_cap(DEFAULT_ACCOUNT_CAP), zdr_endpoints=zdr_endpoints, require_zdr=args.require_zdr, pool="auto", diff --git a/scripts/ci/contextual_orchestrator_review_policy.py b/scripts/ci/contextual_orchestrator_review_policy.py index 53e66cfa36..222f354d21 100644 --- a/scripts/ci/contextual_orchestrator_review_policy.py +++ b/scripts/ci/contextual_orchestrator_review_policy.py @@ -6,6 +6,13 @@ policy permits them. Models without a complete price vector remain visible in audit counts but are never admitted to CI review. Partial, malformed, or contradictory price vectors fail closed. + +This module is an admission boundary, not a router. It therefore must not invent +candidate-count caps, per-provider quotas, price/ZDR/provider ordering, hand-set +priorities, or fallback preferences. Every row satisfying the explicit pool, +price, credential-source, and optional ZDR predicates remains admitted with +neutral priority. Downstream model choice requires its own evidence-backed +routing contract. """ from __future__ import annotations @@ -15,7 +22,6 @@ import math import re import sys -from collections import Counter from pathlib import Path from typing import Any, Iterable, Mapping @@ -29,6 +35,9 @@ route_key, ) +# Compatibility-only values retained while callers migrate away from the old +# command surface. They are deliberately ignored by admission and therefore do +# not affect candidate membership, ordering, or priority. DEFAULT_CATALOG_LIMIT = 12 DEFAULT_ACCOUNT_CAP = 4 @@ -49,11 +58,7 @@ COST_FREE = "free" COST_PRICED = "priced" COST_UNKNOWN = "unknown" -_COST_EVIDENCE_RANK: Mapping[str, int] = { - COST_FREE: 0, - COST_PRICED: 1, - COST_UNKNOWN: 2, -} +_COST_EVIDENCE_VALUES = frozenset({COST_FREE, COST_PRICED, COST_UNKNOWN}) _AGENT_ID_RE = re.compile(r"^[a-z][a-z0-9]*_[a-z0-9]+(?:_[a-z0-9]+)*$") @@ -73,7 +78,10 @@ def _normalize_agent_id(candidate: str, provider_name: str) -> str: parts = [part for part in slug.split("_") if part] if len(parts) == 1: parts.insert(0, provider_name) - return "_".join(parts) + normalized = "_".join(parts) + if not _AGENT_ID_RE.fullmatch(normalized): + raise PolicyError(f"model agent id {candidate!r} cannot be normalized safely") + return normalized def _route_key(provider_name: str, model: str) -> str: @@ -209,7 +217,7 @@ def parse_discovery_report(report: Mapping[str, Any]) -> list[dict[str, Any]]: def _cost_evidence(row: Mapping[str, Any]) -> str: """Return a validated cost-evidence tier from a normalized row.""" evidence = row.get("cost_evidence") - if evidence in _COST_EVIDENCE_RANK: + if evidence in _COST_EVIDENCE_VALUES: return str(evidence) # Backward compatibility for callers that build normalized-like rows by # hand rather than using parse_discovery_report(). @@ -228,23 +236,24 @@ def _free_pool_source_admitted(row: Mapping[str, Any]) -> bool: def build_zdr_prioritized_catalog( rows: Iterable[Mapping[str, Any]], *, - limit: int = DEFAULT_CATALOG_LIMIT, - account_cap: int = DEFAULT_ACCOUNT_CAP, + limit: object = DEFAULT_CATALOG_LIMIT, + account_cap: object = DEFAULT_ACCOUNT_CAP, zdr_endpoints: frozenset[str] = frozenset(), require_zdr: bool = False, pool: str = "free", ) -> dict[str, Any]: - """Select a free-first, ZDR-aware, credential-account-diverse catalog. - - ``orchestrator/free`` first applies a source-identity invariant: only rows - whose credential source is in :data:`FREE_POOL_CREDENTIAL_NAMES` are free - candidates. This is independent from global credential discovery, so an - OpenAI model may remain visible to audit or ``orchestrator/auto`` while - contributing zero free-pool candidates. - - Existing discovery-wide counters keep their historical meaning so runtime - enrichment cannot silently rewrite the contract. Additional - ``free_pool_*`` fields expose the narrower admitted subset explicitly. + """Admit every route satisfying explicit pool and evidence predicates. + + ``limit`` and ``account_cap`` remain accepted only so older callers can roll + forward without a flag-day. They are intentionally non-authoritative and + are not inspected, validated, serialized, or allowed to remove, rank, or + prioritize a candidate. The admission set is fully determined by explicit + cost evidence, ``orchestrator/free`` credential-source authorization, and + the caller's optional ZDR requirement. + + Input order is preserved only as discovery provenance. Every emitted agent + has neutral priority, so this module does not convert that serialization + order into routing authority. """ if pool not in {"free", "auto"}: raise PolicyError(f"unsupported review pool {pool!r}") @@ -255,9 +264,15 @@ def build_zdr_prioritized_catalog( all_priced_rows = [row for row in all_rows if _cost_evidence(row) == COST_PRICED] all_unknown_rows = [row for row in all_rows if _cost_evidence(row) == COST_UNKNOWN] candidate_rows = ( - free_pool_rows if pool == "free" else [*all_free_rows, *all_priced_rows] + free_pool_rows + if pool == "free" + else [ + row + for row in all_rows + if _cost_evidence(row) in {COST_FREE, COST_PRICED} + ] ) - eligible_rows = [ + picked = [ row for row in candidate_rows if not require_zdr @@ -267,31 +282,6 @@ def build_zdr_prioritized_catalog( zdr_endpoints=zdr_endpoints, ) ] - eligible_rows.sort( - key=lambda row: ( - _COST_EVIDENCE_RANK[_cost_evidence(row)], - 0 - if is_zdr_model( - str(row["provider"]), - model=str(row["model"]), - zdr_endpoints=zdr_endpoints, - ) - else 1, - str(row["provider"]), - str(row["model"]), - ) - ) - - per_account: Counter[str] = Counter() - picked: list[Mapping[str, Any]] = [] - for row in eligible_rows: - account = provider_account(str(row["provider"])) - if per_account[account] >= account_cap: - continue - per_account[account] += 1 - picked.append(row) - if len(picked) >= limit: - break if not picked: route_kind = "attested ZDR" if require_zdr else pool @@ -302,13 +292,11 @@ def build_zdr_prioritized_catalog( catalog_rows: list[dict[str, Any]] = [] zdr_count = 0 - for rank, row in enumerate(picked): + for row in picked: provider = str(row["provider"]) model = str(row["model"]) evidence = _cost_evidence(row) - zdr = is_zdr_model( - provider, model=model, zdr_endpoints=zdr_endpoints - ) + zdr = is_zdr_model(provider, model=model, zdr_endpoints=zdr_endpoints) if zdr: zdr_count += 1 catalog_rows.append( @@ -323,7 +311,7 @@ def build_zdr_prioritized_catalog( f"cost:{evidence}", "zdr" if zdr else "non-zdr", ], - "priority": -rank, + "priority": 0, "disabled": False, "provider_name": provider, "provider_exclusions": [], @@ -341,7 +329,6 @@ def build_zdr_prioritized_catalog( free_pool_account_diversity = len( {provider_account(str(row["provider"])) for row in free_pool_rows} ) - selected_evidence = [_cost_evidence(row) for row in picked] return { "agents": catalog_rows, @@ -361,6 +348,8 @@ def build_zdr_prioritized_catalog( "priced_selected_count": selected_evidence.count(COST_PRICED), "unknown_selected_count": selected_evidence.count(COST_UNKNOWN), "zdr_selected_count": zdr_count, + "legacy_limit_ignored": True, + "legacy_account_cap_ignored": True, "zdr_sources": sorted( { provider_zdr_scope(str(row["provider"])).source @@ -411,8 +400,8 @@ def build_catalog_from_paths( *, out_path: str, report_path: str, - limit: int = DEFAULT_CATALOG_LIMIT, - account_cap: int = DEFAULT_ACCOUNT_CAP, + limit: object = DEFAULT_CATALOG_LIMIT, + account_cap: object = DEFAULT_ACCOUNT_CAP, zdr_endpoints_path: str | None = None, require_zdr: bool = False, pool: str = "free", @@ -448,8 +437,16 @@ def _build_parser() -> argparse.ArgumentParser: ) parser.add_argument("--out", required=True, help="Path to write agents JSON") parser.add_argument("--report", required=True, help="Path to write audit JSON") - parser.add_argument("--limit", type=int, default=DEFAULT_CATALOG_LIMIT) - parser.add_argument("--account-cap", type=int, default=DEFAULT_ACCOUNT_CAP) + parser.add_argument( + "--limit", + default=DEFAULT_CATALOG_LIMIT, + help="Deprecated compatibility input; does not affect admission.", + ) + parser.add_argument( + "--account-cap", + default=DEFAULT_ACCOUNT_CAP, + help="Deprecated compatibility input; does not affect admission.", + ) parser.add_argument("--zdr-endpoints", default=None) parser.add_argument("--require-zdr", action="store_true") parser.add_argument("--pool", choices=("free", "auto"), default="free") diff --git a/scripts/ci/contextual_orchestrator_review_sidecar.sh b/scripts/ci/contextual_orchestrator_review_sidecar.sh index 0ab2ae66d2..366cd02b06 100755 --- a/scripts/ci/contextual_orchestrator_review_sidecar.sh +++ b/scripts/ci/contextual_orchestrator_review_sidecar.sh @@ -9,7 +9,7 @@ # are registered into the process-local KV by the launcher in the SAME process # that performs live model discovery and serves requests — never read back at # request time. The in-process free-priced discovery evidence is turned into a -# ZDR-prioritized, credential-account-diverse agents catalog by +# evidence-admitted agents catalog by # scripts/ci/contextual_orchestrator_review_policy.py for the `orchestrator/free` # (fail-closed zero-cost) pool. set -euo pipefail @@ -35,12 +35,6 @@ SIDECAR_LOG_SANITIZER="$ORG_REPO_ROOT/scripts/ci/sanitize_contextual_orchestrato # finishes, letting the shell script wait for a deterministic marker instead # of guessing whether the async sanitizer has caught up. SIDECAR_DISCOVERY_DIAGNOSTICS_SENTINEL="discovery_diagnostics_complete" -CATALOG_LIMIT="${ORCHESTRATOR_CATALOG_LIMIT:-12}" -# Each KV credential is an independent account, including two credentials for -# the same vendor or endpoint. The account cap prevents one credential from -# consuming the bounded twelve-route preflight catalog without inventing a -# provider-family equivalence relation. -CATALOG_ACCOUNT_CAP="${ORCHESTRATOR_CATALOG_ACCOUNT_CAP:-8}" ORCHESTRATOR_GITHUB_ENV="${GITHUB_ENV:-}" sidecar_python="$(command -v python3)" @@ -279,8 +273,6 @@ esac log "starting review sidecar on ${ORCHESTRATOR_HOST}:${ORCHESTRATOR_PORT}" cp "$ORCHESTRATOR_LAUNCHER" "$ORCHESTRATOR_WORK/launch_sidecar.py" -export ORCHESTRATOR_CATALOG_LIMIT="$CATALOG_LIMIT" -export ORCHESTRATOR_CATALOG_ACCOUNT_CAP="$CATALOG_ACCOUNT_CAP" # Stream stdout/stderr through the redacting sanitizer as two named, awaitable # processes (not bare `> >(...)` substitutions, whose PIDs bash never exposes) # so a failure handler can wait for the sanitizer to finish flushing before it diff --git a/scripts/ci/source_fix_1591_free_identity.py b/scripts/ci/source_fix_1591_free_identity.py new file mode 100644 index 0000000000..aad1ac9f62 --- /dev/null +++ b/scripts/ci/source_fix_1591_free_identity.py @@ -0,0 +1,93 @@ +"""One-shot exact-head repair for PR #1591 central free-pool identity boundaries.""" + +from __future__ import annotations + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] +POLICY = ROOT / "scripts/ci/contextual_orchestrator_review_policy.py" +LAUNCHER = ROOT / "scripts/ci/contextual_orchestrator_review_launcher.py" +SIDECAR = ROOT / "scripts/ci/contextual_orchestrator_review_sidecar.sh" +CHANGELOG = ROOT / "CHANGELOG.md" +BASELINE = ROOT / "docs/product-technical-gap-baseline.md" + + +def replace_once(path: Path, old: str, new: str) -> None: + """Replace exactly one source fragment or fail closed on source drift.""" + text = path.read_text(encoding="utf-8") + count = text.count(old) + if count != 1: + raise RuntimeError(f"{path}: expected exactly one replacement target, found {count}") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +def patch_policy() -> None: + """Reject normalized runtime identity collisions before catalog emission.""" + replace_once( + POLICY, + """ catalog_rows: list[dict[str, Any]] = []\n zdr_count = 0\n for row in picked:\n provider = str(row[\"provider\"])\n model = str(row[\"model\"])\n evidence = _cost_evidence(row)\n zdr = is_zdr_model(provider, model=model, zdr_endpoints=zdr_endpoints)\n if zdr:\n zdr_count += 1\n catalog_rows.append(\n {\n \"id\": _normalize_agent_id(str(row[\"agent_id\"]), provider),\n""", + """ catalog_rows: list[dict[str, Any]] = []\n seen_agent_ids: set[str] = set()\n zdr_count = 0\n for row in picked:\n provider = str(row[\"provider\"])\n model = str(row[\"model\"])\n agent_id = _normalize_agent_id(str(row[\"agent_id\"]), provider)\n if agent_id in seen_agent_ids:\n raise PolicyError(\n f\"agent id collision after normalization: {agent_id!r}; \"\n \"distinct review routes require distinct runtime identities\"\n )\n seen_agent_ids.add(agent_id)\n evidence = _cost_evidence(row)\n zdr = is_zdr_model(provider, model=model, zdr_endpoints=zdr_endpoints)\n if zdr:\n zdr_count += 1\n catalog_rows.append(\n {\n \"id\": agent_id,\n""", + ) + + +def patch_central_free_only() -> None: + """Make the central review entry points accept only orchestrator/free.""" + replace_once( + LAUNCHER, + ' parser.add_argument("--pool", choices=("free", "auto"), default="free")\n', + ' parser.add_argument("--pool", choices=("free",), default="free")\n', + ) + replace_once( + SIDECAR, + """orchestrator_pool=\"${CONTEXTUAL_ORCHESTRATOR_POOL:-free}\"\ncase \"$orchestrator_pool\" in\n free|auto)\n pool_args=(--pool \"$orchestrator_pool\")\n ;;\n *)\n fail \"CONTEXTUAL_ORCHESTRATOR_POOL must be free or auto\"\n ;;\nesac\n""", + """orchestrator_pool=\"${CONTEXTUAL_ORCHESTRATOR_POOL:-free}\"\nif [ \"$orchestrator_pool\" != \"free\" ]; then\n fail \"CONTEXTUAL_ORCHESTRATOR_POOL must be free\"\nfi\npool_args=(--pool free)\n""", + ) + + +def patch_docs() -> None: + """Record the exact causal boundary without inventing routing evidence.""" + changelog = CHANGELOG.read_text(encoding="utf-8") + line = ( + "- Make the central contextual-orchestrator review entry point strictly `orchestrator/free` " + "and fail closed when distinct discovered routes normalize to the same runtime agent identity. " + "This removes the retired paid-inclusive configuration path and prevents identity collisions " + "from erasing failover candidates without introducing a replacement ranking heuristic.\n" + ) + if line not in changelog: + anchor = "## [Unreleased]\n" + if anchor not in changelog: + raise RuntimeError("CHANGELOG.md lacks [Unreleased] anchor") + CHANGELOG.write_text(changelog.replace(anchor, anchor + line, 1), encoding="utf-8") + + baseline = BASELINE.read_text(encoding="utf-8") + marker = "## 2026-09-02 central free-pool reachability and identity repair" + if marker not in baseline: + BASELINE.write_text( + baseline.rstrip() + + "\n\n" + + marker + + "\n\n" + + "PR #1591 exact-head review identified two remaining admission/runtime identity defects. " + + "The central launcher and sidecar still accepted the retired `auto` pool even though " + + "OpenCode, Noema, and Strix are governed as `orchestrator/free` only. The entry points now " + + "reject every non-free pool value. Separately, two distinct discovered routes could normalize " + + "to the same `ModelAgent.id`, which is runtime identity used by failover and evidence state; " + + "catalog construction now fails closed on any such collision instead of silently collapsing " + + "a route. No priority, provider order, model name, quota, weight, threshold, or fallback score " + + "is introduced. The remaining no-evidence name-order routing defect belongs to " + + "ContextualWisdomLab/contextual-orchestrator and is being repaired in canonical PR #1000; " + + "`.github` must not invent a local priority to mask it.\n", + encoding="utf-8", + ) + + +def main() -> None: + """Apply the exact bounded repair.""" + patch_policy() + patch_central_free_only() + patch_docs() + + +if __name__ == "__main__": + main() diff --git a/tests/test_contextual_orchestrator_central_free_only.py b/tests/test_contextual_orchestrator_central_free_only.py new file mode 100644 index 0000000000..b15467e822 --- /dev/null +++ b/tests/test_contextual_orchestrator_central_free_only.py @@ -0,0 +1,25 @@ +"""Central review sidecar pool-boundary regression contracts.""" + +from __future__ import annotations + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +LAUNCHER = ROOT / "scripts" / "ci" / "contextual_orchestrator_review_launcher.py" +SIDECAR = ROOT / "scripts" / "ci" / "contextual_orchestrator_review_sidecar.sh" + + +def test_launcher_exposes_only_free_pool() -> None: + """Noema/OpenCode/Strix cannot reactivate the retired paid-inclusive pool.""" + launcher = LAUNCHER.read_text(encoding="utf-8") + assert 'parser.add_argument("--pool", choices=("free",), default="free")' in launcher + assert 'choices=("free", "auto")' not in launcher + + +def test_sidecar_rejects_any_pool_other_than_free() -> None: + """Environment configuration cannot reactivate orchestrator/auto centrally.""" + sidecar = SIDECAR.read_text(encoding="utf-8") + assert 'if [ "$orchestrator_pool" != "free" ]; then' in sidecar + assert 'fail "CONTEXTUAL_ORCHESTRATOR_POOL must be free"' in sidecar + assert 'free|auto)' not in sidecar diff --git a/tests/test_contextual_orchestrator_no_heuristic_admission.py b/tests/test_contextual_orchestrator_no_heuristic_admission.py new file mode 100644 index 0000000000..db08f95412 --- /dev/null +++ b/tests/test_contextual_orchestrator_no_heuristic_admission.py @@ -0,0 +1,92 @@ +"""Regression contracts for evidence-only contextual-orchestrator admission.""" + +from __future__ import annotations + +import pytest + +from scripts.ci import contextual_orchestrator_review_policy as policy + + +def _free_row(index: int, *, provider: str = "openrouter") -> dict[str, object]: + """Return one normalized evidence-eligible free review route.""" + credential = { + "bytez": "BYTEZ_API_KEY", + "nvidia_nim": "NVIDIA_NIM_API_KEY", + "nvidia_nim_sub": "NVIDIA_NIM_API_KEY_SUB", + "openrouter": "OPENROUTER_API_KEY", + }[provider] + return { + "provider": provider, + "model": f"review-model-{index:02d}", + "agent_id": f"{provider}_review_model_{index:02d}", + "is_free": True, + "cost_evidence": policy.COST_FREE, + "prompt_price_per_1k": 0.0, + "completion_price_per_1k": 0.0, + "currency_code": "USD", + "base_url": f"https://{provider}.example/v1", + "credential_key": credential, + "auth_scheme": "Bearer", + } + + +def test_free_pool_admits_every_evidence_eligible_route_despite_legacy_caps() -> None: + """Legacy cap arguments may not evict evidence-eligible free candidates.""" + rows = [_free_row(index) for index in range(13)] + + result = policy.build_zdr_prioritized_catalog( + rows, + pool="free", + limit=1, + account_cap=1, + ) + + assert {entry["model"] for entry in result["agents"]} == { + row["model"] for row in rows + } + assert result["report"]["selected_count"] == len(rows) + + +def test_free_pool_admission_assigns_no_hand_authored_priority() -> None: + """Admission leaves every eligible model neutral for evidence-based routing.""" + rows = [ + _free_row(0, provider="bytez"), + _free_row(1, provider="nvidia_nim"), + _free_row(2, provider="nvidia_nim_sub"), + _free_row(3, provider="openrouter"), + ] + + result = policy.build_zdr_prioritized_catalog(rows, pool="free") + + assert {entry["priority"] for entry in result["agents"]} == {0} + + +def test_normalized_agent_identity_collision_fails_closed() -> None: + """Two distinct routes may not share the runtime identity used for failover.""" + first = _free_row(0) + second = _free_row(1) + first["agent_id"] = "openrouter/model-a" + second["agent_id"] = "openrouter-model-a" + assert first["model"] != second["model"] + + with pytest.raises(policy.PolicyError, match="agent id collision"): + policy.build_zdr_prioritized_catalog([first, second], pool="free") + + +def test_legacy_ignored_inputs_accept_arbitrary_values() -> None: + """Ignored compatibility inputs cannot become an accidental admission contract.""" + rows = [_free_row(index) for index in range(3)] + sentinel = object() + + result = policy.build_zdr_prioritized_catalog( + rows, + pool="free", + limit="retired-limit", + account_cap=sentinel, + ) + + assert [entry["model"] for entry in result["agents"]] == [ + row["model"] for row in rows + ] + assert result["report"]["legacy_limit_ignored"] is True + assert result["report"]["legacy_account_cap_ignored"] is True diff --git a/tests/test_contextual_orchestrator_review_policy.py b/tests/test_contextual_orchestrator_review_policy.py index 4cda949897..cfdb26a0e8 100644 --- a/tests/test_contextual_orchestrator_review_policy.py +++ b/tests/test_contextual_orchestrator_review_policy.py @@ -89,6 +89,12 @@ def test_normalize_agent_id(candidate: str, provider: str, expected: str) -> Non assert policy._normalize_agent_id(candidate, provider) == expected +def test_normalize_agent_id_fails_closed_when_no_identifier_remains() -> None: + """Punctuation-only identities cannot silently become an empty agent id.""" + with pytest.raises(policy.PolicyError, match="cannot be normalized safely"): + policy._normalize_agent_id("::", "openrouter") + + def test_is_valid_is_free_rejects_non_scalar_markers() -> None: """Non-scalar or missing free markers are not valid discovery evidence.""" assert policy._is_valid_is_free([]) is False @@ -181,37 +187,46 @@ def test_parse_discovery_report_rejects_invalid_rows(report: dict[str, object]) policy.parse_discovery_report(report) -def test_build_catalog_is_zdr_first_and_free_only() -> None: - """ZDR-compliant routes outrank non-ZDR free routes; priced routes stay out.""" +def test_build_catalog_is_free_only_and_records_zdr_without_ranking() -> None: + """Free admission excludes priced/OpenAI rows but does not turn ZDR into a rank.""" parsed = policy.parse_discovery_report(_report()) result = policy.build_zdr_prioritized_catalog( parsed, - limit=12, - account_cap=4, + limit=1, + account_cap=1, zdr_endpoints=ZDR_FEED, ) agents = result["agents"] - assert agents[0]["model"] == "deepseek/deepseek-r1:free" - assert "zdr" in agents[0]["tags"] models = [agent["model"] for agent in agents] + assert models == [ + "deepseek/deepseek-r1:free", + "nvidia/nemotron-3-nano-30b-a3b", + "meta/llama-3.3-70b-instruct", + "qwen2.5-coder", + ] + assert "zdr" in agents[0]["tags"] assert "gpt-4.1" not in models + assert "gpt-4o-mini" not in models assert result["report"]["pool"] == "orchestrator/free" assert result["report"]["zdr_selected_count"] == 1 assert result["report"]["zdr_endpoints_feed_used"] is True assert result["report"]["selected_count"] == len(agents) + assert result["report"]["legacy_limit_ignored"] == 1 + assert result["report"]["legacy_account_cap_ignored"] == 1 + assert {agent["priority"] for agent in agents} == {0} for agent in agents: assert agent["disabled"] is False assert "cost:free" in agent["tags"] assert agent["credential_key"] -def test_build_auto_catalog_admits_price_evidenced_routes() -> None: - """The Strix auto pool can use priced routes without weakening the free pool.""" +def test_build_auto_catalog_admits_price_evidenced_routes_without_ranking() -> None: + """The audit auto pool retains priced routes but admission stays neutral.""" parsed = policy.parse_discovery_report(_report()) result = policy.build_zdr_prioritized_catalog( parsed, - limit=12, - account_cap=4, + limit=1, + account_cap=1, zdr_endpoints=ZDR_FEED, pool="auto", ) @@ -227,14 +242,22 @@ def test_build_auto_catalog_admits_price_evidenced_routes() -> None: assert result["report"]["total_routes"] == 6 assert result["report"]["free_selected_count"] == 5 assert result["report"]["priced_selected_count"] == 1 + assert {agent["priority"] for agent in agents} == {0} -def test_build_auto_catalog_order_is_independent_of_discovery_order() -> None: - """Equivalent route tiers have deterministic provider/model priority.""" +def test_build_auto_catalog_preserves_discovery_provenance_not_priority() -> None: + """Serialization follows discovery provenance while priorities stay neutral.""" parsed = policy.parse_discovery_report(_report()) forward = policy.build_zdr_prioritized_catalog(parsed, pool="auto") reversed_result = policy.build_zdr_prioritized_catalog(reversed(parsed), pool="auto") - assert forward["report"]["selected"] == reversed_result["report"]["selected"] + assert [row["model"] for row in forward["report"]["selected"]] == [ + row["model"] for row in parsed + ] + assert [row["model"] for row in reversed_result["report"]["selected"]] == [ + row["model"] for row in reversed(parsed) + ] + assert {agent["priority"] for agent in forward["agents"]} == {0} + assert {agent["priority"] for agent in reversed_result["agents"]} == {0} @pytest.mark.parametrize( @@ -258,11 +281,11 @@ def test_priced_routes_require_complete_published_price_evidence( def test_build_auto_catalog_keeps_private_targets_zdr_only() -> None: - """Private Strix auto routing still excludes every unattested route.""" + """Private auto admission excludes every unattested route.""" result = policy.build_zdr_prioritized_catalog( policy.parse_discovery_report(_report()), - limit=12, - account_cap=4, + limit=1, + account_cap=1, zdr_endpoints=ZDR_FEED, require_zdr=True, pool="auto", @@ -278,11 +301,10 @@ def test_build_catalog_reports_free_account_diversity() -> None: """Diversity counts independently credentialed accounts with free routes.""" result = policy.build_zdr_prioritized_catalog( policy.parse_discovery_report(_report()), - limit=12, - account_cap=4, zdr_endpoints=ZDR_FEED, ) assert result["report"]["free_account_diversity"] == 5 + assert result["report"]["free_pool_account_diversity"] == 4 def test_build_catalog_counts_same_vendor_credentials_independently() -> None: @@ -306,9 +328,7 @@ def test_build_catalog_counts_same_vendor_credentials_independently() -> None: ] } result = policy.build_zdr_prioritized_catalog( - policy.parse_discovery_report(single_family_report), - limit=12, - account_cap=4, + policy.parse_discovery_report(single_family_report) ) assert result["report"]["free_account_diversity"] == 2 @@ -321,67 +341,78 @@ def test_build_catalog_rejects_unknown_pool() -> None: ) -def test_build_catalog_assigns_unique_priorities() -> None: - """Each selected agent gets a distinct priority so TaskOrchestrator cannot tie on id.""" +@pytest.mark.parametrize(("field", "value"), [("limit", True), ("account_cap", 1.5)]) +def test_build_catalog_ignores_legacy_cap_input_types(field: str, value: object) -> None: + """Retired compatibility inputs cannot regain admission authority through type gates.""" + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(_report()), **{field: value} + ) + assert result["report"][f"legacy_{field}_ignored"] is True + + +def test_build_catalog_assigns_neutral_priorities() -> None: + """Admission cannot create a hand-authored preference for eligible agents.""" result = policy.build_zdr_prioritized_catalog( policy.parse_discovery_report(_report()), - limit=12, - account_cap=4, zdr_endpoints=ZDR_FEED, ) - priorities = [agent["priority"] for agent in result["agents"]] - assert priorities == sorted(priorities, reverse=True) - assert len(priorities) == len(set(priorities)) - assert result["agents"][0]["priority"] == 0 + assert {agent["priority"] for agent in result["agents"]} == {0} assert result["report"]["total_free_routes"] == 5 -def test_build_catalog_applies_account_cap() -> None: - """An account cap keeps one credential from absorbing the pool.""" +def test_build_catalog_ignores_account_cap() -> None: + """A legacy account cap cannot evict an evidence-eligible free route.""" report = { "models": [ - {"provider": "nvidia_nim", "model": f"m{i}", "agent_id": f"nim_a{i}", "is_free": True, **FREE_PRICE} + { + "provider": "nvidia_nim", + "model": f"m{i}", + "agent_id": f"nim_a{i}", + "is_free": True, + **FREE_PRICE, + } for i in range(6) ] + [ { "provider": "nvidia_nim_sub", "model": f"s{i}", - "agent_id": f"nim_b{i}", - "is_free": True, - **FREE_PRICE, + "agent_id": f"nim_b{i}", + "is_free": True, + **FREE_PRICE, } for i in range(6) ] - + [ - {"provider": "openrouter", "model": f"o{i}", "agent_id": f"or_{i}", "is_free": True, **FREE_PRICE} - for i in range(3) - ] } result = policy.build_zdr_prioritized_catalog( - policy.parse_discovery_report(report), limit=12, account_cap=2 + policy.parse_discovery_report(report), limit=1, account_cap=1 ) account_counts: dict[str, int] = {} for agent in result["agents"]: account = policy.provider_account(agent["provider_name"]) account_counts[account] = account_counts.get(account, 0) + 1 - assert account_counts["nvidia_nim"] == 2 - assert account_counts["nvidia_nim_sub"] == 2 - assert account_counts["openrouter"] == 2 + assert account_counts == {"nvidia_nim": 6, "nvidia_nim_sub": 6} + assert result["report"]["selected_count"] == 12 -def test_build_catalog_respects_limit() -> None: - """The catalog never exceeds the configured agent limit.""" +def test_build_catalog_ignores_limit() -> None: + """A legacy total-route limit cannot truncate evidence-eligible admission.""" report = { "models": [ - {"provider": "openrouter", "model": f"m{i}", "agent_id": f"or_{i}", "is_free": True, **FREE_PRICE} + { + "provider": "openrouter", + "model": f"m{i}", + "agent_id": f"or_{i}", + "is_free": True, + **FREE_PRICE, + } for i in range(20) ] } result = policy.build_zdr_prioritized_catalog( - policy.parse_discovery_report(report), limit=5, account_cap=100 + policy.parse_discovery_report(report), limit=1, account_cap=1 ) - assert len(result["agents"]) == 5 + assert len(result["agents"]) == 20 def test_build_catalog_fails_closed_without_free_models() -> None: @@ -400,16 +431,12 @@ def test_build_catalog_fails_closed_without_free_models() -> None: ] } with pytest.raises(policy.PolicyError, match="no free"): - policy.build_zdr_prioritized_catalog( - policy.parse_discovery_report(report), limit=12, account_cap=4 - ) + policy.build_zdr_prioritized_catalog(policy.parse_discovery_report(report)) def test_build_catalog_uses_static_table_without_feed() -> None: """Without a feed, OpenRouter is not granted ZDR for every free route.""" - result = policy.build_zdr_prioritized_catalog( - policy.parse_discovery_report(_report()), limit=12, account_cap=4 - ) + result = policy.build_zdr_prioritized_catalog(policy.parse_discovery_report(_report())) assert result["report"]["zdr_endpoints_feed_used"] is False assert result["report"]["zdr_selected_count"] == 0 assert "zdr" not in result["agents"][0]["tags"] @@ -464,8 +491,8 @@ def test_build_catalog_from_paths_writes_both_files(tmp_path) -> None: str(discovery), out_path=str(catalog), report_path=str(report), - limit=12, - account_cap=4, + limit=1, + account_cap=1, zdr_endpoints_path=str(feed), ) assert catalog.exists() @@ -489,13 +516,14 @@ def test_main_success_writes_catalog(tmp_path) -> None: "--report", str(report), "--limit", - "12", + "1", "--account-cap", - "4", + "1", ] ) assert exit_code == 0 - assert catalog.read_text(encoding="utf-8") + payload = json.loads(catalog.read_text(encoding="utf-8")) + assert len(payload["agents"]) == 4 def test_main_policy_error_returns_one(tmp_path) -> None: @@ -537,12 +565,11 @@ def test_main_requires_discovery_report_arg() -> None: with pytest.raises(SystemExit): policy.main(["--out", "x.json", "--report", "y.json"]) + def test_private_catalog_admits_only_attested_zdr_routes() -> None: """Private-target evidence never falls through to a non-ZDR free route.""" result = policy.build_zdr_prioritized_catalog( policy.parse_discovery_report(_report()), - limit=12, - account_cap=4, zdr_endpoints=ZDR_FEED, require_zdr=True, ) @@ -559,7 +586,5 @@ def test_private_catalog_fails_closed_without_attested_zdr_route() -> None: with pytest.raises(policy.PolicyError, match="ZDR"): policy.build_zdr_prioritized_catalog( policy.parse_discovery_report(_report()), - limit=12, - account_cap=4, require_zdr=True, - ) + ) \ No newline at end of file diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index 559c2d1e99..d3455e9502 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/tests/test_contextual_orchestrator_review_runtime_preflight.py @@ -1105,37 +1105,6 @@ def test_finish_reason_length_escalates_and_can_succeed() -> None: assert report["escalations_used"] == 1 -def test_escalation_budget_is_shared_and_bounded_across_candidates() -> None: - """Once ``REVIEW_PREFLIGHT_MAX_ESCALATIONS`` is spent, a further candidate - that would otherwise qualify is rejected immediately, without a second - call -- the shared budget is per-run, not per-candidate. - """ - namespace = _load_launcher() - preflight = namespace["_preflight_review_agents"] - max_escalations = namespace["REVIEW_PREFLIGHT_MAX_ESCALATIONS"] - - length_response = {"choices": [{"finish_reason": "length", "message": {"content": ""}}]} - agents = [ - SimpleNamespace(id=f"budget_user_{index}", provider_name="openrouter", model="x/free") - for index in range(max_escalations) - ] - exhausted = SimpleNamespace( - id="budget_exhausted", provider_name="openrouter", model="x/free" - ) - client = _ProbeClient( - {agent.id: dict(length_response) for agent in agents} - | {exhausted.id: dict(length_response)} - ) - - with pytest.raises(namespace["ReviewPreflightError"]) as failure: - preflight([*agents, exhausted], client=client) - - exhausted_row = failure.value.report["routes"][-1] - assert exhausted_row["attempts"] == 1 - assert exhausted_row["error_type"] == "escalation_budget_exhausted" - assert failure.value.report["escalations_used"] == max_escalations - assert len(client.calls) == max_escalations * 2 + 1 - @pytest.mark.parametrize( ("http_status", "exception_type_name"), @@ -1409,122 +1378,101 @@ def test_preflight_uses_priced_fallback_only_after_primary_routes_reject() -> No assert failure.value.report["primary_attempt"]["ready_count"] == 0 -def test_fallback_escalation_budget_is_shared_with_primary_and_bounds_worst_case() -> None: - """Regression for Devin Review's fallback-retries-exceed-startup-deadline - finding: ``_preflight_review_agents`` used to start ``escalations_used`` - fresh on every call, so ``_preflight_with_fallback`` calling it twice (up - to 8 primary routes, then up to 4 fallback routes) could spend the full - ``REVIEW_PREFLIGHT_MAX_ESCALATIONS`` budget in EACH stage -- up to 8 - escalations total, 200s worst case (12 base attempts + 8 escalations x - 10s), blowing past Layer 1's 180s healthz-readiness watchdog and - contradicting the ADR's own claimed 160s worst case. - - This drives all 8 primary routes and all 4 fallback routes (the exact - ``REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES`` split) through a response that - always qualifies for escalation and never resolves, so every one of the - 12 candidates *would* escalate if the budget were not shared. Asserts - the run spends at most ``REVIEW_PREFLIGHT_MAX_ESCALATIONS`` escalations - in total (not per stage), and that the resulting worst-case attempt count - keeps total elapsed time at or under 160s -- both stages' escalation - counts are visible in the returned evidence. - """ +def test_fallback_escalation_is_independent_of_primary_catalog_order() -> None: + """Primary starvation cannot consume a fallback candidate's own retry.""" namespace = _load_launcher() preflight = namespace["_preflight_with_fallback"] - max_escalations = namespace["REVIEW_PREFLIGHT_MAX_ESCALATIONS"] - primary_limit = namespace["REVIEW_PREFLIGHT_PRIMARY_ROUTE_LIMIT"] - total_route_limit = namespace["REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES"] - fallback_limit = total_route_limit - primary_limit - - budget_starved_response = { - "choices": [{"finish_reason": "length", "message": {"content": ""}}] - } primary_agents = [ SimpleNamespace(id=f"primary_{index}", provider_name="openrouter", model="x/free") - for index in range(primary_limit) + for index in range(6) ] fallback_agents = [ SimpleNamespace(id=f"fallback_{index}", provider_name="openrouter", model="y/priced") - for index in range(fallback_limit) + for index in range(3) ] - client = _ProbeClient( - {agent.id: dict(budget_starved_response) for agent in [*primary_agents, *fallback_agents]} - ) + starved = {"choices": [{"finish_reason": "length", "message": {"content": ""}}]} + client = _ProbeClient({agent.id: dict(starved) for agent in [*primary_agents, *fallback_agents]}) with pytest.raises(namespace["ReviewPreflightError"]) as failure: preflight(primary_agents, fallback_agents, client=client) - report = failure.value.report - assert report["escalations_used"] == max_escalations - assert report["primary_attempt"]["escalations_used"] == max_escalations + assert failure.value.report["escalations_used"] == len(fallback_agents) + assert failure.value.report["primary_attempt"]["escalations_used"] == len(primary_agents) + assert len(client.calls) == 2 * (len(primary_agents) + len(fallback_agents)) + assert all( + row.get("error_type") != "escalation_budget_exhausted" + for report in (failure.value.report["primary_attempt"], failure.value.report) + for row in report["routes"] + ) - total_attempts = len(client.calls) - # Exactly the ADR's own worst-case arithmetic: 12 base attempts (one per - # candidate across both stages) + 4 escalations (the shared cap) = 16. - assert total_attempts == total_route_limit + max_escalations -def test_preflight_stage_limits_share_one_startup_budget() -> None: - """Free-first and priced-fallback probes share one bounded route budget.""" +def test_every_budget_starved_route_gets_its_own_escalation() -> None: + """Catalog order cannot deny a candidate its own evidence-bearing retry.""" namespace = _load_launcher() - primary = namespace["_bounded_primary_catalog_limit"]( - 99, pool="auto", has_free_rows=True - ) - fallback = namespace["_bounded_fallback_catalog_limit"]( - 99, primary_count=primary - ) - assert (primary, fallback) == (8, 4) - assert primary + fallback == namespace["REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES"] + preflight = namespace["_preflight_review_agents"] + agents = [ + SimpleNamespace( + id=f"starved_{index}", provider_name="openrouter", model=f"starved/{index}" + ) + for index in range(6) + ] + starved = { + "choices": [{"finish_reason": "length", "message": {"content": ""}}] + } + client = _ProbeClient({agent.id: dict(starved) for agent in agents}) + with pytest.raises(namespace["ReviewPreflightError"]) as failure: + preflight(agents, client=client) -def test_catalog_account_cap_defaults_to_the_caller_supplied_policy_default( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """The per-account cap falls back to ``policy.DEFAULT_ACCOUNT_CAP``, not the total budget. - - Regression for a real, observed failure mode - (ContextualWisdomLab/.github#1415, reported as "빈 깡통 경로 너무 많다"): a - sibling helper (``_catalog_family_cap()``) fell back to - ``REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES`` -- the *total* preflight budget -- - instead of the intended per-account cap whenever its env var was unset. - That silently disabled per-account diversification: in a live production - run, two NVIDIA NIM credentials sharing one rate-limited upstream jointly - consumed all 12 preflight slots, of which 10 (83%) were then rejected via - 429/404/timeout. This module's own equivalent helper must never resolve - to the same value as the total-routes budget when given the real - ``policy.DEFAULT_ACCOUNT_CAP``, which is strictly smaller. - """ - namespace = _load_launcher() - monkeypatch.delenv("ORCHESTRATOR_CATALOG_ACCOUNT_CAP", raising=False) - cap = namespace["_catalog_account_cap"](policy.DEFAULT_ACCOUNT_CAP) - assert cap == policy.DEFAULT_ACCOUNT_CAP - assert cap != namespace["REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES"] - assert cap < namespace["REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES"] + rows = failure.value.report["routes"] + assert [row["attempts"] for row in rows] == [2] * len(agents) + assert all(row.get("error_type") != "escalation_budget_exhausted" for row in rows) + assert failure.value.report["escalations_used"] == len(agents) + assert len(client.calls) == 2 * len(agents) -def test_catalog_account_cap_honors_an_explicit_override( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """An operator-set ``ORCHESTRATOR_CATALOG_ACCOUNT_CAP`` still takes effect.""" +def test_preflight_keeps_more_than_twelve_admitted_primary_routes() -> None: + """Admission cardinality cannot crash or truncate runtime preflight.""" namespace = _load_launcher() - monkeypatch.setenv("ORCHESTRATOR_CATALOG_ACCOUNT_CAP", "6") - assert namespace["_catalog_account_cap"](policy.DEFAULT_ACCOUNT_CAP) == 6 + preflight = namespace["_preflight_with_fallback"] + agents = [ + SimpleNamespace(id=f"ready_{index}", provider_name="openrouter", model=f"model/{index}") + for index in range(13) + ] + client = _ProbeClient({agent.id: _openai_text("OK") for agent in agents}) + viable, report, fallback_used = preflight(agents, [], client=client) -def test_main_sources_the_account_cap_default_from_policy_not_a_magic_number() -> None: - """``main()`` must wire the cap default from ``policy.DEFAULT_ACCOUNT_CAP``. + assert viable == agents + assert report["ready_count"] == len(agents) + assert fallback_used is False + assert [call[0] for call in client.calls] == agents - A hand-typed literal (or, worse, a total-routes-scale constant) can - silently drift out of sync with ``policy.DEFAULT_ACCOUNT_CAP`` with no - test catching it -- the exact drift that produced - ContextualWisdomLab/.github#1415's real preflight-budget waste. This - source-level contract test pins both ``build_zdr_prioritized_catalog`` - call sites in ``main()`` to the single source of truth and forbids the - total-routes constant from ever reappearing as the account-cap fallback. - """ - source = _LAUNCHER.read_text(encoding="utf-8") - assert source.count("account_cap=_catalog_account_cap(DEFAULT_ACCOUNT_CAP)") == 2 - assert "ORCHESTRATOR_CATALOG_FAMILY_CAP" not in source - assert 'os.environ.get("ORCHESTRATOR_CATALOG_ACCOUNT_CAP", "4")' not in source + +def test_auto_fallback_keeps_all_admitted_routes_after_primary_failure() -> None: + """Auto-pool fallback is evidence-triggered, not cardinality-truncated.""" + namespace = _load_launcher() + preflight = namespace["_preflight_with_fallback"] + primary = [ + SimpleNamespace(id=f"free_{index}", provider_name="openrouter", model=f"free/{index}") + for index in range(9) + ] + fallback = [ + SimpleNamespace(id=f"priced_{index}", provider_name="openrouter", model=f"priced/{index}") + for index in range(5) + ] + client = _ProbeClient( + {agent.id: TimeoutError("unavailable") for agent in primary} + | {agent.id: _openai_text("OK") for agent in fallback} + ) + + viable, report, fallback_used = preflight(primary, fallback, client=client) + + assert viable == fallback + assert fallback_used is True + assert report["fallback_reason"] == "primary_routes_unavailable" + assert [call[0] for call in client.calls] == [*primary, *fallback] def test_zdr_admission_selects_priced_tier_when_free_routes_are_not_private() -> None: @@ -1768,3 +1716,17 @@ def test_sidecar_stream_sanitizer_omits_no_summary_for_fully_safe_input( assert main() == 0 assert output.getvalue() == "client_disconnected\n" + + +def test_launcher_has_no_legacy_catalog_admission_caps() -> None: + """Runtime bootstrap must not restore retired catalog admission authority.""" + namespace = _load_launcher() + source = _LAUNCHER.read_text(encoding="utf-8") + + assert "_bounded_primary_catalog_limit" not in namespace + assert "_bounded_fallback_catalog_limit" not in namespace + assert "_catalog_account_cap" not in namespace + assert "REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES" not in source + assert "REVIEW_PREFLIGHT_PRIMARY_ROUTE_LIMIT" not in source + assert "ORCHESTRATOR_CATALOG_LIMIT" not in source + assert "ORCHESTRATOR_CATALOG_ACCOUNT_CAP" not in source