From 25a4126f0c36e24983938573383d0d41144f52f7 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 12:33:18 +0300 Subject: [PATCH 01/13] feat(api): add Fabric skill attribution Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../unreleased/added-20260830-122126.yaml | 6 ++ docs/commands/index.md | 1 + docs/essentials/parameters.md | 9 ++- src/fabric_cli/client/fab_api_client.py | 12 +++- src/fabric_cli/core/fab_constant.py | 5 +- src/fabric_cli/core/fab_context.py | 18 +++++ src/fabric_cli/core/fab_decorators.py | 9 ++- src/fabric_cli/errors/common.py | 11 ++- src/fabric_cli/parsers/fab_global_params.py | 14 +++- tests/conftest.py | 1 + tests/test_core/test_fab_api_client.py | 67 +++++++++++++++++++ tests/test_core/test_fab_context.py | 21 ++++++ tests/test_core/test_fab_decorators.py | 57 ++++++++++++++++ tests/test_parsers/test_fab_global_params.py | 28 ++++++++ 14 files changed, 252 insertions(+), 7 deletions(-) create mode 100644 .changes/unreleased/added-20260830-122126.yaml create mode 100644 tests/test_core/test_fab_decorators.py diff --git a/.changes/unreleased/added-20260830-122126.yaml b/.changes/unreleased/added-20260830-122126.yaml new file mode 100644 index 000000000..4947ad5c0 --- /dev/null +++ b/.changes/unreleased/added-20260830-122126.yaml @@ -0,0 +1,6 @@ +kind: added +body: Add Fabric skill attribution to Fabric API requests through the global ``--skill`` option or ``FABRIC_SKILL`` environment variable +time: 2026-08-30T12:21:26.157507+03:00 +custom: + Author: shirasassoon + AuthorLink: https://github.com/shirasassoon diff --git a/docs/commands/index.md b/docs/commands/index.md index fa1e87fde..a647f72c8 100644 --- a/docs/commands/index.md +++ b/docs/commands/index.md @@ -100,6 +100,7 @@ The following parameters are available for all commands: - `-h, --help`: Display help information for the command - `--output_format`: Specify the output format (`text` or `json`). +- `--skill`: Attribute Fabric API requests to a Fabric skill. ## Common Parameters diff --git a/docs/essentials/parameters.md b/docs/essentials/parameters.md index 180d68a61..cbb4600d9 100644 --- a/docs/essentials/parameters.md +++ b/docs/essentials/parameters.md @@ -63,9 +63,16 @@ For a complete list of commands, see the [Commands page](../commands/index.md). | `--retain_n_hours` | Retain specified hours for `table vacuum` | | `--schedule` | Set the job schedule for `job` | | `--show_headers` | Turn headers on for `api` | +| `--skill` | Attribute Fabric API requests to a Fabric skill | | `--start` | Start date in UTC for `job` | | `--target` | Specify the target for `ln` | | `--timeout` | Specify the timeout of the command in seconds for `job` | | `--type` | Specify the type for `ln` or `job` | | `--vorder` | Apply v-order for `table optimize` | -| `--zorder` | Apply Z-order indexing for `table optimize` | \ No newline at end of file +| `--zorder` | Apply Z-order indexing for `table optimize` | + +`--skill` adds the `x-ms-fabric-skill` header to Fabric control-plane +requests made by the command, including internal requests and long-running +operation polling. Set `FABRIC_SKILL` to apply the attribution through an +environment variable; an explicit `--skill` value takes precedence. The +header is not added to OneLake, Azure, or Power BI requests. diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index 99df81c0c..6f5c007db 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -113,7 +113,11 @@ def do_request( # Build headers from fabric_cli.core.fab_context import Context as FabContext - ctxt_cmd = FabContext().command + fab_context = FabContext() + ctxt_cmd = fab_context.command + fabric_skill = fab_context.fabric_skill + if not isinstance(fabric_skill, str): + fabric_skill = None headers = { "Authorization": "Bearer " + str(token), @@ -132,6 +136,12 @@ def do_request( fab_constant.ERROR_INVALID_OPERATION, ) + if audience_value not in ("storage", "azure", "powerbi") and fabric_skill: + for header_name in list(headers): + if header_name.lower() == fab_constant.FABRIC_SKILL_HEADER: + del headers[header_name] + headers[fab_constant.FABRIC_SKILL_HEADER] = fabric_skill + try: session = _get_session() retries_count = 3 diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index 531aa091f..4febfe8ca 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -19,8 +19,7 @@ ) API_ENDPOINT_POWER_BI = ( - validate_and_get_env_variable( - "FAB_API_ENDPOINT_POWER_BI", "api.powerbi.com") + validate_and_get_env_variable("FAB_API_ENDPOINT_POWER_BI", "api.powerbi.com") + "/v1.0/myorg" ) @@ -68,6 +67,8 @@ FAB_HOST_APP_ENV_VAR = "FAB_HOST_APP" FAB_HOST_APP_VERSION_ENV_VAR = "FAB_HOST_APP_VERSION" +FABRIC_SKILL_ENV_VAR = "FABRIC_SKILL" +FABRIC_SKILL_HEADER = "x-ms-fabric-skill" # Other constants FAB_CAPACITY_NAME_NONE = "none" diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index ed18b7c93..cc23075cd 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -5,7 +5,9 @@ import json import os import platform +import re import sys +from typing import Optional import psutil @@ -23,6 +25,7 @@ class Context: def __init__(self): self._context: FabricElement = None self._command: str = None + self._fabric_skill: Optional[str] = None self._runtime_mode: str = fab_constant.FAB_MODE_COMMANDLINE session_id = self._get_context_session_id() self._context_file = os.path.join( @@ -72,6 +75,21 @@ def command(self) -> str: def command(self, command: str) -> None: self._command = command + @property + def fabric_skill(self) -> Optional[str]: + return self._fabric_skill + + @fabric_skill.setter + def fabric_skill(self, fabric_skill: Optional[str]) -> None: + if fabric_skill is not None and not re.fullmatch( + r"[A-Za-z0-9][A-Za-z0-9._-]{0,127}", fabric_skill + ): + raise FabricCLIError( + ErrorMessages.Common.invalid_fabric_skill_name(), + fab_constant.ERROR_INVALID_INPUT, + ) + self._fabric_skill = fabric_skill + def reset_context(self) -> None: self.cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) self.context = self.context.tenant diff --git a/src/fabric_cli/core/fab_decorators.py b/src/fabric_cli/core/fab_decorators.py index fb245957f..3d4ae636d 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import os from functools import wraps import fabric_cli.core.fab_logger as fab_logger @@ -8,6 +9,7 @@ ERROR_UNAUTHORIZED, EXIT_CODE_AUTHORIZATION_REQUIRED, EXIT_CODE_ERROR, + FABRIC_SKILL_ENV_VAR, ) from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.utils import fab_ui @@ -21,7 +23,7 @@ def getinstance(*args, **kwargs): if class_ not in instances: instances[class_] = class_(*args, **kwargs) return instances[class_] - + return getinstance @@ -64,7 +66,12 @@ def decorator(func): def wrapper(*args, **kwargs): # Import Context locally to avoid circular import from fabric_cli.core.fab_context import Context + Context().command = args[0].command_path + skill = getattr(args[0], "skill", None) + Context().fabric_skill = ( + skill if skill is not None else os.environ.get(FABRIC_SKILL_ENV_VAR) + ) return func(*args, **kwargs) return wrapper diff --git a/src/fabric_cli/errors/common.py b/src/fabric_cli/errors/common.py index 6fcfc0009..96d37da56 100644 --- a/src/fabric_cli/errors/common.py +++ b/src/fabric_cli/errors/common.py @@ -99,6 +99,13 @@ def operation_cancelled(error: str) -> str: def invalid_headers_format() -> str: return "The headers format is invalid" + @staticmethod + def invalid_fabric_skill_name() -> str: + return ( + "The Fabric skill name must start with a letter or number and contain " + "only letters, numbers, periods, underscores, or hyphens (maximum 128 characters)" + ) + @staticmethod def invalid_json_content(content: str, error: Optional[str]) -> str: base_msg = f"The JSON content is invalid: {content}" @@ -252,7 +259,9 @@ def query_not_supported_for_set(query: str) -> str: @staticmethod def invalid_definition_format(valid_formats: list[str]) -> str: if valid_formats: - message = f"Only the following formats are supported: {', '.join(valid_formats)}" + message = ( + f"Only the following formats are supported: {', '.join(valid_formats)}" + ) else: message = "No formats are supported" return f"Invalid format. {message}" diff --git a/src/fabric_cli/parsers/fab_global_params.py b/src/fabric_cli/parsers/fab_global_params.py index edb72bd87..1e63d21d3 100644 --- a/src/fabric_cli/parsers/fab_global_params.py +++ b/src/fabric_cli/parsers/fab_global_params.py @@ -1,6 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import argparse + def add_global_flags(parser) -> None: """ @@ -11,7 +13,7 @@ def add_global_flags(parser) -> None: """ # Add help flag parser.add_argument("-help", action="help") - + # Add format flag to override output format parser.add_argument( "--output_format", @@ -19,3 +21,13 @@ def add_global_flags(parser) -> None: choices=["json", "text"], help="Override output format type. Optional", ) + + parser.add_argument( + "--skill", + required=False, + default=argparse.SUPPRESS, + help=( + "Attribute Fabric API requests to a Fabric skill. " + "Overrides the FABRIC_SKILL environment variable. Optional" + ), + ) diff --git a/tests/conftest.py b/tests/conftest.py index ceb3b8f28..4545838db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -85,6 +85,7 @@ def reset_context(): context_instance = Context() context_instance._context = None context_instance._command = None + context_instance._fabric_skill = None context_instance._loading_context = False context_instance._runtime_mode = fab_constant.FAB_MODE_COMMANDLINE diff --git a/tests/test_core/test_fab_api_client.py b/tests/test_core/test_fab_api_client.py index 488a8b0f3..f01d266bd 100644 --- a/tests/test_core/test_fab_api_client.py +++ b/tests/test_core/test_fab_api_client.py @@ -14,6 +14,7 @@ ) from fabric_cli.core import fab_constant from fabric_cli.core.fab_auth import FabAuth +from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_exceptions import FabricAPIError @@ -310,6 +311,72 @@ def __init__(self): assert "ErrorCode" == excinfo.value.status_code +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +@pytest.mark.parametrize( + "audience, expects_skill_header", + [ + (None, True), + ("fabric", True), + ("storage", False), + ("azure", False), + ("powerbi", False), + ], +) +def test_do_request_fabric_skill_header_scoped_to_fabric_api( + mock_get_token, audience, expects_skill_header, reset_context +): + class DummyResponse: + status_code = 200 + text = "{}" + content = b"{}" + headers = {} + + reset_context.fabric_skill = "semantic-model-authoring" + dummy_args = Namespace(uri="items", method="get", audience=audience) + + with patch( + "requests.Session.request", return_value=DummyResponse() + ) as mock_request: + do_request(dummy_args) + + request_headers = mock_request.call_args.kwargs["headers"] + if expects_skill_header: + assert ( + request_headers[fab_constant.FABRIC_SKILL_HEADER] + == "semantic-model-authoring" + ) + else: + assert fab_constant.FABRIC_SKILL_HEADER not in request_headers + + +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +def test_do_request_fabric_skill_overrides_custom_header(mock_get_token, reset_context): + class DummyResponse: + status_code = 200 + text = "{}" + content = b"{}" + headers = {} + + reset_context.fabric_skill = "semantic-model-authoring" + dummy_args = Namespace( + uri="items", + method="get", + audience=None, + headers={"X-MS-FABRIC-SKILL": "other-skill"}, + ) + + with patch( + "requests.Session.request", return_value=DummyResponse() + ) as mock_request: + do_request(dummy_args) + + assert ( + mock_request.call_args.kwargs["headers"][fab_constant.FABRIC_SKILL_HEADER] + == "semantic-model-authoring" + ) + assert "X-MS-FABRIC-SKILL" not in mock_request.call_args.kwargs["headers"] + + @pytest.mark.parametrize( "host_app_env, host_app_version_env, expected_suffix", [ diff --git a/tests/test_core/test_fab_context.py b/tests/test_core/test_fab_context.py index 129f1158d..c8776174b 100644 --- a/tests/test_core/test_fab_context.py +++ b/tests/test_core/test_fab_context.py @@ -55,6 +55,24 @@ def test_context_workspace(): Context().reset_context() +def test_fabric_skill_valid_name_success(): + Context().fabric_skill = "semantic-model-authoring" + + assert Context().fabric_skill == "semantic-model-authoring" + + +@pytest.mark.parametrize( + "skill_name", + ["", "-invalid", "invalid skill", "invalid\nheader", "a" * 129], +) +def test_fabric_skill_invalid_name_failure(skill_name): + with pytest.raises(FabricCLIError) as excinfo: + Context().fabric_skill = skill_name + + assert excinfo.value.message == ErrorMessages.Common.invalid_fabric_skill_name() + assert excinfo.value.status_code == fab_constant.ERROR_INVALID_INPUT + + def test_context_virtual_workspace(): _tenant = hierarchy.Tenant(name="tenant_name", id="0000") _workspace = hierarchy.VirtualWorkspace(name=".capacities", id=None, parent=_tenant) @@ -440,6 +458,7 @@ def mock_get_command_context(): # region Runtime Mode + class TestRuntimeMode: """Verify Context.set_runtime_mode / get_runtime_mode behaviour after mode-setting removal.""" @@ -472,7 +491,9 @@ def test_runtime_mode_not_in_config_defaults_success(self): def test_runtime_mode_not_module_level_success(self): """Runtime mode must live on Context, not as module-level functions.""" from fabric_cli.core import fab_context as ctx_module + assert not hasattr(ctx_module, "set_runtime_mode") assert not hasattr(ctx_module, "get_runtime_mode") + # endregion diff --git a/tests/test_core/test_fab_decorators.py b/tests/test_core/test_fab_decorators.py new file mode 100644 index 000000000..8c08bc01a --- /dev/null +++ b/tests/test_core/test_fab_decorators.py @@ -0,0 +1,57 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +from argparse import Namespace + +import pytest + +from fabric_cli.core import fab_constant +from fabric_cli.core.fab_context import Context +from fabric_cli.core.fab_decorators import set_command_context +from fabric_cli.core.fab_exceptions import FabricCLIError + +pytestmark = pytest.mark.usefixtures("reset_context") + + +def test_set_command_context_uses_skill_argument_over_environment(monkeypatch): + monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") + + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill == "argument-skill" + + command(Namespace(command_path="export", skill="argument-skill")) + + +def test_set_command_context_uses_skill_environment_variable(monkeypatch): + monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") + + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill == "environment-skill" + + command(Namespace(command_path="export", skill=None)) + + +def test_set_command_context_clears_previous_skill(monkeypatch): + Context().fabric_skill = "previous-skill" + monkeypatch.delenv(fab_constant.FABRIC_SKILL_ENV_VAR, raising=False) + + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill is None + + command(Namespace(command_path="export", skill=None)) + + +def test_set_command_context_rejects_empty_argument_instead_of_using_environment( + monkeypatch, +): + monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") + + @set_command_context() + def command(args: Namespace) -> None: + pytest.fail("Command should not run with an invalid skill name") + + with pytest.raises(FabricCLIError): + command(Namespace(command_path="export", skill="")) diff --git a/tests/test_parsers/test_fab_global_params.py b/tests/test_parsers/test_fab_global_params.py index 1886ee52b..cfeeb8dfb 100644 --- a/tests/test_parsers/test_fab_global_params.py +++ b/tests/test_parsers/test_fab_global_params.py @@ -8,6 +8,7 @@ import pytest from fabric_cli.parsers import fab_global_params +from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers def test_add_global_flags(): @@ -36,6 +37,11 @@ def test_add_global_flags(): assert not format_flag.required assert "Override output format type" in format_flag.help + skill_flag = next(a for a in all_flags if "--skill" in a.option_strings) + assert skill_flag.dest == "skill" + assert not skill_flag.required + assert "FABRIC_SKILL" in skill_flag.help + def test_add_global_flags_parser_integration(): """Test that global flags work correctly in parser.""" @@ -54,6 +60,28 @@ def test_add_global_flags_parser_integration(): args = parser.parse_args(["--output_format", "text"]) assert args.output_format == "text" + args = parser.parse_args(["--skill", "semantic-model-authoring"]) + assert args.skill == "semantic-model-authoring" + # Test invalid output format (should raise SystemExit) with pytest.raises(SystemExit): parser.parse_args(["--output_format", "invalid"]) + + +@pytest.mark.parametrize( + "command", + [ + ["--skill", "semantic-model-authoring", "export", "ws.Workspace", "-o", "out"], + ["export", "--skill", "semantic-model-authoring", "ws.Workspace", "-o", "out"], + ["export", "ws.Workspace", "-o", "out", "--skill", "semantic-model-authoring"], + ["--skill", "semantic-model-authoring", "job", "run", "list", "ws.Notebook"], + ["job", "--skill", "semantic-model-authoring", "run", "list", "ws.Notebook"], + ["job", "run", "list", "ws.Notebook", "--skill", "semantic-model-authoring"], + ], +) +def test_skill_flag_preserved_across_command_tree(command): + parser, _ = create_parser_and_subparsers() + + args = parser.parse_args(command) + + assert args.skill == "semantic-model-authoring" From a2ecdf052732437b513757e7a2715ea8bb3efb5b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 12:45:42 +0300 Subject: [PATCH 02/13] chore: skip changelog for skill attribution Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changes/unreleased/added-20260830-122126.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 .changes/unreleased/added-20260830-122126.yaml diff --git a/.changes/unreleased/added-20260830-122126.yaml b/.changes/unreleased/added-20260830-122126.yaml deleted file mode 100644 index 4947ad5c0..000000000 --- a/.changes/unreleased/added-20260830-122126.yaml +++ /dev/null @@ -1,6 +0,0 @@ -kind: added -body: Add Fabric skill attribution to Fabric API requests through the global ``--skill`` option or ``FABRIC_SKILL`` environment variable -time: 2026-08-30T12:21:26.157507+03:00 -custom: - Author: shirasassoon - AuthorLink: https://github.com/shirasassoon From 779305b32bf5a81e96e1ef9c8d669b1d77198e98 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 12:48:43 +0300 Subject: [PATCH 03/13] docs: keep skill attribution internal Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/commands/index.md | 1 - docs/essentials/parameters.md | 7 ------- 2 files changed, 8 deletions(-) diff --git a/docs/commands/index.md b/docs/commands/index.md index 5b221d797..208839c2a 100644 --- a/docs/commands/index.md +++ b/docs/commands/index.md @@ -101,7 +101,6 @@ The following parameters are available for all commands: - `-h, --help`: Display help information for the command - `--output_format`: Specify the output format (`text` or `json`). -- `--skill`: Attribute Fabric API requests to a Fabric skill. ## Common Parameters diff --git a/docs/essentials/parameters.md b/docs/essentials/parameters.md index cbb4600d9..b5964fd38 100644 --- a/docs/essentials/parameters.md +++ b/docs/essentials/parameters.md @@ -63,16 +63,9 @@ For a complete list of commands, see the [Commands page](../commands/index.md). | `--retain_n_hours` | Retain specified hours for `table vacuum` | | `--schedule` | Set the job schedule for `job` | | `--show_headers` | Turn headers on for `api` | -| `--skill` | Attribute Fabric API requests to a Fabric skill | | `--start` | Start date in UTC for `job` | | `--target` | Specify the target for `ln` | | `--timeout` | Specify the timeout of the command in seconds for `job` | | `--type` | Specify the type for `ln` or `job` | | `--vorder` | Apply v-order for `table optimize` | | `--zorder` | Apply Z-order indexing for `table optimize` | - -`--skill` adds the `x-ms-fabric-skill` header to Fabric control-plane -requests made by the command, including internal requests and long-running -operation polling. Set `FABRIC_SKILL` to apply the attribution through an -environment variable; an explicit `--skill` value takes precedence. The -header is not added to OneLake, Azure, or Power BI requests. From ecefc1e7e2202cd6aed9cb75ded79edc03be28c0 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 13:01:29 +0300 Subject: [PATCH 04/13] refactor: align skill attribution with telemetry Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_context.py | 5 +---- src/fabric_cli/core/fab_logger.py | 4 ++-- src/fabric_cli/errors/common.py | 7 ------- src/fabric_cli/parsers/fab_global_params.py | 5 +---- tests/test_core/test_fab_context.py | 8 +++----- tests/test_core/test_fab_decorators.py | 8 +++----- tests/test_core/test_fab_logger.py | 18 ++++++++++++++++++ tests/test_parsers/test_fab_global_params.py | 2 +- 8 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index cc23075cd..3a6623ea6 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -84,10 +84,7 @@ def fabric_skill(self, fabric_skill: Optional[str]) -> None: if fabric_skill is not None and not re.fullmatch( r"[A-Za-z0-9][A-Za-z0-9._-]{0,127}", fabric_skill ): - raise FabricCLIError( - ErrorMessages.Common.invalid_fabric_skill_name(), - fab_constant.ERROR_INVALID_INPUT, - ) + fabric_skill = None self._fabric_skill = fabric_skill def reset_context(self) -> None: diff --git a/src/fabric_cli/core/fab_logger.py b/src/fabric_cli/core/fab_logger.py index 3c20420c0..0995ed0f4 100644 --- a/src/fabric_cli/core/fab_logger.py +++ b/src/fabric_cli/core/fab_logger.py @@ -73,8 +73,8 @@ def log_debug_http_request( for key, value in headers.items(): if key.lower() == "authorization": value = "*****" # Mask authorization token - elif key.lower() == "user-agent": - continue # Skip logging the User-Agent header + elif key.lower() in ("user-agent", fab_constant.FABRIC_SKILL_HEADER): + continue # Skip logging telemetry headers logger.debug(f" '{key}': '{value}'") # Body diff --git a/src/fabric_cli/errors/common.py b/src/fabric_cli/errors/common.py index 96d37da56..27c9f6cf9 100644 --- a/src/fabric_cli/errors/common.py +++ b/src/fabric_cli/errors/common.py @@ -99,13 +99,6 @@ def operation_cancelled(error: str) -> str: def invalid_headers_format() -> str: return "The headers format is invalid" - @staticmethod - def invalid_fabric_skill_name() -> str: - return ( - "The Fabric skill name must start with a letter or number and contain " - "only letters, numbers, periods, underscores, or hyphens (maximum 128 characters)" - ) - @staticmethod def invalid_json_content(content: str, error: Optional[str]) -> str: base_msg = f"The JSON content is invalid: {content}" diff --git a/src/fabric_cli/parsers/fab_global_params.py b/src/fabric_cli/parsers/fab_global_params.py index 1e63d21d3..4288dfbaa 100644 --- a/src/fabric_cli/parsers/fab_global_params.py +++ b/src/fabric_cli/parsers/fab_global_params.py @@ -26,8 +26,5 @@ def add_global_flags(parser) -> None: "--skill", required=False, default=argparse.SUPPRESS, - help=( - "Attribute Fabric API requests to a Fabric skill. " - "Overrides the FABRIC_SKILL environment variable. Optional" - ), + help=argparse.SUPPRESS, ) diff --git a/tests/test_core/test_fab_context.py b/tests/test_core/test_fab_context.py index c8776174b..24958032f 100644 --- a/tests/test_core/test_fab_context.py +++ b/tests/test_core/test_fab_context.py @@ -65,12 +65,10 @@ def test_fabric_skill_valid_name_success(): "skill_name", ["", "-invalid", "invalid skill", "invalid\nheader", "a" * 129], ) -def test_fabric_skill_invalid_name_failure(skill_name): - with pytest.raises(FabricCLIError) as excinfo: - Context().fabric_skill = skill_name +def test_fabric_skill_invalid_name_ignored(skill_name): + Context().fabric_skill = skill_name - assert excinfo.value.message == ErrorMessages.Common.invalid_fabric_skill_name() - assert excinfo.value.status_code == fab_constant.ERROR_INVALID_INPUT + assert Context().fabric_skill is None def test_context_virtual_workspace(): diff --git a/tests/test_core/test_fab_decorators.py b/tests/test_core/test_fab_decorators.py index 8c08bc01a..653df0c4f 100644 --- a/tests/test_core/test_fab_decorators.py +++ b/tests/test_core/test_fab_decorators.py @@ -8,7 +8,6 @@ from fabric_cli.core import fab_constant from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_decorators import set_command_context -from fabric_cli.core.fab_exceptions import FabricCLIError pytestmark = pytest.mark.usefixtures("reset_context") @@ -44,14 +43,13 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill=None)) -def test_set_command_context_rejects_empty_argument_instead_of_using_environment( +def test_set_command_context_ignores_empty_argument_instead_of_using_environment( monkeypatch, ): monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") @set_command_context() def command(args: Namespace) -> None: - pytest.fail("Command should not run with an invalid skill name") + assert Context().fabric_skill is None - with pytest.raises(FabricCLIError): - command(Namespace(command_path="export", skill="")) + command(Namespace(command_path="export", skill="")) diff --git a/tests/test_core/test_fab_logger.py b/tests/test_core/test_fab_logger.py index 3d92f0562..db56d16a8 100644 --- a/tests/test_core/test_fab_logger.py +++ b/tests/test_core/test_fab_logger.py @@ -14,6 +14,7 @@ from requests import RequestException from fabric_cli.core import fab_logger as logger +from fabric_cli.core import fab_constant from fabric_cli.core import fab_state_config @@ -103,6 +104,23 @@ def test_log_debug_http_request_user_agent(monkeypatch): ) +def test_log_debug_http_request_fabric_skill_not_logged(monkeypatch): + monkeypatch.setattr(fab_state_config, "get_config", lambda x: "1") + + with patch.object(logger, "get_logger") as mock_get_logger: + logger.log_debug_http_request( + "GET", + "http://example.com", + {fab_constant.FABRIC_SKILL_HEADER: "semantic-model-authoring"}, + 10, + ) + + logged_messages = [ + call.args[0] for call in mock_get_logger.return_value.debug.call_args_list + ] + assert not any("semantic-model-authoring" in message for message in logged_messages) + + def test_log_debug_http_request_authorization(monkeypatch): monkeypatch.setattr(fab_state_config, "get_config", lambda x: "1") logger.log_debug_http_request( diff --git a/tests/test_parsers/test_fab_global_params.py b/tests/test_parsers/test_fab_global_params.py index cfeeb8dfb..0b33458c5 100644 --- a/tests/test_parsers/test_fab_global_params.py +++ b/tests/test_parsers/test_fab_global_params.py @@ -40,7 +40,7 @@ def test_add_global_flags(): skill_flag = next(a for a in all_flags if "--skill" in a.option_strings) assert skill_flag.dest == "skill" assert not skill_flag.required - assert "FABRIC_SKILL" in skill_flag.help + assert skill_flag.help == argparse.SUPPRESS def test_add_global_flags_parser_integration(): From cd63e1b744d33acb3cf837db458a28b2af2609e0 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 13:38:16 +0300 Subject: [PATCH 05/13] refactor: use command-scoped skill attribution Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_constant.py | 1 - src/fabric_cli/core/fab_decorators.py | 7 +------ tests/test_core/test_fab_decorators.py | 24 +++--------------------- 3 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index ffa752d14..cf4b75e6b 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -67,7 +67,6 @@ FAB_HOST_APP_ENV_VAR = "FAB_HOST_APP" FAB_HOST_APP_VERSION_ENV_VAR = "FAB_HOST_APP_VERSION" -FABRIC_SKILL_ENV_VAR = "FABRIC_SKILL" FABRIC_SKILL_HEADER = "x-ms-fabric-skill" # Other constants diff --git a/src/fabric_cli/core/fab_decorators.py b/src/fabric_cli/core/fab_decorators.py index 3d4ae636d..84dff64c0 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -1,7 +1,6 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -import os from functools import wraps import fabric_cli.core.fab_logger as fab_logger @@ -9,7 +8,6 @@ ERROR_UNAUTHORIZED, EXIT_CODE_AUTHORIZATION_REQUIRED, EXIT_CODE_ERROR, - FABRIC_SKILL_ENV_VAR, ) from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.utils import fab_ui @@ -68,10 +66,7 @@ def wrapper(*args, **kwargs): from fabric_cli.core.fab_context import Context Context().command = args[0].command_path - skill = getattr(args[0], "skill", None) - Context().fabric_skill = ( - skill if skill is not None else os.environ.get(FABRIC_SKILL_ENV_VAR) - ) + Context().fabric_skill = getattr(args[0], "skill", None) return func(*args, **kwargs) return wrapper diff --git a/tests/test_core/test_fab_decorators.py b/tests/test_core/test_fab_decorators.py index 653df0c4f..b3294905c 100644 --- a/tests/test_core/test_fab_decorators.py +++ b/tests/test_core/test_fab_decorators.py @@ -5,16 +5,13 @@ import pytest -from fabric_cli.core import fab_constant from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_decorators import set_command_context pytestmark = pytest.mark.usefixtures("reset_context") -def test_set_command_context_uses_skill_argument_over_environment(monkeypatch): - monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") - +def test_set_command_context_uses_skill_argument(): @set_command_context() def command(args: Namespace) -> None: assert Context().fabric_skill == "argument-skill" @@ -22,19 +19,8 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill="argument-skill")) -def test_set_command_context_uses_skill_environment_variable(monkeypatch): - monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") - - @set_command_context() - def command(args: Namespace) -> None: - assert Context().fabric_skill == "environment-skill" - - command(Namespace(command_path="export", skill=None)) - - -def test_set_command_context_clears_previous_skill(monkeypatch): +def test_set_command_context_clears_previous_skill(): Context().fabric_skill = "previous-skill" - monkeypatch.delenv(fab_constant.FABRIC_SKILL_ENV_VAR, raising=False) @set_command_context() def command(args: Namespace) -> None: @@ -43,11 +29,7 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill=None)) -def test_set_command_context_ignores_empty_argument_instead_of_using_environment( - monkeypatch, -): - monkeypatch.setenv(fab_constant.FABRIC_SKILL_ENV_VAR, "environment-skill") - +def test_set_command_context_ignores_empty_argument(): @set_command_context() def command(args: Namespace) -> None: assert Context().fabric_skill is None From 5e0ba0f1e117e8b353750155894ff4898cc1f031 Mon Sep 17 00:00:00 2001 From: shirasassoon <66449905+shirasassoon@users.noreply.github.com> Date: Sun, 30 Aug 2026 13:40:49 +0300 Subject: [PATCH 06/13] revert format change --- src/fabric_cli/errors/common.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/fabric_cli/errors/common.py b/src/fabric_cli/errors/common.py index 27c9f6cf9..6fcfc0009 100644 --- a/src/fabric_cli/errors/common.py +++ b/src/fabric_cli/errors/common.py @@ -252,9 +252,7 @@ def query_not_supported_for_set(query: str) -> str: @staticmethod def invalid_definition_format(valid_formats: list[str]) -> str: if valid_formats: - message = ( - f"Only the following formats are supported: {', '.join(valid_formats)}" - ) + message = f"Only the following formats are supported: {', '.join(valid_formats)}" else: message = "No formats are supported" return f"Invalid format. {message}" From 8c28418ac4b920a613fea9e3e22b1c1e6a81a644 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Sun, 30 Aug 2026 13:46:46 +0300 Subject: [PATCH 07/13] test: consolidate skill attribution coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/essentials/parameters.md | 2 +- src/fabric_cli/core/fab_decorators.py | 3 +- src/fabric_cli/errors/common.py | 4 +- tests/test_core/test_fab_api_client.py | 67 -------- tests/test_core/test_fab_context.py | 19 --- tests/test_core/test_fab_decorators.py | 37 ----- tests/test_core/test_fab_logger.py | 18 -- tests/test_core/test_fab_skill_attribution.py | 156 ++++++++++++++++++ tests/test_parsers/test_fab_global_params.py | 28 ---- 9 files changed, 159 insertions(+), 175 deletions(-) delete mode 100644 tests/test_core/test_fab_decorators.py create mode 100644 tests/test_core/test_fab_skill_attribution.py diff --git a/docs/essentials/parameters.md b/docs/essentials/parameters.md index b5964fd38..180d68a61 100644 --- a/docs/essentials/parameters.md +++ b/docs/essentials/parameters.md @@ -68,4 +68,4 @@ For a complete list of commands, see the [Commands page](../commands/index.md). | `--timeout` | Specify the timeout of the command in seconds for `job` | | `--type` | Specify the type for `ln` or `job` | | `--vorder` | Apply v-order for `table optimize` | -| `--zorder` | Apply Z-order indexing for `table optimize` | +| `--zorder` | Apply Z-order indexing for `table optimize` | \ No newline at end of file diff --git a/src/fabric_cli/core/fab_decorators.py b/src/fabric_cli/core/fab_decorators.py index 84dff64c0..90acee6ae 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -21,7 +21,7 @@ def getinstance(*args, **kwargs): if class_ not in instances: instances[class_] = class_(*args, **kwargs) return instances[class_] - + return getinstance @@ -64,7 +64,6 @@ def decorator(func): def wrapper(*args, **kwargs): # Import Context locally to avoid circular import from fabric_cli.core.fab_context import Context - Context().command = args[0].command_path Context().fabric_skill = getattr(args[0], "skill", None) return func(*args, **kwargs) diff --git a/src/fabric_cli/errors/common.py b/src/fabric_cli/errors/common.py index 27c9f6cf9..6fcfc0009 100644 --- a/src/fabric_cli/errors/common.py +++ b/src/fabric_cli/errors/common.py @@ -252,9 +252,7 @@ def query_not_supported_for_set(query: str) -> str: @staticmethod def invalid_definition_format(valid_formats: list[str]) -> str: if valid_formats: - message = ( - f"Only the following formats are supported: {', '.join(valid_formats)}" - ) + message = f"Only the following formats are supported: {', '.join(valid_formats)}" else: message = "No formats are supported" return f"Invalid format. {message}" diff --git a/tests/test_core/test_fab_api_client.py b/tests/test_core/test_fab_api_client.py index 4589f1728..2b16c07f0 100644 --- a/tests/test_core/test_fab_api_client.py +++ b/tests/test_core/test_fab_api_client.py @@ -14,7 +14,6 @@ ) from fabric_cli.core import fab_constant from fabric_cli.core.fab_auth import FabAuth -from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_exceptions import FabricAPIError @@ -311,72 +310,6 @@ def __init__(self): assert "ErrorCode" == excinfo.value.status_code -@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") -@pytest.mark.parametrize( - "audience, expects_skill_header", - [ - (None, True), - ("fabric", True), - ("storage", False), - ("azure", False), - ("powerbi", False), - ], -) -def test_do_request_fabric_skill_header_scoped_to_fabric_api( - mock_get_token, audience, expects_skill_header, reset_context -): - class DummyResponse: - status_code = 200 - text = "{}" - content = b"{}" - headers = {} - - reset_context.fabric_skill = "semantic-model-authoring" - dummy_args = Namespace(uri="items", method="get", audience=audience) - - with patch( - "requests.Session.request", return_value=DummyResponse() - ) as mock_request: - do_request(dummy_args) - - request_headers = mock_request.call_args.kwargs["headers"] - if expects_skill_header: - assert ( - request_headers[fab_constant.FABRIC_SKILL_HEADER] - == "semantic-model-authoring" - ) - else: - assert fab_constant.FABRIC_SKILL_HEADER not in request_headers - - -@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") -def test_do_request_fabric_skill_overrides_custom_header(mock_get_token, reset_context): - class DummyResponse: - status_code = 200 - text = "{}" - content = b"{}" - headers = {} - - reset_context.fabric_skill = "semantic-model-authoring" - dummy_args = Namespace( - uri="items", - method="get", - audience=None, - headers={"X-MS-FABRIC-SKILL": "other-skill"}, - ) - - with patch( - "requests.Session.request", return_value=DummyResponse() - ) as mock_request: - do_request(dummy_args) - - assert ( - mock_request.call_args.kwargs["headers"][fab_constant.FABRIC_SKILL_HEADER] - == "semantic-model-authoring" - ) - assert "X-MS-FABRIC-SKILL" not in mock_request.call_args.kwargs["headers"] - - @patch.object(FabAuth(), "get_access_token", return_value="dummy-token") def test_do_request_429_without_retry_after_header_retries_with_default_interval( mock_get_token, diff --git a/tests/test_core/test_fab_context.py b/tests/test_core/test_fab_context.py index 24958032f..129f1158d 100644 --- a/tests/test_core/test_fab_context.py +++ b/tests/test_core/test_fab_context.py @@ -55,22 +55,6 @@ def test_context_workspace(): Context().reset_context() -def test_fabric_skill_valid_name_success(): - Context().fabric_skill = "semantic-model-authoring" - - assert Context().fabric_skill == "semantic-model-authoring" - - -@pytest.mark.parametrize( - "skill_name", - ["", "-invalid", "invalid skill", "invalid\nheader", "a" * 129], -) -def test_fabric_skill_invalid_name_ignored(skill_name): - Context().fabric_skill = skill_name - - assert Context().fabric_skill is None - - def test_context_virtual_workspace(): _tenant = hierarchy.Tenant(name="tenant_name", id="0000") _workspace = hierarchy.VirtualWorkspace(name=".capacities", id=None, parent=_tenant) @@ -456,7 +440,6 @@ def mock_get_command_context(): # region Runtime Mode - class TestRuntimeMode: """Verify Context.set_runtime_mode / get_runtime_mode behaviour after mode-setting removal.""" @@ -489,9 +472,7 @@ def test_runtime_mode_not_in_config_defaults_success(self): def test_runtime_mode_not_module_level_success(self): """Runtime mode must live on Context, not as module-level functions.""" from fabric_cli.core import fab_context as ctx_module - assert not hasattr(ctx_module, "set_runtime_mode") assert not hasattr(ctx_module, "get_runtime_mode") - # endregion diff --git a/tests/test_core/test_fab_decorators.py b/tests/test_core/test_fab_decorators.py deleted file mode 100644 index b3294905c..000000000 --- a/tests/test_core/test_fab_decorators.py +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -from argparse import Namespace - -import pytest - -from fabric_cli.core.fab_context import Context -from fabric_cli.core.fab_decorators import set_command_context - -pytestmark = pytest.mark.usefixtures("reset_context") - - -def test_set_command_context_uses_skill_argument(): - @set_command_context() - def command(args: Namespace) -> None: - assert Context().fabric_skill == "argument-skill" - - command(Namespace(command_path="export", skill="argument-skill")) - - -def test_set_command_context_clears_previous_skill(): - Context().fabric_skill = "previous-skill" - - @set_command_context() - def command(args: Namespace) -> None: - assert Context().fabric_skill is None - - command(Namespace(command_path="export", skill=None)) - - -def test_set_command_context_ignores_empty_argument(): - @set_command_context() - def command(args: Namespace) -> None: - assert Context().fabric_skill is None - - command(Namespace(command_path="export", skill="")) diff --git a/tests/test_core/test_fab_logger.py b/tests/test_core/test_fab_logger.py index db56d16a8..3d92f0562 100644 --- a/tests/test_core/test_fab_logger.py +++ b/tests/test_core/test_fab_logger.py @@ -14,7 +14,6 @@ from requests import RequestException from fabric_cli.core import fab_logger as logger -from fabric_cli.core import fab_constant from fabric_cli.core import fab_state_config @@ -104,23 +103,6 @@ def test_log_debug_http_request_user_agent(monkeypatch): ) -def test_log_debug_http_request_fabric_skill_not_logged(monkeypatch): - monkeypatch.setattr(fab_state_config, "get_config", lambda x: "1") - - with patch.object(logger, "get_logger") as mock_get_logger: - logger.log_debug_http_request( - "GET", - "http://example.com", - {fab_constant.FABRIC_SKILL_HEADER: "semantic-model-authoring"}, - 10, - ) - - logged_messages = [ - call.args[0] for call in mock_get_logger.return_value.debug.call_args_list - ] - assert not any("semantic-model-authoring" in message for message in logged_messages) - - def test_log_debug_http_request_authorization(monkeypatch): monkeypatch.setattr(fab_state_config, "get_config", lambda x: "1") logger.log_debug_http_request( diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py new file mode 100644 index 000000000..f0161891e --- /dev/null +++ b/tests/test_core/test_fab_skill_attribution.py @@ -0,0 +1,156 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import argparse +from argparse import Namespace +from unittest.mock import patch + +import pytest + +from fabric_cli.client.fab_api_client import do_request +from fabric_cli.core import fab_constant +from fabric_cli.core import fab_logger as logger +from fabric_cli.core import fab_state_config +from fabric_cli.core.fab_auth import FabAuth +from fabric_cli.core.fab_context import Context +from fabric_cli.core.fab_decorators import set_command_context +from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers +from fabric_cli.parsers import fab_global_params + +pytestmark = pytest.mark.usefixtures("reset_context") + + +class DummyResponse: + status_code = 200 + text = "{}" + content = b"{}" + headers: dict[str, str] = {} + + +def test_skill_argument_is_hidden(): + parser = argparse.ArgumentParser() + fab_global_params.add_global_flags(parser) + + skill_flag = next( + action for action in parser._actions if "--skill" in action.option_strings + ) + + assert skill_flag.help == argparse.SUPPRESS + assert "--skill" not in parser.format_help() + + +@pytest.mark.parametrize( + "command", + [ + ["--skill", "semantic-model-authoring", "export", "ws.Workspace", "-o", "out"], + ["export", "--skill", "semantic-model-authoring", "ws.Workspace", "-o", "out"], + ["export", "ws.Workspace", "-o", "out", "--skill", "semantic-model-authoring"], + ["--skill", "semantic-model-authoring", "job", "run", "ws.Notebook"], + ["job", "run", "ws.Notebook", "--skill", "semantic-model-authoring"], + ], +) +def test_skill_argument_is_preserved_across_command_tree(command): + parser, _ = create_parser_and_subparsers() + + args = parser.parse_args(command) + + assert args.skill == "semantic-model-authoring" + + +def test_command_context_uses_skill_argument(): + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill == "argument-skill" + + command(Namespace(command_path="export", skill="argument-skill")) + + +def test_command_context_clears_previous_skill(): + Context().fabric_skill = "previous-skill" + + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill is None + + command(Namespace(command_path="export", skill=None)) + + +@pytest.mark.parametrize( + "skill_name", + ["", "-invalid", "invalid skill", "invalid\nheader", "a" * 129], +) +def test_invalid_skill_name_is_ignored(skill_name): + Context().fabric_skill = skill_name + + assert Context().fabric_skill is None + + +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +@pytest.mark.parametrize( + "audience, expects_skill_header", + [ + (None, True), + ("fabric", True), + ("storage", False), + ("azure", False), + ("powerbi", False), + ], +) +def test_skill_header_is_scoped_to_fabric_api( + mock_get_token, audience, expects_skill_header +): + Context().fabric_skill = "semantic-model-authoring" + args = Namespace(uri="items", method="get", audience=audience) + + with patch( + "requests.Session.request", return_value=DummyResponse() + ) as mock_request: + do_request(args) + + request_headers = mock_request.call_args.kwargs["headers"] + if expects_skill_header: + assert ( + request_headers[fab_constant.FABRIC_SKILL_HEADER] + == "semantic-model-authoring" + ) + else: + assert fab_constant.FABRIC_SKILL_HEADER not in request_headers + + +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +def test_skill_argument_overrides_custom_header(mock_get_token): + Context().fabric_skill = "semantic-model-authoring" + args = Namespace( + uri="items", + method="get", + audience=None, + headers={"X-MS-FABRIC-SKILL": "other-skill"}, + ) + + with patch( + "requests.Session.request", return_value=DummyResponse() + ) as mock_request: + do_request(args) + + request_headers = mock_request.call_args.kwargs["headers"] + assert ( + request_headers[fab_constant.FABRIC_SKILL_HEADER] == "semantic-model-authoring" + ) + assert "X-MS-FABRIC-SKILL" not in request_headers + + +def test_skill_header_is_not_logged(monkeypatch): + monkeypatch.setattr(fab_state_config, "get_config", lambda key: "1") + + with patch.object(logger, "get_logger") as mock_get_logger: + logger.log_debug_http_request( + "GET", + "http://example.com", + {fab_constant.FABRIC_SKILL_HEADER: "semantic-model-authoring"}, + 10, + ) + + logged_messages = [ + call.args[0] for call in mock_get_logger.return_value.debug.call_args_list + ] + assert not any("semantic-model-authoring" in message for message in logged_messages) diff --git a/tests/test_parsers/test_fab_global_params.py b/tests/test_parsers/test_fab_global_params.py index 0b33458c5..1886ee52b 100644 --- a/tests/test_parsers/test_fab_global_params.py +++ b/tests/test_parsers/test_fab_global_params.py @@ -8,7 +8,6 @@ import pytest from fabric_cli.parsers import fab_global_params -from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers def test_add_global_flags(): @@ -37,11 +36,6 @@ def test_add_global_flags(): assert not format_flag.required assert "Override output format type" in format_flag.help - skill_flag = next(a for a in all_flags if "--skill" in a.option_strings) - assert skill_flag.dest == "skill" - assert not skill_flag.required - assert skill_flag.help == argparse.SUPPRESS - def test_add_global_flags_parser_integration(): """Test that global flags work correctly in parser.""" @@ -60,28 +54,6 @@ def test_add_global_flags_parser_integration(): args = parser.parse_args(["--output_format", "text"]) assert args.output_format == "text" - args = parser.parse_args(["--skill", "semantic-model-authoring"]) - assert args.skill == "semantic-model-authoring" - # Test invalid output format (should raise SystemExit) with pytest.raises(SystemExit): parser.parse_args(["--output_format", "invalid"]) - - -@pytest.mark.parametrize( - "command", - [ - ["--skill", "semantic-model-authoring", "export", "ws.Workspace", "-o", "out"], - ["export", "--skill", "semantic-model-authoring", "ws.Workspace", "-o", "out"], - ["export", "ws.Workspace", "-o", "out", "--skill", "semantic-model-authoring"], - ["--skill", "semantic-model-authoring", "job", "run", "list", "ws.Notebook"], - ["job", "--skill", "semantic-model-authoring", "run", "list", "ws.Notebook"], - ["job", "run", "list", "ws.Notebook", "--skill", "semantic-model-authoring"], - ], -) -def test_skill_flag_preserved_across_command_tree(command): - parser, _ = create_parser_and_subparsers() - - args = parser.parse_args(command) - - assert args.skill == "semantic-model-authoring" From d0c0fbb85701390e72e6e7b622f34d235ed9b668 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Mon, 31 Aug 2026 11:51:17 +0300 Subject: [PATCH 08/13] remove validation --- src/fabric_cli/client/fab_api_client.py | 5 +---- src/fabric_cli/core/fab_context.py | 8 ++------ tests/test_core/test_fab_skill_attribution.py | 5 ++--- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index a8c3185ee..cf817c31e 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -136,10 +136,7 @@ def do_request( fab_constant.ERROR_INVALID_OPERATION, ) - if audience_value not in ("storage", "azure", "powerbi") and fabric_skill: - for header_name in list(headers): - if header_name.lower() == fab_constant.FABRIC_SKILL_HEADER: - del headers[header_name] + if fabric_skill and audience_value not in ("storage", "azure", "powerbi"): headers[fab_constant.FABRIC_SKILL_HEADER] = fabric_skill try: diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index 3a6623ea6..fe2b2d44e 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -80,12 +80,8 @@ def fabric_skill(self) -> Optional[str]: return self._fabric_skill @fabric_skill.setter - def fabric_skill(self, fabric_skill: Optional[str]) -> None: - if fabric_skill is not None and not re.fullmatch( - r"[A-Za-z0-9][A-Za-z0-9._-]{0,127}", fabric_skill - ): - fabric_skill = None - self._fabric_skill = fabric_skill + def fabric_skill(self, value: Optional[str]) -> None: + self._fabric_skill = value def reset_context(self) -> None: self.cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py index f0161891e..83e9d3653 100644 --- a/tests/test_core/test_fab_skill_attribution.py +++ b/tests/test_core/test_fab_skill_attribution.py @@ -8,9 +8,8 @@ import pytest from fabric_cli.client.fab_api_client import do_request -from fabric_cli.core import fab_constant +from fabric_cli.core import fab_constant, fab_state_config from fabric_cli.core import fab_logger as logger -from fabric_cli.core import fab_state_config from fabric_cli.core.fab_auth import FabAuth from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_decorators import set_command_context @@ -140,7 +139,7 @@ def test_skill_argument_overrides_custom_header(mock_get_token): def test_skill_header_is_not_logged(monkeypatch): - monkeypatch.setattr(fab_state_config, "get_config", lambda key: "1") + monkeypatch.setattr(fab_state_config, "get_config", lambda key: "true") with patch.object(logger, "get_logger") as mock_get_logger: logger.log_debug_http_request( From bcecf0ae925286fe8a90eff604a6b60b29890f42 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Mon, 31 Aug 2026 12:16:03 +0300 Subject: [PATCH 09/13] header exclusion logic --- src/fabric_cli/client/fab_api_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index cf817c31e..179d7aac5 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -136,7 +136,8 @@ def do_request( fab_constant.ERROR_INVALID_OPERATION, ) - if fabric_skill and audience_value not in ("storage", "azure", "powerbi"): + # The fabric skills header is only applicable for the Fabric audience (None or "fabric") + if fabric_skill and audience_value in (None, "fabric"): headers[fab_constant.FABRIC_SKILL_HEADER] = fabric_skill try: From 2073a7fe459e0218f650cc674597e5d14b14ba46 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 2 Sep 2026 14:36:55 +0300 Subject: [PATCH 10/13] fix failing test --- tests/test_core/test_fab_skill_attribution.py | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py index 83e9d3653..0f96d6f4e 100644 --- a/tests/test_core/test_fab_skill_attribution.py +++ b/tests/test_core/test_fab_skill_attribution.py @@ -74,16 +74,6 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill=None)) -@pytest.mark.parametrize( - "skill_name", - ["", "-invalid", "invalid skill", "invalid\nheader", "a" * 129], -) -def test_invalid_skill_name_is_ignored(skill_name): - Context().fabric_skill = skill_name - - assert Context().fabric_skill is None - - @patch.object(FabAuth(), "get_access_token", return_value="dummy-token") @pytest.mark.parametrize( "audience, expects_skill_header", @@ -116,28 +106,6 @@ def test_skill_header_is_scoped_to_fabric_api( assert fab_constant.FABRIC_SKILL_HEADER not in request_headers -@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") -def test_skill_argument_overrides_custom_header(mock_get_token): - Context().fabric_skill = "semantic-model-authoring" - args = Namespace( - uri="items", - method="get", - audience=None, - headers={"X-MS-FABRIC-SKILL": "other-skill"}, - ) - - with patch( - "requests.Session.request", return_value=DummyResponse() - ) as mock_request: - do_request(args) - - request_headers = mock_request.call_args.kwargs["headers"] - assert ( - request_headers[fab_constant.FABRIC_SKILL_HEADER] == "semantic-model-authoring" - ) - assert "X-MS-FABRIC-SKILL" not in request_headers - - def test_skill_header_is_not_logged(monkeypatch): monkeypatch.setattr(fab_state_config, "get_config", lambda key: "true") From e5f97a61f26e5bbe6674b669a36166e2c3f9672a Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 2 Sep 2026 14:46:30 +0300 Subject: [PATCH 11/13] feedback --- src/fabric_cli/client/fab_api_client.py | 2 -- src/fabric_cli/core/fab_context.py | 2 +- tests/test_core/test_fab_api_client.py | 1 + tests/test_core/test_fab_skill_attribution.py | 26 ++++++++++++++++++- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index 179d7aac5..0e896e27f 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -116,8 +116,6 @@ def do_request( fab_context = FabContext() ctxt_cmd = fab_context.command fabric_skill = fab_context.fabric_skill - if not isinstance(fabric_skill, str): - fabric_skill = None headers = { "Authorization": "Bearer " + str(token), diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index fe2b2d44e..788729f50 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -81,7 +81,7 @@ def fabric_skill(self) -> Optional[str]: @fabric_skill.setter def fabric_skill(self, value: Optional[str]) -> None: - self._fabric_skill = value + self._fabric_skill = value if isinstance(value, str) else None def reset_context(self) -> None: self.cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) diff --git a/tests/test_core/test_fab_api_client.py b/tests/test_core/test_fab_api_client.py index 2b16c07f0..2d40c7976 100644 --- a/tests/test_core/test_fab_api_client.py +++ b/tests/test_core/test_fab_api_client.py @@ -490,6 +490,7 @@ def test_do_request_user_agent_header( # Configure mocks mock_auth.return_value.get_access_token.return_value = "dummy-token" mock_context.return_value.command = "test-command" + mock_context.return_value.fabric_skill = None class DummyResponse: status_code = 200 diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py index 0f96d6f4e..2cf977b07 100644 --- a/tests/test_core/test_fab_skill_attribution.py +++ b/tests/test_core/test_fab_skill_attribution.py @@ -74,6 +74,30 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill=None)) +@pytest.mark.parametrize( + "value", + [123, True, [], {}, object()], + ids=["integer", "boolean", "list", "dictionary", "object"], +) +def test_fabric_skill_setter_normalizes_non_string_value(value): + Context().fabric_skill = value + + assert Context().fabric_skill is None + + +@pytest.mark.parametrize( + "skill", + [123, True, [], {}, object()], + ids=["integer", "boolean", "list", "dictionary", "object"], +) +def test_command_context_normalizes_non_string_skill_argument(skill): + @set_command_context() + def command(args: Namespace) -> None: + assert Context().fabric_skill is None + + command(Namespace(command_path="export", skill=skill)) + + @patch.object(FabAuth(), "get_access_token", return_value="dummy-token") @pytest.mark.parametrize( "audience, expects_skill_header", @@ -86,7 +110,7 @@ def command(args: Namespace) -> None: ], ) def test_skill_header_is_scoped_to_fabric_api( - mock_get_token, audience, expects_skill_header + _mock_get_token, audience, expects_skill_header ): Context().fabric_skill = "semantic-model-authoring" args = Namespace(uri="items", method="get", audience=audience) From f4b1db79566bac03f32deb76acd8a792ccdbac05 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 2 Sep 2026 14:49:56 +0300 Subject: [PATCH 12/13] add prefix --- tests/test_core/test_fab_skill_attribution.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py index 2cf977b07..6fc18f0b9 100644 --- a/tests/test_core/test_fab_skill_attribution.py +++ b/tests/test_core/test_fab_skill_attribution.py @@ -26,7 +26,7 @@ class DummyResponse: headers: dict[str, str] = {} -def test_skill_argument_is_hidden(): +def test_skill_argument_is_hidden_success(): parser = argparse.ArgumentParser() fab_global_params.add_global_flags(parser) @@ -48,7 +48,7 @@ def test_skill_argument_is_hidden(): ["job", "run", "ws.Notebook", "--skill", "semantic-model-authoring"], ], ) -def test_skill_argument_is_preserved_across_command_tree(command): +def test_skill_argument_is_preserved_across_command_tree_success(command): parser, _ = create_parser_and_subparsers() args = parser.parse_args(command) @@ -56,7 +56,7 @@ def test_skill_argument_is_preserved_across_command_tree(command): assert args.skill == "semantic-model-authoring" -def test_command_context_uses_skill_argument(): +def test_command_context_uses_skill_argument_success(): @set_command_context() def command(args: Namespace) -> None: assert Context().fabric_skill == "argument-skill" @@ -64,7 +64,7 @@ def command(args: Namespace) -> None: command(Namespace(command_path="export", skill="argument-skill")) -def test_command_context_clears_previous_skill(): +def test_command_context_clears_previous_skill_success(): Context().fabric_skill = "previous-skill" @set_command_context() @@ -79,7 +79,7 @@ def command(args: Namespace) -> None: [123, True, [], {}, object()], ids=["integer", "boolean", "list", "dictionary", "object"], ) -def test_fabric_skill_setter_normalizes_non_string_value(value): +def test_fabric_skill_setter_normalizes_non_string_value_success(value): Context().fabric_skill = value assert Context().fabric_skill is None @@ -90,7 +90,7 @@ def test_fabric_skill_setter_normalizes_non_string_value(value): [123, True, [], {}, object()], ids=["integer", "boolean", "list", "dictionary", "object"], ) -def test_command_context_normalizes_non_string_skill_argument(skill): +def test_command_context_normalizes_non_string_skill_argument_success(skill): @set_command_context() def command(args: Namespace) -> None: assert Context().fabric_skill is None @@ -109,7 +109,7 @@ def command(args: Namespace) -> None: ("powerbi", False), ], ) -def test_skill_header_is_scoped_to_fabric_api( +def test_skill_header_is_scoped_to_fabric_api_success( _mock_get_token, audience, expects_skill_header ): Context().fabric_skill = "semantic-model-authoring" @@ -130,7 +130,7 @@ def test_skill_header_is_scoped_to_fabric_api( assert fab_constant.FABRIC_SKILL_HEADER not in request_headers -def test_skill_header_is_not_logged(monkeypatch): +def test_skill_header_is_not_logged_success(monkeypatch): monkeypatch.setattr(fab_state_config, "get_config", lambda key: "true") with patch.object(logger, "get_logger") as mock_get_logger: From 1e98fde29c0bd4bd6709e7af68cdb7e51ae4cfc4 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 2 Sep 2026 15:39:28 +0300 Subject: [PATCH 13/13] update tests --- src/fabric_cli/core/fab_context.py | 1 - tests/test_core/test_fab_skill_attribution.py | 58 +++++++++++++++---- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index 788729f50..bc3c8fe65 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -5,7 +5,6 @@ import json import os import platform -import re import sys from typing import Optional diff --git a/tests/test_core/test_fab_skill_attribution.py b/tests/test_core/test_fab_skill_attribution.py index 6fc18f0b9..3ce02da8f 100644 --- a/tests/test_core/test_fab_skill_attribution.py +++ b/tests/test_core/test_fab_skill_attribution.py @@ -76,26 +76,42 @@ def command(args: Namespace) -> None: @pytest.mark.parametrize( "value", - [123, True, [], {}, object()], - ids=["integer", "boolean", "list", "dictionary", "object"], + [ + "", + " ", + "bad\nvalue", + "bad\rvalue", + "-leading", + "a", + "semantic-model_authoring.v2", + "a" * 129, + ], + ids=[ + "empty", + "whitespace", + "newline", + "carriage-return", + "leading-hyphen", + "single-character", + "punctuation", + "long", + ], ) -def test_fabric_skill_setter_normalizes_non_string_value_success(value): +def test_fabric_skill_setter_preserves_string_value_success(value): Context().fabric_skill = value - assert Context().fabric_skill is None + assert Context().fabric_skill == value @pytest.mark.parametrize( - "skill", + "value", [123, True, [], {}, object()], ids=["integer", "boolean", "list", "dictionary", "object"], ) -def test_command_context_normalizes_non_string_skill_argument_success(skill): - @set_command_context() - def command(args: Namespace) -> None: - assert Context().fabric_skill is None +def test_fabric_skill_setter_normalizes_non_string_value_success(value): + Context().fabric_skill = value - command(Namespace(command_path="export", skill=skill)) + assert Context().fabric_skill is None @patch.object(FabAuth(), "get_access_token", return_value="dummy-token") @@ -130,14 +146,32 @@ def test_skill_header_is_scoped_to_fabric_api_success( assert fab_constant.FABRIC_SKILL_HEADER not in request_headers -def test_skill_header_is_not_logged_success(monkeypatch): +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +def test_skill_header_is_omitted_when_skill_is_not_set_success(_mock_get_token): + args = Namespace(uri="items", method="get", audience="fabric") + + with patch( + "requests.Session.request", return_value=DummyResponse() + ) as mock_request: + do_request(args) + + request_headers = mock_request.call_args.kwargs["headers"] + assert fab_constant.FABRIC_SKILL_HEADER not in request_headers + + +@pytest.mark.parametrize( + "header_name", + [fab_constant.FABRIC_SKILL_HEADER, "X-MS-FABRIC-SKILL"], + ids=["canonical-case", "uppercase"], +) +def test_skill_header_is_not_logged_success(monkeypatch, header_name): monkeypatch.setattr(fab_state_config, "get_config", lambda key: "true") with patch.object(logger, "get_logger") as mock_get_logger: logger.log_debug_http_request( "GET", "http://example.com", - {fab_constant.FABRIC_SKILL_HEADER: "semantic-model-authoring"}, + {header_name: "semantic-model-authoring"}, 10, )