chore(ci): bump ORCHESTRATOR_PIN_SHA past #941's NVIDIA-account-independence fix to current main - #1465
chore(ci): bump ORCHESTRATOR_PIN_SHA past #941's NVIDIA-account-independence fix to current main#1465seonghobae wants to merge 2 commits into
Conversation
Vendors contextual-orchestrator's request-time provider-transport-failure classification fix (#922) plus everything else that landed on that repo's main since the last pin (#912, #868, #920, #921, #928, #925, #929 -- 148 commits total). requirements.lock is unchanged and every __main__.py CLI change is additive, so the sidecar's dependency surface is unaffected. No provider credentials are available in this sandbox, so an actually- executed end-to-end sidecar run isn't possible here; live verification is left as a follow-up for the next hosted noema-review/opencode-review/strix run against this pin, per the gap-baseline entry.
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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. Comment |
|
Standing down on the required Devin Review separately reported no issues on this head. Not a flake — re-running would not help. Keeping this PR watched; not bypass-merging it myself since (as noted in the PR body) this change's correctness depends on live provider behavior this sandbox can't exercise, so it should get an actual hosted review/CI pass once the outage clears, or the user's explicit go-ahead to bypass. Generated by Claude Code |
|
Also standing down on the classic Generated by Claude Code |
|
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. |
… 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
seonghobae
changed the title
|
Closing as superseded: Generated by Claude Code |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| set -euo pipefail | ||
|
|
||
| ORCHESTRATOR_PIN_SHA="${ORCHESTRATOR_PIN_SHA:-30c6d71680e659f25a0a433d4726ad0d437f9757}" | ||
| ORCHESTRATOR_PIN_SHA="${ORCHESTRATOR_PIN_SHA:-0adca4703df67f8f31d3ea5b04a1e07ed775dd6c}" |
There was a problem hiding this comment.
🟡 Vision routes remain in general reviews
The pinned release cannot exclude multimodal routes because the sidecar drops their input tags. They can pass preflight and reject real review requests.
Prompt for agents
Update the sidecar integration so its catalog uses the pinned orchestrator's general-purpose free-serving eligibility, or preserve discovered input-modality metadata as input:<modality> agent tags. The current launcher calls free_discovered_models and _report_rows omits input_modalities, while contextual_orchestrator_review_policy emits only review/cost/ZDR tags. Consequently both general_free_serving_candidates and TaskOrchestrator._is_general_free_agent lack the evidence needed to exclude vision-input routes. Add an integration regression using a free text+image model that can pass a plain chat probe but must not enter orchestrator/free.
Was this helpful? React with 👍 or 👎 to provide feedback.
| set -euo pipefail | ||
|
|
||
| ORCHESTRATOR_PIN_SHA="${ORCHESTRATOR_PIN_SHA:-30c6d71680e659f25a0a433d4726ad0d437f9757}" | ||
| ORCHESTRATOR_PIN_SHA="${ORCHESTRATOR_PIN_SHA:-0adca4703df67f8f31d3ea5b04a1e07ed775dd6c}" |
There was a problem hiding this comment.
🔴 OpenRouter slowdown aborts all reviews
The pinned release can spend the entire 180-second startup budget fetching optional OpenRouter metadata. A slowdown kills every review before provider preflight.
Prompt for agents
Bound the newly pinned OpenRouter privacy-metadata fanout within the sidecar's total startup deadline. At this revision, discover_provider_models calls _openrouter_free_model_endpoints for every token-free OpenRouter model using eight workers and a per-request 15-second timeout; the current catalog contains dozens of candidates, so stalled requests can consume or exceed the shell's 180-second readiness watchdog before other discovery and route preflight complete. Add one shared discovery deadline or a much smaller bounded metadata budget, and verify that optional OpenRouter metadata failure degrades only that evidence source rather than terminating the entire review sidecar.
Was this helpful? React with 👍 or 👎 to provide feedback.
| **prerequisite** for the separate, independent fix to | ||
| `scripts/ci/contextual_orchestrator_review_policy.py`'s own | ||
| `PROVIDER_FAMILIES` hardcoded map (this repo's own copy of the identical | ||
| wrong assumption, never touched by #941 since that PR only fixed | ||
| `contextual-orchestrator` itself) to have any live effect on the | ||
| `free_family_diversity` evidence PR #1433 added and PR #1437 (open) gates | ||
| Strix's `orchestrator/free` eligibility on. |
There was a problem hiding this comment.
🔍 NVIDIA diversity remains partially disabled
Upstream now separates both NVIDIA accounts, while local PROVIDER_FAMILIES still collapses them. Catalog caps and diversity evidence remain unchanged pending follow-up.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded pin path: #1468 already merged the independent credential-account and explicit model_group sidecar policy at d7b01bc. Follow-up #1469 now advances that exact shared sidecar pin to contextual-orchestrator c107e3e5, adding bounded transient per-account discovery retry after the live Bytez HTTP 500 observation. This PR remains conflicting and its older pin should not replace #1469. |
2 participants
Summary
contextual-orchestratorPR #922 (fix(routing): classify primary provider transport failures explicitly— the classification fix behind PR #1437's own request-time failover acceptance contract) merged. Its own body flagged this as an explicit, deliberately-deferred follow-up: "the vendoredORCHESTRATOR_PIN_SHAinContextualWisdomLab/.github'sscripts/ci/contextual_orchestrator_review_sidecar.shneeds a follow-up bump in a separate PR there — intentionally not done in this PR."Extended scope (2026-08-31 session investigation). This PR originally bumped the pin to
79c6841b9b3645d1f14b943985825d2302071f5b(contextual-orchestratormain tip at the time). Verifying that fix landed in production, this session foundgit merge-base --is-ancestorshows that target SHA predatescontextual-orchestratorPR #941 (fix(discovery): keep credential accounts independent, merged) and its stale-test follow-up #945 by two more merged PRs — so merging this PR as originally scoped would still have shipped a stale pin. The bump has been extended the rest of the way, from30c6d71680e659f25a0a433d4726ad0d437f9757(the #1430 pin) directly tocontextual-orchestrator's currentmaintip,0adca4703df67f8f31d3ea5b04a1e07ed775dd6c, in the three places the pin contract lives: the sidecar script default, the contract test'sORCH_PIN_SHA, and ADR-0003's "today" reference.Why #941/#945 matter here specifically:
contextual-orchestrator'smodel_discovery.pyused to wrongly assume the two independentnvidia_nim/nvidia_nim_subKV credentials share one model catalog/outage domain, collapsing them into one "provider family" for pool-diversity purposes — an assumptiondocs/planning/adrs/0015-durable-provider-catalog.md(accepted 2026-08-22) already contradicted. #941 removed that collapsing; #945 fixed the one stale test that still asserted it. Without this pin bump, every hostednoema-review/opencode-review/strixrun kept using the old, buggy collapsing logic regardless of what was already merged upstream incontextual-orchestrator— this pin was the actual reason the fix never reached production. This bump is a prerequisite for a separate, independent fix (opened as a companion PR) toscripts/ci/contextual_orchestrator_review_policy.py's own hardcodedPROVIDER_FAMILIESmap — this repo's own copy of the identical wrong assumption, never touched by #941 since that PR only fixedcontextual-orchestratoritself — to have any live effect on thefree_family_diversityevidence PR #1433 added and open PR #1437 (draft) gates Strix'sorchestrator/freeeligibility on.This also vendors everything else that landed on
contextual-orchestratormainsince the #1430 pin: #912 (video job resource normalization), #868 (gateway default chat model + configured-gateway discovery + OpenRouter ZDR/privacy metadata), #920/#921 (cleanup/docs), #928 (provider-catalog-sync outage tolerance), #925 (stream_options.include_usagefor tools passthrough), #929 (Bytez raw-tokenAuthorizationheader fix), #941/#945 (above), and an unrelated free-pool correctness fix that excludes vision-input-only models (e.g. NVIDIA NIM'smeta/llama-3.2-90b-vision-instruct, root cause ofContextualWisdomLab/.github#1198) from the blindorchestrator/freechat pool.Developer experience
79c6841b) was already reviewed by this PR's first revision. This session's incremental review covers only the new span (79c6841b→0adca470, i.e. fix(semgrep): make the pinned image digest authoritative #941/[Fleet incident] Govern orphaned GitHub Actions workflow lifecycle organization-wide #945 plus the vision-input exclusion):requirements.lockis byte-for-byte unchanged across the whole span (no--require-hashesdrift), and every change tocontextual_orchestrator/__main__.py(the CLI the launcher invokes) is strictly additive — new optional--verboseflags and a new discovery report field, zero removed arguments or subcommands. Theregister-credentialand--serve --agents ... --port ... --auth-token ...invocations the sidecar/launcher depend on are untouched.docs/product-technical-gap-baseline.md.User experience
Central CI review (OpenCode, Noema, Strix) now runs against a
contextual-orchestratorgateway that includes #922's request-time failover classification fix and #941/#945's NVIDIA-credential-independence fix, once this is merged and the next required-workflow review executes. Once merged, this also unblocks the companionPROVIDER_FAMILIESfix (scripts/ci/contextual_orchestrator_review_policy.py) from actually reflecting correctedfree_family_diversityevidence in production.Test plan
coverage run -m pytest tests -q(full suite) — 2092 passed, 1 skipped, 21 subtests passedcoverage report --show-missing— 100%interrogate— 100%BYTEZ_API_KEY/NVIDIA_NIM_API_KEY/NVIDIA_NIM_API_KEY_SUB/OPENROUTER_API_KEY/OPENAI_API_KEY), so an actually-executed end-to-end sidecar run against real providers isn't possible here. The real proof is the next hostednoema-review/opencode-review/strixrun against this new pin, once the ongoing org-wideopencode-reviewoutage clears enough to observe one.Known trust-boundary limitation
This PR touches
scripts/ci/contextual_orchestrator_review_sidecar.sh, one of the trustedpull_request_targetscripts. Per this repo's own documented trust boundary, the required review workflows run the base branch's trusted scripts, so this PR's own CI checks validate againstmain's current (pre-bump) sidecar, not this diff — a previously-diagnosed limitation, not a new one, and not by itself grounds for a bypass merge.Generated by Claude Code