refactor: ponytail audit cleanup (-836 lines, -2 direct deps) - #98
Merged
Merged
Conversation
The recorded MessageID was a local 'smtp-<to>-<n>' string (and '%!d(<nil>)' for web sends, which never set the sequence context value); the actual message had no Message-ID header, so the provider assigned one we never saw. Sends now carry <random@sender-domain> and record that, and the email.SequenceKey context plumbing is gone.
CaptchaInfo keeps only Found and Type; Confidence, FrameSrc, ElementID and the per-detector Description were never read (the description table overrode it). Same checks, same order - covered by a new Chrome fixture test per branch.
A job paused by the daily cap used to be saved to pending_job*.json and resumed on the next 'serve' start. Its remaining list is exactly what 'Send all' (eligible) or an automated cycle picks up next anyway, and it needed a history re-check to avoid double-sending. Removes JobPersistence, PersistentJobState, checkPendingJob, resumePendingJob and saveJobProgress. Leftover pending_job*.json files are ignored.
email.SendRemoval renders the request, sends it and returns the history record; CLI send, the web Send all job and the single-broker send keep only their own loop policy (cap, pause, auth cutoff, output).
Monitor.ScanAndStore (fetch, classify, store new, advance pipeline, archive) and RecordReply (one reply, used by --watch) replace three copies. Along the way: - web scans archived the UIDs of archive-folder emails too; ArchiveEmails applies them to INBOX, where they can name unrelated messages. Only INBOX UIDs are archived now. - new replies found by the web scan and by --watch now advance the broker's pipeline status, as 'eraser monitor' already did. - scan/rescan/reclassify/setup HTML fragments used Tailwind classes that exist nowhere (no Tailwind build); they use the layout's .alert, .badge, .btn and text-error/text-warning classes now.
The Brokers page still promised that remaining emails continue when the app restarts. Also removes leftover pending_job*.json files, and makes FindBrokerResponseBySubject return the body so the rescan's 'fill body only if empty' check does what it says.
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.
Applies the ponytail-audit findings, one commit per finding. The branch removes 836 lines net across 28 files and two direct dependencies (
goquerywith itscascadia, andgoogle/uuid, which stays only as a transitive dependency of modernc sqlite).Cuts
CaptchaInfokeeps onlyFoundandType; the other fields were never read. Same checks in the same order. The newTestDetectCaptchahas one Chrome fixture per branch. It skips locally without Chrome and runs on CI's Ubuntu runner. I also ran all 9 fixtures in a real browser and they pass.JobPersistence,PersistentJobState,checkPendingJob,resumePendingJobandsaveJobProgressare gone. A paused job's remaining list is exactly what "Send all" or automation picks up next anyway. The Brokers page wording is updated, and leftoverpending_job*.jsonfiles are deleted at startup.Monitor.ScanAndStoreandinbox.RecordReplyreplace the separate implementations in the CLImonitor, the web scan and the web rescan.email.SendRemovaldoes render → send → history record for CLIsend, the web "Send all" job and the single-broker send.GetBrokerResponsesbuilds its WHERE clause instead of carrying four query variants.RateLimiterloses a cleanup goroutine it didn't need (the key set is fixed),email.SequenceKeycontext plumbing is gone, goquery is replaced by anx/net/htmltokenizer, uuid bycrypto/rand.Text().Bugs fixed along the way
ArchiveEmailsapplies UIDs to INBOX, where an archive-folder UID can name an unrelated message. Only INBOX UIDs are archived now.Message-ID: <random@sender-domain>header, and that's what gets recorded. Before, no header was sent and the recorded ID was a local placeholder (%!d(<nil>)for web sends).monitor --watchnow advance the broker's pipeline status, aseraser monitoralready did..alert,.badge,.btnandtext-error/text-warningclasses.Behaviour change to note: with
auto_archiveon, a rescan now archives INBOX replies, the same as a scan.Verified:
go vet ./... && go test ./...pass, and-raceon web, inbox and history. New tests cover SendRemoval (sent, failed, and render error), the Message-ID header matching the recorded ID (fake SMTP server), RecordReply (new / seen / reclassified, pipeline advance, profile attribution), HTML href extraction, and the CAPTCHA fixtures. A dry-runeraser auto --oncecycle completes.BEGIN_COMMIT_OVERRIDE
fix(inbox): web scans no longer archive unrelated INBOX messages
fix(email): send and record a real Message-ID header
fix(inbox): new replies from web scans and monitor --watch advance pipeline status
fix(web): scan, setup and send-status messages render with the app's styles
refactor: ponytail audit cleanup (-836 lines, -2 direct deps)
END_COMMIT_OVERRIDE