Fix appconfig issues#2683
Merged
Merged
Conversation
…y, and delete response - FindAppConfigStore: remove redundant subscriptionIdentifier parameter; error message now correctly names the missing store - SetKeyValue: throw ArgumentException for tags missing '=' separator instead of silently creating empty-value tags - GetConfigurationClient: apply user retry policy to ConfigurationClient (data-plane), not just the ARM discovery call - DeleteKeyValue: return Task<bool> indicating whether the key existed before deletion; surfaces 'existed' field in kv delete response - Update unit tests and docs to reflect new behavior
Response now includes 'message' field: - 'Key <key> deleted successfully.' when the key existed and was removed - 'Key <key> did not exist in store <store>.' when the key was already absent
…' maps to empty value
g2vinay
requested review from
JasonYeMSFT,
KarishmaGhiya,
anannya03,
sandeep-sen,
wbreza and
xirzec
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes App Configuration key-value operations by correcting store lookup arguments, applying retry options to data-plane clients, and surfacing whether delete operations actually removed an existing key.
Changes:
- Removes the redundant subscription lookup parameter from App Configuration store discovery.
- Applies configured retry policy to
ConfigurationClientoperations. - Adds
existed/messagedelete result fields with tests and documentation/changelog updates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tools/Azure.Mcp.Tools.AppConfig/src/Services/AppConfigService.cs |
Updates delete return value, store lookup signature, and retry policy configuration. |
tools/Azure.Mcp.Tools.AppConfig/src/Services/IAppConfigService.cs |
Changes delete contract to return whether the key existed. |
tools/Azure.Mcp.Tools.AppConfig/src/Commands/KeyValue/KeyValueDeleteCommand.cs |
Adds delete result metadata and user-facing message. |
tools/Azure.Mcp.Tools.AppConfig/tests/Azure.Mcp.Tools.AppConfig.Tests/KeyValue/KeyValueDeleteCommandTests.cs |
Updates command tests for new delete response behavior. |
tools/Azure.Mcp.Tools.AppConfig/tests/Azure.Mcp.Tools.AppConfig.Tests/Services/AppConfigServiceTests.cs |
Updates regression guard for store lookup signature. |
servers/Azure.Mcp.Server/docs/azmcp-commands.md |
Documents delete response shape. |
servers/Azure.Mcp.Server/changelog-entries/1779262158643.yaml |
Adds changelog entry for the App Configuration fixes. |
alzimmermsft
approved these changes
May 27, 2026
github-project-automation
Bot
moved this from Untriaged
to In Progress
in Azure MCP Server
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed multiple bugs in the App Configuration toolset:
- Store lookup no longer passes subscription as both subscription-based arguments when resolving the store by name
- The user-supplied retry policy is now applied to all data-plane key-value operations (get, set, delete, lock); previously it was only applied to the ARM store-discovery call, causing custom retry settings to have no effect on actual configuration reads and writes
- The
kv deletecommand response now includes anexistedfield (trueif the key was present and deleted,falseif it was already absent) and a human-readablemessage, allowing callers to distinguish a successful deletion from a no-op on a non-existent key