diff --git a/cmd/eraser/cmd_auto.go b/cmd/eraser/cmd_auto.go index b3e130c..3a5cde1 100644 --- a/cmd/eraser/cmd_auto.go +++ b/cmd/eraser/cmd_auto.go @@ -85,11 +85,9 @@ func runAutoLoop(every time.Duration) error { } } -// waitForNextCycle sleeps for every and reports whether the user asked to -// stop. Signals are caught only while waiting: during a cycle Ctrl+C keeps -// its default and kills the process, which is safe mid-send (history is -// written per broker, the OS drops the lock) and doesn't make the user wait -// out a 15-minute send. A var so tests can end the loop. +// waitForNextCycle reports whether to stop. Signals are caught only here: +// mid-cycle Ctrl+C just kills the process, which is safe (history is per +// broker, the OS drops the lock). A var so tests can end the loop. var waitForNextCycle = func(every time.Duration) (stop bool) { ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() diff --git a/cmd/eraser/cmd_send.go b/cmd/eraser/cmd_send.go index 4f856a5..c1ffc43 100644 --- a/cmd/eraser/cmd_send.go +++ b/cmd/eraser/cmd_send.go @@ -93,10 +93,8 @@ func countWithEmail(brokers []broker.Broker) int { return n } -// capSends keeps brokers up to the n-th one with an email. Email-less brokers -// are only skipped with a note, so they mustn't use up the daily budget: they -// never get a history row, sort first on every run, and would otherwise eat -// the same slots each time. +// capSends keeps brokers up to the n-th one with an email. Email-less ones +// never get a history row and sort first every run; they mustn't eat budget. func capSends(brokers []broker.Broker, n int) []broker.Broker { for i, b := range brokers { if strings.TrimSpace(b.Email) == "" { diff --git a/cmd/eraser/cmd_watch_test.go b/cmd/eraser/cmd_watch_test.go index 28fcd37..c93b5da 100644 --- a/cmd/eraser/cmd_watch_test.go +++ b/cmd/eraser/cmd_watch_test.go @@ -1,8 +1,7 @@ //go:build !race -// go-imap v1.2.1 writes IDLE's DONE from its own goroutine onto the writer -// the next command uses, which -race flags; that's upstream, so this test -// only runs without the race detector. +// go-imap v1.2.1 writes IDLE's DONE from its own goroutine; -race flags it +// (upstream), so this only runs without the race detector. package main diff --git a/internal/browser/browser.go b/internal/browser/browser.go index fbe282b..78b463e 100644 --- a/internal/browser/browser.go +++ b/internal/browser/browser.go @@ -133,10 +133,8 @@ func (b *Browser) NavigateAndFill(url string, brokerID string, autoSubmit bool) } } - // The first Run starts Chrome and ties it to that Run's context. Start it - // on the long-lived b.ctx: started on the per-call timeout context below, - // cancelling that at return closed the browser for every later call - // (`eraser fill --pending` fills several forms with one Browser). + // Start Chrome on b.ctx: the first Run's context owns the browser, so the + // per-call timeout ctx below would close it after one form (fill --pending). if err := chromedp.Run(b.ctx); err != nil { result.ErrorMessage = fmt.Sprintf("browser failed to start: %v", err) return result, err diff --git a/internal/browser/browser_chrome_test.go b/internal/browser/browser_chrome_test.go index b3dba05..ea62401 100644 --- a/internal/browser/browser_chrome_test.go +++ b/internal/browser/browser_chrome_test.go @@ -107,13 +107,10 @@ func TestSubmitButtonTextPattern_MirrorsJSRegex(t *testing.T) { } } -// Ubuntu 24.04 runners block the unprivileged user namespaces Chrome's -// sandbox needs. The pages under test are local fixtures, so CI runs -// without it. +// Ubuntu 24.04 runners can't give Chrome its sandbox; pages are local fixtures. func init() { if os.Getenv("GITHUB_ACTIONS") == "true" { - // A cold Chrome start under -race on a shared runner can outlast - // chromedp's 20s default for the DevTools URL. + // cold start under -race can outlast chromedp's 20s default extraAllocatorOptions = append(extraAllocatorOptions, chromedp.NoSandbox, chromedp.WSURLReadTimeout(90*time.Second)) } } diff --git a/internal/inbox/imap_watch_test.go b/internal/inbox/imap_watch_test.go index b7d026b..403cb30 100644 --- a/internal/inbox/imap_watch_test.go +++ b/internal/inbox/imap_watch_test.go @@ -1,8 +1,7 @@ //go:build !race -// go-imap v1.2.1's client writes IDLE's DONE from its own goroutine onto the -// writer the next command uses, which -race flags; that's upstream, so this -// test only runs without the race detector. +// go-imap v1.2.1 writes IDLE's DONE from its own goroutine; -race flags it +// (upstream), so this only runs without the race detector. package inbox @@ -20,10 +19,8 @@ import ( "github.com/drumandbytes/eraser/internal/config" ) -// New mail during IDLE must not wedge the connection: the follow-up SELECT -// draws an unsolicited EXISTS, which deadlocked when the watch loop was also -// the only reader of the client's update channel. Scripted, because go-imap's -// own server races when it broadcasts updates. +// New mail during IDLE: the follow-up SELECT's unsolicited EXISTS must not +// wedge the watch. Scripted, since go-imap's server races on updates. func TestWatchForNewEmailsSurvivesNewMail(t *testing.T) { reIdled := make(chan struct{}) addr := fakeIMAPServer(t, func(conn net.Conn, br *bufio.Reader) error { diff --git a/internal/inbox/monitor.go b/internal/inbox/monitor.go index cd31791..6b27684 100644 --- a/internal/inbox/monitor.go +++ b/internal/inbox/monitor.go @@ -396,12 +396,9 @@ func (m *Monitor) WatchForNewEmails(ctx context.Context, callback func(Email)) e return fmt.Errorf("failed to select mailbox: %w", err) } - // The client delivers unilateral responses (EXISTS after the SELECT in - // FetchBrokerEmails, for one) on Updates and blocks until they're read, - // so draining it here in the loop deadlocked on the first new mail. A - // separate reader keeps the connection moving and just flags new mail. - // ponytail: the drain goroutine lives as long as the connection; fine - // for one watch per Monitor. + // Updates blocks until read, and FetchBrokerEmails' SELECT emits EXISTS + // into it, so it needs its own reader or the loop deadlocks. + // ponytail: drain goroutine lives as long as the connection. updates := make(chan client.Update, 16) newMail := make(chan struct{}, 1) go func() { @@ -446,8 +443,7 @@ func (m *Monitor) WatchForNewEmails(ctx context.Context, callback func(Email)) e callback(email) } - // The fetch's own SELECT reports EXISTS too; that's covered by - // the fetch just done, so don't let it bounce IDLE straight away. + // the fetch's own SELECT flagged new mail too; drop it select { case <-newMail: default: diff --git a/internal/web/handlers_send_test.go b/internal/web/handlers_send_test.go index 072233c..5d25853 100644 --- a/internal/web/handlers_send_test.go +++ b/internal/web/handlers_send_test.go @@ -18,9 +18,8 @@ import ( "github.com/drumandbytes/eraser/internal/history" ) -// fakeRelay is a plaintext SMTP relay that accepts any number of -// connections. A recipient in reject gets a 550; authFail answers MAIL FROM -// with a 535, the way a provider that has locked the account does. +// fakeRelay is plaintext SMTP. reject -> 550 on RCPT; authFail -> 535 on +// MAIL FROM, like a provider that locked the account. type fakeRelay struct { addr string mu sync.Mutex diff --git a/internal/web/server.go b/internal/web/server.go index 03353d7..5039a04 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -323,8 +323,7 @@ func (s *Server) parseTemplates() (map[string]*template.Template, error) { func (s *Server) Start() error { router := s.setupRouter() - // lifeMu: serve's Ctrl+C handler may call Shutdown before or while this - // runs; it used to read httpServer unsynchronised (nil = panic). + // serve's Ctrl+C handler can call Shutdown before or during this s.lifeMu.Lock() if s.shutDown { s.lifeMu.Unlock() @@ -342,9 +341,7 @@ func (s *Server) Start() error { srv := s.httpServer s.lifeMu.Unlock() - // Listen before opening the browser, so it opens only once there's a - // server to load (it used to open after a blind 500ms, even on a - // taken port). + // listen first: open the browser only once there's a server to load ln, err := net.Listen("tcp", srv.Addr) if err != nil { cancel() @@ -364,8 +361,7 @@ func (s *Server) Start() error { return nil } -// Shutdown gracefully shuts down the server. Before Start it just makes -// Start return straight away. +// Shutdown stops the server; called before Start, Start returns at once. func (s *Server) Shutdown(ctx context.Context) error { s.lifeMu.Lock() defer s.lifeMu.Unlock()