From 541f35e1385250454f80513672d5e84ff7a279b1 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Wed, 22 Jul 2026 22:32:23 +0500 Subject: [PATCH] fix(integrations): recompute invoke_separator from retained parsed_options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit with_integration_setting recomputed invoke_separator from the raw parsed_options argument. When only script_type changes (parsed_options and raw_options both None), the previously-stored parsed_options are retained on the setting, but the separator was derived from the None argument — dropping an options-dependent separator (e.g. Copilot --skills -> "-") back to the default ".", desynchronizing invoke_separator from the stored options. Derive the separator from current.get("parsed_options") — the options actually stored after the update — so it stays consistent in every branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/integration_runtime.py | 9 ++++++- tests/integrations/test_integration_state.py | 28 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/integration_runtime.py b/src/specify_cli/integration_runtime.py index 9bf8aeac96..98bc6a05b0 100644 --- a/src/specify_cli/integration_runtime.py +++ b/src/specify_cli/integration_runtime.py @@ -64,8 +64,15 @@ def with_integration_setting( elif raw_options is not None: current.pop("parsed_options", None) + # Recompute the separator from the options actually STORED on ``current`` + # after the update, not the raw ``parsed_options`` argument. When only + # ``script_type`` changes (``parsed_options`` and ``raw_options`` both + # None), the previously-stored ``parsed_options`` are retained above, so + # deriving the separator from the argument (None) would drop an + # options-dependent separator (e.g. Copilot ``--skills`` -> "-") back to + # the default ".". current["invoke_separator"] = integration.effective_invoke_separator( - parsed_options, project_root + current.get("parsed_options"), project_root ) settings[key] = current return settings diff --git a/tests/integrations/test_integration_state.py b/tests/integrations/test_integration_state.py index 1d6bdb0268..fc12d436a4 100644 --- a/tests/integrations/test_integration_state.py +++ b/tests/integrations/test_integration_state.py @@ -84,3 +84,31 @@ def test_write_integration_json_strips_integration_key(tmp_path): assert state["integration"] == "claude" assert state["default_integration"] == "claude" assert state["installed_integrations"] == ["claude"] + + +def test_with_integration_setting_recomputes_separator_from_retained_options(): + """Updating only script_type must not drop an options-dependent separator. + + Copilot resolves the command-ref separator to '-' when '--skills' options + are stored and '.' otherwise. A second call that changes only script_type + (parsed_options=None, raw_options=None) retains the stored parsed_options, + so invoke_separator must stay '-', not be recomputed from the None argument. + """ + from specify_cli.integrations import get_integration + from specify_cli.integration_runtime import with_integration_setting + + copilot = get_integration("copilot") + + settings = with_integration_setting( + {}, "copilot", copilot, parsed_options={"skills": True} + ) + assert settings["copilot"]["invoke_separator"] == "-" + + settings2 = with_integration_setting( + {"integration_settings": settings}, "copilot", copilot, script_type="ps" + ) + # parsed_options are retained (only script_type changed) ... + assert settings2["copilot"]["parsed_options"] == {"skills": True} + assert settings2["copilot"]["script"] == "ps" + # ... so the separator must reflect them, not the (None) argument. + assert settings2["copilot"]["invoke_separator"] == "-"