feat: route hourly product agents through contextual orchestrator - #165
Conversation
|
Current head |
|
Warning Review limit reachedNext included review available in 51 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 (9)
📝 WalkthroughWalkthrough시간별 제품 개발 워크플로가 Contextual Orchestrator 게이트웨이를 사용하도록 변경되었습니다. 게이트웨이는 다섯 제공자의 자격 증명을 등록하고 모델을 검색합니다. OpenCode는 인증된 loopback 게이트웨이를 통해 단일 모델을 실행합니다. 관련 ADR, 운영 문서, 검증 테스트도 갱신되었습니다. Changes시간별 Contextual Orchestrator 실행
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to Hourly jobs now route provider credentials through the gateway, but the current tests do not verify that each credential is registered exactly once and removed from the process environment before serving. That leaves a concrete credential-leakage or startup-integrity regression path, so the PR should not merge until this check is added or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Workflow as 시간별 워크플로
participant Bootstrap as Contextual Orchestrator 부트스트랩
participant Providers as 다섯 제공자
participant Gateway as loopback 게이트웨이
participant OpenCode
Workflow->>Bootstrap: 자격 증명과 게이트웨이 토큰 전달
Bootstrap->>Gateway: 자격 증명 KV 등록
Bootstrap->>Providers: 모델 검색
Bootstrap->>Gateway: 최대 3개 모델 활성화 및 서버 시작
Workflow->>Gateway: health 및 models 검증
Workflow->>OpenCode: loopback URL과 게이트웨이 토큰 전달
OpenCode->>Gateway: 단일 contextual-orchestrator 모델 실행
Gateway->>Providers: 모델 요청 라우팅
Gateway-->>OpenCode: 모델 응답 반환
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The exact head advanced to |
|
Current-head verification: Resolved the CodeRabbit bootstrap-test finding by executing the real Exact-head local evidence: focused workflow contract 10 tests passed; quality suite 93 tests passed; statement/branch coverage 100%; pinned contextual-orchestrator archive SHA/import/five-key bootstrap passed; actionlint, ruff, compileall, documentation, docstring, workspace, and diff checks passed. Hosted Checks are rerunning for this exact HEAD. Merge remains gated on current required Checks and an independent qualifying approval; no bypass used. |
|
Current HEAD 37f85fe remains the exact scheduler head. The only terminal failure-looking entries are from Rust Foundation CI run 32367771909, whose workflow and every job concluded cancelled before any steps ran; they are not source-test failures. Fresh exact-head jobs in the newer run set remain queued, including Semgrep, dependency/security, Strix, Noema, and OpenCode gates. Local focused workflow contract and full quality coverage already passed; no cancellation retrigger commit is being invented. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Verified exact current HEAD |
|
Rechecked the review findings against current HEAD |
|
Fixed the reproduced discovery-to-chat routing defect at exact HEAD |
|
Current exact-head refresh |
|
@cwl-noema-review please independently review exact current HEAD |
|
Fixed the scheduler queue race and scope at exact current HEAD |
|
Documentation follow-up at exact current HEAD |
|
Final scheduler hardening at exact current HEAD |
|
Added one structural assertion at exact current HEAD |
|
@opencode-agent @cwl-noema-review Review-only request. Inspect the exact current pull-request head and base below; do not approve or merge on stale evidence.
|
|
@opencode-agent @cwl-noema-review Review-only request for the exact current head after the documentation consistency fix. repository: ContextualWisdomLab/TEPP |
|
Queued @cwl-noema-review and @opencode-agent for PR #165 at head |
|
Queued @cwl-noema-review and @opencode-agent for PR #165 at head |
|
Queued @cwl-noema-review and @opencode-agent for PR #165 at head |
|
@opencode-agent @cwl-noema-review Review-only request for exact current head `3a0fbfc24a2294e43ad04ee72064362e4385dee9`. Re-review contextual-orchestrator hourly product-agent credential isolation, auto-discovery boundaries, live tests, and protected-merge requirements. Do not merge or enable auto-merge. |
|
Queued @cwl-noema-review and @opencode-agent for PR #165 at head |
|
Current-head review refresh for 3a0fbfc:
|
|
Exact-head review request The branch is now merge-ready against current |
|
Queued @cwl-noema-review and @opencode-agent for PR #165 at head |
|
Current-head review refresh for 34083c3:
|
|
Current-head review refresh: The previously reported bootstrap credential test now executes Fresh local evidence:
Hosted |
Maintainer audit — exact head 34083c3
|
Exact-head review disposition
|
…-orchestrator # Conflicts: # CHANGELOG.md # docs/adr/README.md
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| | [0016](0016-tdt-chronos-event-intelligence-boundary.md) | TDT, CHRONOS, and Event Ontology intelligence boundary | Accepted | accepted-target | Separates observed evidence, detection/tracking, prediction/schema inference, temporal consistency, and promoted transition authority. | | ||
| | [0017](0017-hourly-contextual-orchestrator-gateway.md) | Hourly contextual-orchestrator gateway and all-provider model discovery | Accepted | active-PR | Keeps proposal-model execution behind a pinned loopback gateway while preserving independent verifier, publisher, reviewer, and merge authority. | |
There was a problem hiding this comment.
🟡 Duplicate ADR 0016 entry with conflicting maturity
A second ADR 0016 row was added to the index alongside the existing one, and the two disagree on implementation maturity (accepted-target here versus active-PR at the older row). The same decision is now listed twice with conflicting authority metadata. The set-based check in validate_adr_graph does not detect the duplicate.
Prompt for agents
The ADR index table in docs/adr/README.md now contains two rows for ADR 0016: a newly added row (line 24) with implementation maturity 'accepted-target', and the pre-existing row (line 26) with 'active-PR'. This PR should only have added the ADR 0017 row. Remove the duplicate 0016 row so exactly one 0016 entry remains, and ensure the table stays in ascending numeric order (0015, 0016, 0017). Decide which 0016 maturity value is correct and keep only that single row.
Was this helpful? React with 👍 or 👎 to provide feedback.
| python3 scripts/run_contextual_orchestrator.py \ | ||
| --host 127.0.0.1 --port "$CONTEXTUAL_ORCHESTRATOR_PORT" \ | ||
| --report "$report_file" >"$log_file" 2>&1 & | ||
| echo $! >"$RUNNER_TEMP/contextual-orchestrator.pid" | ||
| ready=false | ||
| for _ in $(seq 1 120); do | ||
| if curl --fail --silent --show-error "http://127.0.0.1:${CONTEXTUAL_ORCHESTRATOR_PORT}/healthz" >/dev/null; then | ||
| ready=true | ||
| break | ||
| fi | ||
| if ! kill -0 "$(cat "$RUNNER_TEMP/contextual-orchestrator.pid")" 2>/dev/null; then | ||
| cat "$log_file" | ||
| exit 1 | ||
| fi | ||
| sleep 1 | ||
| done | ||
| [ "$ready" = true ] | ||
| curl --fail --silent --show-error \ | ||
| -H "Authorization: Bearer $(cat "$token_file")" \ | ||
| "http://127.0.0.1:${CONTEXTUAL_ORCHESTRATOR_PORT}/v1/models" \ | ||
| >"$RUNNER_TEMP/contextual-orchestrator-models.json" | ||
| jq -e '.data | length > 0' "$RUNNER_TEMP/contextual-orchestrator-models.json" >/dev/null | ||
| jq -e '.discovered_count > 0 and (.selected_models | length > 0)' "$report_file" >/dev/null | ||
| echo "CONTEXTUAL_ORCHESTRATOR_TOKEN_FILE=$token_file" >>"$GITHUB_ENV" |
There was a problem hiding this comment.
📝 Info: Gateway survives across steps by design
The gateway is launched with & in the start step and used by the later agent step through the token-file path in GITHUB_ENV. This depends on the background process persisting across steps, which hosted runners allow for the job lifetime; the process is reclaimed at job end. No explicit teardown exists, which is acceptable on ephemeral runners.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def _is_general_chat_model(model_id: object) -> bool: | ||
| """Keep endpoint-only and safety-only catalog rows out of chat routing.""" | ||
| if not isinstance(model_id, str): | ||
| return False | ||
| tokens = tuple(_MODEL_TOKEN_RE.findall(model_id.casefold())) | ||
| if not tokens or any( | ||
| token in _NON_CHAT_MODEL_TOKENS | ||
| or token.startswith(_NON_CHAT_MODEL_PREFIXES) | ||
| for token in tokens | ||
| ): | ||
| return False | ||
| return not any( | ||
| token == "safety" | ||
| or token == "guard" | ||
| or token == "shieldgemma" | ||
| or token.startswith("nemoguard") | ||
| for token in tokens | ||
| ) |
There was a problem hiding this comment.
📝 Info: Chat-model token filter could exclude legitimate chat models sharing an excluded token
_is_general_chat_model (run_contextual_orchestrator.py) tokenizes on [a-z0-9]+ and rejects any model whose tokens include entries like e5, clip, image, speech, guard, safety, etc. This is intentional to keep embedding/image/moderation/safety endpoints out of chat routing, but it is a substring/token match that could incidentally drop a genuine chat model whose identifier happens to contain a token such as e5 or image. Given discovery draws from five providers with heterogeneous naming, a false exclusion would silently shrink the chat pool; the fail-closed guards (no general chat candidates) mitigate total loss but not partial over-exclusion. Not a bug given current known provider catalogs, but flagged for awareness if provider model names change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fi | ||
| [ "$(jq 'length' <<<"$open_issues")" -eq 0 ] || { echo '::error::open_issue_after_generation'; exit 1; } | ||
| live_base="$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/heads/${DEFAULT_BRANCH}" --jq '.object.sha')" | ||
| [ "$live_base" = "$expected_base" ] || { echo '::error::base_branch_advanced'; exit 1; } |
There was a problem hiding this comment.
🔍 Publisher revalidation references an unset variable, making publication fail-closed
In the publisher's "Revalidate queue and default-branch head" step, line 624 compares $live_base against $expected_base (lowercase), but the environment variable is exported as EXPECTED_BASE (uppercase) at line 608 and used correctly on line 612. Under set -euo pipefail, referencing the unset $expected_base triggers an unbound variable error, so this step always fails before it can create a PR. This is a pre-existing latent bug (the line is unchanged context, not part of this PR's added lines), but this PR adds the open-issue revalidation block (hourly-nim-product-development.yml) directly above it, so a reviewer touching this step should fix the casing while here. Note it likely went unnoticed because the workflow has not run to publication (credentials unconfigured).
Was this helpful? React with 👍 or 👎 to provide feedback.
| "nvidia/llama-3.3-nemotron-super-49b-v1.5": {"tool_call": true}, | ||
| "nvidia/nemotron-3-super-120b-a12b": {"tool_call": true}, | ||
| "deepseek-ai/deepseek-v4-pro": {"tool_call": true} | ||
| "contextual-orchestrator": {"tool_call": true} |
There was a problem hiding this comment.
🔍 OpenCode single virtual model name depends on unverifiable orchestrator routing contract
OpenCode is configured with a single model contextual-orchestrator (hourly-nim-product-development.yml) and invoked with --model contextual-orchestrator/contextual-orchestrator (line 310). The gateway's discovered /v1/models list (validated at line 283) contains the actual provider model IDs, not necessarily a virtual model literally named contextual-orchestrator. Whether the gateway accepts a chat/completions request with model contextual-orchestrator and internally routes to a discovered model is an assumption about the pinned ContextualWisdomLab/contextual-orchestrator commit that cannot be verified from this repo (the package is downloaded at runtime). If the gateway rejects unknown model names, the agent run fails at request time even though healthz and model-list probes pass. Worth confirming against the orchestrator's routing behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "options": { | ||
| "baseURL": "https://integrate.api.nvidia.com/v1", | ||
| "apiKey": "{env:NVIDIA_NIM_API_KEY}" | ||
| "baseURL": "http://127.0.0.1:18000/v1", |
There was a problem hiding this comment.
📝 Info: OpenCode baseURL hardcodes port while workflow parameterizes it
The OpenCode provider config hardcodes http://127.0.0.1:18000/v1 (hourly-nim-product-development.yml) while the gateway is started with --port "$CONTEXTUAL_ORCHESTRATOR_PORT" and the health/model probes use ${CONTEXTUAL_ORCHESTRATOR_PORT}. Today the env var is 18000 so they agree, but changing CONTEXTUAL_ORCHESTRATOR_PORT would silently break OpenCode's connection because the config would still point at 18000. Not a bug now, but a fragile coupling worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo $! >"$RUNNER_TEMP/contextual-orchestrator.pid" | ||
| ready=false | ||
| for _ in $(seq 1 120); do | ||
| if curl --fail --silent --show-error "http://127.0.0.1:${CONTEXTUAL_ORCHESTRATOR_PORT}/healthz" >/dev/null; then |
There was a problem hiding this comment.
🔍 Liveness probe assumes /healthz is exempt from bearer auth
The startup step polls http://127.0.0.1:.../healthz with curl --fail and no Authorization header (hourly-nim-product-development.yml), while the gateway is constructed with SecurityConfig(auth_token=gateway_token) (run_contextual_orchestrator.py). If the gateway enforces bearer auth on /healthz, the probe will receive 401, ready never becomes true, and the step fails after ~120s on every run. The authenticated /v1/models probe (:279-282) does send the token. Confirm the pinned gateway leaves /healthz unauthenticated.
Was this helpful? React with 👍 or 👎 to provide feedback.
| selected = select_top_n_cheapest_discovered_agents(chat_discovered, price_book, 3) | ||
| if not selected: | ||
| raise RuntimeError("model discovery selected no general chat candidates") | ||
| agents = [ | ||
| replace(agent_from_discovered(model, priority=3 - index), disabled=False) | ||
| for index, model in enumerate(selected) |
There was a problem hiding this comment.
🔍 select_top_n_cheapest_discovered_agents then agent_from_discovered relies on unverified API shapes
_selected_agents passes raw discovered models to select_top_n_cheapest_discovered_agents(discovered, price_book, 3) and then maps each result through agent_from_discovered(model, ...) before dataclasses.replace(..., disabled=False) (run_contextual_orchestrator.py). The function name suggests it might already return agents; the tests stub it to return the discovered models unchanged. If the real orchestrator API returns agent objects rather than discovered-model objects, agent_from_discovered and .provider_name/.model attribute access could break at runtime. This cannot be verified here because the package is fetched at CI time. Confirm the exact signatures at the pinned commit.
Was this helpful? React with 👍 or 👎 to provide feedback.
| select_top_n_cheapest_discovered_agents, | ||
| ) | ||
|
|
||
| discovered, errors = discover_all_models() |
There was a problem hiding this comment.
🔍 Discovery runs before /healthz is served; 120s budget must cover 5-provider network discovery
In run_contextual_orchestrator.py, _selected_agents() (which calls discover_all_models() making outbound calls to all five providers) executes before _start_gateway() starts the HTTP server, so /healthz is unavailable until discovery + report write complete. The workflow readiness loop (hourly-nim-product-development.yml) allows 120 seconds and exits early only if the process dies. If any provider's model-list endpoint is slow or hangs without an internal timeout, discovery can exceed the budget and the step fails closed (acceptable), but a hung provider call could also leave the background python process holding the port until job teardown. Confirm discover_all_models applies per-provider timeouts.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - `contextual_orchestrator_credentials_unavailable` | ||
| - `maintainer_app_unavailable` | ||
|
|
||
| A failed model candidate is discarded before a later candidate runs. A cleanup |
There was a problem hiding this comment.
🔍 Stale failure-recovery text still describes the removed multi-candidate fallback
This PR removes the multi-model fallback loop from the workflow (old hourly-nim-product-development.yml iterated over $OPENCODE_MODELS with git reset --hard/reinstall between candidates); the new step runs opencode run exactly once against the orchestrator gateway. However HOURLY_NIM_PRODUCT_DEVELOPMENT.md still states "A failed model candidate is discarded before a later candidate runs. A cleanup or reinstall failure stops fallback." That sentence no longer matches the workflow behavior and should be updated. (This line is outside the PR's changed hunks, so it is reported here as an analysis rather than an inline bug.)
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Gap
The hourly scheduler named contextual-orchestrator in its prompt but sent OpenCode directly to one NVIDIA NIM provider. It now uses the actual contextual-orchestrator gateway and auto model discovery.
Design
e226e1197bdfc890c9d8e5b9b648c78857d7e465with SHA-256 verification;Verification
python3 -m unittest tests.quality.test_hourly_nim_product_developmentpython3 scripts/validate_documentation.pypython3 scripts/check_docstrings.pypython3 scripts/check_workspace_contract.pyactionlint .github/workflows/hourly-nim-product-development.ymlgit diff --checkResearch and residual risk
APA 7 references remain in
docs/doctoring/hourly-nim-opencode-development.md, including contextual-orchestrator, NIST SSDF, ISO/IEC 23894, and ISO/IEC 42001. Provider terms, retention, regions, and confidentiality must be reviewed before enabling all five secrets. No release, merge, deployment, Figma artifact, or scientific claim is made by this PR.Summary by CodeRabbit
새로운 기능
보안 및 운영 개선
문서