From 3aa3580bcc4e16cdfd8cb189683cb3d30e0d5a07 Mon Sep 17 00:00:00 2001 From: xuyuanhao Date: Sat, 4 Apr 2026 15:26:27 +0800 Subject: [PATCH 1/2] fix: suppress internal config keys from apm config get output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apm config get (no argument) iterated the raw config dict and passed any key it did not recognise to a bare else branch, printing it as-is. This caused the internal default_client key to appear in the output even though users cannot set it via apm config set — leading to a confusing get/set asymmetry reported in #564. Remove the else fallthrough so that only user-settable keys (currently auto-integrate) are printed. Internal keys are silently skipped. Update the test that asserted the old passthrough behaviour and add an explicit assertion that default_client is absent from the output. Fixes #564 --- src/apm_cli/commands/config.py | 5 ++--- tests/unit/test_config_command.py | 10 ++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/apm_cli/commands/config.py b/src/apm_cli/commands/config.py index 7794ce205..69945540e 100644 --- a/src/apm_cli/commands/config.py +++ b/src/apm_cli/commands/config.py @@ -166,8 +166,7 @@ def get(key): config_data = get_config() logger.progress("APM Configuration:") for k, v in config_data.items(): - # Map internal keys to user-friendly names + # Only expose user-settable keys; internal keys (e.g. default_client) + # are silently skipped to avoid confusing users who cannot set them. if k == "auto_integrate": click.echo(f" auto-integrate: {v}") - else: - click.echo(f" {k}: {v}") diff --git a/tests/unit/test_config_command.py b/tests/unit/test_config_command.py index d9944e766..7b4501b82 100644 --- a/tests/unit/test_config_command.py +++ b/tests/unit/test_config_command.py @@ -230,14 +230,16 @@ def test_get_all_config(self): result = self.runner.invoke(config, ["get"]) assert result.exit_code == 0 assert "auto-integrate: True" in result.output + # Internal keys must not appear — users cannot set them via apm config set + assert "default_client" not in result.output - def test_get_all_config_unknown_key_passthrough(self): - """Unknown config keys are shown as-is.""" - fake_config = {"some_other_key": "value"} + def test_get_all_config_internal_keys_suppressed(self): + """Internal config keys are not shown to users.""" + fake_config = {"default_client": "vscode"} with patch("apm_cli.config.get_config", return_value=fake_config): result = self.runner.invoke(config, ["get"]) assert result.exit_code == 0 - assert "some_other_key: value" in result.output + assert "default_client" not in result.output class TestAutoIntegrateFunctions: From 9a958ad78eeb6be0d2f56739ebee683ec7963b70 Mon Sep 17 00:00:00 2001 From: xuyuanhao Date: Sat, 4 Apr 2026 18:52:54 +0800 Subject: [PATCH 2/2] fix: use get_auto_integrate() instead of raw config iteration Address Copilot review feedback: - Use get_auto_integrate() to always show auto-integrate with its effective value (including default), fixing the case where a fresh install shows no user-settable keys at all. - Replace em dash with ASCII hyphen in test comments to comply with the repo ASCII-only rule for source files. - Remove unused get_config import. --- src/apm_cli/commands/config.py | 13 +++++-------- tests/unit/test_config_command.py | 14 ++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/apm_cli/commands/config.py b/src/apm_cli/commands/config.py index 69945540e..9108f2fea 100644 --- a/src/apm_cli/commands/config.py +++ b/src/apm_cli/commands/config.py @@ -147,7 +147,7 @@ def get(key): apm config get auto-integrate apm config get """ - from ..config import get_config, get_auto_integrate + from ..config import get_auto_integrate logger = CommandLogger("config get") if key: @@ -162,11 +162,8 @@ def get(key): ) sys.exit(1) else: - # Show all config - config_data = get_config() + # Show all user-settable keys with their effective values (including + # defaults). Iterating raw config keys would hide settings that + # have not been written yet (e.g. auto_integrate on a fresh install). logger.progress("APM Configuration:") - for k, v in config_data.items(): - # Only expose user-settable keys; internal keys (e.g. default_client) - # are silently skipped to avoid confusing users who cannot set them. - if k == "auto_integrate": - click.echo(f" auto-integrate: {v}") + click.echo(f" auto-integrate: {get_auto_integrate()}") diff --git a/tests/unit/test_config_command.py b/tests/unit/test_config_command.py index 7b4501b82..6fff74106 100644 --- a/tests/unit/test_config_command.py +++ b/tests/unit/test_config_command.py @@ -225,21 +225,19 @@ def test_get_unknown_key(self): def test_get_all_config(self): """Show all config when no key is provided.""" - fake_config = {"auto_integrate": True, "default_client": "vscode"} - with patch("apm_cli.config.get_config", return_value=fake_config): + with patch("apm_cli.config.get_auto_integrate", return_value=True): result = self.runner.invoke(config, ["get"]) assert result.exit_code == 0 assert "auto-integrate: True" in result.output - # Internal keys must not appear — users cannot set them via apm config set + # Internal keys must not appear - users cannot set them via apm config set assert "default_client" not in result.output - def test_get_all_config_internal_keys_suppressed(self): - """Internal config keys are not shown to users.""" - fake_config = {"default_client": "vscode"} - with patch("apm_cli.config.get_config", return_value=fake_config): + def test_get_all_config_fresh_install(self): + """auto-integrate is shown even on a fresh install with no key in the file.""" + with patch("apm_cli.config.get_auto_integrate", return_value=True): result = self.runner.invoke(config, ["get"]) assert result.exit_code == 0 - assert "default_client" not in result.output + assert "auto-integrate: True" in result.output class TestAutoIntegrateFunctions: