Restrict email processing to INBOX only #97
Conversation
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
- Added check in processMailbox() to only process emails from INBOX folder - Updated tests to verify non-INBOX folders are not processed - Removed obsolete tests that expected emails to move between non-INBOX folders - All 8 unit tests pass (1 integration test requires database) Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix email moving logic to restrict folder changes
Restrict email processing to INBOX only
Nov 11, 2025
HNygard
approved these changes
Dec 6, 2025
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 27. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Pull request overview
This PR restricts email processing to only the INBOX folder, preventing unintended reorganization of emails in non-INBOX folders like Sent and thread folders. Previously, emails were being moved between these folders, causing already-filed messages to be reorganized unexpectedly.
Key Changes:
- Added early return guard in
ThreadEmailMover::processMailbox()that skips processing for any mailbox other than 'INBOX' - Replaced 3 parameterized tests expecting non-INBOX moves with a single test verifying no-op behavior for non-INBOX mailboxes
- Non-INBOX processing now returns empty results without fetching or moving any emails
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| organizer/src/class/ThreadEmailMover.php | Added early return check at the start of processMailbox() to skip any mailbox that isn't 'INBOX', returning empty unmatched list and maxed_out=false |
| organizer/src/tests/ThreadEmailMoverTest.php | Removed data provider and 3 parameterized tests for non-INBOX behavior; added single test testProcessNonInboxMailboxDoesNothing() verifying that no emails are fetched or moved from non-INBOX mailboxes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
46
to
52
| // Only process emails from INBOX - never move emails from other folders | ||
| if ($mailbox !== 'INBOX') { | ||
| return array( | ||
| 'unmatched' => [], | ||
| 'maxed_out' => false, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The strict equality check $mailbox !== 'INBOX' will prevent processing of any folder that isn't exactly 'INBOX'. This is the intended behavior based on the PR description.
However, this means functions like processSentFolder() in update-imap-functions.php that call processMailbox() with 'Sent' or 'INBOX.Sent' will now silently return empty results without any indication that they were skipped. Consider adding a debug log message here to make the skip explicit and easier to debug, for example:
$this->connection->logDebug("Skipping mailbox '$mailbox' - only INBOX is processed");
This would help operators understand the behavior when reviewing logs.
Contributor
Author
There was a problem hiding this comment.
Added debug logging in commit 5e6584e. The log message now reads: "Skipping mailbox '$mailbox' - only INBOX is processed"
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Addresses review feedback to make the skip behavior explicit in logs, helping operators understand when folders are being skipped. Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Task: Change logic so that we never move email out of a folder other than INBOX
✅ Completed
Changes Made
ThreadEmailMover.php:
processMailbox()method to only process emails when$mailbox === 'INBOX'ThreadEmailMoverTest.php:
testProcessNonInboxMailboxDoesNothing()Test Results
Impact Analysis
update-imap-functions.phpwill now effectively do nothing (as intended)Security
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.