Conversation
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>
There was a problem hiding this comment.
Pull request overview
This PR adjusts DIFC filter-mode behavior so agents can distinguish “no results” from “results existed but were withheld by DIFC”, specifically by turning the “single expected item was fully filtered” case into an MCP error instead of an empty success response.
Changes:
- Add
buildDIFCSingleItemFilteredError()to produce a consistent[Filtered] ... filtered by <policy> (<reason>)error message for a fully-blocked single item. - Update Phase 5 DIFC filtering in
callBackendToolto return anIsErrorMCP result (HTTP 403) when a collection ends up with exactly one filtered item and zero accessible items. - Add/extend tests covering: multi-item all-filtered notice behavior, single-item filtered error behavior, and genuinely empty collections.
Show a summary per file
| File | Description |
|---|---|
| internal/server/unified.go | Returns an MCP error (IsError) for the “single item fully filtered” case during Phase 5 filtering. |
| internal/server/difc_log.go | Adds helper to build the single-item filtered error message with [Filtered] marker and policy labeling. |
| internal/server/call_backend_tool_difc_test.go | Adds Phase 5 tests for multi-item all-filtered notices, single-item filtered errors, and true empty results. |
| internal/server/difc_log_test.go | Adds unit tests for the single-item filtered error message formatting across policy/reason/description cases. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
| // **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) |
|
@copilot address the review feedback #4427 (review) |
…st 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>
Fixed in 2686cf0. Added |
When DIFC integrity/secrecy filtering removes all items from a tool response, agents receive a result indistinguishable from a genuinely empty repo — causing targeted-dispatch workflows to drift into scheduled mode and report-generating agents to create duplicate issues.
Changes
internal/server/difc_log.go— new helpersbuildDIFCSingleItemFilteredError(): builds a structurederrorfor the single-item-entirely-filtered case with a[Filtered]marker, resource description, policy type (secrecy/integrity/access), and denial reasonisSingularReadTool(toolName): returnstruefor singular-read tools (get_*,*_read) andfalsefor collection tools (list_*,search_*), ensuring the MCP error path only fires for tools that are expected to return a single iteminternal/server/unified.go— Phase 5 single-item branchaccessible == 0 && filtered == 1and the tool is a singular-read (get_*/*_read): returnIsErrorMCP result viabuildDIFCSingleItemFilteredErrorinstead of empty array + text noticelist_*,search_*) with any number of filtered items, or whenfiltered > 1: existing notice behavior preserved — agents see an empty/partial list with a[Filtered]text notice appendedinternal/server/call_backend_tool_difc_test.go— 4 new Phase 5 testsAllItemsFiltered_NoticePresent: multi-item list entirely filtered → notice in content,IsError=falseSingleItemFiltered_ReturnsMCPError: singular-read tool with 1 filtered item →IsError=true, content contains[Filtered]GenuinelyEmptyCollection_NoNotice: truly empty backend result → no notice, no errorListToolSingleItemFiltered_NoticeNotError:list_issuesreturning exactly 1 filtered item → notice only,IsError=false(regression guard)internal/server/difc_log_test.go— unit testsbuildDIFCSingleItemFilteredErrorcovering integrity violation, secrecy violation, missing description, and missing reasonTestIsSingularReadTool: table-driven test covering all key tool name patterns