Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions internal/server/difc_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"encoding/json"
"fmt"
"strings"

"github.com/github/gh-aw-mcpg/internal/difc"
"github.com/github/gh-aw-mcpg/internal/logger"
Expand Down Expand Up @@ -93,3 +94,59 @@ func extractNumberField(m map[string]interface{}) string {
}
return ""
}

// maxFilteredItemsInNotice is the maximum number of individual item descriptions
// to include inline in the DIFC filtered notice surfaced to the agent.
const maxFilteredItemsInNotice = 5

// buildDIFCFilteredNotice builds a human-readable notice for the agent when items are
// removed from a tool response by DIFC integrity policy in filter/propagate mode.
//
// The notice is surfaced as an additional text content block appended to the tool
// response so that agents (and targeted-dispatch workflows) are aware that items exist
// but were withheld, rather than concluding the result set is genuinely empty.
//
// For up to maxFilteredItemsInNotice items the description and reason for each item are
// included. For larger sets only the count is reported to keep the message concise.
func buildDIFCFilteredNotice(filtered *difc.FilteredCollectionLabeledData) string {
if filtered == nil {
return ""
}
n := filtered.GetFilteredCount()
if n == 0 {
return ""
}

// For a small number of filtered items, include per-item descriptions and reasons.
if n <= maxFilteredItemsInNotice {
parts := make([]string, 0, n)
for _, detail := range filtered.Filtered {
desc := ""
if detail.Item.Labels != nil {
desc = detail.Item.Labels.Description
}
// Skip items that carry no useful identifying information.
if desc == "" && detail.Reason == "" {
continue
}
if desc != "" && detail.Reason != "" {
parts = append(parts, fmt.Sprintf("%s (%s)", desc, detail.Reason))
} else if desc != "" {
parts = append(parts, desc)
} else {
parts = append(parts, detail.Reason)
}
}
if len(parts) > 0 {
return fmt.Sprintf(
"[DIFC] %d item(s) in this response were removed by integrity policy and are not shown: %s.",
n, strings.Join(parts, "; "),
)
Comment on lines +121 to +144
}
}

return fmt.Sprintf(
"[DIFC] %d item(s) in this response were removed by integrity policy and are not shown.",
n,
)
}
100 changes: 100 additions & 0 deletions internal/server/difc_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -274,3 +275,102 @@ func TestBuildFilteredItemLogEntry_NonMapData(t *testing.T) {
assert.Empty(t, entry.AuthorLogin)
})
}

// TestBuildDIFCFilteredNotice_NilInput verifies that a nil input returns an empty string
// without panicking.
func TestBuildDIFCFilteredNotice_NilInput(t *testing.T) {
assert.NotPanics(t, func() {
assert.Empty(t, buildDIFCFilteredNotice(nil))
})
}

// TestBuildDIFCFilteredNotice_EmptyFiltered verifies that no notice is returned when
// there are no filtered items.
func TestBuildDIFCFilteredNotice_EmptyFiltered(t *testing.T) {
f := &difc.FilteredCollectionLabeledData{
Filtered: []difc.FilteredItemDetail{},
}
assert.Empty(t, buildDIFCFilteredNotice(f))
}

// TestBuildDIFCFilteredNotice_SingleItem verifies the notice for a single filtered item
// includes the item description and reason.
func TestBuildDIFCFilteredNotice_SingleItem(t *testing.T) {
f := &difc.FilteredCollectionLabeledData{
Filtered: []difc.FilteredItemDetail{
newTestFilteredItem(nil, "issue:org/repo#14", "integrity too low for agent context", nil, nil),
},
TotalCount: 1,
}

notice := buildDIFCFilteredNotice(f)

assert.NotEmpty(t, notice)
assert.Contains(t, notice, "[DIFC]")
assert.Contains(t, notice, "1 item(s)")
assert.Contains(t, notice, "issue:org/repo#14")
assert.Contains(t, notice, "integrity too low for agent context")
}

