Skip to content

fix(ci): gate Strix's orchestrator/free access on live diversity evidence - #1437

Draft
seonghobae wants to merge 14 commits into
mainfrom
claude/noema-opencode-strix-orchestration-sexqzc
Draft

fix(ci): gate Strix's orchestrator/free access on live diversity evidence#1437
seonghobae wants to merge 14 commits into
mainfrom
claude/noema-opencode-strix-orchestration-sexqzc

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Status: exact-head restacked corrective lane — Draft, unmerged

This is the canonical corrective lane for the unsafe unconditional Strix orchestrator/free policy. It is now restacked on protected main@1cf2f9120a2cd494ed5079135bb10822c27b5947 at exact head a1dc62dfc9a5a938deff9995913e013766e55e2c.

The published merge commit is bound to tree dfd756c3eaf8bb173785df1fd6c8e19b4d75cfc4 and has the previous PR head 29baf5e52c49a13393b265ae14a6e176b17f5ad1 plus current protected main as parents. No history rewrite or force push was used.

Causal repair

  • strix.yml boots the richer auto catalog and selects orchestrator/free only when this run's own free_family_diversity evidence is an integer at least 2.
  • Diversity 0/1, absent, unreadable, malformed, boolean, or otherwise unverifiable evidence stays on orchestrator/auto.
  • The launcher retains an opt-in post-preflight serving floor that fails closed when fewer than two independent outage-domain families survive.
  • Ordinary provider/runtime failures remain non-passing evidence; this lane does not fabricate vulnerability findings or convert infrastructure failure into source success.
  • The direct-NIM cleanup is no longer mixed into this PR: protected main integrated it independently through chore(ci): remove orphaned direct-NVIDIA-NIM model resolver #1442, and the exact current diff contains 20 Strix diversity/source-contract files.

Test-first evidence

The stale branch initially produced a realistic RED:

1 failed, 242 passed, 20 subtests
tests/test_contextual_orchestrator_review_runtime_preflight.py::
  test_preflight_uses_priced_fallback_only_after_primary_routes_reject

Root cause: an obsolete branch assertion required a duplicate primary attempt, while current main's ADR-0005 contract advances directly to priced fallback for a transport exception. The main merge had also dropped the branch's serving-diversity implementation while leaving its call site. The narrow repair restored only the diversity boundary/tests and retained main's fallback semantics.

Exact published-tree GREEN after adopting all intervening protected-main scheduler, direct-NIM, and SSRF repairs:

focused owner/security suites: 532 passed
Strix required-workflow smoke: passed
full suite under branch coverage: 2032 passed, 1 skipped, 36 subtests
statements: 10,250 / 10,250
branches: 4,084 / 4,084
production docstrings: 100%
git diff --check: passed

Live evidence

At publication, GitHub reports the PR mergeable and Draft with 0 formal reviews and 0 review threads. Exact-head Security Scan, SAST, CodeQL, OSV, SBOM, Python Security, Secret Scan, Scorecard, commercial-readiness, and changed-path workflows were newly queued/in progress; pending, queued, skipped, or absent evidence is not treated as passing.

Do not merge from source evidence alone. Require terminal exact-current-head security/supply-chain checks, authoritative Strix evidence, a substantive current-head formal review verdict, and live governance. After protected-main integration, unchanged consumer canaries must prove coherent authoritative Strix reports (or explicit typed provider-unavailable non-pass), never a green check backed by an incomplete scan.

claude added 5 commits August 30, 2026 09:45
…evidence

Add free_family_diversity to the contextual-orchestrator review policy
report so a future strix.yml change can evidence-gate a move to
orchestrator/free instead of relying on a static default. Strix stays
pinned to orchestrator/auto in this PR: ADR-0003's 2026-08-30 addendum
and the accompanying doctoring record explain why today's directive
instruction to route Strix through orchestrator/free conflicts with the
2026-08-29 single-outage-domain finding, and what evidence a follow-up
PR needs before it can safely flip the pin.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
…job log

The review sidecar already computed a safe, regex-validated error_code and
http_status when the gateway's own /v1/chat/completions preflight rejected
every candidate route, but only wrote it into the
CONTEXTUAL_ORCHESTRATOR_PREFLIGHT_EVIDENCE artifact file -- invisible in the
job log a CI operator reads first. Every "gateway preflight returned HTTP
502" failure across the org's PR backlog right now (including this PR's own
noema-review run, and .github#1429's) looked identical and gave no signal
about which upstream failure category caused it. Print the same
already-bounded fields directly to the job log so the real cause is visible
without downloading the artifact.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
select_nvidia_nim_model.py made real, direct HTTPS calls to
integrate.api.nvidia.com (bypassing the contextual-orchestrator gateway) to
resolve a live model id for the scheduled autofix worker. That worker was
already migrated to the gateway's own auto-discovery; grep across every
workflow in the repo confirms zero remaining callers. Removes the script and
its test as part of today's direct-NIM-communication cleanup.

