Skip to content

Commit 42d80bc

Browse files
Aitomatesclaude
andcommitted
fix(validating): guard null fileResult from MCP before ParseInnerJson
Prevents NullReferenceException when MCP returns null for a test file path. Logs a warning and marks compilation as failed instead of crashing. Also simplify McpToolResult constructor calls in tests to positional args. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8c93516 commit 42d80bc

4 files changed

Lines changed: 14 additions & 6 deletions

File tree

src/GsdOrchestrator.Tests/McpToolDispatcherSecondaryRateLimitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public async Task CallAsync_SecondaryRateLimit_EntersBranchThenCancels()
2525
var client = Substitute.For<IMcpClient>();
2626
var callCount = 0;
2727
client.CallToolAsync(Arg.Any<string>(), Arg.Any<JsonObject>(), Arg.Any<CancellationToken>())
28-
.Returns(ci =>
28+
.Returns<Task<McpToolResult>>(ci =>
2929
{
3030
callCount++;
3131
throw new McpException("secondary rate limit hit", isTransient: true, isSecondaryRateLimit: true);

src/GsdOrchestrator.Tests/ReviewingStateErrorPathTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ public async Task GetPr_ReturnsError_FallsBackAndCompletes()
4040
{
4141
var m = Substitute.For<IMcpClient>();
4242
m.CallToolAsync(Arg.Is<string>("get_pull_request"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
43-
.Returns(Task.FromResult(new McpToolResult("err",isError:true)));
43+
.Returns(Task.FromResult(new McpToolResult("err",true)));
4444
m.CallToolAsync(Arg.Is<string>("create_pull_request_review"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
45-
.Returns(Task.FromResult(new McpToolResult("{}",isError:false)));
45+
.Returns(Task.FromResult(new McpToolResult("{}",false)));
4646
var res = await Sut(m,LlmApprove()).ExecuteAsync(PrCtx(),CancellationToken.None);
4747
Assert.Equal(WorkflowState.Done,res.CurrentState);
4848
}
@@ -54,7 +54,7 @@ public async Task GetPr_ThrowsMcpException_FallsBackAndCompletes()
5454
m.CallToolAsync(Arg.Is<string>("get_pull_request"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
5555
.ThrowsAsync(new McpException("notfound",isTransient:false));
5656
m.CallToolAsync(Arg.Is<string>("create_pull_request_review"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
57-
.Returns(Task.FromResult(new McpToolResult("{}",isError:false)));
57+
.Returns(Task.FromResult(new McpToolResult("{}",false)));
5858
var res = await Sut(m,LlmApprove()).ExecuteAsync(PrCtx(),CancellationToken.None);
5959
Assert.Equal(WorkflowState.Done,res.CurrentState);
6060
}
@@ -95,7 +95,7 @@ public async Task SubmitReview_ReturnsError_ThrowsMcpException()
9595
m.CallToolAsync(Arg.Is<string>("get_pull_request"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
9696
.Returns(Task.FromResult(new McpToolResult("{\"title\":\"T\",\"body\":\"\"}",false)));
9797
m.CallToolAsync(Arg.Is<string>("create_pull_request_review"),Arg.Any<JsonObject>(),Arg.Any<CancellationToken>())
98-
.Returns(Task.FromResult(new McpToolResult("fail",isError:true)));
98+
.Returns(Task.FromResult(new McpToolResult("fail",true)));
9999
await Assert.ThrowsAsync<McpException>(()=>Sut(m,LlmApprove()).ExecuteAsync(PrCtx(),CancellationToken.None));
100100
}
101101

src/GsdOrchestrator.Tests/States/ValidatingStateTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,12 @@ private static IMcpClient BuildMcpClientWithFileContent(string fileContent)
186186
var mcp = BuildMcpClientAhead();
187187
var base64 = Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes(fileContent));
188188
var json = string.Format("{{\"content\":\"{0}\"}}", base64);
189+
var result = new McpToolResult(json, false);
189190
mcp.CallToolAsync(
190191
Arg.Is<string>("get_file_contents"),
191192
Arg.Any<JsonObject>(),
192193
Arg.Any<CancellationToken>())
193-
.Returns(Task.FromResult(new McpToolResult(json, false)));
194+
.Returns(_ => Task.FromResult(result));
194195
return mcp;
195196
}
196197

src/GsdOrchestrator/Workflows/States/ValidatingState.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,13 @@ public async Task<GsdWorkflowContext> ExecuteAsync(GsdWorkflowContext ctx, Cance
142142
["ref"] = branch.BranchName
143143
}, ct);
144144

145+
if (fileResult is null)
146+
{
147+
_logger.LogWarning("Test file {Path} returned null from MCP", generatedTest.TestPath);
148+
testCompilationPassed = false;
149+
continue;
150+
}
151+
145152
var fileJson = fileResult.ParseInnerJson();
146153
var b64 = fileJson?["content"]?.GetValue<string>()?.Replace("\n", "") ?? "";
147154
var content = b64.Length > 0

0 commit comments

Comments
 (0)