Skip to content

feat: auto-prefix bare email recipients with user: in scion message#335

Merged
ptone merged 1 commit into
GoogleCloudPlatform:mainfrom
ptone:fix/bare-email-recipient-auto-prefix
Jun 7, 2026
Merged

feat: auto-prefix bare email recipients with user: in scion message#335
ptone merged 1 commit into
GoogleCloudPlatform:mainfrom
ptone:fix/bare-email-recipient-auto-prefix

Conversation

@ptone
Copy link
Copy Markdown
Member

@ptone ptone commented Jun 7, 2026

Instead of returning an error when a bare email address (without the "user:" prefix) is passed as a recipient to scion message, auto-resolve it as a user recipient by internally prepending "user:". This matches the existing behavior of classifyRecipient() for group (set[]) recipients.

Fixes #159

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

Instead of returning an error when a bare email address (without the
"user:" prefix) is passed as a recipient to `scion message`, auto-resolve
it as a user recipient by internally prepending "user:". This matches
the existing behavior of classifyRecipient() for group (set[]) recipients.

Fixes #159
Copy link
Copy Markdown
Contributor

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

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 updates the recipient parsing logic in cmd/message.go to automatically prepend the user: prefix to bare email addresses instead of returning an error. The corresponding tests in cmd/message_test.go have been updated to reflect this change. Feedback on the tests suggests strengthening the assertions to ensure that bare emails are indeed parsed as user recipients (by asserting the specific "sending messages to users requires Hub mode" error) rather than just asserting the absence of the previous email-specific error.

Comment thread cmd/message_test.go
Comment on lines +1031 to 1034
if err != nil {
assert.NotContains(t, err.Error(), "looks like an email address")
assert.NotContains(t, err.Error(), "missing the \"user:\" prefix")
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current assertions only verify that the error does not contain email-specific prefix messages. However, if the email parsing logic were broken and fell through to treating the email as an agent name, the test would still pass because the resulting error (agent '...' not found or not running) also does not contain those substrings.

To make this test robust and ensure that the bare email is actually parsed as a user recipient, assert that the command fails with the expected user-recipient error (sending messages to users requires Hub mode).

			require.Error(t, err)
			assert.Contains(t, err.Error(), "sending messages to users requires Hub mode")

@ptone ptone merged commit e01216e into GoogleCloudPlatform:main Jun 7, 2026
7 of 8 checks passed
@scion-gteam scion-gteam Bot deleted the fix/bare-email-recipient-auto-prefix branch June 7, 2026 02:20
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