test(api-gateway): unit tests for OIDC auth-config + CLI (crate ~0%→71%)

[SharedQA]

Unit-test coverage for the api-gateway crate, the workspace's biggest unit gap (was 2 tests, only proxy.rs partial).

Result (measured, cargo-llvm-cov)

File	Before	After (lines)
auth_info.rs	0%	70.5%
main.rs (CLI)	0%	64.7%
proxy.rs	~73%	73%
core_types.rs	0%	0% (runtime-only)
crate TOTAL	~near-0	70.7% lines (regions 73.0%, fns 74.0%)

27 tests pass; cargo clippy --all-targets --all-features clean (no unwrap/expect in tests, per the workspace lint).

What's covered

• OIDC auth-config transform — extracted AuthInfoResponse::from_config (scopes split on whitespace, response_type pinned to code, field passthrough); config default + deny_unknown_fields; JSON wire-shape roundtrip.
• CLI parsing — default→run, all subcommands, -v/-vv count, --config/-c, --print-config, unknown-subcommand rejection, clap self-check.

Not unit-tested

main()→run_server/run_migrate, register_rest, and core_types::init need the live modkit runtime (ModuleCtx, client hub) — that's integration scope (tracked separately), not unit. This PR covers the crate's pure/testable surface.

Opened as draft for review before it's marked ready.

Summary by CodeRabbit

Release Notes

• Tests
  • Added unit coverage for authentication config generation (including scope parsing and a consistent response type), CLI flag/subcommand parsing, and proxy routing/forwarding behavior (upstream failures mapped to 503, correct prefix/query preservation, and hop-by-hop header handling).
• Chores
  • Updated development dependencies for the API gateway service.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dd53cbff-6b87-4271-975a-04e624273ae2

📥 Commits

Reviewing files that changed from the base of the PR and between 20a5f5b and da1c3b9.

📒 Files selected for processing (1)

• src/backend/services/api-gateway/src/proxy.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/backend/services/api-gateway/src/proxy.rs

---

📝 Walkthrough

Walkthrough

This PR expands api-gateway test coverage for auth config responses, CLI argument parsing, and proxy forwarding behavior, while extracting shared auth response construction logic and updating test dependencies for JSON-based assertions.

Changes

API gateway coverage expansion

Layer / File(s)	Summary
Auth config response construction src/backend/services/api-gateway/Cargo.toml, src/backend/services/api-gateway/src/auth_info.rs	AuthInfoResponse::from_config centralizes response assembly from config, the auth config route uses it, and tests cover scope parsing, fixed response type, config serde rules, and JSON round-tripping. Test dependency updated from tower to serde_json.
CLI parsing test coverage src/backend/services/api-gateway/src/main.rs	Clap-based tests verify default command handling, run/check/migrate parsing, repeated verbosity flags, config-related flags, and rejection of unknown arguments.
Proxy behavior and test support src/backend/services/api-gateway/src/proxy.rs	Test helpers spin up an ephemeral upstream and build proxy state, while tests verify config defaults, hop-by-hop header filtering, unreachable upstream 503 behavior, and forwarding of stripped paths, queries, bodies, and eligible headers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• constructorfabric/insight#1434 — This PR adds the api-gateway auth and proxy test coverage described in the issue, including auth response tests and proxy forwarding/error-path tests.

Suggested reviewers

• cyberantonz

Poem

🐇 I twitched my nose through routes and flags,
and nibbled scopes from config bags.
Through proxy burrows, headers sped,
while hop-by-hop were left for dead.
A tidy test patch, crisp and bright—
moonlit code and carrots tonight.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding unit tests for OIDC auth-config and CLI to the api-gateway crate, with a specific coverage improvement metric (0%→71%).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[SharedQA]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cyberantonz]

Please wait for a while, the project is migrating to gears version, so there will be some changes.

[SharedQA]

Overlaps #1314 (migrate to gears) on auth_info.rs/main.rs/proxy.rs. These tests assert the pre-gears APP__modules__/from_config shape; once #1314 lands, rebase to assert the APP__gears__ config instead. Tracking so we reconcile rather than conflict.

[cyberantonz]

Some issues

[coderabbitai]

🧹 Nitpick comments (1)

src/backend/services/api-gateway/src/proxy.rs (1)

466-469: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert hop-by-hop header removal explicitly.

Line 467 currently verifies authorization forwarding, but this test doesn’t prove connection was dropped. Add a second assertion for connection absence so the test matches its name and intent.

Proposed test tightening

-        let upstream = Router::new().route(
-            "/v1/h",
-            get(|headers: axum::http::HeaderMap| async move {
-                let auth = headers.contains_key("authorization");
-                (StatusCode::OK, format!("auth={auth}"))
-            }),
-        );
+        let upstream = Router::new().route(
+            "/v1/h",
+            get(|headers: axum::http::HeaderMap| async move {
+                let auth = headers.contains_key("authorization");
+                let conn = headers.contains_key("connection");
+                (StatusCode::OK, format!("auth={auth};conn={conn}"))
+            }),
+        );
@@
-        assert_eq!(
-            &body[..],
-            b"auth=true",
-            "authorization forwarded end-to-end"
-        );
+        assert_eq!(&body[..], b"auth=true;conn=false");

Also applies to: 474-485

🤖 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/backend/services/api-gateway/src/proxy.rs` around lines 466 - 469, The
test handler in the anonymous route handler starting at line 466 currently only
checks for the presence of the "authorization" header but does not verify that
the "connection" hop-by-hop header was removed as intended. Modify the handler
to also check for the absence of the "connection" header and include both checks
in the response format (e.g., "auth={auth},connection={has_connection}"). This
will ensure the test explicitly verifies both that authorization headers are
forwarded and that hop-by-hop headers like connection are properly dropped,
matching the test's intended purpose. Apply the same fix to the other similar
test cases mentioned in the comment range.

🤖 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.

Nitpick comments:
In `@src/backend/services/api-gateway/src/proxy.rs`:
- Around line 466-469: The test handler in the anonymous route handler starting
at line 466 currently only checks for the presence of the "authorization" header
but does not verify that the "connection" hop-by-hop header was removed as
intended. Modify the handler to also check for the absence of the "connection"
header and include both checks in the response format (e.g.,
"auth={auth},connection={has_connection}"). This will ensure the test explicitly
verifies both that authorization headers are forwarded and that hop-by-hop
headers like connection are properly dropped, matching the test's intended
purpose. Apply the same fix to the other similar test cases mentioned in the
comment range.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f7793e8-538b-4f5d-8405-b83b6b334f15

📥 Commits

Reviewing files that changed from the base of the PR and between 667931a and 20a5f5b.

⛔ Files ignored due to path filters (1)

• src/backend/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• src/backend/services/api-gateway/Cargo.toml
• src/backend/services/api-gateway/src/auth_info.rs
• src/backend/services/api-gateway/src/main.rs
• src/backend/services/api-gateway/src/proxy.rs

[ktursunov]

OIDC and API Gateway are reworked in scope of #1583