The opencode.jsonc nvidia-nim/github-models provider catalogs and the
opencode-review-dispatch.yml OPENCODE_MODEL_CANDIDATES fallback pool are a
separate, actively-tested mechanism (not dead code) and are flagged in
docs/doctoring/direct-nvidia-nim-communication-removal.md as a dedicated
follow-up rather than removed here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
Records the Strix free-pool diversity-evidence reconciliation (#1433), the
gateway-preflight visibility fix, the direct-NIM cleanup, and the four
parallel PR-triage agents' real fixes (noema#500, .github#1238/#1271,
.github#1435, contextual-orchestrator#857's stale workflow removal) under
section 5.1's next-increment list -- a different location than #1432's
in-flight "2026-08-30 autonomous loop snapshot" append, to avoid colliding
with that PR's own diff.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
Standardize all three central CI review/security consumers (OpenCode, Noema,
Strix) on the fail-closed zero-cost orchestrator/free pool, retiring Strix's
separate orchestrator/auto split from ADR-0003. Verified first that the
provider-family-diversity cap in contextual_orchestrator_review_policy.py
already applies identically to both pools (no policy/sidecar code change
needed) and that the ContextualWisdomLab/contextual-orchestrator#919
Models.dev-cost generalization widens which provider families can appear in
the free catalog. Documented the residual risk (live-market family
concentration, and a separate request-time gateway failover gap being fixed
upstream) rather than claiming the concern is fully closed. See
docs/adr/0020-strix-orchestrator-free-pool.md, which supersedes ADR-0003's
Strix-specific wiring bullet only (kept as history with an Amendment).

Also removes scripts/ci/select_nvidia_nim_model.py, an orphaned helper that
called integrate.api.nvidia.com directly and was wired into nothing since the
ADR-0003 gateway migration, and corrects several doctoring/automation docs
that still described the scheduled autofix worker's model credential as a
single hardcoded NVIDIA_NIM_API_KEY rather than the current five-secret
contextual-orchestrator/orchestrator/free gateway routing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

Exact-head governance review at a2ef0ea2cb63e62ffa1d53e30569895378fe52ac: keep this Draft and do not merge in its current form.

The source itself acknowledges that the 2026-08-29 single-family outage-domain condition is not eliminated, that provider diversity is only a live-market possibility, and that request-time failover remains broken. A per-family cap does not create a second family. Moving required Strix from the correctness-first orchestrator/auto pool to orchestrator/free before current evidence proves at least two independent available families therefore reintroduces the exact availability regression ADR-0003 was adopted to prevent. “OpenCode/Noema already accept this risk” is not evidence that a required security scanner may safely inherit it.

There is also an existing canonical evidence owner, #1433, which adds measured free_family_diversity without changing Strix behavior. Converge on that lane rather than landing a competing broad migration. Acceptance for any later pool switch must be behavior-first:

  1. protected-main discovery evidence reports at least two independently credentialed/provider-family free routes;
  2. a negative fixture proves diversity 0/1 retains orchestrator/auto rather than weakening availability;
  3. request-time route failure demonstrably advances to another admitted route or returns typed non-passing provider evidence;
  4. unchanged exact-head Strix canaries produce authoritative reports;
  5. the unrelated direct-NIM dead-code/docs cleanup is split or adopted by its actual owner instead of being bundled into this policy transition.

Do not treat the explicit product directive, updated string assertions, or administrator-bypassed predecessor merges as operational acceptance.

The sidecar's internal launcher preflight (_preflight_review_agents, run
before the server ever answers healthz) is a distinct failure layer from the
sidecar script's external gateway smoke test fixed in 57c04ea. It already
computed a bounded per-route report (agent_id/provider/model/error_type/
http_status, never raw provider text) but only wrote it to the
--preflight-out artifact file; the sanitized stderr stream showed only
"review sidecar preflight failed" with the real routes silently dropped into
omitted_unstructured_lines. This is the failure mode PR .github#1433's own
noema-review hit on its second push.

Print one bounded, independently re-validated diagnostic line per rejected
route (including the nested primary_attempt when the priced fallback pool
also rejects everything) and allowlist the new format in the stream
sanitizer.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX

Copy link
Copy Markdown
Contributor Author

Agreed, and thank you for the detailed review — this is correct and I'm not going to route around it.

On the failing strix check: confirmed it's scripts/ci/strix_required_workflow_smoke.sh — a trusted, base-branch-pinned contract test — correctly rejecting this PR's ungated pool flip (FAIL: Strix must not retain the free default route). That's the test doing its job, not a flake or an unrelated failure. I will not weaken, delete, or route around it to force this green.

On converging with #1433: agreed that's the correct lane. #1433's free_family_diversity reporting (additive, zero behavior change, already at 100% coverage on the touched file) is the right foundation. This PR jumped straight to an unconditional flip instead of building the evidence gate on top of it — that was the mistake.

