feat: add --from flag and default-sender, add screening new mail control from cli#73
Open
erikd234 wants to merge 6 commits intobasecamp:mainfrom
Open
feat: add --from flag and default-sender, add screening new mail control from cli#73erikd234 wants to merge 6 commits intobasecamp:mainfrom
erikd234 wants to merge 6 commits intobasecamp:mainfrom
Conversation
- Add --from flag to compose and reply commands to override sender - Add default_sender config (hey config set/get/unset default-sender) - Priority: --from flag > config default_sender > HEY default - Resolves sender email to ID via identity endpoint - Bypasses SDK service methods when override is active to control acting_sender_id
…pdate - Config tests: default value, global config load, set/unset, save/reload, JSON omitempty, Values() output - Cmd tests: normalizeConfigKey hyphen-to-underscore conversion - Update surface snapshot for new --from flags and config get/unset commands
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for overriding the sender identity for compose/reply via a new --from flag and a persisted default_sender config value.
Changes:
- Introduces
default_senderconfiguration (load/save/show, set/get/unset) with source tracking. - Adds sender resolution via the identity endpoint and uses
acting_sender_idwhen an override is active. - Extends CLI surface/options for
composeandreplywith--from.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config_default_sender_test.go | Adds unit tests covering default_sender behavior and persistence. |
| internal/config/config.go | Adds DefaultSender field, load/save behavior, values listing, and unset support. |
| internal/cmd/sender_test.go | Tests CLI config-key normalization (hyphen → underscore). |
| internal/cmd/sender.go | Implements sender ID resolution and sender selection priority. |
| internal/cmd/reply.go | Adds --from and bypasses SDK service call to set acting_sender_id when needed. |
| internal/cmd/config.go | Adds config get/unset, supports default_sender, improves key normalization and errors. |
| internal/cmd/compose.go | Adds --from and bypasses SDK service call to set acting_sender_id when needed. |
| .surface | Updates surfaced commands/flags for completions/docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+138
to
+140
| "entry": map[string]any{ | ||
| "addressed": addressed, | ||
| }, |
Comment on lines
+31
to
+34
| return 0, output.ErrUsage(fmt.Sprintf( | ||
| "no sender matching %q (available: %s)", | ||
| email, strings.Join(available, ", "), | ||
| )) |
Comment on lines
212
to
+223
| } | ||
| return nil | ||
| } | ||
|
|
||
| // UnsetField clears a configuration field so it reverts to default. | ||
| func (c *Config) UnsetField(key string) { | ||
| switch key { | ||
| case "default_sender": | ||
| c.DefaultSender = "" | ||
| if c.sources != nil { | ||
| c.sources["default_sender"] = SourceDefault | ||
| } |
Comment on lines
+95
to
+106
| hasSenderOverride := c.from != "" || cfg.DefaultSender != "" | ||
| if hasSenderOverride { | ||
| senderID, err := effectiveSenderID(ctx, c.from) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| body := map[string]any{ | ||
| "acting_sender_id": senderID, | ||
| "message": map[string]any{ | ||
| "content": message, | ||
| }, | ||
| } |
Adds 'hey screener' with subcommands: - list: show pending screener items - approve <id>: screen in to Imbox - deny <id>: screen out - spam <id>: mark as spam - feed <id>: screen in to The Feed - trail <id>: screen in to Paper Trail Uses the /clearances HTML endpoint for listing and PATCH /clearances/<id> for actions, since the SDK doesn't have a Clearances service yet.
- 12 tests covering list (empty/populated), approve, deny, spam, feed, trail, and missing-args validation for all subcommands - httptest server mocks the /clearances HTML and PATCH endpoints - Fixed feed/trail box ID regex to use (?s) for cross-line matching - Updated surface snapshot for new screener commands
🔍 New:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds sender override support for hey compose/hey reply via a new --from flag and persisted default_sender config, and introduces a new hey screener command to manage pending sender approvals from the CLI.
Changes:
- Add
default_senderto config (load/save/show, set/get/unset) with hyphen/underscore key normalization. - Add
--fromto compose/reply and resolve sender email toacting_sender_idvia identity endpoint, bypassing SDK helpers when overridden. - Add
hey screenercommand (plus tests) to list and action pending clearances using HTML scraping + PATCH.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Adds DefaultSender field, sources tracking, flag setter, unset helper, and includes value in Values(). |
| internal/config/config_default_sender_test.go | Adds tests for default_sender defaults, persistence, sources, and JSON omission when empty. |
| internal/cmd/config.go | Adds config key normalization + get/unset subcommands and includes default_sender in show. |
| internal/cmd/compose.go | Adds --from and override path that posts raw mutations with acting_sender_id. |
| internal/cmd/reply.go | Adds --from and override path that posts raw mutations with acting_sender_id. |
| internal/cmd/sender.go | Introduces sender email → sender ID resolution and “effective sender” selection logic. |
| internal/cmd/sender_test.go | Tests config key normalization behavior. |
| internal/cmd/screener.go | Introduces hey screener command, HTML scraping for items/box IDs, and PATCH actions. |
| internal/cmd/screener_test.go | Adds integration-style tests with an HTTP test server for list/actions and missing-arg errors. |
| internal/cmd/root.go | Registers the new screener command. |
| .surface | Updates CLI surface list with new flags/subcommands (from/get/unset/screener). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+212
to
+222
| // Extract emails | ||
| emailRe := regexp.MustCompile(`[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}`) | ||
| allEmails := emailRe.FindAllString(body, -1) | ||
|
|
||
| // Filter out the user's own emails (they appear in nav/header) | ||
| senderEmails := []string{} | ||
| for _, e := range allEmails { | ||
| lower := strings.ToLower(e) | ||
| if lower != "erik.dahl@hey.com" && lower != "erik@parrotapp.com" { | ||
| senderEmails = append(senderEmails, e) | ||
| } |
Comment on lines
+212
to
+214
| // Extract emails | ||
| emailRe := regexp.MustCompile(`[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}`) | ||
| allEmails := emailRe.FindAllString(body, -1) |
Comment on lines
+225
to
+227
| // Extract subjects | ||
| subjectRe := regexp.MustCompile(`<span[^>]*>([^<]*(?:Re:|Fwd:|We'|Your |Hi |Hello|Thank|Welcome|Confirm|Order|Invoice|Receipt|Upgrade)[^<]*)</span>`) | ||
| subjects := subjectRe.FindAllStringSubmatch(body, -1) |
Comment on lines
+262
to
+272
| // Build items: match posting IDs with senders and subjects | ||
| for i, id := range uniqueIDs { | ||
| item := screenerItem{PostingID: id} | ||
| if i < len(senderEmails) { | ||
| item.Sender = senderEmails[i] | ||
| } | ||
| if i < len(subjectTexts) { | ||
| item.Subject = subjectTexts[i] | ||
| } | ||
| items = append(items, item) | ||
| } |
| // Extract feed/trail box IDs from hidden form inputs. | ||
| // The HTML has forms where designation_box_id appears before the feedbox/trailbox button, | ||
| // so we use (?s) to match across newlines. | ||
| feedRe := regexp.MustCompile(`(?s)designation_box_id[^>]*value="(\d+)"[^<]*</input>?[^<]*<button[^>]*feedboxButton`) |
| } | ||
| } | ||
|
|
||
| trailRe := regexp.MustCompile(`(?s)designation_box_id[^>]*value="(\d+)"[^<]*</input>?[^<]*<button[^>]*trailboxButton`) |
Comment on lines
+31
to
+34
| return 0, output.ErrUsage(fmt.Sprintf( | ||
| "no sender matching %q (available: %s)", | ||
| email, strings.Join(available, ", "), | ||
| )) |
| return &cobra.Command{ | ||
| Use: "unset <key>", | ||
| Short: "Clear a configuration value", | ||
| Example: ` hey config unset default_sender`, |
erikd234
changed the title
- Replace erik@parrotapp.com in test with generic alice@hey.com - Replace hardcoded email filter in screener with dynamic identity lookup - No personal emails remain in source code
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds sender selection controls and screener management to the HEY CLI, enabling per-message sender overrides and basic Screener workflows from the command line.
Changes:
- Add
--fromsender override tohey composeandhey reply, plusdefault_senderconfig support with precedence rules. - Extend
hey configwithget/unsetand key normalization (default-sender⇢default_sender). - Introduce
hey screenercommand (list + approve/deny/spam/feed/trail) with HTML parsing and PATCH actions, plus tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Adds DefaultSender field, source tracking, and unset support. |
| internal/config/config_default_sender_test.go | Tests default-sender behavior, persistence, and JSON omission. |
| internal/cmd/config.go | Adds key normalization plus config get/unset; updates show. |
| internal/cmd/compose.go | Implements sender override behavior via raw mutations when active. |
| internal/cmd/reply.go | Implements sender override behavior via raw mutations when active. |
| internal/cmd/sender.go | Adds sender ID resolution and effective sender selection logic. |
| internal/cmd/sender_test.go | Tests config key normalization. |
| internal/cmd/screener.go | Adds screener command, HTML parsing, and clearance PATCH actions. |
| internal/cmd/screener_test.go | Adds screener list/action tests using an HTTP test server. |
| internal/cmd/root.go | Registers the new screener command. |
| .surface | Updates CLI surface list to include new flags/commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+138
to
+140
| "entry": map[string]any{ | ||
| "addressed": addressed, | ||
| }, |
Comment on lines
+84
to
+89
| items, feedBoxID, trailBoxID, err := fetchScreenerItems(cmd.Context()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _ = feedBoxID | ||
| _ = trailBoxID |
Comment on lines
+245
to
+266
| feedRe := regexp.MustCompile(`(?s)designation_box_id[^>]*value="(\d+)"[^<]*</input>?[^<]*<button[^>]*feedboxButton`) | ||
| if m := feedRe.FindStringSubmatch(body); m != nil { | ||
| feedBoxID = m[1] | ||
| } | ||
| // Alternative pattern: button target appears after the input in the same form | ||
| if feedBoxID == "" { | ||
| altFeedRe := regexp.MustCompile(`(?s)designation_box_id"[^>]*value="(\d+)".*?feedboxButton`) | ||
| if m := altFeedRe.FindStringSubmatch(body); m != nil { | ||
| feedBoxID = m[1] | ||
| } | ||
| } | ||
|
|
||
| trailRe := regexp.MustCompile(`(?s)designation_box_id[^>]*value="(\d+)"[^<]*</input>?[^<]*<button[^>]*trailboxButton`) | ||
| if m := trailRe.FindStringSubmatch(body); m != nil { | ||
| trailBoxID = m[1] | ||
| } | ||
| if trailBoxID == "" { | ||
| altTrailRe := regexp.MustCompile(`(?s)designation_box_id"[^>]*value="(\d+)".*?trailboxButton`) | ||
| if m := altTrailRe.FindStringSubmatch(body); m != nil { | ||
| trailBoxID = m[1] | ||
| } | ||
| } |
Comment on lines
+299
to
+303
| client := &http.Client{ | ||
| CheckRedirect: func(req *http.Request, via []*http.Request) error { | ||
| return http.ErrUseLastResponse // Don't follow redirects | ||
| }, | ||
| } |
| return "", err | ||
| } | ||
|
|
||
| client := &http.Client{} |
Comment on lines
+288
to
+289
| reqURL := cfg.BaseURL + "/clearances/" + postingID | ||
| req, err := http.NewRequestWithContext(ctx, "PATCH", reqURL, strings.NewReader(values.Encode())) |
| Action: "deny", | ||
| Command: "hey screener deny <id>", | ||
| Description: "Screen out sender", | ||
| }, |
| fmt.Fprintf(w, clearancesHTMLTemplate, itemsHTML) | ||
|
|
||
| case r.Method == "PATCH" && strings.HasPrefix(r.URL.Path, "/clearances/"): | ||
| _ = r.ParseForm() |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cmd/screener.go">
<violation number="1" location="internal/cmd/screener.go:219">
P2: Identity lookup errors are silently ignored, which can leave unfiltered self-emails and cause misleading sender-to-posting mapping in screener output.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Filter out the authenticated user's own emails (they appear in nav/header). | ||
| // Look up dynamically via the identity endpoint. | ||
| ownEmails := map[string]bool{} | ||
| if identity, err := sdk.Identity().GetIdentity(ctx); err == nil && identity != nil { |
Contributor
There was a problem hiding this comment.
P2: Identity lookup errors are silently ignored, which can leave unfiltered self-emails and cause misleading sender-to-posting mapping in screener output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/screener.go, line 219:
<comment>Identity lookup errors are silently ignored, which can leave unfiltered self-emails and cause misleading sender-to-posting mapping in screener output.</comment>
<file context>
@@ -213,11 +213,17 @@ func fetchScreenerItems(ctx context.Context) ([]screenerItem, string, string, er
+ // Filter out the authenticated user's own emails (they appear in nav/header).
+ // Look up dynamically via the identity endpoint.
+ ownEmails := map[string]bool{}
+ if identity, err := sdk.Identity().GetIdentity(ctx); err == nil && identity != nil {
+ for _, s := range identity.Senders {
+ ownEmails[strings.ToLower(s.EmailAddress)] = true
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Add sender override to compose and reply, plus a new
hey screenercommand to manage The Screener. This lets you choose the sender per message and screen senders into Imbox, Feed, or Paper Trail from the CLI.New Features
--fromonhey composeandhey replyto set the sender email per message.default_sender; manage withhey config set|get|unset default-sender(hyphen or underscore accepted).--from>default_sender> HEY default. Sender email is resolved to an ID via the identity endpoint.acting_sender_idvia raw mutations; otherwise behavior is unchanged.hey config shownow includesdefault_sender.hey screenerto list pending items andapprove|deny|spam|feed|trail <posting-id>; uses/clearancesHTML andPATCH /clearances/<id>while SDK support is pending.Bug Fixes
hey screener(multiline regex).hey screenernow filters your own addresses via identity lookup.hey screener(list/actions) and config key normalization.Written for commit 1b43316. Summary will update on new commits.