[mikemikimike]

Summary

• Return HTTP 422 when Admin UI resource and prompt forms omit required fields.
• Return HTTP 422 for missing gateway URL and missing A2A name or endpoint fields.
• Preserve partial updates for federated resources when only editable fields are supplied.
• Reject missing or blank prompt templates with a validation warning.
• Add unit and end-to-end regressions for the validation and update paths.

Tests

• uv run --extra plugins pytest -q tests/e2e/test_admin_apis.py tests/unit/mcpgateway/test_admin.py
• Focused Windows validation: 11 regression tests and the targeted admin API end-to-end cases passed.
• The remaining six test_admin.py failures reproduce on the base branch and are Windows path/encoding behavior, not failures in these changes.

Fixes #6937

Issue: #6937

[msureshkumar88]

Thank you for the fix. The admin_edit_prompt guard is correct: a missing template returned 500 on main and now returns 422. The e2e tests for it pass.

Two problems block the merge. The PR also leaves most of #6937 open.

blocking | mcpgateway/admin.py:13696 | The new `name` guard in admin_edit_resource breaks federated-resource edits. The UI sends no name for gateway-backed resources (admin_ui/resources.js:685 disables the field; admin_ui/formSubmitHandlers.js:1159 documents it). Main returns 200 for this request. The PR returns 422. | Remove the hunk. `ResourceUpdate.name` is Optional.
blocking | tests/unit/mcpgateway/test_admin.py (TestAdminResourceUriConflictMessage) | Three existing tests fail on this branch with `assert 422 == 200`: `test_admin_edit_resource_explicit_base[None|Weekly Report|gateway-report]`. They pass on main. | Fix the cause above. Run the full `tests/unit/mcpgateway/test_admin.py` before pushing.
blocking | mcpgateway/admin.py:13584-13589, 13986-13989 | admin_add_resource and admin_add_prompt still return 500 on a missing `name`, `uri`, `content` or `template`. The issue title and repro steps name the add handlers. The PR says "Fixes #6937". | Replace `str(form["x"])` with `form.get("x")`. Do not wrap it in `str()`, because `str(None)` gives "None". Change the `display_name` fallback `or form["name"]` too. `ResourceCreate` and `PromptCreate` require these fields, so the existing `ValidationError` branch returns 422.
functionally-impacting | tests/e2e/test_admin_apis.py:~613 | An existing e2e test asserts 500 for a missing field on the add path. It fails after the add fix. | Change the expected status to 422.
functionally-impacting | tests/unit/mcpgateway/test_admin.py | No unit test covers the new branches. Only the e2e tests reach them, so `make diff-cover` is at risk. | Add unit tests for the add and edit_prompt missing-field cases.
functionally-impacting | mcpgateway/admin.py:~13307, ~16450 | admin_edit_gateway (`url`) and admin_add_a2a_agent (`name`, `endpoint_url`) still return 500 on a missing field. | Fix them here, or file a follow-up and change "Fixes #6937" to "Refs #6937".
suggestion | mcpgateway/admin.py:14093 | `"template" not in form` accepts `template=""`. | Decide between a 422 and partial-update semantics. `PromptUpdate.template` is Optional.
suggestion | tests/e2e/test_admin_apis.py:584,728 | The tests use the nonexistent id `missing-id` and pass only because the guard runs before any lookup. | Use a real id so the test proves the guard order.
minor | mcpgateway/admin.py | The new early returns log nothing. | Add a `LOGGER.warning` that names the missing field.
minor | PR | The merge commit and the `test_tls_compose.py` entry in the PR body are unrelated to the change. | Rebase and drop the entry.

Verified on this branch. No Alembic, UI, or docs change is needed. The smallest path is: revert the resource hunk, change the bracket accesses in the two add handlers, and flip the e2e assertion to 422.

[mikemikimike]

Thanks for the detailed review. I addressed the findings in the updated PR:

• Removed the admin_edit_resource name guard so federated resource edits can omit the optional name and retain partial-update behavior.
• Changed the resource and prompt create handlers to pass omitted required values through to Pydantic validation, including the prompt display-name fallback. The existing e2e expectation now checks for HTTP 422.
• Added unit regressions for missing resource/prompt fields, gateway URL, and A2A name/endpoint. Prompt edits now reject missing, empty, and whitespace-only templates and log a warning.
• Rebased the PR onto the current main as a linear commit series and removed the unrelated TLS test command from the PR description.

The focused route regressions pass, and tests/e2e/test_admin_apis.py passes with one environment-dependent skip. I also ran the full tests/unit/mcpgateway/test_admin.py module. Its six failures are platform-specific: Windows rejects the CR/LF symlink filename used by one test, and the host GBK encoding cannot decode five UTF-8 template files. The same six failures reproduce on a clean main checkout.

[msureshkumar88]

Thank you for the thorough rework. All findings from the previous review are resolved:

• The admin_edit_resource name guard is removed, and the federated-edit tests pass again.
• admin_add_resource, admin_add_prompt, admin_edit_gateway, and admin_add_a2a_agent now return 422 on a missing required field. They pass form.get() to Pydantic, so str(None) no longer leaks through.
• admin_edit_prompt rejects a missing, empty, or whitespace-only template and logs a warning.
• Unit tests cover the new branches, and the e2e test uses a real prompt id.

I verified on 9124068b1: tests/unit/mcpgateway/test_admin.py and tests/e2e/test_admin_apis.py pass (one connectivity skip).

Two items before merge:

functionally-impacting | .secrets.baseline | The "Run pre-commit hooks" check fails. The new tests move the audited secret in tests/unit/mcpgateway/test_admin.py from line 7324 to 7401. | Run `make detect-secrets-scan` and commit `.secrets.baseline`.
minor | mcpgateway/admin.py:14094 | admin_edit_prompt rejects a blank template, but admin_add_prompt accepts one because PromptCreate allows "". | Align the add path here or in a follow-up.

The pytest (py3.12) failures in test_a2a_jsonrpc_passthrough.py do not come from this PR. main fixes them in #7331, so a rebase clears them.

[mikemikimike]

Thanks for checking the updated PR. I also compared the full test results against the PR base (526ada23) on Windows with the same test settings. Of the 156 failing/erroring test nodes on the PR head, 143 also failed on the base. These include platform-specific failures such as missing os.fchmod, signal.SIGHUP, and time.tzset, and Windows rejecting a CR/LF in a symlink filename; those failures are not introduced by this PR.

The remaining 13 nodes passed when rerun separately on both the head and base, so their full-suite failures were not reproducible in those reruns. The full runs did not meet the coverage threshold (19% on the head and 81% on the base), and the difference remains unexplained. The focused review regressions passed (11 tests), and the admin API e2e suite passed (41 tests, one connectivity skip).

[gandhipratik203]

LGTM!

[Lang-Akshay]

LGTM