Skip to content

fix: remove deprecated PUT /admin/users endpoint and add sunset marker pre-commit hook - #6304

Open
prakhar-singh1928 wants to merge 3 commits into
mainfrom
bug/sunset-markers
Open

fix: remove deprecated PUT /admin/users endpoint and add sunset marker pre-commit hook#6304
prakhar-singh1928 wants to merge 3 commits into
mainfrom
bug/sunset-markers

Conversation

@prakhar-singh1928

@prakhar-singh1928 prakhar-singh1928 commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

🔗 Related Issue

Closes #6270
Closes #6298


📝 Summary

The deprecated PUT /admin/users/{user_email} endpoint in mcpgateway/routers/email_auth.py passed its sunset date, causing test_admin_get_update_delete_user to fail in CI due to a time-bomb assert datetime.now(...) < datetime(2026, ...) inside the test suite.

This PR does two things:

  1. Removes the deprecated endpoint. update_user_deprecated and its update_user_delegate helper are deleted. The delegate body was already inlined into update_user (PATCH) as documented in the delegate's own docstring. The Response import that was only needed by the deprecated endpoint is also removed. Associated [#2754]-guarded test blocks are removed from both test_email_auth_router.py and scripts/test_email_auth_api.py.

  2. Replaces the broken enforcement pattern. The time-bomb assert inside a pytest test is inherently fragile — it will always eventually fail in CI regardless of code correctness. It is replaced by a new check-sunset-markers pre-commit hook (.github/tools/check_sunset_markers.py) that scans *.py files at commit time for [#NNNN] remove after / [#NNNN] Code to be removed after markers and fails immediately, preventing future sunset time-bombs from entering the codebase.


📏 Reviewability

  • This PR has one clear purpose
  • The linked issue is not labeled triage
  • Unrelated bugs or improvements are tracked in separate issues/PRs
  • Tests are included with the code they validate
  • If AI-assisted, I understand and can explain the generated changes

🏷️ Type of Change

  • Bug fix
  • Chore (deps, CI, tooling)

🧪 Verification

Check Command Status
Sunset marker hook runs clean uv run .github/tools/check_sunset_markers.py ✅ exits 0
Pre-commit hooks pass make pre-commit
Lint suite make lint
Unit tests (previously failing test now passes) make test
Coverage ≥ 80% make coverage

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • Tests added/updated for changes
  • Documentation updated (if applicable) — no user-facing docs reference the deprecated PUT endpoint
  • No secrets or credentials committed

📓 Notes

Why a pre-commit hook instead of a test assertion?
A pytest assertion like assert datetime.now() < sunset_date guarantees a future CI failure — it has nothing to do with code correctness and will break the suite even on a green codebase. A pre-commit hook fires at the right moment (before the marker ever lands in main) and never produces false failures after the fact.

Scope of the hook: scans all *.py files under the repo root, skipping .-prefixed dirs, node_modules, __pycache__, build, and dist. Pattern is case-insensitive and anchored to the [#NNNN] issue reference format used in this codebase.

…r pre-commit hook

- Remove deprecated PUT /admin/users/{user_email} endpoint and its
  update_user_deprecated function from mcpgateway/routers/email_auth.py
- Inline update_user_delegate body back into update_user (PATCH) as
  documented in the delegate's docstring
- Remove Response import no longer needed after endpoint removal
- Remove [#2754]-guarded test blocks from test_email_auth_router.py
  including the time-bomb sunset assertion that was blocking the suite
- Remove [#2754]-guarded PUT test block from scripts/test_email_auth_api.py
- Add .github/tools/check_sunset_markers.py pre-commit hook that scans
  for [#NNNN] remove/Code to be removed after ... markers and fails fast
  at commit time, preventing future time-bombs from entering the codebase
- Register check-sunset-markers hook in .pre-commit-config.yaml

Closes #6270

Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
@prakhar-singh1928 prakhar-singh1928 changed the title fix: remove deprecated PUT /admin/users endpoint and add sunset marke… fix: remove deprecated PUT /admin/users endpoint and add sunset marker pre-commit hook Aug 19, 2026
Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
Lang-Akshay
Lang-Akshay previously approved these changes Aug 21, 2026

@Lang-Akshay Lang-Akshay left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM ✅

@Lang-Akshay
Lang-Akshay added this pull request to the merge queue Aug 21, 2026
@Lang-Akshay
Lang-Akshay removed this pull request from the merge queue due to a manual request Aug 21, 2026

@madhu-mohan-jaishankar madhu-mohan-jaishankar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

don't think we need a pre-commit check just to check sunset markers. Also, the comments may differ tomorrow if somebody writes like "deprecate/code no needed", etc.

Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>

@madhu-mohan-jaishankar madhu-mohan-jaishankar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants