Skip to content
18 changes: 10 additions & 8 deletions conversation/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,16 @@ func BlocksToPost(
arguments = unsharedToolUseArgumentsRedaction
}
toolCall := llm.ToolCall{
ID: block.ID,
Name: block.Name,
ServerOrigin: block.ServerOrigin,
Arguments: arguments,
MCPBareName: block.MCPBareName,
Status: StatusFromString(block.Status),
Title: block.Title,
Description: block.Description,
ID: block.ID,
Name: block.Name,
ServerOrigin: block.ServerOrigin,
Arguments: arguments,
MCPBareName: block.MCPBareName,
Status: StatusFromString(block.Status),
Title: block.Title,
Description: block.Description,
UserInteraction: block.UserInteraction,
WouldAutoExecute: block.WouldAutoExecute,
}
if redactToolUse {
toolCall.MCPBareName = ""
Expand Down
51 changes: 51 additions & 0 deletions conversation/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,57 @@ func TestBlocksToPostRehydratesToolCatalogMetadata(t *testing.T) {
assert.Equal(t, "Get Issue", toolCall.Title)
}

func TestBlocksToPostPreservesRejectionSignalsThroughRedaction(t *testing.T) {
blocks := []ContentBlock{
{
Type: BlockTypeToolUse,
ID: "q-1",
Name: "AskUserQuestion",
Status: StatusRejected,
UserInteraction: llm.UserInteractionSelect,
Shared: new(true),
},
{Type: BlockTypeToolResult, ToolUseID: "q-1", Content: "User skipped the question", Status: StatusError, Shared: new(true)},
{
Type: BlockTypeToolUse,
ID: "auto-1",
Name: "jira__get_issue",
Input: json.RawMessage(`{"key":"secret"}`),
Status: StatusRejected,
WouldAutoExecute: true,
Shared: new(false),
},
{Type: BlockTypeToolResult, ToolUseID: "auto-1", Content: "Tool call rejected by user", Status: StatusError, Shared: new(true)},
{
Type: BlockTypeToolUse,
ID: "human-1",
Name: "search",
Input: json.RawMessage(`{"q":"secret"}`),
Status: StatusRejected,
Shared: new(false),
},
{Type: BlockTypeToolResult, ToolUseID: "human-1", Content: "Tool call rejected by user", Status: StatusError, Shared: new(true)},
}

got := BlocksToPost(blocks, "assistant", PostConversionOptions{RedactUnshared: true})
require.Len(t, got.ToolUse, 3)

byID := map[string]llm.ToolCall{}
for _, tc := range got.ToolUse {
byID[tc.ID] = tc
}

assert.Equal(t, llm.UserInteractionSelect, byID["q-1"].UserInteraction)
assert.False(t, byID["q-1"].WouldAutoExecute)
assert.Empty(t, byID["auto-1"].UserInteraction)
assert.True(t, byID["auto-1"].WouldAutoExecute)
assert.JSONEq(t, `{}`, string(byID["auto-1"].Arguments))
assert.Empty(t, byID["human-1"].UserInteraction)
assert.False(t, byID["human-1"].WouldAutoExecute)
assert.JSONEq(t, `{}`, string(byID["human-1"].Arguments))
assert.Equal(t, "Tool call rejected by user", byID["human-1"].Result)
}

func TestRoleMapping(t *testing.T) {
tests := []struct {
roleStr string
Expand Down
23 changes: 12 additions & 11 deletions conversation/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,18 @@ func toolUseBlocks(
// Tool use ends an assistant turn, so calls always come last.
for _, tc := range toolCalls {
blocks = append(blocks, ContentBlock{
Type: BlockTypeToolUse,
ID: tc.ID,
Name: tc.Name,
ServerOrigin: tc.ServerOrigin,
Input: tc.Arguments,
MCPBareName: tc.MCPBareName,
Status: StatusToString(tc.Status),
Shared: new(shared),
UserInteraction: tc.UserInteraction,
Title: tc.Title,
Description: tc.Description,
Type: BlockTypeToolUse,
ID: tc.ID,
Name: tc.Name,
ServerOrigin: tc.ServerOrigin,
Input: tc.Arguments,
MCPBareName: tc.MCPBareName,
Status: StatusToString(tc.Status),
Shared: new(shared),
UserInteraction: tc.UserInteraction,
WouldAutoExecute: tc.WouldAutoExecute,
Title: tc.Title,
Description: tc.Description,
})
}

Expand Down
20 changes: 12 additions & 8 deletions conversation/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,16 @@ func TestToolUseBlocksStatuses(t *testing.T) {

func TestToolUseBlocksPreservesApprovalMetadata(t *testing.T) {
blocks := toolUseBlocks("", llm.ReasoningData{}, nil, nil, []llm.ToolCall{{
ID: "tc1",
Name: "jira__get_issue",
Description: "Get a Jira issue",
Title: "Get Issue",
ServerOrigin: "https://jira.example.com",
Arguments: json.RawMessage(`{"key":"MM-1"}`),
MCPBareName: "get_issue",
Status: llm.ToolCallStatusPending,
ID: "tc1",
Name: "jira__get_issue",
Description: "Get a Jira issue",
Title: "Get Issue",
ServerOrigin: "https://jira.example.com",
Arguments: json.RawMessage(`{"key":"MM-1"}`),
MCPBareName: "get_issue",
Status: llm.ToolCallStatusPending,
WouldAutoExecute: true,
UserInteraction: llm.UserInteractionSelect,
}}, false)

require.Len(t, blocks, 1)
Expand All @@ -79,6 +81,8 @@ func TestToolUseBlocksPreservesApprovalMetadata(t *testing.T) {
assert.Equal(t, "get_issue", blocks[0].MCPBareName)
assert.Equal(t, "Get Issue", blocks[0].Title)
assert.Equal(t, "Get a Jira issue", blocks[0].Description)
assert.True(t, blocks[0].WouldAutoExecute)
assert.Equal(t, llm.UserInteractionSelect, blocks[0].UserInteraction)
}

func TestToolUseBlocksIncludesServerToolActivity(t *testing.T) {
Expand Down
20 changes: 11 additions & 9 deletions conversation/tool_use_writer_parity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,21 @@ var persistedToolUseFields = []string{
"title",
"description",
"user_interaction",
"would_auto_execute",
}

func parityToolCall() llm.ToolCall {
return llm.ToolCall{
ID: "tc-1",
Name: "mattermost__create_post",
Description: "Create a post",
Title: "Create Post",
Arguments: json.RawMessage(`{"channel_id":"c1"}`),
Status: llm.ToolCallStatusSuccess,
MCPBareName: "create_post",
UserInteraction: llm.UserInteractionSelect,
ServerOrigin: "embedded://mattermost",
ID: "tc-1",
Name: "mattermost__create_post",
Description: "Create a post",
Title: "Create Post",
Arguments: json.RawMessage(`{"channel_id":"c1"}`),
Status: llm.ToolCallStatusSuccess,
MCPBareName: "create_post",
UserInteraction: llm.UserInteractionSelect,
WouldAutoExecute: true,
ServerOrigin: "embedded://mattermost",
}
}

Expand Down
105 changes: 67 additions & 38 deletions conversations/ask_user_question_flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ func TestHandleToolCallAnswersUserQuestion(t *testing.T) {

if tc.wantFollowUp {
assert.Len(t, lm.requests, 1, "expected a follow-up LLM request")
assert.Zero(t, countUserMessagesContaining(lm.requests[0].Posts, llm.ToolRejectionUserMessage),
"answered or skipped questions must not receive tool-rejection guidance")
} else {
assert.Empty(t, lm.requests, "expected no follow-up LLM request")
}
Expand Down Expand Up @@ -395,43 +397,49 @@ func TestStreamToolFollowUpInteractiveFlag(t *testing.T) {
// TestHandleToolCallAutoExecutesPolicyEligiblePendingTools pins the deferred
// auto-execution contract: marked tools run server-side without appearing in
// accepted_tool_ids, including when an interrupted all-auto batch is resumed
// with an empty list. A policy disabled since the pause must fall back to
// rejection.
// with an empty list. A policy or license change since the pause must fall
// back to a non-user-rejection result so the follow-up cannot blame the user.
func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
const origin = "https://jira.example.com"

cases := []struct {
name string
wouldAutoExecute bool
includeQuestion bool
unlicensed bool
policyChecker mapPolicyChecker
wantToolStatus string
wantToolResult string
wantToolShared bool
wantFollowUp bool
name string
wouldAutoExecute bool
includeQuestion bool
unlicensed bool
policyChecker mapPolicyChecker
wantToolStatus string
wantToolResult string
wantToolUseShared bool
wantResultShared bool
wantFollowUp bool
wantRejectionGuidance bool
}{
{
name: "interrupted all-auto batch resumes with empty accepted list",
wouldAutoExecute: true,
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: true}},
},
wantToolStatus: conversation.StatusAutoApproved,
wantToolResult: "restored-result",
wantToolShared: true,
wantFollowUp: true,
wantToolStatus: conversation.StatusAutoApproved,
wantToolResult: "restored-result",
wantToolUseShared: true,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: false,
},
{
name: "interrupted all-auto resume rejects when policy was disabled",
wouldAutoExecute: true,
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: false}},
},
wantToolStatus: conversation.StatusRejected,
wantToolResult: "Tool call rejected by user",
wantToolShared: false,
wantFollowUp: false, // nothing executed, so nothing to follow up on
wantToolStatus: conversation.StatusRejected,
wantToolResult: toolCallPolicyDeniedResult,
wantToolUseShared: false,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: false,
},
{
// Remote MCP tools are license-gated at supply time: an
Expand All @@ -444,10 +452,12 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: true}},
},
wantToolStatus: conversation.StatusRejected,
wantToolResult: "Tool call rejected by user",
wantToolShared: false,
wantFollowUp: false, // nothing executed, so nothing to follow up on
wantToolStatus: conversation.StatusRejected,
wantToolResult: toolCallPolicyDeniedResult,
wantToolUseShared: false,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: false,
},
{
name: "auto_run_everywhere policy executes on resume",
Expand All @@ -456,10 +466,12 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: true}},
},
wantToolStatus: conversation.StatusAutoApproved,
wantToolResult: "restored-result",
wantToolShared: true,
wantFollowUp: true,
wantToolStatus: conversation.StatusAutoApproved,
wantToolResult: "restored-result",
wantToolUseShared: true,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: false,
},
{
name: "policy disabled since the pause rejects instead",
Expand All @@ -468,10 +480,12 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: false}},
},
wantToolStatus: conversation.StatusRejected,
wantToolResult: "Tool call rejected by user",
wantToolShared: false,
wantFollowUp: true, // the answered question still warrants a follow-up
wantToolStatus: conversation.StatusRejected,
wantToolResult: toolCallPolicyDeniedResult,
wantToolUseShared: false,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: false,
},
{
name: "unmarked tool does not auto-run even if policy flipped to auto",
Expand All @@ -480,10 +494,12 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
policyChecker: mapPolicyChecker{
origin: {"get_issue": {policy: mcp.ToolPolicyAutoRunEverywhere, enabled: true}},
},
wantToolStatus: conversation.StatusRejected,
wantToolResult: "Tool call rejected by user",
wantToolShared: false,
wantFollowUp: true,
wantToolStatus: conversation.StatusRejected,
wantToolResult: toolCallRejectedByUserResult,
wantToolUseShared: false,
wantResultShared: true,
wantFollowUp: true,
wantRejectionGuidance: true,
},
}

