Expose concurrency lock metadata (lockScope/lockKey) in StructuredContent output#430
Conversation
…ent for Build/Run/Test/Publish actions Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/77f2c5ea-985d-4c67-a482-b869b29333c2 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/77f2c5ea-985d-4c67-a482-b869b29333c2 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…ant, extract GetConcurrencyOperationType helper Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/77f2c5ea-985d-4c67-a482-b869b29333c2 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…tnet restore) Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/77f2c5ea-985d-4c67-a482-b869b29333c2 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…ntation Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/77f2c5ea-985d-4c67-a482-b869b29333c2 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
|
@copilot address comments |
There was a problem hiding this comment.
Pull request overview
This PR extends the MCP server’s dotnet_project structured output to include explicit concurrency lock metadata (lockScope/lockKey plus contention fields) so consumers can reason about lock granularity and diagnose contention without parsing plain-text output.
Changes:
- Add
LockScope,LockInfo, andConcurrencyAwareResulttypes, and embedlockInfointoBuildResultand other concurrency-gated project actions’ structured output. - Update concurrency execution plumbing to produce lock metadata and detect contention for Build/Run/Test/Publish.
- Add a new test suite for lock metadata behavior and update the machine-readable contract documentation.
Show a summary per file
| File | Description |
|---|---|
| DotNetMcp/Tools/Cli/DotNetCliTools.Project.Consolidated.cs | Adds lockInfo to Build structured content and introduces ConcurrencyAwareResult structured content for Run/Test/Publish. |
| DotNetMcp/Tools/Cli/DotNetCliTools.Core.cs | Changes concurrency execution to return (text, lockInfo) and adds helpers for scope/key computation and conflict detection. |
| DotNetMcp/Errors/ErrorResultFactory.cs | Extends ParseBuildOutput to accept and propagate optional lockInfo. |
| DotNetMcp/Errors/ErrorResult.cs | Introduces LockScope, LockInfo, ConcurrencyAwareResult, and adds LockInfo? to BuildResult/ErrorResponse. |
| DotNetMcp.Tests/Execution/ConcurrencyLockInfoTests.cs | Adds tests validating lockInfo presence/scope/key/contended behavior for concurrency-gated actions. |
| doc/machine-readable-contract.md | Documents lockInfo schema, selection rules, and examples for dotnet_project structured content. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 5
| var conflictedLockInfo = BuildLockInfo(operationType, target, isContended: true); | ||
| var errorResponse = ErrorResultFactory.CreateConcurrencyConflict(operationType, target, conflictingOperation!); | ||
| return $"Error: {errorResponse.Errors[0].Message}\nHint: {errorResponse.Errors[0].Hint}"; | ||
| return ($"Error: {errorResponse.Errors[0].Message}\nHint: {errorResponse.Errors[0].Hint}", conflictedLockInfo); |
There was a problem hiding this comment.
The concurrency-conflict branch returns text that does not include the standard trailing "Exit Code: N" line that DotNetCommandExecutor appends to command results. That breaks downstream logic that infers failure via Exit Code parsing (e.g., IsCommandFailure/ParseBuildOutput) and can cause conflicts to be treated as successful operations. Consider appending an explicit non-zero exit code (e.g., -1) to the returned text (while keeping the existing prefix used by IsConcurrencyConflictText), or otherwise preserving the same output invariant as ExecuteCommandAsync.
| return ($"Error: {errorResponse.Errors[0].Message}\nHint: {errorResponse.Errors[0].Hint}", conflictedLockInfo); | |
| return ($"Error: {errorResponse.Errors[0].Message}\nHint: {errorResponse.Errors[0].Hint}\nExit Code: -1", conflictedLockInfo); |
| var lockInfo = BuildLockInfo("build", GetOperationTarget(effectiveProject, workingDirectory), | ||
| isContended: IsConcurrencyConflictText(textResult)); | ||
| var buildResult = ErrorResultFactory.ParseBuildOutput(textResult, effectiveProject, configuration, lockInfo); |
There was a problem hiding this comment.
Build output is always parsed into BuildResult, but in the concurrency-conflict case the text returned by ExecuteWithConcurrencyCheck currently lacks an "Exit Code:" line. ParseBuildOutput defaults exitCode to 0 when that line is missing, so the structured BuildResult can incorrectly report Success=true and "Build succeeded" even though the operation was rejected due to contention. Either ensure the conflict text includes a non-zero Exit Code line, or special-case conflict results here to return an error/failed BuildResult representation.
| var lockInfo = BuildLockInfo("build", GetOperationTarget(effectiveProject, workingDirectory), | |
| isContended: IsConcurrencyConflictText(textResult)); | |
| var buildResult = ErrorResultFactory.ParseBuildOutput(textResult, effectiveProject, configuration, lockInfo); | |
| var isConcurrencyConflict = IsConcurrencyConflictText(textResult); | |
| var lockInfo = BuildLockInfo("build", GetOperationTarget(effectiveProject, workingDirectory), | |
| isContended: isConcurrencyConflict); | |
| // ParseBuildOutput defaults to exit code 0 when no "Exit Code:" line is present. | |
| // Concurrency-conflict responses may not include that line, so normalize the text | |
| // before parsing to ensure the structured BuildResult is marked as failed. | |
| var buildTextResult = isConcurrencyConflict && !textResult.Contains("Exit Code:", StringComparison.Ordinal) | |
| ? $"{textResult}{Environment.NewLine}Exit Code: 1" | |
| : textResult; | |
| var buildResult = ErrorResultFactory.ParseBuildOutput(buildTextResult, effectiveProject, configuration, lockInfo); |
| When a conflicting operation is already in progress, the text content contains the error message **and** `structuredContent` exposes the lock details with `lockContended: true`: | ||
|
|
||
| ```json | ||
| { | ||
| "content": [{ "type": "text", "text": "Error: Cannot execute 'build' on 'MyApp.csproj' because a conflicting operation is already in progress: build on /home/user/projects/myapp/myapp.csproj (started at 2026-04-11 05:30:00)\nHint: Wait for the conflicting operation to complete, or cancel it before retrying this operation." }], | ||
| "structuredContent": { | ||
| "success": false, | ||
| "errorCount": 0, | ||
| "warningCount": 0, | ||
| "summary": "", | ||
| "lockInfo": { | ||
| "lockScope": "project", | ||
| "lockKey": "/home/user/projects/myapp/myapp.csproj", | ||
| "lockContended": true, | ||
| "lockWaitedMs": 0 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The concurrency-conflict example output omits the usual "Exit Code: N" line and shows structuredContent.success=false. With the current implementation, missing an Exit Code line causes ParseBuildOutput to treat the result as exitCode=0 (Success=true). Either update the example to match the actual output, or adjust the conflict text/code path so the structured BuildResult reliably reports failure on contention.
| /// <summary>Lock is scoped to a specific .csproj file.</summary> | ||
| Project, | ||
| /// <summary>Lock is scoped to a specific .sln or .slnx file.</summary> | ||
| Solution, | ||
| /// <summary>Lock is scoped to a working directory (used when no project/solution file is specified).</summary> | ||
| WorkingDirectory, |
There was a problem hiding this comment.
The XML doc comment for LockScope.Project says it is scoped to a ".csproj" file, but DetermineLockScope treats .fsproj and .vbproj as project scope too. Update the comment to reflect all supported project file types so the contract/docs stay accurate.
| // The New action does not use concurrency control, so no lockInfo expected. | ||
| var result = await _tools.DotnetProject( | ||
| action: DotnetProjectAction.New, | ||
| template: "console", | ||
| name: "TestApp"); | ||
|
|
||
| // New returns plain text (no structured content) — just verify no crash and no lockInfo | ||
| if (result.StructuredContent.HasValue) | ||
| { | ||
| Assert.False(result.StructuredContent!.Value.TryGetProperty("lockInfo", out _), | ||
| "New action should not expose lock metadata"); | ||
| } |
There was a problem hiding this comment.
This test invokes DotnetProjectAction.New without specifying an output directory, which can create files in the test process working directory and make runs flaky (collisions between parallel runs / leftover artifacts). Prefer using a unique temp output directory and cleaning it up (or choose a non-mutating action to assert the absence of lockInfo).
| // The New action does not use concurrency control, so no lockInfo expected. | |
| var result = await _tools.DotnetProject( | |
| action: DotnetProjectAction.New, | |
| template: "console", | |
| name: "TestApp"); | |
| // New returns plain text (no structured content) — just verify no crash and no lockInfo | |
| if (result.StructuredContent.HasValue) | |
| { | |
| Assert.False(result.StructuredContent!.Value.TryGetProperty("lockInfo", out _), | |
| "New action should not expose lock metadata"); | |
| } | |
| var output = System.IO.Path.Join( | |
| System.IO.Path.GetTempPath(), | |
| $"DotNetMcp.Tests.{nameof(ConcurrencyLockInfoTests)}.{nameof(DotnetProject_New_DoesNotContainLockInfo)}.{System.Guid.NewGuid():N}"); | |
| try | |
| { | |
| // The New action does not use concurrency control, so no lockInfo expected. | |
| var result = await _tools.DotnetProject( | |
| action: DotnetProjectAction.New, | |
| template: "console", | |
| name: "TestApp", | |
| output: output); | |
| // New returns plain text (no structured content) — just verify no crash and no lockInfo | |
| if (result.StructuredContent.HasValue) | |
| { | |
| Assert.False(result.StructuredContent!.Value.TryGetProperty("lockInfo", out _), | |
| "New action should not expose lock metadata"); | |
| } | |
| } | |
| finally | |
| { | |
| if (System.IO.Directory.Exists(output)) | |
| { | |
| System.IO.Directory.Delete(output, recursive: true); | |
| } | |
| } |
…edAccessException in test cleanup Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/5eed388e-8411-4188-a023-af6fd5174cd6 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Consumers of
dotnet_projectmachine-readable output had no way to observe which resource was locked, at what granularity, or whether a concurrency conflict occurred — making orchestration and contention diagnosis opaque.What changed
New types (
ErrorResult.cs)LockScopeenum —project | solution | workingDirectory | global(serializes as camelCase JSON)LockInfo—lockScope,lockKey(normalized absolute path, OS case preserved),lockContended?,lockWaitedMs?ConcurrencyAwareResult— lightweight structured wrapper for Run/Test/PublishLockInfo?added toBuildResultandErrorResponseInfrastructure (
DotNetCliTools.Core.cs)ExecuteWithConcurrencyChecknow returns(string text, LockInfo lockInfo)— conflict setslockContended: true, lockWaitedMs: 0DetermineLockScope,BuildLockInfo,GetConcurrencyOperationType,IsConcurrencyConflictTextConcurrencyConflictErrorTextPrefixconstant documents the coupling between conflict-text emission and detectionConsolidated tool (
DotnetProject)lockInfoembedded in existingBuildResultStructuredContentConcurrencyAwareResult { lockInfo }StructuredContent.csproj/.fsproj/.vbproj→project;.sln/.slnx→solution; directory →workingDirectory)Example output
Successful build on a
.csproj:{ "structuredContent": { "success": true, "summary": "Build succeeded", "lockInfo": { "lockScope": "project", "lockKey": "/repo/MyApp.csproj" } } }Conflict case:
{ "structuredContent": { "lockInfo": { "lockScope": "project", "lockKey": "/repo/MyApp.csproj", "lockContended": true, "lockWaitedMs": 0 } } }Docs & tests
doc/machine-readable-contract.mdupdated with field table, selection rules, and examplesConcurrencyLockInfoTestscovering scope detection, key stability, and contention across all four actions