From 8b4352365db840cb6fce7c551f3405a144886453 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:38:19 +0000 Subject: [PATCH 1/3] Initial plan From 450141bc20ca1071aa224e88475eca04660b6520 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:50:48 +0000 Subject: [PATCH 2/3] Add DIFC filtering metadata: single-item error + filter notices + tests Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/b1eeb6c5-66b6-45e8-909d-8467b1f2da0e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../server/call_backend_tool_difc_test.go | 163 ++++++++++++++++++ internal/server/difc_log.go | 25 +++ internal/server/difc_log_test.go | 60 +++++++ internal/server/unified.go | 12 ++ 4 files changed, 260 insertions(+) diff --git a/internal/server/call_backend_tool_difc_test.go b/internal/server/call_backend_tool_difc_test.go index f9b70d9e7..d39b9ddc6 100644 --- a/internal/server/call_backend_tool_difc_test.go +++ b/internal/server/call_backend_tool_difc_test.go @@ -524,6 +524,169 @@ func TestCallBackendTool_Phase5_NilLabeledData_PassesBackendResult(t *testing.T) assert.NotNil(data, "backend result should be passed through when LabelResponse returns nil") } +// TestCallBackendTool_Phase5_FilterMode_AllItemsFiltered_NoticePresent verifies that +// when all items in a multi-item collection are filtered in filter mode the result is +// not an error but the DIFC filter notice IS present so the agent knows items exist +// but were withheld. +func TestCallBackendTool_Phase5_FilterMode_AllItemsFiltered_NoticePresent(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + listResponse := map[string]interface{}{ + "content": []map[string]interface{}{ + {"type": "text", "text": `[{"id":1},{"id":2}]`}, + }, + "isError": false, + } + backend := newBackendWithToolResponse(t, "list_issues", listResponse) + defer backend.Close() + + // Both items carry a restricted secrecy tag the agent does not have. + item1Labels := difc.NewLabeledResource("restricted item 1") + item1Labels.Secrecy.Label.Add(difc.Tag("private:restricted/repo")) + item2Labels := difc.NewLabeledResource("restricted item 2") + item2Labels.Secrecy.Label.Add(difc.Tag("private:restricted/repo")) + allFilteredCollection := &difc.CollectionLabeledData{ + Items: []difc.LabeledItem{ + {Data: map[string]interface{}{"id": 1}, Labels: item1Labels}, + {Data: map[string]interface{}{"id": 2}, Labels: item2Labels}, + }, + } + + g := &difcTestGuard{ + name: "difc-phase5-all-filtered-guard", + labelResponseResult: allFilteredCollection, + } + g.labelAgentResult = &guard.LabelAgentResult{ + Agent: guard.AgentLabelsPayload{Secrecy: []string{}, Integrity: []string{}}, + DIFCMode: "filter", + NormalizedPolicy: map[string]interface{}{ + "scope_kind": "all", + "min-integrity": "none", + }, + } + us := makeUnifiedWithGuard(t, "difc-phase5-all-filtered-type", g, backend, "filter") + + result, _, err := us.callBackendTool(callCtx("session-p5af"), "test-server", "list_issues", nil) + + require.NotNil(result) + assert.NoError(err, "filter mode with all items filtered must not return a Go error") + assert.False(result.IsError, "result must not be marked IsError when multiple items are all filtered") + + // The filter notice must be present so the agent knows items exist but were withheld. + var foundNotice bool + for _, c := range result.Content { + if tc, ok := c.(*sdk.TextContent); ok { + if strings.Contains(tc.Text, "[Filtered]") || strings.Contains(tc.Text, "filtered") { + foundNotice = true + break + } + } + } + assert.True(foundNotice, "result must contain a DIFC filter notice when multiple items are all filtered") +} + +// TestCallBackendTool_Phase5_FilterMode_SingleItemFiltered_ReturnsMCPError verifies +// that when exactly one item is present and it is entirely filtered in filter mode, +// callBackendTool returns an IsError CallToolResult (MCP error) whose text includes +// "[Filtered]". This prevents agents from treating a filtered single-item read +// (e.g. issue_read) as "resource not found". +func TestCallBackendTool_Phase5_FilterMode_SingleItemFiltered_ReturnsMCPError(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + listResponse := map[string]interface{}{ + "content": []map[string]interface{}{ + {"type": "text", "text": `[{"id":42,"number":42}]`}, + }, + "isError": false, + } + backend := newBackendWithToolResponse(t, "issue_read", listResponse) + defer backend.Close() + + // Single item with a restricted secrecy tag that the agent does not hold. + // The DIFC read check for secrecy requires resource.Secrecy ⊆ agent.Secrecy + // (agent must have all secrecy tags the resource has). Since the agent has no + // secrecy tags and the item requires "private:org/repo", the check fails. + singleItemLabels := difc.NewLabeledResource("issue:org/repo#42") + singleItemLabels.Secrecy.Label.Add(difc.Tag("private:org/repo")) + singleItemCollection := &difc.CollectionLabeledData{ + Items: []difc.LabeledItem{ + {Data: map[string]interface{}{"id": 42, "number": float64(42)}, Labels: singleItemLabels}, + }, + } + + g := &difcTestGuard{ + name: "difc-phase5-single-filtered-guard", + labelResponseResult: singleItemCollection, + } + g.labelAgentResult = &guard.LabelAgentResult{ + Agent: guard.AgentLabelsPayload{Secrecy: []string{}, Integrity: []string{}}, + DIFCMode: "filter", + NormalizedPolicy: map[string]interface{}{ + "scope_kind": "all", + "min-integrity": "none", + }, + } + us := makeUnifiedWithGuard(t, "difc-phase5-single-filtered-type", g, backend, "filter") + + result, _, err := us.callBackendTool(callCtx("session-p5sf"), "test-server", "issue_read", nil) + + require.NotNil(result) + assert.True(result.IsError, "single filtered item must produce an IsError MCP result") + require.Error(err, "a Go error must accompany the IsError result") + assert.Contains(err.Error(), "[Filtered]", + "error message must contain [Filtered] marker") +} + +// TestCallBackendTool_Phase5_FilterMode_GenuinelyEmptyCollection_NoNotice verifies +// that when the backend returns an empty collection (no items at all, not filtered), +// the result is an empty success with no DIFC filter notice. +func TestCallBackendTool_Phase5_FilterMode_GenuinelyEmptyCollection_NoNotice(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + listResponse := map[string]interface{}{ + "content": []map[string]interface{}{ + {"type": "text", "text": `[]`}, + }, + "isError": false, + } + backend := newBackendWithToolResponse(t, "list_issues", listResponse) + defer backend.Close() + + // Guard returns an empty collection — no items at all. + emptyCollection := &difc.CollectionLabeledData{Items: []difc.LabeledItem{}} + + g := &difcTestGuard{ + name: "difc-phase5-empty-guard", + labelResponseResult: emptyCollection, + } + g.labelAgentResult = &guard.LabelAgentResult{ + Agent: guard.AgentLabelsPayload{Secrecy: []string{}, Integrity: []string{}}, + DIFCMode: "filter", + NormalizedPolicy: map[string]interface{}{ + "scope_kind": "all", + "min-integrity": "none", + }, + } + us := makeUnifiedWithGuard(t, "difc-phase5-empty-type", g, backend, "filter") + + result, _, err := us.callBackendTool(callCtx("session-p5e"), "test-server", "list_issues", nil) + + require.NotNil(result) + assert.NoError(err, "genuinely empty collection must not produce a Go error") + assert.False(result.IsError, "genuinely empty collection must not be an MCP error") + + // No filter notice should be present because no items were withheld. + for _, c := range result.Content { + if tc, ok := c.(*sdk.TextContent); ok { + assert.NotContains(tc.Text, "[Filtered]", + "filter notice must NOT be present when the collection is genuinely empty") + } + } +} + // ─── Phase 6: Label accumulation ───────────────────────────────────────────── // TestCallBackendTool_Phase6_PropagateModeAccumulatesLabels verifies that in diff --git a/internal/server/difc_log.go b/internal/server/difc_log.go index 06b931be9..eacd63db3 100644 --- a/internal/server/difc_log.go +++ b/internal/server/difc_log.go @@ -104,6 +104,31 @@ func extractNumberField(m map[string]interface{}) string { // to include inline in the DIFC filtered notice surfaced to the agent. const maxFilteredItemsInNotice = 5 +// buildDIFCSingleItemFilteredError constructs an error for when exactly one item is +// entirely blocked by DIFC policy. Unlike the notice approach (which appends a text +// annotation to a partial or empty list), this returns an actual Go error that the caller +// can surface as an MCP IsError result. It prevents agents from misinterpreting a +// "filtered" single-item response (e.g. issue_read) as "resource not found". +func buildDIFCSingleItemFilteredError(detail difc.FilteredItemDetail) error { + policyLabel := difcPolicyLabel([]difc.FilteredItemDetail{detail}) + + desc := "" + if detail.Item.Labels != nil { + desc = detail.Item.Labels.Description + } + + var msg string + if desc != "" { + msg = fmt.Sprintf("[Filtered] %s exists but is not accessible — filtered by %s", desc, policyLabel) + } else { + msg = fmt.Sprintf("[Filtered] resource exists but is not accessible — filtered by %s", policyLabel) + } + if detail.Reason != "" { + msg = fmt.Sprintf("%s (%s)", msg, detail.Reason) + } + return fmt.Errorf("%s", msg) +} + // buildDIFCFilteredNotice builds a human-readable notice for the agent when items are // removed from a tool response by DIFC policy in filter/propagate mode. // diff --git a/internal/server/difc_log_test.go b/internal/server/difc_log_test.go index 519cfdcb4..4259d4eaf 100644 --- a/internal/server/difc_log_test.go +++ b/internal/server/difc_log_test.go @@ -485,3 +485,63 @@ func TestDifcPolicyLabel(t *testing.T) { }) } } + +// TestBuildDIFCSingleItemFilteredError_IntegrityViolation verifies that an integrity +// violation produces an error message containing [Filtered], the resource description, +// and the denial reason. +func TestBuildDIFCSingleItemFilteredError_IntegrityViolation(t *testing.T) { + detail := newIntegrityFilteredItem("issue:org/repo#42", "integrity too low for agent context") + + err := buildDIFCSingleItemFilteredError(detail) + + require.Error(t, err) + assert.Contains(t, err.Error(), "[Filtered]") + assert.Contains(t, err.Error(), "issue:org/repo#42") + assert.Contains(t, err.Error(), "integrity policy") + assert.Contains(t, err.Error(), "integrity too low for agent context") +} + +// TestBuildDIFCSingleItemFilteredError_SecrecyViolation verifies that a secrecy +// violation produces an error message containing "secrecy policy". +func TestBuildDIFCSingleItemFilteredError_SecrecyViolation(t *testing.T) { + detail := newSecrecyFilteredItem("resource:actions_get", "secrecy requirements not met") + + err := buildDIFCSingleItemFilteredError(detail) + + require.Error(t, err) + assert.Contains(t, err.Error(), "[Filtered]") + assert.Contains(t, err.Error(), "resource:actions_get") + assert.Contains(t, err.Error(), "secrecy policy") + assert.Contains(t, err.Error(), "secrecy requirements not met") +} + +// TestBuildDIFCSingleItemFilteredError_NoDescription verifies that a missing description +// falls back to a generic "resource" label and still produces a valid error. +func TestBuildDIFCSingleItemFilteredError_NoDescription(t *testing.T) { + detail := difc.FilteredItemDetail{ + Item: difc.LabeledItem{Labels: difc.NewLabeledResource("")}, + Reason: "integrity too low", + IsSecrecyViolation: false, + } + + err := buildDIFCSingleItemFilteredError(detail) + + require.Error(t, err) + assert.Contains(t, err.Error(), "[Filtered]") + assert.Contains(t, err.Error(), "resource exists but is not accessible") + assert.Contains(t, err.Error(), "integrity policy") +} + +// TestBuildDIFCSingleItemFilteredError_NoReason verifies that a missing reason still +// produces a valid error that is not blank. +func TestBuildDIFCSingleItemFilteredError_NoReason(t *testing.T) { + detail := newIntegrityFilteredItem("issue:org/repo#7", "") + + err := buildDIFCSingleItemFilteredError(detail) + + require.Error(t, err) + assert.Contains(t, err.Error(), "[Filtered]") + assert.Contains(t, err.Error(), "issue:org/repo#7") + // No trailing "()" should appear when reason is empty. + assert.NotContains(t, err.Error(), "()") +} diff --git a/internal/server/unified.go b/internal/server/unified.go index f481ddde2..89be3d83e 100644 --- a/internal/server/unified.go +++ b/internal/server/unified.go @@ -625,6 +625,18 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName if filtered.GetFilteredCount() > 0 { logUnified.Printf("[DIFC] Filtered out %d items due to DIFC policy", filtered.GetFilteredCount()) logFilteredItems(serverID, toolName, filtered) + + // **Single-item entirely filtered**: return a structured MCP error so the agent + // cannot misinterpret "filtered" as "resource not found" (e.g. issue_read). + // When exactly one item was expected and it is blocked, an error is clearer + // than an empty array accompanied by a text notice. + if filtered.GetAccessibleCount() == 0 && filtered.GetFilteredCount() == 1 { + filteredErr := buildDIFCSingleItemFilteredError(filtered.Filtered[0]) + logger.LogWarn("difc", "Single item filtered — returning MCP error: %v", filteredErr) + httpStatusCode = 403 + return mcp.NewErrorCallToolResult(filteredErr) + } + difcFiltered = filtered } From 2686cf0aaf0f4e1c05d800f901a954c50ed71bcc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:05:02 +0000 Subject: [PATCH 3/3] Gate single-item MCP error on isSingularReadTool to avoid breaking list tools Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/29d4dec2-93e8-4a77-99e5-008db8696a62 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../server/call_backend_tool_difc_test.go | 61 +++++++++++++++++++ internal/server/difc_log.go | 12 ++++ internal/server/difc_log_test.go | 31 ++++++++++ internal/server/unified.go | 8 ++- 4 files changed, 109 insertions(+), 3 deletions(-) diff --git a/internal/server/call_backend_tool_difc_test.go b/internal/server/call_backend_tool_difc_test.go index d39b9ddc6..5e924b5d2 100644 --- a/internal/server/call_backend_tool_difc_test.go +++ b/internal/server/call_backend_tool_difc_test.go @@ -687,6 +687,67 @@ func TestCallBackendTool_Phase5_FilterMode_GenuinelyEmptyCollection_NoNotice(t * } } +// TestCallBackendTool_Phase5_FilterMode_ListToolSingleItemFiltered_NoticeNotError +// verifies that when a list tool (list_issues) returns exactly ONE item and that item +// is filtered, the result is a notice-bearing success — NOT an MCP error. This guards +// against the regression where the single-item MCP error path was incorrectly triggered +// for collection tools with a 1-element result set. +func TestCallBackendTool_Phase5_FilterMode_ListToolSingleItemFiltered_NoticeNotError(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + listResponse := map[string]interface{}{ + "content": []map[string]interface{}{ + {"type": "text", "text": `[{"id":1,"title":"only issue"}]`}, + }, + "isError": false, + } + backend := newBackendWithToolResponse(t, "list_issues", listResponse) + defer backend.Close() + + // Single item with a restricted secrecy tag — the agent has no secrecy tags. + itemLabels := difc.NewLabeledResource("issue:org/repo#1") + itemLabels.Secrecy.Label.Add(difc.Tag("private:org/repo")) + singleItemCollection := &difc.CollectionLabeledData{ + Items: []difc.LabeledItem{ + {Data: map[string]interface{}{"id": 1}, Labels: itemLabels}, + }, + } + + g := &difcTestGuard{ + name: "difc-phase5-list-single-filtered-guard", + labelResponseResult: singleItemCollection, + } + g.labelAgentResult = &guard.LabelAgentResult{ + Agent: guard.AgentLabelsPayload{Secrecy: []string{}, Integrity: []string{}}, + DIFCMode: "filter", + NormalizedPolicy: map[string]interface{}{ + "scope_kind": "all", + "min-integrity": "none", + }, + } + us := makeUnifiedWithGuard(t, "difc-phase5-list-single-filtered-type", g, backend, "filter") + + result, _, err := us.callBackendTool(callCtx("session-p5lsf"), "test-server", "list_issues", nil) + + require.NotNil(result) + // list_issues is a collection tool — even with exactly 1 filtered item it must NOT + // return an MCP error. The agent should see an empty list + filter notice. + assert.NoError(err, "list tool with 1 filtered item must not return a Go error") + assert.False(result.IsError, "list tool with 1 filtered item must not be IsError") + + var foundNotice bool + for _, c := range result.Content { + if tc, ok := c.(*sdk.TextContent); ok { + if strings.Contains(tc.Text, "[Filtered]") { + foundNotice = true + break + } + } + } + assert.True(foundNotice, "list tool with 1 filtered item must still carry a filter notice") +} + // ─── Phase 6: Label accumulation ───────────────────────────────────────────── // TestCallBackendTool_Phase6_PropagateModeAccumulatesLabels verifies that in diff --git a/internal/server/difc_log.go b/internal/server/difc_log.go index eacd63db3..59d16978f 100644 --- a/internal/server/difc_log.go +++ b/internal/server/difc_log.go @@ -104,6 +104,18 @@ func extractNumberField(m map[string]interface{}) string { // to include inline in the DIFC filtered notice surfaced to the agent. const maxFilteredItemsInNotice = 5 +// isSingularReadTool returns true when toolName refers to a tool that is expected +// to return a single item (e.g. issue_read, get_issue, pull_request_read). List and +// search tools — whose result set is inherently variable and may legitimately contain +// just one matching item — return false so they keep the notice-only behavior even +// when that single item is filtered. +// +// The heuristic: tools with the "list_" or "search_" prefix are collection tools; +// everything else (get_*, *_read, etc.) is treated as a singular read. +func isSingularReadTool(toolName string) bool { + return !strings.HasPrefix(toolName, "list_") && !strings.HasPrefix(toolName, "search_") +} + // buildDIFCSingleItemFilteredError constructs an error for when exactly one item is // entirely blocked by DIFC policy. Unlike the notice approach (which appends a text // annotation to a partial or empty list), this returns an actual Go error that the caller diff --git a/internal/server/difc_log_test.go b/internal/server/difc_log_test.go index 4259d4eaf..1bf2ecce7 100644 --- a/internal/server/difc_log_test.go +++ b/internal/server/difc_log_test.go @@ -545,3 +545,34 @@ func TestBuildDIFCSingleItemFilteredError_NoReason(t *testing.T) { // No trailing "()" should appear when reason is empty. assert.NotContains(t, err.Error(), "()") } + +// TestIsSingularReadTool verifies the heuristic that distinguishes singular-read tools +// (get_*, *_read) from collection tools (list_*, search_*). +func TestIsSingularReadTool(t *testing.T) { + tests := []struct { + toolName string + singular bool + }{ + {"issue_read", true}, + {"pull_request_read", true}, + {"get_issue", true}, + {"get_pull_request", true}, + {"get_file_contents", true}, + {"get_repository", true}, + {"get_commit", true}, + {"list_issues", false}, + {"list_pull_requests", false}, + {"list_commits", false}, + {"list_branches", false}, + {"search_issues", false}, + {"search_pull_requests", false}, + {"search_code", false}, + {"search_repositories", false}, + } + for _, tc := range tests { + t.Run(tc.toolName, func(t *testing.T) { + assert.Equal(t, tc.singular, isSingularReadTool(tc.toolName), + "isSingularReadTool(%q) should be %v", tc.toolName, tc.singular) + }) + } +} diff --git a/internal/server/unified.go b/internal/server/unified.go index 89be3d83e..503337196 100644 --- a/internal/server/unified.go +++ b/internal/server/unified.go @@ -628,9 +628,11 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName // **Single-item entirely filtered**: return a structured MCP error so the agent // cannot misinterpret "filtered" as "resource not found" (e.g. issue_read). - // When exactly one item was expected and it is blocked, an error is clearer - // than an empty array accompanied by a text notice. - if filtered.GetAccessibleCount() == 0 && filtered.GetFilteredCount() == 1 { + // Only apply this to singular-read tools (get_*, *_read). Collection tools + // (list_*, search_*) may legitimately return exactly one item that gets filtered + // and should still receive the notice-only behavior so agents see an empty list + // rather than an unexpected error. + if isSingularReadTool(toolName) && filtered.GetAccessibleCount() == 0 && filtered.GetFilteredCount() == 1 { filteredErr := buildDIFCSingleItemFilteredError(filtered.Filtered[0]) logger.LogWarn("difc", "Single item filtered — returning MCP error: %v", filteredErr) httpStatusCode = 403