Plan, addressing each acceptance criterion:

  1. Pull in feat(ci): report free-route provider-family diversity for Strix pool evidence #1433's free_family_diversity reporting as the base.
  2. Wire strix.yml to select orchestrator/free only when free_family_diversity >= 2, else orchestrator/auto — an evidence-gated conditional, not a static pin.
  3. Add a negative fixture proving diversity 0/1 retains orchestrator/auto.
  4. Extend strix_required_workflow_smoke.sh to assert the new conditional contract (still fail-closed, now verifying the gate itself rather than a bare value) — extending the safety net, not loosening it.
  5. Criterion 3 (request-time failover) depends on the separate contextual-orchestrator gateway fix currently in progress for the orchestrator/free 502 pattern; I won't claim that criterion satisfied until that fix is verified merged and the vendored pin here is bumped accordingly.
  6. Criterion 4 (Strix canary reports) — I'll check what canary mechanism exists and verify against it; will say plainly if I can't find one rather than assume.
  7. Per your point 5: splitting the direct-NIM-communication cleanup (dead select_nvidia_nim_model.py + doctoring corrections) out of this PR into its own, separate, smaller PR — it's genuinely orthogonal to the pool-gating question.

This PR stays Draft and unmerged while that work is done. Will update here once pushed.


Generated by Claude Code

claude added 2 commits August 30, 2026 10:26
…ma-opencode-strix-orchestration-sexqzc

Pulls in #1433's free_family_diversity evidence foundation
(scripts/ci/contextual_orchestrator_review_policy.py) plus its bundled
preflight-evidence-surfacing fixes and NIM-cleanup, ahead of restructuring
this branch's Strix pool-routing change into an evidence-gated conditional
per the exact-head review on #1437.

Resolved conflicts in docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md
and docs/product-goal-directive.md by combining both branches' narratives into
one that reflects the final evidence-gated state (both documents are rewritten
further in a follow-up commit on this branch).
…ence

Replaces the unconditional pool flip a human exact-head governance review
rejected on this PR with the evidence-gated conditional the review required:
strix.yml now reads free_family_diversity (added by #1433, merged into this
branch above) from the sidecar's own discovery run and selects
orchestrator/free only when it is >= 2, falling back to orchestrator/auto
--- which the sidecar always boots regardless of the resolved model name, so
the fallback is a genuine priced-route safety net, not an alias for the same
single-family catalog --- in every other case, including any evidence that
is missing, unreadable, or malformed.

- .github/workflows/strix.yml: new "Resolve Strix model from free-route
  diversity evidence" step; "Gate Strix secrets" reverted to its static
  orchestrator/auto base and auto-only override allowlist; "Prepare Strix
  model input file" now accepts both pool literals, sourced from the new
  step's output.
- tests/test_strix_contextual_orchestrator_contract.py: negative fixture
  proving diversity 0/1 (and every malformed-evidence shape) stays on
  orchestrator/auto, executed against the workflow's own extracted step
  (this repo's established pattern for testing embedded workflow-YAML
  behavior, already used for the neighboring "Gate Strix secrets" step).
- scripts/ci/strix_required_workflow_smoke.sh: extends (never weakens) the
  prior bare-pin assertions into assert_free_pool_gated_by_diversity, a
  structural check that orchestrator/free is reachable only inside the
  diversity conditional and the safe auto default is always set first.
- Reconciled docs/adr/0003 and docs/product-goal-directive.md's merge
  conflicts (both branches independently documented the Strix pool
  question) into one narrative describing the corrected, evidence-gated
  outcome. Rewrote docs/adr/0020-strix-orchestrator-free-pool.md end to end:
  the rejected unconditional draft, the corrected decision, honest status
  of the review's 5 acceptance criteria (request-time failover not
  confirmed merged upstream; no dedicated "Strix canary" mechanism found
  beyond the existing push-trigger convention), and residual risk.
