feat: temporal tasks scheduler #37
Conversation
WalkthroughThis PR adds a task management system with Temporal workflow integration. It introduces a new tasks table with CRUD database operations via SQLC, configures Temporal client initialization with support for both self-hosted and Temporal Cloud deployments, and exposes REST API endpoints for creating, retrieving, and deleting tasks. The Temporal client is initialized at startup and used to schedule workflows via Cron expressions. Additionally, deep research messaging support is extended with new database queries, and configuration fields are added for Temporal connection details. All database schema changes are versioned via goose migrations. Sequence DiagramsequenceDiagram
participant Client as Client
participant HTTP as HTTP Handler
participant Auth as Auth Middleware
participant Storage as Task Storage
participant Temporal as Temporal ScheduleClient
participant Response as Response
rect rgb(200, 220, 240)
note over Client,Response: CreateTask Flow
Client->>HTTP: POST /api/v1/tasks (CreateTaskRequest)
HTTP->>Auth: Extract & verify user
Auth-->>HTTP: User ID
HTTP->>Storage: CreateTask(userID, chatID, name, content, cron)
Storage-->>HTTP: Task
HTTP->>Temporal: ScheduleClient.Create(ctx, workflowID, options)
Note over Temporal: Schedule workflow with cron
Temporal-->>HTTP: Schedule created
HTTP->>Response: TaskResponse (200 OK)
Response-->>Client: Task details
end
rect rgb(220, 240, 220)
note over Client,Response: GetTasks Flow
Client->>HTTP: GET /api/v1/tasks
HTTP->>Auth: Extract & verify user
Auth-->>HTTP: User ID
HTTP->>Storage: GetTasksByChatID(chatID)
Storage-->>HTTP: []Task
HTTP->>Response: TasksResponse (200 OK)
Response-->>Client: List of tasks
end
rect rgb(240, 220, 220)
note over Client,Response: DeleteTask Flow
Client->>HTTP: DELETE /api/v1/tasks/:id
HTTP->>Auth: Extract & verify user
Auth-->>HTTP: User ID
HTTP->>Storage: DeleteTaskByIDAndChatID(taskID, chatID)
Storage-->>HTTP: Confirm deletion
HTTP->>Response: Success message (200 OK)
Response-->>Client: Deletion confirmed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The changes span multiple heterogeneous components: auto-generated SQLC database code (low cognitive load), new handler logic with authentication and storage operations (moderate), Temporal client initialization and configuration (moderate), REST API wiring (simple), database schema and queries (straightforward), and dependency management (high due to multiple new transitive dependencies). While many changes are generated or repetitive patterns, the handler logic involves external service integration (Temporal) and error handling across multiple failure paths, requiring careful review of authentication flows, storage interactions, and workflow scheduling semantics. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
internal/storage/pg/sqlc/models.go (1)
60-69: Type inconsistency confirmed: Task.ID uses uuid.UUID here.The
Taskstruct generated in this package usesuuid.UUIDfor theIDfield (line 61), but the separatetaskspackage configuration insqlc.yamlmaps UUIDs tostring. This creates incompatible types across packages.This issue is addressed in the review comment for
sqlc.yamllines 72-103.
🧹 Nitpick comments (5)
internal/tasks/models.go (2)
5-13: Consider adding UserID field for API consistency.The
Taskstruct includesChatIDbut omitsUserID, even though the database schema (migration 007) includes both fields. If clients need to know which user owns a task, consider adding:type Task struct { ID string `json:"id"` + UserID string `json:"user_id"` ChatID string `json:"chat_id"` Name string `json:"name"`
21-28: Inconsistent field exposure between Task and TaskResponse.
TaskincludesChatID(line 7) butTaskResponseomits it. If both structs represent the same entity for different API contexts, they should expose the same fields for consistency. Consider addingChatID(and possiblyUserID) toTaskResponse.go.mod (1)
3-3: Validate the Go toolchain directive and pin it for reproducible builds.Ensure
go 1.24.2is available in your CI/build images. Consider adding a toolchain line to lock the compiler version.Apply this diff:
module github.com/eternisai/enchanted-proxy -go 1.24.2 +go 1.24.2 +toolchain go1.24.2Please confirm your CI uses Go 1.24.2 or update to the nearest supported patch. If not, adjust both lines accordingly.
internal/temporal/temporal.go (1)
38-60: Remove the custom gRPC unary interceptor; rely on SDK headers and credentials.Temporal Go SDK already carries namespace in API calls and uses the API key credentials. Injecting
temporal-namespacevia a gRPC interceptor is unnecessary and may interfere with SDK interceptors.Apply this diff:
if config.CloudAPIKey != "" { clientOptions.ConnectionOptions = client.ConnectionOptions{ TLS: &tls.Config{ MinVersion: tls.VersionTLS12, }, - DialOptions: []grpc.DialOption{ - grpc.WithUnaryInterceptor( - func(ctx context.Context, method string, req any, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { - return invoker( - metadata.AppendToOutgoingContext(ctx, "temporal-namespace", config.Namespace), - method, - req, - reply, - cc, - opts..., - ) - }, - ), - }, } clientOptions.Credentials = client.NewAPIKeyStaticCredentials(config.CloudAPIKey) }If you must attach extra metadata, prefer Temporal SDK client interceptors or
grpc.WithChainUnaryInterceptorto avoid clobbering internal interceptors. Also confirm whethertls.Config{ServerName: <host>}is required for your Cloud endpoint.internal/tasks/handlers.go (1)
62-62: Use configuration for TaskQueue instead of hardcoding.The task queue name is hardcoded. Based on the PR summary mentioning Temporal configuration fields, this should use a configured value.
Assuming the configuration includes a
TemporalTaskQueuefield (mentioned in the AI summary), pass it to the handler and use it here:- TaskQueue: "task-queue", + TaskQueue: h.temporalTaskQueue,And update the Handler struct and constructor:
type Handler struct { queries tasksdb.Querier temporalClient client.Client + temporalTaskQueue string } -func NewHandler(queries tasksdb.Querier, temporalClient client.Client) *Handler { +func NewHandler(queries tasksdb.Querier, temporalClient client.Client, temporalTaskQueue string) *Handler { return &Handler{ queries: queries, temporalClient: temporalClient, + temporalTaskQueue: temporalTaskQueue, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
.claude/settings.local.json(1 hunks)cmd/server/main.go(6 hunks)go.mod(6 hunks)internal/config/config.go(2 hunks)internal/storage/pg/database.go(2 hunks)internal/storage/pg/migrations/007_create_tasks.sql(1 hunks)internal/storage/pg/queries/tasks/db.go(1 hunks)internal/storage/pg/queries/tasks/models.go(1 hunks)internal/storage/pg/queries/tasks/querier.go(1 hunks)internal/storage/pg/queries/tasks/tasks.sql(1 hunks)internal/storage/pg/queries/tasks/tasks.sql.go(1 hunks)internal/storage/pg/sqlc/deep_research_messages.sql.go(1 hunks)internal/storage/pg/sqlc/models.go(2 hunks)internal/storage/pg/sqlc/querier.go(2 hunks)internal/tasks/handlers.go(1 hunks)internal/tasks/models.go(1 hunks)internal/temporal/temporal.go(1 hunks)sqlc.yaml(1 hunks)
🔇 Additional comments (18)
.claude/settings.local.json (1)
9-12: LGTM!The added Bash command allowances are appropriate for local development workflows involving database connectivity checks and SQLC code generation.
internal/storage/pg/database.go (1)
10-10: LGTM!The TasksQueries field addition follows the established pattern for the existing Queries field, maintaining consistency in the database layer architecture.
Also applies to: 17-17, 44-44
internal/storage/pg/migrations/007_create_tasks.sql (2)
13-15: LGTM!The indexes are well-designed for expected query patterns: filtering by user_id, chat_id, and time-ordered retrieval.
2-11: Remove incorrect assertion about updated_at handling; clarify design considerations.The verification reveals the main technical concern is inaccurate:
updated_at handling: The
UpdateTaskquery atinternal/storage/pg/queries/tasks/tasks.sql(line 34) explicitly setsupdated_at = NOW()on every update. Additionally,CreateTaskinitializes both timestamps viaNOW(). The field is automatically updated at the database level—no explicit application code is required.Foreign key constraints: Confirmed absent. This is a valid design consideration if referential integrity is needed.
Cron validation: No explicit validation found in available Go code. Whether this is validated at the application layer (handlers, middleware) or elsewhere cannot be determined from the queries alone.
internal/storage/pg/sqlc/models.go (1)
1-11: Generated code - no action needed.This is SQLC-generated code. Any necessary changes should be made to the source SQL schema or
sqlc.yamlconfiguration.internal/config/config.go (1)
87-91: LGTM!Temporal configuration follows the established pattern with sensible defaults for local development (localhost:7233, default namespace) and cloud deployment (optional API key).
Also applies to: 202-206
internal/storage/pg/queries/tasks/querier.go (1)
1-22: LGTM!This SQLC-generated interface provides comprehensive CRUD operations for tasks, including scoped delete operations by user and chat. The method signatures are consistent with the tasks package configuration.
internal/tasks/models.go (1)
15-19: No changes required—ChatID and UserID are properly derived from authenticated context.The handler correctly extracts
userIDfromauth.GetUserUUID(c)at line 29 with an authentication guard (lines 30–33), then populates bothUserIDandChatIDfields from this authenticated context (lines 43–44) before passing to the database. The verification concern is satisfied.cmd/server/main.go (3)
157-157: Wiring looks good.Passing
db.TasksQueriesandtemporalClientintotasks.NewHandleris the right seam.
212-214: REST server input extended correctly.New
tasksHandlerfield is consistently threaded through.
389-396: All authorization and tenancy checks are properly implemented.Verification confirms:
- DeleteTask: Uses
DeleteTaskByIDAndChatIDwithuserIDasChatIDparameter—deletes are scoped to the caller's resources, not rawid- GetTasks: Uses
GetTasksByChatIDwithuserID—returns only caller's tasks- CreateTask: Sets both
UserIDandChatIDto the authenticated user's ID—enforces ownership at creation- All handlers validate authentication via
auth.GetUserUUID()before any database operationinternal/storage/pg/queries/tasks/db.go (1)
1-31: LGTM.Standard sqlc scaffolding;
WithTxpattern is correct.go.mod (1)
26-28: Versions confirmed compatible—no transitive conflicts detected.The
go mod tidyverification shows thatgo.temporal.io/sdk v1.37.0(which depends on gRPC v1.67.1) andgo.temporal.io/api v1.53.0(which depends on gRPC v1.66.0) are both satisfied by the declaredgoogle.golang.org/grpc v1.72.0. The module resolution completed successfully without errors or constraint violations, confirming the versions are compatible under Go's semantic versioning rules.internal/storage/pg/queries/tasks/models.go (1)
1-90: LGTM! SQLC-generated models.This file is generated by SQLC and follows standard conventions. The model definitions are appropriate for the database schema.
internal/storage/pg/sqlc/querier.go (1)
12-39: LGTM! SQLC-generated interface extensions.The new methods for deep research message operations are properly defined with appropriate signatures.
internal/storage/pg/queries/tasks/tasks.sql.go (1)
1-218: LGTM! SQLC-generated task queries.The generated CRUD operations follow SQLC conventions and implement proper error handling and scanning patterns.
internal/storage/pg/sqlc/deep_research_messages.sql.go (1)
1-182: LGTM! SQLC-generated deep research message queries.The generated operations for managing deep research messages follow SQLC conventions with appropriate error handling.
internal/tasks/handlers.go (1)
44-44: No changes needed. The ChatID assignment is correct for the current single-user task design.The codebase consistently treats tasks as user-scoped. The
ChatIDfield stores the user's ID throughout task creation, retrieval, and deletion (handlers lines 44, 98, and 139 all use this pattern). While the codebase does support multi-user Telegram chats, the task system doesn't integrate with that—it maintains isolated per-user task management by design. The assignment is intentional and consistent.Likely an incorrect or invalid review comment.
| // Initialize Temporal client | ||
| log.Info("initializing temporal client", | ||
| slog.String("address", config.AppConfig.TemporalAddress), | ||
| slog.String("namespace", config.AppConfig.TemporalNamespace), | ||
| slog.String("task_queue", config.AppConfig.TemporalTaskQueue), | ||
| ) | ||
| temporalClient, err := temporal.NewTemporalClientFromConfig(config.AppConfig) | ||
| if err != nil { | ||
| log.Error("failed to initialize temporal client", slog.String("error", err.Error())) | ||
| os.Exit(1) | ||
| } | ||
| log.Info("temporal client initialized") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Close the Temporal client on shutdown to avoid leaking the gRPC connection.
Add a deferred close right after successful init.
Apply this diff:
temporalClient, err := temporal.NewTemporalClientFromConfig(config.AppConfig)
if err != nil {
log.Error("failed to initialize temporal client", slog.String("error", err.Error()))
os.Exit(1)
}
log.Info("temporal client initialized")
+ // Ensure Temporal client is closed on shutdown.
+ defer func() {
+ if err := temporalClient.Close(); err != nil {
+ log.Warn("temporal client close error", slog.String("error", err.Error()))
+ }
+ }()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Initialize Temporal client | |
| log.Info("initializing temporal client", | |
| slog.String("address", config.AppConfig.TemporalAddress), | |
| slog.String("namespace", config.AppConfig.TemporalNamespace), | |
| slog.String("task_queue", config.AppConfig.TemporalTaskQueue), | |
| ) | |
| temporalClient, err := temporal.NewTemporalClientFromConfig(config.AppConfig) | |
| if err != nil { | |
| log.Error("failed to initialize temporal client", slog.String("error", err.Error())) | |
| os.Exit(1) | |
| } | |
| log.Info("temporal client initialized") | |
| // Initialize Temporal client | |
| log.Info("initializing temporal client", | |
| slog.String("address", config.AppConfig.TemporalAddress), | |
| slog.String("namespace", config.AppConfig.TemporalNamespace), | |
| slog.String("task_queue", config.AppConfig.TemporalTaskQueue), | |
| ) | |
| temporalClient, err := temporal.NewTemporalClientFromConfig(config.AppConfig) | |
| if err != nil { | |
| log.Error("failed to initialize temporal client", slog.String("error", err.Error())) | |
| os.Exit(1) | |
| } | |
| log.Info("temporal client initialized") | |
| // Ensure Temporal client is closed on shutdown. | |
| defer func() { | |
| if err := temporalClient.Close(); err != nil { | |
| log.Warn("temporal client close error", slog.String("error", err.Error())) | |
| } | |
| }() |
🤖 Prompt for AI Agents
In cmd/server/main.go around lines 92 to 104, the Temporal client is created but
not closed on shutdown, leaking the gRPC connection; after temporalClient is
successfully initialized, add a deferred call to close it (e.g., defer func() {
if err := temporalClient.Close(); err != nil { log.Error("error closing temporal
client", slog.String("error", err.Error())) } }()) so the client is closed when
main exits and any close error is logged.
| -- name: GetTaskByID :one | ||
| SELECT * FROM tasks | ||
| WHERE id = $1; | ||
|
|
There was a problem hiding this comment.
Avoid footguns: prefer tenant‑scoped queries for read/update/delete.
GetTaskByID, UpdateTask and DeleteTask operate on bare id. Easy to misuse from handlers.
Add tenant‑scoped variants and use them from handlers:
-- name: GetTaskByID :one
SELECT * FROM tasks
WHERE id = $1;
+-- name: GetTaskByIDAndUserID :one
+SELECT * FROM tasks
+WHERE id = $1 AND user_id = $2;
+
-- name: DeleteTask :exec
DELETE FROM tasks
WHERE id = $1;
-- name: DeleteTaskByIDAndChatID :exec
DELETE FROM tasks
WHERE id = $1 AND chat_id = $2;
-- name: DeleteTaskByIDAndUserID :exec
DELETE FROM tasks
WHERE id = $1 AND user_id = $2;
-- name: UpdateTask :one
UPDATE tasks
SET name = $2, content = $3, cron = $4, updated_at = NOW()
WHERE id = $1
RETURNING *;
+
+-- name: UpdateTaskByIDAndUserID :one
+UPDATE tasks
+SET name = $3, content = $4, cron = $5, updated_at = NOW()
+WHERE id = $1 AND user_id = $2
+RETURNING *;Ensure indexes exist: (user_id, created_at), (chat_id, created_at), and (id, user_id) to back these queries. If missing, add in the migration.
Also applies to: 20-31, 32-36
| return | ||
| } | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
Use request context instead of context.Background().
The handler should propagate the request context to respect cancellations and timeouts.
Apply this diff:
- ctx := context.Background()
+ ctx := c.Request.Context()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx := context.Background() | |
| ctx := c.Request.Context() |
🤖 Prompt for AI Agents
In internal/tasks/handlers.go around line 41, the handler currently uses
context.Background() which prevents propagation of request cancellations and
timeouts; replace it with the incoming request's context (e.g., use r.Context()
or the provided http.Request.Context()) and ensure that any downstream calls use
that ctx so cancellations/timeouts propagate through the handler.
| taskId := uuid.New().String() | ||
| id := fmt.Sprintf("task-%s", taskId) | ||
| opts := client.ScheduleOptions{ | ||
| ID: id, | ||
| Action: &client.ScheduleWorkflowAction{ | ||
| ID: id, | ||
| Workflow: "ExecuteTaskWorkflow", | ||
| Args: []any{map[string]any{"name": req.Name}}, | ||
| TaskQueue: "task-queue", | ||
| }, | ||
| Overlap: enums.SCHEDULE_OVERLAP_POLICY_SKIP, | ||
| Spec: client.ScheduleSpec{ | ||
| CronExpressions: []string{req.Cron}, | ||
| }, | ||
| } | ||
|
|
||
| scheduleHandle, err := h.temporalClient.ScheduleClient().Create(ctx, opts) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"temporal error": err.Error()}) | ||
| return | ||
| } | ||
|
|
||
| fmt.Println("scheduleHandle", scheduleHandle) |
There was a problem hiding this comment.
Critical: Temporal schedule ID mismatch and missing database persistence.
Several issues with the Temporal schedule creation:
- A new UUID is generated (line 54) instead of using
task.IDfrom the database, creating a mismatch between the database record and the Temporal schedule. - The Temporal schedule ID is not persisted to the database, making it impossible to manage or delete the schedule later.
- The debug print statement (line 76) should be removed.
- No rollback occurs if schedule creation fails after the database task is created, leaving an orphaned database record.
Apply this diff to fix the schedule ID mismatch and remove the debug statement:
- taskId := uuid.New().String()
- id := fmt.Sprintf("task-%s", taskId)
+ id := fmt.Sprintf("task-%s", task.ID)
opts := client.ScheduleOptions{
ID: id,
Action: &client.ScheduleWorkflowAction{
ID: id,
Workflow: "ExecuteTaskWorkflow",
Args: []any{map[string]any{"name": req.Name}},
TaskQueue: "task-queue",
},
Overlap: enums.SCHEDULE_OVERLAP_POLICY_SKIP,
Spec: client.ScheduleSpec{
CronExpressions: []string{req.Cron},
},
}
scheduleHandle, err := h.temporalClient.ScheduleClient().Create(ctx, opts)
if err != nil {
+ // Rollback: delete the task from database
+ _ = h.queries.DeleteTask(ctx, task.ID)
c.JSON(http.StatusBadRequest, gin.H{"temporal error": err.Error()})
return
}
-
- fmt.Println("scheduleHandle", scheduleHandle)
+ _ = scheduleHandle // unused but returned by APIAdditionally, consider adding a temporal_schedule_id column to the tasks table to persist the schedule ID for future management operations.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
internal/tasks/handlers.go lines 54-76: currently a new UUID is generated for
the Temporal schedule, the schedule ID isn't persisted, there's no rollback on
schedule creation failure, and a debug print remains; change the code to use the
already-created task.ID as the schedule ID (e.g. format "task-{task.ID}"),
persist that schedule ID into a new temporal_schedule_id column on the tasks
table (or update the task record after schedule creation), remove the
fmt.Println debug, and implement rollback logic: create the DB task first, then
attempt to create the Temporal schedule, and if schedule creation fails delete
the previously-created DB task (or use a DB transaction/update flow) and return
an error; ensure all error responses include clear messages and that the DB
update persists the temporal_schedule_id on success.
| return | ||
| } | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
Use request context instead of context.Background().
The handler should propagate the request context to respect cancellations and timeouts.
Apply this diff:
- ctx := context.Background()
+ ctx := c.Request.Context()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx := context.Background() | |
| ctx := c.Request.Context() |
🤖 Prompt for AI Agents
In internal/tasks/handlers.go around line 97, the code creates a new context
with context.Background(); replace this with the HTTP request's context so
cancellations/timeouts are propagated — obtain the request context (e.g.,
r.Context() or req.Context() depending on the handler parameter name) and use
that variable instead of context.Background() for downstream operations.
| func (h *Handler) DeleteTask(c *gin.Context) { | ||
| userID, ok := auth.GetUserUUID(c) | ||
| if !ok { | ||
| c.JSON(http.StatusUnauthorized, gin.H{"error": "User not authenticated"}) | ||
| return | ||
| } | ||
|
|
||
| taskID := c.Param("id") | ||
| if taskID == "" { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Task ID is required"}) | ||
| return | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| err := h.queries.DeleteTaskByIDAndChatID(ctx, tasksdb.DeleteTaskByIDAndChatIDParams{ | ||
| ID: taskID, | ||
| ChatID: userID, | ||
| }) | ||
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{"message": "Task deleted successfully"}) | ||
| } |
There was a problem hiding this comment.
Critical: Temporal schedule not deleted, causing resource leak.
When a task is deleted from the database, the corresponding Temporal schedule should also be deleted to prevent orphaned schedules from continuing to execute.
Apply this diff to delete the Temporal schedule:
func (h *Handler) DeleteTask(c *gin.Context) {
userID, ok := auth.GetUserUUID(c)
if !ok {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User not authenticated"})
return
}
taskID := c.Param("id")
if taskID == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "Task ID is required"})
return
}
ctx := c.Request.Context()
+
+ // Delete Temporal schedule first
+ scheduleID := fmt.Sprintf("task-%s", taskID)
+ scheduleHandle := h.temporalClient.ScheduleClient().GetHandle(ctx, scheduleID)
+ if err := scheduleHandle.Delete(ctx); err != nil {
+ // Log error but continue with database deletion
+ // (schedule may not exist if creation failed previously)
+ }
+
err := h.queries.DeleteTaskByIDAndChatID(ctx, tasksdb.DeleteTaskByIDAndChatIDParams{
ID: taskID,
ChatID: userID,
})
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}
c.JSON(http.StatusOK, gin.H{"message": "Task deleted successfully"})
}Note: This assumes the schedule ID format matches the one used in CreateTask. If you implement the suggestion to store the schedule ID in the database, retrieve it from there instead.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/tasks/handlers.go around lines 123 to 147, the handler deletes the
task row but does not remove the corresponding Temporal schedule, leaving
orphaned schedules; update DeleteTask to also delete the Temporal schedule
(either derive the schedule ID from the task ID using the same format as
CreateTask or, if you changed the schema, read the schedule ID from the DB),
call the Temporal client’s schedule-delete API (handle and log errors — if
schedule deletion fails return an error or continue based on desired semantics),
ensure proper context/timeouts are used, and only return success after the
schedule delete and DB delete steps complete (or implement compensating cleanup
if ordering differs).
| return | ||
| } | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
Use request context instead of context.Background().
The handler should propagate the request context to respect cancellations and timeouts.
Apply this diff:
- ctx := context.Background()
+ ctx := c.Request.Context()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx := context.Background() | |
| ctx := c.Request.Context() |
🤖 Prompt for AI Agents
In internal/tasks/handlers.go around line 136, the handler currently uses ctx :=
context.Background(); replace this with the HTTP request context (e.g., ctx :=
r.Context() or use the provided context parameter) so cancellations and timeouts
propagate to downstream calls, and pass that ctx to any subsequent
function/database/service calls instead of context.Background().
| - engine: "postgresql" | ||
| schema: "./internal/storage/pg/migrations" | ||
| queries: "./internal/storage/pg/queries/tasks/" | ||
| gen: | ||
| go: | ||
| package: "tasks" | ||
| out: "./internal/storage/pg/queries/tasks" | ||
| emit_json_tags: true | ||
| emit_prepared_queries: false | ||
| emit_interface: true | ||
| emit_exact_table_names: false | ||
| emit_empty_slices: true | ||
| emit_exported_queries: false | ||
| emit_result_struct_pointers: false | ||
| emit_params_struct_pointers: false | ||
| emit_methods_with_db_argument: false | ||
| emit_pointers_for_null_types: true | ||
| emit_enum_valid_method: false | ||
| emit_all_enum_values: false | ||
| json_tags_case_style: "camel" | ||
| overrides: | ||
| - column: "*.created_at" | ||
| go_type: "time.Time" | ||
| - column: "*.updated_at" | ||
| go_type: "time.Time" | ||
| - db_type: "text" | ||
| nullable: true | ||
| go_type: | ||
| type: "string" | ||
| pointer: true | ||
| - db_type: "uuid" | ||
| go_type: "string" |
There was a problem hiding this comment.
Fix UUID type inconsistency between packages.
There's a type mismatch for the id field between the two sqlc configurations:
- Main engine (pgdb): No UUID override defined, so it generates
uuid.UUIDtype (seeinternal/storage/pg/sqlc/models.goline 61:ID uuid.UUID) - Tasks engine: Lines 102-103 override
db_type: "uuid"togo_type: "string"
This creates incompatible types when the same tasks table is accessed through different packages. Code converting between pgdb.Task and tasks.Task will fail to compile or require error-prone conversions.
Apply this diff to align both packages to use string for UUIDs:
- db_type: "text"
nullable: true
go_type:
type: "string"
pointer: true
+ - db_type: "uuid"
+ go_type: "string"Add the override to the first engine block (after line 71) to ensure consistency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In sqlc.yaml around lines 72 to 103, the first/postgres engine block is missing
a UUID override causing `id` to be generated as uuid.UUID in one package and
string in the tasks package; add an overrides entry in the first engine's gen.go
section (immediately after line 71) that maps db_type: "uuid" to go_type:
"string" (matching the tasks engine) so both engines generate string for UUIDs
and types remain consistent across packages.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Chores