Skip to content

Fix/filter audience 6703 local#6773

Merged
DOsinga merged 8 commits intomainfrom
fix/filter-audience-6703-local
Jan 28, 2026
Merged

Fix/filter audience 6703 local#6773
DOsinga merged 8 commits intomainfrom
fix/filter-audience-6703-local

Conversation

@DOsinga
Copy link
Collaborator

@DOsinga DOsinga commented Jan 28, 2026

Summary

Follow up on: #6728 (comment)

rabi and others added 5 commits January 27, 2026 14:15
Several provider format handlers were sending all tool result content
items to the LLM, including content tagged for User-only audience.
This caused duplicate output when tools (like shell) return separate
content for assistant and user audiences with the same text.

The affected providers now filter to only include content where:
- audience is None (no restriction), or
- audience contains Role::Assistant

Also fixes bedrock.rs to use consistent filter logic.

Change-Id: Iaa49175e513b769b01eb90cd8d8051ac26e81b56
Signed-off-by: rabi <ramishra@redhat.com>
Instead of inline filtering at the Content level in each format handler,
filter at the Message level using the existing filter_for_audience()
method via agent_visible_content(). This:

- Eliminates code duplication across format handlers
- Uses the centralized filtering logic from filter_for_audience
- Makes the code more maintainable and consistent

The filtering now happens once before format_messages/conversion
functions rather than inline during content iteration.

Addresses review feedback from DOsinga on PR #6728.
Instead of inline filtering at the Content level in each format handler,
filter at the Message level using the existing filter_for_audience()
method via agent_visible_content(). This:

- Eliminates code duplication across format handlers
- Uses the centralized filtering logic from filter_for_audience
- Makes the code more maintainable and consistent

The filtering now happens inside format_messages() and similar functions
rather than requiring callers to filter before calling these functions.

Addresses review feedback from DOsinga on PR #6728.
Resolved merge conflicts where an older version of the refactoring
(with comments and filtering in callers) was trying to merge back in.
Kept the current version where filtering is inside format_messages()
and comments are removed.
Copilot AI review requested due to automatic review settings January 28, 2026 16:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the audience-based filtering of tool result content to additional providers and local/CLI integrations, ensuring that only assistant-visible tool output is sent back to the LLM (avoiding duplicated content when tools emit both user- and assistant-audience items).

Changes:

  • Update several provider formatters (Anthropic, Snowflake, Bedrock, OpenAI Responses) to run messages through agent_visible_content() before constructing provider-specific payloads, so tool results respect per-content audience annotations.
  • Add explicit filtering of MCP tool result Content items for the assistant audience in local/CLI providers (cursor_agent, chatgpt_codex) and in toolshim’s text conversion path.
  • Wire in the necessary Role import and adjust Bedrock’s tool result handling to rely on the centralized audience filtering instead of ad-hoc filtering.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/goose/src/providers/toolshim.rs Filters tool result Content items to only those with no audience or Role::Assistant when converting tool messages to plain text for toolshim mode.
crates/goose/src/providers/formats/snowflake.rs Builds a filtered message list using agent_visible_content() so Snowflake message formatting only includes assistant-visible content and tool results.
crates/goose/src/providers/formats/openai_responses.rs Uses agent_visible_content() when replaying function call outputs, ensuring only assistant-visible tool result content is sent back via the OpenAI Responses API.
crates/goose/src/providers/formats/bedrock.rs Wraps Bedrock message construction in agent_visible_content(), centralizing audience filtering for all Bedrock tool result content.
crates/goose/src/providers/formats/anthropic.rs Runs Anthropic messages through agent_visible_content() before formatting, so tool results sent to Anthropic are filtered by audience.
crates/goose/src/providers/cursor_agent.rs Filters MCP tool result content by assistant audience when embedding it into the cursor-agent CLI prompt, preventing user-only items from being echoed twice.
crates/goose/src/providers/chatgpt_codex.rs Filters tool result content by assistant audience when emitting function_call_output items for the ChatGPT Codex backend.

Douwe Osinga added 3 commits January 28, 2026 11:34
Apply the same refactoring to chatgpt_codex, cursor_agent, and toolshim
providers that was done for anthropic, bedrock, snowflake, and
openai_responses. This removes all inline audience filtering by using
agent_visible_content() to filter at the Message level.
Apply the same refactoring to databricks and openai format modules.
Also fix filter_for_audience() to preserve empty ToolResponses, as
some providers (like Google) need special handling for them.

Note: Google format was not refactored because it needs to preserve
Thinking content, which agent_visible_content() filters out.
Copy link
Collaborator

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we should be doing this before the provider ever gets the messages. Like the .complete() methods on Provider should take a Conversation type that we produce with Conversation::for_agent or something like that

}

// Preserve ToolResponse even when content is empty - some providers
// (like Google) need to handle empty tool responses specially
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without the deleted code, this comment seems a little out of place, you could just drop it

let filtered_messages: Vec<Message> = messages
.iter()
.filter(|m| m.is_agent_visible())
.map(|m| m.agent_visible_content())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe agent_visible_content should return an Option<Message> (None if is_agent_visible would return false) and you could .filter_map these

