Skip to content

Conversation

@katzdave
Copy link
Collaborator

@katzdave katzdave commented Sep 5, 2025

Summary

This PR introduces a new message metadata system to centralize visibility control on the server-side and significantly simplifies the client-side message handling by removing redundant visibility flags and ancestor message tracking.

Key Changes

🏗️ Core Architecture Changes

Server-Side (Rust)

  • New MessageMetadata struct with user_visible and agent_visible flags
  • Enhanced Message struct with metadata field and visibility methods:
    • user_only() - visible only to user, not agent
    • agent_only() - visible only to agent, not user
    • with_visibility(user_visible, agent_visible) - custom visibility
    • is_user_visible() / is_agent_visible() - visibility checks
  • Updated message constructors across providers to use new metadata system
  • Improved context management in auto_compact.rs to respect visibility flags

Client-Side (TypeScript)

  • Removed display and sendToLLM fields from Message interface
  • Eliminated ancestor messages concept - redundant with server-side metadata
  • Simplified convertApiMessageToFrontendMessage - no longer sets visibility flags
  • Updated message streaming to trust server-side filtering as single source of truth
  • Removed client-side message filtering in useMessageStream

🔧 Technical Improvements

  1. Single Source of Truth: Server now controls all message visibility logic
  2. Simplified Client Logic: Removed complex client-side filtering and ancestor tracking
  3. Better Type Safety: Cleaner TypeScript interfaces without optional visibility flags
  4. Consistent API: All providers use the same metadata system

🧪 Test Updates

  • Updated all test files to remove references to display/sendToLLM fields
  • Added comprehensive tests for new metadata functionality
  • Fixed context management tests to work with new visibility system

📁 Files Changed

Rust Core (crates/goose/)

  • conversation/message.rs - New metadata system and visibility methods
  • context_mgmt/auto_compact.rs - Respect message visibility in compaction
  • providers/ - Updated all providers to use new message constructors
  • agents/reply_parts.rs - Use new message creation patterns

Server (crates/goose-server/)

  • routes/ - Updated API endpoints to handle metadata properly

Client (ui/desktop/)

  • types/message.ts - Removed visibility fields from interface
  • hooks/ - Simplified message handling logic
  • components/ - Removed ancestor message tracking
  • __tests__/ - Updated all tests for new message structure

Migration Impact

  • Backward Compatible: Old messages without metadata default to fully visible
  • API Changes: Client no longer needs to handle visibility logic
  • Performance: Reduced client-side complexity and memory usage

Benefits

  1. Centralized Control: All visibility logic lives on the server
  2. Reduced Complexity: Simpler client code with fewer edge cases
  3. Better Maintainability: Single place to modify visibility behavior
  4. Type Safety: Cleaner interfaces without optional visibility flags
  5. Performance: Less client-side processing and memory usage

Testing

  • ✅ All existing tests updated and passing
  • ✅ New metadata functionality thoroughly tested
  • ✅ Message serialization/deserialization verified
  • ✅ Context management respects new visibility system

This change sets the foundation for more sophisticated message visibility control while significantly simplifying the client-side codebase.

- Remove display and sendToLLM fields from TypeScript Message interface
- Remove ancestor messages concept (redundant with server-side metadata)
- Update convertApiMessageToFrontendMessage to not set visibility flags
- Fix all test files to remove references to display/sendToLLM
- Trust server-side filtering as single source of truth for message visibility
- Simplify client to just display all messages it receives from server
…data

* 'main' of github.com:block/goose:
  feat(acp): Read files (#4531)
…data

* 'main' of github.com:block/goose:
  docs: add ampersand to link (#4560)
  Add video link to README for user guidance (#4553)
  docs: social channels (#4552)
  feat: simplify navigation, make reload work (#4498)
  docs: new recipe warning (#4545)
  Add AGENTS.md for AI coding assistant support (#4539)
  docs: non-interactive compact now (#4543)
  fixed css classes and added some accessibility fixes (#4492)
…data

* 'main' of github.com:block/goose:
  feat: add auto-compact threshold configuration UI (#4178)
  Add container detection to developer extension (#4559)
…data

* 'main' of github.com:block/goose:
  improve auto scroll to bottom + detection (#4504)
  Fix unable to get access to microphone in Mac app (#4571)
  Use middleware to verify secret key (#4338)
  adding Vercel MCP (#4562)
  docs: reorganizing CLI commands (#4566)
  Desktop import yaml recipes (#4544)
@katzdave katzdave requested a review from alexhancock September 9, 2025 18:56
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

From an API design perspective I wonder if we should consider reusing the concept of "audience". MCP resource annotations for example have the concept of:

audience: ['user' || 'agent']

https://modelcontextprotocol.io/specification/2025-06-18/server/resources#annotations

I am fine to go with the PR as is though as I think having this controlled in the agent is a great step forward! Then can continue to discuss the properties of MessageMetadata

Just flagging it to you now in case you agree audience would be a good idea, and it's not too big a refactor


// Add an assistant message to continue the conversation (agent_visible=true, user_visible=false)
let assistant_message = Message::assistant()
.with_text("The previous message contains a summary that was prepared because a context limit was reached.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The

Do not mention that you read a summary or that conversation summarization occurred
Just continue the conversation naturally based on the summarized context

seemed helpful to me before in getting it not to make a big deal of the fact that it just read a summary. sometimes it still mentions it, but not nearly as much as if these instructions are removed. given the summary is only visible to the agent, and we want it to feel like we've just continued the conversation I wonder if we can keep this in?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or does this have a better solution now, always inserting the last user message as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack added this back. This message is agent visible only so should definitely go with what we tested more.

Ideally this should behave about the same as before; I was mostly testing that session reload seemed to have a reasonable state.

The cool part is we have lots more levers to tweak now; For example since we're keeping all user messages forever, maybe these are actually useful to leave in the session or inject directly in the future summaries.

…data

* 'main' of github.com:block/goose:
  refactor: add new recipe dependency updater (#4596)
  chore: fix nightly builds to have tags (#4595)
  feat: Import file contents from recipe 'file' input type parameter (#4558)
  also adding this change to the api key send for recipes (#4587)
  Fix local (working directory) recipes storage (#4588)
  fix: don't redact tool calls (#4589)
  Prompt injection detection (simplified - only pattern matching) (#4237)
  feat: add streaming support to Tetrate Agent Router Service provider (#4477)
  docs: goosehints updates (#4581)
  Iand/recipe scanner updates (#4584)
  patching recipe scanning workflows for permissions changes (#4579)
  fix: onboarding endpoints send token secret (#4575)
  Fix : Google AI schema validation by adding missing array items fields (#4569)
  Add unified diff support to text editor (#4522)
@katzdave
Copy link
Collaborator Author

From an API design perspective I wonder if we should consider reusing the concept of "audience". MCP resource annotations for example have the concept of:

audience: ['user' || 'agent']

https://modelcontextprotocol.io/specification/2025-06-18/server/resources#annotations

I am fine to go with the PR as is though as I think having this controlled in the agent is a great step forward! Then can continue to discuss the properties of MessageMetadata

Just flagging it to you now in case you agree audience would be a good idea, and it's not too big a refactor

Good callout, agree that MessageMetadata does a pretty good job of encapsulation so we could probably follow up with that change if we want to match that pattern a little more directly.

@katzdave katzdave merged commit 3f17f40 into main Sep 10, 2025
11 checks passed
@katzdave katzdave deleted the dkatz/message-metadata branch September 10, 2025 21:26
michaelneale added a commit that referenced this pull request Sep 10, 2025
* main: (29 commits)
  docs: update built-in extensions list and fix link (#4601)
  Add Message Metadata for Visibility Control (#4538)
  Remove deprecated Claude 3.5 models (#4590)
  Remove unused loadRecipe function (#4599)
  Send the secret with decodeRecipe (#4597)
  fix markdown links overflowing content and hide agent link previews (#4585)
  refactor: add new recipe dependency updater (#4596)
  chore: fix nightly builds to have tags (#4595)
  feat: Import file contents from recipe 'file' input type parameter (#4558)
  also adding this change to the api key send for recipes (#4587)
  Fix local (working directory) recipes storage (#4588)
  fix: don't redact tool calls (#4589)
  Prompt injection detection (simplified - only pattern matching) (#4237)
  feat: add streaming support to Tetrate Agent Router Service provider (#4477)
  docs: goosehints updates (#4581)
  Iand/recipe scanner updates (#4584)
  patching recipe scanning workflows for permissions changes (#4579)
  fix: onboarding endpoints send token secret (#4575)
  Fix : Google AI schema validation by adding missing array items fields (#4569)
  Add unified diff support to text editor (#4522)
  ...
michaelneale added a commit that referenced this pull request Sep 10, 2025
* main: (30 commits)
  docs: update built-in extensions list and fix link (#4601)
  Add Message Metadata for Visibility Control (#4538)
  Remove deprecated Claude 3.5 models (#4590)
  Remove unused loadRecipe function (#4599)
  Send the secret with decodeRecipe (#4597)
  fix markdown links overflowing content and hide agent link previews (#4585)
  refactor: add new recipe dependency updater (#4596)
  chore: fix nightly builds to have tags (#4595)
  feat: Import file contents from recipe 'file' input type parameter (#4558)
  also adding this change to the api key send for recipes (#4587)
  Fix local (working directory) recipes storage (#4588)
  fix: don't redact tool calls (#4589)
  Prompt injection detection (simplified - only pattern matching) (#4237)
  feat: add streaming support to Tetrate Agent Router Service provider (#4477)
  docs: goosehints updates (#4581)
  Iand/recipe scanner updates (#4584)
  patching recipe scanning workflows for permissions changes (#4579)
  fix: onboarding endpoints send token secret (#4575)
  Fix : Google AI schema validation by adding missing array items fields (#4569)
  Add unified diff support to text editor (#4522)
  ...
thebristolsound pushed a commit to thebristolsound/goose that referenced this pull request Sep 11, 2025
@understood-the-assignment
Copy link
Contributor

Hello, it's possible that this pull request introduced a regression: #4635
Could you have a look, please?

@alexhancock alexhancock mentioned this pull request Sep 23, 2025
HikaruEgashira pushed a commit to HikaruEgashira/goose that referenced this pull request Oct 3, 2025
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