feat(gateway): add session auth passthrough, concrete routing, and mo…#4
Open
YasserYG8 wants to merge 1 commit into
Open
feat(gateway): add session auth passthrough, concrete routing, and mo…#4YasserYG8 wants to merge 1 commit into
YasserYG8 wants to merge 1 commit into
Conversation
…dular cli init generator*
📝 WalkthroughWalkthroughThe PR adds direct routing for concrete models, forwards client authentication headers to providers, adds integration coverage, and introduces ChangesConcrete routing and authentication
CLI template initialization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleChat
participant route
participant Provider
participant Upstream
Client->>handleChat: model and authentication headers
handleChat->>route: ChatRequest
route->>Provider: direct concrete-model request
Provider->>Upstream: forwarded model and headers
Upstream-->>Client: completion response
sequenceDiagram
participant User
participant CLI
participant TEMPLATES
participant FileSystem
User->>CLI: modlane init --claude or --agy
CLI->>TEMPLATES: select template
CLI->>FileSystem: write modlane.yaml and missing .env
FileSystem-->>User: success or error status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 3
🧹 Nitpick comments (3)
src/auth.test.ts (1)
8-10: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider awaiting
server.close()inafterEach.
close()returns a Promise that isn't awaited, so servers may not be fully torn down before the next test starts. While port 0 avoids port conflicts, this can leak file descriptors across a larger suite.♻️ Proposed refactor
-afterEach(() => { - while (servers.length) servers.pop()!.close(); +afterEach(async () => { + while (servers.length) await servers.pop()!.close(); });🤖 Prompt for 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. In `@src/auth.test.ts` around lines 8 - 10, Update the afterEach cleanup for the servers collection to await each server.close() Promise before the next test begins, while preserving cleanup of every server instance.src/cli.ts (2)
47-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
if (!template)check is unreachable.
flagis found viaargv.find((arg) => arg in TEMPLATES), guaranteeing it's a valid key.TEMPLATES[flag]will always be defined, making this branch dead code.♻️ Proposed fix
const template = TEMPLATES[flag]; - if (!template) { - console.error("Error: Template not found."); - return 1; - } try {🤖 Prompt for 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. In `@src/cli.ts` around lines 47 - 51, Remove the unreachable `if (!template)` validation and its error-return branch from the template lookup flow, retaining the `TEMPLATES[flag]` assignment and subsequent processing unchanged.
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive supported flags from
TEMPLATESinstead of hardcoding.The error message hardcodes
--claude, --agy, which will go stale if templates are added or removed. Derive fromObject.keys(TEMPLATES).♻️ Proposed fix
- console.error("Error: Please specify the target agent. Supported: --claude, --agy"); + console.error(`Error: Please specify the target agent. Supported: ${Object.keys(TEMPLATES).join(", ")}`);🤖 Prompt for 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. In `@src/cli.ts` around lines 37 - 39, Update the missing-target error handling in the CLI flow to derive the supported flag list from Object.keys(TEMPLATES) rather than hardcoding --claude and --agy. Format the derived keys consistently with the existing flag message while preserving the current return behavior.
🤖 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 `@src/auth.test.ts`:
- Around line 68-87: Clarify the test scope around the “Header Passthrough
forwards client authorization and x-api-key headers” case: either rename it to
cover only authorization forwarding and remove the misleading x-api-key request
header, or add a separate Anthropic-provider test that asserts
interceptedHeaders["x-api-key"] is forwarded. Do not imply x-api-key support in
the OpenAI-compatible test without an explicit assertion.
In `@src/cli.ts`:
- Around line 52-67: Update the initialization write sequence in the try block
so .env is written before modlane.yaml, preventing modlane.yaml from being left
behind if .env creation fails. Preserve the existing .env existence check and
success messages, and keep the catch handling unchanged unless needed for this
ordering.
In `@src/router.ts`:
- Around line 27-38: Update resolveProviderForModel to throw a clear
gateway-level error when no provider matches the model’s expected kind, instead
of returning the first configured provider or an empty string. Preserve the
existing provider-selection logic for matching Anthropic, OpenAI, and
OpenAI-compatible providers.
---
Nitpick comments:
In `@src/auth.test.ts`:
- Around line 8-10: Update the afterEach cleanup for the servers collection to
await each server.close() Promise before the next test begins, while preserving
cleanup of every server instance.
In `@src/cli.ts`:
- Around line 47-51: Remove the unreachable `if (!template)` validation and its
error-return branch from the template lookup flow, retaining the
`TEMPLATES[flag]` assignment and subsequent processing unchanged.
- Around line 37-39: Update the missing-target error handling in the CLI flow to
derive the supported flag list from Object.keys(TEMPLATES) rather than
hardcoding --claude and --agy. Format the derived keys consistently with the
existing flag message while preserving the current return behavior.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 57b42e99-7ed3-4096-b3e0-9b06395fe2d5
📒 Files selected for processing (8)
src/auth.test.tssrc/cli.tssrc/providers/anthropic.tssrc/providers/openai.tssrc/providers/types.tssrc/router.tssrc/server.tssrc/templates.ts
Comment on lines
+68
to
+87
| test("Header Passthrough forwards client authorization and x-api-key headers", async () => { | ||
| let interceptedHeaders: Record<string, string> = {}; | ||
| const providerUrl = await mockProviderWithHeaders((headers) => { | ||
| interceptedHeaders = headers; | ||
| }); | ||
| const base = await gatewayWith(providerUrl); | ||
|
|
||
| const res = await fetch(`${base}/v1/chat/completions`, { | ||
| method: "POST", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| "authorization": "Bearer my-custom-session-token-123", | ||
| "x-api-key": "my-custom-api-key-456" | ||
| }, | ||
| body: JSON.stringify({ model: "my-custom-model-123", messages: [{ role: "user", content: "hi" }] }), | ||
| }); | ||
|
|
||
| expect(res.status).toBe(200); | ||
| expect(interceptedHeaders.authorization).toBe("Bearer my-custom-session-token-123"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test name claims x-api-key passthrough but only asserts authorization.
The test sends both authorization and x-api-key headers, yet only verifies interceptedHeaders.authorization. Since the OpenAI-compatible adapter doesn't forward x-api-key, this gives false confidence that x-api-key passthrough is covered. Consider either renaming the test to reflect only authorization forwarding, or adding a separate test with an Anthropic provider to verify x-api-key passthrough.
💚 Proposed fix: clarify test scope and add x-api-key assertion
-test("Header Passthrough forwards client authorization and x-api-key headers", async () => {
+test("Header Passthrough forwards client authorization header through OpenAI adapter", async () => {
let interceptedHeaders: Record<string, string> = {};
const providerUrl = await mockProviderWithHeaders((headers) => {
interceptedHeaders = headers;
});
const base = await gatewayWith(providerUrl);
const res = await fetch(`${base}/v1/chat/completions`, {
method: "POST",
headers: {
"content-type": "application/json",
"authorization": "Bearer my-custom-session-token-123",
"x-api-key": "my-custom-api-key-456"
},
body: JSON.stringify({ model: "my-custom-model-123", messages: [{ role: "user", content: "hi" }] }),
});
expect(res.status).toBe(200);
expect(interceptedHeaders.authorization).toBe("Bearer my-custom-session-token-123");
+ // x-api-key is not forwarded by the OpenAI-compatible adapter
+ expect(interceptedHeaders["x-api-key"]).toBeUndefined();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test("Header Passthrough forwards client authorization and x-api-key headers", async () => { | |
| let interceptedHeaders: Record<string, string> = {}; | |
| const providerUrl = await mockProviderWithHeaders((headers) => { | |
| interceptedHeaders = headers; | |
| }); | |
| const base = await gatewayWith(providerUrl); | |
| const res = await fetch(`${base}/v1/chat/completions`, { | |
| method: "POST", | |
| headers: { | |
| "content-type": "application/json", | |
| "authorization": "Bearer my-custom-session-token-123", | |
| "x-api-key": "my-custom-api-key-456" | |
| }, | |
| body: JSON.stringify({ model: "my-custom-model-123", messages: [{ role: "user", content: "hi" }] }), | |
| }); | |
| expect(res.status).toBe(200); | |
| expect(interceptedHeaders.authorization).toBe("Bearer my-custom-session-token-123"); | |
| }); | |
| test("Header Passthrough forwards client authorization header through OpenAI adapter", async () => { | |
| let interceptedHeaders: Record<string, string> = {}; | |
| const providerUrl = await mockProviderWithHeaders((headers) => { | |
| interceptedHeaders = headers; | |
| }); | |
| const base = await gatewayWith(providerUrl); | |
| const res = await fetch(`${base}/v1/chat/completions`, { | |
| method: "POST", | |
| headers: { | |
| "content-type": "application/json", | |
| "authorization": "Bearer my-custom-session-token-123", | |
| "x-api-key": "my-custom-api-key-456" | |
| }, | |
| body: JSON.stringify({ model: "my-custom-model-123", messages: [{ role: "user", content: "hi" }] }), | |
| }); | |
| expect(res.status).toBe(200); | |
| expect(interceptedHeaders.authorization).toBe("Bearer my-custom-session-token-123"); | |
| // x-api-key is not forwarded by the OpenAI-compatible adapter | |
| expect(interceptedHeaders["x-api-key"]).toBeUndefined(); | |
| }); |
🤖 Prompt for 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.
In `@src/auth.test.ts` around lines 68 - 87, Clarify the test scope around the
“Header Passthrough forwards client authorization and x-api-key headers” case:
either rename it to cover only authorization forwarding and remove the
misleading x-api-key request header, or add a separate Anthropic-provider test
that asserts interceptedHeaders["x-api-key"] is forwarded. Do not imply
x-api-key support in the OpenAI-compatible test without an explicit assertion.
Comment on lines
+52
to
+67
| try { | ||
| writeFileSync("modlane.yaml", template.yaml); | ||
| console.log("Created modlane.yaml"); | ||
| if (!existsSync(".env")) { | ||
| writeFileSync(".env", template.env); | ||
| console.log("Created .env"); | ||
| } else { | ||
| console.log(".env already exists, skipping creation"); | ||
| } | ||
| console.log(`\nSuccess! Modlane has been initialized for ${flag.replace("--", "")}.`); | ||
| console.log("To run Modlane, execute: npx modlane start"); | ||
| return 0; | ||
| } catch (err) { | ||
| console.error("Error: Failed to write configuration files:", err); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Partial write leaves user in a stuck state on .env failure.
If writeFileSync("modlane.yaml") succeeds but writeFileSync(".env") throws, modlane.yaml exists and re-running init fails with "already exists". The user must manually delete the file and re-run. Consider writing .env first, or cleaning up modlane.yaml in the catch block.
🛡️ Proposed fix: write .env first, then modlane.yaml
try {
- writeFileSync("modlane.yaml", template.yaml);
- console.log("Created modlane.yaml");
if (!existsSync(".env")) {
writeFileSync(".env", template.env);
console.log("Created .env");
} else {
console.log(".env already exists, skipping creation");
}
+ writeFileSync("modlane.yaml", template.yaml);
+ console.log("Created modlane.yaml");
console.log(`\nSuccess! Modlane has been initialized for ${flag.replace("--", "")}.`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| writeFileSync("modlane.yaml", template.yaml); | |
| console.log("Created modlane.yaml"); | |
| if (!existsSync(".env")) { | |
| writeFileSync(".env", template.env); | |
| console.log("Created .env"); | |
| } else { | |
| console.log(".env already exists, skipping creation"); | |
| } | |
| console.log(`\nSuccess! Modlane has been initialized for ${flag.replace("--", "")}.`); | |
| console.log("To run Modlane, execute: npx modlane start"); | |
| return 0; | |
| } catch (err) { | |
| console.error("Error: Failed to write configuration files:", err); | |
| return 1; | |
| } | |
| try { | |
| if (!existsSync(".env")) { | |
| writeFileSync(".env", template.env); | |
| console.log("Created .env"); | |
| } else { | |
| console.log(".env already exists, skipping creation"); | |
| } | |
| writeFileSync("modlane.yaml", template.yaml); | |
| console.log("Created modlane.yaml"); | |
| console.log(`\nSuccess! Modlane has been initialized for ${flag.replace("--", "")}.`); | |
| console.log("To run Modlane, execute: npx modlane start"); | |
| return 0; | |
| } catch (err) { | |
| console.error("Error: Failed to write configuration files:", err); | |
| return 1; | |
| } |
🤖 Prompt for 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.
In `@src/cli.ts` around lines 52 - 67, Update the initialization write sequence in
the try block so .env is written before modlane.yaml, preventing modlane.yaml
from being left behind if .env creation fails. Preserve the existing .env
existence check and success messages, and keep the catch handling unchanged
unless needed for this ordering.
Comment on lines
+27
to
+38
| function resolveProviderForModel(config: Config, model: string): string { | ||
| const isAnthropic = model.startsWith("claude"); | ||
| for (const [name, prov] of Object.entries(config.providers)) { | ||
| if (isAnthropic && prov.kind === "anthropic") { | ||
| return name; | ||
| } | ||
| if (!isAnthropic && (prov.kind === "openai" || prov.kind === "openai-compatible")) { | ||
| return name; | ||
| } | ||
| } | ||
| return Object.keys(config.providers)[0] || ""; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback in resolveProviderForModel can route to the wrong provider type.
When no provider of the expected kind is found, the function falls back to the first configured provider regardless of type. This means a Claude model could be routed to an OpenAI-compatible provider (or vice versa), producing a confusing upstream API error instead of a clear gateway-level message.
🔧 Proposed fix: throw on no matching provider
function resolveProviderForModel(config: Config, model: string): string {
const isAnthropic = model.startsWith("claude");
for (const [name, prov] of Object.entries(config.providers)) {
if (isAnthropic && prov.kind === "anthropic") {
return name;
}
if (!isAnthropic && (prov.kind === "openai" || prov.kind === "openai-compatible")) {
return name;
}
}
- return Object.keys(config.providers)[0] || "";
+ throw new Error(
+ `No ${isAnthropic ? "anthropic" : "openai-compatible"} provider configured for model: ${model}`
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function resolveProviderForModel(config: Config, model: string): string { | |
| const isAnthropic = model.startsWith("claude"); | |
| for (const [name, prov] of Object.entries(config.providers)) { | |
| if (isAnthropic && prov.kind === "anthropic") { | |
| return name; | |
| } | |
| if (!isAnthropic && (prov.kind === "openai" || prov.kind === "openai-compatible")) { | |
| return name; | |
| } | |
| } | |
| return Object.keys(config.providers)[0] || ""; | |
| } | |
| function resolveProviderForModel(config: Config, model: string): string { | |
| const isAnthropic = model.startsWith("claude"); | |
| for (const [name, prov] of Object.entries(config.providers)) { | |
| if (isAnthropic && prov.kind === "anthropic") { | |
| return name; | |
| } | |
| if (!isAnthropic && (prov.kind === "openai" || prov.kind === "openai-compatible")) { | |
| return name; | |
| } | |
| } | |
| throw new Error( | |
| `No ${isAnthropic ? "anthropic" : "openai-compatible"} provider configured for model: ${model}` | |
| ); | |
| } |
🤖 Prompt for 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.
In `@src/router.ts` around lines 27 - 38, Update resolveProviderForModel to throw
a clear gateway-level error when no provider matches the model’s expected kind,
instead of returning the first configured provider or an empty string. Preserve
the existing provider-selection logic for matching Anthropic, OpenAI, and
OpenAI-compatible providers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds support for Session Authorization Passthrough and Concrete Model Routing, making Modlane fully compatible with Claude Code's native web browser authentication and model splitting. Additionally, it implements a modular scaffolding generator (
modlane init) supporting--claudeand--agy.What Was Added/Changed
Credentials Forwarding (Session Auth Passthrough):
src/server.ts) to captureauthorizationandx-api-keyheaders from incoming agent requests.AnthropicAdapterandOpenAICompatAdapterto forward these headers directly to the target APIs. This enables Claude Code's browser-authenticated billing (claude auth login) to work transparently through Modlane without requiring local API keys in.env.Concrete Model Routing (Bypassing Tiers):
src/router.ts.route()androuteStream()to detect concrete model requests (e.g.,claude-sonnet-5). These requests bypass the virtual tier mapping and are sent directly to the native provider using their original names.Modular CLI Setup Generator (
modlane init):src/templates.ts.npx modlane init --claude(pre-configured for Claude 5 generation models) andnpx modlane init --agy(optimized for Antigravity via OpenRouter) to automatically generatemodlane.yamland.envfiles.Testing & Verification
src/auth.test.tsto test concrete model routing and credentials forwarding.pnpm run typecheckpasses with zero type warnings.pnpm run buildcompiles cleanly.pnpm testand all 29 tests passed successfully.Summary by CodeRabbit
initcommand for creating starter configuration files from built-in templates.