Add automation scheduler service#837
Conversation
gsxdsm
changed the title
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request introduces a comprehensive automation system with a runtime engine, scheduler service, step registry, definition store, and extensive set of built-in automation actions (feature management, AI interaction, code execution, HTTP calls, Git operations, control flow). It includes new API routes for generation, management, scheduling, and webhook triggers, plus UI components for AI-driven automation creation and visual step editing. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive automation scheduler service to the system, enhancing its ability to execute automated workflows and manage resources more efficiently. It also incorporates improvements to error handling and logging, contributing to a more robust and maintainable system. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive automation system, including a runtime engine, scheduler, and a wide array of built-in steps. This is a significant and well-structured feature addition. My review focuses on potential issues related to security, scalability, and long-term reliability. I've identified a critical security issue with a committed session file, a high-severity concern with the custom cron parser, and several medium-severity suggestions regarding in-memory state management and leftover debug code. Overall, the implementation is impressive, and addressing these points will further enhance its robustness.
Note: Security Review did not run due to the size of the PR.
| { | ||
| "cookies": [ | ||
| { | ||
| "name": "automaker_session", | ||
| "value": "48b2fd3c98872c37e7d1445d13dfa9ba688d58f3e4212580611877b8efa2d586", | ||
| "domain": "localhost", | ||
| "path": "/", | ||
| "expires": 9999999999, | ||
| "httpOnly": true, | ||
| "secure": false, | ||
| "sameSite": "Lax" | ||
| } | ||
| ], | ||
| "origins": [] | ||
| } |
There was a problem hiding this comment.
This file contains sensitive session information and should not be committed to the repository. Committing secrets, API keys, or session tokens is a critical security risk. Please remove this file from the commit history and add *.auth.json or a more specific pattern to your .gitignore file to prevent this from happening in the future.
| private parseCronAndGetNext(cronExpr: string, _timezone?: string): Date | null { | ||
| const parts = cronExpr.trim().split(/\s+/); | ||
| if (parts.length !== 5) { | ||
| throw new Error(`Invalid cron expression: expected 5 fields, got ${parts.length}`); | ||
| } | ||
|
|
||
| const [minute, hour, dayOfMonth, month, dayOfWeek] = parts; | ||
| this.validateCronField(minute, 0, 59, 'minute'); | ||
| this.validateCronField(hour, 0, 23, 'hour'); | ||
| this.validateCronField(dayOfMonth, 1, 31, 'day-of-month'); | ||
| this.validateCronField(month, 1, 12, 'month'); | ||
| this.validateCronField(dayOfWeek, 0, 6, 'day-of-week'); | ||
|
|
||
| // Start from the next whole minute (≥ now + 1s) to avoid firing immediately. | ||
| // We advance by 1 minute and zero out sub-minute components so the first | ||
| // candidate is exactly at the next minute boundary. | ||
| const now = new Date(); | ||
| let candidate = new Date(now.getTime() + ONE_MINUTE_MS); | ||
| candidate.setSeconds(0, 0); | ||
|
|
||
| // Search for next matching time (up to 1 year ahead) | ||
| for (let i = 0; i < MAX_CRON_LOOKAHEAD_MINUTES; i++) { | ||
| if ( | ||
| this.cronFieldMatches(minute, candidate.getMinutes(), 0, 59) && | ||
| this.cronFieldMatches(hour, candidate.getHours(), 0, 23) && | ||
| this.cronFieldMatches(dayOfMonth, candidate.getDate(), 1, 31) && | ||
| this.cronFieldMatches(month, candidate.getMonth() + 1, 1, 12) && | ||
| this.cronFieldMatches(dayOfWeek, candidate.getDay(), 0, 6) | ||
| ) { | ||
| return candidate; | ||
| } | ||
|
|
||
| // Advance by 1 minute | ||
| candidate.setMinutes(candidate.getMinutes() + 1); | ||
| } | ||
|
|
||
| logger.warn(`Could not find next run time for cron: ${cronExpr}`); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The custom cron parser implementation is a potential source of bugs and may not support the full standard cron syntax that users might expect (e.g., features like L, W, #). Maintaining a custom cron parser is complex and error-prone.
It is highly recommended to replace this with a well-established and thoroughly tested library like cron-parser or node-cron. This will improve reliability, maintainability, and feature completeness.
| 2026-03-01T20:55:55.690Z | ||
|
|
||
| [object Object] |
There was a problem hiding this comment.
This file appears to have been accidentally committed. It contains [object Object], which often indicates an object was incorrectly stringified during development. This file should be removed from the pull request.
| // Start server with error handling for port conflicts | ||
| const startServer = (port: number, host: string) => { | ||
| server.listen(port, host, () => { | ||
| logger.info('Gemini test - Hello World'); |
There was a problem hiding this comment.
This appears to be a temporary debug log statement. It should be removed before merging to avoid cluttering production logs.
| /** Rate limiting configuration */ | ||
| const RATE_LIMIT_WINDOW_MS = 60_000; // 1 minute | ||
| const RATE_LIMIT_MAX_REQUESTS = 60; // requests per window per IP | ||
| const rateLimitStore = new Map<string, { count: number; resetAt: number }>(); |
There was a problem hiding this comment.
The rate limiting for webhooks is implemented in-memory. This has two main drawbacks:
- The rate limit state is lost on every server restart.
- It won't work correctly in a load-balanced environment with multiple server instances, as each instance would have its own separate rate limit counter.
For a more robust and scalable solution, consider using an external store like Redis to manage rate limit data.
| private readonly runStore = new Map<string, AutomationRun>(); | ||
| /** Ordered map of run IDs to sequence numbers (most-recent first). Uses monotonic counter to avoid Date.now() collisions. */ | ||
| private readonly runOrder = new Map<string, number>(); | ||
| /** Monotonic counter for deterministic ordering of runs tracked within the same millisecond. */ | ||
| private runSequence = 0; | ||
| private readonly maxStoredRuns = 200; |
There was a problem hiding this comment.
The automation run history is stored in-memory. This means that all execution history will be lost whenever the server restarts. For a system that tracks automation executions, this data is often valuable for auditing, debugging, and understanding system behavior over time.
Consider persisting the run history to a more durable storage, such as a database or even a file-based log store, to ensure it survives restarts.
1 participant
Summary
Changes
AutomationRuntimeEngineandAutomationSchedulerServicefor executing automated workflows/api/automationwith null-check middleware to ensure scheduler is initializedsettingsServiceto runtime engine to support AI prompt steps with Claude API credentialsAutomationVariableServicefor managing automation variable scopingSummary by CodeRabbit
New Features
Bug Fixes
Chores