- Reverted the 4 stale-NVIDIA-NIM-doc corrections and the gap-baseline
  removal-target section this PR's first draft bundled in alongside the
  pool change; they move to a separate branch/PR per the review's fifth
  criterion (scripts/ci/select_nvidia_nim_model.py's own deletion remains
  inherited from #1433's independent commit via the merge above).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4

Copy link
Copy Markdown
Contributor Author

Status update: main has moved since this thread's last comment. .github#1434 (a separate session, merged just now at e36a1f71) directly implemented the orchestrator/autoorchestrator/free flip for Strix — the same core change this PR proposes — plus raised ORCHESTRATOR_CATALOG_FAMILY_CAP 4→8 as its risk-reduction measure (a moderate mitigation of the single-family-concentration finding, not the acceptance-criteria evidence gate discussed above) and merged with the risk explicitly documented and accepted in an ADR-0003 amendment. Separately, .github#1436 (also merged) found the actual root cause of the org-wide outage this whole thread was reasoning about: a hardcoded max_tokens:16 in the gateway smoke test rejecting a healthy reasoning-model route — unrelated to pool/provider-diversity reliability at all.

This PR's core change is now redundant with what's on main. Given that:

  • The pool-flip + NIM-cleanup content here can be closed as superseded rather than reworked toward the evidence-gate plan from two comments ago — that plan's premise (flipping the pool is still an open, gated decision) is no longer true.
  • Acceptance criterion 3 from the review above (request-time route failure advancing to another admitted route) is still open — that's the deeper gateway failover gap #1434's own description flags as "a dedicated fix in progress directly in contextual-orchestrator," separate from both the max_tokens fix and the family_cap mitigation. That's real, unresolved work.
  • Criterion 5 (split the direct-NIM cleanup into its own PR) is still worth doing on its own merits — opening that separately now that #1434's independent audit corroborates select_nvidia_nim_model.py is dead code too.

Recommend closing this PR as superseded by #1434/#1436 rather than continuing to develop it. I don't want to close someone else's PR unilaterally, so leaving this for you or the owner to action — happy to close it myself if that's confirmed.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Correcting my own last comment: I posted it based on stale information — the "gate Strix on free_family_diversity >= 2, fall back to auto otherwise" commit (49227aad) had landed 24 seconds before I wrote it, and I hadn't seen it yet. This PR is not redundant with #1434. It's the opposite: it implements exactly the evidence-gated conditional the exact-head governance review above explicitly required, with a negative fixture proving diversity 0/1 stays on auto and an extended strix_required_workflow_smoke.sh assertion — while #1434 (merged to main) did the unconditional flip that same review explicitly rejected as reintroducing "the exact availability regression ADR-0003 was adopted to prevent."

So there's now a real, unresolved conflict between two sessions acting under the same account: main currently carries the unconditional flip the review process rejected on this thread, and this PR carries the gated version the review process required, not yet merged and now dirty against that same main. I don't think it's my place to pick a side here — one route was explicitly reviewed and rejected in writing on this exact PR; the other is live in production. That's a call for you, not something I should resolve by pushing a third variant.

For what it's worth, in a separate conversation just now the same account told me directly (paraphrasing): "free+ZDR isn't even solved yet, so what's the point of keeping the paid-inclusive auto pool" — which reads as favoring #1434's simpler unconditional approach over this PR's gate. If that's the actual call, this PR's remaining value is narrower: its negative-fixture test and the honest ADR-0020 rewrite (acceptance criteria 3/4 marked unresolved rather than assumed) are worth keeping regardless of which pool default wins, even if the conditional gate itself doesn't ship.

Not taking any further action on this PR myself pending that decision.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Exact-head convergence decision at 49227aadaad3f928fa48a8dcd3a4ad27cf1cd631 against protected main@e36a1f716d2ba0334c0e40597d7f9a90eee9cb14:

Keep this Draft as the canonical corrective lane for the unsafe unconditional Strix free-only policy now on main. The controlling acceptance contract remains the one recorded earlier in this PR: live free_family_diversity >= 2 may select orchestrator/free; diversity 0/1, absent or malformed evidence must retain orchestrator/auto. Raising a per-family cap from 4 to 8 does not create an independent outage domain and cannot satisfy that contract.

Next safe source transition:

  1. Restack onto current protected main and adopt fix(zdr): cite NVIDIA's own Trial ToS for the nvidia_nim not-ZDR classification #1434/fix(sidecar): give the gateway preflight probe a real reasoning budget #1436 only where compatible.
  2. Consume the narrow free_family_diversity producer from feat(ci): report free-route provider-family diversity for Strix pool evidence #1433 (current head 50fbf5b3e597905f850ab47ed7a51a9611b529b6) instead of duplicating it.
  3. Preserve the negative 0/1/malformed-evidence fixtures and the smoke assertion proving orchestrator/free is unreachable outside the diversity conditional.
  4. Split the direct-NIM dead-code/docs cleanup from this behavior repair; the current 28-file/652-deletion scope is not an auditable minimal causal patch.
  5. Keep request-time provider failure typed non-passing until a different admitted route actually succeeds; catalog diversity does not prove runtime failover.
  6. After protected integration, require fresh exact-head Strix canaries with an authoritative report. A green string/contract test or the bypassed fix(zdr): cite NVIDIA's own Trial ToS for the nvidia_nim not-ZDR classification #1434 merge is not operational acceptance.

Do not close this as superseded by #1434. #1434 is the contradictory unconditional state this Draft must safely converge, and its admission provenance has now been recorded on #1340. Keep Draft status until the stack is current, narrow, fully tested and current-head reviewed.

claude added 2 commits August 30, 2026 11:03
Second, .github-local half of the 2026-08-30 request-time-failover
investigation (contextual-orchestrator's own routing-level fix is tracked
separately as ContextualWisdomLab/contextual-orchestrator#922, draft, not
yet merged: classify_provider_transport_failure corrects a provider
transport failure being misclassified through a tool-execution-failure
classifier). That fix cannot help here: contextual_orchestrator_review_launcher.py's
_preflight_review_agents calls client.proxy_send_once directly per candidate
and never reaches TaskOrchestrator's own routing/failover at all. Its client
is configured with max_retries=0 and proxy_send_once is a single-shot
transport by design, so before this change a single transient blip (a 503,
a timeout) permanently rejected an otherwise-healthy route with zero retry
budget of this loop's own -- in the worst case where every discovered
candidate hit the same transient blip in one run, the whole sidecar would
exit before healthz regardless of how good the gateway's own failover is.

_is_retryable_preflight_error classifies a caught exception by type and, for
an HTTP failure, status code alone (408/429/500/502/503/504, or a
connection-level failure with no status) -- never from response text, and
with no dependency on the vendored contextual_orchestrator package (which
this file's own coverage-omit note documents as unavailable to this
repository's test suite; importing it at call time here would silently pass
only in environments where it happens to be installed and fail in real CI).
_preflight_review_agents now gives each candidate up to
REVIEW_PREFLIGHT_ATTEMPTS_PER_ROUTE (2) attempts: one bounded retry on a
retryable failure, none on a non-retryable one, preserving the exact same
report/row shape.

Bears directly on the exact-head review's acceptance criteria 3 and 4 for
Strix's orchestrator/free access: a single flaky preflight attempt can no
longer take the whole review pipeline down before Strix ever runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
…bing

Part of implementing the launcher-level runtime floor requested for
criterion 3/4: --require-minimum-serving-diversity, opt-in and off by
default (mirrors the existing --require-zdr pattern exactly) since
unconditionally enabling it today would immediately fail closed for the
sidecar's default pool ("free"), which opencode-review-dispatch.yml and
noema-review.yml both rely on without having asked for or gated on this
guarantee. The launcher-side enforcement and strix.yml's opt-in follow in
the next commits.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae pushed a commit that referenced this pull request Aug 30, 2026
- Use owner/repo#num convention only for cross-repo references; same-repo
  mentions of #1433/#1434/#1437 use bare #num.
- Point the sidecar's CATALOG_FAMILY_CAP comment at a durable commit SHA
  instead of the removing branch's name, which disappears once the branch
  is deleted after merge.
- Correct the doctoring doc's grep-audit claim: one intentional code
  comment still names select_nvidia_nim_model.py on purpose as a
  searchable history pointer, so "zero references" was inaccurate.
claude and others added 2 commits August 30, 2026 11:26
…xqzc

Reconciles this branch's evidence-gated Strix pool-gating work with three
bypass-merged main commits that landed while this PR was under review
(#1434: unconditional Strix free-only flip with the single-outage-domain
risk explicitly accepted, plus a family_cap 4->8 mitigation and an NVIDIA
ZDR citation strengthening; #1436/#1440/#1439: sidecar preflight max_tokens
and gateway-timeout fixes, unrelated to pool selection).

Per the exact-head governance review on this PR (quoted in docs/adr/0003's
new Amendment): an administrator bypass merge -- even one made with the
risk explicitly surfaced and consciously accepted -- is not, by itself,
operational acceptance of the underlying approach once a real review
happens. This merge keeps #1434's orthogonal mitigations (family_cap raise,
ZDR citation) and #1436/#1440/#1439's sidecar fixes, but replaces #1434's
unconditional STRIX_MODEL/CONTEXTUAL_ORCHESTRATOR_POOL flip to
orchestrator/free with this branch's evidence-gated conditional
(free_family_diversity >= 2), restores strix_quick_gate.sh's
is_contextual_orchestrator_model acceptance of orchestrator/auto (which
#1434 had narrowed to free-only, which would have misrouted Strix's own
safe fallback), and fixes several silent regressions where git's line-level
merge took main's since-superseded literal because this branch's own text
happened to match the merge-base after its earlier revert (not real
conflicts, so no marker -- caught by re-running the full test suite and
grepping for every orchestrator/free vs orchestrator/auto assertion
post-merge).

