Conversation
There was a problem hiding this comment.
Pull request overview
Adds scheduling support to the Operator Cloudflare Worker by introducing D1-backed schedule persistence, cron execution, and OpenAI tool-calling to create/list/delete schedules via Telegram.
Changes:
- Add D1 schema + migrations for
schedulesandpending_actions, and wire D1 binding + cron trigger. - Implement scheduling logic (next-run calculation, claiming due schedules, retries) and a scheduled-event handler to deliver messages.
- Extend Telegram/OpenAI integration with tool calling + “YES” confirmation flow, plus message-splitting and URL validation utilities.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks newly added Drizzle dependencies and transitive packages. |
| apps/operator/wrangler.jsonc | Adds D1 binding configuration and a cron trigger for scheduled execution. |
| apps/operator/src/utils/url-validator.ts | Adds SSRF-oriented URL validation helpers for future “monitor” features. |
| apps/operator/src/utils/url-validator.test.ts | Tests URL allowlisting/HTTPS/credential/length checks. |
| apps/operator/src/utils/message.ts | Adds Telegram-safe message splitting utility. |
| apps/operator/src/types/env.ts | Extends Worker bindings typing to include DB: D1Database. |
| apps/operator/src/services/schedule.ts | Implements schedule creation, listing, due-claiming, retries, and next-run computation. |
| apps/operator/src/services/schedule.test.ts | Tests next-run computation and schedule input validation rules. |
| apps/operator/src/services/pending-action.ts | Adds D1-backed pending action storage for confirmation-gated actions. |
| apps/operator/src/services/openai.ts | Adds tool-calling loop + schedule-related tool schema; exports tool types/constants. |
| apps/operator/src/services/openai.test.ts | Adds tests for tool-calling behavior and iteration limits. |
| apps/operator/src/scheduled.ts | Implements cron handler that claims due schedules and sends Telegram messages. |
| apps/operator/src/modules/telegram/routes.test.ts | Updates Telegram route tests to include a D1 binding mock in env. |
| apps/operator/src/modules/telegram/controller.ts | Adds schedule tool execution, confirmation flow, and schedule list/create/delete integration. |
| apps/operator/src/index.ts | Exports Worker handler object with both fetch and scheduled. |
| apps/operator/src/db/schema.ts | Defines Drizzle schema for schedules and pending_actions. |
| apps/operator/package.json | Adds Drizzle deps + migration scripts; enables --test-scheduled in dev. |
| apps/operator/migrations/meta/0000_snapshot.json | Drizzle snapshot metadata for initial schedules table. |
| apps/operator/migrations/meta/0001_snapshot.json | Drizzle snapshot metadata for pending_actions table addition. |
| apps/operator/migrations/meta/_journal.json | Drizzle migration journal tracking applied migration tags. |
| apps/operator/migrations/0000_acoustic_shape.sql | Creates schedules table + index. |
| apps/operator/migrations/0001_cool_thanos.sql | Creates pending_actions table. |
| apps/operator/drizzle.config.ts | Adds Drizzle Kit configuration for generating migrations. |
| .github/workflows/migrations.yml | Adds workflow to apply D1 migrations on main when migrations change. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| "binding": "DB", | ||
| "database_name": "switch-operator-db", | ||
| "database_id": "<run wrangler d1 create switch-operator-db and paste id here>", |
There was a problem hiding this comment.
database_id is currently a placeholder string. Wrangler deploys (and any CI/CD that relies on bindings) will fail until this is replaced with the actual D1 database id or moved into an environment-specific config that is populated in production.
| "database_id": "<run wrangler d1 create switch-operator-db and paste id here>", | |
| "database_id": "REPLACE_WITH_THE_ACTUAL_D1_DATABASE_ID_FOR_switch-operator-db", |
| - weekly: runs every week on the specified day at hour:minute | ||
| - monthly: runs every month on the specified day at hour:minute | ||
|
|
||
| Use fixedMessage for exact text or messagePrompt for AI-generated content.`; |
There was a problem hiding this comment.
The system prompt instructs the model to use fixedMessage/messagePrompt, but the tool schema and tool argument mapping use fixed_message/message_prompt. This mismatch will make tool calls less reliable; align the prompt terminology with the actual tool parameter names.
| Use fixedMessage for exact text or messagePrompt for AI-generated content.`; | |
| Use fixed_message for exact text or message_prompt for AI-generated content.`; |
| const args = JSON.parse(toolCall.function.arguments) as Record< | ||
| string, | ||
| unknown | ||
| >; |
There was a problem hiding this comment.
replyWithTools does a raw JSON.parse(toolCall.function.arguments) with no error handling. If the model returns malformed JSON (which can happen), this will throw and abort the whole request; consider catching parse errors and returning a structured tool error back into the conversation (or at least throwing a more specific error that includes the tool name).
| const args = JSON.parse(toolCall.function.arguments) as Record< | |
| string, | |
| unknown | |
| >; | |
| let args: Record<string, unknown>; | |
| try { | |
| args = JSON.parse(toolCall.function.arguments) as Record< | |
| string, | |
| unknown | |
| >; | |
| } catch (error) { | |
| const messageText = | |
| error instanceof Error ? error.message : String(error); | |
| this.logger.error("failed to parse tool call arguments", { | |
| tool: toolCall.function.name, | |
| iteration: i, | |
| arguments: toolCall.function.arguments, | |
| error: messageText, | |
| }); | |
| messages.push({ | |
| role: "tool", | |
| tool_call_id: toolCall.id, | |
| content: JSON.stringify({ | |
| error: "Invalid tool arguments", | |
| tool: toolCall.function.name, | |
| details: messageText, | |
| }), | |
| }); | |
| continue; | |
| } |
| timezone: (args.timezone as string | undefined) ?? "Europe/Helsinki", | ||
| fixedMessage: args.fixed_message as string | undefined, | ||
| messagePrompt: args.message_prompt as string | undefined, | ||
| sourceUrl: args.source_url as string | undefined, |
There was a problem hiding this comment.
mapToolArgsToInput reads args.source_url and the tool executor has logic for input.sourceUrl, but the create_schedule tool schema (in SCHEDULE_TOOLS) doesn't define a source_url parameter. As written, this code path is dead/unreachable; either add source_url to the tool schema (and validate it) or remove the unused mapping/check until monitors are actually supported.
| sourceUrl: args.source_url as string | undefined, |
![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: 53fe568a70
ℹ️ 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".
| Bindings: z.infer<typeof envSchema> & { | ||
| DB: D1Database; |
There was a problem hiding this comment.
DB is added only to the TypeScript AppEnv binding type here, but the runtime envSchema used by envValidatorMiddleware still does not require it. That means a missing/misconfigured D1 binding passes validation and then fails later when PendingActionService(c.env.DB) in the webhook handler or ScheduleService(env.DB) in cron is constructed, turning configuration mistakes into runtime 500s instead of an early, explicit misconfiguration error.
Useful? React with 👍 / 👎.
| await pendingService.clear(chatId); | ||
| const confirmed = message.text.trim().toUpperCase() === "YES"; | ||
|
|
||
| if (confirmed) { | ||
| const result = await executePendingAction( |
There was a problem hiding this comment.
The pending confirmation row is deleted before the confirmed action is executed. If executePendingAction throws (for example, transient D1 errors during create/delete), the confirmation is lost permanently and a Telegram retry of the same YES update will no longer perform the requested operation. This makes confirmed schedule mutations non-retriable under failure conditions; move the clear step after a successful execution (or restore on failure).
Useful? React with 👍 / 👎.
What
This update introduces scheduling capabilities to the operator application. It includes the creation of a new
schedulestable in the database to manage scheduled tasks, along with necessary migrations and configuration updates. The implementation also adds a utility for message handling and URL validation to support the new features.Added
schedulesandpending_actionstables to the database schema.Created migration scripts for the new database structure.
Updated environment configuration to include database bindings.
Implemented message splitting logic for Telegram messages.
Added URL validation to ensure compliance with security policies.
How to test
Run
pnpm db:migrate:localto apply the new migrations locally.Use
pnpm testto ensure all tests pass, particularly those related to scheduling and message handling.Verify that the application can handle scheduled tasks as expected.
Security review
Secrets / env vars: changed.
Auth / session: not changed.
Network / API calls: changed. (New validation for source URLs.)
Data handling / PII: changed. (New database tables for schedules and actions.)
Dependencies: added. (Introduced
drizzle-ormanddrizzle-kitfor database management.)Justification for changes:
New features require additional database management tools.
Enhanced security measures for URL handling.