Expand Down Expand Up @@ -580,8 +596,10 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
var updatedBlocks []conversation.ContentBlock
require.NoError(t, json.Unmarshal(turns[2].Content, &updatedBlocks))
assert.Equal(t, tc.wantToolStatus, updatedBlocks[0].Status)
assert.Equal(t, tc.wouldAutoExecute, updatedBlocks[0].WouldAutoExecute,
"WouldAutoExecute must stay on the block so follow-up guidance can distinguish policy denial from a user rejection")
require.NotNil(t, updatedBlocks[0].Shared)
assert.Equal(t, tc.wantToolShared, *updatedBlocks[0].Shared)
assert.Equal(t, tc.wantToolUseShared, *updatedBlocks[0].Shared)
if tc.includeQuestion {
assert.Equal(t, conversation.StatusSuccess, updatedBlocks[1].Status)
}
Expand All @@ -591,14 +609,25 @@ func TestHandleToolCallAutoExecutesPolicyEligiblePendingTools(t *testing.T) {
require.Len(t, resultBlocks, len(blocks))
assert.Equal(t, tc.wantToolResult, resultBlocks[0].Content)
require.NotNil(t, resultBlocks[0].Shared)
assert.Equal(t, tc.wantToolShared, *resultBlocks[0].Shared)
assert.Equal(t, tc.wantResultShared, *resultBlocks[0].Shared)
assert.NotNil(t, resultBlocks[0].DecidedAt, "auto/rejected results are terminal")
if tc.includeQuestion {
assert.NotNil(t, resultBlocks[1].DecidedAt, "answer result is terminal")
}

if tc.wantFollowUp {
assert.Len(t, lm.requests, 1, "expected a follow-up LLM request")
require.Len(t, lm.requests, 1, "expected a follow-up LLM request")
requestText := completionRequestText(lm.requests[0])
assert.Contains(t, requestText, tc.wantToolResult)
if tc.wantRejectionGuidance {
requireRejectionGuidanceIsFinalUserPost(t, lm.requests[0].Posts)
} else {
assert.Zero(t, countUserMessagesContaining(lm.requests[0].Posts, llm.ToolRejectionUserMessage),
"policy denial and auto-exec must not receive user-rejection guidance")
if tc.wantToolResult != toolCallRejectedByUserResult {
assert.NotContains(t, requestText, toolCallRejectedByUserResult)
}
}
} else {
assert.Empty(t, lm.requests)
}
Expand Down
Loading
Loading