fix(broker-router): honor --log-level=4 (warn) instead of defaulting to debug

[PRAteek-singHWY]

What

setupLogger() selected the slog level with a switch that only handled 0
(info), 8 (error), and -4 (debug). The --log-level flag help advertises
4=warn, but 4 had no case and fell through to default, which set
slog.LevelDebug. As a result, requesting warn produced the most verbose log
level instead.

This replaces the switch with a direct conversion from the flag value to
slog.Level, so all slog levels (including warn) are honored:

opts.Level = slog.Level(a.brokerCfg.logLevel)

A unit test is added for the level mapping (setupLogger was previously
untested).

Why

The flag value is already a raw slog.Level (Info=0, Warn=4, Error=8,
Debug=-4), so 4 is valid, documented input. Mapping it to debug means an
operator asking for warn gets the noisiest level instead: more log volume, more
risk of sensitive data at debug, and extra work on the broker/router hot paths.
The level is reachable in normal operation because the controller forwards
BROKER_ROUTER_LOG_LEVEL to --log-level (see docs/release-notes/0.0.8.md).

Changes

• cmd/mcp-broker-router/main.go: map --log-level directly to slog.Level in
  setupLogger().
• cmd/main.go: same fix for the controller binary, which had the identical
  switch.
• cmd/mcp-broker-router/main_test.go: add a test covering info/warn/error/debug
  and an arbitrary value.

Testing

• make test-unit passes.
• gofmt/goimports clean on the changed files.

Notes

Bugfix, exempt from the issue-first requirement per CONTRIBUTING.md. Tracking
issue: #1143. Commit is signed off (git commit -s).

Fixes #1143

Summary by CodeRabbit

• New Features
  • Updated --log-level help text to document the expected numeric values.
• Refactor
  • Simplified and standardized log level handling so configured integer values are applied consistently to the logger.
• Tests
  • Added unit tests validating log level behavior for debug, info, warn, error, and an arbitrary/unexpected value.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Two log-level mappings now cast integer values directly to slog.Level in cmd/main.go and cmd/mcp-broker-router/main.go. cmd/mcp-broker-router/main_test.go adds coverage for standard and arbitrary values.

Changes

Log Level Cast Simplification

Layer / File(s)	Summary
Direct slog.Level cast in both entry points cmd/main.go, cmd/mcp-broker-router/main.go	The flag help text includes warn, and both log-level setups now assign slog.Level(value) directly instead of using switch statements.
Unit test for log-level mapping cmd/mcp-broker-router/main_test.go	TestSetupLoggerLevelMapping checks that setupLogger maps info, warn, error, debug, and an arbitrary integer to the expected slog.Level.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the log-level mapping bug fix and is directly related to the changes.
Linked Issues check	✅ Passed	The PR fixes the warned log-level mapping in both binaries and adds unit coverage, matching issue #1143.
Out of Scope Changes check	✅ Passed	The changes stay focused on logger level mapping and tests, with no clear unrelated additions.

✨ 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.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/main.go (1)

58-59: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update CLI help text to include warn mapping.

The --log-level help string omits 4=warn, while the implementation now accepts raw slog levels including warn. Keep the help text aligned with runtime behavior.

Suggested change

-flag.IntVar(&loglevel, "log-level", int(slog.LevelInfo), "log level: 0=info, 8=error, -4=debug")
+flag.IntVar(&loglevel, "log-level", int(slog.LevelInfo), "log level: 0=info, 4=warn, 8=error, -4=debug")

🤖 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 `@cmd/main.go` around lines 58 - 59, The help text for the --log-level flag in
the flag.IntVar call for loglevel is missing the warn level mapping. Update the
help string to include "4=warn" in the sequence of level mappings so it reads
"log level: 0=info, 4=warn, 8=error, -4=debug" to accurately reflect all
supported log levels that the implementation accepts.

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

Outside diff comments:
In `@cmd/main.go`:
- Around line 58-59: The help text for the --log-level flag in the flag.IntVar
call for loglevel is missing the warn level mapping. Update the help string to
include "4=warn" in the sequence of level mappings so it reads "log level:
0=info, 4=warn, 8=error, -4=debug" to accurately reflect all supported log
levels that the implementation accepts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72165569-b996-47f9-b3bf-9d4def8968c3

📥 Commits

Reviewing files that changed from the base of the PR and between e99c0a6 and a9b1831.

📒 Files selected for processing (3)

• cmd/main.go
• cmd/mcp-broker-router/main.go
• cmd/mcp-broker-router/main_test.go

[PRAteek-singHWY]

Hello @jasonmadigan @david-martin , could you please have a look whenever you get a chance?
Thanks.

[github-actions]

Thanks for the contribution, @PRAteek-singHWY! You currently have other non-draft PR(s) open:

• fix(broker): stop forwarding Cookie/Proxy-Authorization to user-specific upstreams #1172

To help us review and merge changes as efficiently as possible, we ask contributors to focus on one PR at a time. Activity on this project can be high, and maintainers have other priorities outside the project, so having a single active PR helps everyone get changes landed faster.

This PR has been moved to draft. Once your other PR(s) are merged or closed, mark this one as ready for review and we will take a look.

Note

This is an experimental process and may change or need manual intervention while we trial it.

[PRAteek-singHWY]

Hi @david-martin , this is now my only active PR (#1172 is merged) and I've rebased it on latest main. Please review whenever you're free.
Thank you.

[Aman-Cool]

@PRAteek-singHWY, Nice fix👍 and I like that you went with the raw slog.Level(loglevel) cast rather than just bolting on a case 4. That switch was a footgun: anything it didn't recognise.., including the advertised 4=warn; fell through to debug, so asking for warn actually handed you the noisiest level. The cast handles the whole level space cleanly.

Both binaries line up now, and the broker-router's behaviour finally matches the 4=warn its help text already promised. Test covers the levels nicely too.

[Aman-Cool]

Quick note on the red e2e check... looks unrelated to this change. It's [Happy] should use and re-use a backend MCP session failing on a session re-init 404 plus a k8s reconcile conflict (StorageError … UID in precondition; the HTTPRoute got recreated mid-reconcile). That's a timing flake in the e2e harness, not something a slog.Level mapping could affect; the other 33 specs passed. A re-run should clear it.

[PRAteek-singHWY]

Quick note on the red e2e check... looks unrelated to this change. It's [Happy] should use and re-use a backend MCP session failing on a session re-init 404 plus a k8s reconcile conflict (StorageError … UID in precondition; the HTTPRoute got recreated mid-reconcile). That's a timing flake in the e2e harness, not something a slog.Level mapping could affect; the other 33 specs passed. A re-run should clear it.

Thank you for reviewing @Aman-Cool sir, i'll re-run it ASAp.

[PRAteek-singHWY]

Re-ran the e2e suite and all checks are green now - the [Happy] should use and re-use a backend MCP session failure was the timing flake @Aman-Cool described and cleared on re-run. Nothing changed in the branch (still at 9ed18e7).

@david-martin @Aman-Cool this is ready for review whenever you have a chance. Thanks.

[Aman-Cool]

@PRAteek-singHWY, The controller help-text follow-up closes the one loose end too ; the broker-router already advertised 4=warn but the controller's --log-level text didn't, so now both binaries agree end to end.., help text, the slog.Level(loglevel) cast, and behaviour all line up. The little table test pinning info/warn/error/debug (plus an arbitrary level) locks the mapping in.

LGTM from me 👍 the fix is correct and about as minimal as it gets ; slog.Level(loglevel) is the right call over re-listing cases. I can't merge it myself, but it looks ready whenever the team have a moment.