// TestBuildDIFCFilteredNotice_MultipleItemsWithinLimit verifies that up to
// maxFilteredItemsInNotice items are listed individually with their descriptions and reasons.
func TestBuildDIFCFilteredNotice_MultipleItemsWithinLimit(t *testing.T) {
f := &difc.FilteredCollectionLabeledData{
Filtered: []difc.FilteredItemDetail{
newTestFilteredItem(nil, "issue:org/repo#1", "integrity too low", nil, nil),
newTestFilteredItem(nil, "issue:org/repo#2", "integrity too low", nil, nil),
newTestFilteredItem(nil, "issue:org/repo#3", "integrity too low", nil, nil),
},
TotalCount: 3,
}

notice := buildDIFCFilteredNotice(f)

assert.NotEmpty(t, notice)
assert.Contains(t, notice, "[DIFC]")
assert.Contains(t, notice, "3 item(s)")
assert.Contains(t, notice, "issue:org/repo#1")
assert.Contains(t, notice, "issue:org/repo#2")
assert.Contains(t, notice, "issue:org/repo#3")
}

// TestBuildDIFCFilteredNotice_ExceedsLimit verifies that when more than
// maxFilteredItemsInNotice items are filtered, only the count is reported.
func TestBuildDIFCFilteredNotice_ExceedsLimit(t *testing.T) {
items := make([]difc.FilteredItemDetail, maxFilteredItemsInNotice+1)
for i := range items {
items[i] = newTestFilteredItem(nil, fmt.Sprintf("issue:org/repo#%d", i+1), "integrity too low", nil, nil)
}
f := &difc.FilteredCollectionLabeledData{
Filtered: items,
TotalCount: len(items),
}

notice := buildDIFCFilteredNotice(f)

assert.NotEmpty(t, notice)
assert.Contains(t, notice, "[DIFC]")
assert.Contains(t, notice, fmt.Sprintf("%d item(s)", len(items)))
// Individual descriptions should NOT appear when the count exceeds the limit.
assert.NotContains(t, notice, "issue:org/repo#1")
}

// TestBuildDIFCFilteredNotice_ItemWithNoDescription verifies that items without
// a description still produce a valid count-only notice.
func TestBuildDIFCFilteredNotice_ItemWithNoDescription(t *testing.T) {
f := &difc.FilteredCollectionLabeledData{
Filtered: []difc.FilteredItemDetail{
{
Item: difc.LabeledItem{Data: "raw", Labels: difc.NewLabeledResource("")},
Reason: "",
},
},
TotalCount: 1,
}

notice := buildDIFCFilteredNotice(f)

assert.NotEmpty(t, notice)
assert.Contains(t, notice, "[DIFC]")
assert.Contains(t, notice, "1 item(s)")
}
12 changes: 12 additions & 0 deletions internal/server/unified.go
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName

// **Phase 5: Reference Monitor performs fine-grained filtering (if applicable)**
var finalResult interface{}
var difcFiltered *difc.FilteredCollectionLabeledData // tracks items removed in filter/propagate mode
if labeledData != nil {
// Guard provided fine-grained labels - check if it's a collection
if collection, ok := labeledData.(*difc.CollectionLabeledData); ok {
Expand Down Expand Up @@ -986,6 +987,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
if filtered.GetFilteredCount() > 0 {
log.Printf("[DIFC] Filtered out %d items due to DIFC policy", filtered.GetFilteredCount())
logFilteredItems(serverID, toolName, filtered)
difcFiltered = filtered
}

// Convert filtered data to result
Expand Down Expand Up @@ -1028,6 +1030,16 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
return newErrorCallToolResult(fmt.Errorf("failed to convert result: %w", err))
}

// If items were filtered by DIFC policy in filter/propagate mode, append a notice so
// the agent knows items exist but were withheld. Without this, an agent receiving an
// empty (or partial) list has no way to distinguish "no items" from "items filtered",
// which can cause targeted-dispatch workflows to silently fall back to scheduled mode.
if difcFiltered != nil {
if notice := buildDIFCFilteredNotice(difcFiltered); notice != "" {
callResult.Content = append(callResult.Content, &sdk.TextContent{Text: notice})
}
}
Comment on lines +1033 to +1041

return callResult, finalResult, nil
}

Expand Down
Loading