Repository navigation
test(gateways): assert OpenGateway route default model and credentials once - #2330
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title is concise, scoped to gateway tests, and matches the diff that adds OpenGateway default model and credential assertions. |
| Description check | |
The description explains the change, rationale, affected files, assertions, and test results. It omits the template's separate Impact and Notes sections and does not include the local preflight checkb… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Risk Surface Disclosed | |
The PR adds only catalog-test assertions. The changed test names the provider route, default model, and credential environment variables. The PR description identifies the auth and routing scope and s… |
| No Hidden Policy Change | |
PASS. The pull request changes only src/integrations/gateways/gitlawb-opengateway.test.ts. The added lines assert existing defaultModel and setup.credentialEnvVars values from the unchanged gate… |
Full details: Description check
-
Explanation
The description explains the change, rationale, affected files, assertions, and test results. It omits the template's separate Impact and Notes sections and does not include the local preflight checkbox, but the required change and validation details are complete.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
Full details: Risk Surface Disclosed
-
Explanation
The PR adds only catalog-test assertions. The changed test names the provider route, default model, and credential environment variables. The PR description identifies the auth and routing scope and states that no source changes or routing/auth behavior changes are included. This supports a no-blocker assessment.
Full details: No Hidden Policy Change
-
Explanation
PASS. The pull request changes only
src/integrations/gateways/gitlawb-opengateway.test.ts. The added lines assert existingdefaultModelandsetup.credentialEnvVarsvalues from the unchanged gateway definition. No product, trust-model, routing, telemetry/network, or permission-policy behavior changes are present in the reviewed diff.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
— jatmn's PR Review agent
PR: Twigpine/openclaude#2330 — test(gateways): assert OpenGateway route default model and credentials once
Head: 1be430eb24fe4a907248b8cc8b7eaf7885c9de47
Range: 3a299281bacfb14bcf73a8c7d871477fbe2c6f49..1be430eb24fe4a907248b8cc8b7eaf7885c9de47 (one commit, one file, +8 lines)
Summary
This is the intended surgical follow-up to the gateway-contract work from #2324: shared OpenGateway route defaultModel and credential env vars are asserted once in gitlawb-opengateway.test.ts instead of being duplicated in each hosted-model test. No scope creep — production gitlawb-opengateway.ts is unchanged.
Value check (concrete)
| Claim | Verdict |
|---|---|
defaultModel: 'mimo-v2.5-pro' matches gateway source |
Yes — defineGateway({ defaultModel: 'mimo-v2.5-pro', … preset.fallbackModel: 'mimo-v2.5-pro' }) in src/integrations/gateways/gitlawb-opengateway.ts. |
setup.credentialEnvVars ordered triple matches gateway source |
Yes — ['OPENGATEWAY_API_KEY', 'OPENAI_API_KEYS', 'OPENAI_API_KEY'] in the gateway setup block (same order as validation.credentialEnvVars). |
| Right home vs #2324 CodeRabbit route-default prompts | Yes — expectGatewayCatalog('gitlawb-opengateway', …) in the gateway’s own contract test is the shared route surface; Ling Tiny, Macaron, and Tencent tests correctly keep descriptor coverage (capabilities, limits, catalog linkage, availableUntil for Ling) without repeating route-level defaults. |
| Assertions test the contract (not a tautology) | Yes — expectGatewayCatalog resolves the live registry via getGateway(routeId) and checks catalog invariants (including that some catalog entry’s apiName equals gateway.defaultModel). Expected values are independent literals; drift in either direction fails the test. |
Automated review / drift audit (#2324 threads)
CodeRabbit on #2324 asked Ling Tiny, Macaron, and Tencent tests to assert shared OpenGateway route defaults/credentials. Those were drift relative to the consolidation goal — resolved with gateway follow-up, not per-file duplication:
| Thread | File | Disposition |
|---|---|---|
PRRT_kwDOR2fj6M6vBv8L |
src/integrations/ling-tiny.test.ts |
Drift — descriptor contract already complete; route defaults belong in gateway test (this PR). |
PRRT_kwDOR2fj6M6vBv8S |
src/integrations/macaron.test.ts |
Drift — same rationale. |
PRRT_kwDOR2fj6M6vBv8a |
src/integrations/tencent.test.ts |
Drift — same rationale. |
On #2330, CodeRabbit is still processing (no inline actionable findings on the diff yet). CodeQL aggregate check is neutral (expected for test-only changes); open repo alerts on main are unchanged and not introduced by this diff.
Optional hardening (not a merge blocker): xiaomi-mimo-token.test.ts asserts validation.credentialEnvVars separately when it differs from setup. OpenGateway keeps identical lists in both blocks today; asserting validation too would be consistency-only, not required to close the #2324 follow-up.
Merge readiness
- Mergeability: GitHub reports
mergeable: true,mergeable_state: blocked(typical while required checks are in flight). - CI at head
1be430eb(snapshot):- Success:
typecheck,smoke-and-tests,dedicated-tests,changes,pr-intent-scan,launcher-node-floor,launcher-compatibility (22)/(24.11.x),unit-tests (2–8),CodeQL (actions). - In progress:
unit-tests (1),Analyze (javascript-typescript),CodeQL (javascript-typescript). - Neutral: top-level
CodeQL. - Skipped:
web(unchanged web surface).
- Success:
- Wait for the remaining unit-test shard and CodeQL JS/TS jobs before treating CI as fully green.
- Duplicate work: None; complements merged #2324 consolidation without re-touching hosted-model files.
Validation (reviewer)
bun test --isolateongitlawb-opengateway.test.ts,ling-tiny.test.ts,macaron.test.ts,tencent.test.ts: 8 pass, 0 fail (afterbun install --frozen-lockfilein review checkout).
🟢 Thanks for the contribution. I do not see any actionable issues from my review.
Summary
Phase 0 item 8 (gateway contract follow-up). The shared
gitlawb-opengatewayroute's default model and credential env vars are now asserted once, in the gateway's own contract test, instead of in each hosted-model test (Ling Tiny, Macaron, Tencent).Changes
src/integrations/gateways/gitlawb-opengateway.test.ts: the existingexpectGatewayCatalogcontract case now also checks:defaultModel: 'mimo-v2.5-pro'setup.credentialEnvVars: ['OPENGATEWAY_API_KEY', 'OPENAI_API_KEYS', 'OPENAI_API_KEY'](full, ordered array)Values come from
src/integrations/gateways/gitlawb-opengateway.ts.No source, helper, or docs changes. The CONTRIBUTING catalog-test guidance already covers default model and credential env vars. The Ling Tiny, Macaron and Tencent tests are unchanged.
Coverage
Testing
bun test --isolate src/integrations/gateways/gitlawb-opengateway.test.ts src/integrations/ling-tiny.test.ts src/integrations/macaron.test.ts src/integrations/tencent.test.ts: 8 pass, 0 failbun run integrations:check: up to datebun run typecheck: passbun run check: passSummary by CodeRabbit