feat: price-aware live model routing + admin KV credential endpoint - #111
feat: price-aware live model routing + admin KV credential endpoint#111seonghobae wants to merge 8 commits into
Conversation
Extend the existing offline "maximize quality, minimize cost" tradeoff (_recommend_config/optimize_orchestration) into live per-request routing: _score_agent now takes the operator-supplied price_per_million as a tie-break, so among agents tied on capability match + priority, the cheaper model wins. Unpriced agents never lose the tie-break, so pools that configure no prices are unaffected. Add --price-per-million to the CLI so this is actually configurable outside of direct construction. Add POST /admin/api/credentials (admin scope) so a credential such as LITELLM_API_KEY can be registered into the KV from the admin console frontend instead of only the register-credential CLI subcommand. This stays inside the boundary ADR 0003 already chose: the gateway writes a named secret into its own KV registry and does not implement Keyverse OIDC identity itself. The value is never echoed back or exposed by any GET endpoint. Docs: document both in docs/architecture.md and docs/kv-credentials.md, including how to wire an agent's credential_key to a KV-registered LITELLM_API_KEY-style gateway credential.
📝 WalkthroughWalkthroughCLI는 모델별 가격을 검증하고 Changes라우팅 및 관리자 보안
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant 관리자브라우저
participant 세션API
participant SecurityConfig
participant 자격증명API
participant KV저장소
관리자브라우저->>세션API: bearer token으로 POST /admin/session
세션API->>SecurityConfig: token 검증 및 opaque session ID 발급
SecurityConfig-->>관리자브라우저: HttpOnly 세션 쿠키
관리자브라우저->>자격증명API: 세션 쿠키와 자격 증명 전송
자격증명API->>KV저장소: 검증된 name과 value 저장
KV저장소-->>자격증명API: 등록 결과
자격증명API-->>관리자브라우저: 이름만 포함한 성공 응답
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contextual_orchestrator/__main__.py`:
- Around line 91-94: Define or correctly import _json_object before
parser.add_argument references it, so normal CLI startup and --help/--serve do
not raise NameError. Implement the parser to accept only JSON objects and
validate that each price value is a valid non-negative numeric value, raising
the argument parser’s standard validation error for invalid input.
In `@contextual_orchestrator/admin.py`:
- Around line 1598-1615: Update saveCredential’s POST request to use the
supported browser-admin authentication mechanism for /admin/api/credentials,
such as a validated server session or an explicitly configured proxy that
injects the admin Bearer token upstream. Ensure the request is authenticated in
deployments requiring admin authorization, and do not store or expose the admin
token in JavaScript.
In `@docs/kv-credentials.md`:
- Around line 121-125: Update the credentials-registration example around the
curl command so the secret value is read from standard input or a protected file
instead of appearing in the command-line -d argument. Keep the request payload
and registered credential behavior unchanged while ensuring the actual secret is
absent from both the command and shell history.
In `@tests/test_security_hardening.py`:
- Around line 100-119: Resolve Ruff S106 findings in the test setup around the
SecurityConfig and post_json calls by replacing repeated token literals with
test-only constants, and apply narrowly scoped noqa suppression only where
literals must remain. Follow the repository’s established test-secret exception
policy without changing the security behavior being tested.
- Around line 98-131: Update
test_admin_credential_endpoint_registers_into_kv_without_echoing_value to retain
the InMemoryCredentialBackend instance passed to set_backend, then after the
admin registration assert that the LITELLM_API_KEY entry contains the submitted
secret value. Keep the existing response and cleanup assertions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36da1999-463f-41e2-a721-a36d111232e9
📒 Files selected for processing (9)
contextual_orchestrator/__main__.pycontextual_orchestrator/admin.pycontextual_orchestrator/orchestrator.pycontextual_orchestrator/server.pydocs/architecture.mddocs/kv-credentials.mdtests/test_admin_contract.pytests/test_paper_contracts.pytests/test_security_hardening.py
Define _json_object for --price-per-million, add nosemgrep on audited SQL/TLS/urllib false positives, mint HttpOnly admin session cookies for browser KV registration, and harden credential docs/tests without embedding secrets in argv.
Remediation on
|
| Finding | Fix |
|---|---|
_json_object NameError |
Implemented CLI JSON object parser with non-negative price validation |
| Admin credential auth | POST /admin/session HttpOnly cookie + apiFetch(..., credentials: same-origin); shell no longer embeds admin token after session establish |
| curl secret in argv | Docs use stdin/jq pipe into --data-binary @- |
| Ruff S106 test secrets | _TEST_* constants with narrow noqa |
| KV assert after register | Backend retained; get_credential asserts stored value |
| Semgrep SQL/TLS/urllib | Audited # nosemgrep on fixed placeholders / intentional TLS opt-out / validated provider URL |
Auto-merge (squash) enabled. Waiting on independent approval + remaining required checks (Strix/Atheris/coverage as applicable).
Pull request was converted to draft
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head59c304f608ef6fd3f9992b6d2e4bd23a2c75a510. -
Head SHA:
59c304f608ef6fd3f9992b6d2e4bd23a2c75a510 -
Workflow run: 31689634573
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (4 files)"]
R3 --> V3["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head59c304f608ef6fd3f9992b6d2e4bd23a2c75a510. -
Head SHA:
59c304f608ef6fd3f9992b6d2e4bd23a2c75a510 -
Workflow run: 31694844993
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (4 files)"]
R3 --> V3["targeted test run"]
1 participant
Status: bounded feature slice — Draft until the #96 security baseline integrates
This branch contains useful price-aware routing and browser-admin/KV work, but it remains incomplete and must not merge from its current protected-main base.
Exact identity and current evidence
main@6841b71935e0b7cb98fb52bcb4709cc5100c8d87149887191cd3ec465cac140ab0e91f90e55084aa31590150639: success31590150715: success31590150685: success31590150599: success31590150724: successExact-head workflow success proves the exercised tree only. It does not make suppression-only inherited findings, incomplete browser-session security, or an unintegrated main-based feature release-ready.
Useful implemented slice
--price-per-millionvalidates a JSON model-price map at the argparse boundary and rejects booleans, negatives, NaN, infinities, and overflow-to-infinity values;/admin/api/credentialswrites a validatedUPPER_SNAKE_CASEcredential into the existing KV registry without echoing its value;SameSite=Strictsession id distinct from the raw admin bearer;Remaining blockers
Secure; there is no trusted effective-origin/TLS policy or fail-closed production rule for HTTPS deployments.SameSite=Strict; reverse-proxy and same-site deployment assumptions are not validated.nosemgrepannotations for raw SQL, dynamic urllib, and unverified TLS instead of inheriting PR fix(security): pin provider egress and repair the Atheris lock #96's source-level provider/response security boundary.Required integration order
Keep this PR Draft. After PR #96 reaches protected
main, selectively rebuild or reconcile the unique price/KV/admin work on that exact protected result, remove suppression-only changes, add RED tests for Secure-cookie policy, CSRF/origin, bounded sessions, restart/multi-process semantics, every raw-bearer disclosure sink, and durable session behavior where selected, then regenerate all exact-head quality/security/review evidence. Merge only with zero valid unresolved findings and qualifying independent non-author approval.Refs #116. Does not close #116 until protected integration and operational acceptance.
Summary by CodeRabbit
새로운 기능
보안 개선
문서