-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Enable spam email processing and improve label management workflow #1948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -354,7 +354,7 @@ export class WorkflowRunner extends DurableObject<ZeroEnv> { | |||
| // Extract thread IDs from messages | ||||
| historyItem.messagesAdded?.forEach((msg) => { | ||||
| if (msg.message?.labelIds?.includes('DRAFT')) return; | ||||
| if (msg.message?.labelIds?.includes('SPAM')) return; | ||||
| // if (msg.message?.labelIds?.includes('SPAM')) return; | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Strategic spam processing relocation! Moving spam filtering downstream to the workflow level is a brilliant architectural decision - like moving engine gimbal control from the booster to the flight computer where it belongs. However, let's clean up this commented code rather than leaving space debris in our codebase. Consider removing the commented line entirely since spam filtering is now handled at the workflow level: - // if (msg.message?.labelIds?.includes('SPAM')) return;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| if (msg.message?.threadId) { | ||||
| threadsAdded.add(msg.message.threadId); | ||||
| } | ||||
|
|
@@ -465,6 +465,8 @@ export class WorkflowRunner extends DurableObject<ZeroEnv> { | |||
| ); | ||||
| } | ||||
| } | ||||
| } else { | ||||
| yield* Console.log('[ZERO_WORKFLOW] No new threads to process'); | ||||
| } | ||||
|
|
||||
| // Process label changes for threads | ||||
|
|
@@ -1070,7 +1072,7 @@ export class WorkflowRunner extends DurableObject<ZeroEnv> { | |||
| history.forEach((historyItem) => { | ||||
| historyItem.messagesAdded?.forEach((msg) => { | ||||
| if (msg.message?.labelIds?.includes('DRAFT')) return; | ||||
| if (msg.message?.labelIds?.includes('SPAM')) return; | ||||
| // if (msg.message?.labelIds?.includes('SPAM')) return; | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consistent architectural alignment! Good to see the same spam filtering removal in the non-Effect version for consistency. However, same recommendation as before - let's remove this commented code entirely rather than leaving it as technical debt. Remove the commented line to keep the codebase clean: - // if (msg.message?.labelIds?.includes('SPAM')) return;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| if (msg.message?.threadId) { | ||||
| threadsAdded.add(msg.message.threadId); | ||||
| } | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -941,6 +941,7 @@ export class ZeroDriver extends DurableObject<ZeroEnv> { | |||||
| Effect.tap(() => | ||||||
| Effect.sync(() => console.log(`[syncThread] Updated database for ${threadId}`)), | ||||||
| ), | ||||||
| Effect.tap(() => Effect.sync(() => this.reloadFolder('inbox'))), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The folder to reload is hard-coded to "inbox", so threads that are synchronised to other folders (e.g., spam, trash, sent) will not trigger a refresh for the correct folder and may leave the UI stale. Use the labels on the freshly-synced thread to decide which folder(s) need to be reloaded instead. (Based on your team's feedback about avoiding hard-coded folder names that break multi-folder workflows.) Prompt for AI agents
Suggested change
|
||||||
| Effect.catchAll((error) => { | ||||||
| console.error(`[syncThread] Failed to update database for ${threadId}:`, error); | ||||||
| return Effect.succeed(undefined); | ||||||
|
|
@@ -966,18 +967,6 @@ export class ZeroDriver extends DurableObject<ZeroEnv> { | |||||
| return Effect.succeed(undefined); | ||||||
| }), | ||||||
| ); | ||||||
| // yield* Effect.tryPromise(() => sendDoState(this.name)).pipe( | ||||||
| // Effect.tap(() => | ||||||
| // Effect.sync(() => { | ||||||
| // result.broadcastSent = true; | ||||||
| // console.log(`[syncThread] Broadcasted do state for ${threadId}`); | ||||||
| // }), | ||||||
| // ), | ||||||
| // Effect.catchAll((error) => { | ||||||
| // console.warn(`[syncThread] Failed to broadcast do state for ${threadId}:`, error); | ||||||
| // return Effect.succeed(undefined); | ||||||
| // }), | ||||||
| // ); | ||||||
| } else { | ||||||
| console.log(`[syncThread] No agent available for broadcasting ${threadId}`); | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes around the URL make this npm script fail on Windows shells; use double quotes (escaped) for cross-platform compatibility.
Prompt for AI agents