-
Notifications
You must be signed in to change notification settings - Fork 1
feat(workers): add Temporal worker structure and initial implementation #23
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
Conversation
- Introduced a new Dockerfile for the Temporal worker, defining multi-stage builds for development and production environments. - Created a comprehensive project structure for Temporal workflows and activities, including necessary TypeScript configurations. - Implemented example workflows and activities, along with a main entry point for the worker. - Added package.json and package-lock.json to manage dependencies for the Temporal worker. These changes establish a foundational setup for developing and deploying Temporal workflows, enhancing the overall project architecture.
📝 Walkthrough## Walkthrough
This change introduces a new standardized structure and documentation rule for Temporal workers and workflows. It adds a new Temporal worker package under `workers/main`, including example workflow and activity implementations, TypeScript configuration, Dockerfile for multi-stage builds, and supporting package scripts. The base Temporal Docker image and docker-compose setup were updated, and a new service for the Temporal worker was added to the compose configuration. Documentation and best practices requirements are formalized via a new rule file.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `.cursor/rules/temporal-project-structure.mdc` | New rule file defining required project structure, documentation standards, and best practices for Temporal-based workflows and workers. |
| `Dockerfile.temporal-worker-main` | New multi-stage Dockerfile for building and running the Temporal worker application, supporting development and production environments with Node.js 20 and nodemon for development. |
| `workers/main/package.json` | New `package.json` defining the Temporal worker project, scripts for development, build, and start, with dependencies and devDependencies specified. |
| `workers/main/tsconfig.json` | New TypeScript configuration file specifying compiler options, output directory, and strict type-checking for the worker project. |
| `workers/main/src/activities/exampleActivity.ts` | New file exporting an example asynchronous activity function that processes a string input and returns a formatted string. |
| `workers/main/src/activities/index.ts` | New index file re-exporting all exports from `exampleActivity.ts`. |
| `workers/main/src/types.ts` | New file defining `ExampleInput` and `ExampleResult` TypeScript interfaces for structuring workflow/activity input and output. |
| `workers/main/src/workflows/exampleWorkflow.ts` | New file exporting an example asynchronous workflow function that returns a greeting string. |
| `workers/main/src/workflows/index.ts` | New index file re-exporting all exports from `exampleWorkflow.ts`. |
| `workers/main/src/index.ts` | New entry point setting up the Temporal worker, dynamically loading workflows and activities, connecting to Temporal server, ensuring a scheduled workflow exists, and starting the worker. |
| `workers/main/src/utils/schedule.ts` | New utility function to create a Temporal schedule if it does not exist, scheduling the example workflow to run hourly. |
| `workers/main/src/__tests__/exampleActivity.test.ts` | New test suite for `exampleActivity` covering normal, empty, and special character inputs using Vitest. |
| `workers/main/src/__tests__/exampleWorkflow.test.ts` | New test suite for `exampleWorkflow` verifying greeting outputs for normal, empty, and special character inputs using Vitest. |
| `workers/main/src/__tests__/schedule.test.ts` | New test suite for `createScheduleIfNotExists` mocking Temporal client behavior to test schedule existence and creation logic. |
| `workers/main/vitest.config.ts` | New Vitest configuration file defining test environment, file patterns, coverage settings, and thresholds. |
| `Dockerfile.temporal` | Updated base image from 1.20.5 to 1.27.2, added PostgreSQL-related build arguments and environment variables, removed explicit schema setup commands, installed netcat for healthcheck, changed healthcheck command and parameters, and simplified gRPC port exposure. |
| `docker-compose.yml` | Removed build args for `temporal` service, updated environment variables including PostgreSQL visibility DB settings, changed healthcheck command, added new `temporal-worker-main` service with build context, environment, volumes, dependencies, and development watch configuration, and removed explicit driver declaration for `app-network`. |
| `docker-compose.prod.yml` | Added new `temporal-worker-main` service configured for production build with environment variable `NODE_ENV=production`. |
| `docs/user-guide/temporal/overview.md` | Added documentation section describing the `workers-shared/` directory for shared utilities, types, and configuration used across Temporal workers. |
| `.dockerignore` | Added ignore rule for `workers/main/node_modules` directory to optimize Docker context. |
| `.github/workflows/code-quality.yml` | Enhanced SonarQube job by adding steps to install dependencies and run tests with coverage in `workers/main` before scanning. |
| `sonar-project.properties` | Added property specifying the path to the JavaScript LCOV coverage report for the `workers/main` directory. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Developer
participant Docker
participant WorkerMain
participant TemporalServer
Developer->>Docker: Build Docker image (Dockerfile.temporal-worker-main)
Docker->>WorkerMain: Start container (entrypoint: build/worker.js)
WorkerMain->>TemporalServer: Connect to Temporal server
WorkerMain->>TemporalServer: Check for 'example-workflow-hourly' schedule
alt Schedule does not exist
WorkerMain->>TemporalServer: Create schedule for 'exampleWorkflow' (hourly)
end
WorkerMain->>TemporalServer: Start worker on 'main-queue' Possibly related PRs
Suggested reviewers
|
- Deleted the README.md file as it contained outdated information regarding the Temporal worker structure. - Removed commented-out sections in index.ts, exampleActivity.ts, and exampleWorkflow.ts to streamline the codebase. These changes enhance code clarity and maintainability by eliminating unnecessary files and comments.
🔍 Vulnerabilities of
|
digest | sha256:1482a347dda785a79bf07fa2ce778ccd73b20c65b3b9ed9d276616dedd9e000b |
vulnerabilities | |
platform | linux/amd64 |
size | 243 MB |
packages | 1628 |
📦 Base Image node:20-alpine
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
- Upgraded the Temporal base image to version 1.27.2 for improved features and stability. - Refactored the Dockerfile to streamline build arguments and environment variables. - Enhanced health check command for the Temporal service to ensure better reliability. - Introduced a new `temporal-worker-main` service in `docker-compose.yml` for handling Temporal tasks, with appropriate environment settings and dependencies. These changes improve the overall architecture and reliability of the Temporal services within the Docker setup.
- Added a newline at the end of the index.ts file to comply with coding standards and improve file formatting. This change enhances code quality by ensuring proper file structure.
- Added newlines at the end of index.ts, types.ts, exampleActivity.ts, and exampleWorkflow.ts to comply with coding standards and improve file formatting. These changes enhance code quality by ensuring proper file structure across the project.
- Added a newline at the end of the tsconfig.json file to comply with coding standards and improve file formatting. This change enhances code quality by ensuring proper file structure across the project.
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.
Actionable comments posted: 14
🧹 Nitpick comments (16)
workers/main/src/workflows/exampleWorkflow.ts (2)
1-1
: Remove unused import.The
proxyActivities
import is not used in the current implementation. Either remove it or demonstrate its usage with an example activity call.-import { proxyActivities } from '@temporalio/workflow';
3-5
: Consider enhancing the example workflow with activity usage.This example workflow works but doesn't demonstrate core Temporal concepts like activity invocation. Consider enhancing it to show a more complete Temporal workflow pattern that uses activities.
import { proxyActivities } from '@temporalio/workflow'; +import type * as activities from '../activities'; +// Create a client for activities +const { exampleActivity } = proxyActivities<typeof activities>({ + startToCloseTimeout: '1 minute', +}); export async function exampleWorkflow(name: string): Promise<string> { - return `Hello, ${name}!`; + // Process the name using an activity + const processedName = await exampleActivity(name); + return `Hello, ${processedName}!`; }workers/main/src/activities/exampleActivity.ts (1)
1-3
: Consider enhancing the example activity with error handling.While this is a good starting point for an example activity, production activities typically require error handling and logging. Consider enhancing it to demonstrate these patterns.
+import { ApplicationFailure } from '@temporalio/activity'; + export async function exampleActivity(name: string): Promise<string> { + try { + // Example validation + if (!name || name.trim() === '') { + throw ApplicationFailure.create({ + message: 'Name cannot be empty', + type: 'ValidationError', + nonRetryable: true, + }); + } + + console.log(`Processing activity for name: ${name}`); return `Processed: ${name}`; + } catch (error) { + console.error(`Activity execution failed:`, error); + throw error; + } }workers/main/tsconfig.json (2)
3-3
: Consider revising the output directory structure.The current
outDir
is set to"../../dist"
, which places compiled files outside the worker's directory. This could lead to conflicts if multiple workers output to the same location.{ "compilerOptions": { - "outDir": "../../dist", + "outDir": "./dist", "rootDir": ".", "module": "commonjs", "target": "es2019", "esModuleInterop": true, "strict": true, "skipLibCheck": true }, "include": ["./**/*.ts"] }
7-9
: Enable source maps for better debugging.Consider enabling source maps to improve the debugging experience, especially important for async code in Temporal workflows.
"esModuleInterop": true, "strict": true, - "skipLibCheck": true + "skipLibCheck": true, + "sourceMap": trueworkers/main/src/types.ts (3)
1-1
: Update comment to use English for consistencyThe comment is currently in Russian ("Пример типов для воркфлоу и активити"). For better maintainability and consistency with the rest of the codebase, consider translating this to English.
-// Пример типов для воркфлоу и активити +// Example types for workflow and activity
7-9
: Remove trailing whitespaceThere's a trailing space at the end of line 9, which should be removed to maintain code cleanliness.
-} +}
3-9
: Consider adding JSDoc comments to the interfacesSince these are example interfaces that will likely be used as templates for future implementations, adding JSDoc comments would improve documentation and code clarity.
+/** + * Input parameters for the example workflow + */ export interface ExampleInput { name: string; } +/** + * Result type returned by the example workflow + */ export interface ExampleResult { message: string; -} +}workers/main/package.json (1)
6-8
: Consider more granular debugging optionsSetting
DEBUG=*
in the scripts enables all debug output, which can be overwhelming. Consider setting more specific debug namespaces or making it configurable via environment variables.- "start": "DEBUG=* npx ts-node src/index.ts", + "start": "DEBUG=@temporalio/* npx ts-node src/index.ts", "build": "tsc", - "dev": "DEBUG=* nodemon --watch ./src --ext ts --exec npx ts-node src/index.ts" + "dev": "DEBUG=@temporalio/* nodemon --watch ./src --ext ts --exec npx ts-node src/index.ts"workers/main/src/index.ts (3)
19-19
: Extract connection address to a named constantThe default Temporal server address is hardcoded. For better maintainability, consider extracting this to a named constant.
-const address = process.env.TEMPORAL_ADDRESS || 'temporal:7233'; +const DEFAULT_TEMPORAL_ADDRESS = 'temporal:7233'; +const address = process.env.TEMPORAL_ADDRESS || DEFAULT_TEMPORAL_ADDRESS;
56-59
: Improve error handling on worker startupThe current error handling just logs and exits. Consider adding more context and a graceful shutdown process.
run().catch((err) => { - console.error(err); + console.error('Fatal error in Temporal worker:', err); + console.error('Worker will now exit'); process.exit(1); });
59-59
: Remove trailing whitespaceThere's a trailing space at the end of line 59.
-}); +});.cursor/rules/temporal-project-structure.mdc (3)
38-41
: Define README.md content requirementsThe structure example shows a README.md but doesn't specify what content should be in it. Consider updating the documentation to specify what should be included in the README.md file.
- `index.ts` — worker entry point (registers workflows/activities, sets task queue) - `types.ts` — (optional) worker-specific types - `README.md` — brief usage and development instructions + +The README.md should include: +- Brief description of the worker's purpose +- Development setup instructions +- Local testing instructions +- Links to more detailed documentation
31-32
: Clarify Dockerfile requirementsThe structure example mentions
Dockerfile.temporal
but doesn't describe what it should contain or requirements for worker-specific Dockerfiles. Consider adding information about Dockerfile requirements.├── docker-compose.yml # Docker compose configuration └── Dockerfile.temporal # Base Temporal worker Dockerfile + +Worker-specific Dockerfiles should be named `Dockerfile.temporal-worker-<worker-name>` and +should extend the base Dockerfile.temporal. They must include: +- Multi-stage builds for development and production +- Proper COPY commands for worker-specific files +- Appropriate CMD to run the worker +- Environment variable definitions
66-66
: Define versioning and compatibility requirementsThe documentation lacks information about versioning and compatibility requirements for Temporal workers. Consider adding a section on versioning and compatibility.
- PRs introducing new Temporal workers or workflows **must** follow this structure and update documentation accordingly. + +## Versioning and Compatibility + +- All workers must specify compatible Temporal SDK versions in their package.json +- Breaking changes to workflows or activities must be version-controlled following the versioning strategy documented in the Temporal Best Practices +- Workflow updates must maintain backward compatibility with running workflows or provide a clear migration pathDockerfile.temporal-worker-main (1)
12-17
: Prune devDependencies for production.
Thebuild
stage inherits all dependencies (including dev) fromdeps
, and you then ship the fullnode_modules
into prod. Consider pruning devDependencies to slim down the final image:FROM node:20-bullseye AS build WORKDIR /app COPY --from=deps /app/node_modules ./node_modules COPY workers/main ./ RUN npm run build +RUN npm prune --production # <— remove devDependencies before handing off to prod
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
workers/main/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (9)
.cursor/rules/temporal-project-structure.mdc
(1 hunks)Dockerfile.temporal-worker-main
(1 hunks)workers/main/package.json
(1 hunks)workers/main/src/activities/exampleActivity.ts
(1 hunks)workers/main/src/index.ts
(1 hunks)workers/main/src/types.ts
(1 hunks)workers/main/src/workflows/exampleWorkflow.ts
(1 hunks)workers/main/src/workflows/index.ts
(1 hunks)workers/main/tsconfig.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
🔇 Additional comments (4)
workers/main/src/workflows/index.ts (1)
1-1
: Appropriate barrel pattern implementation.This export follows best practices for module organization, allowing centralized imports of all workflows from a single entry point.
workers/main/src/types.ts (1)
3-5
: LGTM! Clean interface definitionThe
ExampleInput
interface is well-defined with a clear type for the requiredname
property.workers/main/src/index.ts (1)
1-5
: LGTM! Well organized importsThe imports are well structured, with related imports grouped together.
Dockerfile.temporal-worker-main (1)
1-5
: Good multi-stage dependency installation setup.
Thedeps
stage cleanly installs dependencies withnpm ci
, leveragingpackage*.json
to include lockfiles for reproducible builds.
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
Dockerfile.temporal (2)
4-4
: Remove unused build argument.
TheHOST
arg is declared but never used—clean it up to avoid confusion.🧰 Tools
🪛 GitHub Actions: Code Quality
[error] Hadolint process failed with exit code 1.
22-23
: Parameterize healthcheck for flexibility.
Rather than hardcodingtemporal:7233
, use the environment variables you define:HEALTHCHECK --interval=10s --timeout=5s --start-period=30s --retries=5 \ CMD sh -c "tctl --address $HOST:$TEMPORAL_PORT cluster health && nc -z $HOST $TEMPORAL_PORT"🧰 Tools
🪛 GitHub Actions: Code Quality
[error] Hadolint process failed with exit code 1.
docker-compose.yml (3)
104-104
: Synchronize healthcheck with env vars.
For consistency, replace hardcoded host/port with the service’s env vars:test: ["CMD", "sh", "-c", "tctl --address $${TEMPORAL_ADDRESS} cluster health && nc -z $${TEMPORAL_ADDRESS} $${TEMPORAL_PORT}"]
158-161
: AlignNODE_ENV
with build target.
You’re using thedev
build stage but setNODE_ENV=production
. ConsiderNODE_ENV=development
to enable dev-mode features (hot reload, verbose logging).
163-165
: Optimizenode_modules
volume.
Mounting/app/node_modules
anonymously can lead to stale or inconsistent deps. Define a named volume for clarity and cache efficiency:volumes: - ./workers/main:/app - temporal-worker-node-modules:/app/node_modules volumes: temporal-worker-node-modules:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Dockerfile.temporal
(1 hunks)docker-compose.yml
(3 hunks)workers/main/src/activities/exampleActivity.ts
(1 hunks)workers/main/src/index.ts
(1 hunks)workers/main/src/types.ts
(1 hunks)workers/main/src/workflows/exampleWorkflow.ts
(1 hunks)workers/main/src/workflows/index.ts
(1 hunks)workers/main/tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- workers/main/src/workflows/index.ts
- workers/main/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- workers/main/tsconfig.json
- workers/main/src/index.ts
- workers/main/src/activities/exampleActivity.ts
- workers/main/src/workflows/exampleWorkflow.ts
🧰 Additional context used
🪛 GitHub Actions: Code Quality
Dockerfile.temporal
[warning] 19-19: Hadolint warning DL3018: Pin versions in apk add. Instead of apk add <package>
use apk add <package>=<version>
.
[error] Hadolint process failed with exit code 1.
🔇 Additional comments (7)
Dockerfile.temporal (2)
1-1
: Approve updated base image pin.
Bumping totemporalio/auto-setup:1.27.2
ensures you’re running the latest Temporal server schema and tooling.🧰 Tools
🪛 GitHub Actions: Code Quality
[error] Hadolint process failed with exit code 1.
5-8
: Approve new PostgreSQL build args.
AddingPOSTGRES_SEEDS
,POSTGRES_USER
,POSTGRES_DB_TEMPORAL_VISIBILITY
, andDB_PORT
(and exporting them) aligns this image with your visibility‐DB setup.🧰 Tools
🪛 GitHub Actions: Code Quality
[error] Hadolint process failed with exit code 1.
docker-compose.yml (5)
82-82
: Approve exposing gRPC on all interfaces.
TEMPORAL_FRONTEND_GRPC_BIND_ON_IP=0.0.0.0
is required for external workers to connect—this is correct.
89-89
: Verify theDB
environment variable value.
TheDB
var usually denotes database type (e.g.,postgres
), not engine version. Confirm that theauto-setup
image supportspostgres12
. If not, change to:DB: postgres
149-150
: Approve new Temporal worker service.
Thetemporal-worker-main
service is well-integrated with its build context, Dockerfile, and network dependency ontemporal
.
151-157
: Checkdepends_on
health-check condition support.
condition: service_healthy
works only in Compose v2.1. Confirm your Compose version or switch to an init/wait-for-health script in the worker container to avoid startup races.
167-171
: Validate customdevelop
extension fields.
develop.watch
isn’t part of the official Compose spec. Ensure your CLI/plugin honors it, or migrate to a supported mechanism (e.g., file‐watcher sidecar).
- Changed the worker name from "temporal-worker" to "temporal-worker-main" for clarity. - Updated the main entry point and script paths to reflect the new directory structure. - Added the "@temporalio/client" dependency to the project. - Ensured "nodemon" is listed in devDependencies for development convenience. These changes improve the organization and clarity of the Temporal worker setup.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
workers/main/package.json (1)
5-9
: Restrict nodemon watch path to src directory
Watching the entire project root can trigger unnecessary restarts (e.g., on lock file or config changes). Update thedev
script to watch onlysrc
:-"dev": "nodemon --watch ./ --ext ts --exec npx ts-node src/index.ts" +"dev": "nodemon --watch src --ext ts --exec npx ts-node src/index.ts"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workers/main/package.json
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Code Quality
workers/main/package.json
[error] 1-1: npm ci failed: package.json and package-lock.json are out of sync. Lock file's @temporalio/[email protected] does not satisfy @temporalio/[email protected]. Missing dependencies @temporalio/[email protected], @temporalio/[email protected], and @temporalio/[email protected] from lock file. Run 'npm install' to update lock file.
🔇 Additional comments (3)
workers/main/package.json (3)
1-4
: Skip review: Package metadata
Metadata fields (name
,version
,main
) are correctly set and no issues detected.🧰 Tools
🪛 GitHub Actions: Code Quality
[error] 1-1: npm ci failed: package.json and package-lock.json are out of sync. Lock file's @temporalio/[email protected] does not satisfy @temporalio/[email protected]. Missing dependencies @temporalio/[email protected], @temporalio/[email protected], and @temporalio/[email protected] from lock file. Run 'npm install' to update lock file.
10-18
: Skip review: nodemonConfig
ThenodemonConfig
correctly maps.ts
extensions and watches thesrc
directory as expected.
25-30
: Skip review: devDependencies
The devDependencies (@types/node
,nodemon
,ts-node
,typescript
) are appropriate for a TypeScript worker project.
Renamed the package from `temporal-worker` to `temporal-worker-main` and updated @temporalio/client and its related dependencies to version 1.11.8. This ensures compatibility with the latest features and fixes for Temporal libraries.
- Modified the project structure to include a new `workers-shared/` directory for shared utilities, types, and configuration used by multiple Temporal workers. - Updated the documentation to reflect the new organization, emphasizing the importance of reusing code across workers to avoid duplication and promote consistency. - Clarified the placement of Temporal workers and shared resources within the project. These changes enhance the clarity and maintainability of the project structure and documentation.
- Introduced a new `temporal-worker-main` service in `docker-compose.prod.yml` for production deployment, with environment set to production. - Updated the `NODE_ENV` variable in `docker-compose.yml` to development for the main worker. - Refactored the `Dockerfile.temporal-worker-main` to ensure proper build and runtime configurations, including changes to the output directory and command execution. - Updated dependencies in `package.json` and `package-lock.json` to use version 1.11.6 for `@temporalio/client` and related packages, ensuring compatibility with the latest features. These changes enhance the deployment structure and ensure the Temporal worker operates correctly in both development and production environments.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
workers/main/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (6)
Dockerfile.temporal-worker-main
(1 hunks)docker-compose.prod.yml
(1 hunks)docker-compose.yml
(3 hunks)workers/main/package.json
(1 hunks)workers/main/src/index.ts
(1 hunks)workers/main/tsconfig.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- workers/main/src/index.ts
- workers/main/tsconfig.json
- Dockerfile.temporal-worker-main
- workers/main/package.json
- docker-compose.yml
- Updated the activity module import logic in index.ts to exclude test files (ending with .test.ts and .test.js) from being loaded. This change ensures that only relevant activity modules are included, improving the clarity and performance of the worker. These modifications enhance the organization of the codebase by preventing unnecessary test files from being processed.
- Updated the Dockerfile.temporal to pin the netcat-openbsd package to version 1.217-r0. This change ensures consistency in the build environment and prevents potential issues with future updates. Additionally, modified the error handling in index.ts to check for 'NotFoundError' when describing the workflow, improving the robustness of the scheduling logic. These changes enhance the stability and reliability of the Temporal worker setup.
- Updated the activity module import logic in index.ts to filter out non-function exports and log warnings for ignored exports. This change ensures that only valid activity functions are included, improving the robustness of the worker's functionality. - Added checks to warn when no valid activity functions are found in a module, enhancing error visibility during development. These modifications improve the clarity and reliability of the activity module loading process.
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
workers/main/src/index.ts (5)
33-39
: Fix floating JSDoc commentThis JSDoc comment appears to be unattached to any code element. It might be intended as a file-level documentation, but it's currently not serving its purpose correctly.
Either move this comment to the top of the file as a file-level documentation, or remove it since the
run()
function already has detailed documentation.-/** - * Entry point for the Temporal worker service. - * - * Loads workflow and activity modules, connects to the Temporal server, ensures a schedule exists, - * and starts the worker to process workflows and activities from the task queue. - */
46-67
: Add logging to the schedule creation functionThe function lacks logging of successful operations, which would improve observability and debugging.
async function createScheduleIfNotExists(connection: Connection) { const scheduleClient = new ScheduleClient({ connection }); try { await scheduleClient.getHandle('example-workflow-hourly').describe(); + console.log('Schedule example-workflow-hourly already exists'); } catch (err: any) { if (err?.name === 'NotFoundError' || err?.message?.includes('workflow not found')) { + console.log('Creating schedule example-workflow-hourly...'); await scheduleClient.create({ scheduleId: 'example-workflow-hourly', spec: { cronExpressions: ['0 * * * *'] }, action: { type: 'startWorkflow', workflowType: 'exampleWorkflow', taskQueue: 'main-queue', workflowId: 'example-workflow-hourly', args: [], }, }); + console.log('Schedule example-workflow-hourly created successfully'); } else { + console.error('Failed to check for existing schedule:', err); throw err; } } }
9-29
: Consider adding more dynamic workflow loadingWhile the activity modules are loaded dynamically with validation, there's no corresponding logic for dynamically loading workflow modules. They're only referenced by path in the Worker.create() call.
For consistency and better error reporting, consider implementing similar validation logic for workflows:
const workflowModules = readdirSync(workflowsPath) .filter((f: string) => (f.endsWith('.ts') || f.endsWith('.js')) && !f.endsWith('.test.ts') && !f.endsWith('.test.js')) .map((f: string) => { const mod = require(path.join(workflowsPath, f)); // Log loaded workflows for better observability console.log(`Loaded workflow module: ${f}, exports: ${Object.keys(mod).join(', ')}`); return mod; }); // This would allow you to validate workflows if needed in the future // but still use the workflowsPath for now as temporal handles this🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
77-87
: Add more detailed logging in the run functionAdding more detailed logging would improve the observability of the worker startup process.
async function run() { + console.log(`Connecting to Temporal server at ${address}...`); const connection = await Connection.connect({ address }); + console.log('Connected to Temporal server successfully'); await createScheduleIfNotExists(connection); + console.log(`Starting worker with ${Object.keys(activities).length} activities...`); await Worker.create({ workflowsPath, activities, taskQueue: 'main-queue', }).then(worker => worker.run()); + console.log('Worker started successfully'); }
89-92
: Add more context to the error loggingThe current error handling is minimal. Adding more context would make it easier to diagnose issues.
run().catch((err) => { - console.error(err); + console.error('Temporal worker failed to start:', err); process.exit(1); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workers/main/src/index.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
workers/main/src/index.ts
[error] 13-13: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
…oral - Updated the Dockerfile.temporal to remove the version pin for the netcat-openbsd package. This change allows for the installation of the latest version, ensuring that the build environment can benefit from any updates or fixes provided by the package maintainers. This modification enhances the flexibility of the Docker setup while maintaining the necessary functionality.
- Removed the dynamic loading of activity modules in index.ts, simplifying the import process by directly importing activities from the new activities/index.ts file. This change enhances code clarity and maintainability by reducing complexity in the module loading logic. - Added a new activities/index.ts file to centralize activity exports, improving the organization of activity functions within the project. These modifications contribute to a cleaner and more efficient codebase for the Temporal worker.
- Updated the Dockerfile.temporal to pin the netcat-openbsd package to version 1.226.1.1-r0. This change ensures consistency in the build environment and prevents potential issues with future updates. These modifications enhance the stability and reliability of the Docker setup for the Temporal worker.
- Added a comment to disable SonarCloud warning for Dockerfile best practices. - Removed the unnecessary `ls -al` command from the build step to streamline the Dockerfile. These modifications enhance the clarity and efficiency of the Docker build process for the Temporal worker.
- Added '*.vhdx' and 'workers/main/node_modules' to .dockerignore to prevent unnecessary files from being included in the Docker context. - Removed NODE_ENV setting from docker-compose.prod.yml to streamline production configuration. - Removed NODE_ENV setting from docker-compose.yml to ensure consistency across environments. - Updated Dockerfile.temporal-worker-main to ignore scripts during npm install, improving build efficiency. These changes enhance the Docker setup by optimizing the build process and ensuring cleaner configurations for production and development environments.
- Introduced a new Vitest configuration file (`vitest.config.ts`) to set up testing environment for the project. - Added test cases for `exampleActivity` and `exampleWorkflow` functions to ensure expected behavior and correctness. - Updated `package.json` to include test and coverage scripts using Vitest and c8 for improved testing capabilities. These changes enhance the testing framework of the project, allowing for better validation of functionality and code quality.
- Added `sonar.javascript.lcov.reportPaths` to `sonar-project.properties` for improved coverage reporting in SonarQube. - Updated GitHub Actions workflow to install dependencies and run tests with coverage before executing the SonarQube scan. - Modified the coverage script in `package.json` to include LCOV reporting, enhancing the visibility of test coverage metrics. These changes improve the project's code quality analysis and ensure comprehensive test coverage reporting in SonarQube.
… testing and coverage - Added `@vitest/coverage-v8` and `vite` to `devDependencies` in `package.json` to enhance testing capabilities and coverage reporting. - Updated the coverage script in `package.json` to utilize Vitest for coverage reporting. - Enhanced `vitest.config.ts` to include detailed coverage configuration, specifying thresholds and exclusions for better test coverage management. - Refactored `index.ts` to remove unused schedule creation logic, streamlining the worker's functionality. - Introduced new tests for schedule management, ensuring robustness in the scheduling logic. These changes improve the testing framework and coverage reporting, contributing to better code quality and maintainability.
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
workers/main/package.json (2)
8-8
: 🛠️ Refactor suggestionUpdate nodemon watch path in dev script
The dev script is watching the root directory (./) while the nodemonConfig only watches the src directory. This inconsistency could cause issues.
- "dev": "DEBUG=* nodemon --watch ./ --ext ts --exec npx ts-node src/index.ts", + "dev": "DEBUG=* nodemon --watch ./src --ext ts --exec npx ts-node src/index.ts",
22-25
: 💡 Verification agent🧩 Analysis chain
Ensure consistent versioning across Temporal packages
The current package shows consistent versioning across Temporal packages (all at 1.11.6), which is good. However, based on previous review comments, there was a suggestion to update to 1.11.8. Please verify if this is still needed or if 1.11.6 is the correct version to use.
🌐 Web query:
What is the latest stable version of @temporalio SDK packages?
💡 Result:
Based on the search results, I can provide information about the latest stable versions of Temporal SDK packages across different programming languages.
For the TypeScript SDK, the latest stable version is 1.0.0, which was released as a stable version as mentioned in the announcements section[8].
For the .NET SDK, the latest version is 1.6.0 according to the NuGet Gallery information[9]. The .NET SDK is now generally available (GA) as noted in the announcements[8].
For the Python SDK, the latest version was released on May 9, 2025, though the specific version number isn't explicitly mentioned in the search results[3].
For the Go SDK, there is mention of updates in April 2025, but no specific stable version number is provided in the search results[5].
For the Java SDK, the search results mention v1.27.0 as a recent release with some breaking changes related to Update With Start functionality[2].
For the Ruby SDK, it's currently in pre-release status as announced at Replay 2025[4].
It's worth noting that the NPM package "temporalio" has been deprecated, and instead, users are recommended to directly install the individual packages: @temporalio/client, @temporalio/worker, @temporalio/workflow, and @temporalio/activity[7].
Citations:
- 1: https://github.com/temporalio/sdk-python/releases
- 2: https://github.com/temporalio/sdk-java/releases
- 3: https://pypi.org/project/temporalio/
- 4: https://temporal.io/blog/replay-2025-product-announcements
- 5: https://pkg.go.dev/go.temporal.io/sdk/temporal
- 6: https://github.com/temporalio/temporal/releases
- 7: https://www.npmjs.com/package/temporalio
- 8: https://temporal.io/change-log/type/Announcement
- 9: https://www.nuget.org/packages/Temporalio/
- 10: https://docs.temporal.io/develop/go/nexus
🏁 Script executed:
#!/bin/bash set -e echo "===== Temporal packages in repository =====" rg '@temporalio/' -n echo echo "===== Latest versions from npm =====" npm view @temporalio/client version npm view @temporalio/worker version npm view @temporalio/workflow version npm view @temporalio/activity version || trueLength of output: 3694
Bump @temporalio packages to latest stable (1.11.8)
The
workers/main/package.json
is currently pinning the Temporal SDK at 1.11.6, but npm shows 1.11.8 is the latest:• File:
workers/main/package.json
Lines: 23–25- "@temporalio/client": "1.11.6", - "@temporalio/worker": "1.11.6", - "@temporalio/workflow": "1.11.6" + "@temporalio/client": "1.11.8", + "@temporalio/worker": "1.11.8", + "@temporalio/workflow": "1.11.8"After updating, run
npm install
to update your lockfile accordingly.
🧹 Nitpick comments (8)
workers/main/src/__tests__/exampleWorkflow.test.ts (1)
10-13
: Consider improving empty string handling in the workflow implementationWhile this test correctly verifies the current behavior, the output "Hello, !" for an empty string input might not provide the best user experience. Consider enhancing the workflow implementation to handle empty names more gracefully.
The issue is in the implementation of
exampleWorkflow
rather than the test. Here's a possible enhancement:// In workers/main/src/workflows/exampleWorkflow.ts export async function exampleWorkflow(name: string): Promise<string> { - return `Hello, ${name}!`; + const formattedName = name.trim() || "World"; + return `Hello, ${formattedName}!`; }workers/main/src/__tests__/schedule.test.ts (2)
29-35
: Consider verifying the parameters passed to createWhile the test correctly verifies that the create method is called, it doesn't check if it's called with the correct parameters. Consider adding assertions to verify the schedule configuration.
it('creates schedule if not found', async () => { describeMock.mockRejectedValue({ name: 'NotFoundError' }); createMock.mockResolvedValue({}); const { createScheduleIfNotExists } = await import('../utils/schedule'); await expect(createScheduleIfNotExists({} as any)).resolves.toBeUndefined(); expect(createMock).toHaveBeenCalled(); + expect(createMock).toHaveBeenCalledWith({ + scheduleId: 'example-workflow-hourly', + spec: { cronExpressions: ['0 * * * *'] }, + action: expect.objectContaining({ + type: 'startWorkflow', + workflowType: 'exampleWorkflow', + taskQueue: 'main-queue', + }), + }); });
37-42
: Add test case for message-based error detectionThe implementation in
schedule.ts
handles errors both by name (NotFoundError
) and by message content (includes('workflow not found')
), but only the name-based detection is tested. Add a test case for the message-based detection path.it('creates schedule if error message indicates not found', async () => { describeMock.mockRejectedValue({ message: 'workflow not found' }); createMock.mockResolvedValue({}); const { createScheduleIfNotExists } = await import('../utils/schedule'); await expect(createScheduleIfNotExists({} as any)).resolves.toBeUndefined(); expect(createMock).toHaveBeenCalled(); });workers/main/src/__tests__/exampleActivity.test.ts (1)
10-13
: Consider improving empty string handling in the activity implementationSimilar to the workflow test, this correctly verifies the current behavior, but "Processed: " for an empty string input might not be optimal. Consider enhancing the activity implementation to handle empty inputs more gracefully.
// In workers/main/src/activities/exampleActivity.ts export async function exampleActivity(name: string): Promise<string> { - return `Processed: ${name}`; + if (!name.trim()) { + return "Processed: [Empty Input]"; + } + return `Processed: ${name}`; }workers/main/src/utils/schedule.ts (3)
9-30
: Consider making the schedule ID and workflow configurations more flexibleThe function works well but has hardcoded values for the schedule ID, workflow type, and task queue. For better reusability, consider parameterizing these values.
- export async function createScheduleIfNotExists(connection: Connection) { + export async function createScheduleIfNotExists( + connection: Connection, + options: { + scheduleId?: string; + workflowType?: string; + taskQueue?: string; + cronExpression?: string; + } = {} + ) { + const { + scheduleId = 'example-workflow-hourly', + workflowType = 'exampleWorkflow', + taskQueue = 'main-queue', + cronExpression = '0 * * * *' + } = options; + const scheduleClient = new ScheduleClient({ connection }); try { - await scheduleClient.getHandle('example-workflow-hourly').describe(); + await scheduleClient.getHandle(scheduleId).describe(); } catch (err: any) { if (err?.name === 'NotFoundError' || err?.message?.includes('workflow not found')) { await scheduleClient.create({ - scheduleId: 'example-workflow-hourly', - spec: { cronExpressions: ['0 * * * *'] }, + scheduleId, + spec: { cronExpressions: [cronExpression] }, action: { type: 'startWorkflow', - workflowType: 'exampleWorkflow', - taskQueue: 'main-queue', - workflowId: 'example-workflow-hourly', + workflowType, + taskQueue, + workflowId: scheduleId, args: [], }, }); } else { throw err; } } }
13-14
: Use a more specific error type instead of anyUsing
any
for the error type reduces type safety. Consider using a more specific type for better error handling.- } catch (err: any) { - if (err?.name === 'NotFoundError' || err?.message?.includes('workflow not found')) { + } catch (err: unknown) { + const error = err as Error; + if (error?.name === 'NotFoundError' || error?.message?.includes('workflow not found')) {
17-18
: Document the cron expression meaningThe cron expression '0 * * * *' runs at minute 0 of every hour. Consider adding a comment to clarify this schedule pattern.
await scheduleClient.create({ scheduleId: 'example-workflow-hourly', + // Run at the start of every hour (minute 0) spec: { cronExpressions: ['0 * * * *'] }, action: {
workers/main/package.json (1)
5-11
: Add lint script for code qualityConsider adding a lint script to ensure code quality and consistency across the project.
"scripts": { "start": "DEBUG=* npx ts-node src/index.ts", "build": "tsc", "dev": "DEBUG=* nodemon --watch ./src --ext ts --exec npx ts-node src/index.ts", "test": "vitest run", - "coverage": "vitest run --coverage" + "coverage": "vitest run --coverage", + "lint": "eslint src --ext .ts" },This would require adding ESLint as a devDependency:
"devDependencies": { "@types/node": "^18.0.0", "@vitest/coverage-v8": "3.1.3", "c8": "10.1.3", + "eslint": "^8.0.0", + "@typescript-eslint/eslint-plugin": "^6.0.0", + "@typescript-eslint/parser": "^6.0.0", "nodemon": "^3.0.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
workers/main/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (9)
.github/workflows/code-quality.yml
(1 hunks)sonar-project.properties
(1 hunks)workers/main/package.json
(1 hunks)workers/main/src/__tests__/exampleActivity.test.ts
(1 hunks)workers/main/src/__tests__/exampleWorkflow.test.ts
(1 hunks)workers/main/src/__tests__/schedule.test.ts
(1 hunks)workers/main/src/index.ts
(1 hunks)workers/main/src/utils/schedule.ts
(1 hunks)workers/main/vitest.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- sonar-project.properties
- workers/main/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- workers/main/src/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
workers/main/src/__tests__/exampleWorkflow.test.ts (1)
workers/main/src/workflows/exampleWorkflow.ts (1)
exampleWorkflow
(3-5)
workers/main/src/__tests__/schedule.test.ts (1)
workers/main/src/utils/schedule.ts (1)
createScheduleIfNotExists
(9-30)
workers/main/src/__tests__/exampleActivity.test.ts (1)
workers/main/src/activities/exampleActivity.ts (1)
exampleActivity
(1-3)
🔇 Additional comments (4)
.github/workflows/code-quality.yml (1)
28-31
: Great addition of test coverage for the Temporal worker!These steps properly integrate the test coverage of the new Temporal worker into the SonarQube analysis. Using
npm ci
instead ofnpm install
is a best practice for CI environments, and running coverage before the SonarQube scan ensures that the coverage data is available for the analysis.workers/main/src/__tests__/exampleWorkflow.test.ts (1)
1-19
: Well-structured test cases with good coverage!The tests are well-organized and cover both the happy path and edge cases (empty string, special characters). Using async/await with Vitest is properly implemented.
workers/main/src/__tests__/schedule.test.ts (1)
1-15
: Well-structured mocking approach!The mocking of the
@temporalio/client
module is clean and effective, providing good control over the testing environment for the schedule utility.workers/main/src/__tests__/exampleActivity.test.ts (1)
1-19
: Well-structured test cases with good coverage for the activity!The tests are well-organized and mirror the structure of the workflow tests, providing consistency in the testing approach. Good coverage of normal cases and edge cases.
…coverage reporting - Added `sonar.exclusions` to `sonar-project.properties` to exclude specific directories and files from coverage analysis, enhancing the accuracy of the SonarQube reports. - This change helps in focusing on relevant code and improving overall code quality assessment. These modifications contribute to a more effective integration with SonarQube by refining the coverage metrics.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sonar-project.properties (1)
4-4
: Review SonarQube exclusion patterns for scope and readability
The glob patterns correctly exclude tests, build artifacts, and entry-point files. To avoid unintentionally excluding other parts of the repo and improve maintainability, consider scoping these toworkers/main
and splitting the line for readability.-sonar.exclusions=**/src/__tests__/**,**/src/dist/**,**/src/types.ts,**/src/activities/index.ts,**/src/workflows/index.ts,**/src/index.ts +sonar.exclusions=\ + workers/main/src/__tests__/**,\ + workers/main/src/dist/**,\ + workers/main/src/types.ts,\ + workers/main/src/activities/index.ts,\ + workers/main/src/workflows/index.ts,\ + workers/main/src/index.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sonar-project.properties
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (1)
sonar-project.properties (1)
3-3
:✅ Verification successful
Verify LCOV coverage report path for the Temporal worker
Ensure that the Vitest setup inworkers/main
is configured to emit the LCOV report atworkers/main/coverage/lcov.info
. If the coverage output directory or filename differs, SonarQube won’t pick up the coverage data.You can run this shell snippet from the repo root to confirm the report exists:
🏁 Script executed:
#!/bin/bash # Check for the LCOV report if [ -f "workers/main/coverage/lcov.info" ]; then echo "LCOV report found" exit 0 else echo "LCOV report missing at workers/main/coverage/lcov.info" exit 1 fiLength of output: 172
🏁 Script executed:
#!/bin/bash # Locate Vitest config(s) in workers/main and inspect coverage settings CONFIG_FILES=$(find workers/main -maxdepth 3 -type f \( -name "vitest.config.ts" -o -name "vitest.config.js" \)) if [ -z "$CONFIG_FILES" ]; then echo "No Vitest config found in workers/main" exit 0 fi echo "Found Vitest config(s):" echo "$CONFIG_FILES" echo echo "Coverage-related settings in Vitest config(s):" grep -R "coverage" -n $CONFIG_FILES || echo "No coverage settings found in these files"Length of output: 490
🏁 Script executed:
#!/bin/bash # Show the Vitest coverage configuration block with context grep -R -n -A10 "coverage:" workers/main/vitest.config.tsLength of output: 370
Sonar LCOV Path Is Correct—Generate Coverage Before Analysis
The
sonar.javascript.lcov.reportPaths=workers/main/coverage/lcov.info
entry matches Vitest’s defaultcoverage
output directory when usinglcov
reporter. The “missing” file simply means tests haven’t been run with coverage enabled, not that the path is wrong.• No change needed in
sonar-project.properties
.
• Ensure your CI or local workflow invokes Vitest with--coverage
(e.g.vitest --coverage
) so thatworkers/main/coverage/lcov.info
is generated before running SonarQube.
|
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.
This PR is too large to be reviewed. Can you please make a chain of smaller PRs
- Add tests
- Add eslint
- add rule
- Add worker
These changes establish a foundational setup for developing and deploying Temporal workflows, enhancing the overall project architecture.