Also implements, in response to a live root-cause investigation surfaced
during this merge: a bounded (1-retry) probe in the launcher's own
preflight loop (already committed pre-merge) and a new opt-in
--require-minimum-serving-diversity runtime floor in
contextual_orchestrator_review_launcher.py -- a defense-in-depth backstop,
distinct from strix.yml's decision-time free_family_diversity gate, that
refuses to serve() a post-preflight pool spanning fewer than 2 independent
provider families. Off by default (not yet enabled for any caller,
including Strix): analysis found today's documented single-family-dominated
free-route catalog means enabling it unconditionally would make even the
safe orchestrator/auto fallback refuse to boot, a worse availability
regression than what this change exists to prevent. See
docs/adr/0020-strix-orchestrator-free-pool.md.

Full local suite: 1871 passed, 1 skipped, 36 subtests passed. interrogate
100%. Coverage 99% (single pre-existing miss in pingora_edge_policy.py:274,
unrelated and untouched by this branch, tracked separately).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
@seonghobae seonghobae changed the title fix(ci): route Strix security review through orchestrator/free fix(ci): gate Strix's orchestrator/free access on live diversity evidence Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Status update: restack against protected main complete, per your convergence decision above (not closing as superseded by #1434 — the exact-head governance review on this PR's first draft and your subsequent comment both directly address that scenario, and I'm following the plan you approved, not the bypass merge).

Pushed: merge commit 2b71ac72 (current head), reconciling #1434's now-superseded unconditional flip (kept as history in docs/adr/0003's Amendment, not deleted) and #1436/#1440/#1439's orthogonal sidecar fixes with this PR's evidence-gated conditional. Also fixed a real bug #1434 introduced: strix_quick_gate.sh's is_contextual_orchestrator_model had narrowed to orchestrator/free-only, which would have misrouted Strix's own orchestrator/auto fallback had it ever been requested.

