Skip to content

Honor --debug flag in vmcp#5008

Merged
jhrozek merged 3 commits intomainfrom
fix-vmcp-debug-flag-no-op
Apr 22, 2026
Merged

Honor --debug flag in vmcp#5008
jhrozek merged 3 commits intomainfrom
fix-vmcp-debug-flag-no-op

Conversation

@jhrozek
Copy link
Copy Markdown
Contributor

@jhrozek jhrozek commented Apr 22, 2026

Summary

  • Why: vmcp's --debug CLI flag was effectively a no-op. cmd/vmcp/main.go called viper.GetBool("debug") before app.NewRootCmd().ExecuteContext(ctx) ran, so the flag binding — which happens inside NewRootCmd — had not yet been applied. The logger was always initialized at INFO level, and slog.Debug calls in dependent packages were invisible regardless of whether the flag was passed.
  • What: Move logger initialization into the root command's PersistentPreRunE, which runs after cobra has parsed flags and populated viper. Keep a minimal INFO-level default logger in main() so errors emitted before cobra's Execute still produce structured output.

This matches the pattern already used in cmd/thv/app/commands.go for the main thv CLI — the same bug was fixed there previously.

Type of change

  • Bug fix

Test plan

  • Linting (task lint-fix)
  • Manual verification: built binary, ran vmcp --debug version (DEBUG line present) vs vmcp version (no DEBUG), confirmed the difference.

Changes

File Change
cmd/vmcp/main.go Remove the ineffective pre-flag-parse viper.GetBool("debug") block; install a minimal INFO-level default logger instead.
cmd/vmcp/app/commands.go Add PersistentPreRunE on the root command that reads the parsed --debug flag and reinstalls the logger at the correct level.

Does this introduce a user-facing change?

Yes, in the sense that vmcp --debug now actually enables debug logging. Previously it was silently ignored.

Special notes for reviewers

  • Mirrors the existing fix in cmd/thv/app/commands.go.
  • logging.New is idempotent — calling it twice (once in main, once in PersistentPreRunE) is safe.
  • Verified during the end-to-end test of Fix Cedar upstream-claim evaluation on VirtualMCPServer #5002 (the Cedar Resolved JWT claim keys log only became visible once this fix was in place).

Generated with Claude Code

vmcp's main.go called viper.GetBool("debug") before cobra parsed CLI
flags, so the --debug flag was effectively a no-op — the logger was
always initialized at INFO regardless of the flag. This also meant
slog.Debug calls in dependent packages (e.g. Cedar's
"Resolved JWT claim keys for Cedar evaluation" log at
pkg/authz/authorizers/cedar/core.go:452) were never visible even with
--debug on the command line.

Move logger initialization into the root command's PersistentPreRunE so
it runs after cobra has parsed flags and viper has the parsed value.
Leave a minimal INFO-level default logger installed in main() so any
error emitted before cobra's Execute runs still produces structured
output. Matches the pattern already used in cmd/thv/app/commands.go.
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Apr 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.05%. Comparing base (cffe934) to head (dff85fb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/vmcp/app/commands.go 0.00% 10 Missing ⚠️
cmd/vmcp/main.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5008      +/-   ##
==========================================
+ Coverage   69.02%   69.05%   +0.02%     
==========================================
  Files         554      554              
  Lines       73075    73083       +8     
==========================================
+ Hits        50443    50470      +27     
+ Misses      19620    19603      -17     
+ Partials     3012     3010       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Apr 22, 2026
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Apr 22, 2026
@jhrozek jhrozek merged commit 93e1478 into main Apr 22, 2026
70 of 71 checks passed
@jhrozek jhrozek deleted the fix-vmcp-debug-flag-no-op branch April 22, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants