Skip to content

Conversation

@noahensley
Copy link

📌 Summary

Standardizes root path handling and fixes redirect URLs to avoid unnecessary FastAPI 307 redirects.

Fixes #1588

🔁 Changes Made

Part 1: Standardize root_path access

Replaced all occurrences of request.scope.get("root_path", "") with settings.app_root_path for consistency and cleaner code.

  • Files modified: 4 files, 80 total replacements
    • mcpgateway/admin.py (74 occurrences)
    • mcpgateway/utils/verify_credentials.py (3 occurrences)
    • mcpgateway/routers/oauth_router.py (1 occurrence)
    • mcpgateway/routers/sso.py (1 occurrence)

Part 2: Add trailing slashes to redirect URLs

Added trailing slashes to /admin redirects to prevent FastAPI from issuing unnecessary 307 redirects.

  • Files modified: 2 files, 24 total replacements
    • mcpgateway/admin.py (23 occurrences)
    • mcpgateway/routers/sso.py (1 occurrence)
  • Patterns fixed:
    • {root_path}/admin{root_path}/admin/
    • {root_path}/admin#{root_path}/admin/#

🧪 Verification

Check Result
Unit tests ✅ 3608 passed (same as baseline, no new failures)
Ruff linter ✅ All checks passed
Mypy ℹ️ Pre-existing errors in codebase (not introduced by this PR)

✅ Acceptance Criteria Met

  • ✅ All occurrences of request.scope.get("root_path", "") replaced with settings.app_root_path
  • ✅ Redirect URLs use consistent trailing slash pattern
  • ✅ All existing tests pass
  • ✅ No functional changes to behavior

📝 Notes

  • Unable to run full integration/Docker tests locally - relying on CI/CD

@crivetimihai
Copy link
Member

Thank you for the PR. Could you please rebase and address the lint issues?

@crivetimihai crivetimihai added this to the Release 1.0.0-BETA-2 milestone Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Standardize root_path access pattern across codebase

2 participants