Also found and fixed, from a root-cause investigation surfaced mid-restack: the launcher's own preflight probe loop (_preflight_review_agents) had zero retry budget of its own — bounded to one retry now, gated on the same retryable-vs-not distinction contextual-orchestrator#922 uses on its side. Built (but deliberately not enabled for Strix yet) a --require-minimum-serving-diversity runtime floor in the launcher — full reasoning for why it's not on yet is in the PR description and ADR-0020; short version: today's documented single-family free-route catalog means enabling it now would make even the safe orchestrator/auto fallback refuse to boot.

Full updated status against your 6 numbered points and the original 5 criteria is in the PR description. Two things I want to flag explicitly rather than claim resolved:

  • Point 5 (criterion 3): contextual-orchestrator#922 (the routing-misclassification fix) is still open/draft, not merged — I have not bumped ORCHESTRATOR_PIN_SHA and won't until it merges. A ToolFallbackStoppedError gap was reported to me as likely still unaddressed by chore(deps): bump github/codeql-action/init from 4.37.0 to 4.37.6 #922; I have not independently verified that against chore(deps): bump github/codeql-action/init from 4.37.0 to 4.37.6 #922's current diff myself.
  • Point 6 (criterion 4): I could not identify a dedicated "Strix canary" mechanism beyond strix.yml's own push-trigger convention, and have not observed a live run against this exact head — that requires an actual protected-branch merge, outside what I can do from here.

Criterion 5: NIM-direct-communication cleanup split out to #1445 (draft, against main).

