[2.8] Clarify remove_client token cleanup semantics

[YuanTingHsieh]

What changed

• Hide the legacy interactive-console remove_client command from normal help.
• Clarify docs and API/server docstrings that remove_client only releases the active client token and allows the client to register again.
• Remove the command from the user-facing admin operation table.
• Add/adjust focused tests for the hidden command metadata and legacy Session API routing.

Why

remove_client sounds like it removes or disables a client, but the implementation only clears the current active token. During code freeze this keeps behavior/API stable while reducing user-facing confusion.

Validation

• /private/tmp/nvflare-deploy-test-venv/bin/python -m pytest tests/unit_test/private/fed/server/training_cmds_test.py tests/unit_test/fuel/flare_api/session_new_methods_test.py tests/unit_test/tool/system/system_status_test.py::TestSystemStatus::test_system_parser_rejects_unsupported_remove_client_command -q
• python3 -m compileall -q nvflare/private/fed/server/training_cmds.py nvflare/fuel/flare_api/api_spec.py nvflare/fuel/flare_api/flare_api.py nvflare/private/fed/server/client_manager.py nvflare/private/fed/server/server_engine.py nvflare/private/fed/server/server_engine_internal_spec.py tests/unit_test/private/fed/server/training_cmds_test.py tests/unit_test/fuel/flare_api/session_new_methods_test.py
• git diff upstream/2.8 --check
• GitHub reports the PGP-signed commit as verified.

[greptile-apps]

Greptile Summary

This PR clarifies the semantics of the legacy remove_client command across docs, API specs, and server internals — hiding it from the interactive-console help menu (visible=False) while keeping it functional for backward compatibility, and updating all docstrings and documentation to accurately describe it as a token-release operation rather than a full client removal.

• training_cmds.py: visible=False hides remove_client from normal help output; description updated to state it releases the active token so the client can re-register.
• api_spec.py / flare_api.py / client_manager.py / server_engine.py / server_engine_internal_spec.py: Docstrings corrected to say "release active token" instead of "remove client"; a Note block added recommending disable_client when the intent is to prevent reconnect.
• Docs (nvflare_cli.md, migration_guide.rst, migrating_to_flare_api.rst, operation.rst): remove_client removed from the user-facing operations table; migration guide corrected to show the command does have a FLARE API counterpart (Session.remove_client) with a clarifying note; prose in design and migration docs updated to match.
• Tests: New assertion in training_cmds_test.py verifies visible=False and the updated description keywords; existing session_new_methods_test.py test renamed to reflect the compatibility intent.

Confidence Score: 5/5

Safe to merge — all changes are docstring/description updates, a visibility flag flip, and documentation rewrites with no logic modifications.

No behavioral code was changed. The only runtime-observable difference is that remove_client no longer appears in the interactive help listing (visible=False); it remains fully callable and its implementation is untouched. All docstring and documentation edits are accurate and internally consistent.

No files require special attention.

Important Files Changed

Filename	Overview
nvflare/private/fed/server/training_cmds.py	Sets remove_client CommandSpec to visible=False and updates description to "release a client's active token"; no behavioral change, command still routable.
docs/programming_guide/migrating_to_flare_api.rst	Corrects migration table: remove_client now maps to remove_client (was empty/"not exposed") with a clarifying note; accurate since Session.remove_client() does exist.
nvflare/fuel/flare_api/api_spec.py	Docstring-only update on abstract remove_client; adds a Note block explaining the token-release-only semantics and recommending disable_client.
nvflare/fuel/flare_api/flare_api.py	Matching docstring update on concrete Session.remove_client; no logic changes.
nvflare/private/fed/server/server_engine_internal_spec.py	Docstring update for remove_clients abstract method: "Remove specified clients" → "Remove active client-token entries"; arg description clarified.
tests/unit_test/private/fed/server/training_cmds_test.py	Adds test_remove_client_is_hidden_and_described_as_token_cleanup asserting visible=False, correct usage string, and updated description keywords.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Admin console: help] -->|visible=False| B[remove_client NOT shown in help]
    A -->|visible=True| C[disable_client shown in help]

    D[Admin calls remove_client explicitly] --> E[TrainingCommandModule.remove_client]
    E --> F[ServerEngine.remove_clients]
    F --> G[ClientManager.remove_client\nclears active token entry]
    G --> H[Client may re-register]

    I[Admin calls disable_client] --> J[TrainingCommandModule.disable_client]
    J --> K[ServerEngine.disable_clients\npersists disabled flag]
    K --> L[Client reconnect / heartbeat rejected\nuntil enable_client]

    style B fill:#ffe0b2
    style H fill:#c8e6c9
    style L fill:#ffcdd2

Loading

_{Reviews (2): Last reviewed commit: "Merge branch '2.8' into codex/clarify-re..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR reduces user-facing confusion around the legacy interactive-console remove_client operation by making its behavior explicit: it only clears a client’s active token entry (allowing re-registration), and it is hidden from normal interactive help while remaining available for compatibility.

Changes:

• Hide interactive-console remove_client from normal command help (visible=False) and update its description to reflect token cleanup semantics.
• Clarify token-cleanup semantics across server/internal API docstrings and user/developer documentation (and remove remove_client from the user-facing admin operation table).
• Add/adjust unit tests to assert hidden-command metadata and preserve legacy Session routing behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/unit_test/private/fed/server/training_cmds_test.py	Adds a spec-level test asserting remove_client is hidden and described as active-token cleanup.
tests/unit_test/fuel/flare_api/session_new_methods_test.py	Renames the test to emphasize legacy compatibility while still verifying the same command routing.
nvflare/private/fed/server/training_cmds.py	Updates remove_client CommandSpec description and hides it from normal help; clarifies handler comment.
nvflare/private/fed/server/server_engine.py	Adds a clarifying docstring that remove_clients removes active token entries to allow re-registration.
nvflare/private/fed/server/server_engine_internal_spec.py	Refines the abstract method docstring to describe removal of active client-token entries.
nvflare/private/fed/server/client_manager.py	Clarifies remove_client docstring semantics as removing an active token entry.
nvflare/fuel/flare_api/flare_api.py	Updates Session.remove_client docstring to describe token release and directs users to disable_client for durable blocking.
nvflare/fuel/flare_api/api_spec.py	Mirrors the clarified remove_client semantics in the public SessionSpec docstring.
docs/user_guide/admin_guide/deployment/operation.rst	Removes remove_client from the user-facing admin command table.
docs/programming_guide/migrating_to_flare_api.rst	Updates migration table notes to explicitly describe remove_client as active-token release only.
docs/migration_guide.rst	Clarifies that remove-client is not a supported public CLI and that remove_client is hidden + token-cleanup only.
docs/design/nvflare_cli.md	Updates design docs to describe remove_client as hidden token cleanup and clarifies Session API wording.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.