feat(mail): Phase 3 IMAP selected import#158
Conversation
zoorpha
force-pushed
the
feat/mail-phase2-linking
branch
from
a71595a to
2a5395a
Compare
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Add a partial unique index on mail_accounts(owner_id, name) where deleted_at IS NULL to prevent duplicate active account names per owner. Replace the single-column idx_mail_import_jobs_status index with a composite partial index on (status, created_at) where deleted_at IS NULL to support FIFO polling of import jobs. Add a CHECK constraint on mail_accounts.port to enforce the valid TCP port range (1-65535). Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Adds domain models for IMAP mail accounts and import jobs, including TLS mode and job status enums with string conversions. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Allow clippy::too_many_arguments on MailAccount::new - Remove unused MailImportJobStatus import in domain test - Skip serialization of password_enc for safety Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
…rt_jobs Add create/get/list/update/soft-delete methods for mail_accounts and create/get/list/claim/progress/complete/fail methods for mail_import_jobs to backend/crates/storage/src/metadata.rs. Update SQLx offline query cache for the new queries. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- update_mail_import_job_progress, mark_mail_import_job_completed, and mark_mail_import_job_failed now only touch rows where deleted_at IS NULL. - claim_next_pending_mail_import_job joins mail_accounts and only claims jobs whose account is enabled and not soft-deleted. Combine SELECT, UPDATE, and RETURN into a single CTE + UPDATE ... RETURNING statement. - soft_delete_mail_account cancels pending import jobs for the account in the same transaction. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Add async-imap, tokio-rustls, and webpki-roots dependencies. - Implement ImapClient::connect for TLS and plain TCP; STARTTLS is deferred to a later phase. - Implement ImapSession for login, folder listing/selection, message summary fetching, RFC822 body fetching, and logout. - Export the module from services::imap_client. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Decode RFC 2047 encoded-words in subject/from via mailparse - Add DEFAULT_TIMEOUT around connect/login/IMAP commands - Warn when using plaintext IMAP - Add unit tests for header/address/date parsing Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
…ssing Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Mark import job failed when IMAP connection/login fails after setting status to running, preventing stuck jobs. - Propagate list_folders failures in test_account_connection. - Guard process_import_job against re-processing completed/failed/cancelled jobs. - Validate IMAP UIDs with u32::try_from before fetching. - Validate IMAP port is in 1..=65535 before connecting. - Reject empty selected UID lists when creating import jobs. - Sanitize MailError::Imap responses to avoid leaking raw server messages. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Add reset_stale_running_mail_import_jobs to recover jobs stuck in running. - Add environment-based mail import worker config with sensible defaults. - Use JoinSet to track in-flight import tasks and graceful shutdown. - Only spawn the mail import worker when enabled. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Wrap create_account and delete_account database mutations and their MailAccountCreated/MailAccountDeleted audit events in a single SQL transaction using new MetadataStore helpers create_mail_account_in_tx and soft_delete_mail_account_in_tx. Also prevent a per-message MailImported event append failure from failing the entire IMAP import job; log a warning and continue so the job is not retried and messages are not re-imported. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
- Remove Debug derive from password-bearing mail account request DTOs. - Add validator::Validate to CreateMailAccountRequest, UpdateMailAccountRequest, and CreateMailImportJobRequest with length/range/custom constraints. - Switch create/update mail account and import job handlers to ValidatedJson. - Validate folder query parameter in list_mail_account_messages. - Introduce typed OpenAPI response structs: MailAccountListResponse, MailMessageSummaryListResponse, and MailTestConnectionResponse. - Register new response schemas and add Mail tag description. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Add an ignored integration test for the IMAP selected import flow. The test validates account creation, folder/message listing, job creation, synchronous processing, and completion verification. Also update the test helper's MailService constructor to pass the broadcaster required by the current service signature. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
…zers Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
Signed-off-by: Senol Colak <senol@kubedo.com>
zoorpha
force-pushed
the
feat/mail-phase3-imap-import
branch
from
0f7eb12 to
545c89c
Compare
|
@codex review |
Signed-off-by: Senol Colak <senol@kubedo.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 545c89c8c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Senol Colak <senol@kubedo.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cebf12615
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ling - Track in-flight import job IDs in the worker and exclude them from reset_stale_running_mail_import_jobs so live tasks are not reset. - Mark import jobs failed when their account is deleted/disabled/missing instead of leaving them stuck in running/pending. - Detect unique-name constraint violations on mail account create/update and return a client-visible DuplicateAccountName (HTTP 409). - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bc8a050e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…te key - Extract internal/private IP checks into rustshare_core::validation and reuse them for chat webhook URL validation. - Validate IMAP hosts against internal/private destinations before opening sockets in ImapClient::connect. - Add source_uidvalidity to mail_messages and the unique duplicate key so imports are scoped to the mailbox UIDVALIDITY. - Select the IMAP folder once per import job to capture UIDVALIDITY and pass it through find_mail_message_by_source / import_raw_source. - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46621f80e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…errors - Use the SocketAddr returned by resolve_public_socket_addrs for both TLS and plaintext IMAP connections so a second DNS lookup cannot rebind to an internal address. - Mark import jobs failed when SELECT fails instead of leaving them in running and retrying forever. Signed-off-by: Senol Colak <senol@kubedo.com>
- Catch panics inside the mail import worker task so the in-flight job id is always returned and cleaned up from the worker's set. - Reject unsupported TLS modes (starttls and plaintext none) when creating or updating mail accounts; update the ignored integration test to tls. - Deduplicate selected UIDs when creating an IMAP import job so the job total_messages counter stays accurate. - Add create_mail_message_if_not_exists and use it during imports so a concurrent import of the same source UID is detected before creating artifacts and is treated as already imported. - Regenerate SQLx offline metadata for the new query. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40b3360e1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Cancel both pending and running import jobs when a mail account is soft-deleted, and stop processing early in process_import_job when the job status is no longer 'running'. - Move the mail message insert (with ON CONFLICT DO NOTHING) ahead of creating the artifact folder and source.eml file, so a concurrent import of the same IMAP UID is detected before any visible artifacts are written. - Add MetadataStore helpers get_mail_import_job_status and update_mail_message_folder_id to support the above. - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dd444d64e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Make mark_mail_import_job_completed/failed only update rows whose status is 'running' and return whether a row was updated. process_import_job treats a zero-row update as cancellation and returns Ok instead of overwriting a terminal state. - Wrap artifact creation in import_raw_source and delete the inserted mail_messages row if any artifact step fails, so a retry does not treat the UID as already imported while visible artifacts are missing. - Add MetadataStore helpers delete_mail_message. - Add migration to recreate idx_mail_messages_account_source with NULLS NOT DISTINCT so nullable source_uidvalidity participates in duplicate detection. - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a3a4ae56f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Restrict idx_mail_messages_account_source to IMAP source modes only (imap_selected, imap_archive) and update create_mail_message_if_not_exists ON CONFLICT clause to match. This prevents EML uploads from colliding on the dedupe index. - Enforce the 25 MiB body cap after fetching BODY.PEEK[] as well, so an IMAP server that omits or underreports RFC822.SIZE cannot bypass the limit. - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1042bd91a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Make mark_mail_import_job_running only update rows in 'pending' state and return a bool; process_import_job treats a zero-row update as cancellation. - Loop through all SSRF-validated IMAP addresses in ImapClient::connect, returning the first successful connection and only failing when every resolved address has been tried. - Join the modules table in claim_next_pending_mail_import_job and only claim jobs whose tenant has the 'mail' module enabled. - Regenerate SQLx offline metadata. Signed-off-by: Senol Colak <senol@kubedo.com>
The claim CTE was transitioning jobs to 'running', but process_import_job immediately called mark_mail_import_job_running which only updates rows with status = 'pending'. This caused every claimed job to be treated as cancelled and no mail to be imported. Change claim_next_pending_mail_import_job to a plain SELECT ... FOR UPDATE SKIP LOCKED so it returns the pending job; process_import_job then performs the pending -> running transition as intended. Also skip the ignored IMAP integration test when IMAP_TEST_HOST is empty, localhost, or a 127.* loopback address, since the IMAP client rejects internal destinations for SSRF protection. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com> Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 738741f1f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex correctly flagged that the previous fix removed the atomic claim, releasing the row lock before the job was marked running. Restore the UPDATE ... RETURNING CTE so claim transitions the row to `running` in one statement, and widen `mark_mail_import_job_running` to accept both `pending` and `running` so: - worker-claimed jobs (already running) pass the re-check, - direct callers such as the integration test can pass a pending job, - cancelled/terminal jobs are still rejected. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e4690c883
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 feedback: - UIDs are only stable within a single UIDVALIDITY value. The message-list response now returns the folder's UIDVALIDITY, and create-import-job requests must include it. The value is stored on mail_import_jobs and checked when the worker selects the folder; a mismatch fails the job instead of importing the wrong messages. - Credential decryption failures during job processing now mark the job failed so an undecryptable account does not loop indefinitely through stale-reset retries. Changes: - Add source_uidvalidity column to mail_import_jobs via migration. - Add source_uidvalidity to MailImportJob domain model. - Return (Option<u32>, Vec<ImapMessageSummary>) from ImapSession::fetch_message_summaries and MailService::list_imap_messages. - Add uidvalidity to MailMessageSummaryListResponse and source_uidvalidity to CreateMailImportJobRequest. - Verify UIDVALIDITY in process_import_job and fail the job on mismatch. - Mark job failed when decrypting the stored password fails. Refs #147 Signed-off-by: zoorpha <aaron@kubedo.com> Signed-off-by: Senol Colak <senol@kubedo.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb9d25c178
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| pub async fn fetch_rfc822(&mut self, folder: &str, uid: u32) -> Result<Vec<u8>, ImapError> { | ||
| let _ = self.select_folder(folder).await?; |
There was a problem hiding this comment.
When a mailbox is rebuilt after process_import_job performs its initial UIDVALIDITY check, this re-selects the folder for each UID and discards the new UIDVALIDITY before calling uid_fetch. In that scenario the selected UID can now refer to a different message, so the import may persist the wrong email despite the job carrying a UIDVALIDITY guard.
Useful? React with 👍 / 👎.
| if let Err(cleanup_err) = self | ||
| .metadata_store | ||
| .delete_mail_message(msg.id, owner_id) | ||
| .await |
There was a problem hiding this comment.
If any later step fails after source.eml or attachment files have been uploaded into the message folder, this cleanup only deletes the mail_messages row. Those files and the /Workspace/Mail/... folder remain visible to the user, and a retry creates another folder for the same UID, leaving orphaned artifacts from failed imports.
Useful? React with 👍 / 👎.
2 participants
Refs #147
Summary
This PR adds RustShare Mail Phase 3 — IMAP selected import — on top of the Phase 2 linking branch:
MailMessageartifacts.Security / Safety Notes
rustshare_crypto.Test Plan
cargo fmt --checkcd backend && SQLX_OFFLINE=true cargo clippy --all-features -- -D warningsSQLX_OFFLINE=true cargo check --workspaceSQLX_OFFLINE=true cargo check --testsDATABASE_URL=postgres://scolak@localhost/rustshare SQLX_OFFLINE=true cargo test --workspace --libSQLX_OFFLINE=true cargo test --test mail_imap_import_test --no-runcd backend && DATABASE_URL=postgres://scolak@localhost/rustshare cargo sqlx prepare --workspace --check