Full local suite: 1871 passed, 1 skipped, 36 subtests. interrogate 100%. coverage: 99%, single pre-existing miss in pingora_edge_policy.py:274 (untouched, tracked as #1398).

Remaining Draft, unmerged, as instructed.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

The failing opencode-review check on this head (run 33311821764) isn't this PR's diff — it's a known, already-fixed-but-not-yet-merged bug in the required opencode-review.yml workflow itself.

That workflow always runs off main's copy (base-branch trust boundary for pull_request_target), and main doesn't yet have the draft exemption — it's still in unmerged PR #1443. Until #1443 merges, every draft PR org-wide (this one included) hard-fails this check with "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head," regardless of what's in the PR's own diff. I can't port that fix into this PR directly, since the base-branch trust boundary means only a merge to main can change which workflow copy actually runs.

Practically this isn't blocking anything right now: this PR is intentionally kept in Draft per your instruction to keep the stack narrow and current-head reviewed, and the merge scheduler already skips drafts entirely. Once #1443 merges, this specific failure mode goes away on this PR's next triggered event (push/reopen/etc.) — no action needed here in the meantime.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Fresh operational evidence for the request-time failover acceptance contract, from required Strix on an unchanged central exact head:

  • Target: #1198 b051f5da34998fcd0ed42990d9f5e29b128a59ab against protected main@1ff8268255b061461d9d49b4cab4febf9a8e7bfa.
  • Required run/job: 33325907333 / 99295892400, terminal failure at 2026-08-30 17:56:57 UTC.
  • The trusted sidecar selected contextual-orchestrator/orchestrator/free. All three bounded attempts routed to the same meta/llama-3.2-90b-vision-instruct agent through NVIDIA NIM.
  • Each attempt returned provider HTTP 400 invalid_request_error with retryable: false. After 90s and 180s bounded backoffs, the gateway still emitted STRIX_PROVIDER_UNAVAILABLE: contextual-orchestrator/orchestrator/free exhausted; the gateway owns provider discovery and failover.
  • No authoritative vulnerability report was produced. The required workflow correctly remained non-passing. The exact-head strix-reports artifact is 9736417794, digest sha256:b1697fc0adff6a66934f0bce03f8be76397cdbba2a349a99a21d18059701eb19.

This is provider/model-behavior infrastructure evidence, not a source vulnerability in #1198. It operationally falsifies criterion 3 as currently implemented: a request-time non-retryable failure did not advance to a different admitted route before the free pool was declared exhausted. It also supplies the missing live negative evidence for this Draft's diversity-gated orchestrator/auto fallback.

Acceptance remains: preserve exact repository/head binding and fail-closed result semantics; diversity 0/1/absent/malformed evidence must retain orchestrator/auto; a failing route must advance to another admitted independent route when one exists; and protected-main canaries must produce an authoritative report before this lane is considered complete. Do not rerun the unchanged canary until a causal gateway/pool fix integrates.

Copy link
Copy Markdown
Contributor Author

Traced this. The decision-classification half of criterion 3 was already fixed, sitting unmerged: ContextualWisdomLab/contextual-orchestrator#922 ("classify primary provider transport failures explicitly") replaces the tool-failure classifier a plain provider transport error was incidentally routed through — which could fail_closed on message-text coincidence — with classify_provider_transport_failure, keyed only on the provider taxonomy's own retryable flag: non-retryable (401/403/404/422/..., and 400 per this incident) fails over to the next candidate immediately, retryable (429/5xx) gets one bounded same-agent retry then fails over. That PR is exactly the "in-progress, not-yet-opened-as-a-PR commit" ADR-0020 flags as pending and tracked outside this repository.

Root-caused why the incident still reproduced despite that fix already existing: .github's outer retry loop (strix_quick_gate.sh::run_strix_with_transient_retry) starts a fresh gateway process per attempt and always passes the same --model contextual-orchestrator/orchestrator/free pool name — it never itself picks a different agent, by design, delegating agent selection entirely to the gateway. So three outer attempts hitting the same agent means each fresh gateway invocation independently picked that same first-ranked agent and failed closed within that single invocation — consistent with #922's fix not yet being live on whatever commit this canary ran against, not with a bug in the outer retry loop.

No existing test proved the classification fix actually holds end-to-end for the exact error code this incident hit (a plain 400, not the 5xx cases already covered), so I added test_free_model_advances_through_the_free_pool_on_non_retryable_4xx to #922 (commit 5d60b929) — reproduces the incident shape, passes: a 400 on the primary free route advances immediately, no same-agent retry, to the next free route.

What #922 does not address, and what still needs attention before this Draft's criterion 3 is fully closed: the incident's live run apparently had only one eligible candidate for that specific request shape despite nominal free_family_diversity ≥ 2 (per contextual_orchestrator_review_policy.py's family-collapsing: e.g. nvidia_nim/nvidia_nim_sub collapse to one family). Catalog-level family diversity ≥2 doesn't guarantee _failover_candidates admits ≥2 agents for a given request's role/tag/ZDR filter — that's a request-time candidate-set gap the diversity gate as currently scoped doesn't see. I haven't implemented a fix for that half; flagging it here as the remaining acceptance-criteria item for whoever advances this Draft, rather than guessing at a fix for a mechanism I haven't fully instrumented.

Sequencing note: #922 is currently mergeable_state: blocked (same org-wide opencode-review outage affecting essentially everything right now, not a defect in #922 itself) — once that pipeline recovers and #922 merges, the vendored ORCHESTRATOR_PIN_SHA in .github's contextual_orchestrator_review_sidecar.sh needs a follow-up bump (intentionally not bundled into #922) before its fix is live for Strix's own canary runs.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Fresh complementary protected-main canary for the diversity acceptance contract:

  • Target: #1270 exact head 63cd827f8a66ba36c0bc0af61e0830fbab0622a2.
  • Required Strix run 33328950520 / job 99303971843 checked out and scanned that exact head.
  • The live policy report selected 8 free routes, all in the same provider family nvidia_nim; priced_selected_count=0, unknown_selected_count=0, and zdr_selected_count=0. Bytez discovery separately failed with HTTP 500.
  • This particular scan completed authoritatively with zero findings and artifact 9737188756 (digest sha256:aa8bc70e4bdc3da73216911da419968e778963ca6c4619a295299da317816e4d).

That successful scan is useful availability evidence for one request, but it is not independent outage-domain diversity evidence. It proves the protected-main policy can still choose the free pool when the admitted set has diversity 1. The acceptance boundary therefore remains: diversity 0/1, absent, wrong-typed, or malformed evidence must retain orchestrator/auto, and request-time eligibility must demonstrate a genuinely independent fallback family before this lane can be considered operationally complete. Do not infer closure from this single successful run.

Copy link
Copy Markdown
Contributor Author

Acknowledged — this confirms the exact gap flagged in my earlier comment: 8 admitted routes all in the single nvidia_nim family is family-diversity-1 in substance, and this run only avoided the #1270 incident's fate by luck (that route happened to serve), not because the mechanism is safe. Agreed a passing run here proves nothing about closure. No new action from me beyond what's already tracked (.github#1437's own catalog-level diversity gate, plus the still-open request-time candidate-set question I flagged) — not touching this Draft's code or status per the standing instruction to leave it as-is.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Fresh operational reproduction from #1438 run 33331290092, tied to historical exact target head 50febfe7a9bd74c8c33d1eef6526a33f116f5c2f (the PR has since moved, so this is not current-head passing evidence):

  • attempt 1 verified the target head, discovered a live OpenRouter ZDR feed, passed sidecar health/provider preflight after 48s, and passed the gateway chat/completions preflight on attempt 1;
  • every one of the 8 selected free routes belonged to the nvidia_nim outage-domain family;
  • the actual Strix quick scan then stalled until the 5400-second bound and exited 124, emitting typed STRIX_PROVIDER_UNAVAILABLE; no authoritative vulnerability result was produced;
  • the workflow uploaded diagnostic artifact strix-reports (artifact id 9738976922), but diagnostics are not scan evidence;
  • attempt 2 was cancelled during sidecar provisioning before Strix installation or scanning, so it is also non-passing.

This independently confirms that discovered route count is not provider-family diversity or request-time availability. A same-family catalog that passes discovery/preflight can still exhaust without advancing to an independent provider. Treat this as provider/infrastructure evidence, never as a #1438 source vulnerability or green scan. Do not rerun the unchanged canary again until the causal diversity/request-time failover boundary changes.

Copy link
Copy Markdown
Contributor Author

Fresh request-time diversity evidence from unchanged exact consumer of the central Strix contract:

  • source PR: .github#1460@2cc819a9bca2f960ef1b0e67bf2e382c1eaf6251
  • required repository-dispatch run/job: https://github.com/ContextualWisdomLab/.github/actions/runs/33341300103/job/99337916455
  • terminal result: failure / typed STRIX_PROVIDER_UNAVAILABLE
  • exact PR target was fetched and two scannable changed files were retained
  • all three bounded attempts selected the same provider/model: nvidia_nim_meta_llama_3_2_90b_vision_instruct (meta/llama-3.2-90b-vision-instruct)
  • each attempt returned non-retryable provider HTTP 400; no authoritative vulnerability analysis completed
  • diagnostic artifact: strix-reports@9740909379 (15,883 bytes)

This independently reproduces the causal boundary this Draft owns: increasing catalog candidates or retry count does not create provider-family diversity, and request-time failure must advance to an independent route when live diversity permits. The visible diagnostic finding count is incomplete/model-only evidence and is not treated as a source clean bill.

The current workflow correctly fails closed instead of fabricating a vulnerability or false green. Acceptance for this PR remains: free_family_diversity >= 2 may select orchestrator/free; diversity 0/1/absent/malformed retains orchestrator/auto, followed by a protected-main operational canary demonstrating request-time fallback across an independent provider family.

Adopt current protected main, preserve the live security and scheduler repairs, remove the now-integrated direct-NIM split from this lane, and restore the fail-closed free-family diversity classifier plus its post-preflight runtime backstop tests.

Copy link
Copy Markdown
Contributor Author

Exact-head hosted evidence update for a1dc62dfc9a5a938deff9995913e013766e55e2c on protected main@1cf2f9120a2cd494ed5079135bb10822c27b5947:

  • SAST job 99354624168 records expected_sha=a1dc62df… actual_sha=a1dc62df… and completed success.
  • Trivy job 99354623902 records the same exact-head equality and completed success.
  • Dependency Review job 99354623838 records the same equality and completed success.
  • Security Scan, SAST, CodeQL, OSV, SBOM, Python Security, Secret Scan, Scorecard, exact-artifact coverage, commercial-readiness, and Rust coverage workflows are terminal success on this exact head.
  • Formal Reviews API submissions remain 0; review threads remain 0.
  • Required Strix run 33347600432 was cancelled before any job existed while predecessor/protected-main/other-PR Strix leases were active. It is non-passing infrastructure evidence, not a source finding and not an authoritative scan.
  • Required Noema is still in progress and OpenCode queued. Pending/queued/cancelled evidence is not counted as passing.

The PR remains Draft and unmerged. Source merge alone cannot close the lane: require an authoritative exact-current-head Strix result (or explicit typed provider-unavailable non-pass), a substantive current-head formal verdict, and protected-main consumer acceptance.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Cross-PR model-group correction: #1468 pins the shared Noema/Strix/OpenCode sidecar to contextual-orchestrator 0adca470 and removes the sidecar-local provider-family collapse. Each of BYTEZ_API_KEY, NVIDIA_NIM_API_KEY, NVIDIA_NIM_API_KEY_SUB, OPENROUTER_API_KEY, and OPENAI_API_KEY is an independent credential account; vendor/endpoint identity is not model equivalence. Only an explicit model_group in contextual-orchestrator may share routing evidence. Please rebase/stack on #1468 or treat its sidecar-policy portion as superseding overlapping family_cap/pin assumptions.

seonghobae pushed a commit that referenced this pull request Aug 31, 2026
… main

Merges main (which brought in the SSRF fix and sandboxed-web-e2e isolation
work) and extends this PR's own pin bump beyond its original target
(79c6841b, contextual-orchestrator PR #929) to contextual-orchestrator's
current main tip (0adca4703df67f8f31d3ea5b04a1e07ed775dd6c), which also
vendors:

- #941 (fix(discovery): keep credential accounts independent) + #945 (its
  matching test fix): removes the wrong assumption that nvidia_nim and
  nvidia_nim_sub share one model catalog/outage domain, which
  docs/planning/adrs/0015-durable-provider-catalog.md already contradicted.
  Without this, every hosted noema-review/opencode-review/strix run kept
  using the old collapsing logic regardless of what was merged upstream.
- a vision-input exclusion for the blind orchestrator/free pool (unrelated
  bugfix, additive only, no CLI-surface change).

Reviewed the incremental diff (79c6841b..0adca470) for sidecar dependency-
surface breakage per ADR-0003's own requirement: requirements.lock is
byte-for-byte unchanged, and every __main__.py CLI change is additive (new
--verbose flags, a new report field) -- register-credential and
--serve --agents ... --port ... --auth-token ... are untouched.

Updates the pin in all three places the contract lives (sidecar script
default, the contract test's ORCH_PIN_SHA, ADR-0003's "today" reference)
plus the product-technical-gap-baseline.md review record and CHANGELOG.

This is a prerequisite for the separate PROVIDER_FAMILIES fix in
scripts/ci/contextual_orchestrator_review_policy.py (this repo's own,
independent copy of the same nvidia_nim/nvidia_nim_sub collapsing bug) to
have any live effect on the free_family_diversity evidence PR #1437 reads.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants