Skip to content

feat(messages): enforce 2000 character message length limit#324

Closed
ptone wants to merge 2 commits into
mainfrom
scion/mi-dev-length
Closed

feat(messages): enforce 2000 character message length limit#324
ptone wants to merge 2 commits into
mainfrom
scion/mi-dev-length

Conversation

@ptone

@ptone ptone commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds MaxMessageLength = 2000 constant in pkg/messages/types.go
  • Validates message content length in StructuredMessage.Validate() before dispatch to any integration
  • Adds early validation in handleAgentOutboundMessage handler for agent outbound messages
  • Returns a clear error with current length and suggestion to split into multiple messages

Test plan

  • Added test for message exceeding character limit (2001 chars)
  • Added test for message at exact character limit (2000 chars)
  • All existing tests pass
  • go build ./... passes

Add MaxMessageLength constant (2000 chars) and validate message content
before dispatch to any integration, returning a clear error suggesting
the sender split into multiple messages.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a 2000-character limit (MaxMessageLength) for messages, implementing validation checks in both the agent messaging handler and the structured message validation logic, along with corresponding unit tests. The review feedback highlights two main areas for improvement: first, using utf8.RuneCountInString instead of len([]rune(...)) to avoid inefficient memory allocations; second, conditionally applying the character limit validation only to outbound agent messages rather than unconditionally to all messages, which could otherwise block valid inbound user messages.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread pkg/messages/types.go
Comment on lines +133 to +135
if len([]rune(m.Msg)) > MaxMessageLength {
return fmt.Errorf("message exceeds %d character limit (current: %d chars). Consider splitting into multiple messages using multiple scion message invocations", MaxMessageLength, len([]rune(m.Msg)))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The comment for MaxMessageLength on line 28 states:

// Maximum length of the Msg field in characters for outbound messages.

However, StructuredMessage.Validate() enforces this 2000-character limit unconditionally on all messages. This includes inbound user-to-agent messages (e.g., instructions from user:alice to agent:backend-dev). Since user prompts, code snippets, or logs can easily exceed 2000 characters, this unconditional limit will block valid inbound messages.

If the limit is indeed only intended for outbound messages (agent-to-user or agent-to-agent), we should conditionally apply this check in Validate(), for example by checking if the sender is an agent using SenderPrefix(m.Sender) == "agent".

Additionally, converting the string to []rune twice (once for the check and once for the error message) is inefficient and causes redundant memory allocations. We should use utf8.RuneCountInString(m.Msg) and store the result in a variable.

Note: You will need to import "unicode/utf8" in this file. Also, since validMsg() in types_test.go uses Sender: "user:alice", you will need to update the test cases ("msg exceeds character limit" and "msg at character limit") to set m.Sender = "agent:some-agent" so that the validation is triggered during the tests.

	if SenderPrefix(m.Sender) == "agent" {
		if msgLen := utf8.RuneCountInString(m.Msg); msgLen > MaxMessageLength {
			return fmt.Errorf("message exceeds %d character limit (current: %d chars). Consider splitting into multiple messages using multiple scion message invocations", MaxMessageLength, msgLen)
		}
	}

Comment thread pkg/hub/handlers_agent_messaging.go Outdated
Comment on lines +71 to +74
if msgLen := len([]rune(req.Msg)); msgLen > messages.MaxMessageLength {
ValidationError(w, fmt.Sprintf("message exceeds %d character limit (current: %d chars). Consider splitting into multiple messages using multiple scion message invocations", messages.MaxMessageLength, msgLen), nil)
return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using len([]rune(req.Msg)) to count characters in Go allocates a new slice of runes, which is inefficient and causes unnecessary memory allocations and garbage collection pressure.

Instead, use utf8.RuneCountInString(req.Msg) from the unicode/utf8 package, which counts the runes in-place with $O(1)$ memory complexity.

Note: You will need to import "unicode/utf8" in this file.

Suggested change
if msgLen := len([]rune(req.Msg)); msgLen > messages.MaxMessageLength {
ValidationError(w, fmt.Sprintf("message exceeds %d character limit (current: %d chars). Consider splitting into multiple messages using multiple scion message invocations", messages.MaxMessageLength, msgLen), nil)
return
}
if msgLen := utf8.RuneCountInString(req.Msg); msgLen > messages.MaxMessageLength {
ValidationError(w, fmt.Sprintf("message exceeds %d character limit (current: %d chars). Consider splitting into multiple messages using multiple scion message invocations", messages.MaxMessageLength, msgLen), nil)
return
}

Replace len([]rune(req.Msg)) with utf8.RuneCountInString(req.Msg) to
avoid allocating a temporary []rune slice. Addresses Gemini PR GoogleCloudPlatform#524
MEDIUM finding.
@ptone ptone closed this Jun 28, 2026
@ptone ptone deleted the scion/mi-dev-length branch June 28, 2026 12:11
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.

1 participant