Skip to content

Conversation

@ESnark
Copy link
Contributor

@ESnark ESnark commented Dec 4, 2025

🐛 Bug-fix PR

Before opening this PR please:

  1. make lint - passes ruff, mypy, pylint
  2. make test - all unit + integration tests green
  3. make coverage - ≥ 90 %
  4. make docker docker-run-ssl or make podman podman-run-ssl
  5. Update relevant documentation.
  6. Tested with sqlite and postgres + redis.
  7. Manual regression no longer fails. Ensure the UI and /version work correctly.

📌 Summary

Root path redirect (//admin/) was ignoring APP_ROOT_PATH setting and causing unnecessary 307 redirect.

🔁 Reproduction Steps

  1. Set APP_ROOT_PATH=/api
  2. Request GET /api
  3. Observe redirect to /admin/ instead of /api/admin/
  4. Also observe 303 → 307 double redirect due to missing trailing slash

🐞 Root Cause

root_redirect function used request.scope.get("root_path", "") which returns empty string, while other parts of the codebase use settings.app_root_path. Also missing trailing slash caused extra 307 redirect.

💡 Fix Description

🧪 Verification

Check Command Status
Lint suite make lint
Unit tests make test
Coverage ≥ 90 % make coverage
Manual regression no longer fails steps / screenshots

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

@ESnark ESnark requested a review from crivetimihai as a code owner December 4, 2025 05:08
@ESnark ESnark force-pushed the remove-unnecessary-redirect branch from 7e20d4f to 7ab5bb7 Compare December 4, 2025 06:46
- Changed from request.scope.get("root_path") to settings.app_root_path
  for consistency with other parts of the codebase
- Added trailing slash (/admin/) to avoid 307 redirect
- Removed unused request parameter from root_redirect function
- Updated tests to verify correct redirect path with app_root_path

Closes IBM#1547

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Copy link
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Created a GitHub issues for other similar consistency issues #1588

@crivetimihai crivetimihai merged commit cdb373f into IBM:main Dec 12, 2025
51 checks passed
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.

2 participants