@DOsinga
Copy link
Collaborator Author

DOsinga commented Jan 28, 2026

It seems like we should be doing this before the provider ever gets the messages. Like the .complete() methods on Provider should take a Conversation type that we produce with Conversation::for_agent or something like that

I agree. this is an adaption of a PR that came in though, so don't want to change too much

@DOsinga DOsinga merged commit 2bc3689 into main Jan 28, 2026
16 of 18 checks passed
@DOsinga DOsinga deleted the fix/filter-audience-6703-local branch January 28, 2026 20:49
lifeizhou-ap added a commit that referenced this pull request Jan 28, 2026
* main: (47 commits)
  Upgrade error handling (#6747)
  Fix/filter audience 6703 local (#6773)
  chore: re-sync package-lock.json (#6783)
  upgrade electron to 39.3.0 (#6779)
  allow skipping providers in test_providers.sh (#6778)
  fix: enable custom model entry for OpenRouter provider (#6761)
  Remove codex skills flag support (#6775)
  Improve mcp test (#6671)
  Feat/anthropic custom headers (#6774)
  Fix/GitHub copilot error handling 5845 (#6771)
  fix(ui): respect width parameter in MCP app size-changed notifications (#6376)
  fix: address compilation issue in main (#6776)
  Upgrade GitHub Actions for Node 24 compatibility (#6699)
  fix(google): preserve thought signatures in streaming responses (#6708)
  added reduce motion support for css animations and streaming text (#6551)
  fix: Re-enable subagents for Gemini models (#6513)
  fix(google): use parametersJsonSchema for full JSON Schema support (#6555)
  fix: respect GOOSE_CLI_MIN_PRIORITY for shell streaming output (#6558)
  feat: add requires_auth flag for custom providers without authentication (#6705)
  fix: normalize extension names consistently in ExtensionManager (#6529)
  ...
michaelneale added a commit that referenced this pull request Jan 29, 2026
* main: (30 commits)
  Different approach to determining final confidence level of prompt injection evaluation outcomes (#6729)
  fix: read_resource_tool deadlock causing test_compaction to hang (#6737)
  Upgrade error handling (#6747)
  Fix/filter audience 6703 local (#6773)
  chore: re-sync package-lock.json (#6783)
  upgrade electron to 39.3.0 (#6779)
  allow skipping providers in test_providers.sh (#6778)
  fix: enable custom model entry for OpenRouter provider (#6761)
  Remove codex skills flag support (#6775)
  Improve mcp test (#6671)
  Feat/anthropic custom headers (#6774)
  Fix/GitHub copilot error handling 5845 (#6771)
  fix(ui): respect width parameter in MCP app size-changed notifications (#6376)
  fix: address compilation issue in main (#6776)
  Upgrade GitHub Actions for Node 24 compatibility (#6699)
  fix(google): preserve thought signatures in streaming responses (#6708)
  added reduce motion support for css animations and streaming text (#6551)
  fix: Re-enable subagents for Gemini models (#6513)
  fix(google): use parametersJsonSchema for full JSON Schema support (#6555)
  fix: respect GOOSE_CLI_MIN_PRIORITY for shell streaming output (#6558)
  ...
@rabi
Copy link
Contributor

rabi commented Jan 29, 2026

It seems like we should be doing this before the provider ever gets the messages. Like the .complete() methods on Provider should take a Conversation type that we produce with Conversation::for_agent or something like that

Thanks @DOsinga @jamadeo for this. I've updated #6728 to move the filtering to stream_response_from_provider() in reply_parts.rs, so providers receive pre-filtered messages. Please check if that makes sense.

zanesq added a commit that referenced this pull request Jan 29, 2026
…sion-session

* 'main' of github.com:block/goose: (78 commits)
  copilot instructions: Update "No prerelease docs" instruction (#6795)
  refactor: centralize audience filtering before providers receive messages (#6728)
  update doc to remind contributors to activate hermit and document minimal npm and node version (#6727)
  nit: don't spit out compaction when in term mode as it fills up the screen (#6799)
  fix: correct tool support detection in Tetrate provider model fetching (#6808)
  Session manager fixes (#6809)
  fix(desktop): handle quoted paths with spaces in extension commands (#6430)
  fix: we can default gooseignore without writing it out (#6802)
  fix broken link (#6810)
  docs: add Beads MCP extension tutorial (#6792)
  feat(goose): add support for AWS_BEARER_TOKEN_BEDROCK environment variable (#6739)
  [docs] Add OSS Skills Marketplace (#6752)
  feat: make skills available in codemode (#6763)
  Fix: Recipe Extensions Not Loading in Desktop (#6777)
  Different approach to determining final confidence level of prompt injection evaluation outcomes (#6729)
  fix: read_resource_tool deadlock causing test_compaction to hang (#6737)
  Upgrade error handling (#6747)
  Fix/filter audience 6703 local (#6773)
  chore: re-sync package-lock.json (#6783)
  upgrade electron to 39.3.0 (#6779)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants